txscript: Improve conditional stack.

This commit improves the way the conditional execution stack is handled in
a few ways.

First, the current execution state is now pushed onto the end of the slice
rather than the front of it.  This has been done because it results in
fewer allocations and is therefore more efficient.

Second, the need for allocating and setting an initial true in the
conditional stack has been eliminated.  The vast majority of scripts don't
contain any conditionals, so there is no reason to allocate a slice when
it isn't needed.

Third, a new function has been added to the engine to determine if the
current conditional branch is executing named isBranchExecuting which
handles the fact the conditional execution stack can now be empty and
improves the readability of the code.

Finally, it removes a couple of TODOs which I have verified do not apply.

txscript/engine.go

@@ -100,6 +100,17 @@ func (vm *Engine) hasFlag(flag ScriptFlags) bool {
 	return vm.flags&flag == flag
 }
 
+// isBranchExecuting returns whether or not the current conditional branch is
+// actively executing.  For example, when the data stack has an OP_FALSE on it
+// and an OP_IF is encountered, the branch is inactive until an OP_ELSE or
+// OP_ENDIF is encountered.  It properly handles nested conditionals.
+func (vm *Engine) isBranchExecuting() bool {
+	if len(vm.condStack) == 0 {
+		return true
+	}
+	return vm.condStack[len(vm.condStack)-1] == OpCondTrue
+}
+
 // Execute will execute all script in the script engine and return either nil
 // for successful validation or an error if one occurred.
 func (vm *Engine) Execute() (err error) {
@@ -195,7 +206,7 @@ func (vm *Engine) Step() (done bool, err error) {
 	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) != 1 {
+		if err == nil && len(vm.condStack) != 0 {
 			return false, ErrStackMissingEndif
 		}
 
@@ -531,7 +542,6 @@ func NewEngine(scriptPubKey []byte, tx *wire.MsgTx, txIdx int, flags ScriptFlags
 
 	vm.tx = *tx
 	vm.txIdx = txIdx
-	vm.condStack = []int{OpCondTrue}
 
 	return &vm, nil
 }

txscript/opcode.go

@@ -745,13 +745,13 @@ func (pop *parsedOpcode) exec(vm *Engine) error {
 
 	// If we are not a conditional opcode and we aren't executing, then
 	// we are done now.
-	if vm.condStack[0] != OpCondTrue && !pop.conditional() {
+	if !vm.isBranchExecuting() && !pop.conditional() {
 		return nil
 	}
 
 	// Ensure all executed data push opcodes use the minimal encoding when
 	// the minimal data verification is set.
-	if vm.dstack.verifyMinimalData && vm.condStack[0] == OpCondTrue &&
+	if vm.dstack.verifyMinimalData && vm.isBranchExecuting() &&
 		pop.opcode.value >= 0 && pop.opcode.value <= OP_PUSHDATA4 {
 		if err := pop.checkMinimalDataPush(); err != nil {
 			return err
@@ -896,22 +896,19 @@ func opcodeNop(op *parsedOpcode, vm *Engine) error {
 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
-	var condval int
+	condVal := OpCondFalse
-	if vm.condStack[0] == OpCondTrue {
+	if vm.isBranchExecuting() {
 		ok, err := vm.dstack.PopBool()
 		if err != nil {
 			return err
 		}
 		if ok {
-			condval = OpCondTrue
+			condVal = OpCondTrue
 		}
 	} else {
-		condval = OpCondSkip
+		condVal = OpCondSkip
 	}
-	cond := []int{condval}
+	vm.condStack = append(vm.condStack, condVal)
-	// push condition to the 'head' of the slice
-	vm.condStack = append(cond, vm.condStack...)
-	// TODO(drahn) check if a maximum condtitional stack limit exists
 	return nil
 }
 
@@ -920,37 +917,34 @@ func opcodeIf(op *parsedOpcode, vm *Engine) error {
 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
-	var condval int
+	condVal := OpCondFalse
-	if vm.condStack[0] == OpCondTrue {
+	if vm.isBranchExecuting() {
 		ok, err := vm.dstack.PopBool()
 		if err != nil {
 			return err
 		}
 		if !ok {
-			condval = OpCondTrue
+			condVal = OpCondTrue
 		}
 	} else {
-		condval = OpCondSkip
+		condVal = OpCondSkip
 	}
-	cond := []int{condval}
+	vm.condStack = append(vm.condStack, condVal)
-	// push condition to the 'head' of the slice
-	vm.condStack = append(cond, vm.condStack...)
-	// TODO(drahn) check if a maximum condtitional stack limit exists
 	return nil
 }
 
 // opcodeElse inverts conditional execution for other half of if/else/endif
 func opcodeElse(op *parsedOpcode, vm *Engine) error {
-	if len(vm.condStack) < 2 {
+	if len(vm.condStack) == 0 {
-		// intial true cannot be toggled, only pushed conditionals
 		return ErrStackNoIf
 	}
 
-	switch vm.condStack[0] {
+	conditionalIdx := len(vm.condStack) - 1
+	switch vm.condStack[conditionalIdx] {
 	case OpCondTrue:
-		vm.condStack[0] = OpCondFalse
+		vm.condStack[conditionalIdx] = OpCondFalse
 	case OpCondFalse:
-		vm.condStack[0] = OpCondTrue
+		vm.condStack[conditionalIdx] = OpCondTrue
 	case OpCondSkip:
 		// value doesn't change in skip
 	}
@@ -960,14 +954,11 @@ func opcodeElse(op *parsedOpcode, vm *Engine) error {
 // opcodeEndif terminates a conditional block, removing the  value from the
 // conditional execution stack.
 func opcodeEndif(op *parsedOpcode, vm *Engine) error {
-	if len(vm.condStack) < 2 {
+	if len(vm.condStack) == 0 {
-		// intial true cannot be popped, only pushed conditionals
 		return ErrStackNoIf
 	}
 
-	stk := make([]int, len(vm.condStack)-1, len(vm.condStack)-1)
+	vm.condStack = vm.condStack[:len(vm.condStack)-1]
-	copy(stk, vm.condStack[1:])
-	vm.condStack = stk
 	return nil
 }