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.

Sign up for GitHub

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

Jump to bottom

feat: ensure signers go through the bitcoin validation code path #930

Merged
merged 13 commits into from
Nov 25, 2024
Merged

feat: ensure signers go through the bitcoin validation code path #930

merged 13 commits into from
Nov 25, 2024

Conversation

djordon
Copy link
Collaborator

@djordon djordon commented Nov 24, 2024 •
edited
Loading

Description

Closes #793, closes #696, closes #286, and closes #296.

Changes

  • Do the pre-validation check.
  • Make sure the tx-signer checks the sighash in WSTS messages.
  • Do not construct the Signers' BTC state in the tx-signer.
  • Slight refactor to better reflect the meaning of things of what is being validated, which is the pre-sign request and not the bitcoin context.

Testing Information

I need to add some.

Checklist:

  • I have performed a self-review of my code

@djordon djordon added the sbtc signer binary The sBTC Bootstrap Signer. label Nov 24, 2024
@djordon djordon added this to the sBTC: Deposit ready milestone Nov 24, 2024
Jiloc
Jiloc reviewed Nov 24, 2024
View reviewed changes
Copy link
Collaborator

@Jiloc Jiloc left a comment

Choose a reason for hiding this comment

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

Awesome clean up, makes easier to follow the validation workflow between the data structures. I just added a couple of questions

signer/src/bitcoin/validation.rs Show resolved Hide resolved
signer/src/transaction_signer.rs Show resolved Hide resolved
signer/src/transaction_signer.rs Show resolved Hide resolved
signer/tests/integration/bitcoin_validation.rs Outdated Show resolved Hide resolved
signer/tests/integration/bitcoin_validation.rs Outdated Show resolved Hide resolved
signer/tests/integration/bitcoin_validation.rs Outdated Show resolved Hide resolved
signer/src/transaction_signer.rs Outdated Show resolved Hide resolved
@djordon djordon self-assigned this Nov 24, 2024
Jiloc
Jiloc approved these changes Nov 24, 2024
View reviewed changes
Copy link
Collaborator

@Jiloc Jiloc left a comment

Choose a reason for hiding this comment

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

All good!

@djordon djordon force-pushed the 696-signers-go-through-the-validation-code-path branch from 55fea45 to 5b985aa Compare November 25, 2024 21:10
@djordon djordon changed the base branch from feat/validate-bitcoin-transactions to main November 25, 2024 21:21
@djordon djordon merged commit 0098262 into main Nov 25, 2024
4 checks passed
@djordon djordon deleted the 696-signers-go-through-the-validation-code-path branch November 25, 2024 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jiloc Jiloc Jiloc approved these changes

@matteojug matteojug Awaiting requested review from matteojug

@cylewitruk cylewitruk Awaiting requested review from cylewitruk

Assignees

@djordon djordon

Labels
sbtc signer binary The sBTC Bootstrap Signer.
Projects
sBTC
Status: Done
Milestone
sBTC: Deposit ready
2 participants
@djordon @Jiloc