diff --git a/internal_test.go b/internal_test.go index e03c483d..5886bb8d 100644 --- a/internal_test.go +++ b/internal_test.go @@ -18,6 +18,10 @@ import ( "github.com/btcsuite/btcwire" ) +// TstMaxScriptSize makes the internal maxScriptSize constant available to the +// test package. +const TstMaxScriptSize = maxScriptSize + // this file is present to export some internal interfaces so that we can // test them reliably. @@ -3781,7 +3785,7 @@ func ParseShortForm(script string) ([]byte, error) { builder.script = append(builder.script, bts...) } else if len(tok) >= 2 && tok[0] == '\'' && tok[len(tok)-1] == '\'' { - builder.AddData([]byte(tok[1 : len(tok)-1])) + builder.AddFullData([]byte(tok[1 : len(tok)-1])) } else if opcode, ok := ops[tok]; ok { builder.AddOp(opcode.value) } else { @@ -3789,7 +3793,7 @@ func ParseShortForm(script string) ([]byte, error) { } } - return builder.Script(), nil + return builder.Script() } func TestBitcoindInvalidTests(t *testing.T) { diff --git a/script.go b/script.go index ddecc5bf..e6be9339 100644 --- a/script.go +++ b/script.go @@ -1019,7 +1019,7 @@ func getSigOpCount(pops []parsedOpcode, precise bool) int { // payToPubKeyHashScript creates a new script to pay a transaction // output to a 20-byte pubkey hash. It is expected that the input is a valid // hash. -func payToPubKeyHashScript(pubKeyHash []byte) []byte { +func payToPubKeyHashScript(pubKeyHash []byte) ([]byte, error) { return NewScriptBuilder().AddOp(OP_DUP).AddOp(OP_HASH160). AddData(pubKeyHash).AddOp(OP_EQUALVERIFY).AddOp(OP_CHECKSIG). Script() @@ -1027,14 +1027,14 @@ func payToPubKeyHashScript(pubKeyHash []byte) []byte { // payToScriptHashScript creates a new script to pay a transaction output to a // script hash. It is expected that the input is a valid hash. -func payToScriptHashScript(scriptHash []byte) []byte { +func payToScriptHashScript(scriptHash []byte) ([]byte, error) { return NewScriptBuilder().AddOp(OP_HASH160).AddData(scriptHash). AddOp(OP_EQUAL).Script() } // payToPubkeyScript creates a new script to pay a transaction output to a // public key. It is expected that the input is a valid pubkey. -func payToPubKeyScript(serializedPubKey []byte) []byte { +func payToPubKeyScript(serializedPubKey []byte) ([]byte, error) { return NewScriptBuilder().AddData(serializedPubKey). AddOp(OP_CHECKSIG).Script() } @@ -1047,19 +1047,19 @@ func PayToAddrScript(addr btcutil.Address) ([]byte, error) { if addr == nil { return nil, ErrUnsupportedAddress } - return payToPubKeyHashScript(addr.ScriptAddress()), nil + return payToPubKeyHashScript(addr.ScriptAddress()) case *btcutil.AddressScriptHash: if addr == nil { return nil, ErrUnsupportedAddress } - return payToScriptHashScript(addr.ScriptAddress()), nil + return payToScriptHashScript(addr.ScriptAddress()) case *btcutil.AddressPubKey: if addr == nil { return nil, ErrUnsupportedAddress } - return payToPubKeyScript(addr.ScriptAddress()), nil + return payToPubKeyScript(addr.ScriptAddress()) } return nil, ErrUnsupportedAddress @@ -1085,7 +1085,7 @@ func MultiSigScript(pubkeys []*btcutil.AddressPubKey, nrequired int) ([]byte, er builder.AddInt64(int64(len(pubkeys))) builder.AddOp(OP_CHECKMULTISIG) - return builder.Script(), nil + return builder.Script() } // SignatureScript creates an input signature script for tx to spend @@ -1111,7 +1111,7 @@ func SignatureScript(tx *btcwire.MsgTx, idx int, subscript []byte, hashType SigH pkData = pk.SerializeUncompressed() } - return NewScriptBuilder().AddData(sig).AddData(pkData).Script(), nil + return NewScriptBuilder().AddData(sig).AddData(pkData).Script() } // RawTxInSignature returns the serialized ECDSA signature for the input @@ -1137,7 +1137,7 @@ func p2pkSignatureScript(tx *btcwire.MsgTx, idx int, subScript []byte, hashType return nil, err } - return NewScriptBuilder().AddData(sig).Script(), nil + return NewScriptBuilder().AddData(sig).Script() } // signMultiSig signs as many of the outputs in the provided multisig script as @@ -1169,7 +1169,8 @@ func signMultiSig(tx *btcwire.MsgTx, idx int, subScript []byte, hashType SigHash } - return builder.Script(), signed == nRequired + script, _ := builder.Script() + return script, signed == nRequired } func sign(net *btcnet.Params, tx *btcwire.MsgTx, idx int, subScript []byte, @@ -1277,7 +1278,8 @@ func mergeScripts(net *btcnet.Params, tx *btcwire.MsgTx, idx int, builder := NewScriptBuilder() builder.script = mergedScript builder.AddData(script) - return builder.Script() + finalScript, _ := builder.Script() + return finalScript case MultiSigTy: return mergeMultiSig(tx, idx, addresses, nRequired, pkScript, sigScript, prevScript) @@ -1407,7 +1409,8 @@ sigLoop: builder.AddOp(OP_0) } - return builder.Script() + script, _ := builder.Script() + return script } // KeyDB is an interface type provided to SignTxOutput, it encapsulates @@ -1470,7 +1473,7 @@ func SignTxOutput(net *btcnet.Params, tx *btcwire.MsgTx, idx int, builder.script = realSigScript builder.AddData(sigScript) - sigScript = builder.Script() + sigScript, _ = builder.Script() // TODO keep a copy of the script for merging. } diff --git a/script_test.go b/script_test.go index fbb08147..8ff919a7 100644 --- a/script_test.go +++ b/script_test.go @@ -17,6 +17,18 @@ import ( "github.com/btcsuite/btcwire" ) +// builderScript is a convenience function which is used in the tests. It +// allows access to the script from a known good script built with the builder. +// Any errors are converted to a panic since it is only, and must only, be used +// with hard coded, and therefore, known good, scripts. +func builderScript(builder *btcscript.ScriptBuilder) []byte { + script, err := builder.Script() + if err != nil { + panic(err) + } + return script +} + func TestPushedData(t *testing.T) { var tests = []struct { in []byte @@ -29,7 +41,7 @@ func TestPushedData(t *testing.T) { true, }, { - btcscript.NewScriptBuilder().AddInt64(16777216).AddInt64(10000000).Script(), + builderScript(btcscript.NewScriptBuilder().AddInt64(16777216).AddInt64(10000000)), [][]byte{ {0x00, 0x00, 0x00, 0x01}, // 16777216 {0x80, 0x96, 0x98, 0x00}, // 10000000 @@ -37,9 +49,9 @@ func TestPushedData(t *testing.T) { true, }, { - btcscript.NewScriptBuilder().AddOp(btcscript.OP_DUP).AddOp(btcscript.OP_HASH160). + builderScript(btcscript.NewScriptBuilder().AddOp(btcscript.OP_DUP).AddOp(btcscript.OP_HASH160). AddData([]byte("17VZNX1SN5NtKa8UQFxwQbFeFc3iqRYhem")).AddOp(btcscript.OP_EQUALVERIFY). - AddOp(btcscript.OP_CHECKSIG).Script(), + AddOp(btcscript.OP_CHECKSIG)), [][]byte{ // 17VZNX1SN5NtKa8UQFxwQbFeFc3iqRYhem { @@ -52,8 +64,8 @@ func TestPushedData(t *testing.T) { true, }, { - btcscript.NewScriptBuilder().AddOp(btcscript.OP_PUSHDATA4).AddInt64(1000). - AddOp(btcscript.OP_EQUAL).Script(), + builderScript(btcscript.NewScriptBuilder().AddOp(btcscript.OP_PUSHDATA4).AddInt64(1000). + AddOp(btcscript.OP_EQUAL)), [][]byte{}, false, }, @@ -78,25 +90,37 @@ func TestPushedData(t *testing.T) { } func TestStandardPushes(t *testing.T) { - for i := 0; i < 1000; i++ { + for i := 0; i < 65535; i++ { builder := btcscript.NewScriptBuilder() builder.AddInt64(int64(i)) - if result := btcscript.IsPushOnlyScript(builder.Script()); !result { - t.Errorf("StandardPushesTests IsPushOnlyScript test #%d failed: %x\n", i, builder.Script()) + script, err := builder.Script() + if err != nil { + t.Errorf("StandardPushesTests test #%d unexpected error: %v\n", i, err) + continue } - if result := btcscript.HasCanonicalPushes(builder.Script()); !result { - t.Errorf("StandardPushesTests HasCanonicalPushes test #%d failed: %x\n", i, builder.Script()) + if result := btcscript.IsPushOnlyScript(script); !result { + t.Errorf("StandardPushesTests IsPushOnlyScript test #%d failed: %x\n", i, script) + continue + } + if result := btcscript.HasCanonicalPushes(script); !result { + t.Errorf("StandardPushesTests HasCanonicalPushes test #%d failed: %x\n", i, script) continue } } - for i := 0; i < 1000; i++ { + for i := 0; i <= btcscript.MaxScriptElementSize; i++ { builder := btcscript.NewScriptBuilder() builder.AddData(bytes.Repeat([]byte{0x49}, i)) - if result := btcscript.IsPushOnlyScript(builder.Script()); !result { - t.Errorf("StandardPushesTests IsPushOnlyScript test #%d failed: %x\n", i, builder.Script()) + script, err := builder.Script() + if err != nil { + t.Errorf("StandardPushesTests test #%d unexpected error: %v\n", i, err) + continue } - if result := btcscript.HasCanonicalPushes(builder.Script()); !result { - t.Errorf("StandardPushesTests HasCanonicalPushes test #%d failed: %x\n", i, builder.Script()) + if result := btcscript.IsPushOnlyScript(script); !result { + t.Errorf("StandardPushesTests IsPushOnlyScript test #%d failed: %x\n", i, script) + continue + } + if result := btcscript.HasCanonicalPushes(script); !result { + t.Errorf("StandardPushesTests HasCanonicalPushes test #%d failed: %x\n", i, script) continue } } diff --git a/scriptbuilder.go b/scriptbuilder.go index e34008bb..566189df 100644 --- a/scriptbuilder.go +++ b/scriptbuilder.go @@ -6,6 +6,7 @@ package btcscript import ( "encoding/binary" + "fmt" "math/big" ) @@ -18,9 +19,21 @@ const ( defaultScriptAlloc = 500 ) +// ErrScriptNotCanonical identifies a non-canonical script. The caller can use +// a type assertion to detect this error type. +type ErrScriptNotCanonical string + +// Error implements the error interface. +func (e ErrScriptNotCanonical) Error() string { + return string(e) +} + // ScriptBuilder provides a facility for building custom scripts. It allows -// you to push opcodes, ints, and data while respecting canonical encoding. It -// does not ensure the script will execute correctly. +// you to push opcodes, ints, and data while respecting canonical encoding. In +// general it does not ensure the script will execute correctly, however any +// data pushes which would exceed the maximum allowed script engine limits and +// are therefore guaranteed not to execute will not be pushed and will result in +// the Script function returning an error. // // For example, the following would build a 2-of-3 multisig script for usage in // a pay-to-script-hash (although in this situation MultiSigScript() would be a @@ -29,21 +42,70 @@ const ( // builder.AddOp(btcscript.OP_2).AddData(pubKey1).AddData(pubKey2) // builder.AddData(pubKey3).AddOp(btcscript.OP_3) // builder.AddOp(btcscript.OP_CHECKMULTISIG) -// fmt.Printf("Final multi-sig script: %x\n", builder.Script()) +// script, err := builder.Script() +// if err != nil { +// // Handle the error. +// return +// } +// fmt.Printf("Final multi-sig script: %x\n", script) type ScriptBuilder struct { script []byte + err error } -// AddOp pushes the passed opcode to the end of the script. +// AddOp pushes the passed opcode to the end of the script. The script will not +// be modified if pushing the opcode would cause the script to exceed the +// maximum allowed script engine size. func (b *ScriptBuilder) AddOp(opcode byte) *ScriptBuilder { + if b.err != nil { + return b + } + + // Pushes that would cause the script to exceed the largest allowed + // script size would result in a non-canonical script. + if len(b.script)+1 > maxScriptSize { + str := fmt.Sprintf("adding an opcode would exceed the maximum "+ + "allowed canonical script length of %d", maxScriptSize) + b.err = ErrScriptNotCanonical(str) + return b + } + b.script = append(b.script, opcode) return b } -// AddData pushes the passed data to the end of the script. It automatically -// chooses canonical opcodes depending on the length of the data. A zero length -// buffer will lead to a push of empty data onto the stack. -func (b *ScriptBuilder) AddData(data []byte) *ScriptBuilder { +// canonicalDataSize returns the number of bytes the canonical encoding of the +// data will take. +func canonicalDataSize(data []byte) int { + dataLen := len(data) + + // When the data consists of a single number that can be represented + // by one of the "small integer" opcodes, that opcode will be instead + // of a data push opcode followed by the number. + if dataLen == 0 { + return 1 + } else if dataLen == 1 && data[0] <= 16 { + return 1 + } else if dataLen == 1 && data[0] == 0x81 { + return 1 + } + + if dataLen < OP_PUSHDATA1 { + return 1 + dataLen + } else if dataLen <= 0xff { + return 2 + dataLen + } else if dataLen <= 0xffff { + return 3 + dataLen + } + + return 5 + dataLen +} + +// addData is the internal function that actually pushes the passed data to the +// end of the script. It automatically chooses canonical opcodes depending on +// the length of the data. A zero length buffer will lead to a push of empty +// data onto the stack (OP_0). No data limits are enforced with this function. +func (b *ScriptBuilder) addData(data []byte) *ScriptBuilder { dataLen := len(data) // When the data consists of a single number that can be represented @@ -55,6 +117,9 @@ func (b *ScriptBuilder) AddData(data []byte) *ScriptBuilder { } else if dataLen == 1 && data[0] <= 16 { b.script = append(b.script, byte((OP_1-1)+data[0])) return b + } else if dataLen == 1 && data[0] == 0x81 { + b.script = append(b.script, byte(OP_1NEGATE)) + return b } // Use one of the OP_DATA_# opcodes if the length of the data is small @@ -83,8 +148,76 @@ func (b *ScriptBuilder) AddData(data []byte) *ScriptBuilder { return b } -// AddInt64 pushes the passed integer to the end of the script. +// AddFullData should not typically be used by ordinary users as it does not +// include the checks which prevent data pushes larger than the maximum allowed +// sizes which leads to scripts that can't be executed. This is provided for +// testing purposes such as regression tests where sizes are intentionally made +// larger than allowed. +// +// Use AddData instead. +func (b *ScriptBuilder) AddFullData(data []byte) *ScriptBuilder { + if b.err != nil { + return b + } + + return b.addData(data) +} + +// AddData pushes the passed data to the end of the script. It automatically +// chooses canonical opcodes depending on the length of the data. A zero length +// buffer will lead to a push of empty data onto the stack (OP_0) and any push +// of data greater than MaxScriptElementSize will not modify the script since +// that is not allowed by the script engine. Also, the script will not be +// modified if pushing the data would cause the script to exceed the maximum +// allowed script engine size. +func (b *ScriptBuilder) AddData(data []byte) *ScriptBuilder { + if b.err != nil { + return b + } + + // Pushes that would cause the script to exceed the largest allowed + // script size would result in a non-canonical script. + dataSize := canonicalDataSize(data) + if len(b.script)+dataSize > maxScriptSize { + str := fmt.Sprintf("adding %d bytes of data would exceed the "+ + "maximum allowed canonical script length of %d", + dataSize, maxScriptSize) + b.err = ErrScriptNotCanonical(str) + return b + } + + // Pushes larger than the max script element size would result in a + // script that is not canonical. + dataLen := len(data) + if dataLen > MaxScriptElementSize { + str := fmt.Sprintf("adding a data element of %d bytes would "+ + "exceed the maximum allowed script element size of %d", + dataLen, maxScriptSize) + b.err = ErrScriptNotCanonical(str) + return b + } + + return b.addData(data) +} + +// AddInt64 pushes the passed integer to the end of the script. The script will +// not be modified if pushing the data would cause the script to exceed the +// maximum allowed script engine size. func (b *ScriptBuilder) AddInt64(val int64) *ScriptBuilder { + if b.err != nil { + return b + } + + // Pushes that would cause the script to exceed the largest allowed + // script size would result in a non-canonical script. + if len(b.script)+1 > maxScriptSize { + str := fmt.Sprintf("adding an integer would exceed the "+ + "maximum allow canonical script length of %d", + maxScriptSize) + b.err = ErrScriptNotCanonical(str) + return b + } + // Fast path for small integers and OP_1NEGATE. if val == 0 { b.script = append(b.script, OP_0) @@ -98,8 +231,24 @@ func (b *ScriptBuilder) AddInt64(val int64) *ScriptBuilder { return b.AddData(fromInt(new(big.Int).SetInt64(val))) } -// AddUint64 pushes the passed integer to the end of the script. +// AddUint64 pushes the passed integer to the end of the script. The script +// will not be modified if pushing the data would cause the script to +// exceed the maximum allowed script engine size. func (b *ScriptBuilder) AddUint64(val uint64) *ScriptBuilder { + if b.err != nil { + return b + } + + // Pushes that would cause the script to exceed the largest allowed + // script size would result in a non-canonical script. + if len(b.script)+1 > maxScriptSize { + str := fmt.Sprintf("adding an unsigned integer would exceed "+ + "the maximum allow canonical script length of %d", + maxScriptSize) + b.err = ErrScriptNotCanonical(str) + return b + } + // Fast path for small integers. if val == 0 { b.script = append(b.script, OP_0) @@ -116,12 +265,15 @@ func (b *ScriptBuilder) AddUint64(val uint64) *ScriptBuilder { // Reset resets the script so it has no content. func (b *ScriptBuilder) Reset() *ScriptBuilder { b.script = b.script[0:0] + b.err = nil return b } -// Script returns the currently built script. -func (b *ScriptBuilder) Script() []byte { - return b.script +// Script returns the currently built script. When any errors occured while +// building the script, the script will be returned up the point of the first +// error along with the error. +func (b *ScriptBuilder) Script() ([]byte, error) { + return b.script, b.err } // NewScriptBuilder returns a new instance of a script builder. See diff --git a/scriptbuilder_test.go b/scriptbuilder_test.go index c5989494..265ba882 100644 --- a/scriptbuilder_test.go +++ b/scriptbuilder_test.go @@ -14,6 +14,8 @@ import ( // TestScriptBuilderAddOp tests that pushing opcodes to a script via the // ScriptBuilder API works as expected. func TestScriptBuilderAddOp(t *testing.T) { + t.Parallel() + tests := []struct { name string opcodes []byte @@ -43,7 +45,12 @@ func TestScriptBuilderAddOp(t *testing.T) { for _, opcode := range test.opcodes { builder.AddOp(opcode) } - result := builder.Script() + result, err := builder.Script() + if err != nil { + t.Errorf("ScriptBuilder.AddOp #%d (%s) unexpected "+ + "error: %v", i, test.name, err) + continue + } if !bytes.Equal(result, test.expected) { t.Errorf("ScriptBuilder.AddOp #%d (%s) wrong result\n"+ "got: %x\nwant: %x", i, test.name, result, @@ -56,6 +63,8 @@ func TestScriptBuilderAddOp(t *testing.T) { // TestScriptBuilderAddInt64 tests that pushing signed integers to a script via // the ScriptBuilder API works as expected. func TestScriptBuilderAddInt64(t *testing.T) { + t.Parallel() + tests := []struct { name string val int64 @@ -105,7 +114,12 @@ func TestScriptBuilderAddInt64(t *testing.T) { t.Logf("Running %d tests", len(tests)) for i, test := range tests { builder.Reset().AddInt64(test.val) - result := builder.Script() + result, err := builder.Script() + if err != nil { + t.Errorf("ScriptBuilder.AddInt64 #%d (%s) unexpected "+ + "error: %v", i, test.name, err) + continue + } if !bytes.Equal(result, test.expected) { t.Errorf("ScriptBuilder.AddInt64 #%d (%s) wrong result\n"+ "got: %x\nwant: %x", i, test.name, result, @@ -118,6 +132,8 @@ func TestScriptBuilderAddInt64(t *testing.T) { // TestScriptBuilderAddUint64 tests that pushing unsigned integers to a script // via the ScriptBuilder API works as expected. func TestScriptBuilderAddUint64(t *testing.T) { + t.Parallel() + tests := []struct { name string val uint64 @@ -154,7 +170,12 @@ func TestScriptBuilderAddUint64(t *testing.T) { t.Logf("Running %d tests", len(tests)) for i, test := range tests { builder.Reset().AddUint64(test.val) - result := builder.Script() + result, err := builder.Script() + if err != nil { + t.Errorf("ScriptBuilder.AddUint64 #%d (%s) unexpected "+ + "error: %v", err) + continue + } if !bytes.Equal(result, test.expected) { t.Errorf("ScriptBuilder.AddUint64 #%d (%s) wrong result\n"+ "got: %x\nwant: %x", i, test.name, result, @@ -165,33 +186,48 @@ func TestScriptBuilderAddUint64(t *testing.T) { } // TestScriptBuilderAddData tests that pushing data to a script via the -// ScriptBuilder API works as expected. +// ScriptBuilder API works as expected and conforms to BIP0062. func TestScriptBuilderAddData(t *testing.T) { + t.Parallel() + tests := []struct { name string data []byte expected []byte + useFull bool // use AddFullData instead of AddData. }{ - // Start off with the small ints to ensure canonical encoding. - {name: "push small int 0", data: []byte{0}, expected: []byte{btcscript.OP_0}}, - {name: "push small int 1", data: []byte{1}, expected: []byte{btcscript.OP_1}}, - {name: "push small int 2", data: []byte{2}, expected: []byte{btcscript.OP_2}}, - {name: "push small int 3", data: []byte{3}, expected: []byte{btcscript.OP_3}}, - {name: "push small int 4", data: []byte{4}, expected: []byte{btcscript.OP_4}}, - {name: "push small int 5", data: []byte{5}, expected: []byte{btcscript.OP_5}}, - {name: "push small int 6", data: []byte{6}, expected: []byte{btcscript.OP_6}}, - {name: "push small int 7", data: []byte{7}, expected: []byte{btcscript.OP_7}}, - {name: "push small int 8", data: []byte{8}, expected: []byte{btcscript.OP_8}}, - {name: "push small int 9", data: []byte{9}, expected: []byte{btcscript.OP_9}}, - {name: "push small int 10", data: []byte{10}, expected: []byte{btcscript.OP_10}}, - {name: "push small int 11", data: []byte{11}, expected: []byte{btcscript.OP_11}}, - {name: "push small int 12", data: []byte{12}, expected: []byte{btcscript.OP_12}}, - {name: "push small int 13", data: []byte{13}, expected: []byte{btcscript.OP_13}}, - {name: "push small int 14", data: []byte{14}, expected: []byte{btcscript.OP_14}}, - {name: "push small int 15", data: []byte{15}, expected: []byte{btcscript.OP_15}}, - {name: "push small int 16", data: []byte{16}, expected: []byte{btcscript.OP_16}}, + // BIP0062: Pushing an empty byte sequence must use OP_0. + {name: "push empty byte sequence", data: []byte{}, expected: []byte{btcscript.OP_0}}, + {name: "push 1 byte 0x00", data: []byte{0x00}, expected: []byte{btcscript.OP_0}}, - // 1-byte data push opcodes. + // BIP0062: Pushing a 1-byte sequence of byte 0x01 through 0x10 must use OP_n. + {name: "push 1 byte 0x01", data: []byte{0x01}, expected: []byte{btcscript.OP_1}}, + {name: "push 1 byte 0x02", data: []byte{0x02}, expected: []byte{btcscript.OP_2}}, + {name: "push 1 byte 0x03", data: []byte{0x03}, expected: []byte{btcscript.OP_3}}, + {name: "push 1 byte 0x04", data: []byte{0x04}, expected: []byte{btcscript.OP_4}}, + {name: "push 1 byte 0x05", data: []byte{0x05}, expected: []byte{btcscript.OP_5}}, + {name: "push 1 byte 0x06", data: []byte{0x06}, expected: []byte{btcscript.OP_6}}, + {name: "push 1 byte 0x07", data: []byte{0x07}, expected: []byte{btcscript.OP_7}}, + {name: "push 1 byte 0x08", data: []byte{0x08}, expected: []byte{btcscript.OP_8}}, + {name: "push 1 byte 0x09", data: []byte{0x09}, expected: []byte{btcscript.OP_9}}, + {name: "push 1 byte 0x0a", data: []byte{0x0a}, expected: []byte{btcscript.OP_10}}, + {name: "push 1 byte 0x0b", data: []byte{0x0b}, expected: []byte{btcscript.OP_11}}, + {name: "push 1 byte 0x0c", data: []byte{0x0c}, expected: []byte{btcscript.OP_12}}, + {name: "push 1 byte 0x0d", data: []byte{0x0d}, expected: []byte{btcscript.OP_13}}, + {name: "push 1 byte 0x0e", data: []byte{0x0e}, expected: []byte{btcscript.OP_14}}, + {name: "push 1 byte 0x0f", data: []byte{0x0f}, expected: []byte{btcscript.OP_15}}, + {name: "push 1 byte 0x10", data: []byte{0x10}, expected: []byte{btcscript.OP_16}}, + + // BIP0062: Pushing the byte 0x81 must use OP_1NEGATE. + {name: "push 1 byte 0x81", data: []byte{0x81}, expected: []byte{btcscript.OP_1NEGATE}}, + + // BIP0062: Pushing any other byte sequence up to 75 bytes must + // use the normal data push (opcode byte n, with n the number of + // bytes, followed n bytes of data being pushed). + {name: "push 1 byte 0x11", data: []byte{0x11}, expected: []byte{btcscript.OP_DATA_1, 0x11}}, + {name: "push 1 byte 0x80", data: []byte{0x80}, expected: []byte{btcscript.OP_DATA_1, 0x80}}, + {name: "push 1 byte 0x82", data: []byte{0x82}, expected: []byte{btcscript.OP_DATA_1, 0x82}}, + {name: "push 1 byte 0xff", data: []byte{0xff}, expected: []byte{btcscript.OP_DATA_1, 0xff}}, { name: "push data len 17", data: bytes.Repeat([]byte{0x49}, 17), @@ -203,7 +239,7 @@ func TestScriptBuilderAddData(t *testing.T) { expected: append([]byte{btcscript.OP_DATA_75}, bytes.Repeat([]byte{0x49}, 75)...), }, - // 2-byte data push via OP_PUSHDATA_1. + // BIP0062: Pushing 76 to 255 bytes must use OP_PUSHDATA1. { name: "push data len 76", data: bytes.Repeat([]byte{0x49}, 76), @@ -215,31 +251,67 @@ func TestScriptBuilderAddData(t *testing.T) { expected: append([]byte{btcscript.OP_PUSHDATA1, 255}, bytes.Repeat([]byte{0x49}, 255)...), }, - // 3-byte data push via OP_PUSHDATA_2. + // BIP0062: Pushing 256 to 520 bytes must use OP_PUSHDATA2. { name: "push data len 256", data: bytes.Repeat([]byte{0x49}, 256), expected: append([]byte{btcscript.OP_PUSHDATA2, 0, 1}, bytes.Repeat([]byte{0x49}, 256)...), }, { - name: "push data len 32767", + name: "push data len 520", + data: bytes.Repeat([]byte{0x49}, 520), + expected: append([]byte{btcscript.OP_PUSHDATA2, 0x08, 0x02}, bytes.Repeat([]byte{0x49}, 520)...), + }, + + // BIP0062: OP_PUSHDATA4 can never be used, as pushes over 520 + // bytes are not allowed, and those below can be done using + // other operators. + { + name: "push data len 521", + data: bytes.Repeat([]byte{0x49}, 521), + expected: []byte{}, + }, + { + name: "push data len 32767 (canonical)", + data: bytes.Repeat([]byte{0x49}, 32767), + expected: []byte{}, + }, + { + name: "push data len 65536 (canonical)", + data: bytes.Repeat([]byte{0x49}, 65536), + expected: []byte{}, + }, + + // Additional tests for the PushFullData function that + // intentionally allows data pushes to exceed the limit for + // regression testing purposes. + + // 3-byte data push via OP_PUSHDATA_2. + { + name: "push data len 32767 (non-canonical)", data: bytes.Repeat([]byte{0x49}, 32767), expected: append([]byte{btcscript.OP_PUSHDATA2, 255, 127}, bytes.Repeat([]byte{0x49}, 32767)...), + useFull: true, }, // 5-byte data push via OP_PUSHDATA_4. { - name: "push data len 65536", + name: "push data len 65536 (non-canonical)", data: bytes.Repeat([]byte{0x49}, 65536), expected: append([]byte{btcscript.OP_PUSHDATA4, 0, 0, 1, 0}, bytes.Repeat([]byte{0x49}, 65536)...), + useFull: true, }, } builder := btcscript.NewScriptBuilder() t.Logf("Running %d tests", len(tests)) for i, test := range tests { - builder.Reset().AddData(test.data) - result := builder.Script() + if !test.useFull { + builder.Reset().AddData(test.data) + } else { + builder.Reset().AddFullData(test.data) + } + result, _ := builder.Script() if !bytes.Equal(result, test.expected) { t.Errorf("ScriptBuilder.AddData #%d (%s) wrong result\n"+ "got: %x\nwant: %x", i, test.name, result, @@ -248,3 +320,156 @@ func TestScriptBuilderAddData(t *testing.T) { } } } + +// TestExceedMaxScriptSize ensures that all of the functions that can be used +// to add data to a script don't allow the script to exceed the max allowed +// size. +func TestExceedMaxScriptSize(t *testing.T) { + t.Parallel() + + // Start off by constructing a max size script. + maxScriptSize := btcscript.TstMaxScriptSize + builder := btcscript.NewScriptBuilder() + builder.Reset().AddFullData(make([]byte, maxScriptSize-3)) + origScript, err := builder.Script() + if err != nil { + t.Fatalf("Unexpected error for max size script: %v", err) + } + + // Ensure adding data that would exceed the maximum size of the script + // does not add the data. + script, err := builder.AddData([]byte{0x00}).Script() + if _, ok := err.(btcscript.ErrScriptNotCanonical); !ok || err == nil { + t.Fatalf("ScriptBuilder.AddData allowed exceeding max script "+ + "size: %v", len(script)) + } + if !bytes.Equal(script, origScript) { + t.Fatalf("ScriptBuilder.AddData unexpected modified script - "+ + "got len %d, want len %d", len(script), len(origScript)) + } + + // Ensure adding an opcode that would exceed the maximum size of the + // script does not add the data. + builder.Reset().AddFullData(make([]byte, maxScriptSize-3)) + script, err = builder.AddOp(btcscript.OP_0).Script() + if _, ok := err.(btcscript.ErrScriptNotCanonical); !ok || err == nil { + t.Fatalf("ScriptBuilder.AddOp unexpected modified script - "+ + "got len %d, want len %d", len(script), len(origScript)) + } + if !bytes.Equal(script, origScript) { + t.Fatalf("ScriptBuilder.AddOp unexpected modified script - "+ + "got len %d, want len %d", len(script), len(origScript)) + } + + // Ensure adding an integer that would exceed the maximum size of the + // script does not add the data. + builder.Reset().AddFullData(make([]byte, maxScriptSize-3)) + script, err = builder.AddInt64(0).Script() + if _, ok := err.(btcscript.ErrScriptNotCanonical); !ok || err == nil { + t.Fatalf("ScriptBuilder.AddInt64 unexpected modified script - "+ + "got len %d, want len %d", len(script), len(origScript)) + } + if !bytes.Equal(script, origScript) { + t.Fatalf("ScriptBuilder.AddInt64 unexpected modified script - "+ + "got len %d, want len %d", len(script), len(origScript)) + } + + // Ensure adding an unsigned integer that would exceed the maximum size + // of the script does not add the data. + builder.Reset().AddFullData(make([]byte, maxScriptSize-3)) + script, err = builder.AddUint64(0).Script() + if _, ok := err.(btcscript.ErrScriptNotCanonical); !ok || err == nil { + t.Fatalf("ScriptBuilder.AddUint64 unexpected modified script - "+ + "got len %d, want len %d", len(script), len(origScript)) + } + if !bytes.Equal(script, origScript) { + t.Fatalf("ScriptBuilder.AddUint64 unexpected modified script - "+ + "got len %d, want len %d", len(script), len(origScript)) + } +} + +// TestErroredScript ensures that all of the functions that can be used to add +// data to a script don't modify the script once an error has happened. +func TestErroredScript(t *testing.T) { + t.Parallel() + + // Start off by constructing a near max size script that has enough + // space left to add each data type without an error and force an + // initial error condition. + maxScriptSize := btcscript.TstMaxScriptSize + builder := btcscript.NewScriptBuilder() + builder.Reset().AddFullData(make([]byte, maxScriptSize-8)) + origScript, err := builder.Script() + if err != nil { + t.Fatalf("ScriptBuilder.AddFullData unexpected error: %v", err) + } + script, err := builder.AddData([]byte{0x00, 0x00, 0x00, 0x00, 0x00}).Script() + if _, ok := err.(btcscript.ErrScriptNotCanonical); !ok || err == nil { + t.Fatalf("ScriptBuilder.AddData allowed exceeding max script "+ + "size: %v", len(script)) + } + if !bytes.Equal(script, origScript) { + t.Fatalf("ScriptBuilder.AddData unexpected modified script - "+ + "got len %d, want len %d", len(script), len(origScript)) + } + + // Ensure adding data, even using the non-canonical path, to a script + // that has errored doesn't succeed. + script, err = builder.AddFullData([]byte{0x00}).Script() + if _, ok := err.(btcscript.ErrScriptNotCanonical); !ok || err == nil { + t.Fatal("ScriptBuilder.AddFullData succeeded on errored script") + } + if !bytes.Equal(script, origScript) { + t.Fatalf("ScriptBuilder.AddFullData unexpected modified "+ + "script - got len %d, want len %d", len(script), + len(origScript)) + } + + // Ensure adding data to a script that has errored doesn't succeed. + script, err = builder.AddData([]byte{0x00}).Script() + if _, ok := err.(btcscript.ErrScriptNotCanonical); !ok || err == nil { + t.Fatal("ScriptBuilder.AddData succeeded on errored script") + } + if !bytes.Equal(script, origScript) { + t.Fatalf("ScriptBuilder.AddData unexpected modified "+ + "script - got len %d, want len %d", len(script), + len(origScript)) + } + + // Ensure adding an opcode to a script that has errored doesn't succeed. + script, err = builder.AddOp(btcscript.OP_0).Script() + if _, ok := err.(btcscript.ErrScriptNotCanonical); !ok || err == nil { + t.Fatal("ScriptBuilder.AddOp succeeded on errored script") + } + if !bytes.Equal(script, origScript) { + t.Fatalf("ScriptBuilder.AddOp unexpected modified script - "+ + "got len %d, want len %d", len(script), len(origScript)) + } + + // Ensure adding an integer to a script that has errored doesn't + // succeed. + script, err = builder.AddInt64(0).Script() + if _, ok := err.(btcscript.ErrScriptNotCanonical); !ok || err == nil { + t.Fatal("ScriptBuilder.AddInt64 succeeded on errored script") + } + if !bytes.Equal(script, origScript) { + t.Fatalf("ScriptBuilder.AddInt64 unexpected modified script - "+ + "got len %d, want len %d", len(script), len(origScript)) + } + + // Ensure adding an unsigned integer to a script that has errored + // doesn't succeed. + script, err = builder.AddUint64(0).Script() + if _, ok := err.(btcscript.ErrScriptNotCanonical); !ok || err == nil { + t.Fatal("ScriptBuilder.AddUint64 succeeded on errored script") + } + if !bytes.Equal(script, origScript) { + t.Fatalf("ScriptBuilder.AddUint64 unexpected modified script - "+ + "got len %d, want len %d", len(script), len(origScript)) + } + + // Ensure the error has a message set. + if err.Error() == "" { + t.Fatal("ErrScriptNotCanonical.Error does not have any text") + } +}