From c92c09a8be8a752e46b8ff26988f711de08da579 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Tue, 14 Dec 2021 09:23:36 -0500 Subject: [PATCH] Remove activated feature for removing inactive delegations from stakes cache (#21732) * Remove activated feature for removing inactive delegations from stakes cache * Fix builtin purging --- runtime/src/bank.rs | 22 +++-------- runtime/src/stakes.rs | 92 ++++++++++++++++++------------------------- 2 files changed, 44 insertions(+), 70 deletions(-) diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index b2306721f6..96761b8c84 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -2792,9 +2792,10 @@ impl Bank { fn burn_and_purge_account(&self, program_id: &Pubkey, mut account: AccountSharedData) { self.capitalization.fetch_sub(account.lamports(), Relaxed); - // Resetting account balance to 0 is needed to really purge from AccountsDb and - // flush the Stakes cache + // Both resetting account balance to 0 and zeroing the account data + // is needed to really purge from AccountsDb and flush the Stakes cache account.set_lamports(0); + account.data_as_mut_slice().fill(0); self.store_account(program_id, &account); } @@ -4763,11 +4764,7 @@ impl Bank { .accounts .store_slow_cached(self.slot(), pubkey, account); - self.stakes_cache.check_and_store( - pubkey, - account, - self.stakes_remove_delegation_if_inactive_enabled(), - ); + self.stakes_cache.check_and_store(pubkey, account); } pub fn force_flush_accounts_cache(&self) { @@ -5505,11 +5502,7 @@ impl Bank { for (_i, (pubkey, account)) in (0..message.account_keys_len()).zip(loaded_transaction.accounts.iter()) { - self.stakes_cache.check_and_store( - pubkey, - account, - self.stakes_remove_delegation_if_inactive_enabled(), - ); + self.stakes_cache.check_and_store(pubkey, account); } } } @@ -5768,11 +5761,6 @@ impl Bank { .is_active(&feature_set::leave_nonce_on_success::id()) } - pub fn stakes_remove_delegation_if_inactive_enabled(&self) -> bool { - self.feature_set - .is_active(&feature_set::stakes_remove_delegation_if_inactive::id()) - } - pub fn send_to_tpu_vote_port_enabled(&self) -> bool { self.feature_set .is_active(&feature_set::send_to_tpu_vote_port::id()) diff --git a/runtime/src/stakes.rs b/runtime/src/stakes.rs index 970889e3d1..04d5e00d73 100644 --- a/runtime/src/stakes.rs +++ b/runtime/src/stakes.rs @@ -55,12 +55,7 @@ impl StakesCache { && account.data().len() >= std::mem::size_of::() } - pub fn check_and_store( - &self, - pubkey: &Pubkey, - account: &AccountSharedData, - remove_delegation_on_inactive: bool, - ) { + pub fn check_and_store(&self, pubkey: &Pubkey, account: &AccountSharedData) { if solana_vote_program::check_id(account.owner()) { let new_vote_account = if account.lamports() != 0 && VoteState::is_correct_size_and_initialized(account.data()) @@ -93,17 +88,10 @@ impl StakesCache { (stake, delegation) }); - let remove_delegation = if remove_delegation_on_inactive { - new_delegation.is_none() - } else { - account.lamports() == 0 - }; - - self.0.write().unwrap().update_stake_delegation( - pubkey, - new_delegation, - remove_delegation, - ); + self.0 + .write() + .unwrap() + .update_stake_delegation(pubkey, new_delegation); } } @@ -314,7 +302,6 @@ impl Stakes { &mut self, stake_pubkey: &Pubkey, new_delegation: Option<(u64, Delegation)>, - remove_delegation: bool, ) { // old_stake is stake lamports and voter_pubkey from the pre-store() version let old_stake = self.stake_delegations.get(stake_pubkey).map(|delegation| { @@ -336,13 +323,13 @@ impl Stakes { } } - if remove_delegation { - // when account is removed (lamports == 0), remove it from Stakes as well - // so that given `pubkey` can be used for any owner in the future, while not + if let Some((_stake, delegation)) = new_delegation { + self.stake_delegations.insert(*stake_pubkey, delegation); + } else { + // when stake is no longer delegated, remove it from Stakes so that + // given `pubkey` can be used for any owner in the future, while not // affecting Stakes. self.stake_delegations.remove(stake_pubkey); - } else if let Some((_stake, delegation)) = new_delegation { - self.stake_delegations.insert(*stake_pubkey, delegation); } } @@ -450,8 +437,8 @@ pub mod tests { let ((vote_pubkey, vote_account), (stake_pubkey, mut stake_account)) = create_staked_node_accounts(10); - stakes_cache.check_and_store(&vote_pubkey, &vote_account, true); - stakes_cache.check_and_store(&stake_pubkey, &stake_account, true); + stakes_cache.check_and_store(&vote_pubkey, &vote_account); + stakes_cache.check_and_store(&stake_pubkey, &stake_account); let stake = stake_state::stake_from(&stake_account).unwrap(); { let stakes = stakes_cache.stakes(); @@ -464,7 +451,7 @@ pub mod tests { } stake_account.set_lamports(42); - stakes_cache.check_and_store(&stake_pubkey, &stake_account, true); + stakes_cache.check_and_store(&stake_pubkey, &stake_account); { let stakes = stakes_cache.stakes(); let vote_accounts = stakes.vote_accounts(); @@ -477,7 +464,7 @@ pub mod tests { // activate more let (_stake_pubkey, mut stake_account) = create_stake_account(42, &vote_pubkey); - stakes_cache.check_and_store(&stake_pubkey, &stake_account, true); + stakes_cache.check_and_store(&stake_pubkey, &stake_account); let stake = stake_state::stake_from(&stake_account).unwrap(); { let stakes = stakes_cache.stakes(); @@ -490,7 +477,7 @@ pub mod tests { } stake_account.set_lamports(0); - stakes_cache.check_and_store(&stake_pubkey, &stake_account, true); + stakes_cache.check_and_store(&stake_pubkey, &stake_account); { let stakes = stakes_cache.stakes(); let vote_accounts = stakes.vote_accounts(); @@ -509,14 +496,14 @@ pub mod tests { let ((vote_pubkey, vote_account), (stake_pubkey, stake_account)) = create_staked_node_accounts(10); - stakes_cache.check_and_store(&vote_pubkey, &vote_account, true); - stakes_cache.check_and_store(&stake_pubkey, &stake_account, true); + stakes_cache.check_and_store(&vote_pubkey, &vote_account); + stakes_cache.check_and_store(&stake_pubkey, &stake_account); let ((vote11_pubkey, vote11_account), (stake11_pubkey, stake11_account)) = create_staked_node_accounts(20); - stakes_cache.check_and_store(&vote11_pubkey, &vote11_account, true); - stakes_cache.check_and_store(&stake11_pubkey, &stake11_account, true); + stakes_cache.check_and_store(&vote11_pubkey, &vote11_account); + stakes_cache.check_and_store(&stake11_pubkey, &stake11_account); let vote11_node_pubkey = VoteState::from(&vote11_account).unwrap().node_pubkey; @@ -534,8 +521,8 @@ pub mod tests { let ((vote_pubkey, mut vote_account), (stake_pubkey, stake_account)) = create_staked_node_accounts(10); - stakes_cache.check_and_store(&vote_pubkey, &vote_account, true); - stakes_cache.check_and_store(&stake_pubkey, &stake_account, true); + stakes_cache.check_and_store(&vote_pubkey, &vote_account); + stakes_cache.check_and_store(&stake_pubkey, &stake_account); { let stakes = stakes_cache.stakes(); @@ -545,7 +532,7 @@ pub mod tests { } vote_account.set_lamports(0); - stakes_cache.check_and_store(&vote_pubkey, &vote_account, true); + stakes_cache.check_and_store(&vote_pubkey, &vote_account); { let stakes = stakes_cache.stakes(); @@ -554,7 +541,7 @@ pub mod tests { } vote_account.set_lamports(1); - stakes_cache.check_and_store(&vote_pubkey, &vote_account, true); + stakes_cache.check_and_store(&vote_pubkey, &vote_account); { let stakes = stakes_cache.stakes(); @@ -568,7 +555,7 @@ pub mod tests { let mut pushed = vote_account.data().to_vec(); pushed.push(0); vote_account.set_data(pushed); - stakes_cache.check_and_store(&vote_pubkey, &vote_account, true); + stakes_cache.check_and_store(&vote_pubkey, &vote_account); { let stakes = stakes_cache.stakes(); @@ -580,7 +567,7 @@ pub mod tests { let default_vote_state = VoteState::default(); let versioned = VoteStateVersions::new_current(default_vote_state); VoteState::to(&versioned, &mut vote_account).unwrap(); - stakes_cache.check_and_store(&vote_pubkey, &vote_account, true); + stakes_cache.check_and_store(&vote_pubkey, &vote_account); { let stakes = stakes_cache.stakes(); @@ -589,7 +576,7 @@ pub mod tests { } vote_account.set_data(cache_data); - stakes_cache.check_and_store(&vote_pubkey, &vote_account, true); + stakes_cache.check_and_store(&vote_pubkey, &vote_account); { let stakes = stakes_cache.stakes(); @@ -612,11 +599,11 @@ pub mod tests { let ((vote_pubkey2, vote_account2), (_stake_pubkey2, stake_account2)) = create_staked_node_accounts(10); - stakes_cache.check_and_store(&vote_pubkey, &vote_account, true); - stakes_cache.check_and_store(&vote_pubkey2, &vote_account2, true); + stakes_cache.check_and_store(&vote_pubkey, &vote_account); + stakes_cache.check_and_store(&vote_pubkey2, &vote_account2); // delegates to vote_pubkey - stakes_cache.check_and_store(&stake_pubkey, &stake_account, true); + stakes_cache.check_and_store(&stake_pubkey, &stake_account); let stake = stake_state::stake_from(&stake_account).unwrap(); @@ -633,7 +620,7 @@ pub mod tests { } // delegates to vote_pubkey2 - stakes_cache.check_and_store(&stake_pubkey, &stake_account2, true); + stakes_cache.check_and_store(&stake_pubkey, &stake_account2); { let stakes = stakes_cache.stakes(); @@ -659,11 +646,11 @@ pub mod tests { let (stake_pubkey2, stake_account2) = create_stake_account(10, &vote_pubkey); - stakes_cache.check_and_store(&vote_pubkey, &vote_account, true); + stakes_cache.check_and_store(&vote_pubkey, &vote_account); // delegates to vote_pubkey - stakes_cache.check_and_store(&stake_pubkey, &stake_account, true); - stakes_cache.check_and_store(&stake_pubkey2, &stake_account2, true); + stakes_cache.check_and_store(&stake_pubkey, &stake_account); + stakes_cache.check_and_store(&stake_pubkey2, &stake_account2); { let stakes = stakes_cache.stakes(); @@ -680,8 +667,8 @@ pub mod tests { let ((vote_pubkey, vote_account), (stake_pubkey, stake_account)) = create_staked_node_accounts(10); - stakes_cache.check_and_store(&vote_pubkey, &vote_account, true); - stakes_cache.check_and_store(&stake_pubkey, &stake_account, true); + stakes_cache.check_and_store(&vote_pubkey, &vote_account); + stakes_cache.check_and_store(&stake_pubkey, &stake_account); let stake = stake_state::stake_from(&stake_account).unwrap(); { @@ -714,8 +701,8 @@ pub mod tests { let ((vote_pubkey, vote_account), (stake_pubkey, stake_account)) = create_staked_node_accounts(10); - stakes_cache.check_and_store(&vote_pubkey, &vote_account, true); - stakes_cache.check_and_store(&stake_pubkey, &stake_account, true); + stakes_cache.check_and_store(&vote_pubkey, &vote_account); + stakes_cache.check_and_store(&stake_pubkey, &stake_account); { let stakes = stakes_cache.stakes(); @@ -728,7 +715,6 @@ pub mod tests { stakes_cache.check_and_store( &stake_pubkey, &AccountSharedData::new(1, 0, &stake::program::id()), - true, ); { let stakes = stakes_cache.stakes(); @@ -759,8 +745,8 @@ pub mod tests { let genesis_epoch = 0; let ((vote_pubkey, vote_account), (stake_pubkey, stake_account)) = create_warming_staked_node_accounts(10, genesis_epoch); - stakes_cache.check_and_store(&vote_pubkey, &vote_account, true); - stakes_cache.check_and_store(&stake_pubkey, &stake_account, true); + stakes_cache.check_and_store(&vote_pubkey, &vote_account); + stakes_cache.check_and_store(&stake_pubkey, &stake_account); { let stakes = stakes_cache.stakes();