Fix proposer shuffling decision slot at boundary

beacon_node/beacon_chain/tests/store_tests.rs

@@ -1195,14 +1195,18 @@ fn check_shuffling_compatible(
 ///
 /// - ProtoBlock::proposer_shuffling_root_for_child_block, and
 /// - BeaconState::proposer_shuffling_decision_root{_at_epoch}
-async fn proposer_shuffling_root_consistency_test(parent_slot: u64, child_slot: u64) {
+async fn proposer_shuffling_root_consistency_test(
+    spec: ChainSpec,
+    parent_slot: u64,
+    child_slot: u64,
+) {
     let child_slot = Slot::new(child_slot);
     let db_path = tempdir().unwrap();
-    let store = get_store(&db_path);
+    let store = get_store_generic(&db_path, Default::default(), spec.clone());
     let validators_keypairs =
         types::test_utils::generate_deterministic_keypairs(LOW_VALIDATOR_COUNT);
     let harness = TestHarness::builder(MinimalEthSpec)
-        .default_spec()
+        .spec(spec.into())
         .keypairs(validators_keypairs)
         .fresh_disk_store(store)
         .mock_execution_layer()
@@ -1268,17 +1272,58 @@ async fn proposer_shuffling_root_consistency_test(parent_slot: u64, child_slot:
 
 #[tokio::test]
 async fn proposer_shuffling_root_consistency_same_epoch() {
-    proposer_shuffling_root_consistency_test(32, 39).await;
+    let spec = test_spec::<E>();
+    proposer_shuffling_root_consistency_test(spec, 32, 39).await;
 }
 
 #[tokio::test]
 async fn proposer_shuffling_root_consistency_next_epoch() {
-    proposer_shuffling_root_consistency_test(32, 47).await;
+    let spec = test_spec::<E>();
+    proposer_shuffling_root_consistency_test(spec, 32, 47).await;
 }
 
 #[tokio::test]
 async fn proposer_shuffling_root_consistency_two_epochs() {
-    proposer_shuffling_root_consistency_test(32, 55).await;
+    let spec = test_spec::<E>();
+    proposer_shuffling_root_consistency_test(spec, 32, 55).await;
+}
+
+#[tokio::test]
+async fn proposer_shuffling_root_consistency_at_fork_boundary() {
+    let mut spec = ForkName::Electra.make_genesis_spec(E::default_spec());
+    spec.fulu_fork_epoch = Some(Epoch::new(4));
+
+    // Parent block in epoch prior to Fulu fork epoch, child block in Fulu fork epoch.
+    proposer_shuffling_root_consistency_test(
+        spec.clone(),
+        3 * E::slots_per_epoch(),
+        4 * E::slots_per_epoch(),
+    )
+    .await;
+
+    // Parent block and child block in Fulu fork epoch.
+    proposer_shuffling_root_consistency_test(
+        spec.clone(),
+        4 * E::slots_per_epoch(),
+        4 * E::slots_per_epoch() + 1,
+    )
+    .await;
+
+    // Parent block in Fulu fork epoch and child block in epoch after.
+    proposer_shuffling_root_consistency_test(
+        spec.clone(),
+        4 * E::slots_per_epoch(),
+        5 * E::slots_per_epoch(),
+    )
+    .await;
+
+    // Parent block in epoch prior and child block in epoch after.
+    proposer_shuffling_root_consistency_test(
+        spec,
+        3 * E::slots_per_epoch(),
+        5 * E::slots_per_epoch(),
+    )
+    .await;
 }
 
 #[tokio::test]

consensus/proto_array/src/proto_array_fork_choice.rs

@@ -172,7 +172,13 @@ impl Block {
     ) -> Hash256 {
         let block_epoch = self.current_epoch_shuffling_id.shuffling_epoch;
 
-        if !spec.fork_name_at_epoch(child_block_epoch).fulu_enabled() {
+        // For child blocks in the Fulu fork epoch itself, we want to use the old logic. There is no
+        // lookahead in the first Fulu epoch. So we check whether Fulu is enabled at
+        // `child_block_epoch - 1`, i.e. whether `child_block_epoch > fulu_fork_epoch`.
+        if !spec
+            .fork_name_at_epoch(child_block_epoch.saturating_sub(1_u64))
+            .fulu_enabled()
+        {
             // Prior to Fulu the proposer shuffling decision root for the current epoch is the same
             // as the attestation shuffling for the *next* epoch, i.e. it is determined at the start
             // of the current epoch.

consensus/types/src/chain_spec.rs

@@ -869,7 +869,13 @@ impl ChainSpec {
     ///
     /// The block root at this slot can be used to key the proposer shuffling for the given epoch.
     pub fn proposer_shuffling_decision_slot<E: EthSpec>(&self, epoch: Epoch) -> Slot {
-        if self.fork_name_at_epoch(epoch).fulu_enabled() {
+        // At the Fulu fork epoch itself, the shuffling is computed "the old way" with no lookahead.
+        // Therefore for `epoch == fulu_fork_epoch` we must take the `else` branch. Checking if Fulu
+        // is enabled at `epoch - 1` accomplishes this neatly.
+        if self
+            .fork_name_at_epoch(epoch.saturating_sub(1_u64))
+            .fulu_enabled()
+        {
             // Post-Fulu the proposer shuffling decision slot for epoch N is the slot at the end
             // of epoch N - 2 (note: min_seed_lookahead=1 in all current configs).
             epoch
@@ -2999,4 +3005,32 @@ mod yaml_tests {
             spec.min_epoch_data_availability_boundary(current_epoch)
         );
     }
+
+    #[test]
+    fn proposer_shuffling_decision_root_around_epoch_boundary() {
+        type E = MainnetEthSpec;
+        let fulu_fork_epoch = 5;
+        let spec = {
+            let mut spec = ForkName::Electra.make_genesis_spec(E::default_spec());
+            spec.fulu_fork_epoch = Some(Epoch::new(fulu_fork_epoch));
+            Arc::new(spec)
+        };
+
+        // For epochs prior to AND including the Fulu fork epoch, the decision slot is the end
+        // of the previous epoch (i.e. only 1 slot lookahead).
+        for epoch in (0..=fulu_fork_epoch).map(Epoch::new) {
+            assert_eq!(
+                spec.proposer_shuffling_decision_slot::<E>(epoch),
+                epoch.start_slot(E::slots_per_epoch()) - 1
+            );
+        }
+
+        // For epochs after Fulu, the decision slot is the end of the epoch two epochs prior.
+        for epoch in ((fulu_fork_epoch + 1)..(fulu_fork_epoch + 10)).map(Epoch::new) {
+            assert_eq!(
+                spec.proposer_shuffling_decision_slot::<E>(epoch),
+                (epoch - 1).start_slot(E::slots_per_epoch()) - 1
+            );
+        }
+    }
 }