AVM: Make apps and app accounts available while creation is "pending" (…

…#5425)
algorand · May 30, 2023 · aa6a5bb · aa6a5bb
1 parent 6ad660f
commit aa6a5bb
Show file tree

Hide file tree

Showing 4 changed files with 204 additions and 35 deletions.
diff --git 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
@@ -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
@@ -869,6 +869,156 @@ 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.
+		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:    3 * 1000, // to pay for self, call to inner create, and its call to helper
+			ApprovalProgram: `
+  itxn_begin
+   int appl;      itxn_field TypeEnum
+   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{creator, helper},
+		}
+		fund := txntest.Txn{
+			Type:     "pay",
+			Amount:   200_000,
+			Sender:   addrs[0],
+			Receiver: outerAppAddress,
+		}
+		if ver >= 38 {
+			dl.txgroup("", &fund, &outer)
+		} 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
@@ -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]
+				}
 			}
 		}()
 	}