Handle VoteStateUpdates for outdated roots bigger than slots in existing VoteState #27323
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
carllin
force-pushed
the
FixVotes
branch
3 times, most recently
from
6554ad3 to
484cc9b
Compare
AshwinSekar
reviewed
Aug 23, 2022
programs/vote/src/vote_state/mod.rs
Outdated
| vote_state_update.root = Some(lockout.slot); | ||
| break; | ||
| } | ||
| prev_slot = lockout.slot; |
There was a problem hiding this comment.
is the only purpose of prev_slot to check that we're iterating from biggest to smallest? Isn't the order of vote_state already checked elsewhere?
There was a problem hiding this comment.
Yeah I think vote_state_update ordering is checked in check_update_vote_state_slots_are_valid and returns SlotNotOrdered if not, but the current vote_state itself was never checked I think
AshwinSekar
reviewed
Aug 23, 2022
programs/vote/src/vote_state/mod.rs
Outdated
| // we use this mutable root to fold the root slot case into this loop for performance | ||
| let mut check_root = vote_state_update.root; | ||
| // Index into the new proposed vote state's slots, starting with the root if it exists then | ||
| // we use this mutable root to fold checking the root slot into this the below loop |
There was a problem hiding this comment.
-this : we use this mutable root to fold checking the root slot into the below loop"
AshwinSekar
reviewed
Aug 23, 2022
| let current_vote_state_root = Some(0); | ||
| let vote_state_update_slots_and_lockouts = vec![(5, 1)]; | ||
| let vote_state_update_root = 4; | ||
| let expected_root = Some(4); |
AshwinSekar
previously approved these changes
Aug 23, 2022
mergify
bot
dismissed
AshwinSekar’s stale review
Pull request has been modified.
carllin
commented
Aug 24, 2022
| assert!(vote_state.root_slot.unwrap() < earliest_slot_hash_in_history); | ||
| check_root = None; | ||
| if let Some(new_proposed_root) = root_to_check { | ||
| if is_root_fix_enabled { |
There was a problem hiding this comment.
@AshwinSekar added check here and simplified the asserts
carllin
commented
Aug 24, 2022
| if earliest_slot_hash_in_history > new_proposed_root { | ||
| vote_state_update.root = vote_state.root_slot; | ||
| if is_root_fix_enabled { |
There was a problem hiding this comment.
@AshwinSekar added check here as well
mergify bot
added a commit
that referenced
this pull request
Aug 26, 2022
…ing VoteState (backport #27323) (#27392) Handle VoteStateUpdates for outdated roots bigger than slots in existing VoteState (#27323) * Set root to latest vote in tower <= prposed vote state * fixup tests * PR comments * feature gate (cherry picked from commit ad6c2d8) Co-authored-by: carllin <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
#27361
Summary of Changes
Set root to largest slot in VoteState less than the proposed root
Fixes #27348