diff --git a/bitcoin/script/miniscript.cpp b/bitcoin/script/miniscript.cpp index 958fa45..3937638 100644 --- a/bitcoin/script/miniscript.cpp +++ b/bitcoin/script/miniscript.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2019 The Bitcoin Core developers +// Copyright (c) 2019-2022 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -172,8 +172,8 @@ Type ComputeType(Fragment fragment, Type x, Type y, Type z, const std::vector 16) + (k > 16) + 34 * n_keys; + case Fragment::MULTI: return 1 + BuildScript(n_keys).size() + BuildScript(k).size() + 34 * n_keys; case Fragment::AND_V: return subsize; case Fragment::WRAP_V: return subsize + (sub0typ << "x"_mst); case Fragment::WRAP_S: @@ -278,10 +277,9 @@ size_t ComputeScriptLen(Fragment fragment, Type sub0typ, size_t subsize, uint32_ case Fragment::THRESH: return subsize + n_subs + BuildScript(k).size(); } assert(false); - return 0; } -InputStack& InputStack::Available(Availability avail) { +InputStack& InputStack::SetAvailable(Availability avail) { available = avail; if (avail == Availability::NO) { stack.clear(); @@ -293,37 +291,37 @@ InputStack& InputStack::Available(Availability avail) { return *this; } -InputStack& InputStack::WithSig() { +InputStack& InputStack::SetWithSig() { has_sig = true; return *this; } -InputStack& InputStack::NonCanon() { +InputStack& InputStack::SetNonCanon() { non_canon = true; return *this; } -InputStack& InputStack::Malleable(bool x) { +InputStack& InputStack::SetMalleable(bool x) { malleable = x; return *this; } InputStack operator+(InputStack a, InputStack b) { a.stack = Cat(std::move(a.stack), std::move(b.stack)); - a.size += b.size; + if (a.available != Availability::NO && b.available != Availability::NO) a.size += b.size; a.has_sig |= b.has_sig; a.malleable |= b.malleable; a.non_canon |= b.non_canon; if (a.available == Availability::NO || b.available == Availability::NO) { - a.Available(Availability::NO); + a.SetAvailable(Availability::NO); } else if (a.available == Availability::MAYBE || b.available == Availability::MAYBE) { - a.Available(Availability::MAYBE); + a.SetAvailable(Availability::MAYBE); } return a; } InputStack operator|(InputStack a, InputStack b) { - // If only one (or neither) is valid, pick the other one. + // If only one is invalid, pick the other one. If both are invalid, pick an arbitrary one. if (a.available == Availability::NO) return b; if (b.available == Availability::NO) return a; // If only one of the solutions has a signature, we must pick the other one. @@ -351,9 +349,9 @@ InputStack operator|(InputStack a, InputStack b) { } } -std::optional>>> DecomposeScript(const CScript& script) +std::optional> DecomposeScript(const CScript& script) { - std::vector>> out; + std::vector out; CScript::const_iterator it = script.begin(), itend = script.end(); while (it != itend) { std::vector push_data; @@ -387,7 +385,7 @@ std::optional>>> De return out; } -std::optional ParseScriptNumber(const std::pair>& in) { +std::optional ParseScriptNumber(const Opcode& in) { if (in.first == OP_0) { return 0; } diff --git a/bitcoin/script/miniscript.h b/bitcoin/script/miniscript.h index a0b2c7f..c42b530 100644 --- a/bitcoin/script/miniscript.h +++ b/bitcoin/script/miniscript.h @@ -1,4 +1,4 @@ -// Copyright (c) 2019 The Bitcoin Core developers +// Copyright (c) 2019-2022 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -6,14 +6,15 @@ #define BITCOIN_SCRIPT_MINISCRIPT_H #include +#include #include #include #include #include #include -#include #include +#include #include #include @@ -42,9 +43,9 @@ namespace miniscript { * - Takes its inputs from the top of the stack. * - When satisfied, pushes nothing. * - Cannot be dissatisfied. - * - This is obtained by adding an OP_VERIFY to a B, modifying the last opcode + * - This can be obtained by adding an OP_VERIFY to a B, modifying the last opcode * of a B to its -VERIFY version (only for OP_CHECKSIG, OP_CHECKSIGVERIFY - * and OP_EQUAL), or using IFs where both branches are also Vs. + * and OP_EQUAL), or by combining a V fragment under some conditions. * - For example vc:pk_k(key) = OP_CHECKSIGVERIFY * - "K" Key: * - Takes its inputs from the top of the stack. @@ -179,6 +180,8 @@ inline constexpr Type operator"" _mst(const char* c, size_t l) { return typ; } +using Opcode = std::pair>; + template struct Node; template using NodeRef = std::shared_ptr>; @@ -261,29 +264,36 @@ struct InputStack { //! Construct a valid single-element stack (with an element up to 75 bytes). InputStack(std::vector in) : size(in.size() + 1), stack(Vector(std::move(in))) {} //! Change availability - InputStack& Available(Availability avail); + InputStack& SetAvailable(Availability avail); //! Mark this input stack as having a signature. - InputStack& WithSig(); + InputStack& SetWithSig(); //! Mark this input stack as non-canonical (known to not be necessary in non-malleable satisfactions). - InputStack& NonCanon(); + InputStack& SetNonCanon(); //! Mark this input stack as malleable. - InputStack& Malleable(bool x = true); + InputStack& SetMalleable(bool x = true); //! Concatenate two input stacks. friend InputStack operator+(InputStack a, InputStack b); //! Choose between two potential input stacks. friend InputStack operator|(InputStack a, InputStack b); }; +/** A stack consisting of a single zero-length element (interpreted as 0 by the script interpreter in numeric context). */ static const auto ZERO = InputStack(std::vector()); -static const auto ZERO32 = InputStack(std::vector(32, 0)).Malleable(); +/** A stack consisting of a single malleable 32-byte 0x0000...0000 element (for dissatisfying hash challenges). */ +static const auto ZERO32 = InputStack(std::vector(32, 0)).SetMalleable(); +/** A stack consisting of a single 0x01 element (interpreted as 1 by the script interpreted in numeric context). */ static const auto ONE = InputStack(Vector((unsigned char)1)); +/** The empty stack. */ static const auto EMPTY = InputStack(); -static const auto INVALID = InputStack().Available(Availability::NO); +/** A stack representing the lack of any (dis)satisfactions. */ +static const auto INVALID = InputStack().SetAvailable(Availability::NO); //! A pair of a satisfaction and a dissatisfaction InputStack. struct InputResult { InputStack nsat, sat; - InputResult(InputStack in_nsat, InputStack in_sat) : nsat(std::move(in_nsat)), sat(std::move(in_sat)) {} + + template + InputResult(A&& in_nsat, B&& in_sat) : nsat(std::forward(in_nsat)), sat(std::forward(in_sat)) {} }; //! Class whose objects represent the maximum of a list of integers. @@ -327,6 +337,8 @@ struct StackSize { StackSize(MaxInt in_sat, MaxInt in_dsat) : sat(in_sat), dsat(in_dsat) {}; }; +struct NoDupCheck {}; + } // namespace internal //! A node in a miniscript expression. @@ -352,8 +364,13 @@ struct Node { const Type typ; //! Cached script length (computed by CalcScriptLen). const size_t scriptlen; - //! Whether a public key appears more than once in this node. - const bool duplicate_key; + //! 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 has_duplicate_keys; + //! Compute the length of the script for this miniscript (including children). size_t CalcScriptLen() const { @@ -532,30 +549,6 @@ struct Node { return SanitizeType(ComputeType(fragment, x, y, z, sub_types, k, data.size(), subs.size(), keys.size())); } - //! Returns true if the node contains at least one duplicate key. - bool ContainsDuplicateKey() const { - auto upfn = [this](const Node& node, Span> subs) -> std::optional> { - if (&node != this && node.duplicate_key) return {}; - - size_t keys_count = node.keys.size(); - std::set key_set{node.keys.begin(), node.keys.end()}; - if (key_set.size() != keys_count) return {}; - - for (auto& sub: subs) { - keys_count += sub.size(); - // Small optimization: std::set::merge is linear in the size of the second arg but - // logarithmic in the size of the first. - if (key_set.size() < sub.size()) std::swap(key_set, sub); - key_set.merge(sub); - if (key_set.size() != keys_count) return {}; - } - - return key_set; - }; - - return !TreeEvalMaybe>(upfn); - } - public: template CScript ToScript(const Ctx& ctx) const @@ -622,7 +615,6 @@ struct Node { } } assert(false); - return {}; }; return TreeEval(false, downfn, upfn); } @@ -722,14 +714,15 @@ struct Node { } return std::move(str) + ")"; } - default: assert(false); + default: break; } - return ""; // Should never be reached. + assert(false); }; return TreeEvalMaybe(false, downfn, upfn); } +private: internal::Ops CalcOps() const { switch (fragment) { case Fragment::JUST_1: return {0, 0, {}}; @@ -801,7 +794,6 @@ struct Node { } } assert(false); - return {0, {}, {}}; } internal::StackSize CalcStackSize() const { @@ -852,117 +844,152 @@ struct Node { } } assert(false); - return {{}, {}}; } - template internal::InputResult ProduceInput(const Ctx& ctx) const { using namespace internal; + // Internal function which is invoked for every tree node, constructing satisfaction/dissatisfactions + // given those of its subnodes. auto helper = [&ctx](const Node& node, Span subres) -> InputResult { switch (node.fragment) { case Fragment::PK_K: { std::vector sig; Availability avail = ctx.Sign(node.keys[0], sig); - return InputResult(ZERO, InputStack(std::move(sig)).WithSig().Available(avail)); + return {ZERO, InputStack(std::move(sig)).SetWithSig().SetAvailable(avail)}; } case Fragment::PK_H: { std::vector key = ctx.ToPKBytes(node.keys[0]), sig; Availability avail = ctx.Sign(node.keys[0], sig); - return InputResult(ZERO + InputStack(key), (InputStack(std::move(sig)).WithSig() + InputStack(key)).Available(avail)); + return {ZERO + InputStack(key), (InputStack(std::move(sig)).SetWithSig() + InputStack(key)).SetAvailable(avail)}; } case Fragment::MULTI: { + // sats[j] represents the best stack containing j valid signatures (out of the first i keys). + // In the loop below, these stacks are built up using a dynamic programming approach. + // sats[0] starts off being {0}, due to the CHECKMULTISIG bug that pops off one element too many. std::vector sats = Vector(ZERO); for (size_t i = 0; i < node.keys.size(); ++i) { std::vector sig; Availability avail = ctx.Sign(node.keys[i], sig); - auto sat = InputStack(std::move(sig)).WithSig().Available(avail); + // Compute signature stack for just the i'th key. + auto sat = InputStack(std::move(sig)).SetWithSig().SetAvailable(avail); + // Compute the next sats vector: next_sats[0] is a copy of sats[0] (no signatures). All further + // next_sats[j] are equal to either the existing sats[j], or sats[j-1] plus a signature for the + // current (i'th) key. The very last element needs all signatures filled. std::vector next_sats; next_sats.push_back(sats[0]); for (size_t j = 1; j < sats.size(); ++j) next_sats.push_back(sats[j] | (std::move(sats[j - 1]) + sat)); next_sats.push_back(std::move(sats[sats.size() - 1]) + std::move(sat)); + // Switch over. sats = std::move(next_sats); } + // The dissatisfaction consists of k+1 stack elements all equal to 0. InputStack nsat = ZERO; for (size_t i = 0; i < node.k; ++i) nsat = std::move(nsat) + ZERO; assert(node.k <= sats.size()); - return InputResult(std::move(nsat), std::move(sats[node.k])); + return {std::move(nsat), std::move(sats[node.k])}; } case Fragment::THRESH: { + // sats[k] represents the best stack that satisfies k out of the *last* i subexpressions. + // In the loop below, these stacks are built up using a dynamic programming approach. + // sats[0] starts off empty. std::vector sats = Vector(EMPTY); for (size_t i = 0; i < subres.size(); ++i) { + // Introduce an alias for the i'th last satisfaction/dissatisfaction. auto& res = subres[subres.size() - i - 1]; + // Compute the next sats vector: next_sats[0] is sats[0] plus res.nsat (thus containing all dissatisfactions + // so far. next_sats[j] is either sats[j] + res.nsat (reusing j earlier satisfactions) or sats[j-1] + res.sat + // (reusing j-1 earlier satisfactions plus a new one). The very last next_sats[j] is all satisfactions. std::vector next_sats; next_sats.push_back(sats[0] + res.nsat); for (size_t j = 1; j < sats.size(); ++j) next_sats.push_back((sats[j] + res.nsat) | (std::move(sats[j - 1]) + res.sat)); next_sats.push_back(std::move(sats[sats.size() - 1]) + std::move(res.sat)); + // Switch over. sats = std::move(next_sats); } + // At this point, sats[k].sat is the best satisfaction for the overall thresh() node. The best dissatisfaction + // is computed by gathering all sats[i].nsat for i != k. InputStack nsat = INVALID; for (size_t i = 0; i < sats.size(); ++i) { - // i==k is the satisfaction; i==0 is the canonical dissatisfaction; the rest are non-canonical. - if (i != 0 && i != node.k) sats[i].NonCanon(); + // i==k is the satisfaction; i==0 is the canonical dissatisfaction; + // the rest are non-canonical (a no-signature dissatisfaction - the i=0 + // form - is always available) and malleable (due to overcompleteness). + // Marking the solutions malleable here is not strictly necessary, as they + // should already never be picked in non-malleable solutions due to the + // availability of the i=0 form. + if (i != 0 && i != node.k) sats[i].SetMalleable().SetNonCanon(); + // Include all dissatisfactions (even these non-canonical ones) in nsat. if (i != node.k) nsat = std::move(nsat) | std::move(sats[i]); } assert(node.k <= sats.size()); - return InputResult(std::move(nsat), std::move(sats[node.k])); + return {std::move(nsat), std::move(sats[node.k])}; } case Fragment::OLDER: { - return InputResult(INVALID, ctx.CheckOlder(node.k) ? EMPTY : INVALID); + return {INVALID, ctx.CheckOlder(node.k) ? EMPTY : INVALID}; } case Fragment::AFTER: { - return InputResult(INVALID, ctx.CheckAfter(node.k) ? EMPTY : INVALID); + return {INVALID, ctx.CheckAfter(node.k) ? EMPTY : INVALID}; } case Fragment::SHA256: { std::vector preimage; Availability avail = ctx.SatSHA256(node.data, preimage); - return InputResult(ZERO32, InputStack(std::move(preimage)).Available(avail)); + return {ZERO32, InputStack(std::move(preimage)).SetAvailable(avail)}; } case Fragment::RIPEMD160: { std::vector preimage; Availability avail = ctx.SatRIPEMD160(node.data, preimage); - return InputResult(ZERO32, InputStack(std::move(preimage)).Available(avail)); + return {ZERO32, InputStack(std::move(preimage)).SetAvailable(avail)}; } case Fragment::HASH256: { std::vector preimage; Availability avail = ctx.SatHASH256(node.data, preimage); - return InputResult(ZERO32, InputStack(std::move(preimage)).Available(avail)); + return {ZERO32, InputStack(std::move(preimage)).SetAvailable(avail)}; } case Fragment::HASH160: { std::vector preimage; Availability avail = ctx.SatHASH160(node.data, preimage); - return InputResult(ZERO32, InputStack(std::move(preimage)).Available(avail)); + return {ZERO32, InputStack(std::move(preimage)).SetAvailable(avail)}; } case Fragment::AND_V: { auto& x = subres[0], &y = subres[1]; - return InputResult((y.nsat + x.sat).NonCanon(), y.sat + x.sat); + // As the dissatisfaction here only consist of a single option, it doesn't + // actually need to be listed (it's not required for reasoning about malleability of + // other options), and is never required (no valid miniscript relies on the ability + // to satisfy the type V left subexpression). It's still listed here for + // completeness, as a hypothetical (not currently implemented) satisfier that doesn't + // care about malleability might in some cases prefer it still. + return {(y.nsat + x.sat).SetNonCanon(), y.sat + x.sat}; } case Fragment::AND_B: { auto& x = subres[0], &y = subres[1]; - return InputResult((y.nsat + x.nsat) | (y.sat + x.nsat).NonCanon() | (y.nsat + x.sat).NonCanon(), y.sat + x.sat); + // Note that it is not strictly necessary to mark the 2nd and 3rd dissatisfaction here + // as malleable. While they are definitely malleable, they are also non-canonical due + // to the guaranteed existence of a no-signature other dissatisfaction (the 1st) + // option. Because of that, the 2nd and 3rd option will never be chosen, even if they + // weren't marked as malleable. + return {(y.nsat + x.nsat) | (y.sat + x.nsat).SetMalleable().SetNonCanon() | (y.nsat + x.sat).SetMalleable().SetNonCanon(), y.sat + x.sat}; } case Fragment::OR_B: { auto& x = subres[0], &z = subres[1]; // The (sat(Z) sat(X)) solution is overcomplete (attacker can change either into dsat). - return InputResult(z.nsat + x.nsat, (z.nsat + x.sat) | (z.sat + x.nsat) | (z.sat + x.sat).Malleable()); + return {z.nsat + x.nsat, (z.nsat + x.sat) | (z.sat + x.nsat) | (z.sat + x.sat).SetMalleable().SetNonCanon()}; } case Fragment::OR_C: { auto& x = subres[0], &z = subres[1]; - return InputResult(INVALID, std::move(x.sat) | (z.sat + x.nsat)); + return {INVALID, std::move(x.sat) | (z.sat + x.nsat)}; } case Fragment::OR_D: { auto& x = subres[0], &z = subres[1]; - auto nsat = z.nsat + x.nsat, sat_l = x.sat, sat_r = z.sat + x.nsat; - return InputResult(z.nsat + x.nsat, std::move(x.sat) | (z.sat + x.nsat)); + return {z.nsat + x.nsat, std::move(x.sat) | (z.sat + x.nsat)}; } case Fragment::OR_I: { auto& x = subres[0], &z = subres[1]; - return InputResult((x.nsat + ONE) | (z.nsat + ZERO), (x.sat + ONE) | (z.sat + ZERO)); + return {(x.nsat + ONE) | (z.nsat + ZERO), (x.sat + ONE) | (z.sat + ZERO)}; } case Fragment::ANDOR: { auto& x = subres[0], &y = subres[1], &z = subres[2]; - return InputResult((y.nsat + x.sat).NonCanon() | (z.nsat + x.nsat), (y.sat + x.sat) | (z.sat + x.nsat)); + return {(y.nsat + x.sat).SetNonCanon() | (z.nsat + x.nsat), (y.sat + x.sat) | (z.sat + x.nsat)}; } case Fragment::WRAP_A: case Fragment::WRAP_S: @@ -971,7 +998,7 @@ struct Node { return std::move(subres[0]); case Fragment::WRAP_D: { auto &x = subres[0]; - return InputResult(ZERO, x.sat + ONE); + return {ZERO, x.sat + ONE}; } case Fragment::WRAP_J: { auto &x = subres[0]; @@ -980,17 +1007,17 @@ struct Node { // if a dissatisfaction with a top zero element is found, we don't know whether another one with a // nonzero top stack element exists. Make the conservative assumption that whenever the subexpression is weakly // dissatisfiable, this alternative dissatisfaction exists and leads to malleability. - return InputResult(InputStack(ZERO).Malleable(x.nsat.available != Availability::NO && !x.nsat.has_sig), std::move(x.sat)); + return {InputStack(ZERO).SetMalleable(x.nsat.available != Availability::NO && !x.nsat.has_sig), std::move(x.sat)}; } case Fragment::WRAP_V: { auto &x = subres[0]; - return InputResult(INVALID, std::move(x.sat)); + return {INVALID, std::move(x.sat)}; } - case Fragment::JUST_0: return InputResult(EMPTY, INVALID); - case Fragment::JUST_1: return InputResult(INVALID, EMPTY); + case Fragment::JUST_0: return {EMPTY, INVALID}; + case Fragment::JUST_1: return {INVALID, EMPTY}; } assert(false); - return InputResult(INVALID, INVALID); + return {INVALID, INVALID}; }; auto tester = [&helper](const Node& node, Span subres) -> InputResult { @@ -1023,9 +1050,9 @@ struct Node { if (node.GetType() << "f"_mst && ret.nsat.available != Availability::NO) assert(ret.nsat.has_sig); if (node.GetType() << "s"_mst && ret.sat.available != Availability::NO) assert(ret.sat.has_sig); - // For 'e' nodes, a non-malleable dissatisfaction must exist. - if (node.GetType() << "e"_mst) assert(ret.nsat.available != Availability::NO); - if (node.GetType() << "e"_mst) assert(!ret.nsat.malleable); + // For non-malleable 'e' nodes, a non-malleable dissatisfaction must exist. + if (node.GetType() << "me"_mst) assert(ret.nsat.available != Availability::NO); + if (node.GetType() << "me"_mst) assert(!ret.nsat.malleable); // For 'm' nodes, if a satisfaction exists, it must be non-malleable. if (node.GetType() << "m"_mst && ret.sat.available != Availability::NO) assert(!ret.sat.malleable); @@ -1040,6 +1067,69 @@ struct Node { } public: + /** Update duplicate key information in this Node. + * + * This uses a custom key comparator provided by the context in order to still detect duplicates + * for more complicated types. + */ + template void DuplicateKeyCheck(const Ctx& ctx) const + { + // We cannot use a lambda here, as lambdas are non assignable, and the set operations + // below require moving the comparators around. + struct Comp { + const Ctx* ctx_ptr; + Comp(const Ctx& ctx) : ctx_ptr(&ctx) {} + bool operator()(const Key& a, const Key& b) const { return ctx_ptr->KeyCompare(a, b); } + }; + + // state in the recursive computation: + // - std::nullopt means "this node has duplicates" + // - an std::set means "this node has no duplicate keys, and they are: ...". + using keyset = std::set; + using state = std::optional; + + auto upfn = [&ctx](const Node& node, Span subs) -> state { + // If this node is already known to have duplicates, nothing left to do. + if (node.has_duplicate_keys.has_value() && *node.has_duplicate_keys) return {}; + + // Check if one of the children is already known to have duplicates. + for (auto& sub : subs) { + if (!sub.has_value()) { + node.has_duplicate_keys = true; + return {}; + } + } + + // Start building the set of keys involved in this node and children. + // Start by keys in this node directly. + size_t keys_count = node.keys.size(); + keyset key_set{node.keys.begin(), node.keys.end(), Comp(ctx)}; + if (key_set.size() != keys_count) { + // It already has duplicates; bail out. + node.has_duplicate_keys = true; + return {}; + } + + // Merge the keys from the children into this set. + for (auto& sub : subs) { + keys_count += sub->size(); + // Small optimization: std::set::merge is linear in the size of the second arg but + // logarithmic in the size of the first. + if (key_set.size() < sub->size()) std::swap(key_set, *sub); + key_set.merge(*sub); + if (key_set.size() != keys_count) { + node.has_duplicate_keys = true; + return {}; + } + } + + node.has_duplicate_keys = false; + return key_set; + }; + + TreeEval(upfn); + } + //! Return the size of the script for this expression (faster than ToScript().size()). size_t ScriptSize() const { return scriptlen; } @@ -1063,7 +1153,7 @@ struct Node { const Node* FindInsaneSub() const { return TreeEval([](const Node& node, Span subs) -> const Node* { for (auto& sub: subs) if (sub) return sub; - if (!node.IsSane()) return &node; + if (!node.IsSaneSubexpression()) return &node; return nullptr; }); } @@ -1074,7 +1164,7 @@ struct Node { bool IsSatisfiable(F fn) const { // TreeEval() doesn't support bool as NodeType, so use int instead. - return TreeEval([&fn](const Node& node, Span subs) { + return TreeEval([&fn](const Node& node, Span subs) -> bool { switch (node.fragment) { case Fragment::JUST_0: return false; @@ -1104,7 +1194,7 @@ struct Node { return std::count(subs.begin(), subs.end(), true) >= node.k; default: // wrappers assert(subs.size() == 1); - return !!subs[0]; + return subs[0]; } }); } @@ -1124,15 +1214,17 @@ struct Node { //! Check whether there is no satisfaction path that contains both timelocks and heightlocks bool CheckTimeLocksMix() const { return GetType() << "k"_mst; } + //! Check whether there is no duplicate key across this fragment and all its sub-fragments. + bool CheckDuplicateKey() const { return has_duplicate_keys && !*has_duplicate_keys; } + //! Whether successful non-malleable satisfactions are guaranteed to be valid. bool ValidSatisfactions() const { return IsValid() && CheckOpsLimit() && CheckStackSize(); } - //! Whether the apparent policy of this node matches its script semantics. - bool IsSane() const { return ValidSatisfactions() && IsNonMalleable() && CheckTimeLocksMix() && !duplicate_key; } - + //! Whether the apparent policy of this node matches its script semantics. Doesn't guarantee it is a safe script on its own. + bool IsSaneSubexpression() const { return ValidSatisfactions() && IsNonMalleable() && CheckTimeLocksMix() && CheckDuplicateKey(); } //! Check whether this node is safe as a script on its own. - bool IsSaneTopLevel() const { return IsValidTopLevel() && IsSane() && NeedsSignature(); } + bool IsSane() const { return IsValidTopLevel() && IsSaneSubexpression() && NeedsSignature(); } //! Produce a witness for this script, if possible and given the information available in the context. //! The non-malleable satisfaction is guaranteed to be valid if it exists, and ValidSatisfaction() @@ -1149,13 +1241,21 @@ struct Node { //! Equality testing. bool operator==(const Node& arg) const { return Compare(*this, arg) == 0; } - // Constructors with various argument combinations. - 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()), duplicate_key(ContainsDuplicateKey()) {} - 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()), duplicate_key(ContainsDuplicateKey()) {} - 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()), duplicate_key(ContainsDuplicateKey()) {} - 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()), duplicate_key(ContainsDuplicateKey()) {} - 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()), duplicate_key(ContainsDuplicateKey()) {} - Node(Fragment nt, uint32_t val = 0) : fragment(nt), k(val), ops(CalcOps()), ss(CalcStackSize()), typ(CalcType()), scriptlen(CalcScriptLen()), duplicate_key(ContainsDuplicateKey()) {} + // Constructors with various argument combinations, which bypass the duplicate key check. + Node(internal::NoDupCheck, 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(internal::NoDupCheck, 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(internal::NoDupCheck, 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(internal::NoDupCheck, 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(internal::NoDupCheck, 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(internal::NoDupCheck, Fragment nt, uint32_t val = 0) : fragment(nt), k(val), ops(CalcOps()), ss(CalcStackSize()), 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) : Node(internal::NoDupCheck{}, nt, std::move(sub), std::move(arg), val) { DuplicateKeyCheck(ctx); } + template Node(const Ctx& ctx, Fragment nt, std::vector arg, uint32_t val = 0) : Node(internal::NoDupCheck{}, nt, std::move(arg), val) { DuplicateKeyCheck(ctx);} + template Node(const Ctx& ctx, Fragment nt, std::vector> sub, std::vector key, uint32_t val = 0) : Node(internal::NoDupCheck{}, nt, std::move(sub), std::move(key), val) { DuplicateKeyCheck(ctx); } + template Node(const Ctx& ctx, Fragment nt, std::vector key, uint32_t val = 0) : Node(internal::NoDupCheck{}, nt, std::move(key), val) { DuplicateKeyCheck(ctx); } + template Node(const Ctx& ctx, Fragment nt, std::vector> sub, uint32_t val = 0) : Node(internal::NoDupCheck{}, nt, std::move(sub), val) { DuplicateKeyCheck(ctx); } + template Node(const Ctx& ctx, Fragment nt, uint32_t val = 0) : Node(internal::NoDupCheck{}, nt, val) { DuplicateKeyCheck(ctx); } }; namespace internal { @@ -1248,9 +1348,9 @@ void BuildBack(Fragment nt, std::vector>& constructed, const bool r NodeRef child = std::move(constructed.back()); constructed.pop_back(); if (reverse) { - constructed.back() = MakeNodeRef(nt, Vector(std::move(child), std::move(constructed.back()))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, nt, Vector(std::move(child), std::move(constructed.back()))); } else { - constructed.back() = MakeNodeRef(nt, Vector(std::move(constructed.back()), std::move(child))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, nt, Vector(std::move(constructed.back()), std::move(child))); } } @@ -1264,6 +1364,18 @@ inline NodeRef Parse(Span in, const Ctx& ctx) { using namespace spanparsing; + // Account for the minimum script size for all parsed fragments so far. It "borrows" 1 + // script byte from all leaf nodes, counting it instead whenever a space for a recursive + // expression is added (through andor, and_*, or_*, thresh). This guarantees that all fragments + // increment the script_size by at least one, except for: + // - "0", "1": these leafs are only a single byte, so their subtracted-from increment is 0. + // This is not an issue however, as "space" for them has to be created by combinators, + // which do increment script_size. + // - "v:": the v wrapper adds nothing as in some cases it results in no opcode being added + // (instead transforming another opcode into its VERIFY form). However, the v: wrapper has + // to be interleaved with other fragments to be valid, so this is not a concern. + size_t script_size{1}; + // The two integers are used to hold state for thresh() std::vector> to_parse; std::vector> constructed; @@ -1271,14 +1383,16 @@ inline NodeRef Parse(Span in, const Ctx& ctx) to_parse.emplace_back(ParseContext::WRAPPED_EXPR, -1, -1); while (!to_parse.empty()) { + if (script_size > MAX_STANDARD_P2WSH_SCRIPT_SIZE) return {}; + // Get the current context we are decoding within auto [cur_context, n, k] = to_parse.back(); to_parse.pop_back(); switch (cur_context) { case ParseContext::WRAPPED_EXPR: { - int colon_index = -1; - for (int i = 1; i < (int)in.size(); ++i) { + std::optional colon_index{}; + for (size_t i = 1; i < in.size(); ++i) { if (in[i] == ':') { colon_index = i; break; @@ -1286,106 +1400,131 @@ inline NodeRef Parse(Span in, const Ctx& ctx) if (in[i] < 'a' || in[i] > 'z') break; } // If there is no colon, this loop won't execute - for (int j = 0; j < colon_index; ++j) { + bool last_was_v{false}; + for (size_t j = 0; colon_index && j < *colon_index; ++j) { + if (script_size > MAX_STANDARD_P2WSH_SCRIPT_SIZE) return {}; if (in[j] == 'a') { + script_size += 2; to_parse.emplace_back(ParseContext::ALT, -1, -1); } else if (in[j] == 's') { + script_size += 1; to_parse.emplace_back(ParseContext::SWAP, -1, -1); } else if (in[j] == 'c') { + script_size += 1; to_parse.emplace_back(ParseContext::CHECK, -1, -1); } else if (in[j] == 'd') { + script_size += 3; to_parse.emplace_back(ParseContext::DUP_IF, -1, -1); } else if (in[j] == 'j') { + script_size += 4; to_parse.emplace_back(ParseContext::NON_ZERO, -1, -1); } else if (in[j] == 'n') { + script_size += 1; to_parse.emplace_back(ParseContext::ZERO_NOTEQUAL, -1, -1); } else if (in[j] == 'v') { + // do not permit "...vv...:"; it's not valid, and also doesn't trigger early + // failure as script_size isn't incremented. + if (last_was_v) return {}; to_parse.emplace_back(ParseContext::VERIFY, -1, -1); } else if (in[j] == 'u') { + script_size += 4; to_parse.emplace_back(ParseContext::WRAP_U, -1, -1); } else if (in[j] == 't') { + script_size += 1; to_parse.emplace_back(ParseContext::WRAP_T, -1, -1); } else if (in[j] == 'l') { // The l: wrapper is equivalent to or_i(0,X) - constructed.push_back(MakeNodeRef(Fragment::JUST_0)); + script_size += 4; + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::JUST_0)); to_parse.emplace_back(ParseContext::OR_I, -1, -1); } else { return {}; } + last_was_v = (in[j] == 'v'); } to_parse.emplace_back(ParseContext::EXPR, -1, -1); - in = in.subspan(colon_index + 1); + if (colon_index) in = in.subspan(*colon_index + 1); break; } case ParseContext::EXPR: { if (Const("0", in)) { - constructed.push_back(MakeNodeRef(Fragment::JUST_0)); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::JUST_0)); } else if (Const("1", in)) { - constructed.push_back(MakeNodeRef(Fragment::JUST_1)); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::JUST_1)); } else if (Const("pk(", in)) { auto res = ParseKeyEnd(in, ctx); if (!res) return {}; auto& [key, key_size] = *res; - constructed.push_back(MakeNodeRef(Fragment::WRAP_C, Vector(MakeNodeRef(Fragment::PK_K, Vector(std::move(key)))))); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_C, Vector(MakeNodeRef(internal::NoDupCheck{}, Fragment::PK_K, Vector(std::move(key)))))); in = in.subspan(key_size + 1); + script_size += 34; } else if (Const("pkh(", in)) { auto res = ParseKeyEnd(in, ctx); if (!res) return {}; auto& [key, key_size] = *res; - constructed.push_back(MakeNodeRef(Fragment::WRAP_C, Vector(MakeNodeRef(Fragment::PK_H, Vector(std::move(key)))))); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_C, Vector(MakeNodeRef(internal::NoDupCheck{}, 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(in, ctx); if (!res) return {}; auto& [key, key_size] = *res; - constructed.push_back(MakeNodeRef(Fragment::PK_K, Vector(std::move(key)))); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::PK_K, Vector(std::move(key)))); in = in.subspan(key_size + 1); + script_size += 33; } else if (Const("pk_h(", in)) { auto res = ParseKeyEnd(in, ctx); if (!res) return {}; auto& [key, key_size] = *res; - constructed.push_back(MakeNodeRef(Fragment::PK_H, Vector(std::move(key)))); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, 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(Fragment::SHA256, std::move(hash))); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, 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(Fragment::RIPEMD160, std::move(hash))); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, 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(Fragment::HASH256, std::move(hash))); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, 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(Fragment::HASH160, std::move(hash))); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::HASH160, std::move(hash))); in = in.subspan(hash_size + 1); + script_size += 26; } else if (Const("after(", in)) { int arg_size = FindNextChar(in, ')'); if (arg_size < 1) return {}; int64_t num; if (!ParseInt64(std::string(in.begin(), in.begin() + arg_size), &num)) return {}; if (num < 1 || num >= 0x80000000L) return {}; - constructed.push_back(MakeNodeRef(Fragment::AFTER, num)); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, 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)) { int arg_size = FindNextChar(in, ')'); if (arg_size < 1) return {}; int64_t num; if (!ParseInt64(std::string(in.begin(), in.begin() + arg_size), &num)) return {}; if (num < 1 || num >= 0x80000000L) return {}; - constructed.push_back(MakeNodeRef(Fragment::OLDER, num)); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, 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)) { // Get threshold int next_comma = FindNextChar(in, ','); @@ -1405,7 +1544,8 @@ inline NodeRef Parse(Span in, const Ctx& ctx) } if (keys.size() < 1 || keys.size() > 20) return {}; if (k < 1 || k > (int64_t)keys.size()) return {}; - constructed.push_back(MakeNodeRef(Fragment::MULTI, std::move(keys), k)); + script_size += 2 + (keys.size() > 16) + (k > 16) + 34 * keys.size(); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::MULTI, std::move(keys), k)); } else if (Const("thresh(", in)) { int next_comma = FindNextChar(in, ','); if (next_comma < 1) return {}; @@ -1415,6 +1555,7 @@ inline NodeRef Parse(Span in, const Ctx& ctx) // n = 1 here because we read the first WRAPPED_EXPR before reaching THRESH to_parse.emplace_back(ParseContext::THRESH, 1, k); to_parse.emplace_back(ParseContext::WRAPPED_EXPR, -1, -1); + script_size += 2 + (k > 16) + (k > 0x7f) + (k > 0x7fff) + (k > 0x7fffff); } else if (Const("andor(", in)) { to_parse.emplace_back(ParseContext::ANDOR, -1, -1); to_parse.emplace_back(ParseContext::CLOSE_BRACKET, -1, -1); @@ -1423,21 +1564,29 @@ inline NodeRef Parse(Span in, const Ctx& ctx) to_parse.emplace_back(ParseContext::WRAPPED_EXPR, -1, -1); to_parse.emplace_back(ParseContext::COMMA, -1, -1); to_parse.emplace_back(ParseContext::WRAPPED_EXPR, -1, -1); + script_size += 5; } else { if (Const("and_n(", in)) { to_parse.emplace_back(ParseContext::AND_N, -1, -1); + script_size += 5; } else if (Const("and_b(", in)) { to_parse.emplace_back(ParseContext::AND_B, -1, -1); + script_size += 2; } else if (Const("and_v(", in)) { to_parse.emplace_back(ParseContext::AND_V, -1, -1); + script_size += 1; } else if (Const("or_b(", in)) { to_parse.emplace_back(ParseContext::OR_B, -1, -1); + script_size += 2; } else if (Const("or_c(", in)) { to_parse.emplace_back(ParseContext::OR_C, -1, -1); + script_size += 3; } else if (Const("or_d(", in)) { to_parse.emplace_back(ParseContext::OR_D, -1, -1); + script_size += 4; } else if (Const("or_i(", in)) { to_parse.emplace_back(ParseContext::OR_I, -1, -1); + script_size += 4; } else { return {}; } @@ -1449,39 +1598,40 @@ inline NodeRef Parse(Span in, const Ctx& ctx) break; } case ParseContext::ALT: { - constructed.back() = MakeNodeRef(Fragment::WRAP_A, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_A, Vector(std::move(constructed.back()))); break; } case ParseContext::SWAP: { - constructed.back() = MakeNodeRef(Fragment::WRAP_S, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_S, Vector(std::move(constructed.back()))); break; } case ParseContext::CHECK: { - constructed.back() = MakeNodeRef(Fragment::WRAP_C, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_C, Vector(std::move(constructed.back()))); break; } case ParseContext::DUP_IF: { - constructed.back() = MakeNodeRef(Fragment::WRAP_D, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_D, Vector(std::move(constructed.back()))); break; } case ParseContext::NON_ZERO: { - constructed.back() = MakeNodeRef(Fragment::WRAP_J, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_J, Vector(std::move(constructed.back()))); break; } case ParseContext::ZERO_NOTEQUAL: { - constructed.back() = MakeNodeRef(Fragment::WRAP_N, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_N, Vector(std::move(constructed.back()))); break; } case ParseContext::VERIFY: { - constructed.back() = MakeNodeRef(Fragment::WRAP_V, Vector(std::move(constructed.back()))); + script_size += (constructed.back()->GetType() << "x"_mst); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_V, Vector(std::move(constructed.back()))); break; } case ParseContext::WRAP_U: { - constructed.back() = MakeNodeRef(Fragment::OR_I, Vector(std::move(constructed.back()), MakeNodeRef(Fragment::JUST_0))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::OR_I, Vector(std::move(constructed.back()), MakeNodeRef(internal::NoDupCheck{}, Fragment::JUST_0))); break; } case ParseContext::WRAP_T: { - constructed.back() = MakeNodeRef(Fragment::AND_V, Vector(std::move(constructed.back()), MakeNodeRef(Fragment::JUST_1))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::AND_V, Vector(std::move(constructed.back()), MakeNodeRef(internal::NoDupCheck{}, Fragment::JUST_1))); break; } case ParseContext::AND_B: { @@ -1491,7 +1641,7 @@ inline NodeRef Parse(Span in, const Ctx& ctx) case ParseContext::AND_N: { auto mid = std::move(constructed.back()); constructed.pop_back(); - constructed.back() = MakeNodeRef(Fragment::ANDOR, Vector(std::move(constructed.back()), std::move(mid), MakeNodeRef(Fragment::JUST_0))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::ANDOR, Vector(std::move(constructed.back()), std::move(mid), MakeNodeRef(ctx, Fragment::JUST_0))); break; } case ParseContext::AND_V: { @@ -1519,7 +1669,7 @@ inline NodeRef Parse(Span in, const Ctx& ctx) constructed.pop_back(); auto mid = std::move(constructed.back()); constructed.pop_back(); - constructed.back() = MakeNodeRef(Fragment::ANDOR, Vector(std::move(constructed.back()), std::move(mid), std::move(right))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::ANDOR, Vector(std::move(constructed.back()), std::move(mid), std::move(right))); break; } case ParseContext::THRESH: { @@ -1528,6 +1678,7 @@ inline NodeRef Parse(Span in, const Ctx& ctx) in = in.subspan(1); to_parse.emplace_back(ParseContext::THRESH, n+1, k); to_parse.emplace_back(ParseContext::WRAPPED_EXPR, -1, -1); + script_size += 2; } else if (in[0] == ')') { if (k > n) return {}; in = in.subspan(1); @@ -1538,7 +1689,7 @@ inline NodeRef Parse(Span in, const Ctx& ctx) constructed.pop_back(); } std::reverse(subs.begin(), subs.end()); - constructed.push_back(MakeNodeRef(Fragment::THRESH, std::move(subs), k)); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::THRESH, std::move(subs), k)); } else { return {}; } @@ -1559,8 +1710,11 @@ inline NodeRef Parse(Span in, const Ctx& ctx) // Sanity checks on the produced miniscript assert(constructed.size() == 1); + assert(constructed[0]->ScriptSize() == script_size); if (in.size() > 0) return {}; - return std::move(constructed.front()); + NodeRef tl_node = std::move(constructed.front()); + tl_node->DuplicateKeyCheck(ctx); + return tl_node; } /** Decode a script into opcode/push pairs. @@ -1571,10 +1725,10 @@ inline NodeRef Parse(Span in, const Ctx& ctx) * and OP_EQUALVERIFY are decomposed into OP_CHECKSIG, OP_CHECKMULTISIG, OP_EQUAL * respectively, plus OP_VERIFY. */ -std::optional>>> DecomposeScript(const CScript& script); +std::optional> DecomposeScript(const CScript& script); /** Determine whether the passed pair (created by DecomposeScript) is pushing a number. */ -std::optional ParseScriptNumber(const std::pair>& in); +std::optional ParseScriptNumber(const Opcode& in); enum class DecodeContext { /** A single expression of type B, K, or V. Specifically, this can't be an @@ -1671,12 +1825,12 @@ inline NodeRef DecodeScript(I& in, I last, const Ctx& ctx) // Constants if (in[0].first == OP_1) { ++in; - constructed.push_back(MakeNodeRef(Fragment::JUST_1)); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::JUST_1)); break; } if (in[0].first == OP_0) { ++in; - constructed.push_back(MakeNodeRef(Fragment::JUST_0)); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::JUST_0)); break; } // Public keys @@ -1684,49 +1838,46 @@ inline NodeRef 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(Fragment::PK_K, Vector(std::move(*key)))); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, 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(Fragment::PK_H, Vector(std::move(*key)))); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::PK_H, Vector(std::move(*key)))); break; } // Time locks - if (last - in >= 2 && in[0].first == OP_CHECKSEQUENCEVERIFY) { - auto num = ParseScriptNumber(in[1]); + std::optional num; + if (last - in >= 2 && in[0].first == OP_CHECKSEQUENCEVERIFY && (num = ParseScriptNumber(in[1]))) { in += 2; - if (!num || *num < 1 || *num > 0x7FFFFFFFL) return {}; - constructed.push_back(MakeNodeRef(Fragment::OLDER, *num)); + if (*num < 1 || *num > 0x7FFFFFFFL) return {}; + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::OLDER, *num)); break; } - if (last - in >= 2 && in[0].first == OP_CHECKLOCKTIMEVERIFY) { - auto num = ParseScriptNumber(in[1]); + if (last - in >= 2 && in[0].first == OP_CHECKLOCKTIMEVERIFY && (num = ParseScriptNumber(in[1]))) { in += 2; - if (!num || num < 1 || num > 0x7FFFFFFFL) return {}; - constructed.push_back(MakeNodeRef(Fragment::AFTER, *num)); + if (num < 1 || num > 0x7FFFFFFFL) return {}; + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::AFTER, *num)); break; } // Hashes - if (last - in >= 7 && in[0].first == OP_EQUAL && in[3].first == OP_VERIFY && in[4].first == OP_EQUAL && in[6].first == OP_SIZE) { - auto num = ParseScriptNumber(in[5]); - if (!num || num != 32) return {}; + 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(Fragment::SHA256, in[1].second)); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::SHA256, in[1].second)); in += 7; break; } else if (in[2].first == OP_RIPEMD160 && in[1].second.size() == 20) { - constructed.push_back(MakeNodeRef(Fragment::RIPEMD160, in[1].second)); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::RIPEMD160, in[1].second)); in += 7; break; } else if (in[2].first == OP_HASH256 && in[1].second.size() == 32) { - constructed.push_back(MakeNodeRef(Fragment::HASH256, in[1].second)); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::HASH256, in[1].second)); in += 7; break; } else if (in[2].first == OP_HASH160 && in[1].second.size() == 20) { - constructed.push_back(MakeNodeRef(Fragment::HASH160, in[1].second)); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::HASH160, in[1].second)); in += 7; break; } @@ -1734,9 +1885,8 @@ inline NodeRef DecodeScript(I& in, I last, const Ctx& ctx) // Multi if (last - in >= 3 && in[0].first == OP_CHECKMULTISIG) { std::vector keys; - auto n = ParseScriptNumber(in[1]); - if (!n) return {}; - if (last - in < 3 + *n) return {}; + const auto n = ParseScriptNumber(in[1]); + if (!n || last - in < 3 + *n) return {}; if (*n < 1 || *n > 20) return {}; for (int i = 0; i < *n; ++i) { if (in[2 + i].second.size() != 33) return {}; @@ -1744,12 +1894,11 @@ inline NodeRef DecodeScript(I& in, I last, const Ctx& ctx) if (!key) return {}; keys.push_back(std::move(*key)); } - auto k = ParseScriptNumber(in[2 + *n]); - if (!k) return {}; - if (*k < 1 || *k > *n) return {}; + const auto k = ParseScriptNumber(in[2 + *n]); + if (!k || *k < 1 || *k > *n) return {}; in += 3 + *n; std::reverse(keys.begin(), keys.end()); - constructed.push_back(MakeNodeRef(Fragment::MULTI, std::move(keys), *k)); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::MULTI, std::move(keys), *k)); break; } /** In the following wrappers, we only need to push SINGLE_BKV_EXPR rather @@ -1777,11 +1926,10 @@ inline NodeRef DecodeScript(I& in, I last, const Ctx& ctx) break; } // Thresh - if (last - in >= 3 && in[0].first == OP_EQUAL) { - auto k = ParseScriptNumber(in[1]); - if (!k || *k < 1) return {}; + if (last - in >= 3 && in[0].first == OP_EQUAL && (num = ParseScriptNumber(in[1]))) { + if (*num < 1) return {}; in += 2; - to_parse.emplace_back(DecodeContext::THRESH_W, 0, *k); + to_parse.emplace_back(DecodeContext::THRESH_W, 0, *num); break; } // OP_ENDIF can be WRAP_J, WRAP_D, ANDOR, OR_C, OR_D, or OR_I @@ -1845,38 +1993,38 @@ inline NodeRef 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(Fragment::WRAP_S, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, 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(Fragment::WRAP_A, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_A, Vector(std::move(constructed.back()))); break; } case DecodeContext::CHECK: { if (constructed.empty()) return {}; - constructed.back() = MakeNodeRef(Fragment::WRAP_C, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_C, Vector(std::move(constructed.back()))); break; } case DecodeContext::DUP_IF: { if (constructed.empty()) return {}; - constructed.back() = MakeNodeRef(Fragment::WRAP_D, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_D, Vector(std::move(constructed.back()))); break; } case DecodeContext::VERIFY: { if (constructed.empty()) return {}; - constructed.back() = MakeNodeRef(Fragment::WRAP_V, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_V, Vector(std::move(constructed.back()))); break; } case DecodeContext::NON_ZERO: { if (constructed.empty()) return {}; - constructed.back() = MakeNodeRef(Fragment::WRAP_J, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_J, Vector(std::move(constructed.back()))); break; } case DecodeContext::ZERO_NOTEQUAL: { if (constructed.empty()) return {}; - constructed.back() = MakeNodeRef(Fragment::WRAP_N, Vector(std::move(constructed.back()))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::WRAP_N, Vector(std::move(constructed.back()))); break; } case DecodeContext::AND_V: { @@ -1911,7 +2059,7 @@ inline NodeRef DecodeScript(I& in, I last, const Ctx& ctx) NodeRef right = std::move(constructed.back()); constructed.pop_back(); NodeRef mid = std::move(constructed.back()); - constructed.back() = MakeNodeRef(Fragment::ANDOR, Vector(std::move(left), std::move(mid), std::move(right))); + constructed.back() = MakeNodeRef(internal::NoDupCheck{}, Fragment::ANDOR, Vector(std::move(left), std::move(mid), std::move(right))); break; } case DecodeContext::THRESH_W: { @@ -1935,7 +2083,7 @@ inline NodeRef DecodeScript(I& in, I last, const Ctx& ctx) constructed.pop_back(); subs.push_back(std::move(sub)); } - constructed.push_back(MakeNodeRef(Fragment::THRESH, std::move(subs), k)); + constructed.push_back(MakeNodeRef(internal::NoDupCheck{}, Fragment::THRESH, std::move(subs), k)); break; } case DecodeContext::ENDIF: { @@ -1999,7 +2147,8 @@ inline NodeRef DecodeScript(I& in, I last, const Ctx& ctx) } } if (constructed.size() != 1) return {}; - const NodeRef tl_node = std::move(constructed.front()); + NodeRef 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 {}; @@ -2016,6 +2165,8 @@ inline NodeRef FromString(const std::string& str, const Ctx& template inline NodeRef 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() > MAX_STANDARD_P2WSH_SCRIPT_SIZE) return {}; auto decomposed = DecomposeScript(script); if (!decomposed) return {}; auto it = decomposed->begin(); diff --git a/bitcoin/test/fuzz/miniscript.cpp b/bitcoin/test/fuzz/miniscript.cpp index 254d28c..1b791fc 100644 --- a/bitcoin/test/fuzz/miniscript.cpp +++ b/bitcoin/test/fuzz/miniscript.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2021 The Bitcoin Core developers +// Copyright (c) 2021-2022 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -14,7 +14,7 @@ namespace { -//! Some pre-computed data to simulate challenges. +//! Some pre-computed data for more efficient string roundtrips and to simulate challenges. struct TestData { typedef CPubKey Key; @@ -72,10 +72,18 @@ struct TestData { } } TEST_DATA; -//! Context to parse a Miniscript node to and from Script or text representation. +/** + * Context to parse a Miniscript node to and from Script or text representation. + * Uses an integer (an index in the dummy keys array from the test data) as keys in order + * to focus on fuzzing the Miniscript nodes' test representation, not the key representation. + */ struct ParserContext { typedef CPubKey Key; + bool KeyCompare(const Key& a, const Key& b) const { + return a < b; + } + std::optional ToString(const Key& key) const { auto it = TEST_DATA.dummy_key_idx_map.find(key); @@ -84,12 +92,12 @@ struct ParserContext { return HexStr(Span{&idx, 1}); } - const std::vector ToPKBytes(const Key& key) const + std::vector ToPKBytes(const Key& key) const { return {key.begin(), key.end()}; } - const std::vector ToPKHBytes(const Key& key) const + std::vector ToPKHBytes(const Key& key) const { const auto h = Hash160(key); return {h.begin(), h.end()}; @@ -104,7 +112,7 @@ struct ParserContext { } template - std::optional FromPKBytes(I first, I last) const { + std::optional FromPKBytes(I first, I last) const { CPubKey key; key.Set(first, last); if (!key.IsValid()) return {}; @@ -112,7 +120,7 @@ struct ParserContext { } template - std::optional FromPKHBytes(I first, I last) const { + std::optional FromPKHBytes(I first, I last) const { assert(last - first == 20); CKeyID keyid; std::copy(first, last, keyid.begin()); @@ -124,20 +132,23 @@ struct ParserContext { //! Context that implements naive conversion from/to script only, for roundtrip testing. struct ScriptParserContext { + //! For Script roundtrip we never need the key from a key hash. struct Key { bool is_hash; std::vector data; - - bool operator<(Key k) const { return data < k.data; } }; + bool KeyCompare(const Key& a, const Key& b) const { + return a.data < b.data; + } + const std::vector& ToPKBytes(const Key& key) const { assert(!key.is_hash); return key.data; } - const std::vector ToPKHBytes(const Key& key) const + std::vector ToPKHBytes(const Key& key) const { if (key.is_hash) return key.data; const auto h = Hash160(key.data); @@ -223,6 +234,13 @@ struct CheckerContext: BaseSignatureChecker { bool CheckSequence(const CScriptNum& nSequence) const override { return nSequence.GetInt64() & 1; } } CHECKER_CTX; +//! Context to check for duplicates when instancing a Node. +struct KeyComparator { + bool KeyCompare(const CPubKey& a, const CPubKey& b) const { + return a < b; + } +} KEY_COMP; + // A dummy scriptsig to pass to VerifyScript (we always use Segwit v0). const CScript DUMMY_SCRIPTSIG; @@ -230,10 +248,12 @@ using Fragment = miniscript::Fragment; using NodeRef = miniscript::NodeRef; using Node = miniscript::Node; using Type = miniscript::Type; +// https://github.com/llvm/llvm-project/issues/53444 +// NOLINTNEXTLINE(misc-unused-using-decls) using miniscript::operator"" _mst; //! Construct a miniscript node as a shared_ptr. -template NodeRef MakeNodeRef(Args&&... args) { return miniscript::MakeNodeRef(std::forward(args)...); } +template NodeRef MakeNodeRef(Args&&... args) { return miniscript::MakeNodeRef(KEY_COMP, std::forward(args)...); } /** Information about a yet to be constructed Miniscript node. */ struct NodeInfo { @@ -577,7 +597,7 @@ struct SmartInfo // Remove all recipes which involve non-constructible types. type_it->second.erase(std::remove_if(type_it->second.begin(), type_it->second.end(), [&](const recipe& rec) { - return !std::all_of(rec.second.begin(), rec.second.end(), known_constructible); + return !std::all_of(rec.second.begin(), rec.second.end(), known_constructible); }), type_it->second.end()); // Delete types entirely which have no recipes left. if (type_it->second.empty()) { @@ -700,7 +720,7 @@ NodeRef GenNode(F ConsumeNode, Type root_type = ""_mst, bool strict_valid = fals // Fragment/children have not been decided yet. Decide them. auto node_info = ConsumeNode(type_needed); if (!node_info) return {}; - auto subtypes = std::move(node_info)->subtypes; + auto subtypes = node_info->subtypes; todo.back().second = std::move(node_info); todo.reserve(todo.size() + subtypes.size()); // As elements on the todo stack are processed back to front, construct @@ -713,7 +733,7 @@ NodeRef GenNode(F ConsumeNode, Type root_type = ""_mst, bool strict_valid = fals // 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. - const NodeInfo& info = *todo.back().second; + NodeInfo& info = *todo.back().second; // Gather children from the back of stack. std::vector sub; sub.reserve(info.n_subs); @@ -751,9 +771,9 @@ void TestNode(const NodeRef& node, FuzzedDataProvider& provider) if (!node) return; // Check that it roundtrips to text representation - std::string str; - assert(node->ToString(PARSER_CTX, str)); - auto parsed = miniscript::FromString(str, PARSER_CTX); + std::optional str{node->ToString(PARSER_CTX)}; + assert(str); + auto parsed = miniscript::FromString(*str, PARSER_CTX); assert(parsed); assert(*parsed == *node); @@ -780,7 +800,7 @@ void TestNode(const NodeRef& node, FuzzedDataProvider& provider) assert(decoded->ToScript(PARSER_CTX) == script); assert(decoded->GetType() == node->GetType()); - if (provider.ConsumeBool()) { + if (provider.ConsumeBool() && node->GetOps() < MAX_OPS_PER_SCRIPT && node->ScriptSize() < MAX_STANDARD_P2WSH_SCRIPT_SIZE) { // 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 @@ -835,13 +855,13 @@ void TestNode(const NodeRef& node, FuzzedDataProvider& provider) assert(res || serror == ScriptError::SCRIPT_ERR_OP_COUNT || serror == ScriptError::SCRIPT_ERR_STACK_SIZE); } - if (node->IsSaneTopLevel()) { + if (node->IsSane()) { // 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 + // algorithm succeeds. Given that under IsSane() 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 { @@ -880,6 +900,8 @@ void TestNode(const NodeRef& node, FuzzedDataProvider& provider) assert(mal_success == satisfiable); } +} // namespace + void FuzzInit() { ECC_Start(); @@ -892,8 +914,6 @@ void FuzzInitSmart() SMARTINFO.Init(); } -} // namespace - /** Fuzz target that runs TestNode on nodes generated using ConsumeNodeStable. */ FUZZ_TARGET_INIT(miniscript_stable, FuzzInit) { @@ -923,9 +943,9 @@ FUZZ_TARGET_INIT(miniscript_string, FuzzInit) auto parsed = miniscript::FromString(str, PARSER_CTX); if (!parsed) return; - std::string str2; - assert(parsed->ToString(PARSER_CTX, str2)); - auto parsed2 = miniscript::FromString(str2, PARSER_CTX); + const auto str2 = parsed->ToString(PARSER_CTX); + assert(str2); + auto parsed2 = miniscript::FromString(*str2, PARSER_CTX); assert(parsed2); assert(*parsed == *parsed2); } diff --git a/bitcoin/test/miniscript_tests.cpp b/bitcoin/test/miniscript_tests.cpp index 68c7b38..655d6d7 100644 --- a/bitcoin/test/miniscript_tests.cpp +++ b/bitcoin/test/miniscript_tests.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2019 The Bitcoin Core developers +// Copyright (c) 2019-2022 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -113,6 +113,10 @@ typedef std::pair Challenge; struct KeyConverter { typedef CPubKey Key; + bool KeyCompare(const Key& a, const Key& b) const { + return a < b; + } + //! Convert a public key to bytes. std::vector ToPKBytes(const CPubKey& key) const { return {key.begin(), key.end()}; } @@ -126,27 +130,22 @@ struct KeyConverter { //! Parse a public key from a range of hex characters. template - std::optional FromString(I first, I last) const - { + std::optional FromString(I first, I last) const { auto bytes = ParseHex(std::string(first, last)); - CPubKey key; - key.Set(bytes.begin(), bytes.end()); - if (!key.IsValid()) return {}; - return key; + Key key{bytes.begin(), bytes.end()}; + if (key.IsValid()) return key; + return {}; } template - std::optional bool FromPKBytes(I first, I last) const - { - CPubKey key; - key.Set(first, last); - if (!key.IsValid()) return {}; - return key; + std::optional FromPKBytes(I first, I last) const { + Key key{first, last}; + if (key.IsValid()) return key; + return {}; } template - std::optional FromPKHBytes(I first, I last) const - { + std::optional FromPKHBytes(I first, I last) const { assert(last - first == 20); CKeyID keyid; std::copy(first, last, keyid.begin()); @@ -154,6 +153,10 @@ struct KeyConverter { assert(it != g_testdata->pkmap.end()); return it->second; } + + std::optional ToString(const Key& key) const { + return HexStr(ToPKBytes(key)); + } }; /** A class that encapsulates all signing/hash revealing operations. */ @@ -208,10 +211,10 @@ struct Satisfier : public KeyConverter { * It holds a pointer to a Satisfier object, to determine which timelocks are supposed to be available. */ class TestSignatureChecker : public BaseSignatureChecker { - const Satisfier *ctx; + const Satisfier& ctx; public: - TestSignatureChecker(const Satisfier *in_ctx) : ctx(in_ctx) {} + TestSignatureChecker(const Satisfier& in_ctx LIFETIMEBOUND) : ctx(in_ctx) {} bool CheckECDSASignature(const std::vector& sig, const std::vector& pubkey, const CScript& scriptcode, SigVersion sigversion) const override { CPubKey pk(pubkey); @@ -224,12 +227,12 @@ class TestSignatureChecker : public BaseSignatureChecker { bool CheckLockTime(const CScriptNum& locktime) const override { // Delegate to Satisfier. - return ctx->CheckAfter(locktime.GetInt64()); + return ctx.CheckAfter(locktime.GetInt64()); } bool CheckSequence(const CScriptNum& sequence) const override { // Delegate to Satisfier. - return ctx->CheckOlder(sequence.GetInt64()); + return ctx.CheckOlder(sequence.GetInt64()); } }; @@ -238,7 +241,10 @@ const KeyConverter CONVERTER{}; using Fragment = miniscript::Fragment; using NodeRef = miniscript::NodeRef; +// https://github.com/llvm/llvm-project/issues/53444 +// NOLINTNEXTLINE(misc-unused-using-decls) using miniscript::operator"" _mst; +using Node = miniscript::Node; /** Compute all challenges (pubkeys, hashes, timelocks) that occur in a given Miniscript. */ std::set FindChallenges(const NodeRef& ref) { @@ -274,7 +280,7 @@ void TestSatisfy(const std::string& testcase, const NodeRef& node) { for (int iter = 0; iter < 3; ++iter) { Shuffle(challist.begin(), challist.end(), g_insecure_rand_ctx); Satisfier satisfier; - TestSignatureChecker checker(&satisfier); + TestSignatureChecker checker(satisfier); bool prev_mal_success = false, prev_nonmal_success = false; // Go over all challenges involved in this miniscript in random order. for (int add = -1; add < (int)challist.size(); ++add) { @@ -319,7 +325,7 @@ void TestSatisfy(const std::string& testcase, const NodeRef& node) { BOOST_CHECK(res || serror == ScriptError::SCRIPT_ERR_OP_COUNT || serror == ScriptError::SCRIPT_ERR_STACK_SIZE); } - if (node->IsSaneTopLevel()) { + if (node->IsSane()) { // For sane nodes, the two algorithms behave identically. BOOST_CHECK_EQUAL(mal_success, nonmal_success); } @@ -329,7 +335,7 @@ void TestSatisfy(const std::string& testcase, const NodeRef& node) { // 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->IsSaneTopLevel() || 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. @@ -341,7 +347,7 @@ void TestSatisfy(const std::string& testcase, const NodeRef& node) { // 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->IsSaneTopLevel()) BOOST_CHECK_EQUAL(prev_nonmal_success, satisfiable); + if (node->IsSane()) BOOST_CHECK_EQUAL(prev_nonmal_success, satisfiable); } } @@ -457,7 +463,7 @@ BOOST_AUTO_TEST_CASE(fixed_tests) Test("and_b(hash256(32ba476771d01e37807990ead8719f08af494723de1d228f2c2c07cc0aa40bac),a:and_b(hash256(131772552c01444cd81360818376a040b7c3b2b7b0a53550ee3edde216cec61b),a:older(1)))", "82012088aa2032ba476771d01e37807990ead8719f08af494723de1d228f2c2c07cc0aa40bac876b82012088aa20131772552c01444cd81360818376a040b7c3b2b7b0a53550ee3edde216cec61b876b51b26c9a6c9a", TESTMODE_VALID | TESTMODE_NONMAL, 15, 3); Test("thresh(2,multi(2,03a0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c7,036d2b085e9e382ed10b69fc311a03f8641ccfff21574de0927513a49d9a688a00),a:multi(1,036d2b085e9e382ed10b69fc311a03f8641ccfff21574de0927513a49d9a688a00),ac:pk_k(022f01e5e15cca351daff3843fb70f3c2f0a1bdd05e5af888a67784ef3e10a2a01))", "522103a0434d9e47f3c86235477c7b1ae6ae5d3442d49b1943c2b752a68e2a47e247c721036d2b085e9e382ed10b69fc311a03f8641ccfff21574de0927513a49d9a688a0052ae6b5121036d2b085e9e382ed10b69fc311a03f8641ccfff21574de0927513a49d9a688a0051ae6c936b21022f01e5e15cca351daff3843fb70f3c2f0a1bdd05e5af888a67784ef3e10a2a01ac6c935287", TESTMODE_VALID | TESTMODE_NONMAL | TESTMODE_NEEDSIG, 13, 7); Test("and_n(sha256(d1ec675902ef1633427ca360b290b0b3045a0d9058ddb5e648b4c3c3224c5c68),t:or_i(v:older(4252898),v:older(144)))", "82012088a820d1ec675902ef1633427ca360b290b0b3045a0d9058ddb5e648b4c3c3224c5c68876400676303e2e440b26967029000b269685168", TESTMODE_VALID, 14, 3); - Test("or_d(d:and_v(v:older(4252898),v:older(4252898)),sha256(38df1c1f64a24a77b23393bca50dff872e31edc4f3b5aa3b90ad0b82f4f089b6))", "766303e2e440b26903e2e440b26968736482012088a82038df1c1f64a24a77b23393bca50dff872e31edc4f3b5aa3b90ad0b82f4f089b68768", TESTMODE_VALID, 14, 3); + Test("or_d(nd:and_v(v:older(4252898),v:older(4252898)),sha256(38df1c1f64a24a77b23393bca50dff872e31edc4f3b5aa3b90ad0b82f4f089b6))", "766303e2e440b26903e2e440b2696892736482012088a82038df1c1f64a24a77b23393bca50dff872e31edc4f3b5aa3b90ad0b82f4f089b68768", TESTMODE_VALID, 15, 3); Test("c:and_v(or_c(sha256(9267d3dbed802941483f1afa2a6bc68de5f653128aca9bf1461c5d0a3ad36ed2),v:multi(1,02c44d12c7065d812e8acf28d7cbb19f9011ecd9e9fdf281b0e6a3b5e87d22e7db)),pk_k(03acd484e2f0c7f65309ad178a9f559abde09796974c57e714c35f110dfc27ccbe))", "82012088a8209267d3dbed802941483f1afa2a6bc68de5f653128aca9bf1461c5d0a3ad36ed28764512102c44d12c7065d812e8acf28d7cbb19f9011ecd9e9fdf281b0e6a3b5e87d22e7db51af682103acd484e2f0c7f65309ad178a9f559abde09796974c57e714c35f110dfc27ccbeac", TESTMODE_VALID | TESTMODE_NEEDSIG, 8, 3); Test("c:and_v(or_c(multi(2,036d2b085e9e382ed10b69fc311a03f8641ccfff21574de0927513a49d9a688a00,02352bbf4a4cdd12564f93fa332ce333301d9ad40271f8107181340aef25be59d5),v:ripemd160(1b0f3c404d12075c68c938f9f60ebea4f74941a0)),pk_k(03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556))", "5221036d2b085e9e382ed10b69fc311a03f8641ccfff21574de0927513a49d9a688a002102352bbf4a4cdd12564f93fa332ce333301d9ad40271f8107181340aef25be59d552ae6482012088a6141b0f3c404d12075c68c938f9f60ebea4f74941a088682103fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556ac", TESTMODE_VALID | TESTMODE_NONMAL | TESTMODE_NEEDSIG, 10, 6); Test("and_v(andor(hash256(8a35d9ca92a48eaade6f53a64985e9e2afeb74dcf8acb4c3721e0dc7e4294b25),v:hash256(939894f70e6c3a25da75da0cc2071b4076d9b006563cf635986ada2e93c0d735),v:older(50000)),after(499999999))", "82012088aa208a35d9ca92a48eaade6f53a64985e9e2afeb74dcf8acb4c3721e0dc7e4294b2587640350c300b2696782012088aa20939894f70e6c3a25da75da0cc2071b4076d9b006563cf635986ada2e93c0d735886804ff64cd1db1", TESTMODE_VALID, 14, 3); @@ -504,20 +510,32 @@ BOOST_AUTO_TEST_CASE(fixed_tests) // (for now) have 'd:' be 'u'. This tests we can't use a 'd:' wrapper for a thresh, which requires // its subs to all be 'u' (taken from https://github.com/rust-bitcoin/rust-miniscript/discussions/341). const auto ms_minimalif = miniscript::FromString("thresh(3,c:pk_k(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),sc:pk_k(03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556),sc:pk_k(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798),sdv:older(32))", CONVERTER); - BOOST_CHECK(!ms_minimalif); + BOOST_CHECK(ms_minimalif && !ms_minimalif->IsValid()); // A Miniscript with duplicate keys is not sane const auto ms_dup1 = miniscript::FromString("and_v(v:pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65))", CONVERTER); BOOST_CHECK(ms_dup1); - BOOST_CHECK(!ms_dup1->IsSane()); + BOOST_CHECK(!ms_dup1->IsSane() && !ms_dup1->CheckDuplicateKey()); // Same with a disjunction, and different key nodes (pk and pkh) const auto ms_dup2 = miniscript::FromString("or_b(c:pk_k(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),ac:pk_h(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65))", CONVERTER); - BOOST_CHECK(ms_dup2 && !ms_dup2->IsSane()); + BOOST_CHECK(ms_dup2 && !ms_dup2->IsSane() && !ms_dup2->CheckDuplicateKey()); // Same when the duplicates are leaves or a larger tree const auto ms_dup3 = miniscript::FromString("or_i(and_b(pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),s:pk(03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556)),and_b(older(1),s:pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65)))", CONVERTER); - BOOST_CHECK(ms_dup3 && !ms_dup3->IsSane()); + BOOST_CHECK(ms_dup3 && !ms_dup3->IsSane() && !ms_dup3->CheckDuplicateKey()); // Same when the duplicates are on different levels in the tree const auto ms_dup4 = miniscript::FromString("thresh(2,pkh(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65),s:pk(03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556),a:and_b(dv:older(1),s:pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65)))", CONVERTER); - BOOST_CHECK(ms_dup4 && !ms_dup4->IsSane()); + BOOST_CHECK(ms_dup4 && !ms_dup4->IsSane() && !ms_dup4->CheckDuplicateKey()); + // Sanity check the opposite is true, too. An otherwise sane Miniscript with no duplicate keys is sane. + const auto ms_nondup = miniscript::FromString("pk(03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65)", CONVERTER); + BOOST_CHECK(ms_nondup && ms_nondup->CheckDuplicateKey() && ms_nondup->IsSane()); + // Test we find the first insane sub closer to be a leaf node. This fragment is insane for two reasons: + // 1. It can be spent without a signature + // 2. It contains timelock mixes + // We'll report the timelock mix error, as it's "deeper" (closer to be a leaf node) than the "no 's' property" + // error is. + const auto ms_ins = miniscript::FromString("or_i(and_b(after(1),a:after(1000000000)),pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204))", CONVERTER); + BOOST_CHECK(ms_ins && ms_ins->IsValid() && !ms_ins->IsSane()); + const auto insane_sub = ms_ins->FindInsaneSub(); + BOOST_CHECK(insane_sub && *insane_sub->ToString(CONVERTER) == "and_b(after(1),a:after(1000000000))"); // Timelock tests Test("after(100)", "?", TESTMODE_VALID | TESTMODE_NONMAL); // only heightlock diff --git a/compiler.cpp b/compiler.cpp index d0b6de3..d046ebf 100644 --- a/compiler.cpp +++ b/compiler.cpp @@ -24,7 +24,7 @@ using Fragment = miniscript::Fragment; using miniscript::operator"" _mst; template -Node MakeNode(Args&&... args) { return miniscript::MakeNodeRef(std::forward(args)...); } +Node MakeNode(Args&&... args) { return miniscript::MakeNodeRef(miniscript::internal::NoDupCheck{}, std::forward(args)...); } struct Policy { enum class Type { @@ -952,6 +952,7 @@ bool Compile(const std::string& policy, miniscript::NodeRefDuplicateKeyCheck(COMPILER_CTX); avgcost = res[0].pair.sat; ok = true; } diff --git a/compiler.h b/compiler.h index 6056253..ec5e04a 100644 --- a/compiler.h +++ b/compiler.h @@ -33,6 +33,10 @@ struct CompilerContext { std::copy(key.begin(), key.end(), ret.begin() + 3); return ret; } + + bool KeyCompare(const Key& a, const Key& b) const { + return a < b; + } }; extern const CompilerContext COMPILER_CTX;