erigontech / erigontech/erigon

IBS versionMap unification: complete the single versionedio model (remove the stateObject)

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

Description

Follow-up work deferred after the cache-free parallel-execution change (PR #22409) that removed the resident `stateObject` from the parallel-execution and block-building paths. Design docs: `docs/plans/20260710-ibs-versionmap-unification-followups.md` and `docs/plans/20260709-versionedio-single-source-bal-occ.md`.

## Ground rules (settled — do not re-arbitrate)

- **End state: one versionedio processing model for everything** — serial, parallel, genesis, block building, and RPC all commit through the write-set, with **no `stateObject` at all**.
- **It is not OCC-dependent.** OCC (conflict detection + incarnation retry) is purely a *parallel* concern. Run serially, the exact same versionedio path executes and simply never produces a conflict. There is no "serial vs parallel" commit split to preserve.
- **`noMaterialize` is not a first-class concept.** It is redundant with the parallel-execution decision: `EXEC_PARALLEL = dbg.Exec3Parallel || cfg.experimentalBAL` (`stage_execute.go`) selects the `parallelExecutor`, whose `taskVersion.Reset` sets the flag. The rule is simply **`EXEC_PARALLEL=true` ⇒ versionedio only**. The flag is a transitional artifact, deleted once serial/genesis/RPC move over.
- **No retreat.** No resolution may "keep X on the stateObject path" or "de-version X". Every leftover on `stateObjects` is a path to be *ported forward* to the write-set, never preserved.

## Current state (delivered in #22409)

The parallel executor (`EXEC_PARALLEL=true`) and block building run without a `stateObject` cache — reads resolve from the state reader (own CreateContract/SelfDestruct/Code cells reconstructed onto a transient object), writes go versionedio → `WriteSet.Apply`. Serial execution, genesis, and RPC still commit via `stateObjects`; those are the leftovers below.

## Follow-up 1 — genesis commit via the write-set

Genesis is an **incomplete port**: `ComputeGenesisCommitment` builds the IBS with `NewWithVersionMap(r, &state.VersionMap{})` (so `IsVersioned()` is true), but the write side still commits via `FinalizeTx → stateObjects → stateWriter`, and its `FinalizedWrites` write-set is never applied. The `isGenesis` guard in `txtask.go` (`TxIndex == -1 && BlockNumber() == 0`) exists only to route this "versioned" IBS back onto `MakeWriteSet` — that guard is a mask, and the serial-genesis regression (broadening the `IsVersioned()` branch → empty genesis root) was this bug surfacing.

Not an active bug today (works via `noMaterialize=false` keeping stateObjects populated), but a **hard blocker for the map-drop**: once the stateObject is gone, `FinalizeTx` finds nothing → empty genesis.

**Do:** finish the port. Commit genesis via `FinalizedWrites().Apply(sd, tx, 0, 1, nil, &chain.Rules{}, nil, false)` (every field is recorded as a cell, so the `blockCache == nil` branch has the data), set `noMaterialize` on the genesis IBS, and drop the `FinalizeTx`→writer commit and the `isGenesis` guard. High blast radius — this computes the genesis root of every chain. The returned IBS is re-consumed by three executors with different commit mechanisms (`txtask.go`, `historical_trace_worker.go`, `rpchelper/commitment.go` which discards it); confirm the block-0 commit path for each consumer first.

## Follow-up 2 — serial executor off stateObjects

The serial path still commits via `FinalizeTx` / `MakeWriteSet` / `CommitBlock` over `stateObjects` — a leftover, not a requirement. Running serially is just the versionedio path with no conflicts, so there is nothing OCC-specific it needs. **Do:** run the same write-set commit as the parallel worker; conflict detection/retry simply never fires. With follow-up 1 this unblocks the map-drop.

## Follow-up 3 — drop the parallel-path maps + the flag

Once 1 and 2 land: make versionedio the behaviour (remove the `noMaterialize` flag rather than default it true) and delete `stateObjects`, `stateObjectsDirty`, `nilAccounts`, `balanceInc`. Keep `sdProbe` (the parallel self-destruct probe cache).

## Follow-up 4 — component D cleanup

- Remove the IBS `StateWriter` surface (the `NoopWriter`-suppress pattern is obsolete once writes go versionedio → `WriteSet.Apply` → SharedDomains).
- Collapse `StateV3` (superseded by that path).
- Make the reader a pure SharedDomains adapter (latest + historic).
- Tidies: `finalizeSystemTx`'s intermediary `state.New` reconstruction; the dead `versionedWriteCollector` type (tests still reference it); the intermittent `bbbb` reconcile-drop.

## Follow-up 5 — warm-read throughput (iterative)

`versionedReadCore` re-probes the version map under a single global `RWMutex`; on warm cells the reader-counter atomic bounces across workers (~17% of `warm-extcodehash` samples), leaving the warm family ~2.2× behind geth (lock-free resident hit). These were geth-comparable pre-`noMaterialize`, so it's recoverable. Target model: **early detection + pause on the write side** — a write that publishes identifies and pauses/reschedules in-flight txs that already read the stale value, so readers become a cheap lock-free resident hit instead of re-probing under lock. Give readers a resident, decoded, interned-handle-keyed warm view; move conflict detection to write-publish; cut redundant reads (`Empty()`'s 4 probes → 1); measure each iteration vs geth/reth per-op cost. Mind intern costs (`unique.Make` per `accounts.Address`/`CodeHash`).

## Low-risk quick win

Follow-up 4's `versionedWriteCollector` removal touches no commit path and can be done independently of the map-drop.

## Follow-up 6 — solidify the transitional model; retire legacy-path reliance

The versionedio model is only half-established (parallel executor + block builder on it; genesis, serial executor, RPC, and the BAL regenerator still lean on `stateObjects`/legacy commit code). **Every one of those points is a live correctness risk until migrated** — they can silently diverge on the `noMaterialize` path, and merges with `main` (which keeps evolving the legacy/BAL code) either drop subtle correctness commits or expose incompatibilities. PR #22409 hit this class twice: the access-set model reconciliation, and the BAL regenerator missing the per-tx versionMap flush.

The BAL path is the sharpest example. BAL is produced or replayed in ≥4 places — the parallel executor, `chain_makers`, the block builder (`block_assembler.go`), and `bal/rederive.go` — and each must independently reproduce the same cross-tx discipline: **flush each phase's writes to the versionMap between phases** (`FlushWritesToVersionMap`/`FlushVersionedWrites`) so a later tx's write (e.g. the accumulating coinbase fee) sees the running value. Any new BAL path that forgets it produces wrong BALs on the cache-free model.

Do — this needs a **whole-process audit**, not just a shared helper:
- Audit every call site that crosses a tx/phase boundary in the versionedio model (parallel executor, `chain_makers`, block builder, `bal/rederive.go`, plus any the audit surfaces) — enumerate what each must call and in what order today.
- **Internalize `FlushWritesToVersionMap`.** The per-tx flush must stop being a separate call a site has to remember; fold it into the tx-boundary operation itself (part of `MergeTxIOInto`/commit or `ResetVersionedIO`) so crossing a tx boundary is one call the caller cannot get wrong, collapsing today's `MergeTxIOInto` → `FlushWritesToVersionMap` → `ResetVersionedIO` dance. Simplify the required call surface to that one operation.
- Land follow-ups 1–3 (genesis, serial, map-drop) so the mixed model — the source of the risk — is gone.
- Until then, treat any change to a legacy/stateObject path, or any `main` merge in the BAL/versionedio area, as requiring the per-tx-flush + access-model audit.

## North star — split IBS into a tx context and a block context

Several refactor iterations out (past follow-ups 1–6), the destination these steps converge toward: **remove `IntraBlockState` as a monolith** and replace it with two scoped abstractions and a simplified API for clients (executors, builder, regenerator, RPC) and the interpreter:

- **block context** — owns the versionMap, the committed/domain view, and block-wide accumulation (BAL, block IO); publishes and commits.
- **tx context** — owns one tx's reads/writes/journal/access/transient storage, scoped to a single tx; on close it publishes into the block context.

In this model Follow-up 6's flush internalization is *subsumed* rather than implemented: the per-tx flush is no longer a call at all — **closing a tx context _is_ publishing into the block context**, because the tx→block boundary becomes a real object boundary instead of a sequence a caller drives on a shared IBS. The mixed model, the manual Flush/Merge/Reset dance, and the "which paths remember to flush" footgun disappear together. A direction, not a task; the earlier follow-ups are the iterations that make it reachable.

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.