diff --git a/banking_bench/src/main.rs b/banking_bench/src/main.rs index 70a3ff1316..82d38f8ea8 100644 --- a/banking_bench/src/main.rs +++ b/banking_bench/src/main.rs @@ -41,7 +41,7 @@ fn check_txs( let now = Instant::now(); let mut no_bank = false; loop { - if let Ok((_bank, (entry, _tick_count))) = receiver.recv_timeout(Duration::from_millis(10)) + if let Ok((_bank, (entry, _tick_height))) = receiver.recv_timeout(Duration::from_millis(10)) { total += entry.transactions.len(); } diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index f1dc8da6c5..85059176fb 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -1055,7 +1055,7 @@ mod tests { .map(|(_bank, (entry, _tick_height))| entry) .collect(); trace!("done"); - assert_eq!(entries.len(), genesis_block.ticks_per_slot as usize - 1); + assert_eq!(entries.len(), genesis_block.ticks_per_slot as usize); assert!(entries.verify(&start_hash)); assert_eq!(entries[entries.len() - 1].hash, bank.last_blockhash()); banking_stage.join().unwrap(); diff --git a/core/src/blockstream.rs b/core/src/blockstream.rs index 56cf35f546..4c0156b5cb 100644 --- a/core/src/blockstream.rs +++ b/core/src/blockstream.rs @@ -215,7 +215,7 @@ mod test { let mut entries = Vec::new(); let mut expected_entries = Vec::new(); - let tick_height_initial = 0; + let tick_height_initial = 1; let tick_height_final = tick_height_initial + ticks_per_slot + 2; let mut curr_slot = 0; let leader_pubkey = Pubkey::new_rand(); @@ -223,7 +223,7 @@ mod test { for tick_height in tick_height_initial..=tick_height_final { if tick_height == 5 { blockstream - .emit_block_event(curr_slot, tick_height - 1, &leader_pubkey, blockhash) + .emit_block_event(curr_slot, tick_height, &leader_pubkey, blockhash) .unwrap(); curr_slot += 1; } @@ -238,7 +238,7 @@ mod test { assert_eq!( blockstream.entries().len() as u64, - // one entry per tick (0..=N+2) is +3, plus one block + // one entry per tick (1..=N+2) is +3, plus one block ticks_per_slot + 3 + 1 ); diff --git a/core/src/blockstream_service.rs b/core/src/blockstream_service.rs index c8ef3c2365..85c874081d 100644 --- a/core/src/blockstream_service.rs +++ b/core/src/blockstream_service.rs @@ -66,15 +66,8 @@ impl BlockstreamService { } else { Some(blocktree_meta.parent_slot) }; - let ticks_per_slot = entries - .iter() - .filter(|entry| entry.is_tick()) - .fold(0, |acc, _| acc + 1); - let mut tick_height = if slot > 0 && ticks_per_slot > 0 { - ticks_per_slot * slot - 1 - } else { - 0 - }; + let ticks_per_slot = entries.iter().filter(|entry| entry.is_tick()).count() as u64; + let mut tick_height = ticks_per_slot * slot; for (i, entry) in entries.iter().enumerate() { if entry.is_tick() { @@ -158,7 +151,7 @@ mod test { entries.extend_from_slice(&final_tick); let expected_entries = entries.clone(); - let expected_tick_heights = [5, 6, 7, 8, 8, 9]; + let expected_tick_heights = [6, 7, 8, 9, 9, 10]; blocktree .write_entries( @@ -234,7 +227,7 @@ mod test { let slot = json["s"].as_u64().unwrap(); assert_eq!(1, slot); let height = json["h"].as_u64().unwrap(); - assert_eq!(2 * ticks_per_slot - 1, height); + assert_eq!(2 * ticks_per_slot, height); } } } diff --git a/core/src/blocktree_processor.rs b/core/src/blocktree_processor.rs index cf8ec2d1e4..29f9b93cf7 100644 --- a/core/src/blocktree_processor.rs +++ b/core/src/blocktree_processor.rs @@ -304,29 +304,17 @@ fn process_bank_0( assert_eq!(bank0.slot(), 0); // Fetch all entries for this slot - let mut entries = blocktree.get_slot_entries(0, 0, None).map_err(|err| { + let entries = blocktree.get_slot_entries(0, 0, None).map_err(|err| { warn!("Failed to load entries for slot 0, err: {:?}", err); BlocktreeProcessorError::LedgerVerificationFailed })?; - // The first entry in the ledger is a pseudo-tick used only to ensure the number of ticks - // in slot 0 is the same as the number of ticks in all subsequent slots. It is not - // processed by the bank, skip over it. if entries.is_empty() { warn!("entry0 not present"); return Err(BlocktreeProcessorError::LedgerVerificationFailed); } - let entry0 = entries.remove(0); - if !(entry0.is_tick() && entry0.verify(&bank0.last_blockhash())) { - warn!("Ledger proof of history failed at entry0"); - return Err(BlocktreeProcessorError::LedgerVerificationFailed); - } - if !entries.is_empty() { - verify_and_process_entries(bank0, &entries, entry0.hash, opts)?; - } else { - bank0.register_tick(&entry0.hash); - } + verify_and_process_entries(bank0, &entries, bank0.last_blockhash(), opts)?; bank0.freeze(); @@ -843,7 +831,7 @@ pub mod tests { } = create_genesis_block(2); let bank = Arc::new(Bank::new(&genesis_block)); let keypair = Keypair::new(); - let slot_entries = create_ticks(genesis_block.ticks_per_slot - 1, genesis_block.hash()); + let slot_entries = create_ticks(genesis_block.ticks_per_slot, genesis_block.hash()); let tx = system_transaction::create_user_account( &mint_keypair, &keypair.pubkey(), @@ -938,7 +926,7 @@ pub mod tests { bank.get_balance(&mint_keypair.pubkey()), mint - deducted_from_mint ); - assert_eq!(bank.tick_height(), 2 * genesis_block.ticks_per_slot - 1); + assert_eq!(bank.tick_height(), 2 * genesis_block.ticks_per_slot); assert_eq!(bank.last_blockhash(), last_blockhash); } diff --git a/core/src/broadcast_stage/broadcast_fake_blobs_run.rs b/core/src/broadcast_stage/broadcast_fake_blobs_run.rs index 0b4d865fe1..452ac20299 100644 --- a/core/src/broadcast_stage/broadcast_fake_blobs_run.rs +++ b/core/src/broadcast_stage/broadcast_fake_blobs_run.rs @@ -28,7 +28,7 @@ impl BroadcastRun for BroadcastFakeBlobsRun { // 1) Pull entries from banking stage let receive_results = broadcast_utils::recv_slot_entries(receiver)?; let bank = receive_results.bank.clone(); - let last_tick = receive_results.last_tick; + let last_tick_height = receive_results.last_tick_height; let keypair = &cluster_info.read().unwrap().keypair.clone(); let next_shred_index = blocktree @@ -49,7 +49,7 @@ impl BroadcastRun for BroadcastFakeBlobsRun { let (data_shreds, coding_shreds, _) = shredder.entries_to_shreds( &receive_results.entries, - last_tick == bank.max_tick_height(), + last_tick_height == bank.max_tick_height(), next_shred_index, ); @@ -65,13 +65,13 @@ impl BroadcastRun for BroadcastFakeBlobsRun { let (fake_data_shreds, fake_coding_shreds, _) = shredder.entries_to_shreds( &fake_entries, - last_tick == bank.max_tick_height(), + last_tick_height == bank.max_tick_height(), next_shred_index, ); // If it's the last tick, reset the last block hash to default // this will cause next run to grab last bank's blockhash - if last_tick == bank.max_tick_height() { + if last_tick_height == bank.max_tick_height() { self.last_blockhash = Hash::default(); } diff --git a/core/src/broadcast_stage/broadcast_utils.rs b/core/src/broadcast_stage/broadcast_utils.rs index 26d0f038b4..9e7ee0ca62 100644 --- a/core/src/broadcast_stage/broadcast_utils.rs +++ b/core/src/broadcast_stage/broadcast_utils.rs @@ -10,7 +10,7 @@ pub(super) struct ReceiveResults { pub entries: Vec, pub time_elapsed: Duration, pub bank: Arc, - pub last_tick: u64, + pub last_tick_height: u64, } #[derive(Copy, Clone)] @@ -20,7 +20,7 @@ pub struct UnfinishedSlotInfo { pub parent: u64, } -/// Theis parameter tunes how many entries are received in one iteration of recv loop +/// This parameter tunes how many entries are received in one iteration of recv loop /// This will prevent broadcast stage from consuming more entries, that could have led /// to delays in shredding, and broadcasting shreds to peer validators const RECEIVE_ENTRY_COUNT_THRESHOLD: usize = 8; @@ -28,15 +28,15 @@ const RECEIVE_ENTRY_COUNT_THRESHOLD: usize = 8; pub(super) fn recv_slot_entries(receiver: &Receiver) -> Result { let timer = Duration::new(1, 0); let recv_start = Instant::now(); - let (mut bank, (entry, mut last_tick)) = receiver.recv_timeout(timer)?; + let (mut bank, (entry, mut last_tick_height)) = receiver.recv_timeout(timer)?; let mut entries = vec![entry]; let mut slot = bank.slot(); let mut max_tick_height = bank.max_tick_height(); - assert!(last_tick <= max_tick_height); + assert!(last_tick_height <= max_tick_height); - if last_tick != max_tick_height { + if last_tick_height != max_tick_height { while let Ok((try_bank, (entry, tick_height))) = receiver.try_recv() { // If the bank changed, that implies the previous slot was interrupted and we do not have to // broadcast its entries. @@ -47,15 +47,15 @@ pub(super) fn recv_slot_entries(receiver: &Receiver) -> Result slot = bank.slot(); max_tick_height = bank.max_tick_height(); } - last_tick = tick_height; + last_tick_height = tick_height; entries.push(entry); if entries.len() >= RECEIVE_ENTRY_COUNT_THRESHOLD { break; } - assert!(last_tick <= max_tick_height); - if last_tick == max_tick_height { + assert!(last_tick_height <= max_tick_height); + if last_tick_height == max_tick_height { break; } } @@ -66,7 +66,7 @@ pub(super) fn recv_slot_entries(receiver: &Receiver) -> Result entries, time_elapsed, bank, - last_tick, + last_tick_height, }) } @@ -106,7 +106,7 @@ mod tests { let mut last_hash = genesis_block.hash(); assert!(bank1.max_tick_height() > 1); - let entries: Vec<_> = (0..bank1.max_tick_height() + 1) + let entries: Vec<_> = (1..bank1.max_tick_height() + 1) .map(|i| { let entry = Entry::new(&last_hash, 1, vec![tx.clone()]); last_hash = entry.hash; @@ -118,7 +118,7 @@ mod tests { let result = recv_slot_entries(&r).unwrap(); assert_eq!(result.bank.slot(), bank1.slot()); - assert_eq!(result.last_tick, bank1.max_tick_height()); + assert_eq!(result.last_tick_height, bank1.max_tick_height()); assert_eq!(result.entries, entries); } @@ -133,17 +133,18 @@ mod tests { let mut last_hash = genesis_block.hash(); assert!(bank1.max_tick_height() > 1); // Simulate slot 2 interrupting slot 1's transmission - let expected_last_index = bank1.max_tick_height() - 1; - let last_entry = (0..bank1.max_tick_height()) - .map(|i| { + let expected_last_height = bank1.max_tick_height(); + let last_entry = (1..=bank1.max_tick_height()) + .map(|tick_height| { let entry = Entry::new(&last_hash, 1, vec![tx.clone()]); last_hash = entry.hash; // Interrupt slot 1 right before the last tick - if i == expected_last_index { - s.send((bank2.clone(), (entry.clone(), i))).unwrap(); + if tick_height == expected_last_height { + s.send((bank2.clone(), (entry.clone(), tick_height))) + .unwrap(); Some(entry) } else { - s.send((bank1.clone(), (entry, i))).unwrap(); + s.send((bank1.clone(), (entry, tick_height))).unwrap(); None } }) @@ -153,7 +154,7 @@ mod tests { let result = recv_slot_entries(&r).unwrap(); assert_eq!(result.bank.slot(), bank2.slot()); - assert_eq!(result.last_tick, expected_last_index); + assert_eq!(result.last_tick_height, expected_last_height); assert_eq!(result.entries, vec![last_entry]); } } diff --git a/core/src/broadcast_stage/fail_entry_verification_broadcast_run.rs b/core/src/broadcast_stage/fail_entry_verification_broadcast_run.rs index 6c1b37e71d..2d48f41a01 100644 --- a/core/src/broadcast_stage/fail_entry_verification_broadcast_run.rs +++ b/core/src/broadcast_stage/fail_entry_verification_broadcast_run.rs @@ -21,11 +21,11 @@ impl BroadcastRun for FailEntryVerificationBroadcastRun { // 1) Pull entries from banking stage let mut receive_results = broadcast_utils::recv_slot_entries(receiver)?; let bank = receive_results.bank.clone(); - let last_tick = receive_results.last_tick; + let last_tick_height = receive_results.last_tick_height; // 2) Convert entries to blobs + generate coding blobs. Set a garbage PoH on the last entry // in the slot to make verification fail on validators - if last_tick == bank.max_tick_height() { + if last_tick_height == bank.max_tick_height() { let mut last_entry = receive_results.entries.last_mut().unwrap(); last_entry.hash = Hash::default(); } @@ -47,7 +47,7 @@ impl BroadcastRun for FailEntryVerificationBroadcastRun { let (data_shreds, coding_shreds, _) = shredder.entries_to_shreds( &receive_results.entries, - last_tick == bank.max_tick_height(), + last_tick_height == bank.max_tick_height(), next_shred_index, ); diff --git a/core/src/broadcast_stage/standard_broadcast_run.rs b/core/src/broadcast_stage/standard_broadcast_run.rs index fd0310fe6d..1a7cfa847c 100644 --- a/core/src/broadcast_stage/standard_broadcast_run.rs +++ b/core/src/broadcast_stage/standard_broadcast_run.rs @@ -144,7 +144,7 @@ impl StandardBroadcastRun { let mut receive_elapsed = receive_results.time_elapsed; let num_entries = receive_results.entries.len(); let bank = receive_results.bank.clone(); - let last_tick = receive_results.last_tick; + let last_tick_height = receive_results.last_tick_height; inc_new_counter_info!("broadcast_service-entries_received", num_entries); if self.current_slot_and_parent.is_none() @@ -173,7 +173,7 @@ impl StandardBroadcastRun { let (data_shreds, coding_shreds) = self.entries_to_shreds( blocktree, &receive_results.entries, - last_tick == bank.max_tick_height(), + last_tick_height == bank.max_tick_height(), ); let to_shreds_elapsed = to_shreds_start.elapsed(); @@ -214,10 +214,10 @@ impl StandardBroadcastRun { duration_as_us(&insert_shreds_elapsed), duration_as_us(&broadcast_elapsed), duration_as_us(&clone_and_seed_elapsed), - last_tick == bank.max_tick_height(), + last_tick_height == bank.max_tick_height(), ); - if last_tick == bank.max_tick_height() { + if last_tick_height == bank.max_tick_height() { self.unfinished_slot = None; } @@ -353,7 +353,7 @@ mod test { entries: ticks.clone(), time_elapsed: Duration::new(3, 0), bank: bank0.clone(), - last_tick: (ticks.len() - 1) as u64, + last_tick_height: (ticks.len() - 1) as u64, }; // Step 1: Make an incomplete transmission for slot 0 @@ -391,7 +391,7 @@ mod test { entries: ticks.clone(), time_elapsed: Duration::new(2, 0), bank: bank2.clone(), - last_tick: (ticks.len() - 1) as u64, + last_tick_height: (ticks.len() - 1) as u64, }; standard_broadcast_run .process_receive_results(&cluster_info, &socket, &blocktree, receive_results) @@ -420,7 +420,7 @@ mod test { entries: ticks.clone(), time_elapsed: Duration::new(3, 0), bank: bank0.clone(), - last_tick: (ticks.len() - 1) as u64, + last_tick_height: ticks.len() as u64, }; let mut standard_broadcast_run = StandardBroadcastRun::new(leader_keypair); diff --git a/core/src/leader_schedule_utils.rs b/core/src/leader_schedule_utils.rs index 00c10079f3..604d8fd6bd 100644 --- a/core/src/leader_schedule_utils.rs +++ b/core/src/leader_schedule_utils.rs @@ -30,7 +30,7 @@ pub fn slot_leader_at(slot: u64, bank: &Bank) -> Option { // Returns the number of ticks remaining from the specified tick_height to the end of the // slot implied by the tick_height pub fn num_ticks_left_in_slot(bank: &Bank, tick_height: u64) -> u64 { - bank.ticks_per_slot() - tick_height % bank.ticks_per_slot() - 1 + bank.ticks_per_slot() - tick_height % bank.ticks_per_slot() } fn sort_stakes(stakes: &mut Vec<(Pubkey, u64)>) { diff --git a/core/src/poh_recorder.rs b/core/src/poh_recorder.rs index 80ee717976..72a098cc3c 100644 --- a/core/src/poh_recorder.rs +++ b/core/src/poh_recorder.rs @@ -5,7 +5,7 @@ //! within the specified WorkingBank range. //! //! For Ticks: -//! * tick must be > WorkingBank::min_tick_height && tick must be <= WorkingBank::max_tick_height +//! * new tick_height must be > WorkingBank::min_tick_height && new tick_height must be <= WorkingBank::max_tick_height //! //! For Entries: //! * recorded entry must be >= WorkingBank::min_tick_height && entry must be < WorkingBank::max_tick_height @@ -51,13 +51,13 @@ pub struct PohRecorder { pub poh: Arc>, tick_height: u64, clear_bank_signal: Option>, - start_slot: Slot, // parent slot - start_tick: u64, // first tick this recorder will observe - tick_cache: Vec<(Entry, u64)>, + start_slot: Slot, // parent slot + start_tick_height: u64, // first tick_height this recorder will observe + tick_cache: Vec<(Entry, u64)>, // cache of entry and its tick_height working_bank: Option, sender: Sender, - start_leader_at_tick: Option, - last_leader_tick: u64, // zero if none + leader_first_tick_height: Option, + leader_last_tick_height: u64, // zero if none grace_ticks: u64, id: Pubkey, blocktree: Arc, @@ -77,11 +77,11 @@ impl PohRecorder { Some(&self.blocktree), ); assert_eq!(self.ticks_per_slot, bank.ticks_per_slot()); - let (start_leader_at_tick, last_leader_tick, grace_ticks) = - Self::compute_leader_slot_ticks(next_leader_slot, self.ticks_per_slot); + let (leader_first_tick_height, leader_last_tick_height, grace_ticks) = + Self::compute_leader_slot_tick_heights(next_leader_slot, self.ticks_per_slot); self.grace_ticks = grace_ticks; - self.start_leader_at_tick = start_leader_at_tick; - self.last_leader_tick = last_leader_tick; + self.leader_first_tick_height = leader_first_tick_height; + self.leader_last_tick_height = leader_last_tick_height; } if let Some(ref signal) = self.clear_bank_signal { let _ = signal.try_send(true); @@ -90,17 +90,20 @@ impl PohRecorder { pub fn would_be_leader(&self, within_next_n_ticks: u64) -> bool { self.has_bank() - || self.start_leader_at_tick.map_or(false, |leader_tick| { - let ideal_leader_tick = leader_tick.saturating_sub(self.grace_ticks); - - self.tick_height <= self.last_leader_tick - && self.tick_height >= ideal_leader_tick.saturating_sub(within_next_n_ticks) - }) + || self + .leader_first_tick_height + .map_or(false, |leader_first_tick_height| { + let ideal_leader_tick_height = + leader_first_tick_height.saturating_sub(self.grace_ticks); + self.tick_height + within_next_n_ticks >= ideal_leader_tick_height + && self.tick_height <= self.leader_last_tick_height + }) } pub fn leader_after_slots(&self, slots: u64) -> Option { + let current_slot = self.tick_height.saturating_sub(1) / self.ticks_per_slot; self.leader_schedule_cache - .slot_leader_at(self.tick_height / self.ticks_per_slot + slots, None) + .slot_leader_at(current_slot + slots, None) } pub fn next_slot_leader(&self) -> Option { @@ -123,53 +126,61 @@ impl PohRecorder { self.ticks_per_slot } - /// returns if leader tick has reached, how many grace ticks were afforded, + /// returns if leader slot has been reached, how many grace ticks were afforded, /// imputed leader_slot and self.start_slot - /// reached_leader_tick() == true means "ready for a bank" - pub fn reached_leader_tick(&self) -> (bool, u64, Slot, Slot) { + /// reached_leader_slot() == true means "ready for a bank" + pub fn reached_leader_slot(&self) -> (bool, u64, Slot, Slot) { trace!( - "tick_height {}, start_tick {}, start_leader_at_tick {:?}, grace {}, has_bank {}", + "tick_height {}, start_tick_height {}, leader_first_tick_height {:?}, grace_ticks {}, has_bank {}", self.tick_height, - self.start_tick, - self.start_leader_at_tick, + self.start_tick_height, + self.leader_first_tick_height, self.grace_ticks, self.has_bank() ); - let next_tick = self.tick_height + 1; - - if let Some(target_tick) = self.start_leader_at_tick { - // we've reached target_tick OR poh was reset to run immediately - if next_tick >= target_tick || self.start_tick + self.grace_ticks == target_tick { - assert!(next_tick >= self.start_tick); - let ideal_target_tick = target_tick.saturating_sub(self.grace_ticks); + let next_tick_height = self.tick_height + 1; + let next_leader_slot = (next_tick_height - 1) / self.ticks_per_slot; + if let Some(leader_first_tick_height) = self.leader_first_tick_height { + let target_tick_height = leader_first_tick_height.saturating_sub(1); + // we've approached target_tick_height OR poh was reset to run immediately + if self.tick_height >= target_tick_height + || self.start_tick_height + self.grace_ticks == leader_first_tick_height + { + assert!(next_tick_height >= self.start_tick_height); + let ideal_target_tick_height = target_tick_height.saturating_sub(self.grace_ticks); return ( true, - next_tick.saturating_sub(ideal_target_tick), - next_tick / self.ticks_per_slot, + self.tick_height.saturating_sub(ideal_target_tick_height), + next_leader_slot, self.start_slot, ); } } - (false, 0, next_tick / self.ticks_per_slot, self.start_slot) + (false, 0, next_leader_slot, self.start_slot) } - // returns (start_leader_at_tick, last_leader_tick, grace_ticks) given the next slot this - // recorder will lead - fn compute_leader_slot_ticks( + // returns (leader_first_tick_height, leader_last_tick_height, grace_ticks) given the next + // slot this recorder will lead + fn compute_leader_slot_tick_heights( next_leader_slot: Option<(Slot, Slot)>, ticks_per_slot: u64, ) -> (Option, u64, u64) { next_leader_slot - .map(|(first, last)| { - let first_tick = first * ticks_per_slot; - let last_tick = (last + 1) * ticks_per_slot - 1; + .map(|(first_slot, last_slot)| { + let leader_first_tick_height = first_slot * ticks_per_slot + 1; + let last_tick_height = (last_slot + 1) * ticks_per_slot; + let num_slots = last_slot - first_slot + 1; let grace_ticks = cmp::min( MAX_GRACE_TICKS, - (last_tick - first_tick + 1) / GRACE_TICKS_FACTOR, + ticks_per_slot * num_slots / GRACE_TICKS_FACTOR, ); - (Some(first_tick + grace_ticks), last_tick, grace_ticks) + ( + Some(leader_first_tick_height + grace_ticks), + last_tick_height, + grace_ticks, + ) }) .unwrap_or(( None, @@ -202,14 +213,14 @@ impl PohRecorder { std::mem::swap(&mut cache, &mut self.tick_cache); self.start_slot = start_slot; - self.start_tick = (start_slot + 1) * self.ticks_per_slot; - self.tick_height = self.start_tick - 1; + self.tick_height = (start_slot + 1) * self.ticks_per_slot; + self.start_tick_height = self.tick_height + 1; - let (start_leader_at_tick, last_leader_tick, grace_ticks) = - Self::compute_leader_slot_ticks(next_leader_slot, self.ticks_per_slot); + let (leader_first_tick_height, leader_last_tick_height, grace_ticks) = + Self::compute_leader_slot_tick_heights(next_leader_slot, self.ticks_per_slot); self.grace_ticks = grace_ticks; - self.start_leader_at_tick = start_leader_at_tick; - self.last_leader_tick = last_leader_tick; + self.leader_first_tick_height = leader_first_tick_height; + self.leader_last_tick_height = leader_last_tick_height; } pub fn set_working_bank(&mut self, working_bank: WorkingBank) { @@ -279,8 +290,10 @@ impl PohRecorder { working_bank.max_tick_height, working_bank.bank.slot() ); - self.start_slot = working_bank.max_tick_height / self.ticks_per_slot; - self.start_tick = (self.start_slot + 1) * self.ticks_per_slot; + let working_slot = + (working_bank.max_tick_height / self.ticks_per_slot).saturating_sub(1); + self.start_slot = working_slot; + self.start_tick_height = working_slot * self.ticks_per_slot + 1; self.clear_bank(); } if send_result.is_err() { @@ -305,9 +318,9 @@ impl PohRecorder { let now = Instant::now(); if let Some(poh_entry) = poh_entry { self.tick_height += 1; - trace!("tick {}", self.tick_height); + trace!("tick_height {}", self.tick_height); - if self.start_leader_at_tick.is_none() { + if self.leader_first_tick_height.is_none() { inc_new_counter_warn!( "poh_recorder-tick_overhead", timing::duration_as_ms(&now.elapsed()) as usize @@ -389,8 +402,8 @@ impl PohRecorder { poh_config.hashes_per_tick, ))); let (sender, receiver) = channel(); - let (start_leader_at_tick, last_leader_tick, grace_ticks) = - Self::compute_leader_slot_ticks(next_leader_slot, ticks_per_slot); + let (leader_first_tick_height, leader_last_tick_height, grace_ticks) = + Self::compute_leader_slot_tick_heights(next_leader_slot, ticks_per_slot); ( Self { poh, @@ -400,9 +413,9 @@ impl PohRecorder { sender, clear_bank_signal, start_slot, - start_tick: tick_height + 1, - start_leader_at_tick, - last_leader_tick, + start_tick_height: tick_height + 1, + leader_first_tick_height, + leader_last_tick_height, grace_ticks, id: *id, blocktree: blocktree.clone(), @@ -944,12 +957,13 @@ mod tests { poh_recorder.tick(); poh_recorder.tick(); poh_recorder.tick(); - assert_eq!(poh_recorder.tick_cache.len(), 3); - assert_eq!(poh_recorder.tick_height, 3); + poh_recorder.tick(); + assert_eq!(poh_recorder.tick_cache.len(), 4); + assert_eq!(poh_recorder.tick_height, 4); poh_recorder.reset(hash(b"hello"), 0, Some((4, 4))); // parent slot 0 implies tick_height of 3 assert_eq!(poh_recorder.tick_cache.len(), 0); poh_recorder.tick(); - assert_eq!(poh_recorder.tick_height, 4); + assert_eq!(poh_recorder.tick_height, 5); } Blocktree::destroy(&ledger_path).unwrap(); } @@ -1015,6 +1029,7 @@ mod tests { #[test] fn test_poh_recorder_reset_start_slot() { + solana_logger::setup(); let ledger_path = get_tmp_ledger_path!(); { let blocktree = @@ -1040,7 +1055,7 @@ mod tests { ); let end_slot = 3; - let max_tick_height = (end_slot + 1) * ticks_per_slot - 1; + let max_tick_height = (end_slot + 1) * ticks_per_slot; let working_bank = WorkingBank { bank: bank.clone(), min_tick_height: 1, @@ -1065,7 +1080,7 @@ mod tests { } #[test] - fn test_reached_leader_tick() { + fn test_reached_leader_slot() { solana_logger::setup(); let ledger_path = get_tmp_ledger_path!(); @@ -1087,15 +1102,14 @@ mod tests { &Arc::new(PohConfig::default()), ); - // Test that with no leader slot, we don't reach the leader tick - assert_eq!(poh_recorder.reached_leader_tick().0, false); + // Test that with no next leader slot, we don't reach the leader slot + assert_eq!(poh_recorder.reached_leader_slot().0, false); + // Test that with no next leader slot in reset(), we don't reach the leader slot poh_recorder.reset(bank.last_blockhash(), 0, None); + assert_eq!(poh_recorder.reached_leader_slot().0, false); - // Test that with no leader slot in reset(), we don't reach the leader tick - assert_eq!(poh_recorder.reached_leader_tick().0, false); - - // Provide a leader slot 1 slot down + // Provide a leader slot one slot down poh_recorder.reset(bank.last_blockhash(), 0, Some((2, 2))); let init_ticks = poh_recorder.tick_height(); @@ -1111,16 +1125,19 @@ mod tests { init_ticks + bank.ticks_per_slot() ); - // Test that we don't reach the leader tick because of grace ticks - assert_eq!(poh_recorder.reached_leader_tick().0, false); + // Test that we don't reach the leader slot because of grace ticks + assert_eq!(poh_recorder.reached_leader_slot().0, false); // reset poh now. we should immediately be leader poh_recorder.reset(bank.last_blockhash(), 1, Some((2, 2))); - assert_eq!(poh_recorder.reached_leader_tick().0, true); - assert_eq!(poh_recorder.reached_leader_tick().1, 0); + let (reached_leader_slot, grace_ticks, leader_slot, ..) = + poh_recorder.reached_leader_slot(); + assert_eq!(reached_leader_slot, true); + assert_eq!(grace_ticks, 0); + assert_eq!(leader_slot, 2); - // Now test that with grace ticks we can reach leader ticks - // Set the leader slot 1 slot down + // Now test that with grace ticks we can reach leader slot + // Set the leader slot one slot down poh_recorder.reset(bank.last_blockhash(), 1, Some((3, 3))); // Send one slot worth of ticks ("skips" slot 2) @@ -1129,22 +1146,22 @@ mod tests { } // We are not the leader yet, as expected - assert_eq!(poh_recorder.reached_leader_tick().0, false); + assert_eq!(poh_recorder.reached_leader_slot().0, false); - // Send 1 less tick than the grace ticks - for _ in 0..bank.ticks_per_slot() * NUM_CONSECUTIVE_LEADER_SLOTS / GRACE_TICKS_FACTOR { + // Send the grace ticks + for _ in 0..bank.ticks_per_slot() / GRACE_TICKS_FACTOR { poh_recorder.tick(); } // We should be the leader now - assert_eq!(poh_recorder.reached_leader_tick().0, true); - assert_eq!( - poh_recorder.reached_leader_tick().1, - bank.ticks_per_slot() * NUM_CONSECUTIVE_LEADER_SLOTS / GRACE_TICKS_FACTOR - ); + let (reached_leader_slot, grace_ticks, leader_slot, ..) = + poh_recorder.reached_leader_slot(); + assert_eq!(reached_leader_slot, true); + assert_eq!(grace_ticks, bank.ticks_per_slot() / GRACE_TICKS_FACTOR); + assert_eq!(leader_slot, 3); // Let's test that correct grace ticks are reported - // Set the leader slot 1 slot down + // Set the leader slot one slot down poh_recorder.reset(bank.last_blockhash(), 2, Some((4, 4))); // send ticks for a slot @@ -1153,25 +1170,33 @@ mod tests { } // We are not the leader yet, as expected - assert_eq!(poh_recorder.reached_leader_tick().0, false); + assert_eq!(poh_recorder.reached_leader_slot().0, false); poh_recorder.reset(bank.last_blockhash(), 3, Some((4, 4))); // without sending more ticks, we should be leader now - assert_eq!(poh_recorder.reached_leader_tick().0, true); - assert_eq!(poh_recorder.reached_leader_tick().1, 0); + let (reached_leader_slot, grace_ticks, leader_slot, ..) = + poh_recorder.reached_leader_slot(); + assert_eq!(reached_leader_slot, true); + assert_eq!(grace_ticks, 0); + assert_eq!(leader_slot, 4); // Let's test that if a node overshoots the ticks for its target - // leader slot, reached_leader_tick() will return true, because it's overdue - // Set the leader slot 1 slot down + // leader slot, reached_leader_slot() will return true, because it's overdue + // Set the leader slot one slot down poh_recorder.reset(bank.last_blockhash(), 4, Some((5, 5))); - // Send remaining ticks for the slot (remember we sent extra ticks in the previous part of the test) - for _ in 0..4 * bank.ticks_per_slot() { + // Overshoot ticks for the slot + let overshoot_factor = 4; + for _ in 0..overshoot_factor * bank.ticks_per_slot() { poh_recorder.tick(); } // We are overdue to lead - assert_eq!(poh_recorder.reached_leader_tick().0, true); + let (reached_leader_slot, grace_ticks, leader_slot, ..) = + poh_recorder.reached_leader_slot(); + assert_eq!(reached_leader_slot, true); + assert_eq!(grace_ticks, overshoot_factor * bank.ticks_per_slot()); + assert_eq!(leader_slot, 9); } Blocktree::destroy(&ledger_path).unwrap(); } @@ -1276,30 +1301,30 @@ mod tests { } #[test] - fn test_compute_leader_slots() { + fn test_compute_leader_slot_tick_heights() { assert_eq!( - PohRecorder::compute_leader_slot_ticks(None, 0), + PohRecorder::compute_leader_slot_tick_heights(None, 0), (None, 0, 0) ); assert_eq!( - PohRecorder::compute_leader_slot_ticks(Some((4, 4)), 8), - (Some(36), 39, 4) + PohRecorder::compute_leader_slot_tick_heights(Some((4, 4)), 8), + (Some(37), 40, 4) ); assert_eq!( - PohRecorder::compute_leader_slot_ticks(Some((4, 7)), 8), - (Some(44), 63, MAX_GRACE_TICKS) + PohRecorder::compute_leader_slot_tick_heights(Some((4, 7)), 8), + (Some(45), 64, MAX_GRACE_TICKS) ); assert_eq!( - PohRecorder::compute_leader_slot_ticks(Some((6, 7)), 8), - (Some(56), 63, 8) + PohRecorder::compute_leader_slot_tick_heights(Some((6, 7)), 8), + (Some(57), 64, 8) ); assert_eq!( - PohRecorder::compute_leader_slot_ticks(Some((6, 7)), 4), - (Some(28), 31, 4) + PohRecorder::compute_leader_slot_tick_heights(Some((6, 7)), 4), + (Some(29), 32, 4) ); } } diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index a15c2a49ab..65b855b83c 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -288,14 +288,14 @@ impl ReplayStage { assert!(!poh_recorder.lock().unwrap().has_bank()); - let (reached_leader_tick, _grace_ticks, poh_slot, parent_slot) = - poh_recorder.lock().unwrap().reached_leader_tick(); + let (reached_leader_slot, _grace_ticks, poh_slot, parent_slot) = + poh_recorder.lock().unwrap().reached_leader_slot(); - if !reached_leader_tick { - trace!("{} poh_recorder hasn't reached_leader_tick", my_pubkey); + if !reached_leader_slot { + trace!("{} poh_recorder hasn't reached_leader_slot", my_pubkey); return; } - trace!("{} reached_leader_tick", my_pubkey,); + trace!("{} reached_leader_slot", my_pubkey); let parent = bank_forks .read() @@ -1008,7 +1008,7 @@ mod test { genesis_block.epoch_schedule.warmup = false; genesis_block.ticks_per_slot = 4; let bank0 = Bank::new(&genesis_block); - for _ in 1..genesis_block.ticks_per_slot { + for _ in 0..genesis_block.ticks_per_slot { bank0.register_tick(&Hash::default()); } bank0.freeze(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index ed03f0adbc..b3d17dbb19 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -323,7 +323,7 @@ impl Bank { slots_per_year: parent.slots_per_year, epoch_schedule, rent_collector: parent.rent_collector.clone_with_epoch(epoch), - max_tick_height: (slot + 1) * parent.ticks_per_slot - 1, + max_tick_height: (slot + 1) * parent.ticks_per_slot, block_height: parent.block_height + 1, fee_calculator: FeeCalculator::new_derived( &parent.fee_calculator, @@ -660,7 +660,7 @@ impl Bank { self.ticks_per_slot = genesis_block.ticks_per_slot; self.slots_per_segment = genesis_block.slots_per_segment; - self.max_tick_height = (self.slot + 1) * self.ticks_per_slot - 1; + self.max_tick_height = (self.slot + 1) * self.ticks_per_slot; // ticks/year = seconds/year ... self.slots_per_year = SECONDS_PER_YEAR // * (ns/s)/(ns/tick) / ticks/slot = 1/s/1/tick = ticks/s @@ -807,7 +807,7 @@ impl Bank { let should_register_hash = { if self.ticks_per_slot() != 1 || self.slot() != 0 { let lock = { - if (current_tick_height + 1) % self.ticks_per_slot == self.ticks_per_slot - 1 { + if current_tick_height % self.ticks_per_slot == self.ticks_per_slot - 1 { Some(self.blockhash_queue.write().unwrap()) } else { None @@ -816,7 +816,7 @@ impl Bank { self.tick_height.fetch_add(1, Ordering::Relaxed); inc_new_counter_debug!("bank-register_tick-registered", 1); lock - } else if current_tick_height % self.ticks_per_slot == self.ticks_per_slot - 1 { + } else if current_tick_height % self.ticks_per_slot == 0 { // Register a new block hash if at the last tick in the slot Some(self.blockhash_queue.write().unwrap()) } else { @@ -3236,7 +3236,7 @@ mod tests { assert_eq!(bank.is_votable(), false); // Register enough ticks to hit max tick height - for i in 0..genesis_block.ticks_per_slot - 1 { + for i in 0..genesis_block.ticks_per_slot { bank.register_tick(&hash::hash(format!("hello world {}", i).as_bytes())); } @@ -3249,7 +3249,7 @@ mod tests { assert_eq!(bank.is_votable(), false); // Register enough ticks to hit max tick height - for i in 0..genesis_block.ticks_per_slot - 1 { + for i in 0..genesis_block.ticks_per_slot { bank.register_tick(&hash::hash(format!("hello world {}", i).as_bytes())); } // empty banks aren't votable even at max tick height @@ -3360,7 +3360,7 @@ mod tests { let (genesis_block, _) = create_genesis_block(500); let bank = Arc::new(Bank::new(&genesis_block)); //set tick height to max - let max_tick_height = (bank.slot + 1) * bank.ticks_per_slot - 1; + let max_tick_height = (bank.slot + 1) * bank.ticks_per_slot; bank.tick_height.store(max_tick_height, Ordering::Relaxed); assert!(bank.is_votable()); }