From 78ff01df55997fffe2b72cfe2aedeb07d8a0b3ec 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.
---
 src/script/descriptor.cpp |  2 +-
 src/script/miniscript.h   | 19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 5026470edcfb50..6053c853bbd2df 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -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), miniscript::MakeNodeRef<uint32_t>(*m_node));
+        return std::make_unique<MiniscriptDescriptor>(std::move(providers), miniscript::MakeNodeRef<uint32_t>(m_node->Clone()));
     }
 };
 
diff --git a/src/script/miniscript.h b/src/script/miniscript.h
index 58f24434f064ee..e055be73b24f31 100644
--- a/src/script/miniscript.h
+++ b/src/script/miniscript.h
@@ -523,6 +523,20 @@ struct Node {
         }
     }
 
+    Node<Key> Clone() const
+    {
+        // Use TreeEval() to avoid a stack-overflow due to recursion
+        auto upfn = [](const Node& node, Span<Node> subs) {
+            Node<Key> ret(node);
+            ret.subs.clear();
+            for (const auto& sub : subs) {
+                ret.subs.push_back(MakeNodeRef<Key>(sub));
+            }
+            return std::move(ret);
+        };
+        return TreeEval<Node<Key>>(upfn);
+    }
+
 private:
     //! Cached ops counts.
     const internal::Ops ops;
@@ -630,7 +644,10 @@ struct Node {
             // If evaluation returns std::nullopt, abort immediately.
             if (!result) return {};
             // Replace the last node.subs.size() elements of results with the new result.
-            results.erase(results.end() - node.subs.size(), results.end());
+            // Use pop_back to truncate results to avoid MoveAssignable requirement of erase().
+            for (size_t i = 0; i < node.subs.size(); ++i) {
+                results.pop_back();
+            }
             results.push_back(std::move(*result));
             stack.pop_back();
         }