Skip to content

Conversation

@1ma
Copy link

@1ma 1ma commented Nov 8, 2024

Fixes #65

I've found 2 distinct bugs in the implementation of the SigMsg function from BIP-341.

The first, when the sighash includes ANYONECANPAY the spec says the first chunk of data must be outpoint (36): the COutPoint of this input (32-byte hash + 4-byte little-endian). However Embit was serializing the whole input using the TransactionInput.write_to() method, which includes more fields than just the outpoint and results in 41 bytes of data instead of 36.

Second, when SIGHASH_SINGLE is used the spec says to include sha_single_output (32): the SHA256 of the corresponding output in CTxOut format. In this case Embit was including the serialization of the output itself, not its SHA256 digest.

With these two changes I'm able to spend non-SIGHASH_DEFAULT Taproot transactions made from PSBTs signed with Embit.

I'd like help adding the unit test that reproduces the issue, as for now I can't follow test_taproot.py. I was able to verify that the test suite is currently not covering these execution branches, though:

Screenshot_2024-11-08_11-22-37

Copy link

@ekrembal ekrembal left a comment

Choose a reason for hiding this comment

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

After hours of work, I was able to get to the same code: https://github.com/ekrembal/embit/tree/fix_sighash_taproot. Before creating a PR, I wanted to check the open PRs and saw this. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid Taproot signatures in transactions with non-standard sighash

2 participants