Skip to content

[3.6] CP: don't clear domain RAM under a published SharedDomains (#23046) - #23164

Merged
yperbasis merged 1 commit into
release/3.6from
lupin012/keep_ram_published_sd_36
Aug 11, 2026
Merged

[3.6] CP: don't clear domain RAM under a published SharedDomains (#23046)#23164
yperbasis merged 1 commit into
release/3.6from
lupin012/keep_ram_published_sd_36

Conversation

@lupin012

@lupin012 lupin012 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Problem. RPC read views keep a DomainReader pointing at the published SD's in-memory domain maps. The background-commit teardown (bgSD.Close()mem.Close()ClearRam()) emptied those maps while readers were still using them. A receipt read of the in-flight block then missed silently, fell back to the request's pre-commit tx, and ReceiptAsOf zero-filled the miss: GetReceiptsGasUsed returned GasUsed=0 for every tx of the head block, and eth_feeHistory reward percentiles were silently wrong. Latest-state reads could likewise fall back to the previous block's state mid-request.

Fix. TemporalMemBatch.Close no longer clears the in-memory domain maps: they go to the GC once the last reference drops. With that, ClearRam had no production caller left and is removed entirely — the batch has a single lifetime (write, maybe publish, close-and-drop) and no API can clear the maps under readers. The one internal test that used clear-and-reuse now mirrors what cmd/integration actually does today: a fresh SharedDomains per batch.

Tests. New TestClose_KeepsDomainRamForReaders (red before the fix, green after). One existing assert updated: a post-teardown view now keeps serving the published head instead of falling back to its own tx.

)

Fixes the high-severity finding from the #22987 review.

**Problem.** RPC read views keep a `DomainReader` pointing at the
published SD's in-memory domain maps. The background-commit teardown
(`bgSD.Close()` → `mem.Close()` → `ClearRam()`) emptied those maps while
readers were still using them. A receipt read of the in-flight block
then missed silently, fell back to the request's pre-commit tx, and
`ReceiptAsOf` zero-filled the miss: `GetReceiptsGasUsed` returned
`GasUsed=0` for every tx of the head block, and `eth_feeHistory` reward
percentiles were silently wrong. Latest-state reads could likewise fall
back to the previous block's state mid-request.

**Fix.** `TemporalMemBatch.Close` no longer clears the in-memory domain
maps: they go to the GC once the last reference drops. With that,
`ClearRam` had no production caller left and is removed entirely — the
batch has a single lifetime (write, maybe publish, close-and-drop) and
no API can clear the maps under readers. The one internal test that used
clear-and-reuse now mirrors what `cmd/integration` actually does today:
a fresh `SharedDomains` per batch.

**Tests.** New `TestClose_KeepsDomainRamForReaders` (red before the fix,
green after). One existing assert updated: a post-teardown view now
keeps serving the published head instead of falling back to its own tx.

#22987 (draft) depends on this PR: pinning the overlay across `Fork`
makes this window easier to hit, so that PR stays a draft until this one
is merged.
@lupin012
lupin012 marked this pull request as ready for review August 11, 2026 05:45
@yperbasis yperbasis added this to the 3.6.0 milestone Aug 11, 2026
@AskAlexSharov
AskAlexSharov added this pull request to the merge queue Aug 11, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 11, 2026
@yperbasis
yperbasis added this pull request to the merge queue Aug 11, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 11, 2026
@yperbasis
yperbasis added this pull request to the merge queue Aug 11, 2026
Merged via the queue into release/3.6 with commit 9d4dcbe Aug 11, 2026
107 of 167 checks passed
@yperbasis
yperbasis deleted the lupin012/keep_ram_published_sd_36 branch August 11, 2026 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants