Limit should_build_on_full checks to the previous slot

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)))
             })?;

consensus/fork_choice/src/fork_choice.rs

@@ -1542,9 +1542,10 @@ where
         &self,
         block_root: &Hash256,
         parent_payload_status: PayloadStatus,
+        proposal_slot: Slot,
     ) -> Result<bool, Error<T::Error>> {
         self.proto_array
-            .should_build_on_full::<E>(block_root, parent_payload_status)
+            .should_build_on_full::<E>(block_root, parent_payload_status, proposal_slot)
             .map_err(Error::ProtoArrayStringError)
     }
 

consensus/proto_array/src/fork_choice_test_definition.rs

@@ -124,6 +124,15 @@ pub enum Operation {
         #[serde(default)]
         proposer_boost_root: Option<Hash256>,
     },
+    /// Assert the result of `should_build_on_full` for the parent `block_root`, where
+    /// `parent_payload_status` is the status the proposer would build on and `proposal_slot`
+    /// is the slot being proposed.
+    AssertShouldBuildOnFull {
+        block_root: Hash256,
+        parent_payload_status: PayloadStatus,
+        proposal_slot: Slot,
+        expected: bool,
+    },
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
@@ -606,6 +615,30 @@ impl ForkChoiceTestDefinition {
                         op_index
                     );
                 }
+                Operation::AssertShouldBuildOnFull {
+                    block_root,
+                    parent_payload_status,
+                    proposal_slot,
+                    expected,
+                } => {
+                    let actual = fork_choice
+                        .should_build_on_full::<MainnetEthSpec>(
+                            &block_root,
+                            parent_payload_status,
+                            proposal_slot,
+                        )
+                        .unwrap_or_else(|e| {
+                            panic!(
+                                "should_build_on_full op at index {} returned error: {}",
+                                op_index, e
+                            )
+                        });
+                    assert_eq!(
+                        actual, expected,
+                        "should_build_on_full mismatch at op index {}",
+                        op_index
+                    );
+                }
             }
         }
     }

consensus/proto_array/src/fork_choice_test_definition/gloas_payload.rs

@@ -971,6 +971,91 @@ pub fn get_gloas_proposer_boost_flips_ancestor_test_definition() -> ForkChoiceTe
     }
 }
 
+/// Exercises the slot check in `should_build_on_full`: for a parent from an earlier slot the
+/// function returns `true` without consulting PTC data-availability votes; only for a parent
+/// from the immediately preceding slot are those votes consulted.
+pub fn get_gloas_should_build_on_full_test_definition() -> ForkChoiceTestDefinition {
+    let mut ops = vec![];
+
+    // Block 1 at slot 1, child of genesis.
+    ops.push(Operation::ProcessBlock {
+        slot: Slot::new(1),
+        root: get_root(1),
+        parent_root: get_root(0),
+        justified_checkpoint: get_checkpoint(0),
+        finalized_checkpoint: get_checkpoint(0),
+        execution_payload_parent_hash: Some(get_hash(0)),
+        execution_payload_block_hash: Some(get_hash(1)),
+    });
+
+    // PTC has voted the payload data as *unavailable*: `is_timely` sets `payload_received` so the
+    // votes are consulted, while clearing the data-availability bits gives the "false" votes an
+    // absolute majority.
+    ops.push(Operation::SetPayloadTiebreak {
+        block_root: get_root(1),
+        is_timely: true,
+        is_data_available: false,
+    });
+
+    // An `Empty` parent is never built on full, regardless of slot. The `Empty` check
+    // precedes the slot check, so neither the previous-slot case (block slot 1, proposal
+    // slot 2) nor an earlier-slot case (proposal slot 3) builds on full.
+    ops.push(Operation::AssertShouldBuildOnFull {
+        block_root: get_root(1),
+        parent_payload_status: PayloadStatus::Empty,
+        proposal_slot: Slot::new(2),
+        expected: false,
+    });
+    ops.push(Operation::AssertShouldBuildOnFull {
+        block_root: get_root(1),
+        parent_payload_status: PayloadStatus::Empty,
+        proposal_slot: Slot::new(3),
+        expected: false,
+    });
+
+    // `Full` parent from the immediately preceding slot (block slot 1, proposal slot 2): the PTC
+    // votes are consulted, and since data is unavailable the proposer does not build on full.
+    ops.push(Operation::AssertShouldBuildOnFull {
+        block_root: get_root(1),
+        parent_payload_status: PayloadStatus::Full,
+        proposal_slot: Slot::new(2),
+        expected: false,
+    });
+
+    // `Full` parent from an *earlier* slot (block slot 1, proposal slot 3): the slot check
+    // short-circuits to `true` without consulting the (unavailable) PTC votes.
+    ops.push(Operation::AssertShouldBuildOnFull {
+        block_root: get_root(1),
+        parent_payload_status: PayloadStatus::Full,
+        proposal_slot: Slot::new(3),
+        expected: true,
+    });
+
+    // Flip the PTC view to *available* and re-check the previous-slot case: the votes now permit
+    // building on full, confirming that case actually consults them.
+    ops.push(Operation::SetPayloadTiebreak {
+        block_root: get_root(1),
+        is_timely: true,
+        is_data_available: true,
+    });
+    ops.push(Operation::AssertShouldBuildOnFull {
+        block_root: get_root(1),
+        parent_payload_status: PayloadStatus::Full,
+        proposal_slot: Slot::new(2),
+        expected: true,
+    });
+
+    ForkChoiceTestDefinition {
+        finalized_block_slot: Slot::new(0),
+        justified_checkpoint: get_checkpoint(0),
+        finalized_checkpoint: get_checkpoint(0),
+        operations: ops,
+        execution_payload_parent_hash: Some(get_hash(42)),
+        execution_payload_block_hash: Some(get_hash(0)),
+        spec: Some(gloas_spec()),
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -1160,6 +1245,12 @@ mod tests {
         test.run();
     }
 
+    #[test]
+    fn should_build_on_full_slot_check() {
+        let test = get_gloas_should_build_on_full_test_definition();
+        test.run();
+    }
+
     /// Test that execution payload invalidation propagates across the V17→V29 fork
     /// boundary: after invalidating a V17 parent, head must not select any descendant.
     ///

consensus/proto_array/src/proto_array.rs

@@ -1512,10 +1512,13 @@ impl ProtoArray {
 
     /// Called by the proposer to decide whether to build on the full or empty
     /// parent pending node. Returns false if the PTC has voted the data as unavailable.
+    /// For a parent from an earlier slot the `Empty` or `Full` node has already been resolved
+    /// by attestation weight in `get_head`.
     pub fn should_build_on_full<E: EthSpec>(
         &self,
         fc_node: &IndexedForkChoiceNode,
         proto_node: &ProtoNode,
+        proposal_slot: Slot,
     ) -> Result<bool, Error> {
         if fc_node.payload_status == PayloadStatus::Pending {
             return Err(Error::InvalidPayloadStatus {
@@ -1527,6 +1530,11 @@ impl ProtoArray {
         if fc_node.payload_status == PayloadStatus::Empty {
             return Ok(false);
         }
+
+        if proto_node.slot().saturating_add(1u64) != proposal_slot {
+            return Ok(true);
+        }
+
         // Check that false votes have not achieved an absolute majority. This allows the payload to be
         // considered available when either a majority have voted true or not enough votes have
         // been cast either way.

consensus/proto_array/src/proto_array_fork_choice.rs

@@ -1015,6 +1015,7 @@ impl ProtoArrayForkChoice {
         &self,
         block_root: &Hash256,
         parent_payload_status: PayloadStatus,
+        proposal_slot: Slot,
     ) -> Result<bool, String> {
         let block_index = self
             .proto_array
@@ -1032,7 +1033,7 @@ impl ProtoArrayForkChoice {
             payload_status: parent_payload_status,
         };
         self.proto_array
-            .should_build_on_full::<E>(&fc_node, proto_node)
+            .should_build_on_full::<E>(&fc_node, proto_node, proposal_slot)
             .map_err(|e| format!("{e:?}"))
     }