Make validator::new return ValidatorError. (#1609)

* Make validator::new return ValidatorError.

* Try switching to anyhow and thiserror.

* Switch to matches! on error matching.

core/Cargo.toml

@@ -15,6 +15,7 @@ codecov = { repository = "solana-labs/solana", branch = "master", service = "git
 
 [dependencies]
 ahash = { workspace = true }
+anyhow = { workspace = true }
 arrayvec = { workspace = true }
 base64 = { workspace = true }
 bincode = { workspace = true }

core/src/validator.rs

@@ -5,7 +5,7 @@ use {
     crate::{
         accounts_hash_verifier::AccountsHashVerifier,
         admin_rpc_post_init::AdminRpcRequestMetadataPostInit,
-        banking_trace::{self, BankingTracer},
+        banking_trace::{self, BankingTracer, TraceError},
         cache_block_meta_service::{CacheBlockMetaSender, CacheBlockMetaService},
         cluster_info_vote_listener::VoteTracker,
         completed_data_sets_service::CompletedDataSetsService,
@@ -27,6 +27,7 @@ use {
         tpu::{Tpu, TpuSockets, DEFAULT_TPU_COALESCE},
         tvu::{Tvu, TvuConfig, TvuSockets},
     },
+    anyhow::{anyhow, Context, Result},
     crossbeam_channel::{bounded, unbounded, Receiver},
     lazy_static::lazy_static,
     quinn::Endpoint,
@@ -136,6 +137,7 @@ use {
     },
     strum::VariantNames,
     strum_macros::{Display, EnumString, EnumVariantNames, IntoStaticStr},
+    thiserror::Error,
     tokio::runtime::Runtime as TokioRuntime,
 };
 
@@ -516,7 +518,7 @@ impl Validator {
         tpu_enable_udp: bool,
         tpu_max_connections_per_ipaddr_per_minute: u64,
         admin_rpc_service_post_init: Arc<RwLock<Option<AdminRpcRequestMetadataPostInit>>>,
-    ) -> Result<Self, String> {
+    ) -> Result<Self> {
         let start_time = Instant::now();
 
         let id = identity_keypair.pubkey();
@@ -526,8 +528,9 @@ impl Validator {
         info!("vote account pubkey: {vote_account}");
 
         if !config.no_os_network_stats_reporting {
-            verify_net_stats_access()
-                .map_err(|err| format!("Failed to access network stats: {err:?}"))?;
+            verify_net_stats_access().map_err(|e| {
+                ValidatorError::Other(format!("Failed to access network stats: {e:?}"))
+            })?;
         }
 
         let mut bank_notification_senders = Vec::new();
@@ -546,7 +549,9 @@ impl Validator {
                         geyser_plugin_config_files,
                         rpc_to_plugin_manager_receiver_and_exit,
                     )
-                    .map_err(|err| format!("Failed to load the Geyser plugin: {err:?}"))?,
+                    .map_err(|err| {
+                        ValidatorError::Other(format!("Failed to load the Geyser plugin: {err:?}"))
+                    })?,
                 )
             } else {
                 None
@@ -582,13 +587,13 @@ impl Validator {
         info!("Initializing sigverify done.");
 
         if !ledger_path.is_dir() {
-            return Err(format!(
+            return Err(anyhow!(
                 "ledger directory does not exist or is not accessible: {ledger_path:?}"
             ));
         }
         let genesis_config =
             open_genesis_config(ledger_path, config.max_genesis_archive_unpacked_size)
-                .map_err(|err| format!("Failed to open genesis config: {err}"))?;
+                .context("Failed to open genesis config")?;
 
         metrics_config_sanity_check(genesis_config.cluster_type)?;
 
@@ -601,12 +606,10 @@ impl Validator {
                     wait_for_supermajority_slot + 1,
                     expected_shred_version,
                 )
-                .map_err(|err| {
-                    format!(
-                        "Failed to backup and clear shreds with incorrect \
-                        shred version from blockstore: {err:?}"
-                    )
-                })?;
+                .context(
+                    "Failed to backup and clear shreds with incorrect \
+                        shred version from blockstore",
+                )?;
             }
         }
 
@@ -628,7 +631,7 @@ impl Validator {
             &config.snapshot_config.bank_snapshots_dir,
             &config.account_snapshot_paths,
         )
-        .map_err(|err| format!("failed to clean orphaned account snapshot directories: {err}"))?;
+        .context("failed to clean orphaned account snapshot directories")?;
         timer.stop();
         info!("Cleaning orphaned account snapshot directories done. {timer}");
 
@@ -730,7 +733,8 @@ impl Validator {
             transaction_notifier,
             entry_notifier,
             Some(poh_timing_point_sender.clone()),
-        )?;
+        )
+        .map_err(ValidatorError::Other)?;
         let hard_forks = bank_forks.read().unwrap().root_bank().hard_forks();
         if !hard_forks.is_empty() {
             info!("Hard forks: {:?}", hard_forks);
@@ -746,11 +750,12 @@ impl Validator {
 
         if let Some(expected_shred_version) = config.expected_shred_version {
             if expected_shred_version != node.info.shred_version() {
-                return Err(format!(
+                return Err(ValidatorError::Other(format!(
                     "shred version mismatch: expected {} found: {}",
                     expected_shred_version,
                     node.info.shred_version(),
-                ));
+                ))
+                .into());
             }
         }
 
@@ -887,10 +892,13 @@ impl Validator {
             &bank_forks,
             &leader_schedule_cache,
             &accounts_background_request_sender,
-        )?;
+        )
+        .map_err(ValidatorError::Other)?;
 
         if config.process_ledger_before_services {
-            process_blockstore.process()?;
+            process_blockstore
+                .process()
+                .map_err(ValidatorError::Other)?;
         }
         *start_progress.write().unwrap() = ValidatorStartProgress::StartingServices;
 
@@ -964,7 +972,9 @@ impl Validator {
                         &identity_keypair,
                         node.info
                             .tpu(Protocol::UDP)
-                            .map_err(|err| format!("Invalid TPU address: {err:?}"))?
+                            .map_err(|err| {
+                                ValidatorError::Other(format!("Invalid TPU address: {err:?}"))
+                            })?
                             .ip(),
                     )),
                     Some((&staked_nodes, &identity_keypair.pubkey())),
@@ -1028,7 +1038,8 @@ impl Validator {
                 max_complete_transaction_status_slot,
                 max_complete_rewards_slot,
                 prioritization_fee_cache.clone(),
-            )?;
+            )
+            .map_err(ValidatorError::Other)?;
 
             let pubsub_service = if !config.rpc_config.full_api {
                 None
@@ -1169,8 +1180,7 @@ impl Validator {
             &cluster_info,
             rpc_override_health_check,
             &start_progress,
-        )
-        .map_err(|err| format!("wait_for_supermajority failed: {err:?}"))?;
+        )?;
 
         let blockstore_metric_report_service =
             BlockstoreMetricReportService::new(blockstore.clone(), exit.clone());
@@ -1205,8 +1215,7 @@ impl Validator {
                 &blockstore.banking_trace_path(),
                 exit.clone(),
                 config.banking_trace_dir_byte_limit,
-            )))
-            .map_err(|err| format!("{} [{:?}]", &err, &err))?;
+            )))?;
         if banking_tracer.is_enabled() {
             info!(
                 "Enabled banking trace (dir_byte_limit: {})",
@@ -1369,11 +1378,12 @@ impl Validator {
             outstanding_repair_requests.clone(),
             cluster_slots.clone(),
             wen_restart_repair_slots.clone(),
-        )?;
+        )
+        .map_err(ValidatorError::Other)?;
 
         if in_wen_restart {
             info!("Waiting for wen_restart phase one to finish");
-            match wait_for_wen_restart(WenRestartConfig {
+            wait_for_wen_restart(WenRestartConfig {
                 wen_restart_path: config.wen_restart_proto_path.clone().unwrap(),
                 last_vote,
                 blockstore: blockstore.clone(),
@@ -1386,12 +1396,8 @@ impl Validator {
                 accounts_background_request_sender: accounts_background_request_sender.clone(),
                 genesis_config_hash: genesis_config.hash(),
                 exit: exit.clone(),
-            }) {
-                Ok(()) => {
-                    return Err("wen_restart phase one completedy".to_string());
-                }
-                Err(e) => return Err(format!("wait_for_wen_restart failed: {e:?}")),
-            };
+            })?;
+            return Err(ValidatorError::WenRestartFinished.into());
         }
 
         let (tpu, mut key_notifies) = Tpu::new(
@@ -2330,11 +2336,22 @@ fn initialize_rpc_transaction_history_services(
     }
 }
 
-#[derive(Debug, PartialEq, Eq)]
-enum ValidatorError {
+#[derive(Error, Debug)]
+pub enum ValidatorError {
+    #[error("Bad expected bank hash")]
     BadExpectedBankHash,
+
+    #[error("Ledger does not have enough data to wait for supermajority")]
     NotEnoughLedgerData,
-    Error(String),
+
+    #[error("{0}")]
+    Other(String),
+
+    #[error(transparent)]
+    TraceError(#[from] TraceError),
+
+    #[error("Wen Restart finished, please continue with --wait-for-supermajority")]
+    WenRestartFinished,
 }
 
 // Return if the validator waited on other nodes to start. In this case
@@ -2357,7 +2374,7 @@ fn wait_for_supermajority(
             if let Some(process_blockstore) = process_blockstore {
                 process_blockstore
                     .process()
-                    .map_err(ValidatorError::Error)?;
+                    .map_err(ValidatorError::Other)?;
             }
 
             let bank = bank_forks.read().unwrap().working_bank();
@@ -2761,7 +2778,7 @@ mod tests {
 
         // bank=0, wait=1, should fail
         config.wait_for_supermajority = Some(1);
-        assert_eq!(
+        matches!(
             wait_for_supermajority(
                 &config,
                 None,
@@ -2770,7 +2787,7 @@ mod tests {
                 rpc_override_health_check.clone(),
                 &start_progress,
             ),
-            Err(ValidatorError::NotEnoughLedgerData)
+            Err(ValidatorError::NotEnoughLedgerData),
         );
 
         // bank=1, wait=0, should pass, bank is past the wait slot
@@ -2793,7 +2810,7 @@ mod tests {
         // bank=1, wait=1, equal, but bad hash provided
         config.wait_for_supermajority = Some(1);
         config.expected_bank_hash = Some(hash(&[1]));
-        assert_eq!(
+        matches!(
             wait_for_supermajority(
                 &config,
                 None,
@@ -2802,7 +2819,7 @@ mod tests {
                 rpc_override_health_check,
                 &start_progress,
             ),
-            Err(ValidatorError::BadExpectedBankHash)
+            Err(ValidatorError::BadExpectedBankHash),
         );
     }