Skip to content

wip: fix has_priv_key in fuzzer and add coverage #108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Sjors wants to merge 8 commits into
base: master
Choose a base branch
from
Open

wip: fix has_priv_key in fuzzer and add coverage #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e842eb9
descriptors: add HavePrivateKeys()
Eunovo Jul 22, 2025
2dc74e3
wallet/migration: use HavePrivateKeys in place of ToPrivateString
Eunovo May 13, 2025
5c4db25
descriptor: refactor ToPrivateString for providers
Eunovo Aug 1, 2025
9e5e982
descriptor: ToPrivateString() pass if at least 1 priv key exists
Eunovo Aug 1, 2025
ed945a6
walletrpc: reject listdes with priv key on w-only wallets
Eunovo May 12, 2025
9c7e477
test: Test listdescs with priv works even with missing priv keys
Eunovo May 12, 2025
b3026b1
fuzz: add HasPrivKey miniscript helper
Sjors Jan 6, 2026
fa02243
fuzz: verify has_priv_key
Sjors Jan 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 85 additions & 27 deletions src/script/descriptor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,11 @@ struct PubkeyProvider
/** 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,
Expand Down Expand Up @@ -260,9 +264,9 @@ class OriginPubkeyProvider final : public PubkeyProvider
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
{
Expand Down Expand Up @@ -329,7 +333,10 @@ class ConstPubkeyProvider final : public PubkeyProvider
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;
}
Expand Down Expand Up @@ -492,7 +499,10 @@ class BIP32PubkeyProvider final : public PubkeyProvider
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 += "/*";
Expand Down Expand Up @@ -710,17 +720,14 @@ class MuSigPubkeyProvider final : public PubkeyProvider
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
Expand Down Expand Up @@ -835,6 +842,25 @@ class DescriptorImpl : public Descriptor
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
{
Expand All @@ -851,13 +877,19 @@ class DescriptorImpl : public Descriptor
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)
Expand All @@ -866,6 +898,11 @@ class DescriptorImpl : public Descriptor
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;
Expand All @@ -874,7 +911,7 @@ class DescriptorImpl : public Descriptor
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();
Expand All @@ -886,10 +923,12 @@ class DescriptorImpl : public Descriptor
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
Expand All @@ -901,9 +940,9 @@ class DescriptorImpl : public Descriptor

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
Expand Down Expand Up @@ -1384,24 +1423,38 @@ class TRDescriptor final : public DescriptorImpl
}
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]) {
if (path.size()) ret += '{';
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 += '}';
path.pop_back();
}
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) :
Expand Down Expand Up @@ -1494,15 +1547,16 @@ class StringMaker {
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 {};
Expand Down Expand Up @@ -1542,11 +1596,15 @@ class MiniscriptDescriptor final : public DescriptorImpl
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; }
Expand Down Expand Up @@ -2084,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();
}
Expand Down Expand Up @@ -2481,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()) {
Expand Down
15 changes: 14 additions & 1 deletion src/script/descriptor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,20 @@ struct Descriptor {
/** Whether this descriptor will return one scriptPubKey or multiple (aka is or is not combo) */
virtual bool IsSingleType() const = 0;

/** Convert the descriptor to a private string. This fails if the provided provider does not have the relevant private keys. */
/** 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 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. */
Expand Down
26 changes: 19 additions & 7 deletions src/script/miniscript.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,12 @@ struct Node {

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 ":".
Expand All @@ -838,10 +844,16 @@ struct Node {
(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) {
Expand All @@ -850,13 +862,13 @@ struct Node {
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) + ")";
}
Expand All @@ -877,12 +889,12 @@ struct Node {
}
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) + ")";
}
Expand All @@ -908,7 +920,7 @@ struct Node {
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);
}
Expand All @@ -918,7 +930,7 @@ struct Node {
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);
}
Expand Down
11 changes: 10 additions & 1 deletion src/test/descriptor_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 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
Expand Down Expand Up @@ -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) {
Expand All @@ -261,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
Expand Down
Loading
Loading