rpc: resolve head-sensitive reads on a well-defined view (overlay vs committed) - #22533
rpc: resolve head-sensitive reads on a well-defined view (overlay vs committed)#22533yperbasis wants to merge 14 commits into
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.
…rigontech#23279) Fixes erigontech#23194. Same class of bug as erigontech#23193: block tags resolved on the overlay view while the data scan reads the committed view. During an FCU background-commit window, `eth_getLogs` on `latest` failed transiently and `trace_filter` silently omitted the head block. ## Changes - `eth_getLogs`: resolve user tags with `nil` filters, on the same committed view as the `latest` baseline and the log scan - `trace_filter`: same, plus `CheckBlockExecuted` on an explicit `toBlock` so a not-yet-executed block errors instead of being silently clamped away - `debug_getModifiedAccountsByHash`: add the `startNum > latestBlock` guard its ByNumber twin already has Trade-off (as accepted in erigontech#23193): `pending` resolves to the latest executed block. ## Second commit: getLogsV3 complexity SonarCloud flagged `getLogsV3` on this PR (`go:S3776`, 64 against the 60 allowed). Pure refactor, no behaviour change: the three duplicated maxResults-capped append loops become `appendErigonLogs`, the state-sync lookup becomes `borStateSyncLogs`. 78 → 44 by gocognit, Sonar issue now closed as fixed. ## Notes - erigontech#22533 carries the same `nil`-filters hunks as part of a broader view-consistency pass; whichever merges second rebases trivially. - Medium term, erigontech#22987 introduces a pinned per-request view (`BeginTemporalRoWithOverlay`); migrating these call sites to it is the agreed follow-up — this PR keeps the endpoints correct in the meantime. ## Testing New tests in `overlay_race_test.go`, reusing the overlay helper introduced by erigontech#23193 plus a new `newHeaderAheadTester` helper (canonical header committed one past execution progress). All verified red before the fix and green after: - `TestGetLogs_UsesCommittedFromTag` / `TestGetLogs_UsesCommittedToTag` - `TestTraceFilter_UsesCommittedFromTag` - `TestTraceFilter_FutureToBlockErrors` - `TestGetModifiedAccountsByHash_FutureStartBlockErrors` The refactor commit is behaviour-preserving, so the existing `TestGetLogs_*` tests are its safety net; `TestAppendErigonLogs` and `TestBorStateSyncLogs_NoEvents` / `_EventsError` pin the extracted helpers
|
Assuming #22198 merges first, rebasing this PR will produce a small overlap in Please keep both changes in the resolution: domains, err := execctx.NewSharedDomains(
ctx,
roTx,
log.New(),
execctx.WithoutDeferredBranchUpdates(),
execctx.WithoutSharedBranchCache(),
execctx.WithSequentialCommitment(),
)They provide complementary guarantees: this PR's After resolving the overlap, both |
…nsistency # Conflicts: # rpc/jsonrpc/eth_receipts.go # rpc/jsonrpc/overlay_race_test.go # rpc/jsonrpc/trace_filtering.go
2 participants
Split out of #21293 (
FcuBackgroundCommitgroundwork). With that flag on, the FCU response returns before the MDBX flush+commit lands; several of these fixes matter already today, because state-change notifications are dispatched pre-commit. Head-sensitive RPC reads now resolve on a well-defined view — they see the pre-commit head everywhere or nowhere, never a mix.Overlay view
The head and every dependent block-table read are served from the published
BlockOverlay: borgetSnapshot/getAuthor/getSigners/getSnapshotProposer{,Sequence}/latest-block,eth_getBlockTransactionCountBy{Number,Hash}, graphql latest-block,debug_setHead,debug_getRawHeader, anderigon_getBlockByTimestamp. Behavior change on Polygon: for borgetSnapshot/getSigners/getSnapshotProposer{,Sequence}, nil/latestnow resolves to the overlay-aware forkchoice/executed head instead of the header-stage tip (ReadCurrentHeader), so a catching-up node answers for its executed position rather than the downloaded-header tip — consistent witheth_blockNumber;getAuthoradditionally fixes explicit-tag resolution (negative tags were cast touint64and returned "unknown block").BaseAPI.headerByHashis overlay-aware, covering every by-hash consumer that stays on block tables. Each request pins one overlay read view up front and reuses it for all dependent reads, so an overlay unpublished mid-request cannot drop the request onto the older MDBX snapshot.Committed view
The dependent reads use SD-temporal data, which the overlay does not expose, so tags resolve with
nilfilters and the bounds agree with the data:eth_getLogs/overlay_*range resolution (includingeth_getLogsblock-hash filters, now resolved viaHeaderNumberinstead of a full block decode),trace_filter,eth_getProof,eth_simulateV1,debug_traceBlockBy*,debug_storageRangeAt,debug_accountRange,debug_accountAt(by-hash included — an overlay-resolved head would have no committed history),eth_getWitness. The filters-param contract is documented onrpchelper.GetBlockNumber.eth_getProofadditionally keeps header lookup, commitment reconstruction, and state reads on the caller's single RO snapshot (it previously opened a second read tx — a snapshot-mixing bug); shared branch-cache reads are bound-gated (servableUnderBound, #22467), so a concurrent commit cannot mix snapshots. Nil guards coverparity_listStorageKeys,trace_filter, and theeth_getProofheader lookup.Compatibility: on payload-building nodes,
"pending"resolves to the latest executed block in every committed-view method that accepts the tag.debug_accountRangekeeps its explicit pending rejection.Tests
Overlay tests pin view selection in
TestGetBlockTransactionCountByHash_SeesOverlayHead,TestDebugAccountAt_OverlayHeadHash_CommittedView, andTestGetLogsBlockHashUsesCommittedView, plus view lifetime under concurrent unpublish (the three*_PinsOverlayViewtests).TestGetProofPinsReadSnapshotpins all proof reads to one RO snapshot;TestGetProofMissingHeaderpins a clean error for a missing header.Known limitation (embedded daemon)
Generic latest-state calls (
eth_call,eth_getBalance,eth_getStorageAt,eth_getCode) resolve the overlay head while their temporal state reads stay on the committed snapshot — head N with state N-1 for the commit duration. The SD-aware temporal view needed to close this is tracked in #21314. Genesis ("0x0"/"earliest")eth_getProofrejection is pre-existing and tracked in #22531.