Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: sign and broadcast Stacks transactions #617

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

djordon
Copy link
Contributor

@djordon djordon commented Oct 4, 2024

Description

Closes #616.

Note that we purposefully skip validation for now, but it should be relatively straightforward to hook up.

Changes

  • Make sure the client types are &'static, which they all are. This was needed for borrow checker.
  • Send the transaction ID with the StacksTransactionSignRequest. We can probably remove the digest from that type, we will do that later.
  • Make sure we have a loop for signing stacks transactions.
  • Submit the signed transaction to the stacks node

This PR needs some polish and some tests.

Testing Information

The main test that is needed is that the transaction coordinator broadcasts a signed complete-deposit transaction for a deposit request with a sweep transaction on bitcoin.

Checklist:

  • I have performed a self-review of my code

@djordon djordon added the sbtc bootstrap signer The sBTC Bootstrap Signer. label Oct 4, 2024
@djordon djordon added this to the Minimum Working Deposit Flow milestone Oct 4, 2024
@cylewitruk
Copy link
Member

Note that we purposefully skip validation for now, but it should be relatively straightforward to hook up.

Is there a separate ticket for this?

@djordon
Copy link
Contributor Author

djordon commented Oct 4, 2024

Note that we purposefully skip validation for now, but it should be relatively straightforward to hook up.

Is there a separate ticket for this?

No not yet. There are a few TODOs in this PR that I need to turn into outstanding tickets, this was just one of them.

@djordon djordon changed the title feat: sign and broadcast complete-deposit transactions feat: sign and broadcast Stacks transactions Oct 4, 2024
@djordon djordon force-pushed the 563-create-complete-deposit-transactions branch from 40100f8 to 70da07f Compare October 7, 2024 19:09
@djordon djordon force-pushed the 320-tx-coordinator-sign-stacks-response-transactions branch from 1facab7 to e75104f Compare October 7, 2024 19:22
@AshtonStephens
Copy link
Collaborator

Created ticket to track the lack of integration tests here: #630

Copy link
Collaborator

@matteojug matteojug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nits, so far LGTM.

signer/src/message.rs Show resolved Hide resolved
signer/src/transaction_coordinator.rs Outdated Show resolved Hide resolved
signer/src/transaction_coordinator.rs Outdated Show resolved Hide resolved
Base automatically changed from 563-create-complete-deposit-transactions to main October 9, 2024 13:52
@djordon djordon force-pushed the 320-tx-coordinator-sign-stacks-response-transactions branch from 386bc86 to c07d85c Compare October 9, 2024 13:58
@djordon djordon marked this pull request as ready for review October 9, 2024 14:18
signer/src/transaction_signer.rs Show resolved Hide resolved
Copy link
Member

@cylewitruk cylewitruk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general

Bitcoin: BitcoinInteract + Clone + Send + Sync,
Stacks: StacksInteract + Clone + Send + Sync,
Emily: EmilyInteract + Clone + Send + Sync,
Storage: DbRead + DbWrite + Clone + Sync + Send + 'static,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was it that made these all need to be 'static?

@aldur
Copy link
Collaborator

aldur commented Oct 9, 2024

@djordon can you lay out the test plan so that @matteojug can take it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sbtc bootstrap signer The sBTC Bootstrap Signer.
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

[Feature]: Sign and submit complete-deposit transactions
5 participants