Anchor pre-PR: Dependency cleanup (#6769)

* remove ensure_dir_exists (2 deps saved) * group UNHANDLED_ERRORs into a generic (2 deps saved) * Introduce separate `health_metrics` crate * separate health_metrics crate * remove metrics from warp_utils * move ProcessHealth::observe and SystemHealth::observe to health_metrics * fix errors * nitpick `Cargo.toml`s --------- Co-authored-by: Daniel Knopik <[email protected]> --------- Co-authored-by: ThreeHrSleep <[email protected]>
sigp · Jan 9, 2025 · 997991f · 997991f
1 parent 64eb848
commit 997991f
Show file tree

Hide file tree

Showing 44 changed files with 304 additions and 318 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -30,6 +30,7 @@ members = [
     "common/eth2_interop_keypairs",
     "common/eth2_network_config",
     "common/eth2_wallet_manager",
+    "common/health_metrics",
     "common/lighthouse_version",
     "common/lockfile",
     "common/logging",
@@ -247,6 +248,7 @@ filesystem = { path = "common/filesystem" }
 fork_choice = { path = "consensus/fork_choice" }
 genesis = { path = "beacon_node/genesis" }
 gossipsub = { path = "beacon_node/lighthouse_network/gossipsub/" }
+health_metrics = { path = "common/health_metrics" }
 http_api = { path = "beacon_node/http_api" }
 initialized_validators = { path = "validator_client/initialized_validators" }
 int_to_bytes = { path = "consensus/int_to_bytes" }

diff --git a/account_manager/src/validator/create.rs b/account_manager/src/validator/create.rs
@@ -6,14 +6,13 @@ use account_utils::{
 };
 use clap::{Arg, ArgAction, ArgMatches, Command};
 use clap_utils::FLAG_HEADER;
-use directory::{
-    ensure_dir_exists, parse_path_or_default_with_flag, DEFAULT_SECRET_DIR, DEFAULT_WALLET_DIR,
-};
+use directory::{parse_path_or_default_with_flag, DEFAULT_SECRET_DIR, DEFAULT_WALLET_DIR};
 use environment::Environment;
 use eth2_wallet_manager::WalletManager;
 use slashing_protection::{SlashingDatabase, SLASHING_PROTECTION_FILENAME};
 use std::ffi::OsStr;
 use std::fs;
+use std::fs::create_dir_all;
 use std::path::{Path, PathBuf};
 use types::EthSpec;
 use validator_dir::Builder as ValidatorDirBuilder;
@@ -156,8 +155,8 @@ pub fn cli_run<E: EthSpec>(
         ));
     }
 
-    ensure_dir_exists(&validator_dir)?;
-    ensure_dir_exists(&secrets_dir)?;
+    create_dir_all(&validator_dir).map_err(|_| "Could not create validator dir")?;
+    create_dir_all(&secrets_dir).map_err(|_| "Could not create secrets dir")?;
 
     eprintln!("secrets-dir path {:?}", secrets_dir);
     eprintln!("wallets-dir path {:?}", wallet_base_dir);

diff --git a/account_manager/src/validator/recover.rs b/account_manager/src/validator/recover.rs
@@ -5,10 +5,10 @@ use account_utils::eth2_keystore::{keypair_from_secret, Keystore, KeystoreBuilde
 use account_utils::{random_password, read_mnemonic_from_cli, STDIN_INPUTS_FLAG};
 use clap::{Arg, ArgAction, ArgMatches, Command};
 use clap_utils::FLAG_HEADER;
-use directory::ensure_dir_exists;
 use directory::{parse_path_or_default_with_flag, DEFAULT_SECRET_DIR};
 use eth2_wallet::bip39::Seed;
 use eth2_wallet::{recover_validator_secret_from_mnemonic, KeyType, ValidatorKeystores};
+use std::fs::create_dir_all;
 use std::path::PathBuf;
 use validator_dir::Builder as ValidatorDirBuilder;
 pub const CMD: &str = "recover";
@@ -91,8 +91,8 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin
 
     eprintln!("secrets-dir path: {:?}", secrets_dir);
 
-    ensure_dir_exists(&validator_dir)?;
-    ensure_dir_exists(&secrets_dir)?;
+    create_dir_all(&validator_dir).map_err(|_| "Could not create validator dir")?;
+    create_dir_all(&secrets_dir).map_err(|_| "Could not create secrets dir")?;
 
     eprintln!();
     eprintln!("WARNING: KEY RECOVERY CAN LEAD TO DUPLICATING VALIDATORS KEYS, WHICH CAN LEAD TO SLASHING.");

diff --git a/account_manager/src/wallet/mod.rs b/account_manager/src/wallet/mod.rs
@@ -5,7 +5,8 @@ pub mod recover;
 use crate::WALLETS_DIR_FLAG;
 use clap::{Arg, ArgAction, ArgMatches, Command};
 use clap_utils::FLAG_HEADER;
-use directory::{ensure_dir_exists, parse_path_or_default_with_flag, DEFAULT_WALLET_DIR};
+use directory::{parse_path_or_default_with_flag, DEFAULT_WALLET_DIR};
+use std::fs::create_dir_all;
 use std::path::PathBuf;
 
 pub const CMD: &str = "wallet";
@@ -44,7 +45,7 @@ pub fn cli_run(matches: &ArgMatches) -> Result<(), String> {
     } else {
         parse_path_or_default_with_flag(matches, WALLETS_DIR_FLAG, DEFAULT_WALLET_DIR)?
     };
-    ensure_dir_exists(&wallet_base_dir)?;
+    create_dir_all(&wallet_base_dir).map_err(|_| "Could not create wallet base dir")?;
 
     eprintln!("wallet-dir path: {:?}", wallet_base_dir);
 

diff --git a/beacon_node/beacon_chain/src/beacon_block_streamer.rs b/beacon_node/beacon_chain/src/beacon_block_streamer.rs
@@ -690,9 +690,9 @@ impl<T: BeaconChainTypes> BeaconBlockStreamer<T> {
 async fn send_errors<E: EthSpec>(
     block_roots: Vec<Hash256>,
     sender: UnboundedSender<(Hash256, Arc<BlockResult<E>>)>,
-    beacon_chain_error: BeaconChainError,
+    unhandled_error: BeaconChainError,
 ) {
-    let result = Arc::new(Err(beacon_chain_error));
+    let result = Arc::new(Err(unhandled_error));
     for root in block_roots {
         if sender.send((root, result.clone())).is_err() {
             break;

diff --git a/beacon_node/http_api/Cargo.toml b/beacon_node/http_api/Cargo.toml
@@ -17,6 +17,7 @@ ethereum_serde_utils = { workspace = true }
 ethereum_ssz = { workspace = true }
 execution_layer = { workspace = true }
 futures = { workspace = true }
+health_metrics = { workspace = true }
 hex = { workspace = true }
 lighthouse_network = { workspace = true }
 lighthouse_version = { workspace = true }

diff --git a/beacon_node/http_api/src/attestation_performance.rs b/beacon_node/http_api/src/attestation_performance.rs
@@ -7,7 +7,7 @@ use state_processing::{
 };
 use std::sync::Arc;
 use types::{BeaconState, BeaconStateError, EthSpec, Hash256};
-use warp_utils::reject::{beacon_chain_error, custom_bad_request, custom_server_error};
+use warp_utils::reject::{custom_bad_request, custom_server_error, unhandled_error};
 
 const MAX_REQUEST_RANGE_EPOCHS: usize = 100;
 const BLOCK_ROOT_CHUNK_SIZE: usize = 100;
@@ -50,7 +50,7 @@ pub fn get_attestation_performance<T: BeaconChainTypes>(
     let end_slot = end_epoch.end_slot(T::EthSpec::slots_per_epoch());
 
     // Ensure end_epoch is smaller than the current epoch - 1.
-    let current_epoch = chain.epoch().map_err(beacon_chain_error)?;
+    let current_epoch = chain.epoch().map_err(unhandled_error)?;
     if query.end_epoch >= current_epoch - 1 {
         return Err(custom_bad_request(format!(
             "end_epoch must be less than the current epoch - 1. current: {}, end: {}",
@@ -83,7 +83,7 @@ pub fn get_attestation_performance<T: BeaconChainTypes>(
     let index_range = if target.to_lowercase() == "global" {
         chain
             .with_head(|head| Ok((0..head.beacon_state.validators().len() as u64).collect()))
-            .map_err(beacon_chain_error)?
+            .map_err(unhandled_error::<BeaconChainError>)?
     } else {
         vec![target.parse::<u64>().map_err(|_| {
             custom_bad_request(format!(
@@ -96,10 +96,10 @@ pub fn get_attestation_performance<T: BeaconChainTypes>(
     // Load block roots.
     let mut block_roots: Vec<Hash256> = chain
         .forwards_iter_block_roots_until(start_slot, end_slot)
-        .map_err(beacon_chain_error)?
+        .map_err(unhandled_error)?
         .map(|res| res.map(|(root, _)| root))
         .collect::<Result<Vec<Hash256>, _>>()
-        .map_err(beacon_chain_error)?;
+        .map_err(unhandled_error)?;
     block_roots.dedup();
 
     // Load first block so we can get its parent.
@@ -113,7 +113,7 @@ pub fn get_attestation_performance<T: BeaconChainTypes>(
         .and_then(|maybe_block| {
             maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(*first_block_root))
         })
-        .map_err(beacon_chain_error)?;
+        .map_err(unhandled_error)?;
 
     // Load the block of the prior slot which will be used to build the starting state.
     let prior_block = chain
@@ -122,14 +122,14 @@ pub fn get_attestation_performance<T: BeaconChainTypes>(
             maybe_block
                 .ok_or_else(|| BeaconChainError::MissingBeaconBlock(first_block.parent_root()))
         })
-        .map_err(beacon_chain_error)?;
+        .map_err(unhandled_error)?;
 
     // Load state for block replay.
     let state_root = prior_block.state_root();
     let state = chain
         .get_state(&state_root, Some(prior_slot))
         .and_then(|maybe_state| maybe_state.ok_or(BeaconChainError::MissingBeaconState(state_root)))
-        .map_err(beacon_chain_error)?;
+        .map_err(unhandled_error)?;
 
     // Allocate an AttestationPerformance vector for each validator in the range.
     let mut perfs: Vec<AttestationPerformance> =
@@ -198,7 +198,7 @@ pub fn get_attestation_performance<T: BeaconChainTypes>(
                     .and_then(|maybe_block| {
                         maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(*root))
                     })
-                    .map_err(beacon_chain_error)
+                    .map_err(unhandled_error)
             })
             .collect::<Result<Vec<_>, _>>()?;
 

diff --git a/beacon_node/http_api/src/attester_duties.rs b/beacon_node/http_api/src/attester_duties.rs
@@ -16,9 +16,7 @@ pub fn attester_duties<T: BeaconChainTypes>(
     request_indices: &[u64],
     chain: &BeaconChain<T>,
 ) -> Result<ApiDuties, warp::reject::Rejection> {
-    let current_epoch = chain
-        .epoch()
-        .map_err(warp_utils::reject::beacon_chain_error)?;
+    let current_epoch = chain.epoch().map_err(warp_utils::reject::unhandled_error)?;
 
     // Determine what the current epoch would be if we fast-forward our system clock by
     // `MAXIMUM_GOSSIP_CLOCK_DISPARITY`.
@@ -57,7 +55,7 @@ fn cached_attestation_duties<T: BeaconChainTypes>(
 
     let (duties, dependent_root, execution_status) = chain
         .validator_attestation_duties(request_indices, request_epoch, head_block_root)
-        .map_err(warp_utils::reject::beacon_chain_error)?;
+        .map_err(warp_utils::reject::unhandled_error)?;
 
     convert_to_api_response(
         duties,
@@ -82,7 +80,7 @@ fn compute_historic_attester_duties<T: BeaconChainTypes>(
         let (cached_head, execution_status) = chain
             .canonical_head
             .head_and_execution_status()
-            .map_err(warp_utils::reject::beacon_chain_error)?;
+            .map_err(warp_utils::reject::unhandled_error)?;
         let head = &cached_head.snapshot;
 
         if head.beacon_state.current_epoch() <= request_epoch {
@@ -131,13 +129,13 @@ fn compute_historic_attester_duties<T: BeaconChainTypes>(
     state
         .build_committee_cache(relative_epoch, &chain.spec)
         .map_err(BeaconChainError::from)
-        .map_err(warp_utils::reject::beacon_chain_error)?;
+        .map_err(warp_utils::reject::unhandled_error)?;
 
     let dependent_root = state
         // The only block which decides its own shuffling is the genesis block.
         .attester_shuffling_decision_root(chain.genesis_block_root, relative_epoch)
         .map_err(BeaconChainError::from)
-        .map_err(warp_utils::reject::beacon_chain_error)?;
+        .map_err(warp_utils::reject::unhandled_error)?;
 
     let duties = request_indices
         .iter()
@@ -147,7 +145,7 @@ fn compute_historic_attester_duties<T: BeaconChainTypes>(
                 .map_err(BeaconChainError::from)
         })
         .collect::<Result<_, _>>()
-        .map_err(warp_utils::reject::beacon_chain_error)?;
+        .map_err(warp_utils::reject::unhandled_error)?;
 
     convert_to_api_response(
         duties,
@@ -181,7 +179,7 @@ fn ensure_state_knows_attester_duties_for_epoch<E: EthSpec>(
         // A "partial" state advance is adequate since attester duties don't rely on state roots.
         partial_state_advance(state, Some(state_root), target_slot, spec)
             .map_err(BeaconChainError::from)
-            .map_err(warp_utils::reject::beacon_chain_error)?;
+            .map_err(warp_utils::reject::unhandled_error)?;
     }
 
     Ok(())
@@ -208,7 +206,7 @@ fn convert_to_api_response<T: BeaconChainTypes>(
     let usize_indices = indices.iter().map(|i| *i as usize).collect::<Vec<_>>();
     let index_to_pubkey_map = chain
         .validator_pubkey_bytes_many(&usize_indices)
-        .map_err(warp_utils::reject::beacon_chain_error)?;
+        .map_err(warp_utils::reject::unhandled_error)?;
 
     let data = duties
         .into_iter()