erigontech / erigontech/erigon

execution/execmodule, node/shards: remove the global Events.LatestSD published-SD pointer (decouple readers via a publication id)

Open
#22,494 5 comments 0 reactions 1 assignee Claimed by @yperbasis View on GitHub
tech debt reduction
Dominant language
Go
Stars
3.6k
Forks
1.5k
Avg merge
1d 16h
Merged PRs (30d)
455

Description

## Summary

`node/shards.Events.LatestSD` is a process-global `atomic.Pointer[execctx.SharedDomains]`, set by `PublishOverlay` at each FCU and read by many consumers (RPC cache via `execmodule.Cache`, txpool `OnNewBlock`, the block builder, etc.). It is the only way to refer to "the current state publication," and that single mutable global is a source of coupling that this issue proposes to remove.

## The coupling effect

- **No identity.** The global names only "latest." A consumer cannot reference a *specific* publication, wait for one, or check it is reading the same one another consumer used. Coordination between consumers therefore degrades to matching by block **number** (`lastSeenBlock` / `ParentBlockNum` in the txpool's `best()`), which is fork/reorg-ambiguous (same height, different SD).
- **Unsynchronized read-fills.** Readers off the global run without the exec-module semaphore, so the cache read-fills they trigger (commitment BranchCache, account/storage StateCache) can land from positions not synchronized with the FCU flush/refresh cycle. This is the coherence hazard already tracked in #22214 — a *symptom* of the global, not a separate problem.
- **No lifetime / refcount.** Because the global hands out a bare pointer with no reference tracking, in-flight generations cannot be freed precisely: `ExecModule.drainCommittedGens` is intentionally a no-op (model A) and generations are only released as a unit at shutdown.

## Motivating use case (observed)

Under `fcuBackgroundCommit=true`, `TestAssembleBlockWithStateVerification` intermittently built a block with **0 transactions**. Root cause: when building block `N+1`, the txpool had correctly processed block `N` (`lastSeenBlock=N`, nonce-`N+1` txns `pending`), but the block builder's state reader was not pinned to the *same publication* of block `N` — so it saw a pre-`N` sender nonce and filtered every valid txn as nonce-gapped. The pool and the builder agreed on the block by **number**, but nothing pinned them to the same **publication**. (Fixed as a stopgap on PR #21414 by pinning the builder to the newest generation's retained SD — the same SD the pool reads — but the publication is identified implicitly as "newest generation" rather than by an explicit id.)

## Goal

Remove the mutable `Events.LatestSD` global. Consumers should reference a specific state publication by a stable identifier and coordinate on it, rather than racing to read "the latest global."

## Subtask: introduce a `(db.sd)` publication id

A monotonic major.minor identifier for each published state:

- **major = DB state version** — already exists (`rawdb.GetStateVersion` / `IncrementStateVersion`, bumped per durable commit).
- **minor = SD id** — new; numbers the in-flight generations published between two DB commits (`ExecModule.gens`). `0` under foreground commit, so it degenerates to today's behaviour when bg-commit is off.

This id gives: (a) a fork-unambiguous handle so the txpool can tag the txns it yields with the publication they are valid at and the builder can wait for that id or build on the parent; (b) a replacement for "latest global" references; (c) the refcount key that lets `drainCommittedGens` free generations precisely.

## Related

- #22214 — audit of published-SD readers for shared-cache coherence (a consequence of this coupling).
- #21314 — SD-aware temporal view to remove FcuBackgroundCommit RPC plain-tx reverts.
- #21414 — bg-commit-default reader-consistency fixes (the current stopgap this would generalize).

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.