test: Test that key expression indexes match key count
What changed, and why it matters
This commit adds new internal test-only checks to make sure Bitcoin Core's descriptor parser correctly counts key expressions. It does not change user-facing behavior or fix a known live bug; it is a regression test and small helper-code addition to verify an invariant (every key has a unique index and the count matches).
Treat as a normal test/quality improvement. No urgent action required; review whether the new invariant should also be asserted during descriptor parsing in production code if a real mismatch could ever occur.
Security signals we found
New test-only invariant checks added, not a runtime fix
Changes descriptor key-index accounting code (non-consensus, wallet/script layer)
No validation, networking, or consensus logic modified
No disclosure of vulnerability or CVE referenced in commit
Evidence from the diff
The patch introduces two new pure-virtual methods on the Descriptor interface, GetMaxKeyExpr() and GetKeyCount(), implemented for real descriptors and a stub mock. It also adds a GetKeyCount() virtual on PubkeyProvider (default 1, overridden to 1 + participants.size() for musig()). DescriptorImpl::GetMaxKeyExpr() walks pubkey_args collecting the largest m_expr_index, while GetKeyCount() sums PubkeyProvider::GetKeyCount(). These are exercised in descriptor_tests.cpp and the descriptor_parse fuzz target with BOOST_CHECK/assert that max_key_expr + 1 == key_count. m_expr_index is also changed from protected mutable to public const.
Changed components
src/script/descriptor.cppsrc/script/descriptor.hsrc/test/descriptor_tests.cppsrc/test/fuzz/descriptor_parse.cppsrc/wallet/test/walletload_tests.cppInspect captured patch +63 / −3
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 8558c8e2..2be3301b 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -161,12 +161,11 @@ typedef std::vector<uint32_t> 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<const DescriptorImpl*> 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<const DescriptorImpl*> 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 fe33476d..3ac748a1 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<std::string> 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 8ff44d13..00782c37 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 eeba5f13..6b3084e2 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 ec31cda3..a397ffc0 100644
--- a/src/wallet/test/walletload_tests.cpp
+++ b/src/wallet/test/walletload_tests.cpp
@@ -37,6 +37,8 @@ public:
std::optional<int64_t> MaxSatisfactionElems() const override { return {}; }
void GetPubKeys(std::set<CPubKey>& pubkeys, std::set<CExtPubKey>& ext_pubs) const override {}
std::vector<std::string> 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)
Why this scored 18/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.