AVM: Allow immutable access to foreign app accounts (#3994)

* allow foreign app accounts to be accessed (immutably)

data/transactions/logic/README.md

@@ -180,6 +180,9 @@ _available_.
    associated account of a contract that was created earlier in the
    group is _available_.
 
+ * Since v7, the account associated with any contract present in the
+   `txn.ForeignApplications` field is _available_.
+
 ## Constants
 
 Constants can be pushed onto the stack in two different ways:

data/transactions/logic/README_in.md

@@ -180,6 +180,9 @@ _available_.
    associated account of a contract that was created earlier in the
    group is _available_.
 
+ * Since v7, the account associated with any contract present in the
+   `txn.ForeignApplications` field is _available_.
+
 ## Constants
 
 Constants can be pushed onto the stack in two different ways:

data/transactions/logic/eval.go

@@ -3524,12 +3524,12 @@ func opExtract64Bits(cx *EvalContext) error {
 }
 
 // accountReference yields the address and Accounts offset designated by a
-// stackValue. If the stackValue is the app account or an account of an app in
-// created.apps, and it is not be in the Accounts array, then len(Accounts) + 1
-// is returned as the index. This would let us catch the mistake if the index is
-// used for set/del. If the txn somehow "psychically" predicted the address, and
-// therefore it IS in txn.Accounts, then happy day, we can set/del it.  Return
-// the proper index.
+// stackValue. If the stackValue is the app account, an account of an app in
+// created.apps, or an account of an app in foreignApps, and it is not in the
+// Accounts array, then len(Accounts) + 1 is returned as the index. This would
+// let us catch the mistake if the index is used for set/del. If the txn somehow
+// "psychically" predicted the address, and therefore it IS in txn.Accounts,
+// then happy day, we can set/del it. Return the proper index.
 
 // If we ever want apps to be able to change local state on these accounts
 // (which includes this app's own account!), we will need a change to
@@ -3558,6 +3558,16 @@ func (cx *EvalContext) accountReference(account stackValue) (basics.Address, uin
 		}
 	}
 
+	// Allow an address for an app that was provided in the foreign apps array.
+	if err != nil && cx.version >= appAddressAvailableVersion {
+		for _, appID := range cx.txn.Txn.ForeignApps {
+			foreignAddress := cx.getApplicationAddress(appID)
+			if addr == foreignAddress {
+				return addr, invalidIndex, nil
+			}
+		}
+	}
+
 	// this app's address is also allowed
 	if err != nil {
 		appAddr := cx.getApplicationAddress(cx.appID)

data/transactions/logic/evalAppTxn_test.go

@@ -2907,3 +2907,30 @@ itxn_submit
 
 	TestApp(t, source, ep, "appl depth (8) exceeded")
 }
+
+func TestForeignAppAccountAccess(t *testing.T) {
+	partitiontest.PartitionTest(t)
+
+	ep, tx, ledger := MakeSampleEnv()
+	ledger.NewAccount(appAddr(888), 50_000)
+	tx.ForeignApps = []basics.AppIndex{basics.AppIndex(111)}
+
+	ledger.NewApp(tx.Sender, 111, basics.AppParams{
+		ApprovalProgram:   TestProg(t, "int 1", AssemblerMaxVersion).Program,
+		ClearStateProgram: TestProg(t, "int 1", AssemblerMaxVersion).Program,
+	})
+
+	TestApp(t, `
+itxn_begin
+int pay
+itxn_field TypeEnum
+int 100
+itxn_field Amount
+txn Applications 1
+app_params_get AppAddress
+assert
+itxn_field Receiver
+itxn_submit
+int 1
+`, ep)
+}

data/transactions/logic/opcodes.go

@@ -57,6 +57,11 @@ const txnEffectsVersion = 6
 // the Foreign arrays.
 const createdResourcesVersion = 6
 
+// appAddressAvailableVersion is the first version that allows access to the
+// accounts of applications that were provided in the foreign apps transaction
+// field.
+const appAddressAvailableVersion = 7
+
 // experimental-
 const fidoVersion = 7 // base64, json, secp256r1
 

ledger/internal/apptxn_test.go

@@ -3058,3 +3058,202 @@ check:
 	txns(t, l, eval, &fundA, &callA)
 	endBlock(t, l, eval)
 }
+
+func TestForeignAppAccountsAccessible(t *testing.T) {
+	partitiontest.PartitionTest(t)
+
+	genBalances, addrs, _ := ledgertesting.NewTestGenesis()
+	testConsensusRange(t, 32, 0, func(t *testing.T, ver int) {
+		dl := NewDoubleLedger(t, genBalances, consensusByNumber[ver])
+		defer dl.Close()
+
+		appA := txntest.Txn{
+			Type:   "appl",
+			Sender: addrs[0],
+		}
+
+		appB := txntest.Txn{
+			Type:   "appl",
+			Sender: addrs[0],
+			ApprovalProgram: main(`
+itxn_begin
+	int pay;                itxn_field TypeEnum
+	int 100;     		    itxn_field Amount
+	txn Applications 1
+	app_params_get AppAddress
+	assert
+	itxn_field Receiver
+itxn_submit
+`),
+		}
+
+		vb := dl.fullBlock(&appA, &appB)
+		index0 := vb.Block().Payset[0].ApplicationID
+		index1 := vb.Block().Payset[1].ApplicationID
+
+		fund1 := txntest.Txn{
+			Type:     "pay",
+			Sender:   addrs[0],
+			Receiver: index1.Address(),
+			Amount:   1_000_000_000,
+		}
+		fund0 := fund1
+		fund0.Receiver = index0.Address()
+
+		callTx := txntest.Txn{
+			Type:          "appl",
+			Sender:        addrs[2],
+			ApplicationID: index1,
+			ForeignApps:   []basics.AppIndex{index0},
+		}
+
+		dl.beginBlock()
+		if ver <= 32 {
+			dl.txgroup("invalid Account reference", &fund0, &fund1, &callTx)
+			dl.endBlock()
+			return
+		}
+
+		dl.txgroup("", &fund0, &fund1, &callTx)
+		vb = dl.endBlock()
+
+		require.Equal(t, index0.Address(), vb.Block().Payset[2].EvalDelta.InnerTxns[0].Txn.Receiver)
+		require.Equal(t, uint64(100), vb.Block().Payset[2].EvalDelta.InnerTxns[0].Txn.Amount.Raw)
+	})
+}
+
+// While accounts of foreign apps are available in most contexts, they still
+// cannot be used as mutable references; ie the accounts cannot be used by
+// opcodes that modify local storage.
+func TestForeignAppAccountsImmutable(t *testing.T) {
+	partitiontest.PartitionTest(t)
+
+	genBalances, addrs, _ := ledgertesting.NewTestGenesis()
+	testConsensusRange(t, 32, 0, func(t *testing.T, ver int) {
+		dl := NewDoubleLedger(t, genBalances, consensusByNumber[ver])
+		defer dl.Close()
+
+		appA := txntest.Txn{
+			Type:   "appl",
+			Sender: addrs[0],
+		}
+
+		appB := txntest.Txn{
+			Type:   "appl",
+			Sender: addrs[0],
+			ApprovalProgram: main(`
+txn Applications 1
+app_params_get AppAddress
+byte "X"
+byte "ABC"
+app_local_put
+int 1
+`),
+		}
+
+		vb := dl.fullBlock(&appA, &appB)
+		index0 := vb.Block().Payset[0].ApplicationID
+		index1 := vb.Block().Payset[1].ApplicationID
+
+		fund1 := txntest.Txn{
+			Type:     "pay",
+			Sender:   addrs[0],
+			Receiver: index1.Address(),
+			Amount:   1_000_000_000,
+		}
+		fund0 := fund1
+		fund0.Receiver = index0.Address()
+
+		callTx := txntest.Txn{
+			Type:          "appl",
+			Sender:        addrs[2],
+			ApplicationID: index1,
+			ForeignApps:   []basics.AppIndex{index0},
+		}
+
+		dl.beginBlock()
+		dl.txgroup("invalid Account reference", &fund0, &fund1, &callTx)
+		dl.endBlock()
+	})
+}
+
+// In the case where the foreign app account is also provided in the
+// transaction's account field, mutable references should be allowed.
+func TestForeignAppAccountsMutable(t *testing.T) {
+	partitiontest.PartitionTest(t)
+
+	genBalances, addrs, _ := ledgertesting.NewTestGenesis()
+	testConsensusRange(t, 32, 0, func(t *testing.T, ver int) {
+		dl := NewDoubleLedger(t, genBalances, consensusByNumber[ver])
+		defer dl.Close()
+
+		appA := txntest.Txn{
+			Type:   "appl",
+			Sender: addrs[0],
+			ApprovalProgram: main(`
+itxn_begin
+	int appl
+	itxn_field TypeEnum
+	txn Applications 1
+	itxn_field ApplicationID
+	int OptIn
+	itxn_field OnCompletion
+itxn_submit
+`),
+		}
+
+		appB := txntest.Txn{
+			Type:   "appl",
+			Sender: addrs[0],
+			ApprovalProgram: main(`
+txn OnCompletion
+int OptIn
+==
+bnz done
+txn Applications 1
+app_params_get AppAddress
+assert
+byte "X"
+byte "Y"
+app_local_put
+done:
+`),
+			LocalStateSchema: basics.StateSchema{
+				NumByteSlice: 1,
+			},
+		}
+
+		vb := dl.fullBlock(&appA, &appB)
+		index0 := vb.Block().Payset[0].ApplicationID
+		index1 := vb.Block().Payset[1].ApplicationID
+
+		fund1 := txntest.Txn{
+			Type:     "pay",
+			Sender:   addrs[0],
+			Receiver: index1.Address(),
+			Amount:   1_000_000_000,
+		}
+		fund0 := fund1
+		fund0.Receiver = index0.Address()
+		fund1.Receiver = index1.Address()
+
+		callA := txntest.Txn{
+			Type:          "appl",
+			Sender:        addrs[2],
+			ApplicationID: index0,
+			ForeignApps:   []basics.AppIndex{index1},
+		}
+
+		callB := txntest.Txn{
+			Type:          "appl",
+			Sender:        addrs[2],
+			ApplicationID: index1,
+			ForeignApps:   []basics.AppIndex{index0},
+			Accounts:      []basics.Address{index0.Address()},
+		}
+
+		vb = dl.fullBlock(&fund0, &fund1, &callA, &callB)
+
+		require.Equal(t, "Y", vb.Block().Payset[3].EvalDelta.LocalDeltas[1]["X"].Bytes)
+	})
+}