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

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -501,14 +501,14 @@ impl<E: EthSpec> BeaconBlockResponseWrapper<E> {
         })
     }
 
-    pub fn execution_payload_value(&self) -> Option<Uint256> {
+    pub fn execution_payload_value(&self) -> Uint256 {
         match self {
             BeaconBlockResponseWrapper::Full(resp) => resp.execution_payload_value,
             BeaconBlockResponseWrapper::Blinded(resp) => resp.execution_payload_value,
         }
     }
 
-    pub fn consensus_block_value(&self) -> Option<u64> {
+    pub fn consensus_block_value(&self) -> u64 {
         match self {
             BeaconBlockResponseWrapper::Full(resp) => resp.consensus_block_value,
             BeaconBlockResponseWrapper::Blinded(resp) => resp.consensus_block_value,
@@ -529,9 +529,9 @@ pub struct BeaconBlockResponse<T: EthSpec, Payload: AbstractExecPayload<T>> {
     /// The Blobs / Proofs associated with the new block
     pub blob_items: Option<(KzgProofs<T>, BlobsList<T>)>,
     /// The execution layer reward for the block
-    pub execution_payload_value: Option<Uint256>,
+    pub execution_payload_value: Uint256,
     /// The consensus layer reward to the proposer
-    pub consensus_block_value: Option<u64>,
+    pub consensus_block_value: u64,
 }
 
 impl FinalizationAndCanonicity {
@@ -5286,8 +5286,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             block,
             state,
             blob_items,
-            execution_payload_value: Some(execution_payload_value),
-            consensus_block_value: Some(consensus_block_value),
+            execution_payload_value,
+            consensus_block_value,
         })
     }
 
@@ -5563,6 +5563,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                         parent_block_hash: forkchoice_update_params.head_hash.unwrap_or_default(),
                         payload_attributes: payload_attributes.into(),
                     },
+                    metadata: Default::default(),
                     version: Some(self.spec.fork_name_at_slot::<T::EthSpec>(prepare_slot)),
                 }));
             }

beacon_node/execution_layer/src/test_utils/mock_builder.rs

@@ -335,8 +335,9 @@ pub fn serve<E: EthSpec>(
                     .el
                     .get_payload_by_root(&root)
                     .ok_or_else(|| reject("missing payload for tx root"))?;
-                let resp = ForkVersionedResponse {
+                let resp: ForkVersionedResponse<_> = ForkVersionedResponse {
                     version: Some(fork_name),
+                    metadata: Default::default(),
                     data: payload,
                 };
 
@@ -616,8 +617,9 @@ pub fn serve<E: EthSpec>(
                     .spec
                     .fork_name_at_epoch(slot.epoch(E::slots_per_epoch()));
                 let signed_bid = SignedBuilderBid { message, signature };
-                let resp = ForkVersionedResponse {
+                let resp: ForkVersionedResponse<_> = ForkVersionedResponse {
                     version: Some(fork_name),
+                    metadata: Default::default(),
                     data: signed_bid,
                 };
                 let json_bid = serde_json::to_string(&resp)

beacon_node/http_api/src/lib.rs

@@ -77,12 +77,12 @@ use tokio_stream::{
     StreamExt,
 };
 use types::{
-    Attestation, AttestationData, AttestationShufflingId, AttesterSlashing, BeaconStateError,
-    CommitteeCache, ConfigAndPreset, Epoch, EthSpec, ForkName, ForkVersionedResponse, Hash256,
-    ProposerPreparationData, ProposerSlashing, RelativeEpoch, SignedAggregateAndProof,
-    SignedBlindedBeaconBlock, SignedBlsToExecutionChange, SignedContributionAndProof,
-    SignedValidatorRegistrationData, SignedVoluntaryExit, Slot, SyncCommitteeMessage,
-    SyncContributionData,
+    fork_versioned_response::EmptyMetadata, Attestation, AttestationData, AttestationShufflingId,
+    AttesterSlashing, BeaconStateError, CommitteeCache, ConfigAndPreset, Epoch, EthSpec, ForkName,
+    ForkVersionedResponse, Hash256, ProposerPreparationData, ProposerSlashing, RelativeEpoch,
+    SignedAggregateAndProof, SignedBlindedBeaconBlock, SignedBlsToExecutionChange,
+    SignedContributionAndProof, SignedValidatorRegistrationData, SignedVoluntaryExit, Slot,
+    SyncCommitteeMessage, SyncContributionData,
 };
 use validator::pubkey_to_validator_index;
 use version::{
@@ -2399,6 +2399,7 @@ pub fn serve<T: BeaconChainTypes>(
                             }),
                         _ => Ok(warp::reply::json(&ForkVersionedResponse {
                             version: Some(fork_name),
+                            metadata: EmptyMetadata {},
                             data: bootstrap,
                         })
                         .into_response()),
@@ -2446,6 +2447,7 @@ pub fn serve<T: BeaconChainTypes>(
                             }),
                         _ => Ok(warp::reply::json(&ForkVersionedResponse {
                             version: Some(fork_name),
+                            metadata: EmptyMetadata {},
                             data: update,
                         })
                         .into_response()),
@@ -2493,6 +2495,7 @@ pub fn serve<T: BeaconChainTypes>(
                             }),
                         _ => Ok(warp::reply::json(&ForkVersionedResponse {
                             version: Some(fork_name),
+                            metadata: EmptyMetadata {},
                             data: update,
                         })
                         .into_response()),
@@ -3193,7 +3196,7 @@ pub fn serve<T: BeaconChainTypes>(
                     );
 
                     if endpoint_version == V3 {
-                        produce_block_v3(endpoint_version, accept_header, chain, slot, query).await
+                        produce_block_v3(accept_header, chain, slot, query).await
                     } else {
                         produce_block_v2(endpoint_version, accept_header, chain, slot, query).await
                     }

beacon_node/http_api/src/produce_block.rs

@@ -1,17 +1,3 @@
-use bytes::Bytes;
-use std::sync::Arc;
-use types::{payload::BlockProductionVersion, *};
-
-use beacon_chain::{
-    BeaconBlockResponseWrapper, BeaconChain, BeaconChainTypes, ProduceBlockVerification,
-};
-use eth2::types::{self as api_types, EndpointVersion, SkipRandaoVerification};
-use ssz::Encode;
-use warp::{
-    hyper::{Body, Response},
-    Reply,
-};
-
 use crate::{
     build_block_contents,
     version::{
@@ -20,6 +6,20 @@ use crate::{
         fork_versioned_response, inconsistent_fork_rejection,
     },
 };
+use beacon_chain::{
+    BeaconBlockResponseWrapper, BeaconChain, BeaconChainTypes, ProduceBlockVerification,
+};
+use bytes::Bytes;
+use eth2::types::{
+    self as api_types, EndpointVersion, ProduceBlockV3Metadata, SkipRandaoVerification,
+};
+use ssz::Encode;
+use std::sync::Arc;
+use types::{payload::BlockProductionVersion, *};
+use warp::{
+    hyper::{Body, Response},
+    Reply,
+};
 
 pub fn get_randao_verification(
     query: &api_types::ValidatorBlocksQuery,
@@ -40,7 +40,6 @@ pub fn get_randao_verification(
 }
 
 pub async fn produce_block_v3<T: BeaconChainTypes>(
-    endpoint_version: EndpointVersion,
     accept_header: Option<api_types::Accept>,
     chain: Arc<BeaconChain<T>>,
     slot: Slot,
@@ -68,13 +67,12 @@ pub async fn produce_block_v3<T: BeaconChainTypes>(
             warp_utils::reject::custom_bad_request(format!("failed to fetch a block: {:?}", e))
         })?;
 
-    build_response_v3(chain, block_response_type, endpoint_version, accept_header)
+    build_response_v3(chain, block_response_type, accept_header)
 }
 
 pub fn build_response_v3<T: BeaconChainTypes>(
     chain: Arc<BeaconChain<T>>,
     block_response: BeaconBlockResponseWrapper<T::EthSpec>,
-    endpoint_version: EndpointVersion,
     accept_header: Option<api_types::Accept>,
 ) -> Result<Response<Body>, warp::Rejection> {
     let fork_name = block_response
@@ -84,6 +82,13 @@ pub fn build_response_v3<T: BeaconChainTypes>(
     let consensus_block_value = block_response.consensus_block_value();
     let execution_payload_blinded = block_response.is_blinded();
 
+    let metadata = ProduceBlockV3Metadata {
+        consensus_version: fork_name,
+        execution_payload_blinded,
+        execution_payload_value,
+        consensus_block_value,
+    };
+
     let block_contents = build_block_contents::build_block_contents(fork_name, block_response)?;
 
     match accept_header {
@@ -100,12 +105,17 @@ pub fn build_response_v3<T: BeaconChainTypes>(
             .map_err(|e| -> warp::Rejection {
                 warp_utils::reject::custom_server_error(format!("failed to create response: {}", e))
             }),
-        _ => fork_versioned_response(endpoint_version, fork_name, block_contents)
-            .map(|response| warp::reply::json(&response).into_response())
-            .map(|res| add_consensus_version_header(res, fork_name))
-            .map(|res| add_execution_payload_blinded_header(res, execution_payload_blinded))
-            .map(|res| add_execution_payload_value_header(res, execution_payload_value))
-            .map(|res| add_consensus_block_value_header(res, consensus_block_value)),
+        _ => Ok(warp::reply::json(&ForkVersionedResponse {
+            version: Some(fork_name),
+            metadata,
+            data: block_contents,
+        })
+        .into_response())
+        .map(|res| res.into_response())
+        .map(|res| add_consensus_version_header(res, fork_name))
+        .map(|res| add_execution_payload_blinded_header(res, execution_payload_blinded))
+        .map(|res| add_execution_payload_value_header(res, execution_payload_value))
+        .map(|res| add_consensus_block_value_header(res, consensus_block_value)),
     }
 }
 

beacon_node/http_api/src/version.rs

@@ -1,11 +1,15 @@
-use crate::api_types::fork_versioned_response::ExecutionOptimisticFinalizedForkVersionedResponse;
 use crate::api_types::EndpointVersion;
 use eth2::{
     CONSENSUS_BLOCK_VALUE_HEADER, CONSENSUS_VERSION_HEADER, EXECUTION_PAYLOAD_BLINDED_HEADER,
     EXECUTION_PAYLOAD_VALUE_HEADER,
 };
 use serde::Serialize;
-use types::{ForkName, ForkVersionedResponse, InconsistentFork, Uint256};
+use types::{
+    fork_versioned_response::{
+        ExecutionOptimisticFinalizedForkVersionedResponse, ExecutionOptimisticFinalizedMetadata,
+    },
+    ForkName, ForkVersionedResponse, InconsistentFork, Uint256,
+};
 use warp::reply::{self, Reply, Response};
 
 pub const V1: EndpointVersion = EndpointVersion(1);
@@ -26,6 +30,7 @@ pub fn fork_versioned_response<T: Serialize>(
     };
     Ok(ForkVersionedResponse {
         version: fork_name,
+        metadata: Default::default(),
         data,
     })
 }
@@ -46,8 +51,10 @@ pub fn execution_optimistic_finalized_fork_versioned_response<T: Serialize>(
     };
     Ok(ExecutionOptimisticFinalizedForkVersionedResponse {
         version: fork_name,
-        execution_optimistic: Some(execution_optimistic),
-        finalized: Some(finalized),
+        metadata: ExecutionOptimisticFinalizedMetadata {
+            execution_optimistic: Some(execution_optimistic),
+            finalized: Some(finalized),
+        },
         data,
     })
 }
@@ -73,25 +80,25 @@ pub fn add_execution_payload_blinded_header<T: Reply>(
 /// Add the `Eth-Execution-Payload-Value` header to a response.
 pub fn add_execution_payload_value_header<T: Reply>(
     reply: T,
-    execution_payload_value: Option<Uint256>,
+    execution_payload_value: Uint256,
 ) -> Response {
     reply::with_header(
         reply,
         EXECUTION_PAYLOAD_VALUE_HEADER,
-        execution_payload_value.unwrap_or_default().to_string(),
+        execution_payload_value.to_string(),
     )
     .into_response()
 }
 
 /// Add the `Eth-Consensus-Block-Value` header to a response.
 pub fn add_consensus_block_value_header<T: Reply>(
     reply: T,
-    consensus_payload_value: Option<u64>,
+    consensus_payload_value: u64,
 ) -> Response {
     reply::with_header(
         reply,
         CONSENSUS_BLOCK_VALUE_HEADER,
-        consensus_payload_value.unwrap_or_default().to_string(),
+        consensus_payload_value.to_string(),
     )
     .into_response()
 }

beacon_node/http_api/tests/interactive_tests.rs

@@ -4,6 +4,7 @@ use beacon_chain::{
     test_utils::{AttestationStrategy, BlockStrategy, SyncCommitteeStrategy},
     ChainConfig,
 };
+use eth2::types::ProduceBlockV3Response;
 use eth2::types::{DepositContractData, StateId};
 use execution_layer::{ForkchoiceState, PayloadAttributes};
 use http_api::test_utils::InteractiveTester;
@@ -21,8 +22,6 @@ use types::{
     MinimalEthSpec, ProposerPreparationData, Slot,
 };
 
-use eth2::types::ForkVersionedBeaconBlockType::{Blinded, Full};
-
 type E = MainnetEthSpec;
 
 // Test that the deposit_contract endpoint returns the correct chain_id and address.
@@ -113,8 +112,8 @@ async fn state_by_root_pruned_from_fork_choice() {
             .unwrap()
             .unwrap();
 
-        assert!(response.finalized.unwrap());
-        assert!(!response.execution_optimistic.unwrap());
+        assert!(response.metadata.finalized.unwrap());
+        assert!(!response.metadata.execution_optimistic.unwrap());
 
         let mut state = response.data;
         assert_eq!(state.update_tree_hash_cache().unwrap(), state_root);
@@ -619,15 +618,17 @@ pub async fn proposer_boost_re_org_test(
     let randao_reveal = harness
         .sign_randao_reveal(&state_b, proposer_index, slot_c)
         .into();
-    let unsigned_block_type = tester
+    let (unsigned_block_type, _) = tester
         .client
         .get_validator_blocks_v3::<E>(slot_c, &randao_reveal, None)
         .await
         .unwrap();
 
-    let (unsigned_block_c, block_c_blobs) = match unsigned_block_type {
-        Full(unsigned_block_contents_c) => unsigned_block_contents_c.data.deconstruct(),
-        Blinded(_) => {
+    let (unsigned_block_c, block_c_blobs) = match unsigned_block_type.data {
+        ProduceBlockV3Response::Full(unsigned_block_contents_c) => {
+            unsigned_block_contents_c.deconstruct()
+        }
+        ProduceBlockV3Response::Blinded(_) => {
             panic!("Should not be a blinded block");
         }
     };