From 22e4115312b929502574ba3681ee2c3b3fd14d96 Mon Sep 17 00:00:00 2001
From: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Date: Wed, 22 Jan 2025 21:19:24 +0100
Subject: [PATCH 1/7] doc(miniscript): Remove mention of shared pointers

Correct destructor implementation comment to no longer refer to shared pointers and also move it into the function body, in symmetry with Clone() right below.

Leftover from #30866.
---
 src/script/miniscript.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/script/miniscript.h b/src/script/miniscript.h
index 74600292b01..a1ca3eaa32a 100644
--- a/src/script/miniscript.h
+++ b/src/script/miniscript.h
@@ -532,9 +532,11 @@ struct Node {
     //! The Script context for this node. Either P2WSH or Tapscript.
     const MiniscriptContext m_script_ctx;
 
-    /* Destroy the shared pointers iteratively to avoid a stack-overflow due to recursive calls
-     * to the subs' destructors. */
-    ~Node() {
+    ~Node()
+    {
+        // Destroy the subexpressions iteratively after moving out their
+        // subexpressions to avoid a stack-overflow due to recursive calls to
+        // the subs' destructors.
         while (!subs.empty()) {
             auto node = std::move(subs.back());
             subs.pop_back();

From c6f798b22247bc092e72eed9e9f69a0cbaca5134 Mon Sep 17 00:00:00 2001
From: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Date: Sat, 3 Jan 2026 13:06:00 +0100
Subject: [PATCH 2/7] refactor(miniscript): Make fields non-const & private
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Makes a lot of fields in miniscript.h non-const in order to allow move-operations 2 commits later.

Also fixes adjacent comment typos.

Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
---
 src/script/descriptor.cpp     |   4 +-
 src/script/miniscript.h       | 154 ++++++++++++++++++++--------------
 src/test/fuzz/miniscript.cpp  |  20 ++---
 src/test/miniscript_tests.cpp |  18 ++--
 4 files changed, 110 insertions(+), 86 deletions(-)

diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index f771ee52787..390e7345515 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -1607,8 +1607,8 @@ public:
     {
         // Traverse miniscript tree for unsafe use of older()
         miniscript::ForEachNode(*m_node, [&](const miniscript::Node<uint32_t>& node) {
-            if (node.fragment == miniscript::Fragment::OLDER) {
-                const uint32_t raw = node.k;
+            if (node.Fragment() == miniscript::Fragment::OLDER) {
+                const uint32_t raw = node.K();
                 const uint32_t value_part = raw & ~CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG;
                 if (value_part > CTxIn::SEQUENCE_LOCKTIME_MASK) {
                     const bool is_time_based = (raw & CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG) != 0;
diff --git a/src/script/miniscript.h b/src/script/miniscript.h
index a1ca3eaa32a..1c5f7a8c0f4 100644
--- a/src/script/miniscript.h
+++ b/src/script/miniscript.h
@@ -190,7 +190,7 @@ inline consteval Type operator""_mst(const char* c, size_t l)
 
 using Opcode = std::pair<opcodetype, std::vector<unsigned char>>;
 
-template<typename Key> struct Node;
+template<typename Key> class Node;
 template<typename Key> using NodeRef = std::unique_ptr<const Node<Key>>;
 
 //! Construct a miniscript node as a unique_ptr.
@@ -206,7 +206,7 @@ void ForEachNode(const Node<Key>& root, Fn&& fn)
         const Node<Key>& node = stack.back();
         std::invoke(fn, node);
         stack.pop_back();
-        for (const auto& sub : node.subs) {
+        for (const auto& sub : node.Subs()) {
             stack.emplace_back(*sub);
         }
     }
@@ -364,14 +364,19 @@ struct InputResult {
 };
 
 //! Class whose objects represent the maximum of a list of integers.
-template<typename I>
-struct MaxInt {
-    const bool valid;
-    const I value;
+template <typename I>
+class MaxInt
+{
+    bool valid;
+    I value;
 
+public:
     MaxInt() : valid(false), value(0) {}
     MaxInt(I val) : valid(true), value(val) {}
 
+    bool Valid() const { return valid; }
+    I Value() const { return value; }
+
     friend MaxInt<I> operator+(const MaxInt<I>& a, const MaxInt<I>& b) {
         if (!a.valid || !b.valid) return {};
         return a.value + b.value;
@@ -436,14 +441,16 @@ struct Ops {
  * - It is not a commutative semiring, because a+b can differ from b+a. For example, "OP_1 OP_DROP"
  *   has exec=1, while "OP_DROP OP_1" has exec=0.
  */
-struct SatInfo {
+class SatInfo
+{
     //! Whether a canonical satisfaction/dissatisfaction is possible at all.
-    const bool valid;
+    bool valid;
     //! How much higher the stack size at start of execution can be compared to at the end.
-    const int32_t netdiff;
-    //! Mow much higher the stack size can be during execution compared to at the end.
-    const int32_t exec;
+    int32_t netdiff;
+    //! How much higher the stack size can be during execution compared to at the end.
+    int32_t exec;
 
+public:
     /** Empty script set. */
     constexpr SatInfo() noexcept : valid(false), netdiff(0), exec(0) {}
 
@@ -451,6 +458,10 @@ struct SatInfo {
     constexpr SatInfo(int32_t in_netdiff, int32_t in_exec) noexcept :
         valid{true}, netdiff{in_netdiff}, exec{in_exec} {}
 
+    bool Valid() const { return valid; }
+    int32_t NetDiff() const { return netdiff; }
+    int32_t Exec() const { return exec; }
+
     /** Script set union. */
     constexpr friend SatInfo operator|(const SatInfo& a, const SatInfo& b) noexcept
     {
@@ -496,11 +507,16 @@ struct SatInfo {
     static constexpr SatInfo OP_VERIFY() noexcept { return {1, 1}; }
 };
 
-struct StackSize {
-    const SatInfo sat, dsat;
+class StackSize
+{
+    SatInfo sat, dsat;
 
+public:
     constexpr StackSize(SatInfo in_sat, SatInfo in_dsat) noexcept : sat(in_sat), dsat(in_dsat) {};
     constexpr StackSize(SatInfo in_both) noexcept : sat(in_both), dsat(in_both) {};
+
+    const SatInfo& Sat() const { return sat; }
+    const SatInfo& Dsat() const { return dsat; }
 };
 
 struct WitnessSize {
@@ -517,21 +533,23 @@ struct NoDupCheck {};
 } // namespace internal
 
 //! A node in a miniscript expression.
-template<typename Key>
-struct Node {
+template <typename Key>
+class Node
+{
     //! What node type this node is.
-    const Fragment fragment;
+    enum Fragment fragment;
     //! The k parameter (time for OLDER/AFTER, threshold for THRESH(_M))
-    const uint32_t k = 0;
+    uint32_t k = 0;
     //! The keys used by this expression (only for PK_K/PK_H/MULTI)
-    const std::vector<Key> keys;
-    //! The data bytes in this expression (only for HASH160/HASH256/SHA256/RIPEMD10).
-    const std::vector<unsigned char> data;
+    std::vector<Key> keys;
+    //! The data bytes in this expression (only for HASH160/HASH256/SHA256/RIPEMD160).
+    std::vector<unsigned char> data;
     //! Subexpressions (for WRAP_*/AND_*/OR_*/ANDOR/THRESH)
     mutable std::vector<NodeRef<Key>> subs;
     //! The Script context for this node. Either P2WSH or Tapscript.
-    const MiniscriptContext m_script_ctx;
+    MiniscriptContext m_script_ctx;
 
+public:
     ~Node()
     {
         // Destroy the subexpressions iteratively after moving out their
@@ -561,17 +579,23 @@ struct Node {
         return TreeEval<NodeRef<Key>>(upfn);
     }
 
+    enum Fragment Fragment() const { return fragment; }
+    uint32_t K() const { return k; }
+    const std::vector<Key>& Keys() const { return keys; }
+    const std::vector<unsigned char>& Data() const { return data; }
+    const std::vector<NodeRef<Key>>& Subs() const { return subs; }
+
 private:
     //! Cached ops counts.
-    const internal::Ops ops;
+    internal::Ops ops;
     //! Cached stack size bounds.
-    const internal::StackSize ss;
+    internal::StackSize ss;
     //! Cached witness size bounds.
-    const internal::WitnessSize ws;
+    internal::WitnessSize ws;
     //! Cached expression type (computed by CalcType and fed through SanitizeType).
-    const Type typ;
+    Type typ;
     //! Cached script length (computed by CalcScriptLen).
-    const size_t scriptlen;
+    size_t scriptlen;
     //! Whether a public key appears more than once in this node. This value is initialized
     //! by all constructors except the NoDupCheck ones. The NoDupCheck ones skip the
     //! computation, requiring it to be done manually by invoking DuplicateKeyCheck().
@@ -582,7 +606,7 @@ private:
     // 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)
+    Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum 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).
@@ -1066,45 +1090,45 @@ private:
                 const auto& y{subs[1]->ss};
                 const auto& z{subs[2]->ss};
                 return {
-                    (x.sat + SatInfo::If() + y.sat) | (x.dsat + SatInfo::If() + z.sat),
-                    x.dsat + SatInfo::If() + z.dsat
+                    (x.Sat() + SatInfo::If() + y.Sat()) | (x.Dsat() + SatInfo::If() + z.Sat()),
+                    x.Dsat() + SatInfo::If() + z.Dsat()
                 };
             }
             case Fragment::AND_V: {
                 const auto& x{subs[0]->ss};
                 const auto& y{subs[1]->ss};
-                return {x.sat + y.sat, {}};
+                return {x.Sat() + y.Sat(), {}};
             }
             case Fragment::AND_B: {
                 const auto& x{subs[0]->ss};
                 const auto& y{subs[1]->ss};
-                return {x.sat + y.sat + SatInfo::BinaryOp(), x.dsat + y.dsat + SatInfo::BinaryOp()};
+                return {x.Sat() + y.Sat() + SatInfo::BinaryOp(), x.Dsat() + y.Dsat() + SatInfo::BinaryOp()};
             }
             case Fragment::OR_B: {
                 const auto& x{subs[0]->ss};
                 const auto& y{subs[1]->ss};
                 return {
-                    ((x.sat + y.dsat) | (x.dsat + y.sat)) + SatInfo::BinaryOp(),
-                    x.dsat + y.dsat + SatInfo::BinaryOp()
+                    ((x.Sat() + y.Dsat()) | (x.Dsat() + y.Sat())) + SatInfo::BinaryOp(),
+                    x.Dsat() + y.Dsat() + SatInfo::BinaryOp()
                 };
             }
             case Fragment::OR_C: {
                 const auto& x{subs[0]->ss};
                 const auto& y{subs[1]->ss};
-                return {(x.sat + SatInfo::If()) | (x.dsat + SatInfo::If() + y.sat), {}};
+                return {(x.Sat() + SatInfo::If()) | (x.Dsat() + SatInfo::If() + y.Sat()), {}};
             }
             case Fragment::OR_D: {
                 const auto& x{subs[0]->ss};
                 const auto& y{subs[1]->ss};
                 return {
-                    (x.sat + SatInfo::OP_IFDUP(true) + SatInfo::If()) | (x.dsat + SatInfo::OP_IFDUP(false) + SatInfo::If() + y.sat),
-                    x.dsat + SatInfo::OP_IFDUP(false) + SatInfo::If() + y.dsat
+                    (x.Sat() + SatInfo::OP_IFDUP(true) + SatInfo::If()) | (x.Dsat() + SatInfo::OP_IFDUP(false) + SatInfo::If() + y.Sat()),
+                    x.Dsat() + SatInfo::OP_IFDUP(false) + SatInfo::If() + y.Dsat()
                 };
             }
             case Fragment::OR_I: {
                 const auto& x{subs[0]->ss};
                 const auto& y{subs[1]->ss};
-                return {SatInfo::If() + (x.sat | y.sat), SatInfo::If() + (x.dsat | y.dsat)};
+                return {SatInfo::If() + (x.Sat() | y.Sat()), SatInfo::If() + (x.Dsat() | y.Dsat())};
             }
             // multi(k, key1, key2, ..., key_n) starts off with k+1 stack elements (a 0, plus k
             // signatures), then reaches n+k+3 stack elements after pushing the n keys, plus k and
@@ -1119,16 +1143,16 @@ private:
             case Fragment::WRAP_N:
             case Fragment::WRAP_S: return subs[0]->ss;
             case Fragment::WRAP_C: return {
-                subs[0]->ss.sat + SatInfo::OP_CHECKSIG(),
-                subs[0]->ss.dsat + SatInfo::OP_CHECKSIG()
+                subs[0]->ss.Sat() + SatInfo::OP_CHECKSIG(),
+                subs[0]->ss.Dsat() + SatInfo::OP_CHECKSIG()
             };
             case Fragment::WRAP_D: return {
-                SatInfo::OP_DUP() + SatInfo::If() + subs[0]->ss.sat,
+                SatInfo::OP_DUP() + SatInfo::If() + subs[0]->ss.Sat(),
                 SatInfo::OP_DUP() + SatInfo::If()
             };
-            case Fragment::WRAP_V: return {subs[0]->ss.sat + SatInfo::OP_VERIFY(), {}};
+            case Fragment::WRAP_V: return {subs[0]->ss.Sat() + SatInfo::OP_VERIFY(), {}};
             case Fragment::WRAP_J: return {
-                SatInfo::OP_SIZE() + SatInfo::OP_0NOTEQUAL() + SatInfo::If() + subs[0]->ss.sat,
+                SatInfo::OP_SIZE() + SatInfo::OP_0NOTEQUAL() + SatInfo::If() + subs[0]->ss.Sat(),
                 SatInfo::OP_SIZE() + SatInfo::OP_0NOTEQUAL() + SatInfo::If()
             };
             case Fragment::THRESH: {
@@ -1139,13 +1163,13 @@ private:
                     // element i we need to add OP_ADD (if i>0).
                     auto add = i ? SatInfo::BinaryOp() : SatInfo::Empty();
                     // Construct a variable that will become the next sats, starting with index 0.
-                    auto next_sats = Vector(sats[0] + subs[i]->ss.dsat + add);
+                    auto next_sats = Vector(sats[0] + subs[i]->ss.Dsat() + add);
                     // Then loop to construct next_sats[1..i].
                     for (size_t j = 1; j < sats.size(); ++j) {
-                        next_sats.push_back(((sats[j] + subs[i]->ss.dsat) | (sats[j - 1] + subs[i]->ss.sat)) + add);
+                        next_sats.push_back(((sats[j] + subs[i]->ss.Dsat()) | (sats[j - 1] + subs[i]->ss.Sat())) + add);
                     }
                     // Finally construct next_sats[i+1].
-                    next_sats.push_back(sats[sats.size() - 1] + subs[i]->ss.sat + add);
+                    next_sats.push_back(sats[sats.size() - 1] + subs[i]->ss.Sat() + add);
                     // Switch over.
                     sats = std::move(next_sats);
                 }
@@ -1530,8 +1554,8 @@ public:
 
     //! Return the maximum number of ops needed to satisfy this script non-malleably.
     std::optional<uint32_t> GetOps() const {
-        if (!ops.sat.valid) return {};
-        return ops.count + ops.sat.value;
+        if (!ops.sat.Valid()) return {};
+        return ops.count + ops.sat.Value();
     }
 
     //! Return the number of ops in the script (not counting the dynamic ones that depend on execution).
@@ -1551,14 +1575,14 @@ public:
 
     /** Return the maximum number of stack elements needed to satisfy this script non-malleably. */
     std::optional<uint32_t> GetStackSize() const {
-        if (!ss.sat.valid) return {};
-        return ss.sat.netdiff + static_cast<int32_t>(IsBKW());
+        if (!ss.Sat().Valid()) return {};
+        return ss.Sat().NetDiff() + static_cast<int32_t>(IsBKW());
     }
 
     //! Return the maximum size of the stack during execution of this script.
     std::optional<uint32_t> GetExecStackSize() const {
-        if (!ss.sat.valid) return {};
-        return ss.sat.exec + static_cast<int32_t>(IsBKW());
+        if (!ss.Sat().Valid()) return {};
+        return ss.Sat().Exec() + static_cast<int32_t>(IsBKW());
     }
 
     //! Check the maximum stack size for this script against the policy limit.
@@ -1579,8 +1603,8 @@ public:
     /** Return the maximum size in bytes of a witness to satisfy this script non-malleably. Note this does
      * not include the witness script push. */
     std::optional<uint32_t> GetWitnessSize() const {
-        if (!ws.sat.valid) return {};
-        return ws.sat.value;
+        if (!ws.sat.Valid()) return {};
+        return ws.sat.Value();
     }
 
     //! Return the expression type.
@@ -1687,31 +1711,31 @@ public:
     bool operator==(const Node<Key>& arg) const { return Compare(*this, arg) == 0; }
 
     // Constructors with various argument combinations, which bypass the duplicate key check.
-    Node(internal::NoDupCheck, MiniscriptContext script_ctx, Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<unsigned char> arg, uint32_t val = 0)
+    Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<unsigned char> arg, uint32_t val = 0)
         : fragment(nt), k(val), data(std::move(arg)), subs(std::move(sub)), m_script_ctx{script_ctx}, ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {}
-    Node(internal::NoDupCheck, MiniscriptContext script_ctx, Fragment nt, std::vector<unsigned char> arg, uint32_t val = 0)
+    Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector<unsigned char> arg, uint32_t val = 0)
         : fragment(nt), k(val), data(std::move(arg)), m_script_ctx{script_ctx}, ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {}
-    Node(internal::NoDupCheck, MiniscriptContext script_ctx, Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<Key> key, uint32_t val = 0)
+    Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<Key> key, uint32_t val = 0)
         : fragment(nt), k(val), keys(std::move(key)), m_script_ctx{script_ctx}, subs(std::move(sub)), ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {}
-    Node(internal::NoDupCheck, MiniscriptContext script_ctx, Fragment nt, std::vector<Key> key, uint32_t val = 0)
+    Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector<Key> key, uint32_t val = 0)
         : fragment(nt), k(val), keys(std::move(key)), m_script_ctx{script_ctx}, ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {}
-    Node(internal::NoDupCheck, MiniscriptContext script_ctx, Fragment nt, std::vector<NodeRef<Key>> sub, uint32_t val = 0)
+    Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector<NodeRef<Key>> sub, uint32_t val = 0)
         : fragment(nt), k(val), subs(std::move(sub)), m_script_ctx{script_ctx}, ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {}
-    Node(internal::NoDupCheck, MiniscriptContext script_ctx, Fragment nt, uint32_t val = 0)
+    Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, uint32_t val = 0)
         : fragment(nt), k(val), m_script_ctx{script_ctx}, ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {}
 
     // Constructors with various argument combinations, which do perform the duplicate key check.
-    template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<unsigned char> arg, uint32_t val = 0)
+    template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<unsigned char> arg, uint32_t val = 0)
         : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(sub), std::move(arg), val) { DuplicateKeyCheck(ctx); }
-    template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<unsigned char> arg, uint32_t val = 0)
+    template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, std::vector<unsigned char> arg, uint32_t val = 0)
         : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(arg), val) { DuplicateKeyCheck(ctx);}
-    template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<Key> key, uint32_t val = 0)
+    template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<Key> key, uint32_t val = 0)
         : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(sub), std::move(key), val) { DuplicateKeyCheck(ctx); }
-    template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<Key> key, uint32_t val = 0)
+    template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, std::vector<Key> key, uint32_t val = 0)
         : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(key), val) { DuplicateKeyCheck(ctx); }
-    template <typename Ctx> Node(const Ctx& ctx, Fragment nt, std::vector<NodeRef<Key>> sub, uint32_t val = 0)
+    template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, std::vector<NodeRef<Key>> sub, uint32_t val = 0)
         : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(sub), val) { DuplicateKeyCheck(ctx); }
-    template <typename Ctx> Node(const Ctx& ctx, Fragment nt, uint32_t val = 0)
+    template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, uint32_t val = 0)
         : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, val) { DuplicateKeyCheck(ctx); }
 
     // Delete copy constructor and assignment operator, use Clone() instead
diff --git a/src/test/fuzz/miniscript.cpp b/src/test/fuzz/miniscript.cpp
index 85797a1a6c7..dc1b3c987d3 100644
--- a/src/test/fuzz/miniscript.cpp
+++ b/src/test/fuzz/miniscript.cpp
@@ -991,7 +991,7 @@ NodeRef GenNode(MsCtx script_ctx, F ConsumeNode, Type root_type, bool strict_val
             }
             if (!node->IsValid()) return {};
             // Update resource predictions.
-            if (node->fragment == Fragment::WRAP_V && node->subs[0]->GetType() << "x"_mst) {
+            if (node->Fragment() == Fragment::WRAP_V && node->Subs()[0]->GetType() << "x"_mst) {
                 ops += 1;
                 scriptsize += 1;
             }
@@ -1167,28 +1167,28 @@ void TestNode(const MsCtx script_ctx, const NodeRef& node, FuzzedDataProvider& p
         return sig_ptr != nullptr && sig_ptr->second;
     };
     bool satisfiable = node->IsSatisfiable([&](const Node& node) -> bool {
-        switch (node.fragment) {
+        switch (node.Fragment()) {
         case Fragment::PK_K:
         case Fragment::PK_H:
-            return is_key_satisfiable(node.keys[0]);
+            return is_key_satisfiable(node.Keys()[0]);
         case Fragment::MULTI:
         case Fragment::MULTI_A: {
-            size_t sats = std::count_if(node.keys.begin(), node.keys.end(), [&](const auto& key) {
+            size_t sats = std::ranges::count_if(node.Keys(), [&](const auto& key) {
                 return size_t(is_key_satisfiable(key));
             });
-            return sats >= node.k;
+            return sats >= node.K();
         }
         case Fragment::OLDER:
         case Fragment::AFTER:
-            return node.k & 1;
+            return node.K() & 1;
         case Fragment::SHA256:
-            return TEST_DATA.sha256_preimages.contains(node.data);
+            return TEST_DATA.sha256_preimages.contains(node.Data());
         case Fragment::HASH256:
-            return TEST_DATA.hash256_preimages.contains(node.data);
+            return TEST_DATA.hash256_preimages.contains(node.Data());
         case Fragment::RIPEMD160:
-            return TEST_DATA.ripemd160_preimages.contains(node.data);
+            return TEST_DATA.ripemd160_preimages.contains(node.Data());
         case Fragment::HASH160:
-            return TEST_DATA.hash160_preimages.contains(node.data);
+            return TEST_DATA.hash160_preimages.contains(node.Data());
         default:
             assert(false);
         }
diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp
index 5b7f58a0e72..9b414ca9a4f 100644
--- a/src/test/miniscript_tests.cpp
+++ b/src/test/miniscript_tests.cpp
@@ -305,19 +305,19 @@ std::set<Challenge> FindChallenges(const Node* root)
         const auto* ref{stack.back()};
         stack.pop_back();
 
-        for (const auto& key : ref->keys) {
+        for (const auto& key : ref->Keys()) {
             chal.emplace(ChallengeType::PK, ChallengeNumber(key));
         }
-        switch (ref->fragment) {
-        case Fragment::OLDER: chal.emplace(ChallengeType::OLDER, ref->k); break;
-        case Fragment::AFTER: chal.emplace(ChallengeType::AFTER, ref->k); break;
-        case Fragment::SHA256: chal.emplace(ChallengeType::SHA256, ChallengeNumber(ref->data)); break;
-        case Fragment::RIPEMD160: chal.emplace(ChallengeType::RIPEMD160, ChallengeNumber(ref->data)); break;
-        case Fragment::HASH256: chal.emplace(ChallengeType::HASH256, ChallengeNumber(ref->data)); break;
-        case Fragment::HASH160: chal.emplace(ChallengeType::HASH160, ChallengeNumber(ref->data)); break;
+        switch (ref->Fragment()) {
+        case Fragment::OLDER: chal.emplace(ChallengeType::OLDER, ref->K()); break;
+        case Fragment::AFTER: chal.emplace(ChallengeType::AFTER, ref->K()); break;
+        case Fragment::SHA256: chal.emplace(ChallengeType::SHA256, ChallengeNumber(ref->Data())); break;
+        case Fragment::RIPEMD160: chal.emplace(ChallengeType::RIPEMD160, ChallengeNumber(ref->Data())); break;
+        case Fragment::HASH256: chal.emplace(ChallengeType::HASH256, ChallengeNumber(ref->Data())); break;
+        case Fragment::HASH160: chal.emplace(ChallengeType::HASH160, ChallengeNumber(ref->Data())); break;
         default: break;
         }
-        for (const auto& sub : ref->subs) {
+        for (const auto& sub : ref->Subs()) {
             stack.push_back(sub.get());
         }
     }

From e55b23c170eb1a80a71e2de8b48cf8a0aebda843 Mon Sep 17 00:00:00 2001
From: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Date: Tue, 24 Jun 2025 09:46:52 +0200
Subject: [PATCH 3/7] refactor(miniscript): Remove Node::subs mutability

---
 src/script/descriptor.cpp | 2 +-
 src/script/miniscript.h   | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 390e7345515..8769a262f58 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -2566,7 +2566,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 decltype(node)::element_type* insane_node = node.get();
                 if (const auto sub = node->FindInsaneSub()) insane_node = sub;
                 error = *insane_node->ToString(parser);
                 if (!insane_node->IsValid()) {
diff --git a/src/script/miniscript.h b/src/script/miniscript.h
index 1c5f7a8c0f4..92925396469 100644
--- a/src/script/miniscript.h
+++ b/src/script/miniscript.h
@@ -191,11 +191,11 @@ inline consteval Type operator""_mst(const char* c, size_t l)
 using Opcode = std::pair<opcodetype, std::vector<unsigned char>>;
 
 template<typename Key> class Node;
-template<typename Key> using NodeRef = std::unique_ptr<const Node<Key>>;
+template<typename Key> using NodeRef = std::unique_ptr<Node<Key>>;
 
 //! Construct a miniscript node as a unique_ptr.
 template<typename Key, typename... Args>
-NodeRef<Key> MakeNodeRef(Args&&... args) { return std::make_unique<const Node<Key>>(std::forward<Args>(args)...); }
+NodeRef<Key> MakeNodeRef(Args&&... args) { return std::make_unique<Node<Key>>(std::forward<Args>(args)...); }
 
 //! Unordered traversal of a miniscript node tree.
 template <typename Key, std::invocable<const Node<Key>&> Fn>
@@ -545,7 +545,7 @@ class Node
     //! The data bytes in this expression (only for HASH160/HASH256/SHA256/RIPEMD160).
     std::vector<unsigned char> data;
     //! Subexpressions (for WRAP_*/AND_*/OR_*/ANDOR/THRESH)
-    mutable std::vector<NodeRef<Key>> subs;
+    std::vector<NodeRef<Key>> subs;
     //! The Script context for this node. Either P2WSH or Tapscript.
     MiniscriptContext m_script_ctx;
 

From 15fb34de41cb069e2bad93a64722bdb32ff00690 Mon Sep 17 00:00:00 2001
From: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Date: Wed, 25 Jun 2025 20:19:19 +0200
Subject: [PATCH 4/7] refactor(miniscript): Remove superfluous
 unique_ptr-indirection

Functional parity is achieved through making Node move-able.

Unfortunately ~Node() now needs to have the recursion linter disabled, as it is unable to figure out that recursion stops 1 level down. The former smart pointers must have been circumventing the linter somehow.

NodeRef & MakeNodeRef() are deleted in the following commit (broken out to facilitate review).
---
 src/script/descriptor.cpp     |  32 ++---
 src/script/miniscript.h       | 243 ++++++++++++++++++----------------
 src/test/fuzz/miniscript.cpp  |  40 +++---
 src/test/miniscript_tests.cpp |  48 +++----
 4 files changed, 189 insertions(+), 174 deletions(-)

diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
index 8769a262f58..8558c8e29d7 100644
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -1584,13 +1584,13 @@ public:
 class MiniscriptDescriptor final : public DescriptorImpl
 {
 private:
-    miniscript::NodeRef<uint32_t> m_node;
+    miniscript::Node<uint32_t> m_node;
 
 protected:
     std::vector<CScript> MakeScripts(const std::vector<CPubKey>& keys, std::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);
@@ -1598,15 +1598,15 @@ protected:
                 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)
+    MiniscriptDescriptor(std::vector<std::unique_ptr<PubkeyProvider>> providers, miniscript::Node<uint32_t>&& node)
         : DescriptorImpl(std::move(providers), "?"), m_node(std::move(node))
     {
         // Traverse miniscript tree for unsafe use of older()
-        miniscript::ForEachNode(*m_node, [&](const miniscript::Node<uint32_t>& node) {
+        miniscript::ForEachNode(m_node, [&](const miniscript::Node<uint32_t>& node) {
             if (node.Fragment() == miniscript::Fragment::OLDER) {
                 const uint32_t raw = node.K();
                 const uint32_t value_part = raw & ~CTxIn::SEQUENCE_LOCKTIME_TYPE_FLAG;
@@ -1626,7 +1626,7 @@ public:
                         const DescriptorCache* cache = nullptr) const override
     {
         bool has_priv_key{false};
-        auto res = m_node->ToString(StringMaker(arg, m_pubkey_args, type, cache), has_priv_key);
+        auto res = m_node.ToString(StringMaker(arg, m_pubkey_args, type, cache), has_priv_key);
         if (res) out = *res;
         if (type == StringType::PRIVATE) {
             Assume(res.has_value());
@@ -1639,15 +1639,17 @@ public:
     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 {
+    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();
+    std::optional<int64_t> MaxSatisfactionElems() const override
+    {
+        return m_node.GetStackSize();
     }
 
     std::unique_ptr<DescriptorImpl> Clone() const override
@@ -1657,7 +1659,7 @@ public:
         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());
     }
 };
 
@@ -2566,7 +2568,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.
-                const decltype(node)::element_type* insane_node = node.get();
+                const auto* insane_node = &node.value();
                 if (const auto sub = node->FindInsaneSub()) insane_node = sub;
                 error = *insane_node->ToString(parser);
                 if (!insane_node->IsValid()) {
@@ -2575,7 +2577,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.value() && !insane_node->NeedsSignature()) {
                         error += ": witnesses without signature exist";
                     } else if (!insane_node->CheckTimeLocksMix()) {
                         error += ": contains mixes of timelocks expressed in blocks and seconds";
@@ -2775,7 +2777,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));
         }
     }
 
diff --git a/src/script/miniscript.h b/src/script/miniscript.h
index 92925396469..6e59fa5119f 100644
--- a/src/script/miniscript.h
+++ b/src/script/miniscript.h
@@ -191,11 +191,11 @@ inline consteval Type operator""_mst(const char* c, size_t l)
 using Opcode = std::pair<opcodetype, std::vector<unsigned char>>;
 
 template<typename Key> class Node;
-template<typename Key> using NodeRef = std::unique_ptr<Node<Key>>;
+template<typename Key> using NodeRef = Node<Key>; // <- TODO: Remove in next commit.
 
-//! Construct a miniscript node as a unique_ptr.
+//! Construct a miniscript node (TODO: remove in next commit).
 template<typename Key, typename... Args>
-NodeRef<Key> MakeNodeRef(Args&&... args) { return std::make_unique<Node<Key>>(std::forward<Args>(args)...); }
+Node<Key> MakeNodeRef(Args&&... args) { return Node<Key>(std::forward<Args>(args)...); }
 
 //! Unordered traversal of a miniscript node tree.
 template <typename Key, std::invocable<const Node<Key>&> Fn>
@@ -207,7 +207,7 @@ void ForEachNode(const Node<Key>& root, Fn&& fn)
         std::invoke(fn, node);
         stack.pop_back();
         for (const auto& sub : node.Subs()) {
-            stack.emplace_back(*sub);
+            stack.emplace_back(sub);
         }
     }
 }
@@ -550,6 +550,8 @@ class Node
     MiniscriptContext m_script_ctx;
 
 public:
+    // Permit 1 level deep recursion since we own instances of our own type.
+    // NOLINTBEGIN(misc-no-recursion)
     ~Node()
     {
         // Destroy the subexpressions iteratively after moving out their
@@ -558,23 +560,25 @@ public:
         while (!subs.empty()) {
             auto node = std::move(subs.back());
             subs.pop_back();
-            while (!node->subs.empty()) {
-                subs.push_back(std::move(node->subs.back()));
-                node->subs.pop_back();
+            while (!node.subs.empty()) {
+                subs.push_back(std::move(node.subs.back()));
+                node.subs.pop_back();
             }
         }
     }
+    // NOLINTEND(misc-no-recursion)
 
-    NodeRef<Key> Clone() const
+    Node<Key> Clone() const
     {
         // Use TreeEval() to avoid a stack-overflow due to recursion
         auto upfn = [](const Node& node, std::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));
+            for (auto& child : children) {
+                // It's fine to move from children as they are new nodes having
+                // been produced by calling this function one level down.
+                new_subs.push_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 Node{internal::NoDupCheck{}, node.m_script_ctx, node.fragment, std::move(new_subs), node.keys, node.data, node.k};
         };
         return TreeEval<NodeRef<Key>>(upfn);
     }
@@ -610,12 +614,13 @@ private:
         : 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 {
+    size_t CalcScriptLen() const
+    {
         size_t subsize = 0;
         for (const auto& sub : subs) {
-            subsize += sub->ScriptSize();
+            subsize += sub.ScriptSize();
         }
-        Type sub0type = subs.size() > 0 ? subs[0]->GetType() : ""_mst;
+        Type sub0type = subs.size() > 0 ? subs[0].GetType() : ""_mst;
         return internal::ComputeScriptLen(fragment, sub0type, subsize, k, subs.size(), keys.size(), m_script_ctx);
     }
 
@@ -686,7 +691,7 @@ private:
                  * that child (and all earlier children) will be at the end of `results`. */
                 size_t child_index = stack.back().expanded++;
                 State child_state = downfn(stack.back().state, node, child_index);
-                stack.emplace_back(*node.subs[child_index], 0, std::move(child_state));
+                stack.emplace_back(node.subs[child_index], 0, std::move(child_state));
                 continue;
             }
             // Invoke upfn with the last node.subs.size() elements of results as input.
@@ -764,7 +769,7 @@ private:
             if (b.subs.size() < a.subs.size()) return 1;
             size_t n = a.subs.size();
             for (size_t i = 0; i < n; ++i) {
-                queue.emplace_back(*a.subs[n - 1 - i], *b.subs[n - 1 - i]);
+                queue.emplace_back(a.subs[n - 1 - i], b.subs[n - 1 - i]);
             }
         }
         return 0;
@@ -777,12 +782,12 @@ private:
         // THRESH has a variable number of subexpressions
         std::vector<Type> sub_types;
         if (fragment == Fragment::THRESH) {
-            for (const auto& sub : subs) sub_types.push_back(sub->GetType());
+            for (const auto& sub : subs) sub_types.push_back(sub.GetType());
         }
         // All other nodes than THRESH can be computed just from the types of the 0-3 subexpressions.
-        Type x = subs.size() > 0 ? subs[0]->GetType() : ""_mst;
-        Type y = subs.size() > 1 ? subs[1]->GetType() : ""_mst;
-        Type z = subs.size() > 2 ? subs[2]->GetType() : ""_mst;
+        Type x = subs.size() > 0 ? subs[0].GetType() : ""_mst;
+        Type y = subs.size() > 1 ? subs[1].GetType() : ""_mst;
+        Type z = subs.size() > 2 ? subs[2].GetType() : ""_mst;
 
         return SanitizeType(ComputeType(fragment, x, y, z, sub_types, k, data.size(), subs.size(), keys.size(), m_script_ctx));
     }
@@ -821,7 +826,7 @@ public:
                 case Fragment::WRAP_C: return BuildScript(std::move(subs[0]), verify ? OP_CHECKSIGVERIFY : OP_CHECKSIG);
                 case Fragment::WRAP_D: return BuildScript(OP_DUP, OP_IF, subs[0], OP_ENDIF);
                 case Fragment::WRAP_V: {
-                    if (node.subs[0]->GetType() << "x"_mst) {
+                    if (node.subs[0].GetType() << "x"_mst) {
                         return BuildScript(std::move(subs[0]), OP_VERIFY);
                     } else {
                         return std::move(subs[0]);
@@ -883,9 +888,9 @@ public:
                     node.fragment == Fragment::WRAP_D || node.fragment == Fragment::WRAP_V ||
                     node.fragment == Fragment::WRAP_J || node.fragment == Fragment::WRAP_N ||
                     node.fragment == Fragment::WRAP_C ||
-                    (node.fragment == Fragment::AND_V && node.subs[1]->fragment == Fragment::JUST_1) ||
-                    (node.fragment == Fragment::OR_I && node.subs[0]->fragment == Fragment::JUST_0) ||
-                    (node.fragment == Fragment::OR_I && node.subs[1]->fragment == Fragment::JUST_0));
+                    (node.fragment == Fragment::AND_V && node.subs[1].fragment == Fragment::JUST_1) ||
+                    (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};
@@ -903,15 +908,15 @@ public:
                 case Fragment::WRAP_A: return "a" + std::move(subs[0]);
                 case Fragment::WRAP_S: return "s" + std::move(subs[0]);
                 case Fragment::WRAP_C:
-                    if (node.subs[0]->fragment == Fragment::PK_K) {
+                    if (node.subs[0].fragment == Fragment::PK_K) {
                         // pk(K) is syntactic sugar for c:pk_k(K)
-                        auto key_str = 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) {
+                    if (node.subs[0].fragment == Fragment::PK_H) {
                         // pkh(K) is syntactic sugar for c:pk_h(K)
-                        auto key_str = 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) + ")";
                     }
@@ -922,11 +927,11 @@ public:
                 case Fragment::WRAP_N: return "n" + std::move(subs[0]);
                 case Fragment::AND_V:
                     // t:X is syntactic sugar for and_v(X,1).
-                    if (node.subs[1]->fragment == Fragment::JUST_1) return "t" + std::move(subs[0]);
+                    if (node.subs[1].fragment == Fragment::JUST_1) return "t" + std::move(subs[0]);
                     break;
                 case Fragment::OR_I:
-                    if (node.subs[0]->fragment == Fragment::JUST_0) return "l" + std::move(subs[1]);
-                    if (node.subs[1]->fragment == Fragment::JUST_0) return "u" + std::move(subs[0]);
+                    if (node.subs[0].fragment == Fragment::JUST_0) return "l" + std::move(subs[1]);
+                    if (node.subs[1].fragment == Fragment::JUST_0) return "u" + std::move(subs[0]);
                     break;
                 default: break;
             }
@@ -957,7 +962,7 @@ public:
                 case Fragment::OR_I: return std::move(ret) + "or_i(" + std::move(subs[0]) + "," + std::move(subs[1]) + ")";
                 case Fragment::ANDOR:
                     // and_n(X,Y) is syntactic sugar for andor(X,Y,0).
-                    if (node.subs[2]->fragment == Fragment::JUST_0) return std::move(ret) + "and_n(" + std::move(subs[0]) + "," + std::move(subs[1]) + ")";
+                    if (node.subs[2].fragment == Fragment::JUST_0) return std::move(ret) + "and_n(" + std::move(subs[0]) + "," + std::move(subs[1]) + ")";
                     return std::move(ret) + "andor(" + std::move(subs[0]) + "," + std::move(subs[1]) + "," + std::move(subs[2]) + ")";
                 case Fragment::MULTI: {
                     CHECK_NONFATAL(!is_tapscript);
@@ -1007,59 +1012,59 @@ private:
             case Fragment::RIPEMD160:
             case Fragment::HASH256:
             case Fragment::HASH160: return {4, 0, {}};
-            case Fragment::AND_V: return {subs[0]->ops.count + subs[1]->ops.count, subs[0]->ops.sat + subs[1]->ops.sat, {}};
+            case Fragment::AND_V: return {subs[0].ops.count + subs[1].ops.count, subs[0].ops.sat + subs[1].ops.sat, {}};
             case Fragment::AND_B: {
-                const auto count{1 + subs[0]->ops.count + subs[1]->ops.count};
-                const auto sat{subs[0]->ops.sat + subs[1]->ops.sat};
-                const auto dsat{subs[0]->ops.dsat + subs[1]->ops.dsat};
+                const auto count{1 + subs[0].ops.count + subs[1].ops.count};
+                const auto sat{subs[0].ops.sat + subs[1].ops.sat};
+                const auto dsat{subs[0].ops.dsat + subs[1].ops.dsat};
                 return {count, sat, dsat};
             }
             case Fragment::OR_B: {
-                const auto count{1 + subs[0]->ops.count + subs[1]->ops.count};
-                const auto sat{(subs[0]->ops.sat + subs[1]->ops.dsat) | (subs[1]->ops.sat + subs[0]->ops.dsat)};
-                const auto dsat{subs[0]->ops.dsat + subs[1]->ops.dsat};
+                const auto count{1 + subs[0].ops.count + subs[1].ops.count};
+                const auto sat{(subs[0].ops.sat + subs[1].ops.dsat) | (subs[1].ops.sat + subs[0].ops.dsat)};
+                const auto dsat{subs[0].ops.dsat + subs[1].ops.dsat};
                 return {count, sat, dsat};
             }
             case Fragment::OR_D: {
-                const auto count{3 + subs[0]->ops.count + subs[1]->ops.count};
-                const auto sat{subs[0]->ops.sat | (subs[1]->ops.sat + subs[0]->ops.dsat)};
-                const auto dsat{subs[0]->ops.dsat + subs[1]->ops.dsat};
+                const auto count{3 + subs[0].ops.count + subs[1].ops.count};
+                const auto sat{subs[0].ops.sat | (subs[1].ops.sat + subs[0].ops.dsat)};
+                const auto dsat{subs[0].ops.dsat + subs[1].ops.dsat};
                 return {count, sat, dsat};
             }
             case Fragment::OR_C: {
-                const auto count{2 + subs[0]->ops.count + subs[1]->ops.count};
-                const auto sat{subs[0]->ops.sat | (subs[1]->ops.sat + subs[0]->ops.dsat)};
+                const auto count{2 + subs[0].ops.count + subs[1].ops.count};
+                const auto sat{subs[0].ops.sat | (subs[1].ops.sat + subs[0].ops.dsat)};
                 return {count, sat, {}};
             }
             case Fragment::OR_I: {
-                const auto count{3 + subs[0]->ops.count + subs[1]->ops.count};
-                const auto sat{subs[0]->ops.sat | subs[1]->ops.sat};
-                const auto dsat{subs[0]->ops.dsat | subs[1]->ops.dsat};
+                const auto count{3 + subs[0].ops.count + subs[1].ops.count};
+                const auto sat{subs[0].ops.sat | subs[1].ops.sat};
+                const auto dsat{subs[0].ops.dsat | subs[1].ops.dsat};
                 return {count, sat, dsat};
             }
             case Fragment::ANDOR: {
-                const auto count{3 + subs[0]->ops.count + subs[1]->ops.count + subs[2]->ops.count};
-                const auto sat{(subs[1]->ops.sat + subs[0]->ops.sat) | (subs[0]->ops.dsat + subs[2]->ops.sat)};
-                const auto dsat{subs[0]->ops.dsat + subs[2]->ops.dsat};
+                const auto count{3 + subs[0].ops.count + subs[1].ops.count + subs[2].ops.count};
+                const auto sat{(subs[1].ops.sat + subs[0].ops.sat) | (subs[0].ops.dsat + subs[2].ops.sat)};
+                const auto dsat{subs[0].ops.dsat + subs[2].ops.dsat};
                 return {count, sat, dsat};
             }
             case Fragment::MULTI: return {1, (uint32_t)keys.size(), (uint32_t)keys.size()};
             case Fragment::MULTI_A: return {(uint32_t)keys.size() + 1, 0, 0};
             case Fragment::WRAP_S:
             case Fragment::WRAP_C:
-            case Fragment::WRAP_N: return {1 + subs[0]->ops.count, subs[0]->ops.sat, subs[0]->ops.dsat};
-            case Fragment::WRAP_A: return {2 + subs[0]->ops.count, subs[0]->ops.sat, subs[0]->ops.dsat};
-            case Fragment::WRAP_D: return {3 + subs[0]->ops.count, subs[0]->ops.sat, 0};
-            case Fragment::WRAP_J: return {4 + subs[0]->ops.count, subs[0]->ops.sat, 0};
-            case Fragment::WRAP_V: return {subs[0]->ops.count + (subs[0]->GetType() << "x"_mst), subs[0]->ops.sat, {}};
+            case Fragment::WRAP_N: return {1 + subs[0].ops.count, subs[0].ops.sat, subs[0].ops.dsat};
+            case Fragment::WRAP_A: return {2 + subs[0].ops.count, subs[0].ops.sat, subs[0].ops.dsat};
+            case Fragment::WRAP_D: return {3 + subs[0].ops.count, subs[0].ops.sat, 0};
+            case Fragment::WRAP_J: return {4 + subs[0].ops.count, subs[0].ops.sat, 0};
+            case Fragment::WRAP_V: return {subs[0].ops.count + (subs[0].GetType() << "x"_mst), subs[0].ops.sat, {}};
             case Fragment::THRESH: {
                 uint32_t count = 0;
                 auto sats = Vector(internal::MaxInt<uint32_t>(0));
                 for (const auto& sub : subs) {
-                    count += sub->ops.count + 1;
-                    auto next_sats = Vector(sats[0] + sub->ops.dsat);
-                    for (size_t j = 1; j < sats.size(); ++j) next_sats.push_back((sats[j] + sub->ops.dsat) | (sats[j - 1] + sub->ops.sat));
-                    next_sats.push_back(sats[sats.size() - 1] + sub->ops.sat);
+                    count += sub.ops.count + 1;
+                    auto next_sats = Vector(sats[0] + sub.ops.dsat);
+                    for (size_t j = 1; j < sats.size(); ++j) next_sats.push_back((sats[j] + sub.ops.dsat) | (sats[j - 1] + sub.ops.sat));
+                    next_sats.push_back(sats[sats.size() - 1] + sub.ops.sat);
                     sats = std::move(next_sats);
                 }
                 assert(k < sats.size());
@@ -1086,48 +1091,48 @@ private:
                 {}
             };
             case Fragment::ANDOR: {
-                const auto& x{subs[0]->ss};
-                const auto& y{subs[1]->ss};
-                const auto& z{subs[2]->ss};
+                const auto& x{subs[0].ss};
+                const auto& y{subs[1].ss};
+                const auto& z{subs[2].ss};
                 return {
                     (x.Sat() + SatInfo::If() + y.Sat()) | (x.Dsat() + SatInfo::If() + z.Sat()),
                     x.Dsat() + SatInfo::If() + z.Dsat()
                 };
             }
             case Fragment::AND_V: {
-                const auto& x{subs[0]->ss};
-                const auto& y{subs[1]->ss};
+                const auto& x{subs[0].ss};
+                const auto& y{subs[1].ss};
                 return {x.Sat() + y.Sat(), {}};
             }
             case Fragment::AND_B: {
-                const auto& x{subs[0]->ss};
-                const auto& y{subs[1]->ss};
+                const auto& x{subs[0].ss};
+                const auto& y{subs[1].ss};
                 return {x.Sat() + y.Sat() + SatInfo::BinaryOp(), x.Dsat() + y.Dsat() + SatInfo::BinaryOp()};
             }
             case Fragment::OR_B: {
-                const auto& x{subs[0]->ss};
-                const auto& y{subs[1]->ss};
+                const auto& x{subs[0].ss};
+                const auto& y{subs[1].ss};
                 return {
                     ((x.Sat() + y.Dsat()) | (x.Dsat() + y.Sat())) + SatInfo::BinaryOp(),
                     x.Dsat() + y.Dsat() + SatInfo::BinaryOp()
                 };
             }
             case Fragment::OR_C: {
-                const auto& x{subs[0]->ss};
-                const auto& y{subs[1]->ss};
+                const auto& x{subs[0].ss};
+                const auto& y{subs[1].ss};
                 return {(x.Sat() + SatInfo::If()) | (x.Dsat() + SatInfo::If() + y.Sat()), {}};
             }
             case Fragment::OR_D: {
-                const auto& x{subs[0]->ss};
-                const auto& y{subs[1]->ss};
+                const auto& x{subs[0].ss};
+                const auto& y{subs[1].ss};
                 return {
                     (x.Sat() + SatInfo::OP_IFDUP(true) + SatInfo::If()) | (x.Dsat() + SatInfo::OP_IFDUP(false) + SatInfo::If() + y.Sat()),
                     x.Dsat() + SatInfo::OP_IFDUP(false) + SatInfo::If() + y.Dsat()
                 };
             }
             case Fragment::OR_I: {
-                const auto& x{subs[0]->ss};
-                const auto& y{subs[1]->ss};
+                const auto& x{subs[0].ss};
+                const auto& y{subs[1].ss};
                 return {SatInfo::If() + (x.Sat() | y.Sat()), SatInfo::If() + (x.Dsat() | y.Dsat())};
             }
             // multi(k, key1, key2, ..., key_n) starts off with k+1 stack elements (a 0, plus k
@@ -1141,18 +1146,18 @@ private:
             case Fragment::MULTI_A: return {SatInfo(keys.size() - 1, keys.size())};
             case Fragment::WRAP_A:
             case Fragment::WRAP_N:
-            case Fragment::WRAP_S: return subs[0]->ss;
+            case Fragment::WRAP_S: return subs[0].ss;
             case Fragment::WRAP_C: return {
-                subs[0]->ss.Sat() + SatInfo::OP_CHECKSIG(),
-                subs[0]->ss.Dsat() + SatInfo::OP_CHECKSIG()
+                subs[0].ss.Sat() + SatInfo::OP_CHECKSIG(),
+                subs[0].ss.Dsat() + SatInfo::OP_CHECKSIG()
             };
             case Fragment::WRAP_D: return {
-                SatInfo::OP_DUP() + SatInfo::If() + subs[0]->ss.Sat(),
+                SatInfo::OP_DUP() + SatInfo::If() + subs[0].ss.Sat(),
                 SatInfo::OP_DUP() + SatInfo::If()
             };
-            case Fragment::WRAP_V: return {subs[0]->ss.Sat() + SatInfo::OP_VERIFY(), {}};
+            case Fragment::WRAP_V: return {subs[0].ss.Sat() + SatInfo::OP_VERIFY(), {}};
             case Fragment::WRAP_J: return {
-                SatInfo::OP_SIZE() + SatInfo::OP_0NOTEQUAL() + SatInfo::If() + subs[0]->ss.Sat(),
+                SatInfo::OP_SIZE() + SatInfo::OP_0NOTEQUAL() + SatInfo::If() + subs[0].ss.Sat(),
                 SatInfo::OP_SIZE() + SatInfo::OP_0NOTEQUAL() + SatInfo::If()
             };
             case Fragment::THRESH: {
@@ -1163,13 +1168,13 @@ private:
                     // element i we need to add OP_ADD (if i>0).
                     auto add = i ? SatInfo::BinaryOp() : SatInfo::Empty();
                     // Construct a variable that will become the next sats, starting with index 0.
-                    auto next_sats = Vector(sats[0] + subs[i]->ss.Dsat() + add);
+                    auto next_sats = Vector(sats[0] + subs[i].ss.Dsat() + add);
                     // Then loop to construct next_sats[1..i].
                     for (size_t j = 1; j < sats.size(); ++j) {
-                        next_sats.push_back(((sats[j] + subs[i]->ss.Dsat()) | (sats[j - 1] + subs[i]->ss.Sat())) + add);
+                        next_sats.push_back(((sats[j] + subs[i].ss.Dsat()) | (sats[j - 1] + subs[i].ss.Sat())) + add);
                     }
                     // Finally construct next_sats[i+1].
-                    next_sats.push_back(sats[sats.size() - 1] + subs[i]->ss.Sat() + add);
+                    next_sats.push_back(sats[sats.size() - 1] + subs[i].ss.Sat() + add);
                     // Switch over.
                     sats = std::move(next_sats);
                 }
@@ -1199,35 +1204,35 @@ private:
             case Fragment::HASH256:
             case Fragment::HASH160: return {1 + 32, {}};
             case Fragment::ANDOR: {
-                const auto sat{(subs[0]->ws.sat + subs[1]->ws.sat) | (subs[0]->ws.dsat + subs[2]->ws.sat)};
-                const auto dsat{subs[0]->ws.dsat + subs[2]->ws.dsat};
+                const auto sat{(subs[0].ws.sat + subs[1].ws.sat) | (subs[0].ws.dsat + subs[2].ws.sat)};
+                const auto dsat{subs[0].ws.dsat + subs[2].ws.dsat};
                 return {sat, dsat};
             }
-            case Fragment::AND_V: return {subs[0]->ws.sat + subs[1]->ws.sat, {}};
-            case Fragment::AND_B: return {subs[0]->ws.sat + subs[1]->ws.sat, subs[0]->ws.dsat + subs[1]->ws.dsat};
+            case Fragment::AND_V: return {subs[0].ws.sat + subs[1].ws.sat, {}};
+            case Fragment::AND_B: return {subs[0].ws.sat + subs[1].ws.sat, subs[0].ws.dsat + subs[1].ws.dsat};
             case Fragment::OR_B: {
-                const auto sat{(subs[0]->ws.dsat + subs[1]->ws.sat) | (subs[0]->ws.sat + subs[1]->ws.dsat)};
-                const auto dsat{subs[0]->ws.dsat + subs[1]->ws.dsat};
+                const auto sat{(subs[0].ws.dsat + subs[1].ws.sat) | (subs[0].ws.sat + subs[1].ws.dsat)};
+                const auto dsat{subs[0].ws.dsat + subs[1].ws.dsat};
                 return {sat, dsat};
             }
-            case Fragment::OR_C: return {subs[0]->ws.sat | (subs[0]->ws.dsat + subs[1]->ws.sat), {}};
-            case Fragment::OR_D: return {subs[0]->ws.sat | (subs[0]->ws.dsat + subs[1]->ws.sat), subs[0]->ws.dsat + subs[1]->ws.dsat};
-            case Fragment::OR_I: return {(subs[0]->ws.sat + 1 + 1) | (subs[1]->ws.sat + 1), (subs[0]->ws.dsat + 1 + 1) | (subs[1]->ws.dsat + 1)};
+            case Fragment::OR_C: return {subs[0].ws.sat | (subs[0].ws.dsat + subs[1].ws.sat), {}};
+            case Fragment::OR_D: return {subs[0].ws.sat | (subs[0].ws.dsat + subs[1].ws.sat), subs[0].ws.dsat + subs[1].ws.dsat};
+            case Fragment::OR_I: return {(subs[0].ws.sat + 1 + 1) | (subs[1].ws.sat + 1), (subs[0].ws.dsat + 1 + 1) | (subs[1].ws.dsat + 1)};
             case Fragment::MULTI: return {k * sig_size + 1, k + 1};
             case Fragment::MULTI_A: return {k * sig_size + static_cast<uint32_t>(keys.size()) - k, static_cast<uint32_t>(keys.size())};
             case Fragment::WRAP_A:
             case Fragment::WRAP_N:
             case Fragment::WRAP_S:
-            case Fragment::WRAP_C: return subs[0]->ws;
-            case Fragment::WRAP_D: return {1 + 1 + subs[0]->ws.sat, 1};
-            case Fragment::WRAP_V: return {subs[0]->ws.sat, {}};
-            case Fragment::WRAP_J: return {subs[0]->ws.sat, 1};
+            case Fragment::WRAP_C: return subs[0].ws;
+            case Fragment::WRAP_D: return {1 + 1 + subs[0].ws.sat, 1};
+            case Fragment::WRAP_V: return {subs[0].ws.sat, {}};
+            case Fragment::WRAP_J: return {subs[0].ws.sat, 1};
             case Fragment::THRESH: {
                 auto sats = Vector(internal::MaxInt<uint32_t>(0));
                 for (const auto& sub : subs) {
-                    auto next_sats = Vector(sats[0] + sub->ws.dsat);
-                    for (size_t j = 1; j < sats.size(); ++j) next_sats.push_back((sats[j] + sub->ws.dsat) | (sats[j - 1] + sub->ws.sat));
-                    next_sats.push_back(sats[sats.size() - 1] + sub->ws.sat);
+                    auto next_sats = Vector(sats[0] + sub.ws.dsat);
+                    for (size_t j = 1; j < sats.size(); ++j) next_sats.push_back((sats[j] + sub.ws.dsat) | (sats[j - 1] + sub.ws.sat));
+                    next_sats.push_back(sats[sats.size() - 1] + sub.ws.sat);
                     sats = std::move(next_sats);
                 }
                 assert(k < sats.size());
@@ -1741,6 +1746,10 @@ public:
     // Delete copy constructor and assignment operator, use Clone() instead
     Node(const Node&) = delete;
     Node& operator=(const Node&) = delete;
+
+    // subs is movable, circumventing recursion, so these are permitted.
+    Node(Node&&) noexcept = default;
+    Node& operator=(Node&&) noexcept = default;
 };
 
 namespace internal {
@@ -1844,8 +1853,8 @@ void BuildBack(const MiniscriptContext script_ctx, Fragment nt, std::vector<Node
  * 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(std::span<const char> in, const Ctx& ctx)
+template <typename Key, typename Ctx>
+inline std::optional<Node<Key>> Parse(std::span<const char> in, const Ctx& ctx)
 {
     using namespace script;
 
@@ -2125,7 +2134,7 @@ inline NodeRef<Key> Parse(std::span<const char> in, const Ctx& ctx)
             break;
         }
         case ParseContext::VERIFY: {
-            script_size += (constructed.back()->GetType() << "x"_mst);
+            script_size += (constructed.back().GetType() << "x"_mst);
             constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_V, Vector(std::move(constructed.back())));
             break;
         }
@@ -2214,10 +2223,10 @@ inline NodeRef<Key> Parse(std::span<const char> in, const Ctx& ctx)
     // Sanity checks on the produced miniscript
     assert(constructed.size() >= 1);
     CHECK_NONFATAL(constructed.size() == 1);
-    assert(constructed[0]->ScriptSize() == script_size);
+    assert(constructed[0].ScriptSize() == script_size);
     if (in.size() > 0) return {};
-    NodeRef<Key> tl_node = std::move(constructed.front());
-    tl_node->DuplicateKeyCheck(ctx);
+    Node<Key> tl_node{std::move(constructed.front())};
+    tl_node.DuplicateKeyCheck(ctx);
     return tl_node;
 }
 
@@ -2303,8 +2312,8 @@ enum class DecodeContext {
 };
 
 //! Parse a miniscript from a bitcoin script
-template<typename Key, typename Ctx, typename I>
-inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx)
+template <typename Key, typename Ctx, typename I>
+inline std::optional<Node<Key>> DecodeScript(I& in, I last, const Ctx& ctx)
 {
     // The two integers are used to hold state for thresh()
     std::vector<std::tuple<DecodeContext, int64_t, int64_t>> to_parse;
@@ -2316,7 +2325,7 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx)
 
     while (!to_parse.empty()) {
         // Exit early if the Miniscript is not going to be valid.
-        if (!constructed.empty() && !constructed.back()->IsValid()) return {};
+        if (!constructed.empty() && !constructed.back().IsValid()) return {};
 
         // Get the current context we are decoding within
         auto [cur_context, n, k] = to_parse.back();
@@ -2682,23 +2691,25 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx)
         }
     }
     if (constructed.size() != 1) return {};
-    NodeRef<Key> tl_node = std::move(constructed.front());
-    tl_node->DuplicateKeyCheck(ctx);
+    Node tl_node{std::move(constructed.front())};
+    tl_node.DuplicateKeyCheck(ctx);
     // Note that due to how ComputeType works (only assign the type to the node if the
     // subs' types are valid) this would fail if any node of tree is badly typed.
-    if (!tl_node->IsValidTopLevel()) return {};
+    if (!tl_node.IsValidTopLevel()) return {};
     return tl_node;
 }
 
 } // namespace internal
 
-template<typename Ctx>
-inline NodeRef<typename Ctx::Key> FromString(const std::string& str, const Ctx& ctx) {
+template <typename Ctx>
+inline std::optional<Node<typename Ctx::Key>> FromString(const std::string& str, const Ctx& ctx)
+{
     return internal::Parse<typename Ctx::Key>(str, ctx);
 }
 
-template<typename Ctx>
-inline NodeRef<typename Ctx::Key> FromScript(const CScript& script, const Ctx& ctx) {
+template <typename Ctx>
+inline std::optional<Node<typename Ctx::Key>> FromScript(const CScript& script, const Ctx& ctx)
+{
     using namespace internal;
     // A too large Script is necessarily invalid, don't bother parsing it.
     if (script.size() > MaxScriptSize(ctx.MsContext())) return {};
diff --git a/src/test/fuzz/miniscript.cpp b/src/test/fuzz/miniscript.cpp
index dc1b3c987d3..a122e8f368e 100644
--- a/src/test/fuzz/miniscript.cpp
+++ b/src/test/fuzz/miniscript.cpp
@@ -15,6 +15,7 @@
 #include <util/strencodings.h>
 
 #include <algorithm>
+#include <optional>
 
 namespace {
 
@@ -852,12 +853,13 @@ std::optional<NodeInfo> ConsumeNodeSmart(MsCtx script_ctx, FuzzedDataProvider& p
  * Generate a Miniscript node based on the fuzzer's input.
  *
  * - ConsumeNode is a function object taking a Type, and returning an std::optional<NodeInfo>.
- * - root_type is the required type properties of the constructed NodeRef.
+ * - root_type is the required type properties of the constructed Node.
  * - strict_valid sets whether ConsumeNode is expected to guarantee a NodeInfo that results in
- *   a NodeRef whose Type() matches the type fed to ConsumeNode.
+ *   a Node whose Type() matches the type fed to ConsumeNode.
  */
-template<typename F>
-NodeRef GenNode(MsCtx script_ctx, F ConsumeNode, Type root_type, bool strict_valid = false) {
+template <typename F>
+std::optional<Node> GenNode(MsCtx script_ctx, F ConsumeNode, Type root_type, bool strict_valid = false)
+{
     /** A stack of miniscript Nodes being built up. */
     std::vector<NodeRef> stack;
     /** The queue of instructions. */
@@ -972,26 +974,26 @@ NodeRef GenNode(MsCtx script_ctx, F ConsumeNode, Type root_type, bool strict_val
                 sub.push_back(std::move(*(stack.end() - info.subtypes.size() + i)));
             }
             stack.erase(stack.end() - info.subtypes.size(), stack.end());
-            // Construct new NodeRef.
-            NodeRef node;
-            if (info.keys.empty()) {
-                node = MakeNodeRef(script_ctx, info.fragment, std::move(sub), std::move(info.hash), info.k);
-            } else {
+            // Construct new Node.
+            Node node{[&] {
+                if (info.keys.empty()) {
+                    return Node{miniscript::internal::NoDupCheck{}, script_ctx, info.fragment, std::move(sub), std::move(info.hash), info.k};
+                }
                 assert(sub.empty());
                 assert(info.hash.empty());
-                node = MakeNodeRef(script_ctx, info.fragment, std::move(info.keys), info.k);
-            }
+                return Node{miniscript::internal::NoDupCheck{}, script_ctx, info.fragment, std::move(info.keys), info.k};
+            }()};
             // Verify acceptability.
-            if (!node || (node->GetType() & "KVWB"_mst) == ""_mst) {
+            if ((node.GetType() & "KVWB"_mst) == ""_mst) {
                 assert(!strict_valid);
                 return {};
             }
             if (!(type_needed == ""_mst)) {
-                assert(node->GetType() << type_needed);
+                assert(node.GetType() << type_needed);
             }
-            if (!node->IsValid()) return {};
+            if (!node.IsValid()) return {};
             // Update resource predictions.
-            if (node->Fragment() == Fragment::WRAP_V && node->Subs()[0]->GetType() << "x"_mst) {
+            if (node.Fragment() == Fragment::WRAP_V && node.Subs()[0].GetType() << "x"_mst) {
                 ops += 1;
                 scriptsize += 1;
             }
@@ -1005,9 +1007,9 @@ NodeRef GenNode(MsCtx script_ctx, F ConsumeNode, Type root_type, bool strict_val
         }
     }
     assert(stack.size() == 1);
-    assert(stack[0]->GetStaticOps() == ops);
-    assert(stack[0]->ScriptSize() == scriptsize);
-    stack[0]->DuplicateKeyCheck(KEY_COMP);
+    assert(stack[0].GetStaticOps() == ops);
+    assert(stack[0].ScriptSize() == scriptsize);
+    stack[0].DuplicateKeyCheck(KEY_COMP);
     return std::move(stack[0]);
 }
 
@@ -1032,7 +1034,7 @@ void SatisfactionToWitness(MsCtx ctx, CScriptWitness& witness, const CScript& sc
 }
 
 /** Perform various applicable tests on a miniscript Node. */
-void TestNode(const MsCtx script_ctx, const NodeRef& node, FuzzedDataProvider& provider)
+void TestNode(const MsCtx script_ctx, const std::optional<Node>& node, FuzzedDataProvider& provider)
 {
     if (!node) return;
 
diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp
index 9b414ca9a4f..a5f9a21d033 100644
--- a/src/test/miniscript_tests.cpp
+++ b/src/test/miniscript_tests.cpp
@@ -297,11 +297,11 @@ using miniscript::operator""_mst;
 using Node = miniscript::Node<CPubKey>;
 
 /** Compute all challenges (pubkeys, hashes, timelocks) that occur in a given Miniscript. */
-std::set<Challenge> FindChallenges(const Node* root)
+std::set<Challenge> FindChallenges(const Node& root)
 {
     std::set<Challenge> chal;
 
-    for (std::vector stack{root}; !stack.empty();) {
+    for (std::vector stack{&root}; !stack.empty();) {
         const auto* ref{stack.back()};
         stack.pop_back();
 
@@ -318,7 +318,7 @@ std::set<Challenge> FindChallenges(const Node* root)
         default: break;
         }
         for (const auto& sub : ref->Subs()) {
-            stack.push_back(sub.get());
+            stack.push_back(&sub);
         }
     }
     return chal;
@@ -347,8 +347,8 @@ void SatisfactionToWitness(miniscript::MiniscriptContext ctx, CScriptWitness& wi
 struct MiniScriptTest : BasicTestingSetup {
 /** Run random satisfaction tests. */
 void TestSatisfy(const KeyConverter& converter, const std::string& testcase, const NodeRef& node) {
-    auto script = node->ToScript(converter);
-    const auto challenges{FindChallenges(node.get())}; // Find all challenges in the generated miniscript.
+    auto script = node.ToScript(converter);
+    const auto challenges{FindChallenges(node)}; // Find all challenges in the generated miniscript.
     std::vector<Challenge> challist(challenges.begin(), challenges.end());
     for (int iter = 0; iter < 3; ++iter) {
         std::shuffle(challist.begin(), challist.end(), m_rng);
@@ -365,12 +365,12 @@ void TestSatisfy(const KeyConverter& converter, const std::string& testcase, con
 
             // Run malleable satisfaction algorithm.
             CScriptWitness witness_mal;
-            const bool mal_success = node->Satisfy(satisfier, witness_mal.stack, false) == miniscript::Availability::YES;
+            const bool mal_success = node.Satisfy(satisfier, witness_mal.stack, false) == miniscript::Availability::YES;
             SatisfactionToWitness(converter.MsContext(), witness_mal, script, builder);
 
             // Run non-malleable satisfaction algorithm.
             CScriptWitness witness_nonmal;
-            const bool nonmal_success = node->Satisfy(satisfier, witness_nonmal.stack, true) == miniscript::Availability::YES;
+            const bool nonmal_success = node.Satisfy(satisfier, witness_nonmal.stack, true) == miniscript::Availability::YES;
             // Compute witness size (excluding script push, control block, and witness count encoding).
             const uint64_t wit_size{GetSerializeSize(witness_nonmal.stack) - GetSizeOfCompactSize(witness_nonmal.stack.size())};
             SatisfactionToWitness(converter.MsContext(), witness_nonmal, script, builder);
@@ -379,23 +379,23 @@ void TestSatisfy(const KeyConverter& converter, const std::string& testcase, con
                 // Non-malleable satisfactions are bounded by the satisfaction size plus:
                 // - For P2WSH spends, the witness script
                 // - For Tapscript spends, both the witness script and the control block
-                const size_t max_stack_size{*node->GetStackSize() + 1 + miniscript::IsTapscript(converter.MsContext())};
+                const size_t max_stack_size{*node.GetStackSize() + 1 + miniscript::IsTapscript(converter.MsContext())};
                 BOOST_CHECK(witness_nonmal.stack.size() <= max_stack_size);
                 // If a non-malleable satisfaction exists, the malleable one must also exist, and be identical to it.
                 BOOST_CHECK(mal_success);
                 BOOST_CHECK(witness_nonmal.stack == witness_mal.stack);
-                assert(wit_size <= *node->GetWitnessSize());
+                assert(wit_size <= *node.GetWitnessSize());
 
                 // Test non-malleable satisfaction.
                 ScriptError serror;
                 bool res = VerifyScript(CScript(), script_pubkey, &witness_nonmal, STANDARD_SCRIPT_VERIFY_FLAGS, checker, &serror);
                 // Non-malleable satisfactions are guaranteed to be valid if ValidSatisfactions().
-                if (node->ValidSatisfactions()) BOOST_CHECK(res);
+                if (node.ValidSatisfactions()) BOOST_CHECK(res);
                 // More detailed: non-malleable satisfactions must be valid, or could fail with ops count error (if CheckOpsLimit failed),
                 // or with a stack size error (if CheckStackSize check fails).
                 BOOST_CHECK(res ||
-                            (!node->CheckOpsLimit() && serror == ScriptError::SCRIPT_ERR_OP_COUNT) ||
-                            (!node->CheckStackSize() && serror == ScriptError::SCRIPT_ERR_STACK_SIZE));
+                            (!node.CheckOpsLimit() && serror == ScriptError::SCRIPT_ERR_OP_COUNT) ||
+                            (!node.CheckStackSize() && serror == ScriptError::SCRIPT_ERR_STACK_SIZE));
             }
 
             if (mal_success && (!nonmal_success || witness_mal.stack != witness_nonmal.stack)) {
@@ -407,7 +407,7 @@ void TestSatisfy(const KeyConverter& converter, const std::string& testcase, con
                 BOOST_CHECK(res || serror == ScriptError::SCRIPT_ERR_OP_COUNT || serror == ScriptError::SCRIPT_ERR_STACK_SIZE);
             }
 
-            if (node->IsSane()) {
+            if (node.IsSane()) {
                 // For sane nodes, the two algorithms behave identically.
                 BOOST_CHECK_EQUAL(mal_success, nonmal_success);
             }
@@ -417,7 +417,7 @@ void TestSatisfy(const KeyConverter& converter, const std::string& testcase, con
             // For nonmalleable solutions this is only true if the added condition is PK;
             // for other conditions, adding one may make an valid satisfaction become malleable. If the script
             // is sane, this cannot happen however.
-            if (node->IsSane() || add < 0 || challist[add].first == ChallengeType::PK) {
+            if (node.IsSane() || add < 0 || challist[add].first == ChallengeType::PK) {
                 BOOST_CHECK(nonmal_success >= prev_nonmal_success);
             }
             // Remember results for the next added challenge.
@@ -425,11 +425,11 @@ void TestSatisfy(const KeyConverter& converter, const std::string& testcase, con
             prev_nonmal_success = nonmal_success;
         }
 
-        bool satisfiable = node->IsSatisfiable([](const Node&) { return true; });
+        bool satisfiable = node.IsSatisfiable([](const Node&) { return true; });
         // If the miniscript was satisfiable at all, a satisfaction must be found after all conditions are added.
         BOOST_CHECK_EQUAL(prev_mal_success, satisfiable);
         // If the miniscript is sane and satisfiable, a nonmalleable satisfaction must eventually be found.
-        if (node->IsSane()) BOOST_CHECK_EQUAL(prev_nonmal_success, satisfiable);
+        if (node.IsSane()) BOOST_CHECK_EQUAL(prev_nonmal_success, satisfiable);
     }
 }
 
@@ -472,7 +472,7 @@ void Test(const std::string& ms, const std::string& hexscript, int mode, const K
         if (stacklimit != -1) BOOST_CHECK_MESSAGE((int)*node->GetStackSize() == stacklimit, "Stack limit mismatch: " << ms << " (" << *node->GetStackSize() << " vs " << stacklimit << ")");
         if (max_wit_size) BOOST_CHECK_MESSAGE(*node->GetWitnessSize() == *max_wit_size, "Witness size limit mismatch: " << ms << " (" << *node->GetWitnessSize() << " vs " << *max_wit_size << ")");
         if (stack_exec) BOOST_CHECK_MESSAGE(*node->GetExecStackSize() == *stack_exec, "Stack execution limit mismatch: " << ms << " (" << *node->GetExecStackSize() << " vs " << *stack_exec << ")");
-        TestSatisfy(converter, ms, node);
+        TestSatisfy(converter, ms, *node);
     }
 }
 
@@ -600,11 +600,11 @@ BOOST_AUTO_TEST_CASE(fixed_tests)
     constexpr KeyConverter tap_converter{miniscript::MiniscriptContext::TAPSCRIPT};
     constexpr KeyConverter wsh_converter{miniscript::MiniscriptContext::P2WSH};
     const auto no_pubkey{"ac519c"_hex_u8};
-    BOOST_CHECK(miniscript::FromScript({no_pubkey.begin(), no_pubkey.end()}, tap_converter) == nullptr);
+    BOOST_CHECK(miniscript::FromScript({no_pubkey.begin(), no_pubkey.end()}, tap_converter) == std::nullopt);
     const auto incomplete_multi_a{"ba20c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee5ba519c"_hex_u8};
-    BOOST_CHECK(miniscript::FromScript({incomplete_multi_a.begin(), incomplete_multi_a.end()}, tap_converter) == nullptr);
+    BOOST_CHECK(miniscript::FromScript({incomplete_multi_a.begin(), incomplete_multi_a.end()}, tap_converter) == std::nullopt);
     const auto incomplete_multi_a_2{"ac2079be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798ac20c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee5ba519c"_hex_u8};
-    BOOST_CHECK(miniscript::FromScript({incomplete_multi_a_2.begin(), incomplete_multi_a_2.end()}, tap_converter) == nullptr);
+    BOOST_CHECK(miniscript::FromScript({incomplete_multi_a_2.begin(), incomplete_multi_a_2.end()}, tap_converter) == std::nullopt);
     // Can use multi_a under Tapscript but not P2WSH.
     Test("and_v(v:multi_a(2,03d01115d548e7561b15c38f004d734633687cf4419620095bc5b0f47070afe85a,025601570cb47f238d2b0286db4a990fa0f3ba28d1a319f5e7cf55c2a2444da7cc),after(1231488000))", "?", "20d01115d548e7561b15c38f004d734633687cf4419620095bc5b0f47070afe85aac205601570cb47f238d2b0286db4a990fa0f3ba28d1a319f5e7cf55c2a2444da7ccba529d0400046749b1", TESTMODE_VALID | TESTMODE_NONMAL | TESTMODE_NEEDSIG | TESTMODE_P2WSH_INVALID, 4, 2, {}, {}, 3);
     // Can use more than 20 keys in a multi_a.
@@ -650,13 +650,13 @@ BOOST_AUTO_TEST_CASE(fixed_tests)
     // A Script with a non minimal push is invalid
     constexpr auto nonminpush{"0000210232780000feff00ffffffffffff21ff005f00ae21ae00000000060602060406564c2102320000060900fe00005f00ae21ae00100000060606060606000000000000000000000000000000000000000000000000000000000000000000"_hex_u8};
     const CScript nonminpush_script(nonminpush.begin(), nonminpush.end());
-    BOOST_CHECK(miniscript::FromScript(nonminpush_script, wsh_converter) == nullptr);
-    BOOST_CHECK(miniscript::FromScript(nonminpush_script, tap_converter) == nullptr);
+    BOOST_CHECK(miniscript::FromScript(nonminpush_script, wsh_converter) == std::nullopt);
+    BOOST_CHECK(miniscript::FromScript(nonminpush_script, tap_converter) == std::nullopt);
     // A non-minimal VERIFY (<key> CHECKSIG VERIFY 1)
     constexpr auto nonminverify{"2103a0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c7ac6951"_hex_u8};
     const CScript nonminverify_script(nonminverify.begin(), nonminverify.end());
-    BOOST_CHECK(miniscript::FromScript(nonminverify_script, wsh_converter) == nullptr);
-    BOOST_CHECK(miniscript::FromScript(nonminverify_script, tap_converter) == nullptr);
+    BOOST_CHECK(miniscript::FromScript(nonminverify_script, wsh_converter) == std::nullopt);
+    BOOST_CHECK(miniscript::FromScript(nonminverify_script, tap_converter) == std::nullopt);
     // A threshold as large as the number of subs is valid.
     Test("thresh(2,c:pk_k(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),altv:after(100))", "2103d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65ac6b6300670164b16951686c935287", "20d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65ac6b6300670164b16951686c935287", TESTMODE_VALID | TESTMODE_NEEDSIG | TESTMODE_NONMAL);
     // A threshold of 1 is valid.

From 50cab8570e8f7553a94e750f66ad9228a728e72e Mon Sep 17 00:00:00 2001
From: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Date: Tue, 24 Jun 2025 21:05:12 +0200
Subject: [PATCH 5/7] refactor(miniscript): Remove NodeRef & MakeNodeRef()

(Also removes parameter to TestSatisfy() which existed unused from the start in 22c5b00345063bdeb8b6d3da8b5692d18f92bfb7).
---
 src/script/miniscript.h       | 149 ++++++++++++++++------------------
 src/test/fuzz/miniscript.cpp  |  12 +--
 src/test/miniscript_tests.cpp |   6 +-
 3 files changed, 78 insertions(+), 89 deletions(-)

diff --git a/src/script/miniscript.h b/src/script/miniscript.h
index 6e59fa5119f..0c6454959ec 100644
--- a/src/script/miniscript.h
+++ b/src/script/miniscript.h
@@ -191,11 +191,6 @@ inline consteval Type operator""_mst(const char* c, size_t l)
 using Opcode = std::pair<opcodetype, std::vector<unsigned char>>;
 
 template<typename Key> class Node;
-template<typename Key> using NodeRef = Node<Key>; // <- TODO: Remove in next commit.
-
-//! Construct a miniscript node (TODO: remove in next commit).
-template<typename Key, typename... Args>
-Node<Key> MakeNodeRef(Args&&... args) { return Node<Key>(std::forward<Args>(args)...); }
 
 //! Unordered traversal of a miniscript node tree.
 template <typename Key, std::invocable<const Node<Key>&> Fn>
@@ -545,7 +540,7 @@ class Node
     //! The data bytes in this expression (only for HASH160/HASH256/SHA256/RIPEMD160).
     std::vector<unsigned char> data;
     //! Subexpressions (for WRAP_*/AND_*/OR_*/ANDOR/THRESH)
-    std::vector<NodeRef<Key>> subs;
+    std::vector<Node> subs;
     //! The Script context for this node. Either P2WSH or Tapscript.
     MiniscriptContext m_script_ctx;
 
@@ -571,8 +566,8 @@ public:
     Node<Key> Clone() const
     {
         // Use TreeEval() to avoid a stack-overflow due to recursion
-        auto upfn = [](const Node& node, std::span<NodeRef<Key>> children) {
-            std::vector<NodeRef<Key>> new_subs;
+        auto upfn = [](const Node& node, std::span<Node> children) {
+            std::vector<Node> new_subs;
             for (auto& child : children) {
                 // It's fine to move from children as they are new nodes having
                 // been produced by calling this function one level down.
@@ -580,14 +575,14 @@ public:
             }
             return Node{internal::NoDupCheck{}, node.m_script_ctx, node.fragment, std::move(new_subs), node.keys, node.data, node.k};
         };
-        return TreeEval<NodeRef<Key>>(upfn);
+        return TreeEval<Node>(upfn);
     }
 
     enum Fragment Fragment() const { return fragment; }
     uint32_t K() const { return k; }
     const std::vector<Key>& Keys() const { return keys; }
     const std::vector<unsigned char>& Data() const { return data; }
-    const std::vector<NodeRef<Key>>& Subs() const { return subs; }
+    const std::vector<Node>& Subs() const { return subs; }
 
 private:
     //! Cached ops counts.
@@ -610,7 +605,7 @@ private:
     // 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, enum Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<Key> key, std::vector<unsigned char> arg, uint32_t val)
+    Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector<Node> 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).
@@ -1716,29 +1711,29 @@ public:
     bool operator==(const Node<Key>& arg) const { return Compare(*this, arg) == 0; }
 
     // Constructors with various argument combinations, which bypass the duplicate key check.
-    Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<unsigned char> arg, uint32_t val = 0)
+    Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector<Node> sub, std::vector<unsigned char> arg, uint32_t val = 0)
         : fragment(nt), k(val), data(std::move(arg)), subs(std::move(sub)), m_script_ctx{script_ctx}, ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {}
     Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector<unsigned char> arg, uint32_t val = 0)
         : fragment(nt), k(val), data(std::move(arg)), m_script_ctx{script_ctx}, ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {}
-    Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<Key> key, uint32_t val = 0)
+    Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector<Node> sub, std::vector<Key> key, uint32_t val = 0)
         : fragment(nt), k(val), keys(std::move(key)), m_script_ctx{script_ctx}, subs(std::move(sub)), ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {}
     Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector<Key> key, uint32_t val = 0)
         : fragment(nt), k(val), keys(std::move(key)), m_script_ctx{script_ctx}, ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {}
-    Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector<NodeRef<Key>> sub, uint32_t val = 0)
+    Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector<Node> sub, uint32_t val = 0)
         : fragment(nt), k(val), subs(std::move(sub)), m_script_ctx{script_ctx}, ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {}
     Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, uint32_t val = 0)
         : fragment(nt), k(val), m_script_ctx{script_ctx}, ops(CalcOps()), ss(CalcStackSize()), ws(CalcWitnessSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {}
 
     // Constructors with various argument combinations, which do perform the duplicate key check.
-    template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<unsigned char> arg, uint32_t val = 0)
+    template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, std::vector<Node> sub, std::vector<unsigned char> arg, uint32_t val = 0)
         : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(sub), std::move(arg), val) { DuplicateKeyCheck(ctx); }
     template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, std::vector<unsigned char> arg, uint32_t val = 0)
         : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(arg), val) { DuplicateKeyCheck(ctx);}
-    template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, std::vector<NodeRef<Key>> sub, std::vector<Key> key, uint32_t val = 0)
+    template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, std::vector<Node> sub, std::vector<Key> key, uint32_t val = 0)
         : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(sub), std::move(key), val) { DuplicateKeyCheck(ctx); }
     template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, std::vector<Key> key, uint32_t val = 0)
         : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(key), val) { DuplicateKeyCheck(ctx); }
-    template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, std::vector<NodeRef<Key>> sub, uint32_t val = 0)
+    template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, std::vector<Node> sub, uint32_t val = 0)
         : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(sub), val) { DuplicateKeyCheck(ctx); }
     template <typename Ctx> Node(const Ctx& ctx, enum Fragment nt, uint32_t val = 0)
         : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, val) { DuplicateKeyCheck(ctx); }
@@ -1837,14 +1832,14 @@ std::optional<std::pair<std::vector<unsigned char>, int>> ParseHexStrEnd(std::sp
 
 /** BuildBack pops the last two elements off `constructed` and wraps them in the specified Fragment */
 template<typename Key>
-void BuildBack(const MiniscriptContext script_ctx, Fragment nt, std::vector<NodeRef<Key>>& constructed, const bool reverse = false)
+void BuildBack(const MiniscriptContext script_ctx, Fragment nt, std::vector<Node<Key>>& constructed, const bool reverse = false)
 {
-    NodeRef<Key> child = std::move(constructed.back());
+    Node<Key> child{std::move(constructed.back())};
     constructed.pop_back();
     if (reverse) {
-        constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, script_ctx, nt, Vector(std::move(child), std::move(constructed.back())));
+        constructed.back() = Node<Key>{internal::NoDupCheck{}, script_ctx, nt, Vector(std::move(child), std::move(constructed.back()))};
     } else {
-        constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, script_ctx, nt, Vector(std::move(constructed.back()), std::move(child)));
+        constructed.back() = Node<Key>{internal::NoDupCheck{}, script_ctx, nt, Vector(std::move(constructed.back()), std::move(child))};
     }
 }
 
@@ -1873,7 +1868,7 @@ inline std::optional<Node<Key>> Parse(std::span<const char> in, const Ctx& ctx)
 
     // The two integers are used to hold state for thresh()
     std::vector<std::tuple<ParseContext, int64_t, int64_t>> to_parse;
-    std::vector<NodeRef<Key>> constructed;
+    std::vector<Node<Key>> constructed;
 
     to_parse.emplace_back(ParseContext::WRAPPED_EXPR, -1, -1);
 
@@ -1905,10 +1900,10 @@ inline std::optional<Node<Key>> Parse(std::span<const char> in, const Ctx& ctx)
         if (is_multi_a) {
             // (push + xonly-key + CHECKSIG[ADD]) * n + k + OP_NUMEQUAL(VERIFY), minus one.
             script_size += (1 + 32 + 1) * keys.size() + BuildScript(k).size();
-            constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::MULTI_A, std::move(keys), k));
+            constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::MULTI_A, std::move(keys), k);
         } else {
             script_size += 2 + (keys.size() > 16) + (k > 16) + 34 * keys.size();
-            constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::MULTI, std::move(keys), k));
+            constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::MULTI, std::move(keys), k);
         }
         return true;
     };
@@ -1966,7 +1961,7 @@ inline std::optional<Node<Key>> Parse(std::span<const char> in, const Ctx& ctx)
                 } else if (in[j] == 'l') {
                     // The l: wrapper is equivalent to or_i(0,X)
                     script_size += 4;
-                    constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_0));
+                    constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_0);
                     to_parse.emplace_back(ParseContext::OR_I, -1, -1);
                 } else {
                     return {};
@@ -1979,63 +1974,63 @@ inline std::optional<Node<Key>> Parse(std::span<const char> in, const Ctx& ctx)
         }
         case ParseContext::EXPR: {
             if (Const("0", in)) {
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_0));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_0);
             } else if (Const("1", in)) {
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_1));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_1);
             } else if (Const("pk(", in)) {
                 auto res = ParseKeyEnd<Key, Ctx>(in, ctx);
                 if (!res) return {};
                 auto& [key, key_size] = *res;
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_C, Vector(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_K, Vector(std::move(key))))));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_C, Vector(Node<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_K, Vector(std::move(key)))));
                 in = in.subspan(key_size + 1);
                 script_size += IsTapscript(ctx.MsContext()) ? 33 : 34;
             } else if (Const("pkh(", in)) {
                 auto res = ParseKeyEnd<Key>(in, ctx);
                 if (!res) return {};
                 auto& [key, key_size] = *res;
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_C, Vector(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_H, Vector(std::move(key))))));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_C, Vector(Node<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_H, Vector(std::move(key)))));
                 in = in.subspan(key_size + 1);
                 script_size += 24;
             } else if (Const("pk_k(", in)) {
                 auto res = ParseKeyEnd<Key>(in, ctx);
                 if (!res) return {};
                 auto& [key, key_size] = *res;
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_K, Vector(std::move(key))));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_K, Vector(std::move(key)));
                 in = in.subspan(key_size + 1);
                 script_size += IsTapscript(ctx.MsContext()) ? 32 : 33;
             } else if (Const("pk_h(", in)) {
                 auto res = ParseKeyEnd<Key>(in, ctx);
                 if (!res) return {};
                 auto& [key, key_size] = *res;
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_H, Vector(std::move(key))));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_H, Vector(std::move(key)));
                 in = in.subspan(key_size + 1);
                 script_size += 23;
             } else if (Const("sha256(", in)) {
                 auto res = ParseHexStrEnd(in, 32, ctx);
                 if (!res) return {};
                 auto& [hash, hash_size] = *res;
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::SHA256, std::move(hash)));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::SHA256, std::move(hash));
                 in = in.subspan(hash_size + 1);
                 script_size += 38;
             } else if (Const("ripemd160(", in)) {
                 auto res = ParseHexStrEnd(in, 20, ctx);
                 if (!res) return {};
                 auto& [hash, hash_size] = *res;
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::RIPEMD160, std::move(hash)));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::RIPEMD160, std::move(hash));
                 in = in.subspan(hash_size + 1);
                 script_size += 26;
             } else if (Const("hash256(", in)) {
                 auto res = ParseHexStrEnd(in, 32, ctx);
                 if (!res) return {};
                 auto& [hash, hash_size] = *res;
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::HASH256, std::move(hash)));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::HASH256, std::move(hash));
                 in = in.subspan(hash_size + 1);
                 script_size += 38;
             } else if (Const("hash160(", in)) {
                 auto res = ParseHexStrEnd(in, 20, ctx);
                 if (!res) return {};
                 auto& [hash, hash_size] = *res;
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::HASH160, std::move(hash)));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::HASH160, std::move(hash));
                 in = in.subspan(hash_size + 1);
                 script_size += 26;
             } else if (Const("after(", in)) {
@@ -2043,7 +2038,7 @@ inline std::optional<Node<Key>> Parse(std::span<const char> in, const Ctx& ctx)
                 if (arg_size < 1) return {};
                 const auto num{ToIntegral<int64_t>(std::string_view(in.data(), arg_size))};
                 if (!num.has_value() || *num < 1 || *num >= 0x80000000L) return {};
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::AFTER, *num));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::AFTER, *num);
                 in = in.subspan(arg_size + 1);
                 script_size += 1 + (*num > 16) + (*num > 0x7f) + (*num > 0x7fff) + (*num > 0x7fffff);
             } else if (Const("older(", in)) {
@@ -2051,7 +2046,7 @@ inline std::optional<Node<Key>> Parse(std::span<const char> in, const Ctx& ctx)
                 if (arg_size < 1) return {};
                 const auto num{ToIntegral<int64_t>(std::string_view(in.data(), arg_size))};
                 if (!num.has_value() || *num < 1 || *num >= 0x80000000L) return {};
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::OLDER, *num));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::OLDER, *num);
                 in = in.subspan(arg_size + 1);
                 script_size += 1 + (*num > 16) + (*num > 0x7f) + (*num > 0x7fff) + (*num > 0x7fffff);
             } else if (Const("multi(", in)) {
@@ -2110,40 +2105,40 @@ inline std::optional<Node<Key>> Parse(std::span<const char> in, const Ctx& ctx)
             break;
         }
         case ParseContext::ALT: {
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_A, Vector(std::move(constructed.back())));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_A, Vector(std::move(constructed.back()))};
             break;
         }
         case ParseContext::SWAP: {
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_S, Vector(std::move(constructed.back())));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_S, Vector(std::move(constructed.back()))};
             break;
         }
         case ParseContext::CHECK: {
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_C, Vector(std::move(constructed.back())));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_C, Vector(std::move(constructed.back()))};
             break;
         }
         case ParseContext::DUP_IF: {
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_D, Vector(std::move(constructed.back())));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_D, Vector(std::move(constructed.back()))};
             break;
         }
         case ParseContext::NON_ZERO: {
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_J, Vector(std::move(constructed.back())));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_J, Vector(std::move(constructed.back()))};
             break;
         }
         case ParseContext::ZERO_NOTEQUAL: {
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_N, Vector(std::move(constructed.back())));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_N, Vector(std::move(constructed.back()))};
             break;
         }
         case ParseContext::VERIFY: {
             script_size += (constructed.back().GetType() << "x"_mst);
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_V, Vector(std::move(constructed.back())));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_V, Vector(std::move(constructed.back()))};
             break;
         }
         case ParseContext::WRAP_U: {
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::OR_I, Vector(std::move(constructed.back()), MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_0)));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::OR_I, Vector(std::move(constructed.back()), Node<Key>{internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_0})};
             break;
         }
         case ParseContext::WRAP_T: {
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::AND_V, Vector(std::move(constructed.back()), MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_1)));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::AND_V, Vector(std::move(constructed.back()), Node<Key>{internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_1})};
             break;
         }
         case ParseContext::AND_B: {
@@ -2153,7 +2148,7 @@ inline std::optional<Node<Key>> Parse(std::span<const char> in, const Ctx& ctx)
         case ParseContext::AND_N: {
             auto mid = std::move(constructed.back());
             constructed.pop_back();
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::ANDOR, Vector(std::move(constructed.back()), std::move(mid), MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_0)));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::ANDOR, Vector(std::move(constructed.back()), std::move(mid), Node<Key>{internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_0})};
             break;
         }
         case ParseContext::AND_V: {
@@ -2181,7 +2176,7 @@ inline std::optional<Node<Key>> Parse(std::span<const char> in, const Ctx& ctx)
             constructed.pop_back();
             auto mid = std::move(constructed.back());
             constructed.pop_back();
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::ANDOR, Vector(std::move(constructed.back()), std::move(mid), std::move(right)));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::ANDOR, Vector(std::move(constructed.back()), std::move(mid), std::move(right))};
             break;
         }
         case ParseContext::THRESH: {
@@ -2195,13 +2190,13 @@ inline std::optional<Node<Key>> Parse(std::span<const char> in, const Ctx& ctx)
                 if (k > n) return {};
                 in = in.subspan(1);
                 // Children are constructed in reverse order, so iterate from end to beginning
-                std::vector<NodeRef<Key>> subs;
+                std::vector<Node<Key>> subs;
                 for (int i = 0; i < n; ++i) {
                     subs.push_back(std::move(constructed.back()));
                     constructed.pop_back();
                 }
                 std::reverse(subs.begin(), subs.end());
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::THRESH, std::move(subs), k));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::THRESH, std::move(subs), k);
             } else {
                 return {};
             }
@@ -2317,7 +2312,7 @@ inline std::optional<Node<Key>> DecodeScript(I& in, I last, const Ctx& ctx)
 {
     // The two integers are used to hold state for thresh()
     std::vector<std::tuple<DecodeContext, int64_t, int64_t>> to_parse;
-    std::vector<NodeRef<Key>> constructed;
+    std::vector<Node<Key>> constructed;
 
     // This is the top level, so we assume the type is B
     // (in particular, disallowing top level W expressions)
@@ -2338,12 +2333,12 @@ inline std::optional<Node<Key>> DecodeScript(I& in, I last, const Ctx& ctx)
             // Constants
             if (in[0].first == OP_1) {
                 ++in;
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_1));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_1);
                 break;
             }
             if (in[0].first == OP_0) {
                 ++in;
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_0));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::JUST_0);
                 break;
             }
             // Public keys
@@ -2351,14 +2346,14 @@ inline std::optional<Node<Key>> DecodeScript(I& in, I last, const Ctx& ctx)
                 auto key = ctx.FromPKBytes(in[0].second.begin(), in[0].second.end());
                 if (!key) return {};
                 ++in;
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_K, Vector(std::move(*key))));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_K, Vector(std::move(*key)));
                 break;
             }
             if (last - in >= 5 && in[0].first == OP_VERIFY && in[1].first == OP_EQUAL && in[3].first == OP_HASH160 && in[4].first == OP_DUP && in[2].second.size() == 20) {
                 auto key = ctx.FromPKHBytes(in[2].second.begin(), in[2].second.end());
                 if (!key) return {};
                 in += 5;
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_H, Vector(std::move(*key))));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::PK_H, Vector(std::move(*key)));
                 break;
             }
             // Time locks
@@ -2366,31 +2361,31 @@ inline std::optional<Node<Key>> DecodeScript(I& in, I last, const Ctx& ctx)
             if (last - in >= 2 && in[0].first == OP_CHECKSEQUENCEVERIFY && (num = ParseScriptNumber(in[1]))) {
                 in += 2;
                 if (*num < 1 || *num > 0x7FFFFFFFL) return {};
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::OLDER, *num));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::OLDER, *num);
                 break;
             }
             if (last - in >= 2 && in[0].first == OP_CHECKLOCKTIMEVERIFY && (num = ParseScriptNumber(in[1]))) {
                 in += 2;
                 if (num < 1 || num > 0x7FFFFFFFL) return {};
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::AFTER, *num));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::AFTER, *num);
                 break;
             }
             // Hashes
             if (last - in >= 7 && in[0].first == OP_EQUAL && in[3].first == OP_VERIFY && in[4].first == OP_EQUAL && (num = ParseScriptNumber(in[5])) && num == 32 && in[6].first == OP_SIZE) {
                 if (in[2].first == OP_SHA256 && in[1].second.size() == 32) {
-                    constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::SHA256, in[1].second));
+                    constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::SHA256, in[1].second);
                     in += 7;
                     break;
                 } else if (in[2].first == OP_RIPEMD160 && in[1].second.size() == 20) {
-                    constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::RIPEMD160, in[1].second));
+                    constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::RIPEMD160, in[1].second);
                     in += 7;
                     break;
                 } else if (in[2].first == OP_HASH256 && in[1].second.size() == 32) {
-                    constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::HASH256, in[1].second));
+                    constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::HASH256, in[1].second);
                     in += 7;
                     break;
                 } else if (in[2].first == OP_HASH160 && in[1].second.size() == 20) {
-                    constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::HASH160, in[1].second));
+                    constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::HASH160, in[1].second);
                     in += 7;
                     break;
                 }
@@ -2412,7 +2407,7 @@ inline std::optional<Node<Key>> DecodeScript(I& in, I last, const Ctx& ctx)
                 if (!k || *k < 1 || *k > *n) return {};
                 in += 3 + *n;
                 std::reverse(keys.begin(), keys.end());
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::MULTI, std::move(keys), *k));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::MULTI, std::move(keys), *k);
                 break;
             }
             // Tapscript's equivalent of multi
@@ -2442,7 +2437,7 @@ inline std::optional<Node<Key>> DecodeScript(I& in, I last, const Ctx& ctx)
                 if (keys.size() < (size_t)*k) return {};
                 in += 2 + keys.size() * 2;
                 std::reverse(keys.begin(), keys.end());
-                constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::MULTI_A, std::move(keys), *k));
+                constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::MULTI_A, std::move(keys), *k);
                 break;
             }
             /** In the following wrappers, we only need to push SINGLE_BKV_EXPR rather
@@ -2537,38 +2532,38 @@ inline std::optional<Node<Key>> DecodeScript(I& in, I last, const Ctx& ctx)
         case DecodeContext::SWAP: {
             if (in >= last || in[0].first != OP_SWAP || constructed.empty()) return {};
             ++in;
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_S, Vector(std::move(constructed.back())));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_S, Vector(std::move(constructed.back()))};
             break;
         }
         case DecodeContext::ALT: {
             if (in >= last || in[0].first != OP_TOALTSTACK || constructed.empty()) return {};
             ++in;
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_A, Vector(std::move(constructed.back())));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_A, Vector(std::move(constructed.back()))};
             break;
         }
         case DecodeContext::CHECK: {
             if (constructed.empty()) return {};
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_C, Vector(std::move(constructed.back())));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_C, Vector(std::move(constructed.back()))};
             break;
         }
         case DecodeContext::DUP_IF: {
             if (constructed.empty()) return {};
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_D, Vector(std::move(constructed.back())));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_D, Vector(std::move(constructed.back()))};
             break;
         }
         case DecodeContext::VERIFY: {
             if (constructed.empty()) return {};
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_V, Vector(std::move(constructed.back())));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_V, Vector(std::move(constructed.back()))};
             break;
         }
         case DecodeContext::NON_ZERO: {
             if (constructed.empty()) return {};
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_J, Vector(std::move(constructed.back())));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_J, Vector(std::move(constructed.back()))};
             break;
         }
         case DecodeContext::ZERO_NOTEQUAL: {
             if (constructed.empty()) return {};
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_N, Vector(std::move(constructed.back())));
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::WRAP_N, Vector(std::move(constructed.back()))};
             break;
         }
         case DecodeContext::AND_V: {
@@ -2598,12 +2593,12 @@ inline std::optional<Node<Key>> DecodeScript(I& in, I last, const Ctx& ctx)
         }
         case DecodeContext::ANDOR: {
             if (constructed.size() < 3) return {};
-            NodeRef<Key> left = std::move(constructed.back());
+            Node left{std::move(constructed.back())};
             constructed.pop_back();
-            NodeRef<Key> right = std::move(constructed.back());
+            Node right{std::move(constructed.back())};
             constructed.pop_back();
-            NodeRef<Key> mid = std::move(constructed.back());
-            constructed.back() = MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::ANDOR, Vector(std::move(left), std::move(mid), std::move(right)));
+            Node mid{std::move(constructed.back())};
+            constructed.back() = Node{internal::NoDupCheck{}, ctx.MsContext(), Fragment::ANDOR, Vector(std::move(left), std::move(mid), std::move(right))};
             break;
         }
         case DecodeContext::THRESH_W: {
@@ -2621,13 +2616,13 @@ inline std::optional<Node<Key>> DecodeScript(I& in, I last, const Ctx& ctx)
         }
         case DecodeContext::THRESH_E: {
             if (k < 1 || k > n || constructed.size() < static_cast<size_t>(n)) return {};
-            std::vector<NodeRef<Key>> subs;
+            std::vector<Node<Key>> subs;
             for (int i = 0; i < n; ++i) {
-                NodeRef<Key> sub = std::move(constructed.back());
+                Node sub{std::move(constructed.back())};
                 constructed.pop_back();
                 subs.push_back(std::move(sub));
             }
-            constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::THRESH, std::move(subs), k));
+            constructed.emplace_back(internal::NoDupCheck{}, ctx.MsContext(), Fragment::THRESH, std::move(subs), k);
             break;
         }
         case DecodeContext::ENDIF: {
diff --git a/src/test/fuzz/miniscript.cpp b/src/test/fuzz/miniscript.cpp
index a122e8f368e..d20bf9fbd84 100644
--- a/src/test/fuzz/miniscript.cpp
+++ b/src/test/fuzz/miniscript.cpp
@@ -20,7 +20,6 @@
 namespace {
 
 using Fragment = miniscript::Fragment;
-using NodeRef = miniscript::NodeRef<CPubKey>;
 using Node = miniscript::Node<CPubKey>;
 using Type = miniscript::Type;
 using MsCtx = miniscript::MiniscriptContext;
@@ -317,11 +316,6 @@ const struct KeyComparator {
 // A dummy scriptsig to pass to VerifyScript (we always use Segwit v0).
 const CScript DUMMY_SCRIPTSIG;
 
-//! Construct a miniscript node as a shared_ptr.
-template<typename... Args> NodeRef MakeNodeRef(Args&&... args) {
-    return miniscript::MakeNodeRef<CPubKey>(miniscript::internal::NoDupCheck{}, std::forward<Args>(args)...);
-}
-
 /** Information about a yet to be constructed Miniscript node. */
 struct NodeInfo {
     //! The type of this node
@@ -861,7 +855,7 @@ template <typename F>
 std::optional<Node> GenNode(MsCtx script_ctx, F ConsumeNode, Type root_type, bool strict_valid = false)
 {
     /** A stack of miniscript Nodes being built up. */
-    std::vector<NodeRef> stack;
+    std::vector<Node> stack;
     /** The queue of instructions. */
     std::vector<std::pair<Type, std::optional<NodeInfo>>> todo{{root_type, {}}};
     /** Predict the number of (static) script ops. */
@@ -964,11 +958,11 @@ std::optional<Node> GenNode(MsCtx script_ctx, F ConsumeNode, Type root_type, boo
         } else {
             // The back of todo has fragment and number of children decided, and
             // those children have been constructed at the back of stack. Pop
-            // that entry off todo, and use it to construct a new NodeRef on
+            // that entry off todo, and use it to construct a new Node on
             // stack.
             NodeInfo& info = *todo.back().second;
             // Gather children from the back of stack.
-            std::vector<NodeRef> sub;
+            std::vector<Node> sub;
             sub.reserve(info.subtypes.size());
             for (size_t i = 0; i < info.subtypes.size(); ++i) {
                 sub.push_back(std::move(*(stack.end() - info.subtypes.size() + i)));
diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp
index a5f9a21d033..0cd5b62f7de 100644
--- a/src/test/miniscript_tests.cpp
+++ b/src/test/miniscript_tests.cpp
@@ -292,7 +292,6 @@ public:
 };
 
 using Fragment = miniscript::Fragment;
-using NodeRef = miniscript::NodeRef<CPubKey>;
 using miniscript::operator""_mst;
 using Node = miniscript::Node<CPubKey>;
 
@@ -346,7 +345,8 @@ void SatisfactionToWitness(miniscript::MiniscriptContext ctx, CScriptWitness& wi
 
 struct MiniScriptTest : BasicTestingSetup {
 /** Run random satisfaction tests. */
-void TestSatisfy(const KeyConverter& converter, const std::string& testcase, const NodeRef& node) {
+void TestSatisfy(const KeyConverter& converter, const Node& node)
+{
     auto script = node.ToScript(converter);
     const auto challenges{FindChallenges(node)}; // Find all challenges in the generated miniscript.
     std::vector<Challenge> challist(challenges.begin(), challenges.end());
@@ -472,7 +472,7 @@ void Test(const std::string& ms, const std::string& hexscript, int mode, const K
         if (stacklimit != -1) BOOST_CHECK_MESSAGE((int)*node->GetStackSize() == stacklimit, "Stack limit mismatch: " << ms << " (" << *node->GetStackSize() << " vs " << stacklimit << ")");
         if (max_wit_size) BOOST_CHECK_MESSAGE(*node->GetWitnessSize() == *max_wit_size, "Witness size limit mismatch: " << ms << " (" << *node->GetWitnessSize() << " vs " << *max_wit_size << ")");
         if (stack_exec) BOOST_CHECK_MESSAGE(*node->GetExecStackSize() == *stack_exec, "Stack execution limit mismatch: " << ms << " (" << *node->GetExecStackSize() << " vs " << *stack_exec << ")");
-        TestSatisfy(converter, ms, *node);
+        TestSatisfy(converter, *node);
     }
 }
 

From 198bbaee4959119a63b4038cd0dbb519f4daf6f0 Mon Sep 17 00:00:00 2001
From: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Date: Fri, 21 Nov 2025 16:24:22 +0100
Subject: [PATCH 6/7] refactor(miniscript): Destroy nodes one full subs-vector
 at a time

---
 src/script/miniscript.h | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/script/miniscript.h b/src/script/miniscript.h
index 0c6454959ec..1b7e84f471d 100644
--- a/src/script/miniscript.h
+++ b/src/script/miniscript.h
@@ -552,14 +552,15 @@ public:
         // Destroy the subexpressions iteratively after moving out their
         // subexpressions to avoid a stack-overflow due to recursive calls to
         // the subs' destructors.
-        while (!subs.empty()) {
-            auto node = std::move(subs.back());
-            subs.pop_back();
-            while (!node.subs.empty()) {
-                subs.push_back(std::move(node.subs.back()));
-                node.subs.pop_back();
+        std::vector<std::vector<Node>> queue;
+        queue.push_back(std::move(subs));
+        do {
+            auto flattening{std::move(queue.back())};
+            queue.pop_back();
+            for (Node& n : flattening) {
+                if (!n.subs.empty()) queue.push_back(std::move(n.subs));
             }
-        }
+        } while (!queue.empty());
     }
     // NOLINTEND(misc-no-recursion)
 

From 964c44cdcd6be5f39aed1aeda9c305803eb3b25f Mon Sep 17 00:00:00 2001
From: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Date: Wed, 25 Jun 2025 14:17:14 +0200
Subject: [PATCH 7/7] test(miniscript): Prove avoidance of stack overflow
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Can be tested through emptying the function body of ~Node() or replacing Clone() implementation with naive version:
```C++
Node<Key> Clone() const
{
    std::vector<Node> new_subs;
    new_subs.reserve(subs.size());
    for (const Node& child : subs) {
        new_subs.push_back(child.Clone());
    }
    return Node{internal::NoDupCheck{}, m_script_ctx, fragment, std::move(new_subs), keys, data, k};
}
```

Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
---
 src/test/miniscript_tests.cpp | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp
index 0cd5b62f7de..757f89b63bc 100644
--- a/src/test/miniscript_tests.cpp
+++ b/src/test/miniscript_tests.cpp
@@ -727,4 +727,25 @@ BOOST_AUTO_TEST_CASE(fixed_tests)
     g_testdata.reset();
 }
 
+// Confirm that ~Node(), Node::Clone() and operator=(Node&&) are stack-safe.
+BOOST_AUTO_TEST_CASE(node_deep_destruct)
+{
+    using miniscript::internal::NoDupCheck;
+    using miniscript::Fragment;
+    using NodeU32 = miniscript::Node<uint32_t>;
+
+    constexpr auto ctx{miniscript::MiniscriptContext::P2WSH};
+
+    NodeU32 root{NoDupCheck{}, ctx, Fragment::JUST_1};
+    for (uint32_t i{0}; i < 200'000; ++i) {
+        root = NodeU32{NoDupCheck{}, ctx, Fragment::WRAP_S, Vector(std::move(root))};
+    }
+    BOOST_CHECK_EQUAL(root.ScriptSize(), 200'001);
+
+    auto clone{root.Clone()};
+    BOOST_CHECK_EQUAL(clone.ScriptSize(), root.ScriptSize());
+
+    clone = std::move(root);
+}
+
 BOOST_AUTO_TEST_SUITE_END()