diff --git a/core/src/locktower.rs b/core/src/locktower.rs index 919001754..9cb134952 100644 --- a/core/src/locktower.rs +++ b/core/src/locktower.rs @@ -345,7 +345,7 @@ impl Locktower { fn bank_weight(&self, bank: &Bank, ancestors: &HashMap>) -> u128 { let stake_lockouts = - self.collect_vote_lockouts(bank.slot(), bank.vote_accounts(), ancestors); + self.collect_vote_lockouts(bank.slot(), bank.vote_accounts().into_iter(), ancestors); self.calculate_weight(&stake_lockouts) } diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs index b03965eab..f7d28549c 100644 --- a/core/src/replay_stage.rs +++ b/core/src/replay_stage.rs @@ -403,7 +403,11 @@ impl ReplayStage { .map(|bank| { ( bank, - locktower.collect_vote_lockouts(bank.slot(), bank.vote_accounts(), &ancestors), + locktower.collect_vote_lockouts( + bank.slot(), + bank.vote_accounts().into_iter(), + &ancestors, + ), ) }) .filter(|(b, stake_lockouts)| { diff --git a/core/src/staking_utils.rs b/core/src/staking_utils.rs index 9579ee9b2..c645bac23 100644 --- a/core/src/staking_utils.rs +++ b/core/src/staking_utils.rs @@ -53,9 +53,11 @@ pub fn delegated_stakes_at_epoch(bank: &Bank, epoch_height: u64) -> Option impl Iterator { - bank.vote_accounts().filter_map(|(account_id, account)| { - filter_zero_balances(&account).map(|stake| (account_id, stake, account)) - }) + bank.vote_accounts() + .into_iter() + .filter_map(|(account_id, account)| { + filter_zero_balances(&account).map(|stake| (account_id, stake, account)) + }) } pub fn node_staked_accounts_at_epoch( diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index f2c879af7..3eff3864e 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -13,7 +13,6 @@ use solana_sdk::pubkey::Pubkey; use solana_sdk::signature::{Keypair, KeypairUtil}; use solana_sdk::transaction::Result; use solana_sdk::transaction::{Transaction, TransactionError}; -use solana_vote_api; use std::collections::BTreeMap; use std::env; use std::fs::{create_dir_all, remove_dir_all}; @@ -181,15 +180,6 @@ impl AccountStorageEntry { type AccountStorage = Vec; -#[derive(Default, Debug)] -struct ForkInfo { - /// List of all parents of this fork - parents: Vec, - - /// Cached vote accounts - vote_accounts: RwLock>, -} - // This structure handles the load/store of the accounts #[derive(Default)] pub struct AccountsDB { @@ -203,7 +193,7 @@ pub struct AccountsDB { next_id: AtomicUsize, /// Information related to the fork - fork_infos: RwLock>, + parents_map: RwLock>>, /// Set of storage paths to pick from paths: Vec, @@ -260,7 +250,7 @@ impl AccountsDB { account_index, storage: RwLock::new(vec![]), next_id: AtomicUsize::new(0), - fork_infos: RwLock::new(HashMap::new()), + parents_map: RwLock::new(HashMap::new()), paths, file_size, inc_size, @@ -276,18 +266,16 @@ impl AccountsDB { pub fn add_fork(&self, fork: Fork, parent: Option) { { - let mut fork_infos = self.fork_infos.write().unwrap(); - let mut fork_info = ForkInfo::default(); + let mut parents_map = self.parents_map.write().unwrap(); + let mut parents = Vec::new(); if let Some(parent) = parent { - fork_info.parents.push(parent); - if let Some(parent_fork_info) = fork_infos.get(&parent) { - fork_info - .parents - .extend_from_slice(&parent_fork_info.parents); + parents.push(parent); + if let Some(grandparents) = parents_map.get(&parent) { + parents.extend_from_slice(&grandparents); } } - if let Some(old_fork_info) = fork_infos.insert(fork, fork_info) { - panic!("duplicate forks! {} {:?}", fork, old_fork_info); + if let Some(old_parents) = parents_map.insert(fork, parents) { + panic!("duplicate forks! {} {:?}", fork, old_parents); } } let mut account_maps = self.account_index.account_maps.write().unwrap(); @@ -309,45 +297,6 @@ impl AccountsDB { storage.append(&mut stores); } - fn get_vote_accounts_by_fork( - &self, - fork: Fork, - fork_infos: &HashMap, - vote_accounts: &HashMap, - ) -> HashMap { - fork_infos - .get(&fork) - .unwrap() - .vote_accounts - .read() - .unwrap() - .iter() - .filter_map(|(pubkey, account)| { - if !vote_accounts.contains_key(pubkey) { - Some((*pubkey, account.clone())) - } else { - None - } - }) - .collect() - } - - fn get_vote_accounts(&self, fork: Fork) -> HashMap { - let mut vote_accounts = HashMap::new(); - let fork_infos = self.fork_infos.read().unwrap(); - if let Some(fork_info) = fork_infos.get(&fork) { - vote_accounts = fork_info.vote_accounts.read().unwrap().clone(); - for parent_fork in fork_info.parents.iter() { - let parent_vote_accounts = - self.get_vote_accounts_by_fork(*parent_fork, &fork_infos, &vote_accounts); - for (pubkey, account) in parent_vote_accounts.iter() { - vote_accounts.insert(*pubkey, account.clone()); - } - } - } - vote_accounts - } - pub fn has_accounts(&self, fork: Fork) -> bool { let account_maps = self.account_index.account_maps.read().unwrap(); if let Some(account_map) = account_maps.get(&fork) { @@ -400,10 +349,10 @@ impl AccountsDB { return None; } // find most recent fork that is an ancestor of current_fork - let fork_infos = self.fork_infos.read().unwrap(); - if let Some(fork_info) = fork_infos.get(&fork) { - for parent_fork in fork_info.parents.iter() { - if let Some(account_map) = account_maps.get(&parent_fork) { + let parents_map = self.parents_map.read().unwrap(); + if let Some(parents) = parents_map.get(&fork) { + for parent in parents.iter() { + if let Some(account_map) = account_maps.get(&parent) { let account_map = account_map.read().unwrap(); if let Some(account_info) = account_map.get(&pubkey) { return Some(self.get_account(account_info.id, account_info.offset)); @@ -443,9 +392,9 @@ impl AccountsDB { if !walk_back { return program_accounts; } - let fork_infos = self.fork_infos.read().unwrap(); - if let Some(fork_info) = fork_infos.get(&fork) { - for parent_fork in fork_info.parents.iter() { + let parents_map = self.parents_map.read().unwrap(); + if let Some(parents) = parents_map.get(&fork) { + for parent_fork in parents.iter() { let mut parent_accounts = self.load_program_accounts(*parent_fork, &program_id); program_accounts.append(&mut parent_accounts); } @@ -554,11 +503,11 @@ impl AccountsDB { account_map.clear(); } account_maps.remove(&fork); - let mut fork_infos = self.fork_infos.write().unwrap(); - for (_, fork_info) in fork_infos.iter_mut() { - fork_info.parents.retain(|parent_fork| *parent_fork != fork); + let mut parents_map = self.parents_map.write().unwrap(); + for (_, parents) in parents_map.iter_mut() { + parents.retain(|parent_fork| *parent_fork != fork); } - fork_infos.remove(&fork); + parents_map.remove(&fork); } /// Store the account update. @@ -566,13 +515,6 @@ impl AccountsDB { if account.lamports == 0 && self.is_squashed(fork) { // purge if balance is 0 and no checkpoints self.remove_account_entries(fork, &pubkey); - if solana_vote_api::check_id(&account.owner) { - let fork_infos = self.fork_infos.read().unwrap(); - if let Some(fork_info) = fork_infos.get(&fork) { - let mut vote_accounts = fork_info.vote_accounts.write().unwrap(); - vote_accounts.remove(pubkey); - } - } } else { let (id, offset) = self.append_account(account); let account_maps = self.account_index.account_maps.read().unwrap(); @@ -583,13 +525,6 @@ impl AccountsDB { lamports: account.lamports, }; self.insert_account_entry(&pubkey, &account_info, &mut account_map); - if solana_vote_api::check_id(&account.owner) { - let fork_infos = self.fork_infos.read().unwrap(); - if let Some(fork_info) = fork_infos.get(&fork) { - let mut vote_accounts = fork_info.vote_accounts.write().unwrap(); - vote_accounts.insert(*pubkey, account.clone()); - } - } } } @@ -736,22 +671,17 @@ impl AccountsDB { } fn remove_parents(&self, fork: Fork) -> Vec { - let mut squashed_vote_accounts = self.get_vote_accounts(fork); - squashed_vote_accounts.retain(|_, account| account.lamports != 0); - let mut info = self.fork_infos.write().unwrap(); - let fork_info = info.get_mut(&fork).unwrap(); - let mut vote_accounts = fork_info.vote_accounts.write().unwrap(); - *vote_accounts = squashed_vote_accounts; - fork_info.parents.split_off(0) + let mut parents_map = self.parents_map.write().unwrap(); + let parents = parents_map.get_mut(&fork).unwrap(); + parents.split_off(0) } fn is_squashed(&self, fork: Fork) -> bool { - self.fork_infos + self.parents_map .read() .unwrap() .get(&fork) .unwrap() - .parents .is_empty() } @@ -764,16 +694,16 @@ impl AccountsDB { { let stores = self.storage.read().unwrap(); for parent_fork in parents.iter() { - let parent_map = account_maps.get(&parent_fork).unwrap().read().unwrap(); - if account_map.len() > parent_map.len() { - for (pubkey, account_info) in parent_map.iter() { + let parents_map = account_maps.get(&parent_fork).unwrap().read().unwrap(); + if account_map.len() > parents_map.len() { + for (pubkey, account_info) in parents_map.iter() { if !account_map.contains_key(pubkey) { stores[account_info.id].add_account(); account_map.insert(*pubkey, account_info.clone()); } } } else { - let mut maps = parent_map.clone(); + let mut maps = parents_map.clone(); for (_, account_info) in maps.iter() { stores[account_info.id].add_account(); } @@ -984,13 +914,6 @@ impl Accounts { self.accounts_db.squash(fork); } - pub fn get_vote_accounts(&self, fork: Fork) -> impl Iterator { - self.accounts_db - .get_vote_accounts(fork) - .into_iter() - .filter(|(_, acc)| acc.lamports != 0) - } - pub fn remove_accounts(&self, fork: Fork) { self.accounts_db.remove_accounts(fork); } @@ -1774,93 +1697,6 @@ mod tests { assert_eq!(check_storage(&accounts, 0), true); } - #[test] - fn test_accounts_vote_filter() { - let accounts = Accounts::new(0, None); - let mut vote_account = Account::new(1, 0, &solana_vote_api::id()); - let key = Pubkey::new_rand(); - accounts.store_slow(0, &key, &vote_account); - - accounts.new_from_parent(1, 0); - - let mut vote_accounts: Vec<_> = accounts.get_vote_accounts(1).collect(); - assert_eq!(vote_accounts.len(), 1); - - vote_account.lamports = 0; - accounts.store_slow(1, &key, &vote_account); - - vote_accounts = accounts.get_vote_accounts(1).collect(); - assert_eq!(vote_accounts.len(), 0); - - let mut vote_account1 = Account::new(2, 0, &solana_vote_api::id()); - let key1 = Pubkey::new_rand(); - accounts.store_slow(1, &key1, &vote_account1); - - accounts.squash(1); - vote_accounts = accounts.get_vote_accounts(0).collect(); - assert_eq!(vote_accounts.len(), 1); - vote_accounts = accounts.get_vote_accounts(1).collect(); - assert_eq!(vote_accounts.len(), 1); - - vote_account1.lamports = 0; - accounts.store_slow(1, &key1, &vote_account1); - accounts.store_slow(0, &key, &vote_account); - - vote_accounts = accounts.get_vote_accounts(1).collect(); - assert_eq!(vote_accounts.len(), 0); - } - - #[test] - fn test_account_vote() { - let paths = get_tmp_accounts_path!(); - let accounts_db = AccountsDB::new(0, &paths.paths); - let mut pubkeys: Vec = vec![]; - create_account(&accounts_db, &mut pubkeys, 0, 0, 1); - let accounts = accounts_db.get_vote_accounts(0); - assert_eq!(accounts.len(), 1); - accounts.iter().for_each(|(_, account)| { - assert_eq!(account.owner, solana_vote_api::id()); - }); - let lastkey = Pubkey::new_rand(); - let mut lastaccount = Account::new(1, 0, &solana_vote_api::id()); - accounts_db.store(0, &lastkey, &lastaccount); - assert_eq!(accounts_db.get_vote_accounts(0).len(), 2); - - accounts_db.add_fork(1, Some(0)); - - assert_eq!( - accounts_db.get_vote_accounts(1), - accounts_db.get_vote_accounts(0) - ); - - info!("storing lamports=0 to fork 1"); - // should store a lamports=0 account in 1 - lastaccount.lamports = 0; - accounts_db.store(1, &lastkey, &lastaccount); - // len == 2 because lamports=0 accounts are filtered at the Accounts interface. - assert_eq!(accounts_db.get_vote_accounts(1).len(), 2); - - accounts_db.squash(1); - - // should still be in 0 - assert_eq!(accounts_db.get_vote_accounts(0).len(), 2); - - // delete it from 0 - accounts_db.store(0, &lastkey, &lastaccount); - assert_eq!(accounts_db.get_vote_accounts(0).len(), 1); - assert_eq!(accounts_db.get_vote_accounts(1).len(), 1); - - // Add fork 2 and squash - accounts_db.add_fork(2, Some(1)); - accounts_db.store(2, &lastkey, &lastaccount); - - accounts_db.squash(1); - accounts_db.squash(2); - - assert_eq!(accounts_db.get_vote_accounts(1).len(), 1); - assert_eq!(accounts_db.get_vote_accounts(2).len(), 1); - } - #[test] fn test_accounts_empty_hash_internal_state() { let paths = get_tmp_accounts_path!(); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index b9eee6c1d..bcdc281f9 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -152,6 +152,9 @@ pub struct Bank { /// initialized from genesis epoch_schedule: EpochSchedule, + /// cache of vote_account state for this fork + vote_accounts: RwLock>, + /// staked nodes on epoch boundaries, saved off when a bank.slot() is at /// a leader schedule boundary epoch_vote_accounts: HashMap>, @@ -181,7 +184,7 @@ impl Bank { bank.process_genesis_block(genesis_block); // genesis needs stakes for all epochs up to the epoch implied by // slot = 0 and genesis configuration - let vote_accounts: HashMap<_, _> = bank.vote_accounts().collect(); + let vote_accounts = bank.vote_accounts(); for i in 0..=bank.get_stakers_epoch(bank.slot) { bank.epoch_vote_accounts.insert(i, vote_accounts.clone()); } @@ -199,6 +202,7 @@ impl Bank { bank.transaction_count .store(parent.transaction_count() as usize, Ordering::Relaxed); + bank.vote_accounts = RwLock::new(parent.vote_accounts()); bank.tick_height .store(parent.tick_height.load(Ordering::SeqCst), Ordering::SeqCst); @@ -224,7 +228,7 @@ impl Bank { // if my parent didn't populate for this epoch, we've // crossed a boundary if epoch_vote_accounts.get(&epoch).is_none() { - epoch_vote_accounts.insert(epoch, bank.vote_accounts().collect()); + epoch_vote_accounts.insert(epoch, bank.vote_accounts()); } epoch_vote_accounts }; @@ -330,8 +334,7 @@ impl Bank { .serialize(&mut bootstrap_leader_vote_account.data) .unwrap(); - self.accounts.store_slow( - self.accounts_id, + self.store( &genesis_block.bootstrap_leader_vote_account_id, &bootstrap_leader_vote_account, ); @@ -369,8 +372,7 @@ impl Bank { pub fn register_native_instruction_processor(&self, name: &str, program_id: &Pubkey) { debug!("Adding native program {} under {:?}", name, program_id); let account = native_loader::create_loadable_account(name); - self.accounts - .store_slow(self.accounts_id, program_id, &account); + self.store(program_id, &account); } /// Return the last block hash registered. @@ -744,6 +746,8 @@ impl Bank { self.accounts .store_accounts(self.accounts_id, txs, executed, loaded_accounts); + self.store_vote_accounts(txs, executed, loaded_accounts); + // once committed there is no way to unroll let write_elapsed = now.elapsed(); debug!( @@ -806,6 +810,19 @@ impl Bank { parents } + fn store(&self, pubkey: &Pubkey, account: &Account) { + self.accounts.store_slow(self.accounts_id, pubkey, &account); + + if solana_vote_api::check_id(&account.owner) { + let mut vote_accounts = self.vote_accounts.write().unwrap(); + if account.lamports != 0 { + vote_accounts.insert(*pubkey, account.clone()); + } else { + vote_accounts.remove(pubkey); + } + } + } + pub fn withdraw(&self, pubkey: &Pubkey, lamports: u64) -> Result<()> { match self.get_account(pubkey) { Some(mut account) => { @@ -814,7 +831,8 @@ impl Bank { } account.lamports -= lamports; - self.accounts.store_slow(self.accounts_id, pubkey, &account); + self.store(pubkey, &account); + Ok(()) } None => Err(TransactionError::AccountNotFound), @@ -824,7 +842,7 @@ impl Bank { pub fn deposit(&self, pubkey: &Pubkey, lamports: u64) { let mut account = self.get_account(pubkey).unwrap_or_default(); account.lamports += lamports; - self.accounts.store_slow(self.accounts_id, pubkey, &account); + self.store(pubkey, &account); } pub fn get_account(&self, pubkey: &Pubkey) -> Option { @@ -910,9 +928,40 @@ impl Bank { self.epoch_schedule.get_stakers_epoch(slot) } + /// a bank-level cache of vote accounts + fn store_vote_accounts( + &self, + txs: &[Transaction], + res: &[Result<()>], + loaded: &[Result<(InstructionAccounts, InstructionLoaders)>], + ) { + let mut vote_accounts = self.vote_accounts.write().unwrap(); + + for (i, raccs) in loaded.iter().enumerate() { + if res[i].is_err() || raccs.is_err() { + continue; + } + + let message = &txs[i].message(); + let acc = raccs.as_ref().unwrap(); + for (key, account) in message + .account_keys + .iter() + .zip(acc.0.iter()) + .filter(|(_, account)| solana_vote_api::check_id(&account.owner)) + { + if account.lamports != 0 { + vote_accounts.insert(*key, account.clone()); + } else { + vote_accounts.remove(key); + } + } + } + } + /// current vote accounts for this bank - pub fn vote_accounts(&self) -> impl Iterator { - self.accounts.get_vote_accounts(self.accounts_id) + pub fn vote_accounts(&self) -> HashMap { + self.vote_accounts.read().unwrap().clone() } /// vote accounts for the specific epoch @@ -981,6 +1030,7 @@ mod tests { use solana_sdk::signature::{Keypair, KeypairUtil}; use solana_sdk::system_instruction; use solana_sdk::system_transaction; + use solana_vote_api::vote_instruction; #[test] fn test_bank_new() { @@ -1808,4 +1858,38 @@ mod tests { assert_eq!(bank6.transaction_count(), 1); } + #[test] + fn test_bank_vote_accounts() { + let (genesis_block, mint_keypair) = GenesisBlock::new(500); + let bank = Arc::new(Bank::new(&genesis_block)); + + let vote_accounts = bank.vote_accounts(); + assert_eq!(vote_accounts.len(), 1); // bootstrap leader has + // to have a vote account + + let vote_keypair = Keypair::new(); + let instructions = + vote_instruction::create_account(&mint_keypair.pubkey(), &vote_keypair.pubkey(), 10); + + let transaction = Transaction::new_signed_instructions( + &[&mint_keypair], + instructions, + bank.last_blockhash(), + ); + + bank.process_transaction(&transaction).unwrap(); + + let vote_accounts = bank.vote_accounts(); + + assert_eq!(vote_accounts.len(), 2); + + assert!(vote_accounts.get(&vote_keypair.pubkey()).is_some()); + + assert!(bank.withdraw(&vote_keypair.pubkey(), 10).is_ok()); + + let vote_accounts = bank.vote_accounts(); + + assert_eq!(vote_accounts.len(), 1); + } + }