Skip to content

core, blockstm: address inline PR review comments

4c688e4
Select commit
Loading
Failed to load commit list.
Open

core, blockstm, state: add BlockSTM v2 parallel transaction execution #2210

core, blockstm: address inline PR review comments
4c688e4
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed May 5, 2026 in 31m 35s

Code review found 2 important issues

Found 5 candidates, confirmed 4. See review comments for details.

Details

Severity Count
🔴 Important 2
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important core/blockchain.go:862-867 V2 silently loses stateless witness data when ProcessBlock is called with a witness
🔴 Important core/blockstm/mvstore.go:156-185 SelfDestruct skips recordWrite for SuicidePath; MarkEstimate cannot invalidate stale destruct on re-execution
🟡 Nit core/blockchain.go:753-774 V2 reader cache hit/miss stats are silently dropped

Annotations

Check failure on line 867 in core/blockchain.go

See this annotation in the file changed.

@claude claude / Claude Code Review

V2 silently loses stateless witness data when ProcessBlock is called with a witness

V2 silently drops the stateless-witness pointer when ProcessBlock is called with a non-nil witness. blockchain.go:864 wires the witness via `parallelStatedb.StartPrefetcher("chain", witness, nil)`, but inside V2 `parallel_state_processor.go:1034-1035` immediately calls `finalDB.StopPrefetcher(); finalDB.StartPrefetcher("v2-settle", nil, nil)`, and `StateDB.StartPrefetcher` unconditionally executes `s.witness = witness` — overwriting the pointer to nil for the rest of execution. Every `s.witness 

Check failure on line 185 in core/blockstm/mvstore.go

See this annotation in the file changed.

@claude claude / Claude Code Review

SelfDestruct skips recordWrite for SuicidePath; MarkEstimate cannot invalidate stale destruct on re-execution

**`SelfDestruct` skips `recordWrite` for the SuicidePath key, breaking re-execution invalidation.** `ParallelStateDB.SelfDestruct` (parallel_statedb.go:980-992) flips `s.destructed[addr]` and journals `jkDestruct`, then `FlushToMVStore` (line 428) writes `SuicidePath_<addr>` to MVStore — but the key is never added to `s.WriteKeys`, so `MVStore.MarkEstimate` / `CleanupEstimate` (mvstore.go:156-185) cannot reach it on re-execution. Every other MVStore-targeting writer (SetNonce/SetCode/SetState/Cr

Check warning on line 774 in core/blockchain.go

See this annotation in the file changed.

@claude claude / Claude Code Review

V2 reader cache hit/miss stats are silently dropped

V2 (parallel) reader cache hit/miss stats are silently dropped. `setupBlockReaders` calls `ReadersWithCacheStatsTriple` to create three independent `ReaderWithStats` wrappers (prefetch, process, parallel) and wires the parallel one into `parallelStatedb` (core/blockchain.go:744), but `reportReaderStats` (line 753) only reads stats from prefetch and process — the parallel reader's atomic counters are accumulated and discarded each block. Pure metrics regression (no correctness impact); fix is to