Lengthy comments on op_verif and op_vernotif.

src/chain/operation.cpp

@@ -348,6 +348,72 @@ bool operation::is_invalid(opcode code)
 // opcode: [80, 98, 106, 137-138, 186-255]
 // ****************************************************************************
 // CONSENSUS: These are invalid unless evaluation is precluded by conditional.
+//
+// Satoshi test cases incorrectly refer to op_ver and op_verif as "reserved".
+// Reserved refers to codes that are unused but conditionally acceptable. When
+// the conditional operator skips over them, the script may be valid. On the
+// other hand, "disabled" codes are unconditionally invalid - such as op_cat.
+// The "disabled" codes are in a group outside of the evaluation switch, which
+// makes their unconditional invalidity obvious. The other two disabled codes
+// are not so obvious in behavior, and misidentified in satoshi test vectors:
+//
+// These fail because the scripts are unconditional with both reserved and
+// disabled codes, yet they are all referred to as reserved.
+// { "1", "ver", "op_ver is reserved" }
+// { "1", "verif", "op_verif is reserved" }
+// { "1", "vernotif", "op_vernotif is reserved" }
+// { "1", "reserved", "op_reserved is reserved" }
+// { "1", "reserved1", "op_reserved1 is reserved" }
+// { "1", "reserved2", "op_reserved2 is reserved" }
+//
+// These fail because the scripts either execute conditionally invalid codes
+// (op_ver) or include unconditionally invalid codes, without execution
+// (op_verif, op_vernotif). The comments are correct, contradicting the above.
+// { "1", "if ver else 1 endif", "ver is reserved" }
+// { "0", "if verif else 1 endif", "verif illegal everywhere" }
+// { "0", "if else 1 else verif endif", "verif illegal everywhere" }
+// { "0", "if vernotif else 1 endif", "vernotif illegal everywhere" }
+// { "0", "if else 1 else vernotif endif", "vernotif illegal everywhere" }
+//
+// These fail regardless of conditional exclusion because they are also
+// disabled codes.
+// { "'a' 'b'", "cat", "cat disabled" }
+// { "'a' 'b' 0", "if cat else 1 endif", "cat disabled" }
+// { "'abc' 1 1", "substr", "substr disabled" }
+// { "'abc' 1 1 0", "if substr else 1 endif", "substr disabled" }
+// { "'abc' 2 0", "if left else 1 endif", "left disabled" }
+// { "'abc' 2 0", "if right else 1 endif", "right disabled" }
+// { "'abc'", "if invert else 1 endif", "invert disabled" }
+// { "1 2 0 if and else 1 endif", "nop", "and disabled" }
+// { "1 2 0 if or else 1 endif", "nop", "or disabled" }
+// { "1 2 0 if xor else 1 endif", "nop", "xor disabled" }
+// { "2 0 if 2mul else 1 endif", "nop", "2mul disabled" }
+// { "2 0 if 2div else 1 endif", "nop", "2div disabled" }
+// { "2 2 0 if mul else 1 endif", "nop", "mul disabled" }
+// { "2 2 0 if div else 1 endif", "nop", "div disabled" }
+// { "2 2 0 if mod else 1 endif", "nop", "mod disabled" }
+// { "2 2 0 if lshift else 1 endif", "nop", "lshift disabled" }
+// { "2 2 0 if rshift else 1 endif", "nop", "rshift disabled" }
+//
+// The reason that op_verif and op_vernotif are unconditionally invalid (and
+// therefore behave exactly as "disabled" codes is that they are conditionals.
+// Note that op_ver is not a conditional, so despite the similar name, when it
+// was disabled it became a "reserved" code. The former conditonals are not
+// excludable by remaining conditions. They pass this condition:
+//      else if (fExec || (OP_IF <= opcode && opcode <= OP_ENDIF))
+// which evaluates all codes, yet there were removed from the evaluation. They
+// were part of this switch case, but now this break is not hit by them:
+//      case OP_IF:
+//      case OP_NOTIF: {...}
+//          break;
+// So they fall through to the conditional default case, just as any reserved
+// code would, and halrt the evaluation with the bad_opcode error:
+//      default:
+//          return set_error(serror, SCRIPT_ERR_BAD_OPCODE);
+// Yet because they do not bypass any conditional evaluation, they hit this
+// default unconditionally. So they are "disabled" codes. We use the term
+// "invalid" above because of the confusion that can cause. The only truly
+// "reserved" codes are the op_nop# codes, which were promoted by a hard fork.
 // ****************************************************************************
 bool operation::is_reserved(opcode code)
 {

test/chain/script.hpp

@@ -760,8 +760,8 @@ const script_test_list invalid_context_free_scripts
 
     // Reserved.
     { "1", "ver", "op_ver is reserved" },
-    { "1", "verif", "op_verif is reserved" },
-    { "1", "vernotif", "op_vernotif is reserved" },
+    { "1", "verif", "op_verif is reserved (no, it's disabled)" },
+    { "1", "vernotif", "op_vernotif is reserved (no, it's disabled)" },
     { "1", "reserved", "op_reserved is reserved" },
     { "1", "reserved1", "op_reserved1 is reserved" },
     { "1", "reserved2", "op_reserved2 is reserved" },