From d7a9a35993fd78d4c166275ed76ed5ea2fde00e1 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 14 Dec 2022 12:42:47 +1100 Subject: [PATCH 01/14] add new blob_kzg_commitments field to builderBid --- consensus/types/src/builder_bid.rs | 52 ++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/consensus/types/src/builder_bid.rs b/consensus/types/src/builder_bid.rs index 818ec52b813..cafbb30a40b 100644 --- a/consensus/types/src/builder_bid.rs +++ b/consensus/types/src/builder_bid.rs @@ -1,29 +1,77 @@ +use super::KzgCommitment; use crate::{ AbstractExecPayload, ChainSpec, EthSpec, ExecPayload, ExecutionPayloadHeader, SignedRoot, - Uint256, + Uint256, MainnetEthSpec, }; use bls::PublicKeyBytes; use bls::Signature; use serde::{Deserialize as De, Deserializer, Serialize as Ser, Serializer}; use serde_derive::{Deserialize, Serialize}; +use serde_json::Error; use serde_with::{serde_as, DeserializeAs, SerializeAs}; +use ssz_types::VariableList; use std::marker::PhantomData; +// use superstruct::superstruct; use tree_hash_derive::TreeHash; #[serde_as] #[derive(PartialEq, Debug, Serialize, Deserialize, TreeHash, Clone)] -#[serde(bound = "E: EthSpec, Payload: ExecPayload")] +#[serde(bound = "E: EthSpec, Payload: ExecPayload", deny_unknown_fields)] pub struct BuilderBid> { #[serde_as(as = "BlindedPayloadAsHeader")] pub header: Payload, #[serde(with = "eth2_serde_utils::quoted_u256")] pub value: Uint256, pub pubkey: PublicKeyBytes, + pub blob_kzg_commitments: VariableList, #[serde(skip)] #[tree_hash(skip_hashing)] _phantom_data: PhantomData, } +pub fn deserialize_bid>(str: &str) -> Result, Error> { + dbg!(str); + let bid = serde_json::from_str(str)?; + Ok(bid) +} + +#[cfg(test)] +mod tests { + use log::debug; + use crate::{ExecutionPayload, BlindedPayload, ExecutionPayloadHeaderEip4844}; + use super::*; + + #[test] + fn test_deserialize_bid() { + let str = r#"{ + "header": { + "parent_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "fee_recipient": "0xabcf8e0d4e9587369b2301d0790347320302cc09", + "state_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "receipts_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "logs_bloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", + "prev_randao": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "block_number": "1", + "gas_limit": "1", + "gas_used": "1", + "timestamp": "1", + "extra_data": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "base_fee_per_gas": "1", + "excess_data_gas": "1", + "block_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "transactions_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2" + }, + "value": "1", + "pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a", + "blob_kzg_commitments": [ + "0xa94170080872584e54a1cf092d845703b13907f2e6b3b1c0ad573b910530499e3bcd48c6378846b80d2bfa58c81cf3d5" + ] + }"#; + let result = deserialize_bid::>(str); + dbg!(result); + } +} + impl> SignedRoot for BuilderBid {} /// Validator registration, for use in interacting with servers implementing the builder API. From 80bd9ff32450acce64ae539ee83ae39239b31881 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 15 Dec 2022 10:23:56 +1100 Subject: [PATCH 02/14] add superstruct to BuilderBid --- consensus/types/src/builder_bid.rs | 73 +++++++++++++++++++++++++++--- 1 file changed, 67 insertions(+), 6 deletions(-) diff --git a/consensus/types/src/builder_bid.rs b/consensus/types/src/builder_bid.rs index cafbb30a40b..a9c967505f1 100644 --- a/consensus/types/src/builder_bid.rs +++ b/consensus/types/src/builder_bid.rs @@ -1,4 +1,5 @@ use super::KzgCommitment; +use crate::{BlindedPayloadMerge, BlindedPayload}; use crate::{ AbstractExecPayload, ChainSpec, EthSpec, ExecPayload, ExecutionPayloadHeader, SignedRoot, Uint256, MainnetEthSpec, @@ -7,28 +8,88 @@ use bls::PublicKeyBytes; use bls::Signature; use serde::{Deserialize as De, Deserializer, Serialize as Ser, Serializer}; use serde_derive::{Deserialize, Serialize}; +use serde_with::As; use serde_json::Error; use serde_with::{serde_as, DeserializeAs, SerializeAs}; use ssz_types::VariableList; use std::marker::PhantomData; +use superstruct::superstruct; // use superstruct::superstruct; use tree_hash_derive::TreeHash; -#[serde_as] +#[superstruct( + variants(Merge, Capella, Eip4844), + variant_attributes( + derive( + PartialEq, Debug, Serialize, Deserialize, TreeHash, Clone + ), + serde(bound = "E: EthSpec, Payload: ExecPayload", deny_unknown_fields) + ) +)] #[derive(PartialEq, Debug, Serialize, Deserialize, TreeHash, Clone)] -#[serde(bound = "E: EthSpec, Payload: ExecPayload", deny_unknown_fields)] +#[serde(bound = "E: EthSpec, Payload: ExecPayload", deny_unknown_fields, untagged)] +#[tree_hash(enum_behaviour = "transparent")] pub struct BuilderBid> { - #[serde_as(as = "BlindedPayloadAsHeader")] - pub header: Payload, + // #[serde(with = "As::>")] + #[superstruct(only(Merge), partial_getter(rename = "payload_merge"))] + pub header: Payload::Merge, + + // #[serde(with = "As::>")] + #[superstruct(only(Capella), partial_getter(rename = "payload_capella"))] + pub header: Payload::Capella, + + // #[serde(with = "As::>")] + #[superstruct(only(Eip4844), partial_getter(rename = "payload_eip4844"))] + pub header: Payload::Eip4844, + #[serde(with = "eth2_serde_utils::quoted_u256")] pub value: Uint256, pub pubkey: PublicKeyBytes, - pub blob_kzg_commitments: VariableList, + // pub blob_kzg_commitments: VariableList, #[serde(skip)] #[tree_hash(skip_hashing)] _phantom_data: PhantomData, } +// impl> BuilderBid { +// fn header(&self) -> Payload { +// match self { +// Self::Merge(body) => Ok(Payload::Ref::from(&self.payload_merge())), +// Self::Capella(body) => Ok(Payload::Ref::from(&self.payload_capella())), +// Self::Eip4844(body) => Ok(Payload::Ref::from(&self.payload_eip4844())), +// } +// } +// } + +impl> BuilderBid { + pub fn execution_payload(&self) -> Result, Error> { + self.to_ref().execution_payload() + } +} + +impl<'a, T: EthSpec, Payload: AbstractExecPayload> BuilderBidRef<'a, T, Payload> { + pub fn execution_payload(&self) -> Result, Error> { + match self { + Self::Merge(body) => Ok(Payload::Ref::from(&self.payload_merge())), + Self::Capella(body) => Ok(Payload::Ref::from(&self.payload_capella())), + Self::Eip4844(body) => Ok(Payload::Ref::from(&self.payload_eip4844())), + } + } +} + + +/* + +impl> SerializeAs for BlindedPayloadAsHeader { + fn serialize_as(source: &Payload, serializer: S) -> Result + where + S: Serializer, + { + source.to_execution_payload_header().serialize(serializer) + } +} + */ + pub fn deserialize_bid>(str: &str) -> Result, Error> { dbg!(str); let bid = serde_json::from_str(str)?; @@ -109,7 +170,7 @@ impl<'de, E: EthSpec, Payload: AbstractExecPayload> DeserializeAs<'de, Payloa impl> SignedBuilderBid { pub fn verify_signature(&self, spec: &ChainSpec) -> bool { self.message - .pubkey + .pubkey() .decompress() .map(|pubkey| { let domain = spec.get_builder_domain(); From 2df4c34e6930ba90dae3408d321d29da82833819 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 15 Dec 2022 16:58:41 +1100 Subject: [PATCH 03/14] builder bid compiling with variants \o/ --- beacon_node/execution_layer/src/lib.rs | 24 +-- consensus/types/src/builder_bid.rs | 193 ++++++++++++++----------- 2 files changed, 122 insertions(+), 95 deletions(-) diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index cf717055923..d95959593ac 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -739,7 +739,8 @@ impl ExecutionLayer { self.log(), "Requested blinded execution payload"; "relay_fee_recipient" => match &relay_result { - Ok(Some(r)) => format!("{:?}", r.data.message.header.fee_recipient()), + // FIXME remove clone? + Ok(Some(r)) => format!("{:?}", r.data.message.clone().header().unwrap().fee_recipient()), Ok(None) => "empty response".to_string(), Err(_) => "request failed".to_string(), }, @@ -775,7 +776,8 @@ impl ExecutionLayer { Ok(ProvenancedPayload::Local(local)) } (Ok(Some(relay)), Ok(local)) => { - let header = &relay.data.message.header; + // FIXME remove clone? + let header = relay.data.message.clone().header().unwrap(); info!( self.log(), @@ -799,7 +801,7 @@ impl ExecutionLayer { // NOTE the comment above was removed in the // rebase with unstable.. I think it goes // here now? - BlockProposalContents::Payload(relay.data.message.header), + BlockProposalContents::Payload(header), )), Err(reason) if !reason.payload_invalid() => { info!( @@ -830,7 +832,8 @@ impl ExecutionLayer { } } (Ok(Some(relay)), Err(local_error)) => { - let header = &relay.data.message.header; + // FIXME remove clone? + let header = relay.data.message.clone().header().unwrap(); info!( self.log(), @@ -854,7 +857,7 @@ impl ExecutionLayer { // NOTE the comment above was removed in the // rebase with unstable.. I think it goes // here now? - BlockProposalContents::Payload(relay.data.message.header), + BlockProposalContents::Payload(header), )), // If the payload is valid then use it. The local EE failed // to produce a payload so we have no alternative. @@ -863,7 +866,7 @@ impl ExecutionLayer { // NOTE the comment above was removed in the // rebase with unstable.. I think it goes // here now? - BlockProposalContents::Payload(relay.data.message.header), + BlockProposalContents::Payload(header), )), Err(reason) => { metrics::inc_counter_vec( @@ -1832,11 +1835,12 @@ fn verify_builder_bid>( spec: &ChainSpec, ) -> Result<(), Box> { let is_signature_valid = bid.data.verify_signature(spec); - let header = &bid.data.message.header; - let payload_value = bid.data.message.value; + // FIXME remove clone? + let header = &bid.data.message.clone().header().unwrap(); + let payload_value = bid.data.message.value().clone(); // Avoid logging values that we can't represent with our Prometheus library. - let payload_value_gwei = bid.data.message.value / 1_000_000_000; + let payload_value_gwei = bid.data.message.value() / 1_000_000_000; if payload_value_gwei <= Uint256::from(i64::max_value()) { metrics::set_gauge_vec( &metrics::EXECUTION_LAYER_PAYLOAD_BIDS, @@ -1882,7 +1886,7 @@ fn verify_builder_bid>( } else if !is_signature_valid { Err(Box::new(InvalidBuilderPayload::Signature { signature: bid.data.signature.clone(), - pubkey: bid.data.message.pubkey, + pubkey: *bid.data.message.pubkey(), })) } else { Ok(()) diff --git a/consensus/types/src/builder_bid.rs b/consensus/types/src/builder_bid.rs index a9c967505f1..e32ce49ef68 100644 --- a/consensus/types/src/builder_bid.rs +++ b/consensus/types/src/builder_bid.rs @@ -1,20 +1,17 @@ use super::KzgCommitment; -use crate::{BlindedPayloadMerge, BlindedPayload}; use crate::{ AbstractExecPayload, ChainSpec, EthSpec, ExecPayload, ExecutionPayloadHeader, SignedRoot, - Uint256, MainnetEthSpec, + Uint256 }; use bls::PublicKeyBytes; use bls::Signature; use serde::{Deserialize as De, Deserializer, Serialize as Ser, Serializer}; use serde_derive::{Deserialize, Serialize}; -use serde_with::As; use serde_json::Error; -use serde_with::{serde_as, DeserializeAs, SerializeAs}; +use serde_with::{DeserializeAs, SerializeAs}; use ssz_types::VariableList; use std::marker::PhantomData; use superstruct::superstruct; -// use superstruct::superstruct; use tree_hash_derive::TreeHash; #[superstruct( @@ -30,109 +27,37 @@ use tree_hash_derive::TreeHash; #[serde(bound = "E: EthSpec, Payload: ExecPayload", deny_unknown_fields, untagged)] #[tree_hash(enum_behaviour = "transparent")] pub struct BuilderBid> { - // #[serde(with = "As::>")] #[superstruct(only(Merge), partial_getter(rename = "payload_merge"))] pub header: Payload::Merge, - // #[serde(with = "As::>")] #[superstruct(only(Capella), partial_getter(rename = "payload_capella"))] pub header: Payload::Capella, - // #[serde(with = "As::>")] #[superstruct(only(Eip4844), partial_getter(rename = "payload_eip4844"))] pub header: Payload::Eip4844, #[serde(with = "eth2_serde_utils::quoted_u256")] pub value: Uint256, pub pubkey: PublicKeyBytes, - // pub blob_kzg_commitments: VariableList, + + #[superstruct(only(Eip4844))] + pub blob_kzg_commitments: VariableList, + #[serde(skip)] #[tree_hash(skip_hashing)] _phantom_data: PhantomData, } -// impl> BuilderBid { -// fn header(&self) -> Payload { -// match self { -// Self::Merge(body) => Ok(Payload::Ref::from(&self.payload_merge())), -// Self::Capella(body) => Ok(Payload::Ref::from(&self.payload_capella())), -// Self::Eip4844(body) => Ok(Payload::Ref::from(&self.payload_eip4844())), -// } -// } -// } - impl> BuilderBid { - pub fn execution_payload(&self) -> Result, Error> { - self.to_ref().execution_payload() - } -} - -impl<'a, T: EthSpec, Payload: AbstractExecPayload> BuilderBidRef<'a, T, Payload> { - pub fn execution_payload(&self) -> Result, Error> { + pub fn header(self) -> Result { match self { - Self::Merge(body) => Ok(Payload::Ref::from(&self.payload_merge())), - Self::Capella(body) => Ok(Payload::Ref::from(&self.payload_capella())), - Self::Eip4844(body) => Ok(Payload::Ref::from(&self.payload_eip4844())), + Self::Merge(bid) => Ok(bid.header.into()), + Self::Capella(bid) => Ok(bid.header.into()), + Self::Eip4844(bid) => Ok(bid.header.into()), } } } - -/* - -impl> SerializeAs for BlindedPayloadAsHeader { - fn serialize_as(source: &Payload, serializer: S) -> Result - where - S: Serializer, - { - source.to_execution_payload_header().serialize(serializer) - } -} - */ - -pub fn deserialize_bid>(str: &str) -> Result, Error> { - dbg!(str); - let bid = serde_json::from_str(str)?; - Ok(bid) -} - -#[cfg(test)] -mod tests { - use log::debug; - use crate::{ExecutionPayload, BlindedPayload, ExecutionPayloadHeaderEip4844}; - use super::*; - - #[test] - fn test_deserialize_bid() { - let str = r#"{ - "header": { - "parent_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "fee_recipient": "0xabcf8e0d4e9587369b2301d0790347320302cc09", - "state_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "receipts_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "logs_bloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", - "prev_randao": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "block_number": "1", - "gas_limit": "1", - "gas_used": "1", - "timestamp": "1", - "extra_data": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "base_fee_per_gas": "1", - "excess_data_gas": "1", - "block_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "transactions_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2" - }, - "value": "1", - "pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a", - "blob_kzg_commitments": [ - "0xa94170080872584e54a1cf092d845703b13907f2e6b3b1c0ad573b910530499e3bcd48c6378846b80d2bfa58c81cf3d5" - ] - }"#; - let result = deserialize_bid::>(str); - dbg!(result); - } -} - impl> SignedRoot for BuilderBid {} /// Validator registration, for use in interacting with servers implementing the builder API. @@ -180,3 +105,101 @@ impl> SignedBuilderBid { .unwrap_or(false) } } + + +#[cfg(test)] +mod tests { + use crate::{BlindedPayload, MainnetEthSpec}; + use super::*; + + pub fn deserialize_bid>(str: &str) -> Result, Error> { + dbg!(str); + let bid = serde_json::from_str(str)?; + Ok(bid) + } + + #[test] + fn test_deserialize_builder_bid_merge() { + let str = r#"{ + "header": { + "parent_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "fee_recipient": "0xabcf8e0d4e9587369b2301d0790347320302cc09", + "state_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "receipts_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "logs_bloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", + "prev_randao": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "block_number": "1", + "gas_limit": "1", + "gas_used": "1", + "timestamp": "1", + "extra_data": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "base_fee_per_gas": "1", + "block_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "transactions_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2" + }, + "value": "1", + "pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a" + }"#; + let result = deserialize_bid::>(str); + assert!(result.is_ok()); + } + + + #[test] + fn test_deserialize_builder_bid_capella() { + let str = r#"{ + "header": { + "parent_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "fee_recipient": "0xabcf8e0d4e9587369b2301d0790347320302cc09", + "state_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "receipts_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "logs_bloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", + "prev_randao": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "block_number": "1", + "gas_limit": "1", + "gas_used": "1", + "timestamp": "1", + "extra_data": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "base_fee_per_gas": "1", + "block_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "transactions_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "withdrawals_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2" + }, + "value": "1", + "pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a" + }"#; + let result = deserialize_bid::>(str); + assert!(result.is_ok()); + } + + #[test] + fn test_deserialize_builder_bid_eip4844() { + let str = r#"{ + "header": { + "parent_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "fee_recipient": "0xabcf8e0d4e9587369b2301d0790347320302cc09", + "state_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "receipts_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "logs_bloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", + "prev_randao": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "block_number": "1", + "gas_limit": "1", + "gas_used": "1", + "timestamp": "1", + "extra_data": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "base_fee_per_gas": "1", + "excess_data_gas": "1", + "block_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "transactions_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "withdrawals_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2" + }, + "value": "1", + "pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a", + "blob_kzg_commitments": [ + "0xa94170080872584e54a1cf092d845703b13907f2e6b3b1c0ad573b910530499e3bcd48c6378846b80d2bfa58c81cf3d5" + ] + }"#; + let result = deserialize_bid::>(str); + assert!(result.is_ok()); + } +} \ No newline at end of file From 16fc69ff12c36e5c71c05f37675f9a6116337d63 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 15 Dec 2022 17:11:59 +1100 Subject: [PATCH 04/14] add feature config to tests --- consensus/types/src/builder_bid.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/consensus/types/src/builder_bid.rs b/consensus/types/src/builder_bid.rs index e32ce49ef68..985797da37a 100644 --- a/consensus/types/src/builder_bid.rs +++ b/consensus/types/src/builder_bid.rs @@ -113,7 +113,6 @@ mod tests { use super::*; pub fn deserialize_bid>(str: &str) -> Result, Error> { - dbg!(str); let bid = serde_json::from_str(str)?; Ok(bid) } @@ -146,6 +145,7 @@ mod tests { #[test] + #[cfg(feature = "withdrawals")] fn test_deserialize_builder_bid_capella() { let str = r#"{ "header": { @@ -173,6 +173,7 @@ mod tests { } #[test] + #[cfg(feature = "withdrawals")] fn test_deserialize_builder_bid_eip4844() { let str = r#"{ "header": { From 991ab981b35fa8ee6196fe8741dd72eeb8e57e97 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 15 Dec 2022 18:11:07 +1100 Subject: [PATCH 05/14] Handle kzg commitments from getPayload response --- beacon_node/execution_layer/src/lib.rs | 45 +++++++++++++++++--------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index d95959593ac..5a1c2571b62 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -797,11 +797,16 @@ impl ExecutionLayer { spec, ) { Ok(()) => Ok(ProvenancedPayload::Builder( - //FIXME(sean) the builder API needs to be updated - // NOTE the comment above was removed in the - // rebase with unstable.. I think it goes - // here now? - BlockProposalContents::Payload(header), + match relay.version.unwrap() { + ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => { + BlockProposalContents::Payload(header) + } + ForkName::Eip4844 => BlockProposalContents::PayloadAndBlobs { + payload: header, + blobs: VariableList::default(), + kzg_commitments: relay.data.message.blob_kzg_commitments().unwrap().clone() + }, + } )), Err(reason) if !reason.payload_invalid() => { info!( @@ -853,20 +858,30 @@ impl ExecutionLayer { spec, ) { Ok(()) => Ok(ProvenancedPayload::Builder( - //FIXME(sean) the builder API needs to be updated - // NOTE the comment above was removed in the - // rebase with unstable.. I think it goes - // here now? - BlockProposalContents::Payload(header), + match relay.version.unwrap() { + ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => { + BlockProposalContents::Payload(header) + } + ForkName::Eip4844 => BlockProposalContents::PayloadAndBlobs { + payload: header, + blobs: VariableList::default(), + kzg_commitments: relay.data.message.blob_kzg_commitments().unwrap().clone(), + }, + } )), // If the payload is valid then use it. The local EE failed // to produce a payload so we have no alternative. Err(e) if !e.payload_invalid() => Ok(ProvenancedPayload::Builder( - //FIXME(sean) the builder API needs to be updated - // NOTE the comment above was removed in the - // rebase with unstable.. I think it goes - // here now? - BlockProposalContents::Payload(header), + match relay.version.unwrap() { + ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => { + BlockProposalContents::Payload(header) + } + ForkName::Eip4844 => BlockProposalContents::PayloadAndBlobs { + payload: header, + blobs: VariableList::default(), + kzg_commitments: relay.data.message.blob_kzg_commitments().unwrap().clone(), + }, + } )), Err(reason) => { metrics::inc_counter_vec( From 6c85b1699661df71242ca228e24e1391b8d1d78e Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 15 Dec 2022 18:21:57 +1100 Subject: [PATCH 06/14] Fix comment --- beacon_node/http_api/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index f677df73635..113c8c7393e 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -1126,10 +1126,10 @@ pub fn serve( }, ); /* - * beacon/blocks + * beacon/blinded_blocks */ - // POST beacon/blocks + // POST beacon/blinded_blocks let post_beacon_blinded_blocks = eth_v1 .and(warp::path("beacon")) .and(warp::path("blinded_blocks")) From e90db2d4814d7f785dd9f80340f173496f9f2e44 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 15 Dec 2022 18:39:50 +1100 Subject: [PATCH 07/14] Fix formatting only --- beacon_node/execution_layer/src/lib.rs | 90 +++++++++++++++++--------- consensus/types/src/builder_bid.rs | 24 +++---- 2 files changed, 73 insertions(+), 41 deletions(-) diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 5a1c2571b62..c4fa7c76771 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -796,18 +796,28 @@ impl ExecutionLayer { self.inner.builder_profit_threshold, spec, ) { - Ok(()) => Ok(ProvenancedPayload::Builder( - match relay.version.unwrap() { - ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => { + Ok(()) => { + Ok(ProvenancedPayload::Builder(match relay.version.unwrap() { + ForkName::Base + | ForkName::Altair + | ForkName::Merge + | ForkName::Capella => { BlockProposalContents::Payload(header) } - ForkName::Eip4844 => BlockProposalContents::PayloadAndBlobs { - payload: header, - blobs: VariableList::default(), - kzg_commitments: relay.data.message.blob_kzg_commitments().unwrap().clone() - }, - } - )), + ForkName::Eip4844 => { + BlockProposalContents::PayloadAndBlobs { + payload: header, + blobs: VariableList::default(), + kzg_commitments: relay + .data + .message + .blob_kzg_commitments() + .unwrap() + .clone(), + } + } + })) + } Err(reason) if !reason.payload_invalid() => { info!( self.log(), @@ -857,32 +867,52 @@ impl ExecutionLayer { self.inner.builder_profit_threshold, spec, ) { - Ok(()) => Ok(ProvenancedPayload::Builder( - match relay.version.unwrap() { - ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => { + Ok(()) => { + Ok(ProvenancedPayload::Builder(match relay.version.unwrap() { + ForkName::Base + | ForkName::Altair + | ForkName::Merge + | ForkName::Capella => { BlockProposalContents::Payload(header) } - ForkName::Eip4844 => BlockProposalContents::PayloadAndBlobs { - payload: header, - blobs: VariableList::default(), - kzg_commitments: relay.data.message.blob_kzg_commitments().unwrap().clone(), - }, - } - )), + ForkName::Eip4844 => { + BlockProposalContents::PayloadAndBlobs { + payload: header, + blobs: VariableList::default(), + kzg_commitments: relay + .data + .message + .blob_kzg_commitments() + .unwrap() + .clone(), + } + } + })) + } // If the payload is valid then use it. The local EE failed // to produce a payload so we have no alternative. - Err(e) if !e.payload_invalid() => Ok(ProvenancedPayload::Builder( - match relay.version.unwrap() { - ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => { + Err(e) if !e.payload_invalid() => { + Ok(ProvenancedPayload::Builder(match relay.version.unwrap() { + ForkName::Base + | ForkName::Altair + | ForkName::Merge + | ForkName::Capella => { BlockProposalContents::Payload(header) } - ForkName::Eip4844 => BlockProposalContents::PayloadAndBlobs { - payload: header, - blobs: VariableList::default(), - kzg_commitments: relay.data.message.blob_kzg_commitments().unwrap().clone(), - }, - } - )), + ForkName::Eip4844 => { + BlockProposalContents::PayloadAndBlobs { + payload: header, + blobs: VariableList::default(), + kzg_commitments: relay + .data + .message + .blob_kzg_commitments() + .unwrap() + .clone(), + } + } + })) + } Err(reason) => { metrics::inc_counter_vec( &metrics::EXECUTION_LAYER_GET_PAYLOAD_BUILDER_REJECTIONS, diff --git a/consensus/types/src/builder_bid.rs b/consensus/types/src/builder_bid.rs index 985797da37a..b0aa4e9c751 100644 --- a/consensus/types/src/builder_bid.rs +++ b/consensus/types/src/builder_bid.rs @@ -1,7 +1,7 @@ use super::KzgCommitment; use crate::{ AbstractExecPayload, ChainSpec, EthSpec, ExecPayload, ExecutionPayloadHeader, SignedRoot, - Uint256 + Uint256, }; use bls::PublicKeyBytes; use bls::Signature; @@ -17,14 +17,16 @@ use tree_hash_derive::TreeHash; #[superstruct( variants(Merge, Capella, Eip4844), variant_attributes( - derive( - PartialEq, Debug, Serialize, Deserialize, TreeHash, Clone - ), + derive(PartialEq, Debug, Serialize, Deserialize, TreeHash, Clone), serde(bound = "E: EthSpec, Payload: ExecPayload", deny_unknown_fields) ) )] #[derive(PartialEq, Debug, Serialize, Deserialize, TreeHash, Clone)] -#[serde(bound = "E: EthSpec, Payload: ExecPayload", deny_unknown_fields, untagged)] +#[serde( + bound = "E: EthSpec, Payload: ExecPayload", + deny_unknown_fields, + untagged +)] #[tree_hash(enum_behaviour = "transparent")] pub struct BuilderBid> { #[superstruct(only(Merge), partial_getter(rename = "payload_merge"))] @@ -106,17 +108,18 @@ impl> SignedBuilderBid { } } - #[cfg(test)] mod tests { - use crate::{BlindedPayload, MainnetEthSpec}; use super::*; + use crate::{BlindedPayload, MainnetEthSpec}; - pub fn deserialize_bid>(str: &str) -> Result, Error> { + pub fn deserialize_bid>( + str: &str, + ) -> Result, Error> { let bid = serde_json::from_str(str)?; Ok(bid) } - + #[test] fn test_deserialize_builder_bid_merge() { let str = r#"{ @@ -143,7 +146,6 @@ mod tests { assert!(result.is_ok()); } - #[test] #[cfg(feature = "withdrawals")] fn test_deserialize_builder_bid_capella() { @@ -203,4 +205,4 @@ mod tests { let result = deserialize_bid::>(str); assert!(result.is_ok()); } -} \ No newline at end of file +} From 46829ade087a347103cb5e7fd78ae674f6ce4ed9 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Tue, 10 Jan 2023 13:47:47 +1100 Subject: [PATCH 08/14] Builder bid serde working --- beacon_node/execution_layer/src/lib.rs | 8 +- consensus/types/src/builder_bid.rs | 204 ++++++++++++------------- 2 files changed, 106 insertions(+), 106 deletions(-) diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 2361223da1c..e2db1843973 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -750,7 +750,7 @@ impl ExecutionLayer { "Requested blinded execution payload"; "relay_fee_recipient" => match &relay_result { // FIXME remove clone? - Ok(Some(r)) => format!("{:?}", r.data.message.clone().header().unwrap().fee_recipient()), + Ok(Some(r)) => format!("{:?}", r.data.message.clone().header().fee_recipient()), Ok(None) => "empty response".to_string(), Err(_) => "request failed".to_string(), }, @@ -787,7 +787,7 @@ impl ExecutionLayer { } (Ok(Some(relay)), Ok(local)) => { // FIXME remove clone? - let header = relay.data.message.clone().header().unwrap(); + let header = relay.data.message.header().clone(); info!( self.log(), @@ -858,7 +858,7 @@ impl ExecutionLayer { } (Ok(Some(relay)), Err(local_error)) => { // FIXME remove clone? - let header = relay.data.message.clone().header().unwrap(); + let header = relay.data.message.header().clone(); info!( self.log(), @@ -1911,7 +1911,7 @@ fn verify_builder_bid>( ) -> Result<(), Box> { let is_signature_valid = bid.data.verify_signature(spec); // FIXME remove clone? - let header = &bid.data.message.clone().header().unwrap(); + let header = &bid.data.message.header(); let payload_value = bid.data.message.value().clone(); // Avoid logging values that we can't represent with our Prometheus library. diff --git a/consensus/types/src/builder_bid.rs b/consensus/types/src/builder_bid.rs index b0aa4e9c751..cd704332ae1 100644 --- a/consensus/types/src/builder_bid.rs +++ b/consensus/types/src/builder_bid.rs @@ -7,8 +7,7 @@ use bls::PublicKeyBytes; use bls::Signature; use serde::{Deserialize as De, Deserializer, Serialize as Ser, Serializer}; use serde_derive::{Deserialize, Serialize}; -use serde_json::Error; -use serde_with::{DeserializeAs, SerializeAs}; +use serde_with::{As, DeserializeAs, SerializeAs}; use ssz_types::VariableList; use std::marker::PhantomData; use superstruct::superstruct; @@ -29,14 +28,8 @@ use tree_hash_derive::TreeHash; )] #[tree_hash(enum_behaviour = "transparent")] pub struct BuilderBid> { - #[superstruct(only(Merge), partial_getter(rename = "payload_merge"))] - pub header: Payload::Merge, - - #[superstruct(only(Capella), partial_getter(rename = "payload_capella"))] - pub header: Payload::Capella, - - #[superstruct(only(Eip4844), partial_getter(rename = "payload_eip4844"))] - pub header: Payload::Eip4844, + #[serde(with = "As::>")] + pub header: Payload, #[serde(with = "eth2_serde_utils::quoted_u256")] pub value: Uint256, @@ -50,16 +43,6 @@ pub struct BuilderBid> { _phantom_data: PhantomData, } -impl> BuilderBid { - pub fn header(self) -> Result { - match self { - Self::Merge(bid) => Ok(bid.header.into()), - Self::Capella(bid) => Ok(bid.header.into()), - Self::Eip4844(bid) => Ok(bid.header.into()), - } - } -} - impl> SignedRoot for BuilderBid {} /// Validator registration, for use in interacting with servers implementing the builder API. @@ -113,96 +96,113 @@ mod tests { use super::*; use crate::{BlindedPayload, MainnetEthSpec}; - pub fn deserialize_bid>( - str: &str, - ) -> Result, Error> { - let bid = serde_json::from_str(str)?; - Ok(bid) + type Spec = MainnetEthSpec; + type Payload = BlindedPayload; + + fn deserialize_bid(str: &str) -> BuilderBid { + serde_json::from_str(str).expect("should deserialize to BuilderBid") + } + + fn serialize_bid(bid: BuilderBid) -> String { + serde_json::to_string(&bid).expect("should serialize to json string") + } + + fn to_minified_json(json: &str) -> String { + let mut json_mut = String::from(json); + json_mut.retain(|c| !c.is_whitespace()); + json_mut } #[test] - fn test_deserialize_builder_bid_merge() { - let str = r#"{ - "header": { - "parent_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "fee_recipient": "0xabcf8e0d4e9587369b2301d0790347320302cc09", - "state_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "receipts_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "logs_bloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", - "prev_randao": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "block_number": "1", - "gas_limit": "1", - "gas_used": "1", - "timestamp": "1", - "extra_data": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "base_fee_per_gas": "1", - "block_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "transactions_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2" - }, - "value": "1", - "pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a" - }"#; - let result = deserialize_bid::>(str); - assert!(result.is_ok()); + fn test_serde_builder_bid_merge() { + let expected_json = to_minified_json( + r#"{ + "header": { + "parent_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "fee_recipient": "0xabcf8e0d4e9587369b2301d0790347320302cc09", + "state_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "receipts_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "logs_bloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", + "prev_randao": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "block_number": "1", + "gas_limit": "1", + "gas_used": "1", + "timestamp": "1", + "extra_data": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "base_fee_per_gas": "1", + "block_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "transactions_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2" + }, + "value": "1", + "pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a" + }"#, + ); + let bid = deserialize_bid(&expected_json); + let actual_json = serialize_bid(bid); + assert_eq!(actual_json, expected_json); } #[test] - #[cfg(feature = "withdrawals")] - fn test_deserialize_builder_bid_capella() { - let str = r#"{ - "header": { - "parent_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "fee_recipient": "0xabcf8e0d4e9587369b2301d0790347320302cc09", - "state_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "receipts_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "logs_bloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", - "prev_randao": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "block_number": "1", - "gas_limit": "1", - "gas_used": "1", - "timestamp": "1", - "extra_data": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "base_fee_per_gas": "1", - "block_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "transactions_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "withdrawals_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2" - }, - "value": "1", - "pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a" - }"#; - let result = deserialize_bid::>(str); - assert!(result.is_ok()); + fn test_serde_builder_bid_capella() { + let expected_json = to_minified_json( + r#"{ + "header": { + "parent_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "fee_recipient": "0xabcf8e0d4e9587369b2301d0790347320302cc09", + "state_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "receipts_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "logs_bloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", + "prev_randao": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "block_number": "1", + "gas_limit": "1", + "gas_used": "1", + "timestamp": "1", + "extra_data": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "base_fee_per_gas": "1", + "block_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "transactions_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "withdrawals_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2" + }, + "value": "1", + "pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a" + }"#, + ); + let bid = deserialize_bid(&expected_json); + let actual_json = serialize_bid(bid); + assert_eq!(actual_json, expected_json); } #[test] - #[cfg(feature = "withdrawals")] - fn test_deserialize_builder_bid_eip4844() { - let str = r#"{ - "header": { - "parent_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "fee_recipient": "0xabcf8e0d4e9587369b2301d0790347320302cc09", - "state_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "receipts_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "logs_bloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", - "prev_randao": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "block_number": "1", - "gas_limit": "1", - "gas_used": "1", - "timestamp": "1", - "extra_data": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "base_fee_per_gas": "1", - "excess_data_gas": "1", - "block_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "transactions_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", - "withdrawals_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2" - }, - "value": "1", - "pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a", - "blob_kzg_commitments": [ - "0xa94170080872584e54a1cf092d845703b13907f2e6b3b1c0ad573b910530499e3bcd48c6378846b80d2bfa58c81cf3d5" - ] - }"#; - let result = deserialize_bid::>(str); - assert!(result.is_ok()); + fn test_serde_builder_bid_eip4844() { + let expected_json = to_minified_json( + r#"{ + "header": { + "parent_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "fee_recipient": "0xabcf8e0d4e9587369b2301d0790347320302cc09", + "state_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "receipts_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "logs_bloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", + "prev_randao": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "block_number": "1", + "gas_limit": "1", + "gas_used": "1", + "timestamp": "1", + "extra_data": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "base_fee_per_gas": "1", + "excess_data_gas": "1", + "block_hash": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "transactions_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2", + "withdrawals_root": "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2" + }, + "value": "1", + "pubkey": "0x93247f2209abcacf57b75a51dafae777f9dd38bc7053d1af526f220a7489a6d3a2753e5f3e8b1cfe39b56f43611df74a", + "blob_kzg_commitments": [ + "0xa94170080872584e54a1cf092d845703b13907f2e6b3b1c0ad573b910530499e3bcd48c6378846b80d2bfa58c81cf3d5" + ] + }"#, + ); + let bid = deserialize_bid(&expected_json); + let actual_json = serialize_bid(bid); + assert_eq!(actual_json, expected_json); } } From 75bf1d52ccfdae041970d551cb6d9f94874f5fb6 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Tue, 10 Jan 2023 16:13:26 +1100 Subject: [PATCH 09/14] Remove unnecessary Merge variant from BuilderBid --- consensus/types/src/builder_bid.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consensus/types/src/builder_bid.rs b/consensus/types/src/builder_bid.rs index cd704332ae1..f5b0c9ad3a6 100644 --- a/consensus/types/src/builder_bid.rs +++ b/consensus/types/src/builder_bid.rs @@ -14,7 +14,7 @@ use superstruct::superstruct; use tree_hash_derive::TreeHash; #[superstruct( - variants(Merge, Capella, Eip4844), + variants(Capella, Eip4844), variant_attributes( derive(PartialEq, Debug, Serialize, Deserialize, TreeHash, Clone), serde(bound = "E: EthSpec, Payload: ExecPayload", deny_unknown_fields) From 0b8f154e43dd156c0c76ef7e0a9e5d1794053e6c Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Tue, 10 Jan 2023 18:25:46 +1100 Subject: [PATCH 10/14] Add handling of ExecutionPayloadAndBlobsSidecar response from builder --- beacon_node/builder_client/src/lib.rs | 9 ++- beacon_node/execution_layer/src/lib.rs | 69 +++++++++++++++------- beacon_node/http_api/src/publish_blocks.rs | 16 ++++- consensus/types/src/execution_payload.rs | 16 +++++ 4 files changed, 81 insertions(+), 29 deletions(-) diff --git a/beacon_node/builder_client/src/lib.rs b/beacon_node/builder_client/src/lib.rs index fecf6512ac8..265a985969b 100644 --- a/beacon_node/builder_client/src/lib.rs +++ b/beacon_node/builder_client/src/lib.rs @@ -1,8 +1,7 @@ use eth2::types::builder_bid::SignedBuilderBid; use eth2::types::{ - AbstractExecPayload, BlindedPayload, EthSpec, ExecutionBlockHash, ExecutionPayload, - ForkVersionedResponse, PublicKeyBytes, SignedBeaconBlock, SignedValidatorRegistrationData, - Slot, + AbstractExecPayload, BlindedPayload, EthSpec, ExecutionBlockHash, ForkVersionedResponse, + PublicKeyBytes, SignedBeaconBlock, SignedValidatorRegistrationData, Slot, }; pub use eth2::Error; use eth2::{ok_or_error, StatusCode}; @@ -135,10 +134,10 @@ impl BuilderHttpClient { } /// `POST /eth/v1/builder/blinded_blocks` - pub async fn post_builder_blinded_blocks( + pub async fn post_builder_blinded_blocks( &self, blinded_block: &SignedBeaconBlock>, - ) -> Result>, Error> { + ) -> Result, Error> { let mut path = self.server.full.clone(); path.path_segments_mut() diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index e2db1843973..45efb8e6893 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -35,6 +35,7 @@ use tokio::{ time::sleep, }; use tokio_stream::wrappers::WatchStream; +use types::execution_payload::{ExecutionPayloadAndBlobsSidecar, PayloadWrapper}; use types::{AbstractExecPayload, Blob, ExecPayload, KzgCommitment}; use types::{ BlindedPayload, BlockType, ChainSpec, Epoch, ExecutionBlockHash, ForkName, @@ -1753,7 +1754,8 @@ impl ExecutionLayer { &self, block_root: Hash256, block: &SignedBeaconBlock>, - ) -> Result, Error> { + fork_name: ForkName, + ) -> Result, Error> { debug!( self.log(), "Sending block to builder"; @@ -1763,29 +1765,31 @@ impl ExecutionLayer { if let Some(builder) = self.builder() { let (payload_result, duration) = timed_future(metrics::POST_BLINDED_PAYLOAD_BUILDER, async { - builder - .post_builder_blinded_blocks(block) - .await - .map_err(Error::Builder) - .map(|d| d.data) + match fork_name { + ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => { + let payload = builder + .post_builder_blinded_blocks::>(block) + .await + .map_err(Error::Builder) + .map(|d| d.data)?; + Ok(PayloadWrapper::Payload(payload)) + } + ForkName::Eip4844 => { + let payload_and_blob = builder + .post_builder_blinded_blocks::>(block) + .await + .map_err(Error::Builder) + .map(|d| d.data)?; + Ok(PayloadWrapper::PayloadAndBlob(payload_and_blob)) + }, + } }) .await; - match &payload_result { - Ok(payload) => { - metrics::inc_counter_vec( - &metrics::EXECUTION_LAYER_BUILDER_REVEAL_PAYLOAD_OUTCOME, - &[metrics::SUCCESS], - ); - info!( - self.log(), - "Builder successfully revealed payload"; - "relay_response_ms" => duration.as_millis(), - "block_root" => ?block_root, - "fee_recipient" => ?payload.fee_recipient(), - "block_hash" => ?payload.block_hash(), - "parent_hash" => ?payload.parent_hash() - ) + let payload_maybe = match &payload_result { + Ok(PayloadWrapper::Payload(payload)) => Some(payload), + Ok(PayloadWrapper::PayloadAndBlob(payload_and_blob)) => { + Some(&payload_and_blob.execution_payload) } Err(e) => { metrics::inc_counter_vec( @@ -1804,8 +1808,29 @@ impl ExecutionLayer { .execution_payload() .map(|payload| format!("{}", payload.parent_hash())) .unwrap_or_else(|_| "unknown".to_string()) - ) + ); + None + } + }; + + match payload_maybe { + Some(payload) => { + metrics::inc_counter_vec( + &metrics::EXECUTION_LAYER_BUILDER_REVEAL_PAYLOAD_OUTCOME, + &[metrics::SUCCESS], + ); + + info!( + self.log(), + "Builder successfully revealed payload"; + "relay_response_ms" => duration.as_millis(), + "block_root" => ?block_root, + "fee_recipient" => ?payload.fee_recipient(), + "block_hash" => ?payload.block_hash(), + "parent_hash" => ?payload.parent_hash() + ); } + None => {} } payload_result diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index 4ddef78d8c0..f976e0d4ea3 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -9,6 +9,7 @@ use slot_clock::SlotClock; use std::sync::Arc; use tokio::sync::mpsc::UnboundedSender; use tree_hash::TreeHash; +use types::execution_payload::PayloadWrapper; use types::signed_block_and_blobs::BlockWrapper; use types::{ AbstractExecPayload, BlindedPayload, EthSpec, ExecPayload, ExecutionBlockHash, FullPayload, @@ -203,8 +204,11 @@ async fn reconstruct_block( cached_payload // Otherwise, this means we are attempting a blind block proposal. } else { - let full_payload = el - .propose_blinded_beacon_block(block_root, &block) + let fork_name = chain + .spec + .fork_name_at_epoch(block.slot().epoch(T::EthSpec::slots_per_epoch())); + let payload_wrapper = el + .propose_blinded_beacon_block(block_root, &block, fork_name) .await .map_err(|e| { warp_utils::reject::custom_server_error(format!( @@ -212,6 +216,14 @@ async fn reconstruct_block( e )) })?; + //FIXME: what to do with blob? + let full_payload = match &payload_wrapper { + PayloadWrapper::Payload(payload) => payload, + PayloadWrapper::PayloadAndBlob(payload_and_blob) => { + &payload_and_blob.execution_payload + } + }; + info!(log, "Successfully published a block to the builder network"; "block_hash" => ?full_payload.block_hash()); full_payload }; diff --git a/consensus/types/src/execution_payload.rs b/consensus/types/src/execution_payload.rs index 45f52fb65a7..9d7107e9071 100644 --- a/consensus/types/src/execution_payload.rs +++ b/consensus/types/src/execution_payload.rs @@ -133,3 +133,19 @@ impl ExecutionPayload { + (T::max_withdrawals_per_payload() * ::ssz_fixed_len()) } } + +#[derive(Debug, Clone, Serialize, Deserialize, Encode, TreeHash, Derivative)] +#[derivative(PartialEq, Hash(bound = "T: EthSpec"))] +#[serde(bound = "T: EthSpec")] +pub struct ExecutionPayloadAndBlobsSidecar { + pub execution_payload: ExecutionPayload, + pub blobs_sidecar: BlobsSidecar, +} + +/// A wrapper over a [`ExecutionPayload`] or a [`ExecutionPayloadAndBlobsSidecar`]. +#[derive(Clone, Debug, Derivative)] +#[derivative(PartialEq, Hash(bound = "T: EthSpec"))] +pub enum PayloadWrapper { + Payload(ExecutionPayload), + PayloadAndBlob(ExecutionPayloadAndBlobsSidecar), +} From 9222050e4c30765abcc2bb8fdc4ca93d8a1f628a Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 11 Jan 2023 15:44:41 +1100 Subject: [PATCH 11/14] Publish blobs received from builder --- beacon_node/http_api/src/lib.rs | 4 +- beacon_node/http_api/src/publish_blocks.rs | 149 +++++++++++---------- consensus/types/src/signed_beacon_block.rs | 21 +++ 3 files changed, 101 insertions(+), 73 deletions(-) diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index e8ec77f87b1..752469f21dd 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -50,6 +50,7 @@ use sysinfo::{System, SystemExt}; use system_health::observe_system_health_bn; use tokio::sync::mpsc::{Sender, UnboundedSender}; use tokio_stream::{wrappers::BroadcastStream, StreamExt}; +use types::signed_block_and_blobs::BlockWrapper; use types::{ Attestation, AttestationData, AttesterSlashing, BeaconStateError, BlindedPayload, CommitteeCache, ConfigAndPreset, Epoch, EthSpec, ForkName, FullPayload, @@ -1120,7 +1121,8 @@ pub fn serve( chain: Arc>, network_tx: UnboundedSender>, log: Logger| async move { - publish_blocks::publish_block(None, block, chain, &network_tx, log) + let block_wrapper = BlockWrapper::new(block); + publish_blocks::publish_block(None, block_wrapper, chain, &network_tx, log) .await .map(|()| warp::reply()) }, diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index f976e0d4ea3..0b7fac1710d 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -20,13 +20,13 @@ use warp::Rejection; /// Handles a request from the HTTP API for full blocks. pub async fn publish_block( block_root: Option, - block: Arc>, + block_wrapper: BlockWrapper, chain: Arc>, network_tx: &UnboundedSender>, log: Logger, ) -> Result<(), Rejection> { let seen_timestamp = timestamp_now(); - + let block = block_wrapper.block(); //FIXME(sean) have to move this to prior to publishing because it's included in the blobs sidecar message. //this may skew metrics let block_root = block_root.unwrap_or_else(|| block.canonical_root()); @@ -35,21 +35,35 @@ pub async fn publish_block( // specification is very clear that this is the desired behaviour. let wrapped_block: BlockWrapper = if matches!(block.as_ref(), &SignedBeaconBlock::Eip4844(_)) { - if let Some(sidecar) = chain.blob_cache.pop(&block_root) { - let block_and_blobs = SignedBeaconBlockAndBlobsSidecar { - beacon_block: block, - blobs_sidecar: Arc::new(sidecar), - }; - crate::publish_pubsub_message( - network_tx, - PubsubMessage::BeaconBlockAndBlobsSidecars(block_and_blobs.clone()), - )?; - block_and_blobs.into() - } else { - //FIXME(sean): This should probably return a specific no-blob-cached error code, beacon API coordination required - return Err(warp_utils::reject::broadcast_without_import(format!( - "no blob cached for block" - ))); + let maybe_sidecar = block_wrapper + .blobs(Some(block_root)) + .ok() + .flatten() + .or_else(|| { + chain + .blob_cache + .pop(&block_root) + .map(|blob_sidecar| Arc::new(blob_sidecar)) + }); + + match maybe_sidecar { + Some(sidecar) => { + let block_and_blobs = SignedBeaconBlockAndBlobsSidecar { + beacon_block: block_wrapper.block_cloned(), + blobs_sidecar: sidecar, + }; + crate::publish_pubsub_message( + network_tx, + PubsubMessage::BeaconBlockAndBlobsSidecars(block_and_blobs.clone()), + )?; + block_and_blobs.into() + } + None => { + //FIXME(sean): This should probably return a specific no-blob-cached error code, beacon API coordination required + return Err(warp_utils::reject::broadcast_without_import(format!( + "no blob cached for block" + ))); + } } } else { crate::publish_pubsub_message(network_tx, PubsubMessage::BeaconBlock(block.clone()))?; @@ -163,15 +177,8 @@ pub async fn publish_blinded_block( log: Logger, ) -> Result<(), Rejection> { let block_root = block.canonical_root(); - let full_block = reconstruct_block(chain.clone(), block_root, block, log.clone()).await?; - publish_block::( - Some(block_root), - Arc::new(full_block), - chain, - network_tx, - log, - ) - .await + let block_wrapper = reconstruct_block(chain.clone(), block_root, block, log.clone()).await?; + publish_block::(Some(block_root), block_wrapper, chain, network_tx, log).await } /// Deconstruct the given blinded block, and construct a full block. This attempts to use the @@ -182,58 +189,56 @@ async fn reconstruct_block( block_root: Hash256, block: SignedBeaconBlock>, log: Logger, -) -> Result>, Rejection> { - let full_payload = if let Ok(payload_header) = block.message().body().execution_payload() { - let el = chain.execution_layer.as_ref().ok_or_else(|| { - warp_utils::reject::custom_server_error("Missing execution layer".to_string()) - })?; - - // If the execution block hash is zero, use an empty payload. - let full_payload = if payload_header.block_hash() == ExecutionBlockHash::zero() { - FullPayload::default_at_fork( - chain - .spec - .fork_name_at_epoch(block.slot().epoch(T::EthSpec::slots_per_epoch())), - ) - .into() - // If we already have an execution payload with this transactions root cached, use it. - } else if let Some(cached_payload) = - el.get_payload_by_root(&payload_header.tree_hash_root()) - { - info!(log, "Reconstructing a full block using a local payload"; "block_hash" => ?cached_payload.block_hash()); - cached_payload - // Otherwise, this means we are attempting a blind block proposal. - } else { - let fork_name = chain +) -> Result, Rejection> { + let payload_header = block.message().body().execution_payload().map_err(|e| { + warp_utils::reject::custom_server_error("Unable to add payload to block".to_string()) + })?; + + let el = chain.execution_layer.as_ref().ok_or_else(|| { + warp_utils::reject::custom_server_error("Missing execution layer".to_string()) + })?; + + // If the execution block hash is zero, use an empty payload. + let block_wrapper = if payload_header.block_hash() == ExecutionBlockHash::zero() { + let payload = FullPayload::default_at_fork( + chain .spec - .fork_name_at_epoch(block.slot().epoch(T::EthSpec::slots_per_epoch())); - let payload_wrapper = el - .propose_blinded_beacon_block(block_root, &block, fork_name) - .await - .map_err(|e| { - warp_utils::reject::custom_server_error(format!( - "Blind block proposal failed: {:?}", - e - )) - })?; - //FIXME: what to do with blob? - let full_payload = match &payload_wrapper { - PayloadWrapper::Payload(payload) => payload, - PayloadWrapper::PayloadAndBlob(payload_and_blob) => { - &payload_and_blob.execution_payload - } - }; + .fork_name_at_epoch(block.slot().epoch(T::EthSpec::slots_per_epoch())), + ) + .into(); + block.try_into_full_block_wrapper(Some(payload), None) + // If we already have an execution payload with this transactions root cached, use it. + } else if let Some(cached_payload) = el.get_payload_by_root(&payload_header.tree_hash_root()) { + info!(log, "Reconstructing a full block using a local payload"; "block_hash" => ?cached_payload.block_hash()); + block.try_into_full_block_wrapper(Some(cached_payload), None) + // Otherwise, this means we are attempting a blind block proposal. + } else { + let fork_name = chain + .spec + .fork_name_at_epoch(block.slot().epoch(T::EthSpec::slots_per_epoch())); + let payload_wrapper = el + .propose_blinded_beacon_block(block_root, &block, fork_name) + .await + .map_err(|e| { + warp_utils::reject::custom_server_error(format!( + "Blind block proposal failed: {:?}", + e + )) + })?; - info!(log, "Successfully published a block to the builder network"; "block_hash" => ?full_payload.block_hash()); - full_payload + let (full_payload, maybe_blobs) = match payload_wrapper { + PayloadWrapper::Payload(payload) => (payload, None), + PayloadWrapper::PayloadAndBlob(payload_and_blob) => ( + payload_and_blob.execution_payload, + Some(payload_and_blob.blobs_sidecar), + ), }; - Some(full_payload) - } else { - None + info!(log, "Successfully published a block to the builder network"; "block_hash" => ?full_payload.block_hash()); + block.try_into_full_block_wrapper(Some(full_payload), maybe_blobs) }; - block.try_into_full_block(full_payload).ok_or_else(|| { + block_wrapper.ok_or_else(|| { warp_utils::reject::custom_server_error("Unable to add payload to block".to_string()) }) } diff --git a/consensus/types/src/signed_beacon_block.rs b/consensus/types/src/signed_beacon_block.rs index 89ccb95a164..2840cd3a668 100644 --- a/consensus/types/src/signed_beacon_block.rs +++ b/consensus/types/src/signed_beacon_block.rs @@ -1,9 +1,11 @@ +use crate::signed_block_and_blobs::BlockWrapper; use crate::*; use bls::Signature; use derivative::Derivative; use serde_derive::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; use std::fmt; +use std::sync::Arc; use superstruct::superstruct; use tree_hash::TreeHash; use tree_hash_derive::TreeHash; @@ -455,6 +457,25 @@ impl SignedBeaconBlockEip4844> { } impl SignedBeaconBlock> { + pub fn try_into_full_block_wrapper( + self, + execution_payload: Option>, + blobs_sidecar: Option>, + ) -> Option> { + let maybe_full_block = self.try_into_full_block(execution_payload); + + maybe_full_block.map(|full_block| match full_block { + SignedBeaconBlock::Base(_) + | SignedBeaconBlock::Altair(_) + | SignedBeaconBlock::Merge(_) + | SignedBeaconBlock::Capella(_) => BlockWrapper::new(Arc::new(full_block)), + SignedBeaconBlock::Eip4844(_) => BlockWrapper::new_with_blobs( + Arc::new(full_block), + Arc::new(blobs_sidecar.unwrap_or_default()), + ), + }) + } + pub fn try_into_full_block( self, execution_payload: Option>, From 6d232f91c40bf02853d7b13a09528fa6274261cb Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 11 Jan 2023 16:12:37 +1100 Subject: [PATCH 12/14] Fix code warnings --- beacon_node/http_api/src/block_id.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beacon_node/http_api/src/block_id.rs b/beacon_node/http_api/src/block_id.rs index fd45e14faa1..45c7bed1f7a 100644 --- a/beacon_node/http_api/src/block_id.rs +++ b/beacon_node/http_api/src/block_id.rs @@ -216,10 +216,10 @@ impl BlockId { pub async fn blobs_sidecar( &self, chain: &BeaconChain, - ) -> Result<(Arc>), warp::Rejection> { + ) -> Result>, warp::Rejection> { let root = self.root(chain)?.0; match chain.store.get_blobs(&root) { - Ok(Some(blob)) => Ok((Arc::new(blob))), + Ok(Some(blob)) => Ok(Arc::new(blob)), Ok(None) => Err(warp_utils::reject::custom_not_found(format!( "Blob with block root {} is not in the store", root From a64eaad6793827b1a1c57c5cb46eca1ff0275cfd Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 11 Jan 2023 16:19:51 +1100 Subject: [PATCH 13/14] Add mising kzg trusted setup in tests. Fix compilation errors. --- .../execution_layer/src/test_utils/mock_builder.rs | 8 ++++---- beacon_node/http_api/src/publish_blocks.rs | 6 +++--- common/eth2_network_config/src/lib.rs | 4 ++++ lcli/src/new_testnet.rs | 6 +++++- 4 files changed, 16 insertions(+), 8 deletions(-) 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 06b5e81eb31..8467ceef578 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_builder.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_builder.rs @@ -26,8 +26,7 @@ use task_executor::TaskExecutor; use tempfile::NamedTempFile; use tree_hash::TreeHash; use types::{ - Address, BeaconState, BlindedPayload, ChainSpec, EthSpec, ExecPayload, ForkName, Hash256, Slot, - Uint256, + Address, BeaconState, BlindedPayload, ChainSpec, EthSpec, ExecPayload, Hash256, Slot, Uint256, }; #[derive(Clone)] @@ -306,14 +305,15 @@ impl mev_build_rs::BlindedBlockProvider for MockBuilder { let payload_attributes = PayloadAttributes::new(timestamp, *prev_randao, fee_recipient, None); + let fork_name = self.spec.fork_name_at_slot::(slot); + let payload = self .el .get_full_payload_caching::>( head_execution_hash, &payload_attributes, forkchoice_update_params, - // TODO: do we need to write a test for this if this is Capella fork? - ForkName::Merge, + fork_name, ) .await .map_err(convert_err)? diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index 0b7fac1710d..a19b20ac380 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -26,7 +26,7 @@ pub async fn publish_block( log: Logger, ) -> Result<(), Rejection> { let seen_timestamp = timestamp_now(); - let block = block_wrapper.block(); + let block = block_wrapper.block_cloned(); //FIXME(sean) have to move this to prior to publishing because it's included in the blobs sidecar message. //this may skew metrics let block_root = block_root.unwrap_or_else(|| block.canonical_root()); @@ -49,7 +49,7 @@ pub async fn publish_block( match maybe_sidecar { Some(sidecar) => { let block_and_blobs = SignedBeaconBlockAndBlobsSidecar { - beacon_block: block_wrapper.block_cloned(), + beacon_block: block, blobs_sidecar: sidecar, }; crate::publish_pubsub_message( @@ -190,7 +190,7 @@ async fn reconstruct_block( block: SignedBeaconBlock>, log: Logger, ) -> Result, Rejection> { - let payload_header = block.message().body().execution_payload().map_err(|e| { + let payload_header = block.message().body().execution_payload().map_err(|_| { warp_utils::reject::custom_server_error("Unable to add payload to block".to_string()) })?; diff --git a/common/eth2_network_config/src/lib.rs b/common/eth2_network_config/src/lib.rs index d708b7854ce..8295cf5fd56 100644 --- a/common/eth2_network_config/src/lib.rs +++ b/common/eth2_network_config/src/lib.rs @@ -362,11 +362,15 @@ mod tests { let base_dir = temp_dir.path().join("my_testnet"); let deposit_contract_deploy_block = 42; + let kzg_trusted_setup = serde_json::from_reader(TRUSTED_SETUP) + .map_err(|e| format!("Failed to load trusted setup: {}", e))?; + let testnet: Eth2NetworkConfig = Eth2NetworkConfig { deposit_contract_deploy_block, boot_enr, genesis_state_bytes: genesis_state.as_ref().map(Encode::as_ssz_bytes), config, + kzg_trusted_setup, }; testnet diff --git a/lcli/src/new_testnet.rs b/lcli/src/new_testnet.rs index d8973980feb..2ada8bd3118 100644 --- a/lcli/src/new_testnet.rs +++ b/lcli/src/new_testnet.rs @@ -1,7 +1,7 @@ use clap::ArgMatches; use clap_utils::{parse_optional, parse_required, parse_ssz_optional}; use eth2_hashing::hash; -use eth2_network_config::Eth2NetworkConfig; +use eth2_network_config::{Eth2NetworkConfig, TRUSTED_SETUP}; use ssz::Decode; use ssz::Encode; use state_processing::process_activations; @@ -142,11 +142,15 @@ pub fn run(testnet_dir_path: PathBuf, matches: &ArgMatches) -> Resul None }; + let kzg_trusted_setup = serde_json::from_reader(TRUSTED_SETUP) + .map_err(|e| format!("Failed to load trusted setup: {}", e))?; + let testnet = Eth2NetworkConfig { deposit_contract_deploy_block, boot_enr: Some(vec![]), genesis_state_bytes, config: Config::from_chain_spec::(&spec), + kzg_trusted_setup, }; testnet.write_to_file(testnet_dir_path, overwrite_files) From 1cac42c2532dcaee875d72a0c80b562198fe8fd6 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 11 Jan 2023 16:30:11 +1100 Subject: [PATCH 14/14] Remove unnecessary clones and FIXMEs --- beacon_node/execution_layer/src/lib.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 45efb8e6893..07a91a54515 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -750,8 +750,7 @@ impl ExecutionLayer { self.log(), "Requested blinded execution payload"; "relay_fee_recipient" => match &relay_result { - // FIXME remove clone? - Ok(Some(r)) => format!("{:?}", r.data.message.clone().header().fee_recipient()), + Ok(Some(r)) => format!("{:?}", r.data.message.header().fee_recipient()), Ok(None) => "empty response".to_string(), Err(_) => "request failed".to_string(), }, @@ -787,7 +786,6 @@ impl ExecutionLayer { Ok(ProvenancedPayload::Local(local)) } (Ok(Some(relay)), Ok(local)) => { - // FIXME remove clone? let header = relay.data.message.header().clone(); info!( @@ -858,7 +856,6 @@ impl ExecutionLayer { } } (Ok(Some(relay)), Err(local_error)) => { - // FIXME remove clone? let header = relay.data.message.header().clone(); info!( @@ -1935,7 +1932,6 @@ fn verify_builder_bid>( spec: &ChainSpec, ) -> Result<(), Box> { let is_signature_valid = bid.data.verify_signature(spec); - // FIXME remove clone? let header = &bid.data.message.header(); let payload_value = bid.data.message.value().clone();