error handling and wiring up

beacon_node/network/src/sync/block_lookups/mod.rs

@@ -33,7 +33,9 @@ use beacon_chain::block_verification_types::AsBlock;
 use beacon_chain::data_availability_checker::{
     AvailabilityCheckError, AvailabilityCheckErrorCategory,
 };
-use beacon_chain::{AvailabilityProcessingStatus, BeaconChainTypes, BlockError};
+use beacon_chain::{
+    AvailabilityProcessingStatus, BeaconChainTypes, BlockError, BlockOrEnvelopeError,
+};
 pub use common::RequestState;
 use fnv::FnvHashMap;
 use lighthouse_network::service::api_types::SingleLookupReqId;
@@ -589,8 +591,12 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
 
         let action = match result {
             BlockProcessingResult::Ok(AvailabilityProcessingStatus::Imported(_))
-            | BlockProcessingResult::Err(BlockError::DuplicateFullyImported(..))
-            | BlockProcessingResult::Err(BlockError::GenesisBlock) => {
+            | BlockProcessingResult::Err(BlockOrEnvelopeError::BlockError(
+                BlockError::DuplicateFullyImported(..),
+            ))
+            | BlockProcessingResult::Err(BlockOrEnvelopeError::BlockError(
+                BlockError::GenesisBlock,
+            )) => {
                 // Successfully imported
                 request_state.on_processing_success()?;
                 Action::Continue
@@ -614,7 +620,9 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
                     Action::Retry
                 }
             }
-            BlockProcessingResult::Err(BlockError::DuplicateImportStatusUnknown(..)) => {
+            BlockProcessingResult::Err(BlockOrEnvelopeError::BlockError(
+                BlockError::DuplicateImportStatusUnknown(..),
+            )) => {
                 // This is unreachable because RPC blocks do not undergo gossip verification, and
                 // this error can *only* come from gossip verification.
                 error!(?block_root, "Single block lookup hit unreachable condition");
@@ -630,6 +638,11 @@ impl<T: BeaconChainTypes> BlockLookups<T> {
                 Action::Drop("Block processing ignored".to_owned())
             }
             BlockProcessingResult::Err(e) => {
+                let BlockOrEnvelopeError::BlockError(e) = e else {
+                    // TODO(gloas): handle properly
+                    return Err(LookupRequestError::Failed(format!("{e:?}")));
+                };
+
                 match e {
                     BlockError::BeaconChainError(e) => {
                         // Internal error

beacon_node/network/src/sync/manager.rs

@@ -50,7 +50,8 @@ use crate::sync::custody_backfill_sync::CustodyBackFillSync;
 use crate::sync::network_context::{PeerGroup, RpcResponseResult};
 use beacon_chain::block_verification_types::AsBlock;
 use beacon_chain::{
-    AvailabilityProcessingStatus, BeaconChain, BeaconChainTypes, BlockError, EngineState,
+    AvailabilityProcessingStatus, BeaconChain, BeaconChainTypes, BlockError, BlockOrEnvelopeError,
+    EngineState,
 };
 use futures::StreamExt;
 use lighthouse_network::SyncInfo;
@@ -206,7 +207,7 @@ impl BlockProcessType {
 #[derive(Debug)]
 pub enum BlockProcessingResult {
     Ok(AvailabilityProcessingStatus),
-    Err(BlockError),
+    Err(BlockOrEnvelopeError),
     Ignored,
 }
 
@@ -1449,8 +1450,8 @@ impl<T: BeaconChainTypes> SyncManager<T> {
     }
 }
 
-impl From<Result<AvailabilityProcessingStatus, BlockError>> for BlockProcessingResult {
-    fn from(result: Result<AvailabilityProcessingStatus, BlockError>) -> Self {
+impl From<Result<AvailabilityProcessingStatus, BlockOrEnvelopeError>> for BlockProcessingResult {
+    fn from(result: Result<AvailabilityProcessingStatus, BlockOrEnvelopeError>) -> Self {
         match result {
             Ok(status) => BlockProcessingResult::Ok(status),
             Err(e) => BlockProcessingResult::Err(e),
@@ -1458,8 +1459,14 @@ impl From<Result<AvailabilityProcessingStatus, BlockError>> for BlockProcessingR
     }
 }
 
-impl From<BlockError> for BlockProcessingResult {
-    fn from(e: BlockError) -> Self {
+impl From<BlockOrEnvelopeError> for BlockProcessingResult {
+    fn from(e: BlockOrEnvelopeError) -> Self {
         BlockProcessingResult::Err(e)
     }
 }
+
+impl From<BlockError> for BlockProcessingResult {
+    fn from(e: BlockError) -> Self {
+        BlockProcessingResult::Err(BlockOrEnvelopeError::BlockError(e))
+    }
+}

beacon_node/network/src/sync/tests/lookups.rs

@@ -2089,8 +2089,7 @@ async fn too_many_processing_failures(depth: usize) {
     r.build_chain_and_trigger_last_block(depth).await;
     // Simulate that a peer always returns empty
     r.simulate(
-        SimulateConfig::new()
-            .with_process_result(|| BlockProcessingResult::Err(BlockError::BlockSlotLimitReached)),
+        SimulateConfig::new().with_process_result(|| BlockError::BlockSlotLimitReached.into()),
     )
     .await;
     // We register multiple penalties, the lookup fails and sync does not progress
@@ -2158,9 +2157,10 @@ async fn test_single_block_lookup_duplicate_response() {
     let mut r = TestRig::default();
     r.build_chain_and_trigger_last_block(1).await;
     // Send a DuplicateFullyImported response, the lookup should complete successfully
-    r.simulate(SimulateConfig::new().with_process_result(|| {
-        BlockProcessingResult::Err(BlockError::DuplicateFullyImported(Hash256::ZERO))
-    }))
+    r.simulate(
+        SimulateConfig::new()
+            .with_process_result(|| BlockError::DuplicateFullyImported(Hash256::ZERO).into()),
+    )
     .await;
     // The block was not actually imported
     r.assert_head_slot(0);