diff --git a/beacon_node/beacon_chain/src/migrate.rs b/beacon_node/beacon_chain/src/migrate.rs index 59079344c9..83eae0ae71 100644 --- a/beacon_node/beacon_chain/src/migrate.rs +++ b/beacon_node/beacon_chain/src/migrate.rs @@ -202,11 +202,6 @@ impl, Cold: ItemStore> Migrate old_finalized_block_hash: SignedBeaconBlockHash, new_finalized_block_hash: SignedBeaconBlockHash, ) { - if let Err(e) = process_finalization(self.db.clone(), state_root, &new_finalized_state) { - // This migrator is only used for testing, so we just log to stderr without a logger. - eprintln!("Migration error: {:?}", e); - } - if let Err(e) = Self::prune_abandoned_forks( self.db.clone(), head_tracker, @@ -216,6 +211,11 @@ impl, Cold: ItemStore> Migrate ) { eprintln!("Pruning error: {:?}", e); } + + if let Err(e) = process_finalization(self.db.clone(), state_root, &new_finalized_state) { + // This migrator is only used for testing, so we just log to stderr without a logger. + eprintln!("Migration error: {:?}", e); + } } } @@ -325,6 +325,17 @@ impl, Cold: ItemStore> BackgroundMigrator {} + Err(e) => warn!(log, "Block pruning failed: {:?}", e), + } + match process_finalization(db.clone(), state_root, &state) { Ok(()) => {} Err(Error::HotColdDBError(HotColdDBError::FreezeSlotUnaligned(slot))) => { @@ -342,17 +353,6 @@ impl, Cold: ItemStore> BackgroundMigrator {} - Err(e) => warn!(log, "Block pruning failed: {:?}", e), - } } }); diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 6ab2908bce..bafef37882 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -743,7 +743,7 @@ fn prunes_abandoned_fork_between_two_finalized_checkpoints() { let (stray_blocks, stray_states, _, stray_head, _) = harness.add_stray_blocks( harness.get_head_state(), slot, - slots_per_epoch - 1, + slots_per_epoch - 3, &faulty_validators, ); diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index 48917af735..7bad9ef156 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -208,6 +208,13 @@ impl, Cold: ItemStore> HotColdDB } /// Fetch a state from the store. + /// + /// If `slot` is provided then it will be used as a hint as to which database should + /// be checked. Importantly, if the slot hint is provided and indicates a slot that lies + /// in the freezer database, then only the freezer database will be accessed and `Ok(None)` + /// will be returned if the provided `state_root` doesn't match the state root of the + /// frozen state at `slot`. Consequently, if a state from a non-canonical chain is desired, it's + /// best to set `slot` to `None`, or call `load_hot_state` directly. pub fn get_state( &self, state_root: &Hash256, @@ -217,7 +224,11 @@ impl, Cold: ItemStore> HotColdDB if let Some(slot) = slot { if slot < self.get_split_slot() { - self.load_cold_state_by_slot(slot).map(Some) + // Although we could avoid a DB lookup by shooting straight for the + // frozen state using `load_cold_state_by_slot`, that would be incorrect + // in the case where the caller provides a `state_root` that's off the canonical + // chain. This way we avoid returning a state that doesn't match `state_root`. + self.load_cold_state(state_root) } else { self.load_hot_state(state_root) }