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

fix(bdk): Check if we're using the correct internal key before signing #1200

Merged
merged 1 commit into from Nov 13, 2023

Conversation

danielabrozzoni
Copy link
Member

Description

Fixes #1142

We would previously sign with whatever x_only_pubkey we had in hand, without first checking if it was the right key or not. This effectively meant that adding multiple taproot PrivateKey signers would produce unbroadcastable transactions.

Changelog notice

  • Fix a bug related to taproot signing with internal keys. We would previously sign with the first private key we had, without checking if it was the correct internal key or not.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

ACK c450499

Looks good! I confirmed test fails without the extra && x_only_pubkey == psbt_internal_key check. I just have one nit/suggestion on the test.

...internal key before signing

Fixes bitcoindevkit#1142

We would previously sign with whatever x_only_pubkey we had in hand,
without first checking if it was the right key or not. This effectively
meant that adding multiple taproot PrivateKey signers would produce
unbroadcastable transactions.
Copy link
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

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

ACK e553231

@danielabrozzoni danielabrozzoni merged commit 3fdab87 into bitcoindevkit:master Nov 13, 2023
12 checks passed
@danielabrozzoni danielabrozzoni deleted the issue/1142 branch November 13, 2023 09:09
@notmandatory notmandatory mentioned this pull request Jan 3, 2024
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Taproot PSBT finalization fails often when signing with internal key
3 participants