Clean up opt conf verifier and vote state tracker#13081
Clean up opt conf verifier and vote state tracker#13081ryoqun merged 3 commits intosolana-labs:masterfrom
Conversation
| } | ||
|
|
||
| let root_bank = bank_forks.read().unwrap().root_bank().clone(); | ||
| if last_process_root.elapsed().as_millis() > 400 { |
There was a problem hiding this comment.
maybe is 400 for DEFAULT_MS_PER_SLOT?
| } | ||
| } | ||
| } else { | ||
| let optimistic_confirmed_slots = optimistic_confirmed_slots.unwrap(); |
There was a problem hiding this comment.
idiomatic rust and less unwrap().
| // if we don't have stake information, ignore it | ||
| let epoch = root_bank.epoch_schedule().get_epoch(slot); | ||
| let epoch_stakes = root_bank.epoch_stakes(epoch); | ||
| if *slot <= root || epoch_stakes.is_none() { |
There was a problem hiding this comment.
move looping-related to its proper place
| let total_stake = epoch_stakes.total_stake(); | ||
|
|
||
| // If this vote for this slot qualifies for optimistic confirmation | ||
| if let Some((stake, hash)) = update_optimistic_confirmation_info { |
There was a problem hiding this comment.
putting this into Option isn't needed first of all.
| let epoch_vote_accounts = Stakes::vote_accounts(epoch_stakes.stakes()); | ||
| let total_epoch_stake = epoch_stakes.total_stake(); |
There was a problem hiding this comment.
epoch here isn't particularly significant information. So, I removed it and moved close to its use.
| let new_stake = self.stake + stake; | ||
| self.stake = new_stake; | ||
| ( | ||
| previous_stake <= supermajority_stake && self.stake > supermajority_stake, |
There was a problem hiding this comment.
let's make these transitive in this inequality more explicit.
|
Well, this pr broke ci, it seems.... |
;) 45a995c |
Codecov Report
@@ Coverage Diff @@
## master #13081 +/- ##
=========================================
- Coverage 82.1% 82.1% -0.1%
=========================================
Files 366 366
Lines 86335 86340 +5
=========================================
+ Hits 70958 70960 +2
- Misses 15377 15380 +3 |
| } | ||
|
|
||
| fn update_new_root(&self, root_bank: &Bank) { | ||
| fn progress_slot(&self, root_bank: &Bank) { |
There was a problem hiding this comment.
how about something more descriptive like purge_stale_state()?
|
Thanks for the cleanup!
I think that makes sense! Since the confirmed hash is being tracked already, it should be a natural extension to also check for a mismatch. |
* Clean up opt conf verifier and vote state tracker * Update test to follow new message and some knob * Rename (cherry picked from commit 0264147)
* Clean up opt conf verifier and vote state tracker * Update test to follow new message and some knob * Rename (cherry picked from commit 0264147)
I'm backporting this in the hope of implementing this rather easily and backport this kill switch to v1.3 and v1.4 |
2 participants
Various cleanups when I skimmed the code when working on #10718. I've constantly missing some time to finish this pr off. No more before more bit rot. :)
For example, I had been struggling with too generic function names...
before:
after:
And various nits.
And, there should be no functional changes.
FYI:
@carllin For my thought on the implementation of this kill switch for mismatched vote hash (#12690 (comment)), I'm planning to augment these facilities. Wdyt? So, this is also kind of a preparation for it.