txscript: Make canonicalPush accept raw opcode.

This renames the canonicalPush function to isCanonicalPush and converts
it to accept an opcode as a byte and the associate data as a byte slice
instead of the internal parse opcode data struct in order to make it
more flexible for raw script analysis.

It also updates all callers and tests accordingly.

txscript/engine.go

@@ -875,6 +875,7 @@ func (vm *Engine) SetAltStack(data [][]byte) {
 // engine according to the description provided by each flag.
 func NewEngine(scriptPubKey []byte, tx *wire.MsgTx, txIdx int, flags ScriptFlags,
 	sigCache *SigCache, hashCache *TxSigHashes, inputAmount int64) (*Engine, error) {
+	const scriptVersion = 0
 
 	// The provided transaction input index must refer to a valid input.
 	if txIdx < 0 || txIdx >= len(tx.TxIn) {
@@ -994,7 +995,9 @@ func NewEngine(scriptPubKey []byte, tx *wire.MsgTx, txIdx int, flags ScriptFlags
 			// data push of the witness program, otherwise we
 			// reintroduce malleability.
 			sigPops := vm.scripts[0]
-			if len(sigPops) == 1 && canonicalPush(sigPops[0]) &&
+			if len(sigPops) == 1 &&
+				isCanonicalPush(sigPops[0].opcode.value,
+					sigPops[0].data) &&
 				IsWitnessProgram(sigPops[0].data) {
 
 				witProgram = sigPops[0].data

txscript/script.go

@@ -128,7 +128,7 @@ func IsWitnessProgram(script []byte) bool {
 func isWitnessProgram(pops []parsedOpcode) bool {
 	return len(pops) == 2 &&
 		isSmallInt(pops[0].opcode.value) &&
-		canonicalPush(pops[1]) &&
+		isCanonicalPush(pops[1].opcode.value, pops[1].data) &&
 		(len(pops[1].data) >= 2 && len(pops[1].data) <= 40)
 }
 
@@ -305,13 +305,16 @@ func removeOpcode(pkscript []parsedOpcode, opcode byte) []parsedOpcode {
 	return retScript
 }
 
-// canonicalPush returns true if the object is either not a push instruction
+// isCanonicalPush returns true if the opcode is either not a push instruction
-// or the push instruction contained wherein is matches the canonical form
+// or the data associated with the push instruction uses the smallest
-// or using the smallest instruction to do the job. False otherwise.
+// instruction to do the job.  False otherwise.
-func canonicalPush(pop parsedOpcode) bool {
+//
-	opcode := pop.opcode.value
+// For example, it is possible to push a value of 1 to the stack as "OP_1",
-	data := pop.data
+// "OP_DATA_1 0x01", "OP_PUSHDATA1 0x01 0x01", and others, however, the first
-	dataLen := len(pop.data)
+// only takes a single byte, while the rest take more.  Only the first is
+// considered canonical.
+func isCanonicalPush(opcode byte, data []byte) bool {
+	dataLen := len(data)
 	if opcode > OP_16 {
 		return true
 	}
@@ -336,7 +339,9 @@ func canonicalPush(pop parsedOpcode) bool {
 func removeOpcodeByData(pkscript []parsedOpcode, data []byte) []parsedOpcode {
 	retScript := make([]parsedOpcode, 0, len(pkscript))
 	for _, pop := range pkscript {
-		if !canonicalPush(pop) || !bytes.Contains(pop.data, data) {
+		if !isCanonicalPush(pop.opcode.value, pop.data) ||
+			!bytes.Contains(pop.data, data) {
+
 			retScript = append(retScript, pop)
 		}
 	}

txscript/script_test.go

@@ -3740,31 +3740,25 @@ func TestPushedData(t *testing.T) {
 	}
 }
 
-// TestHasCanonicalPush ensures the canonicalPush function works as expected.
+// TestHasCanonicalPush ensures the isCanonicalPush function works as expected.
 func TestHasCanonicalPush(t *testing.T) {
 	t.Parallel()
 
+	const scriptVersion = 0
 	for i := 0; i < 65535; i++ {
 		script, err := NewScriptBuilder().AddInt64(int64(i)).Script()
 		if err != nil {
-			t.Errorf("Script: test #%d unexpected error: %v\n", i,
+			t.Errorf("Script: test #%d unexpected error: %v\n", i, err)
-				err)
 			continue
 		}
-		if result := IsPushOnlyScript(script); !result {
+		if !IsPushOnlyScript(script) {
-			t.Errorf("IsPushOnlyScript: test #%d failed: %x\n", i,
+			t.Errorf("IsPushOnlyScript: test #%d failed: %x\n", i, script)
-				script)
 			continue
 		}
-		pops, err := parseScript(script)
+		tokenizer := MakeScriptTokenizer(scriptVersion, script)
-		if err != nil {
+		for tokenizer.Next() {
-			t.Errorf("parseScript: #%d failed: %v", i, err)
+			if !isCanonicalPush(tokenizer.Opcode(), tokenizer.Data()) {
-			continue
+				t.Errorf("isCanonicalPush: test #%d failed: %x\n", i, script)
-		}
-		for _, pop := range pops {
-			if result := canonicalPush(pop); !result {
-				t.Errorf("canonicalPush: test #%d failed: %x\n",
-					i, script)
 				break
 			}
 		}
@@ -3774,21 +3768,17 @@ func TestHasCanonicalPush(t *testing.T) {
 		builder.AddData(bytes.Repeat([]byte{0x49}, i))
 		script, err := builder.Script()
 		if err != nil {
-			t.Errorf("StandardPushesTests test #%d unexpected error: %v\n", i, err)
+			t.Errorf("Script: test #%d unexpected error: %v\n", i, err)
 			continue
 		}
-		if result := IsPushOnlyScript(script); !result {
+		if !IsPushOnlyScript(script) {
-			t.Errorf("StandardPushesTests IsPushOnlyScript test #%d failed: %x\n", i, script)
+			t.Errorf("IsPushOnlyScript: test #%d failed: %x\n", i, script)
 			continue
 		}
-		pops, err := parseScript(script)
+		tokenizer := MakeScriptTokenizer(scriptVersion, script)
-		if err != nil {
+		for tokenizer.Next() {
-			t.Errorf("StandardPushesTests #%d failed to TstParseScript: %v", i, err)
+			if !isCanonicalPush(tokenizer.Opcode(), tokenizer.Data()) {
-			continue
+				t.Errorf("isCanonicalPush: test #%d failed: %x\n", i, script)
-		}
-		for _, pop := range pops {
-			if result := canonicalPush(pop); !result {
-				t.Errorf("StandardPushesTests TstHasCanonicalPushes test #%d failed: %x\n", i, script)
 				break
 			}
 		}
@@ -4213,11 +4203,13 @@ func TestIsPayToWitnessPubKeyHash(t *testing.T) {
 	}
 }
 
-// TestHasCanonicalPushes ensures the canonicalPush function properly determines
+// TestHasCanonicalPushes ensures the isCanonicalPush function properly
-// what is considered a canonical push for the purposes of removeOpcodeByData.
+// determines what is considered a canonical push for the purposes of
+// removeOpcodeByData.
 func TestHasCanonicalPushes(t *testing.T) {
 	t.Parallel()
 
+	const scriptVersion = 0
 	tests := []struct {
 		name     string
 		script   string
@@ -4236,20 +4228,20 @@ func TestHasCanonicalPushes(t *testing.T) {
 		},
 	}
 
-	for i, test := range tests {
+	for _, test := range tests {
 		script := mustParseShortForm(test.script)
-		pops, err := parseScript(script)
+		if err := checkScriptParses(scriptVersion, script); err != nil {
-		if err != nil {
 			if test.expected {
-				t.Errorf("TstParseScript #%d failed: %v", i, err)
+				t.Errorf("%q: script parse failed: %v", test.name, err)
 			}
 			continue
 		}
-		for _, pop := range pops {
+		tokenizer := MakeScriptTokenizer(scriptVersion, script)
-			if canonicalPush(pop) != test.expected {
+		for tokenizer.Next() {
-				t.Errorf("canonicalPush: #%d (%s) wrong result"+
+			result := isCanonicalPush(tokenizer.Opcode(), tokenizer.Data())
-					"\ngot: %v\nwant: %v", i, test.name,
+			if result != test.expected {
-					true, test.expected)
+				t.Errorf("%q: isCanonicalPush wrong result\ngot: %v\nwant: %v",
+					test.name, result, test.expected)
 				break
 			}
 		}

txscript/standard.go

@@ -953,7 +953,7 @@ func ExtractAtomicSwapDataPushes(version uint16, pkScript []byte) (*AtomicSwapDa
 	}
 	isAtomicSwap := pops[0].opcode.value == OP_IF &&
 		pops[1].opcode.value == OP_SIZE &&
-		canonicalPush(pops[2]) &&
+		isCanonicalPush(pops[2].opcode.value, pops[2].data) &&
 		pops[3].opcode.value == OP_EQUALVERIFY &&
 		pops[4].opcode.value == OP_SHA256 &&
 		pops[5].opcode.value == OP_DATA_32 &&
@@ -962,7 +962,7 @@ func ExtractAtomicSwapDataPushes(version uint16, pkScript []byte) (*AtomicSwapDa
 		pops[8].opcode.value == OP_HASH160 &&
 		pops[9].opcode.value == OP_DATA_20 &&
 		pops[10].opcode.value == OP_ELSE &&
-		canonicalPush(pops[11]) &&
+		isCanonicalPush(pops[11].opcode.value, pops[11].data) &&
 		pops[12].opcode.value == OP_CHECKLOCKTIMEVERIFY &&
 		pops[13].opcode.value == OP_DROP &&
 		pops[14].opcode.value == OP_DUP &&