From f5e6a54f0597d0bf74d761c7167d14f7d89c10ce Mon Sep 17 00:00:00 2001 From: Mark Mackey Date: Tue, 29 Nov 2022 18:01:47 -0600 Subject: [PATCH] Refactored Execution Layer & Fixed Some Tests --- beacon_node/beacon_chain/src/beacon_chain.rs | 15 ++- .../beacon_chain/src/execution_payload.rs | 21 ++-- .../tests/payload_invalidation.rs | 16 +-- beacon_node/execution_layer/src/engine_api.rs | 22 +++- beacon_node/execution_layer/src/engines.rs | 31 ++---- beacon_node/execution_layer/src/lib.rs | 102 ++++-------------- .../src/test_utils/mock_builder.rs | 18 ++-- .../src/test_utils/mock_execution_layer.rs | 57 ++++------ consensus/types/src/payload.rs | 12 +-- consensus/types/src/withdrawal.rs | 2 +- .../src/test_rig.rs | 44 ++++---- 11 files changed, 133 insertions(+), 207 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index dc0b6e717..20edab090 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -59,7 +59,7 @@ use crate::{metrics, BeaconChainError}; use eth2::types::{EventKind, SseBlock, SyncDuty}; use execution_layer::{ BlockProposalContents, BuilderParams, ChainHealth, ExecutionLayer, FailedCondition, - PayloadAttributes, PayloadAttributesV1, PayloadAttributesV2, PayloadStatus, + PayloadAttributes, PayloadStatus, }; pub use fork_choice::CountUnrealized; use fork_choice::{ @@ -4179,21 +4179,20 @@ impl BeaconChain { .map(|withdrawals_opt| withdrawals_opt.map(|w| w.into())) .map_err(Error::PrepareProposerFailed)?; - let payload_attributes = PayloadAttributes::V2(PayloadAttributesV2 { - timestamp: self - .slot_clock + let payload_attributes = PayloadAttributes::new( + self.slot_clock .start_of(prepare_slot) .ok_or(Error::InvalidSlot(prepare_slot))? .as_secs(), - prev_randao: head_random, - suggested_fee_recipient: execution_layer + head_random, + execution_layer .get_suggested_fee_recipient(proposer as u64) .await, #[cfg(feature = "withdrawals")] withdrawals, #[cfg(not(feature = "withdrawals"))] - withdrawals: None, - }); + None, + ); debug!( self.log, diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index 85aedc659..ff3167c70 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -12,7 +12,7 @@ use crate::{ BeaconChain, BeaconChainError, BeaconChainTypes, BlockError, BlockProductionError, ExecutionPayloadError, }; -use execution_layer::{BlockProposalContents, BuilderParams, PayloadStatus}; +use execution_layer::{BlockProposalContents, BuilderParams, PayloadAttributes, PayloadStatus}; use fork_choice::{InvalidationOperation, PayloadVerificationStatus}; use proto_array::{Block as ProtoBlock, ExecutionStatus}; use slog::debug; @@ -483,20 +483,29 @@ where .await .map_err(BlockProductionError::BeaconChain)?; + let suggested_fee_recipient = execution_layer + .get_suggested_fee_recipient(proposer_index) + .await; + let payload_attributes = PayloadAttributes::new( + timestamp, + random, + suggested_fee_recipient, + #[cfg(feature = "withdrawals")] + withdrawals, + #[cfg(not(feature = "withdrawals"))] + None, + ); + // Note: the suggested_fee_recipient is stored in the `execution_layer`, it will add this parameter. // // This future is not executed here, it's up to the caller to await it. let block_contents = execution_layer .get_payload::( parent_hash, - timestamp, - random, - proposer_index, + &payload_attributes, forkchoice_update_params, builder_params, fork, - #[cfg(feature = "withdrawals")] - withdrawals, &chain.spec, ) .await diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 5c470d4dd..a963f071a 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -14,7 +14,7 @@ use beacon_chain::{ use execution_layer::{ json_structures::{JsonForkchoiceStateV1, JsonPayloadAttributes, JsonPayloadAttributesV1}, test_utils::ExecutionBlockGenerator, - ExecutionLayer, ForkchoiceState, PayloadAttributes, PayloadAttributesV1, + ExecutionLayer, ForkchoiceState, PayloadAttributes, }; use fork_choice::{ CountUnrealized, Error as ForkChoiceError, InvalidationOperation, PayloadVerificationStatus, @@ -985,20 +985,22 @@ async fn payload_preparation() { .await .unwrap(); - let payload_attributes = PayloadAttributes::V1(PayloadAttributesV1 { - timestamp: rig - .harness + let payload_attributes = PayloadAttributes::new( + rig.harness .chain .slot_clock .start_of(next_slot) .unwrap() .as_secs(), - prev_randao: *head + *head .beacon_state .get_randao_mix(head.beacon_state.current_epoch()) .unwrap(), - suggested_fee_recipient: fee_recipient, - }); + fee_recipient, + None, + ) + .downgrade_to_v1() + .unwrap(); assert_eq!(rig.previous_payload_attributes(), payload_attributes); } diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index 65d3b656b..988b04826 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -243,11 +243,11 @@ impl From> for ExecutionBlockWithTransactions #[superstruct( variants(V1, V2), - variant_attributes(derive(Clone, Debug, PartialEq),), + variant_attributes(derive(Clone, Debug, Eq, Hash, PartialEq),), cast_error(ty = "Error", expr = "Error::IncorrectStateVariant"), partial_getter_error(ty = "Error", expr = "Error::IncorrectStateVariant") )] -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, Eq, Hash, PartialEq)] pub struct PayloadAttributes { #[superstruct(getter(copy))] pub timestamp: u64, @@ -260,14 +260,28 @@ pub struct PayloadAttributes { } impl PayloadAttributes { + pub fn new( + timestamp: u64, + prev_randao: Hash256, + suggested_fee_recipient: Address, + withdrawals: Option>, + ) -> Self { + // this should always return the highest version + PayloadAttributes::V2(PayloadAttributesV2 { + timestamp, + prev_randao, + suggested_fee_recipient, + withdrawals, + }) + } + pub fn downgrade_to_v1(self) -> Result { match self { PayloadAttributes::V1(_) => Ok(self), PayloadAttributes::V2(v2) => { - #[cfg(features = "withdrawals")] if v2.withdrawals.is_some() { return Err(Error::BadConversion( - "Downgrading from PayloadAttributesV2 with non-null withdrawaals" + "Downgrading from PayloadAttributesV2 with non-null withdrawals" .to_string(), )); } diff --git a/beacon_node/execution_layer/src/engines.rs b/beacon_node/execution_layer/src/engines.rs index 264303b5d..b5792e383 100644 --- a/beacon_node/execution_layer/src/engines.rs +++ b/beacon_node/execution_layer/src/engines.rs @@ -16,6 +16,7 @@ use types::{Address, ExecutionBlockHash, Hash256}; /// The number of payload IDs that will be stored for each `Engine`. /// /// Since the size of each value is small (~100 bytes) a large number is used for safety. +/// FIXME: check this assumption now that the key includes entire payload attributes which now includes withdrawals const PAYLOAD_ID_LRU_CACHE_SIZE: usize = 512; /// Stores the remembered state of a engine. @@ -97,9 +98,7 @@ pub struct ForkchoiceState { #[derive(Hash, PartialEq, std::cmp::Eq)] struct PayloadIdCacheKey { pub head_block_hash: ExecutionBlockHash, - pub timestamp: u64, - pub prev_randao: Hash256, - pub suggested_fee_recipient: Address, + pub payload_attributes: PayloadAttributes, } #[derive(Debug)] @@ -142,20 +141,13 @@ impl Engine { pub async fn get_payload_id( &self, - head_block_hash: ExecutionBlockHash, - timestamp: u64, - prev_randao: Hash256, - suggested_fee_recipient: Address, + head_block_hash: &ExecutionBlockHash, + payload_attributes: &PayloadAttributes, ) -> Option { self.payload_id_cache .lock() .await - .get(&PayloadIdCacheKey { - head_block_hash, - timestamp, - prev_randao, - suggested_fee_recipient, - }) + .get(&PayloadIdCacheKey::new(head_block_hash, payload_attributes)) .cloned() } @@ -171,8 +163,8 @@ impl Engine { .await?; if let Some(payload_id) = response.payload_id { - if let Some(key) = - payload_attributes.map(|pa| PayloadIdCacheKey::new(&forkchoice_state, &pa)) + if let Some(key) = payload_attributes + .map(|pa| PayloadIdCacheKey::new(&forkchoice_state.head_block_hash, &pa)) { self.payload_id_cache.lock().await.put(key, payload_id); } else { @@ -347,14 +339,11 @@ impl Engine { } } -// TODO: revisit this - do we need to key on withdrawals as well here? impl PayloadIdCacheKey { - fn new(state: &ForkchoiceState, attributes: &PayloadAttributes) -> Self { + fn new(head_block_hash: &ExecutionBlockHash, attributes: &PayloadAttributes) -> Self { Self { - head_block_hash: state.head_block_hash, - timestamp: attributes.timestamp(), - prev_randao: attributes.prev_randao(), - suggested_fee_recipient: attributes.suggested_fee_recipient(), + head_block_hash: head_block_hash.clone(), + payload_attributes: attributes.clone(), } } } diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 96c1e0906..25a19eb0b 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -606,21 +606,15 @@ impl ExecutionLayer { /// /// The result will be returned from the first node that returns successfully. No more nodes /// will be contacted. - #[allow(clippy::too_many_arguments)] pub async fn get_payload>( &self, parent_hash: ExecutionBlockHash, - timestamp: u64, - prev_randao: Hash256, - proposer_index: u64, + payload_attributes: &PayloadAttributes, forkchoice_update_params: ForkchoiceUpdateParameters, builder_params: BuilderParams, current_fork: ForkName, - #[cfg(feature = "withdrawals")] withdrawals: Option>, spec: &ChainSpec, ) -> Result, Error> { - let suggested_fee_recipient = self.get_suggested_fee_recipient(proposer_index).await; - match Payload::block_type() { BlockType::Blinded => { let _timer = metrics::start_timer_vec( @@ -629,14 +623,10 @@ impl ExecutionLayer { ); self.get_blinded_payload( parent_hash, - timestamp, - prev_randao, - suggested_fee_recipient, + payload_attributes, forkchoice_update_params, builder_params, current_fork, - #[cfg(feature = "withdrawals")] - withdrawals, spec, ) .await @@ -648,30 +638,22 @@ impl ExecutionLayer { ); self.get_full_payload( parent_hash, - timestamp, - prev_randao, - suggested_fee_recipient, + payload_attributes, forkchoice_update_params, current_fork, - #[cfg(feature = "withdrawals")] - withdrawals, ) .await } } } - #[allow(clippy::too_many_arguments)] async fn get_blinded_payload>( &self, parent_hash: ExecutionBlockHash, - timestamp: u64, - prev_randao: Hash256, - suggested_fee_recipient: Address, + payload_attributes: &PayloadAttributes, forkchoice_update_params: ForkchoiceUpdateParameters, builder_params: BuilderParams, current_fork: ForkName, - #[cfg(feature = "withdrawals")] withdrawals: Option>, spec: &ChainSpec, ) -> Result, Error> { if let Some(builder) = self.builder() { @@ -691,13 +673,9 @@ impl ExecutionLayer { builder.get_builder_header::(slot, parent_hash, &pubkey), self.get_full_payload_caching( parent_hash, - timestamp, - prev_randao, - suggested_fee_recipient, + payload_attributes, forkchoice_update_params, current_fork, - #[cfg(feature = "withdrawals")] - withdrawals, ) ); @@ -746,7 +724,7 @@ impl ExecutionLayer { falling back to local execution engine." ); Ok(local) - } else if header.prev_randao() != prev_randao { + } else if header.prev_randao() != payload_attributes.prev_randao() { warn!( self.log(), "Invalid prev randao from connected builder, \ @@ -784,7 +762,7 @@ impl ExecutionLayer { bid from connected builder, falling back to local execution engine."); Ok(local) } else { - if header.fee_recipient() != suggested_fee_recipient { + if header.fee_recipient() != payload_attributes.suggested_fee_recipient() { info!( self.log(), "Fee recipient from connected builder does \ @@ -823,13 +801,9 @@ impl ExecutionLayer { } self.get_full_payload_caching( parent_hash, - timestamp, - prev_randao, - suggested_fee_recipient, + payload_attributes, forkchoice_update_params, current_fork, - #[cfg(feature = "withdrawals")] - withdrawals, ) .await } @@ -838,22 +812,15 @@ impl ExecutionLayer { async fn get_full_payload>( &self, parent_hash: ExecutionBlockHash, - timestamp: u64, - prev_randao: Hash256, - suggested_fee_recipient: Address, + payload_attributes: &PayloadAttributes, forkchoice_update_params: ForkchoiceUpdateParameters, current_fork: ForkName, - #[cfg(feature = "withdrawals")] withdrawals: Option>, ) -> Result, Error> { self.get_full_payload_with( parent_hash, - timestamp, - prev_randao, - suggested_fee_recipient, + payload_attributes, forkchoice_update_params, current_fork, - #[cfg(feature = "withdrawals")] - withdrawals, noop, ) .await @@ -863,22 +830,15 @@ impl ExecutionLayer { async fn get_full_payload_caching>( &self, parent_hash: ExecutionBlockHash, - timestamp: u64, - prev_randao: Hash256, - suggested_fee_recipient: Address, + payload_attributes: &PayloadAttributes, forkchoice_update_params: ForkchoiceUpdateParameters, current_fork: ForkName, - #[cfg(feature = "withdrawals")] withdrawals: Option>, ) -> Result, Error> { self.get_full_payload_with( parent_hash, - timestamp, - prev_randao, - suggested_fee_recipient, + payload_attributes, forkchoice_update_params, current_fork, - #[cfg(feature = "withdrawals")] - withdrawals, Self::cache_payload, ) .await @@ -887,20 +847,15 @@ impl ExecutionLayer { async fn get_full_payload_with>( &self, parent_hash: ExecutionBlockHash, - timestamp: u64, - prev_randao: Hash256, - suggested_fee_recipient: Address, + payload_attributes: &PayloadAttributes, forkchoice_update_params: ForkchoiceUpdateParameters, current_fork: ForkName, - #[cfg(feature = "withdrawals")] withdrawals: Option>, f: fn(&ExecutionLayer, &ExecutionPayload) -> Option>, ) -> Result, Error> { - #[cfg(feature = "withdrawals")] - let withdrawals_ref = &withdrawals; self.engine() .request(move |engine| async move { let payload_id = if let Some(id) = engine - .get_payload_id(parent_hash, timestamp, prev_randao, suggested_fee_recipient) + .get_payload_id(&parent_hash, payload_attributes) .await { // The payload id has been cached for this engine. @@ -925,22 +880,11 @@ impl ExecutionLayer { .finalized_hash .unwrap_or_else(ExecutionBlockHash::zero), }; - // This must always be the latest PayloadAttributes - // FIXME: How to non-capella EIP4844 testnets handle this? - let payload_attributes = PayloadAttributes::V2(PayloadAttributesV2 { - timestamp, - prev_randao, - suggested_fee_recipient, - #[cfg(feature = "withdrawals")] - withdrawals: withdrawals_ref.clone(), - #[cfg(not(feature = "withdrawals"))] - withdrawals: None, - }); let response = engine .notify_forkchoice_updated( fork_choice_state, - Some(payload_attributes), + Some(payload_attributes.clone()), self.log(), ) .await?; @@ -969,9 +913,9 @@ impl ExecutionLayer { debug!( self.log(), "Issuing engine_getBlobsBundle"; - "suggested_fee_recipient" => ?suggested_fee_recipient, - "prev_randao" => ?prev_randao, - "timestamp" => timestamp, + "suggested_fee_recipient" => ?payload_attributes.suggested_fee_recipient(), + "prev_randao" => ?payload_attributes.prev_randao(), + "timestamp" => payload_attributes.timestamp(), "parent_hash" => ?parent_hash, ); Some(engine.api.get_blobs_bundle_v1::(payload_id).await) @@ -982,16 +926,16 @@ impl ExecutionLayer { debug!( self.log(), "Issuing engine_getPayload"; - "suggested_fee_recipient" => ?suggested_fee_recipient, - "prev_randao" => ?prev_randao, - "timestamp" => timestamp, + "suggested_fee_recipient" => ?payload_attributes.suggested_fee_recipient(), + "prev_randao" => ?payload_attributes.prev_randao(), + "timestamp" => payload_attributes.timestamp(), "parent_hash" => ?parent_hash, ); engine.api.get_payload::(current_fork, payload_id).await }; let (blob, payload) = tokio::join!(blob_fut, payload_fut); let payload = payload.map(|full_payload| { - if full_payload.fee_recipient() != suggested_fee_recipient { + if full_payload.fee_recipient() != payload_attributes.suggested_fee_recipient() { error!( self.log(), "Inconsistent fee recipient"; @@ -1001,7 +945,7 @@ impl ExecutionLayer { ensure that the value of suggested_fee_recipient is set correctly and \ that the Execution Engine is trusted.", "fee_recipient" => ?full_payload.fee_recipient(), - "suggested_fee_recipient" => ?suggested_fee_recipient, + "suggested_fee_recipient" => ?payload_attributes.suggested_fee_recipient(), ); } if f(self, &full_payload).is_some() { 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 5c69fffbf..06b5e81eb 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_builder.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_builder.rs @@ -1,5 +1,5 @@ use crate::test_utils::DEFAULT_JWT_SECRET; -use crate::{Config, ExecutionLayer, PayloadAttributes, PayloadAttributesV1}; +use crate::{Config, ExecutionLayer, PayloadAttributes}; use async_trait::async_trait; use eth2::types::{BlockId, StateId, ValidatorId}; use eth2::{BeaconNodeHttpClient, Timeouts}; @@ -289,11 +289,8 @@ impl mev_build_rs::BlindedBlockProvider for MockBuilder { .map_err(convert_err)?; // FIXME: think about proper fork here - let payload_attributes = PayloadAttributes::V1(PayloadAttributesV1 { - timestamp, - prev_randao: *prev_randao, - suggested_fee_recipient: fee_recipient, - }); + let payload_attributes = + PayloadAttributes::new(timestamp, *prev_randao, fee_recipient, None); self.el .insert_proposer(slot, head_block_root, val_index, payload_attributes) @@ -306,18 +303,17 @@ impl mev_build_rs::BlindedBlockProvider for MockBuilder { finalized_hash: Some(finalized_execution_hash), }; + let payload_attributes = + PayloadAttributes::new(timestamp, *prev_randao, fee_recipient, None); + let payload = self .el .get_full_payload_caching::>( head_execution_hash, - timestamp, - *prev_randao, - fee_recipient, + &payload_attributes, forkchoice_update_params, // TODO: do we need to write a test for this if this is Capella fork? ForkName::Merge, - #[cfg(feature = "withdrawals")] - None, ) .await .map_err(convert_err)? diff --git a/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs b/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs index 6a9070ca4..e6da676a8 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs @@ -98,35 +98,16 @@ impl MockExecutionLayer { justified_hash: None, finalized_hash: None, }; - - // FIXME: this is just best guess for how to deal with forks here.. - let payload_attributes = match &latest_execution_block { - &Block::PoS(ref pos_block) => match pos_block { - &ExecutionPayload::Merge(_) => PayloadAttributes::V1(PayloadAttributesV1 { - timestamp, - prev_randao, - suggested_fee_recipient: Address::repeat_byte(42), - }), - &ExecutionPayload::Capella(_) | &ExecutionPayload::Eip4844(_) => { - PayloadAttributes::V2(PayloadAttributesV2 { - timestamp, - prev_randao, - suggested_fee_recipient: Address::repeat_byte(42), - // FIXME: think about adding withdrawals here.. - #[cfg(feature = "withdrawals")] - withdrawals: Some(vec![]), - #[cfg(not(feature = "withdrawals"))] - withdrawals: None, - }) - } - }, - // I guess a PoW blocks means we should use Merge? - &Block::PoW(_) => PayloadAttributes::V1(PayloadAttributesV1 { - timestamp, - prev_randao, - suggested_fee_recipient: Address::repeat_byte(42), - }), - }; + let payload_attributes = PayloadAttributes::new( + timestamp, + prev_randao, + Address::repeat_byte(42), + // FIXME: think about how to handle different forks / withdrawals here.. + #[cfg(feature = "withdrawals")] + Some(vec![]), + #[cfg(not(feature = "withdrawals"))] + None, + ); // Insert a proposer to ensure the fork choice updated command works. let slot = Slot::new(0); @@ -152,19 +133,18 @@ impl MockExecutionLayer { slot, chain_health: ChainHealth::Healthy, }; + let suggested_fee_recipient = self.el.get_suggested_fee_recipient(validator_index).await; + let payload_attributes = + PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None); let payload: ExecutionPayload = self .el .get_payload::>( parent_hash, - timestamp, - prev_randao, - validator_index, + &payload_attributes, forkchoice_update_params, builder_params, // FIXME: do we need to consider other forks somehow? What about withdrawals? ForkName::Merge, - #[cfg(feature = "withdrawals")] - Some(vec![]), &self.spec, ) .await @@ -188,19 +168,18 @@ impl MockExecutionLayer { slot, chain_health: ChainHealth::Healthy, }; + let suggested_fee_recipient = self.el.get_suggested_fee_recipient(validator_index).await; + let payload_attributes = + PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None); let payload_header = self .el .get_payload::>( parent_hash, - timestamp, - prev_randao, - validator_index, + &payload_attributes, forkchoice_update_params, builder_params, // FIXME: do we need to consider other forks somehow? What about withdrawals? ForkName::Merge, - #[cfg(feature = "withdrawals")] - Some(vec![]), &self.spec, ) .await diff --git a/consensus/types/src/payload.rs b/consensus/types/src/payload.rs index e7bba25ff..33fa22737 100644 --- a/consensus/types/src/payload.rs +++ b/consensus/types/src/payload.rs @@ -167,15 +167,9 @@ impl<'a, T: EthSpec> From> for ExecutionPayload { impl<'a, T: EthSpec> From> for FullPayload { fn from(full_payload_ref: FullPayloadRef<'a, T>) -> Self { match full_payload_ref { - FullPayloadRef::Merge(payload_ref) => { - FullPayload::Merge(payload_ref.clone()) - } - FullPayloadRef::Capella(payload_ref) => { - FullPayload::Capella(payload_ref.clone()) - } - FullPayloadRef::Eip4844(payload_ref) => { - FullPayload::Eip4844(payload_ref.clone()) - } + FullPayloadRef::Merge(payload_ref) => FullPayload::Merge(payload_ref.clone()), + FullPayloadRef::Capella(payload_ref) => FullPayload::Capella(payload_ref.clone()), + FullPayloadRef::Eip4844(payload_ref) => FullPayload::Eip4844(payload_ref.clone()), } } } diff --git a/consensus/types/src/withdrawal.rs b/consensus/types/src/withdrawal.rs index 36ee63965..c2529747c 100644 --- a/consensus/types/src/withdrawal.rs +++ b/consensus/types/src/withdrawal.rs @@ -10,7 +10,7 @@ use tree_hash_derive::TreeHash; /// Spec v0.12.1 #[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))] #[derive( - Debug, PartialEq, Hash, Clone, Serialize, Deserialize, Encode, Decode, TreeHash, TestRandom, + Debug, PartialEq, Eq, Hash, Clone, Serialize, Deserialize, Encode, Decode, TreeHash, TestRandom, )] pub struct Withdrawal { #[serde(with = "eth2_serde_utils::quoted_u64")] diff --git a/testing/execution_engine_integration/src/test_rig.rs b/testing/execution_engine_integration/src/test_rig.rs index 9ef96687a..1b280d148 100644 --- a/testing/execution_engine_integration/src/test_rig.rs +++ b/testing/execution_engine_integration/src/test_rig.rs @@ -4,8 +4,7 @@ use crate::execution_engine::{ use crate::transactions::transactions; use ethers_providers::Middleware; use execution_layer::{ - BuilderParams, ChainHealth, ExecutionLayer, PayloadAttributes, PayloadAttributesV1, - PayloadStatus, + BuilderParams, ChainHealth, ExecutionLayer, PayloadAttributes, PayloadStatus, }; use fork_choice::ForkchoiceUpdateParameters; use reqwest::{header::CONTENT_TYPE, Client}; @@ -279,11 +278,8 @@ impl TestRig { Slot::new(1), // Insert proposer for the next slot head_root, proposer_index, - PayloadAttributes::V1(PayloadAttributesV1 { - timestamp, - prev_randao, - suggested_fee_recipient: Address::zero(), - }), + // TODO: think about how to test different forks + PayloadAttributes::new(timestamp, prev_randao, Address::zero(), None), ) .await; @@ -316,20 +312,23 @@ impl TestRig { slot: Slot::new(0), chain_health: ChainHealth::Healthy, }; + let suggested_fee_recipient = self + .ee_a + .execution_layer + .get_suggested_fee_recipient(proposer_index) + .await; + let payload_attributes = + PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None); let valid_payload = self .ee_a .execution_layer .get_payload::>( parent_hash, - timestamp, - prev_randao, - proposer_index, + &payload_attributes, forkchoice_update_params, builder_params, // FIXME: think about how to test other forks ForkName::Merge, - #[cfg(feature = "withdrawals")] - None, &self.spec, ) .await @@ -444,20 +443,23 @@ impl TestRig { slot: Slot::new(0), chain_health: ChainHealth::Healthy, }; + let suggested_fee_recipient = self + .ee_a + .execution_layer + .get_suggested_fee_recipient(proposer_index) + .await; + let payload_attributes = + PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None); let second_payload = self .ee_a .execution_layer .get_payload::>( parent_hash, - timestamp, - prev_randao, - proposer_index, + &payload_attributes, forkchoice_update_params, builder_params, // FIXME: think about how to test other forks ForkName::Merge, - #[cfg(feature = "withdrawals")] - None, &self.spec, ) .await @@ -487,11 +489,9 @@ impl TestRig { */ let head_block_hash = valid_payload.block_hash(); let finalized_block_hash = ExecutionBlockHash::zero(); - let payload_attributes = PayloadAttributes::V1(PayloadAttributesV1 { - timestamp: second_payload.timestamp() + 1, - prev_randao: Hash256::zero(), - suggested_fee_recipient: Address::zero(), - }); + // TODO: think about how to handle different forks + let payload_attributes = + PayloadAttributes::new(timestamp, prev_randao, Address::zero(), None); let slot = Slot::new(42); let head_block_root = Hash256::repeat_byte(100); let validator_index = 0;