miniscript: make GetStackSize() and GetOps() return optionals

The value is only set for satisfiable nodes, so it was undefined for
non-satisfiable nodes. Make it clear in the interface by returning
std::nullopt if the node isn't satisfiable instead of an undefined
value.

Author: darosior

src/script/miniscript.h

@@ -1134,20 +1134,32 @@ struct Node {
     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;
+    }
 
     //! Return the expression type.
     Type GetType() const { return typ; }

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);

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