erigontech / erigontech/erigon
execution/stagedsync: remove MemoryBatch from MiningStep — use TemporalTx throughout builder pipeline
- Dominant language
- Go
- Stars
- 3.6k
- Forks
- 1.5k
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 455
Description
## Background
`MiningStep` (in `execution/stagedsync/stageloop/stageloop.go`) builds a new block by running the builder stage pipeline. It has historically opened a `BeginTemporalRw` (MDBX exclusive write transaction) as the base tx, wrapped it in a `MemoryBatch` overlay, and always rolled back the base tx when done. The write transaction was never needed — all actual writes from the mining stages go into the `MemoryBatch` in-memory buffer, which is discarded without flushing.
PR #19821 fixed the immediate symptom — switching `BeginTemporalRw` to `BeginTemporalRo` to stop holding the MDBX exclusive write lock during block builds. However, `MemoryBatch` is still present purely as a compatibility shim to satisfy the `kv.TemporalRwTx` type required by `Sync.Run` and `ExecFunc`.
## Why MemoryBatch is now unnecessary
The builder stage pipeline (`builderstages`) was refactored so that all state writes go through `SharedDomains`, not through the underlying `kv.TemporalRwTx`. Inspection of the builder stages confirms:
- `SpawnBuilderCreateBlockStage` — only reads from `tx` (`rawdb.ReadHeaderByNumber`, `rawdb.ReadHeadersByNumber`)
- `SpawnBuilderExecStage` — only reads from `tx` (`rawdb.ReadHeader`)
- `StageExecuteBlocksCfg` (forward path) — writes state exclusively through `SharedDomains`
No builder stage writes directly to `tx`. `MemoryBatch` exists only to provide a `kv.TemporalRwTx`-shaped object to `Sync.Run`.
## Proposed fix
Broaden the interfaces in the stage pipeline so the builder path can use a plain read-only tx:
1. **`ExecFunc` / `UnwindFunc`** (`execution/stagedsync/stage.go`): change `rwTx kv.TemporalRwTx` → `tx kv.TemporalTx`
2. **`Sync.Run`, `Sync.RunUnwind`, `Sync.RunNoInterrupt`, `Sync.RunSnapshots`** (`execution/stagedsync/sync.go`): change `tx kv.TemporalRwTx` → `tx kv.TemporalTx`
3. **Non-builder stages** that write to `tx` directly: add an internal cast `rwTx := tx.(kv.TemporalRwTx)` — these are only ever called with a real write tx from `StageLoop`/`ProcessFrozenBlocks`
4. **`MiningStep`**: remove `MemoryBatch`, pass `kv.TemporalTx` (from `BeginTemporalRo`) directly to `Sync.Run`
Existing callers of `Sync.Run` that pass a `kv.TemporalRwTx` continue to work unchanged since `TemporalRwTx` satisfies `TemporalTx`.
## Scope
~84 occurrences of `kv.TemporalRwTx` in stage signatures across `execution/`. Stages that need write access would cast internally. A good first step would be to identify and enumerate which stages actually write to `tx` vs which only read, to minimise unsafe casts.
## Related
- PR #19821 — immediate fix (BeginTemporalRw → BeginTemporalRo, MemoryBatch retained as shim)
Contributor guide
Assessment
This issue has not been assessed yet.