rpc: resolve eth_feeHistory on the block overlay view

[lupin012]

eth_feeHistory resolves its head on the committed view while eth_blockNumber publishes the overlay head, so during the forkchoice flush+commit window the fee window lags the published head by one block or more. Seen as intermittent eth_feeHistory/test_07.json failures in the latest QA runs, e.g. 30798564490 (oldestBlock 25673288 vs 25673290 on the reference node — two blocks behind while a commit backlog drained) and 30803768568 (failed on attempt 1, passed on attempt 2).

Fix — each gas-oracle request reads one consistent view, pinned at acquisition:

• Filters.BeginTemporalRoWithOverlay (rpc/rpchelper) opens the tx and captures the published overlay as one consistent pair: publishes carry a monotonic sequence number (Events.OverlaySnapshot), the open retries while the sequence moves, and under sustained publish churn the last capture is served as one pinned view — a slightly stale answer beats a client-visible error. It returns the tx already pinned, and the pin travels inside the tx (the membatchwithdb.OverlayViewCarrier marker, applied by rpchelper.PinToOverlay), so downstream overlay wrap points leave it alone — and the remaining call sites with the same exposure can adopt the helper as a follow-up.
• Both the overlay and the "no overlay published" resolutions are served by a single rpchelper.PinnedRoTx handle: it forwards the tx's optional capabilities (BlockFilesRoTx, FreezeInfo, UnderlyingTx, Pin), routes Apply through the raw tx's guard while handing the pinned handle to the callback, its Rollback releases the raw tx, and an overlay published mid-request cannot leak in through helpers re-resolving the live overlay. This also completes the tip component of eth_gasPrice (the gap noted in the rpc: use overlay tx in GasPrice, BlobBaseFee, BaseFee to avoid flaky test on latest #22006 review).
• NewGasPriceOracleBackend requires the pin carried by the caller's tx (panics on an unpinned tx) instead of re-capturing the live overlay, and Fork() wraps its fresh txs with that same pin, so head resolution, per-block sampling and the parallel fetchers all read one view.
• The fee-history cache key is dual-regime. At or below the frozen boundary (the Snapshots stage progress, read once per request through the pinned tx) entries are number-keyed — the number-to-hash mapping is immutable there, so the hit path costs zero I/O. Above it entries are keyed by the block hash resolved from the canonical-hash index through the pinned tx (OracleBackend.CanonicalHash), so a same-height sibling after a reorg misses by construction; a resolution error degrades the block to uncacheable instead of failing the request. Blocks are fetched by the already-resolved (hash, number) pair, so a stored entry matches its key by construction. CacheableBlockLimit is gone from OracleBackend.

Bounds and data agree: senders and receipts of the in-flight block are served by the overlay because the senders stage and execution run on the overlay tx before publication (execution/execmodule/forkchoice.go), and receipt domain reads go through the overlay since #22511.

Memory bound: a pinned request retains the overlay it captured — including a SharedDomains already superseded by a newer publish, order 10–50 MB — for its own lifetime. The retention is bounded by the request, not by the publish rate, and the memory is released with the last pinned tx.

Tests (overlay_race_test.go, gasprice_test.go), grouped by what they pin:

• overlay head resolution: TestFeeHistory_SeesOverlayHead (header-only path), TestFeeHistory_OverlayHeadWithRewards (block+receipts path via Fork, asserting the receipts' gas is actually read);
• one pin per request: TestGasPriceOracle_PinnedViewIgnoresLaterOverlayPublish, TestGasPriceOracle_ForkKeepsOverlayAfterUnpublish, TestGasPriceOracle_ForkSharesCallerPinnedOverlay (caller-pinned tx), TestGasPriceOracle_NilOverlayPinIgnoresLaterPublish (nil pin), TestGasPrice_BaseFeeFromPinnedOverlay;
• atomic acquisition: TestFeeHistory_HeadCommittedDuringTxAcquisition, TestFeeHistory_PublishCycleDuringTxAcquisition (publish/unpublish cycle), TestFeeHistory_OverlayUnstableDuringTxAcquisition (serves the last capture as one pinned view under churn), TestBlockNumber_PublishCycleDuringTxAcquisition, TestBaseFee_PublishCycleDuringTxAcquisition, TestFillTransaction_PublishCycleDuringTxAcquisition;
• pinned handle: TestBeginTemporalRoWithOverlay_PreservesOptionalInterfaces (optional tx capabilities survive the pin, in both the overlay and the no-overlay case), TestGasPriceOracleBackend_RequiresPinnedTx (the constructor invariant);
• cache identity: TestFeeHistory_DeadOverlayBlockNotServedFromCache, TestFeeHistory_ReorgedCommittedBlockNotServedFromCache, TestGasPriceOracle_CanonicalHashUsesPinnedView (cache key resolved on the pinned view even when the block reader resolves live, as the remote one does), TestFeeHistory_FrozenRangeCachesByNumberWithoutResolution (zero-I/O hit path below the frozen boundary), TestFeeHistory_CanonicalHashErrorDegradesToUncached (a resolution error degrades instead of failing).

All fix tests were verified red before the corresponding fix and green after. The harness also writes the forkchoice head marker to the overlay — what rpchelper.GetLatestBlockNumber resolves from in production.

QA verification

QA - RPC Integration Tests Latest is being dispatched repeatedly on this branch (head 5ed15a18) to confirm the intermittent eth_feeHistory failures are gone. Each run is verified attempt by attempt: no eth_feeHistory failure may appear in any attempt's logs, not just in the final outcome — the suite retries internally, so a green run can hide the flake.

Baseline: the same workflow on main

Scanning the job logs of the last 16 daily runs on main (Jul 30 – Aug 13), 7 had an eth_feeHistory/test_07.json failure (diff mismatch on baseFeePerGas/baseFeePerBlobGas, i.e. the window resolving a different head) in at least one attempt — all but one masked by the retries:

Date	Run	Notes
Aug 11	31445605548	failed on one attempt, recovered on retry
Aug 6	31059907109	failed on multiple attempts before recovering
Aug 5	30963641898	failed in both the serial and the parallel job
Aug 4	30866032675	failed on one attempt
Aug 2	30725527900	failed on one attempt
Jul 30	30502972095	failed on one attempt
Jul 29	30411427484	the only red final outcome (7 tests failed on attempt 1, test_07 among them)

Clean days: Aug 13, 12, 10, 9, 8, 7, 3, 1, Jul 31 — a ~40% daily flake rate.

This branch

Run	GitHub attempt	Internal attempts	eth_feeHistory
31677421836	1	1 (both jobs)	test_07 + test_22 OK in both jobs, 218/218 green
31680032999	1	1 (both jobs)	test_07 + test_22 OK in both jobs, 218/218 green
31680705557	1	1 (both jobs)	test_07 + test_22 OK in both jobs, 218/218 green
31681514833	1	4 (one job)	test_07 + test_22 OK in every attempt of both jobs; the retries were debug_traceBlockByNumber/test_42 (unrelated API, recovered on attempt 4)
31682235233	1	1 (both jobs)	test_07 + test_22 OK in both jobs, 218/218 green

First batch: 5/5 runs with zero eth_feeHistory failures in any attempt (under the main baseline rate, ~7% probability by luck). The flake is bursty (main had a week-long clean streak with the bug still in), so the batch was repeated in the evening under different tip conditions:

Second batch, evening tip conditions, head e2661e05 (review round four + freshly merged main):

Run	GitHub attempt	Internal attempts	eth_feeHistory
31741525947	1	1 (both jobs)	test_07 + test_22 OK in both jobs, 218/218 green
31742431414	1	1 (both jobs)	test_07 + test_22 OK in both jobs, 218/218 green
31743282257	1	2 (one job)	test_07 + test_22 OK in every attempt of both jobs; the retry was debug_traceBlockByNumber/test_44+test_47 (unrelated API, recovered on attempt 2)
31744129344	1	2 (one job)	test_07 + test_22 OK in every attempt of both jobs; same debug_traceBlockByNumber/test_44+test_47 retry (recovered on attempt 2)
31744948065	1	2 (one job)	test_07 + test_22 OK in every attempt of both jobs; the retry was 6 debug_traceBlockByNumber tests (recovered on attempt 2)

Result: 10/10 runs across two batches (morning and evening tip conditions, two heads) with zero eth_feeHistory failures in any attempt of any job — under the main baseline rate (~42% of daily runs flaking), ten consecutive clean runs have a ~0.4% probability of being luck. The post-merge daily scheduled runs on main will keep sampling for free.

The round-five head 9315c32d changes the cache regime and the handle forwarding on the same pinned acquisition path the batches verified; any regression there would surface in the daily runs.

Side observation from the same logs: debug_traceBlockByNumber flaked in 4 of the 10 runs (always recovering on an internal retry) — the same head-consistency class this PR fixes for the gas-oracle endpoints, untouched here and a natural first target for the agreed follow-up call-site migration.

[Copilot]

Pull request overview

This PR fixes an inconsistency in JSON-RPC head resolution during the block-overlay publish/commit window: eth_feeHistory (via the gas oracle) could resolve its head from the committed view while other RPC paths (e.g. eth_blockNumber) report the overlay head, causing fee history windows to lag and intermittently fail RPC tests.

Changes:

• Pin a single overlay-aware temporal read view inside GasPriceOracleBackend so head resolution and per-block sampling are consistent.
• Ensure GasPriceOracleBackend.Fork() uses the same overlay-aware constructor, so fee-history sampling paths that run on the forked tx also see the overlay.
• Add overlay race regression tests covering fee history with and without reward percentiles, and update the test harness to write the forkchoice head marker into the overlay.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
rpc/jsonrpc/eth_system.go	Wraps gas oracle backend tx with WithTemporalOverlay (including forked tx) to make eth_feeHistory consistent with overlay head publication.
rpc/jsonrpc/overlay_race_test.go	Adds forkchoice-head marker setup and new regression tests asserting fee history resolves "latest" on the overlay head.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[yperbasis]

Requesting changes. Findings by severity; the first one is reproduced locally on this branch.

High (blocking) — in-flight block receipts silently read as zero

Read views keep DomainReader = sd (db/state/execctx/domain_shared.go:720). The background-commit teardown (execution/execmodule/forkchoice.go:731-736) calls bgSD.Close() → sd.mem.Close() → ClearRam() (db/state/temporal_mem_batch.go:654), which empties the domain maps while RPC views may still hold them. Reproduced: after sd.Close(), a view created earlier returns ok=false, err=nil for an in-flight ReceiptDomain key. The fallback is the reader's own tx — pre-commit for requests started during the publish window — and ReceiptAsOf zero-fills misses (db/rawdb/rawtemporaldb/accessors_receipt.go:42-60). So GetReceiptsGasUsed returns GasUsed=0 for every tx of the head block and the reward percentiles are silently wrong. The window is realistic: a percentile fan-out over many blocks often still runs when the commit finishes.

The class predates this PR (the receipts generator wraps its tx since #22511), but this PR makes the oracle resolve the in-flight head and then sample its receipts, so it leans on the missing lifetime guarantee much harder.

Suggested fix (can be a prerequisite PR in execctx/execmodule): don't clear domain RAM under a published SD — e.g. a teardown variant that closes writer resources but skips ClearRam, leaving the maps to GC once Events and the last view drop the pointer (GetAsOf touches only domains/storage/unwindChangeset, read-only after publication). Refcounting is the heavier alternative.

Medium — Fork can lose the resolved head → whole request fails

Fork opens its tx first and resolves LatestSD() afterwards (via the constructor). If the commit → PublishOverlay(nil) interval lands in between, the forked backend has neither the overlay nor the committed head block. On the header-only path headerByNumber propagates BlockNotFoundErr unfiltered (rpc/jsonrpc/eth_api.go:397-400), so the whole eth_feeHistory request fails with -32000 block not found: N; on the rewards path the head slot is silently dropped (eth_api.go:301-303). Rare in production (the interval includes an fsync'd commit, so the fork goroutine must be descheduled for >1ms between opening and wrapping), but it turns a stale read into a failed request, and it is new with this PR — before, the window never included the in-flight block.

Suggested fix: capture the overlay once in NewGasPriceOracleBackend and let Fork wrap its fresh tx with the parent's overlay instance instead of re-resolving LatestSD(). Plain-table reads on a closed overlay are safe (memStore close is a no-op on the data). Note this makes the high-severity item easier to hit — fork views then always carry the SD's DomainReader — so both fixes should land together.

Low — rewards test never exercises receipts

TestFeeHistory_OverlayHeadWithRewards uses an empty-body overlay block, so it hits the zero-reward guard (rpc/gasprice/feehistory.go:186) and never reads receipt data. Extend it to a block with transactions plus receipt-domain entries and assert non-zero gas — this is the test that pins the high-severity fix.

Low (non-blocking follow-up) — nested overlay views

With b.tx now a view, downstream re-wraps create view-over-view over the same overlay, sharing one *sync.RWMutex: the receipts generator (rpc/jsonrpc/receipts/receipts_generator.go:214,459,691), headerByNumber (rpc/jsonrpc/eth_api.go:407), and fillFeeDefaults receiving the already-wrapped tx (rpc/jsonrpc/eth_fill_transaction.go:118). GetOne/Has hold RLock while falling back to m.db, so nested views take a recursive read lock. Safe today — nothing write-locks a published overlay — but that invariant is enforced nowhere; a writer added later deadlocks. Making WithOverlay/WithTemporalOverlay idempotent would remove this and the redundant double lookups.

[lupin012]

@yperbasis

High: Will be fixed in a separate PR #23046, as suggested. This PR stays a draft until that one is merged.

Medium: Fixed as suggested: the backend captures the overlay once at construction and Fork reuses that instance instead of re-resolving LatestSD(). Pinned by TestGasPriceOracle_ForkKeepsOverlayAfterUnpublish, red before the fix.

Low1: Extended: the overlay block now has two txs with different tips plus their receipt-domain entries, and the test asserts a low percentile returns the cheap tx's tip — verified red with zero-filled receipt gas.

Low2: (non-blocking follow-up) Agreed, planned as a follow-up PR

[yperbasis]

High

1. Downstream helpers override the pinned overlay. BaseAPI and receipts helpers re-resolve the live overlay on every call (rpc/rpchelper/helper.go:87, receipts/receipts_generator.go:691, eth_api.go:333) and layer it over the pinned view, so a publish during the request still mixes views. Worst case, after a same-height reorg, ReceiptAsOf reads the new fork's cumulative gas at the old block's txNums (ReceiptDomain is exempt from the inMemHistoryReads guard): wrong GasUsed deltas (uint underflow possible) and reward percentiles from another fork. Suggested fix: make WithOverlay/WithTemporalOverlay no-ops when the passed tx already carries an overlay view. A same-instance check is not enough — it would still wrap pinned view A with live overlay B.

2. Overlay results poison FeeHistoryCache. The cache key is {blockNumber, percentiles} — no hash, no reorg/unwind invalidation (rpc/gasprice/feehistory.go:63). With this PR, "latest" can resolve to a not-yet-committed overlay block. If the background commit fails, or a same-height sibling lands, the dead block's fees keep being served until LRU eviction. Fix: skip caching blocks that come from the overlay (or near head), or add the canonical hash to the key.

Medium

3. Overlay capture and tx open are not atomic. The overlay is captured after the caller has opened the tx (rpc/jsonrpc/eth_system.go:504). If commit + PublishOverlay(nil) land in between, the request sees neither the overlay nor the committed block (stale head). In the opposite direction, the pinned overlay can be one FCU ahead of the tx snapshot, leaving a block visible in neither layer. Capture the overlay atomically with tx acquisition — capture-before-open alone only flips the window.

4. eth_gasPrice still mixes heads. The tip comes from the pinned oracle, but the baseFee addend re-resolves the live overlay (rpc/jsonrpc/eth_system.go:273). One-line fix: read the current header through the pinned b.tx.

5. fillFeeDefaults double-wraps the overlay. FillTransaction/signTransaction wrap the tx (eth_fill_transaction.go:61) and newGasOracle now wraps it again: view-over-view, doubled memTx probes, and diverging heads if the overlay flips between the two wraps. Pass the raw dbTx into newGasOracle, or rely on the no-op rule from point 1.

Low / latent

6. Unenforced write-freeze on the published overlay. All read views share one *sync.RWMutex (newReadViewMut copies the parent's mu) and GetOne holds RLock across the fallback into the inner view — a recursive RLock. This is safe only while nothing writes to a published overlay, and nothing enforces that. Consider a runtime assert (e.g. reject Put after publish/Close). Related: Rollback/Commit nil statelessCursors without taking mu (memory_mutation.go:584), unlike the other writers.

7. Test gap. TestGasPriceOracle_ForkKeepsOverlayAfterUnpublish calls PublishOverlay(nil) but never doms.Close(); production always pairs them. Add doms.Close() before backend.Fork() so the test pins the invariant the PR depends on.

8. Overlay-view tax on committed history. Every gas-oracle read now takes the shared RLock plus a guaranteed memTx miss (doubled by the re-wraps) even when all requested blocks are committed. The no-op rule from point 1 removes the doubling; routing reads below the overlay's base block through the raw tx would remove the rest.

Notes

• Longer term, a per-request pin (e.g. at tx acquisition) would fix this class once — rpc/jsonrpc has ~44 independent overlay re-resolves with the same exposure (eth_getLogs, eth_simulateV1, trace workers).
• Nit: the pin-once rationale is repeated at three sites; one canonical place is enough.

[lupin012]

@yperbasis: All points addressed except point 6.

1. WithOverlay/WithTemporalOverlay (and the gas oracle wrap) are now no-ops when the tx already carries an overlay view, checked by type via the new membatchwithdb.CarriesOverlayView — so a pinned view is never re-wrapped with a newer overlay. Test: TestGasPriceOracle_PinnedViewIgnoresLaterOverlayPublish.
2. went with skip-caching: CacheableBlockLimit() resolves the committed head from the unwrapped tx and fee history skips both cache lookup and store above it. Test: TestFeeHistory_DeadOverlayBlockNotServedFromCache.
3. the three entry points acquire tx + overlay via beginTxWithOverlay: capture, open, re-capture, reopen on mismatch (bounded retries). Test: TestFeeHistory_HeadCommittedDuringTxAcquisition.
4. baseFee addend now read via the pinned backend.tx. Test: TestGasPrice_BaseFeeFromPinnedOverlay.
5. resolved by the no-op rule from point 1.
6. this one requires guarding MemoryMutation's write methods, which are shared with execution and mining, so I'd rather not bundle it here. If you think it's worth it, I can open a dedicated PR.
7. done, the test now pairs PublishOverlay(nil) with doms.Close().
8. the no-op rule removes the doubled probes; routing below the overlay base is left as optional follow-up.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (2)

rpc/jsonrpc/eth_system.go:529

• This condition does not guarantee that the transaction and overlay form a consistent snapshot. It explicitly accepts a changed overlay on the third attempt, and pointer equality also misses a complete nil -> overlay -> nil publish/commit cycle during BeginTemporalRo; in either case the returned transaction can predate the commit while the returned overlay is nil or belongs to a newer head, recreating the missing-block window this helper is meant to close. Track a monotonically increasing publication generation together with the overlay and retry until the generation is unchanged; do not return a mismatched pair after a fixed attempt count.

		if current == overlay || attempt == maxAttempts {
			return tx, current, nil

rpc/jsonrpc/eth_system.go:506

• The supplied transaction may already be pinned to overlay A, as in FillTransaction, but this constructor independently records the currently published overlay B. withOverlay then keeps the parent on A because CarriesOverlayView(tx) is true, while Fork wraps its fresh transactions with B, so one oracle operation can resolve its head from A and sample blocks from B. Preserve the originating overlay identity in overlay read views (or pass the already-captured overlay explicitly at every call site) instead of sampling LatestOverlay here.

func NewGasPriceOracleBackend(db kv.TemporalRoDB, tx kv.TemporalTx, baseApi *BaseAPI) *GasPriceOracleBackend {
	return newGasPriceOracleBackendPinned(db, tx, baseApi, baseApi.filters.LatestOverlay())

[yperbasis]

Combined findings from two independent review passes; the first, second and fourth items were also reproduced with regression tests. Line numbers refer to head 5856794.

High

1. Overlay reorgs defeat the number-keyed fee-history cache. Cacheability is only blockNumber <= committed head (rpc/jsonrpc/eth_system.go:542, rpc/gasprice/feehistory.go:392), but an overlay can replace or unwind blocks at or below that height. Two directions: a cached committed block keeps being served after an overlay sibling replaces the same height (reproduced: the old base fee is returned), and during a commit backlog uncommitted overlay siblings below the limit get cached. Fix: key the cache by block hash (structural — also removes CacheableBlockLimit from OracleBackend), or keep the boundary below the earliest overlay-modified height.

2. FillTransaction builds an oracle with two different overlay pins. The tx is wrapped with overlay A (rpc/jsonrpc/eth_fill_transaction.go:61), but NewGasPriceOracleBackend re-captures LatestOverlay() = B (eth_system.go:506), so b.tx stays on A while Fork wraps goroutine txs with B (eth_system.go:566). Reproduced: parent and fork head hashes differ. Pass the overlay together with the wrapped tx (or let the view expose its overlay identity) so the backend and its forks share one pin.

3. The overlay == nil pin is not sticky. A raw tx carries no CarriesOverlayView marker (eth_system.go:550), so downstream helpers re-resolve the live overlay on every call (rpc/rpchelper/helper.go:87, rpc/jsonrpc/eth_api.go:333, :407). An overlay published mid-request leaks in: eth_gasPrice can sum a tip sampled at head N+1 with a base fee read at head N. Make the nil pin explicit so the whole request stays on one view.

Medium

4. The last beginTxWithOverlay attempt returns the (tx, overlay) pair it has just proven inconsistent (eth_system.go:528), which can produce a gapped view — a block visible in neither the tx snapshot nor the overlay (reproduced: stale head returned). Also, the pointer-equality check treats nil==nil as stable, so a full publish/commit/unpublish cycle between the two captures passes on the first attempt. Return an error (or keep retrying until stable), and compare a monotonic publish sequence number instead of pointer identity — that closes both holes.

5. The "rawTx is the caller's tx before the overlay wrap" contract on CacheableBlockLimit (eth_system.go:542) is unenforced and already broken: the FillTransaction path stores a wrapped view in it. Harmless today (that path never reaches FeeHistory), but on a wrapped tx GetLatestBlockNumber resolves the overlay's uncommitted forkchoice head, so the cache limit would include in-flight blocks. Enforce or assert the contract.

Low / follow-ups

6. Two sources remain unpinned in a request meant to hold one consistent view (both pre-existing): Fork opens fresh base txs whose snapshots can postdate a committed reorg (eth_system.go:558), and the pending block comes from the live mining cache (feehistory.go:230).

7. ~45 call sites keep the non-atomic BeginTemporalRo-then-WithOverlay pattern with the same race window this PR closes for three endpoints — including BlobBaseFee (eth_system.go:350), BaseFee (:377) and BlockNumber (:196) in the same file. Promote beginTxWithOverlay to a shared helper on BaseAPI/Filters; fine as a follow-up.

8. Smaller cleanups: the skip-then-wrap sequence exists in three copies (rpc/rpchelper/filters.go:1177, :1211, eth_system.go:549) — extract one helper. readView duplicates the memDb == nil predicate (db/kv/membatchwithdb/memory_mutation.go:61). CarriesOverlayView (memory_mutation.go:1091) is a concrete-type switch that any future tx wrapper silently defeats — a marker-interface method would fail loudly instead.

9. Nits: drop the call-site sentence ("Only the fee-history path pays for the resolution.") from the CacheableBlockLimit doc comment (eth_system.go:537); remove the Co-Authored-By trailer (e035aa3) and the "Generated with Claude Code" footer from the description — repo convention (CLAUDE.md): don't sign commits or PRs; the description's "stays a draft until that one is merged" sentence is stale now that #23046 is merged.

[lupin012]

All points addressed except point 6 (pre-existing, as noted) and the call-site migration of point 7 (follow-up, as agreed). Every behavioral fix is pinned by a regression test that was red before it; point 8 is a pure refactor covered by the existing suite. The branch is rebased linearly onto main; lint and the full unit suite are green

High:

1. Went with the "key the cache by block hash" option: a block replaced by a reorg has a different hash, so its cache entry can no longer be found — no invalidation needed, both directions covered. The hash comes from the canonical-hash index on the pinned view (OracleBackend.CanonicalHash); CacheableBlockLimit is removed, which also closes point 5. Test: TestFeeHistory_ReorgedCommittedBlockNotServedFromCache, red before the fix.
2. Went with the "let the view expose its overlay identity" option: read views now carry the overlay they were created from (membatchwithdb.ViewOverlay), so NewGasPriceOracleBackend adopts the caller's pin instead of re-capturing LatestOverlay(), and Fork reuses that same pin. Test: TestGasPriceOracle_ForkSharesCallerPinnedOverlay, red before the fix (parent and fork resolved different heads).
3. Made explicit: BeginTemporalRoWithOverlay returns the tx already pinned in both cases — a read view when an overlay is published, a noOverlayView otherwise — so a raw tx never reaches the downstream wrap points and the "no overlay" resolution is as sticky as the pinned one. Test: TestGasPriceOracle_NilOverlayPinIgnoresLaterPublish, red before the fix.

Medium:

4. Done as suggested: monotonic publish seq (paired atomically with the SD in Events.OverlaySnapshot), retry while it moves around the open, retryable error after three unstable attempts instead of the inconsistent pair. Tests: TestFeeHistory_PublishCycleDuringTxAcquisition and TestFeeHistory_OverlayUnstableDuringTxAcquisition, both red first.
5. Gone with point 1: no more CacheableBlockLimit, no more rawTx, no contract left to enforce

Low:

6. Acknowledged, both pre-existing as you note. Pinning the MDBX snapshot across Fork's txs would need a structurally different approach (eager tx opening or a shared read view), and the pending block is inherently a live source — leaving both as is
7. Promoted as suggested, one step further: the loop lives in rpchelper as Filters.BeginTemporalRoWithOverlay(ctx, db) and returns the tx already pinned, so each call site converts by just swapping its BeginTemporalRo call. The migration of the ~45 call sites stays a follow-up PR, as agreed — it will start with BlobBaseFee/BaseFee/BlockNumber in this same file
8. Done: skip-then-wrap extracted into PinToOverlay, the redundant readView flag removed (the view's overlay pointer is the marker), and the type switch replaced by the OverlayViewCarrier marker interface
9. Done: the comment went away with CacheableBlockLimit, the trailer is stripped (hence the force-push), and the description is updated

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

db/kv/membatchwithdb/memory_mutation.go:1089

• This method-promotion claim is incorrect for the usual wrapper that embeds kv.Tx or kv.TemporalTx: Go promotes only methods in the embedded interface's static method set, which does not include OverlayView. Such wrappers must forward OverlayView explicitly, so documenting that requirement avoids silently losing the pin.

// OverlayViewCarrier is implemented by txs that are pinned overlay views.
// A tx wrapper that embeds such a tx keeps the marker through method
// promotion, where a concrete-type switch would silently lose it.

[yperbasis]

Combined review (own pass plus verified Copilot/Codex reports), categorized by severity.

High

• rpc/jsonrpc/eth_system.go:528: CanonicalHash delegates to the block reader. In rpcdaemon mode RemoteBlockReader.CanonicalHash ignores b.tx and queries the live service, and the later fetch resolves the number again — a reorg in between caches sibling B's fees under sibling A's hash, and the hash-keyed entry is never invalidated. It also adds one remote call per block even on cache hits. Read kv.HeaderCanonical through the pinned tx, fetch by that exact hash (or validate the fetched header's hash), and fall back to the block reader only for the frozen range. Doing so also removes the new per-block lookup on the warm-cache path and the double hash resolution on misses (rpc/gasprice/feehistory.go:394). (Confirms the unresolved Copilot thread.)

Medium

• rpc/jsonrpc/eth_system.go:354 and :201: eth_baseFee, eth_blobBaseFee, and eth_blockNumber keep the non-atomic BeginTemporalRo → WithTemporalOverlay acquisition — the same race class this PR fixes for the sibling fee endpoints, so the cross-endpoint skew can still fire there. Each is a one-line migration to BeginTemporalRoWithOverlay.
• rpc/jsonrpc/eth_system.go:505: for a tx without a pin, NewGasPriceOracleBackend re-resolves LatestOverlay() with no seq-stability check, so eth_fillTransaction can price maxPriorityFeePerGas and maxFeePerGas on two different heads. Pin at tx acquisition (or reuse the request's captured snapshot) there too.
• db/kv/membatchwithdb/memory_mutation.go:1121: noOverlayView embeds the bare interface, so it (a) drops HasBlockFilesRoTx — every no-overlay request (always, in remote mode) loses the tx-pinned block-files view, paying a view acquisition per read and letting one request straddle a snapshot merge — and (b) promotes Apply, whose callback receives the raw unpinned tx (latent: no rpc/ callers today). Forward BlockFilesRoTx and override Apply as the sibling views do. (Confirms the other unresolved Copilot thread.)
• rpc/rpchelper/filters.go:1227: after 3 acquisition attempts under a moving seq, fee RPCs now fail with errOverlayUnstable — a new client-visible error during FCU catch-up/churn. Prefer serving the last coherent capture (tx plus its captured overlay) over returning an error.

Low

• rpc/rpchelper/filters.go:1214: the (pinned, raw) return contract is a misuse trap — pinned.Rollback() panics on a read view but releases the raw tx on noOverlayView. A single handle whose Rollback releases the raw tx would be safer for the planned follow-up call sites.
• db/kv/membatchwithdb/memory_mutation.go:1087: the OverlayViewCarrier doc overclaims — embedding the tx interface promotes only its static method set, so OverlayView is lost unless the wrapper embeds the concrete view type or forwards it explicitly. Reword.
• rpc/rpchelper/filters.go:1236: LatestOverlay duplicates OverlaySnapshot's resolution chain (via LatestSD and its never-written latestSD fallback); express it as overlay, _ := ff.OverlaySnapshot().
• rpc/jsonrpc/eth_system.go:266 (and the two sibling handlers): newGasOracleFromBackend(NewGasPriceOracleBackend(api.db, tx, api.BaseAPI)) is byte-identical to api.newGasOracle(tx). Relatedly, the backend's overlay field (:498) is derivable from the pinned tx via ViewOverlay(b.tx).
• rpc/gasprice/gasprice_test.go:303: mockOracleBackend.CanonicalHash returns the head hash for every height, violating the ok=false-beyond-head contract and collapsing all cache keys; latent until a test combines the mock with a FeeHistoryCache.
• db/kv/membatchwithdb/memory_mutation.go:1151: the view-of-view branch in newReadViewMut is unreachable (all wrap points guard with CarriesOverlayView); drop it or assert m.overlay == nil.
• Layering: only the OverlayViewCarrier marker needs to live in db/kv/membatchwithdb; PinToOverlay's request-pinning policy could sit next to BeginTemporalRoWithOverlay in rpchelper.

[lupin012]

All points addressed:

H1) Fixed as suggested: CanonicalHash now reads kv.HeaderCanonical through the pinned tx and falls back to the block reader only for the frozen range Test: TestGasPriceOracle_CanonicalHashUsesPinnedView, red before the fix with a reader that resolves on the live view like the remote one.

M1) All three migrated to BeginTemporalRoWithOverlay. Tests: TestBlockNumber_PublishCycleDuringTxAcquisition and TestBaseFee_PublishCycleDuringTxAcquisition, both red before the migration (head hidden by a publish/commit/unpublish cycle landing during the open); eth_blobBaseFee is the identical mechanical change
M2) eeth_fillTransaction now pins at tx acquisition via BeginTemporalRoWithOverlay, so the header, the tip sampling and the fee defaults all read one view and the constructor never re-resolves. Test: TestFillTransaction_PublishCycleDuringTxAcquisition, red before the fix (maxFeePerGas embedded 2×baseFee of the superseded head).
M3) Fixed one level deeper: noOverlayView is gone — both the overlay and the no-overlay case are now served by a single rpchelper.PinnedRoTx that forwards BlockFilesRoTx, hands itself to Apply callbacks, carries the marker, and whose Rollback releases the raw tx (also closing the (pinned, raw) misuse trap from the Low list). Test: TestBeginTemporalRoWithOverlay_PreservesOptionalInterfaces, red before the fix on the no-overlay case
M4) Done: under sustained publish churn the acquisition now serves the last capture — the tx pinned together with the overlay captured around its open, one self-consistent view — instead of failing; errOverlayUnstable is gone. TestFeeHistory_OverlayUnstableDuringTxAcquisition flipped accordingly: it now asserts a coherent window from one pinned sibling view instead of an error

L1-L7) All seven done. The first and the last converged with the noOverlayView fix into the single rpchelper.PinnedRoTx: one handle whose Rollback releases the raw tx, with the request-pinning policy living next to BeginTemporalRoWithOverlay and only the generic marker staying in membatchwithdb. The rest: marker doc reworded to state the interface-embedding caveat, LatestOverlay expressed via OverlaySnapshot, the three handlers reuse newGasOracle and the backend's overlay field is gone (derived from the pinned tx via ViewOverlay), the mock honors the ok=false-beyond-head contract, and the unreachable view-of-view branch is dropped

Verification: re-ran QA - RPC Integration Tests Latest 10× on this branch (two 5-run batches, morning and evening tip conditions) — zero eth_feeHistory failures in any attempt of any job, vs ~40% of main's daily runs flaking at attempt level. Full tables with per-run links in the PR description.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (1)

rpc/jsonrpc/eth_system.go:528

• CanonicalHash now pins the cache key, but the subsequent HeaderByNumber and BlockByNumber calls still resolve numeric blocks through BaseAPI and _blockReader.CanonicalHash. In rpcdaemon mode, RemoteBlockReader ignores the transaction, so a reorg can make the fetchers process live sibling B while the range and cache key are pinned to sibling A; the hash check only prevents cache insertion and does not prevent a mixed response. Resolve the hash through this pinned method and fetch the header/block by that exact hash instead.

func (b *GasPriceOracleBackend) CanonicalHash(ctx context.Context, number uint64) (common.Hash, bool, error) {

[yperbasis]

Combined review of the current head (e2661e0), categorized by severity. High and Medium gate approval; Low are cheap to fold in; the last section is follow-up material.

High

• rpc/gasprice/feehistory.go:394 — the cache-hit path went from a zero-I/O number-keyed LRU get to a per-block CanonicalHash resolution. In remote rpcdaemon mode a fully-memoized eth_feeHistory(1024, latest) now costs roughly 1024–2048 sequential gRPC round trips (a remote GetOne per block, plus the RemoteBlockReader fallback for pruned heights), and a transient error on this auxiliary lookup fails the whole request via return err — the old hit path had no failure mode at all. Degrade to cacheable=false on a resolution error, and skip the resolution (or keep number-keyed entries) at or below the frozen boundary, where the number-to-hash mapping is immutable.

Medium

• rpc/rpchelper/pinned_tx.go:47 — the view comes from overlay.NewReadView(tx), so the handle's required FreezeInfo() promotes to MemoryMutation.FreezeInfo's panic("not supported") whenever an overlay is published (the no-overlay pin works). Delegate to t.raw.FreezeInfo() (or use NewTemporalReadView, which forwards it) and extend TestBeginTemporalRoWithOverlay_PreservesOptionalInterfaces accordingly. Same class: Apply bypasses the temporal tx's closed-tx guard, and the wrapper hides UnderlyingTx() / Pin() of the wrapped view.
• rpc/jsonrpc/eth_system.go:503 — the fallback pin pairs a caller-opened tx with LatestOverlay() resolved at construction time, without the publish-seq bracket — the torn (tx, overlay) pairing BeginTemporalRoWithOverlay's retry loop exists to close. All production callers pre-pin today, so require the pin (assert CarriesOverlayView) instead of silently re-resolving. Also Fork discards the pinned bool at eth_system.go:518.
• rpc/jsonrpc/overlay_race_test.go:702 — the beginHookDB hooks run testify require and t.Cleanup on errgroup fetcher goroutines (Fork → BeginTemporalRo → hook), violating the FailNow same-goroutine contract: a failing hook Goexits the worker, errgroup.Wait returns nil, and the test fails at an unrelated downstream assert. Return errors from the hooks and assert on the test goroutine; that also lets the five copy-pasted publish/commit/unpublish blocks collapse into one helper.
• rpc/gasprice/gasprice_test.go — mockOracleBackend.CanonicalHash returns m.head.Hash() for every height, so all block numbers share one cache key; dormant only because every NewOracle call passes historyCache=nil. Derive a per-height hash.

Low

• rpc/gasprice/feehistory.go:440 — the compute path re-resolves the canonical hash inside HeaderByNumber/BlockByNumber after line 394 already did, plus a keccak per stored block for the fees.header.Hash() == blockHash guard. Fetching by the already-resolved (hash, number) pair makes the guard hold by construction.
• rpc/jsonrpc/eth_fill_transaction.go:53 — the comment claims the pinned view keeps the whole request on one head, but the GetTransactionCount / EstimateGas sub-calls open their own unpinned txs; scope the comment or thread the pinned tx through.
• The pin-once rationale is written out in full at four sites (eth_system.go:498, eth_system.go:516, rpc/rpchelper/filters.go:1226, memory_mutation.go:1105); keep the canonical statement on PinToOverlay and reduce the rest to pointers. PinnedRoTx.Rollback's comment restates the type doc above it.
• rpc/jsonrpc/eth_system.go:502 — the CarriesOverlayView guard duplicates the same check inside PinToOverlay; newGasOracleFromBackend has a single caller one line above it.
• rpc/jsonrpc/overlay_race_test.go:643 — commitOverlayBlock omits the Execution stage-progress and canonical-TxNums markers production writes, so the harness commit only supports header-only reads; complete the markers or note the limitation before more tests build on it.

Follow-ups (fine as separate issues, not gating this PR)

• Fork (eth_system.go:512) pins the parent's overlay over a freshly opened tx: a reorg of depth two or more committing mid-request can mix two chains in one response (also reachable via the exhausted-attempts fallback in rpc/rpchelper/filters.go:1212). Not a regression — pre-PR Fork had no pin at all.
• The per-endpoint migration leaves ~105 handlers and ~40 WithOverlay call sites on the torn-view pattern; a single acquisition chokepoint (a BaseAPI-level begin helper) would cover all sites at once, after which WithOverlay collapses into PinToOverlay.
• PendingBlockAndReceipts (eth_system.go:579) returns the live pendingBlock(), contradicting the "always a pinned view" field comment — pre-existing; route through the pin or scope the comment.
• A pinned request retains the torn-down SharedDomains (order 10–50MB) for its lifetime — bounded and safe by design, but worth a note in the PR description.

[lupin012]

All points addressed:

H1) Done as prescribed: at or below the frozen boundary entries are number-keyed and the hit path is back to zero I/O; above it the hash key stays, and any resolution error degrades the block to cacheable=false instead of failing the request. The boundary comes from the Snapshots stage progress through the pinned tx — one read per request that works in remote mode too, where every reader/tx frozen-API panics. Tests: TestFeeHistory_FrozenRangeCachesByNumberWithoutResolution and TestFeeHistory_CanonicalHashErrorDegradesToUncached, both red before the fix.

M1) Done as prescribed: PinnedRoTx now delegates FreezeInfo to the raw tx, routes Apply through the raw tx's guard while still handing the pinned handle to the callback, and forwards UnderlyingTx/Pin. Test: TestBeginTemporalRoWithOverlay_PreservesOptionalInterfaces, extended and red before the fix.
M2) Done as prescribed: the constructor panics on an unpinned tx instead of silently re-resolving, and the direct-constructor tests pre-pin. With the pin required at the door, Fork derives the parent's overlay from b.tx whose pinned-ness is now a type invariant. Test: TestGasPriceOracleBackend_RequiresPinnedTx, red before the fix.
M3) Hooks are error-returning and report through the goroutine-safe t.Errorf (only the FailNow family is test-goroutine-only); the five publish/commit/unpublish copies collapsed into one newCycleHookDB helper.
M4) The mock derives a per-height hash and honors the ok=false-beyond-head contract.

L1) Done as prescribed: added HeaderByHashNumber/BlockByHashNumber to the oracle backend, so the compute path fetches by the pair already resolved for the cache key — the fees.header.Hash() == blockHash guard now holds by construction and the per-block keccak is gone.
L2–L4) Done as prescribed: scoped the comment in fillFeeDefaults (the nonce/gas-estimate sub-calls still open their own txs — threading the pinned tx through belongs to the call-site migration follow-up); the pin-once rationale now lives only on PinToOverlay, with pointers elsewhere, and the redundant Rollback comment is gone; dropped the duplicated CarriesOverlayView guard and inlined newGasOracleFromBackend into its single caller.
L5) Done: the harness commit now writes the Execution stage progress and the canonical TxNums markers, so tests built on it are not limited to header-only reads.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

[yperbasis]

Requesting changes for one blocking performance regression.

High

rpc/gasprice/feehistory.go:419: the warm cache path still performs one canonical-hash lookup per hot block. FeeHistory calls localBackend.CanonicalHash before consulting the LRU at line 428 for every block above frozenBound. In rpcdaemon mode this reaches remoteTx.GetOne / SeekExact, so each lookup is a remote stream round trip.

The dual cache regime removes this cost only for old frozen ranges. The recent latest window remains above the frozen boundary, so a fully memoized eth_feeHistory(1024, latest) can still issue up to 1,024 remote lookups on every request. This defeats the cache fast path and creates user-controlled RPC amplification.

Please batch-resolve the hot canonical range or add a reorg-aware number-to-hash cache, and cover the warm rpcdaemon path with a benchmark or regression test.

[lupin012]

High — fixed: CanonicalHash becomes CanonicalHashes(from, to), resolved once per request before the fetchers fan out, over the unfrozen tract only. The backend does it as one kv.HeaderCanonical range scan on the pinned tx, so a warm eth_feeHistory(1024, latest) costs one remote round trip instead of up to 1024 (the window is well below the 16384-entry PageSizeLimit). Overlay head still visible, block reader still out of the cache key, scan errors still degrade to uncached.

Test: TestFeeHistory_HotRangeResolvedInOneScan — the mock records the ranges it is asked for; red before the fix with eight single-height resolutions.