execution/state: finalize reader must consume Estimate cells, not just Done#21678
Merged
mh0lt merged 2 commits intoJun 8, 2026
Merged
Conversation
…t Done versionedUpdate, used by versionedStateReader for finalize / system-tx reconstruction, only returned a versionMap value when the floor cell was Done, falling back to the pre-block DB value (recording no dependency) on an Estimate (MVReadResultDependency) floor. Since 221007e flushes invalidated txs' writes as Estimate, the finalize path resurrects stale pre-block state for any in-flight prior write, committing a wrong write set and producing nondeterministic wrong state roots in EL-requests blocks under parallel execution. Consume Estimate floors too: they hold the same latest in-block write a Done floor does, and finalize reads are re-validated (the tx re-executes) if the writer's value later changes. The worker read-path, which aborts on an Estimate read, is unchanged, so the phantom-write fix is preserved.
…D on Estimate read" This reverts commit d7f7a40. That commit worked around a single instance of the finalize reader's Done-only defect — the coinbase EIP-161 empty-removal check in calcFees — by probing the versionMap for an Estimate floor and suppressing the SD-emit. The previous commit fixes the defect at its source in versionedUpdate, so vsReader now returns the coinbase's in-flight value and coinbaseEmptyPre is computed from the real balance rather than a spurious nil. The targeted probe is therefore redundant.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root-cause fix for the nondeterministic wrong-state-root regression on this branch, generalizing the Estimate-read handling that
d7f7a40c5dpatched for one case.versionedUpdate(theversionedStateReaderused for finalize / system-tx reconstruction) only consumed a versionMap floor flaggedDone, treating anEstimate(MVReadResultDependency) floor as "not found" and falling back to the pre-block DB value. Since221007e6a1flushes invalidated txs' writes asEstimate, the finalize path resurrects stale pre-block state whenever a prior in-flight write exists — committing a wrong write set, producing nondeterministic wrong state roots in EL-requests blocks under parallel execution.d7f7a40c5dfixed one manifestation (the coinbase EIP-161 SD check incalcFees) with a targeted probe; that commit's own message names the general defect ("versionedStateReader only honours Done floors and treats MVReadResultDependency … as not found"). This PR fixes it at the source, so every finalize-path read (storage, all account fields, code) is covered.Commits
versionedUpdatenow consumes a floor that isDoneorEstimate. An Estimate floor holds the same latest in-block write a Done floor does; finalize reads are re-validated (the tx re-executes) if the writer's value later changes — the same OCC safety net the rest of the apply loop relies on. The worker read-path, which aborts on an Estimate read, is unchanged, so the phantom-write fix this branch targets is preserved.calcFeescoinbase probe (d7f7a40c5d) — now redundant:vsReaderreturns the coinbase's in-flight value, socoinbaseEmptyPreis computed from the real balance rather than a spurious nil.Verification
The net change is
versionedUpdateonly (the revert cancelsd7f7a40c5d), so the branch is code-identical to "this branch before the coinbase probe, plus the versionedUpdate fix" — the configuration that was load-tested:execution/stagedsync+execution/stateunit tests green (incl. this branch's phantom-write guard);go vet+make lintclean.Single-test runs pass; the bug only surfaces under load (CPU contention perturbs exec3 scheduling), which is why CI caught it but a one-off run does not.
Targets
mh/parallel-exec-flush-invalid-as-estimateso it lands before #21667 merges.