erigontech / erigontech/erigon

Build SD-aware temporal view to remove FcuBackgroundCommit RPC plain-tx reverts

Open
#21,314 4 comments 0 reactions 2 assignees Assigned to @yperbasis View on GitHub
performance RPC
Dominant language
Go
Stars
3.6k
Forks
1.5k
Avg merge
1d 16h
Merged PRs (30d)
455

Description

## Context

Follow-up from the #21293 split — the RPC piece is now #22533 (`FcuBackgroundCommit` groundwork). That PR routes head-sensitive RPC reads through the `SharedDomains` block overlay so the FCU response can return before the MDBX commit lands, but several callsites were intentionally **reverted to plain tx** in 79ce7d9 because their dependent reads touch SD-managed temporal data (state domains, history, inverted indexes) that the current overlay doesn't expose.

## Current limitation

`Filters.WithOverlay(tx)` / `WithTemporalOverlay(tx)` wraps **table-level** reads (canonical hashes, headers, bodies, TxNums, stage progress, forkchoice markers) via `MemoryMutation`. But its temporal methods delegate straight to the passed-in temporal tx — see [`memory_mutation.go:836-889`](https://github.com/erigontech/erigon/blob/main/db/kv/membatchwithdb/memory_mutation.go#L836-L889) for the plain view and [`memory_mutation.go:1041-1064`](https://github.com/erigontech/erigon/blob/main/db/kv/membatchwithdb/memory_mutation.go#L1041-L1064) for `OverlayTemporalReadView`:

```go
func (v *OverlayTemporalReadView) GetLatest(name kv.Domain, k []byte) ([]byte, kv.Step, error) {
return v.temporalTx.GetLatest(name, k)
}
// ... GetAsOf, RangeAsOf, HistorySeek, HistoryRange, IndexRange all delegate to v.temporalTx
```

The SD's `mem` (`TemporalMemBatch`) holds block N's accounts/storage/code/commitment writes and the log inverted indexes during the ~50ms bg-commit window, but the temporal view above delegates around it. So overlay-aware **table** reads see N's headers and canonical hashes, while temporal/domain reads return state at PSV=X (pre-commit, i.e. N-1's state).

This forces a head-vs-state choice at each callsite:
- Use overlay for the head lookup, plain for dependent reads → head says N, data is at N-1 → silently wrong (no error to retry on).
- Use plain everywhere → internally consistent at N-1; \"latest\" lags by one block for ~50ms across calls.

The PR picked option 2 wherever dependent reads need SD-temporal data; option 1 wherever they don't.

## Affected callsites (the 79ce7d9 reverts)

Each of these would benefit from a proper SD-aware temporal view:

- [`rpc/jsonrpc/eth_call.go`](https://github.com/erigontech/erigon/blob/main/rpc/jsonrpc/eth_call.go) — `getProof` / `getWitness`: `domains.SeekCommitment(roTx)` reads the commitment domain; N's commitment writes are in SD.mem.
- [`rpc/jsonrpc/eth_receipts.go`](https://github.com/erigontech/erigon/blob/main/rpc/jsonrpc/eth_receipts.go), [`erigon_receipts.go`](https://github.com/erigontech/erigon/blob/main/rpc/jsonrpc/erigon_receipts.go), [`overlay_api.go`](https://github.com/erigontech/erigon/blob/main/rpc/jsonrpc/overlay_api.go) — `GetLogs` / `getBeginEnd`: `getLogsV3` scans `kv.LogAddrIdx` / `kv.LogTopicIdx`, both SD-managed inverted indexes.
- [`rpc/jsonrpc/eth_simulation.go`](https://github.com/erigontech/erigon/blob/main/rpc/jsonrpc/eth_simulation.go) — `SimulateV1`: `NewSharedDomains(ctx, tx, ...)` ties the simulator to the plain tx's state.
- [`rpc/jsonrpc/debug_execution_witness.go`](https://github.com/erigontech/erigon/blob/main/rpc/jsonrpc/debug_execution_witness.go) — `buildExpectedPostState`: txnum / commitment-seek branch reads through plain tx.
- [`rpc/jsonrpc/eth_call.go`](https://github.com/erigontech/erigon/blob/main/rpc/jsonrpc/eth_call.go) `Call` itself — already documented in code: \"latest\" header resolves to N (overlay) but `eth_call(latest)` evaluates state at N-1. This is the user-visible \"header-vs-state lag\".
- `eth_callBundle`, `eth_createAccessList`, `eth_callMany`, and `erigon_getBalanceChangesInBlock`: latest block selection can see overlay tables while their state, TxNums, or history reads remain on the committed transaction. They must either select a committed target end to end or use the SD-aware temporal view; request-view acquisition and propagation are tracked by #23416.
- [`rpc/jsonrpc/parity_api.go`](https://github.com/erigontech/erigon/blob/main/rpc/jsonrpc/parity_api.go) `ListStorageKeys`: `tx.RangeAsOf(kv.StorageDomain, ...)` over the storage domain history; block N's storage writes are in SD.mem. `NewLatestStateReader(tx).ReadAccountData(...)` at the top is also SD-temporal.

(`debug_api.go SetHead` and `erigon_block.go GetBlockByTimestamp` only touch block tables and were made overlay-consistent in #22533 itself — they are not waiting on this issue.)

## Proposed solution

Build a real SD-aware temporal view that chains reads in the natural priority order:

```
SD.mem (latest in-flight writes for block N)
→ SD.blockOverlay (table-level writes for block N)
→ underlying committed MDBX TemporalTx (state ≤ N-1)
```

Concretely, replace the current `OverlayTemporalReadView` temporal-method delegations with logic that:

1. Consults the SD's `mem` (`TemporalMemBatch`) first for `GetLatest` / `GetAsOf` / `RangeAsOf` / `HistorySeek` / `HistoryRange` / `IndexRange`.
2. Falls through to the underlying `temporalTx` for anything not in mem.
3. Carries the published SD's reference (via `Filters.LatestSD()`) for the lifetime of the view so the read sequence stays coherent even if the overlay is unpublished mid-read.

Key implementation considerations:

- **Lifecycle**: today `bgSD.Close()` rolls back the overlay's `memTx`. RPC readers holding a view via `WithOverlay`/`WithTemporalOverlay` are only protected by `PublishOverlay(nil)` happening *before* `Close()`. The SD-aware view should make this explicit — either refcounting `SD.mem` or copy-on-acquire of the relevant domain state.
- **Concurrency**: `SD.mem` is accessed concurrently by execution (writers) and RPC (readers). The mem batch's existing internal sync needs to support reader concurrency; verify or add as needed.
- **Range semantics**: `RangeAsOf` over a domain currently scans the underlying file/btree state. The SD-aware version needs to merge SD.mem entries into the iteration in tx-num order. This is non-trivial for streaming iterators.
- **Independence from bg-commit timing**: today the overlay is published from `dispatchNotificationsFromOverlay` and unpublished by the bg goroutine. The SD-aware view should hold a stable snapshot regardless of where in the lifecycle the bg commit currently is.

## Acceptance criteria

- [ ] `Filters.WithTemporalOverlay(tx)` returns a temporal view whose `GetLatest`/`GetAsOf`/`RangeAsOf`/`HistorySeek`/`HistoryRange`/`IndexRange` reflect the SD's in-flight mem batch for the head FCU during the bg-commit window.
- [ ] The plain-tx reverts in 79ce7d9 listed above are dropped: each affected RPC handler can use the overlay-aware tx end-to-end and produce results consistent with the head it resolves \"latest\" to.
- [ ] `eth_call(latest)` evaluates against block N's state (not N-1) during the bg-commit window — no more documented \"header-vs-state lag\".
- [ ] No new races: closing the published SD must not invalidate in-flight RPC reads (verify under `-race` with concurrent FCU + RPC load).
- [ ] Performance: no measurable regression on cache-hit RPC paths; overlay miss should still cascade to MDBX through the existing `MemoryMutation` table-fallback machinery.

## Related

- PR #22533 (overlay/committed RPC read split; introduces the reverts this issue removes)
- PR #22269 (enables `FcuBackgroundCommit` by default)
- #20195 (introduced `_GetBlockNumber` overlay-aware \"latest\" resolution — the prerequisite for this work)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.