merge queue: queuing unstable (d8e406b) and #9393 together

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -4248,12 +4248,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         let cached_head = self.canonical_head.cached_head();
         let old_head_slot = cached_head.head_slot();
 
-        // Compute the expected proposer for `current_slot` on the canonical chain. This is used by
-        // `on_block` to gate proposer boost on the block's proposer matching the canonical proposer
-        // (per spec `update_proposer_boost_root` added in v1.7.0-alpha.5).
-        let canonical_head_proposer_index =
-            self.canonical_head_proposer_index(current_slot, &cached_head)?;
-
         // Take an upgradable read lock on fork choice so we can check if this block has already
         // been imported. We don't want to repeat work importing a block that is already imported.
         let fork_choice_reader = self.canonical_head.fork_choice_upgradable_read_lock();
@@ -4285,7 +4279,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     block_delay,
                     &state,
                     payload_verification_status,
-                    canonical_head_proposer_index,
                     &self.spec,
                 )
                 .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))?;
@@ -5033,42 +5026,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         }))
     }
 
-    /// Compute the expected beacon proposer for `slot` on the canonical chain extending `cached_head`.
-    ///
-    /// Uses the beacon proposer cache to avoid recomputing the shuffling on every block import.
-    ///
-    /// This is used by `update_proposer_boost_root` to gate proposer boost on the block's proposer
-    /// matching the canonical proposer, per consensus-specs v1.7.0-alpha.5.
-    ///
-    /// This function should never error unless there is some corruption of the head state. If a
-    /// state advance is needed, it will be handled by the proposer cache.
-    pub fn canonical_head_proposer_index(
-        &self,
-        slot: Slot,
-        cached_head: &CachedHead<T::EthSpec>,
-    ) -> Result<u64, Error> {
-        let proposal_epoch = slot.epoch(T::EthSpec::slots_per_epoch());
-        let head_block_root = cached_head.head_block_root();
-        let head_state = &cached_head.snapshot.beacon_state;
-
-        let shuffling_decision_root = head_state.proposer_shuffling_decision_root_at_epoch(
-            proposal_epoch,
-            head_block_root,
-            &self.spec,
-        )?;
-
-        self.with_proposer_cache::<_, Error>(
-            shuffling_decision_root,
-            proposal_epoch,
-            |proposers| {
-                proposers
-                    .get_slot::<T::EthSpec>(slot)
-                    .map(|p| p.index as u64)
-            },
-            || Ok((cached_head.head_state_root(), head_state.clone())),
-        )
-    }
-
     pub fn get_expected_withdrawals(
         &self,
         forkchoice_update_params: &ForkchoiceUpdateParameters,

beacon_node/beacon_chain/src/block_production/gloas.rs

@@ -163,7 +163,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         let should_build_on_full = self
             .canonical_head
             .fork_choice_read_lock()
-            .should_build_on_full(&parent_root, parent_payload_status)
+            .should_build_on_full(&parent_root, parent_payload_status, produce_at_slot)
             .map_err(|e| {
                 BlockProductionError::BeaconChain(Box::new(BeaconChainError::ForkChoiceError(e)))
             })?;

beacon_node/beacon_chain/src/payload_attestation_verification/gossip_verified_payload_attestation.rs

@@ -70,13 +70,21 @@ impl<T: BeaconChainTypes> VerifiedPayloadAttestationMessage<T> {
         // 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());

beacon_node/beacon_chain/src/payload_attestation_verification/mod.rs

@@ -60,6 +60,18 @@ 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 likely empty.
+    ///
+    /// ## 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

beacon_node/beacon_chain/src/payload_attestation_verification/tests.rs

@@ -3,7 +3,6 @@ use std::time::Duration;
 
 use bls::Signature;
 use slot_clock::{SlotClock, TestingSlotClock};
-use state_processing::AllCaches;
 use types::{
     Domain, Epoch, EthSpec, ForkName, Hash256, MinimalEthSpec, PayloadAttestationData,
     PayloadAttestationMessage, SignedRoot, Slot,
@@ -167,14 +166,33 @@ 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()) {
         return;
     }
     let ctx = TestContext::new();
     let gossip = ctx.gossip_ctx();
-    let slot = Slot::new(1);
+    let slot = Slot::new(0);
 
     let ptc_members = ctx.ptc_members(slot);
     let non_ptc_validator = (0..NUM_VALIDATORS as u64)
@@ -196,7 +214,7 @@ fn invalid_signature() {
     }
     let ctx = TestContext::new();
     let gossip = ctx.gossip_ctx();
-    let slot = Slot::new(1);
+    let slot = Slot::new(0);
 
     let ptc_members = ctx.ptc_members(slot);
     let validator_index = ptc_members[0] as u64;
@@ -216,7 +234,7 @@ fn valid_payload_attestation() {
     }
     let ctx = TestContext::new();
     let gossip = ctx.gossip_ctx();
-    let slot = Slot::new(1);
+    let slot = Slot::new(0);
 
     let ptc_members = ctx.ptc_members(slot);
     let validator_index = ptc_members[0] as u64;
@@ -243,7 +261,7 @@ fn duplicate_after_valid() {
     }
     let ctx = TestContext::new();
     let gossip = ctx.gossip_ctx();
-    let slot = Slot::new(1);
+    let slot = Slot::new(0);
 
     let ptc_members = ctx.ptc_members(slot);
     let validator_index = ptc_members[0] as u64;
@@ -300,10 +318,8 @@ async fn ptc_cache_is_primed_at_gloas_fork_boundary() {
         .mock_execution_layer()
         .build();
 
-    harness.extend_to_slot(fork_boundary_slot).await;
-
     for slot in test_slots {
-        harness.chain.slot_clock.set_slot(slot.as_u64());
+        harness.extend_to_slot(slot).await;
         assert!(
             harness
                 .chain
@@ -350,10 +366,9 @@ async fn ptc_cache_is_primed_at_gloas_fork_boundary() {
     }
 }
 
-/// Exercises payload attestation gossip verification when the message epoch is ahead of the
-/// canonical head due to many missed slots.
+/// Check that a payload attestation whose assigned slot is empty is ignored.
 #[tokio::test]
-async fn stale_head_payload_attestation() {
+async fn stale_head_empty_slot_payload_attestation_ignored() {
     if !fork_name_from_env().is_some_and(|f| f.gloas_enabled()) {
         return;
     }
@@ -363,81 +378,40 @@ async fn stale_head_payload_attestation() {
     let head_slot = Slot::new(slots_per_epoch);
     let missed_epochs = 4;
     let target_slot = Slot::new(slots_per_epoch * (1 + missed_epochs));
-    let target_epoch = target_slot.epoch(slots_per_epoch);
 
-    // GIVEN a chain with blocks through epoch 1 (so the store has states).
+    // Given a chain with blocks through epoch 1, then a slot clock advanced 4 epochs without
+    // producing blocks (simulating missed slots).
     let harness = BeaconChainHarness::builder(E::default())
         .default_spec()
         .deterministic_keypairs(64)
         .fresh_ephemeral_store()
         .mock_execution_layer()
         .build();
     harness.extend_to_slot(head_slot).await;
-
-    let head = harness.chain.canonical_head.cached_head();
-    let head_epoch = head.snapshot.beacon_state.current_epoch();
-    assert!(
-        target_epoch > head_epoch + harness.spec.min_seed_lookahead,
-        "precondition: message epoch must exceed head + min_seed_lookahead"
-    );
-
-    // GIVEN a slot clock advanced to epoch 5 without producing blocks
-    // (simulating missed slots during a liveness failure).
     harness.chain.slot_clock.set_slot(target_slot.as_u64());
 
-    // Advance a reference state to compute the PTC at the target slot.
-    let mut reference_state = head.snapshot.beacon_state.clone();
-    state_processing::state_advance::partial_state_advance(
-        &mut reference_state,
-        Some(head.snapshot.beacon_state_root()),
-        target_slot,
-        &harness.spec,
-    )
-    .expect("should advance reference state");
-    reference_state
-        .build_all_caches(&harness.spec)
-        .expect("should build caches");
-
-    let ptc = reference_state
-        .get_ptc(target_slot, &harness.spec)
-        .expect("should get PTC from reference state");
-    let validator_index = *ptc.0.first().expect("PTC should have at least one member") as u64;
+    let head = harness.chain.canonical_head.cached_head();
 
-    // WHEN a properly-signed payload attestation from a PTC member is verified. The signature
-    // domain should come from the spec fork schedule and genesis validators root, not a loaded
-    // state in the verifier.
-    let domain = harness.spec.get_domain(
-        target_epoch,
-        Domain::PTCAttester,
-        &reference_state.fork(),
-        reference_state.genesis_validators_root(),
-    );
+    // When a payload attestation for empty target slot references a stale block root
+    // it is ignored because target_slot != block.slot
     let data = PayloadAttestationData {
         beacon_block_root: head.head_block_root(),
         slot: target_slot,
         payload_present: true,
         blob_data_available: true,
     };
-    let message = data.signing_root(domain);
-    let signature = harness.validator_keypairs[validator_index as usize]
-        .sk
-        .sign(message);
     let msg = PayloadAttestationMessage {
-        validator_index,
+        validator_index: 0,
         data,
-        signature,
+        signature: Signature::empty(),
     };
 
-    // THEN verification succeeds despite the head being 4 epochs stale.
     let result = harness
         .chain
         .verify_payload_attestation_message_for_gossip(msg);
     assert!(
-        result.is_ok(),
-        "expected Ok (head epoch {}, message epoch {}), got: {:?}",
-        head_epoch,
-        target_epoch,
-        result.unwrap_err()
+        matches!(result, Err(PayloadAttestationError::BlockNotAtSlot { .. })),
+        "expected BlockNotAtSlot, got: {result:?}"
     );
 }
 

beacon_node/beacon_chain/src/payload_bid_verification/gossip_verified_bid.rs

@@ -144,9 +144,17 @@ impl<T: BeaconChainTypes> GossipVerifiedPayloadBid<T> {
         let fork_choice = ctx.canonical_head.fork_choice_read_lock();
 
         // TODO(gloas) reprocess bids whose parent_block_root becomes known & canonical after a reorg?
-        if !fork_choice.contains_block(&bid_parent_block_root) {
-            return Err(PayloadBidError::ParentBlockRootUnknown {
+        let parent_block = fork_choice.get_block(&bid_parent_block_root).ok_or(
+            PayloadBidError::ParentBlockRootUnknown {
                 parent_block_root: bid_parent_block_root,
+            },
+        )?;
+
+        // [REJECT] The bid is for a higher slot than its parent block.
+        if bid_slot <= parent_block.slot {
+            return Err(PayloadBidError::BidNotDescendantOfParent {
+                bid_slot,
+                parent_slot: parent_block.slot,
             });
         }
 

beacon_node/beacon_chain/src/payload_bid_verification/mod.rs

@@ -37,6 +37,8 @@ pub enum PayloadBidError {
     },
     /// The bids slot is not the current slot or the next slot.
     InvalidBidSlot { bid_slot: Slot },
+    /// The bid's slot is not greater than the slot of its parent block.
+    BidNotDescendantOfParent { bid_slot: Slot, parent_slot: Slot },
     /// The slot clock cannot be read.
     UnableToReadSlot,
     /// No proposer preferences for the current slot.