Skip to content

execution/state: finalize reader must consume Estimate cells, not just Done#21678

Merged
mh0lt merged 2 commits into
mh/parallel-exec-flush-invalid-as-estimatefrom
yperbasis/finalize-estimate-read
Jun 8, 2026
Merged

execution/state: finalize reader must consume Estimate cells, not just Done#21678
mh0lt merged 2 commits into
mh/parallel-exec-flush-invalid-as-estimatefrom
yperbasis/finalize-estimate-read

Conversation

@yperbasis

Copy link
Copy Markdown
Member

Summary

Root-cause fix for the nondeterministic wrong-state-root regression on this branch, generalizing the Estimate-read handling that d7f7a40c5d patched for one case.

versionedUpdate (the versionedStateReader used for finalize / system-tx reconstruction) only consumed a versionMap floor flagged Done, treating an Estimate (MVReadResultDependency) floor as "not found" and falling back to the pre-block DB value. Since 221007e6a1 flushes invalidated txs' writes as Estimate, 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.

d7f7a40c5d fixed one manifestation (the coinbase EIP-161 SD check in calcFees) 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

  1. finalize reader must consume Estimate cellsversionedUpdate now consumes a floor that is Done or Estimate. 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.
  2. Revert the calcFees coinbase probe (d7f7a40c5d) — now redundant: vsReader returns the coinbase's in-flight value, so coinbaseEmptyPre is computed from the real balance rather than a spurious nil.

Verification

The net change is versionedUpdate only (the revert cancels d7f7a40c5d), so the branch is code-identical to "this branch before the coinbase probe, plus the versionedUpdate fix" — the configuration that was load-tested:

  • eip7685 EL-requests, 16-worker load, interleaved on one machine: unfixed branch ~4/30 (blocktest) and 9/25 (engine-x) → 0 with this fix.
  • Full prague (2,490 tests) × 4 passes: 0 failures, 0 regressions vs the serial baseline.
  • Full-prague sensitivity (5 passes): unfixed 1/5 → 0/5.
  • execution/stagedsync + execution/state unit tests green (incl. this branch's phantom-write guard); go vet + make lint clean.

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-estimate so it lands before #21667 merges.

yperbasis added 2 commits June 8, 2026 13:24
…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.
@yperbasis yperbasis requested a review from mh0lt as a code owner June 8, 2026 11:26
@mh0lt mh0lt merged commit 89b2f5d into mh/parallel-exec-flush-invalid-as-estimate Jun 8, 2026
@mh0lt mh0lt deleted the yperbasis/finalize-estimate-read branch June 8, 2026 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants