Skip to content

Conversation

giacomocaironi
Copy link

Add some tests to Taproot SigMsg, inspired by BIP 143 test vector

Add some tests to Taproot SigMsg, inspired by BIP 143 test vector
Copy link
Contributor

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

Hey @giacomocaironi, thanks a lot for your work. This looks like it would be quite helpful to a BIP 341 implementer.

I think test vectors should be part of the reference code's test harness. Is it already? This would be the best way to ensure correctness of the test vectors. Also, I'm curious how you generated the vectors.

@maflcko
Copy link
Member

maflcko commented Oct 11, 2021

Previous discussion thread at https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2021-September/019466.html

@giacomocaironi
Copy link
Author

These test vectors are new, I created them from scratch them by randomly generating the keys. The reason I did this is that the official test vector contains non-standard transactions (they have strange version values). I thought it would by a nice addition if we added standard transactions to the test vectors, like BIP 143 did.

But if the hassle is too big, especially near the activation date, I can ignore this and simply pick some transactions from the official test vector instead of mine.

Copy link

@MegoyTambayan MegoyTambayan left a comment

Choose a reason for hiding this comment

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

Nicely Done!

@sipa
Copy link
Member

sipa commented Oct 19, 2021

@giacomocaironi I think @jonasnick is asking that these test vectors are added to Bitcoin Core's implementation, so that we can be sure they match the consensus implementation there.

I'll try implementing them.

@giacomocaironi
Copy link
Author

Closing. This PR is superseded by #1225

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.

6 participants