From 8b0c6db8714461341ea0b3eccf92501b35d79f00 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Mon, 8 Mar 2021 09:58:50 +0900 Subject: [PATCH] Remove old feature: cumulative_rent_related_fixes (#15754) --- ledger-tool/src/main.rs | 28 +++++----------- runtime/src/accounts.rs | 21 +++--------- runtime/src/bank.rs | 62 ++++++----------------------------- runtime/src/rent_collector.rs | 36 ++++++-------------- sdk/src/feature_set.rs | 9 ----- 5 files changed, 32 insertions(+), 124 deletions(-) diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 8ad1dbfa00..7a690b7857 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -2198,26 +2198,14 @@ fn main() { feature_account_balance, ), ); - if base_bank - .get_account(&feature_set::cumulative_rent_related_fixes::id()) - .is_some() - { - // steal some lamports from the pretty old feature not to affect - // capitalizaion, which doesn't affect inflation behavior! - base_bank.store_account( - &feature_set::cumulative_rent_related_fixes::id(), - &Account::default(), - ); - } else { - let old_cap = base_bank.set_capitalization(); - let new_cap = base_bank.capitalization(); - warn!( - "Skewing capitalization a bit to enable simple capitalization as \ - requested: increasing {} from {} to {}", - feature_account_balance, old_cap, new_cap, - ); - assert_eq!(old_cap + feature_account_balance, new_cap); - } + let old_cap = base_bank.set_capitalization(); + let new_cap = base_bank.capitalization(); + warn!( + "Skewing capitalization a bit to enable simple capitalization as \ + requested: increasing {} from {} to {}", + feature_account_balance, old_cap, new_cap, + ); + assert_eq!(old_cap + feature_account_balance, new_cap); } else { warn!("Already simple_capitalization is activated (or scheduled)"); } diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 602ee29b40..e681eedb9b 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -192,7 +192,6 @@ impl Accounts { let mut tx_rent: TransactionRent = 0; let mut accounts = Vec::with_capacity(message.account_keys.len()); let mut account_deps = Vec::with_capacity(message.account_keys.len()); - let rent_fix_enabled = feature_set.cumulative_rent_related_fixes_enabled(); for (i, key) in message.account_keys.iter().enumerate() { let account = if message.is_non_loader_key(key, i) { @@ -213,11 +212,8 @@ impl Accounts { .load(ancestors, key) .map(|(mut account, _)| { if message.is_writable(i) { - let rent_due = rent_collector.collect_from_existing_account( - &key, - &mut account, - rent_fix_enabled, - ); + let rent_due = rent_collector + .collect_from_existing_account(&key, &mut account); (account, rent_due) } else { (account, 0) @@ -836,7 +832,6 @@ impl Accounts { rent_collector: &RentCollector, last_blockhash_with_fee_calculator: &(Hash, FeeCalculator), fix_recent_blockhashes_sysvar_delay: bool, - rent_fix_enabled: bool, ) { let accounts_to_store = self.collect_accounts_to_store( txs, @@ -846,7 +841,6 @@ impl Accounts { rent_collector, last_blockhash_with_fee_calculator, fix_recent_blockhashes_sysvar_delay, - rent_fix_enabled, ); self.accounts_db.store_cached(slot, &accounts_to_store); } @@ -871,7 +865,6 @@ impl Accounts { rent_collector: &RentCollector, last_blockhash_with_fee_calculator: &(Hash, FeeCalculator), fix_recent_blockhashes_sysvar_delay: bool, - rent_fix_enabled: bool, ) -> Vec<(&'a Pubkey, &'a Account)> { let mut accounts = Vec::with_capacity(loaded.len()); for (i, ((raccs, _nonce_rollback), (_, tx))) in loaded @@ -940,11 +933,8 @@ impl Accounts { } } if account.rent_epoch == 0 { - loaded_transaction.rent += rent_collector.collect_from_created_account( - &key, - account, - rent_fix_enabled, - ); + loaded_transaction.rent += + rent_collector.collect_from_created_account(&key, account); } accounts.push((key, &*account)); } @@ -1899,7 +1889,6 @@ mod tests { &rent_collector, &(Hash::default(), FeeCalculator::default()), true, - true, ); assert_eq!(collected_accounts.len(), 2); assert!(collected_accounts @@ -2265,7 +2254,6 @@ mod tests { &rent_collector, &(next_blockhash, FeeCalculator::default()), true, - true, ); assert_eq!(collected_accounts.len(), 2); assert_eq!( @@ -2376,7 +2364,6 @@ mod tests { &rent_collector, &(next_blockhash, FeeCalculator::default()), true, - true, ); assert_eq!(collected_accounts.len(), 1); let collected_nonce_account = collected_accounts diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 92b5604b0c..dc0da85853 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3167,7 +3167,6 @@ impl Bank { &self.rent_collector, &self.last_blockhash_with_fee_calculator(), self.fix_recent_blockhashes_sysvar_delay(), - self.cumulative_rent_related_fixes_enabled(), ); self.collect_rent(executed, loaded_accounts); @@ -3416,11 +3415,9 @@ impl Bank { // parallelize? let mut rent = 0; for (pubkey, mut account) in accounts { - rent += self.rent_collector.collect_from_existing_account( - &pubkey, - &mut account, - self.cumulative_rent_related_fixes_enabled(), - ); + rent += self + .rent_collector + .collect_from_existing_account(&pubkey, &mut account); // Store all of them unconditionally to purge old AppendVec, // even if collected rent is 0 (= not updated). self.store_account(&pubkey, &account); @@ -3539,30 +3536,11 @@ impl Bank { let (parent_epoch, mut parent_slot_index) = self.get_epoch_and_slot_index(self.parent_slot()); - let should_enable = match self.cluster_type() { - ClusterType::MainnetBeta => { - #[cfg(not(test))] - let should_enable = self.cumulative_rent_related_fixes_enabled(); - - // needed for test_rent_eager_across_epoch_with_gap_under_multi_epoch_cycle, - // which depends on ClusterType::MainnetBeta - #[cfg(test)] - let should_enable = true; - - should_enable - } - _ => self.cumulative_rent_related_fixes_enabled(), - }; - let mut partitions = vec![]; if parent_epoch < current_epoch { // this needs to be gated because this potentially can change the behavior // (= bank hash) at each start of epochs - let slot_skipped = if should_enable { - (self.slot() - self.parent_slot()) > 1 - } else { - current_slot_index > 0 - }; + let slot_skipped = (self.slot() - self.parent_slot()) > 1; if slot_skipped { // Generate special partitions because there are skipped slots // exactly at the epoch transition. @@ -3578,7 +3556,7 @@ impl Bank { // this needs to be gated because this potentially can change the behavior // (= bank hash) at each start of epochs - if should_enable && current_slot_index > 0 { + if current_slot_index > 0 { // ... for current epoch partitions.push(self.partition_from_slot_indexes_with_gapped_epochs( 0, @@ -3942,25 +3920,9 @@ impl Bank { } pub fn deposit(&self, pubkey: &Pubkey, lamports: u64) -> u64 { + // This doesn't collect rents intentionally. + // Rents should only be applied to actual TXes let mut account = self.get_account(pubkey).unwrap_or_default(); - - let rent_fix_enabled = self.cumulative_rent_related_fixes_enabled(); - - // don't collect rents if we're in the new behavior; - // in general, it's not worthwhile to account for rents outside the runtime (transactions) - // there are too many and subtly nuanced modification codepaths - if !rent_fix_enabled { - // previously we're too much collecting rents as if it existed since epoch 0... - self.collected_rent.fetch_add( - self.rent_collector.collect_from_existing_account( - pubkey, - &mut account, - rent_fix_enabled, - ), - Relaxed, - ); - } - account.lamports += lamports; self.store_account(pubkey, &account); account.lamports @@ -4631,10 +4593,6 @@ impl Bank { .is_active(&feature_set::no_overflow_rent_distribution::id()) } - pub fn cumulative_rent_related_fixes_enabled(&self) -> bool { - self.feature_set.cumulative_rent_related_fixes_enabled() - } - pub fn stake_program_v2_enabled(&self) -> bool { self.feature_set .is_active(&feature_set::stake_program_v2::id()) @@ -10227,19 +10185,19 @@ pub(crate) mod tests { if bank.slot == 32 { assert_eq!( bank.hash().to_string(), - "4syPxVrVFUpksTre5BB5w7qd3BxSU4WzUT6R2fjFgMJ2" + "7AkMgAb2v4tuoiSf3NnVgaBxSvp7XidbrSwsPEn4ENTp" ); } if bank.slot == 64 { assert_eq!( bank.hash().to_string(), - "4GKgnCxQs6AJxcqYQkxa8oF8gEp13bfRNCm2uzCceA26" + "2JzWWRBtQgdXboaACBRXNNKsHeBtn57uYmqH1AgGUkdG" ); } if bank.slot == 128 { assert_eq!( bank.hash().to_string(), - "9YwXsk2qpM7bZLnWGdtqCmDEygiu1KpEcr4zWWBTUKw6" + "FQnVhDVjhCyfBxFb3bdm3CLiuCePvWuW5TGDsLBZnKAo" ); break; } diff --git a/runtime/src/rent_collector.rs b/runtime/src/rent_collector.rs index bd502020d3..0d64f15d72 100644 --- a/runtime/src/rent_collector.rs +++ b/runtime/src/rent_collector.rs @@ -50,12 +50,7 @@ impl RentCollector { // the account rent collected, if any // #[must_use = "add to Bank::collected_rent"] - pub fn collect_from_existing_account( - &self, - address: &Pubkey, - account: &mut Account, - rent_fix_enabled: bool, - ) -> u64 { + pub fn collect_from_existing_account(&self, address: &Pubkey, account: &mut Account) -> u64 { if account.executable || account.rent_epoch > self.epoch || sysvar::check_id(&account.owner) @@ -81,7 +76,7 @@ impl RentCollector { if exempt || rent_due != 0 { if account.lamports > rent_due { account.rent_epoch = self.epoch - + if rent_fix_enabled && exempt { + + if exempt { // Rent isn't collected for the next epoch // Make sure to check exempt status later in current epoch again 0 @@ -104,15 +99,10 @@ impl RentCollector { } #[must_use = "add to Bank::collected_rent"] - pub fn collect_from_created_account( - &self, - address: &Pubkey, - account: &mut Account, - enable_new_behavior: bool, - ) -> u64 { + pub fn collect_from_created_account(&self, address: &Pubkey, account: &mut Account) -> u64 { // initialize rent_epoch as created at this epoch account.rent_epoch = self.epoch; - self.collect_from_existing_account(address, account, enable_new_behavior) + self.collect_from_existing_account(address, account) } } @@ -139,21 +129,15 @@ mod tests { let rent_collector = RentCollector::default().clone_with_epoch(new_epoch); // collect rent on a newly-created account - let collected = rent_collector.collect_from_created_account( - &solana_sdk::pubkey::new_rand(), - &mut created_account, - true, - ); + let collected = rent_collector + .collect_from_created_account(&solana_sdk::pubkey::new_rand(), &mut created_account); assert!(created_account.lamports < old_lamports); assert_eq!(created_account.lamports + collected, old_lamports); assert_ne!(created_account.rent_epoch, old_epoch); // collect rent on a already-existing account - let collected = rent_collector.collect_from_existing_account( - &solana_sdk::pubkey::new_rand(), - &mut existing_account, - true, - ); + let collected = rent_collector + .collect_from_existing_account(&solana_sdk::pubkey::new_rand(), &mut existing_account); assert!(existing_account.lamports < old_lamports); assert_eq!(existing_account.lamports + collected, old_lamports); assert_ne!(existing_account.rent_epoch, old_epoch); @@ -179,7 +163,7 @@ mod tests { let rent_collector = RentCollector::default().clone_with_epoch(epoch); // first mark account as being collected while being rent-exempt - collected = rent_collector.collect_from_existing_account(&pubkey, &mut account, true); + collected = rent_collector.collect_from_existing_account(&pubkey, &mut account); assert_eq!(account.lamports, huge_lamports); assert_eq!(collected, 0); @@ -187,7 +171,7 @@ mod tests { account.lamports = tiny_lamports; // ... and trigger another rent collection on the same epoch and check that rent is working - collected = rent_collector.collect_from_existing_account(&pubkey, &mut account, true); + collected = rent_collector.collect_from_existing_account(&pubkey, &mut account); assert_eq!(account.lamports, tiny_lamports - collected); assert_ne!(collected, 0); } diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index 8a2ab2cdb6..b22313a612 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -55,10 +55,6 @@ pub mod ristretto_mul_syscall_enabled { solana_sdk::declare_id!("HRe7A6aoxgjKzdjbBv6HTy7tJ4YWqE6tVmYCGho6S9Aq"); } -pub mod cumulative_rent_related_fixes { - solana_sdk::declare_id!("FtjnuAtJTWwX3Kx9m24LduNEhzaGuuPfDW6e14SX2Fy5"); -} - pub mod pull_request_ping_pong_check { solana_sdk::declare_id!("5RzEHTnf6D7JPZCvwEzjM19kzBsyjSU3HoMfXaQmVgnZ"); } @@ -139,7 +135,6 @@ lazy_static! { (spl_token_v2_multisig_fix::id(), "spl-token multisig fix"), (no_overflow_rent_distribution::id(), "no overflow rent distribution"), (ristretto_mul_syscall_enabled::id(), "ristretto multiply syscall"), - (cumulative_rent_related_fixes::id(), "rent fixes (#10206, #10468, #11342)"), (pull_request_ping_pong_check::id(), "ping-pong packet check #12794"), (stake_program_v2::id(), "solana_stake_program v2"), (rewrite_stake::id(), "rewrite stake"), @@ -220,10 +215,6 @@ impl FeatureSet { self.active.get(feature_id).copied() } - pub fn cumulative_rent_related_fixes_enabled(&self) -> bool { - self.is_active(&cumulative_rent_related_fixes::id()) - } - /// List of enabled features that trigger full inflation pub fn full_inflation_features_enabled(&self) -> HashSet { let mut hash_set = FULL_INFLATION_FEATURE_PAIRS