erigontech / erigontech/erigon

commitment: parallel-trie worker contexts read the current head, not the caller's snapshot

Open
#22,209 1 comment 0 reactions 2 assignees Claimed by @awskii View on GitHub
ErigonDB
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.