Remove unused Option<> around ValidatorConfig's SnapshotConfig (#29090)

Remove Option<> around ValidatorConfig's SnapshotConfig

The SnapshotConfig is required and is currently hard-coded to be a
Some().

core/src/accounts_hash_verifier.rs

@@ -48,7 +48,7 @@ impl AccountsHashVerifier {
         known_validators: Option<HashSet<Pubkey>>,
         halt_on_known_validators_accounts_hash_mismatch: bool,
         fault_injection_rate_slots: u64,
-        snapshot_config: Option<SnapshotConfig>,
+        snapshot_config: SnapshotConfig,
     ) -> Self {
         // If there are no accounts packages to process, limit how often we re-check
         const LOOP_LIMITER: Duration = Duration::from_millis(SLOT_MS);
@@ -83,7 +83,7 @@ impl AccountsHashVerifier {
                             &mut hashes,
                             &exit,
                             fault_injection_rate_slots,
-                            snapshot_config.as_ref(),
+                            &snapshot_config,
                         ));
 
                         datapoint_info!(
@@ -184,7 +184,7 @@ impl AccountsHashVerifier {
         hashes: &mut Vec<(Slot, Hash)>,
         exit: &Arc<AtomicBool>,
         fault_injection_rate_slots: u64,
-        snapshot_config: Option<&SnapshotConfig>,
+        snapshot_config: &SnapshotConfig,
     ) {
         let accounts_hash = Self::calculate_and_verify_accounts_hash(&accounts_package);
 
@@ -377,13 +377,11 @@ impl AccountsHashVerifier {
     fn submit_for_packaging(
         accounts_package: AccountsPackage,
         pending_snapshot_package: Option<&PendingSnapshotPackage>,
-        snapshot_config: Option<&SnapshotConfig>,
+        snapshot_config: &SnapshotConfig,
         accounts_hash: AccountsHash,
     ) {
         if pending_snapshot_package.is_none()
-            || !snapshot_config
+            || !snapshot_config.should_generate_snapshots()
-                .map(|snapshot_config| snapshot_config.should_generate_snapshots())
-                .unwrap_or(false)
             || !matches!(
                 accounts_package.package_type,
                 AccountsPackageType::Snapshot(_)
@@ -555,7 +553,7 @@ mod tests {
                 &mut hashes,
                 &exit,
                 0,
-                Some(&snapshot_config),
+                &snapshot_config,
             );
 
             // sleep for 1ms to create a newer timestamp for gossip entry

core/src/validator.rs

@@ -130,7 +130,7 @@ pub struct ValidatorConfig {
     pub geyser_plugin_config_files: Option<Vec<PathBuf>>,
     pub rpc_addrs: Option<(SocketAddr, SocketAddr)>, // (JsonRpc, JsonRpcPubSub)
     pub pubsub_config: PubSubConfig,
-    pub snapshot_config: Option<SnapshotConfig>,
+    pub snapshot_config: SnapshotConfig,
     pub max_ledger_shreds: Option<u64>,
     pub broadcast_stage_type: BroadcastStageType,
     pub turbine_disabled: Arc<AtomicBool>,
@@ -194,7 +194,7 @@ impl Default for ValidatorConfig {
             geyser_plugin_config_files: None,
             rpc_addrs: None,
             pubsub_config: PubSubConfig::default(),
-            snapshot_config: Some(SnapshotConfig::new_load_only()),
+            snapshot_config: SnapshotConfig::new_load_only(),
             broadcast_stage_type: BroadcastStageType::Standard,
             turbine_disabled: Arc::<AtomicBool>::default(),
             enforce_ulimit_nofile: true,
@@ -577,17 +577,13 @@ impl Validator {
         cluster_info.restore_contact_info(ledger_path, config.contact_save_interval);
         let cluster_info = Arc::new(cluster_info);
 
-        // A snapshot config is required.  Remove the Option<> wrapper in the future.
-        assert!(config.snapshot_config.is_some());
-        let snapshot_config = config.snapshot_config.clone().unwrap();
-
         assert!(is_snapshot_config_valid(
-            &snapshot_config,
+            &config.snapshot_config,
             config.accounts_hash_interval_slots,
         ));
 
         let (pending_snapshot_package, snapshot_packager_service) =
-            if snapshot_config.should_generate_snapshots() {
+            if config.snapshot_config.should_generate_snapshots() {
                 // filler accounts make snapshots invalid for use
                 // so, do not publish that we have snapshots
                 let enable_gossip_push = config
@@ -601,7 +597,7 @@ impl Validator {
                     starting_snapshot_hashes,
                     &exit,
                     &cluster_info,
-                    snapshot_config.clone(),
+                    config.snapshot_config.clone(),
                     enable_gossip_push,
                 );
                 (
@@ -629,7 +625,7 @@ impl Validator {
         let accounts_background_request_sender =
             AbsRequestSender::new(snapshot_request_sender.clone());
         let snapshot_request_handler = SnapshotRequestHandler {
-            snapshot_config,
+            snapshot_config: config.snapshot_config.clone(),
             snapshot_request_sender,
             snapshot_request_receiver,
             accounts_package_sender,
@@ -795,7 +791,7 @@ impl Validator {
             let json_rpc_service = JsonRpcService::new(
                 rpc_addr,
                 config.rpc_config.clone(),
-                config.snapshot_config.clone(),
+                Some(config.snapshot_config.clone()),
                 bank_forks.clone(),
                 block_commitment_cache.clone(),
                 blockstore.clone(),
@@ -1446,7 +1442,7 @@ fn load_blockstore(
             &blockstore,
             config.account_paths.clone(),
             config.account_shrink_paths.clone(),
-            config.snapshot_config.as_ref(),
+            Some(&config.snapshot_config),
             &process_options,
             transaction_history_services
                 .cache_block_meta_sender
@@ -1481,7 +1477,7 @@ fn load_blockstore(
     leader_schedule_cache.set_fixed_leader_schedule(config.fixed_leader_schedule.clone());
     {
         let mut bank_forks = bank_forks.write().unwrap();
-        bank_forks.set_snapshot_config(config.snapshot_config.clone());
+        bank_forks.set_snapshot_config(Some(config.snapshot_config.clone()));
         bank_forks.set_accounts_hash_interval_slots(config.accounts_hash_interval_slots);
         if let Some(ref shrink_paths) = config.account_shrink_paths {
             bank_forks
@@ -1658,11 +1654,6 @@ fn maybe_warp_slot(
     accounts_background_request_sender: &AbsRequestSender,
 ) -> Result<(), String> {
     if let Some(warp_slot) = config.warp_slot {
-        let snapshot_config = match config.snapshot_config.as_ref() {
-            Some(config) => config,
-            None => return Err("warp slot requires a snapshot config".to_owned()),
-        };
-
         process_blockstore.process()?;
 
         let mut bank_forks = bank_forks.write().unwrap();
@@ -1695,11 +1686,15 @@ fn maybe_warp_slot(
             ledger_path,
             &bank_forks.root_bank(),
             None,
-            &snapshot_config.full_snapshot_archives_dir,
+            &config.snapshot_config.full_snapshot_archives_dir,
-            &snapshot_config.incremental_snapshot_archives_dir,
+            &config.snapshot_config.incremental_snapshot_archives_dir,
-            snapshot_config.archive_format,
+            config.snapshot_config.archive_format,
-            snapshot_config.maximum_full_snapshot_archives_to_retain,
+            config
-            snapshot_config.maximum_incremental_snapshot_archives_to_retain,
+                .snapshot_config
+                .maximum_full_snapshot_archives_to_retain,
+            config
+                .snapshot_config
+                .maximum_incremental_snapshot_archives_to_retain,
         ) {
             Ok(archive_info) => archive_info,
             Err(e) => return Err(format!("Unable to create snapshot: {e}")),

core/tests/epoch_accounts_hash.rs

@@ -199,7 +199,7 @@ impl BackgroundServices {
             None,
             false,
             0,
-            Some(snapshot_config.clone()),
+            snapshot_config.clone(),
         );
 
         let (snapshot_request_sender, snapshot_request_receiver) = crossbeam_channel::unbounded();

core/tests/snapshots.rs

@@ -231,7 +231,7 @@ fn run_bank_forks_snapshot_n<F>(
         None,
         false,
         0,
-        Some(snapshot_test_config.snapshot_config.clone()),
+        snapshot_test_config.snapshot_config.clone(),
     );
 
     let (snapshot_request_sender, snapshot_request_receiver) = unbounded();
@@ -745,7 +745,7 @@ fn test_bank_forks_incremental_snapshot(
         None,
         false,
         0,
-        Some(snapshot_test_config.snapshot_config.clone()),
+        snapshot_test_config.snapshot_config.clone(),
     );
 
     let (snapshot_request_sender, snapshot_request_receiver) = unbounded();
@@ -1036,7 +1036,7 @@ fn test_snapshots_with_background_services(
         None,
         false,
         0,
-        Some(snapshot_test_config.snapshot_config.clone()),
+        snapshot_test_config.snapshot_config.clone(),
     );
 
     let accounts_background_service = AccountsBackgroundService::new(

local-cluster/src/local_cluster.rs

@@ -148,7 +148,8 @@ impl LocalCluster {
     ) {
         config.account_paths = vec![ledger_path.join("accounts")];
         config.tower_storage = Arc::new(FileTowerStorage::new(ledger_path.to_path_buf()));
-        if let Some(snapshot_config) = &mut config.snapshot_config {
+
+        let snapshot_config = &mut config.snapshot_config;
         let dummy: PathBuf = DUMMY_SNAPSHOT_CONFIG_PATH_MARKER.into();
         if snapshot_config.full_snapshot_archives_dir == dummy {
             snapshot_config.full_snapshot_archives_dir = ledger_path.to_path_buf();
@@ -157,7 +158,6 @@ impl LocalCluster {
             snapshot_config.bank_snapshots_dir = ledger_path.join("snapshot");
         }
     }
-    }
 
     pub fn new(config: &mut ClusterConfig, socket_addr_space: SocketAddrSpace) -> Self {
         assert_eq!(config.validator_configs.len(), config.node_stakes.len());

local-cluster/tests/common.rs

@@ -494,7 +494,7 @@ impl SnapshotValidatorConfig {
 
         // Create the validator config
         let validator_config = ValidatorConfig {
-            snapshot_config: Some(snapshot_config),
+            snapshot_config,
             account_paths: account_storage_paths,
             accounts_hash_interval_slots,
             ..ValidatorConfig::default_for_test()

local-cluster/tests/local_cluster.rs

@@ -488,14 +488,10 @@ fn test_snapshot_download() {
     let full_snapshot_archives_dir = &leader_snapshot_test_config
         .validator_config
         .snapshot_config
-        .as_ref()
-        .unwrap()
         .full_snapshot_archives_dir;
     let incremental_snapshot_archives_dir = &leader_snapshot_test_config
         .validator_config
         .snapshot_config
-        .as_ref()
-        .unwrap()
         .incremental_snapshot_archives_dir;
 
     trace!("Waiting for snapshot");
@@ -518,14 +514,10 @@ fn test_snapshot_download() {
         validator_snapshot_test_config
             .validator_config
             .snapshot_config
-            .as_ref()
-            .unwrap()
             .maximum_full_snapshot_archives_to_retain,
         validator_snapshot_test_config
             .validator_config
             .snapshot_config
-            .as_ref()
-            .unwrap()
             .maximum_incremental_snapshot_archives_to_retain,
         false,
         &mut None,
@@ -580,14 +572,10 @@ fn test_incremental_snapshot_download() {
     let full_snapshot_archives_dir = &leader_snapshot_test_config
         .validator_config
         .snapshot_config
-        .as_ref()
-        .unwrap()
         .full_snapshot_archives_dir;
     let incremental_snapshot_archives_dir = &leader_snapshot_test_config
         .validator_config
         .snapshot_config
-        .as_ref()
-        .unwrap()
         .incremental_snapshot_archives_dir;
 
     debug!("snapshot config:\n\tfull snapshot interval: {}\n\tincremental snapshot interval: {}\n\taccounts hash interval: {}",
@@ -651,14 +639,10 @@ fn test_incremental_snapshot_download() {
         validator_snapshot_test_config
             .validator_config
             .snapshot_config
-            .as_ref()
-            .unwrap()
             .maximum_full_snapshot_archives_to_retain,
         validator_snapshot_test_config
             .validator_config
             .snapshot_config
-            .as_ref()
-            .unwrap()
             .maximum_incremental_snapshot_archives_to_retain,
         false,
         &mut None,
@@ -677,14 +661,10 @@ fn test_incremental_snapshot_download() {
         validator_snapshot_test_config
             .validator_config
             .snapshot_config
-            .as_ref()
-            .unwrap()
             .maximum_full_snapshot_archives_to_retain,
         validator_snapshot_test_config
             .validator_config
             .snapshot_config
-            .as_ref()
-            .unwrap()
             .maximum_incremental_snapshot_archives_to_retain,
         false,
         &mut None,
@@ -824,14 +804,10 @@ fn test_incremental_snapshot_download_with_crossing_full_snapshot_interval_at_st
         validator_snapshot_test_config
             .validator_config
             .snapshot_config
-            .as_ref()
-            .unwrap()
             .maximum_full_snapshot_archives_to_retain,
         validator_snapshot_test_config
             .validator_config
             .snapshot_config
-            .as_ref()
-            .unwrap()
             .maximum_incremental_snapshot_archives_to_retain,
         false,
         &mut None,
@@ -865,14 +841,10 @@ fn test_incremental_snapshot_download_with_crossing_full_snapshot_interval_at_st
         validator_snapshot_test_config
             .validator_config
             .snapshot_config
-            .as_ref()
-            .unwrap()
             .maximum_full_snapshot_archives_to_retain,
         validator_snapshot_test_config
             .validator_config
             .snapshot_config
-            .as_ref()
-            .unwrap()
             .maximum_incremental_snapshot_archives_to_retain,
         false,
         &mut None,
@@ -1259,8 +1231,6 @@ fn test_snapshot_restart_tower() {
     let full_snapshot_archives_dir = &leader_snapshot_test_config
         .validator_config
         .snapshot_config
-        .as_ref()
-        .unwrap()
         .full_snapshot_archives_dir;
 
     let full_snapshot_archive_info = cluster.wait_for_next_full_snapshot(
@@ -1313,8 +1283,6 @@ fn test_snapshots_blockstore_floor() {
     let full_snapshot_archives_dir = &leader_snapshot_test_config
         .validator_config
         .snapshot_config
-        .as_ref()
-        .unwrap()
         .full_snapshot_archives_dir;
 
     let mut config = ClusterConfig {
@@ -1415,8 +1383,6 @@ fn test_snapshots_restart_validity() {
     let full_snapshot_archives_dir = &snapshot_test_config
         .validator_config
         .snapshot_config
-        .as_ref()
-        .unwrap()
         .full_snapshot_archives_dir;
 
     // Set up the cluster with 1 snapshotting validator
@@ -2228,7 +2194,7 @@ fn test_hard_fork_with_gap_in_roots() {
     let validator_b_pubkey = validators[1];
 
     let validator_config = ValidatorConfig {
-        snapshot_config: Some(LocalCluster::create_dummy_load_only_snapshot_config()),
+        snapshot_config: LocalCluster::create_dummy_load_only_snapshot_config(),
         accounts_db_caching_enabled: true,
         ..ValidatorConfig::default()
     };

test-validator/src/lib.rs

@@ -807,14 +807,14 @@ impl TestValidator {
             accounts_hash_interval_slots: 100,
             account_paths: vec![ledger_path.join("accounts")],
             poh_verify: false, // Skip PoH verification of ledger on startup for speed
-            snapshot_config: Some(SnapshotConfig {
+            snapshot_config: SnapshotConfig {
                 full_snapshot_archive_interval_slots: 100,
                 incremental_snapshot_archive_interval_slots: Slot::MAX,
                 bank_snapshots_dir: ledger_path.join("snapshot"),
                 full_snapshot_archives_dir: ledger_path.to_path_buf(),
                 incremental_snapshot_archives_dir: ledger_path.to_path_buf(),
                 ..SnapshotConfig::default()
-            }),
+            },
             enforce_ulimit_nofile: false,
             warp_slot: config.warp_slot,
             validator_exit: config.validator_exit.clone(),

validator/src/bootstrap.rs

@@ -15,10 +15,7 @@ use {
     solana_runtime::{
         snapshot_archive_info::SnapshotArchiveInfoGetter,
         snapshot_package::SnapshotType,
-        snapshot_utils::{
+        snapshot_utils::{self},
-            self, DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN,
-            DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN,
-        },
     },
     solana_sdk::{
         clock::Slot,
@@ -1243,18 +1240,13 @@ fn download_snapshot(
     desired_snapshot_hash: (Slot, Hash),
     snapshot_type: SnapshotType,
 ) -> Result<(), String> {
-    let (maximum_full_snapshot_archives_to_retain, maximum_incremental_snapshot_archives_to_retain) =
+    let maximum_full_snapshot_archives_to_retain = validator_config
-        if let Some(snapshot_config) = validator_config.snapshot_config.as_ref() {
+        .snapshot_config
-            (
+        .maximum_full_snapshot_archives_to_retain;
-                snapshot_config.maximum_full_snapshot_archives_to_retain,
+    let maximum_incremental_snapshot_archives_to_retain = validator_config
-                snapshot_config.maximum_incremental_snapshot_archives_to_retain,
+        .snapshot_config
-            )
+        .maximum_incremental_snapshot_archives_to_retain;
-        } else {
+
-            (
-                DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN,
-                DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN,
-            )
-        };
     *start_progress.write().unwrap() = ValidatorStartProgress::DownloadingSnapshot {
         slot: desired_snapshot_hash.0,
         rpc_addr: rpc_contact_info.rpc,

validator/src/main.rs

@@ -1285,7 +1285,7 @@ pub fn main() {
             (Slot::MAX, Slot::MAX)
         };
 
-    validator_config.snapshot_config = Some(SnapshotConfig {
+    validator_config.snapshot_config = SnapshotConfig {
         usage: if full_snapshot_archive_interval_slots == Slot::MAX {
             SnapshotUsage::LoadOnly
         } else {
@@ -1302,14 +1302,12 @@ pub fn main() {
         maximum_incremental_snapshot_archives_to_retain,
         accounts_hash_debug_verify: validator_config.accounts_db_test_hash_calculation,
         packager_thread_niceness_adj: snapshot_packager_niceness_adj,
-    });
+    };
 
     validator_config.accounts_hash_interval_slots =
         value_t_or_exit!(matches, "accounts-hash-interval-slots", u64);
     if !is_snapshot_config_valid(
-        // SAFETY: Calling `.unwrap()` is safe here because `validator_config.snapshot_config` must
+        &validator_config.snapshot_config,
-        // be `Some`. The Option<> wrapper will be removed later to solidify this requirement.
-        validator_config.snapshot_config.as_ref().unwrap(),
         validator_config.accounts_hash_interval_slots,
     ) {
         eprintln!("Invalid snapshot configuration provided: snapshot intervals are incompatible. \