From 275ac11200416d436714c2ea1859f1d62e397aa2 Mon Sep 17 00:00:00 2001 From: hopinheimer Date: Mon, 2 Mar 2026 15:33:53 -0500 Subject: [PATCH] test fixes --- consensus/fork_choice/src/fork_choice.rs | 18 +++++++- consensus/fork_choice/tests/tests.rs | 42 +++++-------------- .../src/proto_array_fork_choice.rs | 2 +- 3 files changed, 29 insertions(+), 33 deletions(-) diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 112c86eee6..d4c4fa2587 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -178,6 +178,11 @@ pub enum InvalidAttestation { /// A same-slot attestation has a non-zero index, indicating a payload attestation during the /// same slot as the block. Payload attestations must only arrive in subsequent slots. PayloadAttestationDuringSameSlot { slot: Slot }, + /// A gossip payload attestation must be for the current slot. + PayloadAttestationNotCurrentSlot { + attestation_slot: Slot, + current_slot: Slot, + }, } impl From for Error { @@ -1139,7 +1144,7 @@ where fn validate_on_payload_attestation( &self, indexed_payload_attestation: &IndexedPayloadAttestation, - _is_from_block: bool, + is_from_block: bool, ) -> Result<(), InvalidAttestation> { if indexed_payload_attestation.attesting_indices.is_empty() { return Err(InvalidAttestation::EmptyAggregationBitfield); @@ -1159,6 +1164,17 @@ where }); } + // Gossip payload attestations must be for the current slot. + // https://github.com/ethereum/consensus-specs/blob/master/specs/gloas/fork-choice.md + if !is_from_block + && indexed_payload_attestation.data.slot != self.fc_store.get_current_slot() + { + return Err(InvalidAttestation::PayloadAttestationNotCurrentSlot { + attestation_slot: indexed_payload_attestation.data.slot, + current_slot: self.fc_store.get_current_slot(), + }); + } + if self.fc_store.get_current_slot() == block.slot && indexed_payload_attestation.data.payload_present { diff --git a/consensus/fork_choice/tests/tests.rs b/consensus/fork_choice/tests/tests.rs index da5405f06d..68ec79d113 100644 --- a/consensus/fork_choice/tests/tests.rs +++ b/consensus/fork_choice/tests/tests.rs @@ -1047,10 +1047,10 @@ async fn payload_attestation_for_previous_slot_is_accepted_at_next_slot() { assert!(latest_message.payload_present); } -/// Non-block payload attestations at slot S+1 for data.slot S are delayed; they are not applied -/// until a later slot. +/// Gossip payload attestations must be for the current slot. A payload attestation for slot S +/// received at slot S+1 should be rejected per the spec. #[tokio::test] -async fn non_block_payload_attestation_at_next_slot_is_delayed() { +async fn non_block_payload_attestation_for_previous_slot_is_rejected() { let test = ForkChoiceTest::new() .apply_blocks_without_new_attestations(1) .await; @@ -1062,7 +1062,6 @@ async fn non_block_payload_attestation_at_next_slot_is_delayed() { .expect("block A should exist"); let block_a_root = block_a.canonical_root(); let s_plus_1 = block_a.slot().saturating_add(1_u64); - let s_plus_2 = block_a.slot().saturating_add(2_u64); let payload_attestation = IndexedPayloadAttestation:: { attesting_indices: vec![0_u64].try_into().expect("valid attesting indices"), @@ -1080,34 +1079,15 @@ async fn non_block_payload_attestation_at_next_slot_is_delayed() { .fork_choice_write_lock() .on_payload_attestation(s_plus_1, &payload_attestation, false); assert!( - result.is_ok(), - "payload attestation should be accepted for queueing" + matches!( + result, + Err(ForkChoiceError::InvalidAttestation( + InvalidAttestation::PayloadAttestationNotCurrentSlot { .. } + )) + ), + "gossip payload attestation for previous slot should be rejected, got: {:?}", + result ); - - // Vote should not be applied yet; message remains unset. - let latest_before = chain - .canonical_head - .fork_choice_read_lock() - .latest_message(0); - assert!( - latest_before.is_none(), - "non-block payload attestation at S+1 should not apply immediately" - ); - - // Advance fork choice time to S+2, queue should now be processed. - chain - .canonical_head - .fork_choice_write_lock() - .update_time(s_plus_2) - .expect("update_time should succeed"); - - let latest_after = chain - .canonical_head - .fork_choice_read_lock() - .latest_message(0) - .expect("latest message should exist after delay"); - assert_eq!(latest_after.slot, s_plus_2); - assert!(latest_after.payload_present); } /// Specification v0.12.1: diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 37054d9552..01a8f10064 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -659,7 +659,7 @@ impl ProtoArrayForkChoice { )?; // Only re-org a single slot. This prevents cascading failures during asynchrony. - let head_slot_ok = info.head_node.slot() + 1 == current_slot; + let head_slot_ok = info.head_node.slot().saturating_add(1_u64) == current_slot; if !head_slot_ok { return Err(DoNotReOrg::HeadDistance.into()); }