diff options
-rw-r--r-- | src/script/descriptor.cpp | 10 | ||||
-rw-r--r-- | src/script/miniscript.h | 23 | ||||
-rw-r--r-- | src/test/fuzz/miniscript.cpp | 7 | ||||
-rw-r--r-- | src/test/miniscript_tests.cpp | 10 | ||||
-rwxr-xr-x | test/functional/wallet_miniscript.py | 12 |
5 files changed, 48 insertions, 14 deletions
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index dff6429259..f57f167e5d 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -15,6 +15,7 @@ #include <common/args.h> #include <span.h> #include <util/bip32.h> +#include <util/check.h> #include <util/spanparsing.h> #include <util/strencodings.h> #include <util/vector.h> @@ -1553,14 +1554,14 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const error = std::move(parser.m_key_parsing_error); return nullptr; } - if (!node->IsSane()) { + if (!node->IsSane() || node->IsNotSatisfiable()) { // Try to find the first insane sub for better error reporting. auto insane_node = node.get(); if (const auto sub = node->FindInsaneSub()) insane_node = sub; if (const auto str = insane_node->ToString(parser)) error = *str; if (!insane_node->IsValid()) { error += " is invalid"; - } else { + } else if (!node->IsSane()) { error += " is not sane"; if (!insane_node->IsNonMalleable()) { error += ": malleable witnesses exist"; @@ -1573,9 +1574,14 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const } else if (!insane_node->ValidSatisfactions()) { error += ": needs witnesses that may exceed resource limits"; } + } else { + error += " is not satisfiable"; } return nullptr; } + // A signature check is required for a miniscript to be sane. Therefore no sane miniscript + // may have an empty list of public keys. + CHECK_NONFATAL(!parser.m_keys.empty()); return std::make_unique<MiniscriptDescriptor>(std::move(parser.m_keys), std::move(node)); } } diff --git a/src/script/miniscript.h b/src/script/miniscript.h index 7c1a87a7dc..b58740a125 100644 --- a/src/script/miniscript.h +++ b/src/script/miniscript.h @@ -1134,20 +1134,35 @@ public: size_t ScriptSize() const { return scriptlen; } //! Return the maximum number of ops needed to satisfy this script non-malleably. - uint32_t GetOps() const { return ops.count + ops.sat.value; } + std::optional<uint32_t> GetOps() const { + if (!ops.sat.valid) return {}; + return ops.count + ops.sat.value; + } //! Return the number of ops in the script (not counting the dynamic ones that depend on execution). uint32_t GetStaticOps() const { return ops.count; } //! Check the ops limit of this script against the consensus limit. - bool CheckOpsLimit() const { return GetOps() <= MAX_OPS_PER_SCRIPT; } + bool CheckOpsLimit() const { + if (const auto ops = GetOps()) return *ops <= MAX_OPS_PER_SCRIPT; + return true; + } /** Return the maximum number of stack elements needed to satisfy this script non-malleably, including * the script push. */ - uint32_t GetStackSize() const { return ss.sat.value + 1; } + std::optional<uint32_t> GetStackSize() const { + if (!ss.sat.valid) return {}; + return ss.sat.value + 1; + } //! Check the maximum stack size for this script against the policy limit. - bool CheckStackSize() const { return GetStackSize() - 1 <= MAX_STANDARD_P2WSH_STACK_ITEMS; } + bool CheckStackSize() const { + if (const auto ss = GetStackSize()) return *ss - 1 <= MAX_STANDARD_P2WSH_STACK_ITEMS; + return true; + } + + //! Whether no satisfaction exists for this node. + bool IsNotSatisfiable() const { return !GetStackSize(); } //! Return the expression type. Type GetType() const { return typ; } diff --git a/src/test/fuzz/miniscript.cpp b/src/test/fuzz/miniscript.cpp index 08c147af15..56327b9665 100644 --- a/src/test/fuzz/miniscript.cpp +++ b/src/test/fuzz/miniscript.cpp @@ -943,7 +943,8 @@ void TestNode(const NodeRef& node, FuzzedDataProvider& provider) assert(decoded->ToScript(PARSER_CTX) == script); assert(decoded->GetType() == node->GetType()); - if (provider.ConsumeBool() && node->GetOps() < MAX_OPS_PER_SCRIPT && node->ScriptSize() < MAX_STANDARD_P2WSH_SCRIPT_SIZE) { + const auto node_ops{node->GetOps()}; + if (provider.ConsumeBool() && node_ops && *node_ops < 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 @@ -954,7 +955,7 @@ void TestNode(const NodeRef& node, FuzzedDataProvider& provider) // Do not pad more than what would cause MAX_STANDARD_P2WSH_SCRIPT_SIZE to be reached, however, // as that also invalidates scripts. int add = std::min<int>( - MAX_OPS_PER_SCRIPT - node->GetOps(), + MAX_OPS_PER_SCRIPT - *node_ops, MAX_STANDARD_P2WSH_SCRIPT_SIZE - node->ScriptSize()); for (int i = 0; i < add; ++i) script.push_back(OP_NOP); } @@ -972,7 +973,7 @@ void TestNode(const NodeRef& node, FuzzedDataProvider& provider) if (nonmal_success) { // Non-malleable satisfactions are bounded by GetStackSize(). - assert(witness_nonmal.stack.size() <= node->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); diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp index 42e441c41a..2d183c8844 100644 --- a/src/test/miniscript_tests.cpp +++ b/src/test/miniscript_tests.cpp @@ -297,7 +297,7 @@ void TestSatisfy(const std::string& testcase, const NodeRef& node) { if (nonmal_success) { // Non-malleable satisfactions are bounded by GetStackSize(). - BOOST_CHECK(witness_nonmal.stack.size() <= node->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); @@ -375,8 +375,8 @@ void Test(const std::string& ms, const std::string& hexscript, int mode, int ops auto inferred_miniscript = miniscript::FromScript(computed_script, CONVERTER); BOOST_CHECK_MESSAGE(inferred_miniscript, "Cannot infer miniscript from script: " + ms); BOOST_CHECK_MESSAGE(inferred_miniscript->ToScript(CONVERTER) == computed_script, "Roundtrip failure: miniscript->script != miniscript->script->miniscript->script: " + ms); - if (opslimit != -1) BOOST_CHECK_MESSAGE((int)node->GetOps() == opslimit, "Ops limit mismatch: " << ms << " (" << node->GetOps() << " vs " << opslimit << ")"); - if (stacklimit != -1) BOOST_CHECK_MESSAGE((int)node->GetStackSize() == stacklimit, "Stack limit mismatch: " << ms << " (" << node->GetStackSize() << " vs " << stacklimit << ")"); + if (opslimit != -1) BOOST_CHECK_MESSAGE((int)*node->GetOps() == opslimit, "Ops limit mismatch: " << ms << " (" << *node->GetOps() << " vs " << opslimit << ")"); + if (stacklimit != -1) BOOST_CHECK_MESSAGE((int)*node->GetStackSize() == stacklimit, "Stack limit mismatch: " << ms << " (" << *node->GetStackSize() << " vs " << stacklimit << ")"); TestSatisfy(ms, node); } } @@ -498,8 +498,8 @@ BOOST_AUTO_TEST_CASE(fixed_tests) // For CHECKMULTISIG the OP cost is the number of keys, but the stack size is the number of sigs (+1) const auto ms_multi = miniscript::FromString("multi(1,03d30199d74fb5a22d47b6e054e2f378cedacffcb89904a61d75d0dbd407143e65,03fff97bd5755eeea420453a14355235d382f6472f8568a18b2f057a1460297556,0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)", CONVERTER); BOOST_CHECK(ms_multi); - BOOST_CHECK_EQUAL(ms_multi->GetOps(), 4); // 3 pubkeys + CMS - BOOST_CHECK_EQUAL(ms_multi->GetStackSize(), 3); // 1 sig + dummy elem + script push + BOOST_CHECK_EQUAL(*ms_multi->GetOps(), 4); // 3 pubkeys + CMS + BOOST_CHECK_EQUAL(*ms_multi->GetStackSize(), 3); // 1 sig + dummy elem + script push // The 'd:' wrapper leaves on the stack what was DUP'ed at the beginning of its execution. // Since it contains an OP_IF just after on the same element, we can make sure that the element // in question must be OP_1 if OP_IF enforces that its argument must only be OP_1 or the empty diff --git a/test/functional/wallet_miniscript.py b/test/functional/wallet_miniscript.py index 7bc3424bf4..45f0df1c76 100755 --- a/test/functional/wallet_miniscript.py +++ b/test/functional/wallet_miniscript.py @@ -277,6 +277,18 @@ class WalletMiniscriptTest(BitcoinTestFramework): assert not res["success"] assert "is not sane: witnesses without signature exist" in res["error"]["message"] + # Sanity check we wouldn't let an unspendable Miniscript descriptor in + res = self.ms_wo_wallet.importdescriptors( + [ + { + "desc": descsum_create("wsh(0)"), + "active": False, + "timestamp": "now", + } + ] + )[0] + assert not res["success"] and "is not satisfiable" in res["error"]["message"] + # Test we can track any type of Miniscript for ms in MINISCRIPTS: self.watchonly_test(ms) |