Global test (#3587)

* test for global modifications by various inner apps

* CR adjustments to feeCredit

* Test local state sharing too.

* explain clear state details

6 files changed, 303 insertions, 134 deletions

diff --git a/data/transactions/blackbox_test.go b/data/transactions/blackbox_test.go
deleted file mode 100644
index b8f35e9a8..000000000
--- a/data/transactions/blackbox_test.go
+++ /dev/null
@@ -1,82 +0,0 @@
-// Copyright (C) 2019-2022 Algorand, Inc.
-// This file is part of go-algorand
-//
-// go-algorand is free software: you can redistribute it and/or modify
-// it under the terms of the GNU Affero General Public License as
-// published by the Free Software Foundation, either version 3 of the
-// License, or (at your option) any later version.
-//
-// go-algorand is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU Affero General Public License for more details.
-//
-// You should have received a copy of the GNU Affero General Public License
-// along with go-algorand.  If not, see <https://www.gnu.org/licenses/>.
-
-package transactions_test
-
-import (
-	"testing"
-
-	"github.com/algorand/go-algorand/data/transactions"
-	"github.com/algorand/go-algorand/data/txntest"
-	"github.com/algorand/go-algorand/protocol"
-	"github.com/algorand/go-algorand/test/partitiontest"
-	"github.com/stretchr/testify/require"
-)
-
-func TestFeeCredit(t *testing.T) {
-	partitiontest.PartitionTest(t)
-
-	c, err := transactions.FeeCredit([]transactions.SignedTxnWithAD{
-		txntest.Txn{Fee: 5}.SignedTxnWithAD(),
-	}, 5)
-	require.NoError(t, err)
-	require.Equal(t, c, uint64(0))
-
-	c, err = transactions.FeeCredit([]transactions.SignedTxnWithAD{
-		txntest.Txn{Fee: 4}.SignedTxnWithAD(),
-	}, 5)
-	require.Error(t, err)
-
-	c, err = transactions.FeeCredit([]transactions.SignedTxnWithAD{
-		txntest.Txn{Fee: 5}.SignedTxnWithAD(),
-	}, 4)
-	require.NoError(t, err)
-	require.Equal(t, c, uint64(1))
-
-	c, err = transactions.FeeCredit([]transactions.SignedTxnWithAD{
-		txntest.Txn{Fee: 5}.SignedTxnWithAD(),
-		txntest.Txn{Fee: 5}.SignedTxnWithAD(),
-	}, 5)
-	require.NoError(t, err)
-	require.Equal(t, c, uint64(0))
-
-	c, err = transactions.FeeCredit([]transactions.SignedTxnWithAD{
-		txntest.Txn{Fee: 5}.SignedTxnWithAD(),
-		txntest.Txn{Type: protocol.CompactCertTx, Fee: 0}.SignedTxnWithAD(),
-	}, 5)
-	require.NoError(t, err)
-	require.Equal(t, c, uint64(0))
-
-	c, err = transactions.FeeCredit([]transactions.SignedTxnWithAD{
-		txntest.Txn{Fee: 5}.SignedTxnWithAD(),
-		txntest.Txn{Fee: 5}.SignedTxnWithAD(),
-		txntest.Txn{Fee: 5}.SignedTxnWithAD(),
-	}, 5)
-	require.NoError(t, err)
-	require.Equal(t, c, uint64(0))
-
-	c, err = transactions.FeeCredit([]transactions.SignedTxnWithAD{}, 5)
-	require.NoError(t, err)
-	require.Equal(t, c, uint64(0))
-
-	c, err = transactions.FeeCredit([]transactions.SignedTxnWithAD{
-		txntest.Txn{Fee: 5}.SignedTxnWithAD(),
-		txntest.Txn{Fee: 25}.SignedTxnWithAD(),
-		txntest.Txn{Fee: 5}.SignedTxnWithAD(),
-	}, 5)
-	require.NoError(t, err)
-	require.Equal(t, c, uint64(20))
-}
diff --git a/data/transactions/logic/README.md b/data/transactions/logic/README.md
index e7fbda102..9ec223838 100644
--- a/data/transactions/logic/README.md
+++ b/data/transactions/logic/README.md
@@ -134,10 +134,31 @@ must rerun their code to determine if the ApplicationCall transactions
 in their pool would still succeed each time a block is added to the
 blockchain.
 
+Smart contracts have limits on their execution cost (700, consensus
+parameter MaxAppProgramCost). Before v4, this was a static limit on
+the cost of all the instructions in the program. Since then, the cost
+is tracked dynamically during execution and must not exceed
+MaxAppProgramCost. Beginning with v5, programs costs are pooled and
+tracked dynamically across app executions in a group.  If `n`
+application invocations appear in a group, then the total execution
+cost of such calls must not exceed `n`*MaxAppProgramCost. In v6, inner
+application calls become possible, and each such call increases the
+pooled budget by MaxAppProgramCost.
+
+Executions of the ClearStateProgram are more stringent, in order to
+ensure that applications may be closed out, but that applications also
+are assured a chance to clean up their internal state. At the
+beginning of the execution of a ClearStateProgram, the pooled budget
+available must be MaxAppProgramCost or higher. If it is not, the
+containing transaction group fails without clearing the app's
+state. During the execution of the ClearStateProgram, no more than
+MaxAppProgramCost may be drawn. If further execution is attempted, the
+ClearStateProgram fails, and the app's state _is cleared_.
+
+
 ### Resource availability
 
-Smart contracts have limits on their execution budget (700, consensus
-parameter MaxAppProgramCost), and the amount of blockchain state they
+Smart contracts have limits on the amount of blockchain state they
 may examine.  Opcodes may only access blockchain resources such as
 Accounts, Assets, and contract state if the given resource is
 _available_.
@@ -596,7 +617,8 @@ In v5, inner transactions may perform `pay`, `axfer`, `acfg`, and
 `itxn_submit`, the effects of the transaction are visible begining
 with the next instruction with, for example, `balance` and
 `min_balance` checks. In v6, inner transactions may also perform
-`keyreg` and `appl` effects.
+`keyreg` and `appl` effects. Inner `appl` calls fail if they attempt
+to invoke a program with version less than v6.
 
 In v5, only a subset of the transaction's header fields may be set: `Type`/`TypeEnum`,
 `Sender`, and `Fee`. In v6, header fields `Note` and `RekeyTo` may
diff --git a/data/transactions/logic/README_in.md b/data/transactions/logic/README_in.md
index 5933d424d..f6968ab9f 100644
--- a/data/transactions/logic/README_in.md
+++ b/data/transactions/logic/README_in.md
@@ -134,10 +134,31 @@ must rerun their code to determine if the ApplicationCall transactions
 in their pool would still succeed each time a block is added to the
 blockchain.
 
+Smart contracts have limits on their execution cost (700, consensus
+parameter MaxAppProgramCost). Before v4, this was a static limit on
+the cost of all the instructions in the program. Since then, the cost
+is tracked dynamically during execution and must not exceed
+MaxAppProgramCost. Beginning with v5, programs costs are pooled and
+tracked dynamically across app executions in a group.  If `n`
+application invocations appear in a group, then the total execution
+cost of such calls must not exceed `n`*MaxAppProgramCost. In v6, inner
+application calls become possible, and each such call increases the
+pooled budget by MaxAppProgramCost.
+
+Executions of the ClearStateProgram are more stringent, in order to
+ensure that applications may be closed out, but that applications also
+are assured a chance to clean up their internal state. At the
+beginning of the execution of a ClearStateProgram, the pooled budget
+available must be MaxAppProgramCost or higher. If it is not, the
+containing transaction group fails without clearing the app's
+state. During the execution of the ClearStateProgram, no more than
+MaxAppProgramCost may be drawn. If further execution is attempted, the
+ClearStateProgram fails, and the app's state _is cleared_.
+
+
 ### Resource availability
 
-Smart contracts have limits on their execution budget (700, consensus
-parameter MaxAppProgramCost), and the amount of blockchain state they
+Smart contracts have limits on the amount of blockchain state they
 may examine.  Opcodes may only access blockchain resources such as
 Accounts, Assets, and contract state if the given resource is
 _available_.
@@ -311,7 +332,8 @@ In v5, inner transactions may perform `pay`, `axfer`, `acfg`, and
 `itxn_submit`, the effects of the transaction are visible begining
 with the next instruction with, for example, `balance` and
 `min_balance` checks. In v6, inner transactions may also perform
-`keyreg` and `appl` effects.
+`keyreg` and `appl` effects. Inner `appl` calls fail if they attempt
+to invoke a program with version less than v6.
 
 In v5, only a subset of the transaction's header fields may be set: `Type`/`TypeEnum`,
 `Sender`, and `Fee`. In v6, header fields `Note` and `RekeyTo` may
diff --git a/data/transactions/logic/eval.go b/data/transactions/logic/eval.go
index ab4f36587..d3aea8279 100644
--- a/data/transactions/logic/eval.go
+++ b/data/transactions/logic/eval.go
@@ -311,7 +311,7 @@ func NewEvalParams(txgroup []transactions.SignedTxnWithAD, proto *config.Consens
 	var pooledApplicationBudget *int
 	var pooledAllowedInners *int
 
-	credit, _ := transactions.FeeCredit(txgroup, proto.MinTxnFee)
+	credit := feeCredit(txgroup, proto.MinTxnFee)
 
 	if proto.EnableAppCostPooling && apps > 0 {
 		pooledApplicationBudget = new(int)
@@ -337,6 +337,25 @@ func NewEvalParams(txgroup []transactions.SignedTxnWithAD, proto *config.Consens
 	}
 }
 
+// feeCredit returns the extra fee supplied in this top-level txgroup compared
+// to required minfee.  It can make assumptions about overflow because the group
+// is known OK according to TxnGroupBatchVerify. (In essence the group is
+// "WellFormed")
+func feeCredit(txgroup []transactions.SignedTxnWithAD, minFee uint64) uint64 {
+	minFeeCount := uint64(0)
+	feesPaid := uint64(0)
+	for _, stxn := range txgroup {
+		if stxn.Txn.Type != protocol.CompactCertTx {
+			minFeeCount++
+		}
+		feesPaid = basics.AddSaturate(feesPaid, stxn.Txn.Fee.Raw)
+	}
+	// Overflow is impossible, because TxnGroupBatchVerify checked.
+	feeNeeded := minFee * minFeeCount
+
+	return feesPaid - feeNeeded
+}
+
 // NewInnerEvalParams creates an EvalParams to be used while evaluating an inner group txgroup
 func NewInnerEvalParams(txg []transactions.SignedTxnWithAD, caller *EvalContext) *EvalParams {
 	minTealVersion := ComputeMinTealVersion(txg, true)
@@ -3733,12 +3752,9 @@ func opAppGlobalDel(cx *EvalContext) {
 
 func appReference(cx *EvalContext, ref uint64, foreign bool) (basics.AppIndex, error) {
 	if cx.version >= directRefEnabledVersion {
-		if ref == 0 {
+		if ref == 0 || ref == uint64(cx.appID) {
 			return cx.appID, nil
 		}
-		if ref <= uint64(len(cx.Txn.Txn.ForeignApps)) {
-			return basics.AppIndex(cx.Txn.Txn.ForeignApps[ref-1]), nil
-		}
 		for _, appID := range cx.Txn.Txn.ForeignApps {
 			if appID == basics.AppIndex(ref) {
 				return appID, nil
@@ -3752,13 +3768,11 @@ func appReference(cx *EvalContext, ref uint64, foreign bool) (basics.AppIndex, e
 				}
 			}
 		}
-		// It should be legal to use your own app id, which can't be in
-		// ForeignApps during creation, because it is unknown then.  But it can
-		// be discovered in the app code.  It's tempting to combine this with
-		// the == 0 test, above, but it must come after the check for being
-		// below len(ForeignApps)
-		if ref == uint64(cx.appID) {
-			return cx.appID, nil
+		// Allow use of indexes, but this comes last so that clear advice can be
+		// given to anyone who cares about semantics in the first few rounds of
+		// a new network - don't use indexes for references, use the App ID
+		if ref <= uint64(len(cx.Txn.Txn.ForeignApps)) {
+			return basics.AppIndex(cx.Txn.Txn.ForeignApps[ref-1]), nil
 		}
 	} else {
 		// Old rules
@@ -3780,10 +3794,6 @@ func appReference(cx *EvalContext, ref uint64, foreign bool) (basics.AppIndex, e
 
 func asaReference(cx *EvalContext, ref uint64, foreign bool) (basics.AssetIndex, error) {
 	if cx.version >= directRefEnabledVersion {
-		// In recent versions, accept either kind of ASA reference
-		if ref < uint64(len(cx.Txn.Txn.ForeignAssets)) {
-			return basics.AssetIndex(cx.Txn.Txn.ForeignAssets[ref]), nil
-		}
 		for _, assetID := range cx.Txn.Txn.ForeignAssets {
 			if assetID == basics.AssetIndex(ref) {
 				return assetID, nil
@@ -3797,6 +3807,12 @@ func asaReference(cx *EvalContext, ref uint64, foreign bool) (basics.AssetIndex,
 				}
 			}
 		}
+		// Allow use of indexes, but this comes last so that clear advice can be
+		// given to anyone who cares about semantics in the first few rounds of
+		// a new network - don't use indexes for references, use the asa ID.
+		if ref < uint64(len(cx.Txn.Txn.ForeignAssets)) {
+			return basics.AssetIndex(cx.Txn.Txn.ForeignAssets[ref]), nil
+		}
 	} else {
 		// Old rules
 		if foreign {
diff --git a/data/transactions/signedtxn.go b/data/transactions/signedtxn.go
index bbe445f9f..8f4f55ba4 100644
--- a/data/transactions/signedtxn.go
+++ b/data/transactions/signedtxn.go
@@ -18,7 +18,6 @@ package transactions
 
 import (
 	"errors"
-	"fmt"
 
 	"github.com/algorand/go-algorand/crypto"
 	"github.com/algorand/go-algorand/data/basics"
@@ -130,32 +129,3 @@ func WrapSignedTxnsWithAD(txgroup []SignedTxn) []SignedTxnWithAD {
 	}
 	return txgroupad
 }
-
-// FeeCredit computes the amount of fee credit that can be spent on
-// inner txns because it was more than required.
-func FeeCredit(txgroup []SignedTxnWithAD, minFee uint64) (uint64, error) {
-	minFeeCount := uint64(0)
-	feesPaid := uint64(0)
-	for _, stxn := range txgroup {
-		if stxn.Txn.Type != protocol.CompactCertTx {
-			minFeeCount++
-		}
-		feesPaid = basics.AddSaturate(feesPaid, stxn.Txn.Fee.Raw)
-	}
-	feeNeeded, overflow := basics.OMul(minFee, minFeeCount)
-	if overflow {
-		return 0, fmt.Errorf("txgroup fee requirement overflow")
-	}
-	// feesPaid may have saturated. That's ok. Since we know
-	// feeNeeded did not overflow, simple comparison tells us
-	// feesPaid was enough.
-	if feesPaid < feeNeeded {
-		return 0, fmt.Errorf("txgroup had %d in fees, which is less than the minimum %d * %d",
-			feesPaid, minFeeCount, minFee)
-	}
-	// Now, if feesPaid *did* saturate, you will not get "credit" for
-	// all those fees while executing AVM code that might create
-	// transactions.  But you'll get the max uint64 - good luck
-	// spending it.
-	return feesPaid - feeNeeded, nil
-}
diff --git a/ledger/internal/apptxn_test.go b/ledger/internal/apptxn_test.go
index 94c4c4c9b..d000f53f7 100644
--- a/ledger/internal/apptxn_test.go
+++ b/ledger/internal/apptxn_test.go
@@ -3032,3 +3032,224 @@ itxn_submit
 		})
 	}
 }
+
+// TestGlobalChangesAcrossApps ensures that state changes are seen by other app
+// calls when using inners.
+func TestGlobalChangesAcrossApps(t *testing.T) {
+	partitiontest.PartitionTest(t)
+
+	genBalances, addrs, _ := ledgertesting.NewTestGenesis()
+	l := newTestLedger(t, genBalances)
+	defer l.Close()
+
+	appA := txntest.Txn{
+		Type:   "appl",
+		Sender: addrs[0],
+		ApprovalProgram: main(`
+            // Call B : No arguments means: set your global "X" to "ABC"
+			itxn_begin
+			int appl;               itxn_field TypeEnum
+			txn Applications 1;     itxn_field ApplicationID
+			itxn_submit
+
+            // Call C : Checks that B's global X is ABC
+			itxn_begin
+			int appl;               itxn_field TypeEnum
+			txn Applications 2;     itxn_field ApplicationID
+            txn Applications 1;     itxn_field Applications // Pass on access to B
+			itxn_submit
+
+            // Call B again:  1 arg means it checks if X == ABC
+			itxn_begin
+			int appl;               itxn_field TypeEnum
+			txn Applications 1;     itxn_field ApplicationID
+            byte "check, please";   itxn_field ApplicationArgs
+			itxn_submit
+
+            // Check B's state for X
+            txn Applications 1
+            byte "X"
+            app_global_get_ex
+            assert
+            byte "ABC"
+            ==
+            assert
+`),
+	}
+
+	appB := txntest.Txn{
+		Type:   "appl",
+		Sender: addrs[0],
+		ApprovalProgram: main(`
+  txn NumAppArgs
+  bnz check						// 1 arg means check
+  // set
+  byte "X"
+  byte "ABC"
+  app_global_put
+  b end
+check:
+  byte "X"
+  app_global_get
+  byte "ABC"
+  ==
+  assert
+  b end
+`),
+		GlobalStateSchema: basics.StateSchema{
+			NumByteSlice: 1,
+		},
+	}
+
+	appC := txntest.Txn{
+		Type:   "appl",
+		Sender: addrs[0],
+		ApprovalProgram: main(`
+  txn Applications 1
+  byte "X"
+  app_global_get_ex
+  assert
+  byte "ABC"
+  ==
+  assert
+`),
+	}
+
+	eval := nextBlock(t, l, true, nil)
+	txns(t, l, eval, &appA, &appB, &appC)
+	vb := endBlock(t, l, eval)
+	indexA := vb.Block().Payset[0].ApplicationID
+	indexB := vb.Block().Payset[1].ApplicationID
+	indexC := vb.Block().Payset[2].ApplicationID
+
+	fundA := txntest.Txn{
+		Type:     "pay",
+		Sender:   addrs[0],
+		Receiver: indexA.Address(),
+		Amount:   1_000_000,
+	}
+
+	callA := txntest.Txn{
+		Type:          "appl",
+		Sender:        addrs[0],
+		ApplicationID: indexA,
+		ForeignApps:   []basics.AppIndex{indexB, indexC},
+	}
+
+	eval = nextBlock(t, l, true, nil)
+	txns(t, l, eval, &fundA, &callA)
+	endBlock(t, l, eval)
+}
+
+// TestLocalChangesAcrossApps ensures that state changes are seen by other app
+// calls when using inners.
+func TestLocalChangesAcrossApps(t *testing.T) {
+	partitiontest.PartitionTest(t)
+
+	genBalances, addrs, _ := ledgertesting.NewTestGenesis()
+	l := newTestLedger(t, genBalances)
+	defer l.Close()
+
+	appA := txntest.Txn{
+		Type:   "appl",
+		Sender: addrs[0],
+		ApprovalProgram: main(`
+            // Call B : No arguments means: set caller's local "X" to "ABC"
+			itxn_begin
+			int appl;               itxn_field TypeEnum
+			txn Applications 1;     itxn_field ApplicationID
+            int OptIn;              itxn_field OnCompletion
+			itxn_submit
+
+            // Call C : Checks that caller's local X for app B is ABC
+			itxn_begin
+			int appl;               itxn_field TypeEnum
+			txn Applications 2;     itxn_field ApplicationID
+            txn Applications 1;     itxn_field Applications // Pass on access to B
+			itxn_submit
+
+            // Call B again:  1 arg means it checks if caller's local X == ABC
+			itxn_begin
+			int appl;               itxn_field TypeEnum
+			txn Applications 1;     itxn_field ApplicationID
+            byte "check, please";   itxn_field ApplicationArgs
+			itxn_submit
+
+            // Check self local state for B
+            global CurrentApplicationAddress
+            txn Applications 1
+            byte "X"
+            app_local_get_ex
+            assert
+            byte "ABC"
+            ==
+            assert
+`),
+	}
+
+	appB := txntest.Txn{
+		Type:   "appl",
+		Sender: addrs[0],
+		ApprovalProgram: main(`
+  txn NumAppArgs
+  bnz check						// 1 arg means check
+  // set
+  txn Sender
+  byte "X"
+  byte "ABC"
+  app_local_put
+  b end
+check:
+  txn Sender
+  byte "X"
+  app_local_get
+  byte "ABC"
+  ==
+  assert
+  b end
+`),
+		LocalStateSchema: basics.StateSchema{
+			NumByteSlice: 1,
+		},
+	}
+
+	appC := txntest.Txn{
+		Type:   "appl",
+		Sender: addrs[0],
+		ApprovalProgram: main(`
+  txn Sender
+  txn Applications 1
+  byte "X"
+  app_local_get_ex
+  assert
+  byte "ABC"
+  ==
+  assert
+`),
+	}
+
+	eval := nextBlock(t, l, true, nil)
+	txns(t, l, eval, &appA, &appB, &appC)
+	vb := endBlock(t, l, eval)
+	indexA := vb.Block().Payset[0].ApplicationID
+	indexB := vb.Block().Payset[1].ApplicationID
+	indexC := vb.Block().Payset[2].ApplicationID
+
+	fundA := txntest.Txn{
+		Type:     "pay",
+		Sender:   addrs[0],
+		Receiver: indexA.Address(),
+		Amount:   1_000_000,
+	}
+
+	callA := txntest.Txn{
+		Type:          "appl",
+		Sender:        addrs[0],
+		ApplicationID: indexA,
+		ForeignApps:   []basics.AppIndex{indexB, indexC},
+	}
+
+	eval = nextBlock(t, l, true, nil)
+	txns(t, l, eval, &fundA, &callA)
+	endBlock(t, l, eval)
+}