What changed, and why it matters
This commit adds a new helper function called HavePrivateKeys() to Bitcoin Core's descriptor system. It is a preparatory refactor: it gives the code a clearer way to check whether a wallet descriptor is 'watch-only' (has no private keys) before an upcoming change to ToPrivateString(). The commit itself does not change user-visible behavior; it only introduces the new check and updates tests to use it. There is no direct evidence in the commit that this fixes an active security bug, but it is clearly related to correctly identifying whether private keys are present, which can affect wallet security logic.
Treat as a low-risk refactor commit. Review the subsequent commits that change ToPrivateString() behavior to ensure the new HavePrivateKeys() check is actually used in all security-relevant watch-only decisions, particularly in wallet import and descriptor wallet loading paths.
Security signals we found
Refactor of private-key presence detection logic
Preparation for semantic change in ToPrivateString()
Watch-only descriptor classification may affect wallet security decisions
No direct vulnerability or exploit mechanism visible in diff
Evidence from the diff
The patch introduces a new pure virtual Descriptor::HavePrivateKeys(const SigningProvider&) method and a concrete implementation in DescriptorImpl. The implementation recursively checks m_subdescriptor_args and verifies each pubkey argument can produce a private key via GetPrivKey(0, arg, tmp_provider). It returns false if the descriptor has no keys/subdescriptors or if any required private key is missing. Tests are updated to assert HavePrivateKeys() behavior, and a mock descriptor in walletload_tests.cpp implements the new interface. The commit message states this is to replace ToPrivateString() for determining watch-only status, because ToPrivateString() semantics will change in later commits.
Changed components
src/script/descriptor.cppsrc/script/descriptor.hsrc/test/descriptor_tests.cppsrc/wallet/test/walletload_tests.cppInspect captured patch +31 / −1
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index a4294331..e2f7b8e8 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -835,6 +835,25 @@ public:
return true;
}
+ // NOLINTNEXTLINE(misc-no-recursion)
+ bool HavePrivateKeys(const SigningProvider& arg) const override
+ {
+ if (m_pubkey_args.empty() && m_subdescriptor_args.empty()) return false;
+
+ for (const auto& sub: m_subdescriptor_args) {
+ if (!sub->HavePrivateKeys(arg)) return false;
+ }
+
+ FlatSigningProvider tmp_provider;
+ for (const auto& pubkey : m_pubkey_args) {
+ tmp_provider.keys.clear();
+ pubkey->GetPrivKey(0, arg, tmp_provider);
+ if (tmp_provider.keys.empty()) return false;
+ }
+
+ return true;
+ }
+
// NOLINTNEXTLINE(misc-no-recursion)
bool IsRange() const final
{
diff --git a/src/script/descriptor.h b/src/script/descriptor.h
index 9a018300..c7863933 100644
--- a/src/script/descriptor.h
+++ b/src/script/descriptor.h
@@ -111,6 +111,13 @@ struct Descriptor {
/** Whether this descriptor will return one scriptPubKey or multiple (aka is or is not combo) */
virtual bool IsSingleType() const = 0;
+ /** Whether the given provider has all private keys required by this descriptor.
+ * @return `false` if the descriptor doesn't have any keys or subdescriptors,
+ * or if the provider does not have all private keys required by
+ * the descriptor.
+ */
+ virtual bool HavePrivateKeys(const SigningProvider& provider) const = 0;
+
/** Convert the descriptor to a private string. This fails if the provided provider does not have the relevant private keys. */
virtual bool ToPrivateString(const SigningProvider& provider, std::string& out) const = 0;
diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp
index 098007c4..17c756b6 100644
--- a/src/test/descriptor_tests.cpp
+++ b/src/test/descriptor_tests.cpp
@@ -49,7 +49,7 @@ constexpr int SIGNABLE = 1 << 3; // We can sign with this descriptor (this is no
constexpr int DERIVE_HARDENED = 1 << 4; // The final derivation is hardened, i.e. ends with *' or *h
constexpr int MIXED_PUBKEYS = 1 << 5;
constexpr int XONLY_KEYS = 1 << 6; // X-only pubkeys are in use (and thus inferring/caching may swap parity of pubkeys/keyids)
-constexpr int MISSING_PRIVKEYS = 1 << 7; // Not all private keys are available, so ToPrivateString will fail.
+constexpr int MISSING_PRIVKEYS = 1 << 7; // Not all private keys are available. ToPrivateString() will fail and HavePrivateKeys() will return `false`.
constexpr int SIGNABLE_FAILS = 1 << 8; // We can sign with this descriptor, but actually trying to sign will fail
constexpr int MUSIG = 1 << 9; // This is a MuSig so key counts will have an extra key
constexpr int MUSIG_DERIVATION = 1 << 10; // MuSig with BIP 328 derivation from the aggregate key
@@ -243,6 +243,9 @@ void DoCheck(std::string prv, std::string pub, const std::string& norm_pub, int
} else {
BOOST_CHECK_MESSAGE(EqualDescriptor(prv, prv1), "Private ser: " + prv1 + " Private desc: " + prv);
}
+ BOOST_CHECK(!parse_priv->HavePrivateKeys(keys_pub));
+ BOOST_CHECK(parse_pub->HavePrivateKeys(keys_priv));
+
BOOST_CHECK(!parse_priv->ToPrivateString(keys_pub, prv1));
BOOST_CHECK(parse_pub->ToPrivateString(keys_priv, prv1));
if (expected_prv) {
diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp
index 0c69849d..adbd9aa1 100644
--- a/src/wallet/test/walletload_tests.cpp
+++ b/src/wallet/test/walletload_tests.cpp
@@ -26,6 +26,7 @@ public:
bool IsRange() const override { return false; }
bool IsSolvable() const override { return false; }
bool IsSingleType() const override { return true; }
+ bool HavePrivateKeys(const SigningProvider&) const override { return false; }
bool ToPrivateString(const SigningProvider& provider, std::string& out) const override { return false; }
bool ToNormalizedString(const SigningProvider& provider, std::string& out, const DescriptorCache* cache = nullptr) const override { return false; }
bool Expand(int pos, const SigningProvider& provider, std::vector<CScript>& output_scripts, FlatSigningProvider& out, DescriptorCache* write_cache = nullptr) const override { return false; };
Why this scored 28/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.