Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip slot fix #4843

Merged
merged 4 commits into from
Feb 10, 2025
Merged

Skip slot fix #4843

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
575221d
Skip slot fix
bw-solana Feb 7, 2025
e5d3cac
cleanup
bw-solana Feb 7, 2025
5327e4d
cleanup logic
bw-solana Feb 7, 2025
20cafb7
correct comment
bw-solana Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
125 changes: 99 additions & 26 deletions poh/src/poh_recorder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,14 +440,35 @@ impl PohRecorder {

fn is_same_fork_as_previous_leader(&self, slot: Slot) -> bool {
(slot.saturating_sub(NUM_CONSECUTIVE_LEADER_SLOTS)..slot).any(|slot| {
// Check if the last slot Poh reset to was any of the
// Check if the last slot PoH reset to was any of the
// previous leader's slots.
// If so, PoH is currently building on the previous leader's blocks
// If not, PoH is building on a different fork
slot == self.start_slot()
})
}

// Check if the last slot PoH reset onto was the previous leader's last slot.
fn building_off_previous_leader_last_block(&self, my_pubkey: &Pubkey, next_slot: Slot) -> bool {
// Walk backwards from the slot before our next leader slot.
for slot in (next_slot.saturating_sub(NUM_CONSECUTIVE_LEADER_SLOTS)..next_slot).rev() {
// Identify which leader is responsible for building this slot.
let leader_for_slot = self.leader_schedule_cache.slot_leader_at(slot, None);
let Some(leader_for_slot) = leader_for_slot else {
// No leader for this slot, skip
continue;
};

// If the leader for this slot is not me, then it's the previous
// leader's last slot.
if leader_for_slot != *my_pubkey {
// Check if the last slot PoH reset onto was the previous leader's last slot.
return slot == self.start_slot();
}
}
false
}

fn start_slot_was_mine(&self, my_pubkey: &Pubkey) -> bool {
self.start_bank.collector_id() == my_pubkey
}
Expand All @@ -471,8 +492,8 @@ impl PohRecorder {

if self.is_same_fork_as_previous_leader(next_slot) {
// Planning to build off block produced by the leader previous to
// me. Need to wait.
return false;
// me. Check if they've completed all of their slots.
return self.building_off_previous_leader_last_block(my_pubkey, next_slot);
}

if !self.is_new_reset_bank_pending(next_slot) {
Expand Down Expand Up @@ -1918,18 +1939,20 @@ mod tests {
} = create_genesis_config(2);

// Setup start bank.
let bank = Arc::new(Bank::new_for_tests(&genesis_config));
let mut bank = Arc::new(Bank::new_for_tests(&genesis_config));
let prev_hash = bank.last_blockhash();

// Setup leader schedule.
let leader_a_pubkey = validator_pubkey;
let leader_b_pubkey = Pubkey::new_unique();
let leader_c_pubkey = Pubkey::new_unique();
let leader_d_pubkey = Pubkey::new_unique();
let consecutive_leader_slots = NUM_CONSECUTIVE_LEADER_SLOTS as usize;
let mut slot_leaders = Vec::with_capacity(consecutive_leader_slots * 3);
slot_leaders.extend(std::iter::repeat(leader_a_pubkey).take(consecutive_leader_slots));
slot_leaders.extend(std::iter::repeat(leader_b_pubkey).take(consecutive_leader_slots));
slot_leaders.extend(std::iter::repeat(leader_c_pubkey).take(consecutive_leader_slots));
slot_leaders.extend(std::iter::repeat(leader_d_pubkey).take(consecutive_leader_slots));
let mut leader_schedule_cache = LeaderScheduleCache::new_from_bank(&bank);
let fixed_schedule = solana_ledger::leader_schedule::FixedSchedule {
leader_schedule: Arc::new(
Expand All @@ -1953,77 +1976,127 @@ mod tests {
&PohConfig::default(),
Arc::new(AtomicBool::default()),
);
let grace_ticks = bank.ticks_per_slot() * MAX_GRACE_SLOTS;
let ticks_per_slot = bank.ticks_per_slot();
let grace_ticks = ticks_per_slot * MAX_GRACE_SLOTS;
poh_recorder.grace_ticks = grace_ticks;

// Setup leader slot ranges.
let leader_a_start_slot = 0;
let leader_a_end_slot = leader_a_start_slot + NUM_CONSECUTIVE_LEADER_SLOTS - 1;
let leader_b_start_slot = leader_a_end_slot + 1;
let leader_b_end_slot = leader_b_start_slot + NUM_CONSECUTIVE_LEADER_SLOTS - 1;
let leader_c_start_slot = leader_b_end_slot + 1;
let leader_c_end_slot = leader_c_start_slot + NUM_CONSECUTIVE_LEADER_SLOTS - 1;
let leader_d_start_slot = leader_c_end_slot + 1;
let leader_d_end_slot = leader_d_start_slot + NUM_CONSECUTIVE_LEADER_SLOTS - 1;

// Reset onto Leader A's first slot 0.
poh_recorder.reset(
bank.clone(),
Some((leader_a_start_slot + 1, leader_a_end_slot)),
);

// Setup leader start ticks.
let ticks_in_leader_slot_set = bank.ticks_per_slot() * NUM_CONSECUTIVE_LEADER_SLOTS;
let leader_a_start_tick = 0;
let ticks_in_leader_slot_set = ticks_per_slot * NUM_CONSECUTIVE_LEADER_SLOTS;
let leader_a_start_tick = 1;
let leader_b_start_tick = leader_a_start_tick + ticks_in_leader_slot_set;
let leader_c_start_tick = leader_b_start_tick + ticks_in_leader_slot_set;
let leader_d_start_tick = leader_c_start_tick + ticks_in_leader_slot_set;

// True, because we've ticked through all the grace ticks
// True, because we have faked leader A ticking through all the grace ticks.
assert!(poh_recorder.reached_leader_tick(&leader_a_pubkey, leader_a_start_tick));

// True, because from Leader A's perspective, the previous slot was also
// it's own slot, and validators don't give grace periods if previous
// its own slot, and validators don't give grace periods if previous
// slot was also their own.
assert!(
poh_recorder.reached_leader_tick(&leader_a_pubkey, leader_a_start_tick + grace_ticks)
);

// False, because we haven't ticked to our slot yet.
// Reset PoH on Leader A's first slot 0, ticking towards Leader B's leader slots.
poh_recorder.reset(bank.clone(), Some((leader_b_start_slot, leader_b_end_slot)));

// False, because Leader B hasn't ticked to its starting slot yet.
assert!(!poh_recorder.reached_leader_tick(&leader_b_pubkey, leader_b_start_tick));

// Tick through Leader A's slots.
for _ in 0..ticks_in_leader_slot_set {
// Tick through Leader A's remaining slots.
for _ in poh_recorder.tick_height..ticks_in_leader_slot_set {
poh_recorder.tick();
}

// False, because the Poh was reset on slot 0, which is a block produced
// False, because the PoH was reset on slot 0, which is a block produced
// by previous leader A, so a grace period must be given.
assert!(
!poh_recorder.reached_leader_tick(&leader_b_pubkey, leader_b_start_tick + grace_ticks)
);

// Tick through Leader B's grace period.
for _ in 0..grace_ticks {
// Reset onto Leader A's last slot.
for _ in leader_a_start_slot + 1..leader_b_start_slot {
let child_slot = bank.slot() + 1;
bank = Arc::new(Bank::new_from_parent(bank, &leader_a_pubkey, child_slot));
}
poh_recorder.reset(bank.clone(), Some((leader_b_start_slot, leader_b_end_slot)));

// True, because the PoH was reset the last slot produced by the
// previous leader, so we can run immediately.
assert!(
poh_recorder.reached_leader_tick(&leader_b_pubkey, leader_b_start_tick + grace_ticks)
);

// Simulate skipping Leader B's first slot.
poh_recorder.reset(
bank.clone(),
Some((leader_b_start_slot + 1, leader_b_end_slot)),
);
for _ in 0..ticks_per_slot {
poh_recorder.tick();
}

// True, because we've ticked through all the grace ticks
// True, because we're building off the previous leader A's last block.
assert!(
poh_recorder.reached_leader_tick(&leader_b_pubkey, leader_b_start_tick + grace_ticks)
);

// Simulate generating Leader B's second slot.
let child_slot = bank.slot() + 1;
bank = Arc::new(Bank::new_from_parent(bank, &leader_b_pubkey, child_slot));

// Reset PoH targeting Leader D's slots.
poh_recorder.reset(bank, Some((leader_d_start_slot, leader_d_end_slot)));

// Tick through Leader B's remaining slots.
for _ in 0..ticks_in_leader_slot_set - grace_ticks {
for _ in ticks_per_slot..ticks_in_leader_slot_set {
poh_recorder.tick();
}

// Tick through Leader C's slots.
for _ in 0..ticks_in_leader_slot_set {
poh_recorder.tick();
}

// True, because Leader C is not building on any of Leader B's slots.
// The Poh was reset on slot 0, built by Leader A.
// True, because Leader D is not building on any of Leader C's slots.
// The PoH was last reset onto Leader B's second slot.
assert!(
poh_recorder.reached_leader_tick(&leader_c_pubkey, leader_c_start_tick + grace_ticks)
poh_recorder.reached_leader_tick(&leader_d_pubkey, leader_d_start_tick + grace_ticks)
);

// Add some active (partially received) blocks to the active fork.
let active_descendants = vec![NUM_CONSECUTIVE_LEADER_SLOTS];
poh_recorder.update_start_bank_active_descendants(&active_descendants);

// True, because there are pending blocks from Leader B on the active
// fork, but the config to delay for these is not set.
// True, because Leader D observes pending blocks on the active fork,
// but the config to delay for these is not set.
assert!(
poh_recorder.reached_leader_tick(&leader_c_pubkey, leader_c_start_tick + grace_ticks)
poh_recorder.reached_leader_tick(&leader_d_pubkey, leader_d_start_tick + grace_ticks)
);

// Flip the config to delay for pending blocks.
poh_recorder.delay_leader_block_for_pending_fork = true;

// False, because there are pending blocks from Leader B on the active
// fork, and the config to delay for these is set.
// False, because Leader D observes pending blocks on the active fork,
// and the config to delay for these is set.
assert!(
!poh_recorder.reached_leader_tick(&leader_c_pubkey, leader_c_start_tick + grace_ticks)
!poh_recorder.reached_leader_tick(&leader_d_pubkey, leader_d_start_tick + grace_ticks)
);
}

Expand Down
Loading