From 206dabec8e8e000ca30eacff388986f1fd82dac6 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Mon, 18 Dec 2023 16:10:56 +1100 Subject: [PATCH 1/9] Improve block production v3 client --- beacon_node/http_api/tests/tests.rs | 76 ++++++++++++++------------- common/eth2/src/lib.rs | 49 +++++++++++++----- common/eth2/src/types.rs | 79 ++++++++++++++++++++++++++--- consensus/types/src/beacon_block.rs | 13 +++-- 4 files changed, 159 insertions(+), 58 deletions(-) diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index ebd681b59fd..1c3d3ed757e 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -2725,52 +2725,58 @@ impl ApiTester { sk.sign(message).into() }; - let (fork_version_response_bytes, is_blinded_payload) = self + let (response, metadata) = self .client .get_validator_blocks_v3_ssz::(slot, &randao_reveal, None) .await + .unwrap() .unwrap(); - if is_blinded_payload { - let blinded_block = >::from_ssz_bytes( - &fork_version_response_bytes.unwrap(), - &self.chain.spec, - ) - .expect("block contents bytes can be decoded"); - - let signed_blinded_block = - blinded_block.sign(&sk, &fork, genesis_validators_root, &self.chain.spec); - - self.client - .post_beacon_blinded_blocks_ssz(&signed_blinded_block) - .await - .unwrap(); + match response { + ProduceBlockV3Response::Blinded(blinded_block) => { + assert!(metadata.execution_payload_blinded); + assert_eq!( + metadata.consensus_version, + blinded_block.to_ref().fork_name(&self.chain.spec).unwrap() + ); + let signed_blinded_block = + blinded_block.sign(&sk, &fork, genesis_validators_root, &self.chain.spec); - let head_block = self.chain.head_beacon_block().clone_as_blinded(); - assert_eq!(head_block, signed_blinded_block); + self.client + .post_beacon_blinded_blocks_ssz(&signed_blinded_block) + .await + .unwrap(); - self.chain.slot_clock.set_slot(slot.as_u64() + 1); - } else { - let block_contents = >::from_ssz_bytes( - &fork_version_response_bytes.unwrap(), - &self.chain.spec, - ) - .expect("block contents bytes can be decoded"); + let head_block = self.chain.head_beacon_block().clone_as_blinded(); + assert_eq!(head_block, signed_blinded_block); - let signed_block_contents = - block_contents.sign(&sk, &fork, genesis_validators_root, &self.chain.spec); + self.chain.slot_clock.set_slot(slot.as_u64() + 1); + } + ProduceBlockV3Response::Full(block_contents) => { + assert!(!metadata.execution_payload_blinded); + assert_eq!( + metadata.consensus_version, + block_contents + .block() + .to_ref() + .fork_name(&self.chain.spec) + .unwrap() + ); + let signed_block_contents = + block_contents.sign(&sk, &fork, genesis_validators_root, &self.chain.spec); - self.client - .post_beacon_blocks_ssz(&signed_block_contents) - .await - .unwrap(); + self.client + .post_beacon_blocks_ssz(&signed_block_contents) + .await + .unwrap(); - assert_eq!( - self.chain.head_beacon_block().as_ref(), - signed_block_contents.signed_block() - ); + assert_eq!( + self.chain.head_beacon_block().as_ref(), + signed_block_contents.signed_block() + ); - self.chain.slot_clock.set_slot(slot.as_u64() + 1); + self.chain.slot_clock.set_slot(slot.as_u64() + 1); + } } } diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index e045494c9d8..c513d8531b0 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -67,6 +67,8 @@ pub enum Error { InvalidJson(serde_json::Error), /// The server returned an invalid server-sent event. InvalidServerSentEvent(String), + /// The server sent invalid response headers. + InvalidHeaders(String), /// The server returned an invalid SSZ response. InvalidSsz(ssz::DecodeError), /// An I/O error occurred while loading an API token from disk. @@ -97,6 +99,7 @@ impl Error { Error::MissingSignatureHeader => None, Error::InvalidJson(_) => None, Error::InvalidServerSentEvent(_) => None, + Error::InvalidHeaders(_) => None, Error::InvalidSsz(_) => None, Error::TokenReadError(..) => None, Error::NoServerPubkey | Error::NoToken => None, @@ -1907,7 +1910,7 @@ impl BeaconNodeHttpClient { slot: Slot, randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, - ) -> Result<(Option>, bool), Error> { + ) -> Result, ProduceBlockV3Metadata)>, Error> { self.get_validator_blocks_v3_modular_ssz::( slot, randao_reveal, @@ -1924,7 +1927,7 @@ impl BeaconNodeHttpClient { randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, skip_randao_verification: SkipRandaoVerification, - ) -> Result<(Option>, bool), Error> { + ) -> Result, ProduceBlockV3Metadata)>, Error> { let path = self .get_validator_blocks_v3_path::( slot, @@ -1934,7 +1937,7 @@ impl BeaconNodeHttpClient { ) .await?; - let (response_content, response_headers) = self + let (opt_response_bytes, opt_response_headers) = self .get_bytes_response_with_response_headers( path, Accept::Ssz, @@ -1942,15 +1945,37 @@ impl BeaconNodeHttpClient { ) .await?; - let is_blinded_payload = match response_headers { - Some(headers) => headers - .get(EXECUTION_PAYLOAD_BLINDED_HEADER) - .map(|value| value.to_str().unwrap_or_default().to_lowercase() == "true") - .unwrap_or(false), - None => false, - }; - - Ok((response_content, is_blinded_payload)) + opt_response_bytes + .map(|response_bytes| { + // Try to parse metadata only if response content exists. + let response_headers = opt_response_headers + .as_ref() + .ok_or(Error::InvalidHeaders("no headers".into()))?; + let metadata = ProduceBlockV3Metadata::try_from(response_headers) + .map_err(Error::InvalidHeaders)?; + + // Parse bytes based on metadata. + let response = if metadata.execution_payload_blinded { + ProduceBlockV3Response::Blinded( + BlindedBeaconBlock::from_ssz_bytes_for_fork( + &response_bytes, + metadata.consensus_version, + ) + .map_err(Error::InvalidSsz)?, + ) + } else { + ProduceBlockV3Response::Full( + FullBlockContents::from_ssz_bytes_for_fork( + &response_bytes, + metadata.consensus_version, + ) + .map_err(Error::InvalidSsz)?, + ) + }; + + Ok((response, metadata)) + }) + .transpose() } /// `GET v2/validator/blocks/{slot}` in ssz format diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index d8086784b1a..8afc46e9aa4 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -1,9 +1,13 @@ //! This module exposes a superset of the `types` crate. It adds additional types that are only //! required for the HTTP API. -use crate::Error as ServerError; +use crate::{ + Error as ServerError, CONSENSUS_BLOCK_VALUE_HEADER, CONSENSUS_VERSION_HEADER, + EXECUTION_PAYLOAD_BLINDED_HEADER, EXECUTION_PAYLOAD_VALUE_HEADER, +}; use lighthouse_network::{ConnectionDirection, Enr, Multiaddr, PeerConnectionStatus}; use mediatype::{names, MediaType, MediaTypeList}; +use reqwest::header::HeaderMap; use serde::{Deserialize, Deserializer, Serialize}; use serde_json::Value; use ssz::{Decode, DecodeError}; @@ -1430,6 +1434,7 @@ pub mod serde_status_code { } } +// FIXME(sproul): replace by ForkVersionedResponse? pub enum ForkVersionedBeaconBlockType { Full(ForkVersionedResponse>), Blinded(ForkVersionedResponse>), @@ -1535,6 +1540,15 @@ pub enum FullBlockContents { pub type BlockContentsTuple = (BeaconBlock, Option<(KzgProofs, BlobsList)>); +/// Metadata about a `ProduceBlockV3Response` which is returned in headers. +#[derive(Debug)] +pub struct ProduceBlockV3Metadata { + pub consensus_version: ForkName, + pub execution_payload_blinded: bool, + pub execution_payload_value: Uint256, + pub consensus_block_value: u64, +} + impl FullBlockContents { pub fn new(block: BeaconBlock, blob_data: Option<(KzgProofs, BlobsList)>) -> Self { match blob_data { @@ -1556,13 +1570,19 @@ impl FullBlockContents { len: bytes.len(), expected: slot_len, })?; - let slot = Slot::from_ssz_bytes(slot_bytes)?; let fork_at_slot = spec.fork_name_at_slot::(slot); + Self::from_ssz_bytes_for_fork(bytes, fork_at_slot) + } - match fork_at_slot { + /// SSZ decode with fork variant passed in explicitly. + pub fn from_ssz_bytes_for_fork( + bytes: &[u8], + fork_name: ForkName, + ) -> Result { + match fork_name { ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => { - BeaconBlock::from_ssz_bytes(bytes, spec) + BeaconBlock::from_ssz_bytes_for_fork(bytes, fork_name) .map(|block| FullBlockContents::Block(block)) } ForkName::Deneb => { @@ -1573,8 +1593,9 @@ impl FullBlockContents { builder.register_type::>()?; let mut decoder = builder.build()?; - let block = - decoder.decode_next_with(|bytes| BeaconBlock::from_ssz_bytes(bytes, spec))?; + let block = decoder.decode_next_with(|bytes| { + BeaconBlock::from_ssz_bytes_for_fork(bytes, fork_name) + })?; let kzg_proofs = decoder.decode_next()?; let blobs = decoder.decode_next()?; @@ -1643,6 +1664,52 @@ impl Into> for FullBlockContents { pub type SignedBlockContentsTuple = (SignedBeaconBlock, Option<(KzgProofs, BlobsList)>); +fn parse_required_header( + headers: &HeaderMap, + header_name: &str, + parse: impl FnOnce(&str) -> Result, +) -> Result { + let str_value = headers + .get(header_name) + .ok_or_else(|| format!("missing required header {header_name}"))? + .to_str() + .map_err(|e| format!("invalid value in {header_name}: {e}"))?; + parse(str_value) +} + +impl TryFrom<&HeaderMap> for ProduceBlockV3Metadata { + type Error = String; + + fn try_from(headers: &HeaderMap) -> Result { + let consensus_version = parse_required_header(headers, CONSENSUS_VERSION_HEADER, |s| { + s.parse::() + .map_err(|e| format!("invalid {CONSENSUS_VERSION_HEADER}: {e:?}")) + })?; + let execution_payload_blinded = + parse_required_header(headers, EXECUTION_PAYLOAD_BLINDED_HEADER, |s| { + s.parse::() + .map_err(|e| format!("invalid {EXECUTION_PAYLOAD_BLINDED_HEADER}: {e:?}")) + })?; + let execution_payload_value = + parse_required_header(headers, EXECUTION_PAYLOAD_VALUE_HEADER, |s| { + s.parse::() + .map_err(|e| format!("invalid {EXECUTION_PAYLOAD_VALUE_HEADER}: {e:?}")) + })?; + let consensus_block_value = + parse_required_header(headers, CONSENSUS_BLOCK_VALUE_HEADER, |s| { + s.parse::() + .map_err(|e| format!("invalid {CONSENSUS_BLOCK_VALUE_HEADER}: {e:?}")) + })?; + + Ok(ProduceBlockV3Metadata { + consensus_version, + execution_payload_blinded, + execution_payload_value, + consensus_block_value, + }) + } +} + /// A wrapper over a [`SignedBeaconBlock`] or a [`SignedBlockContents`]. #[derive(Clone, Debug, Encode, Serialize, Deserialize)] #[serde(untagged)] diff --git a/consensus/types/src/beacon_block.rs b/consensus/types/src/beacon_block.rs index 7215c779e73..90dff84b39a 100644 --- a/consensus/types/src/beacon_block.rs +++ b/consensus/types/src/beacon_block.rs @@ -112,12 +112,15 @@ impl> BeaconBlock { let slot = Slot::from_ssz_bytes(slot_bytes)?; let fork_at_slot = spec.fork_name_at_slot::(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 { + Ok(map_fork_name!(fork_name, Self, <_>::from_ssz_bytes(bytes)?)) } /// Try decoding each beacon block variant in sequence. From 408e1555aa26c5a6d1596ab10b87424b38cf67f5 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 19 Dec 2023 11:27:06 +1100 Subject: [PATCH 2/9] Delete wayward line --- common/eth2/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index c513d8531b0..b5c4c05421c 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -287,7 +287,6 @@ impl BeaconNodeHttpClient { .await .optional()?; - // let headers = opt_response.headers(); match opt_response { Some(resp) => { let response_headers = resp.headers().clone(); From 78588c6823bf756f49fcf7f39d2ddaff5901cd80 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 19 Dec 2023 12:39:40 +1100 Subject: [PATCH 3/9] Overhaul JSON endpoint as well --- common/eth2/src/lib.rs | 151 ++++++++++++++++++++------------------- common/eth2/src/types.rs | 6 -- 2 files changed, 76 insertions(+), 81 deletions(-) diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index b5c4c05421c..1d9e8ae3b03 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -31,6 +31,7 @@ use serde::{de::DeserializeOwned, Serialize}; use ssz::Encode; use std::convert::TryFrom; use std::fmt; +use std::future::Future; use std::iter::Iterator; use std::path::PathBuf; use std::time::Duration; @@ -127,6 +128,7 @@ pub struct Timeouts { pub get_beacon_blocks_ssz: Duration, pub get_debug_beacon_states: Duration, pub get_deposit_snapshot: Duration, + // FIXME(sproul): rename to get_validator_block pub get_validator_block_ssz: Duration, } @@ -276,12 +278,16 @@ impl BeaconNodeHttpClient { } /// Perform a HTTP GET request using an 'accept' header, returning `None` on a 404 error. - pub async fn get_bytes_response_with_response_headers( + pub async fn get_response_with_response_headers( &self, url: U, accept_header: Accept, timeout: Duration, - ) -> Result<(Option>, Option), Error> { + parser: impl FnOnce(Response, HeaderMap) -> F, + ) -> Result, Error> + where + F: Future>, + { let opt_response = self .get_response(url, |b| b.accept(accept_header).timeout(timeout)) .await @@ -290,12 +296,10 @@ impl BeaconNodeHttpClient { match opt_response { Some(resp) => { let response_headers = resp.headers().clone(); - Ok(( - Some(resp.bytes().await?.into_iter().collect::>()), - Some(response_headers), - )) + let parsed_response = parser(resp, response_headers).await?; + Ok(Some(parsed_response)) } - None => Ok((None, None)), + None => Ok(None), } } @@ -1818,7 +1822,7 @@ impl BeaconNodeHttpClient { } /// returns `GET v3/validator/blocks/{slot}` URL path - pub async fn get_validator_blocks_v3_path( + pub async fn get_validator_blocks_v3_path( &self, slot: Slot, randao_reveal: &SignatureBytes, @@ -1855,7 +1859,7 @@ impl BeaconNodeHttpClient { slot: Slot, randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, - ) -> Result, Error> { + ) -> Result<(ProduceBlockV3Response, ProduceBlockV3Metadata), Error> { self.get_validator_blocks_v3_modular( slot, randao_reveal, @@ -1872,35 +1876,39 @@ impl BeaconNodeHttpClient { randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, skip_randao_verification: SkipRandaoVerification, - ) -> Result, Error> { + ) -> Result<(ProduceBlockV3Response, ProduceBlockV3Metadata), Error> { let path = self - .get_validator_blocks_v3_path::( - slot, - randao_reveal, - graffiti, - skip_randao_verification, - ) + .get_validator_blocks_v3_path(slot, randao_reveal, graffiti, skip_randao_verification) .await?; - let response = self.get_response(path, |b| b).await?; - - let is_blinded_payload = response - .headers() - .get(EXECUTION_PAYLOAD_BLINDED_HEADER) - .map(|value| value.to_str().unwrap_or_default().to_lowercase() == "true") - .unwrap_or(false); + let opt_result = self + .get_response_with_response_headers( + path, + Accept::Json, + self.timeouts.get_validator_block_ssz, + |response, headers| async move { + let metadata = ProduceBlockV3Metadata::try_from(&headers) + .map_err(Error::InvalidHeaders)?; + if metadata.execution_payload_blinded { + let blinded_block = response + .json::>>() + .await? + .data; + Ok((ProduceBlockV3Response::Blinded(blinded_block), metadata)) + } else { + let full_block_contents = response + .json::>>() + .await? + .data; + Ok((ProduceBlockV3Response::Full(full_block_contents), metadata)) + } + }, + ) + .await?; - if is_blinded_payload { - let blinded_payload = response - .json::>>() - .await?; - Ok(ForkVersionedBeaconBlockType::Blinded(blinded_payload)) - } else { - let full_payload = response - .json::>>() - .await?; - Ok(ForkVersionedBeaconBlockType::Full(full_payload)) - } + // Generic handler is optional but this route should never 404 unless unimplemented, so + // treat that as an error. + opt_result.ok_or(Error::StatusCode(StatusCode::NOT_FOUND)) } /// `GET v3/validator/blocks/{slot}` in ssz format @@ -1909,7 +1917,7 @@ impl BeaconNodeHttpClient { slot: Slot, randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, - ) -> Result, ProduceBlockV3Metadata)>, Error> { + ) -> Result<(ProduceBlockV3Response, ProduceBlockV3Metadata), Error> { self.get_validator_blocks_v3_modular_ssz::( slot, randao_reveal, @@ -1926,55 +1934,48 @@ impl BeaconNodeHttpClient { randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, skip_randao_verification: SkipRandaoVerification, - ) -> Result, ProduceBlockV3Metadata)>, Error> { + ) -> Result<(ProduceBlockV3Response, ProduceBlockV3Metadata), Error> { let path = self - .get_validator_blocks_v3_path::( - slot, - randao_reveal, - graffiti, - skip_randao_verification, - ) + .get_validator_blocks_v3_path(slot, randao_reveal, graffiti, skip_randao_verification) .await?; - let (opt_response_bytes, opt_response_headers) = self - .get_bytes_response_with_response_headers( + let opt_response = self + .get_response_with_response_headers( path, Accept::Ssz, self.timeouts.get_validator_block_ssz, - ) - .await?; - - opt_response_bytes - .map(|response_bytes| { - // Try to parse metadata only if response content exists. - let response_headers = opt_response_headers - .as_ref() - .ok_or(Error::InvalidHeaders("no headers".into()))?; - let metadata = ProduceBlockV3Metadata::try_from(response_headers) - .map_err(Error::InvalidHeaders)?; - - // Parse bytes based on metadata. - let response = if metadata.execution_payload_blinded { - ProduceBlockV3Response::Blinded( - BlindedBeaconBlock::from_ssz_bytes_for_fork( - &response_bytes, - metadata.consensus_version, + |response, headers| async move { + let metadata = ProduceBlockV3Metadata::try_from(&headers) + .map_err(Error::InvalidHeaders)?; + let response_bytes = response.bytes().await?; + + // Parse bytes based on metadata. + let response = if metadata.execution_payload_blinded { + ProduceBlockV3Response::Blinded( + BlindedBeaconBlock::from_ssz_bytes_for_fork( + &response_bytes, + metadata.consensus_version, + ) + .map_err(Error::InvalidSsz)?, ) - .map_err(Error::InvalidSsz)?, - ) - } else { - ProduceBlockV3Response::Full( - FullBlockContents::from_ssz_bytes_for_fork( - &response_bytes, - metadata.consensus_version, + } else { + ProduceBlockV3Response::Full( + FullBlockContents::from_ssz_bytes_for_fork( + &response_bytes, + metadata.consensus_version, + ) + .map_err(Error::InvalidSsz)?, ) - .map_err(Error::InvalidSsz)?, - ) - }; + }; - Ok((response, metadata)) - }) - .transpose() + Ok((response, metadata)) + }, + ) + .await?; + + // Generic handler is optional but this route should never 404 unless unimplemented, so + // treat that as an error. + opt_response.ok_or(Error::StatusCode(StatusCode::NOT_FOUND)) } /// `GET v2/validator/blocks/{slot}` in ssz format diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index 8afc46e9aa4..37907c5e27b 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -1434,12 +1434,6 @@ pub mod serde_status_code { } } -// FIXME(sproul): replace by ForkVersionedResponse? -pub enum ForkVersionedBeaconBlockType { - Full(ForkVersionedResponse>), - Blinded(ForkVersionedResponse>), -} - #[cfg(test)] mod tests { use super::*; From 3122c24fb0293e846a994864739dc5c739b9864d Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 19 Dec 2023 12:42:42 +1100 Subject: [PATCH 4/9] Rename timeout param --- common/eth2/src/lib.rs | 13 ++++++------- validator_client/src/lib.rs | 5 ++--- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index 1d9e8ae3b03..19a35386048 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -128,8 +128,7 @@ pub struct Timeouts { pub get_beacon_blocks_ssz: Duration, pub get_debug_beacon_states: Duration, pub get_deposit_snapshot: Duration, - // FIXME(sproul): rename to get_validator_block - pub get_validator_block_ssz: Duration, + pub get_validator_block: Duration, } impl Timeouts { @@ -145,7 +144,7 @@ impl Timeouts { get_beacon_blocks_ssz: timeout, get_debug_beacon_states: timeout, get_deposit_snapshot: timeout, - get_validator_block_ssz: timeout, + get_validator_block: timeout, } } } @@ -1885,7 +1884,7 @@ impl BeaconNodeHttpClient { .get_response_with_response_headers( path, Accept::Json, - self.timeouts.get_validator_block_ssz, + self.timeouts.get_validator_block, |response, headers| async move { let metadata = ProduceBlockV3Metadata::try_from(&headers) .map_err(Error::InvalidHeaders)?; @@ -1943,7 +1942,7 @@ impl BeaconNodeHttpClient { .get_response_with_response_headers( path, Accept::Ssz, - self.timeouts.get_validator_block_ssz, + self.timeouts.get_validator_block, |response, headers| async move { let metadata = ProduceBlockV3Metadata::try_from(&headers) .map_err(Error::InvalidHeaders)?; @@ -2006,7 +2005,7 @@ impl BeaconNodeHttpClient { .get_validator_blocks_path::(slot, randao_reveal, graffiti, skip_randao_verification) .await?; - self.get_bytes_opt_accept_header(path, Accept::Ssz, self.timeouts.get_validator_block_ssz) + self.get_bytes_opt_accept_header(path, Accept::Ssz, self.timeouts.get_validator_block) .await } @@ -2110,7 +2109,7 @@ impl BeaconNodeHttpClient { ) .await?; - self.get_bytes_opt_accept_header(path, Accept::Ssz, self.timeouts.get_validator_block_ssz) + self.get_bytes_opt_accept_header(path, Accept::Ssz, self.timeouts.get_validator_block) .await } diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index d52247df4d2..89fc0376215 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -83,7 +83,7 @@ const HTTP_SYNC_DUTIES_TIMEOUT_QUOTIENT: u32 = 4; const HTTP_GET_BEACON_BLOCK_SSZ_TIMEOUT_QUOTIENT: u32 = 4; const HTTP_GET_DEBUG_BEACON_STATE_QUOTIENT: u32 = 4; const HTTP_GET_DEPOSIT_SNAPSHOT_QUOTIENT: u32 = 4; -const HTTP_GET_VALIDATOR_BLOCK_SSZ_TIMEOUT_QUOTIENT: u32 = 4; +const HTTP_GET_VALIDATOR_BLOCK_TIMEOUT_QUOTIENT: u32 = 4; const DOPPELGANGER_SERVICE_NAME: &str = "doppelganger"; @@ -311,8 +311,7 @@ impl ProductionValidatorClient { / HTTP_GET_BEACON_BLOCK_SSZ_TIMEOUT_QUOTIENT, get_debug_beacon_states: slot_duration / HTTP_GET_DEBUG_BEACON_STATE_QUOTIENT, get_deposit_snapshot: slot_duration / HTTP_GET_DEPOSIT_SNAPSHOT_QUOTIENT, - get_validator_block_ssz: slot_duration - / HTTP_GET_VALIDATOR_BLOCK_SSZ_TIMEOUT_QUOTIENT, + get_validator_block: slot_duration / HTTP_GET_VALIDATOR_BLOCK_TIMEOUT_QUOTIENT, } } else { Timeouts::set_all(slot_duration) From 64be25362884a86d0293d20d10b4a7a1fa21f570 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 19 Dec 2023 12:58:04 +1100 Subject: [PATCH 5/9] Update tests --- .../http_api/tests/interactive_tests.rs | 11 +- beacon_node/http_api/tests/tests.rs | 173 ++++++++---------- 2 files changed, 85 insertions(+), 99 deletions(-) diff --git a/beacon_node/http_api/tests/interactive_tests.rs b/beacon_node/http_api/tests/interactive_tests.rs index 48a2f450e21..040dc7e2eed 100644 --- a/beacon_node/http_api/tests/interactive_tests.rs +++ b/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. @@ -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::(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(_) => { + ProduceBlockV3Response::Full(unsigned_block_contents_c) => { + unsigned_block_contents_c.deconstruct() + } + ProduceBlockV3Response::Blinded(_) => { panic!("Should not be a blinded block"); } }; diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 1c3d3ed757e..97e944ecc79 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -7,7 +7,9 @@ use environment::null_logger; use eth2::{ mixin::{RequestAccept, ResponseForkName, ResponseOptional}, reqwest::RequestBuilder, - types::{BlockId as CoreBlockId, ForkChoiceNode, StateId as CoreStateId, *}, + types::{ + BlockId as CoreBlockId, ForkChoiceNode, ProduceBlockV3Response, StateId as CoreStateId, *, + }, BeaconNodeHttpClient, Error, StatusCode, Timeouts, }; use execution_layer::test_utils::{ @@ -38,8 +40,6 @@ use types::{ MainnetEthSpec, RelativeEpoch, SelectionProof, SignedRoot, Slot, }; -use eth2::types::ForkVersionedBeaconBlockType::{Blinded, Full}; - type E = MainnetEthSpec; const SECONDS_PER_SLOT: u64 = 12; @@ -2729,7 +2729,6 @@ impl ApiTester { .client .get_validator_blocks_v3_ssz::(slot, &randao_reveal, None) .await - .unwrap() .unwrap(); match response { @@ -3549,15 +3548,17 @@ impl ApiTester { let (proposer_index, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); let payload: BlindedPayload = match payload_type { - Blinded(payload) => payload.data.body().execution_payload().unwrap().into(), - Full(_) => panic!("Expecting a blinded payload"), + ProduceBlockV3Response::Blinded(payload) => { + payload.body().execution_payload().unwrap().into() + } + ProduceBlockV3Response::Full(_) => panic!("Expecting a blinded payload"), }; let expected_fee_recipient = Address::from_low_u64_be(proposer_index as u64); @@ -3651,15 +3652,17 @@ impl ApiTester { let (proposer_index, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); let payload: BlindedPayload = match payload_type { - Blinded(payload) => payload.data.body().execution_payload().unwrap().into(), - Full(_) => panic!("Expecting a blinded payload"), + ProduceBlockV3Response::Blinded(payload) => { + payload.body().execution_payload().unwrap().into() + } + ProduceBlockV3Response::Full(_) => panic!("Expecting a blinded payload"), }; let expected_fee_recipient = Address::from_low_u64_be(proposer_index as u64); @@ -3725,15 +3728,17 @@ impl ApiTester { let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); let payload: BlindedPayload = match payload_type { - Blinded(payload) => payload.data.body().execution_payload().unwrap().into(), - Full(_) => panic!("Expecting a blinded payload"), + ProduceBlockV3Response::Blinded(payload) => { + payload.body().execution_payload().unwrap().into() + } + ProduceBlockV3Response::Full(_) => panic!("Expecting a blinded payload"), }; assert_eq!(payload.fee_recipient(), test_fee_recipient); @@ -3813,21 +3818,17 @@ impl ApiTester { let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); let payload: FullPayload = match payload_type { - Full(payload) => payload - .data - .block() - .body() - .execution_payload() - .unwrap() - .into(), - Blinded(_) => panic!("Expecting a blinded payload"), + ProduceBlockV3Response::Full(payload) => { + payload.block().body().execution_payload().unwrap().into() + } + ProduceBlockV3Response::Blinded(_) => panic!("Expecting a blinded payload"), }; assert_eq!(payload.parent_hash(), expected_parent_hash); @@ -3903,21 +3904,17 @@ impl ApiTester { .unwrap(); let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); let payload: FullPayload = match payload_type { - Full(payload) => payload - .data - .block() - .body() - .execution_payload() - .unwrap() - .into(), - Blinded(_) => panic!("Expecting a full payload"), + ProduceBlockV3Response::Full(payload) => { + payload.block().body().execution_payload().unwrap().into() + } + ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; assert_eq!(payload.prev_randao(), expected_prev_randao); @@ -3993,21 +3990,17 @@ impl ApiTester { let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); let payload: FullPayload = match payload_type { - Full(payload) => payload - .data - .block() - .body() - .execution_payload() - .unwrap() - .into(), - Blinded(_) => panic!("Expecting a full payload"), + ProduceBlockV3Response::Full(payload) => { + payload.block().body().execution_payload().unwrap().into() + } + ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; assert_eq!(payload.block_number(), expected_block_number); @@ -4081,21 +4074,17 @@ impl ApiTester { let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); let payload: FullPayload = match payload_type { - Full(payload) => payload - .data - .block() - .body() - .execution_payload() - .unwrap() - .into(), - Blinded(_) => panic!("Expecting a blinded payload"), + ProduceBlockV3Response::Full(payload) => { + payload.block().body().execution_payload().unwrap().into() + } + ProduceBlockV3Response::Blinded(_) => panic!("Expecting a blinded payload"), }; assert!(payload.timestamp() > min_expected_timestamp); @@ -4141,15 +4130,15 @@ impl ApiTester { let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); match payload_type { - Full(_) => (), - Blinded(_) => panic!("Expecting a full payload"), + ProduceBlockV3Response::Full(_) => (), + ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; self @@ -4207,15 +4196,15 @@ impl ApiTester { let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); match payload_type { - Full(_) => (), - Blinded(_) => panic!("Expecting a full payload"), + ProduceBlockV3Response::Full(_) => (), + ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; self @@ -4315,15 +4304,15 @@ impl ApiTester { .get_test_randao(next_slot, next_slot.epoch(E::slots_per_epoch())) .await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(next_slot, &randao_reveal, None) .await .unwrap(); match payload_type { - Blinded(_) => (), - Full(_) => panic!("Expecting a blinded payload"), + ProduceBlockV3Response::Blinded(_) => (), + ProduceBlockV3Response::Full(_) => panic!("Expecting a blinded payload"), }; // Without proposing, advance into the next slot, this should make us cross the threshold @@ -4335,15 +4324,15 @@ impl ApiTester { .get_test_randao(next_slot, next_slot.epoch(E::slots_per_epoch())) .await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(next_slot, &randao_reveal, None) .await .unwrap(); match payload_type { - Full(_) => (), - Blinded(_) => panic!("Expecting a full payload"), + ProduceBlockV3Response::Full(_) => (), + ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; self @@ -4463,15 +4452,15 @@ impl ApiTester { .get_test_randao(next_slot, next_slot.epoch(E::slots_per_epoch())) .await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(next_slot, &randao_reveal, None) .await .unwrap(); match payload_type { - Full(_) => (), - Blinded(_) => panic!("Expecting a full payload"), + ProduceBlockV3Response::Full(_) => (), + ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; // Fill another epoch with blocks, should be enough to finalize. (Sneaky plus 1 because this @@ -4493,15 +4482,15 @@ impl ApiTester { .get_test_randao(next_slot, next_slot.epoch(E::slots_per_epoch())) .await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(next_slot, &randao_reveal, None) .await .unwrap(); match payload_type { - Blinded(_) => (), - Full(_) => panic!("Expecting a blinded payload"), + ProduceBlockV3Response::Blinded(_) => (), + ProduceBlockV3Response::Full(_) => panic!("Expecting a blinded payload"), }; self @@ -4573,21 +4562,17 @@ impl ApiTester { let (proposer_index, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); let payload: FullPayload = match payload_type { - Full(payload) => payload - .data - .block() - .body() - .execution_payload() - .unwrap() - .into(), - Blinded(_) => panic!("Expecting a full payload"), + ProduceBlockV3Response::Full(payload) => { + payload.block().body().execution_payload().unwrap().into() + } + ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; let expected_fee_recipient = Address::from_low_u64_be(proposer_index as u64); @@ -4646,15 +4631,15 @@ impl ApiTester { let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); match payload_type { - Full(_) => (), - Blinded(_) => panic!("Expecting a full payload"), + ProduceBlockV3Response::Full(_) => (), + ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; self @@ -4710,15 +4695,15 @@ impl ApiTester { let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); match payload_type { - Blinded(_) => (), - Full(_) => panic!("Expecting a blinded payload"), + ProduceBlockV3Response::Blinded(_) => (), + ProduceBlockV3Response::Full(_) => panic!("Expecting a blinded payload"), }; self @@ -4774,15 +4759,15 @@ impl ApiTester { let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); match payload_type { - Full(_) => (), - Blinded(_) => panic!("Expecting a full payload"), + ProduceBlockV3Response::Full(_) => (), + ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; self @@ -4838,15 +4823,15 @@ impl ApiTester { let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); match payload_type { - Full(_) => (), - Blinded(_) => panic!("Expecting a full payload"), + ProduceBlockV3Response::Full(_) => (), + ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; self @@ -4900,15 +4885,15 @@ impl ApiTester { let epoch = self.chain.epoch().unwrap(); let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); let _block_contents = match payload_type { - Blinded(payload) => payload.data, - Full(_) => panic!("Expecting a blinded payload"), + ProduceBlockV3Response::Blinded(payload) => payload, + ProduceBlockV3Response::Full(_) => panic!("Expecting a blinded payload"), }; self @@ -4972,15 +4957,15 @@ impl ApiTester { let epoch = self.chain.epoch().unwrap(); let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; - let payload_type = self + let (payload_type, _) = self .client .get_validator_blocks_v3::(slot, &randao_reveal, None) .await .unwrap(); match payload_type { - Full(_) => (), - Blinded(_) => panic!("Expecting a full payload"), + ProduceBlockV3Response::Full(_) => (), + ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; self From 7b6ad718e23da41caec2c18c44c7209552430f5a Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 19 Dec 2023 18:42:55 +1100 Subject: [PATCH 6/9] I broke everything --- beacon_node/beacon_chain/src/beacon_chain.rs | 1 + .../src/test_utils/mock_builder.rs | 6 +- beacon_node/http_api/src/lib.rs | 17 ++-- beacon_node/http_api/src/produce_block.rs | 57 +++++++----- beacon_node/http_api/src/version.rs | 15 +++- .../http_api/tests/interactive_tests.rs | 6 +- beacon_node/http_api/tests/tests.rs | 43 ++++----- common/eth2/src/lib.rs | 26 +++--- common/eth2/src/types.rs | 19 +++- .../types/src/fork_versioned_response.rs | 90 ++++++++++--------- 10 files changed, 163 insertions(+), 117 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 83b593f9bc9..5c1ee313da6 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -5563,6 +5563,7 @@ impl BeaconChain { 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::(prepare_slot)), })); } diff --git a/beacon_node/execution_layer/src/test_utils/mock_builder.rs b/beacon_node/execution_layer/src/test_utils/mock_builder.rs index 7da2022d588..3d4ea51f4bd 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_builder.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_builder.rs @@ -335,8 +335,9 @@ pub fn serve( .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( .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) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 0b25015e264..89dfc9c39d0 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/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( }), _ => Ok(warp::reply::json(&ForkVersionedResponse { version: Some(fork_name), + metadata: EmptyMetadata, data: bootstrap, }) .into_response()), @@ -2446,6 +2447,7 @@ pub fn serve( }), _ => Ok(warp::reply::json(&ForkVersionedResponse { version: Some(fork_name), + metadata: EmptyMetadata, data: update, }) .into_response()), @@ -2493,6 +2495,7 @@ pub fn serve( }), _ => Ok(warp::reply::json(&ForkVersionedResponse { version: Some(fork_name), + metadata: EmptyMetadata, data: update, }) .into_response()), @@ -3193,7 +3196,7 @@ pub fn serve( ); 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 } diff --git a/beacon_node/http_api/src/produce_block.rs b/beacon_node/http_api/src/produce_block.rs index 09b95136b57..5d8acd8cfd7 100644 --- a/beacon_node/http_api/src/produce_block.rs +++ b/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( - endpoint_version: EndpointVersion, accept_header: Option, chain: Arc>, slot: Slot, @@ -68,13 +67,12 @@ pub async fn produce_block_v3( 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( chain: Arc>, block_response: BeaconBlockResponseWrapper, - endpoint_version: EndpointVersion, accept_header: Option, ) -> Result, warp::Rejection> { let fork_name = block_response @@ -84,6 +82,14 @@ pub fn build_response_v3( let consensus_block_value = block_response.consensus_block_value(); let execution_payload_blinded = block_response.is_blinded(); + // FIXME(sproul): block values shouldn't be optional + let metadata = ProduceBlockV3Metadata { + consensus_version: fork_name, + execution_payload_blinded, + execution_payload_value: execution_payload_value.unwrap_or_default(), + consensus_block_value: consensus_block_value.unwrap_or_default(), + }; + let block_contents = build_block_contents::build_block_contents(fork_name, block_response)?; match accept_header { @@ -100,12 +106,17 @@ pub fn build_response_v3( .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)), } } diff --git a/beacon_node/http_api/src/version.rs b/beacon_node/http_api/src/version.rs index 7b069012430..3bbb0269f18 100644 --- a/beacon_node/http_api/src/version.rs +++ b/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( }; Ok(ForkVersionedResponse { version: fork_name, + metadata: Default::default(), data, }) } @@ -46,8 +51,10 @@ pub fn execution_optimistic_finalized_fork_versioned_response( }; Ok(ExecutionOptimisticFinalizedForkVersionedResponse { version: fork_name, - execution_optimistic: Some(execution_optimistic), - finalized: Some(finalized), + metadata: ExecutionOptimisticFinalizedMetadata { + execution_optimistic: Some(execution_optimistic), + finalized: Some(finalized), + }, data, }) } diff --git a/beacon_node/http_api/tests/interactive_tests.rs b/beacon_node/http_api/tests/interactive_tests.rs index 040dc7e2eed..945f538a3a7 100644 --- a/beacon_node/http_api/tests/interactive_tests.rs +++ b/beacon_node/http_api/tests/interactive_tests.rs @@ -112,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); @@ -624,7 +624,7 @@ pub async fn proposer_boost_re_org_test( .await .unwrap(); - let (unsigned_block_c, block_c_blobs) = match unsigned_block_type { + let (unsigned_block_c, block_c_blobs) = match unsigned_block_type.data { ProduceBlockV3Response::Full(unsigned_block_contents_c) => { unsigned_block_contents_c.deconstruct() } diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 97e944ecc79..1b694ecf9d1 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -655,6 +655,7 @@ impl ApiTester { .await .unwrap() .unwrap() + .metadata .finalized .unwrap(); @@ -691,6 +692,7 @@ impl ApiTester { .await .unwrap() .unwrap() + .metadata .finalized .unwrap(); @@ -728,6 +730,7 @@ impl ApiTester { .await .unwrap() .unwrap() + .metadata .finalized .unwrap(); @@ -3554,7 +3557,7 @@ impl ApiTester { .await .unwrap(); - let payload: BlindedPayload = match payload_type { + let payload: BlindedPayload = match payload_type.data { ProduceBlockV3Response::Blinded(payload) => { payload.body().execution_payload().unwrap().into() } @@ -3658,7 +3661,7 @@ impl ApiTester { .await .unwrap(); - let payload: BlindedPayload = match payload_type { + let payload: BlindedPayload = match payload_type.data { ProduceBlockV3Response::Blinded(payload) => { payload.body().execution_payload().unwrap().into() } @@ -3734,7 +3737,7 @@ impl ApiTester { .await .unwrap(); - let payload: BlindedPayload = match payload_type { + let payload: BlindedPayload = match payload_type.data { ProduceBlockV3Response::Blinded(payload) => { payload.body().execution_payload().unwrap().into() } @@ -3824,7 +3827,7 @@ impl ApiTester { .await .unwrap(); - let payload: FullPayload = match payload_type { + let payload: FullPayload = match payload_type.data { ProduceBlockV3Response::Full(payload) => { payload.block().body().execution_payload().unwrap().into() } @@ -3910,7 +3913,7 @@ impl ApiTester { .await .unwrap(); - let payload: FullPayload = match payload_type { + let payload: FullPayload = match payload_type.data { ProduceBlockV3Response::Full(payload) => { payload.block().body().execution_payload().unwrap().into() } @@ -3996,7 +3999,7 @@ impl ApiTester { .await .unwrap(); - let payload: FullPayload = match payload_type { + let payload: FullPayload = match payload_type.data { ProduceBlockV3Response::Full(payload) => { payload.block().body().execution_payload().unwrap().into() } @@ -4080,7 +4083,7 @@ impl ApiTester { .await .unwrap(); - let payload: FullPayload = match payload_type { + let payload: FullPayload = match payload_type.data { ProduceBlockV3Response::Full(payload) => { payload.block().body().execution_payload().unwrap().into() } @@ -4136,7 +4139,7 @@ impl ApiTester { .await .unwrap(); - match payload_type { + match payload_type.data { ProduceBlockV3Response::Full(_) => (), ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; @@ -4202,7 +4205,7 @@ impl ApiTester { .await .unwrap(); - match payload_type { + match payload_type.data { ProduceBlockV3Response::Full(_) => (), ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; @@ -4310,7 +4313,7 @@ impl ApiTester { .await .unwrap(); - match payload_type { + match payload_type.data { ProduceBlockV3Response::Blinded(_) => (), ProduceBlockV3Response::Full(_) => panic!("Expecting a blinded payload"), }; @@ -4330,7 +4333,7 @@ impl ApiTester { .await .unwrap(); - match payload_type { + match payload_type.data { ProduceBlockV3Response::Full(_) => (), ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; @@ -4458,7 +4461,7 @@ impl ApiTester { .await .unwrap(); - match payload_type { + match payload_type.data { ProduceBlockV3Response::Full(_) => (), ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; @@ -4488,7 +4491,7 @@ impl ApiTester { .await .unwrap(); - match payload_type { + match payload_type.data { ProduceBlockV3Response::Blinded(_) => (), ProduceBlockV3Response::Full(_) => panic!("Expecting a blinded payload"), }; @@ -4568,7 +4571,7 @@ impl ApiTester { .await .unwrap(); - let payload: FullPayload = match payload_type { + let payload: FullPayload = match payload_type.data { ProduceBlockV3Response::Full(payload) => { payload.block().body().execution_payload().unwrap().into() } @@ -4637,7 +4640,7 @@ impl ApiTester { .await .unwrap(); - match payload_type { + match payload_type.data { ProduceBlockV3Response::Full(_) => (), ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; @@ -4701,7 +4704,7 @@ impl ApiTester { .await .unwrap(); - match payload_type { + match payload_type.data { ProduceBlockV3Response::Blinded(_) => (), ProduceBlockV3Response::Full(_) => panic!("Expecting a blinded payload"), }; @@ -4765,7 +4768,7 @@ impl ApiTester { .await .unwrap(); - match payload_type { + match payload_type.data { ProduceBlockV3Response::Full(_) => (), ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; @@ -4829,7 +4832,7 @@ impl ApiTester { .await .unwrap(); - match payload_type { + match payload_type.data { ProduceBlockV3Response::Full(_) => (), ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; @@ -4891,7 +4894,7 @@ impl ApiTester { .await .unwrap(); - let _block_contents = match payload_type { + let _block_contents = match payload_type.data { ProduceBlockV3Response::Blinded(payload) => payload, ProduceBlockV3Response::Full(_) => panic!("Expecting a blinded payload"), }; @@ -4963,7 +4966,7 @@ impl ApiTester { .await .unwrap(); - match payload_type { + match payload_type.data { ProduceBlockV3Response::Full(_) => (), ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), }; diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index 19a35386048..5327e155cbd 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -1858,7 +1858,7 @@ impl BeaconNodeHttpClient { slot: Slot, randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, - ) -> Result<(ProduceBlockV3Response, ProduceBlockV3Metadata), Error> { + ) -> Result<(JsonProduceBlockV3Response, ProduceBlockV3Metadata), Error> { self.get_validator_blocks_v3_modular( slot, randao_reveal, @@ -1875,7 +1875,7 @@ impl BeaconNodeHttpClient { randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, skip_randao_verification: SkipRandaoVerification, - ) -> Result<(ProduceBlockV3Response, ProduceBlockV3Metadata), Error> { + ) -> Result<(JsonProduceBlockV3Response, ProduceBlockV3Metadata), Error> { let path = self .get_validator_blocks_v3_path(slot, randao_reveal, graffiti, skip_randao_verification) .await?; @@ -1886,20 +1886,22 @@ impl BeaconNodeHttpClient { Accept::Json, self.timeouts.get_validator_block, |response, headers| async move { - let metadata = ProduceBlockV3Metadata::try_from(&headers) + let header_metadata = ProduceBlockV3Metadata::try_from(&headers) .map_err(Error::InvalidHeaders)?; - if metadata.execution_payload_blinded { - let blinded_block = response - .json::>>() + if header_metadata.execution_payload_blinded { + let blinded_response = response + .json::, + ProduceBlockV3Metadata>>() .await? - .data; - Ok((ProduceBlockV3Response::Blinded(blinded_block), metadata)) + .map_data(ProduceBlockV3Response::Blinded); + Ok((blinded_response, header_metadata)) } else { - let full_block_contents = response - .json::>>() + let full_block_response= response + .json::, + ProduceBlockV3Metadata>>() .await? - .data; - Ok((ProduceBlockV3Response::Full(full_block_contents), metadata)) + .map_data(ProduceBlockV3Response::Full); + Ok((full_block_response, header_metadata)) } }, ) diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index 37907c5e27b..259aa2cfba9 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -1520,6 +1520,9 @@ pub enum ProduceBlockV3Response { Blinded(BlindedBeaconBlock), } +pub type JsonProduceBlockV3Response = + ForkVersionedResponse, ProduceBlockV3Metadata>; + /// A wrapper over a [`BeaconBlock`] or a [`BlockContents`]. #[derive(Debug, Encode, Serialize, Deserialize)] #[serde(untagged)] @@ -1534,12 +1537,24 @@ pub enum FullBlockContents { pub type BlockContentsTuple = (BeaconBlock, Option<(KzgProofs, BlobsList)>); -/// Metadata about a `ProduceBlockV3Response` which is returned in headers. -#[derive(Debug)] +// This value should never be used +fn dummy_consensus_version() -> ForkName { + ForkName::Base +} + +/// Metadata about a `ProduceBlockV3Response` which is returned in the body & headers. +#[derive(Debug, Deserialize, Serialize)] pub struct ProduceBlockV3Metadata { + // The consensus version is serialized & deserialized by `ForkVersionedResponse`. + #[serde( + skip_serializing, + skip_deserializing, + default = "dummy_consensus_version" + )] pub consensus_version: ForkName, pub execution_payload_blinded: bool, pub execution_payload_value: Uint256, + #[serde(with = "serde_utils::quoted_u64")] pub consensus_block_value: u64, } diff --git a/consensus/types/src/fork_versioned_response.rs b/consensus/types/src/fork_versioned_response.rs index 2d97dc12194..b100894f45e 100644 --- a/consensus/types/src/fork_versioned_response.rs +++ b/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 { - #[serde(skip_serializing_if = "Option::is_none")] - pub version: Option, - pub execution_optimistic: Option, - pub finalized: Option, - pub data: T, -} - -impl<'de, F> serde::Deserialize<'de> for ExecutionOptimisticFinalizedForkVersionedResponse -where - F: ForkVersionDeserialize, -{ - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - #[derive(Deserialize)] - struct Helper { - version: Option, - execution_optimistic: Option, - finalized: Option, - 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,38 @@ pub trait ForkVersionDeserialize: Sized + DeserializeOwned { ) -> Result; } -// 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 `()` 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 { +pub struct ForkVersionedResponse { #[serde(skip_serializing_if = "Option::is_none")] pub version: Option, + #[serde(flatten)] + pub metadata: M, pub data: T, } -impl<'de, F> serde::Deserialize<'de> for ForkVersionedResponse +/// Metadata type similar to unit (i.e. `()`) but deserializes from a map (`serde_json::Value`). +#[derive(Debug, PartialEq, Clone, Default, Deserialize, Serialize)] +pub struct EmptyMetadata; + +/// Fork versioned response with extra information about finalization & optimistic execution. +pub type ExecutionOptimisticFinalizedForkVersionedResponse = + ForkVersionedResponse; + +#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] +pub struct ExecutionOptimisticFinalizedMetadata { + pub execution_optimistic: Option, + pub finalized: Option, +} + +impl<'de, F, M> serde::Deserialize<'de> for ForkVersionedResponse where F: ForkVersionDeserialize, + M: DeserializeOwned, { fn deserialize(deserializer: D) -> Result where @@ -71,6 +51,8 @@ where #[derive(Deserialize)] struct Helper { version: Option, + #[serde(flatten)] + metadata: serde_json::Value, data: serde_json::Value, } @@ -79,9 +61,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 +82,22 @@ impl ForkVersionDeserialize for Arc { } } +impl ForkVersionedResponse { + /// Apply a function to the inner `data`, potentially changing its type. + pub fn map_data(self, f: impl FnOnce(T) -> U) -> ForkVersionedResponse { + let ForkVersionedResponse { + version, + metadata, + data, + } = self; + ForkVersionedResponse { + version, + metadata, + data: f(data), + } + } +} + #[cfg(test)] mod fork_version_response_tests { use crate::{ @@ -112,6 +112,7 @@ mod fork_version_response_tests { let response_json = serde_json::to_string(&json!(ForkVersionedResponse::> { version: Some(ForkName::Merge), + metadata: Default::default(), data: ExecutionPayload::Merge(ExecutionPayloadMerge::default()), })) .unwrap(); @@ -129,6 +130,7 @@ mod fork_version_response_tests { let response_json = serde_json::to_string(&json!(ForkVersionedResponse::> { version: Some(ForkName::Capella), + metadata: Default::default(), data: ExecutionPayload::Merge(ExecutionPayloadMerge::default()), })) .unwrap(); From 422d18029347b606c2c4e36132d74847d9ee7540 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Wed, 20 Dec 2023 13:27:28 +1100 Subject: [PATCH 7/9] Ah this is an insane fix --- beacon_node/http_api/src/lib.rs | 6 +++--- consensus/types/src/fork_versioned_response.rs | 5 ++++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index 89dfc9c39d0..1b4f1d35e1c 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -2399,7 +2399,7 @@ pub fn serve( }), _ => Ok(warp::reply::json(&ForkVersionedResponse { version: Some(fork_name), - metadata: EmptyMetadata, + metadata: EmptyMetadata {}, data: bootstrap, }) .into_response()), @@ -2447,7 +2447,7 @@ pub fn serve( }), _ => Ok(warp::reply::json(&ForkVersionedResponse { version: Some(fork_name), - metadata: EmptyMetadata, + metadata: EmptyMetadata {}, data: update, }) .into_response()), @@ -2495,7 +2495,7 @@ pub fn serve( }), _ => Ok(warp::reply::json(&ForkVersionedResponse { version: Some(fork_name), - metadata: EmptyMetadata, + metadata: EmptyMetadata {}, data: update, }) .into_response()), diff --git a/consensus/types/src/fork_versioned_response.rs b/consensus/types/src/fork_versioned_response.rs index b100894f45e..5bfe377a061 100644 --- a/consensus/types/src/fork_versioned_response.rs +++ b/consensus/types/src/fork_versioned_response.rs @@ -26,8 +26,11 @@ pub struct ForkVersionedResponse { } /// 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; +pub struct EmptyMetadata {} /// Fork versioned response with extra information about finalization & optimistic execution. pub type ExecutionOptimisticFinalizedForkVersionedResponse = From 6b85997e208e50bc9b8414005dd466ef54c0ad38 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Wed, 20 Dec 2023 13:44:08 +1100 Subject: [PATCH 8/9] Remove unnecessary optionals --- beacon_node/beacon_chain/src/beacon_chain.rs | 12 ++++++------ beacon_node/http_api/src/produce_block.rs | 5 ++--- beacon_node/http_api/src/version.rs | 8 ++++---- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 5c1ee313da6..b5f9f1eff3f 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -501,14 +501,14 @@ impl BeaconBlockResponseWrapper { }) } - pub fn execution_payload_value(&self) -> Option { + 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 { + 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> { /// The Blobs / Proofs associated with the new block pub blob_items: Option<(KzgProofs, BlobsList)>, /// The execution layer reward for the block - pub execution_payload_value: Option, + pub execution_payload_value: Uint256, /// The consensus layer reward to the proposer - pub consensus_block_value: Option, + pub consensus_block_value: u64, } impl FinalizationAndCanonicity { @@ -5286,8 +5286,8 @@ impl BeaconChain { block, state, blob_items, - execution_payload_value: Some(execution_payload_value), - consensus_block_value: Some(consensus_block_value), + execution_payload_value, + consensus_block_value, }) } diff --git a/beacon_node/http_api/src/produce_block.rs b/beacon_node/http_api/src/produce_block.rs index 5d8acd8cfd7..ff1c7d345c2 100644 --- a/beacon_node/http_api/src/produce_block.rs +++ b/beacon_node/http_api/src/produce_block.rs @@ -82,12 +82,11 @@ pub fn build_response_v3( let consensus_block_value = block_response.consensus_block_value(); let execution_payload_blinded = block_response.is_blinded(); - // FIXME(sproul): block values shouldn't be optional let metadata = ProduceBlockV3Metadata { consensus_version: fork_name, execution_payload_blinded, - execution_payload_value: execution_payload_value.unwrap_or_default(), - consensus_block_value: consensus_block_value.unwrap_or_default(), + execution_payload_value, + consensus_block_value, }; let block_contents = build_block_contents::build_block_contents(fork_name, block_response)?; diff --git a/beacon_node/http_api/src/version.rs b/beacon_node/http_api/src/version.rs index 3bbb0269f18..343defb809f 100644 --- a/beacon_node/http_api/src/version.rs +++ b/beacon_node/http_api/src/version.rs @@ -80,12 +80,12 @@ pub fn add_execution_payload_blinded_header( /// Add the `Eth-Execution-Payload-Value` header to a response. pub fn add_execution_payload_value_header( reply: T, - execution_payload_value: Option, + 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() } @@ -93,12 +93,12 @@ pub fn add_execution_payload_value_header( /// Add the `Eth-Consensus-Block-Value` header to a response. pub fn add_consensus_block_value_header( reply: T, - consensus_payload_value: Option, + 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() } From a3b55fe676dd4127d03803fa1683e7f30e0958d5 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Wed, 20 Dec 2023 15:47:10 +1100 Subject: [PATCH 9/9] Doc fix --- consensus/types/src/fork_versioned_response.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consensus/types/src/fork_versioned_response.rs b/consensus/types/src/fork_versioned_response.rs index 5bfe377a061..195c083e295 100644 --- a/consensus/types/src/fork_versioned_response.rs +++ b/consensus/types/src/fork_versioned_response.rs @@ -13,7 +13,7 @@ pub trait ForkVersionDeserialize: Sized + DeserializeOwned { /// Deserialize is only implemented for types that implement ForkVersionDeserialize. /// -/// The metadata of type M should be set to `()` if you don't care about adding fields other than +/// 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)]