Further expand last_voted_slot terminology#10747
Conversation
| fn last_voted_slot_in_bank(bank: &Bank, vote_account_pubkey: &Pubkey) -> Option<Slot> { | ||
| let vote_account = bank.vote_accounts().get(vote_account_pubkey)?.1.clone(); | ||
| let bank_vote_state = VoteState::deserialize(&vote_account.data).ok()?; | ||
| bank_vote_state.votes.iter().map(|v| v.slot).last() |
There was a problem hiding this comment.
I don't know how much rustc optimizer is clever. But I simply changed the algorithm from map-then-last to last-then-map, considering this would be more straight-forward in this case: https://github.com/solana-labs/solana/pull/10747/files#diff-3666262446ea129567c6d96960c7e23cR411-R418
New code might eliminate this unneeded iteration here.
| self.lockouts.root_slot | ||
| } | ||
|
|
||
| // a slot is not recent if it's older than the newest vote we have |
There was a problem hiding this comment.
Inverted the description for more simpler definition. Also, old description should be older than or equal to...
| if self | ||
| .votes | ||
| .back() | ||
| .map_or(false, |old_vote| old_vote.slot >= vote.slots[i]) |
There was a problem hiding this comment.
Reversed the condition operands for readability. Loop variable dependant expression should be first to be like a subject in a sentence.
| if self | ||
| .votes | ||
| .back() | ||
| .map_or(false, |old_vote| old_vote.slot >= slot) |
There was a problem hiding this comment.
Reversed the condition operands for readability. function arguments should be first to be like a subject in a sentence.
|
@carllin Thanks for approving my recent prs! Could you also review this at your convenient time? |
Codecov Report
@@ Coverage Diff @@
## master #10747 +/- ##
=======================================
Coverage 81.8% 81.8%
=======================================
Files 302 302
Lines 70799 70806 +7
=======================================
+ Hits 57934 57942 +8
+ Misses 12865 12864 -1 |
|
looks good! |
2 participants
Problem
There seems to be 4 adjective words + 1 code idiom to mean the identical qualified state:
most recent,newest,last,latestandself.votes.back(). I think these are just confusing. Or, I'm lacking requisite delicate nuances of words every developer should be acquainted with. ;)Summary of Changes
Settle with
last. Also, I've ignoredcore/tests/fork-selection.rsbecause it's using reversed convention (= newest vote is pushed at front/first (not at back/last) ofself.votes). And I noticed this too much late.... And the file is well out of scope.continuation of #10734
part of #10718 (I'm addressing this as a separate pr because this doesn't require persistent tower per se)