From a3f0a9260ae45170ee1a9efd5950864176df507d Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 24 Feb 2022 15:59:33 -0500 Subject: [PATCH 1/6] Move Satisfiable from unit test to Miniscript, and generalize --- bitcoin/script/miniscript.h | 43 +++++++++++++++++++++++++++++++ bitcoin/test/miniscript_tests.cpp | 33 +++--------------------- 2 files changed, 47 insertions(+), 29 deletions(-) diff --git a/bitcoin/script/miniscript.h b/bitcoin/script/miniscript.h index 9707f72..0aa1d56 100644 --- a/bitcoin/script/miniscript.h +++ b/bitcoin/script/miniscript.h @@ -383,6 +383,8 @@ struct Node { * computes the result of the node. If std::nullopt is returned by upfn, * TreeEvalMaybe() immediately returns std::nullopt. * The return value of TreeEvalMaybe is the result of the root node. + * + * Result type cannot be bool due to the std::vector specialization. */ template std::optional TreeEvalMaybe(State root_state, DownFn downfn, UpFn upfn) const @@ -1008,6 +1010,47 @@ struct Node { }); } + //! Determine whether a Miniscript node is satisfiable. fn(node) will be invoked for all + //! key, time, and hashing nodes, and should return their satisfiability. + template + bool IsSatisfiable(F fn) const + { + // TreeEval() doesn't support bool as NodeType, so use int instead. + return TreeEval([&fn](const Node& node, Span subs) { + switch (node.nodetype) { + case Fragment::JUST_0: + return false; + case Fragment::JUST_1: + return true; + case Fragment::PK_K: + case Fragment::PK_H: + case Fragment::MULTI: + case Fragment::AFTER: + case Fragment::OLDER: + case Fragment::HASH256: + case Fragment::HASH160: + case Fragment::SHA256: + case Fragment::RIPEMD160: + return bool{fn(node)}; + case Fragment::ANDOR: + return (subs[0] && subs[1]) || subs[2]; + case Fragment::AND_V: + case Fragment::AND_B: + return subs[0] && subs[1]; + case Fragment::OR_B: + case Fragment::OR_C: + case Fragment::OR_D: + case Fragment::OR_I: + return subs[0] || subs[1]; + case Fragment::THRESH: + return std::count(subs.begin(), subs.end(), true) >= node.k; + default: // wrappers + assert(subs.size() == 1); + return !!subs[0]; + } + }); + } + //! Check whether this node is valid at all. bool IsValid() const { return !(GetType() == ""_mst) && ScriptSize() <= MAX_STANDARD_P2WSH_SCRIPT_SIZE; } diff --git a/bitcoin/test/miniscript_tests.cpp b/bitcoin/test/miniscript_tests.cpp index 60d3520..bbd744b 100644 --- a/bitcoin/test/miniscript_tests.cpp +++ b/bitcoin/test/miniscript_tests.cpp @@ -234,33 +234,6 @@ using Fragment = miniscript::Fragment; using NodeRef = miniscript::NodeRef; using miniscript::operator"" _mst; -//! Determine whether a Miniscript node is satisfiable at all (and thus isn't equivalent to just "false"). -bool Satisfiable(const NodeRef& ref) { - switch (ref->nodetype) { - case Fragment::JUST_0: - return false; - case Fragment::AND_B: case Fragment::AND_V: - return Satisfiable(ref->subs[0]) && Satisfiable(ref->subs[1]); - case Fragment::OR_B: case Fragment::OR_C: case Fragment::OR_D: case Fragment::OR_I: - return Satisfiable(ref->subs[0]) || Satisfiable(ref->subs[1]); - case Fragment::ANDOR: - return (Satisfiable(ref->subs[0]) && Satisfiable(ref->subs[1])) || Satisfiable(ref->subs[2]); - case Fragment::WRAP_A: case Fragment::WRAP_C: case Fragment::WRAP_S: - case Fragment::WRAP_D: case Fragment::WRAP_V: case Fragment::WRAP_J: - case Fragment::WRAP_N: - return Satisfiable(ref->subs[0]); - case Fragment::PK_K: case Fragment::PK_H: case Fragment::MULTI: - case Fragment::AFTER: case Fragment::OLDER: case Fragment::HASH256: - case Fragment::HASH160: case Fragment::SHA256: case Fragment::RIPEMD160: - case Fragment::JUST_1: - return true; - case Fragment::THRESH: - return std::accumulate(ref->subs.begin(), ref->subs.end(), (size_t)0, [](size_t acc, const NodeRef& ref){return acc + Satisfiable(ref);}) >= ref->k; - } - assert(false); - return false; -} - /** Compute all challenges (pubkeys, hashes, timelocks) that occur in a given Miniscript. */ std::set FindChallenges(const NodeRef& ref) { std::set chal; @@ -357,10 +330,12 @@ void TestSatisfy(const std::string& testcase, const NodeRef& node) { prev_mal_success = mal_success; prev_nonmal_success = nonmal_success; } + + 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(node)); + BOOST_CHECK_EQUAL(prev_mal_success, satisfiable); // If the miniscript is sane and satisfiable, a nonmalleable satisfaction must eventually be found. - if (node->IsSaneTopLevel()) BOOST_CHECK_EQUAL(prev_nonmal_success, Satisfiable(node)); + if (node->IsSaneTopLevel()) BOOST_CHECK_EQUAL(prev_nonmal_success, satisfiable); } } From a821aa6e560bf2d295101911bf34a29eb159e9f0 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 24 Feb 2022 16:02:03 -0500 Subject: [PATCH 2/6] Rename Miniscript::nodetype to fragment --- bitcoin/script/miniscript.cpp | 38 ++++++------- bitcoin/script/miniscript.h | 72 ++++++++++++------------- bitcoin/test/fuzz/miniscript_random.cpp | 2 +- bitcoin/test/miniscript_tests.cpp | 12 ++--- compiler.cpp | 4 +- js_bindings.cpp | 54 +++++++++---------- 6 files changed, 91 insertions(+), 91 deletions(-) diff --git a/bitcoin/script/miniscript.cpp b/bitcoin/script/miniscript.cpp index 5dd09da..9ee1be1 100644 --- a/bitcoin/script/miniscript.cpp +++ b/bitcoin/script/miniscript.cpp @@ -35,51 +35,51 @@ Type SanitizeType(Type e) { return e; } -Type ComputeType(Fragment nodetype, Type x, Type y, Type z, const std::vector& sub_types, uint32_t k, size_t data_size, size_t n_subs, size_t n_keys) { +Type ComputeType(Fragment fragment, Type x, Type y, Type z, const std::vector& sub_types, uint32_t k, size_t data_size, size_t n_subs, size_t n_keys) { // Sanity check on data - if (nodetype == Fragment::SHA256 || nodetype == Fragment::HASH256) { + if (fragment == Fragment::SHA256 || fragment == Fragment::HASH256) { assert(data_size == 32); - } else if (nodetype == Fragment::RIPEMD160 || nodetype == Fragment::HASH160) { + } else if (fragment == Fragment::RIPEMD160 || fragment == Fragment::HASH160) { assert(data_size == 20); } else { assert(data_size == 0); } // Sanity check on k - if (nodetype == Fragment::OLDER || nodetype == Fragment::AFTER) { + if (fragment == Fragment::OLDER || fragment == Fragment::AFTER) { assert(k >= 1 && k < 0x80000000UL); - } else if (nodetype == Fragment::MULTI) { + } else if (fragment == Fragment::MULTI) { assert(k >= 1 && k <= n_keys); - } else if (nodetype == Fragment::THRESH) { + } else if (fragment == Fragment::THRESH) { assert(k >= 1 && k <= n_subs); } else { assert(k == 0); } // Sanity check on subs - if (nodetype == Fragment::AND_V || nodetype == Fragment::AND_B || nodetype == Fragment::OR_B || - nodetype == Fragment::OR_C || nodetype == Fragment::OR_I || nodetype == Fragment::OR_D) { + if (fragment == Fragment::AND_V || fragment == Fragment::AND_B || fragment == Fragment::OR_B || + fragment == Fragment::OR_C || fragment == Fragment::OR_I || fragment == Fragment::OR_D) { assert(n_subs == 2); - } else if (nodetype == Fragment::ANDOR) { + } else if (fragment == Fragment::ANDOR) { assert(n_subs == 3); - } else if (nodetype == Fragment::WRAP_A || nodetype == Fragment::WRAP_S || nodetype == Fragment::WRAP_C || - nodetype == Fragment::WRAP_D || nodetype == Fragment::WRAP_V || nodetype == Fragment::WRAP_J || - nodetype == Fragment::WRAP_N) { + } else if (fragment == Fragment::WRAP_A || fragment == Fragment::WRAP_S || fragment == Fragment::WRAP_C || + fragment == Fragment::WRAP_D || fragment == Fragment::WRAP_V || fragment == Fragment::WRAP_J || + fragment == Fragment::WRAP_N) { assert(n_subs == 1); - } else if (nodetype != Fragment::THRESH) { + } else if (fragment != Fragment::THRESH) { assert(n_subs == 0); } // Sanity check on keys - if (nodetype == Fragment::PK_K || nodetype == Fragment::PK_H) { + if (fragment == Fragment::PK_K || fragment == Fragment::PK_H) { assert(n_keys == 1); - } else if (nodetype == Fragment::MULTI) { + } else if (fragment == Fragment::MULTI) { assert(n_keys >= 1 && n_keys <= 20); } else { assert(n_keys == 0); } - // Below is the per-nodetype logic for computing the expression types. + // Below is the per-fragment logic for computing the expression types. // It heavily relies on Type's << operator (where "X << a_mst" means // "X has all properties listed in a"). - switch (nodetype) { + switch (fragment) { case Fragment::PK_K: return "Konudemsxk"_mst; case Fragment::PK_H: return "Knudemsxk"_mst; case Fragment::OLDER: return @@ -249,8 +249,8 @@ Type ComputeType(Fragment nodetype, Type x, Type y, Type z, const std::vector& sub_types, uint32_t k, size_t data_size, size_t n_subs, size_t n_keys); +Type ComputeType(Fragment fragment, Type x, Type y, Type z, const std::vector& sub_types, uint32_t k, size_t data_size, size_t n_subs, size_t n_keys); //! Helper function for Node::CalcScriptLen. -size_t ComputeScriptLen(Fragment nodetype, Type sub0typ, size_t subsize, uint32_t k, size_t n_subs, size_t n_keys); +size_t ComputeScriptLen(Fragment fragment, Type sub0typ, size_t subsize, uint32_t k, size_t n_subs, size_t n_keys); //! A helper sanitizer/checker for the output of CalcType. Type SanitizeType(Type x); @@ -333,7 +333,7 @@ struct StackSize { template struct Node { //! What node type this node is. - const Fragment nodetype; + const Fragment fragment; //! The k parameter (time for OLDER/AFTER, threshold for THRESH(_M)) const uint32_t k = 0; //! The keys used by this expression (only for PK_K/PK_H/MULTI) @@ -360,7 +360,7 @@ struct Node { subsize += sub->ScriptSize(); } Type sub0type = subs.size() > 0 ? subs[0]->GetType() : ""_mst; - return internal::ComputeScriptLen(nodetype, sub0type, subsize, k, subs.size(), keys.size()); + return internal::ComputeScriptLen(fragment, sub0type, subsize, k, subs.size(), keys.size()); } /* Apply a recursive algorithm to a Miniscript tree, without actual recursive calls. @@ -485,7 +485,7 @@ struct Node { // THRESH has a variable number of subexpressions std::vector sub_types; - if (nodetype == Fragment::THRESH) { + if (fragment == Fragment::THRESH) { 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. @@ -493,7 +493,7 @@ struct Node { Type y = subs.size() > 1 ? subs[1]->GetType() : ""_mst; Type z = subs.size() > 2 ? subs[2]->GetType() : ""_mst; - return SanitizeType(ComputeType(nodetype, x, y, z, sub_types, k, data.size(), subs.size(), keys.size())); + return SanitizeType(ComputeType(fragment, x, y, z, sub_types, k, data.size(), subs.size(), keys.size())); } public: @@ -505,17 +505,17 @@ struct Node { // by an OP_VERIFY (which may need to be combined with the last script opcode). auto downfn = [](bool verify, const Node& node, size_t index) { // For WRAP_V, the subexpression is certainly followed by OP_VERIFY. - if (node.nodetype == Fragment::WRAP_V) return true; + if (node.fragment == Fragment::WRAP_V) return true; // The subexpression of WRAP_S, and the last subexpression of AND_V // inherit the followed-by-OP_VERIFY property from the parent. - if (node.nodetype == Fragment::WRAP_S || - (node.nodetype == Fragment::AND_V && index == 1)) return verify; + if (node.fragment == Fragment::WRAP_S || + (node.fragment == Fragment::AND_V && index == 1)) return verify; return false; }; // The upward function computes for a node, given its followed-by-OP_VERIFY status // and the CScripts of its child nodes, the CScript of the node. auto upfn = [&ctx](bool verify, const Node& node, Span subs) -> CScript { - switch (node.nodetype) { + switch (node.fragment) { case Fragment::PK_K: return BuildScript(ctx.ToPKBytes(node.keys[0])); case Fragment::PK_H: return BuildScript(OP_DUP, OP_HASH160, ctx.ToPKHBytes(node.keys[0]), OP_EQUALVERIFY); case Fragment::OLDER: return BuildScript(node.k, OP_CHECKSEQUENCEVERIFY); @@ -573,30 +573,30 @@ struct Node { // the TreeEvalMaybe algorithm. The State is a boolean: whether the parent node is a // wrapper. If so, non-wrapper expressions must be prefixed with a ":". auto downfn = [](bool, const Node& node, size_t) { - return (node.nodetype == Fragment::WRAP_A || node.nodetype == Fragment::WRAP_S || - node.nodetype == Fragment::WRAP_D || node.nodetype == Fragment::WRAP_V || - node.nodetype == Fragment::WRAP_J || node.nodetype == Fragment::WRAP_N || - node.nodetype == Fragment::WRAP_C || - (node.nodetype == Fragment::AND_V && node.subs[1]->nodetype == Fragment::JUST_1) || - (node.nodetype == Fragment::OR_I && node.subs[0]->nodetype == Fragment::JUST_0) || - (node.nodetype == Fragment::OR_I && node.subs[1]->nodetype == Fragment::JUST_0)); + return (node.fragment == Fragment::WRAP_A || node.fragment == Fragment::WRAP_S || + 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)); }; // The upward function computes for a node, given whether its parent is a wrapper, // and the string representations of its child nodes, the string representation of the node. auto upfn = [&ctx](bool wrapped, const Node& node, Span subs) -> std::optional { std::string ret = wrapped ? ":" : ""; - switch (node.nodetype) { + switch (node.fragment) { 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]->nodetype == Fragment::PK_K) { + if (node.subs[0]->fragment == Fragment::PK_K) { // pk(K) is syntactic sugar for c:pk_k(K) std::string key_str; if (!ctx.ToString(node.subs[0]->keys[0], key_str)) return {}; return std::move(ret) + "pk(" + std::move(key_str) + ")"; } - if (node.subs[0]->nodetype == Fragment::PK_H) { + if (node.subs[0]->fragment == Fragment::PK_H) { // pkh(K) is syntactic sugar for c:pk_h(K) std::string key_str; if (!ctx.ToString(node.subs[0]->keys[0], key_str)) return {}; @@ -609,15 +609,15 @@ struct Node { 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]->nodetype == 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]->nodetype == Fragment::JUST_0) return "l" + std::move(subs[1]); - if (node.subs[1]->nodetype == 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; } - switch (node.nodetype) { + switch (node.fragment) { case Fragment::PK_K: { std::string key_str; if (!ctx.ToString(node.keys[0], key_str)) return {}; @@ -644,7 +644,7 @@ struct Node { 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]->nodetype == 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: { auto str = std::move(ret) + "multi(" + ::ToString(node.k); @@ -673,7 +673,7 @@ struct Node { } internal::Ops CalcOps() const { - switch (nodetype) { + switch (fragment) { case Fragment::JUST_1: return {0, 0, {}}; case Fragment::JUST_0: return {0, {}, 0}; case Fragment::PK_K: return {0, 0, 0}; @@ -747,7 +747,7 @@ struct Node { } internal::StackSize CalcStackSize() const { - switch (nodetype) { + switch (fragment) { case Fragment::JUST_0: return {{}, 0}; case Fragment::JUST_1: case Fragment::OLDER: @@ -803,7 +803,7 @@ struct Node { using namespace internal; auto helper = [&ctx](const Node& node, Span subres) -> InputResult { - switch (node.nodetype) { + switch (node.fragment) { case Fragment::PK_K: { std::vector sig; Availability avail = ctx.Sign(node.keys[0], sig); @@ -1017,7 +1017,7 @@ struct Node { { // TreeEval() doesn't support bool as NodeType, so use int instead. return TreeEval([&fn](const Node& node, Span subs) { - switch (node.nodetype) { + switch (node.fragment) { case Fragment::JUST_0: return false; case Fragment::JUST_1: @@ -1090,7 +1090,7 @@ struct Node { //! Equality testing. bool operator==(const Node& arg) const { - if (nodetype != arg.nodetype) return false; + if (fragment != arg.fragment) return false; if (k != arg.k) return false; if (data != arg.data) return false; if (keys != arg.keys) return false; @@ -1104,12 +1104,12 @@ struct Node { } // Constructors with various argument combinations. - Node(Fragment nt, std::vector> sub, std::vector arg, uint32_t val = 0) : nodetype(nt), k(val), data(std::move(arg)), subs(std::move(sub)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} - Node(Fragment nt, std::vector arg, uint32_t val = 0) : nodetype(nt), k(val), data(std::move(arg)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} - Node(Fragment nt, std::vector> sub, std::vector key, uint32_t val = 0) : nodetype(nt), k(val), keys(std::move(key)), subs(std::move(sub)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} - Node(Fragment nt, std::vector key, uint32_t val = 0) : nodetype(nt), k(val), keys(std::move(key)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} - Node(Fragment nt, std::vector> sub, uint32_t val = 0) : nodetype(nt), k(val), subs(std::move(sub)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} - Node(Fragment nt, uint32_t val = 0) : nodetype(nt), k(val), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} + Node(Fragment nt, std::vector> sub, std::vector arg, uint32_t val = 0) : fragment(nt), k(val), data(std::move(arg)), subs(std::move(sub)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} + Node(Fragment nt, std::vector arg, uint32_t val = 0) : fragment(nt), k(val), data(std::move(arg)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} + Node(Fragment nt, std::vector> sub, std::vector key, uint32_t val = 0) : fragment(nt), k(val), keys(std::move(key)), subs(std::move(sub)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} + Node(Fragment nt, std::vector key, uint32_t val = 0) : fragment(nt), k(val), keys(std::move(key)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} + Node(Fragment nt, std::vector> sub, uint32_t val = 0) : fragment(nt), k(val), subs(std::move(sub)), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} + Node(Fragment nt, uint32_t val = 0) : fragment(nt), k(val), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()) {} }; namespace internal { diff --git a/bitcoin/test/fuzz/miniscript_random.cpp b/bitcoin/test/fuzz/miniscript_random.cpp index 0e6dfbe..efba644 100644 --- a/bitcoin/test/fuzz/miniscript_random.cpp +++ b/bitcoin/test/fuzz/miniscript_random.cpp @@ -318,7 +318,7 @@ NodeRef GenNode(FuzzedDataProvider& provider) { todo.back() = std::move(node_info); for (uint8_t i = 0; i < n_subs; i++) todo.push_back({}); } else { - // The back of todo has nodetype and number of children decided, and + // 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 // stack. diff --git a/bitcoin/test/miniscript_tests.cpp b/bitcoin/test/miniscript_tests.cpp index bbd744b..18b77b7 100644 --- a/bitcoin/test/miniscript_tests.cpp +++ b/bitcoin/test/miniscript_tests.cpp @@ -240,17 +240,17 @@ std::set FindChallenges(const NodeRef& ref) { for (const auto& key : ref->keys) { chal.emplace(ChallengeType::PK, ChallengeNumber(key)); } - if (ref->nodetype == miniscript::Fragment::OLDER) { + if (ref->fragment == miniscript::Fragment::OLDER) { chal.emplace(ChallengeType::OLDER, ref->k); - } else if (ref->nodetype == miniscript::Fragment::AFTER) { + } else if (ref->fragment == miniscript::Fragment::AFTER) { chal.emplace(ChallengeType::AFTER, ref->k); - } else if (ref->nodetype == miniscript::Fragment::SHA256) { + } else if (ref->fragment == miniscript::Fragment::SHA256) { chal.emplace(ChallengeType::SHA256, ChallengeNumber(ref->data)); - } else if (ref->nodetype == miniscript::Fragment::RIPEMD160) { + } else if (ref->fragment == miniscript::Fragment::RIPEMD160) { chal.emplace(ChallengeType::RIPEMD160, ChallengeNumber(ref->data)); - } else if (ref->nodetype == miniscript::Fragment::HASH256) { + } else if (ref->fragment == miniscript::Fragment::HASH256) { chal.emplace(ChallengeType::HASH256, ChallengeNumber(ref->data)); - } else if (ref->nodetype == miniscript::Fragment::HASH160) { + } else if (ref->fragment == miniscript::Fragment::HASH160) { chal.emplace(ChallengeType::HASH160, ChallengeNumber(ref->data)); } for (const auto& sub : ref->subs) { diff --git a/compiler.cpp b/compiler.cpp index 3b3b4c1..3ced936 100644 --- a/compiler.cpp +++ b/compiler.cpp @@ -518,7 +518,7 @@ CostPair CalcCostPair(Fragment nt, const std::vector& s, double l return CostPair{Mul(l, sat) + Mul(r, nsat), nsat}; } } - throw std::runtime_error("Computing CostPair of unknown nodetype"); + throw std::runtime_error("Computing CostPair of unknown fragment"); } std::pair, std::vector> GetPQs(Fragment nt, double p, double q, double l, int m) { @@ -890,7 +890,7 @@ std::string Disassembler(CScript::const_iterator& it, CScript::const_iterator en /* std::string DebugNode(const Node& node) { - switch (node->nodetype) { + switch (node->fragment) { case Fragment::PK_K: return "pk"; case Fragment::PK_H: return "pk_h"; case Fragment::MULTI: return "multi(" + std::to_string(node->k) + " of " + std::to_string(node->keys.size()) + ")"; diff --git a/js_bindings.cpp b/js_bindings.cpp index 0366fc2..a8cc50c 100644 --- a/js_bindings.cpp +++ b/js_bindings.cpp @@ -41,41 +41,41 @@ std::string Props(const miniscript::NodeRef& node, std::string in) } std::string Analyze(const miniscript::NodeRef& node) { - switch (node->nodetype) { - case miniscript::NodeType::PK_K: { + switch (node->fragment) { + case miniscript::Fragment::PK_K: { std::string str; COMPILER_CTX.ToString(node->keys[0], str); return Props(node, "pk_k(" + std::move(str) + ")"); } - case miniscript::NodeType::PK_H: { + case miniscript::Fragment::PK_H: { std::string str; COMPILER_CTX.ToString(node->keys[0], str); return Props(node, "pk_h(" + std::move(str) + ")"); } - case miniscript::NodeType::MULTI: return Props(node, "multi(" + std::to_string(node->k) + " of " + std::to_string(node->keys.size()) + ")"); - case miniscript::NodeType::AFTER: return Props(node, "after(" + std::to_string(node->k) + ")"); - case miniscript::NodeType::OLDER: return Props(node, "older(" + std::to_string(node->k) + ")"); - case miniscript::NodeType::SHA256: return Props(node, "sha256()"); - case miniscript::NodeType::RIPEMD160: return Props(node, "ripemd160()"); - case miniscript::NodeType::HASH256: return Props(node, "hash256()"); - case miniscript::NodeType::HASH160: return Props(node, "hash160()"); - case miniscript::NodeType::JUST_0: return Props(node, "false"); - case miniscript::NodeType::JUST_1: return Props(node, "true"); - case miniscript::NodeType::WRAP_A: return Props(node, "a:") + " " + Analyze(node->subs[0]); - case miniscript::NodeType::WRAP_S: return Props(node, "s:") + " " + Analyze(node->subs[0]); - case miniscript::NodeType::WRAP_C: return Props(node, "c:") + " " + Analyze(node->subs[0]); - case miniscript::NodeType::WRAP_D: return Props(node, "d:") + " " + Analyze(node->subs[0]); - case miniscript::NodeType::WRAP_V: return Props(node, "v:") + " " + Analyze(node->subs[0]); - case miniscript::NodeType::WRAP_N: return Props(node, "n:") + " " + Analyze(node->subs[0]); - case miniscript::NodeType::WRAP_J: return Props(node, "j:") + " " + Analyze(node->subs[0]); - case miniscript::NodeType::AND_V: return Props(node, "and_v") + "
  • " + Analyze(node->subs[0]) + "
  • " + Analyze(node->subs[1]) + "
"; - case miniscript::NodeType::AND_B: return Props(node, "and_b") + "
  • " + Analyze(node->subs[0]) + "
  • " + Analyze(node->subs[1]) + "
"; - case miniscript::NodeType::OR_B: return Props(node, "or_b") + "
  • " + Analyze(node->subs[0]) + "
  • " + Analyze(node->subs[1]) + "
"; - case miniscript::NodeType::OR_C: return Props(node, "or_c") + "
  • " + Analyze(node->subs[0]) + "
  • " + Analyze(node->subs[1]) + "
"; - case miniscript::NodeType::OR_D: return Props(node, "or_d") + "
  • " + Analyze(node->subs[0]) + "
  • " + Analyze(node->subs[1]) + "
"; - case miniscript::NodeType::OR_I: return Props(node, "or_i") + "
  • " + Analyze(node->subs[0]) + "
  • " + Analyze(node->subs[1]) + "
"; - case miniscript::NodeType::ANDOR: return Props(node, "andor [or]") + "
  • andor [and]
    • " + Analyze(node->subs[0]) + "
    • " + Analyze(node->subs[1]) + "
  • " + Analyze(node->subs[2]) + "
"; - case miniscript::NodeType::THRESH: { + case miniscript::Fragment::MULTI: return Props(node, "multi(" + std::to_string(node->k) + " of " + std::to_string(node->keys.size()) + ")"); + case miniscript::Fragment::AFTER: return Props(node, "after(" + std::to_string(node->k) + ")"); + case miniscript::Fragment::OLDER: return Props(node, "older(" + std::to_string(node->k) + ")"); + case miniscript::Fragment::SHA256: return Props(node, "sha256()"); + case miniscript::Fragment::RIPEMD160: return Props(node, "ripemd160()"); + case miniscript::Fragment::HASH256: return Props(node, "hash256()"); + case miniscript::Fragment::HASH160: return Props(node, "hash160()"); + case miniscript::Fragment::JUST_0: return Props(node, "false"); + case miniscript::Fragment::JUST_1: return Props(node, "true"); + case miniscript::Fragment::WRAP_A: return Props(node, "a:") + " " + Analyze(node->subs[0]); + case miniscript::Fragment::WRAP_S: return Props(node, "s:") + " " + Analyze(node->subs[0]); + case miniscript::Fragment::WRAP_C: return Props(node, "c:") + " " + Analyze(node->subs[0]); + case miniscript::Fragment::WRAP_D: return Props(node, "d:") + " " + Analyze(node->subs[0]); + case miniscript::Fragment::WRAP_V: return Props(node, "v:") + " " + Analyze(node->subs[0]); + case miniscript::Fragment::WRAP_N: return Props(node, "n:") + " " + Analyze(node->subs[0]); + case miniscript::Fragment::WRAP_J: return Props(node, "j:") + " " + Analyze(node->subs[0]); + case miniscript::Fragment::AND_V: return Props(node, "and_v") + "
  • " + Analyze(node->subs[0]) + "
  • " + Analyze(node->subs[1]) + "
"; + case miniscript::Fragment::AND_B: return Props(node, "and_b") + "
  • " + Analyze(node->subs[0]) + "
  • " + Analyze(node->subs[1]) + "
"; + case miniscript::Fragment::OR_B: return Props(node, "or_b") + "
  • " + Analyze(node->subs[0]) + "
  • " + Analyze(node->subs[1]) + "
"; + case miniscript::Fragment::OR_C: return Props(node, "or_c") + "
  • " + Analyze(node->subs[0]) + "
  • " + Analyze(node->subs[1]) + "
"; + case miniscript::Fragment::OR_D: return Props(node, "or_d") + "
  • " + Analyze(node->subs[0]) + "
  • " + Analyze(node->subs[1]) + "
"; + case miniscript::Fragment::OR_I: return Props(node, "or_i") + "
  • " + Analyze(node->subs[0]) + "
  • " + Analyze(node->subs[1]) + "
"; + case miniscript::Fragment::ANDOR: return Props(node, "andor [or]") + "
  • andor [and]
    • " + Analyze(node->subs[0]) + "
    • " + Analyze(node->subs[1]) + "
  • " + Analyze(node->subs[2]) + "
"; + case miniscript::Fragment::THRESH: { auto ret = Props(node, "thresh(" + std::to_string(node->k) + " of " + std::to_string(node->subs.size()) + ")") + "
    "; for (const auto& sub : node->subs) { ret += "
  • " + Analyze(sub) + "
    • "; From d908b103d7f1a38c683f28b64c4b913d97d6635e Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 25 Feb 2022 16:12:30 -0500 Subject: [PATCH 3/6] Make only half of keys/preimages available to satisfier --- bitcoin/test/fuzz/miniscript_random.cpp | 30 ++++++++++++++----------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/bitcoin/test/fuzz/miniscript_random.cpp b/bitcoin/test/fuzz/miniscript_random.cpp index efba644..d8377fe 100644 --- a/bitcoin/test/fuzz/miniscript_random.cpp +++ b/bitcoin/test/fuzz/miniscript_random.cpp @@ -20,7 +20,7 @@ struct TestData { // Precomputed public keys, and a dummy signature for each of them. std::vector dummy_keys; std::map dummy_keys_map; - std::map> dummy_sigs; + std::map, bool>> dummy_sigs; // Precomputed hashes of each kind. std::vector> sha256; @@ -46,24 +46,24 @@ struct TestData { std::vector sig; privkey.Sign(uint256S(""), sig); sig.push_back(1); // SIGHASH_ALL - dummy_sigs.insert({pubkey, sig}); + dummy_sigs.insert({pubkey, {sig, i & 1}}); std::vector hash; hash.resize(32); CSHA256().Write(keydata, 32).Finalize(hash.data()); sha256.push_back(hash); - sha256_preimages[hash] = std::vector(keydata, keydata + 32); + if (i & 1) sha256_preimages[hash] = std::vector(keydata, keydata + 32); CHash256().Write(keydata).Finalize(hash); hash256.push_back(hash); - hash256_preimages[hash] = std::vector(keydata, keydata + 32); + if (i & 1) hash256_preimages[hash] = std::vector(keydata, keydata + 32); hash.resize(20); CRIPEMD160().Write(keydata, 32).Finalize(hash.data()); assert(hash.size() == 20); ripemd160.push_back(hash); - ripemd160_preimages[hash] = std::vector(keydata, keydata + 32); + if (i & 1) ripemd160_preimages[hash] = std::vector(keydata, keydata + 32); CHash160().Write(keydata).Finalize(hash); hash160.push_back(hash); - hash160_preimages[hash] = std::vector(keydata, keydata + 32); + if (i & 1) hash160_preimages[hash] = std::vector(keydata, keydata + 32); } } }; @@ -118,8 +118,13 @@ struct SatisfierContext: ParserContext { miniscript::Availability Sign(const CPubKey& key, std::vector& sig) const { const auto it = test_data->dummy_sigs.find(key); if (it == test_data->dummy_sigs.end()) return miniscript::Availability::NO; - sig = it->second; - return miniscript::Availability::YES; + if (it->second.second) { + // Key is "available" + sig = it->second.first; + return miniscript::Availability::YES; + } else { + return miniscript::Availability::NO; + } } //! Lookup generalization for all the hash satisfactions below @@ -149,18 +154,17 @@ struct SatisfierContext: ParserContext { struct CheckerContext: BaseSignatureChecker { TestData *test_data; - // Signature checker methods. Checks the right dummy signature is used. Always assumes timelocks are - // correct. + // Signature checker methods. Checks the right dummy signature is used. bool CheckECDSASignature(const std::vector& sig, const std::vector& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override { const CPubKey key{vchPubKey}; const auto it = test_data->dummy_sigs.find(key); if (it == test_data->dummy_sigs.end()) return false; - return it->second == sig; + return it->second.first == sig; } - bool CheckLockTime(const CScriptNum& nLockTime) const override { return true; } - bool CheckSequence(const CScriptNum& nSequence) const override { return true; } + bool CheckLockTime(const CScriptNum& nLockTime) const override { return nLockTime.GetInt64() & 1; } + bool CheckSequence(const CScriptNum& nSequence) const override { return nSequence.GetInt64() & 1; } }; // The various contexts From c8f70ce9dfa7cf87e517b7af035db0cd077b3e30 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 25 Feb 2022 14:08:12 -0500 Subject: [PATCH 4/6] Compare policy satisfiability with satisfier --- bitcoin/test/fuzz/miniscript_random.cpp | 40 +++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/bitcoin/test/fuzz/miniscript_random.cpp b/bitcoin/test/fuzz/miniscript_random.cpp index d8377fe..ce2a9ea 100644 --- a/bitcoin/test/fuzz/miniscript_random.cpp +++ b/bitcoin/test/fuzz/miniscript_random.cpp @@ -177,6 +177,7 @@ const CScript DUMMY_SCRIPTSIG; using Fragment = miniscript::Fragment; using NodeRef = miniscript::NodeRef; +using Node = miniscript::Node; using miniscript::operator"" _mst; //! Construct a miniscript node as a shared_ptr. @@ -437,4 +438,43 @@ FUZZ_TARGET_INIT(miniscript_random, initialize_miniscript_random) // For sane nodes, the two algorithms behave identically. assert(mal_success == nonmal_success); } + + // Verify that if a node is policy-satisfiable, the malleable satisfaction + // algorithm succeeds. Given that under IsSaneTopLevel() both satisfactions + // are identical, this implies that for such nodes, the non-malleable + // satisfaction will also match the expected policy. + bool satisfiable = node->IsSatisfiable([](const Node& node) -> bool { + switch (node.fragment) { + case Fragment::PK_K: + case Fragment::PK_H: { + auto it = TEST_DATA.dummy_sigs.find(node.keys[0]); + assert(it != TEST_DATA.dummy_sigs.end()); + return it->second.second; + } + case Fragment::MULTI: { + size_t sats = 0; + for (const auto& key : node.keys) { + auto it = TEST_DATA.dummy_sigs.find(key); + assert(it != TEST_DATA.dummy_sigs.end()); + sats += it->second.second; + } + return sats >= node.k; + } + case Fragment::OLDER: + case Fragment::AFTER: + return node.k & 1; + case Fragment::SHA256: + return TEST_DATA.sha256_preimages.count(node.data); + case Fragment::HASH256: + return TEST_DATA.hash256_preimages.count(node.data); + case Fragment::RIPEMD160: + return TEST_DATA.ripemd160_preimages.count(node.data); + case Fragment::HASH160: + return TEST_DATA.hash160_preimages.count(node.data); + default: + assert(false); + } + return false; + }); + assert(mal_success == satisfiable); } From 02a41938653a17d8bb3dfed9f01d71149832b737 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 25 Feb 2022 16:41:16 -0500 Subject: [PATCH 5/6] Perform more checks on non-toplevel scripts --- bitcoin/test/fuzz/miniscript_random.cpp | 37 +++++++++++++++---------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/bitcoin/test/fuzz/miniscript_random.cpp b/bitcoin/test/fuzz/miniscript_random.cpp index ce2a9ea..ab13dc0 100644 --- a/bitcoin/test/fuzz/miniscript_random.cpp +++ b/bitcoin/test/fuzz/miniscript_random.cpp @@ -368,13 +368,32 @@ FUZZ_TARGET_INIT(miniscript_random, initialize_miniscript_random) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - // Generate a top-level node + // Generate a node const auto node = GenNode(fuzzed_data_provider); - if (!node || !node->IsValidTopLevel()) return; + if (!node) return; - // Check roundtrip to Script, and consistency between script size estimation and real size + // Check that it roundtrips to text representation + std::string str; + assert(node->ToString(PARSER_CTX, str)); + auto parsed = miniscript::FromString(str, PARSER_CTX); + assert(parsed); + assert(*parsed == *node); + + // Check consistency between script size estimation and real size. const auto script = node->ToScript(PARSER_CTX); assert(node->ScriptSize() == script.size()); + + // Check consistency of "x" property with the script (type K is excluded, because it can end + // with a push of a key, which could match these opcodes). + if (!(node->GetType() << "K"_mst)) { + bool ends_in_verify = !(node->GetType() << "x"_mst); + assert(ends_in_verify == (script.back() == OP_CHECKSIG || script.back() == OP_CHECKMULTISIG || script.back() == OP_EQUAL)); + } + + // The rest of the checks only apply when testing a valid top-level script. + if (!node->IsValidTopLevel()) return; + + // Check roundtrip to script auto decoded = miniscript::FromScript(script, PARSER_CTX); assert(decoded); // Note we can't use *decoded == *node because the miniscript representation may differ, so we check that: @@ -383,18 +402,6 @@ FUZZ_TARGET_INIT(miniscript_random, initialize_miniscript_random) assert(decoded->ToScript(PARSER_CTX) == script); assert(decoded->GetType() == node->GetType()); - // Check consistency of "x" property with the script (relying on the fact that no - // top-level scripts end with a hash or key push, whose last byte could match these opcodes). - bool ends_in_verify = !(node->GetType() << "x"_mst); - assert(ends_in_verify == (script.back() == OP_CHECKSIG || script.back() == OP_CHECKMULTISIG || script.back() == OP_EQUAL)); - - // Check that it roundtrips to text representation - std::string str; - assert(node->ToString(PARSER_CTX, str)); - auto parsed = miniscript::FromString(str, PARSER_CTX); - assert(parsed); - assert(*parsed == *node); - // Run malleable satisfaction algorithm. const CScript script_pubkey = CScript() << OP_0 << WitnessV0ScriptHash(script); CScriptWitness witness_mal; From 5d0bb8e45b11b85f3c5628fbd97d6eda6e7172a0 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 25 Feb 2022 17:29:54 -0500 Subject: [PATCH 6/6] Optionally pad script to max out ops --- bitcoin/test/fuzz/miniscript_random.cpp | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/bitcoin/test/fuzz/miniscript_random.cpp b/bitcoin/test/fuzz/miniscript_random.cpp index ab13dc0..c35d00b 100644 --- a/bitcoin/test/fuzz/miniscript_random.cpp +++ b/bitcoin/test/fuzz/miniscript_random.cpp @@ -380,7 +380,7 @@ FUZZ_TARGET_INIT(miniscript_random, initialize_miniscript_random) assert(*parsed == *node); // Check consistency between script size estimation and real size. - const auto script = node->ToScript(PARSER_CTX); + auto script = node->ToScript(PARSER_CTX); assert(node->ScriptSize() == script.size()); // Check consistency of "x" property with the script (type K is excluded, because it can end @@ -402,6 +402,22 @@ FUZZ_TARGET_INIT(miniscript_random, initialize_miniscript_random) assert(decoded->ToScript(PARSER_CTX) == script); assert(decoded->GetType() == node->GetType()); + if (fuzzed_data_provider.ConsumeBool()) { + // Optionally pad the script with OP_NOPs to max op the ops limit of the constructed script. + // This makes the script obviously not actually miniscript-compatible anymore, but the + // signatures constructed in this test don't commit to the script anyway, so the same + // miniscript satisfier will work. This increases the sensitivity of the test to the ops + // counting logic being too low, especially for simple scripts. + // Do this optionally because we're not solely interested in cases where the number of ops is + // maximal. + // Do not pad more than what would cause MAX_STANDARD_P2WSH_SCRIPT_SIZE to be reached, however, + // as that also invalidates scripts. + int add = std::min( + MAX_OPS_PER_SCRIPT - node->GetOps(), + MAX_STANDARD_P2WSH_SCRIPT_SIZE - node->ScriptSize()); + for (int i = 0; i < add; ++i) script.push_back(OP_NOP); + } + // Run malleable satisfaction algorithm. const CScript script_pubkey = CScript() << OP_0 << WitnessV0ScriptHash(script); CScriptWitness witness_mal;