[DEV-199] Apply the minimum-if-policy for all transactions (#88)

* [DEV-199] apply minimum if

* [DEV-199] refactor popIfBool

* [DEV-199] use popIfBool in OP_NOTIF

txscript/data/script_tests.json

@@ -72,7 +72,7 @@
 
 ["1 1", "VERIFY", "", "OK"],
 ["1 0x05 0x01 0x00 0x00 0x00 0x00", "VERIFY", "", "OK", "values >4 bytes can be cast to boolean"],
-["1 0x01 0x80", "IF 0 ENDIF", "", "OK", "negative 0 is false"],
+["0x01 0x80", "VERIFY TRUE", "", "VERIFY", "negative 0 is false"],
 
 ["10 0 11", "TOALTSTACK DROP FROMALTSTACK ADD 21 EQUAL", "", "OK"],
 ["'gavin_was_here'", "TOALTSTACK 11 FROMALTSTACK 'gavin_was_here' EQUALVERIFY 11 EQUAL", "", "OK"],
@@ -687,7 +687,7 @@
 
 ["", "NOP", "", "EMPTY_STACK", "Checks EMPTY_STACK error"],
 
-["'abc'", "IF INVERT ELSE 1 ENDIF", "", "DISABLED_OPCODE", "INVERT disabled"],
+["'abc'", "INVERT VERIFY TRUE", "", "DISABLED_OPCODE", "INVERT disabled"],
 ["1 2 0", "IF AND ELSE 1 ENDIF NOP", "", "DISABLED_OPCODE", "AND disabled"],
 ["1 2 0", "IF OR ELSE 1 ENDIF NOP", "", "DISABLED_OPCODE", "OR disabled"],
 ["1 2 0", "IF XOR ELSE 1 ENDIF NOP", "", "DISABLED_OPCODE", "XOR disabled"],
@@ -1064,6 +1064,10 @@
 ["0 0x02 0x0000", "CHECKMULTISIGVERIFY 1", "", "UNKNOWN_ERROR"],
 ["0x02 0x0000 0", "CHECKMULTISIGVERIFY 1", "", "UNKNOWN_ERROR"],
 
+["Check MINIMALIF"],
+["2", "IF TRUE ELSE FALSE", "", "MINIMALIF"],
+["2", "NOTIF TRUE ELSE FALSE", "", "MINIMALIF"],
+
 
 ["Order of CHECKMULTISIG evaluation tests, inverted by swapping the order of"],
 ["pubkeys/signatures so they fail due to the STRICTENC rules on validly encoded"],

txscript/error.go

@@ -222,6 +222,10 @@ const (
 	// reached.
 	ErrUnsatisfiedLockTime
 
+	// ErrMinimalIf is returned if the operand of an OP_IF/OP_NOTIF
+	// is not either an empty vector or [0x01].
+	ErrMinimalIf
+
 	// numErrorCodes is the maximum error code number used in tests.  This
 	// entry MUST be the last entry in the enum.
 	numErrorCodes
@@ -270,6 +274,7 @@ var errorCodeStrings = map[ErrorCode]string{
 	ErrDiscourageUpgradableNOPs:  "ErrDiscourageUpgradableNOPs",
 	ErrNegativeLockTime:          "ErrNegativeLockTime",
 	ErrUnsatisfiedLockTime:       "ErrUnsatisfiedLockTime",
+	ErrMinimalIf:                 "ErrMinimalIf",
 }
 
 // String returns the ErrorCode as a human-readable name.

txscript/error_test.go

@@ -57,6 +57,7 @@ func TestErrorCodeStringer(t *testing.T) {
 		{ErrDiscourageUpgradableNOPs, "ErrDiscourageUpgradableNOPs"},
 		{ErrNegativeLockTime, "ErrNegativeLockTime"},
 		{ErrUnsatisfiedLockTime, "ErrUnsatisfiedLockTime"},
+		{ErrMinimalIf, "ErrMinimalIf"},
 		{0xffff, "Unknown ErrorCode (65535)"},
 	}
 

txscript/opcode.go

@@ -925,6 +925,31 @@ func opcodeNop(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// popIfBool enforces the "minimal if" policy. In order to
+// eliminate an additional source of nuisance malleability, we
+// require the following: for OP_IF and OP_NOTIF, the top stack item MUST
+// either be an empty byte slice, or [0x01]. Otherwise, the item at the top of
+// the stack will be popped and interpreted as a boolean.
+func popIfBool(vm *Engine) (bool, error) {
+	so, err := vm.dstack.PopByteArray()
+	if err != nil {
+		return false, err
+	}
+
+	if len(so) == 1 && so[0] == 0x01 {
+		return true, nil
+	}
+
+	if len(so) == 0 {
+		return false, nil
+	}
+
+	str := fmt.Sprintf("with OP_IF or OP_NOTIF top stack item MUST "+
+		"be an empty byte array or 0x01, and is instead: %v",
+		so[0])
+	return false, scriptError(ErrMinimalIf, str)
+}
+
 // opcodeIf treats the top item on the data stack as a boolean and removes it.
 //
 // An appropriate entry is added to the conditional stack depending on whether
@@ -943,7 +968,7 @@ func opcodeNop(op *parsedOpcode, vm *Engine) error {
 func opcodeIf(op *parsedOpcode, vm *Engine) error {
 	condVal := OpCondFalse
 	if vm.isBranchExecuting() {
-		ok, err := vm.dstack.PopBool()
+		ok, err := popIfBool(vm)
 
 		if err != nil {
 			return err
@@ -978,7 +1003,7 @@ func opcodeIf(op *parsedOpcode, vm *Engine) error {
 func opcodeNotIf(op *parsedOpcode, vm *Engine) error {
 	condVal := OpCondFalse
 	if vm.isBranchExecuting() {
-		ok, err := vm.dstack.PopBool()
+		ok, err := popIfBool(vm)
 		if err != nil {
 			return err
 		}

txscript/reference_test.go

@@ -204,6 +204,8 @@ func parseExpectedResult(expected string) ([]ErrorCode, error) {
 		return []ErrorCode{ErrNegativeLockTime}, nil
 	case "UNSATISFIED_LOCKTIME":
 		return []ErrorCode{ErrUnsatisfiedLockTime}, nil
+	case "MINIMALIF":
+		return []ErrorCode{ErrMinimalIf}, nil
 	}
 
 	return nil, fmt.Errorf("unrecognized expected result in test data: %v",