diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index d53e588d54..86dbad0999 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -3935,7 +3935,7 @@ impl BeaconChain { let fork_choice_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_FORK_CHOICE); match fork_choice.get_head(current_slot, &self.spec) { // This block became the head, add it to the early attester cache. - Ok(new_head_root) if new_head_root == block_root => { + Ok((new_head_root, _)) if new_head_root == block_root => { if let Some(proto_block) = fork_choice.get_block(&block_root) { let new_head_is_optimistic = proto_block.execution_status.is_optimistic_or_invalid(); @@ -4906,6 +4906,7 @@ impl BeaconChain { .and_then(|execution_status| execution_status.block_hash()); let forkchoice_update_params = ForkchoiceUpdateParameters { head_root: info.parent_node.root(), + head_payload_status: canonical_forkchoice_params.head_payload_status, head_hash: parent_head_hash, justified_hash: canonical_forkchoice_params.justified_hash, finalized_hash: canonical_forkchoice_params.finalized_hash, diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 7eb92060a2..5c99f5c4c8 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -776,7 +776,7 @@ where slot_clock.now().ok_or("Unable to read slot")? }; - let initial_head_block_root = fork_choice + let (initial_head_block_root, _head_payload_status) = fork_choice .get_head(current_slot, &self.spec) .map_err(|e| format!("Unable to get fork choice head: {:?}", e))?; diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 0faddd1792..30d4a59709 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -108,6 +108,8 @@ pub struct CachedHead { /// This value may be distinct to the `self.snapshot.beacon_state.finalized_checkpoint`. /// This value should be used over the beacon state value in practically all circumstances. finalized_checkpoint: Checkpoint, + /// The payload status of the head block, as determined by fork choice. + head_payload_status: proto_array::PayloadStatus, /// The `execution_payload.block_hash` of the block at the head of the chain. Set to `None` /// before Bellatrix. head_hash: Option, @@ -227,11 +229,16 @@ impl CachedHead { pub fn forkchoice_update_parameters(&self) -> ForkchoiceUpdateParameters { ForkchoiceUpdateParameters { head_root: self.snapshot.beacon_block_root, + head_payload_status: self.head_payload_status, head_hash: self.head_hash, justified_hash: self.justified_hash, finalized_hash: self.finalized_hash, } } + + pub fn head_payload_status(&self) -> proto_array::PayloadStatus { + self.head_payload_status + } } /// Represents the "canonical head" of the beacon chain. @@ -269,6 +276,7 @@ impl CanonicalHead { snapshot, justified_checkpoint: fork_choice_view.justified_checkpoint, finalized_checkpoint: fork_choice_view.finalized_checkpoint, + head_payload_status: forkchoice_update_params.head_payload_status, head_hash: forkchoice_update_params.head_hash, justified_hash: forkchoice_update_params.justified_hash, finalized_hash: forkchoice_update_params.finalized_hash, @@ -329,6 +337,7 @@ impl CanonicalHead { snapshot: Arc::new(snapshot), justified_checkpoint: fork_choice_view.justified_checkpoint, finalized_checkpoint: fork_choice_view.finalized_checkpoint, + head_payload_status: forkchoice_update_params.head_payload_status, head_hash: forkchoice_update_params.head_hash, justified_hash: forkchoice_update_params.justified_hash, finalized_hash: forkchoice_update_params.finalized_hash, @@ -606,7 +615,7 @@ impl BeaconChain { let mut fork_choice_write_lock = self.canonical_head.fork_choice_write_lock(); // Recompute the current head via the fork choice algorithm. - fork_choice_write_lock.get_head(current_slot, &self.spec)?; + let _ = fork_choice_write_lock.get_head(current_slot, &self.spec)?; // Downgrade the fork choice write-lock to a read lock, without allowing access to any // other writers. @@ -710,6 +719,7 @@ impl BeaconChain { snapshot: Arc::new(new_snapshot), justified_checkpoint: new_view.justified_checkpoint, finalized_checkpoint: new_view.finalized_checkpoint, + head_payload_status: new_forkchoice_update_parameters.head_payload_status, head_hash: new_forkchoice_update_parameters.head_hash, justified_hash: new_forkchoice_update_parameters.justified_hash, finalized_hash: new_forkchoice_update_parameters.finalized_hash, @@ -737,6 +747,7 @@ impl BeaconChain { snapshot: old_cached_head.snapshot.clone(), justified_checkpoint: new_view.justified_checkpoint, finalized_checkpoint: new_view.finalized_checkpoint, + head_payload_status: new_forkchoice_update_parameters.head_payload_status, head_hash: new_forkchoice_update_parameters.head_hash, justified_hash: new_forkchoice_update_parameters.justified_hash, finalized_hash: new_forkchoice_update_parameters.finalized_hash, diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 7adffd3824..13672bbb63 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -1350,7 +1350,7 @@ async fn recover_from_invalid_head_by_importing_blocks() { "the fork block should become the head" ); - let manual_get_head = rig + let (manual_get_head, _) = rig .harness .chain .canonical_head diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 0e187a8f4b..e9af6df3e8 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -5430,10 +5430,12 @@ fn assert_chains_pretty_much_the_same(a: &BeaconChain, b .fork_choice_write_lock() .get_head(slot, &spec) .unwrap() + .0 == b.canonical_head .fork_choice_write_lock() .get_head(slot, &spec) - .unwrap(), + .unwrap() + .0, "fork_choice heads should be equal" ); } 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 7b6c4e8310..aa7e309f2c 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_builder.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_builder.rs @@ -12,7 +12,7 @@ use eth2::{ BeaconNodeHttpClient, CONSENSUS_VERSION_HEADER, CONTENT_TYPE_HEADER, SSZ_CONTENT_TYPE_HEADER, Timeouts, }; -use fork_choice::ForkchoiceUpdateParameters; +use fork_choice::{ForkchoiceUpdateParameters, PayloadStatus as FcPayloadStatus}; use parking_lot::RwLock; use sensitive_url::SensitiveUrl; use ssz::Encode; @@ -934,6 +934,7 @@ impl MockBuilder { finalized_hash: Some(finalized_execution_hash), justified_hash: Some(justified_execution_hash), head_root: head_block_root, + head_payload_status: FcPayloadStatus::Pending, }; let _status = self 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 91966ff65e..0aee30dff0 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 @@ -92,6 +92,7 @@ impl MockExecutionLayer { let head_block_root = Hash256::repeat_byte(42); let forkchoice_update_params = ForkchoiceUpdateParameters { head_root: head_block_root, + head_payload_status: fork_choice::PayloadStatus::Pending, head_hash: Some(parent_hash), justified_hash: None, finalized_hash: None, diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 5dc081d6ce..3c6dd9e5e0 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -4,7 +4,7 @@ use fixed_bytes::FixedBytesExtended; use logging::crit; use proto_array::{ Block as ProtoBlock, DisallowedReOrgOffsets, ExecutionStatus, JustifiedBalances, LatestMessage, - ProposerHeadError, ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold, + PayloadStatus, ProposerHeadError, ProposerHeadInfo, ProtoArrayForkChoice, ReOrgThreshold, }; use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; @@ -333,6 +333,7 @@ fn dequeue_payload_attestations( pub struct ForkchoiceUpdateParameters { /// The most recent result of running `ForkChoice::get_head`. pub head_root: Hash256, + pub head_payload_status: PayloadStatus, pub head_hash: Option, pub justified_hash: Option, pub finalized_hash: Option, @@ -470,14 +471,15 @@ where head_hash: None, justified_hash: None, finalized_hash: None, - // This will be updated during the next call to `Self::get_head`. + // These will be updated during the next call to `Self::get_head`. head_root: Hash256::zero(), + head_payload_status: PayloadStatus::Pending, }, _phantom: PhantomData, }; // Ensure that `fork_choice.forkchoice_update_parameters.head_root` is updated. - fork_choice.get_head(current_slot, spec)?; + let _ = fork_choice.get_head(current_slot, spec)?; Ok(fork_choice) } @@ -544,7 +546,7 @@ where &mut self, system_time_current_slot: Slot, spec: &ChainSpec, - ) -> Result> { + ) -> Result<(Hash256, PayloadStatus), Error> { // Provide the slot (as per the system clock) to the `fc_store` and then return its view of // the current slot. The `fc_store` will ensure that the `current_slot` is never // decreasing, a property which we must maintain. @@ -552,7 +554,7 @@ where let store = &mut self.fc_store; - let head_root = self.proto_array.find_head::( + let (head_root, head_payload_status) = self.proto_array.find_head::( *store.justified_checkpoint(), *store.finalized_checkpoint(), store.justified_balances(), @@ -576,12 +578,13 @@ where .and_then(|b| b.execution_status.block_hash()); self.forkchoice_update_parameters = ForkchoiceUpdateParameters { head_root, + head_payload_status, head_hash, justified_hash, finalized_hash, }; - Ok(head_root) + Ok((head_root, head_payload_status)) } /// Get the block to build on as proposer, taking into account proposer re-orgs. @@ -1745,6 +1748,7 @@ where finalized_hash: None, // Will be updated in the following call to `Self::get_head`. head_root: Hash256::zero(), + head_payload_status: PayloadStatus::Pending, }, _phantom: PhantomData, }; @@ -1766,7 +1770,7 @@ where .set_all_blocks_to_optimistic::(spec)?; // If the second attempt at finding a head fails, return an error since we do not // expect this scenario. - fork_choice.get_head(current_slot, spec)?; + let _ = fork_choice.get_head(current_slot, spec)?; } Ok(fork_choice) diff --git a/consensus/fork_choice/src/lib.rs b/consensus/fork_choice/src/lib.rs index 824fc2dff0..de3e709a84 100644 --- a/consensus/fork_choice/src/lib.rs +++ b/consensus/fork_choice/src/lib.rs @@ -10,5 +10,5 @@ pub use crate::fork_choice::{ }; pub use fork_choice_store::ForkChoiceStore; pub use proto_array::{ - Block as ProtoBlock, ExecutionStatus, InvalidationOperation, ProposerHeadError, + Block as ProtoBlock, ExecutionStatus, InvalidationOperation, PayloadStatus, ProposerHeadError, }; diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index a89073a7b8..de31a0905e 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -29,6 +29,8 @@ pub enum Operation { justified_state_balances: Vec, expected_head: Hash256, current_slot: Slot, + #[serde(default)] + expected_payload_status: Option, }, ProposerBoostFindHead { justified_checkpoint: Checkpoint, @@ -88,11 +90,6 @@ pub enum Operation { block_root: Hash256, expected_status: PayloadStatus, }, - AssertHeadPayloadStatus { - head_root: Hash256, - expected_status: PayloadStatus, - current_slot: Slot, - }, SetPayloadTiebreak { block_root: Hash256, is_timely: bool, @@ -159,11 +156,12 @@ impl ForkChoiceTestDefinition { justified_state_balances, expected_head, current_slot, + expected_payload_status, } => { let justified_balances = JustifiedBalances::from_effective_balances(justified_state_balances) .unwrap(); - let head = fork_choice + let (head, payload_status) = fork_choice .find_head::( justified_checkpoint, finalized_checkpoint, @@ -182,6 +180,13 @@ impl ForkChoiceTestDefinition { "Operation at index {} failed head check. Operation: {:?}", op_index, op ); + if let Some(expected_status) = expected_payload_status { + assert_eq!( + payload_status, expected_status, + "Operation at index {} failed payload status check. Operation: {:?}", + op_index, op + ); + } check_bytes_round_trip(&fork_choice); } Operation::ProposerBoostFindHead { @@ -194,7 +199,7 @@ impl ForkChoiceTestDefinition { let justified_balances = JustifiedBalances::from_effective_balances(justified_state_balances) .unwrap(); - let head = fork_choice + let (head, _payload_status) = fork_choice .find_head::( justified_checkpoint, finalized_checkpoint, @@ -455,25 +460,6 @@ impl ForkChoiceTestDefinition { op_index ); } - Operation::AssertHeadPayloadStatus { - head_root, - expected_status, - current_slot, - } => { - let actual = fork_choice - .head_payload_status::(&head_root, current_slot) - .unwrap_or_else(|| { - panic!( - "AssertHeadPayloadStatus: head root not found at op index {}", - op_index - ) - }); - assert_eq!( - actual, expected_status, - "head_payload_status mismatch at op index {}", - op_index - ); - } Operation::SetPayloadTiebreak { block_root, is_timely, diff --git a/consensus/proto_array/src/fork_choice_test_definition/execution_status.rs b/consensus/proto_array/src/fork_choice_test_definition/execution_status.rs index 59e80dbe66..8743363f9c 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/execution_status.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/execution_status.rs @@ -17,6 +17,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(0), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 2. @@ -57,6 +58,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 1 that comes off the genesis block (this is a fork compared @@ -98,6 +100,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a vote to block 1 @@ -128,6 +131,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(1), current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -171,6 +175,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -228,6 +233,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -279,6 +285,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -329,6 +336,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); // Invalidation of 3 should have removed upstream weight. @@ -383,6 +391,7 @@ pub fn get_execution_status_test_definition_01() -> ForkChoiceTestDefinition { justified_state_balances: balances, expected_head: get_root(1), current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -437,6 +446,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(0), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 2. @@ -477,6 +487,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 1 that comes off the genesis block (this is a fork compared @@ -518,6 +529,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a vote to block 1 @@ -548,6 +560,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(1), current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -591,6 +604,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -648,6 +662,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -712,6 +727,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(3), current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -762,6 +778,7 @@ pub fn get_execution_status_test_definition_02() -> ForkChoiceTestDefinition { justified_state_balances: balances, expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); // Invalidation of 3 should have removed upstream weight. @@ -818,6 +835,7 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(0), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 2. @@ -858,6 +876,7 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 1 that comes off the genesis block (this is a fork compared @@ -899,6 +918,7 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a vote to block 1 @@ -929,6 +949,7 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(1), current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { @@ -972,6 +993,7 @@ pub fn get_execution_status_test_definition_03() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(1), current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::AssertWeight { diff --git a/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs b/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs index 34a4372e27..76f9a95315 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs @@ -11,6 +11,7 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(0), current_slot: Slot::new(0), + expected_payload_status: None, }); // Build the following tree (stick? lol). @@ -65,6 +66,7 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(3), current_slot: Slot::new(0), + expected_payload_status: None, }); // Ensure that with justified epoch 1 we find 3 @@ -86,6 +88,7 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(3), current_slot: Slot::new(0), + expected_payload_status: None, }); // Ensure that with justified epoch 2 we find 3 @@ -103,6 +106,7 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances, expected_head: get_root(3), current_slot: Slot::new(0), + expected_payload_status: None, }); // END OF TESTS @@ -128,6 +132,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(0), current_slot: Slot::new(0), + expected_payload_status: None, }); // Build the following tree. @@ -275,6 +280,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(10), current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above, but with justified epoch 2. ops.push(Operation::FindHead { @@ -286,6 +292,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(10), current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above, but with justified epoch 3. // @@ -301,6 +308,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(10), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a vote to 1. @@ -341,6 +349,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(9), current_slot: Slot::new(0), + expected_payload_status: None, }); // Save as above but justified epoch 2. ops.push(Operation::FindHead { @@ -352,6 +361,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(9), current_slot: Slot::new(0), + expected_payload_status: None, }); // Save as above but justified epoch 3. // @@ -367,6 +377,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(9), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a vote to 2. @@ -407,6 +418,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(10), current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above but justified epoch 2. ops.push(Operation::FindHead { @@ -418,6 +430,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(10), current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above but justified epoch 3. // @@ -433,6 +446,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(10), current_slot: Slot::new(0), + expected_payload_status: None, }); // Ensure that if we start at 1 we find 9 (just: 0, fin: 0). @@ -457,6 +471,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(9), current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above but justified epoch 2. ops.push(Operation::FindHead { @@ -468,6 +483,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(9), current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above but justified epoch 3. // @@ -483,6 +499,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(9), current_slot: Slot::new(0), + expected_payload_status: None, }); // Ensure that if we start at 2 we find 10 (just: 0, fin: 0). @@ -504,6 +521,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(10), current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above but justified epoch 2. ops.push(Operation::FindHead { @@ -515,6 +533,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(10), current_slot: Slot::new(0), + expected_payload_status: None, }); // Same as above but justified epoch 3. // @@ -530,6 +549,7 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances, expected_head: get_root(10), current_slot: Slot::new(0), + expected_payload_status: None, }); // END OF TESTS diff --git a/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs b/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs index 8354b22e47..0fb120328c 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs @@ -78,6 +78,7 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: vec![1], expected_head: get_root(3), current_slot: Slot::new(0), + expected_payload_status: None, }); ops.push(Operation::SetPayloadTiebreak { @@ -91,6 +92,7 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: vec![1], expected_head: get_root(4), current_slot: Slot::new(0), + expected_payload_status: None, }); ForkChoiceTestDefinition { @@ -139,6 +141,8 @@ pub fn get_gloas_payload_probe_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: vec![1, 1], expected_head: get_root(1), current_slot: Slot::new(0), + // With MainnetEthSpec PTC_SIZE=512, 1 bit set out of 256 threshold → not timely → Empty. + expected_payload_status: Some(PayloadStatus::Empty), }); // PTC votes write to bitfields only, not to full/empty weight. // Weight is 0 because no CL attestations target this block. @@ -147,12 +151,6 @@ pub fn get_gloas_payload_probe_test_definition() -> ForkChoiceTestDefinition { expected_full_weight: 0, expected_empty_weight: 0, }); - // With MainnetEthSpec PTC_SIZE=512, 1 bit set out of 256 threshold → not timely → Empty. - ops.push(Operation::AssertHeadPayloadStatus { - head_root: get_root(1), - expected_status: PayloadStatus::Empty, - current_slot: Slot::new(0), - }); // Flip validator 0 to Empty; both bits now clear. ops.push(Operation::ProcessPayloadAttestation { @@ -168,17 +166,13 @@ pub fn get_gloas_payload_probe_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: vec![1, 1], expected_head: get_root(1), current_slot: Slot::new(0), + expected_payload_status: Some(PayloadStatus::Empty), }); ops.push(Operation::AssertPayloadWeights { block_root: get_root(1), expected_full_weight: 0, expected_empty_weight: 0, }); - ops.push(Operation::AssertHeadPayloadStatus { - head_root: get_root(1), - expected_status: PayloadStatus::Empty, - current_slot: Slot::new(0), - }); // Same-slot attestation to a new head candidate should be Pending (no payload bucket change). // Root 5 is an Empty child of root_1 (parent_hash doesn't match root_1's block_hash), @@ -205,17 +199,13 @@ pub fn get_gloas_payload_probe_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: vec![1, 1, 1], expected_head: get_root(5), current_slot: Slot::new(0), + expected_payload_status: Some(PayloadStatus::Empty), }); ops.push(Operation::AssertPayloadWeights { block_root: get_root(5), expected_full_weight: 0, expected_empty_weight: 0, }); - ops.push(Operation::AssertHeadPayloadStatus { - head_root: get_root(5), - expected_status: PayloadStatus::Empty, - current_slot: Slot::new(0), - }); ForkChoiceTestDefinition { finalized_block_slot: Slot::new(0), @@ -289,6 +279,7 @@ pub fn get_gloas_find_head_vote_transition_test_definition() -> ForkChoiceTestDe justified_state_balances: vec![1], expected_head: get_root(3), current_slot: Slot::new(0), + expected_payload_status: None, }); // CL attestation to Empty branch (root 4) from validator 0 → head flips to 4. @@ -303,6 +294,7 @@ pub fn get_gloas_find_head_vote_transition_test_definition() -> ForkChoiceTestDe justified_state_balances: vec![1], expected_head: get_root(4), current_slot: Slot::new(0), + expected_payload_status: None, }); // CL attestation back to Full branch (root 3) → head returns to 3. @@ -317,6 +309,7 @@ pub fn get_gloas_find_head_vote_transition_test_definition() -> ForkChoiceTestDe justified_state_balances: vec![1], expected_head: get_root(3), current_slot: Slot::new(0), + expected_payload_status: None, }); ForkChoiceTestDefinition { @@ -391,6 +384,7 @@ pub fn get_gloas_weight_priority_over_payload_preference_test_definition() justified_state_balances: vec![1], expected_head: get_root(3), current_slot: Slot::new(0), + expected_payload_status: None, }); // Two CL attestations to the Empty branch make it strictly heavier, @@ -411,6 +405,7 @@ pub fn get_gloas_weight_priority_over_payload_preference_test_definition() justified_state_balances: vec![1, 1], expected_head: get_root(4), current_slot: Slot::new(0), + expected_payload_status: None, }); ForkChoiceTestDefinition { @@ -559,6 +554,7 @@ pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDef justified_state_balances: vec![1, 1], expected_head: get_root(4), current_slot: Slot::new(1), + expected_payload_status: None, }); // Step 5: Flip tiebreaker to Full → Full branch wins. @@ -573,6 +569,7 @@ pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDef justified_state_balances: vec![1, 1], expected_head: get_root(3), current_slot: Slot::new(100), + expected_payload_status: None, }); // Step 6: Add extra CL weight to Empty branch → overrides Full tiebreaker. @@ -587,6 +584,7 @@ pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDef justified_state_balances: vec![1, 1, 1], expected_head: get_root(4), current_slot: Slot::new(100), + expected_payload_status: None, }); ForkChoiceTestDefinition { @@ -673,6 +671,7 @@ pub fn get_gloas_payload_received_interleaving_test_definition() -> ForkChoiceTe justified_state_balances: vec![1, 1], expected_head: get_root(1), current_slot: Slot::new(100), + expected_payload_status: None, }); // ProcessExecutionPayload on genesis is a no-op (already received at init). @@ -701,6 +700,7 @@ pub fn get_gloas_payload_received_interleaving_test_definition() -> ForkChoiceTe justified_state_balances: vec![1, 1], expected_head: get_root(1), current_slot: Slot::new(100), + expected_payload_status: None, }); ForkChoiceTestDefinition { diff --git a/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs b/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs index 71d4c035ae..7b5ee31c64 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/no_votes.rs @@ -19,6 +19,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: Hash256::zero(), current_slot: Slot::new(0), + expected_payload_status: None, }, // Add block 2 // @@ -57,6 +58,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }, // Add block 1 // @@ -95,6 +97,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }, // Add block 3 // @@ -137,6 +140,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }, // Add block 4 // @@ -179,6 +183,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(4), current_slot: Slot::new(0), + expected_payload_status: None, }, // Add block 5 with a justified epoch of 2 // @@ -222,6 +227,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(5), current_slot: Slot::new(0), + expected_payload_status: None, }, // Ensure there is no error when starting from a block that has the // wrong justified epoch. @@ -249,6 +255,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(5), current_slot: Slot::new(0), + expected_payload_status: None, }, // Set the justified epoch to 2 and the start block to 5 and ensure 5 is the head. // @@ -268,6 +275,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(5), current_slot: Slot::new(0), + expected_payload_status: None, }, // Add block 6 // @@ -312,6 +320,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances, expected_head: get_root(6), current_slot: Slot::new(0), + expected_payload_status: None, }, ]; diff --git a/consensus/proto_array/src/fork_choice_test_definition/votes.rs b/consensus/proto_array/src/fork_choice_test_definition/votes.rs index 3ba21db48a..cdd9553127 100644 --- a/consensus/proto_array/src/fork_choice_test_definition/votes.rs +++ b/consensus/proto_array/src/fork_choice_test_definition/votes.rs @@ -17,6 +17,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(0), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 2. @@ -57,6 +58,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a block with a hash of 1 that comes off the genesis block (this is a fork compared @@ -98,6 +100,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a vote to block 1 @@ -128,6 +131,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(1), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add a vote to block 2 @@ -158,6 +162,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add block 3. @@ -202,6 +207,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); // Move validator #0 vote from 1 to 3 @@ -236,6 +242,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(2), current_slot: Slot::new(0), + expected_payload_status: None, }); // Move validator #1 vote from 2 to 1 (this is an equivocation, but fork choice doesn't @@ -271,6 +278,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(3), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add block 4. @@ -319,6 +327,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(4), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add block 5, which has a justified epoch of 2. @@ -371,6 +380,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(4), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add block 6, which has a justified epoch of 0. @@ -516,6 +526,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(6), current_slot: Slot::new(0), + expected_payload_status: None, }); // Change fork-choice justified epoch to 1, and the start block to 5 and ensure that 9 is @@ -550,6 +561,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(9), current_slot: Slot::new(0), + expected_payload_status: None, }); // Change fork-choice justified epoch to 1, and the start block to 5 and ensure that 9 is @@ -629,6 +641,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(9), current_slot: Slot::new(0), + expected_payload_status: None, }); // Introduce 2 more validators into the system @@ -691,6 +704,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(10), current_slot: Slot::new(0), + expected_payload_status: None, }); // Set the balances of the last two validators to zero @@ -717,6 +731,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(9), current_slot: Slot::new(0), + expected_payload_status: None, }); // Set the balances of the last two validators back to 1 @@ -743,6 +758,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(10), current_slot: Slot::new(0), + expected_payload_status: None, }); // Remove the last two validators @@ -770,6 +786,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(9), current_slot: Slot::new(0), + expected_payload_status: None, }); // Ensure that pruning below the prune threshold does not prune. @@ -792,6 +809,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(9), current_slot: Slot::new(0), + expected_payload_status: None, }); // Ensure that pruning above the prune threshold does prune. @@ -831,6 +849,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances.clone(), expected_head: get_root(9), current_slot: Slot::new(0), + expected_payload_status: None, }); // Add block 11 @@ -883,6 +902,7 @@ pub fn get_votes_test_definition() -> ForkChoiceTestDefinition { justified_state_balances: balances, expected_head: get_root(11), current_slot: Slot::new(0), + expected_payload_status: None, }); ForkChoiceTestDefinition { diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 3cfd7db265..19be43511f 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -59,7 +59,7 @@ pub enum ExecutionStatus { } /// Represents the status of an execution payload post-Gloas. -#[derive(Clone, Copy, Debug, PartialEq, Encode, Decode, Serialize, Deserialize)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Encode, Decode, Serialize, Deserialize)] #[ssz(enum_behaviour = "tag")] #[repr(u8)] pub enum PayloadStatus { @@ -616,7 +616,7 @@ impl ProtoArrayForkChoice { equivocating_indices: &BTreeSet, current_slot: Slot, spec: &ChainSpec, - ) -> Result { + ) -> Result<(Hash256, PayloadStatus), String> { let old_balances = &mut self.balances; let new_balances = justified_state_balances; let node_slots = self @@ -660,7 +660,6 @@ impl ProtoArrayForkChoice { new_balances, spec, ) - .map(|(root, _payload_status)| root) .map_err(|e| format!("find_head failed: {:?}", e)) } @@ -1007,49 +1006,6 @@ impl ProtoArrayForkChoice { /// Returns the payload status of the head node based on accumulated weights and tiebreaker. /// - /// Returns `Full` if `full_payload_weight > empty_payload_weight`. - /// Returns `Empty` if `empty_payload_weight > full_payload_weight`. - /// On ties: - /// - Previous slot (`slot + 1 == current_slot`): prefer Full only when timely and - /// data available (per `should_extend_payload`). - /// - Otherwise: prefer Full when payload has been received. - /// - /// Returns `None` for V17 nodes. - // TODO(gloas): delete - pub fn head_payload_status( - &self, - head_root: &Hash256, - current_slot: Slot, - ) -> Option { - let node = self.get_proto_node(head_root)?; - let v29 = node.as_v29().ok()?; - - // Replicate the spec's virtual tree walk tiebreaker at the head node. - let use_tiebreaker_only = node.slot() + 1 == current_slot; - - if !use_tiebreaker_only { - // Compare weights, then fall back to tiebreaker. - if v29.full_payload_weight > v29.empty_payload_weight { - return Some(PayloadStatus::Full); - } else if v29.empty_payload_weight > v29.full_payload_weight { - return Some(PayloadStatus::Empty); - } - // Equal weights: prefer FULL if payload received. - if v29.payload_received { - Some(PayloadStatus::Full) - } else { - Some(PayloadStatus::Empty) - } - } else { - // Previous slot: should_extend_payload tiebreaker. - if node.is_payload_timely::() && node.is_payload_data_available::() { - Some(PayloadStatus::Full) - } else { - Some(PayloadStatus::Empty) - } - } - } - /// See `ProtoArray` documentation. pub fn is_descendant(&self, ancestor_root: Hash256, descendant_root: Hash256) -> bool { self.proto_array diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index a1c93d65bb..9f0e6de2ea 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -975,7 +975,7 @@ impl Tester { ) -> Result<(), Error> { let mut fc = self.harness.chain.canonical_head.fork_choice_write_lock(); let slot = self.harness.chain.slot().unwrap(); - let canonical_head = fc.get_head(slot, &self.harness.spec).unwrap(); + let (canonical_head, _) = fc.get_head(slot, &self.harness.spec).unwrap(); let proposer_head_result = fc.get_proposer_head( slot, canonical_head, @@ -1020,21 +1020,11 @@ impl Tester { pub fn check_head_payload_status(&self, expected_status: u8) -> Result<(), Error> { let head = self.find_head()?; - let head_root = head.head_block_root(); - let current_slot = self.harness.chain.slot().map_err(|e| { - Error::InternalError(format!("reading current slot failed with {:?}", e)) - })?; - let fc = self.harness.chain.canonical_head.fork_choice_read_lock(); - let actual_status = fc - .proto_array() - .head_payload_status::(&head_root, current_slot) - .ok_or_else(|| { - Error::InternalError(format!( - "head_payload_status not found for head root {}", - head_root - )) - })?; - check_equal("head_payload_status", actual_status as u8, expected_status) + check_equal( + "head_payload_status", + head.head_payload_status() as u8, + expected_status, + ) } pub fn check_should_override_fcu( diff --git a/testing/execution_engine_integration/src/test_rig.rs b/testing/execution_engine_integration/src/test_rig.rs index 6bf4a1aa52..2c20a41489 100644 --- a/testing/execution_engine_integration/src/test_rig.rs +++ b/testing/execution_engine_integration/src/test_rig.rs @@ -13,7 +13,7 @@ use execution_layer::{ LATEST_TAG, PayloadAttributes, PayloadParameters, PayloadStatus, }; use fixed_bytes::FixedBytesExtended; -use fork_choice::ForkchoiceUpdateParameters; +use fork_choice::{ForkchoiceUpdateParameters, PayloadStatus as FcPayloadStatus}; use reqwest::{Client, header::CONTENT_TYPE}; use sensitive_url::SensitiveUrl; use serde_json::{Value, json}; @@ -294,6 +294,7 @@ impl TestRig { let finalized_block_hash = ExecutionBlockHash::zero(); let forkchoice_update_params = ForkchoiceUpdateParameters { head_root, + head_payload_status: FcPayloadStatus::Pending, head_hash: Some(parent_hash), justified_hash: Some(justified_block_hash), finalized_hash: Some(finalized_block_hash),