Only allow access to txn effects for previous transactions (#3529)

* Only allow access to txn effects for previous transactions

* JP code review

* Ok, dawg

4 files changed, 186 insertions, 213 deletions

diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go
index c4dabbcf2..d06d3f1d1 100644
--- a/data/transactions/logic/eval.go
+++ b/data/transactions/logic/eval.go
@@ -490,20 +490,22 @@ type EvalContext struct {
 // StackType describes the type of a value on the operand stack
 type StackType byte
 
-// StackTypes is an alias for a list of StackType with syntactic sugar
-type StackTypes []StackType
+const (
+	// StackNone in an OpSpec shows that the op pops or yields nothing
+	StackNone StackType = iota
 
-// StackNone in an OpSpec shows that the op pops or yields nothing
-const StackNone StackType = 0
+	// StackAny in an OpSpec shows that the op pops or yield any type
+	StackAny StackType = iota
 
-// StackAny in an OpSpec shows that the op pops or yield any type
-const StackAny StackType = 1
+	// StackUint64 in an OpSpec shows that the op pops or yields a uint64
+	StackUint64 StackType = iota
 
-// StackUint64 in an OpSpec shows that the op pops or yields a uint64
-const StackUint64 StackType = 2
+	// StackBytes in an OpSpec shows that the op pops or yields a []byte
+	StackBytes StackType = iota
+)
 
-// StackBytes in an OpSpec shows that the op pops or yields a []byte
-const StackBytes StackType = 3
+// StackTypes is an alias for a list of StackType with syntactic sugar
+type StackTypes []StackType
 
 func (st StackType) String() string {
 	switch st {
@@ -2267,159 +2269,170 @@ func (cx *EvalContext) fetchField(field TxnField, expectArray bool) (*txnFieldSp
 	return &fs, nil
 }
 
-func opTxn(cx *EvalContext) {
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+1]), false)
+type txnSource int
+
+const (
+	srcGroup      txnSource = iota
+	srcInner                = iota
+	srcInnerGroup           = iota
+)
+
+// opTxnImpl implements all of the txn variants.  Each form of txn opcode should
+// be able to get its work done with one call here, after collecting the args in
+// the most straightforward way possible. They ought to do no error checking, so
+// that it is all collected here.
+func (cx *EvalContext) opTxnImpl(gi uint64, src txnSource, field TxnField, ai uint64, expectArray bool) (sv stackValue, err error) {
+	fs, err := cx.fetchField(field, expectArray)
 	if err != nil {
-		cx.err = err
-		return
+		return sv, err
 	}
 
-	sv, err := cx.txnFieldToStack(cx.Txn, fs, 0, cx.GroupIndex, false)
-	if err != nil {
-		cx.err = err
-		return
+	var group []transactions.SignedTxnWithAD
+	switch src {
+	case srcGroup:
+		if fs.effects && gi >= uint64(cx.GroupIndex) {
+			// Test mode so that error is clearer
+			if cx.runModeFlags == runModeSignature {
+				return sv, fmt.Errorf("txn[%s] not allowed in current mode", fs.field)
+			}
+			return sv, fmt.Errorf("txn effects can only be read from past txns %d %d", gi, cx.GroupIndex)
+		}
+		group = cx.TxnGroup
+	case srcInner:
+		group = cx.getLastInner()
+	case srcInnerGroup:
+		group = cx.getLastInnerGroup()
 	}
-	cx.stack = append(cx.stack, sv)
-}
 
-func opTxna(cx *EvalContext) {
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+1]), true)
+	// We cast the length up, rather than gi down, in case gi overflows `int`.
+	if gi >= uint64(len(group)) {
+		return sv, fmt.Errorf("txn index %d, len(group) is %d", gi, len(group))
+	}
+	tx := &group[gi]
+
+	// int(gi) is safe because gi < len(group). Slices in Go cannot exceed `int`
+	sv, err = cx.txnFieldToStack(tx, fs, ai, int(gi), src != srcGroup)
 	if err != nil {
-		cx.err = err
-		return
+		return sv, err
 	}
 
-	arrayFieldIdx := uint64(cx.program[cx.pc+2])
-	sv, err := cx.txnFieldToStack(cx.Txn, fs, arrayFieldIdx, cx.GroupIndex, false)
+	return sv, nil
+}
+
+func opTxn(cx *EvalContext) {
+	gi := uint64(cx.GroupIndex)
+	field := TxnField(cx.program[cx.pc+1])
+
+	sv, err := cx.opTxnImpl(gi, srcGroup, field, 0, false)
 	if err != nil {
 		cx.err = err
 		return
 	}
+
 	cx.stack = append(cx.stack, sv)
 }
 
-func opTxnas(cx *EvalContext) {
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+1]), true)
+func opTxna(cx *EvalContext) {
+	gi := uint64(cx.GroupIndex)
+	field := TxnField(cx.program[cx.pc+1])
+	ai := uint64(cx.program[cx.pc+2])
+
+	sv, err := cx.opTxnImpl(gi, srcGroup, field, ai, true)
 	if err != nil {
 		cx.err = err
 		return
 	}
 
+	cx.stack = append(cx.stack, sv)
+}
+
+func opTxnas(cx *EvalContext) {
 	last := len(cx.stack) - 1
-	arrayFieldIdx := cx.stack[last].Uint
-	sv, err := cx.txnFieldToStack(cx.Txn, fs, arrayFieldIdx, cx.GroupIndex, false)
+
+	gi := uint64(cx.GroupIndex)
+	field := TxnField(cx.program[cx.pc+1])
+	ai := cx.stack[last].Uint
+
+	sv, err := cx.opTxnImpl(gi, srcGroup, field, ai, true)
 	if err != nil {
 		cx.err = err
 		return
 	}
+
 	cx.stack[last] = sv
 }
 
 func opGtxn(cx *EvalContext) {
-	gi := cx.program[cx.pc+1]
-	if int(gi) >= len(cx.TxnGroup) {
-		cx.err = fmt.Errorf("gtxn lookup TxnGroup[%d] but it only has %d", gi, len(cx.TxnGroup))
-		return
-	}
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+2]), false)
-	if err != nil {
-		cx.err = err
-		return
-	}
+	gi := uint64(cx.program[cx.pc+1])
+	field := TxnField(cx.program[cx.pc+2])
 
-	tx := &cx.TxnGroup[gi]
-	sv, err := cx.txnFieldToStack(tx, fs, 0, int(gi), false)
+	sv, err := cx.opTxnImpl(gi, srcGroup, field, 0, false)
 	if err != nil {
 		cx.err = err
 		return
 	}
+
 	cx.stack = append(cx.stack, sv)
 }
 
 func opGtxna(cx *EvalContext) {
-	gi := cx.program[cx.pc+1]
-	if int(gi) >= len(cx.TxnGroup) {
-		cx.err = fmt.Errorf("gtxna lookup TxnGroup[%d] but it only has %d", gi, len(cx.TxnGroup))
-		return
-	}
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+2]), true)
-	if err != nil {
-		cx.err = err
-		return
-	}
-	arrayFieldIdx := uint64(cx.program[cx.pc+3])
-	tx := &cx.TxnGroup[gi]
-	sv, err := cx.txnFieldToStack(tx, fs, arrayFieldIdx, int(gi), false)
+	gi := uint64(cx.program[cx.pc+1])
+	field := TxnField(cx.program[cx.pc+2])
+	ai := uint64(cx.program[cx.pc+3])
+
+	sv, err := cx.opTxnImpl(gi, srcGroup, field, ai, true)
 	if err != nil {
 		cx.err = err
 		return
 	}
+
 	cx.stack = append(cx.stack, sv)
 }
 
 func opGtxnas(cx *EvalContext) {
 	last := len(cx.stack) - 1
 
-	gi := cx.program[cx.pc+1]
-	if int(gi) >= len(cx.TxnGroup) {
-		cx.err = fmt.Errorf("gtxnas lookup TxnGroup[%d] but it only has %d", gi, len(cx.TxnGroup))
-		return
-	}
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+2]), true)
-	if err != nil {
-		cx.err = err
-		return
-	}
-	arrayFieldIdx := cx.stack[last].Uint
-	tx := &cx.TxnGroup[gi]
-	sv, err := cx.txnFieldToStack(tx, fs, arrayFieldIdx, int(gi), false)
+	gi := uint64(cx.program[cx.pc+1])
+	field := TxnField(cx.program[cx.pc+2])
+	ai := cx.stack[last].Uint
+
+	sv, err := cx.opTxnImpl(gi, srcGroup, field, ai, true)
 	if err != nil {
 		cx.err = err
 		return
 	}
+
 	cx.stack[last] = sv
 }
 
 func opGtxns(cx *EvalContext) {
 	last := len(cx.stack) - 1
+
 	gi := cx.stack[last].Uint
-	if gi >= uint64(len(cx.TxnGroup)) {
-		cx.err = fmt.Errorf("gtxns lookup TxnGroup[%d] but it only has %d", gi, len(cx.TxnGroup))
-		return
-	}
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+1]), false)
-	if err != nil {
-		cx.err = err
-		return
-	}
-	tx := &cx.TxnGroup[gi]
-	sv, err := cx.txnFieldToStack(tx, fs, 0, int(gi), false)
+	field := TxnField(cx.program[cx.pc+1])
+
+	sv, err := cx.opTxnImpl(gi, srcGroup, field, 0, false)
 	if err != nil {
 		cx.err = err
 		return
 	}
+
 	cx.stack[last] = sv
 }
 
 func opGtxnsa(cx *EvalContext) {
 	last := len(cx.stack) - 1
+
 	gi := cx.stack[last].Uint
-	if gi >= uint64(len(cx.TxnGroup)) {
-		cx.err = fmt.Errorf("gtxnsa lookup TxnGroup[%d] but it only has %d", gi, len(cx.TxnGroup))
-		return
-	}
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+1]), true)
-	if err != nil {
-		cx.err = err
-		return
-	}
-	arrayFieldIdx := uint64(cx.program[cx.pc+2])
-	tx := &cx.TxnGroup[gi]
-	sv, err := cx.txnFieldToStack(tx, fs, arrayFieldIdx, int(gi), false)
+	field := TxnField(cx.program[cx.pc+1])
+	ai := uint64(cx.program[cx.pc+2])
+
+	sv, err := cx.opTxnImpl(gi, srcGroup, field, ai, true)
 	if err != nil {
 		cx.err = err
 		return
 	}
+
 	cx.stack[last] = sv
 }
 
@@ -2428,40 +2441,23 @@ func opGtxnsas(cx *EvalContext) {
 	prev := last - 1
 
 	gi := cx.stack[prev].Uint
-	if gi >= uint64(len(cx.TxnGroup)) {
-		cx.err = fmt.Errorf("gtxnsas lookup TxnGroup[%d] but it only has %d", gi, len(cx.TxnGroup))
-		return
-	}
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+1]), true)
-	if err != nil {
-		cx.err = err
-		return
-	}
-	arrayFieldIdx := cx.stack[last].Uint
-	tx := &cx.TxnGroup[gi]
-	sv, err := cx.txnFieldToStack(tx, fs, arrayFieldIdx, int(gi), false)
+	field := TxnField(cx.program[cx.pc+1])
+	ai := cx.stack[last].Uint
+
+	sv, err := cx.opTxnImpl(gi, srcGroup, field, ai, true)
 	if err != nil {
 		cx.err = err
 		return
 	}
+
 	cx.stack[prev] = sv
 	cx.stack = cx.stack[:last]
 }
 
 func opItxn(cx *EvalContext) {
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+1]), false)
-	if err != nil {
-		cx.err = err
-		return
-	}
-
-	if len(cx.Txn.EvalDelta.InnerTxns) == 0 {
-		cx.err = fmt.Errorf("no inner transaction available %d", fs.field)
-		return
-	}
+	field := TxnField(cx.program[cx.pc+1])
 
-	itxn := &cx.Txn.EvalDelta.InnerTxns[len(cx.Txn.EvalDelta.InnerTxns)-1]
-	sv, err := cx.txnFieldToStack(itxn, fs, 0, 0, true)
+	sv, err := cx.opTxnImpl(0, srcInner, field, 0, false)
 	if err != nil {
 		cx.err = err
 		return
@@ -2470,51 +2466,42 @@ func opItxn(cx *EvalContext) {
 }
 
 func opItxna(cx *EvalContext) {
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+1]), true)
-	if err != nil {
-		cx.err = err
-		return
-	}
-	arrayFieldIdx := uint64(cx.program[cx.pc+2])
-
-	if len(cx.Txn.EvalDelta.InnerTxns) == 0 {
-		cx.err = fmt.Errorf("no inner transaction available %d", fs.field)
-		return
-	}
+	field := TxnField(cx.program[cx.pc+1])
+	ai := uint64(cx.program[cx.pc+2])
 
-	itxn := &cx.Txn.EvalDelta.InnerTxns[len(cx.Txn.EvalDelta.InnerTxns)-1]
-	sv, err := cx.txnFieldToStack(itxn, fs, arrayFieldIdx, 0, true)
+	sv, err := cx.opTxnImpl(0, srcInner, field, ai, true)
 	if err != nil {
 		cx.err = err
 		return
 	}
+
 	cx.stack = append(cx.stack, sv)
 }
 
 func opItxnas(cx *EvalContext) {
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+1]), true)
-	if err != nil {
-		cx.err = err
-		return
-	}
-
 	last := len(cx.stack) - 1
-	arrayFieldIdx := cx.stack[last].Uint
 
-	if len(cx.Txn.EvalDelta.InnerTxns) == 0 {
-		cx.err = fmt.Errorf("no inner transaction available %d", fs.field)
-		return
-	}
+	field := TxnField(cx.program[cx.pc+1])
+	ai := cx.stack[last].Uint
 
-	itxn := &cx.Txn.EvalDelta.InnerTxns[len(cx.Txn.EvalDelta.InnerTxns)-1]
-	sv, err := cx.txnFieldToStack(itxn, fs, arrayFieldIdx, 0, true)
+	sv, err := cx.opTxnImpl(0, srcInner, field, ai, true)
 	if err != nil {
 		cx.err = err
 		return
 	}
+
 	cx.stack[last] = sv
 }
 
+func (cx *EvalContext) getLastInner() []transactions.SignedTxnWithAD {
+	inners := cx.Txn.EvalDelta.InnerTxns
+	// If there are no inners yet, return empty slice, which will result in error
+	if len(inners) == 0 {
+		return inners
+	}
+	return inners[len(inners)-1:]
+}
+
 func (cx *EvalContext) getLastInnerGroup() []transactions.SignedTxnWithAD {
 	inners := cx.Txn.EvalDelta.InnerTxns
 	// If there are no inners yet, return empty slice, which will result in error
@@ -2537,72 +2524,45 @@ func (cx *EvalContext) getLastInnerGroup() []transactions.SignedTxnWithAD {
 }
 
 func opGitxn(cx *EvalContext) {
-	lastInnerGroup := cx.getLastInnerGroup()
-	gi := cx.program[cx.pc+1]
-	if int(gi) >= len(lastInnerGroup) {
-		cx.err = fmt.Errorf("gitxn %d ... but last group has %d", gi, len(lastInnerGroup))
-		return
-	}
-	itxn := &lastInnerGroup[gi]
+	gi := uint64(cx.program[cx.pc+1])
+	field := TxnField(cx.program[cx.pc+2])
 
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+2]), false)
+	sv, err := cx.opTxnImpl(gi, srcInnerGroup, field, 0, false)
 	if err != nil {
 		cx.err = err
 		return
 	}
 
-	sv, err := cx.txnFieldToStack(itxn, fs, 0, int(gi), true)
-	if err != nil {
-		cx.err = err
-		return
-	}
 	cx.stack = append(cx.stack, sv)
 }
 
 func opGitxna(cx *EvalContext) {
-	lastInnerGroup := cx.getLastInnerGroup()
-	gi := int(cx.program[cx.pc+1])
-	if gi >= len(lastInnerGroup) {
-		cx.err = fmt.Errorf("gitxna %d ... but last group has %d", gi, len(lastInnerGroup))
-		return
-	}
-	itxn := &lastInnerGroup[gi]
+	gi := uint64(cx.program[cx.pc+1])
+	field := TxnField(cx.program[cx.pc+2])
+	ai := uint64(cx.program[cx.pc+3])
 
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+2]), true)
-	if err != nil {
-		cx.err = err
-		return
-	}
-	arrayFieldIdx := uint64(cx.program[cx.pc+3])
-	sv, err := cx.txnFieldToStack(itxn, fs, arrayFieldIdx, gi, true)
+	sv, err := cx.opTxnImpl(gi, srcInnerGroup, field, ai, true)
 	if err != nil {
 		cx.err = err
 		return
 	}
+
 	cx.stack = append(cx.stack, sv)
 }
 
 func opGitxnas(cx *EvalContext) {
-	lastInnerGroup := cx.getLastInnerGroup()
-	gi := int(cx.program[cx.pc+1])
-	if gi >= len(lastInnerGroup) {
-		cx.err = fmt.Errorf("gitxnas %d ... but last group has %d", gi, len(lastInnerGroup))
-		return
-	}
-	itxn := &lastInnerGroup[gi]
-
-	fs, err := cx.fetchField(TxnField(cx.program[cx.pc+2]), true)
-	if err != nil {
-		cx.err = err
-		return
-	}
 	last := len(cx.stack) - 1
-	arrayFieldIdx := cx.stack[last].Uint
-	sv, err := cx.txnFieldToStack(itxn, fs, arrayFieldIdx, gi, true)
+
+	gi := uint64(cx.program[cx.pc+1])
+	field := TxnField(cx.program[cx.pc+2])
+	ai := cx.stack[last].Uint
+
+	sv, err := cx.opTxnImpl(gi, srcInnerGroup, field, ai, true)
 	if err != nil {
 		cx.err = err
 		return
 	}
+
 	cx.stack[last] = sv
 }
 
@@ -2646,7 +2606,9 @@ func opGaid(cx *EvalContext) {
 
 func opGaids(cx *EvalContext) {
 	last := len(cx.stack) - 1
+
 	gi := cx.stack[last].Uint
+
 	if gi >= uint64(len(cx.TxnGroup)) {
 		cx.err = fmt.Errorf("gaids lookup TxnGroup[%d] but it only has %d", gi, len(cx.TxnGroup))
 		return
diff --git a/data/transactions/logic/evalStateful_test.go b/data/transactions/logic/evalStateful_test.go
index 729d5a606..9bacb5ae3 100644
--- a/data/transactions/logic/evalStateful_test.go
+++ b/data/transactions/logic/evalStateful_test.go
@@ -2259,6 +2259,10 @@ func TestReturnTypes(t *testing.T) {
 		[]byte("aoeu2"),
 		[]byte("aoeu3"),
 	}
+	// We are going to run with GroupIndex=1, so make tx1 interesting too (so
+	// txn can look at things)
+	ep.TxnGroup[1] = ep.TxnGroup[0]
+
 	ep.pastScratch[0] = &scratchSpace{} // for gload
 	ledger.NewAccount(tx.Sender, 1)
 	params := basics.AssetParams{
@@ -2366,29 +2370,35 @@ func TestReturnTypes(t *testing.T) {
 				source := sb.String()
 				ops := testProg(t, source, AssemblerMaxVersion)
 
-				var cx EvalContext
-				cx.EvalParams = ep
-				cx.runModeFlags = m
-				cx.appID = 1
+				// Setup as if evaluation is in tx1, since we want to test gaid
+				// that must look back.
+				cx := EvalContext{
+					EvalParams:   ep,
+					runModeFlags: m,
+					GroupIndex:   1,
+					Txn:          &ep.TxnGroup[1],
+					appID:        1,
+				}
 
 				// These set conditions for some ops that examine the group.
 				// This convinces them all to work.  Revisit.
-				cx.Txn = &ep.TxnGroup[0]
-				cx.GroupIndex = 1
 				cx.TxnGroup[0].ConfigAsset = 100
 
 				eval(ops.Program, &cx)
 
-				require.Equal(
+				assert.Equal(
 					t,
 					len(spec.Returns), len(cx.stack),
 					fmt.Sprintf("\n%s%s expected to return %d values but stack is %#v", ep.Trace, spec.Name, len(spec.Returns), cx.stack),
 				)
 				for i := 0; i < len(spec.Returns); i++ {
 					sp := len(cx.stack) - 1 - i
+					if sp < 0 {
+						continue // We only assert this above, not require.
+					}
 					stackType := cx.stack[sp].argType()
 					retType := spec.Returns[i]
-					require.True(
+					assert.True(
 						t, typecheck(retType, stackType),
 						fmt.Sprintf("%s expected to return %s but actual is %s", spec.Name, retType.String(), stackType.String()),
 					)
@@ -2403,12 +2413,12 @@ func TestTxnEffects(t *testing.T) {
 	t.Parallel()
 	ep, _, _ := makeSampleEnv()
 	// We don't allow the effects fields to see the current or future transactions
-	testApp(t, "byte 0x32; log; txn NumLogs; int 1; ==", ep)
-	testApp(t, "byte 0x32; log; txn Logs 0; byte 0x32; ==", ep)
-	testApp(t, "byte 0x32; log; txn LastLog; byte 0x32; ==", ep)
-	testApp(t, "byte 0x32; log; gtxn 0 NumLogs; int 1; ==", ep)
-	testApp(t, "byte 0x32; log; gtxn 0 Logs 0; byte 0x32; ==", ep)
-	testApp(t, "byte 0x32; log; gtxn 0 LastLog; byte 0x32; ==", ep)
+	testApp(t, "byte 0x32; log; txn NumLogs; int 1; ==", ep, "txn effects can only be read from past txns")
+	testApp(t, "byte 0x32; log; txn Logs 0; byte 0x32; ==", ep, "txn effects can only be read from past txns")
+	testApp(t, "byte 0x32; log; txn LastLog; byte 0x32; ==", ep, "txn effects can only be read from past txns")
+	testApp(t, "byte 0x32; log; gtxn 0 NumLogs; int 1; ==", ep, "txn effects can only be read from past txns")
+	testApp(t, "byte 0x32; log; gtxn 0 Logs 0; byte 0x32; ==", ep, "txn effects can only be read from past txns")
+	testApp(t, "byte 0x32; log; gtxn 0 LastLog; byte 0x32; ==", ep, "txn effects can only be read from past txns")
 
 	// Look at the logs of tx 0
 	testApps(t, []string{"", "byte 0x32; log; gtxn 0 LastLog; byte 0x; =="}, nil, AssemblerMaxVersion, nil)
diff --git a/data/transactions/logic/eval_test.go b/data/transactions/logic/eval_test.go
index 841f180af..5921e7511 100644
--- a/data/transactions/logic/eval_test.go
+++ b/data/transactions/logic/eval_test.go
@@ -829,7 +829,7 @@ func TestGtxnBadIndex(t *testing.T) {
 
 	t.Parallel()
 	program := []byte{0x01, 0x33, 0x1, 0x01}
-	testLogicBytes(t, program, defaultEvalParams(nil), "gtxn lookup")
+	testLogicBytes(t, program, defaultEvalParams(nil), "txn index 1")
 }
 
 func TestGtxnBadField(t *testing.T) {
@@ -1367,30 +1367,31 @@ assert
 int 1
 `
 
+// The additions in v6 were all "effects" so they must look behind.  They use gtxn 2.
 const testTxnProgramTextV6 = testTxnProgramTextV5 + `
 assert
 
-txn CreatedAssetID
+gtxn 2 CreatedAssetID
 int 0
 ==
 assert
 
-txn CreatedApplicationID
+gtxn 2 CreatedApplicationID
 int 0
 ==
 assert
 
-txn NumLogs
+gtxn 2 NumLogs
 int 2
 ==
 assert
 
-txn Logs 1
+gtxn 2 Logs 1
 byte "prefilled"
 ==
 assert
 
-txn LastLog
+gtxn 2 LastLog
 byte "prefilled"
 ==
 assert
@@ -1553,11 +1554,11 @@ func TestTxn(t *testing.T) {
 			// Since we test GroupIndex ==3, we need to fake up such a group
 			ep := defaultEvalParams(nil)
 			ep.TxnGroup = transactions.WrapSignedTxnsWithAD([]transactions.SignedTxn{txn, txn, txn, txn})
-			ep.TxnGroup[3].EvalDelta.Logs = []string{"x", "prefilled"}
+			ep.TxnGroup[2].EvalDelta.Logs = []string{"x", "prefilled"}
 			if v < txnEffectsVersion {
 				testLogicFull(t, ops.Program, 3, ep)
 			} else {
-				// Starting in txnEffectsVersion we can't access all fields in Logic mode
+				// Starting in txnEffectsVersion, there are fields we can't access all fields in Logic mode
 				testLogicFull(t, ops.Program, 3, ep, "not allowed in current mode")
 				// And the early tests use "arg" a lot - not allowed in stateful. So remove those tests.
 				lastArg := strings.Index(source, "arg 10\n==\n&&")
@@ -1939,7 +1940,7 @@ txna ApplicationArgs 0
 	// modify gtxn index
 	saved = ops.Program[2]
 	ops.Program[2] = 0x01
-	testLogicBytes(t, ops.Program, ep, "gtxna lookup TxnGroup[1] but it only has 1")
+	testLogicBytes(t, ops.Program, ep, "txn index 1, len(group) is 1")
 
 	// modify gtxn field
 	ops.Program[2] = saved
diff --git a/data/transactions/logic/fields_test.go b/data/transactions/logic/fields_test.go
index 78cac3569..c3fe020b3 100644
--- a/data/transactions/logic/fields_test.go
+++ b/data/transactions/logic/fields_test.go
@@ -170,7 +170,7 @@ func TestTxnFieldVersions(t *testing.T) {
 }
 
 // TestTxnEffectsAvailable ensures that LogicSigs can not use "effects" fields
-// (ever). And apps can only use effects fields with `txn` after
+// (ever). And apps can only use effects fields with `gtxn` after
 // txnEffectsVersion. (itxn could use them earlier)
 func TestTxnEffectsAvailable(t *testing.T) {
 	partitiontest.PartitionTest(t)
@@ -180,18 +180,18 @@ func TestTxnEffectsAvailable(t *testing.T) {
 		if !fs.effects {
 			continue
 		}
-		source := fmt.Sprintf("txn %s; pop; int 1", fs.field)
+		source := fmt.Sprintf("gtxn 0 %s; pop; int 1", fs.field)
 		if fs.array {
-			source = fmt.Sprintf("txn %s 0; pop; int 1", fs.field)
+			source = fmt.Sprintf("gtxn 0 %s 0; pop; int 1", fs.field)
 		}
 		for v := fs.version; v <= AssemblerMaxVersion; v++ {
 			ops := testProg(t, source, v)
-			ep := defaultEvalParams(nil)
-			ep.TxnGroup[0].Lsig.Logic = ops.Program
-			_, err := EvalSignature(0, ep)
+			ep, _, _ := makeSampleEnv()
+			ep.TxnGroup[1].Lsig.Logic = ops.Program
+			_, err := EvalSignature(1, ep)
 			require.Error(t, err)
 			ep.Ledger = MakeLedger(nil)
-			_, err = EvalApp(ops.Program, 0, 0, ep)
+			_, err = EvalApp(ops.Program, 1, 0, ep)
 			if v < txnEffectsVersion {
 				require.Error(t, err, source)
 			} else {