diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 0a4bc9bc06..8cc2915891 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -818,18 +818,34 @@ impl ReplayStage { parent_slot: Slot, progress_map: &ProgressMap, ) -> bool { - // Check if the next leader slot is part of a consecutive block, in - // which case ignore the propagation check - let is_consecutive_leader = progress_map - .get_propagated_stats(parent_slot) - .unwrap() - .is_leader_slot - && parent_slot == poh_slot - 1; - - if is_consecutive_leader { - return true; + // Assume `NUM_CONSECUTIVE_LEADER_SLOTS` = 4. Then `skip_propagated_check` + // below is true if `poh_slot` is within the same `NUM_CONSECUTIVE_LEADER_SLOTS` + // set of blocks as `latest_leader_slot`. + // + // Example 1 (`poh_slot` directly descended from `latest_leader_slot`): + // + // [B B B B] [B B B latest_leader_slot] poh_slot + // + // Example 2: + // + // [B latest_leader_slot B poh_slot] + // + // In this example, even if there's a block `B` on another fork between + // `poh_slot` and `parent_slot`, because they're in the same + // `NUM_CONSECUTIVE_LEADER_SLOTS` block, we still skip the propagated + // check because it's still within the propagation grace period. + if let Some(latest_leader_slot) = progress_map.get_latest_leader_slot(parent_slot) { + let skip_propagated_check = + poh_slot - latest_leader_slot < NUM_CONSECUTIVE_LEADER_SLOTS; + if skip_propagated_check { + return true; + } } + // Note that `is_propagated(parent_slot)` doesn't necessarily check + // propagation of `parent_slot`, it checks propagation of the latest ancestor + // of `parent_slot` (hence the call to `get_latest_leader_slot()` in the + // check above) progress_map.is_propagated(parent_slot) } @@ -3513,20 +3529,25 @@ pub(crate) mod tests { fn test_check_propagation_for_start_leader() { let mut progress_map = ProgressMap::default(); let poh_slot = 5; - let parent_slot = 3; + let parent_slot = poh_slot - NUM_CONSECUTIVE_LEADER_SLOTS; // If there is no previous leader slot (previous leader slot is None), // should succeed - progress_map.insert(3, ForkProgress::new(Hash::default(), None, None, 0, 0)); + progress_map.insert( + parent_slot, + ForkProgress::new(Hash::default(), None, None, 0, 0), + ); assert!(ReplayStage::check_propagation_for_start_leader( poh_slot, parent_slot, &progress_map, )); - // If the parent was itself the leader, then requires propagation confirmation + // Now if we make the parent was itself the leader, then requires propagation + // confirmation check because the parent is at least NUM_CONSECUTIVE_LEADER_SLOTS + // slots from the `poh_slot` progress_map.insert( - 3, + parent_slot, ForkProgress::new( Hash::default(), None, @@ -3541,7 +3562,7 @@ pub(crate) mod tests { &progress_map, )); progress_map - .get_mut(&3) + .get_mut(&parent_slot) .unwrap() .propagated_stats .is_propagated = true; @@ -3550,12 +3571,16 @@ pub(crate) mod tests { parent_slot, &progress_map, )); - // Now, set up the progress map to show that the previous leader slot of 5 is - // 2 (even though the parent is 3), so 2 needs to see propagation confirmation - // before we can start a leader for block 5 - progress_map.insert(3, ForkProgress::new(Hash::default(), Some(2), None, 0, 0)); + // Now, set up the progress map to show that the `previous_leader_slot` of 5 is + // `parent_slot - 1` (not equal to the actual parent!), so `parent_slot - 1` needs + // to see propagation confirmation before we can start a leader for block 5 + let previous_leader_slot = parent_slot - 1; progress_map.insert( - 2, + parent_slot, + ForkProgress::new(Hash::default(), Some(previous_leader_slot), None, 0, 0), + ); + progress_map.insert( + previous_leader_slot, ForkProgress::new( Hash::default(), None, @@ -3565,17 +3590,17 @@ pub(crate) mod tests { ), ); - // Last leader slot has not seen propagation threshold, so should fail + // `previous_leader_slot` has not seen propagation threshold, so should fail assert!(!ReplayStage::check_propagation_for_start_leader( poh_slot, parent_slot, &progress_map, )); - // If we set the is_propagated = true for the last leader slot, should + // If we set the is_propagated = true for the `previous_leader_slot`, should // allow the block to be generated progress_map - .get_mut(&2) + .get_mut(&previous_leader_slot) .unwrap() .propagated_stats .is_propagated = true; @@ -3585,15 +3610,17 @@ pub(crate) mod tests { &progress_map, )); - // If the root is 3, this filters out slot 2 from the progress map, + // If the root is now set to `parent_slot`, this filters out `previous_leader_slot` from the progress map, // which implies confirmation let bank0 = Bank::new(&genesis_config::create_genesis_config(10000).0); - let bank3 = Bank::new_from_parent(&Arc::new(bank0), &Pubkey::default(), 3); - let mut bank_forks = BankForks::new(bank3); - let bank5 = Bank::new_from_parent(bank_forks.get(3).unwrap(), &Pubkey::default(), 5); + let parent_slot_bank = + Bank::new_from_parent(&Arc::new(bank0), &Pubkey::default(), parent_slot); + let mut bank_forks = BankForks::new(parent_slot_bank); + let bank5 = + Bank::new_from_parent(bank_forks.get(parent_slot).unwrap(), &Pubkey::default(), 5); bank_forks.insert(bank5); - // Should purge only slot 2 from the progress map + // Should purge only `previous_leader_slot` from the progress map progress_map.handle_new_root(&bank_forks); // Should succeed @@ -3605,10 +3632,10 @@ pub(crate) mod tests { } #[test] - fn test_check_propagation_for_consecutive_start_leader() { + fn test_check_propagation_skip_propagation_check() { let mut progress_map = ProgressMap::default(); let poh_slot = 4; - let mut parent_slot = 3; + let mut parent_slot = poh_slot - 1; // Set up the progress map to show that the last leader slot of 4 is 3, // which means 3 and 4 are consecutive leader slots @@ -3622,18 +3649,8 @@ pub(crate) mod tests { 0, ), ); - progress_map.insert( - 2, - ForkProgress::new( - Hash::default(), - None, - Some(ValidatorStakeInfo::default()), - 0, - 0, - ), - ); - // If the last leader slot has not seen propagation threshold, but + // If the previous leader slot has not seen propagation threshold, but // was the direct parent (implying consecutive leader slots), create // the block regardless assert!(ReplayStage::check_propagation_for_start_leader( @@ -3655,10 +3672,41 @@ pub(crate) mod tests { &progress_map, )); - parent_slot = 2; + // Now insert another parent slot 2 for which this validator is also the leader + parent_slot = poh_slot - NUM_CONSECUTIVE_LEADER_SLOTS + 1; + progress_map.insert( + parent_slot, + ForkProgress::new( + Hash::default(), + None, + Some(ValidatorStakeInfo::default()), + 0, + 0, + ), + ); - // Even thought 2 is also a leader slot, because it's not consecutive - // we still have to respect the propagation threshold check + // Even though `parent_slot` and `poh_slot` are separated by another block, + // because they're within `NUM_CONSECUTIVE` blocks of each other, the propagation + // check is still skipped + assert!(ReplayStage::check_propagation_for_start_leader( + poh_slot, + parent_slot, + &progress_map, + )); + + // Once the distance becomes >= NUM_CONSECUTIVE_LEADER_SLOTS, then we need to + // enforce the propagation check + parent_slot = poh_slot - NUM_CONSECUTIVE_LEADER_SLOTS; + progress_map.insert( + parent_slot, + ForkProgress::new( + Hash::default(), + None, + Some(ValidatorStakeInfo::default()), + 0, + 0, + ), + ); assert!(!ReplayStage::check_propagation_for_start_leader( poh_slot, parent_slot,