From ce4c66eb7c5e99e3df1c20d5c0ae8278a714b9f8 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 20 Jan 2026 12:54:12 -0800 Subject: [PATCH 1/5] test: Test that key expression indexes match key count --- src/script/descriptor.cpp | 46 ++++++++++++++++++++++++++-- src/script/descriptor.h | 6 ++++ src/test/descriptor_tests.cpp | 8 +++++ src/test/fuzz/descriptor_parse.cpp | 4 +++ src/wallet/test/walletload_tests.cpp | 2 ++ 5 files changed, 63 insertions(+), 3 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 8558c8e29d7..2be3301be3c 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -161,12 +161,11 @@ typedef std::vector KeyPath; /** Interface for public key objects in descriptors. */ struct PubkeyProvider { -protected: +public: //! Index of this key expression in the descriptor //! E.g. If this PubkeyProvider is key1 in multi(2, key1, key2, key3), then m_expr_index = 0 - uint32_t m_expr_index; + const uint32_t m_expr_index; -public: explicit PubkeyProvider(uint32_t exp_index) : m_expr_index(exp_index) {} virtual ~PubkeyProvider() = default; @@ -229,6 +228,9 @@ public: /** Whether this PubkeyProvider is a BIP 32 extended key that can be derived from */ virtual bool IsBIP32() const = 0; + + /** Get the count of keys known by this PubkeyProvider. Usually one, but may be more for key aggregation schemes */ + virtual size_t GetKeyCount() const { return 1; } }; class OriginPubkeyProvider final : public PubkeyProvider @@ -788,6 +790,10 @@ public: // musig() can only be a BIP 32 key if all participants are bip32 too return std::all_of(m_participants.begin(), m_participants.end(), [](const auto& pubkey) { return pubkey->IsBIP32(); }); } + size_t GetKeyCount() const override + { + return 1 + m_participants.size(); + } }; /** Base class for all Descriptor implementations. */ @@ -1041,6 +1047,40 @@ public: } return all; } + + uint32_t GetMaxKeyExpr() const final + { + uint32_t max_key_expr{0}; + std::vector todo = {this}; + while (!todo.empty()) { + const DescriptorImpl* desc = todo.back(); + todo.pop_back(); + for (const auto& p : desc->m_pubkey_args) { + max_key_expr = std::max(max_key_expr, p->m_expr_index); + } + for (const auto& s : desc->m_subdescriptor_args) { + todo.push_back(s.get()); + } + } + return max_key_expr; + } + + size_t GetKeyCount() const final + { + size_t count{0}; + std::vector todo = {this}; + while (!todo.empty()) { + const DescriptorImpl* desc = todo.back(); + todo.pop_back(); + for (const auto& p : desc->m_pubkey_args) { + count += p->GetKeyCount(); + } + for (const auto& s : desc->m_subdescriptor_args) { + todo.push_back(s.get()); + } + } + return count; + } }; /** A parsed addr(A) descriptor. */ diff --git a/src/script/descriptor.h b/src/script/descriptor.h index fe33476da52..3ac748a1746 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -181,6 +181,12 @@ struct Descriptor { /** Semantic/safety warnings (includes subdescriptors). */ virtual std::vector Warnings() const = 0; + + /** Get the maximum key expression index. Used only for tests */ + virtual uint32_t GetMaxKeyExpr() const = 0; + + /** Get the number of key expressions in this descriptor. Used only for tests */ + virtual size_t GetKeyCount() const = 0; }; /** Parse a `descriptor` string. Included private keys are put in `out`. diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 8ff44d13dcc..00782c37215 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -283,6 +283,14 @@ void DoCheck(std::string prv, std::string pub, const std::string& norm_pub, int BOOST_CHECK_EQUAL(parse_pub->IsRange(), (flags & RANGE) != 0); BOOST_CHECK_EQUAL(parse_priv->IsRange(), (flags & RANGE) != 0); + // Check that the highest key expression index matches the number of keys in the descriptor + BOOST_TEST_INFO("Pub desc: " + pub); + uint32_t key_exprs = parse_pub->GetMaxKeyExpr(); + BOOST_CHECK_EQUAL(key_exprs + 1, parse_pub->GetKeyCount()); + BOOST_TEST_INFO("Priv desc: " + prv); + BOOST_CHECK_EQUAL(key_exprs, parse_priv->GetMaxKeyExpr()); + BOOST_CHECK_EQUAL(key_exprs + 1, parse_priv->GetKeyCount()); + // * For ranged descriptors, the `scripts` parameter is a list of expected result outputs, for subsequent // positions to evaluate the descriptors on (so the first element of `scripts` is for evaluating the // descriptor at 0; the second at 1; and so on). To verify this, we evaluate the descriptors once for diff --git a/src/test/fuzz/descriptor_parse.cpp b/src/test/fuzz/descriptor_parse.cpp index eeba5f1369e..6b3084e23d1 100644 --- a/src/test/fuzz/descriptor_parse.cpp +++ b/src/test/fuzz/descriptor_parse.cpp @@ -61,6 +61,10 @@ static void TestDescriptor(const Descriptor& desc, FlatSigningProvider& sig_prov const bool is_nontop_or_nonsolvable{!*is_solvable || !desc.GetOutputType()}; const bool is_input_size_info_set{max_sat_maxsig && max_sat_nonmaxsig && max_elems}; assert(is_input_size_info_set || is_nontop_or_nonsolvable); + + auto max_key_expr = desc.GetMaxKeyExpr(); + auto key_count = desc.GetKeyCount(); + assert((max_key_expr == 0 && key_count == 0) || max_key_expr + 1 == key_count); } void initialize_descriptor_parse() diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index ec31cda3895..a397ffc092a 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -37,6 +37,8 @@ public: std::optional MaxSatisfactionElems() const override { return {}; } void GetPubKeys(std::set& pubkeys, std::set& ext_pubs) const override {} std::vector Warnings() const override { return {}; } + uint32_t GetMaxKeyExpr() const override { return 0; } + size_t GetKeyCount() const override { return 0; } }; BOOST_FIXTURE_TEST_CASE(wallet_load_descriptors, TestingSetup) From b12281bd86e2298ba6cdd79d55c9d6e23e5136a5 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 23 Dec 2025 13:37:08 -0800 Subject: [PATCH 2/5] miniscript: Use a reference to key_exp_index in KeyParser For key_exp_index to count correctly for miniscript expressions, KeyParser should hold a reference to it. --- src/script/descriptor.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 2be3301be3c..d17805926ca 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -2183,12 +2183,12 @@ struct KeyParser { mutable std::string m_key_parsing_error; //! The script context we're operating within (Tapscript or P2WSH). const miniscript::MiniscriptContext m_script_ctx; - //! The number of keys that were parsed before starting to parse this Miniscript descriptor. - uint32_t m_offset; + //! The current key expression index + uint32_t& m_expr_index; KeyParser(FlatSigningProvider* out LIFETIMEBOUND, const SigningProvider* in LIFETIMEBOUND, - miniscript::MiniscriptContext ctx, uint32_t offset = 0) - : m_out(out), m_in(in), m_script_ctx(ctx), m_offset(offset) {} + miniscript::MiniscriptContext ctx, uint32_t& key_exp_index LIFETIMEBOUND) + : 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); @@ -2206,8 +2206,8 @@ struct KeyParser { { assert(m_out); Key key = m_keys.size(); - uint32_t exp_index = m_offset + key; - auto pk = ParsePubkey(exp_index, {&*begin, &*end}, ParseContext(), *m_out, m_key_parsing_error); + auto pk = ParsePubkey(m_expr_index, {&*begin, &*end}, ParseContext(), *m_out, m_key_parsing_error); + ++m_expr_index; if (pk.empty()) return {}; m_keys.emplace_back(std::move(pk)); return key; @@ -2634,7 +2634,6 @@ std::vector> ParseScript(uint32_t& key_exp_index // A signature check is required for a miniscript to be sane. Therefore no sane miniscript // may have an empty list of public keys. CHECK_NONFATAL(!parser.m_keys.empty()); - key_exp_index += parser.m_keys.size(); // Make sure all vecs are of the same length, or exactly length 1 // For length 1 vectors, clone subdescs until vector is the same length size_t num_multipath = std::max_element(parser.m_keys.begin(), parser.m_keys.end(), @@ -2809,7 +2808,8 @@ std::unique_ptr InferScript(const CScript& script, ParseScriptCo if (ctx == ParseScriptContext::P2WSH || ctx == ParseScriptContext::P2TR) { const auto script_ctx{ctx == ParseScriptContext::P2WSH ? miniscript::MiniscriptContext::P2WSH : miniscript::MiniscriptContext::TAPSCRIPT}; - KeyParser parser(/* out = */nullptr, /* in = */&provider, /* ctx = */script_ctx); + uint32_t key_exp_index = 0; + KeyParser parser(/* out = */nullptr, /* in = */&provider, /* ctx = */script_ctx, key_exp_index); auto node = miniscript::FromScript(script, parser); if (node && node->IsSane()) { std::vector> keys; From 6fd780d4fbc497b657025afe48d0dfbf103ee120 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 23 Dec 2025 13:37:58 -0800 Subject: [PATCH 3/5] descriptors: Increment key_exp_index in ParsePubkey(Inner) Simplifies the callsites for incrementing key_exp_index --- src/script/descriptor.cpp | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index d17805926ca..f761757f3f5 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -1873,7 +1873,7 @@ static DeriveType ParseDeriveType(std::vector>& split, boo } /** Parse a public key that excludes origin information. */ -std::vector> ParsePubkeyInner(uint32_t key_exp_index, const std::span& sp, ParseScriptContext ctx, FlatSigningProvider& out, bool& apostrophe, std::string& error) +std::vector> ParsePubkeyInner(uint32_t& key_exp_index, const std::span& sp, ParseScriptContext ctx, FlatSigningProvider& out, bool& apostrophe, std::string& error) { std::vector> ret; bool permit_uncompressed = ctx == ParseScriptContext::TOP || ctx == ParseScriptContext::P2SH; @@ -1898,6 +1898,7 @@ std::vector> ParsePubkeyInner(uint32_t key_exp_i if (pubkey.IsFullyValid()) { if (permit_uncompressed || pubkey.IsCompressed()) { ret.emplace_back(std::make_unique(key_exp_index, pubkey, false)); + ++key_exp_index; return ret; } else { error = "Uncompressed keys are not allowed"; @@ -1909,6 +1910,7 @@ std::vector> ParsePubkeyInner(uint32_t key_exp_i pubkey.Set(std::begin(fullkey), std::end(fullkey)); if (pubkey.IsFullyValid()) { ret.emplace_back(std::make_unique(key_exp_index, pubkey, true)); + ++key_exp_index; return ret; } } @@ -1921,6 +1923,7 @@ std::vector> ParsePubkeyInner(uint32_t key_exp_i CPubKey pubkey = key.GetPubKey(); out.keys.emplace(pubkey.GetID(), key); ret.emplace_back(std::make_unique(key_exp_index, pubkey, ctx == ParseScriptContext::P2TR)); + ++key_exp_index; return ret; } else { error = "Uncompressed keys are not allowed"; @@ -1944,6 +1947,7 @@ std::vector> ParsePubkeyInner(uint32_t key_exp_i for (auto& path : paths) { ret.emplace_back(std::make_unique(key_exp_index, extpubkey, std::move(path), type, apostrophe)); } + ++key_exp_index; return ret; } @@ -2001,7 +2005,6 @@ std::vector> ParsePubkey(uint32_t& key_exp_index max_multipath_len = std::max(max_multipath_len, pk.size()); providers.emplace_back(std::move(pk)); - key_exp_index++; } if (!any_key_parsed) { error = "musig(): Must contain key expressions"; @@ -2093,6 +2096,7 @@ std::vector> ParsePubkey(uint32_t& key_exp_index // No multipath derivation, MuSigPubkeyProvider uses the first (and only) participant pubkey providers, and the first (and only) path emplace_final_provider(0, 0); } + ++key_exp_index; // Increment key expression index for the MuSigPubkeyProvider too return ret; } @@ -2133,7 +2137,7 @@ std::vector> ParsePubkey(uint32_t& key_exp_index if (providers.empty()) return {}; ret.reserve(providers.size()); for (auto& prov : providers) { - ret.emplace_back(std::make_unique(key_exp_index, info, std::move(prov), apostrophe)); + ret.emplace_back(std::make_unique(prov->m_expr_index, info, std::move(prov), apostrophe)); } return ret; } @@ -2207,7 +2211,6 @@ struct KeyParser { assert(m_out); Key key = m_keys.size(); auto pk = ParsePubkey(m_expr_index, {&*begin, &*end}, ParseContext(), *m_out, m_key_parsing_error); - ++m_expr_index; if (pk.empty()) return {}; m_keys.emplace_back(std::move(pk)); return key; @@ -2279,7 +2282,6 @@ std::vector> ParseScript(uint32_t& key_exp_index error = strprintf("pk(): %s", error); return {}; } - ++key_exp_index; for (auto& pubkey : pubkeys) { ret.emplace_back(std::make_unique(std::move(pubkey), ctx == ParseScriptContext::P2TR)); } @@ -2291,7 +2293,6 @@ std::vector> ParseScript(uint32_t& key_exp_index error = strprintf("pkh(): %s", error); return {}; } - ++key_exp_index; for (auto& pubkey : pubkeys) { ret.emplace_back(std::make_unique(std::move(pubkey))); } @@ -2303,7 +2304,6 @@ std::vector> ParseScript(uint32_t& key_exp_index error = strprintf("combo(): %s", error); return {}; } - ++key_exp_index; for (auto& pubkey : pubkeys) { ret.emplace_back(std::make_unique(std::move(pubkey))); } @@ -2343,7 +2343,6 @@ std::vector> ParseScript(uint32_t& key_exp_index script_size += pks.at(0)->GetSize() + 1; max_providers_len = std::max(max_providers_len, pks.size()); providers.emplace_back(std::move(pks)); - key_exp_index++; } if ((multi || sortedmulti) && (providers.empty() || providers.size() > MAX_PUBKEYS_PER_MULTISIG)) { error = strprintf("Cannot have %u keys in multisig; must have between 1 and %d keys, inclusive", providers.size(), MAX_PUBKEYS_PER_MULTISIG); @@ -2413,7 +2412,6 @@ std::vector> ParseScript(uint32_t& key_exp_index error = strprintf("wpkh(): %s", error); return {}; } - key_exp_index++; for (auto& pubkey : pubkeys) { ret.emplace_back(std::make_unique(std::move(pubkey))); } @@ -2466,7 +2464,6 @@ std::vector> ParseScript(uint32_t& key_exp_index return {}; } size_t max_providers_len = internal_keys.size(); - ++key_exp_index; std::vector>> subscripts; //!< list of multipath expanded script subexpressions std::vector depths; //!< depth in the tree of each subexpression (same length subscripts) if (expr.size()) { @@ -2570,7 +2567,6 @@ std::vector> ParseScript(uint32_t& key_exp_index error = strprintf("rawtr(): %s", error); return {}; } - ++key_exp_index; for (auto& pubkey : output_keys) { ret.emplace_back(std::make_unique(std::move(pubkey))); } From ec0f47b15cb3269015523e6fab8ae9241f4181a1 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 22 Dec 2025 13:30:24 -0800 Subject: [PATCH 4/5] miniscript: Using Func and Expr when parsing keys, hashes, and locktimes Since pk(), pk_k(), pkh(), pk_h(), sha256(), ripemd160(), hash256(), hash160(), after(), and older() all are single argument expressions that are parsed immediately, we can use the Expr and Func parsing functions to determine what the arguments of these expressions are, rather than searching for the next closing parentheses. This fixes an issue when pk(), pk_k(), pkh(), and pk_h() include a musig() expression as Expr properly handles nested expressions. --- src/script/descriptor.cpp | 4 +- src/script/miniscript.h | 125 ++++++++++++++-------------------- src/test/descriptor_tests.cpp | 2 +- src/test/fuzz/miniscript.cpp | 7 +- src/test/miniscript_tests.cpp | 5 +- 5 files changed, 61 insertions(+), 82 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index f761757f3f5..0f14672afa6 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -2206,11 +2206,11 @@ struct KeyParser { assert(false); } - template std::optional FromString(I begin, I end) const + std::optional FromString(std::span& in) const { assert(m_out); Key key = m_keys.size(); - auto pk = ParsePubkey(m_expr_index, {&*begin, &*end}, ParseContext(), *m_out, m_key_parsing_error); + auto pk = ParsePubkey(m_expr_index, in, ParseContext(), *m_out, m_key_parsing_error); if (pk.empty()) return {}; m_keys.emplace_back(std::move(pk)); return key; diff --git a/src/script/miniscript.h b/src/script/miniscript.h index 1b7e84f471d..8733d347136 100644 --- a/src/script/miniscript.h +++ b/src/script/miniscript.h @@ -1806,29 +1806,27 @@ enum class ParseContext { int FindNextChar(std::span in, char m); -/** Parse a key string ending at the end of the fragment's text representation. */ +/** Parse a key expression fully contained within a fragment with the name given by 'func' */ template -std::optional> ParseKeyEnd(std::span in, const Ctx& ctx) +std::optional ParseKey(const std::string& func, std::span& in, const Ctx& ctx) { - int key_size = FindNextChar(in, ')'); - if (key_size < 1) return {}; - auto key = ctx.FromString(in.begin(), in.begin() + key_size); - if (!key) return {}; - return {{std::move(*key), key_size}}; + std::span expr = script::Expr(in); + if (!script::Func(func, expr)) return {}; + return ctx.FromString(expr); } -/** Parse a hex string ending at the end of the fragment's text representation. */ +/** Parse a hex string fully contained within a fragment with the name given by 'func' */ template -std::optional, int>> ParseHexStrEnd(std::span in, const size_t expected_size, +std::optional> ParseHexStr(const std::string& func, std::span& in, const size_t expected_size, const Ctx& ctx) { - int hash_size = FindNextChar(in, ')'); - if (hash_size < 1) return {}; - std::string val = std::string(in.begin(), in.begin() + hash_size); + std::span expr = script::Expr(in); + if (!script::Func(func, expr)) return {}; + std::string val = std::string(expr.begin(), expr.end()); if (!IsHex(val)) return {}; auto hash = ParseHex(val); if (hash.size() != expected_size) return {}; - return {{std::move(hash), hash_size}}; + return hash; } /** BuildBack pops the last two elements off `constructed` and wraps them in the specified Fragment */ @@ -1891,7 +1889,8 @@ inline std::optional> Parse(std::span in, const Ctx& ctx) next_comma = FindNextChar(in, ','); int key_length = (next_comma == -1) ? FindNextChar(in, ')') : next_comma; if (key_length < 1) return false; - auto key = ctx.FromString(in.begin(), in.begin() + key_length); + std::span sp{in.begin(), in.begin() + key_length}; + auto key = ctx.FromString(sp); if (!key) return false; keys.push_back(std::move(*key)); in = in.subspan(key_length + 1); @@ -1978,77 +1977,59 @@ inline std::optional> Parse(std::span in, const Ctx& ctx) constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_0); } else if (Const("1", in)) { constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_1); - } else if (Const("pk(", in)) { - auto res = ParseKeyEnd(in, ctx); - if (!res) return {}; - auto& [key, key_size] = *res; - constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_C, Vector(Node(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_K, Vector(std::move(key))))); - in = in.subspan(key_size + 1); + } else if (Const("pk(", in, /*skip=*/false)) { + std::optional key = ParseKey("pk", in, ctx); + if (!key) return {}; + constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_C, Vector(Node(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_K, Vector(std::move(*key))))); script_size += IsTapscript(ctx.MsContext()) ? 33 : 34; - } else if (Const("pkh(", in)) { - auto res = ParseKeyEnd(in, ctx); - if (!res) return {}; - auto& [key, key_size] = *res; - constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_C, Vector(Node(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_H, Vector(std::move(key))))); - in = in.subspan(key_size + 1); + } else if (Const("pkh(", in, /*skip=*/false)) { + std::optional key = ParseKey("pkh", in, ctx); + if (!key) return {}; + constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_C, Vector(Node(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_H, Vector(std::move(*key))))); script_size += 24; - } else if (Const("pk_k(", in)) { - auto res = ParseKeyEnd(in, ctx); - if (!res) return {}; - auto& [key, key_size] = *res; - constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_K, Vector(std::move(key))); - in = in.subspan(key_size + 1); + } else if (Const("pk_k(", in, /*skip=*/false)) { + std::optional key = ParseKey("pk_k", in, ctx); + if (!key) return {}; + constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_K, Vector(std::move(*key))); script_size += IsTapscript(ctx.MsContext()) ? 32 : 33; - } else if (Const("pk_h(", in)) { - auto res = ParseKeyEnd(in, ctx); - if (!res) return {}; - auto& [key, key_size] = *res; - constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_H, Vector(std::move(key))); - in = in.subspan(key_size + 1); + } else if (Const("pk_h(", in, /*skip=*/false)) { + std::optional key = ParseKey("pk_h", in, ctx); + if (!key) return {}; + constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_H, Vector(std::move(*key))); script_size += 23; - } else if (Const("sha256(", in)) { - auto res = ParseHexStrEnd(in, 32, ctx); - if (!res) return {}; - auto& [hash, hash_size] = *res; - constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::SHA256, std::move(hash)); - in = in.subspan(hash_size + 1); + } else if (Const("sha256(", in, /*skip=*/false)) { + std::optional> hash = ParseHexStr("sha256", in, 32, ctx); + if (!hash) return {}; + constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::SHA256, std::move(*hash)); script_size += 38; - } else if (Const("ripemd160(", in)) { - auto res = ParseHexStrEnd(in, 20, ctx); - if (!res) return {}; - auto& [hash, hash_size] = *res; - constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::RIPEMD160, std::move(hash)); - in = in.subspan(hash_size + 1); + } else if (Const("ripemd160(", in, /*skip=*/false)) { + std::optional> hash = ParseHexStr("ripemd160", in, 20, ctx); + if (!hash) return {}; + constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::RIPEMD160, std::move(*hash)); script_size += 26; - } else if (Const("hash256(", in)) { - auto res = ParseHexStrEnd(in, 32, ctx); - if (!res) return {}; - auto& [hash, hash_size] = *res; - constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::HASH256, std::move(hash)); - in = in.subspan(hash_size + 1); + } else if (Const("hash256(", in, /*skip=*/false)) { + std::optional> hash = ParseHexStr("hash256", in, 32, ctx); + if (!hash) return {}; + constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::HASH256, std::move(*hash)); script_size += 38; - } else if (Const("hash160(", in)) { - auto res = ParseHexStrEnd(in, 20, ctx); - if (!res) return {}; - auto& [hash, hash_size] = *res; - constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::HASH160, std::move(hash)); - in = in.subspan(hash_size + 1); + } else if (Const("hash160(", in, /*skip=*/false)) { + std::optional> hash = ParseHexStr("hash160", in, 20, ctx); + if (!hash) return {}; + constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::HASH160, std::move(*hash)); script_size += 26; - } else if (Const("after(", in)) { - int arg_size = FindNextChar(in, ')'); - if (arg_size < 1) return {}; - const auto num{ToIntegral(std::string_view(in.data(), arg_size))}; + } else if (Const("after(", in, /*skip=*/false)) { + auto expr = Expr(in); + if (!Func("after", expr)) return {}; + const auto num{ToIntegral(std::string_view(expr.begin(), expr.end()))}; if (!num.has_value() || *num < 1 || *num >= 0x80000000L) return {}; constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::AFTER, *num); - in = in.subspan(arg_size + 1); script_size += 1 + (*num > 16) + (*num > 0x7f) + (*num > 0x7fff) + (*num > 0x7fffff); - } else if (Const("older(", in)) { - int arg_size = FindNextChar(in, ')'); - if (arg_size < 1) return {}; - const auto num{ToIntegral(std::string_view(in.data(), arg_size))}; + } else if (Const("older(", in, /*skip=*/false)) { + auto expr = Expr(in); + if (!Func("older", expr)) return {}; + const auto num{ToIntegral(std::string_view(expr.begin(), expr.end()))}; if (!num.has_value() || *num < 1 || *num >= 0x80000000L) return {}; constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::OLDER, *num); - in = in.subspan(arg_size + 1); script_size += 1 + (*num > 16) + (*num > 0x7f) + (*num > 0x7fff) + (*num > 0x7fffff); } else if (Const("multi(", in)) { if (!parse_multi_exp(in, /* is_multi_a = */false)) return {}; diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index 00782c37215..a646fc83476 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -1275,7 +1275,7 @@ BOOST_AUTO_TEST_CASE(descriptor_test) CheckUnparsable("tr(musig(xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc/*,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/*)/0)","tr(musig(xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL/*,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/*)/0)", "tr(): musig(): Cannot have ranged participant keys if musig() also has derivation"); // Fuzzer crash test cases - CheckUnparsable("pk(musig(dd}uue/00/)k(", "pk(musig(dd}uue/00/)k(", "Invalid musig() expression"); + CheckUnparsable("pk(musig(dd}uue/00/)k(", "pk(musig(dd}uue/00/)k(", "'pk(musig(dd}uue/00/)k(' is not a valid descriptor function"); CheckUnparsable("tr(musig(tuus(oldepk(gg)ggggfgg)<,z(((((((((((((((((((((st)", "tr(musig(tuus(oldepk(gg)ggggfgg)<,z(((((((((((((((((((((st)","tr(): Too many ')' in musig() expression"); } diff --git a/src/test/fuzz/miniscript.cpp b/src/test/fuzz/miniscript.cpp index d20bf9fbd84..a7934f7e50f 100644 --- a/src/test/fuzz/miniscript.cpp +++ b/src/test/fuzz/miniscript.cpp @@ -154,10 +154,9 @@ struct ParserContext { return {h.begin(), h.end()}; } - template - std::optional FromString(I first, I last) const { - if (last - first != 2) return {}; - auto idx = ParseHex(std::string(first, last)); + std::optional FromString(std::span& in) const { + if (in.size() != 2) return {}; + auto idx = ParseHex(std::string(in.begin(), in.end())); if (idx.size() != 1) return {}; return TEST_DATA.dummy_keys[idx[0]]; } diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp index 757f89b63bc..9a3e9ea0789 100644 --- a/src/test/miniscript_tests.cpp +++ b/src/test/miniscript_tests.cpp @@ -159,9 +159,8 @@ struct KeyConverter { } //! Parse a public key from a range of hex characters. - template - std::optional FromString(I first, I last) const { - auto bytes = ParseHex(std::string(first, last)); + std::optional FromString(std::span& in) const { + auto bytes = ParseHex(std::string(in.begin(), in.end())); Key key{bytes.begin(), bytes.end()}; if (key.IsValid()) return key; return {}; From 4b53cbd69220c1c786bb23a72c0b26a6f78a38f7 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 22 Dec 2025 13:33:54 -0800 Subject: [PATCH 5/5] test: Test for musig() in various miniscript expressions --- test/functional/wallet_musig.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/functional/wallet_musig.py b/test/functional/wallet_musig.py index 97966e6a317..0e3e4377c08 100755 --- a/test/functional/wallet_musig.py +++ b/test/functional/wallet_musig.py @@ -7,6 +7,7 @@ import re from test_framework.descriptors import descsum_create from test_framework.key import H_POINT +from test_framework.script import hash160 from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -221,7 +222,7 @@ class WalletMuSigTest(BitcoinTestFramework): utxo = wallet.listunspent()[0] else: assert_equal(utxo, wallet.listunspent()[0]) - psbt = wallets[0].walletcreatefundedpsbt(outputs=[{self.def_wallet.getnewaddress(): 5}], inputs=[utxo], change_type="bech32m", changePosition=1)["psbt"] + psbt = wallets[0].walletcreatefundedpsbt(outputs=[{self.def_wallet.getnewaddress(): 5}], inputs=[utxo], change_type="bech32m", changePosition=1, locktime=self.nodes[0].getblockcount())["psbt"] dec_psbt = self.nodes[0].decodepsbt(psbt) assert_equal(len(dec_psbt["inputs"]), 1) @@ -261,6 +262,8 @@ class WalletMuSigTest(BitcoinTestFramework): 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]: @@ -287,6 +290,8 @@ class WalletMuSigTest(BitcoinTestFramework): 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]: @@ -328,6 +333,11 @@ class WalletMuSigTest(BitcoinTestFramework): 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(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) + self.test_success_case("tr(H,and(pk_k(musig/*),after(1)))", "tr($H,and_v(vc:pk_k(musig($0,$1,$2)/<0;1>/*),after(1)))", scriptpath=True) + self.test_success_case("tr(H,and(pkh(musig/*),after(1)))", "tr($H,and_v(v:pkh(musig($0,$1,$2)/<0;1>/*),after(1)))", scriptpath=True) + self.test_success_case("tr(H,and(pk_h(musig/*),after(1)))", "tr($H,and_v(vc:pk_h(musig($0,$1,$2)/<0;1>/*),after(1)))", scriptpath=True) + self.test_success_case("tr(H,{and(pk(musig/*),after(1)),and(pk(musig/*),after(1))})", "tr($H,{and_v(v:pk(musig($0,$2)/0/*),after(1)),and_v(v:pk(musig($1,$2)/0/*),after(1))})", scriptpath=True) self.test_failure_case_1("missing participant nonce", "tr(musig($0/<0;1>/*,$1/<1;2>/*,$2/<2;3>/*))") self.test_failure_case_2("insufficient partial signatures", "rawtr(musig($0/<0;1>/*,$1/<1;2>/*,$2/<2;3>/*))")