From 13738d2ee991f9a34580e9fdf462c6fbf47f48df Mon Sep 17 00:00:00 2001 From: John Jannotti Date: Tue, 30 May 2023 10:19:25 -0400 Subject: [PATCH 1/2] Make apps and app accounts available while creation is "pending" Starting in the same consensus version that allows resource sharing, this PR makes apps and app accounts of "pending" app creates available while their inners are executing. This was the intent all along, the new app was only added to `cx.createdApps` at the conclusion of its execution, rather than at the start. --- data/transactions/logic/eval.go | 43 ++++++---- data/transactions/logic/resources.go | 41 +++++----- ledger/apptxn_test.go | 115 +++++++++++++++++++++++++++ ledger/double_test.go | 5 +- 4 files changed, 169 insertions(+), 35 deletions(-) diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go index 4d24261915..e5e55525e5 100644 --- a/data/transactions/logic/eval.go +++ b/data/transactions/logic/eval.go @@ -539,10 +539,16 @@ func (ep *EvalParams) RecordAD(gi int, ad transactions.ApplyData) { } ep.TxnGroup[gi].ApplyData = ad if aid := ad.ConfigAsset; aid != 0 { - ep.available.createdAsas = append(ep.available.createdAsas, aid) + if ep.available.createdAsas == nil { + ep.available.createdAsas = make(map[basics.AssetIndex]struct{}) + } + ep.available.createdAsas[aid] = struct{}{} } if aid := ad.ApplicationID; aid != 0 { - ep.available.createdApps = append(ep.available.createdApps, aid) + if ep.available.createdApps == nil { + ep.available.createdApps = make(map[basics.AppIndex]struct{}) + } + ep.available.createdApps[aid] = struct{}{} } } @@ -954,14 +960,21 @@ func EvalContract(program []byte, gi int, aid basics.AppIndex, params *EvalParam } } - // If this is a creation, make any "0 index" box refs available now that we - // have an appID. + // If this is a creation... if cx.txn.Txn.ApplicationID == 0 { + // make any "0 index" box refs available now that we have an appID. for _, br := range cx.txn.Txn.Boxes { if br.Index == 0 { cx.EvalParams.available.boxes[boxRef{cx.appID, string(br.Name)}] = false } } + // and add the appID to `createdApps` + if cx.EvalParams.Proto.LogicSigVersion >= sharedResourcesVersion { + if cx.EvalParams.available.createdApps == nil { + cx.EvalParams.available.createdApps = make(map[basics.AppIndex]struct{}) + } + cx.EvalParams.available.createdApps[cx.appID] = struct{}{} + } } // Check the I/O budget for reading if this is the first top-level app call @@ -4248,13 +4261,15 @@ func opExtract64Bits(cx *EvalContext) error { // assignAccount is used to convert a stackValue into a 32-byte account value, // enforcing any "availability" restrictions in force. func (cx *EvalContext) assignAccount(sv stackValue) (basics.Address, error) { - _, err := sv.address() + addr, err := sv.address() if err != nil { return basics.Address{}, err } - addr, _, err := cx.accountReference(sv) - return addr, err + if cx.availableAccount(addr) { + return addr, nil + } + return basics.Address{}, fmt.Errorf("invalid Account reference %s", addr) } // accountReference yields the address and Accounts offset designated by a @@ -4323,7 +4338,7 @@ func (cx *EvalContext) availableAccount(addr basics.Address) bool { // Allow an address for an app that was created in group if cx.version >= createdResourcesVersion { - for _, appID := range cx.available.createdApps { + for appID := range cx.available.createdApps { createdAddress := cx.getApplicationAddress(appID) if addr == createdAddress { return true @@ -5199,10 +5214,8 @@ func (cx *EvalContext) availableAsset(aid basics.AssetIndex) bool { } // or was created in group if cx.version >= createdResourcesVersion { - for _, assetID := range cx.available.createdAsas { - if assetID == aid { - return true - } + if _, ok := cx.available.createdAsas[aid]; ok { + return true } } @@ -5241,10 +5254,8 @@ func (cx *EvalContext) availableApp(aid basics.AppIndex) bool { } // or was created in group if cx.version >= createdResourcesVersion { - for _, appID := range cx.available.createdApps { - if appID == aid { - return true - } + if _, ok := cx.available.createdApps[aid]; ok { + return true } } // Or, it can be the current app diff --git a/data/transactions/logic/resources.go b/data/transactions/logic/resources.go index 4db2d5d595..00045100ec 100644 --- a/data/transactions/logic/resources.go +++ b/data/transactions/logic/resources.go @@ -31,8 +31,8 @@ import ( type resources struct { // These resources were created previously in the group, so they can be used // by later transactions. - createdAsas []basics.AssetIndex - createdApps []basics.AppIndex + createdAsas map[basics.AssetIndex]struct{} + createdApps map[basics.AppIndex]struct{} // These resources have been used by some txn in the group, so they are // available. These maps track the availability of the basic objects (often @@ -101,6 +101,16 @@ func (r *resources) fill(tx *transactions.Transaction, ep *EvalParams) { } func (cx *EvalContext) allows(tx *transactions.Transaction, calleeVer uint64) error { + // if the caller is pre-sharing, it can't prepare transactions with + // resources that are not available, so `tx` is surely legal. + if cx.version < sharedResourcesVersion { + // this is important, not just an optimization, because a pre-sharing + // creation txn has access to the app and app account it is currently + // creating (and therefore can pass that access down), but cx.available + // doesn't track that properly until v9's protocol upgrade. See + // TestInnerAppCreateAndOptin for an example. + return nil + } switch tx.Type { case protocol.PaymentTx, protocol.KeyRegistrationTx, protocol.AssetConfigTx: // these transactions don't touch cross-product resources, so no error is possible @@ -110,7 +120,7 @@ func (cx *EvalContext) allows(tx *transactions.Transaction, calleeVer uint64) er case protocol.AssetFreezeTx: return cx.allowsAssetFreeze(&tx.Header, &tx.AssetFreezeTxnFields) case protocol.ApplicationCallTx: - return cx.allowsApplicationCall(&tx.Header, &tx.ApplicationCallTxnFields, cx.version, calleeVer) + return cx.allowsApplicationCall(&tx.Header, &tx.ApplicationCallTxnFields, calleeVer) default: return fmt.Errorf("unknown inner transaction type %s", tx.Type) } @@ -158,13 +168,11 @@ func (cx *EvalContext) allowsHolding(addr basics.Address, ai basics.AssetIndex) return true } // If an ASA was created in this group, then allow holding access for any allowed account. - for _, created := range r.createdAsas { - if created == ai { - return cx.availableAccount(addr) - } + if _, ok := r.createdAsas[ai]; ok { + return cx.availableAccount(addr) } // If the address was "created" by making its app in this group, then allow for available assets. - for _, created := range r.createdApps { + for created := range r.createdApps { if cx.getApplicationAddress(created) == addr { return cx.availableAsset(ai) } @@ -184,17 +192,15 @@ func (cx *EvalContext) allowsLocals(addr basics.Address, ai basics.AppIndex) boo return true } // All locals of created apps are available - for _, created := range r.createdApps { - if created == ai { - return cx.availableAccount(addr) - } + if _, ok := r.createdApps[ai]; ok { + return cx.availableAccount(addr) } if cx.txn.Txn.ApplicationID == 0 && cx.appID == ai { return cx.availableAccount(addr) } // All locals of created app accounts are available - for _, created := range r.createdApps { + for created := range r.createdApps { if cx.getApplicationAddress(created) == addr { return cx.availableApp(ai) } @@ -315,11 +321,10 @@ func (r *resources) fillApplicationCall(ep *EvalParams, hdr *transactions.Header } } -func (cx *EvalContext) allowsApplicationCall(hdr *transactions.Header, tx *transactions.ApplicationCallTxnFields, callerVer, calleeVer uint64) error { - // If an old (pre resource sharing) app is being called from an app that has - // resource sharing enabled, we need to confirm that no new "cross-product" - // resources have become available. - if callerVer < sharedResourcesVersion || calleeVer >= sharedResourcesVersion { +func (cx *EvalContext) allowsApplicationCall(hdr *transactions.Header, tx *transactions.ApplicationCallTxnFields, calleeVer uint64) error { + // If the callee is at least sharedResourcesVersion, then it will check + // availability properly itself. + if calleeVer >= sharedResourcesVersion { return nil } diff --git a/ledger/apptxn_test.go b/ledger/apptxn_test.go index 28a5b10404..68fd8f34ea 100644 --- a/ledger/apptxn_test.go +++ b/ledger/apptxn_test.go @@ -869,6 +869,121 @@ func TestInnerRekey(t *testing.T) { }) } +// TestInnerAppCreateAndOptin tests a weird way to create an app and opt it into +// an ASA all from one top-level transaction. Part of the trick is to use an +// inner helper app. The app being created rekeys itself to the inner app, +// which funds the outer app and opts it into the ASA. It could have worked +// differently - the inner app could have just funded the outer app, and then +// the outer app could have opted-in. But this technique tests something +// interesting, that the inner app can perform an opt-in on the outer app, which +// tests that the newly created app's holdings are available. In practice, the +// helper shold rekey it back, but we don't bother here. +func TestInnerAppCreateAndOptin(t *testing.T) { + partitiontest.PartitionTest(t) + t.Parallel() + + genBalances, addrs, _ := ledgertesting.NewTestGenesis() + + // v31 allows inner appl and inner rekey + ledgertesting.TestConsensusRange(t, 31, 0, func(t *testing.T, ver int, cv protocol.ConsensusVersion, cfg config.Local) { + dl := NewDoubleLedger(t, genBalances, cv, cfg) + defer dl.Close() + + createasa := txntest.Txn{ + Type: "acfg", + Sender: addrs[0], + AssetParams: basics.AssetParams{Total: 2, UnitName: "$"}, + } + asaID := dl.txn(&createasa).ApplyData.ConfigAsset + require.NotZero(t, asaID) + + // helper app, is called during the creation of an app. When such an + // app is created, it rekeys itself to this helper and calls it. The + // helpers opts the caller into an ASA, and funds the MBR the caller + // needs for that optin. + helper := dl.fundedApp(addrs[0], 1_000_000, + main(` + itxn_begin + int axfer; itxn_field TypeEnum + int `+strconv.Itoa(int(asaID))+`; itxn_field XferAsset + txn Sender; itxn_field Sender // call as the caller! (works because of rekey by caller) + txn Sender; itxn_field AssetReceiver // 0 to self == opt-in + itxn_next + int pay; itxn_field TypeEnum // pay 200kmAlgo to the caller, for MBR + int 200000; itxn_field Amount + txn Sender; itxn_field Receiver + itxn_submit +`)) + // Don't use `main` here, we want to do the work during creation. Rekey + // to the helper and invoke it, trusting it to opt us into the ASA. + createapp := txntest.Txn{ + Type: "appl", + Sender: addrs[0], + Fee: 3 * 1000, // to pay for self, call to helper, and helper's axfer + ApprovalProgram: ` + itxn_begin + int appl; itxn_field TypeEnum + addr ` + helper.Address().String() + `; itxn_field RekeyTo + int ` + strconv.Itoa(int(helper)) + `; itxn_field ApplicationID + txn Assets 0; itxn_field Assets + itxn_submit + int 1 +`, + ForeignApps: []basics.AppIndex{helper}, + ForeignAssets: []basics.AssetIndex{asaID}, + } + appID := dl.txn(&createapp).ApplyData.ApplicationID + require.NotZero(t, appID) + }) +} + +// TestParentGlobals tests that a newly created app can call an inner app, and +// the inner app will have access to the parent globals, even if the originally +// created app ID isn't passed down, because the rule is that "pending" created +// apps are available, starting from v38 +func TestParentGlobals(t *testing.T) { + partitiontest.PartitionTest(t) + t.Parallel() + + genBalances, addrs, _ := ledgertesting.NewTestGenesis() + + // v38 allows parent access, but we start with v31 to make sure we don't mistakenly change it + ledgertesting.TestConsensusRange(t, 31, 0, func(t *testing.T, ver int, cv protocol.ConsensusVersion, cfg config.Local) { + dl := NewDoubleLedger(t, genBalances, cv, cfg) + defer dl.Close() + + // helper app, is called during the creation of an app. this app tries + // to access its parent's globals, by using `global CallerApplicationID` + helper := dl.fundedApp(addrs[0], 1_000_000, + main(` + global CallerApplicationID + byte "X" + app_global_get_ex; pop; pop; // we only care that it didn't panic +`)) + // Don't use `main` here, we want to do the work during creation. + createapp := txntest.Txn{ + Type: "appl", + Sender: addrs[0], + Fee: 2 * 1000, // to pay for self and call to helper + ApprovalProgram: ` + itxn_begin + int appl; itxn_field TypeEnum + int ` + strconv.Itoa(int(helper)) + `; itxn_field ApplicationID + itxn_submit + int 1 +`, + ForeignApps: []basics.AppIndex{helper}, + } + if ver >= 38 { + appID := dl.txn(&createapp).ApplyData.ApplicationID + require.NotZero(t, appID) + } else { + dl.txn(&createapp, "unavailable App") + } + + }) +} + func TestNote(t *testing.T) { partitiontest.PartitionTest(t) t.Parallel() diff --git a/ledger/double_test.go b/ledger/double_test.go index 1c53e54b09..16d38e7681 100644 --- a/ledger/double_test.go +++ b/ledger/double_test.go @@ -77,7 +77,10 @@ func (dl *DoubleLedger) txn(tx *txntest.Txn, problem ...string) (stib *transacti dl.eval = nil } else { vb := dl.endBlock() - stib = &vb.Block().Payset[0] + // It should have a stib, but don't panic here because of an earlier problem. + if len(vb.Block().Payset) > 0 { + stib = &vb.Block().Payset[0] + } } }() } From 3e2686936836734bf3c0e0816182ade5ec757a9a Mon Sep 17 00:00:00 2001 From: John Jannotti Date: Tue, 30 May 2023 12:31:14 -0400 Subject: [PATCH 2/2] extra test for inner creates --- ledger/apptxn_test.go | 45 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/ledger/apptxn_test.go b/ledger/apptxn_test.go index 68fd8f34ea..a2d7dba115 100644 --- a/ledger/apptxn_test.go +++ b/ledger/apptxn_test.go @@ -960,23 +960,58 @@ func TestParentGlobals(t *testing.T) { byte "X" app_global_get_ex; pop; pop; // we only care that it didn't panic `)) + // Don't use `main` here, we want to do the work during creation. + createProgram := ` + itxn_begin + int appl; itxn_field TypeEnum + int ` + strconv.Itoa(int(helper)) + `; itxn_field ApplicationID + itxn_submit + int 1 +` createapp := txntest.Txn{ + Type: "appl", + Sender: addrs[0], + Fee: 2 * 1000, // to pay for self and call to helper + ApprovalProgram: createProgram, + ForeignApps: []basics.AppIndex{helper}, + } + var creator basics.AppIndex + if ver >= 38 { + creator = dl.txn(&createapp).ApplyData.ApplicationID + require.NotZero(t, creator) + } else { + dl.txn(&createapp, "unavailable App") + } + + // Now, test the same pattern, but do it all inside of yet another outer + // app, to show that the parent is available even if it was, itself + // created as an inner. To do so, we also need to get 0.2 MBR to the + // outer app, since it will be creating the "middle" app. + + outerAppAddress := (creator + 3).Address() // creator called an inner, so next is creator+2, then fund + outer := txntest.Txn{ Type: "appl", Sender: addrs[0], - Fee: 2 * 1000, // to pay for self and call to helper + Fee: 3 * 1000, // to pay for self, call to inner create, and its call to helper ApprovalProgram: ` itxn_begin int appl; itxn_field TypeEnum - int ` + strconv.Itoa(int(helper)) + `; itxn_field ApplicationID + byte 0x` + hex.EncodeToString(createapp.SignedTxn().Txn.ApprovalProgram) + `; itxn_field ApprovalProgram + byte 0x` + hex.EncodeToString(createapp.SignedTxn().Txn.ClearStateProgram) + `; itxn_field ClearStateProgram itxn_submit int 1 `, - ForeignApps: []basics.AppIndex{helper}, + ForeignApps: []basics.AppIndex{creator, helper}, + } + fund := txntest.Txn{ + Type: "pay", + Amount: 200_000, + Sender: addrs[0], + Receiver: outerAppAddress, } if ver >= 38 { - appID := dl.txn(&createapp).ApplyData.ApplicationID - require.NotZero(t, appID) + dl.txgroup("", &fund, &outer) } else { dl.txn(&createapp, "unavailable App") }