Skip to content

Conversation

@curryxbo
Copy link
Contributor

@curryxbo curryxbo commented Dec 18, 2025

1. Purpose or design rationale of this PR

...

2. PR title

Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:

  • build: Changes that affect the build system or external dependencies (example scopes: yarn, eslint, typescript)
  • ci: Changes to our CI configuration files and scripts (example scopes: vercel, github, cypress)
  • docs: Documentation-only changes
  • feat: A new feature
  • fix: A bug fix
  • perf: A code change that improves performance
  • refactor: A code change that doesn't fix a bug, or add a feature, or improves performance
  • style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
  • test: Adding missing tests or correcting existing tests

3. Deployment tag versioning

Has the version in params/version.go been updated?

  • This PR doesn't involve a new deployment, git tag, docker image tag, and it doesn't affect traces
  • Yes

4. Breaking change label

Does this PR have the breaking-change label?

  • This PR is not a breaking change
  • Yes

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed gas estimation to correctly calculate intrinsic gas costs for access lists, improving the accuracy of gas cost predictions for affected transactions.

✏️ Tip: You can customize this high-level summary in your review settings.

@curryxbo curryxbo requested a review from a team as a code owner December 18, 2025 13:22
@curryxbo curryxbo requested review from twcctop and removed request for a team December 18, 2025 13:22
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 18, 2025

Walkthrough

The AccessList function in internal/ethapi/api.go was modified to assign the current iteration's access list to args.AccessList before performing gas estimation, ensuring DoEstimateGas correctly accounts for access list intrinsic gas costs. This reordering corrects the timing of when the access list is applied relative to gas calculation.

Changes

Cohort / File(s) Summary
Gas estimation access list ordering
internal/ethapi/api.go
Moved args.AccessList assignment before DoEstimateGas call to ensure gas estimation accounts for access list intrinsic costs; replaced later redundant assignment

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify the gas estimation logic correctly incorporates the access list when assigned at the new location
  • Confirm the removed later assignment was genuinely redundant and not serving an alternate purpose
  • Trace through loop iterations to ensure the reordering doesn't introduce state-ordering bugs

Poem

A rabbit hops through gas with glee,
Access lists now flow more free,
Before the math, the costs align,
Estimation now works fine! 🐰✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix accessList' directly relates to the main change in the pull request, which is fixing the accessList assignment order in the AccessList function to ensure proper gas estimation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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 fix_accesslist

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between efcdc5e and 4d5fda0.

📒 Files selected for processing (1)
  • internal/ethapi/api.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
internal/ethapi/api.go (1)
core/types/access_list_tx.go (1)
  • AccessList (30-30)
⏰ 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). (3)
  • GitHub Check: Analyze (python)
  • GitHub Check: Analyze (c-cpp)
  • GitHub Check: Analyze (go)
🔇 Additional comments (1)
internal/ethapi/api.go (1)

1673-1675: Incorrect EIP-2930 gas cost explanation.

The correct EIP-2930 costs are 2400 gas per address and 1900 gas per storage key, not "2400 gas per storage key + 1900 gas per address" as stated. This reversal of costs demonstrates a fundamental misunderstanding of the access list mechanism and weakens the technical justification for the code change.

Likely an incorrect or invalid review comment.


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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants