Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature flag cleanup: vote_state_update_root_fix #32321

Merged
merged 1 commit into from
Jun 29, 2023
Merged

feature flag cleanup: vote_state_update_root_fix #32321

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 34 additions & 94 deletions programs/vote/src/vote_state/mod.rs
View file
Open in desktop
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