diff --git a/beacon_node/beacon_chain/tests/merge.rs b/beacon_node/beacon_chain/tests/merge.rs index 19e8902a3..ea4e7cb88 100644 --- a/beacon_node/beacon_chain/tests/merge.rs +++ b/beacon_node/beacon_chain/tests/merge.rs @@ -12,17 +12,17 @@ fn verify_execution_payload_chain(chain: &[FullPayload]) { let mut prev_ep: Option> = None; for ep in chain { - assert!(*ep != FullPayload::default()); + assert!(!ep.is_default_with_empty_roots()); assert!(ep.block_hash() != ExecutionBlockHash::zero()); // Check against previous `ExecutionPayload`. if let Some(prev_ep) = prev_ep { - assert_eq!(prev_ep.block_hash(), ep.execution_payload.parent_hash); + assert_eq!(prev_ep.block_hash(), ep.execution_payload().parent_hash()); assert_eq!( - prev_ep.execution_payload.block_number + 1, - ep.execution_payload.block_number + prev_ep.execution_payload().block_number() + 1, + ep.execution_payload().block_number() ); - assert!(ep.execution_payload.timestamp > prev_ep.execution_payload.timestamp); + assert!(ep.execution_payload().timestamp() > prev_ep.execution_payload().timestamp()); } prev_ep = Some(ep.clone()); } @@ -88,7 +88,7 @@ async fn merge_with_terminal_block_hash_override() { if i == 0 { assert_eq!(execution_payload.block_hash(), genesis_pow_block_hash); } - execution_payloads.push(execution_payload); + execution_payloads.push(execution_payload.into()); } verify_execution_payload_chain(execution_payloads.as_slice()); @@ -139,9 +139,14 @@ async fn base_altair_merge_with_terminal_block_after_fork() { let merge_head = &harness.chain.head_snapshot().beacon_block; assert!(merge_head.as_merge().is_ok()); assert_eq!(merge_head.slot(), merge_fork_slot); - assert_eq!( - *merge_head.message().body().execution_payload().unwrap(), - FullPayload::default() + assert!( + merge_head + .message() + .body() + .execution_payload() + .unwrap() + .is_default_with_empty_roots(), + "Merge head is default payload" ); /* @@ -151,13 +156,14 @@ async fn base_altair_merge_with_terminal_block_after_fork() { harness.extend_slots(1).await; let one_after_merge_head = &harness.chain.head_snapshot().beacon_block; - assert_eq!( - *one_after_merge_head + assert!( + one_after_merge_head .message() .body() .execution_payload() - .unwrap(), - FullPayload::default() + .unwrap() + .is_default_with_empty_roots(), + "One after merge head is default payload" ); assert_eq!(one_after_merge_head.slot(), merge_fork_slot + 1); @@ -184,25 +190,34 @@ async fn base_altair_merge_with_terminal_block_after_fork() { harness.extend_slots(1).await; let one_after_merge_head = &harness.chain.head_snapshot().beacon_block; - assert_eq!( - *one_after_merge_head + // FIXME: why is this being tested twice? + assert!( + one_after_merge_head .message() .body() .execution_payload() - .unwrap(), - FullPayload::default() + .unwrap() + .is_default_with_empty_roots(), + "One after merge head is default payload" ); assert_eq!(one_after_merge_head.slot(), merge_fork_slot + 2); /* * Next merge block should include an exec payload. */ - for _ in 0..4 { harness.extend_slots(1).await; let block = &harness.chain.head_snapshot().beacon_block; - execution_payloads.push(block.message().body().execution_payload().unwrap().clone()); + execution_payloads.push( + block + .message() + .body() + .execution_payload() + .unwrap() + .clone() + .into(), + ); } verify_execution_payload_chain(execution_payloads.as_slice()); diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 611b20988..5c470d4dd 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -12,9 +12,9 @@ use beacon_chain::{ INVALID_JUSTIFIED_PAYLOAD_SHUTDOWN_REASON, }; use execution_layer::{ - json_structures::{JsonForkchoiceStateV1, JsonPayloadAttributesV1}, + json_structures::{JsonForkchoiceStateV1, JsonPayloadAttributes, JsonPayloadAttributesV1}, test_utils::ExecutionBlockGenerator, - ExecutionLayer, ForkchoiceState, PayloadAttributes, + ExecutionLayer, ForkchoiceState, PayloadAttributes, PayloadAttributesV1, }; use fork_choice::{ CountUnrealized, Error as ForkChoiceError, InvalidationOperation, PayloadVerificationStatus, @@ -133,7 +133,10 @@ impl InvalidPayloadRig { let attributes: JsonPayloadAttributesV1 = serde_json::from_value(payload_param_json.clone()).unwrap(); - (fork_choice_state.into(), attributes.into()) + ( + fork_choice_state.into(), + JsonPayloadAttributes::V1(attributes).into(), + ) } fn previous_payload_attributes(&self) -> PayloadAttributes { @@ -982,7 +985,7 @@ async fn payload_preparation() { .await .unwrap(); - let payload_attributes = PayloadAttributes { + let payload_attributes = PayloadAttributes::V1(PayloadAttributesV1 { timestamp: rig .harness .chain @@ -995,7 +998,7 @@ async fn payload_preparation() { .get_randao_mix(head.beacon_state.current_epoch()) .unwrap(), suggested_fee_recipient: fee_recipient, - }; + }); assert_eq!(rig.previous_payload_attributes(), payload_attributes); } @@ -1125,7 +1128,7 @@ async fn payload_preparation_before_transition_block() { let (fork_choice_state, payload_attributes) = rig.previous_forkchoice_update_params(); let latest_block_hash = rig.latest_execution_block_hash(); - assert_eq!(payload_attributes.suggested_fee_recipient, fee_recipient); + assert_eq!(payload_attributes.suggested_fee_recipient(), fee_recipient); assert_eq!(fork_choice_state.head_block_hash, latest_block_hash); } @@ -1367,18 +1370,16 @@ async fn build_optimistic_chain( .body() .execution_payload() .unwrap() - .execution_payload - == <_>::default(), + .is_default_with_empty_roots(), "the block *has not* undergone the merge transition" ); assert!( - post_transition_block + !post_transition_block .message() .body() .execution_payload() .unwrap() - .execution_payload - != <_>::default(), + .is_default_with_empty_roots(), "the block *has* undergone the merge transition" ); diff --git a/consensus/types/src/payload.rs b/consensus/types/src/payload.rs index 2507a9f0e..e7bba25ff 100644 --- a/consensus/types/src/payload.rs +++ b/consensus/types/src/payload.rs @@ -148,8 +148,8 @@ impl From> for ExecutionPayload { } impl<'a, T: EthSpec> From> for ExecutionPayload { - fn from(full_payload: FullPayloadRef<'a, T>) -> Self { - match full_payload { + fn from(full_payload_ref: FullPayloadRef<'a, T>) -> Self { + match full_payload_ref { FullPayloadRef::Merge(payload) => { ExecutionPayload::Merge(payload.execution_payload.clone()) } @@ -163,6 +163,23 @@ impl<'a, T: EthSpec> From> for ExecutionPayload { } } +// FIXME: can this be implemented as Deref or Clone somehow? +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()) + } + } + } +} + impl ExecPayload for FullPayload { fn block_type() -> BlockType { BlockType::Full