txscript: Code consistency and doco improvements.

This commit contains a lot of cleanup on the txscript code to make it
more consistent with the code throughout the rest of the project.  It
doesn't change any operational logic.

The following is an overview of the changes:

- Add a significant number of comments throughout in order to better
  explain what the code is doing
- Fix several comment typos
- Move a couple of constants only used by the engine to engine.go
- Move a variable only used by the engine to engine.go
- Fix a couple of format specifiers in the test prints
- Reorder functions so they're defined before/closer to use
- Make the code lint clean with the exception of the opcode definitions

txscript/engine.go

@@ -8,11 +8,12 @@ import (
 	"fmt"
 	"math/big"
 
+	"github.com/btcsuite/btcd/btcec"
 	"github.com/btcsuite/btcd/wire"
 )
 
-// ScriptFlags is a bitmask defining additional operations or
+// ScriptFlags is a bitmask defining additional operations or tests that will be
-// tests that will be done when executing a Script.
+// done when executing a script pair.
 type ScriptFlags uint32
 
 const (
@@ -78,12 +79,24 @@ const (
 		ScriptVerifyCleanStack
 )
 
+const (
+	// maxStackSize is the maximum combined height of stack and alt stack
+	// during execution.
+	maxStackSize = 1000
+
+	// maxScriptSize is the maximum allowed length of a raw script.
+	maxScriptSize = 10000
+)
+
+// halforder is used to tame ECDSA malleability (see BIP0062).
+var halfOrder = new(big.Int).Rsh(btcec.S256().N, 1)
+
 // Engine is the virtual machine that executes scripts.
 type Engine struct {
 	scripts         [][]parsedOpcode
 	scriptIdx       int
 	scriptOff       int
-	lastcodesep     int
+	lastCodeSep     int
 	dstack          stack // data stack
 	astack          stack // alt stack
 	tx              wire.MsgTx
@@ -115,12 +128,12 @@ func (vm *Engine) isBranchExecuting() bool {
 // whether or not it is hidden by conditionals, but some rules still must be
 // tested in this case.
 func (vm *Engine) executeOpcode(pop *parsedOpcode) error {
-	// Disabled opcodes are ``fail on program counter''.
+	// Disabled opcodes are fail on program counter.
-	if pop.disabled() {
+	if pop.isDisabled() {
 		return ErrStackOpDisabled
 	}
 
-	// Always-illegal opcodes are ``fail on program counter''.
+	// Always-illegal opcodes are fail on program counter.
 	if pop.alwaysIllegal() {
 		return ErrStackReservedOpcode
 	}
@@ -136,16 +149,17 @@ func (vm *Engine) executeOpcode(pop *parsedOpcode) error {
 		return ErrStackElementTooBig
 	}
 
-	// If we are not a conditional opcode and we aren't executing, then
+	// Nothing left to do when this is not a conditional opcode and it is
-	// we are done now.
+	// not in an executing branch.
-	if !vm.isBranchExecuting() && !pop.conditional() {
+	if !vm.isBranchExecuting() && !pop.isConditional() {
 		return nil
 	}
 
 	// Ensure all executed data push opcodes use the minimal encoding when
-	// the minimal data verification is set.
+	// the minimal data verification flag is set.
 	if vm.dstack.verifyMinimalData && vm.isBranchExecuting() &&
 		pop.opcode.value >= 0 && pop.opcode.value <= OP_PUSHDATA4 {
+
 		if err := pop.checkMinimalDataPush(); err != nil {
 			return err
 		}
@@ -154,7 +168,179 @@ func (vm *Engine) executeOpcode(pop *parsedOpcode) error {
 	return pop.opcode.opfunc(pop, vm)
 }
 
-// Execute will execute all script in the script engine and return either nil
+// disasm is a helper function to produce the output for DisasmPC and
+// DisasmScript.  It produces the opcode prefixed by the program counter at the
+// provided position in the script.  It does no error checking and leaves that
+// to the caller to provide a valid offset.
+func (vm *Engine) disasm(scriptIdx int, scriptOff int) string {
+	return fmt.Sprintf("%02x:%04x: %s", scriptIdx, scriptOff,
+		vm.scripts[scriptIdx][scriptOff].print(false))
+}
+
+// validPC returns an error if the current script position is valid for
+// execution, nil otherwise.
+func (vm *Engine) validPC() error {
+	if vm.scriptIdx >= len(vm.scripts) {
+		return fmt.Errorf("past input scripts %v:%v %v:xxxx",
+			vm.scriptIdx, vm.scriptOff, len(vm.scripts))
+	}
+	if vm.scriptOff >= len(vm.scripts[vm.scriptIdx]) {
+		return fmt.Errorf("past input scripts %v:%v %v:%04d",
+			vm.scriptIdx, vm.scriptOff, vm.scriptIdx,
+			len(vm.scripts[vm.scriptIdx]))
+	}
+	return nil
+}
+
+// curPC returns either the current script and offset, or an error if the
+// position isn't valid.
+func (vm *Engine) curPC() (script int, off int, err error) {
+	err = vm.validPC()
+	if err != nil {
+		return 0, 0, err
+	}
+	return vm.scriptIdx, vm.scriptOff, nil
+}
+
+// DisasmPC returns the string for the disassembly of the opcode that will be
+// next to execute when Step() is called.
+func (vm *Engine) DisasmPC() (string, error) {
+	scriptIdx, scriptOff, err := vm.curPC()
+	if err != nil {
+		return "", err
+	}
+	return vm.disasm(scriptIdx, scriptOff), nil
+}
+
+// DisasmScript returns the disassembly string for the script at the requested
+// offset index.  Index 0 is the signature script and 1 is the public key
+// script.
+func (vm *Engine) DisasmScript(idx int) (string, error) {
+	if idx >= len(vm.scripts) {
+		return "", ErrStackInvalidIndex
+	}
+
+	var disstr string
+	for i := range vm.scripts[idx] {
+		disstr = disstr + vm.disasm(idx, i) + "\n"
+	}
+	return disstr, nil
+}
+
+// CheckErrorCondition returns nil if the running script has ended and was
+// successful, leaving a a true boolean on the stack.  An error otherwise,
+// including if the script has not finished.
+func (vm *Engine) CheckErrorCondition(finalScript bool) error {
+	// Check execution is actually done.  When pc is past the end of script
+	// array there are no more scripts to run.
+	if vm.scriptIdx < len(vm.scripts) {
+		return ErrStackScriptUnfinished
+	}
+	if finalScript && vm.hasFlag(ScriptVerifyCleanStack) &&
+		vm.dstack.Depth() != 1 {
+
+		return ErrStackCleanStack
+	} else if vm.dstack.Depth() < 1 {
+		return ErrStackEmptyStack
+	}
+
+	v, err := vm.dstack.PopBool()
+	if err != nil {
+		return err
+	}
+	if v == false {
+		// Log interesting data.
+		log.Tracef("%v", newLogClosure(func() string {
+			dis0, _ := vm.DisasmScript(0)
+			dis1, _ := vm.DisasmScript(1)
+			return fmt.Sprintf("scripts failed: script0: %s\n"+
+				"script1: %s", dis0, dis1)
+		}))
+		return ErrStackScriptFailed
+	}
+	return nil
+}
+
+// Step will execute the next instruction and move the program counter to the
+// next opcode in the script, or the next script if the current has ended.  Step
+// will return true in the case that the last opcode was successfully executed.
+//
+// The result of calling Step or any other method is undefined if an error is
+// returned.
+func (vm *Engine) Step() (done bool, err error) {
+	// Verify that it is pointing to a valid script address.
+	err = vm.validPC()
+	if err != nil {
+		return true, err
+	}
+	opcode := &vm.scripts[vm.scriptIdx][vm.scriptOff]
+
+	// Execute the opcode while taking into account several things such as
+	// disabled opcodes, illegal opcodes, maximum allowed operations per
+	// script, maximum script element sizes, and conditionals.
+	err = vm.executeOpcode(opcode)
+	if err != nil {
+		return true, err
+	}
+
+	// The number of elements in the combination of the data and alt stacks
+	// must not exceed the maximum number of stack elements allowed.
+	if vm.dstack.Depth()+vm.astack.Depth() > maxStackSize {
+		return false, ErrStackOverflow
+	}
+
+	// Prepare for next instruction.
+	vm.scriptOff++
+	if vm.scriptOff >= len(vm.scripts[vm.scriptIdx]) {
+		// Illegal to have an `if' that straddles two scripts.
+		if err == nil && len(vm.condStack) != 0 {
+			return false, ErrStackMissingEndif
+		}
+
+		// Alt stack doesn't persist.
+		_ = vm.astack.DropN(vm.astack.Depth())
+
+		vm.numOps = 0 // number of ops is per script.
+		vm.scriptOff = 0
+		if vm.scriptIdx == 0 && vm.bip16 {
+			vm.scriptIdx++
+			vm.savedFirstStack = vm.GetStack()
+		} else if vm.scriptIdx == 1 && vm.bip16 {
+			// Put us past the end for CheckErrorCondition()
+			vm.scriptIdx++
+			// Check script ran successfully and pull the script
+			// out of the first stack and execute that.
+			err := vm.CheckErrorCondition(false)
+			if err != nil {
+				return false, err
+			}
+
+			script := vm.savedFirstStack[len(vm.savedFirstStack)-1]
+			pops, err := parseScript(script)
+			if err != nil {
+				return false, err
+			}
+			vm.scripts = append(vm.scripts, pops)
+
+			// Set stack to be the stack from first script minus the
+			// script itself
+			vm.SetStack(vm.savedFirstStack[:len(vm.savedFirstStack)-1])
+		} else {
+			vm.scriptIdx++
+		}
+		// there are zero length scripts in the wild
+		if vm.scriptIdx < len(vm.scripts) && vm.scriptOff >= len(vm.scripts[vm.scriptIdx]) {
+			vm.scriptIdx++
+		}
+		vm.lastCodeSep = 0
+		if vm.scriptIdx >= len(vm.scripts) {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
+// Execute will execute all scripts in the script engine and return either nil
 // for successful validation or an error if one occurred.
 func (vm *Engine) Execute() (err error) {
 	done := false
@@ -189,170 +375,9 @@ func (vm *Engine) Execute() (err error) {
 	return vm.CheckErrorCondition(true)
 }
 
-// CheckErrorCondition returns nil if the running script has ended and was
+// subScript returns the script since the last OP_CODESEPARATOR.
-// successful, leaving a a true boolean on the stack. An error otherwise,
-// including if the script has not finished.
-func (vm *Engine) CheckErrorCondition(finalScript bool) error {
-	// Check we are actually done. if pc is past the end of script array
-	// then we have run out of scripts to run.
-	if vm.scriptIdx < len(vm.scripts) {
-		return ErrStackScriptUnfinished
-	}
-	if finalScript && vm.hasFlag(ScriptVerifyCleanStack) &&
-		vm.dstack.Depth() != 1 {
-
-		return ErrStackCleanStack
-	} else if vm.dstack.Depth() < 1 {
-		return ErrStackEmptyStack
-	}
-
-	v, err := vm.dstack.PopBool()
-	if err != nil {
-		return err
-	}
-	if v == false {
-		// log interesting data.
-		log.Tracef("%v", newLogClosure(func() string {
-			dis0, _ := vm.DisasmScript(0)
-			dis1, _ := vm.DisasmScript(1)
-			return fmt.Sprintf("scripts failed: script0: %s\n"+
-				"script1: %s", dis0, dis1)
-		}))
-		return ErrStackScriptFailed
-	}
-	return nil
-}
-
-// Step will execute the next instruction and move the program counter to the
-// next opcode in the script, or the next script if the curent has ended. Step
-// will return true in the case that the last opcode was successfully executed.
-// if an error is returned then the result of calling Step or any other method
-// is undefined.
-func (vm *Engine) Step() (done bool, err error) {
-	// verify that it is pointing to a valid script address
-	err = vm.validPC()
-	if err != nil {
-		return true, err
-	}
-	opcode := &vm.scripts[vm.scriptIdx][vm.scriptOff]
-
-	err = vm.executeOpcode(opcode)
-	if err != nil {
-		return true, err
-	}
-
-	if vm.dstack.Depth()+vm.astack.Depth() > maxStackSize {
-		return false, ErrStackOverflow
-	}
-
-	// prepare for next instruction
-	vm.scriptOff++
-	if vm.scriptOff >= len(vm.scripts[vm.scriptIdx]) {
-		// Illegal to have an `if' that straddles two scripts.
-		if err == nil && len(vm.condStack) != 0 {
-			return false, ErrStackMissingEndif
-		}
-
-		// alt stack doesn't persist.
-		_ = vm.astack.DropN(vm.astack.Depth())
-
-		vm.numOps = 0 // number of ops is per script.
-		vm.scriptOff = 0
-		if vm.scriptIdx == 0 && vm.bip16 {
-			vm.scriptIdx++
-			vm.savedFirstStack = vm.GetStack()
-		} else if vm.scriptIdx == 1 && vm.bip16 {
-			// Put us past the end for CheckErrorCondition()
-			vm.scriptIdx++
-			// We check script ran ok, if so then we pull
-			// the script out of the first stack and executre that.
-			err := vm.CheckErrorCondition(false)
-			if err != nil {
-				return false, err
-			}
-
-			script := vm.savedFirstStack[len(vm.savedFirstStack)-1]
-			pops, err := parseScript(script)
-			if err != nil {
-				return false, err
-			}
-			vm.scripts = append(vm.scripts, pops)
-			// Set stack to be the stack from first script
-			// minus the script itself
-			vm.SetStack(vm.savedFirstStack[:len(vm.savedFirstStack)-1])
-		} else {
-			vm.scriptIdx++
-		}
-		// there are zero length scripts in the wild
-		if vm.scriptIdx < len(vm.scripts) && vm.scriptOff >= len(vm.scripts[vm.scriptIdx]) {
-			vm.scriptIdx++
-		}
-		vm.lastcodesep = 0
-		if vm.scriptIdx >= len(vm.scripts) {
-			return true, nil
-		}
-	}
-	return false, nil
-}
-
-// curPC returns either the current script and offset, or an error if the
-// position isn't valid.
-func (vm *Engine) curPC() (script int, off int, err error) {
-	err = vm.validPC()
-	if err != nil {
-		return 0, 0, err
-	}
-	return vm.scriptIdx, vm.scriptOff, nil
-}
-
-// validPC returns an error if the current script position is valid for
-// execution, nil otherwise.
-func (vm *Engine) validPC() error {
-	if vm.scriptIdx >= len(vm.scripts) {
-		return fmt.Errorf("Past input scripts %v:%v %v:xxxx", vm.scriptIdx, vm.scriptOff, len(vm.scripts))
-	}
-	if vm.scriptOff >= len(vm.scripts[vm.scriptIdx]) {
-		return fmt.Errorf("Past input scripts %v:%v %v:%04d", vm.scriptIdx, vm.scriptOff, vm.scriptIdx, len(vm.scripts[vm.scriptIdx]))
-	}
-	return nil
-}
-
-// DisasmScript returns the disassembly string for the script at offset
-// ``idx''.  Where 0 is the scriptSig and 1 is the scriptPubKey.
-func (vm *Engine) DisasmScript(idx int) (string, error) {
-	if idx >= len(vm.scripts) {
-		return "", ErrStackInvalidIndex
-	}
-
-	var disstr string
-	for i := range vm.scripts[idx] {
-		disstr = disstr + vm.disasm(idx, i) + "\n"
-	}
-	return disstr, nil
-}
-
-// DisasmPC returns the string for the disassembly of the opcode that will be
-// next to execute when Step() is called.
-func (vm *Engine) DisasmPC() (string, error) {
-	scriptIdx, scriptOff, err := vm.curPC()
-	if err != nil {
-		return "", err
-	}
-	return vm.disasm(scriptIdx, scriptOff), nil
-}
-
-// disasm is a helper member to produce the output for DisasmPC and
-// DisasmScript. It produces the opcode prefixed by the program counter at the
-// provided position in the script. it does no error checking and leaves that
-// to the caller to provide a valid offse.
-func (vm *Engine) disasm(scriptIdx int, scriptOff int) string {
-	return fmt.Sprintf("%02x:%04x: %s", scriptIdx, scriptOff,
-		vm.scripts[scriptIdx][scriptOff].print(false))
-}
-
-// subScript will return the script since the last OP_CODESEPARATOR
 func (vm *Engine) subScript() []parsedOpcode {
-	return vm.scripts[vm.scriptIdx][vm.lastcodesep:]
+	return vm.scripts[vm.scriptIdx][vm.lastCodeSep:]
 }
 
 // checkHashTypeEncoding returns whether or not the passed hashtype adheres to
@@ -397,11 +422,37 @@ func (vm *Engine) checkSignatureEncoding(sig []byte) error {
 		return nil
 	}
 
+	// The format of a DER encoded signature is as follows:
+	//
+	// 0x30 <total length> 0x02 <length of R> <R> 0x02 <length of S> <S>
+	//   - 0x30 is the ASN.1 identifier for a sequence
+	//   - Total length is 1 byte and specifies length of all remaining data
+	//   - 0x02 is the ASN.1 identifier that specifies an integer follows
+	//   - Length of R is 1 byte and specifies how many bytes R occupies
+	//   - R is the arbitrary length big-endian encoded number which
+	//     represents the R value of the signature.  DER encoding dictates
+	//     that the value must be encoded using the minimum possible number
+	//     of bytes.  This implies the first byte can only be null if the
+	//     highest bit of the next byte is set in order to prevent it from
+	//     being interpreted as a negative number.
+	//   - 0x02 is once again the ASN.1 integer identifier
+	//   - Length of S is 1 byte and specifies how many bytes S occupies
+	//   - S is the arbitrary length big-endian encoded number which
+	//     represents the S value of the signature.  The encoding rules are
+	//     identical as those for R.
+
+	// Minimum length is when both numbers are 1 byte each.
+	// 0x30 + <1-byte> + 0x02 + 0x01 + <byte> + 0x2 + 0x01 + <byte>
 	if len(sig) < 8 {
 		// Too short
 		return fmt.Errorf("malformed signature: too short: %d < 8",
 			len(sig))
 	}
+
+	// Maximum length is when both numbers are 33 bytes each.  It is 33
+	// bytes because a 256-bit integer requires 32 bytes and an additional
+	// leading null byte might required if the high bit is set in the value.
+	// 0x30 + <1-byte> + 0x02 + 0x21 + <33 bytes> + 0x2 + 0x21 + <33 bytes>
 	if len(sig) > 72 {
 		// Too long
 		return fmt.Errorf("malformed signature: too long: %d > 72",
@@ -420,30 +471,30 @@ func (vm *Engine) checkSignatureEncoding(sig []byte) error {
 
 	rLen := int(sig[3])
 
-	// Make sure S is inside the signature
+	// Make sure S is inside the signature.
 	if rLen+5 > len(sig) {
 		return fmt.Errorf("malformed signature: S out of bounds")
 	}
 
 	sLen := int(sig[rLen+5])
 
-	// The length of the elements does not match
+	// The length of the elements does not match the length of the
-	// the length of the signature
+	// signature.
 	if rLen+sLen+6 != len(sig) {
 		return fmt.Errorf("malformed signature: invalid R length")
 	}
 
-	// R elements must be integers
+	// R elements must be integers.
 	if sig[2] != 0x02 {
 		return fmt.Errorf("malformed signature: missing first integer marker")
 	}
 
-	// Zero-length integers are not allowed for R
+	// Zero-length integers are not allowed for R.
 	if rLen == 0 {
 		return fmt.Errorf("malformed signature: R length is zero")
 	}
 
-	// R must not be negative
+	// R must not be negative.
 	if sig[4]&0x80 != 0 {
 		return fmt.Errorf("malformed signature: R value is negative")
 	}
@@ -454,17 +505,17 @@ func (vm *Engine) checkSignatureEncoding(sig []byte) error {
 		return fmt.Errorf("malformed signature: invalid R value")
 	}
 
-	// S elements must be integers
+	// S elements must be integers.
 	if sig[rLen+4] != 0x02 {
 		return fmt.Errorf("malformed signature: missing second integer marker")
 	}
 
-	// Zero-length integers are not allowed for S
+	// Zero-length integers are not allowed for S.
 	if sLen == 0 {
 		return fmt.Errorf("malformed signature: S length is zero")
 	}
 
-	// S must not be negative
+	// S must not be negative.
 	if sig[rLen+6]&0x80 != 0 {
 		return fmt.Errorf("malformed signature: S value is negative")
 	}
@@ -475,7 +526,13 @@ func (vm *Engine) checkSignatureEncoding(sig []byte) error {
 		return fmt.Errorf("malformed signature: invalid S value")
 	}
 
-	// Verify the S value is <= halforder.
+	// Verify the S value is <= half the order of the curve.  This check is
+	// done because when it is higher, the complement modulo the order can
+	// be used instead which is a shorter encoding by 1 byte.  Further,
+	// without enforcing this, it is possible to replace a signature in a
+	// valid transaction with the complement while still being a valid
+	// signature that verifies.  This would result in changing the
+	// transaction hash and thus is source of malleability.
 	if vm.hasFlag(ScriptVerifyLowS) {
 		sValue := new(big.Int).SetBytes(sig[rLen+6 : rLen+6+sLen])
 		if sValue.Cmp(halfOrder) > 0 {
@@ -535,16 +592,35 @@ func (vm *Engine) SetAltStack(data [][]byte) {
 // transaction, and input index.  The flags modify the behavior of the script
 // engine according to the description provided by each flag.
 func NewEngine(scriptPubKey []byte, tx *wire.MsgTx, txIdx int, flags ScriptFlags) (*Engine, error) {
+	// The provided transaction input index must refer to a valid input.
 	if txIdx < 0 || txIdx >= len(tx.TxIn) {
 		return nil, ErrInvalidIndex
 	}
 	scriptSig := tx.TxIn[txIdx].SignatureScript
 
+	// The clean stack flag (ScriptVerifyCleanStack) is not allowed without
+	// the pay-to-script-hash (P2SH) evaluation (ScriptBip16) flag.
+	//
+	// Recall that evaluating a P2SH script without the flag set results in
+	// non-P2SH evaluation which leaves the P2SH inputs on the stack.  Thus,
+	// allowing the clean stack flag without the P2SH flag would make it
+	// possible to have a situation where P2SH would not be a soft fork when
+	// it should be.
 	vm := Engine{flags: flags}
+	if vm.hasFlag(ScriptVerifyCleanStack) && !vm.hasFlag(ScriptBip16) {
+		return nil, ErrInvalidFlags
+	}
+
+	// The signature script must only contain data pushes when the
+	// associated flag is set.
 	if vm.hasFlag(ScriptVerifySigPushOnly) && !IsPushOnlyScript(scriptSig) {
 		return nil, ErrStackNonPushOnly
 	}
 
+	// The engine stores the scripts in parsed form using a slice.  This
+	// allows multiple scripts to be executed in sequence.  For example,
+	// with a pay-to-script-hash transaction, there will be ultimately be
+	// a third script to execute.
 	scripts := [][]byte{scriptSig, scriptPubKey}
 	vm.scripts = make([][]parsedOpcode, len(scripts))
 	for i, scr := range scripts {
@@ -556,20 +632,17 @@ func NewEngine(scriptPubKey []byte, tx *wire.MsgTx, txIdx int, flags ScriptFlags
 		if err != nil {
 			return nil, err
 		}
-
-		// If the first scripts(s) are empty, must start on later ones.
-		if i == 0 && len(scr) == 0 {
-			// This could end up seeing an invalid initial pc if
-			// all scripts were empty. However, that is an invalid
-			// case and should fail.
-			vm.scriptIdx = i + 1
-		}
 	}
 
-	// Parse flags.
+	// Advance the program counter to the public key script if the signature
+	// script is empty since there is nothing to execute for it in that
+	// case.
+	if len(scripts[0]) == 0 {
+		vm.scriptIdx++
+	}
+
 	if vm.hasFlag(ScriptBip16) && isScriptHash(vm.scripts[1]) {
-		// if we are pay to scripthash then we only accept input
+		// Only accept input scripts that push data for P2SH.
-		// scripts that push data
 		if !isPushOnly(vm.scripts[0]) {
 			return nil, ErrStackP2SHNonPushOnly
 		}
@@ -579,9 +652,6 @@ func NewEngine(scriptPubKey []byte, tx *wire.MsgTx, txIdx int, flags ScriptFlags
 		vm.dstack.verifyMinimalData = true
 		vm.astack.verifyMinimalData = true
 	}
-	if vm.hasFlag(ScriptVerifyCleanStack) && !vm.hasFlag(ScriptBip16) {
-		return nil, ErrInvalidFlags
-	}
 
 	vm.tx = *tx
 	vm.txIdx = txIdx

txscript/script.go

@@ -8,10 +8,8 @@ import (
 	"bytes"
 	"encoding/binary"
 	"fmt"
-	"math/big"
 	"time"
 
-	"github.com/btcsuite/btcd/btcec"
 	"github.com/btcsuite/btcd/wire"
 	"github.com/btcsuite/btcutil"
 )
@@ -20,13 +18,6 @@ const (
 	// maxDataCarrierSize is the maximum number of bytes allowed in pushed
 	// data to be considered a nulldata transaction
 	maxDataCarrierSize = 80
-
-	// maxStackSize is the maximum combined height of stack and alt stack
-	// during execution.
-	maxStackSize = 1000
-
-	// maxScriptSize is the maximum allowed length of a raw script.
-	maxScriptSize = 10000
 )
 
 // Bip16Activation is the timestamp where BIP0016 is valid to use in the
@@ -34,9 +25,6 @@ const (
 // This timestamp corresponds to Sun Apr 1 00:00:00 UTC 2012.
 var Bip16Activation = time.Unix(1333238400, 0)
 
-// curve halforder, used to tame ECDSA malleability (see BIP0062)
-var halfOrder = new(big.Int).Rsh(btcec.S256().N, 1)
-
 // SigHashType represents hash type bits at the end of a signature.
 type SigHashType byte
 
@@ -47,6 +35,10 @@ const (
 	SigHashNone         SigHashType = 0x2
 	SigHashSingle       SigHashType = 0x3
 	SigHashAnyOneCanPay SigHashType = 0x80
+
+	// sigHashMask defines the number of bits of the hash type which is used
+	// to identify which outputs are signed.
+	sigHashMask = 0x1f
 )
 
 // These are the constants specified for maximums in individual scripts.
@@ -69,6 +61,8 @@ const (
 	NullDataTy                       // Empty data-only (provably prunable).
 )
 
+// scriptClassToName houses the human-readable strings which describe each
+// script class.
 var scriptClassToName = []string{
 	NonStandardTy: "nonstandard",
 	PubKeyTy:      "pubkey",
@@ -97,17 +91,17 @@ func isSmallInt(op *opcode) bool {
 	return false
 }
 
-// isPubkey returns true if the script passed is a pubkey transaction, false
+// isPubkey returns true if the script passed is a pay-to-pubkey transaction,
-// otherwise.
+// false otherwise.
 func isPubkey(pops []parsedOpcode) bool {
-	// valid pubkeys are either 33 or 65 bytes
+	// Valid pubkeys are either 33 or 65 bytes.
 	return len(pops) == 2 &&
 		(len(pops[0].data) == 33 || len(pops[0].data) == 65) &&
 		pops[1].opcode.value == OP_CHECKSIG
 }
 
-// isPubkeyHash returns true if the script passed is a pubkey hash transaction,
+// isPubkeyHash returns true if the script passed is a pay-to-pubkey-hash
-// false otherwise.
+// transaction, false otherwise.
 func isPubkeyHash(pops []parsedOpcode) bool {
 	return len(pops) == 5 &&
 		pops[0].opcode.value == OP_DUP &&
@@ -118,8 +112,8 @@ func isPubkeyHash(pops []parsedOpcode) bool {
 
 }
 
-// isScriptHash returns true if the script passed is a pay-to-script-hash (P2SH)
+// isScriptHash returns true if the script passed is a pay-to-script-hash
-// transction, false otherwise.
+// transaction, false otherwise.
 func isScriptHash(pops []parsedOpcode) bool {
 	return len(pops) == 3 &&
 		pops[0].opcode.value == OP_HASH160 &&
@@ -128,7 +122,7 @@ func isScriptHash(pops []parsedOpcode) bool {
 }
 
 // IsPayToScriptHash returns true if the script is in the standard
-// Pay-To-Script-Hash format, false otherwise.
+// pay-to-script-hash (P2SH) format, false otherwise.
 func IsPayToScriptHash(script []byte) bool {
 	pops, err := parseScript(script)
 	if err != nil {
@@ -140,9 +134,9 @@ func IsPayToScriptHash(script []byte) bool {
 // isMultiSig returns true if the passed script is a multisig transaction, false
 // otherwise.
 func isMultiSig(pops []parsedOpcode) bool {
+	// The absolute minimum is 1 pubkey:
+	// OP_0/OP_1-16 <pubkey> OP_1 OP_CHECKMULTISIG
 	l := len(pops)
-	// absolute minimum is 1 pubkey so
-	// OP_0/OP_1-16, pubkey, OP_1, OP_CHECKMULTISIG
 	if l < 4 {
 		return false
 	}
@@ -156,9 +150,8 @@ func isMultiSig(pops []parsedOpcode) bool {
 		return false
 	}
 	for _, pop := range pops[1 : l-2] {
-		// valid pubkeys are either 65 or 33 bytes
+		// Valid pubkeys are either 33 or 65 bytes.
-		if len(pop.data) != 33 &&
+		if len(pop.data) != 33 && len(pop.data) != 65 {
-			len(pop.data) != 65 {
 			return false
 		}
 	}
@@ -169,7 +162,7 @@ func isMultiSig(pops []parsedOpcode) bool {
 // false otherwise.
 func isNullData(pops []parsedOpcode) bool {
 	// A nulldata transaction is either a single OP_RETURN or an
-	// OP_RETURN SMALLDATA (where SMALLDATA is a push data up to
+	// OP_RETURN SMALLDATA (where SMALLDATA is a data push up to
 	// maxDataCarrierSize bytes).
 	l := len(pops)
 	if l == 1 && pops[0].opcode.value == OP_RETURN {
@@ -184,11 +177,16 @@ func isNullData(pops []parsedOpcode) bool {
 
 // isPushOnly returns true if the script only pushes data, false otherwise.
 func isPushOnly(pops []parsedOpcode) bool {
-	// technically we cheat here, we don't look at opcodes
+	// NOTE: This function does NOT verify opcodes directly since it is
+	// internal and is only called with parsed opcodes for scripts that did
+	// not have any parse errors.  Thus, consensus is properly maintained.
+
 	for _, pop := range pops {
-		// all opcodes up to OP_16 are data instructions.
+		// All opcodes up to OP_16 are data push instructions.
-		if pop.opcode.value < OP_FALSE ||
+		// NOTE: This does consider OP_RESERVED to be a data push
-			pop.opcode.value > OP_16 {
+		// instruction, but execution of OP_RESERVED will fail anyways
+		// and matches the behavior required by consensus.
+		if pop.opcode.value > OP_16 {
 			return false
 		}
 	}
@@ -196,7 +194,8 @@ func isPushOnly(pops []parsedOpcode) bool {
 }
 
 // IsPushOnlyScript returns whether or not the passed script only pushes data.
-// If the script does not parse false will be returned.
+//
+// False will be returned when the script does not parse.
 func IsPushOnlyScript(script []byte) bool {
 	pops, err := parseScript(script)
 	if err != nil {
@@ -205,6 +204,160 @@ func IsPushOnlyScript(script []byte) bool {
 	return isPushOnly(pops)
 }
 
+// scriptType returns the type of the script being inspected from the known
+// standard types.
+func typeOfScript(pops []parsedOpcode) ScriptClass {
+	if isPubkey(pops) {
+		return PubKeyTy
+	} else if isPubkeyHash(pops) {
+		return PubKeyHashTy
+	} else if isScriptHash(pops) {
+		return ScriptHashTy
+	} else if isMultiSig(pops) {
+		return MultiSigTy
+	} else if isNullData(pops) {
+		return NullDataTy
+	}
+	return NonStandardTy
+}
+
+// GetScriptClass returns the class of the script passed.
+//
+// NonStandardTy will be returned when the script does not parse.
+func GetScriptClass(script []byte) ScriptClass {
+	pops, err := parseScript(script)
+	if err != nil {
+		return NonStandardTy
+	}
+	return typeOfScript(pops)
+}
+
+// parseScriptTemplate is the same as parseScript but allows the passing of the
+// template list for testing purposes.  When there are parse errors, it returns
+// the list of parsed opcodes up to the point of failure along with the error.
+func parseScriptTemplate(script []byte, opcodes *[256]opcode) ([]parsedOpcode, error) {
+	retScript := make([]parsedOpcode, 0, len(script))
+	for i := 0; i < len(script); {
+		instr := script[i]
+		op := opcodes[instr]
+		pop := parsedOpcode{opcode: &op}
+
+		// Parse data out of instruction.
+		switch {
+		// No additional data.  Note that some of the opcodes, notably
+		// OP_1NEGATE, OP_0, and OP_[1-16] represent the data
+		// themselves.
+		case op.length == 1:
+			i++
+
+		// Data pushes of specific lengths -- OP_DATA_[1-75].
+		case op.length > 1:
+			if len(script[i:]) < op.length {
+				return retScript, ErrStackShortScript
+			}
+
+			// Slice out the data.
+			pop.data = script[i+1 : i+op.length]
+			i += op.length
+
+		// Data pushes with parsed lengths -- OP_PUSHDATAP{1,2,4}.
+		case op.length < 0:
+			var l uint
+			off := i + 1
+
+			if len(script[off:]) < -op.length {
+				return retScript, ErrStackShortScript
+			}
+
+			// Next -length bytes are little endian length of data.
+			switch op.length {
+			case -1:
+				l = uint(script[off])
+			case -2:
+				l = ((uint(script[off+1]) << 8) |
+					uint(script[off]))
+			case -4:
+				l = ((uint(script[off+3]) << 24) |
+					(uint(script[off+2]) << 16) |
+					(uint(script[off+1]) << 8) |
+					uint(script[off]))
+			default:
+				return retScript,
+					fmt.Errorf("invalid opcode length %d",
+						op.length)
+			}
+
+			// Move offset to beginning of the data.
+			off += -op.length
+
+			// Disallow entries that do not fit script or were
+			// sign extended.
+			if int(l) > len(script[off:]) || int(l) < 0 {
+				return retScript, ErrStackShortScript
+			}
+
+			pop.data = script[off : off+int(l)]
+			i += 1 - op.length + int(l)
+		}
+
+		retScript = append(retScript, pop)
+	}
+
+	return retScript, nil
+}
+
+// parseScript preparses the script in bytes into a list of parsedOpcodes while
+// applying a number of sanity checks.
+func parseScript(script []byte) ([]parsedOpcode, error) {
+	return parseScriptTemplate(script, &opcodeArray)
+}
+
+// unparseScript reversed the action of parseScript and returns the
+// parsedOpcodes as a list of bytes
+func unparseScript(pops []parsedOpcode) ([]byte, error) {
+	script := make([]byte, 0, len(pops))
+	for _, pop := range pops {
+		b, err := pop.bytes()
+		if err != nil {
+			return nil, err
+		}
+		script = append(script, b...)
+	}
+	return script, nil
+}
+
+// DisasmString formats a disassembled script for one line printing.  When the
+// script fails to parse, the returned string will contain the disassembled
+// script up to the point the failure occurred along with the string '[error]'
+// appended.  In addition, the reason the script failed to parse is returned
+// if the caller wants more information about the failure.
+func DisasmString(buf []byte) (string, error) {
+	disbuf := ""
+	opcodes, err := parseScript(buf)
+	for _, pop := range opcodes {
+		disbuf += pop.print(true) + " "
+	}
+	if disbuf != "" {
+		disbuf = disbuf[:len(disbuf)-1]
+	}
+	if err != nil {
+		disbuf += "[error]"
+	}
+	return disbuf, err
+}
+
+// removeOpcode will remove any opcode matching ``opcode'' from the opcode
+// stream in pkscript
+func removeOpcode(pkscript []parsedOpcode, opcode byte) []parsedOpcode {
+	retScript := make([]parsedOpcode, 0, len(pkscript))
+	for _, pop := range pkscript {
+		if pop.opcode.value != opcode {
+			retScript = append(retScript, pop)
+		}
+	}
+	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.
@@ -231,131 +384,8 @@ func canonicalPush(pop parsedOpcode) bool {
 	return true
 }
 
-// GetScriptClass returns the class of the script passed. If the script does not
+// removeOpcodeByData will return the script minus any opcodes that would push
-// parse then NonStandardTy will be returned.
+// the passed data to the stack.
-func GetScriptClass(script []byte) ScriptClass {
-	pops, err := parseScript(script)
-	if err != nil {
-		return NonStandardTy
-	}
-	return typeOfScript(pops)
-}
-
-// scriptType returns the type of the script being inspected from the known
-// standard types.
-func typeOfScript(pops []parsedOpcode) ScriptClass {
-	// XXX dubious optimisation: order these in order of popularity in the
-	// blockchain
-	if isPubkey(pops) {
-		return PubKeyTy
-	} else if isPubkeyHash(pops) {
-		return PubKeyHashTy
-	} else if isScriptHash(pops) {
-		return ScriptHashTy
-	} else if isMultiSig(pops) {
-		return MultiSigTy
-	} else if isNullData(pops) {
-		return NullDataTy
-	}
-	return NonStandardTy
-
-}
-
-// parseScript preparses the script in bytes into a list of parsedOpcodes while
-// applying a number of sanity checks.
-func parseScript(script []byte) ([]parsedOpcode, error) {
-	return parseScriptTemplate(script, &opcodeArray)
-}
-
-// parseScriptTemplate is the same as parseScript but allows the passing of the
-// template list for testing purposes. On error we return the list of parsed
-// opcodes so far.
-func parseScriptTemplate(script []byte, opcodes *[256]opcode) ([]parsedOpcode, error) {
-	retScript := make([]parsedOpcode, 0, len(script))
-	for i := 0; i < len(script); {
-		instr := script[i]
-		op := opcodes[instr]
-		pop := parsedOpcode{opcode: &op}
-		// parse data out of instruction.
-		switch {
-		case op.length == 1:
-			// no data, done here
-			i++
-		case op.length > 1:
-			if len(script[i:]) < op.length {
-				return retScript, ErrStackShortScript
-			}
-			// slice out the data.
-			pop.data = script[i+1 : i+op.length]
-			i += op.length
-		case op.length < 0:
-			var l uint
-			off := i + 1
-
-			if len(script[off:]) < -op.length {
-				return retScript, ErrStackShortScript
-			}
-
-			// Next -length bytes are little endian length of data.
-			switch op.length {
-			case -1:
-				l = uint(script[off])
-			case -2:
-				l = ((uint(script[off+1]) << 8) |
-					uint(script[off]))
-			case -4:
-				l = ((uint(script[off+3]) << 24) |
-					(uint(script[off+2]) << 16) |
-					(uint(script[off+1]) << 8) |
-					uint(script[off]))
-			default:
-				return retScript,
-					fmt.Errorf("invalid opcode length %d",
-						op.length)
-			}
-
-			off += -op.length // beginning of data
-			// Disallow entries that do not fit script or were
-			// sign extended.
-			if int(l) > len(script[off:]) || int(l) < 0 {
-				return retScript, ErrStackShortScript
-			}
-			pop.data = script[off : off+int(l)]
-			i += 1 - op.length + int(l)
-		}
-		retScript = append(retScript, pop)
-	}
-	return retScript, nil
-}
-
-// unparseScript reversed the action of parseScript and returns the
-// parsedOpcodes as a list of bytes
-func unparseScript(pops []parsedOpcode) ([]byte, error) {
-	script := make([]byte, 0, len(pops))
-	for _, pop := range pops {
-		b, err := pop.bytes()
-		if err != nil {
-			return nil, err
-		}
-		script = append(script, b...)
-	}
-	return script, nil
-}
-
-// removeOpcode will remove any opcode matching ``opcode'' from the opcode
-// stream in pkscript
-func removeOpcode(pkscript []parsedOpcode, opcode byte) []parsedOpcode {
-	retScript := make([]parsedOpcode, 0, len(pkscript))
-	for _, pop := range pkscript {
-		if pop.opcode.value != opcode {
-			retScript = append(retScript, pop)
-		}
-	}
-	return retScript
-}
-
-// removeOpcodeByData will return the pkscript minus any opcodes that would
-// push the data in ``data'' to the stack.
 func removeOpcodeByData(pkscript []parsedOpcode, data []byte) []parsedOpcode {
 	retScript := make([]parsedOpcode, 0, len(pkscript))
 	for _, pop := range pkscript {
@@ -367,157 +397,120 @@ func removeOpcodeByData(pkscript []parsedOpcode, data []byte) []parsedOpcode {
 
 }
 
-// DisasmString formats a disassembled script for one line printing.  When the
+// calcSignatureHash will, given a script and hash type for the current script
-// script fails to parse, the returned string will contain the disassembled
+// engine instance, calculate the signature hash to be used for signing and
-// script up to the point the failure occurred along with the string '[error]'
+// verification.
-// appended.  In addition, the reason the script failed to parse is returned
+func calcSignatureHash(script []parsedOpcode, hashType SigHashType, tx *wire.MsgTx, idx int) []byte {
-// if the caller wants more information about the failure.
+	// The SigHashSingle signature type signs only the corresponding input
-func DisasmString(buf []byte) (string, error) {
+	// and output (the output with the same index number as the input).
-	disbuf := ""
+	//
-	opcodes, err := parseScript(buf)
+	// Since transactions can have more inputs than outputs, this means it
-	for _, pop := range opcodes {
+	// is improper to use SigHashSingle on input indices that don't have a
-		disbuf += pop.print(true) + " "
+	// corresponding output.
+	//
+	// A bug in the original Satoshi client implementation means specifying
+	// an index that is out of range results in a signature hash of 1 (as a
+	// uint256 little endian).  The original intent appeared to be to
+	// indicate failure, but unfortunately, it was never checked and thus is
+	// treated as the actual signature hash.  This buggy behavior is now
+	// part of the consensus and a hard fork would be required to fix it.
+	//
+	// Due to this, care must be taken by software that creates transactions
+	// which make use of SigHashSingle because it can lead to an extremely
+	// dangerous situation where the invalid inputs will end up signing a
+	// hash of 1.  This in turn presents an opportunity for attackers to
+	// cleverly construct transactions which can steal those coins provided
+	// they can reuse signatures.
+	if hashType&sigHashMask == SigHashSingle && idx >= len(tx.TxOut) {
+		var hash wire.ShaHash
+		hash[0] = 0x01
+		return hash[:]
 	}
-	if disbuf != "" {
-		disbuf = disbuf[:len(disbuf)-1]
-	}
-	if err != nil {
-		disbuf += "[error]"
-	}
-	return disbuf, err
-}
 
-// calcScriptHash will, given the a script and hashtype for the current
+	// Remove all instances of OP_CODESEPARATOR from the script.
-// scriptmachine, calculate the doubleSha256 hash of the transaction and
-// script to be used for signature signing and verification.
-func calcScriptHash(script []parsedOpcode, hashType SigHashType, tx *wire.MsgTx, idx int) []byte {
-
-	// remove all instances of OP_CODESEPARATOR still left in the script
 	script = removeOpcode(script, OP_CODESEPARATOR)
 
-	// Make a deep copy of the transaction, zeroing out the script
+	// Make a deep copy of the transaction, zeroing out the script for all
-	// for all inputs that are not currently being processed.
+	// inputs that are not currently being processed.
 	txCopy := tx.Copy()
 	for i := range txCopy.TxIn {
 		var txIn wire.TxIn
 		txIn = *txCopy.TxIn[i]
 		txCopy.TxIn[i] = &txIn
 		if i == idx {
-			// unparseScript cannot fail here, because removeOpcode
+			// UnparseScript cannot fail here because removeOpcode
 			// above only returns a valid script.
-			sigscript, _ := unparseScript(script)
+			sigScript, _ := unparseScript(script)
-			txCopy.TxIn[idx].SignatureScript = sigscript
+			txCopy.TxIn[idx].SignatureScript = sigScript
 		} else {
-			txCopy.TxIn[i].SignatureScript = []byte{}
+			txCopy.TxIn[i].SignatureScript = nil
 		}
 	}
-	// Default behaviour has all outputs set up.
+
+	// Default behavior has all outputs set up.
 	for i := range txCopy.TxOut {
 		var txOut wire.TxOut
 		txOut = *txCopy.TxOut[i]
 		txCopy.TxOut[i] = &txOut
 	}
 
-	switch hashType & 31 {
+	switch hashType & sigHashMask {
 	case SigHashNone:
-		txCopy.TxOut = txCopy.TxOut[0:0] // empty slice
+		txCopy.TxOut = txCopy.TxOut[0:0] // Empty slice.
 		for i := range txCopy.TxIn {
 			if i != idx {
 				txCopy.TxIn[i].Sequence = 0
 			}
 		}
+
 	case SigHashSingle:
-		if idx >= len(txCopy.TxOut) {
-			// This was created by a buggy implementation.
-			// In this case we do the same as bitcoind and bitcoinj
-			// and return 1 (as a uint256 little endian) as an
-			// error. Unfortunately this was not checked anywhere
-			// and thus is treated as the actual
-			// hash.
-			hash := make([]byte, 32)
-			hash[0] = 0x01
-			return hash
-		}
 		// Resize output array to up to and including requested index.
 		txCopy.TxOut = txCopy.TxOut[:idx+1]
-		// all but  current output get zeroed out
+
+		// All but current output get zeroed out.
 		for i := 0; i < idx; i++ {
 			txCopy.TxOut[i].Value = -1
-			txCopy.TxOut[i].PkScript = []byte{}
+			txCopy.TxOut[i].PkScript = nil
 		}
+
 		// Sequence on all other inputs is 0, too.
 		for i := range txCopy.TxIn {
 			if i != idx {
 				txCopy.TxIn[i].Sequence = 0
 			}
 		}
+
 	default:
-		// XXX bitcoind treats undefined hashtypes like normal
+		// Consensus treats undefined hashtypes like normal SigHashAll
-		// SigHashAll for purposes of hash generation.
+		// for purposes of hash generation.
 		fallthrough
 	case SigHashOld:
 		fallthrough
 	case SigHashAll:
-		// nothing special here
+		// Nothing special here.
 	}
 	if hashType&SigHashAnyOneCanPay != 0 {
 		txCopy.TxIn = txCopy.TxIn[idx : idx+1]
 		idx = 0
 	}
 
+	// The final hash is the double sha256 of both the serialized modified
+	// transaction and the hash type (encoded as a 4-byte little-endian
+	// value) appended.
 	var wbuf bytes.Buffer
 	txCopy.Serialize(&wbuf)
-	// Append LE 4 bytes hash type
 	binary.Write(&wbuf, binary.LittleEndian, uint32(hashType))
-
 	return wire.DoubleSha256(wbuf.Bytes())
 }
 
-// GetSigOpCount provides a quick count of the number of signature operations
+// asSmallInt returns the passed opcode, which must be true according to
-// in a script. a CHECKSIG operations counts for 1, and a CHECK_MULTISIG for 20.
+// isSmallInt(), as an integer.
-// If the script fails to parse, then the count up to the point of failure is
+func asSmallInt(op *opcode) int {
-// returned.
+	if op.value == OP_0 {
-func GetSigOpCount(script []byte) int {
-	// We don't check error since parseScript returns the parsed-up-to-error
-	// list of pops.
-	pops, _ := parseScript(script)
-
-	return getSigOpCount(pops, false)
-}
-
-// GetPreciseSigOpCount returns the number of signature operations in
-// scriptPubKey. If bip16 is true then scriptSig may be searched for the
-// Pay-To-Script-Hash script in order to find the precise number of signature
-// operations in the transaction. If the script fails to parse, then the
-// count up to the point of failure is returned.
-func GetPreciseSigOpCount(scriptSig, scriptPubKey []byte, bip16 bool) int {
-	// We don't check error since parseScript returns the parsed-up-to-error
-	// list of pops.
-	pops, _ := parseScript(scriptPubKey)
-	// non P2SH transactions just treated as normal.
-	if !(bip16 && isScriptHash(pops)) {
-		return getSigOpCount(pops, true)
-	}
-
-	// Ok so this is P2SH, get the contained script and count it..
-
-	sigPops, err := parseScript(scriptSig)
-	if err != nil {
-		return 0
-	}
-	if !isPushOnly(sigPops) || len(sigPops) == 0 {
 		return 0
 	}
 
-	shScript := sigPops[len(sigPops)-1].data
+	return int(op.value - (OP_1 - 1))
-	// Means that sigPops is jus OP_1 - OP_16, no sigops there.
-	if shScript == nil {
-		return 0
-	}
-
-	shPops, _ := parseScript(shScript)
-
-	return getSigOpCount(shPops, true)
 }
 
 // getSigOpCount is the implementation function for counting the number of
@@ -542,19 +535,74 @@ func getSigOpCount(pops []parsedOpcode, precise bool) int {
 			if precise && i > 0 &&
 				pops[i-1].opcode.value >= OP_1 &&
 				pops[i-1].opcode.value <= OP_16 {
-				nSigs += int(pops[i-1].opcode.value -
+				nSigs += asSmallInt(pops[i-1].opcode)
-					(OP_1 - 1))
 			} else {
 				nSigs += MaxPubKeysPerMultiSig
 			}
 		default:
-			// not a sigop.
+			// Not a sigop.
 		}
 	}
 
 	return nSigs
 }
 
+// GetSigOpCount provides a quick count of the number of signature operations
+// in a script. a CHECKSIG operations counts for 1, and a CHECK_MULTISIG for 20.
+// If the script fails to parse, then the count up to the point of failure is
+// returned.
+func GetSigOpCount(script []byte) int {
+	// Don't check error since parseScript returns the parsed-up-to-error
+	// list of pops.
+	pops, _ := parseScript(script)
+	return getSigOpCount(pops, false)
+}
+
+// GetPreciseSigOpCount returns the number of signature operations in
+// scriptPubKey.  If bip16 is true then scriptSig may be searched for the
+// Pay-To-Script-Hash script in order to find the precise number of signature
+// operations in the transaction.  If the script fails to parse, then the count
+// up to the point of failure is returned.
+func GetPreciseSigOpCount(scriptSig, scriptPubKey []byte, bip16 bool) int {
+	// Don't check error since parseScript returns the parsed-up-to-error
+	// list of pops.
+	pops, _ := parseScript(scriptPubKey)
+
+	// Treat non P2SH transactions as normal.
+	if !(bip16 && isScriptHash(pops)) {
+		return getSigOpCount(pops, true)
+	}
+
+	// The public key script is a pay-to-script-hash, so parse the signature
+	// script to get the final item.  Scripts that fail to fully parse count
+	// as 0 signature operations.
+	sigPops, err := parseScript(scriptSig)
+	if err != nil {
+		return 0
+	}
+
+	// The signature script must only push data to the stack for P2SH to be
+	// a valid pair, so the signature operation count is 0 when that is not
+	// the case.
+	if !isPushOnly(sigPops) || len(sigPops) == 0 {
+		return 0
+	}
+
+	// The P2SH script is the last item the signature script pushes to the
+	// stack.  When the script is empty, there are no signature operations.
+	shScript := sigPops[len(sigPops)-1].data
+	if len(shScript) == 0 {
+		return 0
+	}
+
+	// Parse the P2SH script and don't check the error since parseScript
+	// returns the parsed-up-to-error list of pops and the consensus rules
+	// dictate signature operations are counted up to the first parse
+	// failure.
+	shPops, _ := parseScript(shScript)
+	return getSigOpCount(shPops, true)
+}
+
 // 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.
@@ -606,8 +654,8 @@ func PayToAddrScript(addr btcutil.Address) ([]byte, error) {
 
 // MultiSigScript returns a valid script for a multisignature redemption where
 // nrequired of the keys in pubkeys are required to have signed the transaction
-// for success. An ErrBadNumRequired will be returned if nrequired is larger than
+// for success.  An ErrBadNumRequired will be returned if nrequired is larger
-// the number of keys provided.
+// than the number of keys provided.
 func MultiSigScript(pubkeys []*btcutil.AddressPubKey, nrequired int) ([]byte, error) {
 	if len(pubkeys) < nrequired {
 		return nil, ErrBadNumRequired
@@ -624,20 +672,22 @@ func MultiSigScript(pubkeys []*btcutil.AddressPubKey, nrequired int) ([]byte, er
 }
 
 // expectedInputs returns the number of arguments required by a script.
-// If the script is of unnown type such that the number can not be determined
+// If the script is of unknown type such that the number can not be determined
 // then -1 is returned. We are an internal function and thus assume that class
-// is the real class of pops (and we can thus assume things that were
+// is the real class of pops (and we can thus assume things that were determined
-// determined while finding out the type).
+// while finding out the type).
 func expectedInputs(pops []parsedOpcode, class ScriptClass) int {
-	// count needed inputs.
 	switch class {
 	case PubKeyTy:
 		return 1
+
 	case PubKeyHashTy:
 		return 2
+
 	case ScriptHashTy:
-		// Not including script, handled below.
+		// Not including script.  That is handled by the caller.
 		return 1
+
 	case MultiSigTy:
 		// Standard multisig has a push a small number for the number
 		// of sigs and number of keys.  Check the first push instruction
@@ -647,6 +697,7 @@ func expectedInputs(pops []parsedOpcode, class ScriptClass) int {
 		// additional item from the stack, add an extra expected input
 		// for the extra push that is required to compensate.
 		return asSmallInt(pops[0].opcode) + 1
+
 	case NullDataTy:
 		fallthrough
 	default:
@@ -657,39 +708,39 @@ func expectedInputs(pops []parsedOpcode, class ScriptClass) int {
 // ScriptInfo houses information about a script pair that is determined by
 // CalcScriptInfo.
 type ScriptInfo struct {
-	// The class of the sigscript, equivalent to calling GetScriptClass
+	// PkScriptClass is the class of the public key script and is equivalent
-	// on the sigScript.
+	// to calling GetScriptClass on it.
 	PkScriptClass ScriptClass
 
-	// NumInputs is the number of inputs provided by the pkScript.
+	// NumInputs is the number of inputs provided by the public key script.
 	NumInputs int
 
-	// ExpectedInputs is the number of outputs required by sigScript and any
+	// ExpectedInputs is the number of outputs required by the signature
-	// pay-to-script-hash scripts. The number will be -1 if unknown.
+	// script and any pay-to-script-hash scripts. The number will be -1 if
+	// unknown.
 	ExpectedInputs int
 
-	// SigOps is the nubmer of signature operations in the script pair.
+	// SigOps is the number of signature operations in the script pair.
 	SigOps int
 }
 
-// CalcScriptInfo returns a structure providing data about the scriptpair that
+// CalcScriptInfo returns a structure providing data about the provided script
-// are provided as arguments. It will error if the pair is in someway invalid
+// pair.  It will error if the pair is in someway invalid such that they can not
-// such that they can not be analysed, i.e. if they do not parse or the
+// be analysed, i.e. if they do not parse or the pkScript is not a push-only
-// pkScript is not a push-only script
+// script
-func CalcScriptInfo(sigscript, pkscript []byte, bip16 bool) (*ScriptInfo, error) {
+func CalcScriptInfo(sigScript, pkScript []byte, bip16 bool) (*ScriptInfo, error) {
+	sigPops, err := parseScript(sigScript)
+	if err != nil {
+		return nil, err
+	}
+
+	pkPops, err := parseScript(pkScript)
+	if err != nil {
+		return nil, err
+	}
+
+	// Push only sigScript makes little sense.
 	si := new(ScriptInfo)
-	// parse both scripts.
-	sigPops, err := parseScript(sigscript)
-	if err != nil {
-		return nil, err
-	}
-
-	pkPops, err := parseScript(pkscript)
-	if err != nil {
-		return nil, err
-	}
-
-	// push only sigScript makes little sense.
 	si.PkScriptClass = typeOfScript(pkPops)
 
 	// Can't have a pkScript that doesn't just push data.
@@ -698,22 +749,21 @@ func CalcScriptInfo(sigscript, pkscript []byte, bip16 bool) (*ScriptInfo, error)
 	}
 
 	si.ExpectedInputs = expectedInputs(pkPops, si.PkScriptClass)
-	// all entries push to stack (or are OP_RESERVED and exec will fail).
+
+	// All entries pushed to stack (or are OP_RESERVED and exec will fail).
 	si.NumInputs = len(sigPops)
 
+	// Count sigops taking into account pay-to-script-hash.
 	if si.PkScriptClass == ScriptHashTy && bip16 {
-		// grab the last push instruction in the script and pull out the
+		// The pay-to-hash-script is the final data push of the
-		// data.
+		// signature script.
 		script := sigPops[len(sigPops)-1].data
-		// check for existance and error else.
 		shPops, err := parseScript(script)
 		if err != nil {
 			return nil, err
 		}
 
-		shClass := typeOfScript(shPops)
+		shInputs := expectedInputs(shPops, typeOfScript(shPops))
-
-		shInputs := expectedInputs(shPops, shClass)
 		if shInputs == -1 {
 			si.ExpectedInputs = -1
 		} else {
@@ -727,16 +777,6 @@ func CalcScriptInfo(sigscript, pkscript []byte, bip16 bool) (*ScriptInfo, error)
 	return si, nil
 }
 
-// asSmallInt returns the passed opcode, which must be true according to
-// isSmallInt(), as an integer.
-func asSmallInt(op *opcode) int {
-	if op.value == OP_0 {
-		return 0
-	}
-
-	return int(op.value - (OP_1 - 1))
-}
-
 // CalcMultiSigStats returns the number of public keys and signatures from
 // a multi-signature transaction script.  The passed script MUST already be
 // known to be a multi-signature script.
@@ -769,6 +809,7 @@ func PushedData(script []byte) ([][]byte, error) {
 	if err != nil {
 		return nil, err
 	}
+
 	var data [][]byte
 	for _, pop := range pops {
 		if pop.data != nil {

txscript/script_test.go

@@ -2875,7 +2875,7 @@ func TestCalcMultiSigStats(t *testing.T) {
 	for i, test := range tests {
 		if _, _, err := txscript.CalcMultiSigStats(test.script); err != test.expected {
 			t.Errorf("CalcMultiSigStats #%d (%s) wrong result\n"+
-				"got: %x\nwant: %x", i, test.name, err,
+				"got: %v\nwant: %v", i, test.name, err,
 				test.expected)
 		}
 	}

txscript/sign.go

@@ -23,7 +23,7 @@ func RawTxInSignature(tx *wire.MsgTx, idx int, subScript []byte,
 	if err != nil {
 		return nil, fmt.Errorf("cannot parse output script: %v", err)
 	}
-	hash := calcScriptHash(parsedScript, hashType, tx, idx)
+	hash := calcSignatureHash(parsedScript, hashType, tx, idx)
 	signature, err := key.Sign(hash)
 	if err != nil {
 		return nil, fmt.Errorf("cannot sign tx input: %s", err)
@@ -291,7 +291,7 @@ sigLoop:
 		// however, assume no sigs etc are in the script since that
 		// would make the transaction nonstandard and thus not
 		// MultiSigTy, so we just need to hash the full thing.
-		hash := calcScriptHash(pkPops, hashType, tx, idx)
+		hash := calcSignatureHash(pkPops, hashType, tx, idx)
 
 		for _, addr := range addresses {
 			// All multisig addresses should be pubkey addreses

txscript/stack.go

@@ -131,30 +131,45 @@ func (s *stack) checkMinimalData(so []byte) error {
 	return nil
 }
 
+// Depth returns the number of items on the stack.
+func (s *stack) Depth() int {
+	return len(s.stk)
+}
+
 // PushByteArray adds the given back array to the top of the stack.
+//
+// Stack transformation: [... x1 x2] -> [... x1 x2 data]
 func (s *stack) PushByteArray(so []byte) {
 	s.stk = append(s.stk, so)
 }
 
 // PushInt converts the provided bignum to a suitable byte array then pushes
 // it onto the top of the stack.
+//
+// Stack transformation: [... x1 x2] -> [... x1 x2 int]
 func (s *stack) PushInt(val *big.Int) {
 	s.PushByteArray(fromInt(val))
 }
 
 // PushBool converts the provided boolean to a suitable byte array then pushes
 // it onto the top of the stack.
+//
+// Stack transformation: [... x1 x2] -> [... x1 x2 bool]
 func (s *stack) PushBool(val bool) {
 	s.PushByteArray(fromBool(val))
 }
 
 // PopByteArray pops the value off the top of the stack and returns it.
+//
+// Stack transformation: [... x1 x2 x3] -> [... x1 x2]
 func (s *stack) PopByteArray() ([]byte, error) {
 	return s.nipN(0)
 }
 
 // PopInt pops the value off the top of the stack, converts it into a bignum and
 // returns it.
+//
+// Stack transformation: [... x1 x2 x3] -> [... x1 x2]
 func (s *stack) PopInt() (*big.Int, error) {
 	so, err := s.PopByteArray()
 	if err != nil {
@@ -168,27 +183,31 @@ func (s *stack) PopInt() (*big.Int, error) {
 	return asInt(so)
 }
 
-// PopBool pops the value off the top of the stack, converts it into a bool and
+// PopBool pops the value off the top of the stack, converts it into a bool, and
 // returns it.
+//
+// Stack transformation: [... x1 x2 x3] -> [... x1 x2]
 func (s *stack) PopBool() (bool, error) {
 	so, err := s.PopByteArray()
 	if err != nil {
 		return false, err
 	}
+
 	return asBool(so), nil
 }
 
 // PeekByteArray returns the nth item on the stack without removing it.
-func (s *stack) PeekByteArray(idx int) (so []byte, err error) {
+func (s *stack) PeekByteArray(idx int) ([]byte, error) {
 	sz := len(s.stk)
 	if idx < 0 || idx >= sz {
 		return nil, ErrStackUnderflow
 	}
+
 	return s.stk[sz-idx-1], nil
 }
 
-// PeekInt returns the nth item on the stack as a bignum without removing it.
+// PeekInt returns the Nth item on the stack as a bignum without removing it.
-func (s *stack) PeekInt(idx int) (i *big.Int, err error) {
+func (s *stack) PeekInt(idx int) (*big.Int, error) {
 	so, err := s.PeekByteArray(idx)
 	if err != nil {
 		return nil, err
@@ -201,24 +220,30 @@ func (s *stack) PeekInt(idx int) (i *big.Int, err error) {
 	return asInt(so)
 }
 
-// PeekBool returns the nth item on the stack as a bool without removing it.
+// PeekBool returns the Nth item on the stack as a bool without removing it.
 func (s *stack) PeekBool(idx int) (i bool, err error) {
 	so, err := s.PeekByteArray(idx)
 	if err != nil {
 		return false, err
 	}
+
 	return asBool(so), nil
 }
 
 // nipN is an internal function that removes the nth item on the stack and
 // returns it.
-func (s *stack) nipN(idx int) (so []byte, err error) {
+//
+// Stack transformation:
+// nipN(0): [... x1 x2 x3] -> [... x1 x2]
+// nipN(1): [... x1 x2 x3] -> [... x1 x3]
+// nipN(2): [... x1 x2 x3] -> [... x2 x3]
+func (s *stack) nipN(idx int) ([]byte, error) {
 	sz := len(s.stk)
 	if idx < 0 || idx > sz-1 {
-		err = ErrStackUnderflow
+		return nil, ErrStackUnderflow
-		return
 	}
-	so = s.stk[sz-idx-1]
+
+	so := s.stk[sz-idx-1]
 	if idx == 0 {
 		s.stk = s.stk[:sz-1]
 	} else if idx == sz-1 {
@@ -230,17 +255,24 @@ func (s *stack) nipN(idx int) (so []byte, err error) {
 		s.stk = s.stk[:sz-idx-1]
 		s.stk = append(s.stk, s1...)
 	}
-	return
+	return so, nil
 }
 
 // NipN removes the Nth object on the stack
+//
+// Stack transformation:
+// NipN(0): [... x1 x2 x3] -> [... x1 x2]
+// NipN(1): [... x1 x2 x3] -> [... x1 x3]
+// NipN(2): [... x1 x2 x3] -> [... x2 x3]
 func (s *stack) NipN(idx int) error {
 	_, err := s.nipN(idx)
 	return err
 }
 
 // Tuck copies the item at the top of the stack and inserts it before the 2nd
-// to top item. e.g.: 2,1 -> 2,1,2
+// to top item.
+//
+// Stack transformation: [... x1 x2] -> [... x2 x1 x2]
 func (s *stack) Tuck() error {
 	so2, err := s.PopByteArray()
 	if err != nil {
@@ -250,27 +282,23 @@ func (s *stack) Tuck() error {
 	if err != nil {
 		return err
 	}
-	s.PushByteArray(so2) // stack 2
+	s.PushByteArray(so2) // stack [... x2]
-	s.PushByteArray(so1) // stack 1,2
+	s.PushByteArray(so1) // stack [... x2 x1]
-	s.PushByteArray(so2) // stack 2,1,2
+	s.PushByteArray(so2) // stack [... x2 x1 x2]
 
 	return nil
 }
 
-// Depth returns the number of items on the stack.
-func (s *stack) Depth() (sz int) {
-	sz = len(s.stk)
-	return
-}
-
 // DropN removes the top N items from the stack.
-// e.g.
+//
-// DropN(1): 1,2,3 -> 1,2
+// Stack transformation:
-// DropN(2): 1,2,3 -> 1
+// DropN(1): [... x1 x2] -> [... x1]
+// DropN(2): [... x1 x2] -> [...]
 func (s *stack) DropN(n int) error {
 	if n < 1 {
 		return ErrStackInvalidArgs
 	}
+
 	for ; n > 0; n-- {
 		_, err := s.PopByteArray()
 		if err != nil {
@@ -281,16 +309,17 @@ func (s *stack) DropN(n int) error {
 }
 
 // DupN duplicates the top N items on the stack.
-// e.g.
+//
-// DupN(1): 1,2,3 -> 1,2,3,3
+// Stack transformation:
-// DupN(2): 1,2,3 -> 1,2,3,2,3
+// DupN(1): [... x1 x2] -> [... x1 x2 x2]
+// DupN(2): [... x1 x2] -> [... x1 x2 x1 x2]
 func (s *stack) DupN(n int) error {
 	if n < 1 {
 		return ErrStackInvalidArgs
 	}
+
 	// Iteratively duplicate the value n-1 down the stack n times.
-	// this leaves us with an in-order duplicate of the top N items on the
+	// This leaves an in-order duplicate of the top n items on the stack.
-	// stack.
 	for i := n; i > 0; i-- {
 		so, err := s.PeekByteArray(n - 1)
 		if err != nil {
@@ -301,16 +330,19 @@ func (s *stack) DupN(n int) error {
 	return nil
 }
 
-// RotN rotates the top 3N items on the stack to the left
+// RotN rotates the top 3N items on the stack to the left N times.
-// e.g.
+//
-// RotN(1): 1,2,3 -> 2,3,1
+// Stack transformation:
+// RotN(1): [... x1 x2 x3] -> [... x2 x3 x1]
+// RotN(2): [... x1 x2 x3 x4 x5 x6] -> [... x3 x4 x5 x6 x1 x2]
 func (s *stack) RotN(n int) error {
 	if n < 1 {
 		return ErrStackInvalidArgs
 	}
-	entry := 3*n - 1
+
 	// Nip the 3n-1th item from the stack to the top n times to rotate
 	// them up to the head of the stack.
+	entry := 3*n - 1
 	for i := n; i > 0; i-- {
 		so, err := s.nipN(entry)
 		if err != nil {
@@ -323,16 +355,18 @@ func (s *stack) RotN(n int) error {
 }
 
 // SwapN swaps the top N items on the stack with those below them.
-// E.g.:
+//
-// SwapN(1): 1,2 -> 2,1
+// Stack transformation:
-// SwapN(2): 1,2,3,4 -> 3,4,1,2
+// SwapN(1): [... x1 x2] -> [... x2 x1]
+// SwapN(2): [... x1 x2 x3 x4] -> [... x3 x4 x1 x2]
 func (s *stack) SwapN(n int) error {
 	if n < 1 {
 		return ErrStackInvalidArgs
 	}
+
 	entry := 2*n - 1
 	for i := n; i > 0; i-- {
-		// swap 2n-1th entry to topj
+		// Swap 2n-1th entry to top.
 		so, err := s.nipN(entry)
 		if err != nil {
 			return err
@@ -343,15 +377,17 @@ func (s *stack) SwapN(n int) error {
 	return nil
 }
 
-// OverN copies N items N spaces back to the top of the stack.
+// OverN copies N items N items back to the top of the stack.
-// e.g.:
+//
-// OverN(1): 1,2 -> 1,2,1
+// Stack transformation:
-// OverN(2): 1,2,3,4 -> 1,2,3,4,1,2
+// OverN(1): [... x1 x2 x3] -> [... x1 x2 x3 x2]
+// OverN(2): [... x1 x2 x3 x4] -> [... x1 x2 x3 x4 x1 x2]
 func (s *stack) OverN(n int) error {
 	if n < 1 {
 		return ErrStackInvalidArgs
 	}
-	// Copy 2n-1th entry to top of the stack
+
+	// Copy 2n-1th entry to top of the stack.
 	entry := 2*n - 1
 	for ; n > 0; n-- {
 		so, err := s.PeekByteArray(entry)
@@ -359,31 +395,33 @@ func (s *stack) OverN(n int) error {
 			return err
 		}
 		s.PushByteArray(so)
-		// 4,1,2,3,4, now code original 3rd entry to top.
 	}
 
 	return nil
 }
 
 // PickN copies the item N items back in the stack to the top.
-// e.g.:
+//
-// PickN(1): 1,2,3 -> 1,2,3,2
+// Stack transformation:
-// PickN(2): 1,2,3 -> 1,2,3,1
+// PickN(0): [x1 x2 x3] -> [x1 x2 x3 x3]
+// PickN(1): [x1 x2 x3] -> [x1 x2 x3 x2]
+// PickN(2): [x1 x2 x3] -> [x1 x2 x3 x1]
 func (s *stack) PickN(n int) error {
 	so, err := s.PeekByteArray(n)
 	if err != nil {
 		return err
 	}
-
 	s.PushByteArray(so)
 
 	return nil
 }
 
 // RollN moves the item N items back in the stack to the top.
-// e.g.:
+//
-// RollN(1): 1,2,3 -> 1,3,2
+// Stack transformation:
-// RollN(2): 1,2,3 -> 2,3,1
+// RollN(0): [x1 x2 x3] -> [x1 x2 x3]
+// RollN(1): [x1 x2 x3] -> [x1 x3 x2]
+// RollN(2): [x1 x2 x3] -> [x2 x3 x1]
 func (s *stack) RollN(n int) error {
 	so, err := s.nipN(n)
 	if err != nil {
@@ -398,7 +436,6 @@ func (s *stack) RollN(n int) error {
 // String returns the stack in a readable format.
 func (s *stack) String() string {
 	var result string
-
 	for _, stack := range s.stk {
 		result += hex.Dump(stack)
 	}