Skip to content

ethapi, eth: implement eth_baseFee RPC method#2231

Open
wjmelements wants to merge 2 commits into
0xPolygon:developfrom
wjmelements:eth-base-fee
Open

ethapi, eth: implement eth_baseFee RPC method#2231
wjmelements wants to merge 2 commits into
0xPolygon:developfrom
wjmelements:eth-base-fee

Conversation

@wjmelements
Copy link
Copy Markdown
Contributor

@wjmelements wjmelements commented May 19, 2026

Summary

Add the nonstandard eth_baseFee method, which returns the calculated base fee of the next block.

Executed tests

None

Rollout notes

Non-breaking

Assisted-by: Claude:claude-sonnet-4-6
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@pratikspatil024
Copy link
Copy Markdown
Member

@claude review once

@claude
Copy link
Copy Markdown

claude Bot commented May 19, 2026

Claude finished @pratikspatil024's task in 1m 37s —— View job


Code Review: eth_baseFee RPC method

  • Fetch team standards
  • Read changed files and gather context
  • Analyze implementation correctness
  • Post review feedback

Overall this is a clean, well-scoped addition that mirrors the existing BlobBaseFee pattern. A few items to address:

Bug: TOCTOU race on CurrentHeader() 🔴

eth/api_backend.go:531-536CurrentHeader() is called twice: once for the IsLondon guard and once for CalcBaseFee. If a new block arrives between the two calls, the header used for the base fee calculation could differ from the one checked for London activation. On a chain producing blocks every 2 seconds (Polygon PoS), this is a realistic window.

// Current (racy):
func (b *EthAPIBackend) BaseFee(ctx context.Context) *big.Int {
    if b.ChainConfig().IsLondon(b.CurrentHeader().Number) {   // ← header A
        return eip1559.CalcBaseFee(b.ChainConfig(), b.CurrentHeader()) // ← header B?
    }
    return nil
}

// Suggested fix — capture header once:
func (b *EthAPIBackend) BaseFee(ctx context.Context) *big.Int {
    header := b.CurrentHeader()
    if b.ChainConfig().IsLondon(header.Number) {
        return eip1559.CalcBaseFee(b.ChainConfig(), header)
    }
    return nil
}

Fix this →

Note: The adjacent BlobBaseFee method (eth/api_backend.go:538-543) has the exact same double-call pattern — a pre-existing issue worth fixing in the same PR if you're here.

Nit: Mock file is hand-edited

eth/filters/IBackend.go — This file is auto-generated (//go:generate mockgen in backend.go:46). The manual addition looks correct and matches the generator output, but consider running go generate ./internal/ethapi/... to regenerate it instead, so the file stays byte-for-byte consistent with the generator.

Missing: Unit test for the RPC method

The PR adds stub implementations for testBackend and backendMock (needed to satisfy the interface), but there's no test exercising the actual eth_baseFee endpoint. A minimal test (similar to how BlobBaseFee would be tested) that verifies:

  1. Returns a non-nil *hexutil.Big on a post-London chain
  2. Returns null on a pre-London chain (if applicable)

would strengthen confidence.

Summary

Item Severity
Double CurrentHeader() call (TOCTOU) Medium — should fix
No dedicated unit test Low — nice to have
Hand-edited mock file Nit

The design is sound — adding BaseFee to the Backend interface alongside BlobBaseFee is the right pattern, and the RPC layer wiring is correct. The TOCTOU fix is a one-line change; with that addressed, this looks good to merge.


Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

LGTM, straightforward read-only RPC method mirroring the existing BlobBaseFee pattern.

Extended reasoning...

Overview

This PR adds a new nonstandard eth_baseFee RPC method that returns the calculated base fee of the next block. It adds BaseFee(ctx) *big.Int to the Backend interface, implements it in EthAPIBackend using eip1559.CalcBaseFee, exposes it via EthereumAPI.BaseFee, and updates the generated gomock and two test backends to satisfy the interface.

Security risks

None identified. This is a read-only RPC method that computes a value already used internally and reachable via other paths (e.g. eth_feeHistory). No state mutation, no auth/crypto/permission surface, no user-controlled input beyond the standard JSON-RPC framing.

Level of scrutiny

Low. The change is mechanical and self-contained: it directly mirrors the adjacent BlobBaseFee implementation in the same file, uses the standard upstream eip1559.CalcBaseFee helper guarded by IsLondon, and the bulk of the diff is regenerated mock/test plumbing required to satisfy the new interface method.

Other factors

  • Bug hunting system found no issues.
  • SonarCloud quality gate passed (0 new issues, 0 security hotspots).
  • Pre-London branch correctly returns nil, matching the BlobBaseFee precedent.
  • Test mocks are updated consistently across api_test.go, transaction_args_test.go, and the generated eth/filters/IBackend.go.

Assisted-by: Claude:claude-sonnet-4-6
@sonarqubecloud
Copy link
Copy Markdown

@wjmelements
Copy link
Copy Markdown
Contributor Author

Nit: Mock file is hand-edited

eth/filters/IBackend.go — This file is auto-generated (//go:generate mockgen in backend.go:46). The manual addition looks correct and matches the generator output, but consider running go generate ./internal/ethapi/... to regenerate it instead, so the file stays byte-for-byte consistent with the generator.

I regenerated it in 7e3a704 using golang/mock v1.6.0 but this made more changes than I expected. Perhaps it has not been regenerated recently.

@pratikspatil024
Copy link
Copy Markdown
Member

@wjmelements Please resolve the conflicts and check the claude comments.

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