diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 9f25186edaa..c6d8ab02b59 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -980,14 +980,17 @@ fn load_parent( // exist in fork choice but not in the database yet. In such a case we simply // indicate that we don't yet know the parent. let root = block.parent_root(); - let parent_block = if let Some(block) = chain + let parent_block = chain .get_block(&block.parent_root()) .map_err(BlockError::BeaconChainError)? - { - block - } else { - return Err(BlockError::ParentUnknown(Box::new(block))); - }; + .ok_or_else(|| { + // Return a `MissingBeaconBlock` error instead of a `ParentUnknown` error since + // we've already checked fork choice for this block. + // + // It's an internal error if the block exists in fork choice but not in the + // database. + BlockError::from(BeaconChainError::MissingBeaconBlock(block.parent_root())) + })?; // Load the parent blocks state from the database, returning an error if it is not found. // It is an error because if we know the parent block we should also know the parent state. diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 87818ac8435..804771330fb 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -745,12 +745,11 @@ where /// Returns a `ProtoBlock` if the block is known **and** a descendant of the finalized root. pub fn get_block(&self, block_root: &Hash256) -> Option { - self.proto_array.get_block(block_root).filter(|block| { - // If available, use the parent_root to perform the lookup since it will involve one - // less lookup. This involves making the assumption that the finalized block will - // always have `block.parent_root` of `None`. - self.is_descendant_of_finalized(block.parent_root.unwrap_or(block.root)) - }) + if self.is_descendant_of_finalized(*block_root) { + self.proto_array.get_block(block_root) + } else { + None + } } /// Return `true` if `block_root` is equal to the finalized root, or a known descendant of it. diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index d8b71243b40..ffa9cbe6bd4 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -377,6 +377,26 @@ impl ForkChoiceTest { self } + + /// Check to ensure that we can read the finalized block. This is a regression test. + pub fn check_finalized_block_is_accessible(self) -> Self { + self.harness + .chain + .fork_choice + .write() + .get_block( + &self + .harness + .chain + .head_info() + .unwrap() + .finalized_checkpoint + .root, + ) + .unwrap(); + + self + } } fn is_safe_to_update(slot: Slot) -> bool { @@ -879,3 +899,11 @@ fn valid_attestation_skip_across_epoch() { |result| result.unwrap(), ); } + +#[test] +fn can_read_finalized_block() { + ForkChoiceTest::new() + .apply_blocks_while(|_, state| state.finalized_checkpoint.epoch == 0) + .apply_blocks(1) + .check_finalized_block_is_accessible(); +}