Fix incorrect signature test vectors in BIP322

[wip-abramson]

While attempting to implement bip322 I was unable to generate the same signature as the ones provided. Furthermore, I was unable to verify these signatures in my implementation. After some digging, I noticed the test vectors in the bip322 PR to Bitcoin Core uses different values.

See util_tests.cpp in bitcoin/bitcoin#24058

This P.R. replaces the current test vectors with the ones from the P.R.

It also adds additional test vectors for the transaction hashes of the to_spend and to_sign transactions. Having these would have speeded up my debugging process significantly.

[rxgrant]

Can you point at the code needed to generate these vectors?

Can you test setting version to 2 to see if the old vectors were leftover cruft from before this fix?

[wip-abramson]

Setting the version to 2 still did not produce the test vectors currently provided in this bip.

I am not sure how these test vectors were generated, both the ones in the bip and the ones used in the P.R. - perhaps a question for @kallewoof

[kallewoof]

Verified.

$ ./bitcoin-cli signmessage bc1q9vza2e8x573nczrlzms0wvx3gsqjx7vavgkx0l ""
AkcwRAIgM2gBAQqvZX15ZiysmKmQpDrG83avLIT492QBzLnQIxYCIBaTpOaD20qRlEylyxFSeEA2ba9YOixpX8z46TSDtS40ASECx/EgAxlkQpQ9hYjgGu6EBCPMVPwVIVJqO4XCsMvViHI=
$ ./bitcoin-cli signmessage bc1q9vza2e8x573nczrlzms0wvx3gsqjx7vavgkx0l "Hello World"
AkcwRAIgZRfIY3p7/DoVTty6YZbWS71bc5Vct9p9Fia83eRmw2QCICK/ENGfwLtptFluMGs2KsqoNSk89pO7F29zJLUx9a/sASECx/EgAxlkQpQ9hYjgGu6EBCPMVPwVIVJqO4XCsMvViHI=
$

[kallewoof]

I am not sure how these test vectors were generated, both the ones in the bip and the ones used in the P.R. - perhaps a question for @kallewoof

Forgot to reply: I believe the test vectors in the BIP here were based on an outdated version. @rxgrant let me know if you have issues with or a different opinion on the matter.

Edit: missing word.

[kallewoof]

Squash?

[wip-abramson]

Sorry @kallewoof are you asking if we should squash, or asking me to squash?

Happy to squash if that is preferable.

[laanwj]

Yes, please squash, this looks like a single atomic change.

[kallewoof]

↑

[wip-abramson]

Is this squashed? I seem to just be adding more commits.

Apologies, I don't think I have ever attempted to squash before

[michaelfolkson]

@wip-abramson: There are 4 commits now. For some help on squashing commits see https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits or page 284 of the Pro Git book. You also have a merge commit that you'll want to remove.

If you'd rather start afresh on your branch and go back to latest top commit on master you can do a git reset -hard insert_current_top_commit_hash, re-add your commit and force push.

edit: It is also recommended that you open a PR from a topic branch rather than the master branch in your fork.

[wip-abramson]

Thanks @michaelfolkson. This looks more like it. Although, if you prefer I could reopen this PR from a branch as suggested.

[michaelfolkson]

@wip-abramson: I don't think this BIPs repo is as strict on that as the Core repo is but one of the BIP editors can correct me if wrong. It is definitely better practice though for any future PRs made to this repo.

[kallewoof]

Typically not as strict, but no need to be overly committy ;)