Fix propagation skip check (#13933)

Co-authored-by: Carl Lin <carl@solana.com>
This commit is contained in:
carllin 2020-12-03 12:31:38 -08:00 committed by GitHub
parent d3a4140899
commit 34b68288c8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 92 additions and 44 deletions

View File

@ -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,