Simplify Bank::clean_accounts() by removing params (#27254)

runtime/src/accounts_background_service.rs

@@ -254,11 +254,7 @@ impl SnapshotRequestHandler {
                 };
 
                 let mut clean_time = Measure::start("clean_time");
-                // Don't clean the slot we're snapshotting because it may have zero-lamport
-                // accounts that were included in the bank delta hash when the bank was frozen,
-                // and if we clean them here, the newly created snapshot's hash may not match
-                // the frozen hash.
-                snapshot_root_bank.clean_accounts(true, false, *last_full_snapshot_slot);
+                snapshot_root_bank.clean_accounts(*last_full_snapshot_slot);
                 clean_time.stop();
 
                 if accounts_db_caching_enabled {
@@ -564,7 +560,7 @@ impl AccountsBackgroundService {
                                 // slots >= bank.slot()
                                 bank.force_flush_accounts_cache();
                             }
-                            bank.clean_accounts(true, false, last_full_snapshot_slot);
+                            bank.clean_accounts(last_full_snapshot_slot);
                             last_cleaned_block_height = bank.block_height();
                         }
                     }

runtime/src/bank.rs

@@ -7159,7 +7159,7 @@ impl Bank {
         let mut clean_time = Measure::start("clean");
         if !accounts_db_skip_shrink && self.slot() > 0 {
             info!("cleaning..");
-            self.clean_accounts(true, true, Some(last_full_snapshot_slot));
+            self._clean_accounts(true, true, Some(last_full_snapshot_slot));
         }
         clean_time.stop();
 
@@ -7443,23 +7443,27 @@ impl Bank {
         debug!("Added precompiled program {:?}", program_id);
     }
 
-    pub fn clean_accounts(
-        &self,
-        skip_last: bool,
-        is_startup: bool,
-        last_full_snapshot_slot: Option<Slot>,
-    ) {
+    pub(crate) fn clean_accounts(&self, last_full_snapshot_slot: Option<Slot>) {
         // Don't clean the slot we're snapshotting because it may have zero-lamport
         // accounts that were included in the bank delta hash when the bank was frozen,
         // and if we clean them here, any newly created snapshot's hash for this bank
         // may not match the frozen hash.
         //
         // So when we're snapshotting, set `skip_last` to true so the highest slot to clean is
         // lowered by one.
-        let highest_slot_to_clean = skip_last.then(|| self.slot().saturating_sub(1));
+        self._clean_accounts(true, false, last_full_snapshot_slot)
+    }
+
+    fn _clean_accounts(
+        &self,
+        skip_last: bool,
+        is_startup: bool,
+        last_full_snapshot_slot: Option<Slot>,
+    ) {
+        let max_clean_root = skip_last.then(|| self.slot().saturating_sub(1));
 
         self.rc.accounts.accounts_db.clean_accounts(
-            highest_slot_to_clean,
+            max_clean_root,
             is_startup,
             last_full_snapshot_slot,
         );
@@ -16260,7 +16264,7 @@ pub(crate) mod tests {
                         current_bank.squash();
                         if current_bank.slot() % 2 == 0 {
                             current_bank.force_flush_accounts_cache();
-                            current_bank.clean_accounts(true, false, None);
+                            current_bank.clean_accounts(None);
                         }
                         prev_bank = current_bank.clone();
                         current_bank = Arc::new(Bank::new_from_parent(

runtime/src/snapshot_utils.rs

@@ -2078,7 +2078,7 @@ pub fn bank_to_full_snapshot_archive(
     assert!(bank.is_complete());
     bank.squash(); // Bank may not be a root
     bank.force_flush_accounts_cache();
-    bank.clean_accounts(true, false, Some(bank.slot()));
+    bank.clean_accounts(Some(bank.slot()));
     bank.update_accounts_hash();
     bank.rehash(); // Bank accounts may have been manually modified by the caller
 
@@ -2125,7 +2125,7 @@ pub fn bank_to_incremental_snapshot_archive(
     assert!(bank.slot() > full_snapshot_slot);
     bank.squash(); // Bank may not be a root
     bank.force_flush_accounts_cache();
-    bank.clean_accounts(true, false, Some(full_snapshot_slot));
+    bank.clean_accounts(Some(full_snapshot_slot));
     bank.update_accounts_hash();
     bank.rehash(); // Bank accounts may have been manually modified by the caller
 
@@ -3771,7 +3771,7 @@ mod tests {
 
         // Ensure account1 has been cleaned/purged from everywhere
         bank4.squash();
-        bank4.clean_accounts(true, false, Some(full_snapshot_slot));
+        bank4.clean_accounts(Some(full_snapshot_slot));
         assert!(
             bank4.get_account_modified_slot(&key1.pubkey()).is_none(),
             "Ensure Account1 has been cleaned and purged from AccountsDb"

runtime/src/system_instruction_processor.rs

@@ -1626,6 +1626,7 @@ mod tests {
             .unwrap();
 
         // super fun time; callback chooses to .clean_accounts(None) or not
+        let bank = Arc::new(Bank::new_from_parent(&bank, &collector, bank.slot() + 1));
         callback(&*bank);
 
         // create a normal account at the same pubkey as the zero-lamports account
@@ -1651,9 +1652,9 @@ mod tests {
             bank.squash();
             bank.force_flush_accounts_cache();
             // do clean and assert that it actually did its job
+            assert_eq!(4, bank.get_snapshot_storages(None).len());
+            bank.clean_accounts(None);
             assert_eq!(3, bank.get_snapshot_storages(None).len());
-            bank.clean_accounts(false, false, None);
-            assert_eq!(2, bank.get_snapshot_storages(None).len());
         });
     }