diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 1f36e0e65ac..0dbff198181 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -69,7 +69,7 @@ use state_processing::{ per_block_processing::{errors::AttestationValidationError, is_merge_transition_complete}, per_slot_processing, state_advance::{complete_state_advance, partial_state_advance}, - BlockSignatureStrategy, SigVerifiedOp, + BlockSignatureStrategy, SigVerifiedOp, VerifyBlockRoot, }; use std::borrow::Cow; use std::cmp::Ordering; @@ -488,7 +488,7 @@ impl BeaconChain { pub fn forwards_iter_block_roots( &self, start_slot: Slot, - ) -> Result>, Error> { + ) -> Result> + '_, Error> { let oldest_block_slot = self.store.get_oldest_block_slot(); if start_slot < oldest_block_slot { return Err(Error::HistoricalBlockError( @@ -501,8 +501,7 @@ impl BeaconChain { let local_head = self.head()?; - let iter = HotColdDB::forwards_block_roots_iterator( - self.store.clone(), + let iter = self.store.forwards_block_roots_iterator( start_slot, local_head.beacon_state, local_head.beacon_block_root, @@ -512,6 +511,43 @@ impl BeaconChain { Ok(iter.map(|result| result.map_err(Into::into))) } + /// Even more efficient variant of `forwards_iter_block_roots` that will avoid cloning the head + /// state if it isn't required for the requested range of blocks. + pub fn forwards_iter_block_roots_until( + &self, + start_slot: Slot, + end_slot: Slot, + ) -> Result> + '_, Error> { + let oldest_block_slot = self.store.get_oldest_block_slot(); + if start_slot < oldest_block_slot { + return Err(Error::HistoricalBlockError( + HistoricalBlockError::BlockOutOfRange { + slot: start_slot, + oldest_block_slot, + }, + )); + } + + self.with_head(move |head| { + let iter = self.store.forwards_block_roots_iterator_until( + start_slot, + end_slot, + || { + ( + head.beacon_state.clone_with_only_committee_caches(), + head.beacon_block_root, + ) + }, + &self.spec, + )?; + Ok(iter + .map(|result| result.map_err(Into::into)) + .take_while(move |result| { + result.as_ref().map_or(true, |(_, slot)| *slot <= end_slot) + })) + }) + } + /// Traverse backwards from `block_root` to find the block roots of its ancestors. /// /// ## Notes @@ -524,14 +560,14 @@ impl BeaconChain { pub fn rev_iter_block_roots_from( &self, block_root: Hash256, - ) -> Result>, Error> { + ) -> Result> + '_, Error> { let block = self .get_block(&block_root)? .ok_or(Error::MissingBeaconBlock(block_root))?; let state = self .get_state(&block.state_root(), Some(block.slot()))? .ok_or_else(|| Error::MissingBeaconState(block.state_root()))?; - let iter = BlockRootsIterator::owned(self.store.clone(), state); + let iter = BlockRootsIterator::owned(&self.store, state); Ok(std::iter::once(Ok((block_root, block.slot()))) .chain(iter) .map(|result| result.map_err(|e| e.into()))) @@ -618,12 +654,12 @@ impl BeaconChain { /// - As this iterator starts at the `head` of the chain (viz., the best block), the first slot /// returned may be earlier than the wall-clock slot. pub fn rev_iter_state_roots_from<'a>( - &self, + &'a self, state_root: Hash256, state: &'a BeaconState, ) -> impl Iterator> + 'a { std::iter::once(Ok((state_root, state.slot()))) - .chain(StateRootsIterator::new(self.store.clone(), state)) + .chain(StateRootsIterator::new(&self.store, state)) .map(|result| result.map_err(Into::into)) } @@ -637,11 +673,10 @@ impl BeaconChain { pub fn forwards_iter_state_roots( &self, start_slot: Slot, - ) -> Result>, Error> { + ) -> Result> + '_, Error> { let local_head = self.head()?; - let iter = HotColdDB::forwards_state_roots_iterator( - self.store.clone(), + let iter = self.store.forwards_state_roots_iterator( start_slot, local_head.beacon_state_root(), local_head.beacon_state, @@ -651,6 +686,36 @@ impl BeaconChain { Ok(iter.map(|result| result.map_err(Into::into))) } + /// Super-efficient forwards state roots iterator that avoids cloning the head if the state + /// roots lie entirely within the freezer database. + /// + /// The iterator returned will include roots for `start_slot..=end_slot`, i.e. it + /// is endpoint inclusive. + pub fn forwards_iter_state_roots_until( + &self, + start_slot: Slot, + end_slot: Slot, + ) -> Result> + '_, Error> { + self.with_head(move |head| { + let iter = self.store.forwards_state_roots_iterator_until( + start_slot, + end_slot, + || { + ( + head.beacon_state.clone_with_only_committee_caches(), + head.beacon_state_root(), + ) + }, + &self.spec, + )?; + Ok(iter + .map(|result| result.map_err(Into::into)) + .take_while(move |result| { + result.as_ref().map_or(true, |(_, slot)| *slot <= end_slot) + })) + }) + } + /// Returns the block at the given slot, if any. Only returns blocks in the canonical chain. /// /// Use the `skips` parameter to define the behaviour when `request_slot` is a skipped slot. @@ -708,18 +773,21 @@ impl BeaconChain { return Ok(Some(root)); } - process_results(self.forwards_iter_state_roots(request_slot)?, |mut iter| { - if let Some((root, slot)) = iter.next() { - if slot == request_slot { - Ok(Some(root)) + process_results( + self.forwards_iter_state_roots_until(request_slot, request_slot)?, + |mut iter| { + if let Some((root, slot)) = iter.next() { + if slot == request_slot { + Ok(Some(root)) + } else { + // Sanity check. + Err(Error::InconsistentForwardsIter { request_slot, slot }) + } } else { - // Sanity check. - Err(Error::InconsistentForwardsIter { request_slot, slot }) + Ok(None) } - } else { - Ok(None) - } - })? + }, + )? } /// Returns the block root at the given slot, if any. Only returns roots in the canonical chain. @@ -790,11 +858,10 @@ impl BeaconChain { return Ok(root_opt); } - if let Some(((prev_root, _), (curr_root, curr_slot))) = - process_results(self.forwards_iter_block_roots(prev_slot)?, |iter| { - iter.tuple_windows().next() - })? - { + if let Some(((prev_root, _), (curr_root, curr_slot))) = process_results( + self.forwards_iter_block_roots_until(prev_slot, request_slot)?, + |iter| iter.tuple_windows().next(), + )? { // Sanity check. if curr_slot != request_slot { return Err(Error::InconsistentForwardsIter { @@ -842,18 +909,21 @@ impl BeaconChain { return Ok(Some(root)); } - process_results(self.forwards_iter_block_roots(request_slot)?, |mut iter| { - if let Some((root, slot)) = iter.next() { - if slot == request_slot { - Ok(Some(root)) + process_results( + self.forwards_iter_block_roots_until(request_slot, request_slot)?, + |mut iter| { + if let Some((root, slot)) = iter.next() { + if slot == request_slot { + Ok(Some(root)) + } else { + // Sanity check. + Err(Error::InconsistentForwardsIter { request_slot, slot }) + } } else { - // Sanity check. - Err(Error::InconsistentForwardsIter { request_slot, slot }) + Ok(None) } - } else { - Ok(None) - } - })? + }, + )? } /// Returns the block at the given root, if any. @@ -1112,12 +1182,13 @@ impl BeaconChain { Ok(state) } Ordering::Less => { - let state_root = process_results(self.forwards_iter_state_roots(slot)?, |iter| { - iter.take_while(|(_, current_slot)| *current_slot >= slot) - .find(|(_, current_slot)| *current_slot == slot) - .map(|(root, _slot)| root) - })? - .ok_or(Error::NoStateForSlot(slot))?; + let state_root = + process_results(self.forwards_iter_state_roots_until(slot, slot)?, |iter| { + iter.take_while(|(_, current_slot)| *current_slot >= slot) + .find(|(_, current_slot)| *current_slot == slot) + .map(|(root, _slot)| root) + })? + .ok_or(Error::NoStateForSlot(slot))?; Ok(self .get_state(&state_root, Some(slot))? @@ -1256,7 +1327,7 @@ impl BeaconChain { beacon_block_root: Hash256, state: &BeaconState, ) -> Result, Error> { - let iter = BlockRootsIterator::new(self.store.clone(), state); + let iter = BlockRootsIterator::new(&self.store, state); let iter_with_head = std::iter::once(Ok((beacon_block_root, state.slot()))) .chain(iter) .map(|result| result.map_err(|e| e.into())); @@ -2983,6 +3054,7 @@ impl BeaconChain { &block, None, BlockSignatureStrategy::VerifyRandao, + VerifyBlockRoot::True, &self.spec, )?; drop(process_timer); @@ -3324,7 +3396,7 @@ impl BeaconChain { .epoch .start_slot(T::EthSpec::slots_per_epoch()); let new_finalized_state_root = process_results( - StateRootsIterator::new(self.store.clone(), &head.beacon_state), + StateRootsIterator::new(&self.store, &head.beacon_state), |mut iter| { iter.find_map(|(state_root, slot)| { if slot == new_finalized_slot { diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 83eb14bf765..c6d937c81e9 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -65,7 +65,7 @@ use state_processing::{ block_signature_verifier::{BlockSignatureVerifier, Error as BlockSignatureVerifierError}, per_block_processing, per_slot_processing, state_advance::partial_state_advance, - BlockProcessingError, BlockSignatureStrategy, SlotProcessingError, + BlockProcessingError, BlockSignatureStrategy, SlotProcessingError, VerifyBlockRoot, }; use std::borrow::Cow; use std::fs; @@ -1185,6 +1185,7 @@ impl<'a, T: BeaconChainTypes> FullyVerifiedBlock<'a, T> { Some(block_root), // Signatures were verified earlier in this function. BlockSignatureStrategy::NoVerification, + VerifyBlockRoot::True, &chain.spec, ) { match err { diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index 6b9af787d70..70e288ec265 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -20,7 +20,7 @@ use state_processing::{ }, signature_sets::Error as SignatureSetError, state_advance::Error as StateAdvanceError, - BlockProcessingError, SlotProcessingError, + BlockProcessingError, BlockReplayError, SlotProcessingError, }; use std::time::Duration; use task_executor::ShutdownReason; @@ -86,6 +86,7 @@ pub enum BeaconChainError { ValidatorPubkeyCacheIncomplete(usize), SignatureSetError(SignatureSetError), BlockSignatureVerifierError(state_processing::block_signature_verifier::Error), + BlockReplayError(BlockReplayError), DuplicateValidatorPublicKey, ValidatorPubkeyCacheFileError(String), ValidatorIndexUnknown(usize), @@ -160,6 +161,7 @@ easy_from_to!(ArithError, BeaconChainError); easy_from_to!(ForkChoiceStoreError, BeaconChainError); easy_from_to!(HistoricalBlockError, BeaconChainError); easy_from_to!(StateAdvanceError, BeaconChainError); +easy_from_to!(BlockReplayError, BeaconChainError); #[derive(Debug)] pub enum BlockProductionError { diff --git a/beacon_node/beacon_chain/src/fork_revert.rs b/beacon_node/beacon_chain/src/fork_revert.rs index 880eb8e67a4..3ae3bf8a3eb 100644 --- a/beacon_node/beacon_chain/src/fork_revert.rs +++ b/beacon_node/beacon_chain/src/fork_revert.rs @@ -3,7 +3,9 @@ use fork_choice::{ForkChoice, PayloadVerificationStatus}; use itertools::process_results; use slog::{info, warn, Logger}; use state_processing::state_advance::complete_state_advance; -use state_processing::{per_block_processing, per_block_processing::BlockSignatureStrategy}; +use state_processing::{ + per_block_processing, per_block_processing::BlockSignatureStrategy, VerifyBlockRoot, +}; use std::sync::Arc; use std::time::Duration; use store::{iter::ParentRootBlockIterator, HotColdDB, ItemStore}; @@ -161,6 +163,7 @@ pub fn reset_fork_choice_to_finalization, Cold: It &block, None, BlockSignatureStrategy::NoVerification, + VerifyBlockRoot::True, spec, ) .map_err(|e| format!("Error replaying block: {:?}", e))?; diff --git a/beacon_node/beacon_chain/src/migrate.rs b/beacon_node/beacon_chain/src/migrate.rs index b2a925bb779..5ae76273211 100644 --- a/beacon_node/beacon_chain/src/migrate.rs +++ b/beacon_node/beacon_chain/src/migrate.rs @@ -360,13 +360,11 @@ impl, Cold: ItemStore> BackgroundMigrator= old_finalized_slot) @@ -416,7 +414,7 @@ impl, Cold: ItemStore> BackgroundMigrator( // Iterate backwards from the given `head_root` and `head_slot` and find the block root at each epoch. let mut iter = std::iter::once(Ok((head_root, head_slot))) - .chain(BlockRootsIterator::from_block(db, head_root).map_err(|e| format!("{:?}", e))?); + .chain(BlockRootsIterator::from_block(&db, head_root).map_err(|e| format!("{:?}", e))?); let mut roots_by_epoch = HashMap::new(); for epoch in relevant_epochs { let start_slot = epoch.start_slot(T::EthSpec::slots_per_epoch()); diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 25995616ddf..574895296dd 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -31,13 +31,13 @@ use rayon::prelude::*; use sensitive_url::SensitiveUrl; use slog::Logger; use slot_clock::TestingSlotClock; -use state_processing::state_advance::complete_state_advance; +use state_processing::{state_advance::complete_state_advance, StateRootStrategy}; use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::str::FromStr; use std::sync::Arc; use std::time::Duration; -use store::{config::StoreConfig, BlockReplay, HotColdDB, ItemStore, LevelDB, MemoryStore}; +use store::{config::StoreConfig, HotColdDB, ItemStore, LevelDB, MemoryStore}; use task_executor::ShutdownReason; use tree_hash::TreeHash; use types::sync_selection_proof::SyncSelectionProof; @@ -527,7 +527,7 @@ where pub fn get_hot_state(&self, state_hash: BeaconStateHash) -> Option> { self.chain .store - .load_hot_state(&state_hash.into(), BlockReplay::Accurate) + .load_hot_state(&state_hash.into(), StateRootStrategy::Accurate) .unwrap() } diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index e3fd4de1b4a..9b97e3c7dcf 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -10,7 +10,7 @@ use slasher::{Config as SlasherConfig, Slasher}; use state_processing::{ common::get_indexed_attestation, per_block_processing::{per_block_processing, BlockSignatureStrategy}, - per_slot_processing, BlockProcessingError, + per_slot_processing, BlockProcessingError, VerifyBlockRoot, }; use std::sync::Arc; use tempfile::tempdir; @@ -978,6 +978,7 @@ fn add_base_block_to_altair_chain() { &base_block, None, BlockSignatureStrategy::NoVerification, + VerifyBlockRoot::True, &harness.chain.spec, ), Err(BlockProcessingError::InconsistentBlockFork( @@ -1096,6 +1097,7 @@ fn add_altair_block_to_base_chain() { &altair_block, None, BlockSignatureStrategy::NoVerification, + VerifyBlockRoot::True, &harness.chain.spec, ), Err(BlockProcessingError::InconsistentBlockFork( diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 24aba9e207c..5c020df492f 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -14,6 +14,7 @@ use lazy_static::lazy_static; use logging::test_logger; use maplit::hashset; use rand::Rng; +use state_processing::BlockReplayer; use std::collections::HashMap; use std::collections::HashSet; use std::convert::TryInto; @@ -126,7 +127,7 @@ fn randomised_skips() { "head should be at the current slot" ); - check_split_slot(&harness, store); + check_split_slot(&harness, store.clone()); check_chain_dump(&harness, num_blocks_produced + 1); check_iterators(&harness); } @@ -358,6 +359,191 @@ fn epoch_boundary_state_attestation_processing() { assert!(checked_pre_fin); } +// Test that the `end_slot` for forwards block and state root iterators works correctly. +#[test] +fn forwards_iter_block_and_state_roots_until() { + let num_blocks_produced = E::slots_per_epoch() * 17; + let db_path = tempdir().unwrap(); + let store = get_store(&db_path); + let harness = get_harness(store.clone(), LOW_VALIDATOR_COUNT); + + let all_validators = &harness.get_all_validators(); + let (mut head_state, mut head_state_root) = harness.get_current_state_and_root(); + let head_block_root = harness.chain.head_info().unwrap().block_root; + let mut block_roots = vec![head_block_root]; + let mut state_roots = vec![head_state_root]; + + for slot in (1..=num_blocks_produced).map(Slot::from) { + let (block_root, mut state) = harness + .add_attested_block_at_slot(slot, head_state, head_state_root, all_validators) + .unwrap(); + head_state_root = state.update_tree_hash_cache().unwrap(); + head_state = state; + block_roots.push(block_root.into()); + state_roots.push(head_state_root); + } + + check_finalization(&harness, num_blocks_produced); + check_split_slot(&harness, store.clone()); + + // The last restore point slot is the point at which the hybrid forwards iterator behaviour + // changes. + let last_restore_point_slot = store.get_latest_restore_point_slot(); + assert!(last_restore_point_slot > 0); + + let chain = &harness.chain; + let head_state = harness.get_current_state(); + let head_slot = head_state.slot(); + assert_eq!(head_slot, num_blocks_produced); + + let test_range = |start_slot: Slot, end_slot: Slot| { + let mut block_root_iter = chain + .forwards_iter_block_roots_until(start_slot, end_slot) + .unwrap(); + let mut state_root_iter = chain + .forwards_iter_state_roots_until(start_slot, end_slot) + .unwrap(); + + for slot in (start_slot.as_u64()..=end_slot.as_u64()).map(Slot::new) { + let block_root = block_roots[slot.as_usize()]; + assert_eq!(block_root_iter.next().unwrap().unwrap(), (block_root, slot)); + + let state_root = state_roots[slot.as_usize()]; + assert_eq!(state_root_iter.next().unwrap().unwrap(), (state_root, slot)); + } + }; + + let split_slot = store.get_split_slot(); + assert!(split_slot > last_restore_point_slot); + + test_range(Slot::new(0), last_restore_point_slot); + test_range(last_restore_point_slot, last_restore_point_slot); + test_range(last_restore_point_slot - 1, last_restore_point_slot); + test_range(Slot::new(0), last_restore_point_slot - 1); + test_range(Slot::new(0), split_slot); + test_range(last_restore_point_slot - 1, split_slot); + test_range(Slot::new(0), head_state.slot()); +} + +#[test] +fn block_replay_with_inaccurate_state_roots() { + let num_blocks_produced = E::slots_per_epoch() * 3 + 31; + let db_path = tempdir().unwrap(); + let store = get_store(&db_path); + let harness = get_harness(store.clone(), LOW_VALIDATOR_COUNT); + let chain = &harness.chain; + + harness.extend_chain( + num_blocks_produced as usize, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::AllValidators, + ); + + // Slot must not be 0 mod 32 or else no blocks will be replayed. + let (mut head_state, head_root) = harness.get_current_state_and_root(); + assert_ne!(head_state.slot() % 32, 0); + + let mut fast_head_state = store + .get_inconsistent_state_for_attestation_verification_only( + &head_root, + Some(head_state.slot()), + ) + .unwrap() + .unwrap(); + assert_eq!(head_state.validators(), fast_head_state.validators()); + + head_state.build_all_committee_caches(&chain.spec).unwrap(); + fast_head_state + .build_all_committee_caches(&chain.spec) + .unwrap(); + + assert_eq!( + head_state + .get_cached_active_validator_indices(RelativeEpoch::Current) + .unwrap(), + fast_head_state + .get_cached_active_validator_indices(RelativeEpoch::Current) + .unwrap() + ); +} + +#[test] +fn block_replayer_hooks() { + let db_path = tempdir().unwrap(); + let store = get_store(&db_path); + let harness = get_harness(store.clone(), LOW_VALIDATOR_COUNT); + let chain = &harness.chain; + + let block_slots = vec![1, 3, 5, 10, 11, 12, 13, 14, 31, 32, 33] + .into_iter() + .map(Slot::new) + .collect::>(); + let max_slot = *block_slots.last().unwrap(); + let all_slots = (0..=max_slot.as_u64()).map(Slot::new).collect::>(); + + let (state, state_root) = harness.get_current_state_and_root(); + let all_validators = harness.get_all_validators(); + let (_, _, end_block_root, mut end_state) = harness.add_attested_blocks_at_slots( + state.clone(), + state_root, + &block_slots, + &all_validators, + ); + + let blocks = store + .load_blocks_to_replay(Slot::new(0), max_slot, end_block_root.into()) + .unwrap(); + + let mut pre_slots = vec![]; + let mut post_slots = vec![]; + let mut pre_block_slots = vec![]; + let mut post_block_slots = vec![]; + + let mut replay_state = BlockReplayer::::new(state, &chain.spec) + .pre_slot_hook(Box::new(|state| { + pre_slots.push(state.slot()); + Ok(()) + })) + .post_slot_hook(Box::new(|state, epoch_summary, is_skip_slot| { + if is_skip_slot { + assert!(!block_slots.contains(&state.slot())); + } else { + assert!(block_slots.contains(&state.slot())); + } + if state.slot() % E::slots_per_epoch() == 0 { + assert!(epoch_summary.is_some()); + } + post_slots.push(state.slot()); + Ok(()) + })) + .pre_block_hook(Box::new(|state, block| { + assert_eq!(state.slot(), block.slot()); + pre_block_slots.push(block.slot()); + Ok(()) + })) + .post_block_hook(Box::new(|state, block| { + assert_eq!(state.slot(), block.slot()); + post_block_slots.push(block.slot()); + Ok(()) + })) + .apply_blocks(blocks, None) + .unwrap() + .into_state(); + + // All but last slot seen by pre-slot hook + assert_eq!(&pre_slots, all_slots.split_last().unwrap().1); + // All but 0th slot seen by post-slot hook + assert_eq!(&post_slots, all_slots.split_first().unwrap().1); + // All blocks seen by both hooks + assert_eq!(pre_block_slots, block_slots); + assert_eq!(post_block_slots, block_slots); + + // States match. + end_state.drop_all_caches().unwrap(); + replay_state.drop_all_caches().unwrap(); + assert_eq!(end_state, replay_state); +} + #[test] fn delete_blocks_and_states() { let db_path = tempdir().unwrap(); @@ -430,7 +616,7 @@ fn delete_blocks_and_states() { // Delete faulty fork // Attempting to load those states should find them unavailable for (state_root, slot) in - StateRootsIterator::new(store.clone(), &faulty_head_state).map(Result::unwrap) + StateRootsIterator::new(&store, &faulty_head_state).map(Result::unwrap) { if slot <= unforked_blocks { break; @@ -441,7 +627,7 @@ fn delete_blocks_and_states() { // Double-deleting should also be OK (deleting non-existent things is fine) for (state_root, slot) in - StateRootsIterator::new(store.clone(), &faulty_head_state).map(Result::unwrap) + StateRootsIterator::new(&store, &faulty_head_state).map(Result::unwrap) { if slot <= unforked_blocks { break; @@ -451,7 +637,7 @@ fn delete_blocks_and_states() { // Deleting the blocks from the fork should remove them completely for (block_root, slot) in - BlockRootsIterator::new(store.clone(), &faulty_head_state).map(Result::unwrap) + BlockRootsIterator::new(&store, &faulty_head_state).map(Result::unwrap) { if slot <= unforked_blocks + 1 { break; diff --git a/beacon_node/store/src/chunked_iter.rs b/beacon_node/store/src/chunked_iter.rs index 7d47e8c99aa..8ef0b6d201d 100644 --- a/beacon_node/store/src/chunked_iter.rs +++ b/beacon_node/store/src/chunked_iter.rs @@ -1,27 +1,26 @@ use crate::chunked_vector::{chunk_key, Chunk, Field}; use crate::{HotColdDB, ItemStore}; use slog::error; -use std::sync::Arc; use types::{ChainSpec, EthSpec, Slot}; /// Iterator over the values of a `BeaconState` vector field (like `block_roots`). /// /// Uses the freezer DB's separate table to load the values. -pub struct ChunkedVectorIter +pub struct ChunkedVectorIter<'a, F, E, Hot, Cold> where F: Field, E: EthSpec, Hot: ItemStore, Cold: ItemStore, { - pub(crate) store: Arc>, + pub(crate) store: &'a HotColdDB, current_vindex: usize, pub(crate) end_vindex: usize, next_cindex: usize, current_chunk: Chunk, } -impl ChunkedVectorIter +impl<'a, F, E, Hot, Cold> ChunkedVectorIter<'a, F, E, Hot, Cold> where F: Field, E: EthSpec, @@ -35,7 +34,7 @@ where /// `HotColdDB::get_latest_restore_point_slot`. We pass it as a parameter so that the caller can /// maintain a stable view of the database (see `HybridForwardsBlockRootsIterator`). pub fn new( - store: Arc>, + store: &'a HotColdDB, start_vindex: usize, last_restore_point_slot: Slot, spec: &ChainSpec, @@ -57,7 +56,7 @@ where } } -impl Iterator for ChunkedVectorIter +impl<'a, F, E, Hot, Cold> Iterator for ChunkedVectorIter<'a, F, E, Hot, Cold> where F: Field, E: EthSpec, diff --git a/beacon_node/store/src/errors.rs b/beacon_node/store/src/errors.rs index 0be8b43d6d9..1147d52c436 100644 --- a/beacon_node/store/src/errors.rs +++ b/beacon_node/store/src/errors.rs @@ -2,6 +2,7 @@ use crate::chunked_vector::ChunkError; use crate::config::StoreConfigError; use crate::hot_cold_store::HotColdDBError; use ssz::DecodeError; +use state_processing::BlockReplayError; use types::{BeaconStateError, Hash256, Slot}; pub type Result = std::result::Result; @@ -39,6 +40,7 @@ pub enum Error { expected: Hash256, computed: Hash256, }, + BlockReplayError(BlockReplayError), } pub trait HandleUnavailable { @@ -91,6 +93,12 @@ impl From for Error { } } +impl From for Error { + fn from(e: BlockReplayError) -> Error { + Error::BlockReplayError(e) + } +} + #[derive(Debug)] pub struct DBError { pub message: String, diff --git a/beacon_node/store/src/forwards_iter.rs b/beacon_node/store/src/forwards_iter.rs index 5a77863d543..353be6bf058 100644 --- a/beacon_node/store/src/forwards_iter.rs +++ b/beacon_node/store/src/forwards_iter.rs @@ -1,74 +1,33 @@ use crate::chunked_iter::ChunkedVectorIter; -use crate::chunked_vector::{BlockRoots, StateRoots}; +use crate::chunked_vector::{BlockRoots, Field, StateRoots}; use crate::errors::{Error, Result}; use crate::iter::{BlockRootsIterator, StateRootsIterator}; use crate::{HotColdDB, ItemStore}; use itertools::process_results; -use std::sync::Arc; use types::{BeaconState, ChainSpec, EthSpec, Hash256, Slot}; -/// Forwards block roots iterator that makes use of the `block_roots` table in the freezer DB. -pub struct FrozenForwardsBlockRootsIterator, Cold: ItemStore> { - inner: ChunkedVectorIter, -} - -/// Forwards block roots iterator that reverses a backwards iterator (only good for short ranges). -pub struct SimpleForwardsBlockRootsIterator { - // Values from the backwards iterator (in slot descending order) - values: Vec<(Hash256, Slot)>, -} +pub type HybridForwardsBlockRootsIterator<'a, E, Hot, Cold> = + HybridForwardsIterator<'a, E, BlockRoots, Hot, Cold>; +pub type HybridForwardsStateRootsIterator<'a, E, Hot, Cold> = + HybridForwardsIterator<'a, E, StateRoots, Hot, Cold>; -/// Fusion of the above two approaches to forwards iteration. Fast and efficient. -pub enum HybridForwardsBlockRootsIterator, Cold: ItemStore> { - PreFinalization { - iter: Box>, - /// Data required by the `PostFinalization` iterator when we get to it. - continuation_data: Box, Hash256)>>, - }, - PostFinalization { - iter: SimpleForwardsBlockRootsIterator, - }, -} - -impl, Cold: ItemStore> - FrozenForwardsBlockRootsIterator -{ - pub fn new( - store: Arc>, +/// Trait unifying `BlockRoots` and `StateRoots` for forward iteration. +pub trait Root: Field { + fn simple_forwards_iterator, Cold: ItemStore>( + store: &HotColdDB, start_slot: Slot, - last_restore_point_slot: Slot, - spec: &ChainSpec, - ) -> Self { - Self { - inner: ChunkedVectorIter::new( - store, - start_slot.as_usize(), - last_restore_point_slot, - spec, - ), - } - } -} - -impl, Cold: ItemStore> Iterator - for FrozenForwardsBlockRootsIterator -{ - type Item = (Hash256, Slot); - - fn next(&mut self) -> Option { - self.inner - .next() - .map(|(slot, block_hash)| (block_hash, Slot::from(slot))) - } + end_state: BeaconState, + end_root: Hash256, + ) -> Result; } -impl SimpleForwardsBlockRootsIterator { - pub fn new, Cold: ItemStore>( - store: Arc>, +impl Root for BlockRoots { + fn simple_forwards_iterator, Cold: ItemStore>( + store: &HotColdDB, start_slot: Slot, end_state: BeaconState, end_block_root: Hash256, - ) -> Result { + ) -> Result { // Iterate backwards from the end state, stopping at the start slot. let values = process_results( std::iter::once(Ok((end_block_root, end_state.slot()))) @@ -78,129 +37,41 @@ impl SimpleForwardsBlockRootsIterator { .collect::>() }, )?; - Ok(Self { values }) + Ok(SimpleForwardsIterator { values }) } } -impl Iterator for SimpleForwardsBlockRootsIterator { - type Item = Result<(Hash256, Slot)>; - - fn next(&mut self) -> Option { - // Pop from the end of the vector to get the block roots in slot-ascending order. - Ok(self.values.pop()).transpose() - } -} - -impl, Cold: ItemStore> - HybridForwardsBlockRootsIterator -{ - pub fn new( - store: Arc>, +impl Root for StateRoots { + fn simple_forwards_iterator, Cold: ItemStore>( + store: &HotColdDB, start_slot: Slot, end_state: BeaconState, - end_block_root: Hash256, - spec: &ChainSpec, - ) -> Result { - use HybridForwardsBlockRootsIterator::*; - - let latest_restore_point_slot = store.get_latest_restore_point_slot(); - - let result = if start_slot < latest_restore_point_slot { - PreFinalization { - iter: Box::new(FrozenForwardsBlockRootsIterator::new( - store, - start_slot, - latest_restore_point_slot, - spec, - )), - continuation_data: Box::new(Some((end_state, end_block_root))), - } - } else { - PostFinalization { - iter: SimpleForwardsBlockRootsIterator::new( - store, - start_slot, - end_state, - end_block_root, - )?, - } - }; - - Ok(result) - } - - fn do_next(&mut self) -> Result> { - use HybridForwardsBlockRootsIterator::*; - - match self { - PreFinalization { - iter, - continuation_data, - } => { - match iter.next() { - Some(x) => Ok(Some(x)), - // Once the pre-finalization iterator is consumed, transition - // to a post-finalization iterator beginning from the last slot - // of the pre iterator. - None => { - let (end_state, end_block_root) = - continuation_data.take().ok_or(Error::NoContinuationData)?; - - *self = PostFinalization { - iter: SimpleForwardsBlockRootsIterator::new( - iter.inner.store.clone(), - Slot::from(iter.inner.end_vindex), - end_state, - end_block_root, - )?, - }; - self.do_next() - } - } - } - PostFinalization { iter } => iter.next().transpose(), - } + end_state_root: Hash256, + ) -> Result { + // Iterate backwards from the end state, stopping at the start slot. + let values = process_results( + std::iter::once(Ok((end_state_root, end_state.slot()))) + .chain(StateRootsIterator::owned(store, end_state)), + |iter| { + iter.take_while(|(_, slot)| *slot >= start_slot) + .collect::>() + }, + )?; + Ok(SimpleForwardsIterator { values }) } } -impl, Cold: ItemStore> Iterator - for HybridForwardsBlockRootsIterator +/// Forwards root iterator that makes use of a flat field table in the freezer DB. +pub struct FrozenForwardsIterator<'a, E: EthSpec, F: Root, Hot: ItemStore, Cold: ItemStore> { - type Item = Result<(Hash256, Slot)>; - - fn next(&mut self) -> Option { - self.do_next().transpose() - } -} - -/// Forwards state roots iterator that makes use of the `state_roots` table in the freezer DB. -pub struct FrozenForwardsStateRootsIterator, Cold: ItemStore> { - inner: ChunkedVectorIter, -} - -/// Forwards state roots iterator that reverses a backwards iterator (only good for short ranges). -pub struct SimpleForwardsStateRootsIterator { - // Values from the backwards iterator (in slot descending order) - values: Vec<(Hash256, Slot)>, + inner: ChunkedVectorIter<'a, F, E, Hot, Cold>, } -/// Fusion of the above two approaches to forwards iteration. Fast and efficient. -pub enum HybridForwardsStateRootsIterator, Cold: ItemStore> { - PreFinalization { - iter: Box>, - /// Data required by the `PostFinalization` iterator when we get to it. - continuation_data: Box, Hash256)>>, - }, - PostFinalization { - iter: SimpleForwardsStateRootsIterator, - }, -} - -impl, Cold: ItemStore> - FrozenForwardsStateRootsIterator +impl<'a, E: EthSpec, F: Root, Hot: ItemStore, Cold: ItemStore> + FrozenForwardsIterator<'a, E, F, Hot, Cold> { pub fn new( - store: Arc>, + store: &'a HotColdDB, start_slot: Slot, last_restore_point_slot: Slot, spec: &ChainSpec, @@ -216,39 +87,25 @@ impl, Cold: ItemStore> } } -impl, Cold: ItemStore> Iterator - for FrozenForwardsStateRootsIterator +impl<'a, E: EthSpec, F: Root, Hot: ItemStore, Cold: ItemStore> Iterator + for FrozenForwardsIterator<'a, E, F, Hot, Cold> { type Item = (Hash256, Slot); fn next(&mut self) -> Option { self.inner .next() - .map(|(slot, state_hash)| (state_hash, Slot::from(slot))) + .map(|(slot, root)| (root, Slot::from(slot))) } } -impl SimpleForwardsStateRootsIterator { - pub fn new, Cold: ItemStore>( - store: Arc>, - start_slot: Slot, - end_state: BeaconState, - end_state_root: Hash256, - ) -> Result { - // Iterate backwards from the end state, stopping at the start slot. - let values = process_results( - std::iter::once(Ok((end_state_root, end_state.slot()))) - .chain(StateRootsIterator::owned(store, end_state)), - |iter| { - iter.take_while(|(_, slot)| *slot >= start_slot) - .collect::>() - }, - )?; - Ok(Self { values }) - } +/// Forwards root iterator that reverses a backwards iterator (only good for short ranges). +pub struct SimpleForwardsIterator { + // Values from the backwards iterator (in slot descending order) + values: Vec<(Hash256, Slot)>, } -impl Iterator for SimpleForwardsStateRootsIterator { +impl Iterator for SimpleForwardsIterator { type Item = Result<(Hash256, Slot)>; fn next(&mut self) -> Option { @@ -257,38 +114,75 @@ impl Iterator for SimpleForwardsStateRootsIterator { } } -impl, Cold: ItemStore> - HybridForwardsStateRootsIterator +/// Fusion of the above two approaches to forwards iteration. Fast and efficient. +pub enum HybridForwardsIterator<'a, E: EthSpec, F: Root, Hot: ItemStore, Cold: ItemStore> { + PreFinalization { + iter: Box>, + /// Data required by the `PostFinalization` iterator when we get to it. + continuation_data: Option, Hash256)>>, + }, + PostFinalizationLazy { + continuation_data: Option, Hash256)>>, + store: &'a HotColdDB, + start_slot: Slot, + }, + PostFinalization { + iter: SimpleForwardsIterator, + }, +} + +impl<'a, E: EthSpec, F: Root, Hot: ItemStore, Cold: ItemStore> + HybridForwardsIterator<'a, E, F, Hot, Cold> { + /// Construct a new hybrid iterator. + /// + /// The `get_state` closure should return a beacon state and final block/state root to backtrack + /// from in the case where the iterated range does not lie entirely within the frozen portion of + /// the database. If an `end_slot` is provided and it is before the database's latest restore + /// point slot then the `get_state` closure will not be called at all. + /// + /// It is OK for `get_state` to hold a lock while this function is evaluated, as the returned + /// iterator is as lazy as possible and won't do any work apart from calling `get_state`. + /// + /// Conversely, if `get_state` does extensive work (e.g. loading data from disk) then this + /// function may block for some time while `get_state` runs. pub fn new( - store: Arc>, + store: &'a HotColdDB, start_slot: Slot, - end_state: BeaconState, - end_state_root: Hash256, + end_slot: Option, + get_state: impl FnOnce() -> (BeaconState, Hash256), spec: &ChainSpec, ) -> Result { - use HybridForwardsStateRootsIterator::*; + use HybridForwardsIterator::*; let latest_restore_point_slot = store.get_latest_restore_point_slot(); let result = if start_slot < latest_restore_point_slot { + let iter = Box::new(FrozenForwardsIterator::new( + store, + start_slot, + latest_restore_point_slot, + spec, + )); + + // No continuation data is needed if the forwards iterator plans to halt before + // `end_slot`. If it tries to continue further a `NoContinuationData` error will be + // returned. + let continuation_data = + if end_slot.map_or(false, |end_slot| end_slot < latest_restore_point_slot) { + None + } else { + Some(Box::new(get_state())) + }; PreFinalization { - iter: Box::new(FrozenForwardsStateRootsIterator::new( - store, - start_slot, - latest_restore_point_slot, - spec, - )), - continuation_data: Box::new(Some((end_state, end_state_root))), + iter, + continuation_data, } } else { - PostFinalization { - iter: SimpleForwardsStateRootsIterator::new( - store, - start_slot, - end_state, - end_state_root, - )?, + PostFinalizationLazy { + continuation_data: Some(Box::new(get_state())), + store, + start_slot, } }; @@ -296,7 +190,7 @@ impl, Cold: ItemStore> } fn do_next(&mut self) -> Result> { - use HybridForwardsStateRootsIterator::*; + use HybridForwardsIterator::*; match self { PreFinalization { @@ -309,28 +203,39 @@ impl, Cold: ItemStore> // to a post-finalization iterator beginning from the last slot // of the pre iterator. None => { - let (end_state, end_state_root) = - continuation_data.take().ok_or(Error::NoContinuationData)?; - - *self = PostFinalization { - iter: SimpleForwardsStateRootsIterator::new( - iter.inner.store.clone(), - Slot::from(iter.inner.end_vindex), - end_state, - end_state_root, - )?, + let continuation_data = continuation_data.take(); + let store = iter.inner.store; + let start_slot = Slot::from(iter.inner.end_vindex); + + *self = PostFinalizationLazy { + continuation_data, + store, + start_slot, }; + self.do_next() } } } + PostFinalizationLazy { + continuation_data, + store, + start_slot, + } => { + let (end_state, end_root) = + *continuation_data.take().ok_or(Error::NoContinuationData)?; + *self = PostFinalization { + iter: F::simple_forwards_iterator(store, *start_slot, end_state, end_root)?, + }; + self.do_next() + } PostFinalization { iter } => iter.next().transpose(), } } } -impl, Cold: ItemStore> Iterator - for HybridForwardsStateRootsIterator +impl<'a, E: EthSpec, F: Root, Hot: ItemStore, Cold: ItemStore> Iterator + for HybridForwardsIterator<'a, E, F, Hot, Cold> { type Item = Result<(Hash256, Slot)>; diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index 05a0eb3dd94..62441ce0f2b 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -22,12 +22,11 @@ use leveldb::iterator::LevelDBIterator; use lru::LruCache; use parking_lot::{Mutex, RwLock}; use serde_derive::{Deserialize, Serialize}; -use slog::{debug, error, info, trace, Logger}; +use slog::{debug, error, info, trace, warn, Logger}; use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; use state_processing::{ - per_block_processing, per_slot_processing, BlockProcessingError, BlockSignatureStrategy, - SlotProcessingError, + BlockProcessingError, BlockReplayer, SlotProcessingError, StateRootStrategy, }; use std::cmp::min; use std::convert::TryInto; @@ -37,16 +36,6 @@ use std::sync::Arc; use std::time::Duration; use types::*; -/// 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 @@ -373,10 +362,10 @@ 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, BlockReplay::Accurate) + self.load_hot_state(state_root, StateRootStrategy::Accurate) } } else { - match self.load_hot_state(state_root, BlockReplay::Accurate)? { + match self.load_hot_state(state_root, StateRootStrategy::Accurate)? { Some(state) => Ok(Some(state)), None => self.load_cold_state(state_root), } @@ -414,7 +403,7 @@ impl, Cold: ItemStore> HotColdDB } .into()) } else { - self.load_hot_state(state_root, BlockReplay::InconsistentStateRoots) + self.load_hot_state(state_root, StateRootStrategy::Inconsistent) } } @@ -439,23 +428,55 @@ impl, Cold: ItemStore> HotColdDB } pub fn forwards_block_roots_iterator( - store: Arc, + &self, start_slot: Slot, end_state: BeaconState, end_block_root: Hash256, spec: &ChainSpec, - ) -> Result>, Error> { - HybridForwardsBlockRootsIterator::new(store, start_slot, end_state, end_block_root, spec) + ) -> Result> + '_, Error> { + HybridForwardsBlockRootsIterator::new( + self, + start_slot, + None, + || (end_state, end_block_root), + spec, + ) + } + + pub fn forwards_block_roots_iterator_until( + &self, + start_slot: Slot, + end_slot: Slot, + get_state: impl FnOnce() -> (BeaconState, Hash256), + spec: &ChainSpec, + ) -> Result, Error> { + HybridForwardsBlockRootsIterator::new(self, start_slot, Some(end_slot), get_state, spec) } pub fn forwards_state_roots_iterator( - store: Arc, + &self, start_slot: Slot, end_state_root: Hash256, end_state: BeaconState, spec: &ChainSpec, - ) -> Result>, Error> { - HybridForwardsStateRootsIterator::new(store, start_slot, end_state, end_state_root, spec) + ) -> Result> + '_, Error> { + HybridForwardsStateRootsIterator::new( + self, + start_slot, + None, + || (end_state, end_state_root), + spec, + ) + } + + pub fn forwards_state_roots_iterator_until( + &self, + start_slot: Slot, + end_slot: Slot, + get_state: impl FnOnce() -> (BeaconState, Hash256), + spec: &ChainSpec, + ) -> Result, Error> { + HybridForwardsStateRootsIterator::new(self, start_slot, Some(end_slot), get_state, spec) } /// Load an epoch boundary state by using the hot state summary look-up. @@ -472,10 +493,10 @@ impl, Cold: ItemStore> HotColdDB { // 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 + // `StateRootStrategy` 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, BlockReplay::Accurate)? + .load_hot_state(&epoch_boundary_state_root, StateRootStrategy::Accurate)? .ok_or(HotColdDBError::MissingEpochBoundaryState( epoch_boundary_state_root, ))?; @@ -620,7 +641,7 @@ impl, Cold: ItemStore> HotColdDB pub fn load_hot_state( &self, state_root: &Hash256, - block_replay: BlockReplay, + state_root_strategy: StateRootStrategy, ) -> Result>, Error> { metrics::inc_counter(&metrics::BEACON_STATE_HOT_GET_COUNT); @@ -648,7 +669,13 @@ 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, block_replay)? + self.replay_blocks( + boundary_state, + blocks, + slot, + no_state_root_iter(), + state_root_strategy, + )? }; Ok(Some(state)) @@ -777,7 +804,22 @@ impl, Cold: ItemStore> HotColdDB )?; // 3. Replay the blocks on top of the low restore point. - self.replay_blocks(low_restore_point, blocks, slot, BlockReplay::Accurate) + // Use a forwards state root iterator to avoid doing any tree hashing. + // The state root of the high restore point should never be used, so is safely set to 0. + let state_root_iter = self.forwards_state_roots_iterator_until( + low_restore_point.slot(), + slot, + || (high_restore_point, Hash256::zero()), + &self.spec, + )?; + + self.replay_blocks( + low_restore_point, + blocks, + slot, + Some(state_root_iter), + StateRootStrategy::Accurate, + ) } /// Get the restore point with the given index, or if it is out of bounds, the split state. @@ -860,89 +902,35 @@ impl, Cold: ItemStore> HotColdDB /// to have any caches built, beyond those immediately required by block processing. fn replay_blocks( &self, - mut state: BeaconState, - mut blocks: Vec>, + state: BeaconState, + blocks: Vec>, target_slot: Slot, - block_replay: BlockReplay, + state_root_iter: Option>>, + state_root_strategy: StateRootStrategy, ) -> Result, Error> { - if block_replay == BlockReplay::InconsistentStateRoots { - for i in 0..blocks.len() { - let prev_block_root = if i > 0 { - blocks[i - 1].canonical_root() - } else { - // Not read. - Hash256::zero() - }; - - let (state_root, parent_root) = match &mut blocks[i] { - SignedBeaconBlock::Base(block) => ( - &mut block.message.state_root, - &mut block.message.parent_root, - ), - SignedBeaconBlock::Altair(block) => ( - &mut block.message.state_root, - &mut block.message.parent_root, - ), - SignedBeaconBlock::Merge(block) => ( - &mut block.message.state_root, - &mut block.message.parent_root, - ), - }; - - *state_root = Hash256::zero(); - if i > 0 { - *parent_root = prev_block_root; - } - } + let mut block_replayer = BlockReplayer::new(state, &self.spec) + .state_root_strategy(state_root_strategy) + .no_signature_verification() + .minimal_block_root_verification(); + + let have_state_root_iterator = state_root_iter.is_some(); + if let Some(state_root_iter) = state_root_iter { + block_replayer = block_replayer.state_root_iter(state_root_iter); } - let state_root_from_prev_block = |i: usize, state: &BeaconState| { - if i > 0 { - let prev_block = blocks[i - 1].message(); - if prev_block.slot() == state.slot() { - Some(prev_block.state_root()) - } else { - None + block_replayer + .apply_blocks(blocks, Some(target_slot)) + .map(|block_replayer| { + if have_state_root_iterator && block_replayer.state_root_miss() { + warn!( + self.log, + "State root iterator miss"; + "slot" => target_slot, + ); } - } else { - None - } - }; - - for (i, block) in blocks.iter().enumerate() { - if block.slot() <= state.slot() { - continue; - } - while state.slot() < block.slot() { - 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, - None, - BlockSignatureStrategy::NoVerification, - &self.spec, - ) - .map_err(HotColdDBError::BlockReplayBlockError)?; - } - - while state.slot() < target_slot { - 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)?; - } - - Ok(state) + block_replayer.into_state() + }) } /// Fetch a copy of the current split slot from memory. @@ -1309,7 +1297,7 @@ pub fn migrate_database, Cold: ItemStore>( // 1. Copy all of the states between the head and the split slot, from the hot DB // to the cold DB. - let state_root_iter = StateRootsIterator::new(store.clone(), frozen_head); + let state_root_iter = StateRootsIterator::new(&store, frozen_head); for maybe_pair in state_root_iter.take_while(|result| match result { Ok((_, slot)) => { slot >= ¤t_split_slot @@ -1423,6 +1411,11 @@ impl StoreItem for Split { } } +/// Type hint. +fn no_state_root_iter() -> Option>> { + None +} + /// Struct for summarising a state in the hot database. /// /// Allows full reconstruction by replaying blocks. diff --git a/beacon_node/store/src/iter.rs b/beacon_node/store/src/iter.rs index a4d34cd3c37..d5448de9832 100644 --- a/beacon_node/store/src/iter.rs +++ b/beacon_node/store/src/iter.rs @@ -2,7 +2,6 @@ use crate::errors::HandleUnavailable; use crate::{Error, HotColdDB, ItemStore}; use std::borrow::Cow; use std::marker::PhantomData; -use std::sync::Arc; use types::{ typenum::Unsigned, BeaconState, BeaconStateError, EthSpec, Hash256, SignedBeaconBlock, Slot, }; @@ -13,19 +12,19 @@ use types::{ /// /// It is assumed that all ancestors for this object are stored in the database. If this is not the /// case, the iterator will start returning `None` prior to genesis. -pub trait AncestorIter, Cold: ItemStore, I: Iterator> { +pub trait AncestorIter<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore, I: Iterator> { /// Returns an iterator over the roots of the ancestors of `self`. - fn try_iter_ancestor_roots(&self, store: Arc>) -> Option; + fn try_iter_ancestor_roots(&self, store: &'a HotColdDB) -> Option; } impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> - AncestorIter> for SignedBeaconBlock + AncestorIter<'a, E, Hot, Cold, BlockRootsIterator<'a, E, Hot, Cold>> for SignedBeaconBlock { /// Iterates across all available prior block roots of `self`, starting at the most recent and ending /// at genesis. fn try_iter_ancestor_roots( &self, - store: Arc>, + store: &'a HotColdDB, ) -> Option> { let state = store .get_state(&self.message().state_root(), Some(self.slot())) @@ -36,13 +35,13 @@ impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> } impl<'a, E: EthSpec, Hot: ItemStore, Cold: ItemStore> - AncestorIter> for BeaconState + AncestorIter<'a, E, Hot, Cold, StateRootsIterator<'a, E, Hot, Cold>> for BeaconState { /// Iterates across all available prior state roots of `self`, starting at the most recent and ending /// at genesis. fn try_iter_ancestor_roots( &self, - store: Arc>, + store: &'a HotColdDB, ) -> Option> { // The `self.clone()` here is wasteful. Some(StateRootsIterator::owned(store, self.clone())) @@ -64,13 +63,13 @@ impl<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> Clone } impl<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> StateRootsIterator<'a, T, Hot, Cold> { - pub fn new(store: Arc>, beacon_state: &'a BeaconState) -> Self { + pub fn new(store: &'a HotColdDB, beacon_state: &'a BeaconState) -> Self { Self { inner: RootsIterator::new(store, beacon_state), } } - pub fn owned(store: Arc>, beacon_state: BeaconState) -> Self { + pub fn owned(store: &'a HotColdDB, beacon_state: BeaconState) -> Self { Self { inner: RootsIterator::owned(store, beacon_state), } @@ -113,21 +112,21 @@ impl<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> Clone impl<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> BlockRootsIterator<'a, T, Hot, Cold> { /// Create a new iterator over all block roots in the given `beacon_state` and prior states. - pub fn new(store: Arc>, beacon_state: &'a BeaconState) -> Self { + pub fn new(store: &'a HotColdDB, beacon_state: &'a BeaconState) -> Self { Self { inner: RootsIterator::new(store, beacon_state), } } /// Create a new iterator over all block roots in the given `beacon_state` and prior states. - pub fn owned(store: Arc>, beacon_state: BeaconState) -> Self { + pub fn owned(store: &'a HotColdDB, beacon_state: BeaconState) -> Self { Self { inner: RootsIterator::owned(store, beacon_state), } } pub fn from_block( - store: Arc>, + store: &'a HotColdDB, block_hash: Hash256, ) -> Result { Ok(Self { @@ -150,7 +149,7 @@ impl<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> Iterator /// Iterator over state and block roots that backtracks using the vectors from a `BeaconState`. pub struct RootsIterator<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> { - store: Arc>, + store: &'a HotColdDB, beacon_state: Cow<'a, BeaconState>, slot: Slot, } @@ -160,7 +159,7 @@ impl<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> Clone { fn clone(&self) -> Self { Self { - store: self.store.clone(), + store: self.store, beacon_state: self.beacon_state.clone(), slot: self.slot, } @@ -168,7 +167,7 @@ impl<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> Clone } impl<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> RootsIterator<'a, T, Hot, Cold> { - pub fn new(store: Arc>, beacon_state: &'a BeaconState) -> Self { + pub fn new(store: &'a HotColdDB, beacon_state: &'a BeaconState) -> Self { Self { store, slot: beacon_state.slot(), @@ -176,7 +175,7 @@ impl<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> RootsIterator<'a, T, } } - pub fn owned(store: Arc>, beacon_state: BeaconState) -> Self { + pub fn owned(store: &'a HotColdDB, beacon_state: BeaconState) -> Self { Self { store, slot: beacon_state.slot(), @@ -185,7 +184,7 @@ impl<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> RootsIterator<'a, T, } pub fn from_block( - store: Arc>, + store: &'a HotColdDB, block_hash: Hash256, ) -> Result { let block = store @@ -310,14 +309,14 @@ pub struct BlockIterator<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> impl<'a, T: EthSpec, Hot: ItemStore, Cold: ItemStore> BlockIterator<'a, T, Hot, Cold> { /// Create a new iterator over all blocks in the given `beacon_state` and prior states. - pub fn new(store: Arc>, beacon_state: &'a BeaconState) -> Self { + pub fn new(store: &'a HotColdDB, beacon_state: &'a BeaconState) -> Self { Self { roots: BlockRootsIterator::new(store, beacon_state), } } /// Create a new iterator over all blocks in the given `beacon_state` and prior states. - pub fn owned(store: Arc>, beacon_state: BeaconState) -> Self { + pub fn owned(store: &'a HotColdDB, beacon_state: BeaconState) -> Self { Self { roots: BlockRootsIterator::owned(store, beacon_state), } @@ -397,9 +396,8 @@ mod test { #[test] fn block_root_iter() { let log = NullLoggerBuilder.build().unwrap(); - let store = Arc::new( - HotColdDB::open_ephemeral(Config::default(), ChainSpec::minimal(), log).unwrap(), - ); + let store = + HotColdDB::open_ephemeral(Config::default(), ChainSpec::minimal(), log).unwrap(); let slots_per_historical_root = MainnetEthSpec::slots_per_historical_root(); let mut state_a: BeaconState = get_state(); @@ -422,7 +420,7 @@ mod test { state_b.state_roots_mut()[0] = state_a_root; store.put_state(&state_a_root, &state_a).unwrap(); - let iter = BlockRootsIterator::new(store, &state_b); + let iter = BlockRootsIterator::new(&store, &state_b); assert!( iter.clone() @@ -445,9 +443,8 @@ mod test { #[test] fn state_root_iter() { let log = NullLoggerBuilder.build().unwrap(); - let store = Arc::new( - HotColdDB::open_ephemeral(Config::default(), ChainSpec::minimal(), log).unwrap(), - ); + let store = + HotColdDB::open_ephemeral(Config::default(), ChainSpec::minimal(), log).unwrap(); let slots_per_historical_root = MainnetEthSpec::slots_per_historical_root(); let mut state_a: BeaconState = get_state(); @@ -475,7 +472,7 @@ mod test { store.put_state(&state_a_root, &state_a).unwrap(); store.put_state(&state_b_root, &state_b).unwrap(); - let iter = StateRootsIterator::new(store, &state_b); + let iter = StateRootsIterator::new(&store, &state_b); assert!( iter.clone() diff --git a/beacon_node/store/src/lib.rs b/beacon_node/store/src/lib.rs index c86a01213c4..8d1993f4616 100644 --- a/beacon_node/store/src/lib.rs +++ b/beacon_node/store/src/lib.rs @@ -30,7 +30,7 @@ pub mod iter; pub use self::chunk_writer::ChunkWriter; pub use self::config::StoreConfig; -pub use self::hot_cold_store::{BlockReplay, HotColdDB, HotStateSummary, Split}; +pub use self::hot_cold_store::{HotColdDB, HotStateSummary, Split}; pub use self::leveldb_store::LevelDB; pub use self::memory_store::MemoryStore; pub use self::partial_beacon_state::PartialBeaconState; diff --git a/beacon_node/store/src/reconstruct.rs b/beacon_node/store/src/reconstruct.rs index a88af95c854..6b808974e71 100644 --- a/beacon_node/store/src/reconstruct.rs +++ b/beacon_node/store/src/reconstruct.rs @@ -3,7 +3,9 @@ use crate::hot_cold_store::{HotColdDB, HotColdDBError}; use crate::{Error, ItemStore, KeyValueStore}; use itertools::{process_results, Itertools}; use slog::info; -use state_processing::{per_block_processing, per_slot_processing, BlockSignatureStrategy}; +use state_processing::{ + per_block_processing, per_slot_processing, BlockSignatureStrategy, VerifyBlockRoot, +}; use std::sync::Arc; use types::{EthSpec, Hash256}; @@ -48,8 +50,7 @@ where // Use a dummy root, as we never read the block for the upper limit state. let upper_limit_block_root = Hash256::repeat_byte(0xff); - let block_root_iter = Self::forwards_block_roots_iterator( - self.clone(), + let block_root_iter = self.forwards_block_roots_iterator( lower_limit_slot, upper_limit_state, upper_limit_block_root, @@ -91,6 +92,7 @@ where &block, Some(block_root), BlockSignatureStrategy::NoVerification, + VerifyBlockRoot::True, &self.spec, ) .map_err(HotColdDBError::BlockReplayBlockError)?; diff --git a/consensus/state_processing/src/block_replayer.rs b/consensus/state_processing/src/block_replayer.rs new file mode 100644 index 00000000000..937348263b4 --- /dev/null +++ b/consensus/state_processing/src/block_replayer.rs @@ -0,0 +1,313 @@ +use crate::{ + per_block_processing, per_epoch_processing::EpochProcessingSummary, per_slot_processing, + BlockProcessingError, BlockSignatureStrategy, SlotProcessingError, VerifyBlockRoot, +}; +use std::marker::PhantomData; +use types::{BeaconState, ChainSpec, EthSpec, Hash256, SignedBeaconBlock, Slot}; + +type PreBlockHook<'a, E, Error> = + Box, &SignedBeaconBlock) -> Result<(), Error> + 'a>; +type PostBlockHook<'a, E, Error> = PreBlockHook<'a, E, Error>; +type PreSlotHook<'a, E, Error> = Box) -> Result<(), Error> + 'a>; +type PostSlotHook<'a, E, Error> = Box< + dyn FnMut(&mut BeaconState, Option>, bool) -> Result<(), Error> + + 'a, +>; +type StateRootIterDefault = std::iter::Empty>; + +/// Efficiently apply blocks to a state while configuring various parameters. +/// +/// Usage follows a builder pattern. +pub struct BlockReplayer< + 'a, + Spec: EthSpec, + Error = BlockReplayError, + StateRootIter = StateRootIterDefault, +> { + state: BeaconState, + spec: &'a ChainSpec, + state_root_strategy: StateRootStrategy, + block_sig_strategy: BlockSignatureStrategy, + verify_block_root: Option, + pre_block_hook: Option>, + post_block_hook: Option>, + pre_slot_hook: Option>, + post_slot_hook: Option>, + state_root_iter: Option, + state_root_miss: bool, + _phantom: PhantomData, +} + +#[derive(Debug)] +pub enum BlockReplayError { + NoBlocks, + SlotProcessing(SlotProcessingError), + BlockProcessing(BlockProcessingError), +} + +impl From for BlockReplayError { + fn from(e: SlotProcessingError) -> Self { + Self::SlotProcessing(e) + } +} + +impl From for BlockReplayError { + fn from(e: BlockProcessingError) -> Self { + Self::BlockProcessing(e) + } +} + +/// Defines how state roots should be computed during block replay. +#[derive(PartialEq)] +pub enum StateRootStrategy { + /// 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. + Inconsistent, +} + +impl<'a, E, Error, StateRootIter> BlockReplayer<'a, E, Error, StateRootIter> +where + E: EthSpec, + StateRootIter: Iterator>, + Error: From, +{ + /// Create a new replayer that will apply blocks upon `state`. + /// + /// Defaults: + /// + /// - Full (bulk) signature verification + /// - Accurate state roots + /// - Full block root verification + pub fn new(state: BeaconState, spec: &'a ChainSpec) -> Self { + Self { + state, + spec, + state_root_strategy: StateRootStrategy::Accurate, + block_sig_strategy: BlockSignatureStrategy::VerifyBulk, + verify_block_root: Some(VerifyBlockRoot::True), + pre_block_hook: None, + post_block_hook: None, + pre_slot_hook: None, + post_slot_hook: None, + state_root_iter: None, + state_root_miss: false, + _phantom: PhantomData, + } + } + + /// Set the replayer's state root strategy different from the default. + pub fn state_root_strategy(mut self, state_root_strategy: StateRootStrategy) -> Self { + if state_root_strategy == StateRootStrategy::Inconsistent { + self.verify_block_root = None; + } + self.state_root_strategy = state_root_strategy; + self + } + + /// Set the replayer's block signature verification strategy. + pub fn block_signature_strategy(mut self, block_sig_strategy: BlockSignatureStrategy) -> Self { + self.block_sig_strategy = block_sig_strategy; + self + } + + /// Disable signature verification during replay. + /// + /// If you are truly _replaying_ blocks then you will almost certainly want to disable + /// signature checks for performance. + pub fn no_signature_verification(self) -> Self { + self.block_signature_strategy(BlockSignatureStrategy::NoVerification) + } + + /// Verify only the block roots of the initial few blocks, and trust the rest. + pub fn minimal_block_root_verification(mut self) -> Self { + self.verify_block_root = None; + self + } + + /// Supply a state root iterator to accelerate slot processing. + /// + /// If possible the state root iterator should return a state root for every slot from + /// `self.state.slot` to the `target_slot` supplied to `apply_blocks` (inclusive of both + /// endpoints). + pub fn state_root_iter(mut self, iter: StateRootIter) -> Self { + self.state_root_iter = Some(iter); + self + } + + /// Run a function immediately before each block that is applied during `apply_blocks`. + /// + /// This can be used to inspect the state as blocks are applied. + pub fn pre_block_hook(mut self, hook: PreBlockHook<'a, E, Error>) -> Self { + self.pre_block_hook = Some(hook); + self + } + + /// Run a function immediately after each block that is applied during `apply_blocks`. + /// + /// This can be used to inspect the state as blocks are applied. + pub fn post_block_hook(mut self, hook: PostBlockHook<'a, E, Error>) -> Self { + self.post_block_hook = Some(hook); + self + } + + /// Run a function immediately before slot processing advances the state to the next slot. + pub fn pre_slot_hook(mut self, hook: PreSlotHook<'a, E, Error>) -> Self { + self.pre_slot_hook = Some(hook); + self + } + + /// Run a function immediately after slot processing has advanced the state to the next slot. + /// + /// The hook receives the state and a bool indicating if this state corresponds to a skipped + /// slot (i.e. it will not have a block applied). + pub fn post_slot_hook(mut self, hook: PostSlotHook<'a, E, Error>) -> Self { + self.post_slot_hook = Some(hook); + self + } + + /// Compute the state root for `slot` as efficiently as possible. + /// + /// The `blocks` should be the full list of blocks being applied and `i` should be the index of + /// the next block that will be applied, or `blocks.len()` if all blocks have already been + /// applied. + fn get_state_root( + &mut self, + slot: Slot, + blocks: &[SignedBeaconBlock], + i: usize, + ) -> Result, Error> { + // If we don't care about state roots then return immediately. + if self.state_root_strategy == StateRootStrategy::Inconsistent { + return Ok(Some(Hash256::zero())); + } + + // If a state root iterator is configured, use it to find the root. + if let Some(ref mut state_root_iter) = self.state_root_iter { + let opt_root = state_root_iter + .take_while(|res| res.as_ref().map_or(true, |(_, s)| *s <= slot)) + .find(|res| res.as_ref().map_or(true, |(_, s)| *s == slot)) + .transpose()?; + + if let Some((root, _)) = opt_root { + return Ok(Some(root)); + } + } + + // Otherwise try to source a root from the previous block. + if let Some(prev_i) = i.checked_sub(1) { + if let Some(prev_block) = blocks.get(prev_i) { + if prev_block.slot() == slot { + return Ok(Some(prev_block.state_root())); + } + } + } + + self.state_root_miss = true; + Ok(None) + } + + /// Apply `blocks` atop `self.state`, taking care of slot processing. + /// + /// If `target_slot` is provided then the state will be advanced through to `target_slot` + /// after the blocks have been applied. + pub fn apply_blocks( + mut self, + blocks: Vec>, + target_slot: Option, + ) -> Result { + for (i, block) in blocks.iter().enumerate() { + // Allow one additional block at the start which is only used for its state root. + if i == 0 && block.slot() <= self.state.slot() { + continue; + } + + while self.state.slot() < block.slot() { + if let Some(ref mut pre_slot_hook) = self.pre_slot_hook { + pre_slot_hook(&mut self.state)?; + } + + let state_root = self.get_state_root(self.state.slot(), &blocks, i)?; + let summary = per_slot_processing(&mut self.state, state_root, self.spec) + .map_err(BlockReplayError::from)?; + + if let Some(ref mut post_slot_hook) = self.post_slot_hook { + let is_skipped_slot = self.state.slot() < block.slot(); + post_slot_hook(&mut self.state, summary, is_skipped_slot)?; + } + } + + if let Some(ref mut pre_block_hook) = self.pre_block_hook { + pre_block_hook(&mut self.state, block)?; + } + + let verify_block_root = self.verify_block_root.unwrap_or_else(|| { + // If no explicit policy is set, verify only the first 1 or 2 block roots if using + // accurate state roots. Inaccurate state roots require block root verification to + // be off. + if i <= 1 && self.state_root_strategy == StateRootStrategy::Accurate { + VerifyBlockRoot::True + } else { + VerifyBlockRoot::False + } + }); + per_block_processing( + &mut self.state, + block, + None, + self.block_sig_strategy, + verify_block_root, + self.spec, + ) + .map_err(BlockReplayError::from)?; + + if let Some(ref mut post_block_hook) = self.post_block_hook { + post_block_hook(&mut self.state, block)?; + } + } + + if let Some(target_slot) = target_slot { + while self.state.slot() < target_slot { + if let Some(ref mut pre_slot_hook) = self.pre_slot_hook { + pre_slot_hook(&mut self.state)?; + } + + let state_root = self.get_state_root(self.state.slot(), &blocks, blocks.len())?; + let summary = per_slot_processing(&mut self.state, state_root, self.spec) + .map_err(BlockReplayError::from)?; + + if let Some(ref mut post_slot_hook) = self.post_slot_hook { + // No more blocks to apply (from our perspective) so we consider these slots + // skipped. + let is_skipped_slot = true; + post_slot_hook(&mut self.state, summary, is_skipped_slot)?; + } + } + } + + Ok(self) + } + + /// After block application, check if a state root miss occurred. + pub fn state_root_miss(&self) -> bool { + self.state_root_miss + } + + /// Convert the replayer into the state that was built. + pub fn into_state(self) -> BeaconState { + self.state + } +} + +impl<'a, E, Error> BlockReplayer<'a, E, Error, StateRootIterDefault> +where + E: EthSpec, + Error: From, +{ + /// If type inference fails to infer the state root iterator type you can use this method + /// to hint that no state root iterator is desired. + pub fn no_state_root_iter(self) -> Self { + self + } +} diff --git a/consensus/state_processing/src/lib.rs b/consensus/state_processing/src/lib.rs index 18fee2e2c3b..cb4ffee7802 100644 --- a/consensus/state_processing/src/lib.rs +++ b/consensus/state_processing/src/lib.rs @@ -16,6 +16,7 @@ mod macros; mod metrics; +pub mod block_replayer; pub mod common; pub mod genesis; pub mod per_block_processing; @@ -25,13 +26,14 @@ pub mod state_advance; pub mod upgrade; pub mod verify_operation; +pub use block_replayer::{BlockReplayError, BlockReplayer, StateRootStrategy}; pub use genesis::{ eth2_genesis_time, initialize_beacon_state_from_eth1, is_valid_genesis_state, process_activations, }; pub use per_block_processing::{ block_signature_verifier, errors::BlockProcessingError, per_block_processing, signature_sets, - BlockSignatureStrategy, BlockSignatureVerifier, VerifySignatures, + BlockSignatureStrategy, BlockSignatureVerifier, VerifyBlockRoot, VerifySignatures, }; pub use per_epoch_processing::{ errors::EpochProcessingError, process_epoch as per_epoch_processing, diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index 0dbb71699d0..ed7275be080 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -68,6 +68,14 @@ impl VerifySignatures { } } +/// Control verification of the latest block header. +#[cfg_attr(feature = "arbitrary-fuzz", derive(Arbitrary))] +#[derive(PartialEq, Clone, Copy)] +pub enum VerifyBlockRoot { + True, + False, +} + /// Updates the state for a new block, whilst validating that the block is valid, optionally /// checking the block proposer signature. /// @@ -84,6 +92,7 @@ pub fn per_block_processing( signed_block: &SignedBeaconBlock, block_root: Option, block_signature_strategy: BlockSignatureStrategy, + verify_block_root: VerifyBlockRoot, spec: &ChainSpec, ) -> Result<(), BlockProcessingError> { let block = signed_block.message(); @@ -120,7 +129,7 @@ pub fn per_block_processing( BlockSignatureStrategy::VerifyRandao => VerifySignatures::False, }; - let proposer_index = process_block_header(state, block, spec)?; + let proposer_index = process_block_header(state, block, verify_block_root, spec)?; if verify_signatures.is_true() { verify_block_signature(state, signed_block, block_root, spec)?; @@ -167,6 +176,7 @@ pub fn per_block_processing( pub fn process_block_header( state: &mut BeaconState, block: BeaconBlockRef<'_, T>, + verify_block_root: VerifyBlockRoot, spec: &ChainSpec, ) -> Result> { // Verify that the slots match @@ -195,14 +205,16 @@ pub fn process_block_header( } ); - let expected_previous_block_root = state.latest_block_header().tree_hash_root(); - verify!( - block.parent_root() == expected_previous_block_root, - HeaderInvalid::ParentBlockRootMismatch { - state: expected_previous_block_root, - block: block.parent_root(), - } - ); + if verify_block_root == VerifyBlockRoot::True { + let expected_previous_block_root = state.latest_block_header().tree_hash_root(); + verify!( + block.parent_root() == expected_previous_block_root, + HeaderInvalid::ParentBlockRootMismatch { + state: expected_previous_block_root, + block: block.parent_root(), + } + ); + } *state.latest_block_header_mut() = block.temporary_block_header(); diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs index f04b0ca9054..b75a79c72e8 100644 --- a/consensus/state_processing/src/per_block_processing/tests.rs +++ b/consensus/state_processing/src/per_block_processing/tests.rs @@ -6,7 +6,10 @@ use crate::per_block_processing::errors::{ DepositInvalid, HeaderInvalid, IndexedAttestationInvalid, IntoWithIndex, ProposerSlashingInvalid, }; -use crate::{per_block_processing::process_operations, BlockSignatureStrategy, VerifySignatures}; +use crate::{ + per_block_processing::process_operations, BlockSignatureStrategy, VerifyBlockRoot, + VerifySignatures, +}; use beacon_chain::test_utils::{BeaconChainHarness, EphemeralHarnessType}; use lazy_static::lazy_static; use ssz_types::Bitfield; @@ -65,6 +68,7 @@ fn valid_block_ok() { &block, None, BlockSignatureStrategy::VerifyIndividual, + VerifyBlockRoot::True, &spec, ); @@ -88,6 +92,7 @@ fn invalid_block_header_state_slot() { &SignedBeaconBlock::from_block(block, signature), None, BlockSignatureStrategy::VerifyIndividual, + VerifyBlockRoot::True, &spec, ); @@ -116,6 +121,7 @@ fn invalid_parent_block_root() { &SignedBeaconBlock::from_block(block, signature), None, BlockSignatureStrategy::VerifyIndividual, + VerifyBlockRoot::True, &spec, ); @@ -145,6 +151,7 @@ fn invalid_block_signature() { &SignedBeaconBlock::from_block(block, Signature::empty()), None, BlockSignatureStrategy::VerifyIndividual, + VerifyBlockRoot::True, &spec, ); @@ -174,6 +181,7 @@ fn invalid_randao_reveal_signature() { &signed_block, None, BlockSignatureStrategy::VerifyIndividual, + VerifyBlockRoot::True, &spec, ); diff --git a/lcli/src/transition_blocks.rs b/lcli/src/transition_blocks.rs index 04d15f5a11e..f78c6b005e7 100644 --- a/lcli/src/transition_blocks.rs +++ b/lcli/src/transition_blocks.rs @@ -1,7 +1,9 @@ use clap::ArgMatches; use eth2_network_config::Eth2NetworkConfig; use ssz::Encode; -use state_processing::{per_block_processing, per_slot_processing, BlockSignatureStrategy}; +use state_processing::{ + per_block_processing, per_slot_processing, BlockSignatureStrategy, VerifyBlockRoot, +}; use std::fs::File; use std::io::prelude::*; use std::path::{Path, PathBuf}; @@ -77,6 +79,7 @@ fn do_transition( &block, None, BlockSignatureStrategy::VerifyIndividual, + VerifyBlockRoot::True, spec, ) .map_err(|e| format!("State transition failed: {:?}", e))?; diff --git a/testing/ef_tests/src/cases/operations.rs b/testing/ef_tests/src/cases/operations.rs index 8ff6d8b81fd..d833846e471 100644 --- a/testing/ef_tests/src/cases/operations.rs +++ b/testing/ef_tests/src/cases/operations.rs @@ -12,7 +12,7 @@ use state_processing::per_block_processing::{ altair, base, process_attester_slashings, process_deposits, process_exits, process_proposer_slashings, }, - process_sync_aggregate, VerifySignatures, + process_sync_aggregate, VerifyBlockRoot, VerifySignatures, }; use std::fmt::Debug; use std::path::Path; @@ -183,7 +183,7 @@ impl Operation for BeaconBlock { spec: &ChainSpec, _: &Operations, ) -> Result<(), BlockProcessingError> { - process_block_header(state, self.to_ref(), spec)?; + process_block_header(state, self.to_ref(), VerifyBlockRoot::True, spec)?; Ok(()) } } diff --git a/testing/ef_tests/src/cases/sanity_blocks.rs b/testing/ef_tests/src/cases/sanity_blocks.rs index cb5708b12e1..c155be877ad 100644 --- a/testing/ef_tests/src/cases/sanity_blocks.rs +++ b/testing/ef_tests/src/cases/sanity_blocks.rs @@ -5,6 +5,7 @@ use crate::decode::{ssz_decode_file_with, ssz_decode_state, yaml_decode_file}; use serde_derive::Deserialize; use state_processing::{ per_block_processing, per_slot_processing, BlockProcessingError, BlockSignatureStrategy, + VerifyBlockRoot, }; use types::{BeaconState, EthSpec, ForkName, RelativeEpoch, SignedBeaconBlock}; @@ -98,6 +99,7 @@ impl Case for SanityBlocks { signed_block, None, BlockSignatureStrategy::VerifyIndividual, + VerifyBlockRoot::True, spec, )?; @@ -106,6 +108,7 @@ impl Case for SanityBlocks { signed_block, None, BlockSignatureStrategy::VerifyBulk, + VerifyBlockRoot::True, spec, )?; diff --git a/testing/ef_tests/src/cases/transition.rs b/testing/ef_tests/src/cases/transition.rs index 6ac56858a37..8e6ba226731 100644 --- a/testing/ef_tests/src/cases/transition.rs +++ b/testing/ef_tests/src/cases/transition.rs @@ -4,6 +4,7 @@ use crate::decode::{ssz_decode_file_with, ssz_decode_state, yaml_decode_file}; use serde_derive::Deserialize; use state_processing::{ per_block_processing, state_advance::complete_state_advance, BlockSignatureStrategy, + VerifyBlockRoot, }; use std::str::FromStr; use types::{BeaconState, Epoch, ForkName, SignedBeaconBlock}; @@ -97,6 +98,7 @@ impl Case for TransitionTest { block, None, BlockSignatureStrategy::VerifyBulk, + VerifyBlockRoot::True, spec, ) .map_err(|e| format!("Block processing failed: {:?}", e))?; diff --git a/testing/state_transition_vectors/src/exit.rs b/testing/state_transition_vectors/src/exit.rs index a52ccf420d7..75f82b3132f 100644 --- a/testing/state_transition_vectors/src/exit.rs +++ b/testing/state_transition_vectors/src/exit.rs @@ -2,7 +2,7 @@ use super::*; use beacon_chain::test_utils::{BeaconChainHarness, EphemeralHarnessType}; use state_processing::{ per_block_processing, per_block_processing::errors::ExitInvalid, BlockProcessingError, - BlockSignatureStrategy, + BlockSignatureStrategy, VerifyBlockRoot, }; use types::{BeaconBlock, BeaconState, Epoch, EthSpec, SignedBeaconBlock}; @@ -66,6 +66,7 @@ impl ExitTest { block, None, BlockSignatureStrategy::VerifyIndividual, + VerifyBlockRoot::True, &E::default_spec(), ) }