Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix indices filter in blobs_sidecar http endpoint #5118

Merged
merged 4 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions beacon_node/http_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1704,18 +1704,19 @@ pub fn serve<T: BeaconChainTypes>(
.and(warp::path("beacon"))
.and(warp::path("blob_sidecars"))
.and(block_id_or_err)
.and(warp::query::<api_types::BlobIndicesQuery>())
.and(warp::path::end())
.and(multi_key_query::<api_types::BlobIndicesQuery>())
.and(task_spawner_filter.clone())
.and(chain_filter.clone())
.and(warp::header::optional::<api_types::Accept>("accept"))
.then(
|block_id: BlockId,
indices: api_types::BlobIndicesQuery,
indices_res: Result<api_types::BlobIndicesQuery, warp::Rejection>,
task_spawner: TaskSpawner<T::EthSpec>,
chain: Arc<BeaconChain<T>>,
accept_header: Option<api_types::Accept>| {
task_spawner.blocking_response_task(Priority::P1, move || {
let indices = indices_res?;
let blob_sidecar_list_filtered =
block_id.blob_sidecar_list_filtered(indices, &chain)?;
match accept_header {
Expand Down
54 changes: 54 additions & 0 deletions beacon_node/http_api/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1587,6 +1587,39 @@ impl ApiTester {
self
}

pub async fn test_get_blob_sidecars(self, use_indices: bool) -> Self {
let block_id = BlockId(CoreBlockId::Finalized);
let (block_root, _, _) = block_id.root(&self.chain).unwrap();
let (block, _, _) = block_id.full_block(&self.chain).await.unwrap();
let num_blobs = block.num_expected_blobs();
let blob_indices = if use_indices {
Some(
(0..num_blobs.saturating_sub(1) as u64)
.into_iter()
.collect::<Vec<_>>(),
)
} else {
None
};
let result = match self
.client
.get_blobs::<E>(CoreBlockId::Root(block_root), blob_indices.as_deref())
.await
{
Ok(result) => result.unwrap().data,
Err(e) => panic!("query failed incorrectly: {e:?}"),
};

assert_eq!(
result.len(),
blob_indices.map_or(num_blobs, |indices| indices.len())
);
let expected = block.slot();
assert_eq!(result.get(0).unwrap().slot(), expected);

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 @@ -6291,6 +6324,27 @@ async fn builder_works_post_deneb() {
.await;
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn get_blob_sidecars() {
let mut config = ApiTesterConfig {
retain_historic_states: false,
spec: E::default_spec(),
};
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_post_beacon_blocks_valid()
.await
.test_get_blob_sidecars(false)
.await
.test_get_blob_sidecars(true)
.await;
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn post_validator_liveness_epoch() {
ApiTester::new()
Expand Down
12 changes: 11 additions & 1 deletion common/eth2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1064,8 +1064,18 @@ impl BeaconNodeHttpClient {
pub async fn get_blobs<T: EthSpec>(
&self,
block_id: BlockId,
indices: Option<&[u64]>,
) -> Result<Option<GenericResponse<BlobSidecarList<T>>>, Error> {
let path = self.get_blobs_path(block_id)?;
let mut path = self.get_blobs_path(block_id)?;
if let Some(indices) = indices {
let indices_string = indices
.iter()
.map(|i| i.to_string())
.collect::<Vec<_>>()
.join(",");
path.query_pairs_mut()
.append_pair("indices", &indices_string);
}
let Some(response) = self.get_response(path, |b| b).await.optional()? else {
return Ok(None);
};
Expand Down
Loading