From 3b0b0ba07d345ef86e270187a1a7d99bd0da7f4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Wed, 14 Jun 2023 01:31:27 +0200 Subject: [PATCH] Cleanup - filter_votes_outside_slot_hashes (#31760) filter_votes_outside_slot_hashes --- programs/vote/src/vote_state/mod.rs | 158 +++++++++------------------- 1 file changed, 50 insertions(+), 108 deletions(-) diff --git a/programs/vote/src/vote_state/mod.rs b/programs/vote/src/vote_state/mod.rs index f1508e6c12..ce60715364 100644 --- a/programs/vote/src/vote_state/mod.rs +++ b/programs/vote/src/vote_state/mod.rs @@ -10,7 +10,7 @@ use { account::{AccountSharedData, ReadableAccount, WritableAccount}, clock::{Epoch, Slot, UnixTimestamp}, epoch_schedule::EpochSchedule, - feature_set::{self, filter_votes_outside_slot_hashes, FeatureSet}, + feature_set::{self, FeatureSet}, hash::Hash, instruction::InstructionError, pubkey::Pubkey, @@ -722,54 +722,54 @@ pub fn process_new_vote_state( Ok(()) } -pub fn process_vote( +fn process_vote_unfiltered( vote_state: &mut VoteState, + vote_slots: &[Slot], vote: &Vote, slot_hashes: &[SlotHash], epoch: Epoch, - feature_set: Option<&FeatureSet>, ) -> Result<(), VoteError> { - if vote.slots.is_empty() { - return Err(VoteError::EmptySlots); - } - let filtered_vote_slots = feature_set.and_then(|feature_set| { - if feature_set.is_active(&filter_votes_outside_slot_hashes::id()) { - let earliest_slot_in_history = - slot_hashes.last().map(|(slot, _hash)| *slot).unwrap_or(0); - Some( - vote.slots - .iter() - .filter(|slot| **slot >= earliest_slot_in_history) - .cloned() - .collect::>(), - ) - } else { - None - } - }); - - let vote_slots = filtered_vote_slots.as_ref().unwrap_or(&vote.slots); - if vote_slots.is_empty() { - return Err(VoteError::VotesTooOldAllFiltered); - } - check_slots_are_valid(vote_state, vote_slots, &vote.hash, slot_hashes)?; - vote_slots .iter() .for_each(|s| vote_state.process_next_vote_slot(*s, epoch)); Ok(()) } +pub fn process_vote( + vote_state: &mut VoteState, + vote: &Vote, + slot_hashes: &[SlotHash], + epoch: Epoch, +) -> Result<(), VoteError> { + if vote.slots.is_empty() { + return Err(VoteError::EmptySlots); + } + let earliest_slot_in_history = slot_hashes.last().map(|(slot, _hash)| *slot).unwrap_or(0); + let vote_slots = vote + .slots + .iter() + .filter(|slot| **slot >= earliest_slot_in_history) + .cloned() + .collect::>(); + if vote_slots.is_empty() { + return Err(VoteError::VotesTooOldAllFiltered); + } + process_vote_unfiltered(vote_state, &vote_slots, vote, slot_hashes, epoch) +} + /// "unchecked" functions used by tests and Tower pub fn process_vote_unchecked(vote_state: &mut VoteState, vote: Vote) { + if vote.slots.is_empty() { + return; + } let slot_hashes: Vec<_> = vote.slots.iter().rev().map(|x| (*x, vote.hash)).collect(); - let _ignored = process_vote( + let _ignored = process_vote_unfiltered( vote_state, + &vote.slots, &vote, &slot_hashes, vote_state.current_epoch(), - None, ); } @@ -1017,13 +1017,7 @@ pub fn process_vote_with_account( ) -> Result<(), InstructionError> { let mut vote_state = verify_and_get_vote_state(vote_account, clock, signers)?; - process_vote( - &mut vote_state, - vote, - slot_hashes, - clock.epoch, - Some(feature_set), - )?; + process_vote(&mut vote_state, vote, slot_hashes, clock.epoch)?; if let Some(timestamp) = vote.timestamp { vote.slots .iter() @@ -1180,19 +1174,19 @@ mod tests { vote_state.increment_credits(0, 100); assert_eq!( vote_state - .set_new_authorized_voter(&solana_sdk::pubkey::new_rand(), 0, 1, |_pubkey| Ok(()),), + .set_new_authorized_voter(&solana_sdk::pubkey::new_rand(), 0, 1, |_pubkey| Ok(())), Ok(()) ); vote_state.increment_credits(1, 200); assert_eq!( vote_state - .set_new_authorized_voter(&solana_sdk::pubkey::new_rand(), 1, 2, |_pubkey| Ok(()),), + .set_new_authorized_voter(&solana_sdk::pubkey::new_rand(), 1, 2, |_pubkey| Ok(())), Ok(()) ); vote_state.increment_credits(2, 300); assert_eq!( vote_state - .set_new_authorized_voter(&solana_sdk::pubkey::new_rand(), 2, 3, |_pubkey| Ok(()),), + .set_new_authorized_voter(&solana_sdk::pubkey::new_rand(), 2, 3, |_pubkey| Ok(())), Ok(()) ); // Simulate votes having occurred @@ -1494,23 +1488,11 @@ mod tests { let slot_hashes: Vec<_> = vote.slots.iter().rev().map(|x| (*x, vote.hash)).collect(); assert_eq!( - process_vote( - &mut vote_state_a, - &vote, - &slot_hashes, - 0, - Some(&FeatureSet::default()) - ), + process_vote(&mut vote_state_a, &vote, &slot_hashes, 0), Ok(()) ); assert_eq!( - process_vote( - &mut vote_state_b, - &vote, - &slot_hashes, - 0, - Some(&FeatureSet::default()) - ), + process_vote(&mut vote_state_b, &vote, &slot_hashes, 0), Ok(()) ); assert_eq!(recent_votes(&vote_state_a), recent_votes(&vote_state_b)); @@ -1523,24 +1505,12 @@ mod tests { let vote = Vote::new(vec![0], Hash::default()); let slot_hashes: Vec<_> = vec![(0, vote.hash)]; assert_eq!( - process_vote( - &mut vote_state, - &vote, - &slot_hashes, - 0, - Some(&FeatureSet::default()) - ), + process_vote(&mut vote_state, &vote, &slot_hashes, 0), Ok(()) ); let recent = recent_votes(&vote_state); assert_eq!( - process_vote( - &mut vote_state, - &vote, - &slot_hashes, - 0, - Some(&FeatureSet::default()) - ), + process_vote(&mut vote_state, &vote, &slot_hashes, 0), Err(VoteError::VoteTooOld) ); assert_eq!(recent, recent_votes(&vote_state)); @@ -1600,13 +1570,7 @@ mod tests { let vote = Vote::new(vec![0], Hash::default()); let slot_hashes: Vec<_> = vec![(*vote.slots.last().unwrap(), vote.hash)]; assert_eq!( - process_vote( - &mut vote_state, - &vote, - &slot_hashes, - 0, - Some(&FeatureSet::default()) - ), + process_vote(&mut vote_state, &vote, &slot_hashes, 0), Ok(()) ); assert_eq!( @@ -1622,13 +1586,7 @@ mod tests { let vote = Vote::new(vec![0], Hash::default()); let slot_hashes: Vec<_> = vec![(*vote.slots.last().unwrap(), vote.hash)]; assert_eq!( - process_vote( - &mut vote_state, - &vote, - &slot_hashes, - 0, - Some(&FeatureSet::default()) - ), + process_vote(&mut vote_state, &vote, &slot_hashes, 0), Ok(()) ); @@ -1647,13 +1605,7 @@ mod tests { let vote = Vote::new(vec![0], Hash::default()); let slot_hashes: Vec<_> = vec![(*vote.slots.last().unwrap(), vote.hash)]; assert_eq!( - process_vote( - &mut vote_state, - &vote, - &slot_hashes, - 0, - Some(&FeatureSet::default()) - ), + process_vote(&mut vote_state, &vote, &slot_hashes, 0), Ok(()) ); @@ -1670,7 +1622,7 @@ mod tests { let vote = Vote::new(vec![], Hash::default()); assert_eq!( - process_vote(&mut vote_state, &vote, &[], 0, Some(&FeatureSet::default())), + process_vote(&mut vote_state, &vote, &[], 0), Err(VoteError::EmptySlots) ); } @@ -1788,7 +1740,7 @@ mod tests { let current_epoch = vote_state1.current_epoch(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None,), + process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), Err(VoteError::TooManyVotes) ); } @@ -1849,7 +1801,7 @@ mod tests { .into_iter() .collect(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None,), + process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), Err(VoteError::ZeroConfirmations) ); @@ -1860,7 +1812,7 @@ mod tests { .into_iter() .collect(); assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None,), + process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), Err(VoteError::ZeroConfirmations) ); } @@ -2013,7 +1965,7 @@ mod tests { // Slot 7 should have expired slot 0 assert_eq!( - process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None,), + process_new_vote_state(&mut vote_state1, bad_votes, None, None, current_epoch, None), Err(VoteError::NewVoteStateLockoutMismatch) ); } @@ -2316,10 +2268,6 @@ mod tests { #[test] fn test_filter_old_votes() { - // Enable feature - let mut feature_set = FeatureSet::default(); - feature_set.activate(&filter_votes_outside_slot_hashes::id(), 0); - let mut vote_state = VoteState::default(); let old_vote_slot = 1; let vote = Vote::new(vec![old_vote_slot], Hash::default()); @@ -2328,7 +2276,7 @@ mod tests { // error with `VotesTooOldAllFiltered` let slot_hashes = vec![(3, Hash::new_unique()), (2, Hash::new_unique())]; assert_eq!( - process_vote(&mut vote_state, &vote, &slot_hashes, 0, Some(&feature_set),), + process_vote(&mut vote_state, &vote, &slot_hashes, 0), Err(VoteError::VotesTooOldAllFiltered) ); @@ -2342,7 +2290,7 @@ mod tests { .1; let vote = Vote::new(vec![old_vote_slot, vote_slot], vote_slot_hash); - process_vote(&mut vote_state, &vote, &slot_hashes, 0, Some(&feature_set)).unwrap(); + process_vote(&mut vote_state, &vote, &slot_hashes, 0).unwrap(); assert_eq!( vote_state .votes @@ -2370,14 +2318,8 @@ mod tests { .find(|(slot, _hash)| slot == vote_slots.last().unwrap()) .unwrap() .1; - process_vote( - &mut vote_state, - &Vote::new(vote_slots, vote_hash), - slot_hashes, - 0, - None, - ) - .unwrap(); + let vote = Vote::new(vote_slots, vote_hash); + process_vote_unfiltered(&mut vote_state, &vote.slots, &vote, slot_hashes, 0).unwrap(); } vote_state