Implements un/serialization of MuSig2 PSBT fields and prepares PSBT to be able to sign for MuSig2 inputs.
Split from #29675
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31247.
See the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Possible typos and grammar issues:
717@@ -672,6 +718,80 @@ struct PSBTInput
718 UnserializeFromVector(s, m_tap_merkle_root);
719 break;
720 }
721+ case PSBT_IN_MUSIG2_PARTICIPANT_PUBKEYS:
722+ {
723+ if (!key_lookup.emplace(key).second) {
724+ throw std::ios_base::failure("Duplicate Key, input participant pubkeys for an aggregate key already provided");
725+ } else if (key.size() != 34) {
726+ throw std::ios_base::failure("Input musig2 participants pubkeys aggregate key is more than 33 bytes");
error message nit:
0 throw std::ios_base::failure("Input musig2 participants pubkeys aggregate key is not 34 bytes");
(assuming we are refering to the complete size of the key here, including the compact-size type prefix; we seem to do so on other key types like PSBT_IN_TAP_SCRIPT_SIG)
Code-review ACK 8da5ab60b58b808d693d251c8605d53ae54ba617
I’ve prepared a branch for adding the MuSig2 key types to the test framework and adding a functional test that verifies the decodepsbt
results on these, feel free to either pull in or ignore (will just open a separate PR after this one is merged then): https://github.com/theStack/bitcoin/tree/202411-test-add_musig2_decodepsbt_test
1371+ musig_keys.push_back(musig_part);
1372+ }
1373+ in.pushKV("musig2_participant_pubkeys", musig_keys);
1374+ }
1375+ if (!input.m_musig2_pubnonces.empty()) {
1376+ UniValue musig_pubnonces(UniValue::UniValue::VARR);
0 UniValue musig_pubnonces(UniValue::VARR);
1386+ }
1387+ }
1388+ in.pushKV("musig2_pubnonces", musig_pubnonces);
1389+ }
1390+ if (!input.m_musig2_partial_sigs.empty()) {
1391+ UniValue musig_partial_sigs(UniValue::UniValue::VARR);
0 UniValue musig_partial_sigs(UniValue::VARR);
926@@ -927,6 +927,37 @@ const RPCResult decodepsbt_inputs{
927 }},
928 {RPCResult::Type::STR_HEX, "taproot_internal_key", /*optional=*/ true, "The hex-encoded Taproot x-only internal key"},
929 {RPCResult::Type::STR_HEX, "taproot_merkle_root", /*optional=*/ true, "The hex-encoded Taproot merkle root"},
930+ {RPCResult::Type::ARR, "musig2_participant_pubkeys", /*optional*/true, "",
/*optional=*/ true
like above
utACK 8da5ab60b58b808d693d251c8605d53ae54ba617
Would have been nice to have tests here but fine for me to keep it for a follow-up since this is part of a larger project anyway.
re-ACK ea53b468e2650dc6e5d6d996cc899846a5ffe901
Just addressed minor review comments.
re-ACK ea53b468e2650dc6e5d6d996cc899846a5ffe901
Verified that all review comments have been adressed since my previous ACK.
Would have been nice to have tests here but fine for me to keep it for a follow-up
#29675 introduces wallet_musig.py
which uses PSBT internally. IIUC all new fields are needed for MuSig2 to work at all, so they’re tested implicitly. Additionally the test inspects musig2_participant_pubkeys
and musig2_partial_sigs
, though not musig2_pubnonces
.
That said, I think it would be better if this PR introduced at least some basic test coverage for e.g. the new errors that are introduced, and a simple round trip. This could be combined with generating test vectors for BIP 373.
Test vectors should probably include some PSBTv2 examples, which would have to go in a followup after #21283.
733+ std::vector<unsigned char> val;
734+ s >> val;
735+ SpanReader s_val{val};
736+ while (!s_val.empty()) {
737+ std::array<unsigned char, 33> part_key_bytes;
738+ s_val >> AsWritableBytes(Span{part_key_bytes});
s_val
has less than 33 bytes remaining, this throws?
re-ACK 7064b1a45a54f4fae2cb390a444c8eca27410c88
Didn’t check the individual test vectors in detail though; from what I can see the two corrections in the BIP (https://github.com/bitcoin/bips/pull/1769) don’t affect this PR.
I compared the test vectors here to those in the BIP and I couldn’t find the “Spend of a Taproot output where a key in a script is a MuSig2 Aggregate Pubkey - With participant pubkeys only” case here.
e.g. this Base64 String
0cHNidP8BAFICAAAAAZqLSlB5a5YAmQ9/4R36ALxw79KWBIr8hnGa8PsfqRk3AQAAAAD9////ARjd9QUAAAAAFgAUyRI+BujX8JZsXRzQ+TMALU63V80AAAAAAAEBKwDh9QUAAAAAIlEgVr20gbTWcQP21d6oqar9NoSmp5tKLiR3mduNSxqG4fgiFcBQkpt0waBJVLeLS2A16XpeB4paDyjsltVHv+6azoA6wCMgC1jjN6pNOFKowpOHxCQI2M++OmE6Xjl+Cp8BpftxB9SswCEWC1jjN6pNOFKowpOHxCQI2M++OmE6Xjl+Cp8BpftxB9QlAbEf7apjoJVlAacwjJO1Y3Nx52E9m4reF4PUnibAbPosJoDdbiEWNGuZWTNXEHydNFnp3rqNPq9E5mNshcf4U+uQulLozQAlAbEf7apjoJVlAacwjJO1Y3Nx52E9m4reF4PUnibAbPosWAsIhyEWT6/WX4FpGG/Cv9siM8d+Yw0QvigKJMcWXAmidhF3XCwlAbEf7apjoJVlAacwjJO1Y3Nx52E9m4reF4PUnibAbPoswySagiEWUJKbdMGgSVS3i0tgNel6XgeKWg8o7JbVR7/ums6AOsAFAHxGHl0hFvkwigGSWMMQSTRPhfidUim1MchFg2+ZsIYB8RO84Db5JQGxH+2qY6CVZQGnMIyTtWNzcedhPZuK3heD1J4mwGz6LH3WVZIBFyBQkpt0waBJVLeLS2A16XpeB4paDyjsltVHv+6azoA6wAEYILEf7apjoJVlAacwjJO1Y3Nx52E9m4reF4PUnibAbPosIhoDC1jjN6pNOFKowpOHxCQI2M++OmE6Xjl+Cp8BpftxB9RjAjRrmVkzVxB8nTRZ6d66jT6vROZjbIXH+FPrkLpS6M0AAk+v1l+BaRhvwr/bIjPHfmMNEL4oCiTHFlwJonYRd1wsAvkwigGSWMMQSTRPhfidUim1MchFg2+ZsIYB8RO84Db5AAA=
Am I missing something or did this one get lost?
Am I missing something or did this one get lost?
Looks like it got lost. Added it now.
re-ACK debacd1f59641c1e9e268c20820f4c8f0fc583f7
Confirmed all the BIP373 test vectors are present in the PR.
re-ACK debacd1f59641c1e9e268c20820f4c8f0fc583f7
Only a missing valid test vector has been added since my previous ACK.
220@@ -217,6 +221,11 @@ struct PSBTInput
221 XOnlyPubKey m_tap_internal_key;
222 uint256 m_tap_merkle_root;
223
224+ // MuSig2 fields
225+ std::map<CPubKey, std::vector<CPubKey>> m_musig2_participants;
226+ std::map<std::pair<CPubKey, uint256>, std::map<CPubKey, std::vector<uint8_t>>> m_musig2_pubnonces;
227+ std::map<std::pair<CPubKey, uint256>, std::map<CPubKey, uint256>> m_musig2_partial_sigs;
I think these 2 can be benefited with some documentation something like below. It was only after I read the below serialisation code that I understood this type.
0/** key is a pair of aggregate public key and leaf hash, value is a map of participant pubkey to pubnonce */
1std::map<std::pair<CPubKey, uint256>, std::map<CPubKey, std::vector<uint8_t>>> m_musig2_pubnonces;
2/** key is a pair of aggregate public key and leaf hash, value is a map of participant pubkey to partial sig */
3std::map<std::pair<CPubKey, uint256>, std::map<CPubKey, uint256>> m_musig2_partial_sigs;
IIUC, these types won’t be able to avoid collision in case the same aggregate public key appears twice while spending a script in the script path spend like in the below test (pubkey[1]
appears twice in the leaf script, zero_fn
script can be ignored for this case). I’m assuming for the same aggregate pubkey appearing multiple times in a script, there can be different signatures (because it appears just like any single-sig) for which there could be separate pubnonces. In the current structure, only the last one read would be stored in the structure while the previous ones overridden.
I don’t think it is a big issue though because I am unable to see any benefit of putting the same aggregate public key multiple times in a leaf script. PLMK if I am missing something and/or if this is not a valid scenario to consider.
359+ }
360+
361+ // Write MuSig2 pubnonces
362+ for (const auto& [agg_key_leaf_hash, pubnonces] : m_musig2_pubnonces) {
363+ const auto& [agg_key, leaf_hash] = agg_key_leaf_hash;
364+ for (const auto& [pubkey, pubnonce] : pubnonces) {
Nit for verbosity here and in the partial sigs loop below.
0-for (const auto& [pubkey, pubnonce] : pubnonces) {
1+for (const auto& [participant_pubkey, pubnonce] : pubnonces) {
731+
732+ std::vector<CPubKey> participants;
733+ std::vector<unsigned char> val;
734+ s >> val;
735+ SpanReader s_val{val};
736+ while (s_val.size() >= 33) {
Here and in other similar occurrences of 33.
0-while (s_val.size() >= 33) {
1+while (s_val.size() >= CPubKey::COMPRESSED_SIZE) {
717@@ -672,6 +718,86 @@ struct PSBTInput
718 UnserializeFromVector(s, m_tap_merkle_root);
719 break;
720 }
721+ case PSBT_IN_MUSIG2_PARTICIPANT_PUBKEYS:
722+ {
723+ if (!key_lookup.emplace(key).second) {
724+ throw std::ios_base::failure("Duplicate Key, input participant pubkeys for an aggregate key already provided");
725+ } else if (key.size() != 34) {
CPubKey::COMPRESSED_SIZE + 1
here instead of 34
would make it more explicit and reader-friendly (similarly in the other key unserialization flows below) like done for PSBT_IN_PARTIAL_SIG
and PSBT_IN_RIPEMD160
cases previously.
747+ }
748+ case PSBT_IN_MUSIG2_PUB_NONCE:
749+ {
750+ if (!key_lookup.emplace(key).second) {
751+ throw std::ios_base::failure("Duplicate Key, input musig2 pubnonce already provided");
752+ } else if (key.size() != 67 && key.size() != 99) {
Maybe?
(2 * CPubKey::COMPRESSED_SIZE + 1)
(2 * CPubKey::COMPRESSED_SIZE + CSHA256::OUTPUT_SIZE + 1)
775+ }
776+ case PSBT_IN_MUSIG2_PARTIAL_SIG:
777+ {
778+ if (!key_lookup.emplace(key).second) {
779+ throw std::ios_base::failure("Duplicate Key, input musig2 partial sig already provided");
780+ } else if (key.size() != 67 && key.size() != 99) {
915@@ -916,6 +916,37 @@ const RPCResult decodepsbt_inputs{
916 }},
compressed
terminology here over plain
like proposed in the BIP change here: https://github.com/bitcoin/bips/pull/1705
345@@ -337,6 +346,43 @@ struct PSBTInput
346 SerializeToVector(s, PSBT_IN_TAP_MERKLE_ROOT);
347 SerializeToVector(s, m_tap_merkle_root);
348 }
349+
350+ // Write MuSig2 Participants
351+ for (const auto& [agg_key, part_pubs] : m_musig2_participants) {
agg_pubkey
?
Using only key
alludes to the private key though in this case there is no aggregate private key but still to be consistent with the rest of the codebase and to get rid of any doubt in the mind of the reader.
1355+ UniValue part_keys(UniValue::VARR);
1356+ for (const auto& pub : parts) {
1357+ part_keys.push_back(HexStr(pub));
1358+ }
1359+ musig_part.pushKV("participant_pubkeys", part_keys);
1360+ musig_keys.push_back(musig_part);
_pubkeys
instead of _keys
In order to be consistent with the codebase
748+ case PSBT_IN_MUSIG2_PUB_NONCE:
749+ {
750+ if (!key_lookup.emplace(key).second) {
751+ throw std::ios_base::failure("Duplicate Key, input musig2 pubnonce already provided");
752+ } else if (key.size() != 67 && key.size() != 99) {
753+ throw std::ios_base::failure("Input musig2 pubnonce key is not expected size");
... is not expected size of either 67 or 99 bytes
1069+ throw std::ios_base::failure("Output musig2 participants pubkeys value size is not a multiple of 33");
1070+ }
1071+
1072+ m_musig2_participants.emplace(agg_key, participants);
1073+ break;
1074+ }
m_musig2_participants
is reused. Only the key type and the error message differ slightly.
Can be extracted into a function and reused in both input and output block.
Partial review.
Some of the suggested comments I have not categorised as nits so that the newer changes are consistent with the rest of the wallet codebase. However, I’m open to them not being implemented if the PR author and other reviewers believe so.
🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/37362505493
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
196@@ -193,6 +197,30 @@ void SerializeHDKeypaths(Stream& s, const std::map<CPubKey, KeyOriginInfo>& hd_k
197 }
198 }
199
200+// Deserialize a PSBT_{IN/OUT}_MUSIG2_PARTICIPANT_PUBKEYS field
201+template<typename Stream>
202+void DeserializeMuSig2ParticipantPubkeys(Stream& s, SpanReader& skey, std::map<CPubKey, std::vector<CPubKey>>& out, std::string context)
203+{
204+ std::array<unsigned char, 33> agg_pubkey_bytes;
I realise that my previous review didn’t mention to remove all instances clearly, though that’s what I had in mind.
There are few instances of 33
and 34
left, I feel now that we are using CPubKey::COMPRESSED_SIZE
in few places already, we might as well use it in the whole commit. I don’t see a reason to keep remaining instances of hardcoded values here.
797+ CPubKey agg_pub{std::span{agg_pubkey_bytes}};
798+ CPubKey part_pub{std::span{part_pubkey_bytes}};
799+
800+ if (!skey.empty()) {
801+ skey >> leaf_hash;
802+ }
This portion is exactly same as the one used in the nonces case, can be extracted into a DeserializeMuSig2PartialSigOrPubNonceKey
function because the format of the key is same in both the cases.
Though the duplication is not enough but the deduplicated version would enforce the similarity of the key format to the reader easily.
773+ skey >> leaf_hash;
774+ }
775+
776+ std::vector<uint8_t> pubnonce;
777+ s >> pubnonce;
778+ if (pubnonce.size() != 66) {
66
here, I wouldn’t use 2 * CPubKey::COMPRESSED_SIZE
and instead I believe a constant for nonce size would be more suitable that could be used later in this commit (part of draft PR) as well: https://github.com/bitcoin/bitcoin/pull/29675/commits/59612a514e348c2c3a8d2fbb82ac8245b139febc
1496+ UniValue part_pubkeys(UniValue::VARR);
1497+ for (const auto& pub : parts) {
1498+ part_pubkeys.push_back(HexStr(pub));
1499+ }
1500+ musig_part.pushKV("participant_pubkeys", part_pubkeys);
1501+ musig_pubkeys.push_back(musig_part);
if
block can be deduplicated by using passing input/output
& in/out
as args to common function.
930+ {RPCResult::Type::ARR, "musig2_pubnonces", /*optional=*/true, "",
931+ {
932+ {RPCResult::Type::OBJ, "", "",
933+ {
934+ {RPCResult::Type::STR_HEX, "participant_pubkey", "The compressed public key of the participant that created this pubnonce."},
935+ {RPCResult::Type::STR_HEX, "aggregate_pubkey", "The compressed aggregate public key for which this pubnonce is for."},
Nit here and in the musig2_partial_sigs
section below.
0- {RPCResult::Type::STR_HEX, "aggregate_pubkey", "The compressed aggregate public key for which this pubnonce is for."},
1+ {RPCResult::Type::STR_HEX, "aggregate_pubkey", "The compressed aggregate public key for which this pubnonce is."},
Nitty nit: I think both the original and the suggestion are wrong.
“The compressed aggregate public key for which this pubnonce is for.”
I had verified the implementation with the spec mentioned in BIP 373 & had navigated through the data stream serialization functions used internally when I reviewed the PR last time but forgot to mention in my review - thanks for implementing the suggested changes in previous review. Though I feel there is still some duplication & hardcoded values that can be removed and I would be quick to review/ack if they, too, are addressed.
From the last time, besides the suggested changes, there are Span -> std::span
and AsWritableBytes -> std::as_writable_bytes
refactors as well that I found in the range diff: git range-diff debacd1...a18efa9
. I had a concern regarding the format of the data structures used in case of duplicated aggregate public keys but that was answered here: #31247 (review).
The whole diff is the addition of the (de)serialization of the MuSig2 fields in the PSBT, introduction of the subsequently required internal data structures to hold these fields, and then addition of the said values in the response of the decodepsbt
RPC all of which seem like straightforward additions IMO. I haven’t reviewed the test vectors as it was done in a previous review by someone else but I’ll take a look at those as well tomorrow.
Code Review 8e08b335da104cab4918e1508af2d6a5ee239bfc
220+
221+ out.emplace(agg_pubkey, participants);
222+}
223+
224+// Deserialize the MuSig2 participant identifiers from PSBT_MUSIG2_{PUBNONCE/PARTIAL_SIG} fields
225+// Both fields contain the same data after the type byte - aggregate pubkey | particitpant pubkey | leaf script hash
Code review ACK f385ff7aba0eab6cf00ede1b808817dfd7047399
Typo may be fixed in the next MuSig2 PR unless you want to retouch.
775+ }
776+ case PSBT_IN_MUSIG2_PUB_NONCE:
777+ {
778+ if (!key_lookup.emplace(key).second) {
779+ throw std::ios_base::failure("Duplicate Key, input musig2 pubnonce already provided");
780+ } else if (key.size() != 2 * CPubKey::COMPRESSED_SIZE + 1 && key.size() != 2 * CPubKey::COMPRESSED_SIZE + CSHA256::OUTPUT_SIZE + 1) {
nit: could add parentheses for better readability (here and below for PSBT_IN_MUSIG2_PARTIAL_SIG
)
0 } else if (key.size() != (2 * CPubKey::COMPRESSED_SIZE + 1) && key.size() != (2 * CPubKey::COMPRESSED_SIZE + CSHA256::OUTPUT_SIZE + 1)) {
re-ACK e261eb8d50c7192260a449e653453e63f59dbeed
(as per $ git range-diff debacd1f...e261eb8d
)
Verified that since my previous ACK, the changes didn’t affect the logic, and were mostly about code deduplication (for deserializing participant pubkeys), getting rid of magic numbers, variable naming (s/key/pubkey/), and adapting the terminology to the recent BIP-373 changes (plain public key -> compressed public key). LGTM.
71+ "cHNidP8BAFICAAAAAVaG3/QAFl9OBApYVfZYCTRyybz4EIsnKl0x8YH3tP+xAQAAAAD9////ARjd9QUAAAAAFgAUyRI+BujX8JZsXRzQ+TMALU63V80AAAAAAAEBKwDh9QUAAAAAIlEgC1jjN6pNOFKowpOHxCQI2M++OmE6Xjl+Cp8BpftxB9QhFgtY4zeqTThSqMKTh8QkCNjPvjphOl45fgqfAaX7cQfUBQAmgN1uIRY0a5lZM1cQfJ00Weneuo0+r0TmY2yFx/hT65C6UujNAAUAWAsIhyEWT6/WX4FpGG/Cv9siM8d+Yw0QvigKJMcWXAmidhF3XCwFAMMkmoIhFvkwigGSWMMQSTRPhfidUim1MchFg2+ZsIYB8RO84Db5BQB91lWSIhoDC1jjN6pNOFKowpOHxCQI2M++OmE6Xjl+Cp8BpftxB9RjAjRrmVkzVxB8nTRZ6d66jT6vROZjbIXH+FPrkLpS6M0AAk+v1l+BaRhvwr/bIjPHfmMNEL4oCiTHFlwJonYRd1wsAvkwigGSWMMQSTRPhfidUim1MchFg2+ZsIYB8RO84Db5QhsCNGuZWTNXEHydNFnp3rqNPq9E5mNshcf4U+uQulLozQALWOM3qk04UqjCk4fEJAjYz746YTpeOX4KnwGl+3EH1EICUpsZ14eczATJFUh/XxNBvGhYsLx05QNsZD03tTpDcbUDt/iv4yY/yz7yRU/hbzpnWcVgDHjmN7jfoNg+hVKIIFZDGwJPr9ZfgWkYb8K/2yIzx35jDRC+KAokxxZcCaJ2EXdcLAMLWOM3qk04UqjCk4fEJAjYz746YTpeOX4KnwGl+3EH1EIDqB2XNJnnT4LStasT8eaenhGl/bQPVGbvZy5uTOugvnkDSZWTdnFQOU7s44TQUcRbYVxS2+HI6zYQ17NEtDt9rW1DGwL5MIoBkljDEEk0T4X4nVIptTHIRYNvmbCGAfETvOA2+QMLWOM3qk04UqjCk4fEJAjYz746YTpeOX4KnwGl+3EH1EIDOUnb/zG51QJR2IIeZsxcl7fjr9fZW/3HdQD9qJR/OuACPyDk8L8LdvDUDNOr/nzjc9ZoW713tfoo5/SftCnRibAAAA==",
72+ "Input musig2 pubnonce key is not expected size"
73+ ],
74+ [
75+ "cHNidP8BAFICAAAAAVaG3/QAFl9OBApYVfZYCTRyybz4EIsnKl0x8YH3tP+xAQAAAAD9////ARjd9QUAAAAAFgAUyRI+BujX8JZsXRzQ+TMALU63V80AAAAAAAEBKwDh9QUAAAAAIlEgC1jjN6pNOFKowpOHxCQI2M++OmE6Xjl+Cp8BpftxB9QhFgtY4zeqTThSqMKTh8QkCNjPvjphOl45fgqfAaX7cQfUBQAmgN1uIRY0a5lZM1cQfJ00Weneuo0+r0TmY2yFx/hT65C6UujNAAUAWAsIhyEWT6/WX4FpGG/Cv9siM8d+Yw0QvigKJMcWXAmidhF3XCwFAMMkmoIhFvkwigGSWMMQSTRPhfidUim1MchFg2+ZsIYB8RO84Db5BQB91lWSIhoDC1jjN6pNOFKowpOHxCQI2M++OmE6Xjl+Cp8BpftxB9RjAjRrmVkzVxB8nTRZ6d66jT6vROZjbIXH+FPrkLpS6M0AAk+v1l+BaRhvwr/bIjPHfmMNEL4oCiTHFlwJonYRd1wsAvkwigGSWMMQSTRPhfidUim1MchFg2+ZsIYB8RO84Db5Qhs0a5lZM1cQfJ00Weneuo0+r0TmY2yFx/hT65C6UujNAAMLWOM3qk04UqjCk4fEJAjYz746YTpeOX4KnwGl+3EH1EICUpsZ14eczATJFUh/XxNBvGhYsLx05QNsZD03tTpDcbUDt/iv4yY/yz7yRU/hbzpnWcVgDHjmN7jfoNg+hVKIIFZDGwJPr9ZfgWkYb8K/2yIzx35jDRC+KAokxxZcCaJ2EXdcLAMLWOM3qk04UqjCk4fEJAjYz746YTpeOX4KnwGl+3EH1EIDqB2XNJnnT4LStasT8eaenhGl/bQPVGbvZy5uTOugvnkDSZWTdnFQOU7s44TQUcRbYVxS2+HI6zYQ17NEtDt9rW1DGwL5MIoBkljDEEk0T4X4nVIptTHIRYNvmbCGAfETvOA2+QMLWOM3qk04UqjCk4fEJAjYz746YTpeOX4KnwGl+3EH1EIDOUnb/zG51QJR2IIeZsxcl7fjr9fZW/3HdQD9qJR/OuACPyDk8L8LdvDUDNOr/nzjc9ZoW713tfoo5/SftCnRibAAAA==",
76+ "Input musig2 pubnonce key is not expected size"
Now that the error is more detailed; the test passes because assert_raises_rpc_error
checks for substring.
But I can pick this up in a follow up.
0- Input musig2 pubnonce key is not expected size
1+ Input musig2 pubnonce key is not expected size of 67 or 99 bytes
tACK e261eb8d50c7192260a449e653453e63f59dbeed
Thanks for addressing the comments. I have checked the test vectors as well, all of them from the BIP are there.
0git range-diff 8e08b33...e261eb8