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& 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>; -template struct Node; +template class Node; template using NodeRef = std::unique_ptr>; //! Construct a miniscript node as a unique_ptr. @@ -206,7 +206,7 @@ void ForEachNode(const Node& root, Fn&& fn) const Node& 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 -struct MaxInt { - const bool valid; - const I value; +template +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 operator+(const MaxInt& a, const MaxInt& 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 -struct Node { +template +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 keys; - //! The data bytes in this expression (only for HASH160/HASH256/SHA256/RIPEMD10). - const std::vector data; + std::vector keys; + //! The data bytes in this expression (only for HASH160/HASH256/SHA256/RIPEMD160). + std::vector data; //! Subexpressions (for WRAP_*/AND_*/OR_*/ANDOR/THRESH) mutable std::vector> 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>(upfn); } + enum Fragment Fragment() const { return fragment; } + uint32_t K() const { return k; } + const std::vector& Keys() const { return keys; } + const std::vector& Data() const { return data; } + const std::vector>& 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> sub, std::vector key, std::vector arg, uint32_t val) + Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector> sub, std::vector key, std::vector 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 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 GetStackSize() const { - if (!ss.sat.valid) return {}; - return ss.sat.netdiff + static_cast(IsBKW()); + if (!ss.Sat().Valid()) return {}; + return ss.Sat().NetDiff() + static_cast(IsBKW()); } //! Return the maximum size of the stack during execution of this script. std::optional GetExecStackSize() const { - if (!ss.sat.valid) return {}; - return ss.sat.exec + static_cast(IsBKW()); + if (!ss.Sat().Valid()) return {}; + return ss.Sat().Exec() + static_cast(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 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& 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> sub, std::vector arg, uint32_t val = 0) + Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector> sub, std::vector 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 arg, uint32_t val = 0) + Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector 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> sub, std::vector key, uint32_t val = 0) + Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector> sub, std::vector 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, uint32_t val = 0) + Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector 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> sub, uint32_t val = 0) + Node(internal::NoDupCheck, MiniscriptContext script_ctx, enum Fragment nt, std::vector> 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 Node(const Ctx& ctx, Fragment nt, std::vector> sub, std::vector arg, uint32_t val = 0) + template Node(const Ctx& ctx, enum Fragment nt, std::vector> sub, std::vector arg, uint32_t val = 0) : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(sub), std::move(arg), val) { DuplicateKeyCheck(ctx); } - template Node(const Ctx& ctx, Fragment nt, std::vector arg, uint32_t val = 0) + template Node(const Ctx& ctx, enum Fragment nt, std::vector arg, uint32_t val = 0) : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(arg), val) { DuplicateKeyCheck(ctx);} - template Node(const Ctx& ctx, Fragment nt, std::vector> sub, std::vector key, uint32_t val = 0) + template Node(const Ctx& ctx, enum Fragment nt, std::vector> sub, std::vector key, uint32_t val = 0) : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(sub), std::move(key), val) { DuplicateKeyCheck(ctx); } - template Node(const Ctx& ctx, Fragment nt, std::vector key, uint32_t val = 0) + template Node(const Ctx& ctx, enum Fragment nt, std::vector key, uint32_t val = 0) : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(key), val) { DuplicateKeyCheck(ctx); } - template Node(const Ctx& ctx, Fragment nt, std::vector> sub, uint32_t val = 0) + template Node(const Ctx& ctx, enum Fragment nt, std::vector> sub, uint32_t val = 0) : Node(internal::NoDupCheck{}, ctx.MsContext(), nt, std::move(sub), val) { DuplicateKeyCheck(ctx); } - template Node(const Ctx& ctx, Fragment nt, uint32_t val = 0) + template 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 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()); } }