[Flow EVM] Add proper meter and gas limit checks for EVM dry operations

[m-Peter]

Closes: https://github.com/onflow/flow-go-internal/issues/7159

Summary by CodeRabbit

• Bug Fixes

  • Enhanced EVM dry-run operations with proper gas metering and pre-checks for sufficient gas limits to prevent operations from exceeding allocation.

• Tests

  • Added comprehensive tests for dry-run and dry-call operations, including scenarios for insufficient computation limits and proper cumulative gas usage tracking across multiple iterations.

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[coderabbitai]

📝 Walkthrough

Walkthrough

These changes add comprehensive tests for EVM dry-run and dry-call operations with gas metering validation, and modify the EVM handler to enforce gas pre-checks, disable metering during execution, and measure gas consumption post-call.

Changes

Cohort / File(s)	Summary
EVM Dry-Run/Dry-Call Tests fvm/evm/evm_test.go	Added four new test cases validating insufficient computation limit errors and proper gas metering for both dryRun and dryCall operations. Wired execution effort weights into bootstrap paths for more realistic test conditions.
EVM Handler Gas Metering fvm/evm/handler/handler.go	Modified dryRunTx to check gas availability before block proposal, disable metering during transaction execution, and meter gas usage post-call with error handling.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• onflow/flow-go#8400: Refactors EVM dry-run flow in the same handler file and introduces related DryRunWithTxData functionality for consistent metering patterns.

Suggested reviewers

• zhangchiqing
• turbolent

Poem

🐰 A dry run hops with metering grace,
Gas limits checked at every pace,
No meters tick within the call,
But measured after, tallied all—
Precise computation, nothing small! ⚡

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main changes: adding proper metering and gas limit checks for EVM dry operations, which is directly reflected in the handler modifications and new test cases.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch mpeter/meter-evm-dry-call-operations

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov-commenter]

Codecov Report

❌ Patch coverage is 55.55556% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
fvm/evm/handler/handler.go	55.55%	2 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

[coderabbitai]

🧹 Nitpick comments (2)

fvm/evm/evm_test.go (2)

3104-3195: Clarify the loop count comment and consider documenting expected computation breakdown.

The Cadence loop while i <= iterations executes iterations + 1 times, so iterations=5 yields 6 calls and iterations=15 yields 16 calls. The comment on line 3174 ("Triple to number of times EVM.dryRun is called") is slightly imprecise — the iteration parameter is tripled (5 → 15), but the actual call count goes from 6 to 16, which is not exactly 3×.

Also, the hardcoded expected values (40 and 102) are brittle — they'll break if MainnetExecutionEffortWeights or EVM gas costs change. A brief inline comment explaining how these values are derived (e.g., 6 iterations × gasConsumed × weight(3) + Cadence overhead) would aid future maintainability.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@fvm/evm/evm_test.go` around lines 3104 - 3195, The test's comment and
hardcoded expectations are misleading because the Cadence loop `while i <=
iterations` calls EVM.dryRun iterations+1 times (so iterations=5 → 6 calls,
iterations=15 → 16 calls); update the inline comment near the `iterations` setup
and the "Triple to number..." line to state that the iteration parameter is
tripled but call count goes 6→16, and either replace the hardcoded asserts for
`output.ComputationUsed` with a computed expected value based on (iterations+1)
× per-call cost × weight retrieved from MainnetExecutionEffortWeights (or add a
clear explanatory comment showing the derivation e.g., "(iterations+1) ×
per-call-cost + Cadence overhead = 40/102"), referencing the test function and
variables `iterations`, `EVM.dryRun`, and the two `assert.Equal(t, uint64(...),
output.ComputationUsed)` assertions so future changes to weights/gas don't
silently break the test.

---

3643-3740: Same comment as the dryRun metering test — loop count nuance and brittle hardcoded values.

Same observation applies: iterations=5 → 6 calls, iterations=15 → 16 calls, and 53/139 are exact computation assertions that will break with weight changes. This is consistent with the dryRun metering test structure.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@fvm/evm/evm_test.go` around lines 3643 - 3740, The test "test EVM.dryCall is
properly metered" uses an inclusive loop (iterations N produces N+1 calls) and
asserts brittle hardcoded computation values (53/139); update the test to
compute the expected computation dynamically: determine calls :=
iterations.Value + 1, capture computationUsed for a baseline iterations (e.g.,
5) and compute perCall cost from that run (or compute expected as baseCost +
calls*perCall), then replace hardcoded asserts on output.ComputationUsed with a
comparison against the calculated expected value (or a tolerant range) using the
variables iterations and output.ComputationUsed; adjust references to
iterations, output.ComputationUsed and the transaction build in the test to use
the computed expectations rather than literals.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@fvm/evm/evm_test.go`:
- Around line 3104-3195: The test's comment and hardcoded expectations are
misleading because the Cadence loop `while i <= iterations` calls EVM.dryRun
iterations+1 times (so iterations=5 → 6 calls, iterations=15 → 16 calls); update
the inline comment near the `iterations` setup and the "Triple to number..."
line to state that the iteration parameter is tripled but call count goes 6→16,
and either replace the hardcoded asserts for `output.ComputationUsed` with a
computed expected value based on (iterations+1) × per-call cost × weight
retrieved from MainnetExecutionEffortWeights (or add a clear explanatory comment
showing the derivation e.g., "(iterations+1) × per-call-cost + Cadence overhead
= 40/102"), referencing the test function and variables `iterations`,
`EVM.dryRun`, and the two `assert.Equal(t, uint64(...), output.ComputationUsed)`
assertions so future changes to weights/gas don't silently break the test.
- Around line 3643-3740: The test "test EVM.dryCall is properly metered" uses an
inclusive loop (iterations N produces N+1 calls) and asserts brittle hardcoded
computation values (53/139); update the test to compute the expected computation
dynamically: determine calls := iterations.Value + 1, capture computationUsed
for a baseline iterations (e.g., 5) and compute perCall cost from that run (or
compute expected as baseCost + calls*perCall), then replace hardcoded asserts on
output.ComputationUsed with a comparison against the calculated expected value
(or a tolerant range) using the variables iterations and output.ComputationUsed;
adjust references to iterations, output.ComputationUsed and the transaction
build in the test to use the computed expectations rather than literals.