diff --git a/bitcoin/script/miniscript.cpp b/bitcoin/script/miniscript.cpp index 31c5f05..5dd09da 100644 --- a/bitcoin/script/miniscript.cpp +++ b/bitcoin/script/miniscript.cpp @@ -323,25 +323,24 @@ InputStack operator+(InputStack a, InputStack b) { return a; } -InputStack Choose(InputStack a, InputStack b, bool nonmalleable) { +InputStack operator|(InputStack a, InputStack b) { // If only one (or neither) is valid, pick the other one. if (a.available == Availability::NO) return b; if (b.available == Availability::NO) return a; - // If both are valid, they must be distinct. - if (nonmalleable) { - // If both options are weak, any result is fine; it just needs the malleable marker. - if (!a.has_sig && !b.has_sig) return a.Malleable(); - // If one option is weak, we must pick that one. - if (!a.has_sig) return a; - if (!b.has_sig) return b; - // If both options are strong, prefer the canonical one. - if (b.non_canon) return a; - if (a.non_canon) return b; - // If both options are strong and canonical, prefer the nonmalleable one. - if (b.malleable) return a; - if (a.malleable) return b; + // If only one of the solutions has a signature, we must pick the other one. + if (!a.has_sig && b.has_sig) return a; + if (!b.has_sig && a.has_sig) return b; + if (!a.has_sig && !b.has_sig) { + // If neither solution requires a signature, the result is inevitably malleable. + a.malleable = true; + b.malleable = true; + } else { + // If both options require a signature, prefer the non-malleable one. + if (b.malleable && !a.malleable) return a; + if (a.malleable && !b.malleable) return b; } - // Pick the smaller between YESes and the bigger between MAYBEs. Prefer YES over MAYBE. + // Between two malleable or two non-malleable solutions, pick the smaller one between + // YESes, and the bigger ones between MAYBEs. Prefer YES over MAYBE. if (a.available == Availability::YES && b.available == Availability::YES) { return std::move(a.size <= b.size ? a : b); } else if (a.available == Availability::MAYBE && b.available == Availability::MAYBE) { diff --git a/bitcoin/script/miniscript.h b/bitcoin/script/miniscript.h index b2c9661..41ddc2d 100644 --- a/bitcoin/script/miniscript.h +++ b/bitcoin/script/miniscript.h @@ -250,6 +250,7 @@ struct InputStack { //! Whether this stack is malleable (can be turned into an equally valid other stack by a third party). bool malleable = false; //! Whether this stack is non-canonical (using a construction known to be unnecessary for satisfaction). + //! Note that this flag does not affect the satisfaction algorithm; it is only used for sanity checking. bool non_canon = false; //! Serialized witness size. size_t size = 0; @@ -270,7 +271,7 @@ struct InputStack { //! Concatenate two input stacks. friend InputStack operator+(InputStack a, InputStack b); //! Choose between two potential input stacks. - friend InputStack Choose(InputStack a, InputStack b, bool nonmalleable); + friend InputStack operator|(InputStack a, InputStack b); }; static const auto ZERO = InputStack(std::vector()); @@ -299,7 +300,7 @@ struct MaxInt { return a.value + b.value; } - friend MaxInt Choose(const MaxInt& a, const MaxInt& b) { + friend MaxInt operator|(const MaxInt& a, const MaxInt& b) { if (!a.valid) return b; if (!b.valid) return a; return std::max(a.value, b.value); @@ -690,30 +691,30 @@ struct Node { } case Fragment::OR_B: { const auto count{1 + subs[0]->ops.count + subs[1]->ops.count}; - const auto sat{Choose(subs[0]->ops.sat + subs[1]->ops.dsat, subs[1]->ops.sat + subs[0]->ops.dsat)}; + const auto sat{(subs[0]->ops.sat + subs[1]->ops.dsat) | (subs[1]->ops.sat + subs[0]->ops.dsat)}; const auto dsat{subs[0]->ops.dsat + subs[1]->ops.dsat}; return {count, sat, dsat}; } case Fragment::OR_D: { const auto count{3 + subs[0]->ops.count + subs[1]->ops.count}; - const auto sat{Choose(subs[0]->ops.sat, subs[1]->ops.sat + subs[0]->ops.dsat)}; + const auto sat{subs[0]->ops.sat | (subs[1]->ops.sat + subs[0]->ops.dsat)}; const auto dsat{subs[0]->ops.dsat + subs[1]->ops.dsat}; return {count, sat, dsat}; } case Fragment::OR_C: { const auto count{2 + subs[0]->ops.count + subs[1]->ops.count}; - const auto sat{Choose(subs[0]->ops.sat, subs[1]->ops.sat + subs[0]->ops.dsat)}; + const auto sat{subs[0]->ops.sat | (subs[1]->ops.sat + subs[0]->ops.dsat)}; return {count, sat, {}}; } case Fragment::OR_I: { const auto count{3 + subs[0]->ops.count + subs[1]->ops.count}; - const auto sat{Choose(subs[0]->ops.sat, subs[1]->ops.sat)}; - const auto dsat{Choose(subs[0]->ops.dsat, subs[1]->ops.dsat)}; + const auto sat{subs[0]->ops.sat | subs[1]->ops.sat}; + const auto dsat{subs[0]->ops.dsat | subs[1]->ops.dsat}; return {count, sat, dsat}; } case Fragment::ANDOR: { const auto count{3 + subs[0]->ops.count + subs[1]->ops.count + subs[2]->ops.count}; - const auto sat{Choose(subs[1]->ops.sat + subs[0]->ops.sat, subs[0]->ops.dsat + subs[2]->ops.sat)}; + const auto sat{(subs[1]->ops.sat + subs[0]->ops.sat) | (subs[0]->ops.dsat + subs[2]->ops.sat)}; const auto dsat{subs[0]->ops.dsat + subs[2]->ops.dsat}; return {count, sat, dsat}; } @@ -731,7 +732,7 @@ struct Node { for (const auto& sub : subs) { count += sub->ops.count + 1; auto next_sats = Vector(sats[0] + sub->ops.dsat); - for (size_t j = 1; j < sats.size(); ++j) next_sats.push_back(Choose(sats[j] + sub->ops.dsat, sats[j - 1] + sub->ops.sat)); + for (size_t j = 1; j < sats.size(); ++j) next_sats.push_back((sats[j] + sub->ops.dsat) | (sats[j - 1] + sub->ops.sat)); next_sats.push_back(sats[sats.size() - 1] + sub->ops.sat); sats = std::move(next_sats); } @@ -756,20 +757,20 @@ struct Node { case Fragment::HASH256: case Fragment::HASH160: return {1, {}}; case Fragment::ANDOR: { - const auto sat{Choose(subs[0]->ss.sat + subs[1]->ss.sat, subs[0]->ss.dsat + subs[2]->ss.sat)}; + const auto sat{(subs[0]->ss.sat + subs[1]->ss.sat) | (subs[0]->ss.dsat + subs[2]->ss.sat)}; const auto dsat{subs[0]->ss.dsat + subs[2]->ss.dsat}; return {sat, dsat}; } case Fragment::AND_V: return {subs[0]->ss.sat + subs[1]->ss.sat, {}}; case Fragment::AND_B: return {subs[0]->ss.sat + subs[1]->ss.sat, subs[0]->ss.dsat + subs[1]->ss.dsat}; case Fragment::OR_B: { - const auto sat{Choose(subs[0]->ss.dsat + subs[1]->ss.sat, subs[0]->ss.sat + subs[1]->ss.dsat)}; + const auto sat{(subs[0]->ss.dsat + subs[1]->ss.sat) | (subs[0]->ss.sat + subs[1]->ss.dsat)}; const auto dsat{subs[0]->ss.dsat + subs[1]->ss.dsat}; return {sat, dsat}; } - case Fragment::OR_C: return {Choose(subs[0]->ss.sat, subs[0]->ss.dsat + subs[1]->ss.sat), {}}; - case Fragment::OR_D: return {Choose(subs[0]->ss.sat, subs[0]->ss.dsat + subs[1]->ss.sat), subs[0]->ss.dsat + subs[1]->ss.dsat}; - case Fragment::OR_I: return {Choose(subs[0]->ss.sat + 1, subs[1]->ss.sat + 1), Choose(subs[0]->ss.dsat + 1, subs[1]->ss.dsat + 1)}; + case Fragment::OR_C: return {subs[0]->ss.sat | (subs[0]->ss.dsat + subs[1]->ss.sat), {}}; + case Fragment::OR_D: return {subs[0]->ss.sat | (subs[0]->ss.dsat + subs[1]->ss.sat), subs[0]->ss.dsat + subs[1]->ss.dsat}; + case Fragment::OR_I: return {(subs[0]->ss.sat + 1) | (subs[1]->ss.sat + 1), (subs[0]->ss.dsat + 1) | (subs[1]->ss.dsat + 1)}; case Fragment::MULTI: return {(uint32_t)keys.size() + 1, (uint32_t)keys.size() + 1}; case Fragment::WRAP_A: case Fragment::WRAP_N: @@ -782,7 +783,7 @@ struct Node { auto sats = Vector(internal::MaxInt(0)); for (const auto& sub : subs) { auto next_sats = Vector(sats[0] + sub->ss.dsat); - for (size_t j = 1; j < sats.size(); ++j) next_sats.push_back(Choose(sats[j] + sub->ss.dsat, sats[j - 1] + sub->ss.sat)); + for (size_t j = 1; j < sats.size(); ++j) next_sats.push_back((sats[j] + sub->ss.dsat) | (sats[j - 1] + sub->ss.sat)); next_sats.push_back(sats[sats.size() - 1] + sub->ss.sat); sats = std::move(next_sats); } @@ -796,10 +797,10 @@ struct Node { template - internal::InputResult ProduceInput(const Ctx& ctx, bool nonmal) const { + internal::InputResult ProduceInput(const Ctx& ctx) const { using namespace internal; - auto helper = [&ctx, nonmal](const Node& node, Span subres) -> InputResult { + auto helper = [&ctx](const Node& node, Span subres) -> InputResult { switch (node.nodetype) { case Fragment::PK_K: { std::vector sig; @@ -819,7 +820,7 @@ struct Node { auto sat = InputStack(std::move(sig)).WithSig().Available(avail); std::vector next_sats; next_sats.push_back(sats[0]); - for (size_t j = 1; j < sats.size(); ++j) next_sats.push_back(Choose(sats[j], std::move(sats[j - 1]) + sat, nonmal)); + 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)); sats = std::move(next_sats); } @@ -834,13 +835,15 @@ struct Node { auto& res = subres[subres.size() - i - 1]; 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(Choose(sats[j] + res.nsat, std::move(sats[j - 1]) + res.sat, nonmal)); + 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)); sats = std::move(next_sats); } InputStack nsat = INVALID; for (size_t i = 0; i < sats.size(); ++i) { - if (i != node.k) nsat = Choose(std::move(nsat), std::move(sats[i]), nonmal); + // 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(); + 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])); @@ -877,28 +880,29 @@ struct Node { } case Fragment::AND_B: { auto& x = subres[0], &y = subres[1]; - return InputResult(Choose(Choose(y.nsat + x.nsat, (y.sat + x.nsat).NonCanon(), nonmal), (y.nsat + x.sat).NonCanon(), nonmal), y.sat + x.sat); + return InputResult((y.nsat + x.nsat) | (y.sat + x.nsat).NonCanon() | (y.nsat + x.sat).NonCanon(), y.sat + x.sat); } case Fragment::OR_B: { auto& x = subres[0], &z = subres[1]; - return InputResult(z.nsat + x.nsat, Choose(Choose(z.nsat + x.sat, z.sat + x.nsat, nonmal), (z.sat + x.sat).NonCanon(), nonmal)); + // 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()); } case Fragment::OR_C: { auto& x = subres[0], &z = subres[1]; - return InputResult(INVALID, Choose(std::move(x.sat), z.sat + x.nsat, nonmal)); + return InputResult(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, Choose(std::move(x.sat), z.sat + x.nsat, nonmal)); + return InputResult(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(Choose(x.nsat + ONE, z.nsat + ZERO, nonmal), Choose(x.sat + ONE, z.sat + ZERO, nonmal)); + return InputResult((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(Choose((y.nsat + x.sat).NonCanon(), z.nsat + x.nsat, nonmal), Choose(y.sat + x.sat, z.sat + x.nsat, nonmal)); + return InputResult((y.nsat + x.sat).NonCanon() | (z.nsat + x.nsat), (y.sat + x.sat) | (z.sat + x.nsat)); } case Fragment::WRAP_A: case Fragment::WRAP_S: @@ -929,25 +933,46 @@ struct Node { return InputResult(INVALID, INVALID); }; - auto tester = [&helper, nonmal](const Node& node, Span subres) -> InputResult { + auto tester = [&helper](const Node& node, Span subres) -> InputResult { auto ret = helper(node, subres); + // Do a consistency check between the satisfaction code and the type checker // (the actual satisfaction code in ProduceInputHelper does not use GetType) + + // For 'z' nodes, available satisfactions/dissatisfactions must have stack size 0. if (node.GetType() << "z"_mst && ret.nsat.available != Availability::NO) assert(ret.nsat.stack.size() == 0); if (node.GetType() << "z"_mst && ret.sat.available != Availability::NO) assert(ret.sat.stack.size() == 0); + + // For 'o' nodes, available satisfactions/dissatisfactions must have stack size 1. if (node.GetType() << "o"_mst && ret.nsat.available != Availability::NO) assert(ret.nsat.stack.size() == 1); if (node.GetType() << "o"_mst && ret.sat.available != Availability::NO) assert(ret.sat.stack.size() == 1); - if (node.GetType() << "n"_mst && ret.sat.available != Availability::NO) assert(ret.sat.stack.back().size() != 0); + + // For 'n' nodes, available satisfactions/dissatisfactions must have stack size 1 or larger. For satisfactions, + // the top element cannot be 0. + if (node.GetType() << "n"_mst && ret.sat.available != Availability::NO) assert(ret.sat.stack.size() >= 1); + if (node.GetType() << "n"_mst && ret.nsat.available != Availability::NO) assert(ret.nsat.stack.size() >= 1); + if (node.GetType() << "n"_mst && ret.sat.available != Availability::NO) assert(!ret.sat.stack.back().empty()); + + // For 'd' nodes, a dissatisfaction must exist, and they must not need a signature. If it is non-malleable, + // it must be canonical. if (node.GetType() << "d"_mst) assert(ret.nsat.available != Availability::NO); + if (node.GetType() << "d"_mst) assert(!ret.nsat.has_sig); + if (node.GetType() << "d"_mst && !ret.nsat.malleable) assert(!ret.nsat.non_canon); + + // For 'f'/'s' nodes, dissatisfactions/satisfactions must have a signature. 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); - if (nonmal) { - if (node.GetType() << "d"_mst) assert(!ret.nsat.has_sig); - if (node.GetType() << "d"_mst && !ret.nsat.malleable) assert(!ret.nsat.non_canon); - if (node.GetType() << "e"_mst) assert(!ret.nsat.malleable); - if (node.GetType() << "m"_mst && ret.sat.available != Availability::NO) assert(!ret.sat.malleable); - if (ret.sat.available != Availability::NO && !ret.sat.malleable) assert(!ret.sat.non_canon); - } + + // 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 '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); + + // If a non-malleable satisfaction exists, it must be canonical. + if (ret.sat.available != Availability::NO && !ret.sat.malleable) assert(!ret.sat.non_canon); + return ret; }; @@ -998,16 +1023,22 @@ struct Node { //! Check whether there is no satisfaction path that contains both timelocks and heightlocks bool CheckTimeLocksMix() const { return GetType() << "k"_mst; } - //! Do all sanity checks. - bool IsSane() const { return IsValid() && IsNonMalleable() && CheckTimeLocksMix() && CheckOpsLimit() && CheckStackSize(); } + //! 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(); } //! Check whether this node is safe as a script on its own. bool IsSaneTopLevel() const { return IsValidTopLevel() && IsSane() && 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() + //! is true. If IsSane() holds, this satisfaction is guaranteed to succeed in case the node's + //! conditions are satisfied (private keys and hash preimages available, locktimes satsified). template Availability Satisfy(const Ctx& ctx, std::vector>& stack, bool nonmalleable = true) const { - auto ret = ProduceInput(ctx, nonmalleable); + auto ret = ProduceInput(ctx); if (nonmalleable && (ret.sat.malleable || !ret.sat.has_sig)) return Availability::NO; stack = std::move(ret.sat.stack); return ret.sat.available; diff --git a/bitcoin/test/fuzz/miniscript_random.cpp b/bitcoin/test/fuzz/miniscript_random.cpp index 36db49d..0e6dfbe 100644 --- a/bitcoin/test/fuzz/miniscript_random.cpp +++ b/bitcoin/test/fuzz/miniscript_random.cpp @@ -390,23 +390,47 @@ FUZZ_TARGET_INIT(miniscript_random, initialize_miniscript_random) assert(parsed); assert(*parsed == *node); - // Check both malleable and non-malleable satisfaction. Note that we only assert the produced witness - // is valid if the Miniscript was sane, as otherwise it could overflow the limits. - CScriptWitness witness; + // Run malleable satisfaction algorithm. const CScript script_pubkey = CScript() << OP_0 << WitnessV0ScriptHash(script); - const bool mal_success = node->Satisfy(SATISFIER_CTX, witness.stack, false) == miniscript::Availability::YES; - if (mal_success && node->IsSaneTopLevel()) { - witness.stack.push_back(std::vector(script.begin(), script.end())); - assert(VerifyScript(DUMMY_SCRIPTSIG, script_pubkey, &witness, STANDARD_SCRIPT_VERIFY_FLAGS, CHECKER_CTX)); + CScriptWitness witness_mal; + const bool mal_success = node->Satisfy(SATISFIER_CTX, witness_mal.stack, false) == miniscript::Availability::YES; + witness_mal.stack.push_back(std::vector(script.begin(), script.end())); + + // Run non-malleable satisfaction algorithm. + CScriptWitness witness_nonmal; + const bool nonmal_success = node->Satisfy(SATISFIER_CTX, witness_nonmal.stack, true) == miniscript::Availability::YES; + witness_nonmal.stack.push_back(std::vector(script.begin(), script.end())); + + if (nonmal_success) { + // Non-malleable satisfactions are bounded by GetStackSize(). + assert(witness_nonmal.stack.size() <= node->GetStackSize()); + // If a non-malleable satisfaction exists, the malleable one must also exist, and be identical to it. + assert(mal_success); + assert(witness_nonmal.stack == witness_mal.stack); + + // Test non-malleable satisfaction. + ScriptError serror; + bool res = VerifyScript(DUMMY_SCRIPTSIG, script_pubkey, &witness_nonmal, STANDARD_SCRIPT_VERIFY_FLAGS, CHECKER_CTX, &serror); + // Non-malleable satisfactions are guaranteed to be valid if ValidSatisfactions(). + if (node->ValidSatisfactions()) assert(res); + // More detailed: non-malleable satisfactions must be valid, or could fail with ops count error (if CheckOpsLimit failed), + // or with a stack size error (if CheckStackSize check failed). + assert(res || + (!node->CheckOpsLimit() && serror == ScriptError::SCRIPT_ERR_OP_COUNT) || + (!node->CheckStackSize() && serror == ScriptError::SCRIPT_ERR_STACK_SIZE)); } - witness.stack.clear(); - const bool nonmal_success = node->Satisfy(SATISFIER_CTX, witness.stack, true) == miniscript::Availability::YES; - if (nonmal_success && node->IsSaneTopLevel()) { - witness.stack.push_back(std::vector(script.begin(), script.end())); - assert(VerifyScript(DUMMY_SCRIPTSIG, script_pubkey, &witness, STANDARD_SCRIPT_VERIFY_FLAGS, CHECKER_CTX)); + + if (mal_success && (!nonmal_success || witness_mal.stack != witness_nonmal.stack)) { + // Test malleable satisfaction only if it's different from the non-malleable one. + ScriptError serror; + bool res = VerifyScript(DUMMY_SCRIPTSIG, script_pubkey, &witness_mal, STANDARD_SCRIPT_VERIFY_FLAGS, CHECKER_CTX, &serror); + // Malleable satisfactions are not guaranteed to be valid under any conditions, but they can only + // fail due to stack or ops limits. + assert(res || serror == ScriptError::SCRIPT_ERR_OP_COUNT || serror == ScriptError::SCRIPT_ERR_STACK_SIZE); + } + + if (node->IsSaneTopLevel()) { + // For sane nodes, the two algorithms behave identically. + assert(mal_success == nonmal_success); } - // If a nonmalleable solution exists, a solution whatsoever must also exist. - assert(mal_success >= nonmal_success); - // If a miniscript is nonmalleable and needs a signature, and a solution exists, a non-malleable solution must also exist. - if (node->IsNonMalleable() && node->NeedsSignature()) assert(nonmal_success == mal_success); } diff --git a/bitcoin/test/miniscript_tests.cpp b/bitcoin/test/miniscript_tests.cpp index 896c48d..c29a236 100644 --- a/bitcoin/test/miniscript_tests.cpp +++ b/bitcoin/test/miniscript_tests.cpp @@ -287,26 +287,6 @@ std::set FindChallenges(const NodeRef& ref) { return chal; } -/** Verify a satisfaction. */ -void Verify(const std::string& testcase, const NodeRef& node, const Satisfier& ctx, std::vector> stack, const CScript& script, bool nonmal) { - // Construct P2WSH scriptPubKey. - CScript spk = GetScriptForDestination(WitnessV0ScriptHash(script)); - // Construct the P2WSH witness (script stack + script). - CScriptWitness witness; - witness.stack = std::move(stack); - witness.stack.push_back(std::vector(script.begin(), script.end())); - // Use a test signature checker aware of which afters/olders we made valid. - TestSignatureChecker checker(&ctx); - ScriptError serror; - if (nonmal) BOOST_CHECK(stack.size() <= node->GetStackSize()); - if (!VerifyScript(CScript(), spk, &witness, STANDARD_SCRIPT_VERIFY_FLAGS, checker, &serror)) { - // The only possible violation should be the ops limit, as that is hard to statically guarantee. - BOOST_CHECK(serror == SCRIPT_ERR_OP_COUNT); - // When using the non-malleable satisfier, and our OpsLimit analysis succeeds, no execution errors should occur at all. - BOOST_CHECK(!(nonmal && node->CheckOpsLimit())); - } -} - /** Run random satisfaction tests. */ void TestSatisfy(const std::string& testcase, const NodeRef& node) { auto script = node->ToScript(CONVERTER); @@ -315,31 +295,72 @@ 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); 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) { if (add >= 0) satisfier.supported.insert(challist[add]); // The first iteration does not add anything - // First try to produce a potentially malleable satisfaction. - std::vector> stack; - bool mal_success = node->Satisfy(satisfier, stack, false) == miniscript::Availability::YES; - if (mal_success) Verify(testcase, node, satisfier, stack, script, false); // And verify it against consensus/standardness. - // Then produce a non-malleable satisfaction. - bool nonmal_success = node->Satisfy(satisfier, stack, true) == miniscript::Availability::YES; - if (nonmal_success) Verify(testcase, node, satisfier, std::move(stack), std::move(script), true); - // If a nonmalleable solution exists, a solution whatsoever must also exist. - BOOST_CHECK(mal_success >= nonmal_success); - // If a miniscript is nonmalleable and needs a signature, and a solution exists, a non-malleable solution must also exist. - if (node->IsNonMalleable() && node->NeedsSignature()) BOOST_CHECK_EQUAL(nonmal_success, mal_success); + + // Run malleable satisfaction algorithm. + const CScript script_pubkey = CScript() << OP_0 << WitnessV0ScriptHash(script); + CScriptWitness witness_mal; + const bool mal_success = node->Satisfy(satisfier, witness_mal.stack, false) == miniscript::Availability::YES; + witness_mal.stack.push_back(std::vector(script.begin(), script.end())); + + // Run non-malleable satisfaction algorithm. + CScriptWitness witness_nonmal; + const bool nonmal_success = node->Satisfy(satisfier, witness_nonmal.stack, true) == miniscript::Availability::YES; + witness_nonmal.stack.push_back(std::vector(script.begin(), script.end())); + + if (nonmal_success) { + // Non-malleable satisfactions are bounded by GetStackSize(). + BOOST_CHECK(witness_nonmal.stack.size() <= node->GetStackSize()); + // If a non-malleable satisfaction exists, the malleable one must also exist, and be identical to it. + BOOST_CHECK(mal_success); + BOOST_CHECK(witness_nonmal.stack == witness_mal.stack); + + // Test non-malleable satisfaction. + ScriptError serror; + bool res = VerifyScript(CScript(), script_pubkey, &witness_nonmal, STANDARD_SCRIPT_VERIFY_FLAGS, checker, &serror); + // Non-malleable satisfactions are guaranteed to be valid if ValidSatisfactions(). + if (node->ValidSatisfactions()) BOOST_CHECK(res); + // More detailed: non-malleable satisfactions must be valid, or could fail with ops count error (if CheckOpsLimit failed), + // or with a stack size error (if CheckStackSize check fails). + BOOST_CHECK(res || + (!node->CheckOpsLimit() && serror == ScriptError::SCRIPT_ERR_OP_COUNT) || + (!node->CheckStackSize() && serror == ScriptError::SCRIPT_ERR_STACK_SIZE)); + } + + if (mal_success && (!nonmal_success || witness_mal.stack != witness_nonmal.stack)) { + // Test malleable satisfaction only if it's different from the non-malleable one. + ScriptError serror; + bool res = VerifyScript(CScript(), script_pubkey, &witness_mal, STANDARD_SCRIPT_VERIFY_FLAGS, checker, &serror); + // Malleable satisfactions are not guaranteed to be valid under any conditions, but they can only + // fail due to stack or ops limits. + BOOST_CHECK(res || serror == ScriptError::SCRIPT_ERR_OP_COUNT || serror == ScriptError::SCRIPT_ERR_STACK_SIZE); + } + + if (node->IsSaneTopLevel()) { + // For sane nodes, the two algorithms behave identically. + BOOST_CHECK_EQUAL(mal_success, nonmal_success); + } + // Adding more satisfied conditions can never remove our ability to produce a satisfaction. BOOST_CHECK(mal_success >= prev_mal_success); - // 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 (add >= 0 && challist[add].first == ChallengeType::PK) BOOST_CHECK(nonmal_success >= prev_nonmal_success); + // 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) { + BOOST_CHECK(nonmal_success >= prev_nonmal_success); + } // Remember results for the next added challenge. prev_mal_success = mal_success; prev_nonmal_success = nonmal_success; } // 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)); + // 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)); } }