diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index e21bcc395aac8f..f3ad383988263f 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -705,6 +705,7 @@ impl VoteState { &self, 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); @@ -736,18 +737,43 @@ impl VoteState { } // Check if the proposed root is too old - if let Some(new_proposed_root) = vote_state_update.root { - // If the root 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 current vote state root for safety. + 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 = self.root_slot; + if is_root_fix_enabled { + let mut prev_slot = Slot::MAX; + let current_root = vote_state_update.root; + for lockout in self.votes.iter().rev() { + let is_slot_bigger_than_root = current_root + .map(|current_root| lockout.slot > current_root) + .unwrap_or(true); + // Ensure we're iterating from biggest to smallest vote in the + // current vote state + assert!(lockout.slot < prev_slot && is_slot_bigger_than_root); + if lockout.slot <= new_proposed_root { + vote_state_update.root = Some(lockout.slot); + break; + } + prev_slot = lockout.slot; + } + } } } - // index into the new proposed vote state's slots, starting with the root if it exists then - // 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 the below loop + // for performance + let mut root_to_check = vote_state_update.root; let mut vote_state_update_index = 0; // index into the slot_hashes, starting at the oldest known @@ -769,12 +795,12 @@ impl VoteState { // because have to ensure that every slot is actually part of the history, not just the most // recent ones while vote_state_update_index < vote_state_update.lockouts.len() && slot_hashes_index > 0 { - let proposed_vote_slot = if let Some(root) = check_root { + let proposed_vote_slot = if let Some(root) = root_to_check { root } else { vote_state_update.lockouts[vote_state_update_index].slot }; - if check_root.is_none() + if root_to_check.is_none() && vote_state_update_index > 0 && proposed_vote_slot <= vote_state_update.lockouts[vote_state_update_index - 1].slot @@ -791,7 +817,7 @@ impl VoteState { // The vote slot does not exist in the SlotHashes history because it's too old, // i.e. older than the oldest slot in the history. assert!(proposed_vote_slot < earliest_slot_hash_in_history); - if !self.contains_slot(proposed_vote_slot) && check_root.is_none() { + if !self.contains_slot(proposed_vote_slot) && root_to_check.is_none() { // If the vote slot is both: // 1) Too old // 2) Doesn't already exist in vote state @@ -799,12 +825,23 @@ impl VoteState { // Then filter it out vote_state_update_indexes_to_filter.push(vote_state_update_index); } - if check_root.is_some() { - // 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!(self.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 { + // 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!(self.root_slot.unwrap() < earliest_slot_hash_in_history); + } + root_to_check = None; } else { vote_state_update_index += 1; } @@ -813,7 +850,7 @@ impl VoteState { // If the vote slot is new enough to be in the slot history, // but is not part of the slot history, then it must belong to another fork, // which means this vote state update is invalid. - if check_root.is_some() { + if root_to_check.is_some() { return Err(VoteError::RootOnDifferentFork); } else { return Err(VoteError::SlotsMismatch); @@ -829,8 +866,8 @@ impl VoteState { // Once the slot in `vote_state_update.lockouts` is found, bump to the next slot // in `vote_state_update.lockouts` and continue. If we were checking the root, // start checking the vote state instead. - if check_root.is_some() { - check_root = None; + if root_to_check.is_some() { + root_to_check = None; } else { vote_state_update_index += 1; slot_hashes_index -= 1; @@ -988,7 +1025,7 @@ impl VoteState { Ok(()) } - //`Ensure check_update_vote_state_slots_are_valid()` runs on the slots in `new_state` + // `Ensure check_update_vote_state_slots_are_valid()` runs on the slots in `new_state` // before `process_new_vote_state()` is called // This function should guarantee the following about `new_state`: @@ -1063,12 +1100,12 @@ impl VoteState { return Err(VoteError::ConfirmationTooLarge); } else if let Some(new_root) = new_root { if vote.slot <= new_root - && - // This check is necessary because - // https://github.com/ryoqun/solana/blob/df55bfb46af039cbc597cd60042d49b9d90b5961/core/src/consensus.rs#L120 - // always sets a root for even empty towers, which is then hard unwrapped here - // https://github.com/ryoqun/solana/blob/df55bfb46af039cbc597cd60042d49b9d90b5961/core/src/consensus.rs#L776 - new_root != Slot::default() + && + // This check is necessary because + // https://github.com/ryoqun/solana/blob/df55bfb46af039cbc597cd60042d49b9d90b5961/core/src/consensus.rs#L120 + // always sets a root for even empty towers, which is then hard unwrapped here + // https://github.com/ryoqun/solana/blob/df55bfb46af039cbc597cd60042d49b9d90b5961/core/src/consensus.rs#L776 + new_root != Slot::default() { return Err(VoteError::SlotSmallerThanRoot); } @@ -1679,22 +1716,42 @@ pub fn process_vote_state_update( vote_account: &mut BorrowedAccount, slot_hashes: &[SlotHash], clock: &Clock, - mut vote_state_update: VoteStateUpdate, + vote_state_update: VoteStateUpdate, signers: &HashSet, feature_set: &FeatureSet, ) -> Result<(), InstructionError> { let mut vote_state = verify_and_get_vote_state(vote_account, clock, signers)?; - vote_state.check_update_vote_state_slots_are_valid(&mut vote_state_update, slot_hashes)?; - vote_state.process_new_vote_state( - vote_state_update.lockouts, - vote_state_update.root, - vote_state_update.timestamp, + do_process_vote_state_update( + &mut vote_state, + slot_hashes, clock.epoch, + vote_state_update, Some(feature_set), )?; vote_account.set_state(&VoteStateVersions::new_current(vote_state)) } +pub fn do_process_vote_state_update( + vote_state: &mut VoteState, + slot_hashes: &[SlotHash], + epoch: u64, + mut vote_state_update: VoteStateUpdate, + feature_set: Option<&FeatureSet>, +) -> Result<(), VoteError> { + vote_state.check_update_vote_state_slots_are_valid( + &mut vote_state_update, + slot_hashes, + feature_set, + )?; + vote_state.process_new_vote_state( + vote_state_update.lockouts, + vote_state_update.root, + vote_state_update.timestamp, + epoch, + feature_set, + ) +} + pub fn create_account_with_authorized( node_pubkey: &Pubkey, authorized_voter: &Pubkey, @@ -3340,7 +3397,8 @@ mod tests { assert_eq!( empty_vote_state.check_update_vote_state_slots_are_valid( &mut vote_state_update, - &empty_slot_hashes + &empty_slot_hashes, + Some(&FeatureSet::all_enabled()) ), Err(VoteError::EmptySlots), ); @@ -3350,7 +3408,8 @@ mod tests { assert_eq!( empty_vote_state.check_update_vote_state_slots_are_valid( &mut vote_state_update, - &empty_slot_hashes + &empty_slot_hashes, + Some(&FeatureSet::all_enabled()) ), Err(VoteError::SlotsMismatch), ); @@ -3366,8 +3425,11 @@ mod tests { // should return error `VoteTooOld` let mut vote_state_update = VoteStateUpdate::from(vec![(latest_vote, 1)]); assert_eq!( - vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes), + vote_state.check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()) + ), Err(VoteError::VoteTooOld), ); @@ -3378,48 +3440,239 @@ mod tests { let slot_hashes = build_slot_hashes(vec![earliest_slot_in_history]); let mut vote_state_update = VoteStateUpdate::from(vec![(earliest_slot_in_history - 1, 1)]); assert_eq!( - vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes), + vote_state.check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()), + ), Err(VoteError::VoteTooOld), ); } - #[test] - fn test_check_update_vote_state_older_than_history_root() { - let slot_hashes = build_slot_hashes(vec![1, 2, 3, 4]); - let mut vote_state = build_vote_state(vec![1, 2, 3, 4], &slot_hashes); + fn run_test_check_update_vote_state_older_than_history_root( + earliest_slot_in_history: Slot, + current_vote_state_slots: Vec, + current_vote_state_root: Option, + vote_state_update_slots_and_lockouts: Vec<(Slot, u32)>, + vote_state_update_root: Slot, + expected_root: Option, + expected_vote_state: Vec, + ) { + assert!(vote_state_update_root < earliest_slot_in_history); + assert_eq!( + expected_root, + current_vote_state_slots + .iter() + .rev() + .find(|slot| **slot <= vote_state_update_root) + .cloned() + ); + let latest_slot_in_history = vote_state_update_slots_and_lockouts + .last() + .unwrap() + .0 + .max(earliest_slot_in_history); + let mut slot_hashes = build_slot_hashes( + (current_vote_state_slots.first().copied().unwrap_or(0)..=latest_slot_in_history) + .collect::>(), + ); - // Test with a `vote_state_update` where the root is less than `earliest_slot_in_history`. - // Root slot in the `vote_state_update` should be updated to match the root slot in the - // current vote state - let earliest_slot_in_history = 5; - let slot_hashes = build_slot_hashes(vec![earliest_slot_in_history, 6, 7, 8]); - let earliest_slot_in_history_hash = slot_hashes + let mut vote_state = build_vote_state(current_vote_state_slots, &slot_hashes); + vote_state.root_slot = current_vote_state_root; + + slot_hashes.retain(|slot| slot.0 >= earliest_slot_in_history); + assert!(!vote_state_update_slots_and_lockouts.is_empty()); + let vote_state_update_hash = slot_hashes .iter() - .find(|(slot, _hash)| *slot == earliest_slot_in_history) + .find(|(slot, _hash)| *slot == vote_state_update_slots_and_lockouts.last().unwrap().0) .unwrap() .1; - let mut vote_state_update = VoteStateUpdate::from(vec![(earliest_slot_in_history, 1)]); - vote_state_update.hash = earliest_slot_in_history_hash; - vote_state_update.root = Some(earliest_slot_in_history - 1); - vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes) - .unwrap(); - assert!(vote_state.root_slot.is_none()); - assert_eq!(vote_state_update.root, vote_state.root_slot); // Test with a `vote_state_update` where the root is less than `earliest_slot_in_history`. // Root slot in the `vote_state_update` should be updated to match the root slot in the // current vote state - vote_state.root_slot = Some(0); - let mut vote_state_update = VoteStateUpdate::from(vec![(earliest_slot_in_history, 1)]); - vote_state_update.hash = earliest_slot_in_history_hash; - vote_state_update.root = Some(earliest_slot_in_history - 1); + 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); vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes) + .check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()), + ) .unwrap(); - assert_eq!(vote_state.root_slot, Some(0)); - assert_eq!(vote_state_update.root, vote_state.root_slot); + assert_eq!(vote_state_update.root, expected_root); + + // The proposed root slot should become the biggest slot in the current vote state less than + // `earliest_slot_in_history`. + assert!(do_process_vote_state_update( + &mut vote_state, + &slot_hashes, + 0, + vote_state_update.clone(), + Some(&FeatureSet::all_enabled()), + ) + .is_ok()); + assert_eq!(vote_state.root_slot, expected_root); + assert_eq!( + vote_state.votes.into_iter().collect::>(), + expected_vote_state, + ); + } + + #[test] + fn test_check_update_vote_state_older_than_history_root() { + // Test when `vote_state_update_root` is in `current_vote_state_slots` but it's not the latest + // slot + let earliest_slot_in_history = 5; + let current_vote_state_slots: Vec = vec![1, 2, 3, 4]; + let current_vote_state_root = None; + let vote_state_update_slots_and_lockouts = vec![(5, 1)]; + let vote_state_update_root = 4; + let expected_root = Some(4); + let expected_vote_state = vec![Lockout { + slot: 5, + confirmation_count: 1, + }]; + run_test_check_update_vote_state_older_than_history_root( + earliest_slot_in_history, + current_vote_state_slots, + current_vote_state_root, + vote_state_update_slots_and_lockouts, + vote_state_update_root, + expected_root, + expected_vote_state, + ); + + // Test when `vote_state_update_root` is in `current_vote_state_slots` but it's not the latest + // slot and the `current_vote_state_root.is_some()`. + let earliest_slot_in_history = 5; + let current_vote_state_slots: Vec = vec![1, 2, 3, 4]; + 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); + let expected_vote_state = vec![Lockout { + slot: 5, + confirmation_count: 1, + }]; + run_test_check_update_vote_state_older_than_history_root( + earliest_slot_in_history, + current_vote_state_slots, + current_vote_state_root, + vote_state_update_slots_and_lockouts, + vote_state_update_root, + expected_root, + expected_vote_state, + ); + + // Test when `vote_state_update_root` is in `current_vote_state_slots` but it's not the latest + // slot + let earliest_slot_in_history = 5; + let current_vote_state_slots: Vec = vec![1, 2, 3, 4]; + let current_vote_state_root = Some(0); + let vote_state_update_slots_and_lockouts = vec![(4, 2), (5, 1)]; + let vote_state_update_root = 3; + let expected_root = Some(3); + let expected_vote_state = vec![ + Lockout { + slot: 4, + confirmation_count: 2, + }, + Lockout { + slot: 5, + confirmation_count: 1, + }, + ]; + run_test_check_update_vote_state_older_than_history_root( + earliest_slot_in_history, + current_vote_state_slots, + current_vote_state_root, + vote_state_update_slots_and_lockouts, + vote_state_update_root, + expected_root, + expected_vote_state, + ); + + // Test when `vote_state_update_root` is not in `current_vote_state_slots` + let earliest_slot_in_history = 5; + let current_vote_state_slots: Vec = vec![1, 2, 4]; + let current_vote_state_root = Some(0); + let vote_state_update_slots_and_lockouts = vec![(4, 2), (5, 1)]; + let vote_state_update_root = 3; + let expected_root = Some(2); + let expected_vote_state = vec![ + Lockout { + slot: 4, + confirmation_count: 2, + }, + Lockout { + slot: 5, + confirmation_count: 1, + }, + ]; + run_test_check_update_vote_state_older_than_history_root( + earliest_slot_in_history, + current_vote_state_slots, + current_vote_state_root, + vote_state_update_slots_and_lockouts, + vote_state_update_root, + expected_root, + expected_vote_state, + ); + + // Test when the `vote_state_update_root` is smaller than all the slots in + // `current_vote_state_slots`, no roots should be set. + let earliest_slot_in_history = 4; + let current_vote_state_slots: Vec = vec![3, 4]; + let current_vote_state_root = None; + let vote_state_update_slots_and_lockouts = vec![(3, 3), (4, 2), (5, 1)]; + let vote_state_update_root = 2; + let expected_root = None; + let expected_vote_state = vec![ + Lockout { + slot: 3, + confirmation_count: 3, + }, + Lockout { + slot: 4, + confirmation_count: 2, + }, + Lockout { + slot: 5, + confirmation_count: 1, + }, + ]; + run_test_check_update_vote_state_older_than_history_root( + earliest_slot_in_history, + current_vote_state_slots, + current_vote_state_root, + vote_state_update_slots_and_lockouts, + vote_state_update_root, + expected_root, + expected_vote_state, + ); + + // Test when `current_vote_state_slots` is empty, no roots should be set + let earliest_slot_in_history = 4; + let current_vote_state_slots: Vec = vec![]; + let current_vote_state_root = None; + let vote_state_update_slots_and_lockouts = vec![(5, 1)]; + let vote_state_update_root = 2; + let expected_root = None; + let expected_vote_state = vec![Lockout { + slot: 5, + confirmation_count: 1, + }]; + run_test_check_update_vote_state_older_than_history_root( + earliest_slot_in_history, + current_vote_state_slots, + current_vote_state_root, + vote_state_update_slots_and_lockouts, + vote_state_update_root, + expected_root, + expected_vote_state, + ); } #[test] @@ -3437,8 +3690,11 @@ mod tests { let mut vote_state_update = VoteStateUpdate::from(vec![(2, 2), (1, 3), (vote_slot, 1)]); vote_state_update.hash = vote_slot_hash; assert_eq!( - vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes), + vote_state.check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()) + ), Err(VoteError::SlotsNotOrdered), ); @@ -3446,8 +3702,11 @@ mod tests { let mut vote_state_update = VoteStateUpdate::from(vec![(2, 2), (2, 2), (vote_slot, 1)]); vote_state_update.hash = vote_slot_hash; assert_eq!( - vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes), + vote_state.check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()), + ), Err(VoteError::SlotsNotOrdered), ); } @@ -3455,7 +3714,7 @@ mod tests { #[test] fn test_check_update_vote_state_older_than_history_slots_filtered() { let slot_hashes = build_slot_hashes(vec![1, 2, 3, 4]); - let vote_state = build_vote_state(vec![1, 2, 3, 4], &slot_hashes); + let mut vote_state = build_vote_state(vec![1, 2, 3, 4], &slot_hashes); // Test with a `vote_state_update` where there: // 1) Exists a slot less than `earliest_slot_in_history` @@ -3470,24 +3729,46 @@ mod tests { .unwrap() .1; let missing_older_than_history_slot = earliest_slot_in_history - 1; - let mut vote_state_update = - VoteStateUpdate::from(vec![(missing_older_than_history_slot, 2), (vote_slot, 3)]); + let mut vote_state_update = VoteStateUpdate::from(vec![ + (1, 4), + (missing_older_than_history_slot, 2), + (vote_slot, 3), + ]); vote_state_update.hash = vote_slot_hash; vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes) + .check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()), + ) .unwrap(); // Check the earlier slot was filtered out assert_eq!( vote_state_update + .clone() .lockouts .into_iter() .collect::>(), - vec![Lockout { - slot: vote_slot, - confirmation_count: 3, - }] + vec![ + Lockout { + slot: 1, + confirmation_count: 4, + }, + Lockout { + slot: vote_slot, + confirmation_count: 3, + } + ] ); + assert!(do_process_vote_state_update( + &mut vote_state, + &slot_hashes, + 0, + vote_state_update, + Some(&FeatureSet::all_enabled()), + ) + .is_ok()); } #[test] @@ -3500,8 +3781,8 @@ mod tests { #[test] fn test_check_update_vote_state_older_than_history_slots_not_filtered() { - let slot_hashes = build_slot_hashes(vec![1, 2, 3, 4]); - let vote_state = build_vote_state(vec![1, 2, 3, 4], &slot_hashes); + let slot_hashes = build_slot_hashes(vec![4]); + let mut vote_state = build_vote_state(vec![4], &slot_hashes); // Test with a `vote_state_update` where there: // 1) Exists a slot less than `earliest_slot_in_history` @@ -3517,35 +3798,48 @@ mod tests { .1; let existing_older_than_history_slot = 4; let mut vote_state_update = - VoteStateUpdate::from(vec![(existing_older_than_history_slot, 2), (vote_slot, 3)]); + VoteStateUpdate::from(vec![(existing_older_than_history_slot, 3), (vote_slot, 2)]); vote_state_update.hash = vote_slot_hash; vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes) + .check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()), + ) .unwrap(); // Check the earlier slot was *NOT* filtered out assert_eq!(vote_state_update.lockouts.len(), 2); assert_eq!( vote_state_update + .clone() .lockouts .into_iter() .collect::>(), vec![ Lockout { slot: existing_older_than_history_slot, - confirmation_count: 2, + confirmation_count: 3, }, Lockout { slot: vote_slot, - confirmation_count: 3, + confirmation_count: 2, } ] ); + assert!(do_process_vote_state_update( + &mut vote_state, + &slot_hashes, + 0, + vote_state_update, + Some(&FeatureSet::all_enabled()), + ) + .is_ok()); } #[test] fn test_check_update_vote_state_older_than_history_slots_filtered_and_not_filtered() { - let slot_hashes = build_slot_hashes(vec![1, 2, 3, 6]); - let vote_state = build_vote_state(vec![1, 2, 3, 6], &slot_hashes); + let slot_hashes = build_slot_hashes(vec![6]); + let mut vote_state = build_vote_state(vec![6], &slot_hashes); // Test with a `vote_state_update` where there exists both a slot: // 1) Less than `earliest_slot_in_history` @@ -3577,11 +3871,16 @@ mod tests { ]); vote_state_update.hash = vote_slot_hash; vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes) + .check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()), + ) .unwrap(); assert_eq!(vote_state_update.lockouts.len(), 3); assert_eq!( vote_state_update + .clone() .lockouts .into_iter() .collect::>(), @@ -3600,6 +3899,14 @@ mod tests { } ] ); + assert!(do_process_vote_state_update( + &mut vote_state, + &slot_hashes, + 0, + vote_state_update, + Some(&FeatureSet::all_enabled()), + ) + .is_ok()); } #[test] @@ -3626,8 +3933,11 @@ mod tests { VoteStateUpdate::from(vec![(missing_vote_slot, 2), (vote_slot, 3)]); vote_state_update.hash = vote_slot_hash; assert_eq!( - vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes), + vote_state.check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()) + ), Err(VoteError::SlotsMismatch), ); @@ -3642,8 +3952,11 @@ mod tests { ]); vote_state_update.hash = vote_slot_hash; assert_eq!( - vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes), + vote_state.check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()) + ), Err(VoteError::SlotsMismatch), ); } @@ -3651,7 +3964,7 @@ mod tests { #[test] fn test_check_update_vote_state_root_on_different_fork() { let slot_hashes = build_slot_hashes(vec![2, 4, 6, 8]); - let vote_state = build_vote_state(vec![2, 4, 6], &slot_hashes); + let vote_state = build_vote_state(vec![6], &slot_hashes); // Test with a `vote_state_update` where: // 1) The root is not present in slot hashes history @@ -3661,8 +3974,9 @@ mod tests { let new_root = 3; // Have to vote for a slot greater than the last vote in the vote state to avoid VoteTooOld - // errors - let vote_slot = vote_state.votes.back().unwrap().slot + 2; + // errors, but also this slot must be present in SlotHashes + let vote_slot = 8; + assert_eq!(vote_slot, slot_hashes.first().unwrap().0); let vote_slot_hash = slot_hashes .iter() .find(|(slot, _hash)| *slot == vote_slot) @@ -3672,8 +3986,11 @@ mod tests { vote_state_update.hash = vote_slot_hash; vote_state_update.root = Some(new_root); assert_eq!( - vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes), + vote_state.check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()) + ), Err(VoteError::RootOnDifferentFork), ); } @@ -3693,8 +4010,11 @@ mod tests { let mut vote_state_update = VoteStateUpdate::from(vec![(8, 2), (missing_vote_slot, 3)]); vote_state_update.hash = vote_slot_hash; assert_eq!( - vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes), + vote_state.check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()) + ), Err(VoteError::SlotsMismatch), ); } @@ -3702,7 +4022,7 @@ mod tests { #[test] fn test_check_update_vote_state_slot_all_slot_hashes_in_update_ok() { let slot_hashes = build_slot_hashes(vec![2, 4, 6, 8]); - let vote_state = build_vote_state(vec![2, 4, 6], &slot_hashes); + let mut vote_state = build_vote_state(vec![2, 4, 6], &slot_hashes); // Test with a `vote_state_update` where every slot in the history is // in the update @@ -3719,12 +4039,17 @@ mod tests { VoteStateUpdate::from(vec![(2, 4), (4, 3), (6, 2), (vote_slot, 1)]); vote_state_update.hash = vote_slot_hash; vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes) + .check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()), + ) .unwrap(); // Nothing in the update should have been filtered out assert_eq!( vote_state_update + .clone() .lockouts .into_iter() .collect::>(), @@ -3747,14 +4072,23 @@ mod tests { } ] ); + + assert!(do_process_vote_state_update( + &mut vote_state, + &slot_hashes, + 0, + vote_state_update, + Some(&FeatureSet::all_enabled()), + ) + .is_ok()); } #[test] fn test_check_update_vote_state_slot_some_slot_hashes_in_update_ok() { let slot_hashes = build_slot_hashes(vec![2, 4, 6, 8, 10]); - let vote_state = build_vote_state(vec![6], &slot_hashes); + let mut vote_state = build_vote_state(vec![6], &slot_hashes); - // Test with a `vote_state_update` where every only some slots in the history are + // Test with a `vote_state_update` where only some slots in the history are // in the update, and others slots in the history are missing. // Have to vote for a slot greater than the last vote in the vote state to avoid VoteTooOld @@ -3768,12 +4102,17 @@ mod tests { let mut vote_state_update = VoteStateUpdate::from(vec![(4, 2), (vote_slot, 1)]); vote_state_update.hash = vote_slot_hash; vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes) + .check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()), + ) .unwrap(); // Nothing in the update should have been filtered out assert_eq!( vote_state_update + .clone() .lockouts .into_iter() .collect::>(), @@ -3788,6 +4127,20 @@ mod tests { } ] ); + + // Because 6 from the original VoteState + // should not have been popped off in the proposed state, + // we should get a lockout conflict + assert_eq!( + do_process_vote_state_update( + &mut vote_state, + &slot_hashes, + 0, + vote_state_update, + Some(&FeatureSet::all_enabled()) + ), + Err(VoteError::LockoutConflict) + ); } #[test] @@ -3805,8 +4158,11 @@ mod tests { VoteStateUpdate::from(vec![(2, 4), (4, 3), (6, 2), (vote_slot, 1)]); vote_state_update.hash = vote_slot_hash; assert_eq!( - vote_state - .check_update_vote_state_slots_are_valid(&mut vote_state_update, &slot_hashes), + vote_state.check_update_vote_state_slots_are_valid( + &mut vote_state_update, + &slot_hashes, + Some(&FeatureSet::all_enabled()) + ), Err(VoteError::SlotHashMismatch), ); } diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index ef07b320e0cba6..d9f054ce7e2040 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -500,6 +500,10 @@ pub mod incremental_snapshot_only_incremental_hash_calculation { solana_sdk::declare_id!("25vqsfjk7Nv1prsQJmA4Xu1bN61s8LXCBGUPp8Rfy1UF"); } +pub mod vote_state_update_root_fix { + solana_sdk::declare_id!("G74BkWBzmsByZ1kxHy44H3wjwp5hp7JbrGRuDpco22tY"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -619,6 +623,7 @@ lazy_static! { (concurrent_replay_of_forks::id(), "Allow slots from different forks to be replayed concurrently #26465"), (check_ping_ancestor_requests::id(), "ancestor hash repair socket ping/pong support #26963"), (incremental_snapshot_only_incremental_hash_calculation::id(), "only hash accounts in incremental snapshot during incremental snapshot creation #26799"), + (vote_state_update_root_fix::id(), "fix root in vote state updates #27361"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()