Skip to content

Commit

Permalink
Return HTTP 404 for pruned blob requests (#6331)
Browse files Browse the repository at this point in the history
* Return HTTP 404 for pruned blob requests
  • Loading branch information
michaelsproul authored Aug 30, 2024
1 parent ae83901 commit e3d2d38
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 3 deletions.
23 changes: 20 additions & 3 deletions beacon_node/http_api/src/block_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
117 changes: 117 additions & 0 deletions beacon_node/http_api/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<E>(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::<E>(test_slot)
.deneb_enabled(),
"Deneb should not be enabled at {test_slot}"
);

match self
.client
.get_blobs::<E>(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
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit e3d2d38

Please sign in to comment.