rpc: separate overlay-backed and committed read paths - #22533
Conversation
Split out of #21293. Overlay-view methods pin one BlockOverlay read view per request; committed-view methods resolve tags with nil filters so the bounds agree with the temporal data they scan. eth_getProof keeps all reads on the caller's single RO snapshot.
…nsistency # Conflicts: # rpc/jsonrpc/eth_call.go # rpc/jsonrpc/eth_call_test.go
trace_block, trace_call, trace_callMany, trace_replayBlockTransactions,
trace_rawTransaction and debug_traceCall{,Many} resolved head-sensitive
block tags through the overlay while replaying on the committed tx, so a
"latest" trace could run block N's transactions against state ending at
N-1. On a mainnet archive node at the tip that made trace_block("latest")
fail 129 of 4134 calls (3.1%) with "nonce too high: tx X state X-1", and
occasionally "insufficient funds"; debug_traceBlockByNumber, which already
builds its context from the plain tx, failed 0 of 4134 over the same run.
The replay reads SD-temporal data, which the overlay does not serve:
OverlayTemporalReadView.GetLatest delegates straight to the committed tx,
so the state side cannot follow the overlay head. Resolving these tags on
the committed view is what makes the bounds and the scan agree; pinning a
single overlay view instead needs the SD-aware temporal view from #21314.
Extends the same treatment already applied to trace_filter and
debug_traceBlockBy*.
|
Merged main (was ~350 commits behind, conflicting) and extended the committed-view rule to the remaining tracing replay paths. Merge conflicts resolved: dropped the The added commit applies the same
Failures were Worth noting for #22969: that panic is behind I did not touch the state-reader path — pinning a single overlay view instead would still read Happy to split the added commit into its own PR if you would rather keep this one at its original scope. |
The BaseAPI helpers derived the overlay view twice — once to resolve the block tag, once to read the header or body — so an overlay unpublished between the two calls dropped the read onto the older MDBX snapshot while the number came from the newer one. headerByNumber, headerByNumberOrHash, blockByNumberWithSenders and blockByHashWithSenders now derive it once and thread it through, and erigon_getBlockByTimestamp reuses the view its search bounds came from. Also corrects a comment that claimed the overlay exposes block tables only: MemoryMutation.GetAsOf and HistorySeek do consult the SharedDomains set by InitBlockOverlay. GetLatest and RangeAsOf are the reads that stay on the committed backing tx, which is what the committed-view resolution depends on.
Resolving the blockHash filter through HeaderNumber alone accepts any header the header-number index knows, including side-chain and header-only ones, while the log scan that follows is by block number. A non-canonical hash therefore returned the canonical block's logs instead of an error. Gate the resolved number on the canonical hash matching.
The tracing methods resolve and replay on the committed view, which holds no pending block, so rpchelper.GetBlockNumber silently resolved "pending" to the latest executed block: a caller asking to trace pending got a trace of a different block, reported as that block, with no error. Reject the tag instead, matching go-ethereum, which answers "tracing on top of pending is not supported" rather than substituting a block. Covers debug_traceBlockByNumber/ByHash, debug_traceCall, debug_traceCallMany, trace_block, trace_replayBlockTransactions, trace_call and trace_callMany.
|
Added an explicit rejection of the With go-ethereum rejects it outright — Separately, |
debug_traceBlockByNumber("pending") answered before this series and the RPC
integration suite pins that (debug_traceBlockByNumber/test_25), so rejecting
the tag there broke mainnet-rpc-integ-tests. go-ethereum draws the line in
the same place: it refuses to execute a call on top of pending, but traces a
pending block rather than erroring.
Keep the rejection on debug_traceCall, debug_traceCallMany, trace_call and
trace_callMany; drop it from debug_traceBlockBy*, trace_block and
trace_replayBlockTransactions.
… read The execution gate read the plain roTx while the block tag was still resolved through the overlay, so during a commit an overlay-resolved head could be reported as not executed. Resolve on the committed view instead, which is also where the commitment-history reads happen. This overlaps the same change in #22533; whichever lands first, the other is a trivial conflict. rawdb.ReadCurrentBlockNumber returns nil when no head header is set, so listStorageKeys dereferenced a nil pointer instead of returning an error.
TestTraceBlockAcceptsPendingTag asserted NotErrorIs, which passes on any unrelated error; all three methods return nil there, so assert NoError. The test now fails if the rejection is widened back onto block tracing, which is the regression that broke mainnet-rpc-integ-tests. trace_call and trace_callMany still advertised 'pending' as an accepted tag, so update those two parameter lists. trace_block and trace_replayBlockTransactions keep accepting it and are left alone. Versioned docs describe shipped releases and are not touched.
Completes the rejection started for the call methods. debug_traceBlockBy*, trace_block and trace_replayBlockTransactions resolve tags on the committed view, where "pending" falls through to the latest executed block, so they answered for the head block and reported it as the pending request. go-ethereum either traces a real pending block or errors; it never substitutes a different one, so answering for latest matches neither branch. Real pending-block tracing needs a pending state source and is left for later; until then an explicit error beats a wrong block. CI note: this changes debug_traceBlockByNumber/test_25 in the rpc-tests suite, updated in erigontech/rpc-tests#588. mainnet-rpc-integ-tests stays red until that merges and RPC_VERSION is bumped.
|
Extended the pending rejection to the block-tracing methods — The earlier split (reject on the call methods, accept on block tracing) was based on go-ethereum tracing a pending block rather than erroring. That is only half of what geth does: Real pending-block tracing needs a pending state source and is a separate change; until then an explicit error beats a wrong block. CI dependency: this changes Docs updated for all four affected |
…n execution (erigontech#23165) Two state-version bugs in the RPC layer, both independent of each other and of the view-consistency work in erigontech#22533. `parity_listStorageKeys` reads the account with a latest-state reader — the state after head block `bn` — but scanned its storage at `Min(bn)`, the first txNum of `bn`, which is the state after `bn-1`. The account and its storage therefore came from different blocks: a slot written in the head block was missing from the listing, and a slot deleted in it was still listed. `state.Dumper`, the equivalent path, uses `Min(blockNumber+1)`. `eth_getProof` resolved a block by canonical hash alone. Canonical hashes exist for blocks the header stage has downloaded but execution has not reached, so a request for one walked the history path and surfaced a `PrunedError` or a root-hash mismatch instead of reporting that the block is not executed yet. ## Changes - `parity_api.go` — `Min(bn)` → `Min(bn+1)` so the storage scan matches the account read. - `eth_call.go` — gate `GetProof` on `rpchelper.CheckBlockExecuted` after resolution.
|
@AskAlexSharov Follow-up to #22533 (comment), rechecked against current head 6d331e5:
The remaining composite and independently acquired view propagation is tracked by #23416. Thanks for the detailed review. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 37 changed files in this pull request and generated 4 comments.
Suppressed comments (1)
rpc/jsonrpc/eth_receipts.go:586
- The selector at line 573 may resolve through overlay generation A, but this independently selects the current overlay for the body, and
getReceiptsreceives the raw transaction and can select another generation. A publish/unpublish between these steps can combine different blocks and receipts. Capture one temporal overlay view before selector resolution and pass it through block and receipt reads.
block, err := api.blockWithSenders(ctx, api.filters.WithOverlay(tx), blockHash, blockNum)
During forkchoice flush and commit, the published
BlockOverlaycan be one block ahead of the MDBX snapshot held by an RPC request. Both are valid views, but combining an overlay block or transaction with committed temporal state or history can produce a response that never existed on-chain.This PR applies two explicit policies to the migrated handlers:
Behavior
eth_call,eth_createAccessList, and GraphQLcallrejectpendingbecause they cannot acquire state matching the requested pending block.nullwhen it cannot fully represent a published pending block.Scope
This PR preserves an overlay generation after it has been selected. Atomic acquisition of the database snapshot and overlay, including pinning the absence of an overlay, remains in #22987. Until then, the temporary
IsOverlayReadViewmarker prevents nested helpers from replacing an existing overlay view. Adoption by the remaining composite and independently transacted RPC paths is tracked by #23416.Receipt generation remains part of #23416: the generator can still select the live overlay internally and use a process-wide state cache instead of remaining bound to the transaction selected by its caller.
This PR does not change
eth_estimateGas. Consistent EstimateGas view selection and safe pending-template handling remain in stacked draft #23485.The generic embedded-daemon latest-state view remains tracked by #21314. Remote block readers also require snapshot propagation or result validation, tracked by #23416.
overlay_getLogsselects its range from committed progress, but its parallel workers still open independent transactions under the same follow-up.Related follow-ups cover witness branch-cache isolation (#22198), selector and error-policy consolidation (#23424 and #23428), coherent replay-cache reuse (#23425), transaction-index validation (#23431), and call-many state-context validation (#23444). Overlay-history fallback when in-memory history is disabled is tracked by #23500; the historical state boundary for ad-hoc trace bundles is tracked by #23501.
Review map
db/kv/membatchwithdb/memory_mutation.go,rpc/rpchelper/filters.go, andrpc/jsonrpc/eth_api.gorpc/jsonrpc/overlay_race_test.goandrpc/jsonrpc/trace_view_consistency_test.goTests
Regression tests exercise overlay publication, replacement, and unpublish windows; reorgs; execution progress; pending selectors; missing data; cache isolation; and endpoint-compatible error behavior.
RPC integration coverage uses the pinned
rpc-testsrelease, which includes the corresponding test correction from erigontech/rpc-tests#588.