make CHECKOUTPUTSVERIFY take stack arg, but check prev. executed opcode

Add prev_opcode variable that is set to the just executed opcode after
main script eval switch. Make CHECKOUTPUTSVERIFY to check that
prev_opcode > OP_PUSHDATA4 to ensure that the hash to be checked is
not constructed dynamically

Author: dgpv

src/script/interpreter.cpp

@@ -292,6 +292,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
     CScript::const_iterator pend = script.end();
     CScript::const_iterator pbegincodehash = script.begin();
     opcodetype opcode;
+    opcodetype prev_opcode = OP_INVALIDOPCODE;
     valtype vchPushValue;
     std::vector<bool> vfExec;
     std::vector<valtype> altstack;
@@ -774,18 +775,34 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
                 {
                     // Don't verify before enabled...
                     if (flags & SCRIPT_VERIFY_OUTPUTS_HASH) {
-                        CScript::const_iterator lookahead = pc;
-                        opcodetype argument;
-                        // Read ahead one opcode as a lookahead argument
-                        if (!script.GetOp(lookahead, argument, vchPushValue))
-                            return set_error(serror, SCRIPT_ERR_BAD_OPCODE);
-                        // If lookahead argument was exactly 32 bytes, check OutputHash
+
+                        if (stack.size() < 1)
+                            return set_error(serror, SCRIPT_ERR_INVALID_STACK_OPERATION);
+
+                        // Previous executed operation must be a data push.
+                        //
+                        // NOTE:
+                        //
+                        // INVALID_STACK_OPERATION might not be appropriate error code
+                        // in this case, but this is closest thing in existing error codes.
+                        //
+                        // Making opcode behavior depend on the previous executed opcode
+                        // adds complexity to the script execution model, but the amount
+                        // of complexity added by this opcode-lookbehind mode is small,
+                        // and this is arguably better than making non-pushdata
+                        // opcodes use data lookahead, because this erodes the consistency
+                        // of the stack machine execution model. The argument here seems
+                        // to boil down to complexity vs model consistency.
+                        // Reducing consistency just externalizes the complexity outside:
+                        // to the users of the script and to the tools that operate on it.
+                        if (prev_opcode > OP_PUSHDATA4)
+                            return set_error(serror, SCRIPT_ERR_INVALID_STACK_OPERATION);
+
+                        valtype& vchHash = stacktop(-1);
+
+                        // If stack argument was exactly 32 bytes, check OutputHash
                         // This is so that we can later add different semantics for this opcode
-                        if (vchPushValue.size() == 32) {
-                            // Argument should be == 0x20 -- will fail later anyways
-                            if (!CheckMinimalPush(vchPushValue, argument)) {
-                                return set_error(serror, SCRIPT_ERR_MINIMALDATA);
-                            }
+                        if (vchHash.size() == 32) {
                             // If multiple inputs allowed, two inputs with the same OutputsHashVerify
                             // would pay only half intended amount!
                             if (!checker.CheckOnlyOneInput()) {
@@ -796,7 +813,6 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
                                 return set_error(serror, SCRIPT_ERR_OUTPUTSHASHVERIFY);
                             }
                         }
-
                     }
                 }
                 break;
@@ -1155,6 +1171,8 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
             if (stack.size() + altstack.size() > MAX_STACK_SIZE)
                 return set_error(serror, SCRIPT_ERR_STACK_SIZE);
 
+            prev_opcode = opcode;
+
             ++opcode_pos;
         }
     }

test/functional/feature_outputshashverify.py

@@ -23,10 +23,10 @@
 def random_bytes(n):
     return bytes(random.getrandbits(8) for i in range(n))
 def random_fake_script():
-    return CScript([OP_CHECKOUTPUTSHASHVERIFY, random_bytes(32)])
+    return CScript([random_bytes(32), OP_CHECKOUTPUTSHASHVERIFY])
 def random_real_outputs_and_script(n):
     outputs = [CTxOut((x+1)*100, CScript(bytes([OP_RETURN, 0x20]) + random_bytes(32))) for x in range(n)]
-    return outputs, CScript(bytes([OP_CHECKOUTPUTSHASHVERIFY, 0x20]) + hash256(b"".join(o.serialize() for o in outputs)))
+    return outputs, CScript([hash256(b"".join(o.serialize() for o in outputs)), OP_CHECKOUTPUTSHASHVERIFY])
 
 def random_tapscript_tree(depth):
 
@@ -39,7 +39,7 @@ def random_tapscript_tree(depth):
     for d in range(1, depth+2):
         idxs =zip(range(0, len(outputs_tree[-d]),2), range(1, len(outputs_tree[-d]), 2))
         for (idx, (a,b)) in enumerate([(outputs_tree[-d][i], outputs_tree[-d][j]) for (i,j) in idxs]):
-            s = CScript(bytes([OP_CHECKOUTPUTSHASHVERIFY, 0x20]) + hash256(b"".join(o.serialize() for o in [a,b])))
+            s = CScript([hash256(b"".join(o.serialize() for o in [a,b])), OP_CHECKOUTPUTSHASHVERIFY])
             a = sum(o.nValue for o in [a,b])
             taproot, tweak, controls = taproot_construct(pubkey1, [s])
             t = CTxOut(a+1000, taproot)