From a8fe1ad5fe0615e6f2bb4d0df1c5726815bfeeab Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Wed, 29 Apr 2015 13:08:56 -0500
Subject: [PATCH] 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      | 466 ++++++++++++++++-------------
 txscript/opcode.go      | 511 ++++++++++++++++++++++++++------
 txscript/script.go      | 639 +++++++++++++++++++++-------------------
 txscript/script_test.go |   2 +-
 txscript/sign.go        |   4 +-
 txscript/stack.go       | 137 +++++----
 6 files changed, 1114 insertions(+), 645 deletions(-)

diff --git a/txscript/engine.go b/txscript/engine.go
index e030b28e..a8ef2a05 100644
--- a/txscript/engine.go
+++ b/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
-// tests that will be done when executing a Script.
+// ScriptFlags is a bitmask defining additional operations or tests that will be
+// 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''.
-	if pop.disabled() {
+	// Disabled opcodes are fail on program counter.
+	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
-	// we are done now.
-	if !vm.isBranchExecuting() && !pop.conditional() {
+	// Nothing left to do when this is not a conditional opcode and it is
+	// not in an executing branch.
+	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
-// 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
+// subScript returns 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 signature
+	// The length of the elements does not match the length of the
+	// 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
-		// scripts that push data
+		// Only accept input scripts that push data for P2SH.
 		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
diff --git a/txscript/opcode.go b/txscript/opcode.go
index be3eb253..4f42a059 100644
--- a/txscript/opcode.go
+++ b/txscript/opcode.go
@@ -18,10 +18,10 @@ import (
 	"github.com/btcsuite/golangcrypto/ripemd160"
 )
 
-// An opcode defines the information related to a txscript opcode.
-// opfunc if present is the function to call to perform the opcode on
-// the script. The current script is passed in as a slice with the firs
-// member being the opcode itself.
+// An opcode defines the information related to a txscript opcode.  opfunc if
+// present is the function to call to perform the opcode on the script.  The
+// current script is passed in as a slice with the first member being the opcode
+// itself.
 type opcode struct {
 	value  byte
 	name   string
@@ -293,7 +293,7 @@ const (
 	OP_INVALIDOPCODE       = 0xff // 255 - bitcoin core internal
 )
 
-// conditional execution constants
+// Conditional execution constants.
 const (
 	OpCondFalse = 0
 	OpCondTrue  = 1
@@ -607,14 +607,16 @@ var opcodeOnelineRepls = map[string]string{
 	"OP_16":      "16",
 }
 
+// parsedOpcode represents an opcode that has been parsed and includes any
+// potential data associated with it.
 type parsedOpcode struct {
 	opcode *opcode
 	data   []byte
 }
 
-// The following opcodes are disabled and are thus always bad to see in the
-// instruction stream (even if turned off by a conditional).
-func (pop *parsedOpcode) disabled() bool {
+// isDisabled returns whether or not the opcode is disabled and thus is always
+// bad to see in the instruction stream (even if turned off by a conditional).
+func (pop *parsedOpcode) isDisabled() bool {
 	switch pop.opcode.value {
 	case OP_CAT:
 		return true
@@ -651,9 +653,9 @@ func (pop *parsedOpcode) disabled() bool {
 	}
 }
 
-// The following opcodes are always illegal when passed over by the program
-// counter even if in a non-executed branch. (it isn't a coincidence that they
-// are conditionals).
+// alwaysIllegal returns whether or not the opcode is always illegal when passed
+// over by the program counter even if in a non-executed branch (it isn't a
+// coincidence that they are conditionals).
 func (pop *parsedOpcode) alwaysIllegal() bool {
 	switch pop.opcode.value {
 	case OP_VERIF:
@@ -665,9 +667,9 @@ func (pop *parsedOpcode) alwaysIllegal() bool {
 	}
 }
 
-// The following opcode are conditional and thus change the conditional
-// execution stack state when passed.
-func (pop *parsedOpcode) conditional() bool {
+// isConditional returns whether or not the opcode is a conditional opcode which
+// changes the conditional execution stack when executed.
+func (pop *parsedOpcode) isConditional() bool {
 	switch pop.opcode.value {
 	case OP_IF:
 		return true
@@ -682,8 +684,11 @@ func (pop *parsedOpcode) conditional() bool {
 	}
 }
 
-// checkMinimalDataPush returns whether or not the current data
-// push uses the correct opcode.
+// checkMinimalDataPush returns whether or not the current data push uses the
+// smallest possible opcode to represent it.  For example, the value 15 could
+// be pushed with OP_DATA_1 15 (among other variations); however, OP_15 is a
+// single opcode that represents the same value and is only a single byte versus
+// two bytes.
 func (pop *parsedOpcode) checkMinimalDataPush() error {
 	data := pop.data
 	dataLen := len(data)
@@ -717,6 +722,8 @@ func (pop *parsedOpcode) checkMinimalDataPush() error {
 	return nil
 }
 
+// print returns a human-readable string representation of the opcode for use
+// in script disassembly.
 func (pop *parsedOpcode) print(oneline bool) string {
 	// The reference implementation one-line disassembly replaces opcodes
 	// which represent values (e.g. OP_0 through OP_16 and OP_1NEGATE)
@@ -756,6 +763,8 @@ func (pop *parsedOpcode) print(oneline bool) string {
 	return fmt.Sprintf("%s 0x%02x", retString, pop.data)
 }
 
+// bytes returns any data associated with the opcode encoded as it would be in
+// a script.  This is used for unparsing scripts from parsed opcodes.
 func (pop *parsedOpcode) bytes() ([]byte, error) {
 	var retbytes []byte
 	if pop.opcode.length > 0 {
@@ -803,60 +812,94 @@ func (pop *parsedOpcode) bytes() ([]byte, error) {
 	return retbytes, nil
 }
 
-// opcode implementation functions from here
+// *******************************************
+// Opcode implementation functions start here.
+// *******************************************
 
+// opcodeDisabled is a common handler for disabled opcodes.  It returns an
+// appropriate error indicating the opcode is disabled.  While it would
+// ordinarily make more sense to detect if the script contains any disabled
+// opcodes before executing in an initial parse step, the consensus rules
+// dictate the script doesn't fail until the program counter passes over a
+// disabled opcode (even when they appear in a branch that is not executed).
 func opcodeDisabled(op *parsedOpcode, vm *Engine) error {
 	return ErrStackOpDisabled
 }
 
+// opcodeReserved is a common handler for all reserved opcodes.  It returns an
+// appropriate error indicating the opcode is reserved.
 func opcodeReserved(op *parsedOpcode, vm *Engine) error {
 	return ErrStackReservedOpcode
 }
 
-// Recognised opcode, but for bitcoind internal use only.
+// opcodeReserved is a common handler for all invalid opcodes.  It returns an
+// appropriate error indicating the opcode is invalid.
 func opcodeInvalid(op *parsedOpcode, vm *Engine) error {
 	return ErrStackInvalidOpcode
 }
 
+// opcodeFalse pushes an empty array to the data stack to represent false.  Note
+// that 0, when encoded as a number according to the numeric encoding consensus
+// rules, is an empty array.
 func opcodeFalse(op *parsedOpcode, vm *Engine) error {
 	vm.dstack.PushByteArray([]byte(""))
-
 	return nil
 }
 
+// opcodePushData is a common handler for the vast majority of opcodes that push
+// raw data (bytes) to the data stack.
 func opcodePushData(op *parsedOpcode, vm *Engine) error {
 	vm.dstack.PushByteArray(op.data)
 	return nil
 }
 
+// opcode1Negate pushes -1, encoded as a number, to the data stack.
 func opcode1Negate(op *parsedOpcode, vm *Engine) error {
 	vm.dstack.PushInt(big.NewInt(-1))
 	return nil
 }
 
+// opcodeN is a common handler for the small integer data push opcodes.  It
+// pushes the numeric value the opcode represents (which will be from 1 to 16)
+// onto the data stack.
 func opcodeN(op *parsedOpcode, vm *Engine) error {
-	// 16 consecutive opcodes add increasing numbers to the stack.
+	// The opcodes are all defined consecutively, so the numeric value is
+	// the difference.
 	vm.dstack.PushInt(big.NewInt(int64(op.opcode.value - (OP_1 - 1))))
 	return nil
 }
 
+// opcodeNop is a common handler for the NOP family of opcodes.  As the name
+// implies it generally does nothing, however, it will return an error when
+// the flag to discourage use of NOPs is set for select opcodes.
 func opcodeNop(op *parsedOpcode, vm *Engine) error {
 	switch op.opcode.value {
 	case OP_NOP1, OP_NOP2, OP_NOP3, OP_NOP4, OP_NOP5,
 		OP_NOP6, OP_NOP7, OP_NOP8, OP_NOP9, OP_NOP10:
 		if vm.hasFlag(ScriptDiscourageUpgradableNops) {
 			return fmt.Errorf("OP_NOP%d reserved for soft-fork "+
-				"upgrades", op.opcode.value-OP_NOP1+1)
+				"upgrades", op.opcode.value-(OP_NOP1-1))
 		}
 	}
 	return nil
 }
 
-// opcodeIf computes true/false based on the value on the stack and pushes
-// the condition on the condStack (conditional execution stack)
+// opcodeIf treats the top item on the data stack as a boolean and removes it.
+//
+// An appropriate entry is added to the conditional stack depending on whether
+// the boolean is true and whether this if is on an executing branch in order
+// to allow proper execution of further opcodes depending on the conditional
+// logic.  When the boolean is true, the first branch will be executed (unless
+// this opcode is nested in a non-executed branch).
+//
+// <expression> if [statements] [else [statements]] endif
+//
+// Note that, unlike for all non-conditional opcodes, this is executed even when
+// it is on a non-executing branch so proper nesting is maintained.
+//
+// Data stack transformation: [... bool] -> [...]
+// Conditional stack transformation: [...] -> [... OpCondValue]
 func opcodeIf(op *parsedOpcode, vm *Engine) error {
-	// opcodeIf will be executed even if it is on the non-execute side
-	// of the conditional, this is so proper nesting is maintained
 	condVal := OpCondFalse
 	if vm.isBranchExecuting() {
 		ok, err := vm.dstack.PopBool()
@@ -873,11 +916,23 @@ func opcodeIf(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
-// opcodeNotIf computes true/false based on the value on the stack and pushes
-// the (inverted) condition on the condStack (conditional execution stack)
+// opcodeNotIf treats the top item on the data stack as a boolean and removes
+// it.
+//
+// An appropriate entry is added to the conditional stack depending on whether
+// the boolean is true and whether this if is on an executing branch in order
+// to allow proper execution of further opcodes depending on the conditional
+// logic.  When the boolean is false, the first branch will be executed (unless
+// this opcode is nested in a non-executed branch).
+//
+// <expression> notif [statements] [else [statements]] endif
+//
+// Note that, unlike for all non-conditional opcodes, this is executed even when
+// it is on a non-executing branch so proper nesting is maintained.
+//
+// Data stack transformation: [... bool] -> [...]
+// Conditional stack transformation: [...] -> [... OpCondValue]
 func opcodeNotIf(op *parsedOpcode, vm *Engine) error {
-	// opcodeIf will be executed even if it is on the non-execute side
-	// of the conditional, this is so proper nesting is maintained
 	condVal := OpCondFalse
 	if vm.isBranchExecuting() {
 		ok, err := vm.dstack.PopBool()
@@ -894,7 +949,11 @@ func opcodeNotIf(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
-// opcodeElse inverts conditional execution for other half of if/else/endif
+// opcodeElse inverts conditional execution for other half of if/else/endif.
+//
+// An error is returned if there has not already been a matching OP_IF.
+//
+// Conditional stack transformation: [... OpCondValue] -> [... !OpCondValue]
 func opcodeElse(op *parsedOpcode, vm *Engine) error {
 	if len(vm.condStack) == 0 {
 		return ErrStackNoIf
@@ -907,13 +966,18 @@ func opcodeElse(op *parsedOpcode, vm *Engine) error {
 	case OpCondFalse:
 		vm.condStack[conditionalIdx] = OpCondTrue
 	case OpCondSkip:
-		// value doesn't change in skip
+		// Value doesn't change in skip since it indicates this opcode
+		// is nested in a non-executed branch.
 	}
 	return nil
 }
 
-// opcodeEndif terminates a conditional block, removing the  value from the
+// opcodeEndif terminates a conditional block, removing the value from the
 // conditional execution stack.
+//
+// An error is returned if there has not already been a matching OP_IF.
+//
+// Conditional stack transformation: [... OpCondValue] -> [...]
 func opcodeEndif(op *parsedOpcode, vm *Engine) error {
 	if len(vm.condStack) == 0 {
 		return ErrStackNoIf
@@ -923,6 +987,8 @@ func opcodeEndif(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeVerify examines the top item on the data stack as a boolean value and
+// verifies it evaluates to true.  An error is returned if it does not.
 func opcodeVerify(op *parsedOpcode, vm *Engine) error {
 	verified, err := vm.dstack.PopBool()
 	if err != nil {
@@ -935,10 +1001,17 @@ func opcodeVerify(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeReturn returns an appropriate error since it is always an error to
+// return early from a script.
 func opcodeReturn(op *parsedOpcode, vm *Engine) error {
 	return ErrStackEarlyReturn
 }
 
+// opcodeToAltStack removes the top item from the main data stack and pushes it
+// onto the alternate data stack.
+//
+// Main data stack transformation: [... x1 x2 x3] -> [... x1 x2]
+// Alt data stack transformation:  [... y1 y2 y3] -> [... y1 y2 y3 x3]
 func opcodeToAltStack(op *parsedOpcode, vm *Engine) error {
 	so, err := vm.dstack.PopByteArray()
 	if err != nil {
@@ -949,6 +1022,11 @@ func opcodeToAltStack(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeFromAltStack removes the top item from the alternate data stack and
+// pushes it onto the main data stack.
+//
+// Main data stack transformation: [... x1 x2 x3] -> [... x1 x2 x3 y1]
+// Alt data stack transformation:  [... y1 y2 y3] -> [... y1 y2]
 func opcodeFromAltStack(op *parsedOpcode, vm *Engine) error {
 	so, err := vm.astack.PopByteArray()
 	if err != nil {
@@ -959,30 +1037,53 @@ func opcodeFromAltStack(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcode2Drop removes the top 2 items from the data stack.
+//
+// Stack transformation: [... x1 x2 x3] -> [... x1]
 func opcode2Drop(op *parsedOpcode, vm *Engine) error {
 	return vm.dstack.DropN(2)
 }
 
+// opcode2Dup duplicates the top 2 items on the data stack.
+//
+// Stack transformation: [... x1 x2 x3] -> [... x1 x2 x3 x2 x3]
 func opcode2Dup(op *parsedOpcode, vm *Engine) error {
 	return vm.dstack.DupN(2)
 }
 
+// opcode3Dup duplicates the top 3 items on the data stack.
+//
+// Stack transformation: [... x1 x2 x3] -> [... x1 x2 x3 x1 x2 x3]
 func opcode3Dup(op *parsedOpcode, vm *Engine) error {
 	return vm.dstack.DupN(3)
 }
 
+// opcode2Over duplicates the 2 items before the top 2 items on the data stack.
+//
+// Stack transformation: [... x1 x2 x3 x4] -> [... x1 x2 x3 x4 x1 x2]
 func opcode2Over(op *parsedOpcode, vm *Engine) error {
 	return vm.dstack.OverN(2)
 }
 
+// opcode2Rot rotates the top 6 items on the data stack to the left twice.
+//
+// Stack transformation: [... x1 x2 x3 x4 x5 x6] -> [... x3 x4 x5 x6 x1 x2]
 func opcode2Rot(op *parsedOpcode, vm *Engine) error {
 	return vm.dstack.RotN(2)
 }
 
+// opcode2Swap swaps the top 2 items on the data stack with the 2 that come
+// before them.
+//
+// Stack transformation: [... x1 x2 x3 x4] -> [... x3 x4 x1 x2]
 func opcode2Swap(op *parsedOpcode, vm *Engine) error {
 	return vm.dstack.SwapN(2)
 }
 
+// opcodeIfDup duplicates the top item of the stack if it is not zero.
+//
+// Stack transformation (x1==0): [... x1] -> [...]
+// Stack transformation (x1!=0): [... x1] -> [... x1]
 func opcodeIfDup(op *parsedOpcode, vm *Engine) error {
 	val, err := vm.dstack.PeekInt(0)
 	if err != nil {
@@ -997,29 +1098,51 @@ func opcodeIfDup(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeDepth pushes the depth of the data stack prior to executing this
+// opcode, encoded as a number, onto the data stack.
+//
+// Stack transformation: [...] -> [... <num of items on the stack>]
+// Example with 2 items: [x1 x2] -> [x1 x2 2]
+// Example with 3 items: [x1 x2 x3] -> [x1 x2 x3 3]
 func opcodeDepth(op *parsedOpcode, vm *Engine) error {
 	vm.dstack.PushInt(big.NewInt(int64(vm.dstack.Depth())))
 	return nil
 }
 
+// opcodeDrop removes the top item from the data stack.
+//
+// Stack transformation: [... x1 x2 x3] -> [... x1 x2]
 func opcodeDrop(op *parsedOpcode, vm *Engine) error {
 	return vm.dstack.DropN(1)
 }
 
+// opcodeDup duplicates the top item on the data stack.
+//
+// Stack transformation: [... x1 x2 x3] -> [... x1 x2 x3 x3]
 func opcodeDup(op *parsedOpcode, vm *Engine) error {
 	return vm.dstack.DupN(1)
 }
 
+// opcodeNip removes the item before the top item on the data stack.
+//
+// Stack transformation: [... x1 x2 x3] -> [... x1 x3]
 func opcodeNip(op *parsedOpcode, vm *Engine) error {
 	return vm.dstack.NipN(1)
 }
 
+// opcodeOver duplicates the item before the top item on the data stack.
+//
+// Stack transformation: [... x1 x2 x3] -> [... x1 x2 x3 x2]
 func opcodeOver(op *parsedOpcode, vm *Engine) error {
 	return vm.dstack.OverN(1)
 }
 
-// Copy object N items back in the stack to the top. Where N is the value in
-// the top of the stack.
+// opcodePick treats the top item on the data stack as an integer and duplicates
+// the item on the stack that number of items back to the top.
+//
+// Stack transformation: [xn ... x2 x1 x0 n] -> [xn ... x2 x1 x0 xn]
+// Example with n=1: [x2 x1 x0 1] -> [x2 x1 x0 x1]
+// Example with n=2: [x2 x1 x0 2] -> [x2 x1 x0 x2]
 func opcodePick(op *parsedOpcode, vm *Engine) error {
 	pidx, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1028,12 +1151,15 @@ func opcodePick(op *parsedOpcode, vm *Engine) error {
 
 	// PopInt promises that the int returned is 32 bit.
 	val := int(pidx.Int64())
-
 	return vm.dstack.PickN(val)
 }
 
-// Move object N items back in the stack to the top. Where N is the value in
-// the top of the stack.
+// opcodeRoll treats the top item on the data stack as an integer and moves
+// the item on the stack that number of items back to the top.
+//
+// Stack transformation: [xn ... x2 x1 x0 n] -> [... x2 x1 x0 xn]
+// Example with n=1: [x2 x1 x0 1] -> [x2 x0 x1]
+// Example with n=2: [x2 x1 x0 2] -> [x1 x0 x2]
 func opcodeRoll(op *parsedOpcode, vm *Engine) error {
 	ridx, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1042,38 +1168,49 @@ func opcodeRoll(op *parsedOpcode, vm *Engine) error {
 
 	// PopInt promises that the int returned is 32 bit.
 	val := int(ridx.Int64())
-
 	return vm.dstack.RollN(val)
 }
 
-// Rotate top three items on the stack to the left.
-// e.g. 1,2,3 -> 2,3,1
+// opcodeRot rotates the top 3 items on the data stack to the left.
+//
+// Stack transformation: [... x1 x2 x3] -> [... x2 x3 x1]
 func opcodeRot(op *parsedOpcode, vm *Engine) error {
 	return vm.dstack.RotN(1)
 }
 
-// Swap the top two items on the stack: 1,2 -> 2,1
+// opcodeSwap swaps the top two items on the stack.
+//
+// Stack transformation: [... x1 x2] -> [... x2 x1]
 func opcodeSwap(op *parsedOpcode, vm *Engine) error {
 	return vm.dstack.SwapN(1)
 }
 
-// The item at the top of the stack is copied and inserted before the
-// second-to-top item. e.g.: 2,1, -> 2,1,2
+// opcodeTuck inserts a duplicate of the top item of the data stack before the
+// second-to-top item.
+//
+// Stack transformation: [... x1 x2] -> [... x2 x1 x2]
 func opcodeTuck(op *parsedOpcode, vm *Engine) error {
 	return vm.dstack.Tuck()
 }
 
-// Push the size of the item on top of the stack onto the stack.
+// opcodeSize pushes the size of the top item of the data stack onto the data
+// stack.
+//
+// Stack transformation: [... x1] -> [... x1 len(x1)]
 func opcodeSize(op *parsedOpcode, vm *Engine) error {
-	i, err := vm.dstack.PeekByteArray(0)
+	so, err := vm.dstack.PeekByteArray(0)
 	if err != nil {
 		return err
 	}
 
-	vm.dstack.PushInt(big.NewInt(int64(len(i))))
+	vm.dstack.PushInt(big.NewInt(int64(len(so))))
 	return nil
 }
 
+// opcodeEqual removes the top 2 items of the data stack, compares them as raw
+// bytes, and pushes the result, encoded as a boolean, back to the stack.
+//
+// Stack transformation: [... x1 x2] -> [... bool]
 func opcodeEqual(op *parsedOpcode, vm *Engine) error {
 	a, err := vm.dstack.PopByteArray()
 	if err != nil {
@@ -1088,6 +1225,13 @@ func opcodeEqual(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeEqualVerify is a combination of opcodeEqual and opcodeVerify.
+// Specifically, it removes the top 2 items of the data stack, compares them,
+// and pushes the result, encoded as a boolean, back to the stack.  Then, it
+// examines the top item on the data stack as a boolean value and verifies it
+// evaluates to true.  An error is returned if it does not.
+//
+// Stack transformation: [... x1 x2] -> [... bool] -> [...]
 func opcodeEqualVerify(op *parsedOpcode, vm *Engine) error {
 	err := opcodeEqual(op, vm)
 	if err == nil {
@@ -1096,6 +1240,10 @@ func opcodeEqualVerify(op *parsedOpcode, vm *Engine) error {
 	return err
 }
 
+// opcode1Add treats the top item on the data stack as an integer and replaces
+// it with its incremented value (plus 1).
+//
+// Stack transformation: [... x1 x2] -> [... x1 x2+1]
 func opcode1Add(op *parsedOpcode, vm *Engine) error {
 	m, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1103,10 +1251,13 @@ func opcode1Add(op *parsedOpcode, vm *Engine) error {
 	}
 
 	vm.dstack.PushInt(new(big.Int).Add(m, big.NewInt(1)))
-
 	return nil
 }
 
+// opcode1Sub treats the top item on the data stack as an integer and replaces
+// it with its decremented value (minus 1).
+//
+// Stack transformation: [... x1 x2] -> [... x1 x2-1]
 func opcode1Sub(op *parsedOpcode, vm *Engine) error {
 	m, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1117,8 +1268,11 @@ func opcode1Sub(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeNegate treats the top item on the data stack as an integer and replaces
+// it with its negation.
+//
+// Stack transformation: [... x1 x2] -> [... x1 -x2]
 func opcodeNegate(op *parsedOpcode, vm *Engine) error {
-	// XXX when we remove types just flip the 0x80 bit of msb
 	m, err := vm.dstack.PopInt()
 	if err != nil {
 		return err
@@ -1128,25 +1282,38 @@ func opcodeNegate(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeAbs treats the top item on the data stack as an integer and replaces it
+// it with its absolute value.
+//
+// Stack transformation: [... x1 x2] -> [... x1 abs(x2)]
 func opcodeAbs(op *parsedOpcode, vm *Engine) error {
-	// XXX when we remove types just &= ~0x80 on msb
 	m, err := vm.dstack.PopInt()
 	if err != nil {
 		return err
 	}
 
 	vm.dstack.PushInt(new(big.Int).Abs(m))
-
 	return nil
 }
 
-// If then input is 0 or 1, it is flipped. Otherwise the output will be 0.
-// (n.b. official client just has 1 is 0, else 0)
+// opcodeNot treats the top item on the data stack as an integer and replaces
+// it with its "inverted" value (0 becomes 1, non-zero becomes 0).
+//
+// NOTE: While it would probably make more sense to treat the top item as a
+// boolean, and push the opposite, which is really what the intention of this
+// opcode is, it is extremely important that is not done because integers are
+// interpreted differently than booleans and the consensus rules for this opcode
+// dictate the item is interpreted as an integer.
+//
+// Stack transformation (x2==0): [... x1 0] -> [... x1 1]
+// Stack transformation (x2!=0): [... x1 1] -> [... x1 0]
+// Stack transformation (x2!=0): [... x1 17] -> [... x1 0]
 func opcodeNot(op *parsedOpcode, vm *Engine) error {
 	m, err := vm.dstack.PopInt()
 	if err != nil {
 		return err
 	}
+
 	if m.Sign() == 0 {
 		vm.dstack.PushInt(big.NewInt(1))
 	} else {
@@ -1155,21 +1322,29 @@ func opcodeNot(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
-// opcode returns 0 if the input is 0, 1 otherwise.
+// opcode0NotEqual treats the top item on the data stack as an integer and
+// replaces it with either a 0 if it is zero, or a 1 if it is not zero.
+//
+// Stack transformation (x2==0): [... x1 0] -> [... x1 0]
+// Stack transformation (x2!=0): [... x1 1] -> [... x1 1]
+// Stack transformation (x2!=0): [... x1 17] -> [... x1 1]
 func opcode0NotEqual(op *parsedOpcode, vm *Engine) error {
 	m, err := vm.dstack.PopInt()
 	if err != nil {
 		return err
 	}
+
 	if m.Sign() != 0 {
 		m.SetInt64(1)
 	}
 	vm.dstack.PushInt(m)
-
 	return nil
 }
 
-// Push result of adding top two entries on stack
+// opcodeAdd treats the top two items on the data stack as integers and replaces
+// them with their sum.
+//
+// Stack transformation: [... x1 x2] -> [... x1+x2]
 func opcodeAdd(op *parsedOpcode, vm *Engine) error {
 	v0, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1185,7 +1360,11 @@ func opcodeAdd(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
-// Push result of subtracting 2nd entry on stack from first.
+// opcodeSub treats the top two items on the data stack as integers and replaces
+// them with the result of subtracting the top entry from the second-to-top
+// entry.
+//
+// Stack transformation: [... x1 x2] -> [... x1-x2]
 func opcodeSub(op *parsedOpcode, vm *Engine) error {
 	v0, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1201,8 +1380,13 @@ func opcodeSub(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
-// If both of the top two entries on the stack are not zero output is 1.
-// Otherwise, 0.
+// opcodeBoolAnd treats the top two items on the data stack as integers.  When
+// both of them are not zero, they are replaced with a 1, otherwise a 0.
+//
+// Stack transformation (x1==0, x2==0): [... 0 0] -> [... 0]
+// Stack transformation (x1!=0, x2==0): [... 5 0] -> [... 0]
+// Stack transformation (x1==0, x2!=0): [... 0 7] -> [... 0]
+// Stack transformation (x1!=0, x2!=0): [... 4 8] -> [... 1]
 func opcodeBoolAnd(op *parsedOpcode, vm *Engine) error {
 	v0, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1223,8 +1407,13 @@ func opcodeBoolAnd(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
-// If either of the top two entries on the stack are not zero output is 1.
-// Otherwise, 0.
+// opcodeBoolOr treats the top two items on the data stack as integers.  When
+// either of them are not zero, they are replaced with a 1, otherwise a 0.
+//
+// Stack transformation (x1==0, x2==0): [... 0 0] -> [... 0]
+// Stack transformation (x1!=0, x2==0): [... 5 0] -> [... 1]
+// Stack transformation (x1==0, x2!=0): [... 0 7] -> [... 1]
+// Stack transformation (x1!=0, x2!=0): [... 4 8] -> [... 1]
 func opcodeBoolOr(op *parsedOpcode, vm *Engine) error {
 	v0, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1245,6 +1434,11 @@ func opcodeBoolOr(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeNumEqual treats the top two items on the data stack as integers.  When
+// they are equal, they are replaced with a 1, otherwise a 0.
+//
+// Stack transformation (x1==x2): [... 5 5] -> [... 1]
+// Stack transformation (x1!=x2): [... 5 7] -> [... 0]
 func opcodeNumEqual(op *parsedOpcode, vm *Engine) error {
 	v0, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1265,6 +1459,14 @@ func opcodeNumEqual(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeNumEqualVerify is a combination of opcodeNumEqual and opcodeVerify.
+//
+// Specifically, treats the top two items on the data stack as integers.  When
+// they are equal, they are replaced with a 1, otherwise a 0.  Then, it examines
+// the top item on the data stack as a boolean value and verifies it evaluates
+// to true.  An error is returned if it does not.
+//
+// Stack transformation: [... x1 x2] -> [... bool] -> [...]
 func opcodeNumEqualVerify(op *parsedOpcode, vm *Engine) error {
 	err := opcodeNumEqual(op, vm)
 	if err == nil {
@@ -1273,6 +1475,11 @@ func opcodeNumEqualVerify(op *parsedOpcode, vm *Engine) error {
 	return err
 }
 
+// opcodeNumNotEqual treats the top two items on the data stack as integers.
+// When they are NOT equal, they are replaced with a 1, otherwise a 0.
+//
+// Stack transformation (x1==x2): [... 5 5] -> [... 0]
+// Stack transformation (x1!=x2): [... 5 7] -> [... 1]
 func opcodeNumNotEqual(op *parsedOpcode, vm *Engine) error {
 	v0, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1293,6 +1500,11 @@ func opcodeNumNotEqual(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeLessThan treats the top two items on the data stack as integers.  When
+// the second-to-top item is less than the top item, they are replaced with a 1,
+// otherwise a 0.
+//
+// Stack transformation: [... x1 x2] -> [... bool]
 func opcodeLessThan(op *parsedOpcode, vm *Engine) error {
 	v0, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1313,6 +1525,11 @@ func opcodeLessThan(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeGreaterThan treats the top two items on the data stack as integers.
+// When the second-to-top item is greater than the top item, they are replaced
+// with a 1, otherwise a 0.
+//
+// Stack transformation: [... x1 x2] -> [... bool]
 func opcodeGreaterThan(op *parsedOpcode, vm *Engine) error {
 	v0, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1332,6 +1549,11 @@ func opcodeGreaterThan(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeLessThanOrEqual treats the top two items on the data stack as integers.
+// When the second-to-top item is less than or equal to the top item, they are
+// replaced with a 1, otherwise a 0.
+//
+// Stack transformation: [... x1 x2] -> [... bool]
 func opcodeLessThanOrEqual(op *parsedOpcode, vm *Engine) error {
 	v0, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1351,6 +1573,11 @@ func opcodeLessThanOrEqual(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeGreaterThanOrEqual treats the top two items on the data stack as
+// integers.  When the second-to-top item is greater than or equal to the top
+// item, they are replaced with a 1, otherwise a 0.
+//
+// Stack transformation: [... x1 x2] -> [... bool]
 func opcodeGreaterThanOrEqual(op *parsedOpcode, vm *Engine) error {
 	v0, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1371,6 +1598,10 @@ func opcodeGreaterThanOrEqual(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeMin treats the top two items on the data stack as integers and replaces
+// them with the minimum of the two.
+//
+// Stack transformation: [... x1 x2] -> [... min(x1, x2)]
 func opcodeMin(op *parsedOpcode, vm *Engine) error {
 	v0, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1391,6 +1622,10 @@ func opcodeMin(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeMax treats the top two items on the data stack as integers and replaces
+// them with the maximum of the two.
+//
+// Stack transformation: [... x1 x2] -> [... max(x1, x2)]
 func opcodeMax(op *parsedOpcode, vm *Engine) error {
 	v0, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1411,8 +1646,14 @@ func opcodeMax(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
-// stack input: x, min, max. Returns 1 if x is within specified range
-// (left inclusive), 0 otherwise
+// opcodeWithin treats the top 3 items on the data stack as integers.  When the
+// value to test is within the specified range (left inclusive), they are
+// replaced with a 1, otherwise a 0.
+//
+// The top item is the max value, the second-top-item is the minimum value, and
+// the third-to-top item is the value to test.
+//
+// Stack transformation: [... x1 min max] -> [... bool]
 func opcodeWithin(op *parsedOpcode, vm *Engine) error {
 	maxVal, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1437,12 +1678,16 @@ func opcodeWithin(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
-// Calculate the hash of hasher over buf.
+// calcHash calculates the hash of hasher over buf.
 func calcHash(buf []byte, hasher hash.Hash) []byte {
 	hasher.Write(buf)
 	return hasher.Sum(nil)
 }
 
+// opcodeRipemd160 treats the top item of the data stack as raw bytes and
+// replaces it with ripemd160(data).
+//
+// Stack transformation: [... x1] -> [... ripemd160(x1)]
 func opcodeRipemd160(op *parsedOpcode, vm *Engine) error {
 	buf, err := vm.dstack.PopByteArray()
 	if err != nil {
@@ -1453,6 +1698,10 @@ func opcodeRipemd160(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeSha1 treats the top item of the data stack as raw bytes and replaces it
+// with sha1(data).
+//
+// Stack transformation: [... x1] -> [... sha1(x1)]
 func opcodeSha1(op *parsedOpcode, vm *Engine) error {
 	buf, err := vm.dstack.PopByteArray()
 	if err != nil {
@@ -1464,6 +1713,10 @@ func opcodeSha1(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeSha256 treats the top item of the data stack as raw bytes and replaces
+// it with sha256(data).
+//
+// Stack transformation: [... x1] -> [... sha256(x1)]
 func opcodeSha256(op *parsedOpcode, vm *Engine) error {
 	buf, err := vm.dstack.PopByteArray()
 	if err != nil {
@@ -1475,6 +1728,10 @@ func opcodeSha256(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeHash160 treats the top item of the data stack as raw bytes and replaces
+// it with ripemd160(sha256(data)).
+//
+// Stack transformation: [... x1] -> [... ripemd160(sha256(x1))]
 func opcodeHash160(op *parsedOpcode, vm *Engine) error {
 	buf, err := vm.dstack.PopByteArray()
 	if err != nil {
@@ -1486,6 +1743,10 @@ func opcodeHash160(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeHash256 treats the top item of the data stack as raw bytes and replaces
+// it with sha256(sha256(data)).
+//
+// Stack transformation: [... x1] -> [... sha256(sha256(x1))]
 func opcodeHash256(op *parsedOpcode, vm *Engine) error {
 	buf, err := vm.dstack.PopByteArray()
 	if err != nil {
@@ -1496,57 +1757,83 @@ func opcodeHash256(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeCodeSeparator stores the current script offset as the most recently
+// seen OP_CODESEPARATOR which is used during signature checking.
+//
+// This opcode does not change the contents of the data stack.
 func opcodeCodeSeparator(op *parsedOpcode, vm *Engine) error {
-	vm.lastcodesep = vm.scriptOff
-
+	vm.lastCodeSep = vm.scriptOff
 	return nil
 }
 
+// opcodeCheckSig treats the top 2 items on the stack as a public key and a
+// signature and replaces them with a bool which indicates if the signature was
+// successfully verified.
+//
+// The process of verifying a signature requires calculating a signature hash in
+// the same way the transaction signer did.  It involves hashing portions of the
+// transaction based on the hash type byte (which is the final byte of the
+// signature) and the portion of the script starting from the most recent
+// OP_CODESEPARATOR (or the beginning of the script if there are none) to the
+// end of the script (with any other OP_CODESEPARATORs removed).  Once this
+// "script hash" is calculated, the signature is checked using standard
+// cryptographic methods against the provided public key.
+//
+// Stack transformation: [... signature pubkey] -> [... bool]
 func opcodeCheckSig(op *parsedOpcode, vm *Engine) error {
-
-	pkStr, err := vm.dstack.PopByteArray()
+	pkBytes, err := vm.dstack.PopByteArray()
 	if err != nil {
 		return err
 	}
 
-	sigStr, err := vm.dstack.PopByteArray()
+	sigBytes, err := vm.dstack.PopByteArray()
 	if err != nil {
 		return err
 	}
 
-	// Signature actually needs needs to be longer than this, but we need
-	// at least  1 byte for the below. btcec will check full length upon
-	// parsing the signature.
-	if len(sigStr) < 1 {
+	// The signature actually needs needs to be longer than this, but at
+	// least 1 byte is needed for the hash type below.  The full length is
+	// checked depending on the script flags and upon parsing the signature.
+	if len(sigBytes) < 1 {
 		vm.dstack.PushBool(false)
 		return nil
 	}
 
-	// Trim off hashtype from the signature string.
-	hashType := SigHashType(sigStr[len(sigStr)-1])
-	sigStr = sigStr[:len(sigStr)-1]
-
+	// Trim off hashtype from the signature string and check if the
+	// signature and pubkey conform to the strict encoding requirements
+	// depending on the flags.
+	//
+	// NOTE: When the strict encoding flags are set, any errors in the
+	// signature or public encoding here result in an immediate script error
+	// (and thus no result bool is pushed to the data stack).  This differs
+	// from the logic below where any errors in parsing the signature is
+	// treated as the signature failure resulting in false being pushed to
+	// the data stack.  This is required because the more general script
+	// validation consensus rules do not have the new strict encoding
+	// requirements enabled by the flags.
+	hashType := SigHashType(sigBytes[len(sigBytes)-1])
+	sigBytes = sigBytes[:len(sigBytes)-1]
 	if err := vm.checkHashTypeEncoding(hashType); err != nil {
 		return err
 	}
-	if err := vm.checkSignatureEncoding(sigStr); err != nil {
+	if err := vm.checkSignatureEncoding(sigBytes); err != nil {
 		return err
 	}
-	if err := vm.checkPubKeyEncoding(pkStr); err != nil {
+	if err := vm.checkPubKeyEncoding(pkBytes); err != nil {
 		return err
 	}
 
-	// Get script from the last OP_CODESEPARATOR and without any subsequent
-	// OP_CODESEPARATORs
+	// Get script starting from the most recent OP_CODESEPARATOR.
 	subScript := vm.subScript()
 
-	// Unlikely to hit any cases here, but remove the signature from
-	// the script if present.
-	subScript = removeOpcodeByData(subScript, sigStr)
+	// Remove the signature since there is no way for a signature to sign
+	// itself.
+	subScript = removeOpcodeByData(subScript, sigBytes)
 
-	hash := calcScriptHash(subScript, hashType, &vm.tx, vm.txIdx)
+	// Generate the signature hash based on the signature hash type.
+	hash := calcSignatureHash(subScript, hashType, &vm.tx, vm.txIdx)
 
-	pubKey, err := btcec.ParsePubKey(pkStr, btcec.S256())
+	pubKey, err := btcec.ParsePubKey(pkBytes, btcec.S256())
 	if err != nil {
 		vm.dstack.PushBool(false)
 		return nil
@@ -1556,9 +1843,9 @@ func opcodeCheckSig(op *parsedOpcode, vm *Engine) error {
 	if vm.hasFlag(ScriptVerifyStrictEncoding) ||
 		vm.hasFlag(ScriptVerifyDERSignatures) {
 
-		signature, err = btcec.ParseDERSignature(sigStr, btcec.S256())
+		signature, err = btcec.ParseDERSignature(sigBytes, btcec.S256())
 	} else {
-		signature, err = btcec.ParseSignature(sigStr, btcec.S256())
+		signature, err = btcec.ParseSignature(sigBytes, btcec.S256())
 	}
 	if err != nil {
 		vm.dstack.PushBool(false)
@@ -1570,6 +1857,11 @@ func opcodeCheckSig(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeCheckSigVerify is a combination of opcodeCheckSig and opcodeVerify.
+// The opcodeCheckSig function is invoked followed by opcodeVerify.  See the
+// documentation for each of those opcodes for more details.
+//
+// Stack transformation: signature pubkey] -> [... bool] -> [...]
 func opcodeCheckSigVerify(op *parsedOpcode, vm *Engine) error {
 	err := opcodeCheckSig(op, vm)
 	if err == nil {
@@ -1587,7 +1879,25 @@ type parsedSigInfo struct {
 	parsed          bool
 }
 
-// stack; sigs <numsigs> pubkeys <numpubkeys>
+// opcodeCheckMultiSig treats the top item on the stack as an integer number of
+// public keys, followed by that many entries as raw data representing the public
+// keys, followed by the integer number of signatures, followed by that many
+// entries as raw data representing the signatures.
+//
+// Due to a bug in the original Satoshi client implementation, an additional
+// dummy argument is also required by the consensus rules, although it is not
+// used.  The dummy value SHOULD be an OP_0, although that is not required by
+// the consensus rules.  When the ScriptStrictMultiSig flag is set, it must be
+// OP_0.
+//
+// All of the aforementioned stack items are replaced with a bool which
+// indicates if the requisite number of signatures were successfully verified.
+//
+// See the opcodeCheckSigVerify documentation for more details about the process
+// for verifying each signature.
+//
+// Stack transformation:
+// [... dummy [sig ...] numsigs [pubkey ...] numpubkeys] -> [... bool]
 func opcodeCheckMultiSig(op *parsedOpcode, vm *Engine) error {
 	numKeys, err := vm.dstack.PopInt()
 	if err != nil {
@@ -1638,24 +1948,28 @@ func opcodeCheckMultiSig(op *parsedOpcode, vm *Engine) error {
 		signatures = append(signatures, sigInfo)
 	}
 
-	// bug in bitcoind means we pop one more stack value than should be
-	// used.
+	// A bug in the original Satoshi client implementation means one more
+	// stack value than should be used must be popped.  Unfortunately, this
+	// buggy behavior is now part of the consensus and a hard fork would be
+	// required to fix it.
 	dummy, err := vm.dstack.PopByteArray()
 	if err != nil {
 		return err
 	}
 
+	// Since the dummy argument is otherwise not checked, it could be any
+	// value which unfortunately provides a source of malleability.  Thus,
+	// there is a script flag to force an error when the value is NOT 0.
 	if vm.hasFlag(ScriptStrictMultiSig) && len(dummy) != 0 {
 		return fmt.Errorf("multisig dummy argument is not zero length: %d",
 			len(dummy))
 	}
 
-	// Trim OP_CODESEPARATORs
+	// Get script starting from the most recent OP_CODESEPARATOR.
 	script := vm.subScript()
 
-	// Remove any of the signatures that happen to be in the script.
-	// can't sign somthing containing the signature you're making, after
-	// all
+	// Remove any of the signatures since there is no way for a signature to
+	// sign itself.
 	for _, sigInfo := range signatures {
 		script = removeOpcodeByData(script, sigInfo.signature)
 	}
@@ -1738,7 +2052,8 @@ func opcodeCheckMultiSig(op *parsedOpcode, vm *Engine) error {
 			continue
 		}
 
-		hash := calcScriptHash(script, hashType, &vm.tx, vm.txIdx)
+		// Generate the signature hash based on the signature hash type.
+		hash := calcSignatureHash(script, hashType, &vm.tx, vm.txIdx)
 
 		if parsedSig.Verify(hash, parsedPubKey) {
 			// PubKey verified, move on to the next signature.
@@ -1751,6 +2066,12 @@ func opcodeCheckMultiSig(op *parsedOpcode, vm *Engine) error {
 	return nil
 }
 
+// opcodeCheckMultiSigVerify is a combination of opcodeCheckMultiSig and
+// opcodeVerify.  The opcodeCheckMultiSig is invoked followed by opcodeVerify.
+// See the documentation for each of those opcodes for more details.
+//
+// Stack transformation:
+// [... dummy [sig ...] numsigs [pubkey ...] numpubkeys] -> [... bool] -> [...]
 func opcodeCheckMultiSigVerify(op *parsedOpcode, vm *Engine) error {
 	err := opcodeCheckMultiSig(op, vm)
 	if err == nil {
diff --git a/txscript/script.go b/txscript/script.go
index 27ec9300..1f5db122 100644
--- a/txscript/script.go
+++ b/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
-// otherwise.
+// isPubkey returns true if the script passed is a pay-to-pubkey transaction,
+// 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,
-// false otherwise.
+// isPubkeyHash returns true if the script passed is a pay-to-pubkey-hash
+// 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)
-// transction, false otherwise.
+// isScriptHash returns true if the script passed is a pay-to-script-hash
+// 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
-		if len(pop.data) != 33 &&
-			len(pop.data) != 65 {
+		// Valid pubkeys are either 33 or 65 bytes.
+		if len(pop.data) != 33 && 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.
-		if pop.opcode.value < OP_FALSE ||
-			pop.opcode.value > OP_16 {
+		// All opcodes up to OP_16 are data push instructions.
+		// NOTE: This does consider OP_RESERVED to be a data push
+		// 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
-// parse then NonStandardTy will be returned.
-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.
+// removeOpcodeByData will return the script minus any opcodes that would push
+// the passed 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
-// 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) + " "
+// calcSignatureHash will, given a script and hash type for the current script
+// engine instance, calculate the signature hash to be used for signing and
+// verification.
+func calcSignatureHash(script []parsedOpcode, hashType SigHashType, tx *wire.MsgTx, idx int) []byte {
+	// The SigHashSingle signature type signs only the corresponding input
+	// and output (the output with the same index number as the input).
+	//
+	// Since transactions can have more inputs than outputs, this means it
+	// is improper to use SigHashSingle on input indices that don't have a
+	// 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
-// 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
+	// Remove all instances of OP_CODESEPARATOR from the script.
 	script = removeOpcode(script, OP_CODESEPARATOR)
 
-	// Make a deep copy of the transaction, zeroing out the script
-	// for all inputs that are not currently being processed.
+	// Make a deep copy of the transaction, zeroing out the script for all
+	// 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)
-			txCopy.TxIn[idx].SignatureScript = sigscript
+			sigScript, _ := unparseScript(script)
+			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
-		// SigHashAll for purposes of hash generation.
+		// Consensus treats undefined hashtypes like normal SigHashAll
+		// 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
-// 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 {
-	// 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 {
+// 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
 	}
 
-	shScript := sigPops[len(sigPops)-1].data
-	// Means that sigPops is jus OP_1 - OP_16, no sigops there.
-	if shScript == nil {
-		return 0
-	}
-
-	shPops, _ := parseScript(shScript)
-
-	return getSigOpCount(shPops, true)
+	return int(op.value - (OP_1 - 1))
 }
 
 // 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 -
-					(OP_1 - 1))
+				nSigs += asSmallInt(pops[i-1].opcode)
 			} 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
-// the number of keys provided.
+// for success.  An ErrBadNumRequired will be returned if nrequired is larger
+// 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
-// determined while finding out the type).
+// is the real class of pops (and we can thus assume things that were determined
+// 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
-	// on the sigScript.
+	// PkScriptClass is the class of the public key script and is equivalent
+	// 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
-	// pay-to-script-hash scripts. The number will be -1 if unknown.
+	// ExpectedInputs is the number of outputs required by the signature
+	// 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
-// are provided as arguments. It will error if the pair is in someway invalid
-// such that they can not be analysed, i.e. if they do not parse or the
-// pkScript is not a push-only script
-func CalcScriptInfo(sigscript, pkscript []byte, bip16 bool) (*ScriptInfo, error) {
+// CalcScriptInfo returns a structure providing data about the provided script
+// pair.  It will error if the pair is in someway invalid such that they can not
+// be analysed, i.e. if they do not parse or the pkScript is not a push-only
+// script
+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
-		// data.
+		// The pay-to-hash-script is the final data push of the
+		// 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, shClass)
+		shInputs := expectedInputs(shPops, typeOfScript(shPops))
 		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 {
diff --git a/txscript/script_test.go b/txscript/script_test.go
index af290679..d75f1ce2 100644
--- a/txscript/script_test.go
+++ b/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)
 		}
 	}
diff --git a/txscript/sign.go b/txscript/sign.go
index 9aa5e631..1c7ff44d 100644
--- a/txscript/sign.go
+++ b/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
diff --git a/txscript/stack.go b/txscript/stack.go
index 6f4c67e6..a8ad05a0 100644
--- a/txscript/stack.go
+++ b/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.
-func (s *stack) PeekInt(idx int) (i *big.Int, err error) {
+// PeekInt returns the Nth item on the stack as a bignum without removing it.
+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
+		return nil, ErrStackUnderflow
 	}
-	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(so1) // stack 1,2
-	s.PushByteArray(so2) // stack 2,1,2
+	s.PushByteArray(so2) // stack [... x2]
+	s.PushByteArray(so1) // stack [... x2 x1]
+	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
-// DropN(2): 1,2,3 -> 1
+//
+// Stack transformation:
+// 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
-// DupN(2): 1,2,3 -> 1,2,3,2,3
+//
+// Stack transformation:
+// 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
-	// stack.
+	// This leaves an in-order duplicate of the top n items on the 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
-// e.g.
-// RotN(1): 1,2,3 -> 2,3,1
+// RotN rotates the top 3N items on the stack to the left N times.
+//
+// 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
-// SwapN(2): 1,2,3,4 -> 3,4,1,2
+//
+// Stack transformation:
+// 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.
-// e.g.:
-// OverN(1): 1,2 -> 1,2,1
-// OverN(2): 1,2,3,4 -> 1,2,3,4,1,2
+// OverN copies N items N items back to the top of the stack.
+//
+// Stack transformation:
+// 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
-// PickN(2): 1,2,3 -> 1,2,3,1
+//
+// Stack transformation:
+// 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
-// RollN(2): 1,2,3 -> 2,3,1
+//
+// Stack transformation:
+// 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)
 	}