Merge bitcoin/bitcoin#24148: Miniscript support in Output Descriptors

ffc79b8 qa: functional test Miniscript watchonly support (Antoine Poinsot)
bfb0367 Miniscript support in output descriptors (Antoine Poinsot)
4a08288 qa: better error reporting on descriptor parsing error (Antoine Poinsot)
d25d58b miniscript: add a helper to find the first insane sub with no child (Antoine Poinsot)
c38c7c5 miniscript: don't check for top level validity at parsing time (Antoine Poinsot)

Pull request description:

  This adds Miniscript support for Output Descriptors without any signing logic (yet). See the OP of #24147 for a description of Miniscript and a rationale of having it in Bitcoin Core.
  On its own, this PR adds "watchonly" support for Miniscript descriptors in the descriptor wallet. A follow-up adds signing support.

  A minified corpus of Miniscript Descriptors for the `descriptor_parse` fuzz target is available at bitcoin-core/qa-assets#92.
  The Miniscript descriptors used in the unit tests here and in #24149 were cross-tested against the Rust implementation at https://github.com/rust-bitcoin/rust-miniscript.

  This PR contains code and insights from Pieter Wuille.

ACKs for top commit:
  Sjors:
    re-utACK ffc79b8
  achow101:
    ACK ffc79b8
  w0xlt:
    reACK bitcoin/bitcoin@ffc79b8

Tree-SHA512: 02d919d38bb626d3c557eca3680ce71117739fa161b7a92cfdb6c9c432ed88870b1ed127ba24248574c40c7428217d7e9bdd986fd8cd7c51fae8c776e1271fb9

Author: achow101

src/script/descriptor.cpp

@@ -6,6 +6,7 @@
 
 #include <key_io.h>
 #include <pubkey.h>
+#include <script/miniscript.h>
 #include <script/script.h>
 #include <script/standard.h>
 
@@ -161,6 +162,20 @@ struct PubkeyProvider
 
     virtual ~PubkeyProvider() = default;
 
+    /** Compare two public keys represented by this provider.
+     * Used by the Miniscript descriptors to check for duplicate keys in the script.
+     */
+    bool operator<(PubkeyProvider& other) const {
+        CPubKey a, b;
+        SigningProvider dummy;
+        KeyOriginInfo dummy_info;
+
+        GetPubKey(0, dummy, a, dummy_info);
+        other.GetPubKey(0, dummy, b, dummy_info);
+
+        return a < b;
+    }
+
     /** Derive a public key.
      *  read_cache is the cache to read keys from (if not nullptr)
      *  write_cache is the cache to write keys to (if not nullptr)
@@ -493,12 +508,12 @@ class BIP32PubkeyProvider final : public PubkeyProvider
 /** Base class for all Descriptor implementations. */
 class DescriptorImpl : public Descriptor
 {
-    //! Public key arguments for this descriptor (size 1 for PK, PKH, WPKH; any size for Multisig).
+protected:
+    //! Public key arguments for this descriptor (size 1 for PK, PKH, WPKH; any size for WSH and Multisig).
     const std::vector<std::unique_ptr<PubkeyProvider>> m_pubkey_args;
     //! The string name of the descriptor function.
     const std::string m_name;
 
-protected:
     //! The sub-descriptor arguments (empty for everything but SH and WSH).
     //! In doc/descriptors.m this is referred to as SCRIPT expressions sh(SCRIPT)
     //! and wsh(SCRIPT), and distinct from KEY expressions and ADDR expressions.
@@ -563,7 +578,7 @@ class DescriptorImpl : public Descriptor
         return true;
     }
 
-    bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type, const DescriptorCache* cache = nullptr) const
+    virtual bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type, const DescriptorCache* cache = nullptr) const
     {
         std::string extra = ToStringExtra();
         size_t pos = extra.size() > 0 ? 1 : 0;
@@ -917,6 +932,89 @@ class TRDescriptor final : public DescriptorImpl
     bool IsSingleType() const final { return true; }
 };
 
+/* We instantiate Miniscript here with a simple integer as key type.
+ * The value of these key integers are an index in the
+ * DescriptorImpl::m_pubkey_args vector.
+ */
+
+/**
+ * The context for converting a Miniscript descriptor into a Script.
+ */
+class ScriptMaker {
+    //! Keys contained in the Miniscript (the evaluation of DescriptorImpl::m_pubkey_args).
+    const std::vector<CPubKey>& m_keys;
+
+public:
+    ScriptMaker(const std::vector<CPubKey>& keys LIFETIMEBOUND) : m_keys(keys) {}
+
+    std::vector<unsigned char> ToPKBytes(uint32_t key) const {
+        return {m_keys[key].begin(), m_keys[key].end()};
+    }
+
+    std::vector<unsigned char> ToPKHBytes(uint32_t key) const {
+        auto id = m_keys[key].GetID();
+        return {id.begin(), id.end()};
+    }
+};
+
+/**
+ * The context for converting a Miniscript descriptor to its textual form.
+ */
+class StringMaker {
+    //! To convert private keys for private descriptors.
+    const SigningProvider* m_arg;
+    //! Keys contained in the Miniscript (a reference to DescriptorImpl::m_pubkey_args).
+    const std::vector<std::unique_ptr<PubkeyProvider>>& m_pubkeys;
+    //! Whether to serialize keys as private or public.
+    bool m_private;
+
+public:
+    StringMaker(const SigningProvider* arg LIFETIMEBOUND, const std::vector<std::unique_ptr<PubkeyProvider>>& pubkeys LIFETIMEBOUND, bool priv)
+        : m_arg(arg), m_pubkeys(pubkeys), m_private(priv) {}
+
+    std::optional<std::string> ToString(uint32_t key) const
+    {
+        std::string ret;
+        if (m_private) {
+            if (!m_pubkeys[key]->ToPrivateString(*m_arg, ret)) return {};
+        } else {
+            ret = m_pubkeys[key]->ToString();
+        }
+        return ret;
+    }
+};
+
+class MiniscriptDescriptor final : public DescriptorImpl
+{
+private:
+    miniscript::NodeRef<uint32_t> m_node;
+
+protected:
+    std::vector<CScript> MakeScripts(const std::vector<CPubKey>& keys, Span<const CScript> scripts,
+                                     FlatSigningProvider& provider) const override
+    {
+        for (const auto& key : keys) provider.pubkeys.emplace(key.GetID(), key);
+        return Vector(m_node->ToScript(ScriptMaker(keys)));
+    }
+
+public:
+    MiniscriptDescriptor(std::vector<std::unique_ptr<PubkeyProvider>> providers, miniscript::NodeRef<uint32_t> node)
+        : DescriptorImpl(std::move(providers), "?"), m_node(std::move(node)) {}
+
+    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 == StringType::PRIVATE))) {
+            out = *res;
+            return true;
+        }
+        return false;
+    }
+
+    bool IsSolvable() const override { return false; } // For now, mark these descriptors as non-solvable (as we don't have signing logic for them).
+    bool IsSingleType() const final { return true; }
+};
+
 ////////////////////////////////////////////////////////////////////////////
 // Parser                                                                 //
 ////////////////////////////////////////////////////////////////////////////
@@ -1058,6 +1156,94 @@ std::unique_ptr<PubkeyProvider> ParsePubkey(uint32_t key_exp_index, const Span<c
     return std::make_unique<OriginPubkeyProvider>(key_exp_index, std::move(info), std::move(provider));
 }
 
+std::unique_ptr<PubkeyProvider> InferPubkey(const CPubKey& pubkey, ParseScriptContext, const SigningProvider& provider)
+{
+    std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey, false);
+    KeyOriginInfo info;
+    if (provider.GetKeyOrigin(pubkey.GetID(), info)) {
+        return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider));
+    }
+    return key_provider;
+}
+
+std::unique_ptr<PubkeyProvider> InferXOnlyPubkey(const XOnlyPubKey& xkey, ParseScriptContext ctx, const SigningProvider& provider)
+{
+    unsigned char full_key[CPubKey::COMPRESSED_SIZE] = {0x02};
+    std::copy(xkey.begin(), xkey.end(), full_key + 1);
+    CPubKey pubkey(full_key);
+    std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey, true);
+    KeyOriginInfo info;
+    if (provider.GetKeyOriginByXOnly(xkey, info)) {
+        return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider));
+    }
+    return key_provider;
+}
+
+/**
+ * The context for parsing a Miniscript descriptor (either from Script or from its textual representation).
+ */
+struct KeyParser {
+    //! The Key type is an index in DescriptorImpl::m_pubkey_args
+    using Key = uint32_t;
+    //! Must not be nullptr if parsing from string.
+    FlatSigningProvider* m_out;
+    //! Must not be nullptr if parsing from Script.
+    const SigningProvider* m_in;
+    //! List of keys contained in the Miniscript.
+    mutable std::vector<std::unique_ptr<PubkeyProvider>> m_keys;
+    //! Used to detect key parsing errors within a Miniscript.
+    mutable std::string m_key_parsing_error;
+
+    KeyParser(FlatSigningProvider* out LIFETIMEBOUND, const SigningProvider* in LIFETIMEBOUND) : m_out(out), m_in(in) {}
+
+    bool KeyCompare(const Key& a, const Key& b) const {
+        return *m_keys.at(a) < *m_keys.at(b);
+    }
+
+    template<typename I> std::optional<Key> FromString(I begin, I end) const
+    {
+        assert(m_out);
+        Key key = m_keys.size();
+        auto pk = ParsePubkey(key, {&*begin, &*end}, ParseScriptContext::P2WSH, *m_out, m_key_parsing_error);
+        if (!pk) return {};
+        m_keys.push_back(std::move(pk));
+        return key;
+    }
+
+    std::optional<std::string> ToString(const Key& key) const
+    {
+        return m_keys.at(key)->ToString();
+    }
+
+    template<typename I> std::optional<Key> FromPKBytes(I begin, I end) const
+    {
+        assert(m_in);
+        CPubKey pubkey(begin, end);
+        if (pubkey.IsValid()) {
+            Key key = m_keys.size();
+            m_keys.push_back(InferPubkey(pubkey, ParseScriptContext::P2WSH, *m_in));
+            return key;
+        }
+        return {};
+    }
+
+    template<typename I> std::optional<Key> FromPKHBytes(I begin, I end) const
+    {
+        assert(end - begin == 20);
+        assert(m_in);
+        uint160 hash;
+        std::copy(begin, end, hash.begin());
+        CKeyID keyid(hash);
+        CPubKey pubkey;
+        if (m_in->GetPubKey(keyid, pubkey)) {
+            Key key = m_keys.size();
+            m_keys.push_back(InferPubkey(pubkey, ParseScriptContext::P2WSH, *m_in));
+            return key;
+        }
+        return {};
+    }
+};
+
 /** Parse a script in a particular context. */
 std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const char>& sp, ParseScriptContext ctx, FlatSigningProvider& out, std::string& error)
 {
@@ -1279,6 +1465,45 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const
         error = "Can only have raw() at top level";
         return nullptr;
     }
+    // Process miniscript expressions.
+    {
+        KeyParser parser(&out, nullptr);
+        auto node = miniscript::FromString(std::string(expr.begin(), expr.end()), parser);
+        if (node) {
+            if (ctx != ParseScriptContext::P2WSH) {
+                error = "Miniscript expressions can only be used in wsh";
+                return nullptr;
+            }
+            if (parser.m_key_parsing_error != "") {
+                error = std::move(parser.m_key_parsing_error);
+                return nullptr;
+            }
+            if (!node->IsSane()) {
+                // 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;
+                if (!insane_node->IsValid()) {
+                    error += " is invalid";
+                } else {
+                    error += " is not sane";
+                    if (!insane_node->IsNonMalleable()) {
+                        error += ": malleable witnesses exist";
+                    } else if (insane_node == node.get() && !insane_node->NeedsSignature()) {
+                        error += ": witnesses without signature exist";
+                    } else if (!insane_node->CheckTimeLocksMix()) {
+                        error += ": contains mixes of timelocks expressed in blocks and seconds";
+                    } else if (!insane_node->CheckDuplicateKey()) {
+                        error += ": contains duplicate public keys";
+                    } else if (!insane_node->ValidSatisfactions()) {
+                        error += ": needs witnesses that may exceed resource limits";
+                    }
+                }
+                return nullptr;
+            }
+            return std::make_unique<MiniscriptDescriptor>(std::move(parser.m_keys), std::move(node));
+        }
+    }
     if (ctx == ParseScriptContext::P2SH) {
         error = "A function is needed within P2SH";
         return nullptr;
@@ -1290,29 +1515,6 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const
     return nullptr;
 }
 
-std::unique_ptr<PubkeyProvider> InferPubkey(const CPubKey& pubkey, ParseScriptContext, const SigningProvider& provider)
-{
-    std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey, false);
-    KeyOriginInfo info;
-    if (provider.GetKeyOrigin(pubkey.GetID(), info)) {
-        return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider));
-    }
-    return key_provider;
-}
-
-std::unique_ptr<PubkeyProvider> InferXOnlyPubkey(const XOnlyPubKey& xkey, ParseScriptContext ctx, const SigningProvider& provider)
-{
-    unsigned char full_key[CPubKey::COMPRESSED_SIZE] = {0x02};
-    std::copy(xkey.begin(), xkey.end(), full_key + 1);
-    CPubKey pubkey(full_key);
-    std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey, true);
-    KeyOriginInfo info;
-    if (provider.GetKeyOriginByXOnly(xkey, info)) {
-        return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider));
-    }
-    return key_provider;
-}
-
 std::unique_ptr<DescriptorImpl> InferMultiA(const CScript& script, ParseScriptContext ctx, const SigningProvider& provider)
 {
     auto match = MatchMultiA(script);
@@ -1426,6 +1628,14 @@ std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptCo
         }
     }
 
+    if (ctx == ParseScriptContext::P2WSH) {
+        KeyParser parser(nullptr, &provider);
+        auto node = miniscript::FromScript(script, parser);
+        if (node && node->IsSane()) {
+            return std::make_unique<MiniscriptDescriptor>(std::move(parser.m_keys), std::move(node));
+        }
+    }
+
     CTxDestination dest;
     if (ExtractDestination(script, dest)) {
         if (GetScriptForDestination(dest) == script) {

src/script/miniscript.h

@@ -429,6 +429,21 @@ struct Node {
         ));
     }
 
+    /** Like TreeEval, but without downfn or State type.
+     *  upfn takes (const Node&, Span<Result>) and returns Result. */
+    template<typename Result, typename UpFn>
+    Result TreeEval(UpFn upfn) const
+    {
+        struct DummyState {};
+        return std::move(*TreeEvalMaybe<Result>(DummyState{},
+            [](DummyState, const Node&, size_t) { return DummyState{}; },
+            [&upfn](DummyState, const Node& node, Span<Result> subs) {
+                Result res{upfn(node, subs)};
+                return std::optional<Result>(std::move(res));
+            }
+        ));
+    }
+
     /** Compare two miniscript subtrees, using a non-recursive algorithm. */
     friend int Compare(const Node<Key>& node1, const Node<Key>& node2)
     {
@@ -818,6 +833,15 @@ struct Node {
     //! Return the expression type.
     Type GetType() const { return typ; }
 
+    //! Find an insane subnode which has no insane children. Nullptr if there is none.
+    const Node* FindInsaneSub() const {
+        return TreeEval<const Node*>([](const Node& node, Span<const Node*> subs) -> const Node* {
+            for (auto& sub: subs) if (sub) return sub;
+            if (!node.IsSaneSubexpression()) return &node;
+            return nullptr;
+        });
+    }
+
     //! Check whether this node is valid at all.
     bool IsValid() const { return !(GetType() == ""_mst) && ScriptSize() <= MAX_STANDARD_P2WSH_SCRIPT_SIZE; }
 
@@ -953,7 +977,11 @@ void BuildBack(const Ctx& ctx, Fragment nt, std::vector<NodeRef<Key>>& construct
     }
 }
 
-//! Parse a miniscript from its textual descriptor form.
+/**
+ * Parse a miniscript from its textual descriptor form.
+ * This does not check whether the script is valid, let alone sane. The caller is expected to use
+ * the `IsValidTopLevel()` and `IsSaneTopLevel()` to check for these properties on the node.
+ */
 template<typename Key, typename Ctx>
 inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx)
 {
@@ -1255,9 +1283,7 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx)
     // Sanity checks on the produced miniscript
     assert(constructed.size() == 1);
     if (in.size() > 0) return {};
-    const NodeRef<Key> tl_node = std::move(constructed.front());
-    if (!tl_node->IsValidTopLevel()) return {};
-    return tl_node;
+    return std::move(constructed.front());
 }
 
 /** Decode a script into opcode/push pairs.