Fix ssz formatting for /light_client/updates beacon API endpoint (#7806)

#7759


  We were incorrectly encoding the full response from `/light_client/updates` instead of only encoding the light client update

Author: eserilev

beacon_node/http_api/src/light_client.rs

@@ -34,13 +34,15 @@ pub fn get_light_client_updates<T: BeaconChainTypes>(
     match accept_header {
         Some(api_types::Accept::Ssz) => {
             let response_chunks = light_client_updates
-                .iter()
-                .map(|update| map_light_client_update_to_ssz_chunk::<T>(&chain, update))
-                .collect::<Vec<LightClientUpdateResponseChunk>>();
+                .into_iter()
+                .flat_map(|update| {
+                    map_light_client_update_to_response_chunk::<T>(&chain, update).as_ssz_bytes()
+                })
+                .collect();
 
             Response::builder()
                 .status(200)
-                .body(response_chunks.as_ssz_bytes())
+                .body(response_chunks)
                 .map(|res: Response<Vec<u8>>| add_ssz_content_type_header(res))
                 .map_err(|e| {
                     warp_utils::reject::custom_server_error(format!(
@@ -146,21 +148,20 @@ pub fn validate_light_client_updates_request<T: BeaconChainTypes>(
     Ok(())
 }
 
-fn map_light_client_update_to_ssz_chunk<T: BeaconChainTypes>(
+fn map_light_client_update_to_response_chunk<T: BeaconChainTypes>(
     chain: &BeaconChain<T>,
-    light_client_update: &LightClientUpdate<T::EthSpec>,
-) -> LightClientUpdateResponseChunk {
+    light_client_update: LightClientUpdate<T::EthSpec>,
+) -> LightClientUpdateResponseChunk<T::EthSpec> {
     let epoch = light_client_update
         .attested_header_slot()
         .epoch(T::EthSpec::slots_per_epoch());
     let fork_digest = chain.compute_fork_digest(epoch);
 
-    let payload = light_client_update.as_ssz_bytes();
-    let response_chunk_len = fork_digest.len() + payload.len();
+    let response_chunk_len = fork_digest.len() + light_client_update.ssz_bytes_len();
 
     let response_chunk = LightClientUpdateResponseChunkInner {
         context: fork_digest,
-        payload,
+        payload: light_client_update,
     };
 
     LightClientUpdateResponseChunk {

beacon_node/http_api/tests/tests.rs

@@ -2216,6 +2216,24 @@ impl ApiTester {
         self
     }
 
+    pub async fn test_get_beacon_light_client_updates_ssz(self) -> Self {
+        let current_epoch = self.chain.epoch().unwrap();
+        let current_sync_committee_period = current_epoch
+            .sync_committee_period(&self.chain.spec)
+            .unwrap();
+
+        match self
+            .client
+            .get_beacon_light_client_updates_ssz::<E>(current_sync_committee_period, 1)
+            .await
+        {
+            Ok(result) => result,
+            Err(e) => panic!("query failed incorrectly: {e:?}"),
+        };
+
+        self
+    }
+
     pub async fn test_get_beacon_light_client_updates(self) -> Self {
         let current_epoch = self.chain.epoch().unwrap();
         let current_sync_committee_period = current_epoch
@@ -7073,6 +7091,8 @@ async fn get_light_client_updates() {
     ApiTester::new_from_config(config)
         .await
         .test_get_beacon_light_client_updates()
+        .await
+        .test_get_beacon_light_client_updates_ssz()
         .await;
 }
 

common/eth2/src/lib.rs

@@ -981,6 +981,32 @@ impl BeaconNodeHttpClient {
         })
     }
 
+    /// `GET beacon/light_client/updates`
+    ///
+    /// Returns `Ok(None)` on a 404 error.
+    pub async fn get_beacon_light_client_updates_ssz<E: EthSpec>(
+        &self,
+        start_period: u64,
+        count: u64,
+    ) -> Result<Option<Vec<u8>>, Error> {
+        let mut path = self.eth_path(V1)?;
+
+        path.path_segments_mut()
+            .map_err(|()| Error::InvalidUrl(self.server.clone()))?
+            .push("beacon")
+            .push("light_client")
+            .push("updates");
+
+        path.query_pairs_mut()
+            .append_pair("start_period", &start_period.to_string());
+
+        path.query_pairs_mut()
+            .append_pair("count", &count.to_string());
+
+        self.get_bytes_opt_accept_header(path, Accept::Ssz, self.timeouts.default)
+            .await
+    }
+
     /// `GET beacon/light_client/bootstrap`
     ///
     /// Returns `Ok(None)` on a 404 error.

common/eth2/src/types.rs

@@ -11,6 +11,7 @@ use multiaddr::Multiaddr;
 use reqwest::header::HeaderMap;
 use serde::{Deserialize, Deserializer, Serialize};
 use serde_utils::quoted_u64::Quoted;
+use ssz::Encode;
 use ssz::{Decode, DecodeError};
 use ssz_derive::{Decode, Encode};
 use std::fmt::{self, Display};
@@ -823,16 +824,32 @@ pub struct LightClientUpdatesQuery {
     pub count: u64,
 }
 
-#[derive(Encode, Decode)]
-pub struct LightClientUpdateResponseChunk {
+pub struct LightClientUpdateResponseChunk<E: EthSpec> {
     pub response_chunk_len: u64,
-    pub response_chunk: LightClientUpdateResponseChunkInner,
+    pub response_chunk: LightClientUpdateResponseChunkInner<E>,
 }
 
-#[derive(Encode, Decode)]
-pub struct LightClientUpdateResponseChunkInner {
+impl<E: EthSpec> Encode for LightClientUpdateResponseChunk<E> {
+    fn is_ssz_fixed_len() -> bool {
+        false
+    }
+
+    fn ssz_bytes_len(&self) -> usize {
+        0_u64.ssz_bytes_len()
+            + self.response_chunk.context.len()
+            + self.response_chunk.payload.ssz_bytes_len()
+    }
+
+    fn ssz_append(&self, buf: &mut Vec<u8>) {
+        buf.extend_from_slice(&self.response_chunk_len.to_le_bytes());
+        buf.extend_from_slice(&self.response_chunk.context);
+        self.response_chunk.payload.ssz_append(buf);
+    }
+}
+
+pub struct LightClientUpdateResponseChunkInner<E: EthSpec> {
     pub context: [u8; 4],
-    pub payload: Vec<u8>,
+    pub payload: LightClientUpdate<E>,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Hash)]