AVM: fix early eval exits for Debugger

daemon/algod/api/server/v2/dryrun_test.go

@@ -388,7 +388,7 @@ func checkAppCallResponse(t *testing.T, response *generated.DryrunResponse, msg
 			if response.Txns[idx].AppCallMessages != nil {
 				messages := *response.Txns[idx].AppCallMessages
 				assert.GreaterOrEqual(t, len(messages), 1)
-				assert.Equal(t, msg, messages[len(messages)-1])
+				assert.Contains(t, messages[len(messages)-1], msg)
 			}
 		}
 	}
@@ -1092,8 +1092,8 @@ int 1`)
 	require.NoError(t, err)
 	approval := ops.Program
 	ops, err = logic.AssembleString("int 1")
-	clst := ops.Program
 	require.NoError(t, err)
+	clst := ops.Program
 	var appIdx basics.AppIndex = 1
 	creator := randomAddress()
 	sender := randomAddress()
@@ -1424,8 +1424,8 @@ int 1`
 	approval := ops.Program
 
 	ops, err = logic.AssembleString("int 1")
-	clst := ops.Program
 	require.NoError(t, err)
+	clst := ops.Program
 
 	sender, err := basics.UnmarshalChecksumAddress("47YPQTIGQEO7T4Y4RWDYWEKV6RTR2UNBQXBABEEGM72ESWDQNCQ52OPASU")
 	a.NoError(err)
@@ -1483,8 +1483,8 @@ int 0
 	require.NoError(t, err)
 	approval := ops.Program
 	ops, err = logic.AssembleString("int 1")
-	clst := ops.Program
 	require.NoError(t, err)
+	clst := ops.Program
 	var appIdx basics.AppIndex = 1
 	creator := randomAddress()
 	rewardBase := uint64(10000000)
@@ -1551,8 +1551,8 @@ int 1
 	require.NoError(t, err)
 
 	ops, err := logic.AssembleString("int 1")
-	clst := ops.Program
 	require.NoError(t, err)
+	clst := ops.Program
 
 	sender, err := basics.UnmarshalChecksumAddress("47YPQTIGQEO7T4Y4RWDYWEKV6RTR2UNBQXBABEEGM72ESWDQNCQ52OPASU")
 	a.NoError(err)
@@ -1628,8 +1628,8 @@ int 1`)
 	require.NoError(t, err)
 
 	ops, err := logic.AssembleString("int 1")
-	clst := ops.Program
 	require.NoError(t, err)
+	clst := ops.Program
 
 	sender, err := basics.UnmarshalChecksumAddress("47YPQTIGQEO7T4Y4RWDYWEKV6RTR2UNBQXBABEEGM72ESWDQNCQ52OPASU")
 	a.NoError(err)
@@ -1732,7 +1732,7 @@ func TestDryrunCheckEvalDeltasReturned(t *testing.T) {
 
 	// Test that a PASS and REJECT dryrun both return the dryrun evaldelta.
 	for i := range []int{0, 1} {
-		ops, _ := logic.AssembleString(fmt.Sprintf(`
+		ops, err := logic.AssembleString(fmt.Sprintf(`
 #pragma version 6
 txna ApplicationArgs 0
 txna ApplicationArgs 1
@@ -1742,6 +1742,7 @@ txna ApplicationArgs 0
 int %d
 app_local_put
 int %d`, expectedUint, i))
+		require.NoError(t, err)
 		dr.ProtocolVersion = string(dryrunProtoVersion)
 
 		dr.Txns = []transactions.SignedTxn{
@@ -1793,5 +1794,41 @@ int %d`, expectedUint, i))
 			logResponse(t, &response)
 		}
 	}
+}
 
+// TestDryrunEarlyExit is a regression test. Ensures that we no longer exit so
+// early in eval() that problems are caused by the debugState being nil.
+func TestDryrunEarlyExit(t *testing.T) {
+	partitiontest.PartitionTest(t)
+	t.Parallel()
+
+	var dr DryrunRequest
+	var response generated.DryrunResponse
+
+	ops, err := logic.AssembleString("#pragma version 5 \n int 1")
+	require.NoError(t, err)
+	dr.ProtocolVersion = string(dryrunProtoVersion)
+
+	dr.Txns = []transactions.SignedTxn{
+		txntest.Txn{
+			ApplicationID: 1,
+			Type:          protocol.ApplicationCallTx,
+		}.SignedTxn(),
+	}
+	dr.Apps = []generated.Application{{
+		Id: 1,
+		Params: generated.ApplicationParams{
+			ApprovalProgram: ops.Program,
+		},
+	}}
+	dr.Accounts = []generated.Account{{
+		Status:  "Online",
+		Address: basics.Address{}.String(),
+	}}
+	doDryrunRequest(&dr, &response)
+	checkAppCallPass(t, &response)
+
+	ops.Program[0] = 100 // version too high
+	doDryrunRequest(&dr, &response)
+	checkAppCallResponse(t, &response, "program version 100 greater than max")
 }

data/transactions/logic/README.md

@@ -30,8 +30,15 @@ arguments from it and pushing results to it. Some operations have
 _immediate_ arguments that are encoded directly into the instruction,
 rather than coming from the stack.
 
-The maximum stack depth is 1000. If the stack depth is
-exceeded or if a byte-array element exceed 4096 bytes, the program fails.
+The maximum stack depth is 1000. If the stack depth is exceeded or if
+a byte-array element exceeds 4096 bytes, the program fails. If an
+opcode is documented to access a position in the stack that does not
+exist, the operation fails. Most often, this is an attempt to access
+an element below the stack -- the simplest example is an operation
+like `concat` which expects two arguments on the stack. If the stack
+has fewer than two elements, the operation fails. Some operations, like
+`frame_dig` and `proto` could fail because of an attempt to access
+above the current stack.
 
 ## Scratch Space
 
@@ -188,12 +195,12 @@ _available_.
    `txn.ForeignApplications` field is _available_.
 
  * A Box is _available_ to an Approval Program if _any_ transaction in
-   the same group contains a box reference that denotes the box. The
-   index `i` in the box reference refers to the `ith` application in
-   the containing transaction's ForeignApplications array, with the
-   usual convention that 0 indicates the application ID of the app
-   called by that transaction. No box is ever _available_ to a
-   ClearStateProgram.
+   the same group contains a box reference (`txn.Boxes`) that denotes
+   the box. A box reference contains an index `i`, and name `n`. The
+   index refers to the `ith` application in the transaction's
+   ForeignApplications array, with the usual convention that 0
+   indicates the application ID of the app called by that
+   transaction. No box is ever _available_ to a ClearStateProgram.
 
 ## Constants
 
@@ -632,8 +639,8 @@ Account fields used in the `acct_params_get` opcode.
 
 | Opcode | Description |
 | - | -- |
-| `balance` | balance for account A, in microalgos. The balance is observed after the effects of previous transactions in the group, and after the fee for the current transaction is deducted. |
-| `min_balance` | minimum required balance for account A, in microalgos. Required balance is affected by [ASA](https://developer.algorand.org/docs/features/asa/#assets-overview) and [App](https://developer.algorand.org/docs/features/asc1/stateful/#minimum-balance-requirement-for-a-smart-contract) usage. When creating or opting into an app, the minimum balance grows before the app code runs, therefore the increase is visible there. When deleting or closing out, the minimum balance decreases after the app executes. |
+| `balance` | balance for account A, in microalgos. The balance is observed after the effects of previous transactions in the group, and after the fee for the current transaction is deducted. Changes caused by inner transactions are observable immediately following `itxn_submit` |
+| `min_balance` | minimum required balance for account A, in microalgos. Required balance is affected by ASA, App, and Box usage. When creating or opting into an app, the minimum balance grows before the app code runs, therefore the increase is visible there. When deleting or closing out, the minimum balance decreases after the app executes. Changes caused by inner transactions or box usage are observable immediately following the opcode effecting the change. |
 | `app_opted_in` | 1 if account A is opted in to application B, else 0 |
 | `app_local_get` | local state of the key B in the current application in account A |
 | `app_local_get_ex` | X is the local state of application B, key C in account A. Y is 1 if key existed, else 0 |

data/transactions/logic/README_in.md

@@ -30,8 +30,15 @@ arguments from it and pushing results to it. Some operations have
 _immediate_ arguments that are encoded directly into the instruction,
 rather than coming from the stack.
 
-The maximum stack depth is 1000. If the stack depth is
-exceeded or if a byte-array element exceed 4096 bytes, the program fails.
+The maximum stack depth is 1000. If the stack depth is exceeded or if
+a byte-array element exceeds 4096 bytes, the program fails. If an
+opcode is documented to access a position in the stack that does not
+exist, the operation fails. Most often, this is an attempt to access
+an element below the stack -- the simplest example is an operation
+like `concat` which expects two arguments on the stack. If the stack
+has fewer than two elements, the operation fails. Some operations, like
+`frame_dig` and `proto` could fail because of an attempt to access
+above the current stack.
 
 ## Scratch Space
 
@@ -188,12 +195,12 @@ _available_.
    `txn.ForeignApplications` field is _available_.
 
  * A Box is _available_ to an Approval Program if _any_ transaction in
-   the same group contains a box reference that denotes the box. The
-   index `i` in the box reference refers to the `ith` application in
-   the containing transaction's ForeignApplications array, with the
-   usual convention that 0 indicates the application ID of the app
-   called by that transaction. No box is ever _available_ to a
-   ClearStateProgram.
+   the same group contains a box reference (`txn.Boxes`) that denotes
+   the box. A box reference contains an index `i`, and name `n`. The
+   index refers to the `ith` application in the transaction's
+   ForeignApplications array, with the usual convention that 0
+   indicates the application ID of the app called by that
+   transaction. No box is ever _available_ to a ClearStateProgram.
 
 ## Constants
 

data/transactions/logic/TEAL_opcodes.md

@@ -834,7 +834,7 @@ Almost all smart contracts should use simpler and smaller methods (such as the [
 
 - Opcode: 0x60
 - Stack: ..., A → ..., uint64
-- balance for account A, in microalgos. The balance is observed after the effects of previous transactions in the group, and after the fee for the current transaction is deducted.
+- balance for account A, in microalgos. The balance is observed after the effects of previous transactions in the group, and after the fee for the current transaction is deducted. Changes caused by inner transactions are observable immediately following `itxn_submit`
 - Availability: v2
 - Mode: Application
 
@@ -1033,7 +1033,7 @@ params: Txn.ForeignApps offset or an _available_ app id. Return: did_exist flag
 
 - Opcode: 0x78
 - Stack: ..., A → ..., uint64
-- minimum required balance for account A, in microalgos. Required balance is affected by [ASA](https://developer.algorand.org/docs/features/asa/#assets-overview) and [App](https://developer.algorand.org/docs/features/asc1/stateful/#minimum-balance-requirement-for-a-smart-contract) usage. When creating or opting into an app, the minimum balance grows before the app code runs, therefore the increase is visible there. When deleting or closing out, the minimum balance decreases after the app executes.
+- minimum required balance for account A, in microalgos. Required balance is affected by ASA, App, and Box usage. When creating or opting into an app, the minimum balance grows before the app code runs, therefore the increase is visible there. When deleting or closing out, the minimum balance decreases after the app executes. Changes caused by inner transactions or box usage are observable immediately following the opcode effecting the change.
 - Availability: v3
 - Mode: Application
 

data/transactions/logic/doc.go

@@ -151,8 +151,8 @@ var opDocByName = map[string]string{
 	"replace2":          "Copy of A with the bytes starting at S replaced by the bytes of B. Fails if S+len(B) exceeds len(A)",
 	"replace3":          "Copy of A with the bytes starting at B replaced by the bytes of C. Fails if B+len(C) exceeds len(A)",
 	"base64_decode":     "decode A which was base64-encoded using _encoding_ E. Fail if A is not base64 encoded with encoding E",
-	"balance":           "balance for account A, in microalgos. The balance is observed after the effects of previous transactions in the group, and after the fee for the current transaction is deducted.",
-	"min_balance":       "minimum required balance for account A, in microalgos. Required balance is affected by [ASA](https://developer.algorand.org/docs/features/asa/#assets-overview) and [App](https://developer.algorand.org/docs/features/asc1/stateful/#minimum-balance-requirement-for-a-smart-contract) usage. When creating or opting into an app, the minimum balance grows before the app code runs, therefore the increase is visible there. When deleting or closing out, the minimum balance decreases after the app executes.",
+	"balance":           "balance for account A, in microalgos. The balance is observed after the effects of previous transactions in the group, and after the fee for the current transaction is deducted. Changes caused by inner transactions are observable immediately following `itxn_submit`",
+	"min_balance":       "minimum required balance for account A, in microalgos. Required balance is affected by ASA, App, and Box usage. When creating or opting into an app, the minimum balance grows before the app code runs, therefore the increase is visible there. When deleting or closing out, the minimum balance decreases after the app executes. Changes caused by inner transactions or box usage are observable immediately following the opcode effecting the change.",
 	"app_opted_in":      "1 if account A is opted in to application B, else 0",
 	"app_local_get":     "local state of the key B in the current application in account A",
 	"app_local_get_ex":  "X is the local state of application B, key C in account A. Y is 1 if key existed, else 0",

data/transactions/logic/eval.go

@@ -810,29 +810,11 @@ func eval(program []byte, cx *EvalContext) (pass bool, err error) {
 		}
 	}()
 
-	defer func() {
-		// Ensure we update the debugger before exiting
-		if cx.Debugger != nil {
-			errDbg := cx.Debugger.Complete(cx.refreshDebugState(err))
-			if err == nil {
-				err = errDbg
-			}
-		}
-	}()
-
-	if (cx.EvalParams.Proto == nil) || (cx.EvalParams.Proto.LogicSigVersion == 0) {
-		err = errLogicSigNotSupported
-		return
-	}
-	if cx.txn.Lsig.Args != nil && len(cx.txn.Lsig.Args) > transactions.EvalMaxArgs {
-		err = errTooManyArgs
-		return
-	}
+	// Avoid returning for any reason until after cx.debugState is setup. That
+	// require cx to be minimally setup, too.
 
-	version, vlen, err := versionCheck(program, cx.EvalParams)
-	if err != nil {
-		return false, err
-	}
+	version, vlen, verr := versionCheck(program, cx.EvalParams)
+	// defer verr check until after cx and debugState is setup
 
 	cx.version = version
 	cx.pc = vlen
@@ -848,6 +830,23 @@ func eval(program []byte, cx *EvalContext) (pass bool, err error) {
 		if derr := cx.Debugger.Register(cx.refreshDebugState(err)); derr != nil {
 			return false, derr
 		}
+		defer func() {
+			// Ensure we update the debugger before exiting
+			errDbg := cx.Debugger.Complete(cx.refreshDebugState(err))
+			if err == nil {
+				err = errDbg
+			}
+		}()
+	}
+
+	if (cx.EvalParams.Proto == nil) || (cx.EvalParams.Proto.LogicSigVersion == 0) {
+		return false, errLogicSigNotSupported
+	}
+	if cx.txn.Lsig.Args != nil && len(cx.txn.Lsig.Args) > transactions.EvalMaxArgs {
+		return false, errTooManyArgs
+	}
+	if verr != nil {
+		return false, verr
 	}
 
 	for (err == nil) && (cx.pc < len(cx.program)) {

data/transactions/logic/langspec.json

@@ -1342,7 +1342,7 @@
       "Args": ".",
       "Returns": "U",
       "Size": 1,
-      "Doc": "balance for account A, in microalgos. The balance is observed after the effects of previous transactions in the group, and after the fee for the current transaction is deducted.",
+      "Doc": "balance for account A, in microalgos. The balance is observed after the effects of previous transactions in the group, and after the fee for the current transaction is deducted. Changes caused by inner transactions are observable immediately following `itxn_submit`",
       "DocExtra": "params: Txn.Accounts offset (or, since v4, an _available_ account address), _available_ application id (or, since v4, a Txn.ForeignApps offset). Return: value.",
       "Groups": [
         "State Access"
@@ -1555,7 +1555,7 @@
       "Args": ".",
       "Returns": "U",
       "Size": 1,
-      "Doc": "minimum required balance for account A, in microalgos. Required balance is affected by [ASA](https://developer.algorand.org/docs/features/asa/#assets-overview) and [App](https://developer.algorand.org/docs/features/asc1/stateful/#minimum-balance-requirement-for-a-smart-contract) usage. When creating or opting into an app, the minimum balance grows before the app code runs, therefore the increase is visible there. When deleting or closing out, the minimum balance decreases after the app executes.",
+      "Doc": "minimum required balance for account A, in microalgos. Required balance is affected by ASA, App, and Box usage. When creating or opting into an app, the minimum balance grows before the app code runs, therefore the increase is visible there. When deleting or closing out, the minimum balance decreases after the app executes. Changes caused by inner transactions or box usage are observable immediately following the opcode effecting the change.",
       "DocExtra": "params: Txn.Accounts offset (or, since v4, an _available_ account address), _available_ application id (or, since v4, a Txn.ForeignApps offset). Return: value.",
       "Groups": [
         "State Access"