From 041e08e69b592516195a24484641150b3cbb241a Mon Sep 17 00:00:00 2001 From: realbigsean Date: Tue, 23 Jan 2024 08:49:20 -0500 Subject: [PATCH 1/4] add get blobs unit test --- Cargo.lock | 1 + beacon_node/http_api/Cargo.toml | 1 + beacon_node/http_api/tests/tests.rs | 44 +++++++++++++++++++++++++++++ common/eth2/src/lib.rs | 12 +++++++- 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 4c830105ded..eb870eb63e6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3412,6 +3412,7 @@ dependencies = [ "bs58 0.4.0", "bytes", "directory", + "env_logger 0.9.3", "environment", "eth1", "eth2", diff --git a/beacon_node/http_api/Cargo.toml b/beacon_node/http_api/Cargo.toml index b58e0442f7c..f35431e1145 100644 --- a/beacon_node/http_api/Cargo.toml +++ b/beacon_node/http_api/Cargo.toml @@ -49,6 +49,7 @@ environment = { workspace = true } serde_json = { workspace = true } proto_array = { workspace = true } genesis = { workspace = true } +env_logger = { workspace = true } [[test]] name = "bn_http_api_tests" diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 4e0b4d761e6..415395c9ab8 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -1587,6 +1587,31 @@ impl ApiTester { self } + pub async fn test_get_blob_sidecars(self) -> 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() as u64; + let mut blob_indices = vec![]; + for i in 0..num_blobs { + blob_indices.push(i); + } + let _ = env_logger::builder().is_test(true).try_init(); + let result = match self + .client + .get_blobs::(CoreBlockId::Root(block_root), Some(&blob_indices)) + .await + { + Ok(result) => result.unwrap().data, + Err(e) => panic!("query failed incorrectly: {e:?}"), + }; + + 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 @@ -6291,6 +6316,25 @@ 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() + .await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn post_validator_liveness_epoch() { ApiTester::new() diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index bed5044b4be..16801be8e23 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -1064,8 +1064,18 @@ impl BeaconNodeHttpClient { pub async fn get_blobs( &self, block_id: BlockId, + indices: Option<&[u64]>, ) -> Result>>, 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::>() + .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); }; From 2bd903fb0287cfd78d1323cb16ffa8ad15fb812e Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 23 Jan 2024 20:37:08 +0530 Subject: [PATCH 2/4] Use a multi_key_query for blob_sidecar indices --- beacon_node/http_api/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 3777e61420b..1594668e5c8 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -1704,18 +1704,19 @@ pub fn serve( .and(warp::path("beacon")) .and(warp::path("blob_sidecars")) .and(block_id_or_err) - .and(warp::query::()) .and(warp::path::end()) + .and(multi_key_query::()) .and(task_spawner_filter.clone()) .and(chain_filter.clone()) .and(warp::header::optional::("accept")) .then( |block_id: BlockId, - indices: api_types::BlobIndicesQuery, + indices_res: Result, task_spawner: TaskSpawner, chain: Arc>, accept_header: Option| { 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 { From d66ccc1387e974b272e3ce33ac590fb81b52f2b2 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 23 Jan 2024 22:43:16 +0530 Subject: [PATCH 3/4] Fix test --- beacon_node/http_api/tests/tests.rs | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 415395c9ab8..933f98661cd 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -1587,25 +1587,33 @@ impl ApiTester { self } - pub async fn test_get_blob_sidecars(self) -> 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() as u64; - let mut blob_indices = vec![]; - for i in 0..num_blobs { - blob_indices.push(i); - } - let _ = env_logger::builder().is_test(true).try_init(); + 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::>(), + ) + } else { + None + }; let result = match self .client - .get_blobs::(CoreBlockId::Root(block_root), Some(&blob_indices)) + .get_blobs::(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); @@ -6331,7 +6339,9 @@ async fn get_blob_sidecars() { .await .test_post_beacon_blocks_valid() .await - .test_get_blob_sidecars() + .test_get_blob_sidecars(false) + .await + .test_get_blob_sidecars(true) .await; } From d00c37798b27ed07bdb77141634ac33e7cda21ea Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 23 Jan 2024 22:47:46 +0530 Subject: [PATCH 4/4] Remove env_logger --- Cargo.lock | 1 - beacon_node/http_api/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eb870eb63e6..4c830105ded 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3412,7 +3412,6 @@ dependencies = [ "bs58 0.4.0", "bytes", "directory", - "env_logger 0.9.3", "environment", "eth1", "eth2", diff --git a/beacon_node/http_api/Cargo.toml b/beacon_node/http_api/Cargo.toml index f35431e1145..b58e0442f7c 100644 --- a/beacon_node/http_api/Cargo.toml +++ b/beacon_node/http_api/Cargo.toml @@ -49,7 +49,6 @@ environment = { workspace = true } serde_json = { workspace = true } proto_array = { workspace = true } genesis = { workspace = true } -env_logger = { workspace = true } [[test]] name = "bn_http_api_tests"