feature flag cleanup: vote_state_update_root_fix (#32321)

This commit is contained in:
Ashwin Sekar 2023-06-28 21:26:58 -07:00 committed by GitHub
parent c0d8891d36
commit cbb5381a98
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 34 additions and 94 deletions

View File

@ -179,7 +179,6 @@ fn check_update_vote_state_slots_are_valid(
vote_state: &VoteState, vote_state: &VoteState,
vote_state_update: &mut VoteStateUpdate, vote_state_update: &mut VoteStateUpdate,
slot_hashes: &[(Slot, Hash)], slot_hashes: &[(Slot, Hash)],
feature_set: Option<&FeatureSet>,
) -> Result<(), VoteError> { ) -> Result<(), VoteError> {
if vote_state_update.lockouts.is_empty() { if vote_state_update.lockouts.is_empty() {
return Err(VoteError::EmptySlots); return Err(VoteError::EmptySlots);
@ -211,10 +210,6 @@ fn check_update_vote_state_slots_are_valid(
} }
// Check if the proposed root is too old // 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; let original_proposed_root = vote_state_update.root;
if let Some(new_proposed_root) = original_proposed_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 // If the new proposed root `R` is less than the earliest slot hash in the history
@ -222,22 +217,20 @@ fn check_update_vote_state_slots_are_valid(
// the root to the latest vote in the current vote that's less than R. // the root to the latest vote in the current vote that's less than R.
if earliest_slot_hash_in_history > new_proposed_root { if earliest_slot_hash_in_history > new_proposed_root {
vote_state_update.root = vote_state.root_slot; vote_state_update.root = vote_state.root_slot;
if is_root_fix_enabled { let mut prev_slot = Slot::MAX;
let mut prev_slot = Slot::MAX; let current_root = vote_state_update.root;
let current_root = vote_state_update.root; for vote in vote_state.votes.iter().rev() {
for vote in vote_state.votes.iter().rev() { let is_slot_bigger_than_root = current_root
let is_slot_bigger_than_root = current_root .map(|current_root| vote.slot() > current_root)
.map(|current_root| vote.slot() > current_root) .unwrap_or(true);
.unwrap_or(true); // Ensure we're iterating from biggest to smallest vote in the
// Ensure we're iterating from biggest to smallest vote in the // current vote state
// current vote state assert!(vote.slot() < prev_slot && is_slot_bigger_than_root);
assert!(vote.slot() < prev_slot && is_slot_bigger_than_root); if vote.slot() <= new_proposed_root {
if vote.slot() <= new_proposed_root { vote_state_update.root = Some(vote.slot());
vote_state_update.root = Some(vote.slot()); break;
break;
}
prev_slot = vote.slot();
} }
prev_slot = vote.slot();
} }
} }
} }
@ -304,21 +297,14 @@ fn check_update_vote_state_slots_are_valid(
vote_state_update_indexes_to_filter.push(vote_state_update_index); vote_state_update_indexes_to_filter.push(vote_state_update_index);
} }
if let Some(new_proposed_root) = root_to_check { 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
// 1. Because `root_to_check.is_some()`, then we know that // we haven't checked the root yet in this loop, so
// we haven't checked the root yet in this loop, so // `proposed_vote_slot` == `new_proposed_root` == `vote_state_update.root`.
// `proposed_vote_slot` == `new_proposed_root` == `vote_state_update.root`. assert_eq!(new_proposed_root, proposed_vote_slot);
assert_eq!(new_proposed_root, proposed_vote_slot); // 2. We know from the assert earlier in the function that
// 2. We know from the assert earlier in the function that // `proposed_vote_slot < earliest_slot_hash_in_history`,
// `proposed_vote_slot < earliest_slot_hash_in_history`, // so from 1. we know that `new_proposed_root < 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);
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);
}
root_to_check = None; root_to_check = None;
} else { } else {
vote_state_update_index = vote_state_update_index.checked_add(1).expect( vote_state_update_index = vote_state_update_index.checked_add(1).expect(
@ -1054,12 +1040,7 @@ pub fn do_process_vote_state_update(
mut vote_state_update: VoteStateUpdate, mut vote_state_update: VoteStateUpdate,
feature_set: Option<&FeatureSet>, feature_set: Option<&FeatureSet>,
) -> Result<(), VoteError> { ) -> Result<(), VoteError> {
check_update_vote_state_slots_are_valid( check_update_vote_state_slots_are_valid(vote_state, &mut vote_state_update, slot_hashes)?;
vote_state,
&mut vote_state_update,
slot_hashes,
feature_set,
)?;
process_new_vote_state( process_new_vote_state(
vote_state, vote_state,
vote_state_update.lockouts, vote_state_update.lockouts,
@ -2337,7 +2318,6 @@ mod tests {
&empty_vote_state, &empty_vote_state,
&mut vote_state_update, &mut vote_state_update,
&empty_slot_hashes, &empty_slot_hashes,
Some(&FeatureSet::all_enabled())
), ),
Err(VoteError::EmptySlots), Err(VoteError::EmptySlots),
); );
@ -2349,7 +2329,6 @@ mod tests {
&empty_vote_state, &empty_vote_state,
&mut vote_state_update, &mut vote_state_update,
&empty_slot_hashes, &empty_slot_hashes,
Some(&FeatureSet::all_enabled())
), ),
Err(VoteError::SlotsMismatch), Err(VoteError::SlotsMismatch),
); );
@ -2369,7 +2348,6 @@ mod tests {
&vote_state, &vote_state,
&mut vote_state_update, &mut vote_state_update,
&slot_hashes, &slot_hashes,
Some(&FeatureSet::all_enabled())
), ),
Err(VoteError::VoteTooOld), Err(VoteError::VoteTooOld),
); );
@ -2385,7 +2363,6 @@ mod tests {
&vote_state, &vote_state,
&mut vote_state_update, &mut vote_state_update,
&slot_hashes, &slot_hashes,
Some(&FeatureSet::all_enabled()),
), ),
Err(VoteError::VoteTooOld), Err(VoteError::VoteTooOld),
); );
@ -2436,13 +2413,8 @@ mod tests {
let mut vote_state_update = VoteStateUpdate::from(vote_state_update_slots_and_lockouts); let mut vote_state_update = VoteStateUpdate::from(vote_state_update_slots_and_lockouts);
vote_state_update.hash = vote_state_update_hash; vote_state_update.hash = vote_state_update_hash;
vote_state_update.root = Some(vote_state_update_root); vote_state_update.root = Some(vote_state_update_root);
check_update_vote_state_slots_are_valid( check_update_vote_state_slots_are_valid(&vote_state, &mut vote_state_update, &slot_hashes)
&vote_state, .unwrap();
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled()),
)
.unwrap();
assert_eq!(vote_state_update.root, expected_root); assert_eq!(vote_state_update.root, expected_root);
// The proposed root slot should become the biggest slot in the current vote state less than // The proposed root slot should become the biggest slot in the current vote state less than
@ -2610,7 +2582,6 @@ mod tests {
&vote_state, &vote_state,
&mut vote_state_update, &mut vote_state_update,
&slot_hashes, &slot_hashes,
Some(&FeatureSet::all_enabled())
), ),
Err(VoteError::SlotsNotOrdered), Err(VoteError::SlotsNotOrdered),
); );
@ -2623,7 +2594,6 @@ mod tests {
&vote_state, &vote_state,
&mut vote_state_update, &mut vote_state_update,
&slot_hashes, &slot_hashes,
Some(&FeatureSet::all_enabled()),
), ),
Err(VoteError::SlotsNotOrdered), Err(VoteError::SlotsNotOrdered),
); );
@ -2653,13 +2623,8 @@ mod tests {
(vote_slot, 3), (vote_slot, 3),
]); ]);
vote_state_update.hash = vote_slot_hash; vote_state_update.hash = vote_slot_hash;
check_update_vote_state_slots_are_valid( check_update_vote_state_slots_are_valid(&vote_state, &mut vote_state_update, &slot_hashes)
&vote_state, .unwrap();
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled()),
)
.unwrap();
// Check the earlier slot was filtered out // Check the earlier slot was filtered out
assert_eq!( assert_eq!(
@ -2704,13 +2669,8 @@ mod tests {
let mut vote_state_update = let mut vote_state_update =
VoteStateUpdate::from(vec![(existing_older_than_history_slot, 3), (vote_slot, 2)]); VoteStateUpdate::from(vec![(existing_older_than_history_slot, 3), (vote_slot, 2)]);
vote_state_update.hash = vote_slot_hash; vote_state_update.hash = vote_slot_hash;
check_update_vote_state_slots_are_valid( check_update_vote_state_slots_are_valid(&vote_state, &mut vote_state_update, &slot_hashes)
&vote_state, .unwrap();
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled()),
)
.unwrap();
// Check the earlier slot was *NOT* filtered out // Check the earlier slot was *NOT* filtered out
assert_eq!(vote_state_update.lockouts.len(), 2); assert_eq!(vote_state_update.lockouts.len(), 2);
assert_eq!( assert_eq!(
@ -2768,13 +2728,8 @@ mod tests {
(vote_slot, 1), (vote_slot, 1),
]); ]);
vote_state_update.hash = vote_slot_hash; vote_state_update.hash = vote_slot_hash;
check_update_vote_state_slots_are_valid( check_update_vote_state_slots_are_valid(&vote_state, &mut vote_state_update, &slot_hashes)
&vote_state, .unwrap();
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled()),
)
.unwrap();
assert_eq!(vote_state_update.lockouts.len(), 3); assert_eq!(vote_state_update.lockouts.len(), 3);
assert_eq!( assert_eq!(
vote_state_update vote_state_update
@ -2826,7 +2781,6 @@ mod tests {
&vote_state, &vote_state,
&mut vote_state_update, &mut vote_state_update,
&slot_hashes, &slot_hashes,
Some(&FeatureSet::all_enabled())
), ),
Err(VoteError::SlotsMismatch), Err(VoteError::SlotsMismatch),
); );
@ -2846,7 +2800,6 @@ mod tests {
&vote_state, &vote_state,
&mut vote_state_update, &mut vote_state_update,
&slot_hashes, &slot_hashes,
Some(&FeatureSet::all_enabled())
), ),
Err(VoteError::SlotsMismatch), Err(VoteError::SlotsMismatch),
); );
@ -2881,7 +2834,6 @@ mod tests {
&vote_state, &vote_state,
&mut vote_state_update, &mut vote_state_update,
&slot_hashes, &slot_hashes,
Some(&FeatureSet::all_enabled())
), ),
Err(VoteError::RootOnDifferentFork), Err(VoteError::RootOnDifferentFork),
); );
@ -2906,7 +2858,6 @@ mod tests {
&vote_state, &vote_state,
&mut vote_state_update, &mut vote_state_update,
&slot_hashes, &slot_hashes,
Some(&FeatureSet::all_enabled())
), ),
Err(VoteError::SlotsMismatch), Err(VoteError::SlotsMismatch),
); );
@ -2931,13 +2882,8 @@ mod tests {
let mut vote_state_update = let mut vote_state_update =
VoteStateUpdate::from(vec![(2, 4), (4, 3), (6, 2), (vote_slot, 1)]); VoteStateUpdate::from(vec![(2, 4), (4, 3), (6, 2), (vote_slot, 1)]);
vote_state_update.hash = vote_slot_hash; vote_state_update.hash = vote_slot_hash;
check_update_vote_state_slots_are_valid( check_update_vote_state_slots_are_valid(&vote_state, &mut vote_state_update, &slot_hashes)
&vote_state, .unwrap();
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled()),
)
.unwrap();
// Nothing in the update should have been filtered out // Nothing in the update should have been filtered out
assert_eq!( assert_eq!(
@ -2982,13 +2928,8 @@ mod tests {
.1; .1;
let mut vote_state_update = VoteStateUpdate::from(vec![(4, 2), (vote_slot, 1)]); let mut vote_state_update = VoteStateUpdate::from(vec![(4, 2), (vote_slot, 1)]);
vote_state_update.hash = vote_slot_hash; vote_state_update.hash = vote_slot_hash;
check_update_vote_state_slots_are_valid( check_update_vote_state_slots_are_valid(&vote_state, &mut vote_state_update, &slot_hashes)
&vote_state, .unwrap();
&mut vote_state_update,
&slot_hashes,
Some(&FeatureSet::all_enabled()),
)
.unwrap();
// Nothing in the update should have been filtered out // Nothing in the update should have been filtered out
assert_eq!( assert_eq!(
@ -3037,7 +2978,6 @@ mod tests {
&vote_state, &vote_state,
&mut vote_state_update, &mut vote_state_update,
&slot_hashes, &slot_hashes,
Some(&FeatureSet::all_enabled())
), ),
Err(VoteError::SlotHashMismatch), Err(VoteError::SlotHashMismatch),
); );