Merge remote-tracking branch 'Eunovo/list-descriptors-with-partial-keys' into 2025/06/musig2-power

Author: Sjors

src/script/descriptor.cpp

@@ -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,
@@ -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
     {
@@ -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;
     }
@@ -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 += "/*";
@@ -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
@@ -837,6 +844,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
     {
@@ -853,13 +879,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)
@@ -868,6 +900,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;
@@ -876,7 +913,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();
@@ -888,10 +925,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
@@ -903,9 +942,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
@@ -1398,24 +1437,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) :
@@ -1508,15 +1561,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 {};
@@ -1573,11 +1627,10 @@ 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;
-        }
-        return false;
+        bool has_priv_key{false};
+        auto res = m_node->ToString(StringMaker(arg, m_pubkey_args, type, cache), has_priv_key);
+        if (res) out = *res;
+        return type == StringType::PRIVATE ? has_priv_key : res.has_value();
     }
 
     bool IsSolvable() const override { return true; }
@@ -2132,7 +2185,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();
     }
@@ -2550,7 +2603,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()) {

src/script/descriptor.h

@@ -111,7 +111,20 @@ struct Descriptor {
     /** Whether this descriptor will return at most 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. */

src/script/miniscript.h

@@ -843,6 +843,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 ":".
@@ -855,10 +861,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) {
@@ -867,13 +879,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) + ")";
                     }
@@ -894,12 +906,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) + ")";
                 }
@@ -925,7 +937,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);
                     }
@@ -935,7 +947,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);
                     }

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 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
@@ -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) {
@@ -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