Merge pull request #4535 from qu0b/blob/empty

API - Handle blocks without blobs gracefully

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -1102,10 +1102,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     pub fn get_blobs_checking_early_attester_cache(
         &self,
         block_root: &Hash256,
-    ) -> Result<Option<BlobSidecarList<T::EthSpec>>, Error> {
+    ) -> Result<BlobSidecarList<T::EthSpec>, Error> {
         self.early_attester_cache
             .get_blobs(*block_root)
-            .map_or_else(|| self.get_blobs(block_root), |blobs| Ok(Some(blobs)))
+            .map_or_else(|| self.get_blobs(block_root), Ok)
     }
 
     /// Returns the block at the given root, if any.
@@ -1185,11 +1185,11 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     /// - block and blobs are inconsistent in the database
     /// - this method is called with a pre-deneb block root
     /// - this method is called for a blob that is beyond the prune depth
-    pub fn get_blobs(
-        &self,
-        block_root: &Hash256,
-    ) -> Result<Option<BlobSidecarList<T::EthSpec>>, Error> {
-        Ok(self.store.get_blobs(block_root)?)
+    pub fn get_blobs(&self, block_root: &Hash256) -> Result<BlobSidecarList<T::EthSpec>, Error> {
+        match self.store.get_blobs(block_root)? {
+            Some(blobs) => Ok(blobs),
+            None => Ok(BlobSidecarList::default()),
+        }
     }
 
     pub fn get_blinded_block(

beacon_node/beacon_chain/src/test_utils.rs

@@ -698,14 +698,14 @@ where
         let block = self.chain.head_beacon_block();
         let block_root = block.canonical_root();
         let blobs = self.chain.get_blobs(&block_root).unwrap();
-        RpcBlock::new(block, blobs).unwrap()
+        RpcBlock::new(block, Some(blobs)).unwrap()
     }
 
     pub fn get_full_block(&self, block_root: &Hash256) -> RpcBlock<E> {
         let block = self.chain.get_blinded_block(block_root).unwrap().unwrap();
         let full_block = self.chain.store.make_full_block(block_root, block).unwrap();
         let blobs = self.chain.get_blobs(block_root).unwrap();
-        RpcBlock::new(Arc::new(full_block), blobs).unwrap()
+        RpcBlock::new(Arc::new(full_block), Some(blobs)).unwrap()
     }
 
     pub fn get_all_validators(&self) -> Vec<usize> {

beacon_node/beacon_chain/tests/attestation_production.rs

@@ -134,7 +134,8 @@ async fn produces_attestations() {
             assert_eq!(data.target.root, target_root, "bad target root");
 
             let rpc_block =
-                RpcBlock::<MainnetEthSpec>::new(Arc::new(block.clone()), blobs.clone()).unwrap();
+                RpcBlock::<MainnetEthSpec>::new(Arc::new(block.clone()), Some(blobs.clone()))
+                    .unwrap();
             let beacon_chain::data_availability_checker::MaybeAvailableBlock::Available(available_block) = chain
                 .data_availability_checker
                 .check_rpc_block_availability(rpc_block)
@@ -209,7 +210,8 @@ async fn early_attester_cache_old_request() {
         .get_blobs(&head.beacon_block_root)
         .expect("should get blobs");
 
-    let rpc_block = RpcBlock::<MainnetEthSpec>::new(head.beacon_block.clone(), head_blobs).unwrap();
+    let rpc_block =
+        RpcBlock::<MainnetEthSpec>::new(head.beacon_block.clone(), Some(head_blobs)).unwrap();
     let beacon_chain::data_availability_checker::MaybeAvailableBlock::Available(available_block) = harness.chain
         .data_availability_checker
         .check_rpc_block_availability(rpc_block)

beacon_node/beacon_chain/tests/block_verification.rs

@@ -66,12 +66,12 @@ async fn get_chain_segment() -> (Vec<BeaconSnapshot<E>>, Vec<Option<BlobSidecarL
             beacon_block: Arc::new(full_block),
             beacon_state: snapshot.beacon_state,
         });
-        segment_blobs.push(
+        segment_blobs.push(Some(
             harness
                 .chain
                 .get_blobs(&snapshot.beacon_block_root)
                 .unwrap(),
-        )
+        ))
     }
     (segment, segment_blobs)
 }
@@ -114,26 +114,22 @@ async fn get_chain_segment_with_signed_blobs() -> (
             .chain
             .get_blobs(&snapshot.beacon_block_root)
             .unwrap()
-            .map(|blobs| {
-                let blobs = blobs
-                    .into_iter()
-                    .map(|blob| {
-                        let block_root = blob.block_root;
-                        let blob_index = blob.index;
-                        SignedBlobSidecar {
-                            message: blob,
-                            signature: harness
-                                .blob_signature_cache
-                                .read()
-                                .get(&BlobSignatureKey::new(block_root, blob_index))
-                                .unwrap()
-                                .clone(),
-                        }
-                    })
-                    .collect::<Vec<_>>();
-                VariableList::from(blobs)
-            });
-        segment_blobs.push(signed_blobs)
+            .into_iter()
+            .map(|blob| {
+                let block_root = blob.block_root;
+                let blob_index = blob.index;
+                SignedBlobSidecar {
+                    message: blob,
+                    signature: harness
+                        .blob_signature_cache
+                        .read()
+                        .get(&BlobSignatureKey::new(block_root, blob_index))
+                        .unwrap()
+                        .clone(),
+                }
+            })
+            .collect::<Vec<_>>();
+        segment_blobs.push(Some(VariableList::from(signed_blobs)))
     }
     (segment, segment_blobs)
 }

beacon_node/beacon_chain/tests/store_tests.rs

@@ -2179,7 +2179,7 @@ async fn weak_subjectivity_sync() {
         beacon_chain
             .process_block(
                 full_block.canonical_root(),
-                RpcBlock::new(Arc::new(full_block), blobs).unwrap(),
+                RpcBlock::new(Arc::new(full_block), Some(blobs)).unwrap(),
                 NotifyExecutionLayer::Yes,
                 || Ok(()),
             )
@@ -2239,7 +2239,7 @@ async fn weak_subjectivity_sync() {
         if let MaybeAvailableBlock::Available(block) = harness
             .chain
             .data_availability_checker
-            .check_rpc_block_availability(RpcBlock::new(Arc::new(full_block), blobs).unwrap())
+            .check_rpc_block_availability(RpcBlock::new(Arc::new(full_block), Some(blobs)).unwrap())
             .expect("should check availability")
         {
             available_blocks.push(block);

beacon_node/http_api/src/block_id.rs

@@ -258,14 +258,9 @@ impl BlockId {
         chain: &BeaconChain<T>,
     ) -> Result<BlobSidecarList<T::EthSpec>, warp::Rejection> {
         let root = self.root(chain)?.0;
-        match chain.get_blobs(&root) {
-            Ok(Some(blob_sidecar_list)) => Ok(blob_sidecar_list),
-            Ok(None) => Err(warp_utils::reject::custom_not_found(format!(
-                "No blobs with block root {} found in the store",
-                root
-            ))),
-            Err(e) => Err(warp_utils::reject::beacon_chain_error(e)),
-        }
+        chain
+            .get_blobs(&root)
+            .map_err(warp_utils::reject::beacon_chain_error)
     }
 
     pub async fn blob_sidecar_list_filtered<T: BeaconChainTypes>(

beacon_node/network/src/network_beacon_processor/rpc_methods.rs

@@ -11,7 +11,7 @@ use lighthouse_network::rpc::methods::{
 use lighthouse_network::rpc::StatusMessage;
 use lighthouse_network::rpc::*;
 use lighthouse_network::{PeerId, PeerRequestId, ReportSource, Response, SyncInfo};
-use slog::{debug, error, trace, warn};
+use slog::{debug, error, warn};
 use slot_clock::SlotClock;
 use std::collections::{hash_map::Entry, HashMap};
 use std::sync::Arc;
@@ -247,7 +247,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                 };
 
                 match blob_list_result.as_ref() {
-                    Ok(Some(blobs_sidecar_list)) => {
+                    Ok(blobs_sidecar_list) => {
                         'inner: for blob_sidecar in blobs_sidecar_list.iter() {
                             if blob_sidecar.index == index {
                                 self.send_response(
@@ -260,14 +260,6 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                             }
                         }
                     }
-                    Ok(None) => {
-                        debug!(
-                            self.log,
-                            "Peer requested unknown blobs";
-                            "peer" => %peer_id,
-                            "request_root" => ?root
-                        );
-                    }
                     Err(e) => {
                         debug!(
                             self.log,
@@ -767,7 +759,7 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
 
         for root in block_roots {
             match self.chain.get_blobs(&root) {
-                Ok(Some(blob_sidecar_list)) => {
+                Ok(blob_sidecar_list) => {
                     for blob_sidecar in blob_sidecar_list.iter() {
                         blobs_sent += 1;
                         self.send_network_message(NetworkMessage::SendResponse {
@@ -777,15 +769,6 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                         });
                     }
                 }
-                Ok(None) => {
-                    trace!(
-                        self.log,
-                        "No blobs in the store for block root";
-                        "request" => ?req,
-                        "peer" => %peer_id,
-                        "block_root" => ?root
-                    );
-                }
                 Err(e) => {
                     error!(
                         self.log,

beacon_node/network/src/network_beacon_processor/tests.rs

@@ -1087,12 +1087,7 @@ async fn test_blobs_by_range() {
             .block_root_at_slot(Slot::new(slot), WhenSlotSkipped::None)
             .unwrap();
         blob_count += root
-            .and_then(|root| {
-                rig.chain
-                    .get_blobs(&root)
-                    .unwrap_or_default()
-                    .map(|blobs| blobs.len())
-            })
+            .map(|root| rig.chain.get_blobs(&root).unwrap_or_default().len())
             .unwrap_or(0);
     }
     let mut actual_count = 0;