From 2ee284a0a082e01d1e57f74bfb8005c2b7f44395 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:59:09 -0300 Subject: [PATCH 1/2] fix(beacon): persist unrealized justifications so get_head survives a restart Beacon fork choice kept the per-block unrealized justified checkpoint (consensus-specs' store.unrealized_justifications[block_root]) only in an in-memory BeaconScratch map. A restart emptied that map with nothing to refill it: once a pre-restart block was a leaf from an epoch older than the store's clock, get_voting_source needed its entry and hit a hard SpecAssert on every tick, freezing get_head. Operators saw a follower that imported blocks and gossiped fine but never advanced its head after a restart or resume. Add a new storage table, BeaconUnrealizedJustifications, keyed by block root with (slot, Checkpoint) as the value. Root-keyed rather than slot||root like LiveChain/BlockProof: both readers of this table (get_voting_source, is_ffg_competitive) look up a root with no slot in hand, so a root-only key keeps that lookup a single point read; the slot rides in the value only for the pruner, which is fine since the table only ever holds the unfinalized window's worth of leaves between two finalizations. The in-memory map stays as a write-through cache in front of the table, since get_voting_source reads it for every block from a prior epoch. The write happens right after compute_pulled_up_tip's caller (on_block) has already committed the block's own insert_signed_block/insert_state, not in the same atomic batch: compute_pulled_up_tip needs the block's post-state, which insert_state hands off to the background state writer rather than committing synchronously. A crash in that narrow window leaves one block without its entry, surfacing as the same SpecAssert an unpatched build raised on every block, now scoped to a single block instead of the whole unfinalized window. Pruned on finalization inside update_checkpoints' Chain::Beacon arm, reusing the block_entry(&finalized.root) lookup PR #46 introduced for LiveChain pruning, on the same horizon (the finalized block's own slot rather than the stored checkpoint's epoch-start slot), so the finalized block's own entry always survives a resume with nothing imported past it yet. Bumps DB_VERSION to 4: a directory written by the previous version has no row in the new table for any block it already imported, which would otherwise look identical to "no unrealized justification computed yet" rather than "this directory predates the table". --- CLAUDE.md | 24 +- .../src/beacon/fork_choice.rs | 183 +++++++++++- crates/storage/src/api/tables.rs | 32 +- crates/storage/src/store.rs | 279 ++++++++++++++++-- docs/data_storage.md | 120 ++++++-- 5 files changed, 570 insertions(+), 68 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index d4cfc073..74730bac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -775,7 +775,7 @@ snapshot (`States`) + diff (`StateDiffs`) pairs; `BlockRoots` and `LiveChain` index by slot for range serving and fork choice. Attestations and gossip signatures are not persisted; they live in in-memory `Store` buffers consumed during the tick pipeline. See [`docs/data_storage.md`](docs/data_storage.md) -for the full reference: what each of the ten tables holds and how it's +for the full reference: what each of the eleven tables holds and how it's keyed, the snapshot/diff reconstruction algorithm, the block-import write sequence, pruning rules, what never changes at runtime, and startup/restore behavior. @@ -795,12 +795,22 @@ behavior. which is what keeps a parked column from satisfying the availability gate. Its only index is the chain actor's in-memory `sidecars_awaiting_parent`, so `start_actor` clears the whole table at startup. -- `DB_VERSION` is 4: `Config` gained `PRESET_BASE` and `CONFIG_NAME` (as - `ConfigName`, a bounded string) at the front of its encoding, and it is - SSZ-encoded under `KEY_CONFIG`, so a data directory written by an earlier - version decodes into the wrong fields. (3 was the runtime keys a - `config.yaml` supplies.) `Store::from_db_state` refuses any other version - outright; there is no migration. +- `BeaconUnrealizedJustifications` (root -> `(slot, Checkpoint)`) persists the + beacon store's per-block unrealized justified checkpoint + (consensus-specs' `store.unrealized_justifications`), which used to live + only in an in-memory `BeaconScratch` map. A restart emptied that map with + nothing to refill it, so `get_voting_source` hit a hard `SpecAssert` on the + first pre-restart leaf from an older epoch and `get_head` froze. The + in-memory map stays as a write-through cache in front of the table. Pruned + on finalization to the same horizon as `LiveChain` (the finalized block's + own slot, keeping that block's own entry). +- `DB_VERSION` is 5: the new unrealized-justifications table means a + directory written by an earlier version has no row there for any block it + already imported, which would otherwise look identical to "no unrealized + justification computed yet" rather than "this directory predates the + table". (4 was `PRESET_BASE`/`CONFIG_NAME` at the front of `Config`; 3 was + the runtime keys a `config.yaml` supplies.) `Store::from_db_state` refuses + any other version outright; there is no migration. ### State Root Computation - Always computed via `hash_tree_root()` after full state transition diff --git a/crates/blockchain/state_transition/src/beacon/fork_choice.rs b/crates/blockchain/state_transition/src/beacon/fork_choice.rs index 94b52472..0747926e 100644 --- a/crates/blockchain/state_transition/src/beacon/fork_choice.rs +++ b/crates/blockchain/state_transition/src/beacon/fork_choice.rs @@ -991,13 +991,18 @@ pub fn get_forkchoice_store( // this file's only outright whole-`BeaconState` clone. `checkpoint_state` // now derives that second copy on demand instead of caching it, so the // anchor is written once. + // Captured before `anchor_state` moves into `insert_state` below: + // `set_unrealized_justification` needs the anchor's own slot to persist + // alongside its checkpoint (see `Table::BeaconUnrealizedJustifications`), + // and `insert_state` takes the state by value. + let anchor_slot = anchor_state.slot(); store .insert_signed_block(anchor_root, anchor_block) .expect("insert"); store .insert_state(anchor_root, anchor_state) .expect("insert"); - store.set_unrealized_justification(anchor_root, justified_checkpoint); + store.set_unrealized_justification(anchor_root, anchor_slot, justified_checkpoint); Ok(store) } @@ -2054,7 +2059,7 @@ pub fn compute_pulled_up_tip( let current_justified = state.current_justified_checkpoint(); let finalized = state.finalized_checkpoint(); - store.set_unrealized_justification(block_root, current_justified); + store.set_unrealized_justification(block_root, block_slot, current_justified); update_unrealized_checkpoints(store, current_justified, finalized); // If the block is from a prior epoch, apply the realized values. `block_slot` @@ -3144,7 +3149,7 @@ mod tests { "the test must exercise two different values" ); - store.set_unrealized_justification(block_root, unrealized); + store.set_unrealized_justification(block_root, 0, unrealized); let mut state = crate::beacon::helpers::test_state::with_validators(1); *state.current_justified_checkpoint_mut() = realized; @@ -3183,6 +3188,178 @@ mod tests { assert_eq!(slot, store.block_entry(&head).expect("head block").0); } + /// Imports `block` with `post_state` as its post-state through the part + /// of [`on_block`] fork choice later reads back: the block and state rows, + /// then [`compute_pulled_up_tip`], the only production writer of a + /// non-anchor block's unrealized justification. `state_transition` is + /// skipped, since it would need properly signed blocks and nothing here + /// depends on it. + fn import_unchecked( + store: &mut Store, + block: SignedBeaconBlock, + mut post_state: BeaconState, + config: &Config, + ) -> Root { + let root = block.message_hash_tree_root(); + let slot = block.slot(); + *post_state.slot_mut() = slot; + store.insert_signed_block(root, block).unwrap(); + store.insert_state(root, post_state).unwrap(); + compute_pulled_up_tip(store, root, slot, config).expect("pulled-up tip"); + root + } + + /// A post-state for a block building on `anchor_root` in the anchor's + /// own epoch, whose justified checkpoint is that anchor: the shape every + /// block in the anchor epoch has, and one that keeps every leaf below + /// viable (`voting_source.epoch == justified.epoch`) whatever the clock + /// reads, so a failure can only come from the lookup, not the filter. + fn post_state_over(anchor_state: &BeaconState, anchor_root: Root) -> BeaconState { + let mut state = anchor_state.clone(); + *state.current_justified_checkpoint_mut() = Checkpoint { + epoch: compute_epoch_at_slot(anchor_state.slot()), + root: anchor_root, + }; + state + } + + /// The resume path `fetch_initial_beacon_state` takes: reopen the + /// directory, check the anchor states, repair the head. Nothing else + /// happens to the store before the chain actor starts ticking it. + fn resume(backend: Arc) -> Store { + let mut store = Store::from_db_state(backend) + .expect("same version") + .expect("a beacon directory"); + store + .verify_anchor_states() + .expect("anchor states persisted"); + store.repair_head().expect("head has a state"); + store + } + + /// The store clock two epochs past the anchor's, the first tick a node + /// restarted into a later epoch runs. + fn two_epochs_past(anchor_slot: Slot, config: &Config) -> u64 { + (anchor_slot + 2 * preset::SLOTS_PER_EPOCH) * config.seconds_per_slot + } + + /// Reproduces the unrealized-justification loss on resume: the + /// pre-restart head is a leaf from an epoch older than the store's clock, + /// so `get_voting_source` needs its entry, and a reopened store used to + /// have none. + /// + /// Failed before `Table::BeaconUnrealizedJustifications` existed, with + /// `SpecAssert("block_root in store.unrealized_justifications")`; passes + /// now that the entry is persisted rather than living only in + /// `BeaconScratch`. + #[test] + fn get_head_survives_a_restart_over_a_pre_restart_leaf() { + let config = Config::active(); + let (anchor_state, anchor_block) = anchor_pair(); + let anchor_slot = anchor_state.slot(); + let anchor_root = anchor_block.message_hash_tree_root(); + let backend: Arc = Arc::new(InMemoryBackend::new()); + let mut store = + get_forkchoice_store(backend.clone(), anchor_state.clone(), anchor_block, &config) + .expect("the pair matches"); + + // anchor -> a -> b, all in the anchor's epoch. + let post_state = post_state_over(&anchor_state, anchor_root); + let a = import_unchecked( + &mut store, + block(anchor_slot + 1, anchor_root), + post_state.clone(), + &config, + ); + let b = import_unchecked(&mut store, block(anchor_slot + 2, a), post_state, &config); + + // Before the restart, with the clock already past b's epoch, the map + // answers b's lookup and b is the head. + on_tick(&mut store, two_epochs_past(anchor_slot, &config), &config); + assert_eq!(get_head(&mut store, &config).expect("pre-restart"), b); + + // Restart: the writer flushes on drop, then the directory reopens. + drop(store); + let mut resumed = resume(backend); + assert_eq!(resumed.beacon_head().map(|(_, root)| root), Some(b)); + assert!( + resumed.unrealized_justification(&b).is_some(), + "the persisted entry must survive the restart, before the in-memory \ + cache has been refilled by anything" + ); + + // The first tick after the restart: same time, same tree, same head + // expected. + on_tick(&mut resumed, two_epochs_past(anchor_slot, &config), &config); + let head = get_head(&mut resumed, &config).map_err(|err| format!("{err:?}")); + assert_eq!( + head, + Ok(b), + "a restart must not change what get_head can compute" + ); + } + + /// The same loss, but for a stale fork leaf rather than the head: a + /// block imported after the restart extends the head (so the head is no + /// longer a leaf and has a fresh entry), yet a pre-restart sibling branch + /// is still a leaf above the justified root, and its persisted entry must + /// still answer, from before the restart, with nothing having recomputed + /// it since. + /// + /// Failed before like [`get_head_survives_a_restart_over_a_pre_restart_leaf`]. + #[test] + fn get_head_survives_a_restart_past_a_pre_restart_fork_leaf() { + let config = Config::active(); + let (anchor_state, anchor_block) = anchor_pair(); + let anchor_slot = anchor_state.slot(); + let anchor_root = anchor_block.message_hash_tree_root(); + let backend: Arc = Arc::new(InMemoryBackend::new()); + let mut store = + get_forkchoice_store(backend.clone(), anchor_state.clone(), anchor_block, &config) + .expect("the pair matches"); + + // anchor -> a -> {b, f}: f is the branch the network did not build on. + let post_state = post_state_over(&anchor_state, anchor_root); + let a = import_unchecked( + &mut store, + block(anchor_slot + 1, anchor_root), + post_state.clone(), + &config, + ); + let b = import_unchecked( + &mut store, + block(anchor_slot + 2, a), + post_state.clone(), + &config, + ); + let f = import_unchecked( + &mut store, + block(anchor_slot + 3, a), + post_state.clone(), + &config, + ); + on_tick(&mut store, two_epochs_past(anchor_slot, &config), &config); + get_head(&mut store, &config).expect("pre-restart"); + + drop(store); + let mut resumed = resume(backend); + on_tick(&mut resumed, two_epochs_past(anchor_slot, &config), &config); + + // The chain moves on from b after the restart; c gets a fresh entry. + let c = import_unchecked(&mut resumed, block(anchor_slot + 4, b), post_state, &config); + assert!(resumed.unrealized_justification(&c).is_some()); + assert!( + resumed.unrealized_justification(&f).is_some(), + "f's entry was persisted before the restart and nothing has pruned it" + ); + + let head = get_head(&mut resumed, &config).map_err(|err| format!("{err:?}")); + assert!( + head.as_ref().is_ok_and(|head| *head == c || *head == f), + "a stale pre-restart fork leaf must not stop get_head: {head:?}" + ); + } + /// The specification's assertion cannot hold for a checkpoint-synced /// anchor: the finalized state sits at the epoch boundary, so when that /// slot was empty it has advanced past its own `latest_block_header` and diff --git a/crates/storage/src/api/tables.rs b/crates/storage/src/api/tables.rs index f2d2f0db..0fb6c052 100644 --- a/crates/storage/src/api/tables.rs +++ b/crates/storage/src/api/tables.rs @@ -61,10 +61,38 @@ pub enum Table { /// re-deriving anything. Emptied by the replay that verifies a row and by /// the finality eviction that gives up on one. PendingDataColumns, + /// Beacon per-block unrealized justified checkpoint: root -> + /// `(slot, Checkpoint)`. + /// + /// The specification's `store.unrealized_justifications[block_root]`, + /// consensus-specs' own scratch, used to live only in + /// `BeaconScratch::unrealized_justifications`. A restart emptied that map, + /// and nothing refilled it, so `get_voting_source`/`is_ffg_competitive` + /// hit a hard `SpecAssert` on the first pre-restart leaf from an epoch + /// older than the store's clock, freezing `get_head`. This table is what + /// makes the value survive a restart; the in-memory map stays too, as a + /// write-through cache over it, since `get_voting_source` reads this for + /// every block from a prior epoch. + /// + /// Keyed by root alone rather than `slot ‖ root` (`encode_slot_root_key`, + /// as `LiveChain`/`BlockProof` are): both readers of this table + /// (`get_voting_source`, `is_ffg_competitive`) look up a root with no slot + /// in hand, so a root-only key keeps that lookup a single point read. The + /// slot rides + /// along in the *value* instead, purely so the pruner (which does need it) + /// does not have to decode a whole block to find it. The trade is a + /// pruning pass that scans the whole table rather than stopping early on a + /// slot-ordered prefix, which is cheap here: the table only ever holds the + /// unfinalized window's worth of leaves between two finalizations. + /// + /// Pruned on finalization, on the same horizon `LiveChain` is (the + /// finalized block's own slot): see `Store::update_checkpoints`'s + /// `Chain::Beacon` arm. + BeaconUnrealizedJustifications, } /// All table variants. -pub const ALL_TABLES: [Table; 10] = [ +pub const ALL_TABLES: [Table; 11] = [ Table::BlockHeaders, Table::BlockBodies, Table::BlockProof, @@ -75,6 +103,7 @@ pub const ALL_TABLES: [Table; 10] = [ Table::LiveChain, Table::DataColumns, Table::PendingDataColumns, + Table::BeaconUnrealizedJustifications, ]; impl Table { @@ -91,6 +120,7 @@ impl Table { Table::LiveChain => "live_chain", Table::DataColumns => "data_columns", Table::PendingDataColumns => "pending_data_columns", + Table::BeaconUnrealizedJustifications => "beacon_unrealized_justifications", } } } diff --git a/crates/storage/src/store.rs b/crates/storage/src/store.rs index 8962d7b3..770a95ae 100644 --- a/crates/storage/src/store.rs +++ b/crates/storage/src/store.rs @@ -172,7 +172,13 @@ const KEY_ANCHOR_SLOT: &[u8] = b"anchor_slot"; /// /// 4 added `PRESET_BASE` and `CONFIG_NAME` to `Config`, at the front of its /// encoding, for the same reason and with the same consequence as 3. -pub const DB_VERSION: u64 = 4; +/// +/// 5 added `Table::BeaconUnrealizedJustifications`: a directory written by the +/// previous version has no row there for any block it already imported, which +/// would otherwise look identical to "no unrealized justification computed +/// yet" rather than "this directory predates the table", the same silent +/// wrong answer every prior bump exists to rule out. +pub const DB_VERSION: u64 = 5; /// The consensus protocol a data directory holds. /// @@ -637,20 +643,27 @@ impl GossipSignatureBuffer { } /// Beacon fork-choice state that is per-slot or per-epoch scratch rather than -/// chain history: nothing here survives a restart, and nothing here is worth -/// the write amplification of persisting. +/// chain history: most of it does not survive a restart, and is not worth the +/// write amplification of persisting. /// /// `proposer_boost_root` resets every slot, `block_timeliness` is read only by /// the same-slot reorg helpers, `equivocating_indices` is rebuilt by replaying /// attester slashings on sync, `latest_messages` is rebuilt by the first epoch -/// of attestations, `pow_blocks` stands in for a call to an execution client -/// that a restarted node would simply make again, and -/// `unrealized_justifications` is recomputed by replaying epoch processing on a -/// copy of a block's post-state, which a node resuming from an anchor does -/// anyway as it re-imports the unfinalized window. `optimistic_roots` and -/// `payload_statuses` are likewise answers an execution client can be asked -/// for again, and `el_block_hashes` is a cache over data already decodable -/// from the block itself. +/// of attestations, and `pow_blocks` stands in for a call to an execution +/// client that a restarted node would simply make again. `optimistic_roots` +/// and `payload_statuses` are likewise answers an execution client can be +/// asked for again, and `el_block_hashes` is a cache over data already +/// decodable from the block itself. +/// +/// `unrealized_justifications` is the one exception: it is a write-through +/// cache over `Table::BeaconUnrealizedJustifications`, not scratch that a +/// restart may safely lose. A resumed node does *not* re-import the +/// unfinalized window from its anchor the way a fresh sync would, so a value +/// computed only in memory would be gone for good the moment a pre-restart +/// leaf from an older epoch needed it again, and `get_voting_source` would +/// fail outright rather than recompute it. See +/// [`Store::unrealized_justification`] and +/// [`Store::set_unrealized_justification`]. /// /// Most of this is uncapped: the per-validator maps are bounded by the /// validator set, and the per-block ones (`block_timeliness`, @@ -668,6 +681,9 @@ pub(crate) struct BeaconScratch { pub(crate) equivocating_indices: HashSet, pub(crate) latest_messages: HashMap, pub(crate) pow_blocks: HashMap, + /// In-memory half of `Table::BeaconUnrealizedJustifications`; see this + /// struct's own documentation for why this map, unlike its neighbours, + /// has a persisted backing table. pub(crate) unrealized_justifications: HashMap, /// Beacon roots imported on an execution client's `NOT_VALIDATED` answer, /// against the slot the unfinalized-window bound prunes them by. @@ -715,6 +731,27 @@ fn decode_slot_root_key(bytes: &[u8]) -> (u64, H256) { (slot, root) } +/// Encode a `Table::BeaconUnrealizedJustifications` value: the block's own +/// slot (8 bytes big-endian, matching [`encode_slot_root_key`]'s convention, +/// though nothing here depends on byte order since this is a value rather +/// than a key), then the checkpoint's SSZ. +/// +/// The slot rides in the value rather than the key; see the table's own doc +/// comment (`crates/storage/src/api/tables.rs`) for why keying by root alone +/// is what keeps [`Store::unrealized_justification`] a single point read. +fn encode_unrealized_justification_value(slot: u64, checkpoint: BeaconCheckpoint) -> Vec { + let mut bytes = slot.to_be_bytes().to_vec(); + bytes.extend_from_slice(&checkpoint.to_ssz()); + bytes +} + +/// The inverse of [`encode_unrealized_justification_value`]. +fn decode_unrealized_justification_value(bytes: &[u8]) -> (u64, BeaconCheckpoint) { + let slot = u64::from_be_bytes(bytes[..8].try_into().expect("valid slot bytes")); + let checkpoint = BeaconCheckpoint::from_ssz_bytes(&bytes[8..]).expect("valid checkpoint"); + (slot, checkpoint) +} + fn encode_block_root_key(slot: u64) -> Vec { slot.to_be_bytes().to_vec() } @@ -1828,10 +1865,21 @@ impl Store { Some((block_slot, _)) => { let pruned_chain = self.prune_live_chain(block_slot).expect("prune live chain"); - if pruned_chain > 0 { + // Same horizon as `LiveChain` above, and for the + // same reason: the finalized block's own row must + // survive, since a resumed node with nothing + // imported past the anchor needs the anchor's own + // entry to be a leaf `get_head` can still walk to. + let pruned_justifications = self + .prune_unrealized_justifications(block_slot) + .expect("prune unrealized justifications"); + if pruned_chain > 0 || pruned_justifications > 0 { info!( finalized_slot = finalized.slot, - block_slot, pruned_chain, "Pruned finalized beacon live chain" + block_slot, + pruned_chain, + pruned_justifications, + "Pruned finalized beacon live chain" ); } } @@ -3438,26 +3486,131 @@ impl Store { /// one. /// /// `get_voting_source` reads this for every block from a prior epoch, so - /// it is the hottest map in the scratch; recomputing a missing entry means - /// replaying epoch processing on a copy of that block's post-state. That - /// still does not make it chain history: a restarted node re-imports the - /// unfinalized window from its anchor and refills the map as it goes. + /// the in-memory cache is checked first and a hit costs no backend round + /// trip. A miss falls through to `Table::BeaconUnrealizedJustifications`, + /// which is what lets this answer correctly right after a restart, before + /// anything has repopulated the cache: see that table's own doc comment + /// for the `get_head` freeze a purely in-memory map used to cause. pub fn unrealized_justification(&self, root: &H256) -> Option { - self.beacon + if let Some(checkpoint) = self + .beacon .lock() .unwrap() .unrealized_justifications .get(root) .copied() - } + { + return Some(checkpoint); + } - /// Records `root`'s unrealized justification. - pub fn set_unrealized_justification(&mut self, root: H256, checkpoint: BeaconCheckpoint) { + let view = self.backend.begin_read().expect("read view"); + view.get(Table::BeaconUnrealizedJustifications, &root.to_ssz()) + .expect("get") + .map(|bytes| decode_unrealized_justification_value(&bytes).1) + } + + /// Records `root`'s unrealized justification, persisting it to + /// `Table::BeaconUnrealizedJustifications` and updating the in-memory + /// cache the same call. + /// + /// `slot` is `root`'s own block slot, not the checkpoint's epoch: the + /// table keys by root alone (see its own doc comment for why), so the + /// slot travels in the value instead, for + /// [`Store::prune_unrealized_justifications`] to read back without + /// decoding a whole block. + /// + /// Called from `compute_pulled_up_tip` right after `insert_signed_block` + /// has already committed the block's own row, not inside that commit's + /// batch: `compute_pulled_up_tip` needs the block's *post-state*, which + /// only exists once `insert_signed_block`/`insert_state` have run, and + /// `insert_state` itself hands off to the background state writer (see + /// `state_writer.rs`) rather than committing synchronously. A crash + /// between the block's commit and this one leaves an imported block with + /// no unrealized-justification row, exactly the state a version-3 + /// directory is in for every block it already holds; `get_voting_source` + /// then fails on that one root the same way it used to fail on all of + /// them, rather than losing the whole restart. + pub fn set_unrealized_justification( + &mut self, + root: H256, + slot: u64, + checkpoint: BeaconCheckpoint, + ) { self.beacon .lock() .unwrap() .unrealized_justifications .insert(root, checkpoint); + + let mut batch = self.backend.begin_write().expect("write batch"); + let entries = vec![( + root.to_ssz(), + encode_unrealized_justification_value(slot, checkpoint), + )]; + batch + .put_batch(Table::BeaconUnrealizedJustifications, entries) + .expect("put unrealized justification"); + batch.commit().expect("commit"); + } + + /// Prune `Table::BeaconUnrealizedJustifications` entries whose own block + /// sits strictly below `finalized_block_slot`, always keeping any entry + /// at or above it. + /// + /// `finalized_block_slot` is the finalized block's *own* slot, the same + /// horizon `prune_live_chain` is called with from `update_checkpoints`'s + /// `Chain::Beacon` arm (`block_entry(&finalized.root)`, not the stored + /// checkpoint's epoch-start slot) — see that arm's own comment for why the + /// two differ, and why using it here means the finalized block's own + /// entry, needed the moment nothing has imported past it yet, always + /// survives. + /// + /// Unlike [`prune_live_chain`](Self::prune_live_chain), this table is + /// keyed by root, not `slot ‖ root`, so there is no slot-ordered prefix to + /// stop early on: every row's value has to be decoded to read its slot + /// back out. That is cheap here, since the table only ever holds the + /// unfinalized window's worth of leaves between two finalizations. + /// + /// Also evicts the same roots from the in-memory cache, so a stale answer + /// there cannot outlive the row backing it. + pub fn prune_unrealized_justifications( + &mut self, + finalized_block_slot: u64, + ) -> Result { + let view = self.backend.begin_read().expect("read view"); + let keys_to_delete: Vec> = view + .prefix_iterator(Table::BeaconUnrealizedJustifications, &[]) + .expect("iterator") + .filter_map(|res| res.ok()) + .filter(|(_, value)| { + decode_unrealized_justification_value(value).0 < finalized_block_slot + }) + .map(|(key, _)| key.to_vec()) + .collect(); + drop(view); + + let count = keys_to_delete.len(); + if count == 0 { + return Ok(0); + } + + let mut batch = self.backend.begin_write().expect("write batch"); + batch + .delete_batch( + Table::BeaconUnrealizedJustifications, + keys_to_delete.clone(), + ) + .expect("delete pruned unrealized justifications"); + batch.commit().expect("commit"); + + let mut scratch = self.beacon.lock().unwrap(); + for key in &keys_to_delete { + scratch + .unrealized_justifications + .remove(&H256::from_slice(key)); + } + + Ok(count) } // ============ Data Columns ============ @@ -7306,26 +7459,96 @@ mod tests { } #[test] - fn an_unrealized_justification_is_scratch_not_chain_history() { - // Shared across clones of one `Store`, since the scratch sits behind - // an `Arc`, but gone once the process reopens the directory: a - // restarted node refills the map as it re-imports the unfinalized - // window. + fn an_unrealized_justification_persists_across_a_reopened_store() { + // Shared across clones of one `Store`, since the write-through cache + // sits behind an `Arc`, and still answers on a reopened store, whose + // cache starts empty but shares the same backend: that fall-through + // to `Table::BeaconUnrealizedJustifications` is what a restart no + // longer loses. let backend = Arc::new(InMemoryBackend::new()); let mut store = beacon_test_store(backend.clone()); let root = H256::from([1u8; 32]); + let slot = 40; let cp = BeaconCheckpoint { epoch: 5, root: H256::from([2u8; 32]), }; - store.set_unrealized_justification(root, cp); + store.set_unrealized_justification(root, slot, cp); assert_eq!(store.unrealized_justification(&root), Some(cp)); assert_eq!(store.unrealized_justification(&H256::from([3u8; 32])), None); assert_eq!(store.clone().unrealized_justification(&root), Some(cp)); let reopened = beacon_test_store(backend); - assert_eq!(reopened.unrealized_justification(&root), None); + assert_eq!( + reopened.unrealized_justification(&root), + Some(cp), + "a fresh store's empty cache must fall through to the persisted row" + ); + } + + #[test] + fn beacon_finalization_prunes_unrealized_justifications_to_the_finalized_blocks_own_slot() { + // Same empty-epoch-start-slot shape as + // `beacon_finalization_prunes_live_chain_to_the_finalized_blocks_own_slot`: + // epoch 1 starts at slot 32, but nobody built a block there, so the + // finalized checkpoint's own block sits at slot 31. Pruning to the + // stored (epoch-start) slot would delete that block's own entry; + // pruning to its real slot must not. + let anchor = beacon_test_block(0, H256::ZERO); + let anchor_root = anchor.message_hash_tree_root(); + let mut store = Store::init_beacon( + Arc::new(InMemoryBackend::new()), + 0, + Config::mainnet(), + anchor_root, + Checkpoint::default(), + 0, + ); + store + .insert_signed_block(anchor_root, anchor) + .expect("insert anchor"); + store.set_unrealized_justification(anchor_root, 0, BeaconCheckpoint::default()); + + let block_31 = beacon_test_block(31, anchor_root); + let root_31 = block_31.message_hash_tree_root(); + store + .insert_signed_block(root_31, block_31) + .expect("insert block 31"); + store.set_unrealized_justification(root_31, 31, BeaconCheckpoint::default()); + + let head = beacon_test_block(33, root_31); + let head_root = head.message_hash_tree_root(); + store + .insert_signed_block(head_root, head) + .expect("insert head"); + store.set_unrealized_justification(head_root, 33, BeaconCheckpoint::default()); + + let finalized = Store::beacon_checkpoint_as_stored(BeaconCheckpoint { + epoch: 1, + root: root_31, + }); + assert_eq!( + finalized.slot, 32, + "the epoch's own start slot, not block 31's, must be what gets stored" + ); + store + .update_checkpoints(ForkCheckpoints::new(head_root, None, Some(finalized))) + .expect("advance finalized"); + + assert_eq!( + store.unrealized_justification(&anchor_root), + None, + "entries below the finalized block must be pruned" + ); + assert!( + store.unrealized_justification(&root_31).is_some(), + "the finalized block's own entry must survive" + ); + assert!( + store.unrealized_justification(&head_root).is_some(), + "entries above the finalized block must remain" + ); } // ============ Data Column Tests ============ diff --git a/docs/data_storage.md b/docs/data_storage.md index 260afff3..faf1fe13 100644 --- a/docs/data_storage.md +++ b/docs/data_storage.md @@ -2,7 +2,7 @@ This doc explains how ethlambda saves data. Especially, the split between the fork choice `Store` and the `StorageBackend` trait, -what each of the ten tables holds, and which data is in-memory only. +what each of the eleven tables holds, and which data is in-memory only. ## Overview @@ -93,32 +93,32 @@ is built from it, and clones are handed to the BlockChain and P2P actors. INSIDE THE STORE ──────────────── - ┌──────────────────────────── Store ────────────────────────────┐ - │ │ - │ PERSISTED (via backend) IN-MEMORY ONLY │ - │ ────────────────────── ────────────────────── │ - │ ┌─────────────────────┐ ┌──────────────────────┐ │ - │ │ BlockHeaders │ │ new_payloads │ │ - │ │ BlockBodies │ │ (pending aggregated │ │ - │ │ BlockProof │ │ attestations) │ │ - │ │ BlockRoots │ │ known_payloads │ │ - │ │ States │ │ (fork-choice-active │ │ - │ │ StateDiffs │ │ attestations) │ │ - │ │ Metadata │ │ gossip_signatures │ │ - │ │ LiveChain │ │ (raw XMSS sigs │ │ - │ │ DataColumns │ │ awaiting │ │ - │ │ PendingDataColumns │ │ aggregation) │ │ - │ └─────────────────────┘ │ state_cache (LRU) │ │ - │ └──────────────────────┘ │ - │ Survives restarts, except │ - │ PendingDataColumns, which Lost on restart. │ - │ is cleared at startup. │ - └───────────────────────────────────────────────────────────────┘ + ┌───────────────────────────────── Store ──────────────────────────────────┐ + │ │ + │ PERSISTED (via backend) IN-MEMORY ONLY │ + │ ─────────────────────── ────────────── │ + │ ┌────────────────────────────────┐ ┌──────────────────────┐ │ + │ │ BlockHeaders │ │ new_payloads │ │ + │ │ BlockBodies │ │ (pending aggregated │ │ + │ │ BlockProof │ │ attestations) │ │ + │ │ BlockRoots │ │ known_payloads │ │ + │ │ States │ │ (fork-choice-active │ │ + │ │ StateDiffs │ │ attestations) │ │ + │ │ Metadata │ │ gossip_signatures │ │ + │ │ LiveChain │ │ (raw XMSS sigs │ │ + │ │ DataColumns │ │ awaiting │ │ + │ │ PendingDataColumns │ │ aggregation) │ │ + │ │ BeaconUnrealizedJustifications │ │ state_cache (LRU) │ │ + │ └────────────────────────────────┘ └──────────────────────┘ │ + │ Survives restarts, except │ + │ PendingDataColumns, which Lost on restart. │ + │ is cleared at startup. │ + └──────────────────────────────────────────────────────────────────────────┘ ``` ## The Tables -The ten variants of the `Table` enum (`crates/storage/src/api/tables.rs`): +The eleven variants of the `Table` enum (`crates/storage/src/api/tables.rs`): | Table | Key | Value | Pruned? | | ------------------ | --------------------------- | ----------------------------------------- | --------------------------------- | @@ -132,6 +132,7 @@ The ten variants of the `Table` enum (`crates/storage/src/api/tables.rs`): | `LiveChain` | slot ‖ root | `parent_root` | yes: below finalized | | `DataColumns` | slot ‖ root ‖ column_index | `DataColumnSidecar` (SSZ-encoded) | no (see [DataColumns](#datacolumns) below) | | `PendingDataColumns` | slot ‖ root ‖ column_index | `DataColumnSidecar` (SSZ-encoded), unverified | yes: on replay, below finalized, and wholly at startup | +| `BeaconUnrealizedJustifications` | root | `(slot, Checkpoint)` (beacon only) | yes: below the finalized block's own slot (see [below](#beaconunrealizedjustifications)) | ### Key encoding @@ -155,6 +156,16 @@ Four key layouts are used: big-endian slot, since the value already holds the root. This table is never pruned, so the ordering buys nothing here; it is kept only for consistency with the other slot-prefixed keys. +- **Root-keyed with the slot in the value** (`BeaconUnrealizedJustifications`) + is the inverse trade from the slot-prefixed tables above: both of this + table's readers (`get_voting_source`, `is_ffg_competitive`) look up a root + with no slot in hand, so keying by root alone keeps that lookup a single + point read, and the slot rides along in the value purely for the pruner + to read back. The cost is on the pruner's side instead: with no slot + prefix to stop early on, it decodes every row's value to compare its slot, + cheap here since the table only ever holds the unfinalized window's worth + of leaves between two finalizations. See + [BeaconUnrealizedJustifications](#beaconunrealizedjustifications) below. ### BlockHeaders @@ -501,6 +512,47 @@ preserving, and the block it belongs to asks for its columns again. Without that, every crash would leave every row it had parked unreadable, kept until the directory was deleted. +### BeaconUnrealizedJustifications + +`root → (slot, Checkpoint)`, **beacon only**. Persists the beacon fork +choice's per-block unrealized justified checkpoint — consensus-specs' +`store.unrealized_justifications[block_root]` — which used to live only in +`BeaconScratch::unrealized_justifications`, an in-memory `HashMap` with no +backing table. A restart emptied that map with nothing to refill it: once a +pre-restart block was a leaf from an epoch older than the store's clock, +`get_voting_source` (and, through it, `filter_block_tree`/`get_head`) hit a +hard `SpecAssert("block_root in store.unrealized_justifications")` on every +tick, freezing the head. This table is what makes the value survive a +restart; `Store::unrealized_justification`/`Store::set_unrealized_justification` +keep the in-memory map too, as a write-through cache in front of it, since +`get_voting_source` reads this for every block from a prior epoch and a cache +hit costs no backend round trip. + +Keyed by root alone rather than `slot ‖ root`, unlike `LiveChain`/`BlockProof`: +see [Key encoding](#key-encoding) above for why. The slot travels in the value +instead, written alongside the checkpoint by `Store::set_unrealized_justification`. + +Two writers: the anchor, at bootstrap (`get_forkchoice_store`), and +`compute_pulled_up_tip`, called from `on_block` for every newly imported +block. Both call `Store::set_unrealized_justification` after the block's own +`insert_signed_block`/`insert_state` have already run — `compute_pulled_up_tip` +needs the block's post-state, which does not exist until those have — so the +write is not in the same atomic batch as the block's import. A crash between +the two leaves that one block without an unrealized-justification entry, +which surfaces as the same `SpecAssert` an older, unpatched build would raise +on every block: a narrower version of the bug this table exists to fix, +scoped to the single most-recently-imported block rather than the whole +unfinalized window. + +Pruned on finalization, on the same horizon `LiveChain` is (the finalized +block's own slot, read from `BlockHeaders` via `block_entry`, not the stored +checkpoint's epoch-start slot — see [LiveChain](#livechain) above for why): +`Store::prune_unrealized_justifications` deletes every entry below that slot +and evicts the same roots from the in-memory cache, keeping the finalized +block's own entry. That entry has to survive: after a restart with nothing +yet imported past the anchor, the anchor itself is the only leaf `get_head` +has to work with. + ## State Storage: Snapshots + Diffs Storing a full `State` per block would be wasteful: most fields never change @@ -646,6 +698,12 @@ a deferred heavy phase. - `prune_stale_aggregated_payloads`: drops in-memory aggregated payloads (both pending and known) whose target slot is at or below the finalized slot. +- `prune_unrealized_justifications`: **beacon only**, and only alongside + `prune_live_chain` in the same `Chain::Beacon` arm, on the same horizon + (the finalized block's own slot) — see + [BeaconUnrealizedJustifications](#beaconunrealizedjustifications) above. + Deletes entries below that slot from the table and evicts the same roots + from the in-memory cache, keeping the finalized block's own entry. **Deferred** (`prune_old_data`, called after a batch of blocks has been processed): @@ -672,9 +730,10 @@ where a future pruner is meant to land ## In-Memory Only (Lost on Restart) -Five `Store` fields never touch the backend. All are bounded buffers (or, for -`beacon`, bounded in practice by the validator set and the unfinalized window) -shared across `Store` clones: +Five `Store` fields never touch the backend directly, `beacon` with one +exception (see its row below). All are bounded buffers (or, for `beacon`, +bounded in practice by the validator set and the unfinalized window) shared +across `Store` clones: | Buffer | Capacity | Contents | | ------------------- | --------------- | ---------------------------------------------------------------------------------------- | @@ -682,7 +741,7 @@ shared across `Store` clones: | `known_payloads` | 512 messages | Fork-choice-active aggregated proofs | | `gossip_signatures` | 2048 signatures | Raw per-validator XMSS signatures awaiting aggregation (each ~3 KB, so ~6 MB worst case) | | `state_cache` | 32 states | LRU memoization of block *and* checkpoint post-states (either chain), each held behind an `Arc` so a hit is not a copy; one bound covers both kinds, keyed apart by a small enum, and a miss is just a reconstruction rather than an error | -| `beacon` | unbounded | Beacon fork-choice scratch: proposer boost root, block timeliness, equivocating validator indices, latest messages, PoW blocks, and unrealized justifications. None of it is persisted: proposer boost resets every slot, timeliness is read only by the same-slot reorg helpers, equivocators come back from replaying attester slashings on sync, latest messages from one epoch of attestations, PoW blocks stand in for an execution-client call a restarted node would simply make again, and unrealized justifications are refilled as a node re-imports the unfinalized window from its anchor | +| `beacon` | unbounded | Beacon fork-choice scratch: proposer boost root, block timeliness, equivocating validator indices, latest messages, PoW blocks, and unrealized justifications. Most of it is not persisted: proposer boost resets every slot, timeliness is read only by the same-slot reorg helpers, equivocators come back from replaying attester slashings on sync, latest messages from one epoch of attestations, and PoW blocks stand in for an execution-client call a restarted node would simply make again. Unrealized justifications are the exception: this map is a write-through cache in front of `BeaconUnrealizedJustifications` (see [that table](#beaconunrealizedjustifications) above), so it is not lost, only refilled lazily on the next miss | The payload buffers evict FIFO when full, and redundant proofs (whose participants are a subset of an existing proof for the same attestation data) @@ -696,7 +755,7 @@ pools. After a restart these buffers start empty: pending attestations and un-aggregated gossip signatures are lost and must be re-collected from the -network. Everything persisted in the ten tables survives, except `PendingDataColumns`, which is cleared outright: its only index is in memory. +network. Everything persisted in the eleven tables survives, except `PendingDataColumns`, which is cleared outright: its only index is in memory. ## Startup and Restore @@ -723,7 +782,10 @@ reading is zero there. `init_beacon`'s own atomic batch is the beacon-directory keys listed under [Metadata](#metadata) above; the anchor's `States`/`BlockHeaders`/`BlockBodies` entries are written by its caller instead, through the same `insert_state` and -`insert_signed_block` an ordinary block import uses. +`insert_signed_block` an ordinary block import uses, followed by the anchor's +own `BeaconUnrealizedJustifications` row (`set_unrealized_justification`), the +same order every later block's import follows too — see +[BeaconUnrealizedJustifications](#beaconunrealizedjustifications) above. `from_db_state` is the restore path: it reads `db_version`, `preset`, `chain`, `config` and `anchor_slot` from `Metadata`, returning `None` for an empty DB. A format From 9e8d58838997a0ffcd596315ac9fe2c4da4e9897 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Fri, 25 Sep 2026 12:37:58 -0300 Subject: [PATCH 2/2] fix(beacon): fall back to the store's justified checkpoint on a missing unrealized justification After the persistence fix, a missing unrealized justification is only possible in one narrow crash window: the state writer flushes a block's post-state, the process dies before that block's own entry lands, and resume's has_state check then skips re-importing it. Raising SpecAssert on that miss left get_voting_source (and, through it, filter_block_tree/ get_head) permanently unable to resolve a head through that leaf, for as long as it stayed a leaf. Prysm treats a rebuilt node's forkchoice store the same way: every block it seeds at startup gets the store's current justified checkpoint rather than a freshly computed unrealized one (buildForkchoiceChain, and the doubly-linked-tree store's insert). That value is always viable in filter_block_tree, since voting_source.epoch == store.justified.epoch holds by construction, so the fallback can only make a leaf more likely to survive filtering, never change what weight it casts once it does. get_voting_source now falls back to store.beacon_justified_checkpoint() on a miss instead of raising, logging at debug (get_head runs every tick, so a warn would repeat for as long as the block stays a leaf). The fallback is read-time only: it is never written into the table or the in-memory cache, since it is an approximation of the real value, and persisting it would later be read back as exact. is_ffg_competitive keeps raising on a miss; nothing in production calls it. Doc comments in tables.rs, store.rs and docs/data_storage.md that described a miss as fatal are updated to describe the crash window and the fallback. Store::delete_unrealized_justification is a new test-only helper that removes one entry from both the table and the cache, for simulating that crash window without a whole-process restart. --- .../src/beacon/fork_choice.rs | 117 +++++++++++++++++- crates/storage/src/api/tables.rs | 8 ++ crates/storage/src/store.rs | 28 +++++ docs/data_storage.md | 15 ++- 4 files changed, 157 insertions(+), 11 deletions(-) diff --git a/crates/blockchain/state_transition/src/beacon/fork_choice.rs b/crates/blockchain/state_transition/src/beacon/fork_choice.rs index 0747926e..c97a95d1 100644 --- a/crates/blockchain/state_transition/src/beacon/fork_choice.rs +++ b/crates/blockchain/state_transition/src/beacon/fork_choice.rs @@ -160,7 +160,7 @@ use std::sync::Arc; use ethlambda_storage::{CacheKey, ForkCheckpoints, StorageBackend}; use ethlambda_types::ShortRoot; -use tracing::{error, warn}; +use tracing::{debug, error, warn}; use crate::beacon::config::Config; use crate::beacon::constants; @@ -1296,6 +1296,21 @@ pub fn compute_weights( /// `current_justified_checkpoint` happened to be at the time it was /// processed; a block from the current epoch has no unrealized value to pull /// up to yet, so its own post-state's checkpoint is used directly. +/// +/// A missing unrealized justification is the specification's "unhandled +/// exception" case, and `Store::unrealized_justification`'s own doc comment +/// says when this build can actually produce one: a narrow crash window +/// between a block's `insert_signed_block`/`insert_state` committing and +/// `compute_pulled_up_tip`'s own write landing, which `has_state` then skips +/// re-importing on resume. Rather than raise here (which would freeze +/// `get_head` on that one leaf for as long as it stays a leaf), this falls +/// back to `store.beacon_justified_checkpoint()`, the way Prysm seeds every +/// block it rebuilds at startup (`buildForkchoiceChain` / +/// doubly-linked-tree's `insert`). That value is always viable in +/// `filter_block_tree` (`voting_source.epoch == store.justified.epoch`), so +/// the fallback can only ever make a block *more* likely to survive +/// filtering, never change what weight it casts once it does: this function +/// answers "does this leaf count", not "how much". pub fn get_voting_source( store: &Store, index: &HashMap, @@ -1309,11 +1324,22 @@ pub fn get_voting_source( let block_epoch = compute_epoch_at_slot(block_slot); if current_epoch > block_epoch { - store - .unrealized_justification(&block_root) - .ok_or(Error::SpecAssert( - "block_root in store.unrealized_justifications", - )) + if let Some(checkpoint) = store.unrealized_justification(&block_root) { + return Ok(checkpoint); + } + let justified = store.beacon_justified_checkpoint(); + // debug, not warn: get_head runs every tick, so a miss that persists + // while the block stays a leaf would otherwise repeat every tick + // until it is pruned or wins the head. + debug!( + block_root = %ShortRoot(&block_root.0), + block_slot, + justified_epoch = justified.epoch, + justified_root = %ShortRoot(&justified.root.0), + "No unrealized justification for a prior-epoch block; \ + falling back to the store's justified checkpoint" + ); + Ok(justified) } else { let head_state = store .get_state(&block_root) @@ -1597,6 +1623,11 @@ pub fn is_shuffling_stable(slot: Slot) -> bool { /// Whether `head_root` and `parent_root` would cast the same FFG vote if /// either were head, so that reorging one for the other costs nothing on the /// justification side. +/// +/// Unlike [`get_voting_source`], a miss here still raises: only the spec +/// tests reach this function, through `get_proposer_head`/ +/// `should_override_forkchoice_update`, neither of which any production path +/// calls yet. pub fn is_ffg_competitive(store: &Store, head_root: Root, parent_root: Root) -> Result { let head = store .unrealized_justification(&head_root) @@ -3163,6 +3194,38 @@ mod tests { ); } + /// The crash-window fallback: no `set_unrealized_justification` call was + /// ever made for `block_root`, which is what a `SpecAssert` here used to + /// raise on. `get_voting_source` must instead answer exactly + /// `store.beacon_justified_checkpoint()`, not some other computed value. + #[test] + fn get_voting_source_falls_back_to_the_justified_checkpoint_on_a_miss() { + let config = Config::active(); + let mut store = empty_store(); + // Same clock skew as + // `get_voting_source_pulls_up_a_prior_epoch_blocks_vote`, so this + // takes the same `current_epoch > block_epoch` branch. + store + .set_time_ms(seconds_to_milliseconds( + config.seconds_per_slot * preset::SLOTS_PER_EPOCH * 2, + )) + .unwrap(); + + let block_root = Root::repeat_byte(5); + store + .insert_signed_block(block_root, block(0, Root::ZERO)) + .unwrap(); + assert_eq!( + store.unrealized_justification(&block_root), + None, + "the test must exercise a genuine miss" + ); + + let voting_source = + get_voting_source(&store, &store.block_index(), block_root, &config).unwrap(); + assert_eq!(voting_source, store.beacon_justified_checkpoint()); + } + #[test] fn get_head_persists_the_head_it_computed() { let config = Config::active(); @@ -3360,6 +3423,48 @@ mod tests { ); } + /// Simulates the one crash window `Store::unrealized_justification`'s + /// doc comment describes, rather than a whole-process restart: a block + /// imports and commits normally, but its own unrealized-justification + /// write is then dropped by hand + /// ([`Store::delete_unrealized_justification`]), the way it would be + /// missing had the process died between `insert_state` and + /// `compute_pulled_up_tip`'s own commit. `get_head` must still resolve to + /// it once it becomes the only leaf, falling back rather than failing. + #[test] + fn get_head_survives_a_dropped_unrealized_justification() { + let config = Config::active(); + let (anchor_state, anchor_block) = anchor_pair(); + let anchor_slot = anchor_state.slot(); + let anchor_root = anchor_block.message_hash_tree_root(); + let backend: Arc = Arc::new(InMemoryBackend::new()); + let mut store = + get_forkchoice_store(backend.clone(), anchor_state.clone(), anchor_block, &config) + .expect("the pair matches"); + + let post_state = post_state_over(&anchor_state, anchor_root); + let a = import_unchecked( + &mut store, + block(anchor_slot + 1, anchor_root), + post_state, + &config, + ); + store.delete_unrealized_justification(a); + assert_eq!( + store.unrealized_justification(&a), + None, + "the test must exercise a genuine miss, table and cache both" + ); + + on_tick(&mut store, two_epochs_past(anchor_slot, &config), &config); + let head = get_head(&mut store, &config).map_err(|err| format!("{err:?}")); + assert_eq!( + head, + Ok(a), + "a dropped unrealized justification must fall back rather than fail get_head: {head:?}" + ); + } + /// The specification's assertion cannot hold for a checkpoint-synced /// anchor: the finalized state sits at the epoch boundary, so when that /// slot was empty it has advanced past its own `latest_block_header` and diff --git a/crates/storage/src/api/tables.rs b/crates/storage/src/api/tables.rs index 0fb6c052..42f74fc0 100644 --- a/crates/storage/src/api/tables.rs +++ b/crates/storage/src/api/tables.rs @@ -74,6 +74,14 @@ pub enum Table { /// write-through cache over it, since `get_voting_source` reads this for /// every block from a prior epoch. /// + /// A miss is now possible only in one narrow crash window: the store + /// writer flushes a block's post-state, the process dies before that + /// block's own row here lands, and resume's `has_state` check then skips + /// re-importing it. `get_voting_source` no longer raises on that miss; it + /// falls back to the store's justified checkpoint (see its own doc + /// comment). `is_ffg_competitive` still raises, since nothing in + /// production calls it. + /// /// Keyed by root alone rather than `slot ‖ root` (`encode_slot_root_key`, /// as `LiveChain`/`BlockProof` are): both readers of this table /// (`get_voting_source`, `is_ffg_competitive`) look up a root with no slot diff --git a/crates/storage/src/store.rs b/crates/storage/src/store.rs index 770a95ae..03a2dd3b 100644 --- a/crates/storage/src/store.rs +++ b/crates/storage/src/store.rs @@ -3491,6 +3491,10 @@ impl Store { /// which is what lets this answer correctly right after a restart, before /// anything has repopulated the cache: see that table's own doc comment /// for the `get_head` freeze a purely in-memory map used to cause. + /// + /// `None` can still come back genuinely, in the narrow crash window that + /// table's doc comment describes; `get_voting_source` is the caller that + /// falls back for it rather than treating this as unreachable. pub fn unrealized_justification(&self, root: &H256) -> Option { if let Some(checkpoint) = self .beacon @@ -3613,6 +3617,30 @@ impl Store { Ok(count) } + /// Deletes `root`'s unrealized-justification row and its in-memory cache + /// entry, without touching anything else. + /// + /// Not a production writer: `prune_unrealized_justifications` is the only + /// other thing that removes a row, and it always removes a whole slot + /// range at once, never one arbitrary root. This exists for tests that + /// need to simulate the one crash window + /// [`Store::unrealized_justification`]'s own doc comment describes, where + /// a block's post-state committed but its unrealized-justification write + /// never landed. + pub fn delete_unrealized_justification(&mut self, root: H256) { + let mut batch = self.backend.begin_write().expect("write batch"); + batch + .delete_batch(Table::BeaconUnrealizedJustifications, vec![root.to_ssz()]) + .expect("delete unrealized justification"); + batch.commit().expect("commit"); + + self.beacon + .lock() + .unwrap() + .unrealized_justifications + .remove(&root); + } + // ============ Data Columns ============ // // Written on arrival (after verification), not at block import: the diff --git a/docs/data_storage.md b/docs/data_storage.md index faf1fe13..1a48a8dd 100644 --- a/docs/data_storage.md +++ b/docs/data_storage.md @@ -538,11 +538,16 @@ block. Both call `Store::set_unrealized_justification` after the block's own `insert_signed_block`/`insert_state` have already run — `compute_pulled_up_tip` needs the block's post-state, which does not exist until those have — so the write is not in the same atomic batch as the block's import. A crash between -the two leaves that one block without an unrealized-justification entry, -which surfaces as the same `SpecAssert` an older, unpatched build would raise -on every block: a narrower version of the bug this table exists to fix, -scoped to the single most-recently-imported block rather than the whole -unfinalized window. +the two leaves that one block without an unrealized-justification entry; +`has_state` then skips re-importing it on resume, so nothing ever fills the +row in afterward either. `get_voting_source` no longer treats that miss as +fatal: it falls back to `store.beacon_justified_checkpoint()`, the way Prysm +seeds a rebuilt node's forkchoice store, and logs at `debug` rather than +raising. That is safe because the fallback only affects whether the block +counts as viable in `filter_block_tree` (`voting_source.epoch == +store.justified.epoch` is always true for it), never how much weight it +casts once it does. `is_ffg_competitive` still raises on a miss: nothing in +production calls it. Pruned on finalization, on the same horizon `LiveChain` is (the finalized block's own slot, read from `BlockHeaders` via `block_entry`, not the stored