Clean up blockv3 metadata and client (#5015)

* Improve block production v3 client

* Delete wayward line

* Overhaul JSON endpoint as well

* Rename timeout param

* Update tests

* I broke everything

* Ah this is an insane fix

* Remove unnecessary optionals

* Doc fix

consensus/types/src/beacon_block.rs

@@ -112,12 +112,15 @@ impl<T: EthSpec, Payload: AbstractExecPayload<T>> BeaconBlock<T, Payload> {
 
         let slot = Slot::from_ssz_bytes(slot_bytes)?;
         let fork_at_slot = spec.fork_name_at_slot::<T>(slot);
+        Self::from_ssz_bytes_for_fork(bytes, fork_at_slot)
+    }
 
-        Ok(map_fork_name!(
-            fork_at_slot,
-            Self,
-            <_>::from_ssz_bytes(bytes)?
-        ))
+    /// Custom SSZ decoder that takes a `ForkName` as context.
+    pub fn from_ssz_bytes_for_fork(
+        bytes: &[u8],
+        fork_name: ForkName,
+    ) -> Result<Self, ssz::DecodeError> {
+        Ok(map_fork_name!(fork_name, Self, <_>::from_ssz_bytes(bytes)?))
     }
 
     /// Try decoding each beacon block variant in sequence.

consensus/types/src/fork_versioned_response.rs

@@ -4,47 +4,6 @@ use serde::{Deserialize, Deserializer, Serialize};
 use serde_json::value::Value;
 use std::sync::Arc;
 
-// Deserialize is only implemented for types that implement ForkVersionDeserialize
-#[derive(Debug, PartialEq, Clone, Serialize)]
-pub struct ExecutionOptimisticFinalizedForkVersionedResponse<T> {
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub version: Option<ForkName>,
-    pub execution_optimistic: Option<bool>,
-    pub finalized: Option<bool>,
-    pub data: T,
-}
-
-impl<'de, F> serde::Deserialize<'de> for ExecutionOptimisticFinalizedForkVersionedResponse<F>
-where
-    F: ForkVersionDeserialize,
-{
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        #[derive(Deserialize)]
-        struct Helper {
-            version: Option<ForkName>,
-            execution_optimistic: Option<bool>,
-            finalized: Option<bool>,
-            data: serde_json::Value,
-        }
-
-        let helper = Helper::deserialize(deserializer)?;
-        let data = match helper.version {
-            Some(fork_name) => F::deserialize_by_fork::<'de, D>(helper.data, fork_name)?,
-            None => serde_json::from_value(helper.data).map_err(serde::de::Error::custom)?,
-        };
-
-        Ok(ExecutionOptimisticFinalizedForkVersionedResponse {
-            version: helper.version,
-            execution_optimistic: helper.execution_optimistic,
-            finalized: helper.finalized,
-            data,
-        })
-    }
-}
-
 pub trait ForkVersionDeserialize: Sized + DeserializeOwned {
     fn deserialize_by_fork<'de, D: Deserializer<'de>>(
         value: Value,
@@ -52,17 +11,41 @@ pub trait ForkVersionDeserialize: Sized + DeserializeOwned {
     ) -> Result<Self, D::Error>;
 }
 
-// Deserialize is only implemented for types that implement ForkVersionDeserialize
+/// Deserialize is only implemented for types that implement ForkVersionDeserialize.
+///
+/// The metadata of type M should be set to `EmptyMetadata` if you don't care about adding fields other than
+/// version. If you *do* care about adding other fields you can mix in any type that implements
+/// `Deserialize`.
 #[derive(Debug, PartialEq, Clone, Serialize)]
-pub struct ForkVersionedResponse<T> {
+pub struct ForkVersionedResponse<T, M = EmptyMetadata> {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub version: Option<ForkName>,
+    #[serde(flatten)]
+    pub metadata: M,
     pub data: T,
 }
 
-impl<'de, F> serde::Deserialize<'de> for ForkVersionedResponse<F>
+/// Metadata type similar to unit (i.e. `()`) but deserializes from a map (`serde_json::Value`).
+///
+/// Unfortunately the braces are semantically significant, i.e. `struct EmptyMetadata;` does not
+/// work.
+#[derive(Debug, PartialEq, Clone, Default, Deserialize, Serialize)]
+pub struct EmptyMetadata {}
+
+/// Fork versioned response with extra information about finalization & optimistic execution.
+pub type ExecutionOptimisticFinalizedForkVersionedResponse<T> =
+    ForkVersionedResponse<T, ExecutionOptimisticFinalizedMetadata>;
+
+#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
+pub struct ExecutionOptimisticFinalizedMetadata {
+    pub execution_optimistic: Option<bool>,
+    pub finalized: Option<bool>,
+}
+
+impl<'de, F, M> serde::Deserialize<'de> for ForkVersionedResponse<F, M>
 where
     F: ForkVersionDeserialize,
+    M: DeserializeOwned,
 {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
@@ -71,6 +54,8 @@ where
         #[derive(Deserialize)]
         struct Helper {
             version: Option<ForkName>,
+            #[serde(flatten)]
+            metadata: serde_json::Value,
             data: serde_json::Value,
         }
 
@@ -79,9 +64,11 @@ where
             Some(fork_name) => F::deserialize_by_fork::<'de, D>(helper.data, fork_name)?,
             None => serde_json::from_value(helper.data).map_err(serde::de::Error::custom)?,
         };
+        let metadata = serde_json::from_value(helper.metadata).map_err(serde::de::Error::custom)?;
 
         Ok(ForkVersionedResponse {
             version: helper.version,
+            metadata,
             data,
         })
     }
@@ -98,6 +85,22 @@ impl<F: ForkVersionDeserialize> ForkVersionDeserialize for Arc<F> {
     }
 }
 
+impl<T, M> ForkVersionedResponse<T, M> {
+    /// Apply a function to the inner `data`, potentially changing its type.
+    pub fn map_data<U>(self, f: impl FnOnce(T) -> U) -> ForkVersionedResponse<U, M> {
+        let ForkVersionedResponse {
+            version,
+            metadata,
+            data,
+        } = self;
+        ForkVersionedResponse {
+            version,
+            metadata,
+            data: f(data),
+        }
+    }
+}
+
 #[cfg(test)]
 mod fork_version_response_tests {
     use crate::{
@@ -112,6 +115,7 @@ mod fork_version_response_tests {
         let response_json =
             serde_json::to_string(&json!(ForkVersionedResponse::<ExecutionPayload<E>> {
                 version: Some(ForkName::Merge),
+                metadata: Default::default(),
                 data: ExecutionPayload::Merge(ExecutionPayloadMerge::default()),
             }))
             .unwrap();
@@ -129,6 +133,7 @@ mod fork_version_response_tests {
         let response_json =
             serde_json::to_string(&json!(ForkVersionedResponse::<ExecutionPayload<E>> {
                 version: Some(ForkName::Capella),
+                metadata: Default::default(),
                 data: ExecutionPayload::Merge(ExecutionPayloadMerge::default()),
             }))
             .unwrap();