diff --git a/daemon/algod/api/server/v2/dryrun_test.go b/daemon/algod/api/server/v2/dryrun_test.go index 9ff33b541b..4a314608f5 100644 --- a/daemon/algod/api/server/v2/dryrun_test.go +++ b/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") } diff --git a/data/transactions/logic/README.md b/data/transactions/logic/README.md index fc2ec30766..850d02a3e1 100644 --- a/data/transactions/logic/README.md +++ b/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 | diff --git a/data/transactions/logic/README_in.md b/data/transactions/logic/README_in.md index f2e87a6140..6dfcfe9664 100644 --- a/data/transactions/logic/README_in.md +++ b/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 diff --git a/data/transactions/logic/TEAL_opcodes.md b/data/transactions/logic/TEAL_opcodes.md index cc842b5b11..cd2bd5842d 100644 --- a/data/transactions/logic/TEAL_opcodes.md +++ b/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 diff --git a/data/transactions/logic/doc.go b/data/transactions/logic/doc.go index 5790062cd7..243c22ec2c 100644 --- a/data/transactions/logic/doc.go +++ b/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", diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go index 700b79c612..34db841c12 100644 --- a/data/transactions/logic/eval.go +++ b/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)) { diff --git a/data/transactions/logic/langspec.json b/data/transactions/logic/langspec.json index 0b10ebdcb9..22bcd3f345 100644 --- a/data/transactions/logic/langspec.json +++ b/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"