diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 71a5ff3fce..d199ccda94 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1374,32 +1374,35 @@ impl ExecutionPendingBlock { .observe_proposal(block_root, block.message()) .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))?; - if let Some(parent) = chain + match chain .canonical_head .fork_choice_read_lock() - .get_block(&block.parent_root()) + .is_parent_imported_status(block.as_block()) { - // Reject any block where the parent has an invalid payload. It's impossible for a valid - // block to descend from an invalid parent. - if parent.execution_status.is_invalid() { - return Err(BlockError::ParentExecutionPayloadInvalid { + ParentImportedStatus::Imported(parent) => { + // Reject any block where the parent has an invalid payload. It's impossible for a valid + // block to descend from an invalid parent. + if parent.execution_status.is_invalid() { + return Err(BlockError::ParentExecutionPayloadInvalid { + parent_root: block.parent_root(), + }); + } + } + ParentImportedStatus::UnknownBlock | ParentImportedStatus::UnimportedPayload => { + // Reject any block if its parent is not known to fork choice. + // + // A block that is not in fork choice is either: + // + // - Not yet imported: we should reject this block because we should only import a child + // after its parent has been fully imported. + // - Pre-finalized: if the parent block is _prior_ to finalization, we should ignore it + // because it will revert finalization. Note that the finalized block is stored in fork + // choice, so we will not reject any child of the finalized block (this is relevant during + // genesis). + return Err(BlockError::ParentUnknown { parent_root: block.parent_root(), }); } - } else { - // Reject any block if its parent is not known to fork choice. - // - // A block that is not in fork choice is either: - // - // - Not yet imported: we should reject this block because we should only import a child - // after its parent has been fully imported. - // - Pre-finalized: if the parent block is _prior_ to finalization, we should ignore it - // because it will revert finalization. Note that the finalized block is stored in fork - // choice, so we will not reject any child of the finalized block (this is relevant during - // genesis). - return Err(BlockError::ParentUnknown { - parent_root: block.parent_root(), - }); } /* @@ -1867,7 +1870,7 @@ fn verify_parent_block_is_known( // once the parent payload is retrieved). If execution_payload verification of block's execution // payload parent by an execution node is complete, verify the block's execution payload // parent (defined by bid.parent_block_hash) passes all validation. - match fork_choice_read_lock.is_parent_imported(&block) { + match fork_choice_read_lock.is_parent_imported_status(&block) { ParentImportedStatus::Imported(parent) => Ok((parent, block)), ParentImportedStatus::UnknownBlock | ParentImportedStatus::UnimportedPayload => { Err(BlockError::ParentUnknown { @@ -1900,7 +1903,7 @@ fn load_parent>( if !chain .canonical_head .fork_choice_read_lock() - .contains_block(&block.parent_root()) + .is_parent_imported(block.as_block()) { return Err(BlockError::ParentUnknown { parent_root: block.parent_root(), diff --git a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs index 7347fda517..2febc4cd4e 100644 --- a/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs +++ b/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs @@ -9,7 +9,6 @@ use crate::sync::network_context::{ }; use beacon_chain::BeaconChainTypes; use beacon_chain::BlockProcessStatus; -use beacon_chain::ParentImportedStatus; use beacon_chain::block_verification_types::AsBlock; use educe::Educe; use lighthouse_network::service::api_types::Id; @@ -41,10 +40,6 @@ pub struct AwaitingParent { } impl AwaitingParent { - pub fn parent_is_genesis(&self) -> bool { - self.parent_root == Hash256::ZERO - } - pub fn parent_root(&self) -> Hash256 { self.parent_root } @@ -627,16 +622,6 @@ impl SingleBlockLookup { self.awaiting_parent } - /// The parent relationship implied by this lookup's downloaded block: the parent root plus - /// (post-gloas) the parent's committed payload hash taken from this block's bid. `None` until - /// the block has been downloaded. Used to donate this lookup's peers to a FULL parent's - /// payload fetch. - pub fn downloaded_parent(&self) -> Option { - self.block_request - .peek_block() - .map(|block| AwaitingParent::from_block(block)) - } - /// Mark this lookup as no longer awaiting a parent lookup. Components can be sent for /// processing. pub fn resolve_awaiting_parent(&mut self) { @@ -737,27 +722,21 @@ impl SingleBlockLookup { // Check if the parent block is known to fork-choice. If the block is FULL // expect the payload to be imported too. - match cx + if !cx .chain .canonical_head .fork_choice_read_lock() .is_parent_imported(block) { - // Parent block is imported (and, if this block is FULL, its payload too): - // safe to send this block for processing. - ParentImportedStatus::Imported(_) => {} // Parent block is unknown, or it's FULL and the parent's payload has not // been imported yet. Park this lookup until the parent resolves. - ParentImportedStatus::UnknownBlock - | ParentImportedStatus::UnimportedPayload => { - let awaiting_parent = AwaitingParent::from_block(block); - self.awaiting_parent = Some(awaiting_parent); - return Ok(LookupResult::ParentUnknown { - awaiting_parent, - block_root: self.block_root, - peers: self.all_peers(), - }); - } + let awaiting_parent = AwaitingParent::from_block(block); + self.awaiting_parent = Some(awaiting_parent); + return Ok(LookupResult::ParentUnknown { + awaiting_parent, + block_root: self.block_root, + peers: self.all_peers(), + }); } let block = block.clone(); diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index b2d8ba4b57..b90a2463f1 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1556,7 +1556,15 @@ where } /// Returns the import status of the parent - pub fn is_parent_imported(&self, block: &SignedBeaconBlock) -> ParentImportedStatus { + pub fn is_parent_imported(&self, block: &SignedBeaconBlock) -> bool { + matches!( + self.is_parent_imported_status(block), + ParentImportedStatus::Imported(_) + ) + } + + /// Returns the import status of the parent + pub fn is_parent_imported_status(&self, block: &SignedBeaconBlock) -> ParentImportedStatus { if let Some(proto_block) = self.get_block(&block.parent_root()) { if let Ok(bid) = block.message().body().signed_execution_payload_bid() && proto_block.is_child_full(bid)