Merge of #5675

beacon_node/beacon_chain/src/block_verification_types.rs

@@ -96,13 +96,18 @@ impl<E: EthSpec> RpcBlock<E> {
     }
 
     /// Constructs a new `BlockAndBlobs` variant after making consistency
-    /// checks between the provided blocks and blobs.
+    /// checks between the provided blocks and blobs. This struct makes no
+    /// guarantees about whether blobs should be present, only that they are
+    /// consistent with the block. An empty list passed in for `blobs` is
+    /// viewed the same as `None` passed in.
     pub fn new(
         block_root: Option<Hash256>,
         block: Arc<SignedBeaconBlock<E>>,
         blobs: Option<BlobSidecarList<E>>,
     ) -> Result<Self, AvailabilityCheckError> {
         let block_root = block_root.unwrap_or_else(|| get_block_root(&block));
+        // Treat empty blob lists as if they are missing.
+        let blobs = blobs.filter(|b| !b.is_empty());
 
         if let (Some(blobs), Ok(block_commitments)) = (
             blobs.as_ref(),

beacon_node/network/src/sync/block_sidecar_coupling.rs

@@ -130,4 +130,30 @@ mod tests {
         assert!(info.is_finished());
         info.into_responses().unwrap();
     }
+
+    #[test]
+    fn empty_blobs_into_responses() {
+        let mut info = BlocksAndBlobsRequestInfo::<E>::new(ByRangeRequestType::BlocksAndBlobs);
+        let mut rng = XorShiftRng::from_seed([42; 16]);
+        let blocks = (0..4)
+            .map(|_| {
+                // Always generate some blobs.
+                generate_rand_block_and_blobs::<E>(ForkName::Deneb, NumBlobs::Number(3), &mut rng).0
+            })
+            .collect::<Vec<_>>();
+
+        // Send blocks and complete terminate response
+        for block in blocks {
+            info.add_block_response(Some(block.into()));
+        }
+        info.add_block_response(None);
+        // Expect no blobs returned
+        info.add_sidecar_response(None);
+
+        // Assert response is finished and RpcBlocks can be constructed, even if blobs weren't returned.
+        // This makes sure we don't expect blobs here when they have expired. Checking this logic should
+        // be hendled elsewhere.
+        assert!(info.is_finished());
+        info.into_responses().unwrap();
+    }
 }