miniscript: Desmartpointerify

Removes a layer of shared_ptr, which greatly clarifies ownership. Makes a lot of members in miniscript.h non-const in order to allow implicit (move) assignment operator(s) when Node is used directly in std::vector. In exchange, MiniscriptDescriptor::m_node is now const.
hodlinator · Nov 13, 2024 · 392b858 · 392b858
1 parent 6a00ea1
commit 392b858
Show file tree

Hide file tree

Showing 4 changed files with 293 additions and 278 deletions.
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
@@ -1308,31 +1308,31 @@ class StringMaker {
 class MiniscriptDescriptor final : public DescriptorImpl
 {
 private:
-    miniscript::NodeRef<uint32_t> m_node;
+    const miniscript::Node<uint32_t> m_node;
 
 protected:
     std::vector<CScript> MakeScripts(const std::vector<CPubKey>& keys, Span<const CScript> scripts,
                                      FlatSigningProvider& provider) const override
     {
-        const auto script_ctx{m_node->GetMsCtx()};
+        const auto script_ctx{m_node.GetMsCtx()};
         for (const auto& key : keys) {
             if (miniscript::IsTapscript(script_ctx)) {
                 provider.pubkeys.emplace(Hash160(XOnlyPubKey{key}), key);
             } else {
                 provider.pubkeys.emplace(key.GetID(), key);
             }
         }
-        return Vector(m_node->ToScript(ScriptMaker(keys, script_ctx)));
+        return Vector(m_node.ToScript(ScriptMaker(keys, script_ctx)));
     }
 
 public:
-    MiniscriptDescriptor(std::vector<std::unique_ptr<PubkeyProvider>> providers, miniscript::NodeRef<uint32_t> node)
-        : DescriptorImpl(std::move(providers), "?"), m_node(std::move(node)) {}
+    MiniscriptDescriptor(std::vector<std::unique_ptr<PubkeyProvider>> providers, miniscript::Node<uint32_t>&& node)
+        : DescriptorImpl{std::move(providers), "?"}, m_node{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))) {
+        if (const auto res = m_node.ToString(StringMaker(arg, m_pubkey_args, type == StringType::PRIVATE))) {
             out = *res;
             return true;
         }
@@ -1342,15 +1342,15 @@ class MiniscriptDescriptor final : public DescriptorImpl
     bool IsSolvable() const override { return true; }
     bool IsSingleType() const final { return true; }
 
-    std::optional<int64_t> ScriptSize() const override { return m_node->ScriptSize(); }
+    std::optional<int64_t> ScriptSize() const override { return m_node.ScriptSize(); }
 
     std::optional<int64_t> MaxSatSize(bool) const override {
         // For Miniscript we always assume high-R ECDSA signatures.
-        return m_node->GetWitnessSize();
+        return m_node.GetWitnessSize();
     }
 
     std::optional<int64_t> MaxSatisfactionElems() const override {
-        return m_node->GetStackSize();
+        return m_node.GetStackSize();
     }
 
     std::unique_ptr<DescriptorImpl> Clone() const override
@@ -1360,7 +1360,7 @@ class MiniscriptDescriptor final : public DescriptorImpl
         for (const auto& arg : m_pubkey_args) {
             providers.push_back(arg->Clone());
         }
-        return std::make_unique<MiniscriptDescriptor>(std::move(providers), m_node->Clone());
+        return std::make_unique<MiniscriptDescriptor>(std::move(providers), m_node.Clone());
     }
 };
 
@@ -2101,7 +2101,7 @@ std::vector<std::unique_ptr<DescriptorImpl>> ParseScript(uint32_t& key_exp_index
             }
             if (!node->IsSane() || node->IsNotSatisfiable()) {
                 // Try to find the first insane sub for better error reporting.
-                auto insane_node = node.get();
+                const auto* insane_node = &*node;
                 if (const auto sub = node->FindInsaneSub()) insane_node = sub;
                 if (const auto str = insane_node->ToString(parser)) error = *str;
                 if (!insane_node->IsValid()) {
@@ -2110,7 +2110,7 @@ std::vector<std::unique_ptr<DescriptorImpl>> ParseScript(uint32_t& key_exp_index
                     error += " is not sane";
                     if (!insane_node->IsNonMalleable()) {
                         error += ": malleable witnesses exist";
-                    } else if (insane_node == node.get() && !insane_node->NeedsSignature()) {
+                    } else if (insane_node == &*node && !insane_node->NeedsSignature()) {
                         error += ": witnesses without signature exist";
                     } else if (!insane_node->CheckTimeLocksMix()) {
                         error += ": contains mixes of timelocks expressed in blocks and seconds";
@@ -2154,7 +2154,7 @@ std::vector<std::unique_ptr<DescriptorImpl>> ParseScript(uint32_t& key_exp_index
                 for (auto& pub : parser.m_keys) {
                     pubs.emplace_back(std::move(pub.at(i)));
                 }
-                ret.emplace_back(std::make_unique<MiniscriptDescriptor>(std::move(pubs), node));
+                ret.emplace_back(std::make_unique<MiniscriptDescriptor>(std::move(pubs), node->Clone()));
             }
             return ret;
         }
@@ -2310,7 +2310,7 @@ std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptCo
             for (auto& key : parser.m_keys) {
                 keys.emplace_back(std::move(key.at(0)));
             }
-            return std::make_unique<MiniscriptDescriptor>(std::move(keys), std::move(node));
+            return std::make_unique<MiniscriptDescriptor>(std::move(keys), std::move(*node));
         }
     }