Add reason of NoRequestNeeded to lookup sync

Author: dapplion

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

@@ -221,11 +221,11 @@ impl<T: BeaconChainTypes> SingleBlockLookup<T> {
                     // with this `req_id`.
                     request.get_state_mut().on_download_start(req_id)?
                 }
-                LookupRequestResult::NoRequestNeeded => {
+                LookupRequestResult::NoRequestNeeded(reason) => {
                     // Lookup sync event safety: Advances this request to the terminal `Processed`
                     // state. If all requests reach this state, the request is marked as completed
                     // in `Self::continue_requests`.
-                    request.get_state_mut().on_completed_request()?
+                    request.get_state_mut().on_completed_request(reason)?
                 }
                 // Sync will receive a future event to make progress on the request, do nothing now
                 LookupRequestResult::Pending(reason) => {
@@ -357,13 +357,13 @@ pub struct DownloadResult<T: Clone> {
 
 #[derive(IntoStaticStr)]
 pub enum State<T: Clone> {
-    AwaitingDownload(&'static str),
+    AwaitingDownload(/* reason */ &'static str),
     Downloading(ReqId),
     AwaitingProcess(DownloadResult<T>),
     /// Request is processing, sent by lookup sync
     Processing(DownloadResult<T>),
     /// Request is processed
-    Processed,
+    Processed(/* reason */ &'static str),
 }
 
 /// Object representing the state of a single block or blob lookup request.
@@ -554,7 +554,7 @@ impl<T: Clone> SingleLookupRequestState<T> {
     pub fn on_processing_success(&mut self) -> Result<(), LookupRequestError> {
         match &self.state {
             State::Processing(_) => {
-                self.state = State::Processed;
+                self.state = State::Processed("processing success");
                 Ok(())
             }
             other => Err(LookupRequestError::BadState(format!(
@@ -564,10 +564,10 @@ impl<T: Clone> SingleLookupRequestState<T> {
     }
 
     /// Mark a request as complete without any download or processing
-    pub fn on_completed_request(&mut self) -> Result<(), LookupRequestError> {
+    pub fn on_completed_request(&mut self, reason: &'static str) -> Result<(), LookupRequestError> {
         match &self.state {
             State::AwaitingDownload { .. } => {
-                self.state = State::Processed;
+                self.state = State::Processed(reason);
                 Ok(())
             }
             other => Err(LookupRequestError::BadState(format!(
@@ -598,11 +598,11 @@ impl<T: Clone> std::fmt::Display for State<T> {
 impl<T: Clone> std::fmt::Debug for State<T> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            Self::AwaitingDownload(status) => write!(f, "AwaitingDownload({:?})", status),
+            Self::AwaitingDownload(reason) => write!(f, "AwaitingDownload({})", reason),
             Self::Downloading(req_id) => write!(f, "Downloading({:?})", req_id),
             Self::AwaitingProcess(d) => write!(f, "AwaitingProcess({:?})", d.peer_group),
             Self::Processing(d) => write!(f, "Processing({:?})", d.peer_group),
-            Self::Processed { .. } => write!(f, "Processed"),
+            Self::Processed(reason) => write!(f, "Processed({})", reason),
         }
     }
 }

beacon_node/network/src/sync/network_context.rs

@@ -28,7 +28,6 @@ use rand::thread_rng;
 use requests::ActiveDataColumnsByRootRequest;
 pub use requests::LookupVerifyError;
 use slog::{debug, error, warn};
-use slot_clock::SlotClock;
 use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 use std::sync::Arc;
@@ -146,8 +145,9 @@ pub enum LookupRequestResult<I = ReqId> {
     /// A request is sent. Sync MUST receive an event from the network in the future for either:
     /// completed response or failed request
     RequestSent(I),
-    /// No request is sent, and no further action is necessary to consider this request completed
-    NoRequestNeeded,
+    /// No request is sent, and no further action is necessary to consider this request completed.
+    /// Includes a reason why this request is not needed.
+    NoRequestNeeded(&'static str),
     /// No request is sent, but the request is not completed. Sync MUST receive some future event
     /// that makes progress on the request. For example: request is processing from a different
     /// source (i.e. block received from gossip) and sync MUST receive an event with that processing
@@ -544,7 +544,9 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
             // Block is fully validated. If it's not yet imported it's waiting for missing block
             // components. Consider this request completed and do nothing.
             BlockProcessStatus::ExecutionValidated { .. } => {
-                return Ok(LookupRequestResult::NoRequestNeeded)
+                return Ok(LookupRequestResult::NoRequestNeeded(
+                    "block execution validated",
+                ))
             }
         }
 
@@ -595,21 +597,6 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
         block_root: Hash256,
         downloaded_block: Option<Arc<SignedBeaconBlock<T::EthSpec>>>,
     ) -> Result<LookupRequestResult, RpcRequestSendError> {
-        // Check if we are into deneb, and before peerdas
-        if !self
-            .chain
-            .data_availability_checker
-            .blobs_required_for_epoch(
-                // TODO(das): use the block's slot
-                self.chain
-                    .slot_clock
-                    .now_or_genesis()
-                    .ok_or(RpcRequestSendError::SlotClockError)?
-                    .epoch(T::EthSpec::slots_per_epoch()),
-            )
-        {
-            return Ok(LookupRequestResult::NoRequestNeeded);
-        }
         let Some(block) = downloaded_block.or_else(|| {
             // If the block is already being processed or fully validated, retrieve how many blobs
             // it expects. Consider any stage of the block. If the block root has been validated, we
@@ -639,7 +626,12 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
 
         // Check if we are into peerdas
         if !self.chain.should_fetch_blobs(block_epoch) {
-            return Ok(LookupRequestResult::NoRequestNeeded);
+            return Ok(LookupRequestResult::NoRequestNeeded("blobs not required"));
+        }
+
+        // No data required for this block
+        if expected_blobs == 0 {
+            return Ok(LookupRequestResult::NoRequestNeeded("no data"));
         }
 
         let imported_blob_indexes = self
@@ -654,7 +646,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
 
         if indices.is_empty() {
             // No blobs required, do not issue any request
-            return Ok(LookupRequestResult::NoRequestNeeded);
+            return Ok(LookupRequestResult::NoRequestNeeded("no indices to fetch"));
         }
 
         let req_id = self.next_id();
@@ -752,12 +744,12 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
 
         // Check if we are into peerdas
         if !self.chain.should_fetch_custody_columns(block_epoch) {
-            return Ok(LookupRequestResult::NoRequestNeeded);
+            return Ok(LookupRequestResult::NoRequestNeeded("columns not required"));
         }
 
         // No data required for this block
         if expected_blobs == 0 {
-            return Ok(LookupRequestResult::NoRequestNeeded);
+            return Ok(LookupRequestResult::NoRequestNeeded("no data"));
         }
 
         let custody_indexes_imported = self
@@ -777,7 +769,7 @@ impl<T: BeaconChainTypes> SyncNetworkContext<T> {
 
         if custody_indexes_to_fetch.is_empty() {
             // No indexes required, do not issue any request
-            return Ok(LookupRequestResult::NoRequestNeeded);
+            return Ok(LookupRequestResult::NoRequestNeeded("no indices to fetch"));
         }
 
         let req_id = self.next_id();

beacon_node/network/src/sync/network_context/custody.rs

@@ -296,7 +296,7 @@ impl<T: BeaconChainTypes> ActiveCustodyRequest<T> {
                     self.active_batch_columns_requests
                         .insert(req_id, ActiveBatchColumnsRequest { indices, peer_id });
                 }
-                LookupRequestResult::NoRequestNeeded => unreachable!(),
+                LookupRequestResult::NoRequestNeeded(_) => unreachable!(),
                 LookupRequestResult::Pending(_) => unreachable!(),
             }
         }