[r3.5] db/state: CP route receipt domain reads through overlay DomainReader - #22951
Merged
lupin012 merged 3 commits intoAug 4, 2026
Merged
Conversation
…mainReader (#22511) as per issue #22504 where the node would sometimes serve corrupted logIndex values for newly-executed blocks. This was because `rawtemporaldb.ReceiptAsOf` was reading receipt metadata from the database (stale), rather than the in-memory uncommitted state. Users find this to be an incorrect log ordering in JSON-RPC receipts (`eth_getTransactionReceipt`), breaking downstream consumers (indexers, explorers, bridges) that depend on deterministic log indices. Receipt generation pulls uncommitted data for newly executed blocks by calling `rawtemporaldb.ReceiptAsOf`. This translates into `GetAsOf` and `HistorySeek` calls on the `ReceiptDomain`. The generator wraps the transaction in a block overlay, but `MemoryMutation.GetAsOf` and `MemoryMutation.HistorySeek` were going straight through to the underlying database transaction (`m.db.GetAsOf` and `m.db.HistorySeek`). Since they completely bypassed the uncommitted in-memory overlay, queries for new blocks would fall back to the database history and read stale values (such as LogIndexAfterTxKey) from the previous block. To address this, we have: 1. Added a `DomainReader` interface to `MemoryMutation` to push read-views back into the uncommitted memory overlay. 2. Intercept `GetAsOf` and `HistorySeek` calls in `MemoryMutation` and `OverlayTemporalReadView` for all domains. We first query the `DomainReader` (implemented by `SharedDomains`) to fetch the uncommitted memory state. If a value is found and there is no error, we use it; if not found or if the query errors (e.g. `inMemHistoryReads` is disabled for that domain), we silently fall back to the database transaction. 3. Gave `TemporalMemBatch` the ability to skip the `inMemHistoryReads` guard for `ReceiptDomain` in particular. This ensures that lookups on receipt indexes always work from the overlay even when full history tracking is disabled. 4. In receipt generator methods, wrapped the transaction in `WithTemporalOverlay` to ensure all reads go through the uncommitted memory overlay. We added a regression test `TestBlockOverlay_DomainReadsRegression`. The test writes receipt domain data to `SharedDomains` and checks: - The production path (`NewReadView` returns `*MemoryMutation`) correctly intercepts `GetAsOf` and `HistorySeek` for returning the uncommitted data. - The secondary path (`BlockOverlayTemporalTx` returning `*OverlayTemporalReadView`) also correctly resolves the in-memory receipt values. Closes: #22504 (cherry picked from commit 114c90a)
…llowing them (#22893) Independent correctness fix split out of #21414 (per the request to break it up). Stands alone on `main`. `MemoryMutation` and `OverlayTemporalReadView` `GetAsOf`/`HistorySeek` gated the `DomainReader` result with `err == nil && ok`, so a reader error fell through to the committed tx and was silently hidden. This propagates the error and keeps the `ok`-based committed fallback (a tombstone is `ok=true`, so it is not resurrected). No dependency on the background-commit work; part of the split recorded on #21414. (cherry picked from commit 8324709)
Covers the two cherry-picks below it, each verified red before its fix and green after: - TestReceiptAsOf_InFlightBlockLogIndex — ReceiptAsOf through the overlay read view must return the in-flight block's LogIndexAfterTx. Without #22511 it returns the last committed block's value (0x7 instead of 0x3), which is the wrong first log index applied to every transaction of that block. Uses the production key and accessor, unlike the storage-level test #22511 shipped. - TestDomainReadErrorsPropagate — a DomainReader error must reach the caller instead of falling through to the committed tx. Without #22893 the error is swallowed and the read silently answers with stale data. Covers both MemoryMutation and OverlayTemporalReadView. - TestGetReceiptLogIndexThroughOverlay — pins the production wiring: the overlay is seeded with a log index the committed tx does not hold, so only a read routed through Filters.WithTemporalOverlay can produce it. Drop that call from GetReceipt and this test fails; the other two stay green.
lupin012
changed the title
lupin012
marked this pull request as ready for review
August 3, 2026 06:08
lupin012
requested review from
AskAlexSharov,
sudeepdino008 and
yperbasis
as code owners
August 3, 2026 06:08
sudeepdino008
approved these changes
Aug 3, 2026
pull Bot
pushed a commit
to Dustin4444/erigon
that referenced
this pull request
Aug 3, 2026
…erlay (erigontech#22961) Port of the tests added in the release-branch backports erigontech#22951 (`release/3.5`) and erigontech#22960 (`release/3.6`). All three pass on `main` — both erigontech#22511 and erigontech#22893 are here — so they land as regression coverage. Each was written against the branch where its fix was missing and verified red there first: | Test | Verified red on | Without | |---|---|---| | `TestReceiptAsOf_InFlightBlockLogIndex` | `release/3.5` | erigontech#22511 — returns `0x7` instead of `0x3` | | `TestDomainReadErrorsPropagate` | `release/3.6` | erigontech#22893 — error swallowed, `but got nil` | | `TestGetReceiptLogIndexThroughOverlay` | `main` | the `WithTemporalOverlay` call in `GetReceipt` | The third one is the reason this is worth having on `main` regardless of the backports: `Filters.WithOverlay` / `WithTemporalOverlay` had **no test coverage anywhere in the tree**. Remove `tx = g.filters.WithTemporalOverlay(tx)` from `GetReceipt` and the bug reported on erigontech#22106 returns in production while CI stays green — verified, that test now fails. The other two also add coverage the upstream fixes did not ship: erigontech#22511's own test works at storage level with a synthetic key, so nothing exercised `LogIndexAfterTxKey` through `rawtemporaldb.ReceiptAsOf`; erigontech#22893 shipped with no test at all. One adaptation from the release branches: `AppendReceipt` is named `AppendReceiptMetadata` here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick of #22511 and #22893 to
release/3.5.release/3.5serves wronglogIndexvalues for a block whose commit is in flight.Generator.GetReceiptadmits the block through the block overlay (CheckBlockExecuted(g.filters.WithOverlay(tx), …)) but reads its metadata from the committed tx: on a history missDomainRoTx.GetAsOffalls back toGetLatest, soReceiptAsOfanswers with the previous writing block's final in-block log count, applied to every transaction of the affected block. The value is then cached in the RPC layer, which is why a restart clears it.Both fixes have been on
mainsince mid-July and neither was ever backported. Reported on #22106 (v3.5.1–v3.5.4, Ethereum mainnet archive), with a detailed root-cause analysis in this comment.#22893 is a direct follow-up: the code #22511 added gated the
DomainReaderresult onerr == nil && ok, so a reader error fell through to the committed tx and reopened the same window. Both are needed, in this order.Conflict resolution
db/kv/membatchwithdb/memory_mutation.go—OverlayTemporalReadViewon 3.5 still hasAggForkablesTxandUnmarked, removed on main by #22287. Kept the 3.5 methods; they are orthogonal to the fix.Tests
Neither upstream PR shipped a test for the RPC-visible behaviour, so this adds three, each verified red before its fix and green after:
TestReceiptAsOf_InFlightBlockLogIndex—ReceiptAsOfthrough the overlay read view must return the in-flight value. Without db/state: route receipt domain GetAsOf/HistorySeek through overlay DomainReader #22511:0x7instead of0x3.TestDomainReadErrorsPropagate— aDomainReadererror must reach the caller, not fall through to the committed tx. Without db/kv/membatchwithdb: surface DomainReader read errors instead of swallowing them #22893 the error is swallowed. CoversMemoryMutationandOverlayTemporalReadView.TestGetReceiptLogIndexThroughOverlay— pins the production wiring. The overlay is seeded with a log index the committed tx does not hold, so only a read routed throughFilters.WithTemporalOverlaycan produce it. Removing that call fromGetReceiptfails this test while the other two stay green — that path had no coverage anywhere in the repo.Note for release
No resync or DB rewind is needed. The wrong values were never persisted — they were computed at RPC time and held in the generator's in-memory cache, which the upgrade's own restart clears. Worth stating, because the early theory on #22106 was a corrupted
ReceiptDomainand operators were advised to rewind or resync.Not verified
No test crosses the whole
eth_getLogspath, and nobody has confirmed the fix on a live 3.5 node. The reporter on #22106 has a reproduction that hits 8 times out of 8 — a build of this branch tested against it would settle it.