Deneb review common/eth2 (#4698)

* Update comments and small cleanup.

* Deserialize into `SsePayloadAttributesV3` for Deneb fork. Update `SignedBlockContents::blobs_cloned` to return blobs for `BlindedBlockAndBlobSidecars`.

* Improve code readability and error handling when converting blinded block into full block.

beacon_node/http_api/src/lib.rs

@@ -1729,7 +1729,7 @@ pub fn serve<T: BeaconChainTypes>(
         );
 
     /*
-     * beacon/blobs
+     * beacon/blob_sidecars
      */
 
     // GET beacon/blob_sidecars/{block_id}

beacon_node/http_api/src/publish_blocks.rs

@@ -373,8 +373,8 @@ pub async fn reconstruct_block<T: BeaconChainTypes>(
             .try_into_full_block_and_blobs(Some(full_payload_contents))
             .map(ProvenancedBlock::builder),
     }
-    .ok_or_else(|| {
-        warp_utils::reject::custom_server_error("Unable to add payload to block".to_string())
+    .map_err(|e| {
+        warp_utils::reject::custom_server_error(format!("Unable to add payload to block: {e:?}"))
     })
 }
 

common/eth2/src/lib.rs

@@ -918,7 +918,7 @@ impl BeaconNodeHttpClient {
         Ok(Some(response.json().await?))
     }
 
-    /// `GET v1/beacon/blobs/{block_id}`
+    /// `GET v1/beacon/blob_sidecars/{block_id}`
     ///
     /// Returns `Ok(None)` on a 404 error.
     pub async fn get_blobs<T: EthSpec>(
@@ -931,8 +931,7 @@ impl BeaconNodeHttpClient {
             None => return Ok(None),
         };
 
-        let GenericResponse { data } = response.json().await?;
-        Ok(Some(GenericResponse { data }))
+        Ok(Some(response.json().await?))
     }
 
     /// `GET v1/beacon/blinded_blocks/{block_id}`

common/eth2/src/types.rs

@@ -978,9 +978,12 @@ impl ForkVersionDeserialize for SsePayloadAttributes {
             ForkName::Merge => serde_json::from_value(value)
                 .map(Self::V1)
                 .map_err(serde::de::Error::custom),
-            ForkName::Capella | ForkName::Deneb => serde_json::from_value(value)
+            ForkName::Capella => serde_json::from_value(value)
                 .map(Self::V2)
                 .map_err(serde::de::Error::custom),
+            ForkName::Deneb => serde_json::from_value(value)
+                .map(Self::V3)
+                .map_err(serde::de::Error::custom),
             ForkName::Base | ForkName::Altair => Err(serde::de::Error::custom(format!(
                 "SsePayloadAttributes deserialization for {fork_name} not implemented"
             ))),
@@ -1519,8 +1522,10 @@ impl<T: EthSpec, Payload: AbstractExecPayload<T>> SignedBlockContents<T, Payload
             SignedBlockContents::BlockAndBlobSidecars(block_and_sidecars) => {
                 Some(block_and_sidecars.signed_blob_sidecars.clone())
             }
+            SignedBlockContents::BlindedBlockAndBlobSidecars(block_and_sidecars) => {
+                Some(block_and_sidecars.signed_blinded_blob_sidecars.clone())
+            }
             SignedBlockContents::Block(_block) => None,
-            SignedBlockContents::BlindedBlockAndBlobSidecars(_) => None,
         }
     }
 
@@ -1543,48 +1548,47 @@ impl<T: EthSpec> SignedBlockContents<T, BlindedPayload<T>> {
     pub fn try_into_full_block_and_blobs(
         self,
         maybe_full_payload_contents: Option<FullPayloadContents<T>>,
-    ) -> Option<SignedBlockContents<T, FullPayload<T>>> {
+    ) -> Result<SignedBlockContents<T, FullPayload<T>>, String> {
         match self {
             SignedBlockContents::BlindedBlockAndBlobSidecars(blinded_block_and_blob_sidecars) => {
-                maybe_full_payload_contents.and_then(|full_payload_contents| {
-                    match full_payload_contents.deconstruct() {
-                        (full_payload, Some(blobs_bundle)) => {
-                            let maybe_full_block = blinded_block_and_blob_sidecars
-                                .signed_blinded_block
-                                .try_into_full_block(Some(full_payload));
-                            let full_blob_sidecars: Vec<_> = blinded_block_and_blob_sidecars
+                match maybe_full_payload_contents {
+                    None | Some(FullPayloadContents::Payload(_)) => {
+                        Err("Can't build full block contents without payload and blobs".to_string())
+                    }
+                    Some(FullPayloadContents::PayloadAndBlobs(payload_and_blobs)) => {
+                        let signed_block = blinded_block_and_blob_sidecars
+                            .signed_blinded_block
+                            .try_into_full_block(Some(payload_and_blobs.execution_payload))
+                            .ok_or("Failed to build full block with payload".to_string())?;
+                        let signed_blob_sidecars: SignedBlobSidecarList<T> =
+                            blinded_block_and_blob_sidecars
                                 .signed_blinded_blob_sidecars
                                 .into_iter()
-                                .zip(blobs_bundle.blobs)
+                                .zip(payload_and_blobs.blobs_bundle.blobs)
                                 .map(|(blinded_blob_sidecar, blob)| {
                                     blinded_blob_sidecar.into_full_blob_sidecars(blob)
                                 })
-                                .collect();
-
-                            maybe_full_block.map(|signed_block| {
-                                SignedBlockContents::BlockAndBlobSidecars(
-                                    SignedBeaconBlockAndBlobSidecars {
-                                        signed_block,
-                                        signed_blob_sidecars: VariableList::from(
-                                            full_blob_sidecars,
-                                        ),
-                                    },
-                                )
-                            })
-                        }
-                        // Can't build full block contents without full blobs
-                        _ => None,
+                                .collect::<Vec<_>>()
+                                .into();
+
+                        Ok(SignedBlockContents::new(
+                            signed_block,
+                            Some(signed_blob_sidecars),
+                        ))
                     }
-                })
+                }
             }
             SignedBlockContents::Block(blinded_block) => {
                 let full_payload_opt = maybe_full_payload_contents.map(|o| o.deconstruct().0);
                 blinded_block
                     .try_into_full_block(full_payload_opt)
                     .map(SignedBlockContents::Block)
+                    .ok_or("Can't build full block without payload".to_string())
             }
-            // Unexpected scenario for blinded block proposal
-            SignedBlockContents::BlockAndBlobSidecars(_) => None,
+            SignedBlockContents::BlockAndBlobSidecars(_) => Err(
+                "BlockAndBlobSidecars variant not expected when constructing full block"
+                    .to_string(),
+            ),
         }
     }
 }