Merge pull request #7810 from onflow/janez/metering-refactor-and-cleanup

Refactor and cleanup FVM metering

Author: janezpodhostnik

fvm/environment/account_creator.go

@@ -265,7 +265,7 @@ func (creator *accountCreator) CreateAccount(
 
 	// don't enforce limit during account creation
 	var address flow.Address
-	creator.txnState.RunWithAllLimitsDisabled(func() {
+	creator.txnState.RunWithMeteringDisabled(func() {
 		address, err = creator.createAccount(flow.ConvertAddress(runtimePayer))
 	})
 

fvm/environment/accounts.go

@@ -74,7 +74,7 @@ func (a *StatefulAccounts) AllocateSlabIndex(
 	// compute storage size changes)
 	// this way the getValue would load this value from deltas
 	key := atree.SlabIndexToLedgerKey(index)
-	a.txnState.RunWithAllLimitsDisabled(func() {
+	a.txnState.RunWithMeteringDisabled(func() {
 		err = a.txnState.Set(
 			flow.NewRegisterID(address, string(key)),
 			[]byte{})

fvm/environment/meter.go

@@ -104,10 +104,6 @@ func (meter *meterImpl) ComputationAvailable(usage common.ComputationUsage) bool
 	return meter.txnState.ComputationAvailable(usage)
 }
 
-func (meter *meterImpl) ComputationRemaining(kind common.ComputationKind) uint64 {
-	return meter.txnState.ComputationRemaining(kind)
-}
-
 func (meter *meterImpl) ComputationUsed() (uint64, error) {
 	return meter.txnState.TotalComputationUsed(), nil
 }

fvm/executionParameters.go

@@ -127,7 +127,7 @@ func (computer ExecutionParametersComputer) Compute(
 ) {
 	var overrides derived.StateExecutionParameters
 	var err error
-	computer.txnState.RunWithAllLimitsDisabled(func() {
+	computer.txnState.RunWithMeteringDisabled(func() {
 		overrides, err = computer.getExecutionParameters()
 	})
 

fvm/meter/computation_meter.go

@@ -118,23 +118,6 @@ func (m *ComputationMeter) ComputationAvailable(usage common.ComputationUsage) b
 	return potentialComputationUsage <= m.params.computationLimit
 }
 
-// ComputationRemaining returns the remaining computation (intensity) left in the transaction for the given type
-func (m *ComputationMeter) ComputationRemaining(kind common.ComputationKind) uint64 {
-	w, ok := m.params.computationWeights[kind]
-	// if not found return has capacity
-	// given the behaviour of MeterComputation is ignoring intensities without a set weight
-	if !ok {
-		return math.MaxUint64
-	}
-
-	remainingComputationUsage := m.params.computationLimit - m.computationUsed
-	if remainingComputationUsage <= 0 {
-		return 0
-	}
-
-	return remainingComputationUsage / w
-}
-
 // ComputationIntensities returns all the measured computational intensities
 func (m *ComputationMeter) ComputationIntensities() MeteredComputationIntensities {
 	return m.computationIntensities

fvm/meter/meter_test.go

@@ -214,16 +214,16 @@ func TestWeightedComputationMetering(t *testing.T) {
 		))
 	})
 
-	t.Run("check computation remaining", func(t *testing.T) {
+	t.Run("check computation available", func(t *testing.T) {
 		m := meter.NewMeter(
 			meter.DefaultParameters().
 				WithComputationLimit(10).
 				WithComputationWeights(
 					map[common.ComputationKind]uint64{0: 1 << meter.MeterExecutionInternalPrecisionBytes}),
 		)
 
-		remaining := m.ComputationRemaining(0)
-		require.Equal(t, uint64(10), remaining)
+		available := m.ComputationAvailable(common.ComputationUsage{Kind: 0, Intensity: 10})
+		require.True(t, available)
 
 		err := m.MeterComputation(
 			common.ComputationUsage{
@@ -234,10 +234,10 @@ func TestWeightedComputationMetering(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, uint64(1), m.TotalComputationUsed())
 
-		require.Equal(t, uint64(9), m.ComputationRemaining(0))
+		require.False(t, m.ComputationAvailable(common.ComputationUsage{Kind: 0, Intensity: 10}))
 
-		// test a type without a weight (default zero)
-		require.Equal(t, uint64(math.MaxUint64), m.ComputationRemaining(1))
+		// test a type without a weight (default MaxUint64)
+		require.True(t, m.ComputationAvailable(common.ComputationUsage{Kind: 1, Intensity: math.MaxUint64}))
 	})
 
 	t.Run("merge meters", func(t *testing.T) {

fvm/storage/state/execution_state.go

@@ -2,7 +2,6 @@ package state
 
 import (
 	"fmt"
-	"math"
 
 	"github.com/onflow/cadence/common"
 	"github.com/onflow/crypto/hash"
@@ -81,27 +80,27 @@ func (params StateParameters) WithMaxValueSizeAllowed(
 }
 
 type limitsController struct {
-	enforceLimits       bool
+	meteringEnabled     bool
 	maxKeySizeAllowed   uint64
 	maxValueSizeAllowed uint64
 }
 
 func newLimitsController(params StateParameters) *limitsController {
 	return &limitsController{
-		enforceLimits:       true,
+		meteringEnabled:     true,
 		maxKeySizeAllowed:   params.maxKeySizeAllowed,
 		maxValueSizeAllowed: params.maxValueSizeAllowed,
 	}
 }
 
-func (controller *limitsController) RunWithAllLimitsDisabled(f func()) {
+func (controller *limitsController) RunWithMeteringDisabled(f func()) {
 	if f == nil {
 		return
 	}
-	current := controller.enforceLimits
-	controller.enforceLimits = false
+	current := controller.meteringEnabled
+	controller.meteringEnabled = false
 	f()
-	controller.enforceLimits = current
+	controller.meteringEnabled = current
 }
 
 // NewExecutionState constructs a new state
@@ -176,7 +175,7 @@ func (state *ExecutionState) Get(id flow.RegisterID) (flow.RegisterValue, error)
 	var value []byte
 	var err error
 
-	if state.enforceLimits {
+	if state.meteringEnabled {
 		if err = state.checkSize(id, []byte{}); err != nil {
 			return nil, err
 		}
@@ -189,7 +188,7 @@ func (state *ExecutionState) Get(id flow.RegisterID) (flow.RegisterValue, error)
 		return nil, fmt.Errorf("failed to read %s: %w", id, getError)
 	}
 
-	err = state.meter.MeterStorageRead(id, value, state.enforceLimits)
+	err = state.meter.MeterStorageRead(id, value, state.meteringEnabled)
 	return value, err
 }
 
@@ -199,7 +198,7 @@ func (state *ExecutionState) Set(id flow.RegisterID, value flow.RegisterValue) e
 		return fmt.Errorf("cannot Set on a finalized state")
 	}
 
-	if state.enforceLimits {
+	if state.meteringEnabled {
 		if err := state.checkSize(id, value); err != nil {
 			return err
 		}
@@ -212,7 +211,7 @@ func (state *ExecutionState) Set(id flow.RegisterID, value flow.RegisterValue) e
 		return fmt.Errorf("failed to update %s: %w", id, setError)
 	}
 
-	return state.meter.MeterStorageWrite(id, value, state.enforceLimits)
+	return state.meter.MeterStorageWrite(id, value, state.meteringEnabled)
 }
 
 // MeterComputation meters computation usage
@@ -221,7 +220,7 @@ func (state *ExecutionState) MeterComputation(usage common.ComputationUsage) err
 		return fmt.Errorf("cannot MeterComputation on a finalized state")
 	}
 
-	if state.enforceLimits {
+	if state.meteringEnabled {
 		return state.meter.MeterComputation(usage)
 	}
 	return nil
@@ -234,26 +233,12 @@ func (state *ExecutionState) ComputationAvailable(usage common.ComputationUsage)
 		return false
 	}
 
-	if state.enforceLimits {
+	if state.meteringEnabled {
 		return state.meter.ComputationAvailable(usage)
 	}
 	return true
 }
 
-// ComputationRemaining returns the available computation capacity without metering
-func (state *ExecutionState) ComputationRemaining(kind common.ComputationKind) uint64 {
-	if state.finalized {
-		// if state is finalized return 0
-		return 0
-	}
-
-	if state.enforceLimits {
-		return state.meter.ComputationRemaining(kind)
-	}
-
-	return math.MaxUint64
-}
-
 // TotalComputationUsed returns total computation used
 func (state *ExecutionState) TotalComputationUsed() uint64 {
 	return state.meter.TotalComputationUsed()
@@ -275,7 +260,7 @@ func (state *ExecutionState) MeterMemory(usage common.MemoryUsage) error {
 		return fmt.Errorf("cannot MeterMemory on a finalized state")
 	}
 
-	if state.enforceLimits {
+	if state.meteringEnabled {
 		return state.meter.MeterMemory(usage)
 	}
 
@@ -302,7 +287,7 @@ func (state *ExecutionState) MeterEmittedEvent(byteSize uint64) error {
 		return fmt.Errorf("cannot MeterEmittedEvent on a finalized state")
 	}
 
-	if state.enforceLimits {
+	if state.meteringEnabled {
 		return state.meter.MeterEmittedEvent(byteSize)
 	}
 

fvm/storage/state/transaction_state.go

@@ -22,7 +22,6 @@ func (id NestedTransactionId) StateForTestingOnly() *ExecutionState {
 type Meter interface {
 	MeterComputation(usage common.ComputationUsage) error
 	ComputationAvailable(usage common.ComputationUsage) bool
-	ComputationRemaining(kind common.ComputationKind) uint64
 	ComputationIntensities() meter.MeteredComputationIntensities
 	TotalComputationLimit() uint64
 	TotalComputationUsed() uint64
@@ -36,8 +35,13 @@ type Meter interface {
 	MeterEmittedEvent(byteSize uint64) error
 	TotalEmittedEventBytes() uint64
 
-	// RunWithAllLimitsDisabled runs f with limits disabled
-	RunWithAllLimitsDisabled(f func())
+	// RunWithMeteringDisabled runs f with limits disabled
+	// This function can be used to run a function that fits one of these cases:
+	// - the function should not fail due to metering limits
+	// - the function is not invokable by the user and has a constant execution time (e.g. fee deduction)
+	// - the function is metered once before calling `RunWithMeteringDisabled` with a special weight (e.g. create account)
+	// 		and doesn't need additional metering inside the function
+	RunWithMeteringDisabled(f func())
 }
 
 // NestedTransactionPreparer provides active transaction states and facilitates
@@ -453,10 +457,6 @@ func (txnState *transactionState) ComputationAvailable(usage common.ComputationU
 	return txnState.current().ComputationAvailable(usage)
 }
 
-func (txnState *transactionState) ComputationRemaining(kind common.ComputationKind) uint64 {
-	return txnState.current().ComputationRemaining(kind)
-}
-
 func (txnState *transactionState) MeterMemory(usage common.MemoryUsage) error {
 	return txnState.current().MeterMemory(usage)
 }
@@ -493,6 +493,6 @@ func (txnState *transactionState) TotalEmittedEventBytes() uint64 {
 	return txnState.current().TotalEmittedEventBytes()
 }
 
-func (txnState *transactionState) RunWithAllLimitsDisabled(f func()) {
-	txnState.current().RunWithAllLimitsDisabled(f)
+func (txnState *transactionState) RunWithMeteringDisabled(f func()) {
+	txnState.current().RunWithMeteringDisabled(f)
 }

fvm/transactionInvoker.go

@@ -281,7 +281,7 @@ func (executor *transactionExecutor) ExecuteTransactionBody() error {
 
 	if executor.errs.CollectedError() {
 		invalidator = nil
-		executor.txnState.RunWithAllLimitsDisabled(executor.errorExecution)
+		executor.txnState.RunWithMeteringDisabled(executor.errorExecution)
 		if executor.errs.CollectedFailure() {
 			return executor.errs.ErrorOrNil()
 		}
@@ -360,7 +360,7 @@ func (executor *transactionExecutor) normalExecution() (
 	var maxTxFees uint64
 	// run with limits disabled since this is a static cost check
 	// and should be accounted for in the inclusion cost.
-	executor.txnState.RunWithAllLimitsDisabled(func() {
+	executor.txnState.RunWithMeteringDisabled(func() {
 		maxTxFees, err = executor.CheckPayerBalanceAndReturnMaxFees(
 			executor.proc,
 			executor.txnState,
@@ -424,7 +424,7 @@ func (executor *transactionExecutor) normalExecution() (
 		return
 	}
 
-	executor.txnState.RunWithAllLimitsDisabled(func() {
+	executor.txnState.RunWithMeteringDisabled(func() {
 		err = executor.deductTransactionFees()
 	})
 

fvm/transactionPayerBalanceChecker.go

@@ -73,7 +73,7 @@ func (_ TransactionPayerBalanceChecker) CheckPayerBalanceAndReturnMaxFees(
 
 	var resultValue cadence.Value
 	var err error
-	txnState.RunWithAllLimitsDisabled(func() {
+	txnState.RunWithMeteringDisabled(func() {
 		// Don't meter the payer balance check.
 		// It has a static cost, and its cost should be part of the inclusion fees, not part of execution fees.
 		resultValue, err = env.CheckPayerBalanceAndGetMaxTxFees(