erigontech / erigontech/erigon
IBS 2-Cache Phase 3: Fix timing hole; eliminate rs.accounts
- Dominant language
- Go
- Stars
- 3.6k
- Forks
- 1.5k
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 463
Description
Part of #19623 — Rationalize IntraBlockState to a 2-Cache Model.
Depends on #19701 (Phase 2).
## Goal
Make domain apply synchronous so `SharedDomains` is always up-to-date before block N+1 workers start, then remove the `rs.accounts` / `StateV3Buffered.accounts` workaround that patches the race.
## Background: the timing hole
In `execLoop()`, block N+1 workers are scheduled as soon as block N's `blockExecutor.result` is complete:
```go
blockExecutor.applyResults <- blockResult // async
// block N+1 workers start HERE — but SharedDomains may not have block N's writes yet
blockExecutor, ok = pe.blockExecutors[blockResult.BlockNum+1]
if ok {
blockExecutor.scheduleExecution(ctx, pe)
}
```
`rs.accounts` patches this by being populated synchronously in `finalize()` before `applyResults` is sent, so N+1 workers find block N's writes there. Eliminating `rs.accounts` requires making the domain apply synchronous in `execLoop()`.
## Changes
- In `execLoop()`, after `blockExecutor.result` is complete, call `applyBlockWritesToDomains(blockResult)` **synchronously** before calling `scheduleExecution(blockResult.BlockNum+1)`.
- Remove `StateV3Buffered.accounts` (the `rs.accounts` map) from `rw_v3.go` and `bufferedReader`.
- Remove the `rs.accounts` update side effect from `BufferedWriter` / `finalize()` — now dead code.
- Workers now read from `SharedDomains` directly (via `ReaderV3`) for cross-block state.
## New feature tests
- **`TestCrossBlockTimingRace`** (Phase 1, was designed to **pass** with `rs.accounts`) — must still pass after this change, now relying on synchronous domain apply rather than `rs.accounts`.
- **`TestCrossBlockApplyOrdering`** — assert that applying block N synchronously before scheduling N+1 means N+1 workers never see stale state, even under goroutine scheduling pressure.
- **`TestDomainConsistencyAfterBlockApply`** — after `applyBlockWritesToDomains`, assert the `SharedDomains` snapshot matches what `VersionedWrites` predicted.
- **`TestNoRsAccountsRace`** — run with `-race` over a multi-block parallel execution scenario; confirm no data races on the eliminated `rs.accounts` map.
## Regression tests
- **`TestCrossBlockStateReadConsistency`** (Phase 1) — must pass; block N+1 workers must still read block N's committed writes via `SharedDomains`.
- **`TestSnapshotRandomWithVersionMap`** (Phase 1) — must pass unchanged.
- All Phase 2 regression tests must continue to pass.
- Run `TestExecutionSpecBlockchain` to confirm no regressions.
- Run `go test -race ./execution/stagedsync/...` — no data races.
## Files affected
- `execution/stagedsync/exec3_parallel.go` — synchronize domain apply in `execLoop`
- `execution/state/rw_v3.go` — remove `StateV3Buffered.accounts`, `bufferedReader`
- `execution/exec/state.go` — worker state reader simplified (no longer needs `StateV3Buffered`)
## Acceptance criteria
- `TestCrossBlockTimingRace` passes without `rs.accounts`
- `-race` test run is clean
- Identical state roots on multi-block replay
- `make test-short` passes
Contributor guide
Research direction
Start in execution/stagedsync/exec3_parallel.go at execLoop and trace applyBlockWritesToDomains through the state files execution/state/rw_v3.go and execution/exec/state.go. Run the named cross-block tests and go test -race ./execution/stagedsync/... first. Done means the timing and consistency tests pass without rs.accounts, state roots remain identical, and make test-short passes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- blockchain
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100