descriptor: refactor ToPrivateString for providers
What the developer wrote
Authored by Novo
descriptor: refactor ToPrivateString for providers
This commit modifies the Pubkey providers to return the public string
if private data is not available.
This is setup for a future commit to make Descriptor::ToPrivateString
return strings with missing private key information.
Co-authored-by: rkrux <rkrux.connect@gmail.com>
What changed, and why it matters
This commit refactors how Bitcoin Core's descriptor code turns key information into text when private keys are requested but not available. Previously, if any private key was missing, the whole private-string output could be dropped or partially empty. Now the code falls back to including the public version of the missing key instead, and reports whether any private keys were actually included. It is a behavior change in output formatting, not a fix for an active exploit, and the commit message explicitly calls it setup for a later change.
Review the follow-up commit that changes Descriptor::ToPrivateString to ensure callers handle the new 'false but non-empty public string' semantics correctly. Verify that no caller assumes a false return means an empty output, and that wallet or RPC code does not accidentally expose public descriptors as private ones. No immediate patch is required for this commit alone.
Security signals we found
Behavior change in private-key serialization fallback
Potential information disclosure: public key material is now emitted when private data is unavailable, whereas before the output could be cleared
No cryptographic weakness introduced; change is in error-handling/output path
Commit message frames this as preparatory refactor, not a security fix
Evidence from the diff
The change updates PubkeyProvider::ToPrivateString implementations so that when private key material is unavailable they populate the output with the public string and return false, rather than returning false with an empty or incomplete string. OriginPubkeyProvider now propagates the child provider’s success flag while still building the string. ConstPubkeyProvider and BIP32PubkeyProvider fall back to ToString(StringType::PUBLIC) when GetPrivKey/GetExtKey fail. MultiPubkeyProvider now always concatenates the returned tmp string and no longer clears the entire output if no private keys were found. This is a partial refactor: Descriptor::ToPrivateString itself is not yet changed, but the providers now guarantee a usable string even on missing private data.
Changed components
src/script/descriptor.cppPubkeyProvider and subclasses (OriginPubkeyProvider, ConstPubkeyProvider, BIP32PubkeyProvider, MultiPubkeyProvider)Descriptor string serialization (ToPrivateString)Inspect captured patch +16 / −9
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index e2f7b8e8..89af1d93 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -204,7 +204,11 @@ public:
/** Get the descriptor string form. */
virtual std::string ToString(StringType type=StringType::PUBLIC) const = 0;
- /** Get the descriptor string form including private data (if available in arg). */
+ /** Get the descriptor string form including private data (if available in arg).
+ * If the private data is not available, the output string in the "out" parameter
+ * will not contain any private key information,
+ * and this function will return "false".
+ */
virtual bool ToPrivateString(const SigningProvider& arg, std::string& out) const = 0;
/** Get the descriptor string form with the xpub at the last hardened derivation,
@@ -260,9 +264,9 @@ public:
bool ToPrivateString(const SigningProvider& arg, std::string& ret) const override
{
std::string sub;
- if (!m_provider->ToPrivateString(arg, sub)) return false;
+ bool has_priv_key{m_provider->ToPrivateString(arg, sub)};
ret = "[" + OriginString(StringType::PUBLIC) + "]" + std::move(sub);
- return true;
+ return has_priv_key;
}
bool ToNormalizedString(const SigningProvider& arg, std::string& ret, const DescriptorCache* cache) const override
{
@@ -329,7 +333,10 @@ public:
bool ToPrivateString(const SigningProvider& arg, std::string& ret) const override
{
std::optional<CKey> key = GetPrivKey(arg);
- if (!key) return false;
+ if (!key) {
+ ret = ToString(StringType::PUBLIC);
+ return false;
+ }
ret = EncodeSecret(*key);
return true;
}
@@ -492,7 +499,10 @@ public:
bool ToPrivateString(const SigningProvider& arg, std::string& out) const override
{
CExtKey key;
- if (!GetExtKey(arg, key)) return false;
+ if (!GetExtKey(arg, key)) {
+ out = ToString(StringType::PUBLIC);
+ return false;
+ }
out = EncodeExtKey(key) + FormatHDKeypath(m_path, /*apostrophe=*/m_apostrophe);
if (IsRange()) {
out += "/*";
@@ -710,17 +720,14 @@ public:
std::string tmp;
if (pubkey->ToPrivateString(arg, tmp)) {
any_privkeys = true;
- out += tmp;
- } else {
- out += pubkey->ToString();
}
+ out += tmp;
}
out += ")";
out += FormatHDKeypath(m_path);
if (IsRangedDerivation()) {
out += "/*";
}
- if (!any_privkeys) out.clear();
return any_privkeys;
}
bool ToNormalizedString(const SigningProvider& arg, std::string& out, const DescriptorCache* cache = nullptr) const override
Why this scored 21/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.