fix/consolidate some error handling

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -971,7 +971,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         }
         Ok((
             self.get_block(block_root).await?.map(Arc::new),
-            self.get_blobs(block_root).ok().flatten().map(Arc::new),
+            self.get_blobs(block_root)?.map(Arc::new),
         ))
     }
 
@@ -1044,9 +1044,17 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
 
     /// Returns the blobs at the given root, if any.
     ///
-    /// ## Errors
+    /// Returns `Ok(None)` if the blobs are not found. This could indicate the blob has been pruned
+    /// or that the block it is referenced by doesn't exist in our database.
     ///
-    /// May return a database error.
+    /// If we can find the corresponding block in our database, we know whether we *should* have
+    /// blobs. If we should have blobs and no blobs are found, this will error. If we shouldn't,
+    /// this will reconstruct an empty `BlobsSidecar`.
+    ///
+    /// ## Errors
+    /// - any database read errors
+    /// - block and blobs are inconsistent in the database
+    /// - this method is called with a pre-eip4844 block root
     pub fn get_blobs(
         &self,
         block_root: &Hash256,
@@ -1054,23 +1062,33 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         match self.store.get_blobs(block_root)? {
             Some(blobs) => Ok(Some(blobs)),
             None => {
-                if let Ok(Some(block)) = self.get_blinded_block(block_root) {
+                // Check for the corresponding block to understand whether we *should* have blobs.
+                self
+                    .get_blinded_block(block_root)?
+                    .map(|block| {
+                        // If there are no KZG commitments in the block, we know the sidecar should
+                        // be empty.
                         let expected_kzg_commitments = block.message().body().blob_kzg_commitments()?;
-
+                        if expected_kzg_commitments.is_empty() {
-                    if expected_kzg_commitments.len() > 0 {
-                        Err(Error::DBInconsistent(format!(
-                            "Expected kzg commitments but no blobs stored for block root {}",
-                            block_root
-                        )))
-                    } else {
                             Ok(Some(BlobsSidecar::empty_from_parts(
                                 *block_root,
                                 block.slot(),
                             )))
-                    }
                         } else {
+                            if let Some(boundary) = self.data_availability_boundary() {
+                                // We should have blobs for all blocks after the boundary.
+                                if boundary <= block.epoch() {
+                                    return Err(Error::DBInconsistent(format!(
+                                        "Expected kzg commitments but no blobs stored for block root {}",
+                                        block_root
+                                    )))
+                                }
+                            }
                             Ok(None)
                         }
+                    })
+                    .transpose()
+                    .map(Option::flatten)
             }
         }
     }

beacon_node/network/src/beacon_processor/worker/rpc_methods.rs

@@ -247,35 +247,38 @@ impl<T: BeaconChainTypes> Worker<T> {
                             );
                         }
                         Ok((Some(block), None)) => {
-                            let data_availability_boundary_by_root = self.chain.finalized_data_availability_boundary();
+                            let finalized_data_availability_boundary = self.chain.finalized_data_availability_boundary();
                             let block_epoch = block.epoch();
 
-                           if Some(block_epoch) >= data_availability_boundary_by_root {
+                           if Some(block_epoch) >= finalized_data_availability_boundary {
                                 debug!(
                                     self.log,
                                     "Peer requested block and blob that should be available, but no blob found";
                                     "peer" => %peer_id,
                                     "request_root" => ?root,
-                                    "data_availability_boundary_by_root" => data_availability_boundary_by_root,
+                                    "finalized_data_availability_boundary" => finalized_data_availability_boundary,
                                 );
                                 self.send_error_response(
                                     peer_id,
                                     RPCResponseErrorCode::ResourceUnavailable,
-                                    "No blob for requested block.".into(),
+                                    "Blobs unavailable".into(),
                                     request_id,
                                 );
+                               send_response = false;
+                               break;
                             } else {
                                 debug!(
                                     self.log,
-                                    "Peer requested block and blob older than the data availability boundary for ByRoot request, no blob found";
+                                    "Peer requested block and blob older than the data availability \
+                                    boundary for ByRoot request, no blob found";
                                     "peer" => %peer_id,
                                     "request_root" => ?root,
-                                    "data_availability_boundary_by_root" => data_availability_boundary_by_root,
+                                    "finalized_data_availability_boundary" => finalized_data_availability_boundary,
                                 );
                                 self.send_error_response(
                                     peer_id,
                                     RPCResponseErrorCode::ResourceUnavailable,
-                                    format!("No blob for requested block. Requested blob is older than the data availability boundary for a ByRoot request, currently at epoch {:?}", data_availability_boundary_by_root),
+                                    "Blobs unavailable".into(),
                                     request_id,
                                 );
                             }
@@ -717,7 +720,7 @@ impl<T: BeaconChainTypes> Worker<T> {
         let block_roots = block_roots.into_iter().flatten().collect::<Vec<_>>();
 
         let mut blobs_sent = 0;
-        let send_response = true;
+        let mut send_response = true;
 
         for root in block_roots {
             match self.chain.get_blobs(&root) {
@@ -735,6 +738,13 @@ impl<T: BeaconChainTypes> Worker<T> {
                         "No blobs or block in the store for block root";
                         "block_root" => ?root
                     );
+                    self.send_error_response(
+                        peer_id,
+                        RPCResponseErrorCode::ResourceUnavailable,
+                        "Blobs unavailable".into(),
+                        request_id,
+                    );
+                    send_response = false;
                     break;
                 }
                 Err(e) => {
@@ -744,6 +754,13 @@ impl<T: BeaconChainTypes> Worker<T> {
                         "block_root" => ?root,
                         "error" => ?e
                     );
+                    self.send_error_response(
+                        peer_id,
+                        RPCResponseErrorCode::ServerError,
+                        "Failed fetching blobs".into(),
+                        request_id,
+                    );
+                    send_response = false;
                     break;
                 }
             }