From 7b15e2cb442c0bfec76d7d4b9abdd31c7a538da8 Mon Sep 17 00:00:00 2001 From: Shuvam Pandey Date: Sat, 25 Jul 2026 03:17:46 +0545 Subject: [PATCH 1/2] descriptor: fix duplicate check for hardened keys The miniscript duplicate key check compares two key expressions by deriving each of them at index 0, and it did that with an empty signing provider. Any expression with a hardened step could therefore not be derived and came back empty, so two of them compared equal and the descriptor was rejected with "contains duplicate public keys" even though the keys were different. musig() makes this easy to run into, since one participant on a hardened path keeps the whole aggregate key from resolving, but plain key expressions are affected just the same. Derive with the signing provider that is filled while parsing, or with the one we are inferring from, since that is where the private keys for the hardened steps are. If both keys still cannot be derived, compare the key expressions instead, so that two different expressions are not mistaken for one another. A duplicate written two different ways can still be missed if either spelling cannot be derived. The private key that is missing for the comparison is also needed to expand the descriptor, so deriveaddresses and importdescriptors both refuse it. Once the needed private keys are available, both expressions resolve and the duplicate is caught again. --- src/script/descriptor.cpp | 27 ++++++++++++++------------- src/test/descriptor_tests.cpp | 7 +++++++ test/functional/wallet_musig.py | 8 ++++++-- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index b632e133a15..c2f14b2c14d 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -32,6 +32,7 @@ #include #include +#include #include #include #include @@ -186,18 +187,6 @@ public: virtual ~PubkeyProvider() = default; - /** Compare two public keys represented by this provider. - * Used by the Miniscript descriptors to check for duplicate keys in the script. - */ - bool operator<(PubkeyProvider& other) const { - FlatSigningProvider dummy; - - std::optional a = GetPubKey(0, dummy, dummy); - std::optional b = other.GetPubKey(0, dummy, dummy); - - return a < b; - } - /** Derive a public key and put it into out. * read_cache is the cache to read keys from (if not nullptr) * write_cache is the cache to write keys to (if not nullptr) @@ -2264,7 +2253,19 @@ struct KeyParser { : m_out(out), m_in(in), m_script_ctx(ctx), m_expr_index(key_exp_index) {} bool KeyCompare(const Key& a, const Key& b) const { - return *m_keys.at(a).at(0) < *m_keys.at(b).at(0); + // Deriving a hardened step needs the private key, so use the provider that was filled + // while parsing, or the one we are inferring from, rather than an empty one. + const SigningProvider& provider{m_out ? *m_out : (m_in ? *m_in : DUMMY_SIGNING_PROVIDER)}; + const PubkeyProvider& key_a{*m_keys.at(a).at(0)}; + const PubkeyProvider& key_b{*m_keys.at(b).at(0)}; + FlatSigningProvider out_a, out_b; + const std::optional pub_a{key_a.GetPubKey(0, provider, out_a)}; + const std::optional pub_b{key_b.GetPubKey(0, provider, out_b)}; + if (pub_a && pub_b) return *pub_a < *pub_b; + // Keys that cannot be derived sort before the ones that can, and are compared by their + // expression so that two different keys are not taken for duplicates. + if (pub_a.has_value() != pub_b.has_value()) return !pub_a.has_value(); + return key_a.ToString() < key_b.ToString(); } ParseScriptContext ParseContext() const { diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 7366953f959..f3eb76e55fc 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -1114,6 +1114,10 @@ BOOST_AUTO_TEST_CASE(descriptor_test) CheckUnparsable("wsh(or_b(sha256(cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),s:pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)))", "wsh(or_b(sha256(cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),s:pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)))", "or_b(sha256(cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),s:pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)) is not sane: malleable witnesses exist"); CheckUnparsable("wsh(and_b(and_b(older(1),a:older(100000000)),s:pk(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd)))", "wsh(and_b(and_b(older(1),a:older(100000000)),s:pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)))", "and_b(older(1),a:older(100000000)) is not sane: contains mixes of timelocks expressed in blocks and seconds"); CheckUnparsable("wsh(and_b(or_b(pkh(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd),s:pk(Kx9HCDjGiwFcgVNhTrS5z5NeZdD6veeam61eDxLDCkGWujvL4Gnn)),s:pk(L4gM1FBdyHNpkzsFh9ipnofLhpZRp2mwobpeULy1a6dBTvw8Ywtd)))", "wsh(and_b(or_b(pkh(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),s:pk(032707170c71d8f75e4ca4e3fce870b9409dcaf12b051d3bcadff74747fa7619c0)),s:pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)))", "and_b(or_b(pkh(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204),s:pk(032707170c71d8f75e4ca4e3fce870b9409dcaf12b051d3bcadff74747fa7619c0)),s:pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)) is not sane: contains duplicate public keys"); + // Two keys that only differ after a hardened step are not duplicates. + Check("wsh(and_v(v:pk(xprv9s21ZrQH143K31xYSDQpPDxsXRTUcvj2iNHm5NUtrGiGG5e2DtALGdso3pGz6ssrdK4PFmM8NSpSBHNqPqm55Qn3LqFtT2emdEXVYsCzC2U/2147483647'/0),pk(xprv9s21ZrQH143K31xYSDQpPDxsXRTUcvj2iNHm5NUtrGiGG5e2DtALGdso3pGz6ssrdK4PFmM8NSpSBHNqPqm55Qn3LqFtT2emdEXVYsCzC2U/2147483647'/1)))", "wsh(and_v(v:pk(xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/2147483647'/0),pk(xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/2147483647'/1)))", "wsh(and_v(v:pk([bd16bee5/2147483647h]xpub69H7F5dQzmVd3vPuLKtcXJziMEQByuDidnX3YdwgtNsecY5HRGtAAQC5mXTt4dsv9RzyjgDjAQs9VGVV6ydYCHnprc9vvaA5YtqWyL6hyds/0),pk([bd16bee5/2147483647h]xpub69H7F5dQzmVd3vPuLKtcXJziMEQByuDidnX3YdwgtNsecY5HRGtAAQC5mXTt4dsv9RzyjgDjAQs9VGVV6ydYCHnprc9vvaA5YtqWyL6hyds/1)))", HARDENED, {{"0020cc001315f76b134f2027a7dff589fd9bbdfebc4691a77e0f589fafdadb01f662"}}, OutputType::BECH32, /*op_desc_id=*/std::nullopt, {{0xFFFFFFFFUL, 0}, {0xFFFFFFFFUL, 1}}); + // But the same key twice is. + CheckUnparsable("wsh(and_v(v:pk(xprv9s21ZrQH143K31xYSDQpPDxsXRTUcvj2iNHm5NUtrGiGG5e2DtALGdso3pGz6ssrdK4PFmM8NSpSBHNqPqm55Qn3LqFtT2emdEXVYsCzC2U/2147483647'/0),pk(xprv9s21ZrQH143K31xYSDQpPDxsXRTUcvj2iNHm5NUtrGiGG5e2DtALGdso3pGz6ssrdK4PFmM8NSpSBHNqPqm55Qn3LqFtT2emdEXVYsCzC2U/2147483647'/0)))", "wsh(and_v(v:pk(xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/2147483647'/0),pk(xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/2147483647'/0)))", "and_v(v:pk(xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/2147483647'/0),pk(xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/2147483647'/0)) is not sane: contains duplicate public keys"); // Valid with extended keys. Check("wsh(and_v(v:ripemd160(095ff41131e5946f3c85f79e44adbcf8e27e080e),multi(1,xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0)))", "wsh(and_v(v:ripemd160(095ff41131e5946f3c85f79e44adbcf8e27e080e),multi(1,xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0)))", "wsh(and_v(v:ripemd160(095ff41131e5946f3c85f79e44adbcf8e27e080e),multi(1,xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0)))", DEFAULT, {{"0020acf425291b98a1d7e0d4690139442abc289175be32ef1f75945e339924246d73"}}, OutputType::BECH32, /*op_desc_id=*/uint256{"0634b326edc66f9e2660562564d7a8fcca55f91dc4555ce0a51883cc72e0fa41"}, {{},{0}}); // Valid under sh(wsh()) and with a mix of xpubs and raw keys. @@ -1176,6 +1180,9 @@ BOOST_AUTO_TEST_CASE(descriptor_test) Check("tr(KwDiBf89QgGbjEhKnhXJuH7LrciVrZi3qYjgd9M7rFU74sHUHy8S,pk(musig(xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/0/*))","tr(f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9,pk(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/0/*))","tr(f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9,pk(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/0/*))", MISSING_PRIVKEYS | XONLY_KEYS | RANGE | MUSIG | MUSIG_DERIVATION, {{"512068983d461174afc90c26f3b2821d8a9ced9534586a756763b68371a404635cc8"}, {"5120368e2d864115181bdc8bb5dc8684be8d0760d5c33315570d71a21afce4afd43e"}, {"512097a1e6270b33ad85744677418bae5f59ea9136027223bc6e282c47c167b471d5"}}, OutputType::BECH32M, /*op_desc_id=*/std::nullopt, {{}, {0, 0}, {0, 1}, {0, 2}}); Check("tr(musig(xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/1,xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/1)/2)", "tr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1,xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1)/2)", "tr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1,xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1)/2)", XONLY_KEYS | MUSIG | MUSIG_DERIVATION | UNIQUE_XPUBS, {{"5120a17ceacd6422bd5ffd9f165807b254b7d68ad39f179cc4f11545a6835227e97c"}}, OutputType::BECH32M, /*op_desc_id=*/std::nullopt, {{1}, {2}}); Check("rawtr(musig(xprv9s21ZrQH143K31xYSDQpPDxsXRTUcvj2iNHm5NUtrGiGG5e2DtALGdso3pGz6ssrdK4PFmM8NSpSBHNqPqm55Qn3LqFtT2emdEXVYsCzC2U/2147483647'/0,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/1)","rawtr(musig(xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/2147483647'/0,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/1)","rawtr(musig([bd16bee5/2147483647h]xpub69H7F5dQzmVd3vPuLKtcXJziMEQByuDidnX3YdwgtNsecY5HRGtAAQC5mXTt4dsv9RzyjgDjAQs9VGVV6ydYCHnprc9vvaA5YtqWyL6hyds/0,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/1)", MISSING_PRIVKEYS | HARDENED | XONLY_KEYS | MUSIG | MUSIG_DERIVATION, {{"5120ebf2bcce516ef6567a9001ce6e5dc43a02bb62d37b51d86d773fa96dcd3a8d4c"}}, OutputType::BECH32M, /*op_desc_id=*/std::nullopt, {{}, {0xFFFFFFFFUL,0}, {1}}); + // A musig() with a hardened participant and a key on a hardened path are not duplicates + // just because neither of them can be derived without the private keys. + Check("tr(KwDiBf89QgGbjEhKnhXJuH7LrciVrZi3qYjgd9M7rFU74sHUHy8S,and_v(v:pk(musig(xprv9s21ZrQH143K31xYSDQpPDxsXRTUcvj2iNHm5NUtrGiGG5e2DtALGdso3pGz6ssrdK4PFmM8NSpSBHNqPqm55Qn3LqFtT2emdEXVYsCzC2U/2147483647',xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/0/*),pk(xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/2147483647'/*)))", "tr(f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9,and_v(v:pk(musig(xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/2147483647',xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/0/*),pk(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/2147483647'/*)))", "tr(f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9,and_v(v:pk(musig([bd16bee5/2147483647h]xpub69H7F5dQzmVd3vPuLKtcXJziMEQByuDidnX3YdwgtNsecY5HRGtAAQC5mXTt4dsv9RzyjgDjAQs9VGVV6ydYCHnprc9vvaA5YtqWyL6hyds,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/0/*),pk([31a507b8/2147483647h]xpub6FnCn6njftEsJk8fVPgMd9wrx9V8kbev3sQd1aQKHhUqY5es9yhtRWZBTb6YxtxVCZZYBoFWSEGDDD9m6hFCSZEyzr1e7NGixS6iTZ4jTJq/*)))", MISSING_PRIVKEYS | HARDENED | XONLY_KEYS | RANGE | MUSIG | MUSIG_DERIVATION | MIXED_MUSIG, {{"51209e6f89f5d818d823b2d6f5369da1622e9c5886733e496ae1f8dedbd7cb7e75f8"}, {"5120a9cd4ed7371eda00f4e9a7f72ba9b7eacac03e4bae45799eaa443dba0e2a23cb"}, {"5120a0b097ec0957a48ad2cc6fb214abdb63c35674abed9b6d10349721ffd004578f"}}, OutputType::BECH32M, /*op_desc_id=*/std::nullopt, {{}, {0xFFFFFFFFUL}, {0, 0}, {0, 1}, {0, 2}, {0xFFFFFFFFUL, 0}, {0xFFFFFFFFUL, 1}, {0xFFFFFFFFUL, 2}}); CheckMultipath("rawtr(musig(xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/<1;2;3>/0/*,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0/*,xprv9s21ZrQH143K3jUwNHoqQNrtzJnJmx4Yup8NkNLdVQCymYbPbJXnPhwkfTfxZfptcs3rLAPUXS39oDLgrNKQGwbGsEmJJ8BU3RzQuvShEG4/0/0/<3;4;5>/*))", "rawtr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/<1;2;3>/0/*,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0/*,xpub661MyMwAqRbcGDZQUKLqmWodYLcoBQnQH33yYkkF3jjxeLvY8qr2wWGEWkiKFaaQfJCoi3HeEq3Dc5DptfbCyjD38fNhSqtKc1UHaP4ba3t/0/0/<3;4;5>/*))", { diff --git a/test/functional/wallet_musig.py b/test/functional/wallet_musig.py index fbf0558ec99..59bf532dfdc 100755 --- a/test/functional/wallet_musig.py +++ b/test/functional/wallet_musig.py @@ -202,6 +202,9 @@ class WalletMuSigTest(BitcoinTestFramework): wallets, keys = self.create_wallets_and_keys_from_pattern(pat) self.construct_and_import_musig_descriptor_in_wallets(pat, wallets, keys, only_one_musig_wallet) + # The participant maps are keyed by the aggregate pubkey, which does not depend on the + # order of the participants nor on the derivation applied to the aggregate. + expected_participant_maps = len({tuple(sorted(musig.split(","))) for musig in MUSIG_RE.findall(pat)}) expected_pubnonces = 0 expected_partial_sigs = 0 for musig in MUSIG_RE.findall(pat): @@ -252,9 +255,9 @@ class WalletMuSigTest(BitcoinTestFramework): dec_psbt = self.nodes[0].decodepsbt(psbt) assert_equal(len(dec_psbt["inputs"]), 1) - assert_equal(len(dec_psbt["inputs"][0]["musig2_participant_pubkeys"]), pattern.count("musig(")) + assert_equal(len(dec_psbt["inputs"][0]["musig2_participant_pubkeys"]), expected_participant_maps) if has_internal: - assert_equal(len(dec_psbt["outputs"][1]["musig2_participant_pubkeys"]), pattern.count("musig(")) + assert_equal(len(dec_psbt["outputs"][1]["musig2_participant_pubkeys"]), expected_participant_maps) # Check all participant pubkeys in the input and change output psbt_maps = [dec_psbt["inputs"][0]] @@ -346,6 +349,7 @@ class WalletMuSigTest(BitcoinTestFramework): self.test_success_case("tr(H,pk(musig/*))", "tr($H,pk(musig($0,$1,$2)/<0;1>/*))", scriptpath=True) self.test_success_case("tr(H,{pk(musig/*), pk(musig/*)})", "tr($H,{pk(musig($0,$1,$2)/<0;1>/*),pk(musig($3,$4,$5)/0/*)})", scriptpath=True) self.test_success_case("tr(H,{pk(musig/*), pk(same keys different musig/*)})", "tr($H,{pk(musig($0,$1,$2)/<0;1>/*),pk(musig($1,$2)/0/*)})", scriptpath=True) + self.test_success_case("tr(H,and(pk(musig/*),pk(same musig, other derivation/*)))", "tr($H,and_v(v:pk(musig($0,$1,$2)/<0;1>/*),pk(musig($0,$1,$2)/<2;3>/*)))", scriptpath=True) self.test_success_case("tr(musig/*,{pk(partial keys diff musig-1/*),pk(partial keys diff musig-2/*)})}", "tr(musig($0,$1,$2)/<3;4>/*,{pk(musig($0,$1)/<5;6>/*),pk(musig($1,$2)/7/*)})") self.test_success_case("tr(musig/*,{pk(partial keys diff musig-1/*),pk(partial keys diff musig-2/*)})} script-path", "tr(musig($0,$1,$2)/<3;4>/*,{pk(musig($0,$1)/<5;6>/*),pk(musig($1,$2)/7/*)})", scriptpath=True, nosign_wallets=[0]) self.test_success_case("tr(H,and(pk(musig/*),after(1)))", "tr($H,and_v(v:pk(musig($0,$1,$2)/<0;1>/*),after(1)))", scriptpath=True) From b42f7fade0cd9a6fb3d69fcdff71436a99ba0005 Mon Sep 17 00:00:00 2001 From: Shuvam Pandey Date: Sat, 25 Jul 2026 11:35:36 +0545 Subject: [PATCH 2/2] descriptor: don't prepend key origins twice OriginPubkeyProvider::GetPubKey() derives its sub-provider straight into the output provider and then prepends its origin to the entry it finds there. The sub-providers insert with emplace(), so if the same key was already expanded by another key expression the insert is a no-op and the entry that gets the origin prepended is the one that already has it. musig() is where this shows up, because it expands its participants into the provider it is given, so two musig() expressions in the same key expression list end up expanding a shared participant twice. A participant origin of m/86h/1h/0h then comes out as m/86h/1h/0h/86h/1h/0h in the input and output taproot BIP32 derivation maps of a PSBT, so a signer that follows them derives the wrong key. When the two expressions declare different origins for the participant, the fingerprint of one is combined with the path of both. Derive into a temporary provider and apply the origin there. Merge the rest into the output, then insert the clean origin separately. This avoids stacking paths and makes an explicit origin replace an implicit one already present for the same key. --- src/script/descriptor.cpp | 14 +++++++++++--- src/test/descriptor_tests.cpp | 5 +++++ test/functional/wallet_musig.py | 7 ++++++- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index c2f14b2c14d..0d5b4e0a8d5 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -266,13 +266,21 @@ public: OriginPubkeyProvider(uint32_t exp_index, KeyOriginInfo info, std::unique_ptr provider, bool apostrophe) : PubkeyProvider(exp_index), m_origin(std::move(info)), m_provider(std::move(provider)), m_apostrophe(apostrophe) {} std::optional GetPubKey(int pos, const SigningProvider& arg, FlatSigningProvider& out, const DescriptorCache* read_cache = nullptr, DescriptorCache* write_cache = nullptr) const override { - std::optional pub = m_provider->GetPubKey(pos, arg, out, read_cache, write_cache); + // Derive into a temporary provider. Another key expression may have already put this + // key into out with its origin prefixed, and prefixing that entry would double it up. + FlatSigningProvider subprovider; + std::optional pub = m_provider->GetPubKey(pos, arg, subprovider, read_cache, write_cache); if (!pub) return std::nullopt; - Assert(out.pubkeys.contains(pub->GetID())); - auto& [pubkey, suborigin] = out.origins[pub->GetID()]; + const CKeyID keyid{pub->GetID()}; + Assert(subprovider.pubkeys.contains(keyid)); + auto& [pubkey, suborigin] = subprovider.origins[keyid]; Assert(pubkey == *pub); // m_provider must have a valid origin by this point. suborigin.fingerprint = m_origin.fingerprint; suborigin.path.insert(suborigin.path.begin(), m_origin.path.begin(), m_origin.path.end()); + auto origin{subprovider.origins.extract(keyid)}; + out.Merge(std::move(subprovider)); + // An explicit origin takes precedence over an implicit one for the same key. + out.origins.insert_or_assign(keyid, std::move(origin.mapped())); return pub; } bool IsRange() const override { return m_provider->IsRange(); } diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index f3eb76e55fc..ab3a91e55d1 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -1183,6 +1183,11 @@ BOOST_AUTO_TEST_CASE(descriptor_test) // A musig() with a hardened participant and a key on a hardened path are not duplicates // just because neither of them can be derived without the private keys. Check("tr(KwDiBf89QgGbjEhKnhXJuH7LrciVrZi3qYjgd9M7rFU74sHUHy8S,and_v(v:pk(musig(xprv9s21ZrQH143K31xYSDQpPDxsXRTUcvj2iNHm5NUtrGiGG5e2DtALGdso3pGz6ssrdK4PFmM8NSpSBHNqPqm55Qn3LqFtT2emdEXVYsCzC2U/2147483647',xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/0/*),pk(xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/2147483647'/*)))", "tr(f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9,and_v(v:pk(musig(xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/2147483647',xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/0/*),pk(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/2147483647'/*)))", "tr(f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9,and_v(v:pk(musig([bd16bee5/2147483647h]xpub69H7F5dQzmVd3vPuLKtcXJziMEQByuDidnX3YdwgtNsecY5HRGtAAQC5mXTt4dsv9RzyjgDjAQs9VGVV6ydYCHnprc9vvaA5YtqWyL6hyds,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y)/0/*),pk([31a507b8/2147483647h]xpub6FnCn6njftEsJk8fVPgMd9wrx9V8kbev3sQd1aQKHhUqY5es9yhtRWZBTb6YxtxVCZZYBoFWSEGDDD9m6hFCSZEyzr1e7NGixS6iTZ4jTJq/*)))", MISSING_PRIVKEYS | HARDENED | XONLY_KEYS | RANGE | MUSIG | MUSIG_DERIVATION | MIXED_MUSIG, {{"51209e6f89f5d818d823b2d6f5369da1622e9c5886733e496ae1f8dedbd7cb7e75f8"}, {"5120a9cd4ed7371eda00f4e9a7f72ba9b7eacac03e4bae45799eaa443dba0e2a23cb"}, {"5120a0b097ec0957a48ad2cc6fb214abdb63c35674abed9b6d10349721ffd004578f"}}, OutputType::BECH32M, /*op_desc_id=*/std::nullopt, {{}, {0xFFFFFFFFUL}, {0, 0}, {0, 1}, {0, 2}, {0xFFFFFFFFUL, 0}, {0xFFFFFFFFUL, 1}, {0xFFFFFFFFUL, 2}}); + // The origin of a participant that is expanded more than once only gets prepended once. + Check("tr(musig([0f056943/86h/1h/0h]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/1,[0f056943/86h/1h/0h]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/1)/2)", "tr(musig([0f056943/86h/1h/0h]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1,[0f056943/86h/1h/0h]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1)/2)", "tr(musig([0f056943/86h/1h/0h]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1,[0f056943/86h/1h/0h]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1)/2)", XONLY_KEYS | MUSIG | MUSIG_DERIVATION | UNIQUE_XPUBS, {{"5120a17ceacd6422bd5ffd9f165807b254b7d68ad39f179cc4f11545a6835227e97c"}}, OutputType::BECH32M, /*op_desc_id=*/std::nullopt, {{86 | 0x80000000UL, 1 | 0x80000000UL, 0 | 0x80000000UL, 1}, {2}}); + // An explicit origin replaces the implicit origin from an earlier expression. + const std::string mixed_origin_pub{"tr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1,[0f056943/86h]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/1)/2)"}; + Check("tr(musig(xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/1,[0f056943/86h]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/1)/2)", mixed_origin_pub, mixed_origin_pub, XONLY_KEYS | MUSIG | MUSIG_DERIVATION | UNIQUE_XPUBS, {{"5120a17ceacd6422bd5ffd9f165807b254b7d68ad39f179cc4f11545a6835227e97c"}}, OutputType::BECH32M, /*op_desc_id=*/std::nullopt, {{86 | 0x80000000UL, 1}, {2}}); CheckMultipath("rawtr(musig(xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/<1;2;3>/0/*,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0/*,xprv9s21ZrQH143K3jUwNHoqQNrtzJnJmx4Yup8NkNLdVQCymYbPbJXnPhwkfTfxZfptcs3rLAPUXS39oDLgrNKQGwbGsEmJJ8BU3RzQuvShEG4/0/0/<3;4;5>/*))", "rawtr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/<1;2;3>/0/*,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0/*,xpub661MyMwAqRbcGDZQUKLqmWodYLcoBQnQH33yYkkF3jjxeLvY8qr2wWGEWkiKFaaQfJCoi3HeEq3Dc5DptfbCyjD38fNhSqtKc1UHaP4ba3t/0/0/<3;4;5>/*))", { diff --git a/test/functional/wallet_musig.py b/test/functional/wallet_musig.py index 59bf532dfdc..e23c9496154 100755 --- a/test/functional/wallet_musig.py +++ b/test/functional/wallet_musig.py @@ -263,6 +263,7 @@ class WalletMuSigTest(BitcoinTestFramework): psbt_maps = [dec_psbt["inputs"][0]] if has_internal: psbt_maps.append(dec_psbt["outputs"][1]) + origin_paths = {ORIGIN_PATH_RE.search(pub).group(1) for _, pub in keys} for psbt_map in psbt_maps: part_pks = set() for agg in psbt_map["musig2_participant_pubkeys"]: @@ -270,9 +271,13 @@ class WalletMuSigTest(BitcoinTestFramework): part_pks.add(part_pub[2:]) # Check that there are as many participants as we expected assert_equal(len(part_pks), len(keys)) - # Check that each participant has a derivation path + # Check that each participant has a derivation path, and that its origin appears in + # that path just once no matter how many musig() expressions the participant is in for deriv_path in psbt_map["taproot_bip32_derivs"]: if deriv_path["pubkey"] in part_pks: + origin = next((o for o in origin_paths if deriv_path["path"].startswith(f"m{o}")), None) + assert origin is not None, deriv_path["path"] + assert_equal(deriv_path["path"].count(origin), 1) part_pks.remove(deriv_path["pubkey"]) assert_equal(len(part_pks), 0)