Update script builder for BIP0062 and enforce limits.

BIP0062 defines specific rules and canonical encodings for data pushes.

The existing script builder code already conformed to all but one of the
canonical data push rules that was added after it was originally
implemented (adding a single byte of 0x81 must be converted to
OP_1NEGATE).  This commit implements that case and expands the existing
tests to explicitly cover all cases mentioned in BIP0062.

In addition, as a part of this change, the AddData function has been
modified so that any attempt to push more than the maximum script element
size bytes (520) in one push or any pushes the would cause the script to
exceed the maximum script bytes allowed by the script engine (10000) will
result in the final call to the Script function to only return the script
up to the point of the first error along with the error. This change
should have little effect on existing callers since they are almost
positively not creating scripts which violate these rules as they could
never be executed, however it does mean they need to check the new error
return.

Since the regression tests intentionally need to be able to exceed that
limit, a new function named AddFullData has been added which does not
enforce the limits, but still provides canonical encoding of the pushed
data.

Note that this commit does not affect consensus rules nor modify the
script engine.

Also, the tests have been marked so they can run in parallel.

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) {

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.
 	}
 

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).
+			builderScript(btcscript.NewScriptBuilder().AddOp(btcscript.OP_PUSHDATA4).AddInt64(1000).
-				AddOp(btcscript.OP_EQUAL).Script(),
+				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 {
+		script, err := builder.Script()
-			t.Errorf("StandardPushesTests IsPushOnlyScript test #%d failed: %x\n", i, builder.Script())
+		if err != nil {
+			t.Errorf("StandardPushesTests test #%d unexpected error: %v\n", i, err)
+			continue
 		}
-		if result := btcscript.HasCanonicalPushes(builder.Script()); !result {
+		if result := btcscript.IsPushOnlyScript(script); !result {
-			t.Errorf("StandardPushesTests HasCanonicalPushes test #%d failed: %x\n", i, builder.Script())
+			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 {
+		script, err := builder.Script()
-			t.Errorf("StandardPushesTests IsPushOnlyScript test #%d failed: %x\n", i, builder.Script())
+		if err != nil {
+			t.Errorf("StandardPushesTests test #%d unexpected error: %v\n", i, err)
+			continue
 		}
-		if result := btcscript.HasCanonicalPushes(builder.Script()); !result {
+		if result := btcscript.IsPushOnlyScript(script); !result {
-			t.Errorf("StandardPushesTests HasCanonicalPushes test #%d failed: %x\n", i, builder.Script())
+			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
 		}
 	}

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
+// you to push opcodes, ints, and data while respecting canonical encoding.  In
-// does not ensure the script will execute correctly.
+// 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
+// canonicalDataSize returns the number of bytes the canonical encoding of the
-// chooses canonical opcodes depending on the length of the data. A zero length
+// data will take.
-// buffer will lead to a push of empty data onto the stack.
+func canonicalDataSize(data []byte) int {
-func (b *ScriptBuilder) AddData(data []byte) *ScriptBuilder {
+	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.
+// Script returns the currently built script.  When any errors occured while
-func (b *ScriptBuilder) Script() []byte {
+// building the script, the script will be returned up the point of the first
-	return b.script
+// 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

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.
+		// BIP0062: Pushing an empty byte sequence must use OP_0.
-		{name: "push small int 0", data: []byte{0}, expected: []byte{btcscript.OP_0}},
+		{name: "push empty byte sequence", data: []byte{}, expected: []byte{btcscript.OP_0}},
-		{name: "push small int 1", data: []byte{1}, expected: []byte{btcscript.OP_1}},
+		{name: "push 1 byte 0x00", data: []byte{0x00}, expected: []byte{btcscript.OP_0}},
-		{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}},
 
-		// 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)
+		if !test.useFull {
-		result := builder.Script()
+			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")
+	}
+}