diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs index aa24d08b2d6..9c8ec92e596 100644 --- a/beacon_node/beacon_chain/src/attestation_verification.rs +++ b/beacon_node/beacon_chain/src/attestation_verification.rs @@ -775,7 +775,12 @@ where metrics::start_timer(&metrics::ATTESTATION_PROCESSING_STATE_READ_TIMES); let mut state = chain - .get_state(&target_block.state_root, Some(target_block.slot))? + .store + .get_inconsistent_state_for_attestation_verification_only( + &target_block.state_root, + Some(target_block.slot), + ) + .map_err(BeaconChainError::from)? .ok_or_else(|| BeaconChainError::MissingBeaconState(target_block.state_root))?; metrics::stop_timer(state_read_timer); diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 38acf1b1799..92460dba125 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -390,9 +390,22 @@ impl GossipVerifiedBlock { }); } + let block_root = get_block_root(&block); + // Do not gossip a block from a finalized slot. check_block_against_finalized_slot(&block.message, chain)?; + // Check if the block is already known. We know it is post-finalization, so it is + // sufficient to check the fork choice. + // + // In normal operation this isn't necessary, however it is useful immediately after a + // reboot if the `observed_block_producers` cache is empty. In that case, without this + // check, we will load the parent and state from disk only to find out later that we + // already know this block. + if chain.fork_choice.read().contains_block(&block_root) { + return Err(BlockError::BlockIsAlreadyKnown); + } + // Check that we have not already received a block with a valid signature for this slot. if chain .observed_block_producers @@ -415,7 +428,6 @@ impl GossipVerifiedBlock { )?; let (mut parent, block) = load_parent(block, chain)?; - let block_root = get_block_root(&block); let state = cheap_state_advance_to_obtain_committees( &mut parent.beacon_state, @@ -672,7 +684,10 @@ impl<'a, T: BeaconChainTypes> FullyVerifiedBlock<'a, T> { let state_root = state.update_tree_hash_cache()?; let op = if state.slot % T::EthSpec::slots_per_epoch() == 0 { - StoreOp::PutState(state_root.into(), Cow::Owned(state.clone())) + StoreOp::PutState( + state_root.into(), + Cow::Owned(state.clone_with(CloneConfig::committee_caches_only())), + ) } else { StoreOp::PutStateSummary( state_root.into(), diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index 7bad9ef1567..692e747d79f 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -30,6 +30,16 @@ use types::*; /// 32-byte key for accessing the `split` of the freezer DB. pub const SPLIT_DB_KEY: &str = "FREEZERDBSPLITFREEZERDBSPLITFREE"; +/// Defines how blocks should be replayed on states. +#[derive(PartialEq)] +pub enum BlockReplay { + /// Perform all transitions faithfully to the specification. + Accurate, + /// Don't compute state roots, eventually computing an invalid beacon state that can only be + /// used for obtaining shuffling. + InconsistentStateRoots, +} + /// On-disk database that stores finalized states efficiently. /// /// Stores vector fields like the `block_roots` and `state_roots` separately, and only stores @@ -230,16 +240,40 @@ impl, Cold: ItemStore> HotColdDB // 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) + self.load_hot_state(state_root, BlockReplay::Accurate) } } else { - match self.load_hot_state(state_root)? { + match self.load_hot_state(state_root, BlockReplay::Accurate)? { Some(state) => Ok(Some(state)), None => self.load_cold_state(state_root), } } } + /// Fetch a state from the store, but don't compute all of the values when replaying blocks + /// upon that state (e.g., state roots). Additionally, only states from the hot store are + /// returned. + /// + /// See `Self::get_state` for information about `slot`. + /// + /// ## Warning + /// + /// The returned state **is not a valid beacon state**, it can only be used for obtaining + /// shuffling to process attestations. + pub fn get_inconsistent_state_for_attestation_verification_only( + &self, + state_root: &Hash256, + slot: Option, + ) -> Result>, Error> { + metrics::inc_counter(&metrics::BEACON_STATE_GET_COUNT); + + if slot.map_or(false, |slot| slot < self.get_split_slot()) { + Ok(None) + } else { + self.load_hot_state(state_root, BlockReplay::InconsistentStateRoots) + } + } + /// Delete a state, ensuring it is removed from the LRU cache, as well as from on-disk. /// /// It is assumed that all states being deleted reside in the hot DB, even if their slot is less @@ -283,8 +317,11 @@ impl, Cold: ItemStore> HotColdDB }) = self.load_hot_state_summary(state_root)? { // NOTE: minor inefficiency here because we load an unnecessary hot state summary + // + // `BlockReplay` should be irrelevant here since we never replay blocks for an epoch + // boundary state in the hot DB. let state = self - .load_hot_state(&epoch_boundary_state_root)? + .load_hot_state(&epoch_boundary_state_root, BlockReplay::Accurate)? .ok_or_else(|| { HotColdDBError::MissingEpochBoundaryState(epoch_boundary_state_root) })?; @@ -415,7 +452,11 @@ impl, Cold: ItemStore> HotColdDB /// Load a post-finalization state from the hot database. /// /// Will replay blocks from the nearest epoch boundary. - pub fn load_hot_state(&self, state_root: &Hash256) -> Result>, Error> { + pub fn load_hot_state( + &self, + state_root: &Hash256, + block_replay: BlockReplay, + ) -> Result>, Error> { metrics::inc_counter(&metrics::BEACON_STATE_HOT_GET_COUNT); if let Some(HotStateSummary { @@ -436,7 +477,7 @@ impl, Cold: ItemStore> HotColdDB } else { let blocks = self.load_blocks_to_replay(boundary_state.slot, slot, latest_block_root)?; - self.replay_blocks(boundary_state, blocks, slot)? + self.replay_blocks(boundary_state, blocks, slot, block_replay)? }; Ok(Some(state)) @@ -567,7 +608,7 @@ impl, Cold: ItemStore> HotColdDB )?; // 3. Replay the blocks on top of the low restore point. - self.replay_blocks(low_restore_point, blocks, slot) + self.replay_blocks(low_restore_point, blocks, slot, BlockReplay::Accurate) } /// Get a suitable block root for backtracking from `high_restore_point` to the state at `slot`. @@ -624,9 +665,19 @@ impl, Cold: ItemStore> HotColdDB fn replay_blocks( &self, mut state: BeaconState, - blocks: Vec>, + mut blocks: Vec>, target_slot: Slot, + block_replay: BlockReplay, ) -> Result, Error> { + if block_replay == BlockReplay::InconsistentStateRoots { + for i in 0..blocks.len() { + blocks[i].message.state_root = Hash256::zero(); + if i > 0 { + blocks[i].message.parent_root = blocks[i - 1].canonical_root() + } + } + } + let state_root_from_prev_block = |i: usize, state: &BeaconState| { if i > 0 { let prev_block = &blocks[i - 1].message; @@ -646,10 +697,14 @@ impl, Cold: ItemStore> HotColdDB } while state.slot < block.message.slot { - let state_root = state_root_from_prev_block(i, &state); + let state_root = match block_replay { + BlockReplay::Accurate => state_root_from_prev_block(i, &state), + BlockReplay::InconsistentStateRoots => Some(Hash256::zero()), + }; per_slot_processing(&mut state, state_root, &self.spec) .map_err(HotColdDBError::BlockReplaySlotError)?; } + per_block_processing( &mut state, &block, @@ -661,7 +716,10 @@ impl, Cold: ItemStore> HotColdDB } while state.slot < target_slot { - let state_root = state_root_from_prev_block(blocks.len(), &state); + let state_root = match block_replay { + BlockReplay::Accurate => state_root_from_prev_block(blocks.len(), &state), + BlockReplay::InconsistentStateRoots => Some(Hash256::zero()), + }; per_slot_processing(&mut state, state_root, &self.spec) .map_err(HotColdDBError::BlockReplaySlotError)?; } diff --git a/consensus/types/src/beacon_state/tree_hash_cache.rs b/consensus/types/src/beacon_state/tree_hash_cache.rs index 0c8899c0255..6abc795a137 100644 --- a/consensus/types/src/beacon_state/tree_hash_cache.rs +++ b/consensus/types/src/beacon_state/tree_hash_cache.rs @@ -7,6 +7,7 @@ use rayon::prelude::*; use ssz_derive::{Decode, Encode}; use ssz_types::VariableList; use std::cmp::Ordering; +use std::iter::ExactSizeIterator; use tree_hash::{mix_in_length, MerkleHasher, TreeHash}; /// The number of fields on a beacon state. @@ -288,17 +289,17 @@ impl ValidatorsListTreeHashCache { fn recalculate_tree_hash_root(&mut self, validators: &[Validator]) -> Result { let mut list_arena = std::mem::take(&mut self.list_arena); - let leaves = self - .values - .leaves(validators)? - .into_iter() - .flatten() - .map(|h| h.to_fixed_bytes()) - .collect::>(); + let leaves = self.values.leaves(validators)?; + let num_leaves = leaves.iter().map(|arena| arena.len()).sum(); + + let leaves_iter = ForcedExactSizeIterator { + iter: leaves.into_iter().flatten().map(|h| h.to_fixed_bytes()), + len: num_leaves, + }; let list_root = self .list_cache - .recalculate_merkle_root(&mut list_arena, leaves.into_iter())?; + .recalculate_merkle_root(&mut list_arena, leaves_iter)?; self.list_arena = list_arena; @@ -306,6 +307,29 @@ impl ValidatorsListTreeHashCache { } } +/// Provides a wrapper around some `iter` if the number of items in the iterator is known to the +/// programmer but not the compiler. This allows use of `ExactSizeIterator` in some occasions. +/// +/// Care should be taken to ensure `len` is accurate. +struct ForcedExactSizeIterator { + iter: I, + len: usize, +} + +impl> Iterator for ForcedExactSizeIterator { + type Item = V; + + fn next(&mut self) -> Option { + self.iter.next() + } +} + +impl> ExactSizeIterator for ForcedExactSizeIterator { + fn len(&self) -> usize { + self.len + } +} + /// Provides a cache for each of the `Validator` objects in `state.validators` and computes the /// roots of these using Rayon parallelization. #[derive(Debug, PartialEq, Clone, Default, Encode, Decode)]