Port various rent fixes to runtime feature (bp #12842) (#13067)

* Port various rent fixes to runtime feature (#12842) * Port various rent fixes to runtime feature * Fix CI * Use more consistent naming... (cherry picked from commit 608b81b) # Conflicts: # runtime/src/bank.rs * Fix conflict Co-authored-by: Ryo Onodera <[email protected]>
solana-labs · Oct 21, 2020 · 63fe350 · 63fe350
1 parent b384ce9
commit 63fe350
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 71 deletions.
diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs
@@ -144,6 +144,8 @@ impl Accounts {
             let mut payer_index = None;
             let mut tx_rent: TransactionRent = 0;
             let mut accounts = 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 Self::is_non_loader_key(message, key, i) {
                     if payer_index.is_none() {
@@ -163,7 +165,11 @@ impl Accounts {
                                 .map(|(mut account, _)| {
                                     if message.is_writable(i) {
                                         let rent_due = rent_collector
-                                            .collect_from_existing_account(&key, &mut account);
+                                            .collect_from_existing_account(
+                                                &key,
+                                                &mut account,
+                                                rent_fix_enabled,
+                                            );
                                         (account, rent_due)
                                     } else {
                                         (account, 0)
@@ -721,6 +727,8 @@ impl Accounts {
     }
 
     /// Store the accounts into the DB
+    // allow(clippy) needed for various gating flags
+    #[allow(clippy::too_many_arguments)]
     pub fn store_accounts(
         &self,
         slot: Slot,
@@ -731,6 +739,7 @@ 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,
@@ -740,6 +749,7 @@ impl Accounts {
             rent_collector,
             last_blockhash_with_fee_calculator,
             fix_recent_blockhashes_sysvar_delay,
+            rent_fix_enabled,
         );
         self.accounts_db.store(slot, &accounts_to_store);
     }
@@ -767,6 +777,7 @@ 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, _hash_age_kind), (_, tx))) in loaded
@@ -807,7 +818,11 @@ impl Accounts {
                 );
                 if message.is_writable(i) {
                     if account.rent_epoch == 0 {
-                        acc.2 += rent_collector.collect_from_created_account(&key, account);
+                        acc.2 += rent_collector.collect_from_created_account(
+                            &key,
+                            account,
+                            rent_fix_enabled,
+                        );
                     }
                     accounts.push((key, &*account));
                 }
@@ -1123,7 +1138,6 @@ mod tests {
                 lamports_per_byte_year: 42,
                 ..Rent::default()
             },
-            ClusterType::Development,
         );
         let min_balance = rent_collector.rent.minimum_balance(nonce::State::size());
         let fee_calculator = FeeCalculator::new(min_balance);
@@ -1801,6 +1815,7 @@ mod tests {
             &rent_collector,
             &(Hash::default(), FeeCalculator::default()),
             true,
+            true,
         );
         assert_eq!(collected_accounts.len(), 2);
         assert!(collected_accounts

diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs
@@ -805,9 +805,7 @@ impl Bank {
             slots_per_year: parent.slots_per_year,
             epoch_schedule,
             collected_rent: AtomicU64::new(0),
-            rent_collector: parent
-                .rent_collector
-                .clone_with_epoch(epoch, parent.cluster_type()),
+            rent_collector: parent.rent_collector.clone_with_epoch(epoch),
             max_tick_height: (slot + 1) * parent.ticks_per_slot,
             block_height: parent.block_height + 1,
             fee_calculator: fee_rate_governor.create_fee_calculator(),
@@ -925,9 +923,7 @@ impl Bank {
             fee_rate_governor: fields.fee_rate_governor,
             collected_rent: AtomicU64::new(fields.collected_rent),
             // clone()-ing is needed to consider a gated behavior in rent_collector
-            rent_collector: fields
-                .rent_collector
-                .clone_with_epoch(fields.epoch, genesis_config.cluster_type),
+            rent_collector: fields.rent_collector.clone_with_epoch(fields.epoch),
             epoch_schedule: fields.epoch_schedule,
             inflation: Arc::new(RwLock::new(fields.inflation)),
             stakes: RwLock::new(fields.stakes),
@@ -1599,7 +1595,6 @@ impl Bank {
             &self.epoch_schedule,
             self.slots_per_year,
             &genesis_config.rent,
-            self.cluster_type(),
         );
 
         // Add additional native programs specified in the genesis config
@@ -2522,6 +2517,7 @@ 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);
 
@@ -2764,9 +2760,11 @@ impl Bank {
         // parallelize?
         let mut rent = 0;
         for (pubkey, mut account) in accounts {
-            rent += self
-                .rent_collector
-                .collect_from_existing_account(&pubkey, &mut account);
+            rent += self.rent_collector.collect_from_existing_account(
+                &pubkey,
+                &mut account,
+                self.cumulative_rent_related_fixes_enabled(),
+            );
             // Store all of them unconditionally to purge old AppendVec,
             // even if collected rent is 0 (= not updated).
             self.store_account(&pubkey, &account);
@@ -2884,12 +2882,9 @@ impl Bank {
             self.get_epoch_and_slot_index(self.parent_slot());
 
         let should_enable = match self.cluster_type() {
-            ClusterType::Development => true,
-            ClusterType::Devnet => true,
-            ClusterType::Testnet => current_epoch >= 97,
             ClusterType::MainnetBeta => {
                 #[cfg(not(test))]
-                let should_enable = current_epoch >= Epoch::max_value();
+                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
@@ -2898,6 +2893,7 @@ impl Bank {
 
                 should_enable
             }
+            _ => self.cumulative_rent_related_fixes_enabled(),
         };
 
         let mut partitions = vec![];
@@ -3244,21 +3240,19 @@ impl Bank {
     pub fn deposit(&self, pubkey: &Pubkey, lamports: u64) -> u64 {
         let mut account = self.get_account(pubkey).unwrap_or_default();
 
-        let should_be_in_new_behavior = match self.cluster_type() {
-            ClusterType::Development => true,
-            ClusterType::Devnet => true,
-            ClusterType::Testnet => self.epoch() >= 97,
-            ClusterType::MainnetBeta => self.epoch() >= Epoch::max_value(),
-        };
+        let rent_fix_enabled = self.cumulative_rent_related_fixes_enabled();
 
         // don't collect rents if we're in the new behavior;
         // in genral, it's not worthwhile to account for rents outside the runtime (transactions)
         // there are too many and subtly nuanced modification codepaths
-        if !should_be_in_new_behavior {
+        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),
+                self.rent_collector.collect_from_existing_account(
+                    pubkey,
+                    &mut account,
+                    rent_fix_enabled,
+                ),
                 Relaxed,
             );
         }
@@ -3818,6 +3812,10 @@ 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()
+    }
+
     // This is called from snapshot restore AND for each epoch boundary
     // The entire code path herein must be idempotent
     fn apply_feature_activations(&mut self, init_finish_or_warp: bool) {
@@ -4023,7 +4021,8 @@ mod tests {
     use crate::{
         accounts_index::{AccountMap, Ancestors},
         genesis_utils::{
-            create_genesis_config_with_leader, GenesisConfigInfo, BOOTSTRAP_VALIDATOR_LAMPORTS,
+            activate_all_features, create_genesis_config_with_leader, GenesisConfigInfo,
+            BOOTSTRAP_VALIDATOR_LAMPORTS,
         },
         native_loader::NativeLoaderError,
         process_instruction::InvokeContext,
@@ -4989,7 +4988,8 @@ mod tests {
 
     #[test]
     fn test_rent_eager_across_epoch_with_full_gap() {
-        let (genesis_config, _mint_keypair) = create_genesis_config(1);
+        let (mut genesis_config, _mint_keypair) = create_genesis_config(1);
+        activate_all_features(&mut genesis_config);
 
         let mut bank = Arc::new(Bank::new(&genesis_config));
         assert_eq!(bank.rent_collection_partitions(), vec![(0, 0, 32)]);
@@ -5011,7 +5011,8 @@ mod tests {
 
     #[test]
     fn test_rent_eager_across_epoch_with_half_gap() {
-        let (genesis_config, _mint_keypair) = create_genesis_config(1);
+        let (mut genesis_config, _mint_keypair) = create_genesis_config(1);
+        activate_all_features(&mut genesis_config);
 
         let mut bank = Arc::new(Bank::new(&genesis_config));
         assert_eq!(bank.rent_collection_partitions(), vec![(0, 0, 32)]);
@@ -5559,7 +5560,8 @@ mod tests {
     fn test_rent_eager_collect_rent_in_partition() {
         solana_logger::setup();
 
-        let (genesis_config, _mint_keypair) = create_genesis_config(1);
+        let (mut genesis_config, _mint_keypair) = create_genesis_config(1);
+        activate_all_features(&mut genesis_config);
 
         let zero_lamport_pubkey = Pubkey::new_rand();
         let rent_due_pubkey = Pubkey::new_rand();

diff --git a/runtime/src/feature_set.rs b/runtime/src/feature_set.rs
@@ -111,6 +111,11 @@ impl FeatureSet {
     pub fn is_active(&self, feature_id: &Pubkey) -> bool {
         self.active.contains(feature_id)
     }
+
+    pub fn cumulative_rent_related_fixes_enabled(&self) -> bool {
+        self.is_active(&cumulative_rent_related_fixes::id())
+    }
+
     /// All features enabled, useful for testing
     pub fn all_enabled() -> Self {
         Self {