From cee207c64caee6b2471ab73dce0b11c8762346cb Mon Sep 17 00:00:00 2001
From: Dave Collins <dave@davec.name>
Date: Fri, 12 Aug 2016 19:29:28 -0500
Subject: [PATCH] txscript: Expose AddOps on ScriptBuilder. (#734)

This exposes a new function on the ScriptBuilder type named AddOps that
allows multiple opcodes to be added via a single call and adds tests to
exercise the new function.

Finally, it updates a couple of places in the signing code that were
abusing the interface by setting its private script directly to use the
new public function instead.
---
 txscript/scriptbuilder.go      | 21 +++++++++++++++++++++
 txscript/scriptbuilder_test.go | 20 ++++++++++++++++++++
 txscript/sign.go               |  7 +++----
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/txscript/scriptbuilder.go b/txscript/scriptbuilder.go
index 6c8f641e..b22af624 100644
--- a/txscript/scriptbuilder.go
+++ b/txscript/scriptbuilder.go
@@ -73,6 +73,27 @@ func (b *ScriptBuilder) AddOp(opcode byte) *ScriptBuilder {
 	return b
 }
 
+// AddOps pushes the passed opcodes to the end of the script.  The script will
+// not be modified if pushing the opcodes would cause the script to exceed the
+// maximum allowed script engine size.
+func (b *ScriptBuilder) AddOps(opcodes []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)+len(opcodes) > maxScriptSize {
+		str := fmt.Sprintf("adding opcodes would exceed the maximum "+
+			"allowed canonical script length of %d", maxScriptSize)
+		b.err = ErrScriptNotCanonical(str)
+		return b
+	}
+
+	b.script = append(b.script, opcodes...)
+	return b
+}
+
 // canonicalDataSize returns the number of bytes the canonical encoding of the
 // data will take.
 func canonicalDataSize(data []byte) int {
diff --git a/txscript/scriptbuilder_test.go b/txscript/scriptbuilder_test.go
index 677c0cf0..904fd79f 100644
--- a/txscript/scriptbuilder_test.go
+++ b/txscript/scriptbuilder_test.go
@@ -38,6 +38,7 @@ func TestScriptBuilderAddOp(t *testing.T) {
 		},
 	}
 
+	// Run tests and individually add each op via AddOp.
 	builder := txscript.NewScriptBuilder()
 	t.Logf("Running %d tests", len(tests))
 	for i, test := range tests {
@@ -58,6 +59,25 @@ func TestScriptBuilderAddOp(t *testing.T) {
 			continue
 		}
 	}
+
+	// Run tests and bulk add ops via AddOps.
+	t.Logf("Running %d tests", len(tests))
+	for i, test := range tests {
+		builder.Reset()
+		result, err := builder.AddOps(test.opcodes).Script()
+		if err != nil {
+			t.Errorf("ScriptBuilder.AddOps #%d (%s) unexpected "+
+				"error: %v", i, test.name, err)
+			continue
+		}
+		if !bytes.Equal(result, test.expected) {
+			t.Errorf("ScriptBuilder.AddOps #%d (%s) wrong result\n"+
+				"got: %x\nwant: %x", i, test.name, result,
+				test.expected)
+			continue
+		}
+	}
+
 }
 
 // TestScriptBuilderAddInt64 tests that pushing signed integers to a script via
diff --git a/txscript/sign.go b/txscript/sign.go
index e810bd0c..f208a865 100644
--- a/txscript/sign.go
+++ b/txscript/sign.go
@@ -203,7 +203,7 @@ func mergeScripts(chainParams *chaincfg.Params, tx *wire.MsgTx, idx int,
 
 		// Reappend the script and return the result.
 		builder := NewScriptBuilder()
-		builder.script = mergedScript
+		builder.AddOps(mergedScript)
 		builder.AddData(script)
 		finalScript, _ := builder.Script()
 		return finalScript
@@ -394,10 +394,9 @@ func SignTxOutput(chainParams *chaincfg.Params, tx *wire.MsgTx, idx int,
 			return nil, err
 		}
 
-		// This is a bad thing. Append the p2sh script as the last
-		// push in the script.
+		// Append the p2sh script as the last push in the script.
 		builder := NewScriptBuilder()
-		builder.script = realSigScript
+		builder.AddOps(realSigScript)
 		builder.AddData(sigScript)
 
 		sigScript, _ = builder.Script()