diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index efa16978e02..cd4032f55d9 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/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::(); + 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::(); + 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::(); + 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] diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 8c7b58c4d41..dea853d245d 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/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. diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 6670fff6298..7916e9fcdb1 100644 --- a/consensus/types/src/chain_spec.rs +++ b/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(&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::(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::(epoch), + (epoch - 1).start_slot(E::slots_per_epoch()) - 1 + ); + } + } }