Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion fvm/fvm_blockcontext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1299,10 +1299,13 @@ func TestBlockContext_ExecuteTransaction_InteractionLimitReached(t *testing.T) {
chain)
require.NoError(t, err)

// The account count is sized so that the metered interaction
// exceeds MaxStateInteractionSize, triggering the interaction
// limit from within Cadence execution.
_, txBodyBuilder := testutil.CreateMultiAccountCreationTransaction(
t,
chain,
40)
60)
Comment on lines +1303 to +1309

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the ledger-interaction failure cause.

The test only checks for a Cadence runtime error. A computation, memory, or event limit failure would also pass. Assert that output.Err contains errors.LimitKindLedgerInteraction to verify the new threshold behavior.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@fvm/fvm_blockcontext_test.go` around lines 1303 - 1309, Update the test using
CreateMultiAccountCreationTransaction to assert that output.Err contains
errors.LimitKindLedgerInteraction, in addition to the existing Cadence runtime
error check, so it verifies the specific ledger-interaction limit cause.


txBodyBuilder.SetProposalKey(chain.ServiceAddress(), 0, 0).
SetPayer(accounts[0])
Expand Down
20 changes: 11 additions & 9 deletions fvm/meter/interaction_meter.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,18 @@ func NewInteractionMeter(params InteractionMeterParameters) InteractionMeter {

// MeterStorageRead captures storage read bytes count.
//
// Only the first read of a given register is counted; later reads are treated
// as free cache hits. Consequently, if a read is not recorded here (e.g.
// because it happened while metering was disabled), a subsequent recorded read
// of the same register is counted as a fresh storage read.
//
// Expected error returns during normal operation:
// - [errors.LimitExceededError] with [errors.LimitKindLedgerInteraction] if
// the total bytes of storage interactions (reads + writes) exceeds the
// storage interaction limit and enforceLimit is true
// storage interaction limit
func (m *InteractionMeter) MeterStorageRead(
storageKey flow.RegisterID,
value flow.RegisterValue,
enforceLimit bool,
) error {

// all reads are on a View which only read from storage at the first read of a given key
Expand All @@ -69,7 +73,7 @@ func (m *InteractionMeter) MeterStorageRead(
m.reads[storageKey] = readByteSize
}

return m.checkStorageInteractionLimit(enforceLimit)
return m.checkStorageInteractionLimit()
}

// MeterStorageWrite captures storage written bytes count.
Expand All @@ -80,11 +84,10 @@ func (m *InteractionMeter) MeterStorageRead(
// Expected error returns during normal operation:
// - [errors.LimitExceededError] with [errors.LimitKindLedgerInteraction] if
// the total bytes of storage interactions (reads + writes) exceeds the
// storage interaction limit and enforceLimit is true
// storage interaction limit
func (m *InteractionMeter) MeterStorageWrite(
storageKey flow.RegisterID,
value flow.RegisterValue,
enforceLimit bool,
) error {
updateSize := getStorageKeyValueSize(storageKey, value)
m.replaceWrite(storageKey, updateSize)
Expand All @@ -95,7 +98,7 @@ func (m *InteractionMeter) MeterStorageWrite(
m.reads[storageKey] = 0
}

return m.checkStorageInteractionLimit(enforceLimit)
return m.checkStorageInteractionLimit()
}

// replaceWrite replaces the write size of a given key with the new size, because
Expand Down Expand Up @@ -128,9 +131,8 @@ func (m *InteractionMeter) replaceWrite(
m.totalStorageBytesWritten += newSize
}

func (m *InteractionMeter) checkStorageInteractionLimit(enforceLimit bool) error {
if enforceLimit &&
m.TotalBytesOfStorageInteractions() > m.params.storageInteractionLimit {
func (m *InteractionMeter) checkStorageInteractionLimit() error {
if m.TotalBytesOfStorageInteractions() > m.params.storageInteractionLimit {
return errors.NewLimitExceededError(
errors.LimitKindLedgerInteraction,
m.params.storageInteractionLimit)
Expand Down
8 changes: 4 additions & 4 deletions fvm/meter/interaction_meter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,22 +219,22 @@ func TestInteractionMeter_Merge(t *testing.T) {

var err error
if c.ParentReads != nil {
err = parentMeter.MeterStorageRead(key, c.ParentReads, false)
err = parentMeter.MeterStorageRead(key, c.ParentReads)
require.NoError(t, err)
}

if c.ChildReads != nil {
err = childMeter.MeterStorageRead(key, c.ChildReads, false)
err = childMeter.MeterStorageRead(key, c.ChildReads)
require.NoError(t, err)
}

if c.ParentWrites != nil {
err = parentMeter.MeterStorageWrite(key, c.ParentWrites, false)
err = parentMeter.MeterStorageWrite(key, c.ParentWrites)
require.NoError(t, err)
}

if c.ChildWrites != nil {
err = childMeter.MeterStorageWrite(key, c.ChildWrites, false)
err = childMeter.MeterStorageWrite(key, c.ChildWrites)
require.NoError(t, err)
}

Expand Down
87 changes: 18 additions & 69 deletions fvm/meter/meter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -661,12 +661,12 @@ func TestStorageLimits(t *testing.T) {
size1 := meter.GetStorageKeyValueSizeForTesting(key1, val1)

// first read of key1
err := meter1.MeterStorageRead(key1, val1, false)
err := meter1.MeterStorageRead(key1, val1)
require.NoError(t, err)
require.Equal(t, meter1.TotalBytesReadFromStorage(), size1)

// second read of key1
err = meter1.MeterStorageRead(key1, val1, false)
err = meter1.MeterStorageRead(key1, val1)
require.NoError(t, err)
require.Equal(t, meter1.TotalBytesReadFromStorage(), size1)

Expand All @@ -675,7 +675,7 @@ func TestStorageLimits(t *testing.T) {
val2 := []byte{0x3, 0x2, 0x1}
size2 := meter.GetStorageKeyValueSizeForTesting(key2, val2)

err = meter1.MeterStorageRead(key2, val2, false)
err = meter1.MeterStorageRead(key2, val2)
require.NoError(t, err)
require.Equal(t, meter1.TotalBytesReadFromStorage(), size1+size2)
})
Expand All @@ -690,36 +690,23 @@ func TestStorageLimits(t *testing.T) {
val2 := []byte{0x1, 0x2, 0x3, 0x4}

// first write of key1
err := meter1.MeterStorageWrite(key1, val1, false)
err := meter1.MeterStorageWrite(key1, val1)
require.NoError(t, err)
require.Equal(t, meter1.TotalBytesWrittenToStorage(), meter.GetStorageKeyValueSizeForTesting(key1, val1))

// second write of key1 with val2
err = meter1.MeterStorageWrite(key1, val2, false)
err = meter1.MeterStorageWrite(key1, val2)
require.NoError(t, err)
require.Equal(t, meter1.TotalBytesWrittenToStorage(), meter.GetStorageKeyValueSizeForTesting(key1, val2))

// first write of key2
key2 := flow.NewRegisterID(flow.EmptyAddress, "2")
err = meter1.MeterStorageWrite(key2, val2, false)
err = meter1.MeterStorageWrite(key2, val2)
require.NoError(t, err)
require.Equal(t, meter1.TotalBytesWrittenToStorage(),
meter.GetStorageKeyValueSizeForTesting(key1, val2)+meter.GetStorageKeyValueSizeForTesting(key2, val2))
})

t.Run("metering storage read - exceeding limit - not enforced", func(t *testing.T) {
meter1 := meter.NewMeter(
meter.DefaultParameters().WithStorageInteractionLimit(1),
)

key1 := flow.NewRegisterID(flow.EmptyAddress, "1")
val1 := []byte{0x1, 0x2, 0x3}

err := meter1.MeterStorageRead(key1, val1, false /* not enforced */)
require.NoError(t, err)
require.Equal(t, meter1.TotalBytesReadFromStorage(), meter.GetStorageKeyValueSizeForTesting(key1, val1))
})

t.Run("metering storage read - exceeding limit - enforced", func(t *testing.T) {
testLimit := uint64(1)
meter1 := meter.NewMeter(
Expand All @@ -729,24 +716,11 @@ func TestStorageLimits(t *testing.T) {
key1 := flow.NewRegisterID(flow.EmptyAddress, "1")
val1 := []byte{0x1, 0x2, 0x3}

err := meter1.MeterStorageRead(key1, val1, true /* enforced */)
err := meter1.MeterStorageRead(key1, val1)

unittest.RequireLimitExceededError(t, err, errors.LimitKindLedgerInteraction, testLimit)
})

t.Run("metering storage written - exceeding limit - not enforced", func(t *testing.T) {
testLimit := uint64(1)
meter1 := meter.NewMeter(
meter.DefaultParameters().WithStorageInteractionLimit(testLimit),
)

key1 := flow.NewRegisterID(flow.EmptyAddress, "1")
val1 := []byte{0x1, 0x2, 0x3}

err := meter1.MeterStorageWrite(key1, val1, false /* not enforced */)
require.NoError(t, err)
})

t.Run("metering storage written - exceeding limit - enforced", func(t *testing.T) {
testLimit := uint64(1)
meter1 := meter.NewMeter(
Expand All @@ -756,7 +730,7 @@ func TestStorageLimits(t *testing.T) {
key1 := flow.NewRegisterID(flow.EmptyAddress, "1")
val1 := []byte{0x1, 0x2, 0x3}

err := meter1.MeterStorageWrite(key1, val1, true /* enforced */)
err := meter1.MeterStorageWrite(key1, val1)

unittest.RequireLimitExceededError(t, err, errors.LimitKindLedgerInteraction, testLimit)
})
Expand All @@ -774,38 +748,13 @@ func TestStorageLimits(t *testing.T) {
size2 := meter.GetStorageKeyValueSizeForTesting(key2, val2)

// read of key1
err := meter1.MeterStorageRead(key1, val1, false)
require.NoError(t, err)
require.Equal(t, meter1.TotalBytesReadFromStorage(), size1)
require.Equal(t, meter1.TotalBytesOfStorageInteractions(), size1)

// write of key2
err = meter1.MeterStorageWrite(key2, val2, false)
require.NoError(t, err)
require.Equal(t, meter1.TotalBytesWrittenToStorage(), size2)
require.Equal(t, meter1.TotalBytesOfStorageInteractions(), size1+size2)
})

t.Run("metering storage read and written - exceeding limit - not enforced", func(t *testing.T) {
key1 := flow.NewRegisterID(flow.EmptyAddress, "1")
key2 := flow.NewRegisterID(flow.EmptyAddress, "2")
val1 := []byte{0x1, 0x2, 0x3}
val2 := []byte{0x1, 0x2, 0x3, 0x4}
size1 := meter.GetStorageKeyValueSizeForTesting(key1, val1)
size2 := meter.GetStorageKeyValueSizeForTesting(key2, val2)

meter1 := meter.NewMeter(
meter.DefaultParameters().WithStorageInteractionLimit(size1 + size2 - 1),
)

// read of key1
err := meter1.MeterStorageRead(key1, val1, false)
err := meter1.MeterStorageRead(key1, val1)
require.NoError(t, err)
require.Equal(t, meter1.TotalBytesReadFromStorage(), size1)
require.Equal(t, meter1.TotalBytesOfStorageInteractions(), size1)

// write of key2
err = meter1.MeterStorageWrite(key2, val2, false)
err = meter1.MeterStorageWrite(key2, val2)
require.NoError(t, err)
require.Equal(t, meter1.TotalBytesWrittenToStorage(), size2)
require.Equal(t, meter1.TotalBytesOfStorageInteractions(), size1+size2)
Expand All @@ -824,13 +773,13 @@ func TestStorageLimits(t *testing.T) {
)

// read of key1
err := meter1.MeterStorageRead(key1, val1, true)
err := meter1.MeterStorageRead(key1, val1)
require.NoError(t, err)
require.Equal(t, meter1.TotalBytesReadFromStorage(), size1)
require.Equal(t, meter1.TotalBytesOfStorageInteractions(), size1)

// write of key2
err = meter1.MeterStorageWrite(key2, val2, true)
err = meter1.MeterStorageWrite(key2, val2)
unittest.RequireLimitExceededError(t, err, errors.LimitKindLedgerInteraction, testLimit)
})

Expand All @@ -842,13 +791,13 @@ func TestStorageLimits(t *testing.T) {
readKey1 := flow.NewRegisterID(flow.EmptyAddress, "r1")
readVal1 := []byte{0x1, 0x2, 0x3}
readSize1 := meter.GetStorageKeyValueSizeForTesting(readKey1, readVal1)
err := meter1.MeterStorageRead(readKey1, readVal1, false)
err := meter1.MeterStorageRead(readKey1, readVal1)
require.NoError(t, err)

writeKey1 := flow.NewRegisterID(flow.EmptyAddress, "w1")
writeVal1 := []byte{0x1, 0x2, 0x3, 0x4}
writeSize1 := meter.GetStorageKeyValueSizeForTesting(writeKey1, writeVal1)
err = meter1.MeterStorageWrite(writeKey1, writeVal1, false)
err = meter1.MeterStorageWrite(writeKey1, writeVal1)
require.NoError(t, err)

// meter 2
Expand All @@ -860,17 +809,17 @@ func TestStorageLimits(t *testing.T) {
writeVal2 := []byte{0x1, 0x2, 0x3, 0x4, 0x5}
writeSize2 := meter.GetStorageKeyValueSizeForTesting(writeKey2, writeVal2)

err = meter1.MeterStorageRead(readKey1, readVal1, false)
err = meter1.MeterStorageRead(readKey1, readVal1)
require.NoError(t, err)

err = meter1.MeterStorageWrite(writeKey1, writeVal1, false)
err = meter1.MeterStorageWrite(writeKey1, writeVal1)
require.NoError(t, err)

// read the same key value as meter1
err = meter2.MeterStorageRead(readKey1, readVal1, false)
err = meter2.MeterStorageRead(readKey1, readVal1)
require.NoError(t, err)

err = meter2.MeterStorageWrite(writeKey2, writeVal2, false)
err = meter2.MeterStorageWrite(writeKey2, writeVal2)
require.NoError(t, err)

// merge
Expand Down
27 changes: 20 additions & 7 deletions fvm/storage/state/execution_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ func newLimitsController(params StateParameters) *limitsController {
}
}

// RunWithMeteringDisabled runs f with metering disabled. While metering is
// disabled, none of the metered quantities (computation, memory, events, and
// ledger interaction) are accumulated or limited. The previous metering state
// is restored afterwards, so nested calls behave correctly.
func (controller *limitsController) RunWithMeteringDisabled(f func()) {
if f == nil {
return
Expand Down Expand Up @@ -167,8 +171,9 @@ func (state *ExecutionState) DropChanges() error {
return state.spockState.DropChanges()
}

// Get returns a register value given owner and key. Limits are only enforced
// when metering is enabled.
// Get returns a register value given owner and key. Storage interaction is only
// metered (accumulated and limited) when metering is enabled; when metering is
// disabled the read is neither counted nor limited.
//
// Expected error returns during normal operation:
// - [errors.StateKeySizeLimitError] if the key exceeds the key size limit
Expand Down Expand Up @@ -197,12 +202,17 @@ 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.meteringEnabled)
return value, err
if state.meteringEnabled {
if err = state.meter.MeterStorageRead(id, value); err != nil {
return value, err
}
}
return value, nil
}

// Set updates state delta with a register update. Limits are only enforced
// when metering is enabled.
// Set updates state delta with a register update. Storage interaction is only
// metered (accumulated and limited) when metering is enabled; when metering is
// disabled the write is neither counted nor limited.
//
// Expected error returns during normal operation:
// - [errors.StateKeySizeLimitError] or [errors.StateValueSizeLimitError] if
Expand All @@ -229,7 +239,10 @@ 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.meteringEnabled)
if state.meteringEnabled {
return state.meter.MeterStorageWrite(id, value)
}
return nil
}

// MeterComputation meters computation usage. It is a no-op if metering is
Expand Down
Loading