descriptor: ToPrivateString() pass if at least 1 priv key exists
What changed, and why it matters
This Bitcoin Core change alters how wallet descriptors are printed when some (but not all) private keys are present. Previously, exporting a private descriptor would fail entirely if any single key was missing. Now it succeeds and exports the private keys it does have, falling back to public keys for the rest. The commit message and code comments frame this as a usability improvement for multi-key descriptors such as taproot (tr()). There is no direct evidence in the commit of a security vulnerability being fixed; it appears to be a behavior change that could affect information disclosure expectations.
Treat as a functional/behavior change rather than a critical security patch. Review whether downstream consumers of ToPrivateString (e.g., listdescriptors RPC, backup tools) assume all-or-nothing private output and could now leak partial private key material unexpectedly. Consider adding release-note documentation and verifying that callers check the boolean return value before treating the output as fully private.
Security signals we found
Behavior change in private-key export: partial private disclosure instead of all-or-nothing
Updated API contract in descriptor.h documents fallback to public keys and false return only when no private keys exist
Test flag MISSING_PRIVKEYS comment changed to reflect new ToPrivateString semantics
No explicit security bug, CVE, or vulnerability fix described in commit message or code
Evidence from the diff
The patch refactors Descriptor::ToPrivateString() and related helpers so that private-string generation returns true if at least one private key is available, rather than requiring every key. It threads a has_priv_key boolean through miniscript ToString() and descriptor subscript helpers, accumulates success with OR logic, and falls back to public keys for missing private keys. The header comment is updated to document the new contract. Tests are adjusted to expect partial private output when some keys are present. No memory-safety bug, crash, or cryptographic flaw is visible in the diff.
Changed components
src/script/descriptor.cppsrc/script/descriptor.hsrc/script/miniscript.hsrc/test/descriptor_tests.cppsrc/test/fuzz/miniscript.cppsrc/test/miniscript_tests.cppInspect captured patch +90 / −30
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 89af1d93..89766b52 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -877,13 +877,19 @@ public:
virtual bool ToStringSubScriptHelper(const SigningProvider* arg, std::string& ret, const StringType type, const DescriptorCache* cache = nullptr) const
{
size_t pos = 0;
+ bool is_private{type == StringType::PRIVATE};
+ // For private string output, track if at least one key has a private key available.
+ // Initialize to true for non-private types.
+ bool any_success{!is_private};
for (const auto& scriptarg : m_subdescriptor_args) {
if (pos++) ret += ",";
std::string tmp;
- if (!scriptarg->ToStringHelper(arg, tmp, type, cache)) return false;
+ bool subscript_res{scriptarg->ToStringHelper(arg, tmp, type, cache)};
+ if (!is_private && !subscript_res) return false;
+ any_success = any_success || subscript_res;
ret += tmp;
}
- return true;
+ return any_success;
}
// NOLINTNEXTLINE(misc-no-recursion)
@@ -892,6 +898,11 @@ public:
std::string extra = ToStringExtra();
size_t pos = extra.size() > 0 ? 1 : 0;
std::string ret = m_name + "(" + extra;
+ bool is_private{type == StringType::PRIVATE};
+ // For private string output, track if at least one key has a private key available.
+ // Initialize to true for non-private types.
+ bool any_success{!is_private};
+
for (const auto& pubkey : m_pubkey_args) {
if (pos++) ret += ",";
std::string tmp;
@@ -900,7 +911,7 @@ public:
if (!pubkey->ToNormalizedString(*arg, tmp, cache)) return false;
break;
case StringType::PRIVATE:
- if (!pubkey->ToPrivateString(*arg, tmp)) return false;
+ any_success = pubkey->ToPrivateString(*arg, tmp) || any_success;
break;
case StringType::PUBLIC:
tmp = pubkey->ToString();
@@ -912,10 +923,12 @@ public:
ret += tmp;
}
std::string subscript;
- if (!ToStringSubScriptHelper(arg, subscript, type, cache)) return false;
+ bool subscript_res{ToStringSubScriptHelper(arg, subscript, type, cache)};
+ if (!is_private && !subscript_res) return false;
+ any_success = any_success || subscript_res;
if (pos && subscript.size()) ret += ',';
out = std::move(ret) + std::move(subscript) + ")";
- return true;
+ return any_success;
}
std::string ToString(bool compat_format) const final
@@ -927,9 +940,9 @@ public:
bool ToPrivateString(const SigningProvider& arg, std::string& out) const override
{
- bool ret = ToStringHelper(&arg, out, StringType::PRIVATE);
+ bool has_priv_key{ToStringHelper(&arg, out, StringType::PRIVATE)};
out = AddChecksum(out);
- return ret;
+ return has_priv_key;
}
bool ToNormalizedString(const SigningProvider& arg, std::string& out, const DescriptorCache* cache) const override final
@@ -1410,8 +1423,20 @@ protected:
}
bool ToStringSubScriptHelper(const SigningProvider* arg, std::string& ret, const StringType type, const DescriptorCache* cache = nullptr) const override
{
- if (m_depths.empty()) return true;
+ if (m_depths.empty()) {
+ // If there are no sub-descriptors and a PRIVATE string
+ // is requested, return `false` to indicate that the presence
+ // of a private key depends solely on the internal key (which is checked
+ // in the caller), not on any sub-descriptor. This ensures correct behavior for
+ // descriptors like tr(internal_key) when checking for private keys.
+ return type != StringType::PRIVATE;
+ }
std::vector<bool> path;
+ bool is_private{type == StringType::PRIVATE};
+ // For private string output, track if at least one key has a private key available.
+ // Initialize to true for non-private types.
+ bool any_success{!is_private};
+
for (size_t pos = 0; pos < m_depths.size(); ++pos) {
if (pos) ret += ',';
while ((int)path.size() <= m_depths[pos]) {
@@ -1419,7 +1444,9 @@ protected:
path.push_back(false);
}
std::string tmp;
- if (!m_subdescriptor_args[pos]->ToStringHelper(arg, tmp, type, cache)) return false;
+ bool subscript_res{m_subdescriptor_args[pos]->ToStringHelper(arg, tmp, type, cache)};
+ if (!is_private && !subscript_res) return false;
+ any_success = any_success || subscript_res;
ret += tmp;
while (!path.empty() && path.back()) {
if (path.size() > 1) ret += '}';
@@ -1427,7 +1454,7 @@ protected:
}
if (!path.empty()) path.back() = true;
}
- return true;
+ return any_success;
}
public:
TRDescriptor(std::unique_ptr<PubkeyProvider> internal_key, std::vector<std::unique_ptr<DescriptorImpl>> descs, std::vector<int> depths) :
@@ -1520,15 +1547,16 @@ public:
const DescriptorCache* cache LIFETIMEBOUND)
: m_arg(arg), m_pubkeys(pubkeys), m_type(type), m_cache(cache) {}
- std::optional<std::string> ToString(uint32_t key) const
+ std::optional<std::string> ToString(uint32_t key, bool& has_priv_key) const
{
std::string ret;
+ has_priv_key = false;
switch (m_type) {
case DescriptorImpl::StringType::PUBLIC:
ret = m_pubkeys[key]->ToString();
break;
case DescriptorImpl::StringType::PRIVATE:
- if (!m_pubkeys[key]->ToPrivateString(*m_arg, ret)) return {};
+ has_priv_key = m_pubkeys[key]->ToPrivateString(*m_arg, ret);
break;
case DescriptorImpl::StringType::NORMALIZED:
if (!m_pubkeys[key]->ToNormalizedString(*m_arg, ret, m_cache)) return {};
@@ -1568,11 +1596,15 @@ public:
bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type,
const DescriptorCache* cache = nullptr) const override
{
- if (const auto res = m_node->ToString(StringMaker(arg, m_pubkey_args, type, cache))) {
- out = *res;
- return true;
+ bool has_priv_key{false};
+ auto res = m_node->ToString(StringMaker(arg, m_pubkey_args, type, cache), has_priv_key);
+ if (res) out = *res;
+ if (type == StringType::PRIVATE) {
+ Assume(res.has_value());
+ return has_priv_key;
+ } else {
+ return res.has_value();
}
- return false;
}
bool IsSolvable() const override { return true; }
@@ -2110,7 +2142,7 @@ struct KeyParser {
return key;
}
- std::optional<std::string> ToString(const Key& key) const
+ std::optional<std::string> ToString(const Key& key, bool&) const
{
return m_keys.at(key).at(0)->ToString();
}
@@ -2507,7 +2539,7 @@ std::vector<std::unique_ptr<DescriptorImpl>> ParseScript(uint32_t& key_exp_index
// Try to find the first insane sub for better error reporting.
auto insane_node = node.get();
if (const auto sub = node->FindInsaneSub()) insane_node = sub;
- if (const auto str = insane_node->ToString(parser)) error = *str;
+ error = *insane_node->ToString(parser);
if (!insane_node->IsValid()) {
error += " is invalid";
} else if (!node->IsSane()) {
diff --git a/src/script/descriptor.h b/src/script/descriptor.h
index c7863933..47c56885 100644
--- a/src/script/descriptor.h
+++ b/src/script/descriptor.h
@@ -118,7 +118,13 @@ struct 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. */
+ /** Convert the descriptor to a private string. This uses public keys if the relevant private keys are not in the SigningProvider.
+ * If none of the relevant private keys are available, the output string in the "out" parameter will not contain any private key information,
+ * and this function will return "false".
+ * @param[in] provider The SigningProvider to query for private keys.
+ * @param[out] out The resulting descriptor string, containing private keys if available.
+ * @returns true if at least one private key available.
+ */
virtual bool ToPrivateString(const SigningProvider& provider, std::string& out) const = 0;
/** Convert the descriptor to a normalized string. Normalized descriptors have the xpub at the last hardened step. This fails if the provided provider does not have the private keys to derive that xpub. */
diff --git a/src/script/miniscript.h b/src/script/miniscript.h
index 54ae777c..6e684e6d 100644
--- a/src/script/miniscript.h
+++ b/src/script/miniscript.h
@@ -826,6 +826,12 @@ public:
template<typename CTx>
std::optional<std::string> ToString(const CTx& ctx) const {
+ bool dummy{false};
+ return ToString(ctx, dummy);
+ }
+
+ template<typename CTx>
+ std::optional<std::string> ToString(const CTx& ctx, bool& has_priv_key) const {
// To construct the std::string representation for a Miniscript object, we use
// the TreeEvalMaybe algorithm. The State is a boolean: whether the parent node is a
// wrapper. If so, non-wrapper expressions must be prefixed with a ":".
@@ -838,10 +844,16 @@ public:
(node.fragment == Fragment::OR_I && node.subs[0]->fragment == Fragment::JUST_0) ||
(node.fragment == Fragment::OR_I && node.subs[1]->fragment == Fragment::JUST_0));
};
+ auto toString = [&ctx, &has_priv_key](Key key) -> std::optional<std::string> {
+ bool fragment_has_priv_key{false};
+ auto key_str{ctx.ToString(key, fragment_has_priv_key)};
+ if (key_str) has_priv_key = has_priv_key || fragment_has_priv_key;
+ return key_str;
+ };
// The upward function computes for a node, given whether its parent is a wrapper,
// and the string representations of its child nodes, the string representation of the node.
const bool is_tapscript{IsTapscript(m_script_ctx)};
- auto upfn = [&ctx, is_tapscript](bool wrapped, const Node& node, std::span<std::string> subs) -> std::optional<std::string> {
+ auto upfn = [is_tapscript, &toString](bool wrapped, const Node& node, std::span<std::string> subs) -> std::optional<std::string> {
std::string ret = wrapped ? ":" : "";
switch (node.fragment) {
@@ -850,13 +862,13 @@ public:
case Fragment::WRAP_C:
if (node.subs[0]->fragment == Fragment::PK_K) {
// pk(K) is syntactic sugar for c:pk_k(K)
- auto key_str = ctx.ToString(node.subs[0]->keys[0]);
+ auto key_str = toString(node.subs[0]->keys[0]);
if (!key_str) return {};
return std::move(ret) + "pk(" + std::move(*key_str) + ")";
}
if (node.subs[0]->fragment == Fragment::PK_H) {
// pkh(K) is syntactic sugar for c:pk_h(K)
- auto key_str = ctx.ToString(node.subs[0]->keys[0]);
+ auto key_str = toString(node.subs[0]->keys[0]);
if (!key_str) return {};
return std::move(ret) + "pkh(" + std::move(*key_str) + ")";
}
@@ -877,12 +889,12 @@ public:
}
switch (node.fragment) {
case Fragment::PK_K: {
- auto key_str = ctx.ToString(node.keys[0]);
+ auto key_str = toString(node.keys[0]);
if (!key_str) return {};
return std::move(ret) + "pk_k(" + std::move(*key_str) + ")";
}
case Fragment::PK_H: {
- auto key_str = ctx.ToString(node.keys[0]);
+ auto key_str = toString(node.keys[0]);
if (!key_str) return {};
return std::move(ret) + "pk_h(" + std::move(*key_str) + ")";
}
@@ -908,7 +920,7 @@ public:
CHECK_NONFATAL(!is_tapscript);
auto str = std::move(ret) + "multi(" + util::ToString(node.k);
for (const auto& key : node.keys) {
- auto key_str = ctx.ToString(key);
+ auto key_str = toString(key);
if (!key_str) return {};
str += "," + std::move(*key_str);
}
@@ -918,7 +930,7 @@ public:
CHECK_NONFATAL(is_tapscript);
auto str = std::move(ret) + "multi_a(" + util::ToString(node.k);
for (const auto& key : node.keys) {
- auto key_str = ctx.ToString(key);
+ auto key_str = toString(key);
if (!key_str) return {};
str += "," + std::move(*key_str);
}
diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp
index 17c756b6..d49a4885 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. ToPrivateString() will fail and HavePrivateKeys() will return `false`.
+constexpr int MISSING_PRIVKEYS = 1 << 7; // Not all private keys are available. ToPrivateString() will return true if there is at least one private key 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
@@ -264,6 +264,12 @@ void DoCheck(std::string prv, std::string pub, const std::string& norm_pub, int
parse_pub->ExpandPrivate(0, keys_priv, pub_prov);
BOOST_CHECK_MESSAGE(EqualSigningProviders(priv_prov, pub_prov), "Private desc: " + prv + " Pub desc: " + pub);
+ } else if (keys_priv.keys.size() > 0) {
+ // If there is at least one private key, ToPrivateString() should return true and include that key
+ std::string prv_str;
+ BOOST_CHECK(parse_priv->ToPrivateString(keys_priv, prv_str));
+ size_t checksum_len = 9; // Including the '#' character
+ BOOST_CHECK_MESSAGE(prv == prv_str.substr(0, prv_str.length() - checksum_len), prv);
}
// Check that private can produce the normalized descriptors
diff --git a/src/test/fuzz/miniscript.cpp b/src/test/fuzz/miniscript.cpp
index 42195313..edc9772a 100644
--- a/src/test/fuzz/miniscript.cpp
+++ b/src/test/fuzz/miniscript.cpp
@@ -124,10 +124,14 @@ struct ParserContext {
return a < b;
}
- std::optional<std::string> ToString(const Key& key) const
+ std::optional<std::string> ToString(const Key& key, bool& has_priv_key) const
{
+ has_priv_key = false;
auto it = TEST_DATA.dummy_key_idx_map.find(key);
- if (it == TEST_DATA.dummy_key_idx_map.end()) return {};
+ if (it == TEST_DATA.dummy_key_idx_map.end()) {
+ return HexStr(key);
+ }
+ has_priv_key = true;
uint8_t idx = it->second;
return HexStr(std::span{&idx, 1});
}
diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp
index 4d31968c..9f7d4b7e 100644
--- a/src/test/miniscript_tests.cpp
+++ b/src/test/miniscript_tests.cpp
@@ -188,7 +188,7 @@ struct KeyConverter {
return g_testdata->pkmap.at(keyid);
}
- std::optional<std::string> ToString(const Key& key) const {
+ std::optional<std::string> ToString(const Key& key, bool&) const {
return HexStr(ToPKBytes(key));
}
Why this scored 23/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.