From 25e3dc930025a1847b86fa03845dc3c40df4d8f6 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 15 Aug 2022 01:31:00 +0000 Subject: [PATCH] Fix block verification and checkpoint sync caches (#3466) ## Issue Addressed Closes https://github.com/sigp/lighthouse/issues/2962 ## Proposed Changes Build all caches on the checkpoint state before storing it in the database. Additionally, fix a bug in `signature_verify_chain_segment` which prevented block verification from succeeding unless the previous epoch cache was already built. The previous epoch cache is required to verify the signatures of attestations included from previous epochs, even when all the blocks in the segment are from the same epoch. The comments around `signature_verify_chain_segment` have also been updated to reflect the fact that it should only be used on a chain of blocks from a single epoch. I believe this restriction had already been added at some point in the past and that the current comments were just outdated (and I think because the proposer shuffling can change in the next epoch based on the blocks applied in the current epoch that this limitation is essential). --- beacon_node/beacon_chain/src/beacon_chain.rs | 9 ++++----- beacon_node/beacon_chain/src/block_verification.rs | 8 ++++++-- beacon_node/beacon_chain/src/builder.rs | 6 ++++++ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 54c961e34d..f7d08c395d 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -2293,8 +2293,10 @@ impl BeaconChain { // Determine the epoch of the first block in the remaining segment. let start_epoch = block.slot().epoch(T::EthSpec::slots_per_epoch()); - // The `last_index` indicates the position of the last block that is in the current - // epoch of `start_epoch`. + // The `last_index` indicates the position of the first block in an epoch greater + // than the current epoch: partitioning the blocks into a run of blocks in the same + // epoch and everything else. These same-epoch blocks can all be signature-verified with + // the same `BeaconState`. let last_index = filtered_chain_segment .iter() .position(|(_root, block)| { @@ -2302,9 +2304,6 @@ impl BeaconChain { }) .unwrap_or(filtered_chain_segment.len()); - // Split off the first section blocks that are all either within the current epoch of - // the first block. These blocks can all be signature-verified with the same - // `BeaconState`. let mut blocks = filtered_chain_segment.split_off(last_index); std::mem::swap(&mut blocks, &mut filtered_chain_segment); diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 73330e7b56..4d84fe35e0 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -504,8 +504,8 @@ fn process_block_slash_info( /// /// ## Errors /// -/// The given `chain_segment` must span no more than two epochs, otherwise an error will be -/// returned. +/// The given `chain_segment` must contain only blocks from the same epoch, otherwise an error +/// will be returned. pub fn signature_verify_chain_segment( mut chain_segment: Vec<(Hash256, Arc>)>, chain: &BeaconChain, @@ -1702,6 +1702,9 @@ fn cheap_state_advance_to_obtain_committees<'a, E: EthSpec>( let block_epoch = block_slot.epoch(E::slots_per_epoch()); if state.current_epoch() == block_epoch { + // Build both the current and previous epoch caches, as the previous epoch caches are + // useful for verifying attestations in blocks from the current epoch. + state.build_committee_cache(RelativeEpoch::Previous, spec)?; state.build_committee_cache(RelativeEpoch::Current, spec)?; Ok(Cow::Borrowed(state)) @@ -1719,6 +1722,7 @@ fn cheap_state_advance_to_obtain_committees<'a, E: EthSpec>( partial_state_advance(&mut state, state_root_opt, target_slot, spec) .map_err(|e| BlockError::BeaconChainError(BeaconChainError::from(e)))?; + state.build_committee_cache(RelativeEpoch::Previous, spec)?; state.build_committee_cache(RelativeEpoch::Current, spec)?; Ok(Cow::Owned(state)) diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 252b7cef5a..cba9a56982 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -403,6 +403,12 @@ where )); } + // Prime all caches before storing the state in the database and computing the tree hash + // root. + weak_subj_state + .build_all_caches(&self.spec) + .map_err(|e| format!("Error building caches on checkpoint state: {e:?}"))?; + let computed_state_root = weak_subj_state .update_tree_hash_cache() .map_err(|e| format!("Error computing checkpoint state root: {:?}", e))?;