feat(l1): overlay + read cascade for deep reorgs - #6687
Conversation
|
Lines of code reportTotal lines added: Detailed view |
f24e035 to
5f49d31
Compare
88abf41 to
47c9c25
Compare
Adds a per-block reverse-diff journal in a new STATE_HISTORY column family. Each non-batch trie commit captures pre-images for every overwritten trie node and flat-KV entry, then stages a JournalEntry in the same write batch as the trie writes (atomic on commit). Finality pruning issues a single delete_range when finalized advances. No reorg consumer yet; this PR is the audit-trail foundation for the deep-reorg work in #6685. Skipped entirely during full sync (batch_mode).
Per review: - Document the version-bump strategy on JOURNAL_VERSION so PRs 2/3/4 have a policy to follow: drain at finality on bump, or grow per-version decode arms if history needs to survive the upgrade. - Add debug_assert!(layers_to_commit.len() == 1) in TrieLayerCache::commit to runtime-enforce the single-layer invariant the ATTRIBUTION NOTE describes; catches any future change that quietly merges multiple blocks into one journal entry.
f01b356 to
21d8e48
Compare
47c9c25 to
50900ec
Compare
Builds on the journal (#6686) and overlay (#6687) to drive deep-reorg applies through the engine API. Issue #6685. Storage primitives in crates/storage/store.rs: - install_overlay_for_reorg: builds an Overlay from STATE_HISTORY[D..=T] and atomically swaps the layer cache for a fresh one with the overlay installed; failure leaves the existing cache intact. - highest_state_history_block_number, is_state_in_layer_cache, clear_reorg_overlay, abort_reorg. - Section 9 reconciliation inside apply_trie_updates: on the first new-chain commit, fold overlay-only entries into the write batch, stage delete_range(STATE_HISTORY, T..=D) before the new T journal entry, and clear the overlay on success. Disk advances D-old to T-new in a single atomic write. Blockchain (crates/blockchain/blockchain.rs): - reorg_in_progress AtomicBool + ReorgGuard RAII; enter_reorg and is_reorg_in_progress; store() accessor for the orchestrator. Fork choice (crates/blockchain/fork_choice.rs): - apply_fork_choice_with_deep_reorg wraps apply_fork_choice. The 128 REORG_DEPTH_LIMIT cap is left in place; PR 4 lifts it. Only the StateNotReachable case is routed to reorg_apply_deep, so mainline behavior is unchanged. - reorg_apply_deep walks to the pivot, installs the overlay, replays side-chain blocks AND head via Blockchain::add_block, then runs forkchoice_update. AbortReorgGuard resets the cache on any failure path or panic; disarmed on success. Engine API: - engine/fork_choice.rs handler switched to the new wrapper. - engine/payload.rs try_execute_payload: when the parent block is known but its state is materialized in neither the layer cache nor disk, stash the block and return ACCEPTED; matches geth's catalyst path. A later forkchoiceUpdated drives the deep-reorg apply. - New PayloadStatus::accepted() constructor. Tests: - 4 storage unit tests covering the new primitives (install/abort/clear, highest_state_history_block_number). - 2 ReorgGuard tests covering RAII semantics including panic unwinding. Workspace builds clean; cargo clippy --workspace -- -D warnings clean; cargo fmt --check clean. The tooling/reorgs deep-reorg scenario is deferred to PR 4, where the 128-block cap is lifted and a CLI knob for commit_threshold makes the end-to-end path naturally reachable in the simulator. Closes part of #6685. Stacked on #6687.
…urnal # Conflicts: # crates/storage/api/mod.rs # crates/storage/backend/in_memory.rs # crates/storage/backend/rocksdb.rs # crates/storage/store.rs
…urnal # Conflicts: # crates/storage/store.rs
… too_many_arguments
Builds on the journal added in #6686. Introduces: - Overlay struct: in-memory aggregated reverse-diff bridging on-disk state at the cache edge D to the virtual state at a deep-reorg pivot T-1. Built by walking STATE_HISTORY[D..=T] descending, with older-entry-wins semantics so the post-walk value at every key is the pre-image at T-1. - OverlayCf enum + classify_by_key_length, reusing PR 1's classify_trie_key. - TrieLayerCache: optional overlay field plus set/clear/lookup accessors. - TrieWrapper::get cascade now reads: layer cache -> overlay -> disk. No orchestration wired yet; PR 3 will install/clear overlays via apply_fork_choice_with_deep_reorg. 14 unit tests covering descending-range load, older-wins, bloom miss, hash mismatch, missing entry, swapped-args rejection, CF classification, cascade short-circuit, and edge cases.
50900ec to
413f93c
Compare
Builds on the journal (#6686) and overlay (#6687) to drive deep-reorg applies through the engine API. Issue #6685. Storage primitives in crates/storage/store.rs: - install_overlay_for_reorg: builds an Overlay from STATE_HISTORY[D..=T] and atomically swaps the layer cache for a fresh one with the overlay installed; failure leaves the existing cache intact. - highest_state_history_block_number, is_state_in_layer_cache, clear_reorg_overlay, abort_reorg. - Section 9 reconciliation inside apply_trie_updates: on the first new-chain commit, fold overlay-only entries into the write batch, stage delete_range(STATE_HISTORY, T..=D) before the new T journal entry, and clear the overlay on success. Disk advances D-old to T-new in a single atomic write. Blockchain (crates/blockchain/blockchain.rs): - reorg_in_progress AtomicBool + ReorgGuard RAII; enter_reorg and is_reorg_in_progress; store() accessor for the orchestrator. Fork choice (crates/blockchain/fork_choice.rs): - apply_fork_choice_with_deep_reorg wraps apply_fork_choice. The 128 REORG_DEPTH_LIMIT cap is left in place; PR 4 lifts it. Only the StateNotReachable case is routed to reorg_apply_deep, so mainline behavior is unchanged. - reorg_apply_deep walks to the pivot, installs the overlay, replays side-chain blocks AND head via Blockchain::add_block, then runs forkchoice_update. AbortReorgGuard resets the cache on any failure path or panic; disarmed on success. Engine API: - engine/fork_choice.rs handler switched to the new wrapper. - engine/payload.rs try_execute_payload: when the parent block is known but its state is materialized in neither the layer cache nor disk, stash the block and return ACCEPTED; matches geth's catalyst path. A later forkchoiceUpdated drives the deep-reorg apply. - New PayloadStatus::accepted() constructor. Tests: - 4 storage unit tests covering the new primitives (install/abort/clear, highest_state_history_block_number). - 2 ReorgGuard tests covering RAII semantics including panic unwinding. Workspace builds clean; cargo clippy --workspace -- -D warnings clean; cargo fmt --check clean. The tooling/reorgs deep-reorg scenario is deferred to PR 4, where the 128-block cap is lifted and a CLI knob for commit_threshold makes the end-to-end path naturally reachable in the simulator. Closes part of #6685. Stacked on #6687.
🤖 Kimi Code ReviewThis is a well-designed implementation of a reverse-diff overlay for deep-reorg handling. The three-state lookup semantics ( Minor suggestions:
Correctness verification:
Testing: No critical issues found. The Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
Greptile SummaryThis PR adds the in-memory overlay used by deep-reorg state reads. The main changes are:
Confidence Score: 4/5The overlay read cascade needs a root-scoping fix before merging.
crates/storage/layering.rs
|
| Filename | Overview |
|---|---|
| crates/storage/layering.rs | Adds overlay construction, lookup, cache accessors, and the trie read cascade; the cascade should scope overlay reads to the reorg roots it virtualizes. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[TrieWrapper::get] --> B[Apply storage prefix when present]
B --> C{Layer cache hit for state_root and key?}
C -- yes --> D[Return layer value]
C -- no --> E{Overlay has key?}
E -- yes --> F[Return overlay value or missing]
E -- no --> G[Read disk]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[TrieWrapper::get] --> B[Apply storage prefix when present]
B --> C{Layer cache hit for state_root and key?}
C -- yes --> D[Return layer value]
C -- no --> E{Overlay has key?}
E -- yes --> F[Return overlay value or missing]
E -- no --> G[Read disk]
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
crates/storage/layering.rs:458-460
**Overlay Applies Across Roots**
When an overlay is installed on the shared cache, any `TrieWrapper` that misses the layer chain consults it before disk, even if its `state_root` is not part of the deep-reorg execution. A key present in the overlay as an older value or `None` can make unrelated root reads return the pivot state instead of the on-disk canonical state.
Reviews (1): Last reviewed commit: "feat(l1): add overlay + read cascade for..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if let Some(overlay_result) = self.inner.lookup_overlay(key.as_ref()) { | ||
| return Ok(overlay_result); | ||
| } |
There was a problem hiding this comment.
When an overlay is installed on the shared cache, any TrieWrapper that misses the layer chain consults it before disk, even if its state_root is not part of the deep-reorg execution. A key present in the overlay as an older value or None can make unrelated root reads return the pivot state instead of the on-disk canonical state.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/storage/layering.rs
Line: 458-460
Comment:
**Overlay Applies Across Roots**
When an overlay is installed on the shared cache, any `TrieWrapper` that misses the layer chain consults it before disk, even if its `state_root` is not part of the deep-reorg execution. A key present in the overlay as an older value or `None` can make unrelated root reads return the pivot state instead of the on-disk canonical state.
How can I resolve this? If you propose a fix, please make it concise.
🤖 Codex Code Review
Aside from those points, the read-cascade ordering and the journal hash verification are sensible. I couldn’t run the Rust tests here because the environment blocks Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
🤖 Claude Code ReviewReview:
|
… feat/state-history-overlay # Conflicts: # crates/storage/layering.rs
The state-history work makes apply_fork_choice reach the deep-reorg path (journal prune + PersistMessage::Commit backlog flush), inflating the forkchoiceUpdated futures. Since map_engine_requests awaits each match arm inline, its future is sized to the largest arm and shared by every engine request, so an unboxed forkchoiceUpdated overflowed the 2MB tokio worker stack in debug builds (Reorg Tests). Box these arms like the newPayload arms.
… feat/state-history-overlay
…urnal # Conflicts: # crates/networking/rpc/rpc.rs
|
Audit of this PR (effective diff High — layerless safe-commit root now hard-errors instead of no-op'ing (regression of a fixed case; regression tests deleted)
Before this PR, branch (c) of // (c) ... Still requires a layer: a canonical root need not have one
// (`put_batch` skips blocks whose state root equals their parent's, which on L2 is every empty block).
if parent_state_root == safe_root {
return self.has_layer(safe_root).then_some(safe_root);
}and
The invariant the new error message claims (" Medium — the new early return strands the flat-KV generator (Stop without Continue)
Low — stale
|
…erlay # Conflicts: # crates/storage/layering.rs # crates/storage/store.rs
> Part of the deep-reorg support effort tracked in #6685. This is PR 3 of 4, stacked on #6687. Wires up the orchestration layer; the deep-reorg path is scaffolded but dormant until PR 4 lifts the cap. ## Summary - Adds `install_overlay_for_reorg`, `abort_reorg`, `clear_reorg_overlay`, `is_state_in_layer_cache`, and `highest_state_history_block_number` to `store.rs`. - Section 9 reconciliation in `apply_trie_updates`: on the first new-chain commit after a deep reorg, overlay-only entries are folded into the write batch, the old `STATE_HISTORY[T..=D]` range is deleted, and the overlay is cleared. Disk advances from the OLD chain's edge directly to the new tip in one atomic write. - `reorg_in_progress: AtomicBool` + `ReorgGuard` RAII in `Blockchain`; concurrent FCUs short-circuit to `SYNCING` while a reorg apply pass is in flight. - `apply_fork_choice_with_deep_reorg` in `fork_choice.rs`: routes `StateNotReachable` into `reorg_apply_deep`, all other results fall through unchanged. `AbortReorgGuard` resets the cache on any failure or panic. - Engine API `fork_choice.rs` handler switched to the new wrapper (takes `&context.blockchain`). `try_execute_payload` stashes blocks and returns `ACCEPTED` when the parent is known but its state is not materialized; a later `forkchoiceUpdated` drives the apply. - Removes stale `#[allow(dead_code)]` markers from layering.rs now that the overlay accessors are consumed. - 4 storage unit tests for the new primitives; 2 `ReorgGuard` tests including panic-unwind. ## What's in this PR - `crates/storage/store.rs`: `install_overlay_for_reorg`, `abort_reorg`, `clear_reorg_overlay`, `is_state_in_layer_cache`, `highest_state_history_block_number`, Section 9 reconciliation inside `apply_trie_updates`. 4 new tests. - `crates/blockchain/blockchain.rs`: `reorg_in_progress: AtomicBool`, `ReorgGuard`, `enter_reorg`, `is_reorg_in_progress`, `store()` accessor. 2 new tests. - `crates/blockchain/fork_choice.rs`: `apply_fork_choice_with_deep_reorg`, `reorg_apply_deep`, `AbortReorgGuard`, `map_chain_error_for_fcu`. Head is included in the replay iterator via `chain(once((head.number, head_hash)))`, fixing the missing-head bug from #6561. - `crates/networking/rpc/engine/fork_choice.rs`: switched to `apply_fork_choice_with_deep_reorg`. - `crates/networking/rpc/engine/payload.rs`: ACCEPTED stash path for unmaterialized parent state. - `crates/networking/rpc/types/payload.rs`: `PayloadStatus::accepted()` constructor. - `crates/storage/layering.rs`: drop stale `#[allow(dead_code)]` attrs; `len`/`is_empty` keep theirs (PR 4 target). ## What's NOT in this PR - The 128-block `REORG_DEPTH_LIMIT` cap is not lifted; the deep-reorg path is unreachable on mainline until PR 4. Behavior for end users is unchanged after this merge. - `tooling/reorgs` deep-reorg simulator scenario: deferred to PR 4 (requires a CLI knob or cap removal to be end-to-end reachable). - `map_chain_error_for_fcu` collapses all `ChainError` variants to `StateNotReachable`; a richer mapping with `InvalidAncestor` + last-valid-hash is follow-up. - Crash-during-reconciliation injection test (open question in #6685): deferred. - Historical state RPC queries: not in #6685's scope. ## Test plan - `cargo build --workspace` clean. - `cargo clippy --workspace --no-deps -- -D warnings` clean. - `cargo fmt --check` clean. - `cargo test -p ethrex-storage` (45/45 pass, includes 4 new deep-reorg primitive tests). - `cargo test -p ethrex-blockchain` (2/2 new `ReorgGuard` tests pass, including panic-unwind). ## References - Tracking issue: #6685 - PR 1 (state-history journal): #6686 - PR 2 (overlay + read cascade): #6687
> Part 4 of 4 in the deep-reorg support effort tracked in #6685. Stacks on > #6689 (orchestration). Removes the hard 128-block cap, adds Prometheus > observability, and ships a Kurtosis-driven integration test harness. ## Summary - Replaces `REORG_DEPTH_LIMIT = 128` with `compute_reorg_ceiling`, a **physical** limit: the ceiling is the deeper of the in-memory layer-cache retention and the reverse-diff journal reach (`latest - (lowest_state_history_block - 1)`), capped at `latest` and by the operator override. Finality is deliberately **not** an input, matching geth/nethermind/reth: `-38006 TooDeepReorg` means "deeper than the client can reconstruct," not "crosses the finalized block." Finality is still enforced by the spec's no-reorg-past-finalized skip and the safe/finalized connectivity checks. - `--max-reorg-depth` CLI flag (env: `ETHREX_MAX_REORG_DEPTH`), default `None`. Plumbed through L1 + L2 init. - O(1) `Store::highest/lowest_state_history_block_number` via new `StorageReadView::first_key`/`last_key` primitives. - 8 Prometheus metrics under `ethrex_reorg_*` / `ethrex_deep_reorg_*`, registered with the scrape registry: 3 gauges (overlay entries/bytes, journal length), 2 histograms (depth, reconcile duration), 3 counters (attempts/success/aborts). - Kurtosis enclave config + assertoor scenario + manual GitHub workflow for end-to-end network-partition driven deep-reorg testing. ## What changed where | Phase | Commit | Scope | |---|---|---| | 1 | `bb7d3e5a7e`, `adb57933c2` | O(1) state-history edge lookup | | 2 | `dcd4741dd5`, `40b4f54750`, `c694beacb` | Lift the cap, physical (layer-cache + journal reach) | | 3 | `61c8722503`, `d0e5692b07` | Prometheus metrics + registry wiring | | 4 | `ef26cac986` | Rustdoc surface, operator docs | | 5 | `031461cf63` | Kurtosis + simulator harness | | 6 | tbd | Salt sibling chains in the simulator so chain B truly diverges from chain A | ## Simulator test calibration The first version of `deep_reorg_beyond_128` produced byte-identical chains because `Chain::get_next_payload_attributes` derives `prev_randao` and `parent_beacon_block_root` deterministically from the parent hash. Both `base_chain.fork()` calls yielded the same blocks, so chain B never actually diverged ; the FCU to "chain B" was a no-op reorg back to chain A's tip, and the EL got stuck in cascading deep-reorgs because `engine_newPayload` short-circuited on `HEADERS` already containing each block. Fix: `Chain` now carries a `salt: u8` mixed into the payload-attributes seed, set on chain B via `base_chain.fork().with_salt(1)`. Existing single-line tests (salt stays at 0) are byte-compatible. Doc comment on the field spells out the trap so the next person doesn't reintroduce it. With the salt, exactly one deep_reorg fires (chain B's first FCU back to genesis), then chain B extends to 200 blocks via the normal shallow path. Test passes in ~1.8 s. ## Simulator scenarios Two deep-reorg scenarios ship in `tooling/reorgs`: - **`deep_reorg_beyond_128`** ; single node. Builds chain A canonical to 150, then FCUs to a salted chain B that forks at genesis. The first FCU is a 150-deep reorg, well past the legacy 128 cap. Asserts the reorg is accepted and chain B advances to 200. - **`deep_reorg_side_chain_replay`** ; two nodes, exercises the `ACCEPTED` stash path end to end. Node 0 builds a 100-block common prefix; node 1 replays it, then builds a salted chain B to 300. Node 0 extends chain A to 300 (evicting the common ancestor's state from cache), then ingests chain B's 200-block divergent suffix via `newPayload` only (each block stashes as `ACCEPTED`, no FCU). A final FCU to chain B's tip drives one `reorg_apply_deep` with `side_chain_len = 200`. ## Bug 4 closeout A "stack overflow in engine_newPayload on overlay-backed state" was tracked as an open issue from the prior handoff. Root cause: LEVM's `Stack::default()` materializes a 32 KB `[U256; 1024]` temp on the stack before `Box::new` moves it to the heap. In debug builds (which the simulator was using) this consumes ~32 KB per call; combined with the deep-reorg async chain it tipped the 2 MB tokio worker stack. Release builds placement-new directly into the heap, so production is unaffected. Documented as a debug-build artifact ; no EL code change in this PR. ## What's not in this PR - Removing the journal entirely (full reorg-to-finality is now physically possible; pruning at finality still applies). - Per-batch journaling in full-sync mode (`batch_mode == true` continues to skip the journal). - Hardening `engine_newPayload`'s "header already known" short-circuit to verify the state is materialized before replying VALID. Only surfaces when the CL re-sends a known-hash block after a cache-wiping reorg ; deferred to a follow-up. ## EELS consume-enginex The `consume-enginex` sim reuses one client across tests and resets it to genesis before each, so a test that leaves a deep chain turns the next reset into a deep canonical rewind. That is the case #6985 targets. Ran the minimal failing pair against a release build of this branch (hive master `9207a978`, pinned `tests@v20.0.0` fixtures). `256_empty_blocks` leaves the client at block 257, then `INVALID-debug` resets to genesis, a 257-deep rewind: ``` ./hive --client-file ../fixtures/hive/clients.yaml --client ethrex \ --sim ethereum/eels/consume-enginex --sim.buildarg fixtures=tests@v20.0.0 \ --sim.limit '.*fork_Osaka-blockchain_test_engine_x-(256_empty_blocks|test_program_program_INVALID-debug).*' ``` Result: 2/2 pass at default settings (no `ETHREX_MAX_REORG_DEPTH`), and 2/2 with `=512`. The reset goes through `reorg_apply_deep` instead of being rejected with `-38006`: the physical ceiling (journal reach) covers a full-height rewind (a fixed 128 cap does not), and the overlay replay rebuilds the evicted state. Sim and fixtures are upstream on hive master, no custom fork. Only the minimal pair was run, not the full suite, and this is not wired to CI here. ## Test plan - [x] `cargo test -p ethrex-blockchain -p ethrex-test -p ethrex-storage --features rocksdb` ; 465 pass. - [x] `cargo clippy -p ethrex-blockchain -p ethrex-storage -p ethrex-metrics --all-targets --no-deps -- -D warnings`. - [x] In-process simulator against a **release** ethrex binary (run from `tooling/reorgs`, `reorgs` is not a workspace member so `-p reorgs` does not apply): - `cargo run -- deep_reorg_beyond_128 /abs/path/target/release/ethrex` ; one deep_reorg fires, chain B advances to 200, exit 0 in ~1.8 s. - `cargo run -- deep_reorg_side_chain_replay /abs/path/target/release/ethrex` ; ACCEPTED-stash path then one `reorg_apply_deep` (side_chain_len 200), exit 0 in ~3.5 s. - [x] `cargo doc -p ethrex-storage -p ethrex-blockchain --no-deps` ; zero `missing documentation` on new public surface. - [x] EELS `consume-enginex` deep-reorg pair passes on hive master + `tests@v20.0.0`, at default settings and with `ETHREX_MAX_REORG_DEPTH=512` (see section above). - [ ] Kurtosis enclave run (manual; see `tooling/reorgs/disruptoor/README.md`). - [ ] CI green. Closes #6685 alongside the prior three PRs. --------- Co-authored-by: ilitteri <ivanlitteri@hotmail.com>
- Canonical-head deep reorgs installed the overlay serving head's PARENT state root while head was never re-executed, so head's state never became readable and every FCU retry looped. The pivot for the head-canonical case is head itself (unwind [head+1, edge]). - ethrex_reorg_depth histogram observed head - pivot (~1 in the canonical-head case, invisible exactly for the deepest unwinds); now observes latest - pivot, matching the TooDeepReorg gate's definition. - Multi-layer commits with an overlay installed tripped the reconciliation debug_assert in debug builds and journaled upper layers' pre-images against old-chain disk in release (silent corruption on a future unwind). commit_to_disk now commits only the bottom layer while an overlay is installed; the backlog drains after the first reconciliation commit clears the overlay. - abort_reorg swapped the layer cache without the persist-worker rendezvous install_overlay_for_reorg performs; a mid-flight phase1 could install a side-chain layer over a base with no overlay underneath. It now rendezvous-pings the worker the same way. - Journal pruning at finality advance raced Overlay::from_journal (no snapshot isolation) via syncer-driven FCUs that bypass the reorg mutex. enter_reorg now pauses STATE_HISTORY pruning for the duration of the apply pass; pruning catches up on the next advance. - newPayload executed blocks concurrently with the deep-reorg replay against the freshly swapped cache; it now returns SYNCING while a reorg is in progress.
…ay is installed Journal entries written while the flat-KV generator is running lack pre-images for keys past the generator frontier, and the generator's own flat-KV writes are never journaled. A deep reorg unwinding into those entries could therefore read stale flat-KV values for the chain being reorged away, silently producing wrong state (#7001). While an overlay serves the read's state root, disable every flat-KV fast path so reads go through the trie nodes, which are always journaled and complete in the overlay: - TrieWrapper::flatkeyvalue_computed returns false for served roots, so Trie::get walks trie nodes (covers execution and open_*_trie readers); - get_storage_at_root / get_storage_at_root_with_known_storage_root skip the EMPTY_TRIE_HASH shortcut, so the storage walk starts from the real root read via the state trie; - get_account_states_batch_by_root routes all addresses through the per-address trie fallback instead of the direct ACCOUNT_FLATKEYVALUE multi_get (which never consulted the overlay at all). Reads at roots the overlay does not serve are unchanged. Closes #7001 (option 2 from the issue: trie-fallback during an active overlay).
- ethrex_reorg_journal_length gauge was only updated on the deep-reorg success path (0 forever in steady state, contradicting its docs); now updated on every successful apply_fork_choice (O(1) via first/last key). - --max-reorg-depth docs claimed Some(0) restores the pre-stack behavior (a 128-block cap) and returns SYNCING; it actually rejects EVERY reorg (including routine 1-2 block ones) with -38006 TooDeepReorg. Fixed the rustdoc, the storage README, and monitoring.md, which also described the ceiling as finality-based rather than physical. - Stale test comments in smoke_tests.rs and tooling/reorgs described a finality-based ceiling; the tests pass via the physical ceiling (in-memory retention floor). Comments now match the mechanism. - forkchoice_update_inner: read the pruning-pause flag inside the 'static spawn_blocking closure via the shared Arc (fixes E0521).
|
Pushed three commits addressing the findings from the review comments above (audits of this PR, #6689, #6724, and #7001):
Verified: Not addressed (by design, needs explicit sign-off rather than code): the absence of a default bound on reorg depth/overlay memory when finality stalls (see the #6724 review comment), and the case-1→case-2 journal hash-verification skip. |
…on tests Adversarial review of the review fixes surfaced one more latent bug and adds the missing regression coverage: - Reconciliation layer matching by block number (layer.block_number == overlay.to_block) lost the bridge fold whenever the bottom committed layer sat above to_block — reachable on L2, where root-preserving blocks create no layer. The bridge entries would then be neither written to disk nor journaled before the overlay is cleared. Since the bottom-layer-only rule makes any overlay-backed commit single-layer by construction, the reconciliation layer is now identified by overlay presence instead of block number. - overlay_backed_commit_only_commits_bottom_layer_per_pass: drives a 3-layer backlog at a commit while an overlay is installed (the shape that panicked the debug_assert pre-fix) and asserts only the bottom layer commits, the overlay is consumed, the bridge keys land in T's journal entry, and the backlog drains correctly on the next pass. - journal_pruning_pauses_during_reorg_and_catches_up_after: asserts a finality advance prunes nothing while the pause flag is held and prunes cumulatively once released.
|
Adversarial round on the review fixes (
Suites: storage lib 87, blockchain lib 28, smoke_tests 12, clippy clean. |
Code comments referenced the tracking issue (#6685), the stack PRs (#6686/#6687/#6689/#6724), the incomplete-journal issue (#7001), and the design plan ("Section 9" reconciliation, numbered ceiling cases, task numbers). All of that context lives in the tracker, not the code, so the references are removed and the comments now describe only the mechanism itself. Also corrects two comments whose content was stale, not just their references: the commit_to_disk single-writer comment (the real invariant is the persist-worker single thread plus disjoint key spaces for the FKV generator and snap-sync healing), the Overlay::from_journal SAFETY comment (now describes the pruning-pause that exists), and the deferral NOTE in apply_fork_choice (the overlay-window trie fallback now covers the after-the-fact incomplete-entry case). Includes a cargo fmt pass over the adversarial-round test (fixes the fmt gate of lint-l1).
Block execution (EVM + recursive trie merkleization) recurses deeply, and tokio worker threads have small stacks: a long side-chain replay executed inline on the FCU task overflowed the worker stack partway through the replay (fatal SIGABRT, killing the node mid-FCU). The overflow was marginal and layout-dependent, so it flipped between pass and crash across builds — reproduced deterministically in the reorg simulator's side-chain scenario with a debug binary, and confirmed via crash-report backtrace (trie branch decode during Trie::get on a nearly-exhausted stack). The replay loop now runs on a scoped std::thread with a 16 MiB stack (spawned per apply pass; the async block fetch is driven from it via Handle::block_on). The FCU task blocks on the thread exactly as it did on the inline loop, and a replay-thread panic propagates through resume_unwind so the abort guard still resets the cache. Verified: the previously crashing scenario passes 10/10 stress iterations, full 10-scenario simulator suite green, ethrex-blockchain lib tests pass, fmt/clippy clean.
3 participants
Summary
Overlaystruct incrates/storage/layering.rsthat aggregates reverse-diffs fromSTATE_HISTORY[D..=T](descending, older-entry-wins) to reconstruct virtual state at pivotT-1.TrieLayerCachewith optional overlay field and wiresTrieWrapper::getto cascade: layer cache -> overlay -> disk.What's in this PR
Overlaystruct: walks the journal range descending, applies older-entry-wins semantics, holds a bloom filter for fast miss short-circuits.OverlayCfenum +classify_by_key_length: routes CF entries to the right column family using the same key-length logic as PR 1'sclassify_trie_key.OverlayError::InvalidRange { from_block, to_block }: hard error for swapped caller args (not an I/O error variant).TrieLayerCacheset/clear/lookup accessors for the overlay field.TrieWrapper::getcascade hook integrated into the existing read path.What's NOT in this PR
apply_fork_choice_with_deep_reorg,install_overlay_for_reorg, and the reorg-in-progress flag are in PR 3.from_blockaccessor rename (triggers clippywrong_self_convention, suppressed); deferred to PR 3.AtomicBloomFilterfor a non-atomic variant during construction for perf; deferred to PR 3 or PR 4.Test plan
cargo fmtclean.cargo clippyclean.crates/storage/layering.rs.cargo test -p ethrex-storagepasses all 30 tests on both InMemory and RocksDB backends.References