Add is_optimistic_candidate_block to fork choice

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::{
+use fork_choice::{ForkChoice, ForkChoiceStore, PayloadVerificationStatus};
-    Error as ForkChoiceError, 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 {
+            if !chain
-                // pass if the justified checkpoint has execution enabled
+                .fork_choice
-                let justified_root = state.current_justified_checkpoint().root;
+                .read()
-                let justified_checkpoint_execution_status = chain
+                .is_optimistic_candidate_block(
-                    .fork_choice
+                    current_slot,
-                    .read()
+                    block.slot(),
-                    .get_block(&justified_root)
+                    &block.parent_root(),
-                    .map(|block| block.execution_status)
+                    &chain.spec,
-                    .ok_or(BeaconChainError::ForkChoiceError(
+                )
-                        ForkChoiceError::MissingProtoArrayBlock(justified_root),
+                .map_err(BeaconChainError::from)?
-                    ))?;
+            {
-                if matches!(
+                return Err(ExecutionPayloadError::UnverifiedNonOptimisticCandidate.into());
-                    justified_checkpoint_execution_status,
-                    ExecutionStatus::Irrelevant(_)
-                ) {
-                    return Err(ExecutionPayloadError::UnverifiedNonOptimisticCandidate.into());
-                }
             }
         }
 

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<bool, Error<T::Error>> {
         // 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)
     }