diff --git a/Cargo.lock b/Cargo.lock index 22ae3bdc9b..5e65145fec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5669,6 +5669,8 @@ dependencies = [ "libsecp256k1 0.6.0", "log 0.4.14", "memmap2 0.5.0", + "num-derive", + "num-traits", "num_cpus", "ouroboros", "rand 0.7.3", diff --git a/programs/bpf/Cargo.lock b/programs/bpf/Cargo.lock index c63a059d0c..659ad21db2 100644 --- a/programs/bpf/Cargo.lock +++ b/programs/bpf/Cargo.lock @@ -3370,6 +3370,8 @@ dependencies = [ "lazy_static", "log", "memmap2 0.5.0", + "num-derive", + "num-traits", "num_cpus", "ouroboros", "rand 0.7.3", diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index d898d81cf3..4387d3c39e 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -26,6 +26,8 @@ lazy_static = "1.4.0" log = "0.4.14" memmap2 = "0.5.0" num_cpus = "1.13.0" +num-derive = { version = "0.3" } +num-traits = { version = "0.2" } ouroboros = "0.13.0" rand = "0.7.0" rayon = "1.5.1" diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index c70de78c47..bd753caba7 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -66,6 +66,8 @@ use { dashmap::DashMap, itertools::Itertools, log::*, + num_derive::ToPrimitive, + num_traits::ToPrimitive, rayon::{ iter::{IntoParallelIterator, IntoParallelRefIterator, ParallelIterator}, ThreadPool, ThreadPoolBuilder, @@ -1054,6 +1056,19 @@ struct VoteWithStakeDelegations { delegations: Vec<(Pubkey, (StakeState, AccountSharedData))>, } +#[derive(Debug, Clone, PartialEq, ToPrimitive)] +enum InvalidReason { + Missing, + BadState, + WrongOwner, +} + +struct LoadVoteAndStakeAccountsResult { + vote_with_stake_delegations_map: DashMap, + invalid_stake_keys: DashMap, + invalid_vote_keys: DashMap, +} + #[derive(Debug, Default)] pub struct NewBankOptions { pub vote_only_bank: bool, @@ -2166,9 +2181,12 @@ impl Bank { &self, thread_pool: &ThreadPool, reward_calc_tracer: Option, - ) -> DashMap { + ) -> LoadVoteAndStakeAccountsResult { let stakes = self.stakes.read().unwrap(); - let accounts = DashMap::with_capacity(stakes.vote_accounts().as_ref().len()); + let vote_with_stake_delegations_map = + DashMap::with_capacity(stakes.vote_accounts().as_ref().len()); + let invalid_stake_keys: DashMap = DashMap::new(); + let invalid_vote_keys: DashMap = DashMap::new(); thread_pool.install(|| { stakes @@ -2176,87 +2194,134 @@ impl Bank { .par_iter() .for_each(|(stake_pubkey, delegation)| { let vote_pubkey = &delegation.voter_pubkey; - let stake_account = match self.get_account_with_fixed_root(stake_pubkey) { - Some(stake_account) => stake_account, - None => return, - }; + if invalid_vote_keys.contains_key(vote_pubkey) { + return; + } - // fetch vote account from stakes cache if it hasn't been cached locally - let fetched_vote_account = if !accounts.contains_key(vote_pubkey) { - let vote_account = match self.get_account_with_fixed_root(vote_pubkey) { - Some(vote_account) => vote_account, - None => return, - }; + let stake_delegation = match self.get_account_with_fixed_root(stake_pubkey) { + Some(stake_account) => { + if stake_account.owner() != &solana_stake_program::id() { + invalid_stake_keys.insert(*stake_pubkey, InvalidReason::WrongOwner); + return; + } - let vote_state: VoteState = - match StateMut::::state(&vote_account) { - Ok(vote_state) => vote_state.convert_to_current(), - Err(err) => { - debug!( - "failed to deserialize vote account {}: {}", - vote_pubkey, err - ); + match stake_account.state().ok() { + Some(stake_state) => (*stake_pubkey, (stake_state, stake_account)), + None => { + invalid_stake_keys + .insert(*stake_pubkey, InvalidReason::BadState); return; } - }; - - Some((vote_state, vote_account)) - } else { - None + } + } + None => { + invalid_stake_keys.insert(*stake_pubkey, InvalidReason::Missing); + return; + } }; - let fetched_vote_account_owner = fetched_vote_account - .as_ref() - .map(|(_vote_state, vote_account)| vote_account.owner()); + let mut vote_delegations = if let Some(vote_delegations) = + vote_with_stake_delegations_map.get_mut(vote_pubkey) + { + vote_delegations + } else { + let vote_account = match self.get_account_with_fixed_root(vote_pubkey) { + Some(vote_account) => { + if vote_account.owner() != &solana_vote_program::id() { + invalid_vote_keys + .insert(*vote_pubkey, InvalidReason::WrongOwner); + return; + } + vote_account + } + None => { + invalid_vote_keys.insert(*vote_pubkey, InvalidReason::Missing); + return; + } + }; + + let vote_state = if let Ok(vote_state) = + StateMut::::state(&vote_account) + { + vote_state.convert_to_current() + } else { + invalid_vote_keys.insert(*vote_pubkey, InvalidReason::BadState); + return; + }; + + vote_with_stake_delegations_map + .entry(*vote_pubkey) + .or_insert_with(|| VoteWithStakeDelegations { + vote_state: Arc::new(vote_state), + vote_account, + delegations: vec![], + }) + }; if let Some(reward_calc_tracer) = reward_calc_tracer.as_ref() { reward_calc_tracer(&RewardCalculationEvent::Staking( stake_pubkey, &InflationPointCalculationEvent::Delegation( *delegation, - fetched_vote_account_owner - .cloned() - .unwrap_or_else(solana_vote_program::id), + solana_vote_program::id(), ), )); } - // filter invalid delegation accounts - if stake_account.owner() != &solana_stake_program::id() - || (fetched_vote_account_owner.is_some() - && fetched_vote_account_owner != Some(&solana_vote_program::id())) - { - datapoint_warn!( - "bank-stake_delegation_accounts-invalid-account", - ("slot", self.slot() as i64, i64), - ("stake-address", format!("{:?}", stake_pubkey), String), - ("vote-address", format!("{:?}", vote_pubkey), String), - ); - return; - } - - let stake_delegation = match stake_account.state().ok() { - Some(stake_state) => (*stake_pubkey, (stake_state, stake_account)), - None => return, - }; - - if let Some((vote_state, vote_account)) = fetched_vote_account { - accounts - .entry(*vote_pubkey) - .or_insert_with(|| VoteWithStakeDelegations { - vote_state: Arc::new(vote_state), - vote_account, - delegations: vec![], - }); - } - - if let Some(mut stake_delegation_accounts) = accounts.get_mut(vote_pubkey) { - stake_delegation_accounts.delegations.push(stake_delegation); - } + vote_delegations.delegations.push(stake_delegation); }); }); - accounts + LoadVoteAndStakeAccountsResult { + vote_with_stake_delegations_map, + invalid_vote_keys, + invalid_stake_keys, + } + } + + fn handle_invalid_stakes_cache_keys( + &self, + invalid_stake_keys: DashMap, + invalid_vote_keys: DashMap, + ) { + if invalid_stake_keys.is_empty() && invalid_vote_keys.is_empty() { + return; + } + + // Prune invalid stake delegations and vote accounts that were + // not properly evicted in normal operation. + let mut maybe_stakes_cache = if self + .feature_set + .is_active(&feature_set::evict_invalid_stakes_cache_entries::id()) + { + Some(self.stakes.write().unwrap()) + } else { + None + }; + + for (stake_pubkey, reason) in invalid_stake_keys { + if let Some(stakes_cache) = maybe_stakes_cache.as_mut() { + stakes_cache.remove_stake_delegation(&stake_pubkey); + } + datapoint_warn!( + "bank-stake_delegation_accounts-invalid-account", + ("slot", self.slot() as i64, i64), + ("stake-address", format!("{:?}", stake_pubkey), String), + ("reason", reason.to_i64().unwrap_or_default(), i64), + ); + } + + for (vote_pubkey, reason) in invalid_vote_keys { + if let Some(stakes_cache) = maybe_stakes_cache.as_mut() { + stakes_cache.remove_vote_account(&vote_pubkey); + } + datapoint_warn!( + "bank-stake_delegation_accounts-invalid-account", + ("slot", self.slot() as i64, i64), + ("vote-address", format!("{:?}", vote_pubkey), String), + ("reason", reason.to_i64().unwrap_or_default(), i64), + ); + } } /// iterate over all stakes, redeem vote credits for each stake we can @@ -2270,13 +2335,22 @@ impl Bank { thread_pool: &ThreadPool, ) -> f64 { let stake_history = self.stakes.read().unwrap().history().clone(); - let vote_and_stake_accounts = self.load_vote_and_stake_accounts_with_thread_pool( - thread_pool, - reward_calc_tracer.as_ref(), - ); + let vote_with_stake_delegations_map = { + let LoadVoteAndStakeAccountsResult { + vote_with_stake_delegations_map, + invalid_stake_keys, + invalid_vote_keys, + } = self.load_vote_and_stake_accounts_with_thread_pool( + thread_pool, + reward_calc_tracer.as_ref(), + ); + + self.handle_invalid_stakes_cache_keys(invalid_stake_keys, invalid_vote_keys); + vote_with_stake_delegations_map + }; let points: u128 = thread_pool.install(|| { - vote_and_stake_accounts + vote_with_stake_delegations_map .par_iter() .map(|entry| { let VoteWithStakeDelegations { @@ -2307,8 +2381,8 @@ impl Bank { // pay according to point value let point_value = PointValue { rewards, points }; let vote_account_rewards: DashMap = - DashMap::with_capacity(vote_and_stake_accounts.len()); - let stake_delegation_iterator = vote_and_stake_accounts.into_par_iter().flat_map( + DashMap::with_capacity(vote_with_stake_delegations_map.len()); + let stake_delegation_iterator = vote_with_stake_delegations_map.into_par_iter().flat_map( |( vote_pubkey, VoteWithStakeDelegations { @@ -8282,6 +8356,7 @@ pub(crate) mod tests { let thread_pool = ThreadPoolBuilder::new().num_threads(1).build().unwrap(); let validator_points: u128 = bank0 .load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer()) + .vote_with_stake_delegations_map .into_iter() .map( |( @@ -14287,8 +14362,9 @@ pub(crate) mod tests { ); let bank = Arc::new(Bank::new_for_tests(&genesis_config)); let thread_pool = ThreadPoolBuilder::new().num_threads(1).build().unwrap(); - let vote_and_stake_accounts = - bank.load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer()); + let vote_and_stake_accounts = bank + .load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer()) + .vote_with_stake_delegations_map; assert_eq!(vote_and_stake_accounts.len(), 2); let mut vote_account = bank @@ -14328,8 +14404,9 @@ pub(crate) mod tests { // Accounts must be valid stake and vote accounts let thread_pool = ThreadPoolBuilder::new().num_threads(1).build().unwrap(); - let vote_and_stake_accounts = - bank.load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer()); + let vote_and_stake_accounts = bank + .load_vote_and_stake_accounts_with_thread_pool(&thread_pool, null_tracer()) + .vote_with_stake_delegations_map; assert_eq!(vote_and_stake_accounts.len(), 0); } diff --git a/runtime/src/stakes.rs b/runtime/src/stakes.rs index c58f0c2b93..2874a8188a 100644 --- a/runtime/src/stakes.rs +++ b/runtime/src/stakes.rs @@ -150,6 +150,18 @@ impl Stakes { && account.data().len() >= std::mem::size_of::() } + pub fn remove_vote_account(&mut self, vote_pubkey: &Pubkey) { + self.vote_accounts.remove(vote_pubkey); + } + + pub fn remove_stake_delegation(&mut self, stake_pubkey: &Pubkey) { + if let Some(removed_delegation) = self.stake_delegations.remove(stake_pubkey) { + let removed_stake = removed_delegation.stake(self.epoch, Some(&self.stake_history)); + self.vote_accounts + .sub_stake(&removed_delegation.voter_pubkey, removed_stake); + } + } + pub fn store( &mut self, pubkey: &Pubkey, diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 71e64b9612..00541c3566 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -271,6 +271,10 @@ pub mod reject_non_rent_exempt_vote_withdraws { solana_sdk::declare_id!("7txXZZD6Um59YoLMF7XUNimbMjsqsWhc7g2EniiTrmp1"); } +pub mod evict_invalid_stakes_cache_entries { + solana_sdk::declare_id!("EMX9Q7TVFAmQ9V1CggAkhMzhXSg8ECp7fHrWQX2G1chf"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -333,6 +337,7 @@ lazy_static! { (reject_empty_instruction_without_program::id(), "fail instructions which have native_loader as program_id directly"), (fixed_memcpy_nonoverlapping_check::id(), "use correct check for nonoverlapping regions in memcpy syscall"), (reject_non_rent_exempt_vote_withdraws::id(), "fail vote withdraw instructions which leave the account non-rent-exempt"), + (evict_invalid_stakes_cache_entries::id(), "evict invalid stakes cache entries on epoch boundaries"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()