prefetcher: builder-phase prefetch + streaming worker pool #2192
Claude / Claude Code Review
completed
Apr 20, 2026 in 35m 9s
Code review found 2 important issues
Found 4 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 2 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | miner/worker.go:2586-2599 |
Within-iteration plan-overflow duplicate: same tx in batch and bonus |
| 🔴 Important | miner/worker.go:1936-1938 |
scanOverflow h.Pop() breaks multi-pass freed-gas design |
| 🟡 Nit | core/state_prefetcher.go:126-140 |
PrefetchStream worker panic recovery omits fails.Add(1) |
Annotations
Check failure on line 2599 in miner/worker.go
claude / Claude Code Review
Within-iteration plan-overflow duplicate: same tx in batch and bonus
In `runBuilderTxProvider`, a tx T can appear in both `batch` (from `collectPlanBatch`) and `bonus` (from `scanOverflow`) within the same 2ms loop iteration, causing `forwardTxs` to send T twice and dispatch two workers to execute identical EVM paths on separate throwaway state copies. The `sentThisPhase` fix added in commit 88c4e44 only guards against cross-iteration re-emission (T forwarded in iteration N, re-emitted in N+1); it provides no protection within iteration N itself because `sentThis
Check failure on line 1938 in miner/worker.go
claude / Claude Code Review
scanOverflow h.Pop() breaks multi-pass freed-gas design
In scanOverflow, when an account's head tx has gas > remaining, h.Pop() permanently removes that account from the overflowHeap. Because overflowHeap is constructed once in runBuilderTxProvider and scanOverflow is called on it in every 2ms loop iteration with a growing extendedBudget, accounts popped in early iterations (when budget is small) are gone by the time the budget grows large enough to accommodate them. This directly undermines the PR's stated design: freed-gas budget accumulates across
Check warning on line 140 in core/state_prefetcher.go
claude / Claude Code Review
PrefetchStream worker panic recovery omits fails.Add(1)
When a worker goroutine panics inside `prefetchOneTx` (e.g., in `statedb.Copy()`, `ApplyMessage`, or `IntermediateRoot`), the deferred `recover()` at lines 132–136 increments `blockPrefetchWorkerPanicMeter` but does not call `ctx.fails.Add(1)`. Since `processTx` already incremented `txIndex` before dispatching to `prefetchOneTx`, `blockPrefetchTxsValidMeter.Mark(processed - ctx.fails.Load())` at stream end overcounts valid prefetches by 1 per panicked worker. Fix: add `ctx.fails.Add(1)` inside t
Loading