Repository: bitcoin/bitcoin
File: src/script/miniscript.h (C/C++)

Code before:
    const std::vector<Key> keys;
    //! The data bytes in this expression (only for HASH160/HASH256/SHA256/RIPEMD10).
    const 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;

    /* Destroy the shared pointers iteratively to avoid a stack-overflow due to recursive calls
     * to the subs' destructors. */
    ~Node() {
        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();
            }
        }
    }

private:
    //! Cached ops counts.
    const internal::Ops ops;
    //! Cached stack size bounds.
    const internal::StackSize ss;
    //! Cached witness size bounds.
    const internal::WitnessSize ws;
    //! Cached expression type (computed by CalcType and fed through SanitizeType).
    const Type typ;
    //! Cached script length (computed by CalcScriptLen).
    const 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().
    //! DuplicateKeyCheck(), or a non-NoDupCheck constructor, will compute has_duplicate_keys
    //! for all subnodes as well.
    mutable std::optional<bool> has_duplicate_keys;


    //! Compute the length of the script for this miniscript (including children).
    size_t CalcScriptLen() const {
        size_t subsize = 0;
        for (const auto& sub : subs) {
            subsize += sub->ScriptSize();
        }
        static constexpr auto NONE_MST{""_mst};
        Type sub0type = subs.size() > 0 ? subs[0]->GetType() : NONE_MST;
        return internal::ComputeScriptLen(fragment, sub0type, subsize, k, subs.size(), keys.size(), m_script_ctx);
    }


Review comment (suggestion):
nit: Could make less verbose:
```suggestion
            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}};
```
Good call on adding the `make_unique` comment.

Code after:
    const std::vector<Key> keys;
    //! The data bytes in this expression (only for HASH160/HASH256/SHA256/RIPEMD10).
    const 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;

    /* Destroy the shared pointers iteratively to avoid a stack-overflow due to recursive calls
     * to the subs' destructors. */
    ~Node() {
        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();
            }
        }
    }

    NodeRef<Key> Clone() const
    {
        // Use TreeEval() to avoid a stack-overflow due to recursion
        auto upfn = [](const Node& node, Span<NodeRef<Key>> children) {
            std::vector<NodeRef<Key>> new_subs;
            for (auto child = children.begin(); child != children.end(); ++child) {
                new_subs.emplace_back(std::move(*child));
            }
            // std::make_unique (and therefore MakeNodeRef) doesn't work on private constructors
            return std::unique_ptr<Node>{new Node{internal::NoDupCheck{}, node.m_script_ctx, node.fragment, std::move(new_subs), node.keys, node.data, node.k}};
        };
        return TreeEval<NodeRef<Key>>(upfn);
    }

private:
    //! Cached ops counts.
    const internal::Ops ops;
    //! Cached stack size bounds.
    const internal::StackSize ss;
    //! Cached witness size bounds.
    const internal::WitnessSize ws;
    //! Cached expression type (computed by CalcType and fed through SanitizeType).
    const Type typ;
    //! Cached script length (computed by CalcScriptLen).
    const 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().
    //! DuplicateKeyCheck(), or a non-NoDupCheck constructor, will compute has_duplicate_keys
    //! for all subnodes as well.