Update PoH speed check to derive rate from Bank#2447
Update PoH speed check to derive rate from Bank#2447steviez merged 4 commits intoanza-xyz:masterfrom
Conversation
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.
| 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); |
There was a problem hiding this comment.
Not really sure ns_per_slot should be public in Bank, but seemingly a refactor for after this:
Line 730 in 4e5af1e
| target_tick_duration: Duration::from_millis(solana_sdk::clock::MS_PER_TICK), | ||
| target_tick_duration: target_tick_duration(), |
There was a problem hiding this comment.
Technically, this change isn't critical for the PR. But, the constant MS_PER_TICK is 6ms; 6 ms/tick * 64 ticks/slot = 384 ms/slot. It annoyed me that this didn't come out to the 400ms, and I noticed when observing the info log from speed check. Can revert it back if desired
bw-solana
left a comment
bw-solana
left a comment
There was a problem hiding this comment.
LGTM. Left one suggestion that would reduce indent level
steviez
left a comment
steviez
left a comment
There was a problem hiding this comment.
Ended up doing the let-else change you mentioned as I also thought this change might be worthy of a CHANGELOG entry (since it could now be an error at startup for nodes that can't hash fast enough)
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
2 participants
Problem
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.
Summary of Changes
So, update the PoH speed check to derive PoH hash rate from a Bank instead.
Running this on MNB, I see the following log emitted:
10 million is the correct value; the log output shared in my previous PR (#2400) will show that we were previously showing 2 million as the target.