erigontech / erigontech/erigon

Better integration tests for unwind issues

Open
#21,860 1 comment 0 reactions 1 assignee Claimed by @yperbasis View on GitHub
QA
Dominant language
Go
Stars
3.6k
Forks
1.5k
Avg merge
1d 16h
Merged PRs (30d)
455

Description

Recently we've been plagued with unwind/re-org issues (e.g. #20169, #20995, #21515, #21681, #22399), so we clearly need to improve our integration tests to catch them earlier. In particular, we need to tests re-orgs with pruning interfering (see #21824).

---

## Status (updated 2026-07-07)

Every root-cause fix behind the incidents above was revert-tested against the integration suite to find which failure classes the tests actually catch. Coverage landed in two PRs:

- #21973 (merged) — the StateChurn suite: unwind/redo, side-chain reorg, unwind across domain-step boundaries with snapshot files; plus red-on-revert stage/unit regression tests for the #21515 diffset fallback and the #21681/#21824 overlay prune.
- #22300 (in review) — restart-after-unwind durability, fork bounces, **re-orgs with pruning interfering** (the scenario requested above), reorg across contract creation at the same address (the #20995 EIP-684 collision shape), unwind to the snapshot-file boundary, over-deep unwind rejection, and devp2p-path bad-block rejection/recovery at the exec-module level. Revert-verified: the suite goes red under reverts of the #21981, #20483 (read side) and #20710 fixes, and when StateCache unwind/delete invalidation is disabled.

### Bugs surfaced by writing these tests

Each comes with a ready-made red test described in the issue:

- [ ] #22298 — after a rejected bad-tip FCU, forkchoice to the already-canonical valid ancestor re-executes the rejected block
- [ ] #22299 — txpool pending nonce stays durably stale after fork-bounce reorgs (after the fix, remove the nonce-pinning workaround in `churnAndAssert` so the suite guards this)
- [ ] #22301 — a too-deep FCU passes the changeset-based unwind gate, partially unwinds, fails at `SeekCommitment` and wedges block production

### Remaining test work

- [ ] **Crash-window harness.** The FCU transaction (canonical markers, TxNums, stage progress) and the domain-state commit land in different MDBX transactions, and background commit/prune is the production default — a process kill between the two commits is the last untested producer of #21515-style index/state divergence. Needs a fault-injection knob or a subprocess-based tester that SIGKILLs mid-churn and verifies recovery on restart; naturally extends to kills during unwind, prune and file build/merge. (Clean restart is covered by #22300.)
- [ ] **dbg-mode matrix for the churn suite in CI** — run it with batch commitments engaged during multi-block catch-up, with streaming commitment, and serial vs parallel exec. Makes the #21088 hash-aware-changeset-lookup class reachable; CI configuration only, no new test code.
- [ ] **Randomized reorg fuzzing** (nightly QA job): random schedules of extend / reorg / bounce / restart / prune / invalid-block-inject against the StateChurn `trackedSum` per-block oracle — the hand-written scenarios keep missing exactly the interleavings a generator would enumerate.
- [ ] **Kurtosis-level reorg/bounce scenarios** with real CLs — the fork/builder/cache interleavings historically surface on devnets (e.g. #22152), not in in-process testers.

### Production hardening

For the classes that revert-testing proved *unreachable from outside* on current main (the cleared-markers ordering has no producer — the FCU unwinds before rewriting markers in one atomic tx; the no-op-unwind overlay-prune branch is shielded by FCU-tx rollback plus a fresh SharedDomains per FCU), the stage-level tests remain the guards. To keep future refactors honest:

- [ ] Make `unwindExec3` fail loudly instead of silently no-op'ing when execution progress says blocks were executed but no diffsets are found for any of them — permanently converts the #21515 silent-corruption mode into an error.

### Smaller items

- [ ] Tighten `TestDomain_DeletedKeyNotResurrectedByFiles`: it does not go red on the `getLatestFromDb` tombstone-as-miss revert it is named for (the invariant is actually pinned by `TestDomain_UnwindRestoresDeletionMarker` and the multi-step unwind test).
- [ ] Optional: a reverting-CREATE clause in the StateChurn contract to bring the journal code-size revert class (#20039, EXTCODESIZE after a reverted CREATE) in-house; currently guarded only by the EEST suite in CI.

### Known, accepted limitation

The #20483 write side (an unwind losing deletion tombstones) is not observable at any integration-test scale: unmerged snapshot-file layers retain deletion markers, so the loss self-heals (verified by instrumentation — hundreds of skipped tombstone restores with no observable effect, even with file freezing forced). The production resurrection required mainnet-scale compaction. `TestDomain_UnwindRestoresDeletionMarker` remains its guard.

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.