From e3d2d38497ba8e62d000ace76ad8796ec3491bf5 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Fri, 30 Aug 2024 15:28:21 +1000 Subject: [PATCH] Return HTTP 404 for pruned blob requests (#6331) * Return HTTP 404 for pruned blob requests --- beacon_node/http_api/src/block_id.rs | 23 +++++- beacon_node/http_api/tests/tests.rs | 117 +++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 3 deletions(-) diff --git a/beacon_node/http_api/src/block_id.rs b/beacon_node/http_api/src/block_id.rs index f35df2f5e8..668efc513f 100644 --- a/beacon_node/http_api/src/block_id.rs +++ b/beacon_node/http_api/src/block_id.rs @@ -274,10 +274,27 @@ impl BlockId { warp_utils::reject::custom_not_found(format!("beacon block with root {}", root)) })?; + // Error if the block is pre-Deneb and lacks blobs. + let blob_kzg_commitments = block.message().body().blob_kzg_commitments().map_err(|_| { + warp_utils::reject::custom_bad_request( + "block is pre-Deneb and has no blobs".to_string(), + ) + })?; + // Return the `BlobSidecarList` identified by `self`. - let blob_sidecar_list = chain - .get_blobs(&root) - .map_err(warp_utils::reject::beacon_chain_error)?; + let blob_sidecar_list = if !blob_kzg_commitments.is_empty() { + chain + .store + .get_blobs(&root) + .map_err(|e| warp_utils::reject::beacon_chain_error(e.into()))? + .ok_or_else(|| { + warp_utils::reject::custom_not_found(format!( + "no blobs stored for block {root}" + )) + })? + } else { + BlobSidecarList::default() + }; let blob_sidecar_list_filtered = match indices.indices { Some(vec) => { diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 62c196d917..6e6f72b6c0 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -1667,6 +1667,93 @@ impl ApiTester { self } + /// Test fetching of blob sidecars that are not available in the database due to pruning. + /// + /// If `zero_blobs` is false, test a block with >0 blobs, which should be unavailable. + /// If `zero_blobs` is true, then test a block with 0 blobs, which should still be available. + pub async fn test_get_blob_sidecars_pruned(self, zero_blobs: bool) -> Self { + // Prune all blobs prior to the database's split epoch. + let store = &self.chain.store; + let split_epoch = store.get_split_slot().epoch(E::slots_per_epoch()); + let force_prune = true; + self.chain + .store + .try_prune_blobs(force_prune, split_epoch) + .unwrap(); + + let oldest_blob_slot = store.get_blob_info().oldest_blob_slot.unwrap(); + + assert_ne!( + oldest_blob_slot, 0, + "blob pruning should have pruned some blobs" + ); + + // Find a block with either 0 blobs or 1+ depending on the value of `zero_blobs`. + let mut test_slot = None; + for slot in 0..oldest_blob_slot.as_u64() { + let block_id = BlockId(CoreBlockId::Slot(Slot::new(slot))); + let (block, _, _) = block_id.blinded_block(&self.chain).unwrap(); + let num_blobs = block.num_expected_blobs(); + + if (zero_blobs && num_blobs == 0) || (!zero_blobs && num_blobs > 0) { + test_slot = Some(Slot::new(slot)); + break; + } + } + let test_slot = test_slot.expect(&format!( + "should be able to find a block matching zero_blobs={zero_blobs}" + )); + + match self + .client + .get_blobs::(CoreBlockId::Slot(test_slot), None) + .await + { + Ok(result) => { + if zero_blobs { + assert_eq!( + &result.unwrap().data[..], + &[], + "empty blobs are always available" + ); + } else { + assert_eq!(result, None, "blobs should have been pruned"); + } + } + Err(e) => panic!("failed with non-404 status: {e:?}"), + } + + self + } + + pub async fn test_get_blob_sidecars_pre_deneb(self) -> Self { + let oldest_blob_slot = self.chain.store.get_blob_info().oldest_blob_slot.unwrap(); + assert_ne!( + oldest_blob_slot, 0, + "oldest_blob_slot should be non-zero and post-Deneb" + ); + let test_slot = oldest_blob_slot - 1; + assert!( + !self + .chain + .spec + .fork_name_at_slot::(test_slot) + .deneb_enabled(), + "Deneb should not be enabled at {test_slot}" + ); + + match self + .client + .get_blobs::(CoreBlockId::Slot(test_slot), None) + .await + { + Ok(result) => panic!("queries for pre-Deneb slots should fail. got: {result:?}"), + Err(e) => assert_eq!(e.status().unwrap(), 400), + } + + self + } + pub async fn test_beacon_blocks_attestations(self) -> Self { for block_id in self.interesting_block_ids() { let result = self @@ -6846,6 +6933,36 @@ async fn get_blob_sidecars() { .await; } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn get_blob_sidecars_pruned() { + let mut config = ApiTesterConfig::default(); + config.spec.altair_fork_epoch = Some(Epoch::new(0)); + config.spec.bellatrix_fork_epoch = Some(Epoch::new(0)); + config.spec.capella_fork_epoch = Some(Epoch::new(0)); + config.spec.deneb_fork_epoch = Some(Epoch::new(0)); + + ApiTester::new_from_config(config) + .await + .test_get_blob_sidecars_pruned(false) + .await + .test_get_blob_sidecars_pruned(true) + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn get_blob_sidecars_pre_deneb() { + let mut config = ApiTesterConfig::default(); + config.spec.altair_fork_epoch = Some(Epoch::new(0)); + config.spec.bellatrix_fork_epoch = Some(Epoch::new(0)); + config.spec.capella_fork_epoch = Some(Epoch::new(0)); + config.spec.deneb_fork_epoch = Some(Epoch::new(1)); + + ApiTester::new_from_config(config) + .await + .test_get_blob_sidecars_pre_deneb() + .await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn post_validator_liveness_epoch() { ApiTester::new()