add missing fields to get blob sidecars request (#5987)

* add missing fields to get blob sidecars request

* add fork versioned  response impl

* only compute the block root once

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into add-missing-fields-get-blob-sidecars

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into add-missing-fields-get-blob-sidecars

* fetch root first

fetch from cache if its a head block

* fmt

* always load from db

beacon_node/http_api/src/block_id.rs

@@ -123,6 +123,15 @@ impl BlockId {
         }
     }
 
+    pub fn blinded_block_by_root<T: BeaconChainTypes>(
+        root: &Hash256,
+        chain: &BeaconChain<T>,
+    ) -> Result<Option<SignedBlindedBeaconBlock<T::EthSpec>>, warp::Rejection> {
+        chain
+            .get_blinded_block(root)
+            .map_err(warp_utils::reject::beacon_chain_error)
+    }
+
     /// Return the `SignedBeaconBlock` identified by `self`.
     pub fn blinded_block<T: BeaconChainTypes>(
         &self,
@@ -149,38 +158,32 @@ impl BlockId {
             }
             CoreBlockId::Slot(slot) => {
                 let (root, execution_optimistic, finalized) = self.root(chain)?;
-                chain
+                BlockId::blinded_block_by_root(&root, chain).and_then(|block_opt| match block_opt {
-                    .get_blinded_block(&root)
+                    Some(block) => {
-                    .map_err(warp_utils::reject::beacon_chain_error)
+                        if block.slot() != *slot {
-                    .and_then(|block_opt| match block_opt {
+                            return Err(warp_utils::reject::custom_not_found(format!(
-                        Some(block) => {
+                                "slot {} was skipped",
-                            if block.slot() != *slot {
+                                slot
-                                return Err(warp_utils::reject::custom_not_found(format!(
+                            )));
-                                    "slot {} was skipped",
-                                    slot
-                                )));
-                            }
-                            Ok((block, execution_optimistic, finalized))
                         }
-                        None => Err(warp_utils::reject::custom_not_found(format!(
+                        Ok((block, execution_optimistic, finalized))
-                            "beacon block with root {}",
+                    }
-                            root
+                    None => Err(warp_utils::reject::custom_not_found(format!(
-                        ))),
+                        "beacon block with root {}",
-                    })
+                        root
+                    ))),
+                })
             }
             _ => {
                 let (root, execution_optimistic, finalized) = self.root(chain)?;
-                let block = chain
+                let block = BlockId::blinded_block_by_root(&root, chain).and_then(|root_opt| {
-                    .get_blinded_block(&root)
+                    root_opt.ok_or_else(|| {
-                    .map_err(warp_utils::reject::beacon_chain_error)
+                        warp_utils::reject::custom_not_found(format!(
-                    .and_then(|root_opt| {
+                            "beacon block with root {}",
-                        root_opt.ok_or_else(|| {
+                            root
-                            warp_utils::reject::custom_not_found(format!(
+                        ))
-                                "beacon block with root {}",
+                    })
-                                root
+                })?;
-                            ))
-                        })
-                    })?;
                 Ok((block, execution_optimistic, finalized))
             }
         }
@@ -252,23 +255,30 @@ impl BlockId {
         }
     }
 
-    /// Return the `BlobSidecarList` identified by `self`.
+    #[allow(clippy::type_complexity)]
-    pub fn blob_sidecar_list<T: BeaconChainTypes>(
+    pub fn get_blinded_block_and_blob_list_filtered<T: BeaconChainTypes>(
-        &self,
-        chain: &BeaconChain<T>,
-    ) -> Result<BlobSidecarList<T::EthSpec>, warp::Rejection> {
-        let root = self.root(chain)?.0;
-        chain
-            .get_blobs(&root)
-            .map_err(warp_utils::reject::beacon_chain_error)
-    }
-
-    pub fn blob_sidecar_list_filtered<T: BeaconChainTypes>(
         &self,
         indices: BlobIndicesQuery,
         chain: &BeaconChain<T>,
-    ) -> Result<BlobSidecarList<T::EthSpec>, warp::Rejection> {
+    ) -> Result<
-        let blob_sidecar_list = self.blob_sidecar_list(chain)?;
+        (
+            SignedBlindedBeaconBlock<T::EthSpec>,
+            BlobSidecarList<T::EthSpec>,
+            ExecutionOptimistic,
+            Finalized,
+        ),
+        warp::Rejection,
+    > {
+        let (root, execution_optimistic, finalized) = self.root(chain)?;
+        let block = BlockId::blinded_block_by_root(&root, chain)?.ok_or_else(|| {
+            warp_utils::reject::custom_not_found(format!("beacon block with root {}", root))
+        })?;
+
+        // 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_filtered = match indices.indices {
             Some(vec) => {
                 let list = blob_sidecar_list
@@ -280,7 +290,12 @@ impl BlockId {
             }
             None => blob_sidecar_list,
         };
-        Ok(blob_sidecar_list_filtered)
+        Ok((
+            block,
+            blob_sidecar_list_filtered,
+            execution_optimistic,
+            finalized,
+        ))
     }
 }
 

beacon_node/http_api/src/lib.rs

@@ -1736,8 +1736,12 @@ pub fn serve<T: BeaconChainTypes>(
              accept_header: Option<api_types::Accept>| {
                 task_spawner.blocking_response_task(Priority::P1, move || {
                     let indices = indices_res?;
-                    let blob_sidecar_list_filtered =
+                    let (block, blob_sidecar_list_filtered, execution_optimistic, finalized) =
-                        block_id.blob_sidecar_list_filtered(indices, &chain)?;
+                        block_id.get_blinded_block_and_blob_list_filtered(indices, &chain)?;
+                    let fork_name = block
+                        .fork_name(&chain.spec)
+                        .map_err(inconsistent_fork_rejection)?;
+
                     match accept_header {
                         Some(api_types::Accept::Ssz) => Response::builder()
                             .status(200)
@@ -1749,11 +1753,19 @@ pub fn serve<T: BeaconChainTypes>(
                                     e
                                 ))
                             }),
-                        _ => Ok(warp::reply::json(&api_types::GenericResponse::from(
+                        _ => {
-                            blob_sidecar_list_filtered,
+                            // Post as a V2 endpoint so we return the fork version.
-                        ))
+                            let res = execution_optimistic_finalized_fork_versioned_response(
-                        .into_response()),
+                                V2,
+                                fork_name,
+                                execution_optimistic,
+                                finalized,
+                                &blob_sidecar_list_filtered,
+                            )?;
+                            Ok(warp::reply::json(&res).into_response())
+                        }
                     }
+                    .map(|resp| add_consensus_version_header(resp, fork_name))
                 })
             },
         );

common/eth2/src/lib.rs

@@ -1141,7 +1141,8 @@ impl BeaconNodeHttpClient {
         &self,
         block_id: BlockId,
         indices: Option<&[u64]>,
-    ) -> Result<Option<GenericResponse<BlobSidecarList<E>>>, Error> {
+    ) -> Result<Option<ExecutionOptimisticFinalizedForkVersionedResponse<BlobSidecarList<E>>>, Error>
+    {
         let mut path = self.get_blobs_path(block_id)?;
         if let Some(indices) = indices {
             let indices_string = indices

consensus/types/src/blob_sidecar.rs

@@ -1,9 +1,10 @@
 use crate::test_utils::TestRandom;
+use crate::ForkName;
 use crate::{
     beacon_block_body::BLOB_KZG_COMMITMENTS_INDEX, BeaconBlockHeader, BeaconStateError, Blob,
     Epoch, EthSpec, FixedVector, Hash256, SignedBeaconBlockHeader, Slot, VariableList,
 };
-use crate::{KzgProofs, SignedBeaconBlock};
+use crate::{ForkVersionDeserialize, KzgProofs, SignedBeaconBlock};
 use bls::Signature;
 use derivative::Derivative;
 use kzg::{Blob as KzgBlob, Kzg, KzgCommitment, KzgProof, BYTES_PER_BLOB, BYTES_PER_FIELD_ELEMENT};
@@ -273,3 +274,12 @@ pub type BlobSidecarList<E> = VariableList<Arc<BlobSidecar<E>>, <E as EthSpec>::
 pub type FixedBlobSidecarList<E> =
     FixedVector<Option<Arc<BlobSidecar<E>>>, <E as EthSpec>::MaxBlobsPerBlock>;
 pub type BlobsList<E> = VariableList<Blob<E>, <E as EthSpec>::MaxBlobCommitmentsPerBlock>;
+
+impl<E: EthSpec> ForkVersionDeserialize for BlobSidecarList<E> {
+    fn deserialize_by_fork<'de, D: serde::Deserializer<'de>>(
+        value: serde_json::value::Value,
+        _: ForkName,
+    ) -> Result<Self, D::Error> {
+        serde_json::from_value::<BlobSidecarList<E>>(value).map_err(serde::de::Error::custom)
+    }
+}