Skip to content

ethapi, eth: implement eth_baseFee RPC method#2231

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

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

Conversation

@wjmelements

@wjmelements wjmelements commented May 19, 2026

Copy link
Copy Markdown
Contributor

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

@claude claude Bot left a comment

Copy link
Copy Markdown

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

claude Bot commented May 19, 2026

Copy link
Copy Markdown

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.


@claude claude Bot left a comment

Copy link
Copy Markdown

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

@wjmelements

Copy link
Copy Markdown
Contributor Author

One of the test failures was because of the go version.

Assisted-by: Claude:claude-sonnet-4-6
@wjmelements

Copy link
Copy Markdown
Contributor Author

@pratikspatil024 please take a look

@pratikspatil024

Copy link
Copy Markdown
Member

@claude review

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Prior feedback (TOCTOU, regenerated mocks, unit tests) is fully addressed; remaining off-by-one is a one-block nit with no impact on Bor mainnet/Amoy.

Extended reasoning...

Overview

This PR adds the nonstandard eth_baseFee RPC method, which returns the calculated base fee of the next block — mirroring the existing BlobBaseFee pattern. The scope is small: a new method on the Backend interface, a one-liner RPC wrapper, the EthAPIBackend implementation, mock and test-backend conformance, and unit tests for both BaseFee and BlobBaseFee. The PR also bumps Go from 1.26.3 → 1.26.4 across Dockerfile, .golangci.yml, and go.mod files.

Security risks

None of substance. The endpoint is read-only, exposes only the base-fee derivation already computable from public block data, and the underlying eip1559.CalcBaseFee helper already guards pre-London parents internally. No auth, crypto, permissions, or input-validation surface is touched.

Level of scrutiny

Low-to-moderate. This is a non-breaking, additive RPC method following an established pattern. The wiring (interface, RPC layer, backend implementation) is mechanical and matches the sibling BlobBaseFee exactly. The test additions and mock regeneration further reduce risk.

Other factors

My prior review on this PR flagged three items: a TOCTOU race from double-calling CurrentHeader(), a hand-edited mock file, and missing unit tests. All three are resolved in the current revision — BaseFee and BlobBaseFee now capture the header once, IBackend.go was regenerated via golang/mock v1.6.0, and TestBaseFee/TestBlobBaseFee cover the active-fork, inactive-fork, and missing-config paths. SonarQube quality gate passes. The one remaining inline-commented nit — an off-by-one in the IsLondon(header.Number) guard at the London transition block — has negligible practical impact: Bor mainnet (LondonBlock 22640000) and Amoy (73100) crossed London long ago, so it only manifests on future devnets with a non-zero LondonBlock. Approving on that basis.

Comment thread eth/api_backend.go
…transition

Assisted-by: Claude:claude-sonnet-4-6
@sonarqubecloud

Copy link
Copy Markdown

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