Add SnapshotArchiveInfo and refactor functions in snapshot_utils (#18232)

core/src/snapshot_packager_service.rs

@@ -156,9 +156,10 @@ mod tests {
         }
 
         // Create a packageable snapshot
-        let output_tar_path = snapshot_utils::get_snapshot_archive_path(
+        let output_tar_path = snapshot_utils::build_snapshot_archive_path(
             snapshot_package_output_path,
-            &(42, Hash::default()),
+            42,
+            &Hash::default(),
             ArchiveFormat::TarBzip2,
         );
         let snapshot_package = AccountsPackage::new(

core/tests/snapshots.rs

@@ -148,7 +148,7 @@ mod tests {
         let old_last_bank = old_bank_forks.get(old_last_slot).unwrap();
 
         let check_hash_calculation = false;
-        let (deserialized_bank, _timing) = snapshot_utils::bank_from_archive(
+        let (deserialized_bank, _timing) = snapshot_utils::bank_from_snapshot_archive(
             account_paths,
             &[],
             &old_bank_forks
@@ -156,9 +156,10 @@ mod tests {
                 .as_ref()
                 .unwrap()
                 .snapshot_path,
-            snapshot_utils::get_snapshot_archive_path(
+            snapshot_utils::build_snapshot_archive_path(
                 snapshot_package_output_path.to_path_buf(),
-                &(old_last_bank.slot(), old_last_bank.get_accounts_hash()),
+                old_last_bank.slot(),
+                &old_last_bank.get_accounts_hash(),
                 ArchiveFormat::TarBzip2,
             ),
             ArchiveFormat::TarBzip2,
@@ -418,9 +419,10 @@ mod tests {
                 let options = CopyOptions::new();
                 fs_extra::dir::copy(&last_snapshot_path, &saved_snapshots_dir, &options).unwrap();
 
-                saved_archive_path = Some(snapshot_utils::get_snapshot_archive_path(
+                saved_archive_path = Some(snapshot_utils::build_snapshot_archive_path(
                     snapshot_package_output_path.to_path_buf(),
-                    &(slot, accounts_hash),
+                    slot,
+                    &accounts_hash,
                     ArchiveFormat::TarBzip2,
                 ));
             }

download-utils/src/lib.rs

@@ -259,9 +259,10 @@ pub fn download_snapshot<'a, 'b>(
         ArchiveFormat::TarGzip,
         ArchiveFormat::TarBzip2,
     ] {
-        let desired_snapshot_package = snapshot_utils::get_snapshot_archive_path(
+        let desired_snapshot_package = snapshot_utils::build_snapshot_archive_path(
             snapshot_output_dir.to_path_buf(),
-            &desired_snapshot_hash,
+            desired_snapshot_hash.0,
+            &desired_snapshot_hash.1,
             *compression,
         );
 

ledger/src/bank_forks_utils.rs

@@ -11,7 +11,7 @@ use log::*;
 use solana_runtime::{
     bank_forks::BankForks,
     snapshot_config::SnapshotConfig,
-    snapshot_utils::{self, ArchiveFormat},
+    snapshot_utils::{self, SnapshotArchiveInfo},
 };
 use solana_sdk::{clock::Slot, genesis_config::GenesisConfig, hash::Hash};
 use std::{fs, path::PathBuf, process, result};
@@ -53,11 +53,9 @@ pub fn load(
         fs::create_dir_all(&snapshot_config.snapshot_path)
             .expect("Couldn't create snapshot directory");
 
-        if let Some((archive_filename, (archive_slot, archive_hash, archive_format))) =
+        if let Some(snapshot_archive_info) = snapshot_utils::get_highest_snapshot_archive_info(
-            snapshot_utils::get_highest_snapshot_archive_path(
             &snapshot_config.snapshot_package_output_path,
-            )
+        ) {
-        {
             return load_from_snapshot(
                 genesis_config,
                 blockstore,
@@ -67,10 +65,7 @@ pub fn load(
                 process_options,
                 transaction_status_sender,
                 cache_block_meta_sender,
-                archive_filename,
+                &snapshot_archive_info,
-                archive_slot,
-                archive_hash,
-                archive_format,
             );
         } else {
             info!("No snapshot package available; will load from genesis");
@@ -118,12 +113,12 @@ fn load_from_snapshot(
     process_options: ProcessOptions,
     transaction_status_sender: Option<&TransactionStatusSender>,
     cache_block_meta_sender: Option<&CacheBlockMetaSender>,
-    archive_filename: PathBuf,
+    snapshot_archive_info: &SnapshotArchiveInfo,
-    archive_slot: Slot,
-    archive_hash: Hash,
-    archive_format: ArchiveFormat,
 ) -> LoadResult {
-    info!("Loading snapshot package: {:?}", archive_filename);
+    info!(
+        "Loading snapshot package: {:?}",
+        &snapshot_archive_info.path
+    );
 
     // Fail hard here if snapshot fails to load, don't silently continue
     if account_paths.is_empty() {
@@ -131,12 +126,12 @@ fn load_from_snapshot(
         process::exit(1);
     }
 
-    let (deserialized_bank, timings) = snapshot_utils::bank_from_archive(
+    let (deserialized_bank, timings) = snapshot_utils::bank_from_snapshot_archive(
         &account_paths,
         &process_options.frozen_accounts,
         &snapshot_config.snapshot_path,
-        &archive_filename,
+        &snapshot_archive_info.path,
-        archive_format,
+        snapshot_archive_info.archive_format,
         genesis_config,
         process_options.debug_keys.clone(),
         Some(&crate::builtins::get(process_options.bpf_jit)),
@@ -156,10 +151,11 @@ fn load_from_snapshot(
         deserialized_bank.get_accounts_hash(),
     );
 
-    if deserialized_bank_slot_and_hash != (archive_slot, archive_hash) {
+    if deserialized_bank_slot_and_hash != (snapshot_archive_info.slot, snapshot_archive_info.hash) {
         error!(
             "Snapshot has mismatch:\narchive: {:?}\ndeserialized: {:?}",
-            archive_hash, deserialized_bank_slot_and_hash
+            (snapshot_archive_info.slot, snapshot_archive_info.hash),
+            deserialized_bank_slot_and_hash
         );
         process::exit(1);
     }

local-cluster/tests/local_cluster.rs

@@ -1671,12 +1671,13 @@ fn test_snapshot_download() {
         wait_for_next_snapshot(&cluster, snapshot_package_output_path);
 
     trace!("found: {:?}", archive_filename);
-    let validator_archive_path = snapshot_utils::get_snapshot_archive_path(
+    let validator_archive_path = snapshot_utils::build_snapshot_archive_path(
         validator_snapshot_test_config
             .snapshot_output_path
             .path()
             .to_path_buf(),
-        &archive_snapshot_hash,
+        archive_snapshot_hash.0,
+        &archive_snapshot_hash.1,
         ArchiveFormat::TarBzip2,
     );
 
@@ -1746,12 +1747,13 @@ fn test_snapshot_restart_tower() {
         wait_for_next_snapshot(&cluster, snapshot_package_output_path);
 
     // Copy archive to validator's snapshot output directory
-    let validator_archive_path = snapshot_utils::get_snapshot_archive_path(
+    let validator_archive_path = snapshot_utils::build_snapshot_archive_path(
         validator_snapshot_test_config
             .snapshot_output_path
             .path()
             .to_path_buf(),
-        &archive_snapshot_hash,
+        archive_snapshot_hash.0,
+        &archive_snapshot_hash.1,
         ArchiveFormat::TarBzip2,
     );
     fs::hard_link(archive_filename, &validator_archive_path).unwrap();
@@ -1806,9 +1808,9 @@ fn test_snapshots_blockstore_floor() {
 
     trace!("Waiting for snapshot tar to be generated with slot",);
 
-    let (archive_filename, (archive_slot, archive_hash, _)) = loop {
+    let archive_info = loop {
         let archive =
-            snapshot_utils::get_highest_snapshot_archive_path(&snapshot_package_output_path);
+            snapshot_utils::get_highest_snapshot_archive_info(&snapshot_package_output_path);
         if archive.is_some() {
             trace!("snapshot exists");
             break archive.unwrap();
@@ -1817,16 +1819,17 @@ fn test_snapshots_blockstore_floor() {
     };
 
     // Copy archive to validator's snapshot output directory
-    let validator_archive_path = snapshot_utils::get_snapshot_archive_path(
+    let validator_archive_path = snapshot_utils::build_snapshot_archive_path(
         validator_snapshot_test_config
             .snapshot_output_path
             .path()
             .to_path_buf(),
-        &(archive_slot, archive_hash),
+        archive_info.slot,
+        &archive_info.hash,
         ArchiveFormat::TarBzip2,
     );
-    fs::hard_link(archive_filename, &validator_archive_path).unwrap();
+    fs::hard_link(archive_info.path, &validator_archive_path).unwrap();
-    let slot_floor = archive_slot;
+    let slot_floor = archive_info.slot;
 
     // Start up a new node from a snapshot
     let validator_stake = 5;
@@ -3122,14 +3125,21 @@ fn wait_for_next_snapshot(
         last_slot
     );
     loop {
-        if let Some((filename, (slot, hash, _))) =
+        if let Some(snapshot_archive_info) =
-            snapshot_utils::get_highest_snapshot_archive_path(snapshot_package_output_path)
+            snapshot_utils::get_highest_snapshot_archive_info(snapshot_package_output_path)
         {
-            trace!("snapshot for slot {} exists", slot);
+            trace!("snapshot for slot {} exists", snapshot_archive_info.slot);
-            if slot >= last_slot {
+            if snapshot_archive_info.slot >= last_slot {
-                return (filename, (slot, hash));
+                return (
+                    snapshot_archive_info.path,
+                    (snapshot_archive_info.slot, snapshot_archive_info.hash),
+                );
             }
-            trace!("snapshot slot {} < last_slot {}", slot, last_slot);
+            trace!(
+                "snapshot slot {} < last_slot {}",
+                snapshot_archive_info.slot,
+                last_slot
+            );
         }
         sleep(Duration::from_millis(5000));
     }

rpc/src/rpc.rs

@@ -49,7 +49,7 @@ use {
         inline_spl_token_v2_0::{SPL_TOKEN_ACCOUNT_MINT_OFFSET, SPL_TOKEN_ACCOUNT_OWNER_OFFSET},
         non_circulating_supply::calculate_non_circulating_supply,
         snapshot_config::SnapshotConfig,
-        snapshot_utils::get_highest_snapshot_archive_path,
+        snapshot_utils,
     },
     solana_sdk::{
         account::{AccountSharedData, ReadableAccount},
@@ -2255,8 +2255,9 @@ pub mod rpc_minimal {
 
             meta.snapshot_config
                 .and_then(|snapshot_config| {
-                    get_highest_snapshot_archive_path(&snapshot_config.snapshot_package_output_path)
+                    snapshot_utils::get_highest_snapshot_archive_slot(
-                        .map(|(_, (slot, _, _))| slot)
+                        &snapshot_config.snapshot_package_output_path,
+                    )
                 })
                 .ok_or_else(|| RpcCustomError::NoSnapshot.into())
         }

rpc/src/rpc_service.rs

@@ -198,13 +198,14 @@ impl RequestMiddleware for RpcRequestMiddleware {
         if let Some(ref snapshot_config) = self.snapshot_config {
             if request.uri().path() == "/snapshot.tar.bz2" {
                 // Convenience redirect to the latest snapshot
-                return if let Some((snapshot_archive, _)) =
+                return if let Some(snapshot_archive_info) =
-                    snapshot_utils::get_highest_snapshot_archive_path(
+                    snapshot_utils::get_highest_snapshot_archive_info(
                         &snapshot_config.snapshot_package_output_path,
                     ) {
                     RpcRequestMiddleware::redirect(&format!(
                         "/{}",
-                        snapshot_archive
+                        snapshot_archive_info
+                            .path
                             .file_name()
                             .unwrap_or_else(|| std::ffi::OsStr::new(""))
                             .to_str()

runtime/src/snapshot_utils.rs

@@ -38,6 +38,20 @@ use {
     thiserror::Error,
 };
 
+/// Information about a snapshot archive: its path, slot, hash, and archive format
+pub struct SnapshotArchiveInfo {
+    /// Path to the snapshot archive file
+    pub path: PathBuf,
+
+    /// Slot that the snapshot was made
+    pub slot: Slot,
+
+    /// Hash of the accounts at this slot
+    pub hash: Hash,
+
+    /// Archive format for the snapshot file
+    pub archive_format: ArchiveFormat,
+}
 pub const SNAPSHOT_STATUS_CACHE_FILE_NAME: &str = "status_cache";
 
 pub const MAX_SNAPSHOTS: usize = 8; // Save some snapshots but not too many
@@ -47,6 +61,9 @@ const DEFAULT_SNAPSHOT_VERSION: SnapshotVersion = SnapshotVersion::V1_2_0;
 const TMP_SNAPSHOT_PREFIX: &str = "tmp-snapshot-";
 pub const DEFAULT_MAX_SNAPSHOTS_TO_RETAIN: usize = 2;
 
+pub const SNAPSHOT_ARCHIVE_FILENAME_REGEX: &str =
+    r"^snapshot-(\d+)-([[:alnum:]]+)\.(tar|tar\.bz2|tar\.zst|tar\.gz)$";
+
 #[derive(Copy, Clone, Eq, PartialEq, Debug)]
 pub enum SnapshotVersion {
     V1_2_0,
@@ -237,6 +254,7 @@ pub fn remove_tmp_snapshot_archives(snapshot_path: &Path) {
     }
 }
 
+/// Make a snapshot archive out of the AccountsPackage
 pub fn archive_snapshot_package(
     snapshot_package: &AccountsPackage,
     maximum_snapshots_to_retain: usize,
@@ -593,9 +611,9 @@ fn serialize_status_cache(
     Ok(())
 }
 
+/// Remove the snapshot directory for this slot
 pub fn remove_snapshot<P: AsRef<Path>>(slot: Slot, snapshot_path: P) -> Result<()> {
     let slot_snapshot_dir = get_bank_snapshot_dir(&snapshot_path, slot);
-    // Remove the snapshot directory for this slot
     fs::remove_dir_all(slot_snapshot_dir)?;
     Ok(())
 }
@@ -610,8 +628,9 @@ pub struct BankFromArchiveTimings {
 // From testing, 4 seems to be a sweet spot for ranges of 60M-360M accounts and 16-64 cores. This may need to be tuned later.
 pub const PARALLEL_UNTAR_READERS_DEFAULT: usize = 4;
 
+/// Rebuild a bank from a snapshot archive
 #[allow(clippy::too_many_arguments)]
-pub fn bank_from_archive<P: AsRef<Path> + std::marker::Sync>(
+pub fn bank_from_snapshot_archive<P>(
     account_paths: &[PathBuf],
     frozen_account_pubkeys: &[Pubkey],
     snapshot_path: &Path,
@@ -625,7 +644,10 @@ pub fn bank_from_archive<P: AsRef<Path> + std::marker::Sync>(
     limit_load_slot_count_from_snapshot: Option<usize>,
     shrink_ratio: AccountShrinkThreshold,
     test_hash_calculation: bool,
-) -> Result<(Bank, BankFromArchiveTimings)> {
+) -> Result<(Bank, BankFromArchiveTimings)>
+where
+    P: AsRef<Path> + std::marker::Sync,
+{
     let unpack_dir = tempfile::Builder::new()
         .prefix(TMP_SNAPSHOT_PREFIX)
         .tempdir_in(snapshot_path)?;
@@ -684,15 +706,18 @@ pub fn bank_from_archive<P: AsRef<Path> + std::marker::Sync>(
     Ok((bank, timings))
 }
 
-pub fn get_snapshot_archive_path(
+/// Build the snapshot archive path from its components: the snapshot archive output directory, the
+/// snapshot slot, the accounts hash, and the archive format.
+pub fn build_snapshot_archive_path(
     snapshot_output_dir: PathBuf,
-    snapshot_hash: &(Slot, Hash),
+    slot: Slot,
+    hash: &Hash,
     archive_format: ArchiveFormat,
 ) -> PathBuf {
     snapshot_output_dir.join(format!(
         "snapshot-{}-{}.{}",
-        snapshot_hash.0,
+        slot,
-        snapshot_hash.1,
+        hash,
         get_archive_ext(archive_format),
     ))
 }
@@ -709,13 +734,9 @@ fn archive_format_from_str(archive_format: &str) -> Option<ArchiveFormat> {
 
 /// Parse a snapshot archive filename into its Slot, Hash, and Archive Format
 fn parse_snapshot_archive_filename(archive_filename: &str) -> Option<(Slot, Hash, ArchiveFormat)> {
-    let snapshot_archive_filename_regex =
+    let regex = Regex::new(SNAPSHOT_ARCHIVE_FILENAME_REGEX);
-        Regex::new(r"^snapshot-(\d+)-([[:alnum:]]+)\.(tar|tar\.bz2|tar\.zst|tar\.gz)$");
 
-    snapshot_archive_filename_regex
+    regex.ok()?.captures(archive_filename).and_then(|captures| {
-        .ok()?
-        .captures(archive_filename)
-        .and_then(|captures| {
         let slot = captures.get(1).map(|x| x.as_str().parse::<Slot>())?.ok()?;
         let hash = captures.get(2).map(|x| x.as_str().parse::<Hash>())?.ok()?;
         let archive_format = captures
@@ -726,44 +747,71 @@ fn parse_snapshot_archive_filename(archive_filename: &str) -> Option<(Slot, Hash
     })
 }
 
-/// Get a list of the snapshot archives in a directory, sorted by Slot in descending order
+/// Get a list of the snapshot archives in a directory
-fn get_snapshot_archives<P: AsRef<Path>>(
+pub fn get_snapshot_archives<P>(snapshot_output_dir: P) -> Vec<SnapshotArchiveInfo>
-    snapshot_output_dir: P,
+where
-) -> Vec<(PathBuf, (Slot, Hash, ArchiveFormat))> {
+    P: AsRef<Path>,
+{
     match fs::read_dir(&snapshot_output_dir) {
         Err(err) => {
             info!("Unable to read snapshot directory: {}", err);
             vec![]
         }
-        Ok(files) => {
+        Ok(files) => files
-            let mut archives: Vec<_> = files
             .filter_map(|entry| {
                 if let Ok(entry) = entry {
                     let path = entry.path();
                     if path.is_file() {
-                            if let Some(snapshot_hash) = parse_snapshot_archive_filename(
+                        if let Some((slot, hash, archive_format)) = parse_snapshot_archive_filename(
                             path.file_name().unwrap().to_str().unwrap(),
                         ) {
-                                return Some((path, snapshot_hash));
+                            return Some(SnapshotArchiveInfo {
+                                path,
+                                slot,
+                                hash,
+                                archive_format,
+                            });
                         }
                     }
                 }
                 None
             })
-                .collect();
+            .collect(),
-
-            archives.sort_by(|a, b| (b.1).0.cmp(&(a.1).0)); // reverse sort by slot
-            archives
-        }
     }
 }
 
-/// Get the snapshot archive with the highest Slot in a directory
+/// Get a sorted list of the snapshot archives in a directory
-pub fn get_highest_snapshot_archive_path<P: AsRef<Path>>(
+fn get_sorted_snapshot_archives<P>(snapshot_output_dir: P) -> Vec<SnapshotArchiveInfo>
-    snapshot_output_dir: P,
+where
-) -> Option<(PathBuf, (Slot, Hash, ArchiveFormat))> {
+    P: AsRef<Path>,
-    let archives = get_snapshot_archives(snapshot_output_dir);
+{
-    archives.into_iter().next()
+    let mut snapshot_archives = get_snapshot_archives(snapshot_output_dir);
+    sort_snapshot_archives(&mut snapshot_archives);
+    snapshot_archives
+}
+
+/// Sort the list of snapshot archives by slot, in descending order
+fn sort_snapshot_archives(snapshot_archives: &mut Vec<SnapshotArchiveInfo>) {
+    snapshot_archives.sort_unstable_by(|a, b| b.slot.cmp(&a.slot));
+}
+
+/// Get the highest slot of the snapshots in a directory
+pub fn get_highest_snapshot_archive_slot<P>(snapshot_output_dir: P) -> Option<Slot>
+where
+    P: AsRef<Path>,
+{
+    get_highest_snapshot_archive_info(snapshot_output_dir)
+        .map(|snapshot_archive_info| snapshot_archive_info.slot)
+}
+
+/// Get the path (and metadata) for the snapshot archive with the highest slot in a directory
+pub fn get_highest_snapshot_archive_info<P>(snapshot_output_dir: P) -> Option<SnapshotArchiveInfo>
+where
+    P: AsRef<Path>,
+{
+    get_sorted_snapshot_archives(snapshot_output_dir)
+        .into_iter()
+        .next()
 }
 
 pub fn purge_old_snapshot_archives<P: AsRef<Path>>(
@@ -775,12 +823,12 @@ pub fn purge_old_snapshot_archives<P: AsRef<Path>>(
         snapshot_output_dir.as_ref(),
         maximum_snapshots_to_retain
     );
-    let mut archives = get_snapshot_archives(snapshot_output_dir);
+    let mut archives = get_sorted_snapshot_archives(snapshot_output_dir);
     // Keep the oldest snapshot so we can always play the ledger from it.
     archives.pop();
     let max_snaps = max(1, maximum_snapshots_to_retain);
     for old_archive in archives.into_iter().skip(max_snaps) {
-        fs::remove_file(old_archive.0)
+        fs::remove_file(old_archive.path)
             .unwrap_or_else(|err| info!("Failed to remove old snapshot: {:}", err));
     }
 }
@@ -993,7 +1041,7 @@ pub fn purge_old_snapshots(snapshot_path: &Path) {
     }
 }
 
-// Gather the necessary elements for a snapshot of the given `root_bank`
+/// Gather the necessary elements for a snapshot of the given `root_bank`
 pub fn snapshot_bank(
     root_bank: &Bank,
     status_cache_slot_deltas: Vec<BankSlotDelta>,
@@ -1035,6 +1083,9 @@ pub fn snapshot_bank(
 
 /// Convenience function to create a snapshot archive out of any Bank, regardless of state.  The
 /// Bank will be frozen during the process.
+///
+/// Requires:
+///     - `bank` is complete
 pub fn bank_to_snapshot_archive<P: AsRef<Path>, Q: AsRef<Path>>(
     snapshot_path: P,
     bank: &Bank,
@@ -1055,7 +1106,7 @@ pub fn bank_to_snapshot_archive<P: AsRef<Path>, Q: AsRef<Path>>(
 
     let temp_dir = tempfile::tempdir_in(snapshot_path)?;
 
-    let storages: Vec<_> = bank.get_snapshot_storages();
+    let storages = bank.get_snapshot_storages();
     let slot_snapshot_paths = add_snapshot(&temp_dir, bank, &storages, snapshot_version)?;
     let package = package_snapshot(
         bank,
@@ -1104,9 +1155,10 @@ pub fn process_accounts_package_pre(
         ("calculate_hash", time.as_us(), i64),
     );
 
-    let tar_output_file = get_snapshot_archive_path(
+    let tar_output_file = build_snapshot_archive_path(
         accounts_package.snapshot_output_dir,
-        &(accounts_package.slot, hash),
+        accounts_package.slot,
+        &hash,
         accounts_package.archive_format,
     );
 
@@ -1128,6 +1180,10 @@ mod tests {
     use super::*;
     use assert_matches::assert_matches;
     use bincode::{deserialize_from, serialize_into};
+    use solana_sdk::{
+        genesis_config::create_genesis_config,
+        signature::{Keypair, Signer},
+    };
     use std::mem::size_of;
 
     #[test]
@@ -1246,47 +1302,104 @@ mod tests {
             Some((43, Hash::default(), ArchiveFormat::TarZstd))
         );
         assert_eq!(
-            parse_snapshot_archive_filename(&format!("snapshot-42-{}.tar", Hash::default())),
+            parse_snapshot_archive_filename(&format!("snapshot-44-{}.tar", Hash::default())),
-            Some((42, Hash::default(), ArchiveFormat::Tar))
+            Some((44, Hash::default(), ArchiveFormat::Tar))
         );
 
         assert!(parse_snapshot_archive_filename("invalid").is_none());
         assert!(parse_snapshot_archive_filename("snapshot-bad!slot-bad!hash.bad!ext").is_none());
 
         assert!(parse_snapshot_archive_filename("snapshot-12345678-bad!hash.bad!ext").is_none());
-        assert!(parse_snapshot_archive_filename("snapshot-12345678-HASH1234.bad!ext").is_none());
+        assert!(parse_snapshot_archive_filename(&format!(
+            "snapshot-12345678-{}.bad!ext",
+            Hash::new_unique()
+        ))
+        .is_none());
         assert!(parse_snapshot_archive_filename("snapshot-12345678-bad!hash.tar").is_none());
 
-        assert!(parse_snapshot_archive_filename("snapshot-bad!slot-HASH1234.bad!ext").is_none());
+        assert!(parse_snapshot_archive_filename(&format!(
-        assert!(parse_snapshot_archive_filename("snapshot-12345678-HASH1234.bad!ext").is_none());
+            "snapshot-bad!slot-{}.bad!ext",
-        assert!(parse_snapshot_archive_filename("snapshot-bad!slot-HASH1234.tar").is_none());
+            Hash::new_unique()
+        ))
+        .is_none());
+        assert!(parse_snapshot_archive_filename(&format!(
+            "snapshot-12345678-{}.bad!ext",
+            Hash::new_unique()
+        ))
+        .is_none());
+        assert!(parse_snapshot_archive_filename(&format!(
+            "snapshot-bad!slot-{}.tar",
+            Hash::new_unique()
+        ))
+        .is_none());
 
         assert!(parse_snapshot_archive_filename("snapshot-bad!slot-bad!hash.tar").is_none());
         assert!(parse_snapshot_archive_filename("snapshot-12345678-bad!hash.tar").is_none());
-        assert!(parse_snapshot_archive_filename("snapshot-bad!slot-HASH1234.tar").is_none());
+        assert!(parse_snapshot_archive_filename(&format!(
+            "snapshot-bad!slot-{}.tar",
+            Hash::new_unique()
+        ))
+        .is_none());
     }
 
-    #[test]
+    /// A test helper function that creates snapshot archive files.  Creates snapshot files in the
-    fn test_get_snapshot_archives() {
+    /// range (`min_snapshot_slot`, `max_snapshot_slot`].  Additionally, "bad" files are created
-        let temp_snapshot_archives_dir = tempfile::TempDir::new().unwrap();
+    /// for snapshots to ensure the tests properly filter them out.
-
+    fn common_create_snapshot_archive_files(
-        let min_slot = 123;
+        snapshot_dir: &Path,
-        let max_slot = 456;
+        min_snapshot_slot: Slot,
-        for slot in min_slot..max_slot {
+        max_snapshot_slot: Slot,
-            let snapshot_filename = format!("snapshot-{}-{}.tar", slot, Hash::default());
+    ) {
-            let snapshot_filepath = temp_snapshot_archives_dir.path().join(snapshot_filename);
+        for snapshot_slot in min_snapshot_slot..max_snapshot_slot {
+            let snapshot_filename = format!("snapshot-{}-{}.tar", snapshot_slot, Hash::default());
+            let snapshot_filepath = snapshot_dir.join(snapshot_filename);
             File::create(snapshot_filepath).unwrap();
         }
 
         // Add in a snapshot with a bad filename and high slot to ensure filename are filtered and
         // sorted correctly
-        let bad_filename = format!("snapshot-{}-{}.bad!ext", max_slot + 1, Hash::default());
+        let bad_filename = format!("snapshot-{}-bad!hash.tar", max_snapshot_slot + 1);
-        let bad_filepath = temp_snapshot_archives_dir.path().join(bad_filename);
+        let bad_filepath = snapshot_dir.join(bad_filename);
         File::create(bad_filepath).unwrap();
+    }
 
-        let results = get_snapshot_archives(temp_snapshot_archives_dir);
+    #[test]
-        assert_eq!(results.len(), max_slot - min_slot);
+    fn test_get_snapshot_archives() {
-        assert_eq!(results[0].1 .0 as usize, max_slot - 1);
+        solana_logger::setup();
+        let temp_snapshot_archives_dir = tempfile::TempDir::new().unwrap();
+        let min_slot = 123;
+        let max_slot = 456;
+        common_create_snapshot_archive_files(temp_snapshot_archives_dir.path(), min_slot, max_slot);
+
+        let snapshot_archives = get_snapshot_archives(temp_snapshot_archives_dir);
+        assert_eq!(snapshot_archives.len() as Slot, max_slot - min_slot);
+    }
+
+    #[test]
+    fn test_get_sorted_snapshot_archives() {
+        solana_logger::setup();
+        let temp_snapshot_archives_dir = tempfile::TempDir::new().unwrap();
+        let min_slot = 12;
+        let max_slot = 45;
+        common_create_snapshot_archive_files(temp_snapshot_archives_dir.path(), min_slot, max_slot);
+
+        let sorted_snapshot_archives = get_sorted_snapshot_archives(temp_snapshot_archives_dir);
+        assert_eq!(sorted_snapshot_archives.len() as Slot, max_slot - min_slot);
+        assert_eq!(sorted_snapshot_archives[0].slot, max_slot - 1);
+    }
+
+    #[test]
+    fn test_get_highest_snapshot_archive_slot() {
+        solana_logger::setup();
+        let temp_snapshot_archives_dir = tempfile::TempDir::new().unwrap();
+        let min_slot = 123;
+        let max_slot = 456;
+        common_create_snapshot_archive_files(temp_snapshot_archives_dir.path(), min_slot, max_slot);
+
+        assert_eq!(
+            get_highest_snapshot_archive_slot(temp_snapshot_archives_dir.path()),
+            Some(max_slot - 1)
+        );
     }
 
     fn common_test_purge_old_snapshot_archives(
@@ -1339,4 +1452,140 @@ mod tests {
         let expected_snapshots = vec![&snap1_name, &snap2_name, &snap3_name];
         common_test_purge_old_snapshot_archives(&snapshot_names, 2, &expected_snapshots);
     }
+
+    /// Test roundtrip of bank to snapshot, then back again.  This test creates the simplest bank
+    /// possible, so the contents of the snapshot archive will be quite minimal.
+    #[test]
+    fn test_roundtrip_bank_to_snapshot_to_bank_simple() {
+        solana_logger::setup();
+        let genesis_config = GenesisConfig::default();
+        let original_bank = Bank::new(&genesis_config);
+
+        while !original_bank.is_complete() {
+            original_bank.register_tick(&Hash::new_unique());
+        }
+
+        let accounts_dir = tempfile::TempDir::new().unwrap();
+        let snapshot_dir = tempfile::TempDir::new().unwrap();
+        let snapshot_package_output_dir = tempfile::TempDir::new().unwrap();
+        let snapshot_archive_format = ArchiveFormat::Tar;
+
+        let snapshot_archive_path = bank_to_snapshot_archive(
+            snapshot_dir.path(),
+            &original_bank,
+            None,
+            snapshot_package_output_dir.path(),
+            snapshot_archive_format,
+            None,
+            1,
+        )
+        .unwrap();
+
+        let (roundtrip_bank, _) = bank_from_snapshot_archive(
+            &[PathBuf::from(accounts_dir.path())],
+            &[],
+            snapshot_dir.path(),
+            &snapshot_archive_path,
+            snapshot_archive_format,
+            &genesis_config,
+            None,
+            None,
+            AccountSecondaryIndexes::default(),
+            false,
+            None,
+            AccountShrinkThreshold::default(),
+            false,
+        )
+        .unwrap();
+
+        assert_eq!(original_bank, roundtrip_bank);
+    }
+
+    /// Test roundtrip of bank to snapshot, then back again.  This test is more involved than the
+    /// simple version above; creating multiple banks over multiple slots and doing multiple
+    /// transfers.  So this snapshot should contain more data.
+    #[test]
+    fn test_roundtrip_bank_to_snapshot_to_bank_complex() {
+        solana_logger::setup();
+        let collector = Pubkey::new_unique();
+        let key1 = Keypair::new();
+        let key2 = Keypair::new();
+        let key3 = Keypair::new();
+        let key4 = Keypair::new();
+        let key5 = Keypair::new();
+
+        let (genesis_config, mint_keypair) = create_genesis_config(1_000_000);
+        let bank0 = Arc::new(Bank::new(&genesis_config));
+        bank0.transfer(1, &mint_keypair, &key1.pubkey()).unwrap();
+        bank0.transfer(2, &mint_keypair, &key2.pubkey()).unwrap();
+        bank0.transfer(3, &mint_keypair, &key3.pubkey()).unwrap();
+        while !bank0.is_complete() {
+            bank0.register_tick(&Hash::new_unique());
+        }
+
+        let slot = 1;
+        let bank1 = Arc::new(Bank::new_from_parent(&bank0, &collector, slot));
+        bank1.transfer(3, &mint_keypair, &key3.pubkey()).unwrap();
+        bank1.transfer(4, &mint_keypair, &key4.pubkey()).unwrap();
+        bank1.transfer(5, &mint_keypair, &key5.pubkey()).unwrap();
+        while !bank1.is_complete() {
+            bank1.register_tick(&Hash::new_unique());
+        }
+
+        let slot = slot + 1;
+        let bank2 = Arc::new(Bank::new_from_parent(&bank1, &collector, slot));
+        bank2.transfer(1, &mint_keypair, &key1.pubkey()).unwrap();
+        while !bank2.is_complete() {
+            bank2.register_tick(&Hash::new_unique());
+        }
+
+        let slot = slot + 1;
+        let bank3 = Arc::new(Bank::new_from_parent(&bank2, &collector, slot));
+        bank3.transfer(1, &mint_keypair, &key1.pubkey()).unwrap();
+        while !bank3.is_complete() {
+            bank3.register_tick(&Hash::new_unique());
+        }
+
+        let slot = slot + 1;
+        let bank4 = Arc::new(Bank::new_from_parent(&bank3, &collector, slot));
+        bank4.transfer(1, &mint_keypair, &key1.pubkey()).unwrap();
+        while !bank4.is_complete() {
+            bank4.register_tick(&Hash::new_unique());
+        }
+
+        let accounts_dir = tempfile::TempDir::new().unwrap();
+        let snapshot_dir = tempfile::TempDir::new().unwrap();
+        let snapshot_package_output_dir = tempfile::TempDir::new().unwrap();
+        let snapshot_archive_format = ArchiveFormat::Tar;
+
+        let full_snapshot_archive_path = bank_to_snapshot_archive(
+            snapshot_dir.path(),
+            &bank4,
+            None,
+            snapshot_package_output_dir.path(),
+            snapshot_archive_format,
+            None,
+            std::usize::MAX,
+        )
+        .unwrap();
+
+        let (roundtrip_bank, _) = bank_from_snapshot_archive(
+            &[PathBuf::from(accounts_dir.path())],
+            &[],
+            snapshot_dir.path(),
+            &full_snapshot_archive_path,
+            snapshot_archive_format,
+            &genesis_config,
+            None,
+            None,
+            AccountSecondaryIndexes::default(),
+            false,
+            None,
+            AccountShrinkThreshold::default(),
+            false,
+        )
+        .unwrap();
+
+        assert_eq!(*bank4, roundtrip_bank);
+    }
 }

validator/src/main.rs

@@ -48,10 +48,7 @@ use {
         },
         hardened_unpack::MAX_GENESIS_ARCHIVE_UNPACKED_SIZE,
         snapshot_config::SnapshotConfig,
-        snapshot_utils::{
+        snapshot_utils::{self, ArchiveFormat, SnapshotVersion, DEFAULT_MAX_SNAPSHOTS_TO_RETAIN},
-            get_highest_snapshot_archive_path, ArchiveFormat, SnapshotVersion,
-            DEFAULT_MAX_SNAPSHOTS_TO_RETAIN,
-        },
     },
     solana_sdk::{
         clock::{Slot, DEFAULT_S_PER_SLOT},
@@ -471,8 +468,9 @@ fn get_rpc_node(
         blacklist_timeout = Instant::now();
 
         let mut highest_snapshot_hash: Option<(Slot, Hash)> =
-            get_highest_snapshot_archive_path(snapshot_output_dir)
+            snapshot_utils::get_highest_snapshot_archive_info(snapshot_output_dir).map(
-                .map(|(_path, (slot, hash, _compression))| (slot, hash));
+                |snapshot_archive_info| (snapshot_archive_info.slot, snapshot_archive_info.hash),
+            );
         let eligible_rpc_peers = if snapshot_not_required {
             rpc_peers
         } else {
@@ -853,8 +851,7 @@ fn rpc_bootstrap(
                 let mut use_local_snapshot = false;
 
                 if let Some(highest_local_snapshot_slot) =
-                    get_highest_snapshot_archive_path(snapshot_output_dir)
+                    snapshot_utils::get_highest_snapshot_archive_slot(snapshot_output_dir)
-                        .map(|(_path, (slot, _hash, _compression))| slot)
                 {
                     if highest_local_snapshot_slot
                         > snapshot_hash.0.saturating_sub(maximum_local_snapshot_age)