Refactored Execution Layer & Fixed Some Tests

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<T: BeaconChainTypes> BeaconChain<T> {
         .map(|withdrawals_opt| withdrawals_opt.map(|w| w.into()))
         .map_err(Error::PrepareProposerFailed)?;
 
-        let payload_attributes = PayloadAttributes::V2(PayloadAttributesV2 {
+        let payload_attributes = PayloadAttributes::new(
-            timestamp: self
+            self.slot_clock
-                .slot_clock
                 .start_of(prepare_slot)
                 .ok_or(Error::InvalidSlot(prepare_slot))?
                 .as_secs(),
-            prev_randao: head_random,
+            head_random,
-            suggested_fee_recipient: execution_layer
+            execution_layer
                 .get_suggested_fee_recipient(proposer as u64)
                 .await,
             #[cfg(feature = "withdrawals")]
             withdrawals,
             #[cfg(not(feature = "withdrawals"))]
-            withdrawals: None,
+            None,
-        });
+        );
 
         debug!(
             self.log,

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::<Payload>(
             parent_hash,
-            timestamp,
+            &payload_attributes,
-            random,
-            proposer_index,
             forkchoice_update_params,
             builder_params,
             fork,
-            #[cfg(feature = "withdrawals")]
-            withdrawals,
             &chain.spec,
         )
         .await

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 {
+    let payload_attributes = PayloadAttributes::new(
-        timestamp: rig
+        rig.harness
-            .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);
 }
 

beacon_node/execution_layer/src/engine_api.rs

@@ -243,11 +243,11 @@ impl<T: EthSpec> From<ExecutionPayload<T>> for ExecutionBlockWithTransactions<T>
 
 #[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<Vec<Withdrawal>>,
+    ) -> 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<Self, Error> {
         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(),
                     ));
                 }

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 payload_attributes: PayloadAttributes,
-    pub prev_randao: Hash256,
-    pub suggested_fee_recipient: Address,
 }
 
 #[derive(Debug)]
@@ -142,20 +141,13 @@ impl Engine {
 
     pub async fn get_payload_id(
         &self,
-        head_block_hash: ExecutionBlockHash,
+        head_block_hash: &ExecutionBlockHash,
-        timestamp: u64,
+        payload_attributes: &PayloadAttributes,
-        prev_randao: Hash256,
-        suggested_fee_recipient: Address,
     ) -> Option<PayloadId> {
         self.payload_id_cache
             .lock()
             .await
-            .get(&PayloadIdCacheKey {
+            .get(&PayloadIdCacheKey::new(head_block_hash, payload_attributes))
-                head_block_hash,
-                timestamp,
-                prev_randao,
-                suggested_fee_recipient,
-            })
             .cloned()
     }
 
@@ -171,8 +163,8 @@ impl Engine {
             .await?;
 
         if let Some(payload_id) = response.payload_id {
-            if let Some(key) =
+            if let Some(key) = payload_attributes
-                payload_attributes.map(|pa| PayloadIdCacheKey::new(&forkchoice_state, &pa))
+                .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,
+            head_block_hash: head_block_hash.clone(),
-            timestamp: attributes.timestamp(),
+            payload_attributes: attributes.clone(),
-            prev_randao: attributes.prev_randao(),
-            suggested_fee_recipient: attributes.suggested_fee_recipient(),
         }
     }
 }

beacon_node/execution_layer/src/lib.rs

@@ -606,21 +606,15 @@ impl<T: EthSpec> ExecutionLayer<T> {
     ///
     /// 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<Payload: AbstractExecPayload<T>>(
         &self,
         parent_hash: ExecutionBlockHash,
-        timestamp: u64,
+        payload_attributes: &PayloadAttributes,
-        prev_randao: Hash256,
-        proposer_index: u64,
         forkchoice_update_params: ForkchoiceUpdateParameters,
         builder_params: BuilderParams,
         current_fork: ForkName,
-        #[cfg(feature = "withdrawals")] withdrawals: Option<Vec<Withdrawal>>,
         spec: &ChainSpec,
     ) -> Result<BlockProposalContents<T, Payload>, 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<T: EthSpec> ExecutionLayer<T> {
                 );
                 self.get_blinded_payload(
                     parent_hash,
-                    timestamp,
+                    payload_attributes,
-                    prev_randao,
-                    suggested_fee_recipient,
                     forkchoice_update_params,
                     builder_params,
                     current_fork,
-                    #[cfg(feature = "withdrawals")]
-                    withdrawals,
                     spec,
                 )
                 .await
@@ -648,30 +638,22 @@ impl<T: EthSpec> ExecutionLayer<T> {
                 );
                 self.get_full_payload(
                     parent_hash,
-                    timestamp,
+                    payload_attributes,
-                    prev_randao,
-                    suggested_fee_recipient,
                     forkchoice_update_params,
                     current_fork,
-                    #[cfg(feature = "withdrawals")]
-                    withdrawals,
                 )
                 .await
             }
         }
     }
 
-    #[allow(clippy::too_many_arguments)]
     async fn get_blinded_payload<Payload: AbstractExecPayload<T>>(
         &self,
         parent_hash: ExecutionBlockHash,
-        timestamp: u64,
+        payload_attributes: &PayloadAttributes,
-        prev_randao: Hash256,
-        suggested_fee_recipient: Address,
         forkchoice_update_params: ForkchoiceUpdateParameters,
         builder_params: BuilderParams,
         current_fork: ForkName,
-        #[cfg(feature = "withdrawals")] withdrawals: Option<Vec<Withdrawal>>,
         spec: &ChainSpec,
     ) -> Result<BlockProposalContents<T, Payload>, Error> {
         if let Some(builder) = self.builder() {
@@ -691,13 +673,9 @@ impl<T: EthSpec> ExecutionLayer<T> {
                         builder.get_builder_header::<T, Payload>(slot, parent_hash, &pubkey),
                         self.get_full_payload_caching(
                             parent_hash,
-                            timestamp,
+                            payload_attributes,
-                            prev_randao,
-                            suggested_fee_recipient,
                             forkchoice_update_params,
                             current_fork,
-                            #[cfg(feature = "withdrawals")]
-                            withdrawals,
                         )
                     );
 
@@ -746,7 +724,7 @@ impl<T: EthSpec> ExecutionLayer<T> {
                                     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<T: EthSpec> ExecutionLayer<T> {
                                     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<T: EthSpec> ExecutionLayer<T> {
         }
         self.get_full_payload_caching(
             parent_hash,
-            timestamp,
+            payload_attributes,
-            prev_randao,
-            suggested_fee_recipient,
             forkchoice_update_params,
             current_fork,
-            #[cfg(feature = "withdrawals")]
-            withdrawals,
         )
         .await
     }
@@ -838,22 +812,15 @@ impl<T: EthSpec> ExecutionLayer<T> {
     async fn get_full_payload<Payload: AbstractExecPayload<T>>(
         &self,
         parent_hash: ExecutionBlockHash,
-        timestamp: u64,
+        payload_attributes: &PayloadAttributes,
-        prev_randao: Hash256,
-        suggested_fee_recipient: Address,
         forkchoice_update_params: ForkchoiceUpdateParameters,
         current_fork: ForkName,
-        #[cfg(feature = "withdrawals")] withdrawals: Option<Vec<Withdrawal>>,
     ) -> Result<BlockProposalContents<T, Payload>, Error> {
         self.get_full_payload_with(
             parent_hash,
-            timestamp,
+            payload_attributes,
-            prev_randao,
-            suggested_fee_recipient,
             forkchoice_update_params,
             current_fork,
-            #[cfg(feature = "withdrawals")]
-            withdrawals,
             noop,
         )
         .await
@@ -863,22 +830,15 @@ impl<T: EthSpec> ExecutionLayer<T> {
     async fn get_full_payload_caching<Payload: AbstractExecPayload<T>>(
         &self,
         parent_hash: ExecutionBlockHash,
-        timestamp: u64,
+        payload_attributes: &PayloadAttributes,
-        prev_randao: Hash256,
-        suggested_fee_recipient: Address,
         forkchoice_update_params: ForkchoiceUpdateParameters,
         current_fork: ForkName,
-        #[cfg(feature = "withdrawals")] withdrawals: Option<Vec<Withdrawal>>,
     ) -> Result<BlockProposalContents<T, Payload>, Error> {
         self.get_full_payload_with(
             parent_hash,
-            timestamp,
+            payload_attributes,
-            prev_randao,
-            suggested_fee_recipient,
             forkchoice_update_params,
             current_fork,
-            #[cfg(feature = "withdrawals")]
-            withdrawals,
             Self::cache_payload,
         )
         .await
@@ -887,20 +847,15 @@ impl<T: EthSpec> ExecutionLayer<T> {
     async fn get_full_payload_with<Payload: AbstractExecPayload<T>>(
         &self,
         parent_hash: ExecutionBlockHash,
-        timestamp: u64,
+        payload_attributes: &PayloadAttributes,
-        prev_randao: Hash256,
-        suggested_fee_recipient: Address,
         forkchoice_update_params: ForkchoiceUpdateParameters,
         current_fork: ForkName,
-        #[cfg(feature = "withdrawals")] withdrawals: Option<Vec<Withdrawal>>,
         f: fn(&ExecutionLayer<T>, &ExecutionPayload<T>) -> Option<ExecutionPayload<T>>,
     ) -> Result<BlockProposalContents<T, Payload>, 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<T: EthSpec> ExecutionLayer<T> {
                             .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<T: EthSpec> ExecutionLayer<T> {
                             debug!(
                                 self.log(),
                                 "Issuing engine_getBlobsBundle";
-                                "suggested_fee_recipient" => ?suggested_fee_recipient,
+                                "suggested_fee_recipient" => ?payload_attributes.suggested_fee_recipient(),
-                                "prev_randao" => ?prev_randao,
+                                "prev_randao" => ?payload_attributes.prev_randao(),
-                                "timestamp" => timestamp,
+                                "timestamp" => payload_attributes.timestamp(),
                                 "parent_hash" => ?parent_hash,
                             );
                             Some(engine.api.get_blobs_bundle_v1::<T>(payload_id).await)
@@ -982,16 +926,16 @@ impl<T: EthSpec> ExecutionLayer<T> {
                     debug!(
                         self.log(),
                         "Issuing engine_getPayload";
-                        "suggested_fee_recipient" => ?suggested_fee_recipient,
+                        "suggested_fee_recipient" => ?payload_attributes.suggested_fee_recipient(),
-                        "prev_randao" => ?prev_randao,
+                        "prev_randao" => ?payload_attributes.prev_randao(),
-                        "timestamp" => timestamp,
+                        "timestamp" => payload_attributes.timestamp(),
                         "parent_hash" => ?parent_hash,
                     );
                     engine.api.get_payload::<T>(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<T: EthSpec> ExecutionLayer<T> {
                             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() {

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<E: EthSpec> mev_build_rs::BlindedBlockProvider for MockBuilder<E> {
             .map_err(convert_err)?;
 
         // FIXME: think about proper fork here
-        let payload_attributes = PayloadAttributes::V1(PayloadAttributesV1 {
+        let payload_attributes =
-            timestamp,
+            PayloadAttributes::new(timestamp, *prev_randao, fee_recipient, None);
-            prev_randao: *prev_randao,
-            suggested_fee_recipient: fee_recipient,
-        });
 
         self.el
             .insert_proposer(slot, head_block_root, val_index, payload_attributes)
@@ -306,18 +303,17 @@ impl<E: EthSpec> mev_build_rs::BlindedBlockProvider for MockBuilder<E> {
             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::<BlindedPayload<E>>(
                 head_execution_hash,
-                timestamp,
+                &payload_attributes,
-                *prev_randao,
-                fee_recipient,
                 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)?

beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs

@@ -98,35 +98,16 @@ impl<T: EthSpec> MockExecutionLayer<T> {
             justified_hash: None,
             finalized_hash: None,
         };
-
+        let payload_attributes = PayloadAttributes::new(
-        // FIXME: this is just best guess for how to deal with forks here..
+            timestamp,
-        let payload_attributes = match &latest_execution_block {
+            prev_randao,
-            &Block::PoS(ref pos_block) => match pos_block {
+            Address::repeat_byte(42),
-                &ExecutionPayload::Merge(_) => PayloadAttributes::V1(PayloadAttributesV1 {
+            // FIXME: think about how to handle different forks / withdrawals here..
-                    timestamp,
+            #[cfg(feature = "withdrawals")]
-                    prev_randao,
+            Some(vec![]),
-                    suggested_fee_recipient: Address::repeat_byte(42),
+            #[cfg(not(feature = "withdrawals"))]
-                }),
+            None,
-                &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),
-            }),
-        };
 
         // Insert a proposer to ensure the fork choice updated command works.
         let slot = Slot::new(0);
@@ -152,19 +133,18 @@ impl<T: EthSpec> MockExecutionLayer<T> {
             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<T> = self
             .el
             .get_payload::<FullPayload<T>>(
                 parent_hash,
-                timestamp,
+                &payload_attributes,
-                prev_randao,
-                validator_index,
                 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<T: EthSpec> MockExecutionLayer<T> {
             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::<BlindedPayload<T>>(
                 parent_hash,
-                timestamp,
+                &payload_attributes,
-                prev_randao,
-                validator_index,
                 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

consensus/types/src/payload.rs

@@ -167,15 +167,9 @@ impl<'a, T: EthSpec> From<FullPayloadRef<'a, T>> for ExecutionPayload<T> {
 impl<'a, T: EthSpec> From<FullPayloadRef<'a, T>> for FullPayload<T> {
     fn from(full_payload_ref: FullPayloadRef<'a, T>) -> Self {
         match full_payload_ref {
-            FullPayloadRef::Merge(payload_ref) => {
+            FullPayloadRef::Merge(payload_ref) => FullPayload::Merge(payload_ref.clone()),
-                FullPayload::Merge(payload_ref.clone())
+            FullPayloadRef::Capella(payload_ref) => FullPayload::Capella(payload_ref.clone()),
-            }
+            FullPayloadRef::Eip4844(payload_ref) => FullPayload::Eip4844(payload_ref.clone()),
-            FullPayloadRef::Capella(payload_ref) => {
-                FullPayload::Capella(payload_ref.clone())
-            }
-            FullPayloadRef::Eip4844(payload_ref) => {
-                FullPayload::Eip4844(payload_ref.clone())
-            }
         }
     }
 }

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")]

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,
+    BuilderParams, ChainHealth, ExecutionLayer, PayloadAttributes, PayloadStatus,
-    PayloadStatus,
 };
 use fork_choice::ForkchoiceUpdateParameters;
 use reqwest::{header::CONTENT_TYPE, Client};
@@ -279,11 +278,8 @@ impl<E: GenericExecutionEngine> TestRig<E> {
                 Slot::new(1), // Insert proposer for the next slot
                 head_root,
                 proposer_index,
-                PayloadAttributes::V1(PayloadAttributesV1 {
+                // TODO: think about how to test different forks
-                    timestamp,
+                PayloadAttributes::new(timestamp, prev_randao, Address::zero(), None),
-                    prev_randao,
-                    suggested_fee_recipient: Address::zero(),
-                }),
             )
             .await;
 
@@ -316,20 +312,23 @@ impl<E: GenericExecutionEngine> TestRig<E> {
             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::<FullPayload<MainnetEthSpec>>(
                 parent_hash,
-                timestamp,
+                &payload_attributes,
-                prev_randao,
-                proposer_index,
                 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<E: GenericExecutionEngine> TestRig<E> {
             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::<FullPayload<MainnetEthSpec>>(
                 parent_hash,
-                timestamp,
+                &payload_attributes,
-                prev_randao,
-                proposer_index,
                 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<E: GenericExecutionEngine> TestRig<E> {
          */
         let head_block_hash = valid_payload.block_hash();
         let finalized_block_hash = ExecutionBlockHash::zero();
-        let payload_attributes = PayloadAttributes::V1(PayloadAttributesV1 {
+        // TODO: think about how to handle different forks
-            timestamp: second_payload.timestamp() + 1,
+        let payload_attributes =
-            prev_randao: Hash256::zero(),
+            PayloadAttributes::new(timestamp, prev_randao, Address::zero(), None);
-            suggested_fee_recipient: Address::zero(),
-        });
         let slot = Slot::new(42);
         let head_block_root = Hash256::repeat_byte(100);
         let validator_index = 0;