From 5facd36edbf5b3d90a44d6e467d2fae4bf8cd146 Mon Sep 17 00:00:00 2001 From: steviez Date: Mon, 5 Aug 2024 12:33:28 -0500 Subject: [PATCH 1/4] Update PoH speed check to derive rate from Bank The PoH speed check currently determines the target hash rate by reading values from genesis. However, the hashes per tick rate has been increased via feature gates. Thus, the speed check is using a slower hash rate for comparison than what is actually active. So, update the PoH speed check to derive PoH hash rate from a Bank instead. --- core/src/validator.rs | 50 +++++++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/core/src/validator.rs b/core/src/validator.rs index 757c29f30299a9..2264b66781bca8 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -735,6 +735,11 @@ impl Validator { Some(poh_timing_point_sender.clone()), ) .map_err(ValidatorError::Other)?; + + if !config.no_poh_speed_test { + check_poh_speed(&bank_forks.read().unwrap().root_bank(), None)?; + } + let hard_forks = bank_forks.read().unwrap().root_bank().hard_forks(); if !hard_forks.is_empty() { info!("Hard forks: {:?}", hard_forks); @@ -745,7 +750,6 @@ impl Validator { &genesis_config.hash(), Some(&hard_forks), )); - Self::print_node_info(&node); if let Some(expected_shred_version) = config.expected_shred_version { @@ -1677,18 +1681,16 @@ fn active_vote_account_exists_in_bank(bank: &Bank, vote_account: &Pubkey) -> boo false } -fn check_poh_speed( - genesis_config: &GenesisConfig, - maybe_hash_samples: Option, -) -> Result<(), ValidatorError> { - if let Some(hashes_per_tick) = genesis_config.hashes_per_tick() { - let ticks_per_slot = genesis_config.ticks_per_slot(); +fn check_poh_speed(bank: &Bank, maybe_hash_samples: Option) -> Result<(), ValidatorError> { + if let Some(hashes_per_tick) = bank.hashes_per_tick() { + let ticks_per_slot = bank.ticks_per_slot(); let hashes_per_slot = hashes_per_tick * ticks_per_slot; let hash_samples = maybe_hash_samples.unwrap_or(hashes_per_slot); let hash_time = compute_hash_time(hash_samples); let my_hashes_per_second = (hash_samples as f64 / hash_time.as_secs_f64()) as u64; - let target_slot_duration = Duration::from_nanos(genesis_config.ns_per_slot() as u64); + + let target_slot_duration = Duration::from_nanos(bank.ns_per_slot as u64); let target_hashes_per_second = (hashes_per_slot as f64 / target_slot_duration.as_secs_f64()) as u64; @@ -1703,7 +1705,10 @@ fn check_poh_speed( target: target_hashes_per_second, }); } + } else { + warn!("Unable to read hashes per tick from Bank, skipping PoH speed check"); } + Ok(()) } @@ -1835,10 +1840,6 @@ fn load_genesis( } } - if !config.no_poh_speed_test { - check_poh_speed(&genesis_config, None)?; - } - Ok(genesis_config) } @@ -2937,11 +2938,25 @@ mod tests { )); } + fn target_tick_duration() -> Duration { + // DEFAULT_MS_PER_SLOT = 400 + // DEFAULT_TICKS_PER_SLOT = 64 + // MS_PER_TICK = 6 + // + // But, DEFAULT_MS_PER_SLOT / DEFAULT_TICKS_PER_SLOT = 6.25 + // + // So, convert to microseconds first to avoid the integer rounding error + let target_tick_duration_us = solana_sdk::clock::DEFAULT_MS_PER_SLOT * 1000 + / solana_sdk::clock::DEFAULT_TICKS_PER_SLOT; + assert_eq!(target_tick_duration_us, 6250); + Duration::from_micros(target_tick_duration_us) + } + #[test] fn test_poh_speed() { solana_logger::setup(); let poh_config = PohConfig { - target_tick_duration: Duration::from_millis(solana_sdk::clock::MS_PER_TICK), + target_tick_duration: target_tick_duration(), // make PoH rate really fast to cause the panic condition hashes_per_tick: Some(100 * solana_sdk::clock::DEFAULT_HASHES_PER_TICK), ..PohConfig::default() @@ -2950,13 +2965,15 @@ mod tests { poh_config, ..GenesisConfig::default() }; - assert!(check_poh_speed(&genesis_config, Some(10_000)).is_err()); + let bank = Bank::new_for_tests(&genesis_config); + assert!(check_poh_speed(&bank, Some(10_000)).is_err()); } #[test] fn test_poh_speed_no_hashes_per_tick() { + solana_logger::setup(); let poh_config = PohConfig { - target_tick_duration: Duration::from_millis(solana_sdk::clock::MS_PER_TICK), + target_tick_duration: target_tick_duration(), hashes_per_tick: None, ..PohConfig::default() }; @@ -2964,6 +2981,7 @@ mod tests { poh_config, ..GenesisConfig::default() }; - check_poh_speed(&genesis_config, Some(10_000)).unwrap(); + let bank = Bank::new_for_tests(&genesis_config); + check_poh_speed(&bank, Some(10_000)).unwrap(); } } From 74043a5d55261a210dc22bd3342f9237848ea7ad Mon Sep 17 00:00:00 2001 From: steviez Date: Mon, 5 Aug 2024 21:30:09 -0500 Subject: [PATCH 2/4] Use let-else to avoid indentation --- core/src/validator.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/core/src/validator.rs b/core/src/validator.rs index 2264b66781bca8..84e7da194b00d1 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -1682,7 +1682,11 @@ fn active_vote_account_exists_in_bank(bank: &Bank, vote_account: &Pubkey) -> boo } fn check_poh_speed(bank: &Bank, maybe_hash_samples: Option) -> Result<(), ValidatorError> { - if let Some(hashes_per_tick) = bank.hashes_per_tick() { + let Some(hashes_per_tick) = bank.hashes_per_tick() else { + warn!("Unable to read hashes per tick from Bank, skipping PoH speed check"); + return Ok(()); + }; + let ticks_per_slot = bank.ticks_per_slot(); let hashes_per_slot = hashes_per_tick * ticks_per_slot; let hash_samples = maybe_hash_samples.unwrap_or(hashes_per_slot); @@ -1705,9 +1709,6 @@ fn check_poh_speed(bank: &Bank, maybe_hash_samples: Option) -> Result<(), V target: target_hashes_per_second, }); } - } else { - warn!("Unable to read hashes per tick from Bank, skipping PoH speed check"); - } Ok(()) } From 9bfce7fc69089c7786f8c750c0ee076de74bd030 Mon Sep 17 00:00:00 2001 From: steviez Date: Mon, 5 Aug 2024 21:31:36 -0500 Subject: [PATCH 3/4] cargo fmt --- core/src/validator.rs | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/core/src/validator.rs b/core/src/validator.rs index 84e7da194b00d1..a33828e929487c 100644 --- a/core/src/validator.rs +++ b/core/src/validator.rs @@ -1687,28 +1687,28 @@ fn check_poh_speed(bank: &Bank, maybe_hash_samples: Option) -> Result<(), V return Ok(()); }; - let ticks_per_slot = bank.ticks_per_slot(); - let hashes_per_slot = hashes_per_tick * ticks_per_slot; - let hash_samples = maybe_hash_samples.unwrap_or(hashes_per_slot); + let ticks_per_slot = bank.ticks_per_slot(); + let hashes_per_slot = hashes_per_tick * ticks_per_slot; + let hash_samples = maybe_hash_samples.unwrap_or(hashes_per_slot); - let hash_time = compute_hash_time(hash_samples); - let my_hashes_per_second = (hash_samples as f64 / hash_time.as_secs_f64()) as u64; + let hash_time = compute_hash_time(hash_samples); + let my_hashes_per_second = (hash_samples as f64 / hash_time.as_secs_f64()) as u64; - let target_slot_duration = Duration::from_nanos(bank.ns_per_slot as u64); - let target_hashes_per_second = - (hashes_per_slot as f64 / target_slot_duration.as_secs_f64()) as u64; + let target_slot_duration = Duration::from_nanos(bank.ns_per_slot as u64); + let target_hashes_per_second = + (hashes_per_slot as f64 / target_slot_duration.as_secs_f64()) as u64; - info!( - "PoH speed check: \ + info!( + "PoH speed check: \ computed hashes per second {my_hashes_per_second}, \ target hashes per second {target_hashes_per_second}" - ); - if my_hashes_per_second < target_hashes_per_second { - return Err(ValidatorError::PohTooSlow { - mine: my_hashes_per_second, - target: target_hashes_per_second, - }); - } + ); + if my_hashes_per_second < target_hashes_per_second { + return Err(ValidatorError::PohTooSlow { + mine: my_hashes_per_second, + target: target_hashes_per_second, + }); + } Ok(()) } From ee8278a4108771e0ca7cb1980c95147a9e588789 Mon Sep 17 00:00:00 2001 From: steviez Date: Mon, 5 Aug 2024 21:55:51 -0500 Subject: [PATCH 4/4] Add entry to CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1da9bd05f68b70..c5ab97ef8658dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ Release channels have their own copy of this changelog: * removed the unreleased `redelegate` instruction processor and CLI commands (#2213) * Changes * SDK: removed the `respan` macro. This was marked as "internal use only" and was no longer used internally. + * `agave-validator`: Update PoH speed check to compare against current hash rate from a Bank (#2447) ## [2.0.0] * Breaking