From 69d71f8f86bfa91db7d08aa12833433479837fa1 Mon Sep 17 00:00:00 2001 From: behzad nouri Date: Mon, 3 Jan 2022 21:07:47 +0000 Subject: [PATCH] removes epoch_authorized_voters from VoteTracker (#22192) https://github.com/solana-labs/solana/pull/22169 verifies authorized-voter early on in vote-listener pipeline; and so VoteTracker no longer needs to maintain and check for epoch authorized voters. --- core/src/cluster_info_vote_listener.rs | 359 ++----------------------- 1 file changed, 22 insertions(+), 337 deletions(-) diff --git a/core/src/cluster_info_vote_listener.rs b/core/src/cluster_info_vote_listener.rs index 9691895f38..1ed08b0e78 100644 --- a/core/src/cluster_info_vote_listener.rs +++ b/core/src/cluster_info_vote_listener.rs @@ -28,15 +28,11 @@ use { rpc_subscriptions::RpcSubscriptions, }, solana_runtime::{ - bank::Bank, - bank_forks::BankForks, - commitment::VOTE_THRESHOLD_SIZE, - epoch_stakes::{EpochAuthorizedVoters, EpochStakes}, - vote_sender_types::ReplayVoteReceiver, + bank::Bank, bank_forks::BankForks, commitment::VOTE_THRESHOLD_SIZE, + epoch_stakes::EpochStakes, vote_sender_types::ReplayVoteReceiver, }, solana_sdk::{ - clock::{Epoch, Slot, DEFAULT_MS_PER_SLOT, DEFAULT_TICKS_PER_SLOT}, - epoch_schedule::EpochSchedule, + clock::{Slot, DEFAULT_MS_PER_SLOT, DEFAULT_TICKS_PER_SLOT}, hash::Hash, pubkey::Pubkey, signature::Signature, @@ -49,6 +45,7 @@ use { }, std::{ collections::{HashMap, HashSet}, + iter::repeat, sync::{ atomic::{AtomicBool, Ordering}, Arc, Mutex, RwLock, @@ -60,7 +57,6 @@ use { // Map from a vote account to the authorized voter for an epoch pub type ThresholdConfirmedSlots = Vec<(Slot, Hash)>; -pub type VotedHashUpdates = HashMap>; pub type VerifiedLabelVotePacketsSender = CrossbeamSender>; pub type VerifiedLabelVotePacketsReceiver = CrossbeamReceiver>; pub type VerifiedVoteTransactionsSender = CrossbeamSender>; @@ -87,14 +83,14 @@ pub struct SlotVoteTracker { } impl SlotVoteTracker { - pub fn get_voted_slot_updates(&mut self) -> Option> { + pub(crate) fn get_voted_slot_updates(&mut self) -> Option> { self.voted_slot_updates.take() } - pub fn get_or_insert_optimistic_votes_tracker(&mut self, hash: Hash) -> &mut VoteStakeTracker { + fn get_or_insert_optimistic_votes_tracker(&mut self, hash: Hash) -> &mut VoteStakeTracker { self.optimistic_votes_tracker.entry(hash).or_default() } - pub fn optimistic_votes_tracker(&self, hash: &Hash) -> Option<&VoteStakeTracker> { + pub(crate) fn optimistic_votes_tracker(&self, hash: &Hash) -> Option<&VoteStakeTracker> { self.optimistic_votes_tracker.get(hash) } } @@ -103,82 +99,29 @@ impl SlotVoteTracker { pub struct VoteTracker { // Map from a slot to a set of validators who have voted for that slot slot_vote_trackers: RwLock>>>, - // Don't track votes from people who are not staked, acts as a spam filter - epoch_authorized_voters: RwLock>>, - leader_schedule_epoch: RwLock, - current_epoch: RwLock, - epoch_schedule: EpochSchedule, } impl VoteTracker { - pub fn new(root_bank: &Bank) -> Self { - let current_epoch = root_bank.epoch(); - let vote_tracker = Self { - leader_schedule_epoch: RwLock::new(current_epoch), - current_epoch: RwLock::new(current_epoch), - epoch_schedule: *root_bank.epoch_schedule(), - ..VoteTracker::default() - }; + pub(crate) fn new(root_bank: &Bank) -> Self { + let vote_tracker = VoteTracker::default(); vote_tracker.progress_with_new_root_bank(root_bank); - assert_eq!( - *vote_tracker.leader_schedule_epoch.read().unwrap(), - root_bank.get_leader_schedule_epoch(root_bank.slot()) - ); - assert_eq!(*vote_tracker.current_epoch.read().unwrap(), current_epoch,); vote_tracker } - pub fn get_or_insert_slot_tracker(&self, slot: Slot) -> Arc> { - let mut slot_tracker = self.slot_vote_trackers.read().unwrap().get(&slot).cloned(); - - if slot_tracker.is_none() { - let new_slot_tracker = Arc::new(RwLock::new(SlotVoteTracker { - voted: HashMap::new(), - optimistic_votes_tracker: HashMap::default(), - voted_slot_updates: None, - gossip_only_stake: 0, - })); - self.slot_vote_trackers - .write() - .unwrap() - .insert(slot, new_slot_tracker.clone()); - slot_tracker = Some(new_slot_tracker); + fn get_or_insert_slot_tracker(&self, slot: Slot) -> Arc> { + if let Some(slot_vote_tracker) = self.slot_vote_trackers.read().unwrap().get(&slot) { + return slot_vote_tracker.clone(); } - - slot_tracker.unwrap() + let mut slot_vote_trackers = self.slot_vote_trackers.write().unwrap(); + slot_vote_trackers.entry(slot).or_default().clone() } - pub fn get_slot_vote_tracker(&self, slot: Slot) -> Option>> { + pub(crate) fn get_slot_vote_tracker(&self, slot: Slot) -> Option>> { self.slot_vote_trackers.read().unwrap().get(&slot).cloned() } - pub fn get_authorized_voter(&self, pubkey: &Pubkey, slot: Slot) -> Option { - let epoch = self.epoch_schedule.get_epoch(slot); - self.epoch_authorized_voters - .read() - .unwrap() - .get(&epoch) - .map(|epoch_authorized_voters| epoch_authorized_voters.get(pubkey)) - .unwrap_or(None) - .cloned() - } - - pub fn vote_contains_authorized_voter( - vote_tx: &Transaction, - authorized_voter: &Pubkey, - ) -> bool { - let message = &vote_tx.message; - for (i, key) in message.account_keys.iter().enumerate() { - if message.is_signer(i) && key == authorized_voter { - return true; - } - } - - false - } - #[cfg(test)] - pub fn insert_vote(&self, slot: Slot, pubkey: Pubkey) { + pub(crate) fn insert_vote(&self, slot: Slot, pubkey: Pubkey) { let mut w_slot_vote_trackers = self.slot_vote_trackers.write().unwrap(); let slot_vote_tracker = w_slot_vote_trackers.entry(slot).or_default(); @@ -193,59 +136,16 @@ impl VoteTracker { } } - fn progress_leader_schedule_epoch(&self, root_bank: &Bank) { - // Update with any newly calculated epoch state about future epochs - let start_leader_schedule_epoch = *self.leader_schedule_epoch.read().unwrap(); - let mut greatest_leader_schedule_epoch = start_leader_schedule_epoch; - for leader_schedule_epoch in - start_leader_schedule_epoch..=root_bank.get_leader_schedule_epoch(root_bank.slot()) - { - let exists = self - .epoch_authorized_voters - .read() - .unwrap() - .contains_key(&leader_schedule_epoch); - if !exists { - let epoch_authorized_voters = root_bank - .epoch_stakes(leader_schedule_epoch) - .unwrap() - .epoch_authorized_voters() - .clone(); - self.epoch_authorized_voters - .write() - .unwrap() - .insert(leader_schedule_epoch, epoch_authorized_voters); - greatest_leader_schedule_epoch = leader_schedule_epoch; - } - } - - if greatest_leader_schedule_epoch != start_leader_schedule_epoch { - *self.leader_schedule_epoch.write().unwrap() = greatest_leader_schedule_epoch; - } - } - fn purge_stale_state(&self, root_bank: &Bank) { // Purge any outdated slot data let new_root = root_bank.slot(); - let root_epoch = root_bank.epoch(); self.slot_vote_trackers .write() .unwrap() .retain(|slot, _| *slot >= new_root); - - let current_epoch = *self.current_epoch.read().unwrap(); - if root_epoch != current_epoch { - // If root moved to a new epoch, purge outdated state - self.epoch_authorized_voters - .write() - .unwrap() - .retain(|epoch, _| *epoch >= root_epoch); - *self.current_epoch.write().unwrap() = root_epoch; - } } fn progress_with_new_root_bank(&self, root_bank: &Bank) { - self.progress_leader_schedule_epoch(root_bank); self.purge_stale_state(root_bank); } } @@ -788,39 +688,6 @@ impl ClusterInfoVoteListener { } } - fn filter_gossip_votes( - vote_tracker: &VoteTracker, - vote_pubkey: &Pubkey, - vote: &VoteTransaction, - gossip_tx: &Transaction, - ) -> bool { - if vote.is_empty() { - return false; - } - let last_vote_slot = vote.last_voted_slot().unwrap(); - // Votes from gossip need to be verified as they have not been - // verified by the replay pipeline. Determine the authorized voter - // based on the last vote slot. This will drop votes from authorized - // voters trying to make votes for slots earlier than the epoch for - // which they are authorized - let actual_authorized_voter = - vote_tracker.get_authorized_voter(vote_pubkey, last_vote_slot); - - if actual_authorized_voter.is_none() { - return false; - } - - // Voting without the correct authorized pubkey, dump the vote - if !VoteTracker::vote_contains_authorized_voter( - gossip_tx, - &actual_authorized_voter.unwrap(), - ) { - return false; - } - - true - } - fn filter_and_confirm_with_new_votes( vote_tracker: &VoteTracker, gossip_vote_txs: Vec, @@ -836,17 +703,12 @@ impl ClusterInfoVoteListener { let mut new_optimistic_confirmed_slots = vec![]; // Process votes from gossip and ReplayStage - for (is_gossip, (vote_pubkey, vote, _)) in gossip_vote_txs + let votes = gossip_vote_txs .iter() - .filter_map(|gossip_tx| { - vote_transaction::parse_vote_transaction(gossip_tx) - .filter(|(vote_pubkey, vote, _)| { - Self::filter_gossip_votes(vote_tracker, vote_pubkey, vote, gossip_tx) - }) - .map(|v| (true, v)) - }) - .chain(replayed_votes.into_iter().map(|v| (false, v))) - { + .filter_map(vote_transaction::parse_vote_transaction) + .zip(repeat(/*is_gossip:*/ true)) + .chain(replayed_votes.into_iter().zip(repeat(/*is_gossip:*/ false))); + for ((vote_pubkey, vote, _), is_gossip) in votes { Self::track_new_votes_and_notify_confirmations( vote, &vote_pubkey, @@ -994,73 +856,6 @@ mod tests { assert_eq!(packet_batches.len(), 1); } - fn run_vote_contains_authorized_voter(hash: Option) { - let node_keypair = Keypair::new(); - let vote_keypair = Keypair::new(); - let authorized_voter = Keypair::new(); - - let vote_tx = vote_transaction::new_vote_transaction( - vec![0], - Hash::default(), - Hash::default(), - &node_keypair, - &vote_keypair, - &authorized_voter, - hash, - ); - - // Check that the two signing keys pass the check - assert!(VoteTracker::vote_contains_authorized_voter( - &vote_tx, - &node_keypair.pubkey() - )); - - assert!(VoteTracker::vote_contains_authorized_voter( - &vote_tx, - &authorized_voter.pubkey() - )); - - // Non signing key shouldn't pass the check - assert!(!VoteTracker::vote_contains_authorized_voter( - &vote_tx, - &vote_keypair.pubkey() - )); - - // Set the authorized voter == vote keypair - let vote_tx = vote_transaction::new_vote_transaction( - vec![0], - Hash::default(), - Hash::default(), - &node_keypair, - &vote_keypair, - &vote_keypair, - hash, - ); - - // Check that the node_keypair and vote keypair pass the authorized voter check - assert!(VoteTracker::vote_contains_authorized_voter( - &vote_tx, - &node_keypair.pubkey() - )); - - assert!(VoteTracker::vote_contains_authorized_voter( - &vote_tx, - &vote_keypair.pubkey() - )); - - // The other keypair should not pass the check - assert!(!VoteTracker::vote_contains_authorized_voter( - &vote_tx, - &authorized_voter.pubkey() - )); - } - - #[test] - fn test_vote_contains_authorized_voter() { - run_vote_contains_authorized_voter(None); - run_vote_contains_authorized_voter(Some(Hash::default())); - } - #[test] fn test_update_new_root() { let (vote_tracker, bank, _, _) = setup(); @@ -1094,15 +889,11 @@ mod tests { .get_first_slot_in_epoch(current_epoch + 1), ); vote_tracker.progress_with_new_root_bank(&new_epoch_bank); - assert_eq!( - *vote_tracker.current_epoch.read().unwrap(), - current_epoch + 1 - ); } #[test] fn test_update_new_leader_schedule_epoch() { - let (vote_tracker, bank, _, _) = setup(); + let (_, bank, _, _) = setup(); // Check outdated slots are purged with new root let leader_schedule_epoch = bank.get_leader_schedule_epoch(bank.slot()); @@ -1120,25 +911,6 @@ mod tests { bank.get_leader_schedule_epoch(next_leader_schedule_computed), next_leader_schedule_epoch ); - let next_leader_schedule_bank = - Bank::new_from_parent(&bank, &Pubkey::default(), next_leader_schedule_computed); - vote_tracker.progress_leader_schedule_epoch(&next_leader_schedule_bank); - assert_eq!( - *vote_tracker.leader_schedule_epoch.read().unwrap(), - next_leader_schedule_epoch - ); - assert_eq!( - vote_tracker - .epoch_authorized_voters - .read() - .unwrap() - .get(&next_leader_schedule_epoch) - .unwrap(), - next_leader_schedule_bank - .epoch_stakes(next_leader_schedule_epoch) - .unwrap() - .epoch_authorized_voters() - ); } #[test] @@ -1581,59 +1353,6 @@ mod tests { run_test_process_votes3(Some(Hash::default())); } - #[test] - fn test_get_voters_by_epoch() { - // Create some voters at genesis - let (vote_tracker, bank, validator_voting_keypairs, _) = setup(); - let last_known_epoch = bank.get_leader_schedule_epoch(bank.slot()); - let last_known_slot = bank - .epoch_schedule() - .get_last_slot_in_epoch(last_known_epoch); - - // Check we can get the authorized voters - for keypairs in &validator_voting_keypairs { - assert!(vote_tracker - .get_authorized_voter(&keypairs.vote_keypair.pubkey(), last_known_slot) - .is_some()); - assert!(vote_tracker - .get_authorized_voter(&keypairs.vote_keypair.pubkey(), last_known_slot + 1) - .is_none()); - } - - // Create the set of relevant voters for the next epoch - let new_epoch = last_known_epoch + 1; - let first_slot_in_new_epoch = bank.epoch_schedule().get_first_slot_in_epoch(new_epoch); - let new_keypairs: Vec<_> = (0..10).map(|_| ValidatorVoteKeypairs::new_rand()).collect(); - let new_epoch_authorized_voters: HashMap<_, _> = new_keypairs - .iter() - .chain(validator_voting_keypairs[0..5].iter()) - .map(|keypair| (keypair.vote_keypair.pubkey(), keypair.vote_keypair.pubkey())) - .collect(); - - vote_tracker - .epoch_authorized_voters - .write() - .unwrap() - .insert(new_epoch, Arc::new(new_epoch_authorized_voters)); - - // These keypairs made it into the new epoch - for keypairs in new_keypairs - .iter() - .chain(validator_voting_keypairs[0..5].iter()) - { - assert!(vote_tracker - .get_authorized_voter(&keypairs.vote_keypair.pubkey(), first_slot_in_new_epoch) - .is_some()); - } - - // These keypairs were not refreshed in new epoch - for keypairs in validator_voting_keypairs[5..10].iter() { - assert!(vote_tracker - .get_authorized_voter(&keypairs.vote_keypair.pubkey(), first_slot_in_new_epoch) - .is_none()); - } - } - #[test] fn test_vote_tracker_references() { // Create some voters at genesis @@ -1699,17 +1418,6 @@ mod tests { // Setup next epoch let old_epoch = bank.get_leader_schedule_epoch(bank.slot()); let new_epoch = old_epoch + 1; - let new_epoch_vote_accounts: HashMap<_, _> = vec![( - validator0_keypairs.vote_keypair.pubkey(), - validator0_keypairs.vote_keypair.pubkey(), - )] - .into_iter() - .collect(); - vote_tracker - .epoch_authorized_voters - .write() - .unwrap() - .insert(new_epoch, Arc::new(new_epoch_vote_accounts)); // Test with votes across two epochs let first_slot_in_new_epoch = bank.epoch_schedule().get_first_slot_in_epoch(new_epoch); @@ -1783,29 +1491,6 @@ mod tests { optimistically_confirmed_bank, )); - // Integrity Checks - let current_epoch = bank.epoch(); - let leader_schedule_epoch = bank.get_leader_schedule_epoch(bank.slot()); - - // Check the vote tracker has all the known epoch state on construction - for epoch in current_epoch..=leader_schedule_epoch { - assert_eq!( - vote_tracker - .epoch_authorized_voters - .read() - .unwrap() - .get(&epoch) - .unwrap(), - bank.epoch_stakes(epoch).unwrap().epoch_authorized_voters() - ); - } - - // Check the epoch state is correct - assert_eq!( - *vote_tracker.leader_schedule_epoch.read().unwrap(), - leader_schedule_epoch, - ); - assert_eq!(*vote_tracker.current_epoch.read().unwrap(), current_epoch); ( Arc::new(vote_tracker), bank,