From f3d556e3f9ff92b43b70ceed4b8aa52ffe1ceb0a Mon Sep 17 00:00:00 2001 From: carllin Date: Thu, 26 Mar 2020 17:55:17 -0700 Subject: [PATCH] Refactor VoteTracker (#9084) * Refactor VoteTracker Co-authored-by: Carl --- core/src/cluster_info_vote_listener.rs | 727 ++++++++++--------------- core/src/consensus.rs | 7 +- core/src/replay_stage.rs | 4 +- runtime/src/genesis_utils.rs | 5 +- 4 files changed, 287 insertions(+), 456 deletions(-) diff --git a/core/src/cluster_info_vote_listener.rs b/core/src/cluster_info_vote_listener.rs index d312ad2632..bff892cc95 100644 --- a/core/src/cluster_info_vote_listener.rs +++ b/core/src/cluster_info_vote_listener.rs @@ -14,16 +14,15 @@ use log::*; use solana_ledger::bank_forks::BankForks; use solana_metrics::inc_new_counter_debug; use solana_perf::packet::{self, Packets}; -use solana_runtime::bank::Bank; +use solana_runtime::{bank::Bank, epoch_stakes::EpochAuthorizedVoters}; use solana_sdk::{ - account::Account, clock::{Epoch, Slot}, epoch_schedule::EpochSchedule, program_utils::limited_deserialize, pubkey::Pubkey, transaction::Transaction, }; -use solana_vote_program::{vote_instruction::VoteInstruction, vote_state::VoteState}; +use solana_vote_program::vote_instruction::VoteInstruction; use std::{ collections::{HashMap, HashSet}, sync::{ @@ -35,13 +34,12 @@ use std::{ }; // Map from a vote account to the authorized voter for an epoch -pub type EpochAuthorizedVoters = HashMap, Arc>; -pub type NodeIdToVoteAccounts = HashMap>>; pub type VerifiedVotePacketsSender = CrossbeamSender>; pub type VerifiedVotePacketsReceiver = CrossbeamReceiver>; pub type VerifiedVoteTransactionsSender = CrossbeamSender>; pub type VerifiedVoteTransactionsReceiver = CrossbeamReceiver>; +#[derive(Default)] pub struct SlotVoteTracker { voted: HashSet>, updates: Option>>, @@ -54,50 +52,33 @@ impl SlotVoteTracker { } } +#[derive(Default)] 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>, - // Map from node id to the set of associated vote accounts - node_id_to_vote_accounts: RwLock>, - all_pubkeys: RwLock>>, + epoch_authorized_voters: RwLock>>, + leader_schedule_epoch: RwLock, + current_epoch: RwLock, + keys: RwLock>>, epoch_schedule: EpochSchedule, } impl VoteTracker { pub fn new(root_bank: &Bank) -> Self { let current_epoch = root_bank.epoch(); - let leader_schedule_epoch = root_bank - .epoch_schedule() - .get_leader_schedule_epoch(root_bank.slot()); - let vote_tracker = Self { - epoch_authorized_voters: RwLock::new(HashMap::new()), - slot_vote_trackers: RwLock::new(HashMap::new()), - node_id_to_vote_accounts: RwLock::new(HashMap::new()), - all_pubkeys: RwLock::new(HashSet::new()), + leader_schedule_epoch: RwLock::new(current_epoch), + current_epoch: RwLock::new(current_epoch), epoch_schedule: *root_bank.epoch_schedule(), + ..VoteTracker::default() }; - - // Parse voter information about all the known epochs - for epoch in current_epoch..=leader_schedule_epoch { - let (new_epoch_authorized_voters, new_node_id_to_vote_accounts, new_pubkeys) = - VoteTracker::parse_epoch_state( - epoch, - root_bank - .epoch_vote_accounts(epoch) - .expect("Epoch vote accounts must exist"), - &vote_tracker.all_pubkeys.read().unwrap(), - ); - vote_tracker.process_new_leader_schedule_epoch_state( - epoch, - new_epoch_authorized_voters, - new_node_id_to_vote_accounts, - new_pubkeys, - ); - } - + vote_tracker.process_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 } @@ -105,24 +86,7 @@ impl VoteTracker { self.slot_vote_trackers.read().unwrap().get(&slot).cloned() } - // Returns Some if the given pubkey is a staked voter for the epoch at the given - // slot. Note this decision uses bank.EpochStakes not live stakes. - pub fn get_voter_pubkey(&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_key_value(pubkey) - .map(|(key, _)| key) - }) - .unwrap_or(None) - .cloned() - } - - pub fn get_authorized_voter(&self, pubkey: &Pubkey, slot: Slot) -> Option> { + pub fn get_authorized_voter(&self, pubkey: &Pubkey, slot: Slot) -> Option { let epoch = self.epoch_schedule.get_epoch(slot); self.epoch_authorized_voters .read() @@ -133,85 +97,6 @@ impl VoteTracker { .cloned() } - pub fn parse_epoch_state( - epoch: Epoch, - epoch_vote_acounts: &HashMap, - all_pubkeys: &HashSet>, - ) -> ( - EpochAuthorizedVoters, - NodeIdToVoteAccounts, - HashSet>, - ) { - let mut new_pubkeys = HashSet::new(); - let mut node_id_to_vote_accounts: NodeIdToVoteAccounts = HashMap::new(); - // Get all known vote accounts with nonzero stake and read out their - // authorized voters - let epoch_authorized_voters = epoch_vote_acounts - .iter() - .filter_map(|(key, (stake, account))| { - let vote_state = VoteState::from(&account); - if vote_state.is_none() { - datapoint_warn!( - "cluster_info_vote_listener", - ( - "warn", - format!("Unable to get vote_state from account {}", key), - String - ), - ); - return None; - } - let vote_state = vote_state.unwrap(); - if *stake > 0 { - // Read out the authorized voters - let mut authorized_voters = vote_state.authorized_voters().clone(); - authorized_voters.get_and_cache_authorized_voter_for_epoch(epoch); - let authorized_voter = authorized_voters - .get_authorized_voter(epoch) - .expect("Authorized voter for current epoch must be known"); - - // Get Arcs for all the needed keys - let unduplicated_authorized_voter_key = all_pubkeys - .get(&authorized_voter) - .cloned() - .unwrap_or_else(|| { - new_pubkeys - .get(&authorized_voter) - .cloned() - .unwrap_or_else(|| { - let new_key = Arc::new(authorized_voter); - new_pubkeys.insert(new_key.clone()); - new_key - }) - }); - - let unduplicated_key = all_pubkeys.get(key).cloned().unwrap_or_else(|| { - new_pubkeys.get(key).cloned().unwrap_or_else(|| { - let new_key = Arc::new(*key); - new_pubkeys.insert(new_key.clone()); - new_key - }) - }); - - node_id_to_vote_accounts - .entry(vote_state.node_pubkey) - .or_default() - .push(unduplicated_key.clone()); - - Some((unduplicated_key, unduplicated_authorized_voter_key)) - } else { - None - } - }) - .collect(); - - ( - epoch_authorized_voters, - node_id_to_vote_accounts, - new_pubkeys, - ) - } - pub fn vote_contains_authorized_voter( vote_tx: &Transaction, authorized_voter: &Pubkey, @@ -226,51 +111,82 @@ impl VoteTracker { false } - // Given a set of validator node ids `N` and vote accounts `V`, removes the vote accounts - // from `V` that belong to `N` - pub fn node_id_to_vote_accounts(&self, node_ids: &[Pubkey], slot: Slot) -> Vec> { - let epoch = self.epoch_schedule.get_epoch(slot); - if let Some(node_id_to_vote_accounts) = - self.node_id_to_vote_accounts.read().unwrap().get(&epoch) - { - node_ids - .iter() - .flat_map(|node_id| { - node_id_to_vote_accounts - .get(node_id) - .cloned() - .unwrap_or_else(|| vec![]) - .into_iter() - }) - .collect() + #[cfg(test)] + pub fn insert_vote(&self, slot: Slot, pubkey: Arc) { + let mut w_slot_vote_trackers = self.slot_vote_trackers.write().unwrap(); + + let slot_vote_tracker = w_slot_vote_trackers.entry(slot).or_default(); + + let mut w_slot_vote_tracker = slot_vote_tracker.write().unwrap(); + + w_slot_vote_tracker.voted.insert(pubkey.clone()); + if let Some(ref mut updates) = w_slot_vote_tracker.updates { + updates.push(pubkey.clone()) } else { - vec![] + w_slot_vote_tracker.updates = Some(vec![pubkey.clone()]); + } + + self.keys.write().unwrap().insert(pubkey); + } + + fn update_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 process_new_leader_schedule_epoch_state( - &self, - new_leader_schedule_epoch: Epoch, - new_epoch_authorized_voters: EpochAuthorizedVoters, - new_node_id_to_vote_accounts: NodeIdToVoteAccounts, - new_pubkeys: HashSet>, - ) { - self.epoch_authorized_voters + fn update_new_root(&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() - .insert(new_leader_schedule_epoch, new_epoch_authorized_voters); - self.node_id_to_vote_accounts - .write() - .unwrap() - .insert(new_leader_schedule_epoch, new_node_id_to_vote_accounts); - for key in new_pubkeys { - self.all_pubkeys.write().unwrap().insert(key); - } + .retain(|slot, _| *slot >= new_root); - self.all_pubkeys - .write() - .unwrap() - .retain(|pubkey| Arc::strong_count(pubkey) > 1); + 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.keys + .write() + .unwrap() + .retain(|pubkey| Arc::strong_count(pubkey) > 1); + *self.current_epoch.write().unwrap() = root_epoch; + } + } + + fn process_new_root_bank(&self, root_bank: &Bank) { + self.update_leader_schedule_epoch(root_bank); + self.update_new_root(root_bank); } } @@ -449,45 +365,16 @@ impl ClusterInfoVoteListener { vote_tracker: Arc, bank_forks: &RwLock, ) -> Result<()> { - let (mut old_leader_schedule_epoch, mut last_root) = { - let root_bank = bank_forks.read().unwrap().root_bank().clone(); - ( - root_bank.get_leader_schedule_epoch(root_bank.slot()), - root_bank.slot(), - ) - }; - loop { if exit.load(Ordering::Relaxed) { return Ok(()); } let root_bank = bank_forks.read().unwrap().root_bank().clone(); - if root_bank.slot() != last_root { - Self::process_new_root(&vote_tracker, root_bank.slot()); - last_root = root_bank.slot(); - } - - let new_leader_schedule_epoch = - { root_bank.get_leader_schedule_epoch(root_bank.slot()) }; - - if old_leader_schedule_epoch != new_leader_schedule_epoch { - assert!(vote_tracker - .epoch_authorized_voters - .read() - .unwrap() - .get(&new_leader_schedule_epoch) - .is_none()); - Self::process_new_leader_schedule_epoch( - &root_bank, - &vote_tracker, - new_leader_schedule_epoch, - ); - old_leader_schedule_epoch = new_leader_schedule_epoch; - } + vote_tracker.process_new_root_bank(&root_bank); if let Err(e) = - Self::get_and_process_votes(&vote_txs_receiver, &vote_tracker, last_root) + Self::get_and_process_votes(&vote_txs_receiver, &vote_tracker, root_bank.slot()) { match e { Error::CrossbeamRecvTimeoutError(RecvTimeoutError::Disconnected) => { @@ -573,21 +460,25 @@ impl ClusterInfoVoteListener { continue; } - // Only accept votes from authorized vote pubkeys with non-zero stake - // that we determined at leader_schedule_epoch boundaries - if let Some(vote_pubkey) = vote_tracker.get_voter_pubkey(&vote_pubkey, slot) - { - // Don't insert if we already have marked down this pubkey - // voting for this slot - if let Some(slot_tracker) = all_slot_trackers.read().unwrap().get(&slot) - { - if slot_tracker.read().unwrap().voted.contains(&vote_pubkey) { - continue; - } + // Don't insert if we already have marked down this pubkey + // voting for this slot + let maybe_slot_tracker = + all_slot_trackers.read().unwrap().get(&slot).cloned(); + if let Some(slot_tracker) = maybe_slot_tracker { + if slot_tracker.read().unwrap().voted.contains(vote_pubkey) { + continue; } - - diff.entry(slot).or_default().insert(vote_pubkey.clone()); } + let mut unduplicated_pubkey = + vote_tracker.keys.read().unwrap().get(vote_pubkey).cloned(); + if unduplicated_pubkey.is_none() { + let new_key = Arc::new(*vote_pubkey); + vote_tracker.keys.write().unwrap().insert(new_key.clone()); + unduplicated_pubkey = Some(new_key); + } + diff.entry(slot) + .or_default() + .insert(unduplicated_pubkey.unwrap()); } } } @@ -602,12 +493,13 @@ impl ClusterInfoVoteListener { .cloned(); if let Some(slot_tracker) = slot_tracker { let mut w_slot_tracker = slot_tracker.write().unwrap(); - let mut updates = w_slot_tracker.updates.take().unwrap_or_else(|| vec![]); + if w_slot_tracker.updates.is_none() { + w_slot_tracker.updates = Some(vec![]); + } for pk in slot_diff { w_slot_tracker.voted.insert(pk.clone()); - updates.push(pk); + w_slot_tracker.updates.as_mut().unwrap().push(pk); } - w_slot_tracker.updates = Some(updates); } else { let voted: HashSet<_> = slot_diff.into_iter().collect(); let new_slot_tracker = SlotVoteTracker { @@ -622,47 +514,6 @@ impl ClusterInfoVoteListener { } } } - - fn process_new_root(vote_tracker: &VoteTracker, new_root: Slot) { - let root_epoch = vote_tracker.epoch_schedule.get_epoch(new_root); - vote_tracker - .slot_vote_trackers - .write() - .unwrap() - .retain(|slot, _| *slot >= new_root); - vote_tracker - .node_id_to_vote_accounts - .write() - .unwrap() - .retain(|epoch, _| epoch >= &root_epoch); - vote_tracker - .epoch_authorized_voters - .write() - .unwrap() - .retain(|epoch, _| epoch >= &root_epoch); - } - - fn process_new_leader_schedule_epoch( - root_bank: &Bank, - vote_tracker: &VoteTracker, - new_leader_schedule_epoch: Epoch, - ) { - let (new_epoch_authorized_voters, new_node_id_to_vote_accounts, new_pubkeys) = - VoteTracker::parse_epoch_state( - new_leader_schedule_epoch, - root_bank - .epoch_vote_accounts(new_leader_schedule_epoch) - .expect("Epoch vote accounts must exist"), - &vote_tracker.all_pubkeys.read().unwrap(), - ); - - vote_tracker.process_new_leader_schedule_epoch_state( - new_leader_schedule_epoch, - new_epoch_authorized_voters, - new_node_id_to_vote_accounts, - new_pubkeys, - ); - } } #[cfg(test)] @@ -675,7 +526,7 @@ mod tests { }; use solana_sdk::hash::Hash; use solana_sdk::signature::{Keypair, Signer}; - use solana_vote_program::{vote_state::create_account, vote_transaction}; + use solana_vote_program::vote_transaction; #[test] fn test_max_vote_tx_fits() { @@ -761,21 +612,92 @@ mod tests { )); } + #[test] + fn test_update_new_root() { + let (vote_tracker, bank, _) = setup(); + + // Check outdated slots are purged with new root + let new_voter = Arc::new(Pubkey::new_rand()); + // Make separate copy so the original doesn't count toward + // the ref count, which would prevent cleanup + let new_voter_ = Arc::new(*new_voter); + vote_tracker.insert_vote(bank.slot(), new_voter_); + assert!(vote_tracker + .slot_vote_trackers + .read() + .unwrap() + .contains_key(&bank.slot())); + let bank1 = Bank::new_from_parent(&bank, &Pubkey::default(), bank.slot() + 1); + vote_tracker.process_new_root_bank(&bank1); + assert!(!vote_tracker + .slot_vote_trackers + .read() + .unwrap() + .contains_key(&bank.slot())); + + // Check `keys` and `epoch_authorized_voters` are purged when new + // root bank moves to the next epoch + assert!(vote_tracker.keys.read().unwrap().contains(&new_voter)); + let current_epoch = bank.epoch(); + let new_epoch_bank = Bank::new_from_parent( + &bank, + &Pubkey::default(), + bank.epoch_schedule() + .get_first_slot_in_epoch(current_epoch + 1), + ); + vote_tracker.process_new_root_bank(&new_epoch_bank); + assert!(!vote_tracker.keys.read().unwrap().contains(&new_voter)); + assert_eq!( + *vote_tracker.current_epoch.read().unwrap(), + current_epoch + 1 + ); + } + + #[test] + fn test_update_new_leader_schedule_epoch() { + let (vote_tracker, bank, _) = setup(); + + // Check outdated slots are purged with new root + let leader_schedule_epoch = bank.get_leader_schedule_epoch(bank.slot()); + let next_leader_schedule_epoch = leader_schedule_epoch + 1; + let mut next_leader_schedule_computed = bank.slot(); + loop { + next_leader_schedule_computed += 1; + if bank.get_leader_schedule_epoch(next_leader_schedule_computed) + == next_leader_schedule_epoch + { + break; + } + } + assert_eq!( + 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.update_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] fn test_process_votes() { // Create some voters at genesis - let validator_voting_keypairs: Vec<_> = (0..10) - .map(|_| ValidatorVoteKeypairs::new(Keypair::new(), Keypair::new(), Keypair::new())) - .collect(); - let GenesisConfigInfo { genesis_config, .. } = - genesis_utils::create_genesis_config_with_vote_accounts( - 10_000, - &validator_voting_keypairs, - ); - let bank = Bank::new(&genesis_config); - - // Send some votes to process - let vote_tracker = Arc::new(VoteTracker::new(&bank)); + let (vote_tracker, _, validator_voting_keypairs) = setup(); let (votes_sender, votes_receiver) = unbounded(); let vote_slots = vec![1, 2]; @@ -813,19 +735,8 @@ mod tests { #[test] fn test_process_votes2() { // Create some voters at genesis - let num_voters = 10; - let validator_voting_keypairs: Vec<_> = (0..num_voters) - .map(|_| ValidatorVoteKeypairs::new(Keypair::new(), Keypair::new(), Keypair::new())) - .collect(); - let GenesisConfigInfo { genesis_config, .. } = - genesis_utils::create_genesis_config_with_vote_accounts( - 10_000, - &validator_voting_keypairs, - ); - let bank = Bank::new(&genesis_config); - + let (vote_tracker, _, validator_voting_keypairs) = setup(); // Send some votes to process - let vote_tracker = Arc::new(VoteTracker::new(&bank)); let (votes_sender, votes_receiver) = unbounded(); for (i, keyset) in validator_voting_keypairs.chunks(2).enumerate() { @@ -868,30 +779,14 @@ mod tests { #[test] fn test_get_voters_by_epoch() { // Create some voters at genesis - let validator_voting_keypairs: Vec<_> = (0..10) - .map(|_| ValidatorVoteKeypairs::new(Keypair::new(), Keypair::new(), Keypair::new())) - .collect(); - let GenesisConfigInfo { genesis_config, .. } = - genesis_utils::create_genesis_config_with_vote_accounts( - 10_000, - &validator_voting_keypairs, - ); - let bank = Bank::new(&genesis_config); - - let vote_tracker = VoteTracker::new(&bank); + 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 voters and authorized voters + // Check we can get the authorized voters for keypairs in &validator_voting_keypairs { - assert!(vote_tracker - .get_voter_pubkey(&keypairs.vote_keypair.pubkey(), last_known_slot) - .is_some()); - assert!(vote_tracker - .get_voter_pubkey(&keypairs.vote_keypair.pubkey(), last_known_slot + 1) - .is_none()); assert!(vote_tracker .get_authorized_voter(&keypairs.vote_keypair.pubkey(), last_known_slot) .is_some()); @@ -906,48 +801,23 @@ mod tests { let new_keypairs: Vec<_> = (0..10) .map(|_| ValidatorVoteKeypairs::new(Keypair::new(), Keypair::new(), Keypair::new())) .collect(); - let new_epoch_vote_accounts: HashMap<_, _> = new_keypairs + let new_epoch_authorized_voters: HashMap<_, _> = new_keypairs .iter() .chain(validator_voting_keypairs[0..5].iter()) - .map(|keypair| { - ( - keypair.vote_keypair.pubkey(), - ( - 1, - bank.get_account(&keypair.vote_keypair.pubkey()) - .unwrap_or(create_account( - &keypair.vote_keypair.pubkey(), - &keypair.vote_keypair.pubkey(), - 0, - 100, - )), - ), - ) - }) + .map(|keypair| (keypair.vote_keypair.pubkey(), keypair.vote_keypair.pubkey())) .collect(); - let (new_epoch_authorized_voters, new_node_id_to_vote_accounts, new_pubkeys) = - VoteTracker::parse_epoch_state( - new_epoch, - &new_epoch_vote_accounts, - &vote_tracker.all_pubkeys.read().unwrap(), - ); - - vote_tracker.process_new_leader_schedule_epoch_state( - new_epoch, - new_epoch_authorized_voters, - new_node_id_to_vote_accounts, - new_pubkeys, - ); + 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_voter_pubkey(&keypairs.vote_keypair.pubkey(), first_slot_in_new_epoch) - .is_some()); assert!(vote_tracker .get_authorized_voter(&keypairs.vote_keypair.pubkey(), first_slot_in_new_epoch) .is_some()); @@ -955,46 +825,12 @@ mod tests { // These keypairs were not refreshed in new epoch for keypairs in validator_voting_keypairs[5..10].iter() { - assert!(vote_tracker - .get_voter_pubkey(&keypairs.vote_keypair.pubkey(), first_slot_in_new_epoch) - .is_none()); assert!(vote_tracker .get_authorized_voter(&keypairs.vote_keypair.pubkey(), first_slot_in_new_epoch) .is_none()); } } - #[test] - fn test_node_id_to_vote_accounts() { - // Create some voters at genesis - let validator_voting_keypairs: Vec<_> = (0..10) - .map(|_| ValidatorVoteKeypairs::new(Keypair::new(), Keypair::new(), Keypair::new())) - .collect(); - let GenesisConfigInfo { genesis_config, .. } = - genesis_utils::create_genesis_config_with_vote_accounts( - 10_000, - &validator_voting_keypairs, - ); - let bank = Bank::new(&genesis_config); - - // Send some votes to process - let vote_tracker = VoteTracker::new(&bank); - - // Given all the node id's, should diff out all the vote accounts - let node_ids: Vec<_> = validator_voting_keypairs - .iter() - .map(|v| v.node_keypair.pubkey()) - .collect(); - let vote_accounts: Vec<_> = validator_voting_keypairs - .iter() - .map(|v| Arc::new(v.vote_keypair.pubkey())) - .collect(); - assert_eq!( - vote_tracker.node_id_to_vote_accounts(&node_ids, bank.slot()), - vote_accounts - ); - } - #[test] fn test_vote_tracker_references() { // The number of references that get stored for a pubkey every time @@ -1011,6 +847,7 @@ mod tests { genesis_utils::create_genesis_config_with_vote_accounts( 10_000, &validator_voting_keypairs, + 100, ); let bank = Bank::new(&genesis_config); @@ -1028,91 +865,39 @@ mod tests { &validator0_keypairs.vote_keypair, )]; - let mut current_ref_count = Arc::strong_count( - &vote_tracker - .get_voter_pubkey(&validator0_keypairs.vote_keypair.pubkey(), bank.slot()) - .unwrap(), - ); - - { - ClusterInfoVoteListener::process_votes(&vote_tracker, vote_tx, 0); - let ref_count = Arc::strong_count( - &vote_tracker - .get_voter_pubkey(&validator0_keypairs.vote_keypair.pubkey(), bank.slot()) - .unwrap(), - ); - - // This pubkey voted for a slot, so ref count goes up - current_ref_count += ref_count_per_vote; - assert_eq!(ref_count, current_ref_count); - } - - // Move into the next epoch, a new set of voters is introduced, with some - // old voters also still present - let new_pubkey = Pubkey::new_rand(); - - // Pubkey of a vote account that will stick around for the next epoch - let old_refreshed_pubkey = validator0_keypairs.vote_keypair.pubkey(); - let old_refreshed_account = bank.get_account(&old_refreshed_pubkey).unwrap(); - - // Pubkey of a vote account that will be removed in the next epoch - let old_outdated_pubkey = validator_voting_keypairs[1].vote_keypair.pubkey(); - let new_epoch = bank.get_leader_schedule_epoch(bank.slot()) + 1; - let first_slot_in_new_epoch = bank.epoch_schedule().get_first_slot_in_epoch(new_epoch); - - // Create the set of relevant voters for the next epoch - let new_epoch_vote_accounts: HashMap<_, _> = vec![ - ((old_refreshed_pubkey.clone(), (1, old_refreshed_account))), - ( - new_pubkey.clone(), - (1, create_account(&new_pubkey, &new_pubkey, 0, 100)), - ), - ] - .into_iter() - .collect(); - - let (new_epoch_authorized_voters, new_node_id_to_vote_accounts, new_pubkeys) = - VoteTracker::parse_epoch_state( - new_epoch, - &new_epoch_vote_accounts, - &vote_tracker.all_pubkeys.read().unwrap(), - ); - - assert_eq!( - new_pubkeys, - vec![Arc::new(new_pubkey)].into_iter().collect() - ); - - // Should add 3 new references to `old_refreshed_pubkey`, two in `new_epoch_authorized_voters`, - // (one for the voter, one for the authorized voter b/c both are the same key) and - // one in `new_node_id_to_vote_accounts`s - vote_tracker.process_new_leader_schedule_epoch_state( - new_epoch, - new_epoch_authorized_voters, - new_node_id_to_vote_accounts, - new_pubkeys, - ); - - assert!(vote_tracker - .get_voter_pubkey(&new_pubkey, first_slot_in_new_epoch) - .is_some()); - assert!(vote_tracker - .get_voter_pubkey(&old_outdated_pubkey, first_slot_in_new_epoch) - .is_none()); - - // Make sure new copies of the same pubkeys aren't constantly being - // introduced when the same voter is in both the old and new epoch - // Instead, only the ref count should go up. + ClusterInfoVoteListener::process_votes(&vote_tracker, vote_tx, 0); let ref_count = Arc::strong_count( &vote_tracker - .get_voter_pubkey(&old_refreshed_pubkey, first_slot_in_new_epoch) + .keys + .read() + .unwrap() + .get(&validator0_keypairs.vote_keypair.pubkey()) .unwrap(), ); - // Ref count goes up by 3 (see above comments) - current_ref_count += 3; + // This pubkey voted for a slot, so ref count is `ref_count_per_vote + 1`, + // +1 in `vote_tracker.keys` and +ref_count_per_vote for the one vote + let mut current_ref_count = ref_count_per_vote + 1; assert_eq!(ref_count, current_ref_count); + // 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); + // Make 2 new votes in two different epochs, ref count should go up // by 2 * ref_count_per_vote let vote_txs: Vec<_> = [bank.slot() + 2, first_slot_in_new_epoch] @@ -1134,12 +919,56 @@ mod tests { let ref_count = Arc::strong_count( &vote_tracker - .get_voter_pubkey(&old_refreshed_pubkey, first_slot_in_new_epoch) + .keys + .read() + .unwrap() + .get(&validator0_keypairs.vote_keypair.pubkey()) .unwrap(), ); - - // Ref count goes up by 2 (see above comments) current_ref_count += 2 * ref_count_per_vote; assert_eq!(ref_count, current_ref_count); } + + fn setup() -> (Arc, Arc, Vec) { + let validator_voting_keypairs: Vec<_> = (0..10) + .map(|_| ValidatorVoteKeypairs::new(Keypair::new(), Keypair::new(), Keypair::new())) + .collect(); + let GenesisConfigInfo { genesis_config, .. } = + genesis_utils::create_genesis_config_with_vote_accounts( + 10_000, + &validator_voting_keypairs, + 100, + ); + let bank = Bank::new(&genesis_config); + let vote_tracker = VoteTracker::new(&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), + Arc::new(bank), + validator_voting_keypairs, + ) + } } diff --git a/core/src/consensus.rs b/core/src/consensus.rs index 8acc218ab3..20277dc49c 100644 --- a/core/src/consensus.rs +++ b/core/src/consensus.rs @@ -686,13 +686,14 @@ pub mod test { // Setup BankForks with bank 0 and all the validator accounts pub(crate) fn initialize_state( validator_keypairs_map: &HashMap, + stake: u64, ) -> (BankForks, HashMap) { let validator_keypairs: Vec<_> = validator_keypairs_map.values().collect(); let GenesisConfigInfo { genesis_config, mint_keypair, voting_keypair: _, - } = create_genesis_config_with_vote_accounts(1_000_000_000, &validator_keypairs); + } = create_genesis_config_with_vote_accounts(1_000_000_000, &validator_keypairs, stake); let bank0 = Bank::new(&genesis_config); @@ -779,7 +780,7 @@ pub mod test { ); // Initialize BankForks - let (bank_forks, mut progress) = initialize_state(&keypairs); + let (bank_forks, mut progress) = initialize_state(&keypairs, 10_000); let bank_forks = RwLock::new(bank_forks); // Create the tree of banks @@ -859,7 +860,7 @@ pub mod test { votes.extend((45..=50).into_iter()); let mut cluster_votes: HashMap> = HashMap::new(); - let (bank_forks, mut progress) = initialize_state(&keypairs); + let (bank_forks, mut progress) = initialize_state(&keypairs, 10_000); let bank_forks = RwLock::new(bank_forks); // Simulate the votes. Should fail on trying to come back to the main fork diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index 2c631c1248..79de13d2c7 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -2172,7 +2172,7 @@ pub(crate) mod tests { ValidatorVoteKeypairs::new(node_keypair, vote_keypair, stake_keypair), ); - let (bank_forks, mut progress) = initialize_state(&keypairs); + let (bank_forks, mut progress) = initialize_state(&keypairs, 10_000); let bank0 = bank_forks.get(0).unwrap().clone(); let my_keypairs = keypairs.get(&node_pubkey).unwrap(); let vote_tx = vote_transaction::new_vote_transaction( @@ -2285,7 +2285,7 @@ pub(crate) mod tests { ValidatorVoteKeypairs::new(node_keypair, vote_keypair, stake_keypair), ); - let (bank_forks, mut progress) = initialize_state(&keypairs); + let (bank_forks, mut progress) = initialize_state(&keypairs, 10_000); let bank_forks = Arc::new(RwLock::new(bank_forks)); let mut tower = Tower::new_with_key(&node_pubkey); diff --git a/runtime/src/genesis_utils.rs b/runtime/src/genesis_utils.rs index fdd7e183c6..2accac24b6 100644 --- a/runtime/src/genesis_utils.rs +++ b/runtime/src/genesis_utils.rs @@ -43,6 +43,7 @@ pub fn create_genesis_config(mint_lamports: u64) -> GenesisConfigInfo { pub fn create_genesis_config_with_vote_accounts( mint_lamports: u64, voting_keypairs: &[impl Borrow], + stake: u64, ) -> GenesisConfigInfo { let mut genesis_config_info = create_genesis_config(mint_lamports); for validator_voting_keypairs in voting_keypairs { @@ -51,13 +52,13 @@ pub fn create_genesis_config_with_vote_accounts( let stake_pubkey = validator_voting_keypairs.borrow().stake_keypair.pubkey(); // Create accounts - let vote_account = vote_state::create_account(&vote_pubkey, &node_pubkey, 0, 100); + let vote_account = vote_state::create_account(&vote_pubkey, &node_pubkey, 0, stake); let stake_account = stake_state::create_account( &stake_pubkey, &vote_pubkey, &vote_account, &genesis_config_info.genesis_config.rent, - 100, + stake, ); // Put newly created accounts into genesis