diff --git a/beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs b/beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs index 3e9f9e4b60..f8a1143b2e 100644 --- a/beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs +++ b/beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs @@ -70,13 +70,21 @@ impl VerifiedPayloadAttestationMessage { // 2. Blocks we've seen that are invalid (REJECT). // Presently both cases return IGNORE. let beacon_block_root = payload_attestation_message.data.beacon_block_root; - if ctx + let block = ctx .canonical_head .fork_choice_read_lock() .get_block(&beacon_block_root) - .is_none() - { - return Err(Error::UnknownHeadBlock { beacon_block_root }); + .ok_or(Error::UnknownHeadBlock { beacon_block_root })?; + + // [IGNORE] The block referenced by `data.beacon_block_root` is at slot `data.slot`, i.e. + // the block has `block.slot == data.slot`. A PTC member assigned to an empty slot must not + // attest, so ignore messages that reference an earlier block. + if block.slot != slot { + return Err(Error::BlockNotAtSlot { + beacon_block_root, + block_slot: block.slot, + data_slot: slot, + }); } let message_epoch = slot.epoch(T::EthSpec::slots_per_epoch()); diff --git a/beacon_node/beacon_chain/src/payload_attestation_verification/mod.rs b/beacon_node/beacon_chain/src/payload_attestation_verification/mod.rs index 89ae1bbbdd..bc7f4c1d63 100644 --- a/beacon_node/beacon_chain/src/payload_attestation_verification/mod.rs +++ b/beacon_node/beacon_chain/src/payload_attestation_verification/mod.rs @@ -60,6 +60,19 @@ pub enum Error { /// The attestation points to a block we have not yet imported. It's unclear if the /// attestation is valid or not. UnknownHeadBlock { beacon_block_root: Hash256 }, + /// The block referenced by `data.beacon_block_root` is not at slot `data.slot`, i.e. the + /// PTC member's assigned slot was empty (the message references the last canonical block at + /// an earlier slot). + /// + /// ## Peer scoring + /// + /// PTC members should not attest for empty slots, so we + /// ignore the message. + BlockNotAtSlot { + beacon_block_root: Hash256, + block_slot: Slot, + data_slot: Slot, + }, /// The validator index is not a member of the PTC for this slot. /// /// ## Peer scoring diff --git a/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs b/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs index d4b82c41fc..a56d98a67a 100644 --- a/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs +++ b/beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs @@ -167,6 +167,25 @@ fn unknown_head_block() { ); } +#[test] +fn block_not_at_slot() { + if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { + return; + } + let ctx = TestContext::new(); + let gossip = ctx.gossip_ctx(); + + // The genesis block is at slot 0, but the message claims slot 1. A PTC member assigned to an + // empty slot must not attest, so this must be ignored (per consensus-specs #5281). + let msg = make_payload_attestation(Slot::new(1), 0, ctx.genesis_block_root); + let result = VerifiedPayloadAttestationMessage::new(msg, &gossip); + assert!( + matches!(result, Err(PayloadAttestationError::BlockNotAtSlot { .. })), + "expected BlockNotAtSlot, got: {:?}", + result + ); +} + #[test] fn not_in_ptc() { if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) { @@ -174,7 +193,9 @@ fn not_in_ptc() { } let ctx = TestContext::new(); let gossip = ctx.gossip_ctx(); - let slot = Slot::new(1); + // The genesis block is at slot 0, so attest at slot 0 to satisfy the `block.slot == data.slot` + // gossip rule. Slot 0 is still within the propagation range at the current slot. + let slot = Slot::new(0); let ptc_members = ctx.ptc_members(slot); let non_ptc_validator = (0..NUM_VALIDATORS as u64) @@ -196,7 +217,9 @@ fn invalid_signature() { } let ctx = TestContext::new(); let gossip = ctx.gossip_ctx(); - let slot = Slot::new(1); + // The genesis block is at slot 0, so attest at slot 0 to satisfy the `block.slot == data.slot` + // gossip rule. Slot 0 is still within the propagation range at the current slot. + let slot = Slot::new(0); let ptc_members = ctx.ptc_members(slot); let validator_index = ptc_members[0] as u64; @@ -216,7 +239,9 @@ fn valid_payload_attestation() { } let ctx = TestContext::new(); let gossip = ctx.gossip_ctx(); - let slot = Slot::new(1); + // The genesis block is at slot 0, so attest at slot 0 to satisfy the `block.slot == data.slot` + // gossip rule. Slot 0 is still within the propagation range at the current slot. + let slot = Slot::new(0); let ptc_members = ctx.ptc_members(slot); let validator_index = ptc_members[0] as u64; @@ -243,7 +268,9 @@ fn duplicate_after_valid() { } let ctx = TestContext::new(); let gossip = ctx.gossip_ctx(); - let slot = Slot::new(1); + // The genesis block is at slot 0, so attest at slot 0 to satisfy the `block.slot == data.slot` + // gossip rule. Slot 0 is still within the propagation range at the current slot. + let slot = Slot::new(0); let ptc_members = ctx.ptc_members(slot); let validator_index = ptc_members[0] as u64; diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index 65c95eff35..96dce36de8 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -4003,6 +4003,14 @@ impl NetworkBeaconProcessor { ); self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore); } + PayloadAttestationError::BlockNotAtSlot { .. } => { + debug!( + %peer_id, + %message_slot, + "Payload attestation references block at wrong slot" + ); + self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore); + } PayloadAttestationError::NotInPTC { .. } => { self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Reject); self.gossip_penalize_peer( diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index cd1065a1e7..b3fb38a869 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -1539,11 +1539,11 @@ impl ProtoArray { // considered available when either a majority have voted true or not enough votes have // been cast either way. if proto_node.payload_data_availability::(false)? { - return Ok(false) + return Ok(false); } - + if proto_node.payload_timeliness::(false)? { - return Ok(false) + return Ok(false); } Ok(true)