Bubble up errors in bank_fork_utils instead of exiting process

There are operations in bank_fork_utils that may fail; we explicitly
call std::process::exit() on several of these. Granted we may end up
exiting the process higher up the callstack, bubbling the errors up
allow a caller that could handle the error to do so.

core/src/validator.rs

@@ -1801,7 +1801,7 @@ fn load_blockstore(
                 .map(|service| service.sender()),
             accounts_update_notifier,
             exit,
-        );
+        )?;
 
     // Before replay starts, set the callbacks in each of the banks in BankForks so that
     // all dropped banks come through the `pruned_banks_receiver` channel. This way all bank

ledger-tool/src/ledger_utils.rs

@@ -16,9 +16,7 @@ use {
             AccessType, BlockstoreOptions, BlockstoreRecoveryMode, LedgerColumnOptions,
             ShredStorageType,
         },
-        blockstore_processor::{
-            self, BlockstoreProcessorError, ProcessOptions, TransactionStatusSender,
-        },
+        blockstore_processor::{self, ProcessOptions, TransactionStatusSender},
     },
     solana_measure::measure,
     solana_rpc::transaction_status_service::TransactionStatusService,
@@ -71,7 +69,7 @@ pub fn load_and_process_ledger(
     process_options: ProcessOptions,
     snapshot_archive_path: Option<PathBuf>,
     incremental_snapshot_archive_path: Option<PathBuf>,
-) -> Result<(Arc<RwLock<BankForks>>, Option<StartingSnapshotHashes>), BlockstoreProcessorError> {
+) -> Result<(Arc<RwLock<BankForks>>, Option<StartingSnapshotHashes>), String> {
     let bank_snapshots_dir = if blockstore.is_primary_access() {
         blockstore.ledger_path().join("snapshot")
     } else {
@@ -245,7 +243,7 @@ pub fn load_and_process_ledger(
             None, // Maybe support this later, though
             accounts_update_notifier,
             exit.clone(),
-        );
+        )?;
     let block_verification_method = value_t!(
         arg_matches,
         "block_verification_method",
@@ -345,7 +343,8 @@ pub fn load_and_process_ledger(
         None, // Maybe support this later, though
         &accounts_background_request_sender,
     )
-    .map(|_| (bank_forks, starting_snapshot_hashes));
+    .map(|_| (bank_forks, starting_snapshot_hashes))
+    .map_err(|err| err.to_string());
 
     exit.store(true, Ordering::Relaxed);
     accounts_background_service.join().unwrap();

ledger/src/bank_forks_utils.rs

@@ -2,8 +2,7 @@ use {
     crate::{
         blockstore::Blockstore,
         blockstore_processor::{
-            self, BlockstoreProcessorError, CacheBlockMetaSender, ProcessOptions,
-            TransactionStatusSender,
+            self, CacheBlockMetaSender, ProcessOptions, TransactionStatusSender,
         },
         entry_notifier_service::EntryNotifierSender,
         leader_schedule_cache::LeaderScheduleCache,
@@ -25,7 +24,7 @@ use {
     solana_sdk::genesis_config::GenesisConfig,
     std::{
         path::PathBuf,
-        process, result,
+        result,
         sync::{atomic::AtomicBool, Arc, RwLock},
     },
 };
@@ -36,7 +35,7 @@ pub type LoadResult = result::Result<
         LeaderScheduleCache,
         Option<StartingSnapshotHashes>,
     ),
-    BlockstoreProcessorError,
+    String,
 >;
 
 /// Load the banks via genesis or a snapshot then processes all full blocks in blockstore
@@ -68,8 +67,7 @@ pub fn load(
         entry_notification_sender,
         accounts_update_notifier,
         exit,
-    );
-
+    )?;
     blockstore_processor::process_blockstore_from_root(
         blockstore,
         &bank_forks,
@@ -80,7 +78,9 @@ pub fn load(
         entry_notification_sender,
         &AbsRequestSender::default(),
     )
-    .map(|_| (bank_forks, leader_schedule_cache, starting_snapshot_hashes))
+    .map_err(|err| format!("Unable to process blockstore from root: {err}"))?;
+
+    Ok((bank_forks, leader_schedule_cache, starting_snapshot_hashes))
 }
 
 #[allow(clippy::too_many_arguments)]
@@ -95,11 +95,7 @@ pub fn load_bank_forks(
     entry_notification_sender: Option<&EntryNotifierSender>,
     accounts_update_notifier: Option<AccountsUpdateNotifier>,
     exit: Arc<AtomicBool>,
-) -> (
-    Arc<RwLock<BankForks>>,
-    LeaderScheduleCache,
-    Option<StartingSnapshotHashes>,
-) {
+) -> LoadResult {
     fn get_snapshots_to_load(
         snapshot_config: Option<&SnapshotConfig>,
     ) -> Option<(
@@ -157,7 +153,7 @@ pub fn load_bank_forks(
                 process_options,
                 accounts_update_notifier,
                 exit,
-            );
+            )?;
             (bank_forks, Some(starting_snapshot_hashes))
         } else {
             info!("Processing ledger from genesis");
@@ -193,7 +189,7 @@ pub fn load_bank_forks(
             .for_each(|hard_fork_slot| root_bank.register_hard_fork(*hard_fork_slot));
     }
 
-    (bank_forks, leader_schedule_cache, starting_snapshot_hashes)
+    Ok((bank_forks, leader_schedule_cache, starting_snapshot_hashes))
 }
 
 #[allow(clippy::too_many_arguments)]
@@ -207,11 +203,10 @@ fn bank_forks_from_snapshot(
     process_options: &ProcessOptions,
     accounts_update_notifier: Option<AccountsUpdateNotifier>,
     exit: Arc<AtomicBool>,
-) -> (Arc<RwLock<BankForks>>, StartingSnapshotHashes) {
+) -> Result<(Arc<RwLock<BankForks>>, StartingSnapshotHashes), String> {
     // Fail hard here if snapshot fails to load, don't silently continue
     if account_paths.is_empty() {
-        error!("Account paths not present when booting from snapshot");
-        process::exit(1);
+        return Err("Account paths not present when booting from snapshot".to_string());
     }
 
     let latest_snapshot_archive_slot = std::cmp::max(
@@ -261,8 +256,8 @@ fn bank_forks_from_snapshot(
             accounts_update_notifier,
             exit,
         )
-        .unwrap_or_else(|err| {
-            error!(
+        .map_err(|err| {
+            format!(
                 "Failed to load bank: {err} \
                 \nfull snapshot archive: {} \
                 \nincremental snapshot archive: {}",
@@ -271,19 +266,17 @@ fn bank_forks_from_snapshot(
                     .as_ref()
                     .map(|archive| archive.path().display().to_string())
                     .unwrap_or("none".to_string()),
-            );
-            process::exit(1);
-        });
+            )
+        })?;
         bank
     } else {
-        let Some(bank_snapshot) = latest_bank_snapshot else {
-            error!(
+        let bank_snapshot = latest_bank_snapshot.ok_or_else(||
+            format!(
                 "There is no local state to startup from. Ensure --{} is *not* set to \"{}\" and restart.",
                 use_snapshot_archives_at_startup::cli::LONG_ARG,
-                UseSnapshotArchivesAtStartup::Never.to_string(),
-            );
-            process::exit(1);
-        };
+                UseSnapshotArchivesAtStartup::Never,
+            )
+        )?;
 
         // If a newer snapshot archive was downloaded, it is possible that its slot is
         // higher than the local bank we will load.  Did the user intend for this?
@@ -318,14 +311,13 @@ fn bank_forks_from_snapshot(
             accounts_update_notifier,
             exit,
         )
-        .unwrap_or_else(|err| {
-            error!(
+        .map_err(|err| {
+            format!(
                 "Failed to load bank: {err} \
                 \nsnapshot: {}",
                 bank_snapshot.snapshot_path().display(),
-            );
-            process::exit(1);
-        });
+            )
+        })?;
         bank
     };
 
@@ -349,5 +341,5 @@ fn bank_forks_from_snapshot(
         incremental: incremental_snapshot_hash,
     };
 
-    (BankForks::new_rw_arc(bank), starting_snapshot_hashes)
+    Ok((BankForks::new_rw_arc(bank), starting_snapshot_hashes))
 }

ledger/src/blockstore_processor.rs

@@ -751,7 +751,8 @@ pub fn test_process_blockstore(
         None,
         None,
         exit,
-    );
+    )
+    .unwrap();
 
     process_blockstore_from_root(
         blockstore,