Generalise reconstruct_historic_states for ranged replay#9222
Generalise reconstruct_historic_states for ranged replay#9222mergify[bot] merged 1 commit intosigp:unstablefrom
Conversation
Splits the inner replay loop out of `reconstruct_historic_states` into a new public `reconstruct_historic_states_on_range(with_state_at_slot, from_slot, to_slot, write_block_roots, on_commit)` so it can be reused by an upcoming ERA file importer (separate PR). `reconstruct_historic_states` keeps its signature and behaviour, now a thin shell that derives the slot range from the anchor/split, then delegates to the ranged helper with `write_block_roots = false` and an `on_commit` closure that performs the existing per-commit anchor advance and the final flip to `as_archive_anchor()`. The ranged helper: - iterates `BeaconBlockRoots` directly via `FrozenForwardsIterator`, - detects skipped slots via `block.slot() != iterator slot` instead of tuple-window comparison, - supports a starting state at any slot `<= from_slot` (catches up via `per_slot_processing` in a `while`), - optionally writes the `BeaconBlockRoots` index inline (for callers populating the cold DB rather than reading an already-populated one), - runs a tree-hash equality check against the last stored state root at the final slot of the batch. Verified with `make lint`, `cargo fmt --check`, `cargo nextest run -p store`, and `cargo nextest run --release -p beacon_chain --test beacon_chain_tests store_tests::weak_subjectivity_sync` (7/7 pass — covers the existing reconstruction code path).
dapplion
force-pushed
the
era-reconstruct-on-range
branch
from
0f6e43b to
044c442
Compare
|
Some required checks have failed. Could you please take a look @dapplion? 🙏 |
mergify
Bot
added
waiting-on-author
michaelsproul
left a comment
michaelsproul
left a comment
There was a problem hiding this comment.
Looks good. We have quite good tests for reconstruction in the beacon chain tests, so I'm confident this change doesn't cause any regressions.
| /// `on_commit(slot)` is invoked after each atomic commit (whenever the hierarchy says to | ||
| /// commit, plus once at the final slot) so callers can update anchor metadata or log | ||
| /// progress. |
There was a problem hiding this comment.
Do you need a different definition of on_commit for the Era use case?
There was a problem hiding this comment.
I need to not update the anchor at all during reconstruction as it would be jumping around. Would be fine if reconstruct_historic_states just updates the anchor once after completing num_blocks?
There was a problem hiding this comment.
How would that help? Isn't there fundamentally something a bit wonky with a linear anchor and parallel updates?
There was a problem hiding this comment.
When you start the node from ERA files the DB is empty. Then it does funky things (parallel reconstruction) and ends up in a valid fully synced state. The node is not meant to be used during parallel reconstruction
There was a problem hiding this comment.
Ok we can cross that bridge when we come to it
michaelsproul
added
ready-for-merge
Merge Queue Status
This pull request spent 31 minutes 9 seconds in the queue, including 28 minutes 45 seconds running CI. Required conditions to merge
|
2 participants
Upcoming work to support ERA files uses
reconstruct_historic_statesbut needs slight tweaks to reconstruct arbitrary ranges of slots.This PR changes no functionality in practice but makes the scary production code changes upfront so the later bigger ERA files PR is less risky.
reconstruct_historic_stateskeeps its signature and behaviour, now a thin shell that derives the slot range from the anchor/split, then delegates to the ranged helper with anon_commitclosure that performs the existing per-commit anchor advance and the final flip toas_archive_anchor().