diff --git a/src/musig.cpp b/src/musig.cpp index af998085d82..d211790c43b 100644 --- a/src/musig.cpp +++ b/src/musig.cpp @@ -125,10 +125,10 @@ bool MuSig2SecNonce::IsValid() return m_impl->IsValid(); } -uint256 MuSig2SessionID(const CPubKey& script_pubkey, const CPubKey& part_pubkey, const uint256& sighash) +uint256 MuSig2SessionID(const CPubKey& script_pubkey, const CPubKey& part_pubkey, const uint256& sighash, const std::vector& pubnonce) { HashWriter hasher; - hasher << script_pubkey << part_pubkey << sighash; + hasher << script_pubkey << part_pubkey << sighash << pubnonce; return hasher.GetSHA256(); } diff --git a/src/musig.h b/src/musig.h index b61c2f3f49b..4e1ad5e9e6b 100644 --- a/src/musig.h +++ b/src/musig.h @@ -57,7 +57,11 @@ public: bool IsValid(); }; -uint256 MuSig2SessionID(const CPubKey& script_pubkey, const CPubKey& part_pubkey, const uint256& sighash); +/** + * Computes an arbitrary unique session ID to identify ongoing signing sessions. + * It is the SHA256 of the aggregate xonly key, the participant pubkey, the sighash, and the pubnonce + */ +uint256 MuSig2SessionID(const CPubKey& script_pubkey, const CPubKey& part_pubkey, const uint256& sighash, const std::vector& pubnonce); std::vector CreateMuSig2Nonce(MuSig2SecNonce& secnonce, const uint256& sighash, const CKey& our_seckey, const CPubKey& aggregate_pubkey, const std::vector& pubkeys); std::optional CreateMuSig2PartialSig(const uint256& hash, const CKey& our_seckey, const CPubKey& aggregate_pubkey, const std::vector& pubkeys, const std::map>& pubnonces, MuSig2SecNonce& secnonce, const std::vector>& tweaks); diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 6716d044128..f38ac2bee8e 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -137,7 +137,7 @@ std::vector MutableTransactionSignatureCreator::CreateMuSig2Nonce(const if (out.empty()) return {}; // Store the secnonce in the SigningProvider - provider.SetMuSig2SecNonce(MuSig2SessionID(script_pubkey, part_pubkey, *sighash), std::move(secnonce)); + provider.SetMuSig2SecNonce(MuSig2SessionID(script_pubkey, part_pubkey, *sighash, out), std::move(secnonce)); return out; } @@ -170,7 +170,9 @@ bool MutableTransactionSignatureCreator::CreateMuSig2PartialSig(const SigningPro if (!sighash.has_value()) return false; // Retrieve the secnonce - uint256 session_id = MuSig2SessionID(script_pubkey, part_pubkey, *sighash); + auto part_pubnonce_it = pubnonces.find(part_pubkey); + if (part_pubnonce_it == pubnonces.end()) return false; + uint256 session_id = MuSig2SessionID(script_pubkey, part_pubkey, *sighash, part_pubnonce_it->second); std::optional> secnonce = provider.GetMuSig2SecNonce(session_id); if (!secnonce || !secnonce->get().IsValid()) return false; diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 89f15c8db1c..9a5554d94b0 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -300,7 +300,7 @@ private: * must be done in order to prevent nonce reuse. * * The session id is an arbitrary value set by the signer in order for the signing logic - * to find ongoing signing sessions. It is the SHA256 of aggregate xonly key, + participant pubkey + sighash. + * to find ongoing signing sessions, see MuSig2SessionID. */ mutable std::map m_musig2_secnonces; diff --git a/test/functional/wallet_musig.py b/test/functional/wallet_musig.py index 0e3e4377c08..c392abaf86a 100755 --- a/test/functional/wallet_musig.py +++ b/test/functional/wallet_musig.py @@ -12,6 +12,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_greater_than, + assert_not_equal, ) PRIVKEY_RE = re.compile(r"^tr\((.+?)/.+\)#.{8}$") @@ -111,6 +112,31 @@ class WalletMuSigTest(BitcoinTestFramework): return wallets, psbt + def assert_musig_signer_data(self, first, second, different_key): + assert_equal(first["participant_pubkey"], second["participant_pubkey"]) + assert_equal(first["aggregate_pubkey"], second["aggregate_pubkey"]) + if "leaf_hash" in first: + assert_equal(first["leaf_hash"], second["leaf_hash"]) + else: + assert "leaf_hash" not in second + + assert_not_equal(first[different_key], second[different_key]) + + def assert_musig_aggregate_in_script(self, signer_data, pattern, psbtin): + pubkey = signer_data["aggregate_pubkey"][2:] + if "pkh" in pattern or "pk_h" in pattern: + pubkey = hash160(bytes.fromhex(pubkey)).hex() + if pubkey in psbtin["witness_utxo"]["scriptPubKey"]["hex"]: + return + elif "taproot_scripts" in psbtin: + for leaf_scripts in psbtin["taproot_scripts"]: + if pubkey in leaf_scripts["script"]: + break + else: + assert False, "Aggregate pubkey not seen as output key, or in any scripts" + else: + assert False, "Aggregate pubkey not seen as output key or internal key" + def test_failure_case_1(self, comment, pat): self.log.info(f"Testing {comment}") wallets, psbt = self.setup_musig_scenario(pat) @@ -247,66 +273,57 @@ class WalletMuSigTest(BitcoinTestFramework): part_pks.remove(deriv_path["pubkey"]) assert_equal(len(part_pks), 0) + # Run 2 signing sessions simultaneously to verify no nonce reuse # Add pubnonces nonce_psbts = [] + nonce_psbts2 = [] for i, wallet in enumerate(wallets): if nosign_wallets and i in nosign_wallets: continue - proc = wallet.walletprocesspsbt(psbt=psbt, sighashtype=sighash_type) - assert_equal(proc["complete"], False) - nonce_psbts.append(proc["psbt"]) + for psbt_list in [nonce_psbts, nonce_psbts2]: + proc = wallet.walletprocesspsbt(psbt=psbt, sighashtype=sighash_type) + assert_equal(proc["complete"], False) + psbt_list.append(proc["psbt"]) comb_nonce_psbt = self.nodes[0].combinepsbt(nonce_psbts) + comb_nonce_psbt2 = self.nodes[0].combinepsbt(nonce_psbts2) dec_psbt = self.nodes[0].decodepsbt(comb_nonce_psbt) + dec_psbt2 = self.nodes[0].decodepsbt(comb_nonce_psbt2) assert_equal(len(dec_psbt["inputs"][0]["musig2_pubnonces"]), expected_pubnonces) - for pn in dec_psbt["inputs"][0]["musig2_pubnonces"]: - pubkey = pn["aggregate_pubkey"][2:] - if "pkh" in pattern or "pk_h" in pattern: - pubkey = hash160(bytes.fromhex(pubkey)).hex() - if pubkey in dec_psbt["inputs"][0]["witness_utxo"]["scriptPubKey"]["hex"]: - continue - elif "taproot_scripts" in dec_psbt["inputs"][0]: - for leaf_scripts in dec_psbt["inputs"][0]["taproot_scripts"]: - if pubkey in leaf_scripts["script"]: - break - else: - assert False, "Aggregate pubkey for pubnonce not seen as output key, or in any scripts" - else: - assert False, "Aggregate pubkey for pubnonce not seen as output key or internal key" + assert_equal(len(dec_psbt2["inputs"][0]["musig2_pubnonces"]), expected_pubnonces) + for pn, pn2 in zip(dec_psbt["inputs"][0]["musig2_pubnonces"], dec_psbt2["inputs"][0]["musig2_pubnonces"]): + self.assert_musig_signer_data(pn, pn2, "pubnonce") + self.assert_musig_aggregate_in_script(pn, pattern, dec_psbt["inputs"][0]) # Add partial sigs psig_psbts = [] + psig_psbts2 = [] for i, wallet in enumerate(wallets): if nosign_wallets and i in nosign_wallets: continue - proc = wallet.walletprocesspsbt(psbt=comb_nonce_psbt, sighashtype=sighash_type) - assert_equal(proc["complete"], False) - psig_psbts.append(proc["psbt"]) + for psbt, psbt_list in [(comb_nonce_psbt, psig_psbts), (comb_nonce_psbt2, psig_psbts2)]: + proc = wallet.walletprocesspsbt(psbt=psbt, sighashtype=sighash_type) + assert_equal(proc["complete"], False) + psbt_list.append(proc["psbt"]) comb_psig_psbt = self.nodes[0].combinepsbt(psig_psbts) + comb_psig_psbt2 = self.nodes[0].combinepsbt(psig_psbts2) dec_psbt = self.nodes[0].decodepsbt(comb_psig_psbt) + dec_psbt2 = self.nodes[0].decodepsbt(comb_psig_psbt2) assert_equal(len(dec_psbt["inputs"][0]["musig2_partial_sigs"]), expected_partial_sigs) - for ps in dec_psbt["inputs"][0]["musig2_partial_sigs"]: - pubkey = ps["aggregate_pubkey"][2:] - if "pkh" in pattern or "pk_h" in pattern: - pubkey = hash160(bytes.fromhex(pubkey)).hex() - if pubkey in dec_psbt["inputs"][0]["witness_utxo"]["scriptPubKey"]["hex"]: - continue - elif "taproot_scripts" in dec_psbt["inputs"][0]: - for leaf_scripts in dec_psbt["inputs"][0]["taproot_scripts"]: - if pubkey in leaf_scripts["script"]: - break - else: - assert False, "Aggregate pubkey for partial sig not seen as output key or in any scripts" - else: - assert False, "Aggregate pubkey for partial sig not seen as output key" + assert_equal(len(dec_psbt2["inputs"][0]["musig2_partial_sigs"]), expected_partial_sigs) + for ps, ps2 in zip(dec_psbt["inputs"][0]["musig2_partial_sigs"], dec_psbt2["inputs"][0]["musig2_partial_sigs"]): + self.assert_musig_signer_data(ps, ps2, "partial_sig") + self.assert_musig_aggregate_in_script(ps, pattern, dec_psbt["inputs"][0]) # Non-participant aggregates partial sigs and send finalized = self.nodes[0].finalizepsbt(psbt=comb_psig_psbt, extract=False) - assert_equal(finalized["complete"], True) + finalized2 = self.nodes[0].finalizepsbt(psbt=comb_psig_psbt2, extract=False) + assert_equal(finalized["complete"], finalized2["complete"], True) witness = self.nodes[0].decodepsbt(finalized["psbt"])["inputs"][0]["final_scriptwitness"] + assert_not_equal(witness, self.nodes[0].decodepsbt(finalized2["psbt"])["inputs"][0]["final_scriptwitness"]) if scriptpath: assert_greater_than(len(witness), 1) else: