Split the bank snapshot construction from file PR. These are the non-essential helper changes

core/src/validator.rs

@@ -86,7 +86,7 @@ use {
         snapshot_config::SnapshotConfig,
         snapshot_hash::StartingSnapshotHashes,
         snapshot_package::PendingSnapshotPackage,
-        snapshot_utils,
+        snapshot_utils::{self, move_and_async_delete_path},
     },
     solana_sdk::{
         clock::Slot,
@@ -2002,94 +2002,15 @@ fn get_stake_percent_in_gossip(bank: &Bank, cluster_info: &ClusterInfo, log: boo
     online_stake_percentage as u64
 }
 
-/// Delete directories/files asynchronously to avoid blocking on it.
-/// Fist, in sync context, rename the original path to *_deleted,
-/// then spawn a thread to delete the renamed path.
-/// If the process is killed and the deleting process is not done,
-/// the leftover path will be deleted in the next process life, so
-/// there is no file space leaking.
-pub fn move_and_async_delete_path(path: impl AsRef<Path> + Copy) {
-    let mut path_delete = PathBuf::new();
-    path_delete.push(path);
-    path_delete.set_file_name(format!(
-        "{}{}",
-        path_delete.file_name().unwrap().to_str().unwrap(),
-        "_to_be_deleted"
-    ));
-
-    if path_delete.exists() {
-        std::fs::remove_dir_all(&path_delete).unwrap();
-    }
-
-    if !path.as_ref().exists() {
-        return;
-    }
-
-    if let Err(err) = std::fs::rename(path, &path_delete) {
-        warn!(
-            "Path renaming failed: {}.  Falling back to rm_dir in sync mode",
-            err.to_string()
-        );
-        delete_contents_of_path(path);
-        return;
-    }
-
-    Builder::new()
-        .name("solDeletePath".to_string())
-        .spawn(move || {
-            std::fs::remove_dir_all(path_delete).unwrap();
-        })
-        .unwrap();
-}
-
-/// Delete the files and subdirectories in a directory.
-/// This is useful if the process does not have permission
-/// to delete the top level directory it might be able to
-/// delete the contents of that directory.
-fn delete_contents_of_path(path: impl AsRef<Path> + Copy) {
-    if let Ok(dir_entries) = std::fs::read_dir(path) {
-        for entry in dir_entries.flatten() {
-            let sub_path = entry.path();
-            let metadata = match entry.metadata() {
-                Ok(metadata) => metadata,
-                Err(err) => {
-                    warn!(
-                        "Failed to get metadata for {}. Error: {}",
-                        sub_path.display(),
-                        err.to_string()
-                    );
-                    break;
-                }
-            };
-            if metadata.is_dir() {
-                if let Err(err) = std::fs::remove_dir_all(&sub_path) {
-                    warn!(
-                        "Failed to remove sub directory {}.  Error: {}",
-                        sub_path.display(),
-                        err.to_string()
-                    );
-                }
-            } else if metadata.is_file() {
-                if let Err(err) = std::fs::remove_file(&sub_path) {
-                    warn!(
-                        "Failed to remove file {}.  Error: {}",
-                        sub_path.display(),
-                        err.to_string()
-                    );
-                }
-            }
-        }
-    } else {
-        warn!(
-            "Failed to read the sub paths of {}",
-            path.as_ref().display()
-        );
-    }
-}
-
 fn cleanup_accounts_paths(config: &ValidatorConfig) {
     for accounts_path in &config.account_paths {
         move_and_async_delete_path(accounts_path);
+        // Let the empty top accounts/ dir exist.  It was created at very early
+        // stage.  It should not disappear after clearing its content.
+        if std::fs::metadata(accounts_path).is_err() {
+            // not the /mnt/account case
+            std::fs::create_dir_all(accounts_path).unwrap();
+        }
     }
     if let Some(ref shrink_paths) = config.account_shrink_paths {
         for accounts_path in shrink_paths {

ledger-tool/src/main.rs

@@ -24,10 +24,7 @@ use {
         },
     },
     solana_cli_output::{CliAccount, CliAccountNewConfig, OutputFormat},
-    solana_core::{
-        system_monitor_service::{SystemMonitorService, SystemMonitorStatsReportConfig},
-        validator::move_and_async_delete_path,
-    },
+    solana_core::system_monitor_service::{SystemMonitorService, SystemMonitorStatsReportConfig},
     solana_entry::entry::Entry,
     solana_geyser_plugin_manager::geyser_plugin_service::GeyserPluginService,
     solana_ledger::{
@@ -63,8 +60,8 @@ use {
         snapshot_hash::StartingSnapshotHashes,
         snapshot_minimizer::SnapshotMinimizer,
         snapshot_utils::{
-            self, ArchiveFormat, SnapshotVersion, DEFAULT_ARCHIVE_COMPRESSION,
-            SUPPORTED_ARCHIVE_COMPRESSION,
+            self, move_and_async_delete_path, ArchiveFormat, SnapshotVersion,
+            DEFAULT_ARCHIVE_COMPRESSION, SUPPORTED_ARCHIVE_COMPRESSION,
         },
     },
     solana_sdk::{

runtime/src/accounts_db.rs

@@ -5410,7 +5410,9 @@ impl AccountsDb {
                     let old_id = ret.append_vec_id();
                     ret.recycle(slot, self.next_id());
                     debug!(
-                        "recycling store: {} {:?} old_id: {}",
+                        "recycling store: slot param {},  slot {}, id{}, path {:?} old_id: {}",
+                        slot,
+                        ret.slot(),
                         ret.append_vec_id(),
                         ret.get_path(),
                         old_id

runtime/src/append_vec.rs

@@ -435,14 +435,15 @@ impl AppendVec {
     }
 
     pub fn new_from_file<P: AsRef<Path>>(path: P, current_len: usize) -> io::Result<(Self, usize)> {
-        let new = Self::new_from_file_unchecked(path, current_len)?;
+        let new = Self::new_from_file_unchecked(&path, current_len)?;
 
         let (sanitized, num_accounts) = new.sanitize_layout_and_length();
         if !sanitized {
-            return Err(std::io::Error::new(
-                std::io::ErrorKind::Other,
-                "incorrect layout/length/data",
-            ));
+            let err_msg = format!(
+                "incorrect layout/length/data in the appendvec at path {}",
+                path.as_ref().display()
+            );
+            return Err(std::io::Error::new(std::io::ErrorKind::Other, err_msg));
         }
 
         Ok((new, num_accounts))
@@ -1149,7 +1150,7 @@ pub mod tests {
         }
 
         let result = AppendVec::new_from_file(path, accounts_len);
-        assert_matches!(result, Err(ref message) if message.to_string() == *"incorrect layout/length/data");
+        assert_matches!(result, Err(ref message) if message.to_string().starts_with("incorrect layout/length/data"));
     }
 
     #[test]
@@ -1177,7 +1178,7 @@ pub mod tests {
         let accounts_len = av.len();
         drop(av);
         let result = AppendVec::new_from_file(path, accounts_len);
-        assert_matches!(result, Err(ref message) if message.to_string() == *"incorrect layout/length/data");
+        assert_matches!(result, Err(ref message) if message.to_string().starts_with("incorrect layout/length/data"));
     }
 
     #[test]
@@ -1203,7 +1204,7 @@ pub mod tests {
         let accounts_len = av.len();
         drop(av);
         let result = AppendVec::new_from_file(path, accounts_len);
-        assert_matches!(result, Err(ref message) if message.to_string() == *"incorrect layout/length/data");
+        assert_matches!(result, Err(ref message) if message.to_string().starts_with("incorrect layout/length/data"));
     }
 
     #[test]
@@ -1265,6 +1266,6 @@ pub mod tests {
         let accounts_len = av.len();
         drop(av);
         let result = AppendVec::new_from_file(path, accounts_len);
-        assert_matches!(result, Err(ref message) if message.to_string() == *"incorrect layout/length/data");
+        assert_matches!(result, Err(ref message) if message.to_string().starts_with("incorrect layout/length/data"));
     }
 }

runtime/src/bank.rs

@@ -3185,6 +3185,11 @@ impl Bank {
     }
 
     pub fn freeze(&self) {
+        let builtins = builtins::get();
+        if !self.check_builtins_exist(&builtins) {
+            warn!("builtins missing when freezing the bank");
+        }
+
         // This lock prevents any new commits from BankingStage
         // `process_and_record_transactions_locked()` from coming
         // in after the last tick is observed. This is because in
@@ -3361,7 +3366,10 @@ impl Bank {
                     if native_loader::check_id(account.owner()) {
                         Some(account)
                     } else {
-                        // malicious account is pre-occupying at program_id
+                        info!(
+                            "malicious account {:?} is pre-occupying at program_id {}",
+                            account, program_id
+                        );
                         self.burn_and_purge_account(program_id, account);
                         None
                     }
@@ -6389,6 +6397,17 @@ impl Bank {
         self.rc.accounts.clone()
     }
 
+    ///  Check builtins all exist.
+    pub fn check_builtins_exist(&self, builtins: &Builtins) -> bool {
+        builtins
+            .genesis_builtins
+            .iter()
+            .all(|b| self.get_account_with_fixed_root(&b.id).is_some())
+            && get_precompiles()
+                .iter()
+                .all(|p| self.get_account_with_fixed_root(&p.program_id).is_some())
+    }
+
     fn finish_init(
         &mut self,
         genesis_config: &GenesisConfig,

runtime/src/serde_snapshot.rs

@@ -721,6 +721,10 @@ where
     );
 
     let next_append_vec_id = next_append_vec_id.load(Ordering::Acquire);
+    assert!(
+        next_append_vec_id.checked_sub(1).is_some(),
+        "subtraction underflow"
+    );
     let max_append_vec_id = next_append_vec_id - 1;
     assert!(
         max_append_vec_id <= AppendVecId::MAX / 2,

runtime/src/snapshot_config.rs

@@ -41,7 +41,7 @@ pub struct SnapshotConfig {
     /// This is the `debug_verify` parameter to use when calling `update_accounts_hash()`
     pub accounts_hash_debug_verify: bool,
 
-    // Thread niceness adjustment for snapshot packager service
+    /// Thread niceness adjustment for snapshot packager service
     pub packager_thread_niceness_adj: i8,
 }