diff --git a/txscript/engine.go b/txscript/engine.go index 576adc7188..ef7ad33e3c 100644 --- a/txscript/engine.go +++ b/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 diff --git a/txscript/script.go b/txscript/script.go index 418cb22673..ffc6541b82 100644 --- a/txscript/script.go +++ b/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 -// or the push instruction contained wherein is matches the canonical form -// or using the smallest instruction to do the job. False otherwise. -func canonicalPush(pop parsedOpcode) bool { - opcode := pop.opcode.value - data := pop.data - dataLen := len(pop.data) +// isCanonicalPush returns true if the opcode is either not a push instruction +// or the data associated with the push instruction uses the smallest +// instruction to do the job. False otherwise. +// +// For example, it is possible to push a value of 1 to the stack as "OP_1", +// "OP_DATA_1 0x01", "OP_PUSHDATA1 0x01 0x01", and others, however, the first +// 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) } } diff --git a/txscript/script_test.go b/txscript/script_test.go index 665d9ef633..62c51e4181 100644 --- a/txscript/script_test.go +++ b/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, - err) + t.Errorf("Script: test #%d unexpected error: %v\n", i, err) continue } - if result := IsPushOnlyScript(script); !result { - t.Errorf("IsPushOnlyScript: test #%d failed: %x\n", i, - script) + if !IsPushOnlyScript(script) { + t.Errorf("IsPushOnlyScript: test #%d failed: %x\n", i, script) continue } - pops, err := parseScript(script) - if err != nil { - t.Errorf("parseScript: #%d failed: %v", i, err) - continue - } - for _, pop := range pops { - if result := canonicalPush(pop); !result { - t.Errorf("canonicalPush: test #%d failed: %x\n", - i, script) + tokenizer := MakeScriptTokenizer(scriptVersion, script) + for tokenizer.Next() { + if !isCanonicalPush(tokenizer.Opcode(), tokenizer.Data()) { + t.Errorf("isCanonicalPush: 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) - continue - } - if result := IsPushOnlyScript(script); !result { - t.Errorf("StandardPushesTests IsPushOnlyScript test #%d failed: %x\n", i, script) + t.Errorf("Script: test #%d unexpected error: %v\n", i, err) continue } - pops, err := parseScript(script) - if err != nil { - t.Errorf("StandardPushesTests #%d failed to TstParseScript: %v", i, err) + if !IsPushOnlyScript(script) { + t.Errorf("IsPushOnlyScript: test #%d failed: %x\n", i, script) continue } - for _, pop := range pops { - if result := canonicalPush(pop); !result { - t.Errorf("StandardPushesTests TstHasCanonicalPushes test #%d failed: %x\n", i, script) + tokenizer := MakeScriptTokenizer(scriptVersion, script) + for tokenizer.Next() { + if !isCanonicalPush(tokenizer.Opcode(), tokenizer.Data()) { + t.Errorf("isCanonicalPush: test #%d failed: %x\n", i, script) break } } @@ -4213,11 +4203,13 @@ func TestIsPayToWitnessPubKeyHash(t *testing.T) { } } -// TestHasCanonicalPushes ensures the canonicalPush function properly determines -// what is considered a canonical push for the purposes of removeOpcodeByData. +// TestHasCanonicalPushes ensures the isCanonicalPush function properly +// 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 != nil { + if err := checkScriptParses(scriptVersion, script); 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 { - if canonicalPush(pop) != test.expected { - t.Errorf("canonicalPush: #%d (%s) wrong result"+ - "\ngot: %v\nwant: %v", i, test.name, - true, test.expected) + tokenizer := MakeScriptTokenizer(scriptVersion, script) + for tokenizer.Next() { + result := isCanonicalPush(tokenizer.Opcode(), tokenizer.Data()) + if result != test.expected { + t.Errorf("%q: isCanonicalPush wrong result\ngot: %v\nwant: %v", + test.name, result, test.expected) break } } diff --git a/txscript/standard.go b/txscript/standard.go index ca45b11dd1..d52cfbc0d9 100644 --- a/txscript/standard.go +++ b/txscript/standard.go @@ -945,7 +945,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 && @@ -954,7 +954,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 &&