From f31a93697e0898e3b57db17de8dc10776c6bccf1 Mon Sep 17 00:00:00 2001 From: dapplion <35266934+dapplion@users.noreply.github.com> Date: Wed, 25 Mar 2026 20:08:43 -0500 Subject: [PATCH] Fix test review issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove wrong latest_message assertions from payload attestation test (on_payload_attestation writes to PTC bitfields, not vote tracker) - Fix corrupted comment: "votes.gloas_enabled() to the genesis block" → "votes to the genesis block" - Fix http_api test fallback string: "n/a" → "irrelevant" to match production code - Add issue link to #[ignore] test - Add comment explaining head_payload_status as u8 cast --- beacon_node/http_api/tests/tests.rs | 2 +- consensus/fork_choice/src/fork_choice.rs | 2 +- consensus/fork_choice/tests/tests.rs | 13 ++++--------- testing/ef_tests/src/cases/fork_choice.rs | 8 +++----- 4 files changed, 9 insertions(+), 16 deletions(-) diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index bfec7130f6..14bfb5ce92 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -3176,7 +3176,7 @@ impl ApiTester { .execution_status() .ok() .map(|status| status.to_string()) - .unwrap_or_else(|| "n/a".to_string()), + .unwrap_or_else(|| "irrelevant".to_string()), best_child: node .best_child() .and_then(|index| expected_proto_array.nodes.get(index)) diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 25716a93ce..8fcf1373e1 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1285,7 +1285,7 @@ where // 2. Ignore all attestations to the zero hash. // // (1) becomes weird once we hit finality and fork choice drops the genesis block. (2) is - // fine because votes.gloas_enabled() to the genesis block are not useful; all validators implicitly attest + // fine because votes to the genesis block are not useful; all validators implicitly attest // to genesis just by being present in the chain. if attestation.data().beacon_block_root == Hash256::zero() { return Ok(()); diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index 0a7000d476..532dd7fc4b 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -172,7 +172,8 @@ impl ForkChoiceTest { self } - /// Inspect the queued payload attestations in fork choice. + // TODO(gloas): add inspect_queued_payload_attestations when payload + // attestation queueing tests are implemented. #[allow(dead_code)] pub fn inspect_queued_payload_attestations(self, mut func: F) -> Self where @@ -971,6 +972,8 @@ async fn invalid_attestation_future_block() { /// `validate_on_attestation`, which requires a block to exist at a GLOAS-enabled slot. /// Currently the mock execution layer cannot produce Gloas blocks (no /// `signed_execution_payload_bid` support). +/// TODO(gloas): un-ignore once mock EL supports Gloas blocks. +/// https://github.com/sigp/lighthouse/issues/9025 #[ignore] #[tokio::test] async fn invalid_attestation_payload_during_same_slot() { @@ -1045,14 +1048,6 @@ async fn payload_attestation_for_previous_slot_is_accepted_at_next_slot() { "payload attestation at slot S should be accepted at S+1, got: {:?}", result ); - - let latest_message = chain - .canonical_head - .fork_choice_read_lock() - .latest_message(0) - .expect("latest message should exist"); - assert_eq!(latest_message.slot, current_slot); - assert!(latest_message.payload_present); } /// Gossip payload attestations must be for the current slot. A payload attestation for slot S diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 0c95d1c2d2..22e8453e14 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -1039,11 +1039,9 @@ impl Tester { pub fn check_head_payload_status(&self, expected_status: u8) -> Result<(), Error> { let head = self.find_head()?; - check_equal( - "head_payload_status", - head.head_payload_status() as u8, - expected_status, - ) + // PayloadStatus repr: Empty=0, Full=1, Pending=2 (matches spec constants). + let actual = head.head_payload_status() as u8; + check_equal("head_payload_status", actual, expected_status) } pub fn check_should_override_fcu(