Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AVM: Allow immutable access to foreign app accounts #3994

Merged
merged 11 commits into from
May 23, 2022
22 changes: 15 additions & 7 deletions data/transactions/logic/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -3548,14 +3548,22 @@ func (cx *EvalContext) accountReference(account stackValue) (basics.Address, uin
idx, err := cx.txn.Txn.IndexByAddress(addr, cx.txn.Txn.Sender)

invalidIndex := uint64(len(cx.txn.Txn.Accounts) + 1)
// Allow an address for an app that was created in group
// Allow an address for an app that was created in group or provided
// in the foreign apps array.
if err != nil && cx.version >= createdResourcesVersion {
for _, appID := range cx.created.apps {
createdAddress := cx.getApplicationAddress(appID)
if addr == createdAddress {
return addr, invalidIndex, nil
}
}

for _, appID := range cx.txn.Txn.ForeignApps {
foreignAddress := cx.getApplicationAddress(appID)
if addr == foreignAddress {
return addr, invalidIndex, nil
}
}
algoidurovic marked this conversation as resolved.
Show resolved Hide resolved
}

// this app's address is also allowed
Expand Down
119 changes: 119 additions & 0 deletions ledger/internal/apptxn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3058,3 +3058,122 @@ check:
txns(t, l, eval, &fundA, &callA)
endBlock(t, l, eval)
}

func TestForeignAppAccountsAccessible(t *testing.T) {
partitiontest.PartitionTest(t)

genBalances, addrs, _ := ledgertesting.NewTestGenesis()
l := newTestLedger(t, genBalances)
defer l.Close()
algoidurovic marked this conversation as resolved.
Show resolved Hide resolved

appA := txntest.Txn{
Type: "appl",
Sender: addrs[0],
ApprovalProgram: main(`
int 3
int 3
==
assert
`),
}

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
`),
}

eval := nextBlock(t, l)
txns(t, l, eval, &appA, &appB)
vb := endBlock(t, l, eval)
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()

callTx := txntest.Txn{
Type: "appl",
Sender: addrs[2],
ApplicationID: index1,
ForeignApps: []basics.AppIndex{index0},
}
eval = nextBlock(t, l)
txns(t, l, eval, &fund0, &fund1, &callTx)
endBlock(t, l, eval)
}

func TestForeignAppAccountsImmutable(t *testing.T) {
algoidurovic marked this conversation as resolved.
Show resolved Hide resolved
partitiontest.PartitionTest(t)

genBalances, addrs, _ := ledgertesting.NewTestGenesis()
l := newTestLedger(t, genBalances)
defer l.Close()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if this was also testConsensusRange so that it tests all the versions, starting with the first that works, through vFuture as we add more and more versions.


appA := txntest.Txn{
Type: "appl",
Sender: addrs[0],
ApprovalProgram: main(`
int 3
int 3
==
assert
`),
}

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
`),
}

eval := nextBlock(t, l)
txns(t, l, eval, &appA, &appB)
vb := endBlock(t, l, eval)
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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same


callTx := txntest.Txn{
Type: "appl",
Sender: addrs[2],
ApplicationID: index1,
ForeignApps: []basics.AppIndex{index0},
}
eval = nextBlock(t, l)
txns(t, l, eval, &fund0, &fund1)
txn(t, l, eval, &callTx, "invalid Account reference")
endBlock(t, l, eval)
}