Use execution result's MaxGasConsumed for gas estimation

[m-Peter]

Depends on: onflow/flow-go#7951
Work towards: #840

Description

See also: ethereum/go-ethereum#31735

---

For contributor use:

• Targeted PR against master branch
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Code follows the standards mentioned here.
• Updated relevant documentation
• Re-reviewed Files changed in the Github PR explorer
• Added appropriate labels

Summary by CodeRabbit

• Bug Fixes

  • Improved gas limit estimation by switching to a more accurate gas metric, yielding more reliable transaction execution.

• Tests

  • Updated test fixtures and event expectations to match the revised gas reporting metric.

• Chores

  • Upgraded key dependencies (Flow Go, Flow Go SDK, core contracts/templates, protobuf) and aligned test module dependencies for compatibility and stability.

[coderabbitai]

Walkthrough

Updates dependency versions in project and test go.mod files. Replaces use of GasConsumed+GasRefund with Result.MaxGasConsumed in gas estimation logic and updates a test to set MaxGasConsumed accordingly. No exported API signatures changed.

Changes

Cohort / File(s)	Summary
Dependency updates go.mod, tests/go.mod	Bump onflow/flow-go, onflow/flow-go-sdk and flow-core-contracts subpackages; minor protobuf indirect update and an indirect lru addition in tests.
Gas estimation logic services/requester/requester.go	Compute optimisticGasLimit from result.MaxGasConsumed (plus CallStipend) instead of result.GasConsumed + result.GasRefund; adjust comment.
Test adjustments models/events_test.go	Replace GasRefund usage with MaxGasConsumed in constructed transaction result (value changed from 0→1).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant R as Requester
  participant F as FVM/EVM

  C->>R: Request EstimateGas(call)
  R->>F: Dry-run call with initial gas limit
  F-->>R: result { MaxGasConsumed, ... }
  Note over R: optimisticGasLimit = (MaxGasConsumed + CallStipend) * 64/63
  alt optimisticGasLimit < passingGasLimit
    R->>F: Dry-run with optimisticGasLimit
    F-->>R: result / status
    R->>R: Adjust bounds and continue binary search
  else
    R->>R: Keep existing bounds / proceed
  end
  R-->>C: Return estimated gas

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Add metric to count iterations taken for eth_estimateGas #725 — Modifies EstimateGas logic in services/requester/requester.go (dry-run wrapper and iteration counting).
• Handle ErrHeightOutOfRange errors in EstimateGas #498 — Changes EstimateGas logic and adds error/log handling in requester.go.
• Update eth_estimateGas to correctly handle execution reverts #303 — Also updates how gas consumption/refunds are calculated and handled in requester.go.

Suggested labels

Bugfix

Suggested reviewers

• peterargue
• zhangchiqing

Poem

I twitch my whiskers at MaxGas found,
No refunds hopping on the ground.
I nibble deps and tweak the test,
A burrow polished, tidy, best.
Rabbit hops off — the build runs sound. 🐇✨

Pre-merge checks and finishing touches

❌ 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%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ 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 succinctly and accurately describes the primary change—switching gas estimation to use MaxGasConsumed from the execution result—without extraneous details, making it clear for teammates what the PR achieves.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch mpeter/gas-estimation-use-max-gas-consumed

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ea8aeb3 and 7c0737b.

⛔ Files ignored due to path filters (2)

• go.sum is excluded by !**/*.sum
• tests/go.sum is excluded by !**/*.sum

📒 Files selected for processing (4)

• go.mod (3 hunks)
• models/events_test.go (1 hunks)
• services/requester/requester.go (1 hunks)
• tests/go.mod (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

• go.mod
• services/requester/requester.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: Lint
• GitHub Check: Test

🔇 Additional comments (4)

tests/go.mod (3)

12-12: LGTM! Dependency bump aligns with PR objectives.

The flow-go version bump to the RC that includes the MaxGasConsumed field is necessary for this PR's gas estimation changes.

---

76-76: LGTM! Expected indirect dependency.

The new golang-lru/v2 indirect dependency is expected from upstream flow-go changes.

---

160-161: LGTM! Core contracts library updates.

The flow-core-contracts version bumps to v1.9.0 are consistent with the overall dependency updates.

models/events_test.go (1)

575-595: LGTM! Test data updated to use MaxGasConsumed.

The change from GasRefund to MaxGasConsumed correctly reflects the upstream field rename in the flow-go types.Result struct. The value of 1 is appropriate for this test fixture, matching the GasConsumed value.

---

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.}