erigontech / erigontech/erigon

IBS 2-Cache Phase 3: Fix timing hole; eliminate rs.accounts

Open
#19,702 1 comment 0 reactions 0 assignees View on GitHub
performance tech debt reduction
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.