Merge pull request onflow#5106 from onflow/janez/evm-addres-storage-l…

…imit-exception

Add storage limit check exception for EVM address

fvm/fvm_blockcontext_test.go

@@ -948,6 +948,9 @@ func TestBlockContext_ExecuteTransaction_StorageLimit(t *testing.T) {
 		fvm.WithAccountCreationFee(fvm.DefaultAccountCreationFee),
 		fvm.WithMinimumStorageReservation(fvm.DefaultMinimumStorageReservation),
 		fvm.WithStorageMBPerFLOW(fvm.DefaultStorageMBPerFLOW),
+		// The evm account has a storage exception, and if we don't bootstrap with evm,
+		// the first created account will have that address.
+		fvm.WithSetupEVMEnabled(true),
 	}
 
 	t.Run("Storing too much data fails", newVMTest().withBootstrapProcedureOptions(bootstrapOptions...).
@@ -1859,6 +1862,9 @@ func TestBlockContext_ExecuteTransaction_FailingTransactions(t *testing.T) {
 		fvm.WithAccountCreationFee(fvm.DefaultAccountCreationFee),
 		fvm.WithStorageMBPerFLOW(fvm.DefaultStorageMBPerFLOW),
 		fvm.WithExecutionMemoryLimit(math.MaxUint64),
+		// The evm account has a storage exception, and if we don't bootstrap with evm,
+		// the first created account will have that address.
+		fvm.WithSetupEVMEnabled(true),
 	).run(
 		func(t *testing.T, vm fvm.VM, chain flow.Chain, ctx fvm.Context, snapshotTree snapshot.SnapshotTree) {
 			ctx.LimitAccountStorage = true // this test requires storage limits to be enforced

fvm/transactionInvoker.go

@@ -406,6 +406,7 @@ func (executor *transactionExecutor) normalExecution() (
 	// actual balance, for the purpose of calculating storage capacity, because the payer will have to pay for this tx.
 	executor.txnState.RunWithAllLimitsDisabled(func() {
 		err = executor.CheckStorageLimits(
+			executor.ctx,
 			executor.env,
 			bodySnapshot,
 			executor.proc.Transaction.Payer,

fvm/transactionStorageLimiter.go

@@ -11,6 +11,7 @@ import (
 	"github.com/onflow/flow-go/fvm/environment"
 	"github.com/onflow/flow-go/fvm/errors"
 	"github.com/onflow/flow-go/fvm/storage/snapshot"
+	"github.com/onflow/flow-go/fvm/systemcontracts"
 	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/module/trace"
 )
@@ -33,6 +34,7 @@ type TransactionStorageLimiter struct{}
 // The payers balance is considered to be maxTxFees lower that its actual balance, due to the fact that
 // the fee deduction step happens after the storage limit check.
 func (limiter TransactionStorageLimiter) CheckStorageLimits(
+	ctx Context,
 	env environment.Environment,
 	snapshot *snapshot.ExecutionSnapshot,
 	payer flow.Address,
@@ -44,7 +46,7 @@ func (limiter TransactionStorageLimiter) CheckStorageLimits(
 
 	defer env.StartChildSpan(trace.FVMTransactionStorageUsedCheck).End()
 
-	err := limiter.checkStorageLimits(env, snapshot, payer, maxTxFees)
+	err := limiter.checkStorageLimits(ctx, env, snapshot, payer, maxTxFees)
 	if err != nil {
 		return fmt.Errorf("storage limit check failed: %w", err)
 	}
@@ -55,6 +57,7 @@ func (limiter TransactionStorageLimiter) CheckStorageLimits(
 // storage limit is exceeded.  The returned list include addresses of updated
 // registers (and the payer's address).
 func (limiter TransactionStorageLimiter) getStorageCheckAddresses(
+	ctx Context,
 	snapshot *snapshot.ExecutionSnapshot,
 	payer flow.Address,
 	maxTxFees uint64,
@@ -71,12 +74,17 @@ func (limiter TransactionStorageLimiter) getStorageCheckAddresses(
 		addresses = append(addresses, payer)
 	}
 
+	sc := systemcontracts.SystemContractsForChain(ctx.Chain.ChainID())
 	for id := range snapshot.WriteSet {
 		address, ok := addressFromRegisterId(id)
 		if !ok {
 			continue
 		}
 
+		if limiter.shouldSkipSpecialAddress(ctx, address, sc) {
+			continue
+		}
+
 		_, ok = dedup[address]
 		if ok {
 			continue
@@ -99,12 +107,13 @@ func (limiter TransactionStorageLimiter) getStorageCheckAddresses(
 // checkStorageLimits checks if the transaction changed the storage of any
 // address and exceeded the storage limit.
 func (limiter TransactionStorageLimiter) checkStorageLimits(
+	ctx Context,
 	env environment.Environment,
 	snapshot *snapshot.ExecutionSnapshot,
 	payer flow.Address,
 	maxTxFees uint64,
 ) error {
-	addresses := limiter.getStorageCheckAddresses(snapshot, payer, maxTxFees)
+	addresses := limiter.getStorageCheckAddresses(ctx, snapshot, payer, maxTxFees)
 
 	usages := make([]uint64, len(addresses))
 
@@ -155,3 +164,14 @@ func (limiter TransactionStorageLimiter) checkStorageLimits(
 
 	return nil
 }
+
+// shouldSkipSpecialAddress returns true if the address is a special address where storage
+// limits are not enforced.
+// This is currently only the EVM storage address. This is a temporary solution.
+func (limiter TransactionStorageLimiter) shouldSkipSpecialAddress(
+	ctx Context,
+	address flow.Address,
+	sc *systemcontracts.SystemContracts,
+) bool {
+	return sc.EVM.Address == address
+}

fvm/transactionStorageLimiter_test.go

@@ -8,9 +8,11 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/onflow/flow-go/fvm"
+	"github.com/onflow/flow-go/fvm/environment"
 	fvmmock "github.com/onflow/flow-go/fvm/environment/mock"
 	"github.com/onflow/flow-go/fvm/errors"
 	"github.com/onflow/flow-go/fvm/storage/snapshot"
+	"github.com/onflow/flow-go/fvm/systemcontracts"
 	"github.com/onflow/flow-go/fvm/tracing"
 	"github.com/onflow/flow-go/model/flow"
 )
@@ -24,10 +26,14 @@ func TestTransactionStorageLimiter(t *testing.T) {
 		},
 	}
 
+	ctx := fvm.Context{
+		EnvironmentParams: environment.EnvironmentParams{
+			Chain: flow.Emulator.Chain(),
+		},
+	}
+
 	t.Run("capacity > storage -> OK", func(t *testing.T) {
-		chain := flow.Mainnet.Chain()
 		env := &fvmmock.Environment{}
-		env.On("Chain").Return(chain)
 		env.On("LimitAccountStorage").Return(true)
 		env.On("StartChildSpan", mock.Anything).Return(
 			tracing.NewMockTracerSpan())
@@ -40,13 +46,11 @@ func TestTransactionStorageLimiter(t *testing.T) {
 		)
 
 		d := &fvm.TransactionStorageLimiter{}
-		err := d.CheckStorageLimits(env, executionSnapshot, flow.EmptyAddress, 0)
+		err := d.CheckStorageLimits(ctx, env, executionSnapshot, flow.EmptyAddress, 0)
 		require.NoError(t, err, "Transaction with higher capacity than storage used should work")
 	})
 	t.Run("capacity = storage -> OK", func(t *testing.T) {
-		chain := flow.Mainnet.Chain()
 		env := &fvmmock.Environment{}
-		env.On("Chain").Return(chain)
 		env.On("LimitAccountStorage").Return(true)
 		env.On("StartChildSpan", mock.Anything).Return(
 			tracing.NewMockTracerSpan())
@@ -59,13 +63,11 @@ func TestTransactionStorageLimiter(t *testing.T) {
 		)
 
 		d := &fvm.TransactionStorageLimiter{}
-		err := d.CheckStorageLimits(env, executionSnapshot, flow.EmptyAddress, 0)
+		err := d.CheckStorageLimits(ctx, env, executionSnapshot, flow.EmptyAddress, 0)
 		require.NoError(t, err, "Transaction with equal capacity than storage used should work")
 	})
 	t.Run("capacity = storage -> OK (dedup payer)", func(t *testing.T) {
-		chain := flow.Mainnet.Chain()
 		env := &fvmmock.Environment{}
-		env.On("Chain").Return(chain)
 		env.On("LimitAccountStorage").Return(true)
 		env.On("StartChildSpan", mock.Anything).Return(
 			tracing.NewMockTracerSpan())
@@ -78,13 +80,11 @@ func TestTransactionStorageLimiter(t *testing.T) {
 		)
 
 		d := &fvm.TransactionStorageLimiter{}
-		err := d.CheckStorageLimits(env, executionSnapshot, owner, 0)
+		err := d.CheckStorageLimits(ctx, env, executionSnapshot, owner, 0)
 		require.NoError(t, err, "Transaction with equal capacity than storage used should work")
 	})
 	t.Run("capacity < storage -> Not OK", func(t *testing.T) {
-		chain := flow.Mainnet.Chain()
 		env := &fvmmock.Environment{}
-		env.On("Chain").Return(chain)
 		env.On("LimitAccountStorage").Return(true)
 		env.On("StartChildSpan", mock.Anything).Return(
 			tracing.NewMockTracerSpan())
@@ -97,13 +97,11 @@ func TestTransactionStorageLimiter(t *testing.T) {
 		)
 
 		d := &fvm.TransactionStorageLimiter{}
-		err := d.CheckStorageLimits(env, executionSnapshot, flow.EmptyAddress, 0)
+		err := d.CheckStorageLimits(ctx, env, executionSnapshot, flow.EmptyAddress, 0)
 		require.Error(t, err, "Transaction with lower capacity than storage used should fail")
 	})
 	t.Run("capacity > storage -> OK (payer not updated)", func(t *testing.T) {
-		chain := flow.Mainnet.Chain()
 		env := &fvmmock.Environment{}
-		env.On("Chain").Return(chain)
 		env.On("LimitAccountStorage").Return(true)
 		env.On("StartChildSpan", mock.Anything).Return(
 			tracing.NewMockTracerSpan())
@@ -118,13 +116,11 @@ func TestTransactionStorageLimiter(t *testing.T) {
 		executionSnapshot = &snapshot.ExecutionSnapshot{}
 
 		d := &fvm.TransactionStorageLimiter{}
-		err := d.CheckStorageLimits(env, executionSnapshot, owner, 1)
+		err := d.CheckStorageLimits(ctx, env, executionSnapshot, owner, 1)
 		require.NoError(t, err, "Transaction with higher capacity than storage used should work")
 	})
 	t.Run("capacity < storage -> Not OK (payer not updated)", func(t *testing.T) {
-		chain := flow.Mainnet.Chain()
 		env := &fvmmock.Environment{}
-		env.On("Chain").Return(chain)
 		env.On("LimitAccountStorage").Return(true)
 		env.On("StartChildSpan", mock.Anything).Return(
 			tracing.NewMockTracerSpan())
@@ -139,13 +135,11 @@ func TestTransactionStorageLimiter(t *testing.T) {
 		executionSnapshot = &snapshot.ExecutionSnapshot{}
 
 		d := &fvm.TransactionStorageLimiter{}
-		err := d.CheckStorageLimits(env, executionSnapshot, owner, 1000)
+		err := d.CheckStorageLimits(ctx, env, executionSnapshot, owner, 1000)
 		require.Error(t, err, "Transaction with lower capacity than storage used should fail")
 	})
 	t.Run("if ctx LimitAccountStorage false-> OK", func(t *testing.T) {
-		chain := flow.Mainnet.Chain()
 		env := &fvmmock.Environment{}
-		env.On("Chain").Return(chain)
 		env.On("LimitAccountStorage").Return(false)
 		env.On("StartChildSpan", mock.Anything).Return(
 			tracing.NewMockTracerSpan())
@@ -159,27 +153,63 @@ func TestTransactionStorageLimiter(t *testing.T) {
 		)
 
 		d := &fvm.TransactionStorageLimiter{}
-		err := d.CheckStorageLimits(env, executionSnapshot, flow.EmptyAddress, 0)
+		err := d.CheckStorageLimits(ctx, env, executionSnapshot, flow.EmptyAddress, 0)
 		require.NoError(t, err, "Transaction with higher capacity than storage used should work")
 	})
-	t.Run("non existing accounts or any other errors on fetching storage used -> Not OK", func(t *testing.T) {
-		chain := flow.Mainnet.Chain()
+	t.Run(
+		"non existing accounts or any other errors on fetching storage used -> Not OK",
+		func(t *testing.T) {
+			env := &fvmmock.Environment{}
+			env.On("LimitAccountStorage").Return(true)
+			env.On("StartChildSpan", mock.Anything).Return(
+				tracing.NewMockTracerSpan())
+			env.On("GetStorageUsed", mock.Anything).
+				Return(uint64(0), errors.NewAccountNotFoundError(owner))
+			env.On("AccountsStorageCapacity", mock.Anything, mock.Anything, mock.Anything).Return(
+				cadence.NewArray([]cadence.Value{
+					bytesToUFix64(100),
+				}),
+				nil,
+			)
+
+			d := &fvm.TransactionStorageLimiter{}
+			err := d.CheckStorageLimits(ctx, env, executionSnapshot, flow.EmptyAddress, 0)
+			require.Error(
+				t,
+				err,
+				"check storage used on non existing account (not general registers) should fail",
+			)
+		},
+	)
+	t.Run("special account is skipped", func(t *testing.T) {
+		sc := systemcontracts.SystemContractsForChain(ctx.Chain.ChainID())
+		evm := sc.EVM.Address
+
+		executionSnapshot := &snapshot.ExecutionSnapshot{
+			WriteSet: map[flow.RegisterID]flow.RegisterValue{
+				flow.NewRegisterID(evm, "a"): flow.RegisterValue("foo"),
+			},
+		}
+
 		env := &fvmmock.Environment{}
-		env.On("Chain").Return(chain)
 		env.On("LimitAccountStorage").Return(true)
 		env.On("StartChildSpan", mock.Anything).Return(
 			tracing.NewMockTracerSpan())
-		env.On("GetStorageUsed", mock.Anything).Return(uint64(0), errors.NewAccountNotFoundError(owner))
-		env.On("AccountsStorageCapacity", mock.Anything, mock.Anything, mock.Anything).Return(
-			cadence.NewArray([]cadence.Value{
-				bytesToUFix64(100),
-			}),
-			nil,
-		)
+		env.On("GetStorageUsed", mock.Anything).
+			Return(uint64(0), errors.NewAccountNotFoundError(owner))
+		env.On("AccountsStorageCapacity", mock.Anything, mock.Anything, mock.Anything).
+			Run(func(args mock.Arguments) {
+				require.Len(t, args.Get(0).([]flow.Address), 0)
+			}).
+			Return(
+				// since the special account is skipped, the resulting array from AccountsStorageCapacity should be empty
+				cadence.NewArray([]cadence.Value{}),
+				nil,
+			)
 
 		d := &fvm.TransactionStorageLimiter{}
-		err := d.CheckStorageLimits(env, executionSnapshot, flow.EmptyAddress, 0)
-		require.Error(t, err, "check storage used on non existing account (not general registers) should fail")
+		err := d.CheckStorageLimits(ctx, env, executionSnapshot, flow.EmptyAddress, 0)
+		require.NoError(t, err)
 	})
 }