Remove redundent calls to get_block

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -1420,7 +1420,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 })
                 .collect();
 
-            let execution_optimistic = self.is_optimistic_head(Some(head_block_root))?;
+            let execution_optimistic = self.is_optimistic_head()?;
 
             Ok((duties, dependent_root, execution_optimistic))
         })
@@ -2819,7 +2819,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 snapshot_cache.insert(
                     BeaconSnapshot {
                         beacon_state: state,
-                        beacon_block: signed_block,
+                        beacon_block: signed_block.clone(),
                         beacon_block_root: block_root,
                     },
                     None,
@@ -2844,7 +2844,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 event_handler.register(EventKind::Block(SseBlock {
                     slot,
                     block: block_root,
-                    execution_optimistic: self.is_optimistic_block(&block_root)?,
+                    execution_optimistic: self.is_optimistic_block(&signed_block)?,
                 }));
             }
         }
@@ -3643,8 +3643,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                             current_duty_dependent_root,
                             previous_duty_dependent_root,
                             epoch_transition: is_epoch_transition,
-                            execution_optimistic: self
-                                .is_optimistic_head(Some(beacon_block_root))?,
+                            execution_optimistic: self.is_optimistic_head()?,
                         }));
                     }
                     (Err(e), _) | (_, Err(e)) => {
@@ -3666,7 +3665,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     new_head_block: beacon_block_root,
                     new_head_state: state_root,
                     epoch: head_slot.epoch(T::EthSpec::slots_per_epoch()),
-                    execution_optimistic: self.is_optimistic_head(Some(current_head.block_root))?,
+                    execution_optimistic: self.is_optimistic_head()?,
                 }));
             }
 
@@ -3692,7 +3691,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     observed_delay: block_delays.observed,
                     imported_delay: block_delays.imported,
                     set_as_head_delay: block_delays.set_as_head,
-                    execution_optimistic: self.is_optimistic_head(Some(beacon_block_root))?,
+                    execution_optimistic: self.is_optimistic_head()?,
                 }));
             }
         }
@@ -4098,48 +4097,40 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         }
     }
 
-    /// Returns the value of `execution_optimistic` for `block_root`.
+    /// Returns the value of `execution_optimistic` for `block`.
     ///
     /// Returns `Ok(false)` if the block is pre-Bellatrix, or has `ExecutionStatus::Valid`.
     /// Returns `Ok(true)` if the block has `ExecutionStatus::Unknown`.
-    pub fn is_optimistic_block(&self, block_root: &Hash256) -> Result<bool, BeaconChainError> {
-        if let Some(block) = self.get_block(block_root)? {
-            // Check if the block is pre-Bellatrix.
-            if block.message().execution_payload().is_err() {
-                Ok(false)
-            } else {
-                self.fork_choice
-                    .read()
-                    .is_optimistic_block(block_root)
-                    .map_err(BeaconChainError::ForkChoiceError)
-            }
+    pub fn is_optimistic_block(
+        &self,
+        block: &SignedBeaconBlock<T::EthSpec>,
+    ) -> Result<bool, BeaconChainError> {
+        // Check if the block is pre-Bellatrix.
+        if block.message().execution_payload().is_err() {
+            Ok(false)
         } else {
-            Err(BeaconChainError::MissingBeaconBlock(*block_root))
+            self.fork_choice
+                .read()
+                .is_optimistic_block(&block.canonical_root())
+                .map_err(BeaconChainError::ForkChoiceError)
         }
     }
 
-    /// Returns the value of `execution_optimistic` for `head_block_root` which can be provided
-    /// optionally if available.
+    /// Returns the value of `execution_optimistic` for the current head block.
     ///
     /// Returns `Ok(false)` if the head block is pre-Bellatrix, or has `ExecutionStatus::Valid`.
     /// Returns `Ok(true)` if the head block has `ExecutionStatus::Unknown`.
-    pub fn is_optimistic_head(
-        &self,
-        head_block_root: Option<Hash256>,
-    ) -> Result<bool, BeaconChainError> {
-        let head_block_root = head_block_root.unwrap_or(self.head_info()?.block_root);
-        if let Some(block) = self.get_block(&head_block_root)? {
-            // Check if the block is pre-Bellatrix.
-            if block.message().execution_payload().is_err() {
-                Ok(false)
-            } else {
-                self.fork_choice
-                    .read()
-                    .is_optimistic_block_no_fallback(&head_block_root)
-                    .map_err(BeaconChainError::ForkChoiceError)
-            }
+    pub fn is_optimistic_head(&self) -> Result<bool, BeaconChainError> {
+        let head_info = self.head_info()?;
+        let head_block_root = head_info.block_root;
+        // Check if the block is pre-Bellatrix.
+        if head_info.execution_payload_block_hash.is_none() {
+            Ok(false)
         } else {
-            Err(BeaconChainError::MissingBeaconBlock(head_block_root))
+            self.fork_choice
+                .read()
+                .is_optimistic_block_no_fallback(&head_block_root)
+                .map_err(BeaconChainError::ForkChoiceError)
         }
     }
 
@@ -4282,7 +4273,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     epoch: new_finalized_checkpoint.epoch,
                     block: new_finalized_checkpoint.root,
                     state: new_finalized_state_root,
-                    execution_optimistic: self.is_optimistic_head(None)?,
+                    execution_optimistic: self.is_optimistic_head()?,
                 }));
             }
         }

beacon_node/beacon_chain/src/beacon_proposer_cache.rs

@@ -153,7 +153,7 @@ pub fn compute_proposer_duties_from_head<T: BeaconChainTypes>(
         .proposer_shuffling_decision_root(chain.genesis_block_root)
         .map_err(BeaconChainError::from)?;
 
-    let execution_optimistic = chain.is_optimistic_head(Some(head.beacon_block_root))?;
+    let execution_optimistic = chain.is_optimistic_head()?;
 
     Ok((indices, dependent_root, execution_optimistic, state.fork()))
 }

beacon_node/http_api/src/block_id.rs

@@ -98,26 +98,28 @@ impl BlockId {
         }
     }
 
-    /// Return the `execution_optimistic` value identified by `self`.
-    pub fn is_execution_optimistic<T: BeaconChainTypes>(
+    /// Returns the `block` along with the `execution_optimistic` value identified by `self`.
+    pub fn block_and_is_execution_optimistic<T: BeaconChainTypes>(
         &self,
         chain: &BeaconChain<T>,
-    ) -> Result<bool, warp::Rejection> {
-        match self.0 {
+    ) -> Result<(SignedBeaconBlock<T::EthSpec>, bool), warp::Rejection> {
+        let block = self.block(chain)?;
+        let execution_optimistic = match self.0 {
             // Genesis block is inherently verified.
-            CoreBlockId::Genesis => Ok(false),
-            CoreBlockId::Head => Ok(chain
-                .is_optimistic_head(None)
-                .map_err(warp_utils::reject::beacon_chain_error)?),
+            CoreBlockId::Genesis => false,
+            CoreBlockId::Head => chain
+                .is_optimistic_head()
+                .map_err(warp_utils::reject::beacon_chain_error)?,
             // Slot, Finalized and Justified are determined based on the current head.
-            CoreBlockId::Slot(_) | CoreBlockId::Finalized | CoreBlockId::Justified => Ok(chain
-                .is_optimistic_head(None)
-                .map_err(warp_utils::reject::beacon_chain_error)?),
+            CoreBlockId::Slot(_) | CoreBlockId::Finalized | CoreBlockId::Justified => chain
+                .is_optimistic_head()
+                .map_err(warp_utils::reject::beacon_chain_error)?,
             // If the root is explicitly given, we can determine based on fork-choice.
-            CoreBlockId::Root(block_root) => Ok(chain
-                .is_optimistic_block(&block_root)
-                .map_err(warp_utils::reject::beacon_chain_error)?),
-        }
+            CoreBlockId::Root(_) => chain
+                .is_optimistic_block(&block)
+                .map_err(warp_utils::reject::beacon_chain_error)?,
+        };
+        Ok((block, execution_optimistic))
     }
 }
 

beacon_node/http_api/src/lib.rs

@@ -897,8 +897,8 @@ pub fn serve<T: BeaconChainTypes>(
                     // The value of `execution_optimistic` depends on whether the method used to
                     // compute the response is dependent on the head block.
                     let execution_optimistic = match uses_head {
-                        false => chain.is_optimistic_block(&root),
-                        true => chain.is_optimistic_head(Some(root)),
+                        false => chain.is_optimistic_block(&block),
+                        true => chain.is_optimistic_head(),
                     }
                     .map_err(warp_utils::reject::beacon_chain_error)?;
 
@@ -931,15 +931,14 @@ pub fn serve<T: BeaconChainTypes>(
         .and_then(|block_id: BlockId, chain: Arc<BeaconChain<T>>| {
             blocking_json_task(move || {
                 let root = block_id.root(&chain)?;
-                let block = BlockId::from_root(root).block(&chain)?;
+                let (block, execution_optimistic) =
+                    BlockId::from_root(root).block_and_is_execution_optimistic(&chain)?;
 
                 let canonical = chain
                     .block_root_at_slot(block.slot(), WhenSlotSkipped::None)
                     .map_err(warp_utils::reject::beacon_chain_error)?
                     .map_or(false, |canonical| root == canonical);
 
-                let execution_optimistic = block_id.is_execution_optimistic(&chain)?;
-
                 let data = api_types::BlockHeaderData {
                     root,
                     canonical,
@@ -1199,11 +1198,12 @@ pub fn serve<T: BeaconChainTypes>(
              chain: Arc<BeaconChain<T>>,
              accept_header: Option<api_types::Accept>| {
                 blocking_task(move || {
-                    let block = block_id.block(&chain)?;
+                    let (block, execution_optimistic) =
+                        block_id.block_and_is_execution_optimistic(&chain)?;
                     let fork_name = block
                         .fork_name(&chain.spec)
                         .map_err(inconsistent_fork_rejection)?;
-                    let execution_optimistic = block_id.is_execution_optimistic(&chain)?;
+
                     match accept_header {
                         Some(api_types::Accept::Ssz) => Response::builder()
                             .status(200)
@@ -1235,12 +1235,13 @@ pub fn serve<T: BeaconChainTypes>(
         .and(warp::path::end())
         .and_then(|block_id: BlockId, chain: Arc<BeaconChain<T>>| {
             blocking_json_task(move || {
-                let execution_optimistic = block_id.is_execution_optimistic(&chain)?;
-                block_id
-                    .root(&chain)
-                    .map(api_types::RootData::from)
-                    .map(api_types::GenericResponse::from)
-                    .map(|resp| resp.add_execution_optimistic(execution_optimistic))
+                let (block, execution_optimistic) =
+                    block_id.block_and_is_execution_optimistic(&chain)?;
+
+                Ok(api_types::GenericResponse::from(api_types::RootData::from(
+                    block.canonical_root(),
+                ))
+                .add_execution_optimistic(execution_optimistic))
             })
         });
 
@@ -1251,12 +1252,13 @@ pub fn serve<T: BeaconChainTypes>(
         .and(warp::path::end())
         .and_then(|block_id: BlockId, chain: Arc<BeaconChain<T>>| {
             blocking_json_task(move || {
-                let execution_optimistic = block_id.is_execution_optimistic(&chain)?;
-                block_id
-                    .block(&chain)
-                    .map(|block| block.message().body().attestations().clone())
-                    .map(api_types::GenericResponse::from)
-                    .map(|resp| resp.add_execution_optimistic(execution_optimistic))
+                let (block, execution_optimistic) =
+                    block_id.block_and_is_execution_optimistic(&chain)?;
+
+                Ok(
+                    api_types::GenericResponse::from(block.message().body().attestations().clone())
+                        .add_execution_optimistic(execution_optimistic),
+                )
             })
         });
 
@@ -1720,7 +1722,8 @@ pub fn serve<T: BeaconChainTypes>(
                                 None
                             } else if endpoint_version == V2 {
                                 BlockId::from_root(root)
-                                    .is_execution_optimistic(&chain)
+                                    .block_and_is_execution_optimistic(&chain)
+                                    .map(|(_, execution_optimistic)| execution_optimistic)
                                     .ok()
                             } else {
                                 return Err(unsupported_version_rejection(endpoint_version));

beacon_node/http_api/src/proposer_duties.rs

@@ -114,7 +114,7 @@ fn try_proposer_duties_from_cache<T: BeaconChainTypes>(
     };
 
     let execution_optimistic = chain
-        .is_optimistic_head(Some(head.block_root))
+        .is_optimistic_head()
         .map_err(warp_utils::reject::beacon_chain_error)?;
 
     chain

beacon_node/http_api/src/state_id.rs

@@ -119,7 +119,7 @@ impl StateId {
             | CoreStateId::Justified
             | CoreStateId::Finalized
             | CoreStateId::Root(_) => chain
-                .is_optimistic_head(None)
+                .is_optimistic_head()
                 .map_err(warp_utils::reject::beacon_chain_error),
         }
     }

beacon_node/http_api/src/sync_committees.rs

@@ -40,7 +40,7 @@ pub fn sync_committee_duties<T: BeaconChainTypes>(
     // Even when computing duties from state, any block roots pulled using the request epoch are
     // still dependent on the head. So using `is_optimistic_head` is fine for both cases.
     let execution_optimistic = chain
-        .is_optimistic_head(None)
+        .is_optimistic_head()
         .map_err(warp_utils::reject::beacon_chain_error)?;
 
     // Try using the head's sync committees to satisfy the request. This should be sufficient for