diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 15bb47ef0a..03b4b22bf3 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -55,11 +55,9 @@ use crate::{ }; use eth2::types::EventKind; use execution_layer::PayloadStatusV1Status; -use fork_choice::{ - Error as ForkChoiceError, ForkChoice, ForkChoiceStore, PayloadVerificationStatus, -}; +use fork_choice::{ForkChoice, ForkChoiceStore, PayloadVerificationStatus}; use parking_lot::RwLockReadGuard; -use proto_array::{Block as ProtoBlock, ExecutionStatus}; +use proto_array::Block as ProtoBlock; use safe_arith::ArithError; use slog::{debug, error, Logger}; use slot_clock::SlotClock; @@ -1135,31 +1133,26 @@ impl<'a, T: BeaconChainTypes> FullyVerifiedBlock<'a, T> { // `randao` may change. let payload_verification_status = notify_new_payload(chain, &state, block.message())?; + // If the payload did not validate or invalidate the block, check to see if this block is + // valid for optimistic import. if payload_verification_status == PayloadVerificationStatus::NotVerified { - // Check the optimistic sync conditions before going further - // https://github.com/ethereum/consensus-specs/blob/v1.1.9/sync/optimistic.md#when-to-optimistically-import-blocks let current_slot = chain .slot_clock .now() .ok_or(BeaconChainError::UnableToReadSlot)?; - // pass if current slot is at least SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY ahead of the block - if current_slot - block.slot() < chain.spec.safe_slots_to_import_optimistically { - // pass if the justified checkpoint has execution enabled - let justified_root = state.current_justified_checkpoint().root; - let justified_checkpoint_execution_status = chain - .fork_choice - .read() - .get_block(&justified_root) - .map(|block| block.execution_status) - .ok_or(BeaconChainError::ForkChoiceError( - ForkChoiceError::MissingProtoArrayBlock(justified_root), - ))?; - if matches!( - justified_checkpoint_execution_status, - ExecutionStatus::Irrelevant(_) - ) { - return Err(ExecutionPayloadError::UnverifiedNonOptimisticCandidate.into()); - } + + if !chain + .fork_choice + .read() + .is_optimistic_candidate_block( + current_slot, + block.slot(), + &block.parent_root(), + &chain.spec, + ) + .map_err(BeaconChainError::from)? + { + return Err(ExecutionPayloadError::UnverifiedNonOptimisticCandidate.into()); } } diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 6f93407c06..a9a4e24be7 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -910,7 +910,14 @@ where .is_descendant(self.fc_store.finalized_checkpoint().root, block_root) } - pub fn safe_to_import_optimistically( + /// Returns `Ok(false)` if a block is not viable to be imported optimistically. + /// + /// ## Notes + /// + /// Equivalent to the function with the same name in the optimistic sync specs: + /// + /// https://github.com/ethereum/consensus-specs/blob/dev/sync/optimistic.md#helpers + pub fn is_optimistic_candidate_block( &self, current_slot: Slot, block_slot: Slot, @@ -918,13 +925,24 @@ where spec: &ChainSpec, ) -> Result> { // If the block is sufficiently old, import it. - if block_slot + spec.safe_slots_to_import_optimistically > current_slot { + if block_slot + spec.safe_slots_to_import_optimistically <= current_slot { + return Ok(true); + } + + // If the justified block has execution enabled, then optimistically import any block. + if self + .get_justified_block()? + .execution_status + .is_execution_enabled() + { return Ok(true); } // If the block has an ancestor with a verified parent, import this block. // - // TODO(paul): this is not in the spec, add it. + // TODO: This condition is not yet merged into the spec. See: + // + // https://github.com/ethereum/consensus-specs/pull/2841 // // ## Note // @@ -937,15 +955,6 @@ where return Ok(true); } - // If the justified block has execution enabled, then optimistically import any block. - if !self - .get_justified_block()? - .execution_status - .is_execution_enabled() - { - return Ok(true); - } - Ok(false) }