Skip to content

Commit

Permalink
feature flag cleanup: vote_state_update_root_fix (#32321)
Browse files Browse the repository at this point in the history
  • Loading branch information
@AshwinSekar
AshwinSekar authored Jun 29, 2023
1 parent c0d8891 commit cbb5381
Showing 1 changed file with 34 additions and 94 deletions.
128 changes: 34 additions & 94 deletions programs/vote/src/vote_state/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ fn check_update_vote_state_slots_are_valid(
vote_state: &VoteState,
vote_state_update: &mut VoteStateUpdate,
slot_hashes: &[(Slot, Hash)],
feature_set: Option<&FeatureSet>,
) -> Result<(), VoteError> {
if vote_state_update.lockouts.is_empty() {
return Err(VoteError::EmptySlots);
Expand Down Expand Up @@ -211,33 +210,27 @@ fn check_update_vote_state_slots_are_valid(
}

// Check if the proposed root is too old
let is_root_fix_enabled = feature_set
.map(|feature_set| feature_set.is_active(&feature_set::vote_state_update_root_fix::id()))
.unwrap_or(false);

let original_proposed_root = vote_state_update.root;
if let Some(new_proposed_root) = original_proposed_root {
// If the new proposed root `R` is less than the earliest slot hash in the history
// such that we cannot verify whether the slot was actually was on this fork, set
// the root to the latest vote in the current vote that's less than R.
if earliest_slot_hash_in_history > new_proposed_root {
vote_state_update.root = vote_state.root_slot;
if is_root_fix_enabled {
let mut prev_slot = Slot::MAX;
let current_root = vote_state_update.root;
for vote in vote_state.votes.iter().rev() {
let is_slot_bigger_than_root = current_root
.map(|current_root| vote.slot() > current_root)
.unwrap_or(true);
// Ensure we're iterating from biggest to smallest vote in the
// current vote state
assert!(vote.slot() < prev_slot && is_slot_bigger_than_root);
if vote.slot() <= new_proposed_root {
vote_state_update.root = Some(vote.slot());
break;
}
prev_slot = vote.slot();
let mut prev_slot = Slot::MAX;
let current_root = vote_state_update.root;
for vote in vote_state.votes.iter().rev() {
let is_slot_bigger_than_root = current_root
.map(|current_root| vote.slot() > current_root)
.unwrap_or(true);
// Ensure we're iterating from biggest to smallest vote in the
// current vote state
assert!(vote.slot() < prev_slot && is_slot_bigger_than_root);
if vote.slot() <= new_proposed_root {
vote_state_update.root = Some(vote.slot());
break;
}
prev_slot = vote.slot();
}
}
}
Expand Down Expand Up @@ -304,21 +297,14 @@ fn check_update_vote_state_slots_are_valid(
vote_state_update_indexes_to_filter.push(vote_state_update_index);
}
if let Some(new_proposed_root) = root_to_check {
if is_root_fix_enabled {
// 1. Because `root_to_check.is_some()`, then we know that
// we haven't checked the root yet in this loop, so
// `proposed_vote_slot` == `new_proposed_root` == `vote_state_update.root`.
assert_eq!(new_proposed_root, proposed_vote_slot);
// 2. We know from the assert earlier in the function that
// `proposed_vote_slot < earliest_slot_hash_in_history`,
// so from 1. we know that `new_proposed_root < earliest_slot_hash_in_history`.
assert!(new_proposed_root < earliest_slot_hash_in_history);
} else {
// If the vote state update has a root < earliest_slot_hash_in_history
// then we use the current root. The only case where this can happen
// is if the current root itself is not in slot hashes.
assert!(vote_state.root_slot.unwrap() < earliest_slot_hash_in_history);
}
// 1. Because `root_to_check.is_some()`, then we know that
// we haven't checked the root yet in this loop, so
// `proposed_vote_slot` == `new_proposed_root` == `vote_state_update.root`.
assert_eq!(new_proposed_root, proposed_vote_slot);
// 2. We know from the assert earlier in the function that
// `proposed_vote_slot < earliest_slot_hash_in_history`,
// so from 1. we know that `new_proposed_root < earliest_slot_hash_in_history`.
assert!(new_proposed_root < earliest_slot_hash_in_history);
root_to_check = None;
} else {
vote_state_update_index = vote_state_update_index.checked_add(1).expect(
Expand Down Expand Up @@ -1054,12 +1040,7 @@ pub fn do_process_vote_state_update(
mut vote_state_update: VoteStateUpdate,
feature_set: Option<&FeatureSet>,
) -> Result<(), VoteError> {
check_update_vote_state_slots_are_valid(
vote_state,
&mut vote_state_update,
slot_hashes,
feature_set,
)?;
check_update_vote_state_slots_are_valid(vote_state, &mut vote_state_update, slot_hashes)?;
process_new_vote_state(
vote_state,
vote_state_update.lockouts,
Expand Down Expand Up @@ -2337,7 +2318,6 @@ mod tests {
&empty_vote_state,
&mut vote_state_update,
&empty_slot_hashes,
Some(&FeatureSet::all_enabled())
),
Err(VoteError::EmptySlots),
);
Expand All @@ -2349,7 +2329,6 @@ mod tests {
&empty_vote_state,
&mut vote_state_update,
&empty_slot_hashes,
Some(&FeatureSet::all_enabled())
),
Err(VoteError::SlotsMismatch),
);
Expand All @@ -2369,7 +2348,6 @@ mod tests {
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled())
),
Err(VoteError::VoteTooOld),
);
Expand All @@ -2385,7 +2363,6 @@ mod tests {
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled()),
),
Err(VoteError::VoteTooOld),
);
Expand Down Expand Up @@ -2436,13 +2413,8 @@ mod tests {
let mut vote_state_update = VoteStateUpdate::from(vote_state_update_slots_and_lockouts);
vote_state_update.hash = vote_state_update_hash;
vote_state_update.root = Some(vote_state_update_root);
check_update_vote_state_slots_are_valid(
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled()),
)
.unwrap();
check_update_vote_state_slots_are_valid(&vote_state, &mut vote_state_update, &slot_hashes)
.unwrap();
assert_eq!(vote_state_update.root, expected_root);

// The proposed root slot should become the biggest slot in the current vote state less than
Expand Down Expand Up @@ -2610,7 +2582,6 @@ mod tests {
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled())
),
Err(VoteError::SlotsNotOrdered),
);
Expand All @@ -2623,7 +2594,6 @@ mod tests {
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled()),
),
Err(VoteError::SlotsNotOrdered),
);
Expand Down Expand Up @@ -2653,13 +2623,8 @@ mod tests {
(vote_slot, 3),
]);
vote_state_update.hash = vote_slot_hash;
check_update_vote_state_slots_are_valid(
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled()),
)
.unwrap();
check_update_vote_state_slots_are_valid(&vote_state, &mut vote_state_update, &slot_hashes)
.unwrap();

// Check the earlier slot was filtered out
assert_eq!(
Expand Down Expand Up @@ -2704,13 +2669,8 @@ mod tests {
let mut vote_state_update =
VoteStateUpdate::from(vec![(existing_older_than_history_slot, 3), (vote_slot, 2)]);
vote_state_update.hash = vote_slot_hash;
check_update_vote_state_slots_are_valid(
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled()),
)
.unwrap();
check_update_vote_state_slots_are_valid(&vote_state, &mut vote_state_update, &slot_hashes)
.unwrap();
// Check the earlier slot was *NOT* filtered out
assert_eq!(vote_state_update.lockouts.len(), 2);
assert_eq!(
Expand Down Expand Up @@ -2768,13 +2728,8 @@ mod tests {
(vote_slot, 1),
]);
vote_state_update.hash = vote_slot_hash;
check_update_vote_state_slots_are_valid(
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled()),
)
.unwrap();
check_update_vote_state_slots_are_valid(&vote_state, &mut vote_state_update, &slot_hashes)
.unwrap();
assert_eq!(vote_state_update.lockouts.len(), 3);
assert_eq!(
vote_state_update
Expand Down Expand Up @@ -2826,7 +2781,6 @@ mod tests {
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled())
),
Err(VoteError::SlotsMismatch),
);
Expand All @@ -2846,7 +2800,6 @@ mod tests {
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled())
),
Err(VoteError::SlotsMismatch),
);
Expand Down Expand Up @@ -2881,7 +2834,6 @@ mod tests {
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled())
),
Err(VoteError::RootOnDifferentFork),
);
Expand All @@ -2906,7 +2858,6 @@ mod tests {
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled())
),
Err(VoteError::SlotsMismatch),
);
Expand All @@ -2931,13 +2882,8 @@ mod tests {
let mut vote_state_update =
VoteStateUpdate::from(vec![(2, 4), (4, 3), (6, 2), (vote_slot, 1)]);
vote_state_update.hash = vote_slot_hash;
check_update_vote_state_slots_are_valid(
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled()),
)
.unwrap();
check_update_vote_state_slots_are_valid(&vote_state, &mut vote_state_update, &slot_hashes)
.unwrap();

// Nothing in the update should have been filtered out
assert_eq!(
Expand Down Expand Up @@ -2982,13 +2928,8 @@ mod tests {
.1;
let mut vote_state_update = VoteStateUpdate::from(vec![(4, 2), (vote_slot, 1)]);
vote_state_update.hash = vote_slot_hash;
check_update_vote_state_slots_are_valid(
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled()),
)
.unwrap();
check_update_vote_state_slots_are_valid(&vote_state, &mut vote_state_update, &slot_hashes)
.unwrap();

// Nothing in the update should have been filtered out
assert_eq!(
Expand Down Expand Up @@ -3037,7 +2978,6 @@ mod tests {
&vote_state,
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled())
),
Err(VoteError::SlotHashMismatch),
);
Expand Down

0 comments on commit cbb5381

Please sign in to comment.