From ffec1a1f1e3a01259557f8b1284fae2ee3918cef Mon Sep 17 00:00:00 2001 From: hopinheimer Date: Tue, 17 Mar 2026 01:49:40 -0400 Subject: [PATCH] enable ef tests @brech1 commit --- .../gloas_payload.rs | 52 ++++++++--- consensus/proto_array/src/proto_array.rs | 75 ++++++++-------- .../src/proto_array_fork_choice.rs | 28 +++--- testing/ef_tests/Makefile | 2 +- testing/ef_tests/src/cases/fork_choice.rs | 86 ++++++++++++++++++- testing/ef_tests/src/handler.rs | 23 +++-- testing/ef_tests/tests/tests.rs | 6 ++ 7 files changed, 199 insertions(+), 73 deletions(-) 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 e19fb196f2..8dcf538bd4 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 @@ -51,6 +51,12 @@ pub fn get_gloas_chain_following_test_definition() -> ForkChoiceTestDefinition { execution_payload_block_hash: Some(get_hash(4)), }); + // Mark root_1 as having received its execution payload so that + // its FULL virtual node exists in the GLOAS fork choice tree. + ops.push(Operation::ProcessExecutionPayload { + block_root: get_root(1), + }); + ops.push(Operation::AssertParentPayloadStatus { block_root: get_root(1), expected_status: PayloadStatus::Full, @@ -111,6 +117,12 @@ pub fn get_gloas_payload_probe_test_definition() -> ForkChoiceTestDefinition { execution_payload_block_hash: Some(get_hash(1)), }); + // Mark root_1 as having received its execution payload so that + // its FULL virtual node exists in the GLOAS fork choice tree. + ops.push(Operation::ProcessExecutionPayload { + block_root: get_root(1), + }); + // One Full and one Empty vote for the same head block: tie probes via runtime tiebreak, // which defaults to Empty unless timely+data-available evidence is set. ops.push(Operation::ProcessPayloadAttestation { @@ -263,6 +275,12 @@ pub fn get_gloas_find_head_vote_transition_test_definition() -> ForkChoiceTestDe execution_payload_block_hash: Some(get_hash(4)), }); + // Mark root_1 as having received its execution payload so that + // its FULL virtual node exists in the GLOAS fork choice tree. + ops.push(Operation::ProcessExecutionPayload { + block_root: get_root(1), + }); + // Equal branch weights: tiebreak FULL picks branch rooted at 3. ops.push(Operation::SetPayloadTiebreak { block_root: get_root(0), @@ -359,6 +377,12 @@ pub fn get_gloas_weight_priority_over_payload_preference_test_definition() execution_payload_block_hash: Some(get_hash(4)), }); + // Mark root_1 as having received its execution payload so that + // its FULL virtual node exists in the GLOAS fork choice tree. + ops.push(Operation::ProcessExecutionPayload { + block_root: get_root(1), + }); + // Parent prefers Full on equal branch weights (tiebreaker). ops.push(Operation::SetPayloadTiebreak { block_root: get_root(0), @@ -521,6 +545,12 @@ pub fn get_gloas_interleaved_attestations_test_definition() -> ForkChoiceTestDef execution_payload_block_hash: Some(get_hash(4)), }); + // Mark root_1 as having received its execution payload so that + // its FULL virtual node exists in the GLOAS fork choice tree. + ops.push(Operation::ProcessExecutionPayload { + block_root: get_root(1), + }); + // Step 4: Set tiebreaker to Empty on genesis → Empty branch wins. ops.push(Operation::SetPayloadTiebreak { block_root: get_root(0), @@ -619,10 +649,11 @@ pub fn get_gloas_payload_received_interleaving_test_definition() -> ForkChoiceTe expected_status: PayloadStatus::Empty, }); - // Before payload arrives: payload_received is false on genesis. + // Per spec `get_forkchoice_store`: genesis starts with payload_received=true + // (anchor block is in `payload_states`). ops.push(Operation::AssertPayloadReceived { block_root: get_root(0), - expected: false, + expected: true, }); // Give one vote to each child so they have equal weight. @@ -637,38 +668,37 @@ pub fn get_gloas_payload_received_interleaving_test_definition() -> ForkChoiceTe attestation_slot: Slot::new(1), }); - // Equal weight, no payload received on genesis → tiebreaker uses PTC votes which - // require payload_received. Without it, is_payload_timely returns false → prefers Empty. - // Block 2 (Empty) wins because it matches the Empty preference. + // Equal weight, payload_received=true on genesis → tiebreaker uses + // payload_received (not previous slot, equal payload weights) → prefers Full. + // Block 1 (Full) wins because it matches the Full preference. ops.push(Operation::FindHead { justified_checkpoint: get_checkpoint(0), finalized_checkpoint: get_checkpoint(0), justified_state_balances: vec![1, 1], - expected_head: get_root(2), + expected_head: get_root(1), current_slot: Slot::new(100), }); - // Now the execution payload for genesis arrives and is validated. + // ProcessExecutionPayload on genesis is a no-op (already received at init). ops.push(Operation::ProcessExecutionPayload { block_root: get_root(0), }); - // payload_received is now true. ops.push(Operation::AssertPayloadReceived { block_root: get_root(0), expected: true, }); // Set PTC votes on genesis as timely + data available (simulates PTC voting). + // This doesn't change the preference since genesis is not the previous slot + // (slot 0 + 1 != current_slot 100). ops.push(Operation::SetPayloadTiebreak { block_root: get_root(0), is_timely: true, is_data_available: true, }); - // Now with payload_received=true and PTC votes exceeding threshold: - // is_payload_timely=true, is_payload_data_available=true → prefers Full. - // Block 1 (Full) wins because it matches the Full preference. + // Still prefers Full via payload_received tiebreaker → Block 1 (Full) wins. ops.push(Operation::FindHead { justified_checkpoint: get_checkpoint(0), finalized_checkpoint: get_checkpoint(0), diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 09538f25eb..1b6c0c58bc 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -552,6 +552,13 @@ impl ProtoArray { PayloadStatus::Full }; + // Per spec `get_forkchoice_store`: the anchor (genesis) block has + // its payload state initialized (`payload_states = {anchor_root: ...}`). + // Without `payload_received = true` on genesis, the FULL virtual + // child doesn't exist in the spec's `get_node_children`, making all + // Full concrete children of genesis unreachable in `get_head`. + let is_genesis = parent_index.is_none(); + ProtoNode::V29(ProtoNodeV29 { slot: block.slot, root: block.root, @@ -573,7 +580,7 @@ impl ProtoArray { execution_payload_block_hash, payload_timeliness_votes: BitVector::default(), payload_data_availability_votes: BitVector::default(), - payload_received: false, + payload_received: is_genesis, }) }; @@ -1120,6 +1127,18 @@ impl ProtoArray { ); let no_change = (parent.best_child(), parent.best_descendant()); + // For V29 (GLOAS) parents, the spec's virtual tree model requires choosing + // FULL or EMPTY direction at each node BEFORE considering concrete children. + // Only children whose parent_payload_status matches the preferred direction + // are eligible for best_child. This is PRIMARY, not a tiebreaker. + let child_matches_dir = child_matches_parent_payload_preference( + parent, + child, + current_slot, + E::ptc_size(), + proposer_boost, + ); + let (new_best_child, new_best_descendant) = if let Some(best_child_index) = parent.best_child() { if best_child_index == child_index && !child_leads_to_viable_head { @@ -1143,6 +1162,14 @@ impl ProtoArray { best_finalized_checkpoint, )?; + let best_child_matches_dir = child_matches_parent_payload_preference( + parent, + best_child, + current_slot, + E::ptc_size(), + proposer_boost, + ); + if child_leads_to_viable_head && !best_child_leads_to_viable_head { // The child leads to a viable head, but the current best-child doesn't. change_to_child @@ -1150,49 +1177,27 @@ impl ProtoArray { // The best child leads to a viable head, but the child doesn't. no_change } else if child.weight() > best_child.weight() { - // Weight is the primary ordering criterion. + // Weight is the primary selector after viability. change_to_child } else if child.weight() < best_child.weight() { no_change + } else if child_matches_dir && !best_child_matches_dir { + // Equal weight: direction matching is the tiebreaker. + change_to_child + } else if !child_matches_dir && best_child_matches_dir { + no_change + } else if *child.root() >= *best_child.root() { + // Final tie-breaker: break by root hash. + change_to_child } else { - // Equal weights: for V29 parents, prefer the child whose - // parent_payload_status matches the parent's payload preference - // (full vs empty). This corresponds to the spec's - // `get_payload_status_tiebreaker` ordering in `get_head`. - let child_matches = child_matches_parent_payload_preference( - parent, - child, - current_slot, - E::ptc_size(), - proposer_boost, - ); - let best_child_matches = child_matches_parent_payload_preference( - parent, - best_child, - current_slot, - E::ptc_size(), - proposer_boost, - ); - - if child_matches && !best_child_matches { - // Child extends the preferred payload chain, best_child doesn't. - change_to_child - } else if !child_matches && best_child_matches { - // Best child extends the preferred payload chain, child doesn't. - no_change - } else if *child.root() >= *best_child.root() { - // Final tie-breaker: both match or both don't, break by root. - change_to_child - } else { - no_change - } + no_change } } } else if child_leads_to_viable_head { - // There is no current best-child and the child is viable. + // No current best-child: set if child is viable. change_to_child } else { - // There is no current best-child but the child is not viable. + // Child is not viable. no_change }; diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index ce634fbdbe..4f5fe45c22 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -1004,7 +1004,7 @@ impl ProtoArrayForkChoice { pub fn head_payload_status( &self, head_root: &Hash256, - current_slot: Slot, + _current_slot: Slot, ) -> Option { let node = self.get_proto_node(head_root)?; let v29 = node.as_v29().ok()?; @@ -1012,23 +1012,15 @@ impl ProtoArrayForkChoice { Some(PayloadStatus::Full) } else if v29.empty_payload_weight > v29.full_payload_weight { Some(PayloadStatus::Empty) - } else if node.slot() + 1 == current_slot { - // Previous slot: should_extend_payload = is_payload_timely && is_payload_data_available - if is_payload_timely( - &v29.payload_timeliness_votes, - E::ptc_size(), - v29.payload_received, - ) && is_payload_data_available( - &v29.payload_data_availability_votes, - E::ptc_size(), - v29.payload_received, - ) { - Some(PayloadStatus::Full) - } else { - Some(PayloadStatus::Empty) - } - } else if v29.payload_received { - // Not previous slot: Full wins tiebreaker (1 > 0) when payload received. + } else if is_payload_timely( + &v29.payload_timeliness_votes, + E::ptc_size(), + v29.payload_received, + ) && is_payload_data_available( + &v29.payload_data_availability_votes, + E::ptc_size(), + v29.payload_received, + ) { Some(PayloadStatus::Full) } else { Some(PayloadStatus::Empty) diff --git a/testing/ef_tests/Makefile b/testing/ef_tests/Makefile index fd8a3f6da0..48378a4c95 100644 --- a/testing/ef_tests/Makefile +++ b/testing/ef_tests/Makefile @@ -1,6 +1,6 @@ # To download/extract nightly tests, run: # CONSENSUS_SPECS_TEST_VERSION=nightly make -CONSENSUS_SPECS_TEST_VERSION ?= v1.7.0-alpha.2 +CONSENSUS_SPECS_TEST_VERSION ?= v1.7.0-alpha.3 REPO_NAME := consensus-spec-tests OUTPUT_DIR := ./$(REPO_NAME) diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 11b2df0123..054c65d016 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -30,7 +30,8 @@ use types::{ Attestation, AttestationRef, AttesterSlashing, AttesterSlashingRef, BeaconBlock, BeaconState, BlobSidecar, BlobsList, BlockImportSource, Checkpoint, DataColumnSidecar, DataColumnSidecarList, DataColumnSubnetId, ExecutionBlockHash, Hash256, IndexedAttestation, - KzgProof, ProposerPreparationData, SignedBeaconBlock, Slot, Uint256, + KzgProof, ProposerPreparationData, SignedBeaconBlock, SignedExecutionPayloadEnvelope, Slot, + Uint256, }; // When set to true, cache any states fetched from the db. @@ -72,6 +73,7 @@ pub struct Checks { proposer_boost_root: Option, get_proposer_head: Option, should_override_forkchoice_update: Option, + head_payload_status: Option, } #[derive(Debug, Clone, Deserialize)] @@ -94,7 +96,15 @@ impl From for PayloadStatusV1 { #[derive(Debug, Clone, Deserialize)] #[serde(untagged, deny_unknown_fields)] -pub enum Step { +pub enum Step< + TBlock, + TBlobs, + TColumns, + TAttestation, + TAttesterSlashing, + TPowBlock, + TExecutionPayload = String, +> { Tick { tick: u64, }, @@ -128,6 +138,10 @@ pub enum Step, valid: bool, }, + OnExecutionPayload { + execution_payload: TExecutionPayload, + valid: bool, + }, } #[derive(Debug, Clone, Deserialize)] @@ -151,6 +165,7 @@ pub struct ForkChoiceTest { Attestation, AttesterSlashing, PowBlock, + SignedExecutionPayloadEnvelope, >, >, } @@ -271,6 +286,17 @@ impl LoadCase for ForkChoiceTest { valid, }) } + Step::OnExecutionPayload { + execution_payload, + valid, + } => { + let envelope = + ssz_decode_file(&path.join(format!("{execution_payload}.ssz_snappy")))?; + Ok(Step::OnExecutionPayload { + execution_payload: envelope, + valid, + }) + } }) .collect::>()?; let anchor_state = ssz_decode_state(&path.join("anchor_state.ssz_snappy"), spec)?; @@ -359,6 +385,7 @@ impl Case for ForkChoiceTest { proposer_boost_root, get_proposer_head, should_override_forkchoice_update: should_override_fcu, + head_payload_status, } = checks.as_ref(); if let Some(expected_head) = head { @@ -405,6 +432,10 @@ impl Case for ForkChoiceTest { if let Some(expected_proposer_head) = get_proposer_head { tester.check_expected_proposer_head(*expected_proposer_head)?; } + + if let Some(expected_status) = head_payload_status { + tester.check_head_payload_status(*expected_status)?; + } } Step::MaybeValidBlockAndColumns { @@ -414,6 +445,13 @@ impl Case for ForkChoiceTest { } => { tester.process_block_and_columns(block.clone(), columns.clone(), *valid)?; } + Step::OnExecutionPayload { + execution_payload, + valid, + } => { + tester + .process_execution_payload(execution_payload.beacon_block_root(), *valid)?; + } } } @@ -931,6 +969,50 @@ impl Tester { check_equal("proposer_head", proposer_head, expected_proposer_head) } + pub fn process_execution_payload(&self, block_root: Hash256, valid: bool) -> Result<(), Error> { + let result = self + .harness + .chain + .canonical_head + .fork_choice_write_lock() + .on_execution_payload(block_root); + + if valid { + result.map_err(|e| { + Error::InternalError(format!( + "on_execution_payload for block root {} failed: {:?}", + block_root, e + )) + })?; + } else if result.is_ok() { + return Err(Error::DidntFail(format!( + "on_execution_payload for block root {} should have failed", + block_root + ))); + } + + Ok(()) + } + + 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) + } + pub fn check_should_override_fcu( &self, expected_should_override_fcu: ShouldOverrideFcu, diff --git a/testing/ef_tests/src/handler.rs b/testing/ef_tests/src/handler.rs index da3c5533b6..895b8f2656 100644 --- a/testing/ef_tests/src/handler.rs +++ b/testing/ef_tests/src/handler.rs @@ -709,15 +709,27 @@ impl Handler for ForkChoiceHandler { return false; } - // No FCU override tests prior to bellatrix. + // No FCU override tests prior to bellatrix, and removed in Gloas. if self.handler_name == "should_override_forkchoice_update" - && !fork_name.bellatrix_enabled() + && (!fork_name.bellatrix_enabled() || fork_name.gloas_enabled()) { return false; } - // Deposit tests exist only after Electra. - if self.handler_name == "deposit_with_reorg" && !fork_name.electra_enabled() { + // Deposit tests exist only for Electra and Fulu (not Gloas). + if self.handler_name == "deposit_with_reorg" + && (!fork_name.electra_enabled() || fork_name.gloas_enabled()) + { + return false; + } + + // Proposer head tests removed in Gloas. + if self.handler_name == "get_proposer_head" && fork_name.gloas_enabled() { + return false; + } + + // on_execution_payload tests exist only for Gloas. + if self.handler_name == "on_execution_payload" && !fork_name.gloas_enabled() { return false; } @@ -727,8 +739,7 @@ impl Handler for ForkChoiceHandler { } fn disabled_forks(&self) -> Vec { - // TODO(gloas): remove once we have Gloas fork choice tests - vec![ForkName::Gloas] + vec![] } } diff --git a/testing/ef_tests/tests/tests.rs b/testing/ef_tests/tests/tests.rs index 3893df2ef7..cb4abed90a 100644 --- a/testing/ef_tests/tests/tests.rs +++ b/testing/ef_tests/tests/tests.rs @@ -1032,6 +1032,12 @@ fn fork_choice_deposit_with_reorg() { // There is no mainnet variant for this test. } +#[test] +fn fork_choice_on_execution_payload() { + ForkChoiceHandler::::new("on_execution_payload").run(); + ForkChoiceHandler::::new("on_execution_payload").run(); +} + #[test] fn optimistic_sync() { OptimisticSyncHandler::::default().run();