From b40dceaae9a9ba3d7ba6504f56ad42f954868a0a Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 22 Mar 2023 01:56:32 +1100 Subject: [PATCH] Update get blobs endpoint to return a list of BlobSidecars (#4109) * Update get blobs endpoint to return BlobSidecarList * Update code comment * Update blob retrieval to return BlobSidecarList without Arc * Remove usage of BlobSidecarList type alias to avoid code conflicts * Add clippy allow exception --- beacon_node/beacon_chain/src/beacon_chain.rs | 17 +++++ beacon_node/http_api/src/block_id.rs | 21 +++--- beacon_node/http_api/src/lib.rs | 76 ++++++++++---------- common/eth2/src/lib.rs | 18 +++-- 4 files changed, 77 insertions(+), 55 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index dc7541b63..016bda13a 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1057,6 +1057,23 @@ impl BeaconChain { .map(Some) } + // FIXME(jimmy): temporary method added to unblock API work. This method will be replaced by + // the `get_blobs` method below once the new blob sidecar structure (`BlobSidecarList`) is + // implemented in that method. + #[allow(clippy::type_complexity)] // FIXME: this will be fixed by the `BlobSidecarList` alias in Sean's PR + pub fn get_blob_sidecar_list( + &self, + _block_root: &Hash256, + _data_availability_boundary: Epoch, + ) -> Result< + Option< + VariableList>, ::MaxBlobsPerBlock>, + >, + Error, + > { + unimplemented!("update to use the updated `get_blobs` method instead once this PR is merged: https://github.com/sigp/lighthouse/pull/4104") + } + /// Returns the blobs at the given root, if any. /// /// Returns `Ok(None)` if the blobs and associated block are not found. diff --git a/beacon_node/http_api/src/block_id.rs b/beacon_node/http_api/src/block_id.rs index b484f4079..9183437f9 100644 --- a/beacon_node/http_api/src/block_id.rs +++ b/beacon_node/http_api/src/block_id.rs @@ -1,10 +1,10 @@ use crate::{state_id::checkpoint_slot_and_execution_optimistic, ExecutionOptimistic}; use beacon_chain::{BeaconChain, BeaconChainError, BeaconChainTypes, WhenSlotSkipped}; -use eth2::types::BlockId as CoreBlockId; +use eth2::types::{BlockId as CoreBlockId, VariableList}; use std::fmt; use std::str::FromStr; use std::sync::Arc; -use types::{BlobsSidecar, Hash256, SignedBeaconBlock, SignedBlindedBeaconBlock, Slot}; +use types::{BlobSidecar, EthSpec, Hash256, SignedBeaconBlock, SignedBlindedBeaconBlock, Slot}; /// Wraps `eth2::types::BlockId` and provides a simple way to obtain a block or root for a given /// `BlockId`. @@ -212,19 +212,22 @@ impl BlockId { } } - /// Return the `BlobsSidecar` identified by `self`. - pub async fn blobs_sidecar( + /// Return the `BlobSidecarList` identified by `self`. + pub async fn blob_sidecar_list( &self, chain: &BeaconChain, - ) -> Result>, warp::Rejection> { + ) -> Result< + VariableList>, ::MaxBlobsPerBlock>, + warp::Rejection, + > { let root = self.root(chain)?.0; let Some(data_availability_boundary) = chain.data_availability_boundary() else { - return Err(warp_utils::reject::custom_not_found("Eip4844 fork disabled".into())); + return Err(warp_utils::reject::custom_not_found("Deneb fork disabled".into())); }; - match chain.get_blobs(&root, data_availability_boundary) { - Ok(Some(blob)) => Ok(Arc::new(blob)), + match chain.get_blob_sidecar_list(&root, data_availability_boundary) { + Ok(Some(blobs)) => Ok(blobs), Ok(None) => Err(warp_utils::reject::custom_not_found(format!( - "Blob with block root {} is not in the store", + "No blobs with block root {} found in the store", root ))), Err(e) => Err(warp_utils::reject::beacon_chain_error(e)), diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 6b0518a23..797e8f72b 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -1293,6 +1293,45 @@ pub fn serve( }, ); + /* + * beacon/blobs + */ + + // GET beacon/blobs/{block_id} + let get_blobs = eth_v1 + .and(warp::path("beacon")) + .and(warp::path("blobs")) + .and(block_id_or_err) + .and(warp::path::end()) + .and(chain_filter.clone()) + .and(warp::header::optional::("accept")) + .and_then( + |block_id: BlockId, + chain: Arc>, + accept_header: Option| { + async move { + let blob_sidecar_list = block_id.blob_sidecar_list(&chain).await?; + + match accept_header { + Some(api_types::Accept::Ssz) => Response::builder() + .status(200) + .header("Content-Type", "application/octet-stream") + .body(blob_sidecar_list.as_ssz_bytes().into()) + .map_err(|e| { + warp_utils::reject::custom_server_error(format!( + "failed to create response: {}", + e + )) + }), + _ => Ok(warp::reply::json(&api_types::GenericResponse::from( + blob_sidecar_list, + )) + .into_response()), + } + } + }, + ); + /* * beacon/pool */ @@ -3498,41 +3537,6 @@ pub fn serve( ) }); - // GET lighthouse/beacon/blobs_sidecars/{block_id} - let get_lighthouse_blobs_sidecars = warp::path("lighthouse") - .and(warp::path("beacon")) - .and(warp::path("blobs_sidecars")) - .and(block_id_or_err) - .and(warp::path::end()) - .and(chain_filter.clone()) - .and(warp::header::optional::("accept")) - .and_then( - |block_id: BlockId, - chain: Arc>, - accept_header: Option| { - async move { - let blobs_sidecar = block_id.blobs_sidecar(&chain).await?; - - match accept_header { - Some(api_types::Accept::Ssz) => Response::builder() - .status(200) - .header("Content-Type", "application/octet-stream") - .body(blobs_sidecar.as_ssz_bytes().into()) - .map_err(|e| { - warp_utils::reject::custom_server_error(format!( - "failed to create response: {}", - e - )) - }), - _ => Ok(warp::reply::json(&api_types::GenericResponse::from( - blobs_sidecar, - )) - .into_response()), - } - } - }, - ); - let get_events = eth_v1 .and(warp::path("events")) .and(warp::path::end()) @@ -3627,6 +3631,7 @@ pub fn serve( .uor(get_beacon_block_attestations) .uor(get_beacon_blinded_block) .uor(get_beacon_block_root) + .uor(get_blobs) .uor(get_beacon_pool_attestations) .uor(get_beacon_pool_attester_slashings) .uor(get_beacon_pool_proposer_slashings) @@ -3672,7 +3677,6 @@ pub fn serve( .uor(get_lighthouse_attestation_performance) .uor(get_lighthouse_block_packing_efficiency) .uor(get_lighthouse_merge_readiness) - .uor(get_lighthouse_blobs_sidecars.boxed()) .uor(get_events) .recover(warp_utils::reject::handle_rejection), ) diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index 2a27d31da..a57c2ca3d 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -658,15 +658,13 @@ impl BeaconNodeHttpClient { Ok(path) } - /// Path for `lighthouse/beacon/blobs_sidecars/{block_id}` - pub fn get_blobs_sidecar_path(&self, block_id: BlockId) -> Result { - let mut path = self.server.full.clone(); - + /// Path for `v1/beacon/blobs/{block_id}` + pub fn get_blobs_path(&self, block_id: BlockId) -> Result { + let mut path = self.eth_path(V1)?; path.path_segments_mut() .map_err(|()| Error::InvalidUrl(self.server.clone()))? - .push("lighthouse") .push("beacon") - .push("blobs_sidecars") + .push("blobs") .push(&block_id.to_string()); Ok(path) } @@ -698,14 +696,14 @@ impl BeaconNodeHttpClient { Ok(Some(response.json().await?)) } - /// `GET lighthouse/beacon/blobs_sidecars/{block_id}` + /// `GET v1/beacon/blobs/{block_id}` /// /// Returns `Ok(None)` on a 404 error. - pub async fn get_blobs_sidecar( + pub async fn get_blobs( &self, block_id: BlockId, - ) -> Result>>, Error> { - let path = self.get_blobs_sidecar_path(block_id)?; + ) -> Result>>, Error> { + let path = self.get_blobs_path(block_id)?; let response = match self.get_response(path, |b| b).await.optional()? { Some(res) => res, None => return Ok(None),