Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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 @@ -1300,10 +1300,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.TotalBytesOfStorageInteractions(),
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
89 changes: 19 additions & 70 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 Expand Up @@ -1016,7 +965,7 @@ func TestLimitExceededErrorUsedExceedsLimit(t *testing.T) {
meter.DefaultParameters().WithStorageInteractionLimit(size - 1),
)

err := m.MeterStorageRead(key, value, true)
err := m.MeterStorageRead(key, value)
requireUsedAndLimit(t, err, errors.LimitKindLedgerInteraction, size, size-1)
})

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
Loading