From b9655b658e0649d151e189eb850accc68c05a0e7 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Wed, 1 May 2024 02:10:15 -0400 Subject: [PATCH] Ensure block only range requests don't fail on download (#5675) * ensure pruned blobs don't fail on download * Typo --- .../src/block_verification_types.rs | 7 ++++- .../src/sync/block_sidecar_coupling.rs | 26 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/block_verification_types.rs b/beacon_node/beacon_chain/src/block_verification_types.rs index fb0e0c965f1..d0360bf18e5 100644 --- a/beacon_node/beacon_chain/src/block_verification_types.rs +++ b/beacon_node/beacon_chain/src/block_verification_types.rs @@ -96,13 +96,18 @@ impl RpcBlock { } /// 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, block: Arc>, blobs: Option>, ) -> Result { 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(), diff --git a/beacon_node/network/src/sync/block_sidecar_coupling.rs b/beacon_node/network/src/sync/block_sidecar_coupling.rs index 6a3b568c1c4..d159733cbc7 100644 --- a/beacon_node/network/src/sync/block_sidecar_coupling.rs +++ b/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::::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::(ForkName::Deneb, NumBlobs::Number(3), &mut rng).0 + }) + .collect::>(); + + // 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(); + } }