erigontech / erigontech/erigon
commitment: parallel-trie worker contexts read the current head, not the caller's snapshot
- Dominant language
- Go
- Stars
- 3.6k
- Forks
- 1.5k
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 455
Description
Under `--experimental.parallel-commitment` / `--experimental.streaming-commitment` (both select `ParallelPatriciaHashed`), `ComputeCommitment` installs `concurrentTrieContextFactory`, whose per-worker trie contexts open `db.BeginTemporalRo(ctx)` fresh at fold time (`execution/commitment/commitmentdb/commitment_context.go`, `concurrentTrieContextFactory`) — a new MDBX read view at the then-current committed head, with no pinning to the caller's snapshot. When no custom state reader is set, workers bind their reader to that fresh tx (`NewLatestStateReaderForWorker`) and their read chain bottoms out in `tx.GetLatest` on it.
This is coherent for stage exec, which folds under the exec-module semaphore (head cannot advance mid-fold). It is incoherent for any fold running outside the semaphore: a payload build pinned at head N whose fold overlaps an FCU commit of N+1 gets N+1 values for untouched rows while `sd.mem` holds N-based writes — a mixed-snapshot state root; peers reject the built block as INVALID.
For payload builds the interim mitigation (being added on #22198's branch) is pinning the builder's SharedDomains to the sequential trie via `WithSequentialCommitment()`, mirroring every other one-shot/RPC SD — at the cost of parallel folding for builds under the experimental flags. This issue tracks the proper fix: worker trie contexts should read through a view pinned to the caller's snapshot (per-worker read views at a fixed txnum, or a thread-safe reader over the pinned tx, the way `CloneForWorker` preserves `srcTx` on the `stateReader != nil` path), after which builds can re-enable the parallel variant.
No deterministic regression test exists for the mixed-snapshot race — it needs a hook to park a fold mid-flight while an FCU commits; worth adding alongside the pinning fix.
Found during the review of #22198 (the #22152 fix); the mechanism was verified by code-trace on `main`.
Contributor guide
Assessment
This issue has not been assessed yet.