From 6d11c9c60b5b4a71f8f75cb4eee2f9d0021de310 Mon Sep 17 00:00:00 2001
From: Ava Chow <github@achow101.com>
Date: Tue, 10 Sep 2024 15:50:08 -0400
Subject: [PATCH] descriptor: Add proper Clone function to miniscript::Node

Multipath descriptors requires performing a deep copy, so a Clone
function that does that is added to miniscript::Node instead of the
current shallow copy.

Co-Authored-By: Antoine Poinsot <darosior@protonmail.com>
---
 src/script/descriptor.cpp |  4 ++--
 src/script/miniscript.h   | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 499af47ee55..c8802d2bf89 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -1353,7 +1353,7 @@ public:
         for (const auto& arg : m_pubkey_args) {
             providers.push_back(arg->Clone());
         }
-        return std::make_unique<MiniscriptDescriptor>(std::move(providers), miniscript::MakeNodeRef<uint32_t>(*m_node));
+        return std::make_unique<MiniscriptDescriptor>(std::move(providers), m_node->Clone());
     }
 };
 
@@ -2143,7 +2143,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;
         }
diff --git a/src/script/miniscript.h b/src/script/miniscript.h
index 75f978457c5..dad744aebba 100644
--- a/src/script/miniscript.h
+++ b/src/script/miniscript.h
@@ -528,6 +528,20 @@ struct Node {
         }
     }
 
+    NodeRef<Key> Clone() const
+    {
+        // Use TreeEval() to avoid a stack-overflow due to recursion
+        auto upfn = [](const Node& node, Span<NodeRef<Key>> children) {
+            std::vector<NodeRef<Key>> new_subs;
+            for (auto child = children.begin(); child != children.end(); ++child) {
+                new_subs.emplace_back(std::move(*child));
+            }
+            // std::make_unique (and therefore MakeNodeRef) doesn't work on private constructors
+            return std::unique_ptr<Node>{new Node{internal::NoDupCheck{}, node.m_script_ctx, node.fragment, std::move(new_subs), node.keys, node.data, node.k}};
+        };
+        return TreeEval<NodeRef<Key>>(upfn);
+    }
+
 private:
     //! Cached ops counts.
     const internal::Ops ops;
@@ -546,6 +560,11 @@ private:
     //! for all subnodes as well.
     mutable std::optional<bool> has_duplicate_keys;
 
+    // Constructor which takes all of the data that a Node could possibly contain.
+    // This is kept private as no valid fragment has all of these arguments.
+    // Only used by Clone()
+    Node(internal::NoDupCheck, MiniscriptContext script_ctx, Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<Key> key, std::vector<unsigned char> arg, uint32_t val)
+        : fragment(nt), k(val), keys(key), data(std::move(arg)), subs(std::move(sub)), m_script_ctx{script_ctx}, ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {}
 
     //! Compute the length of the script for this miniscript (including children).
     size_t CalcScriptLen() const {