Refactor to use Config struct

lighthouse/src/main.rs

@@ -503,7 +503,7 @@ fn run<E: EthSpec>(
         eprintln!("Running validator manager for {} network", network_name);
 
         // Pass the entire `environment` to the account manager so it can run blocking operations.
-        validator_manager::run(sub_matches, environment)?;
+        validator_manager::run::<E>(sub_matches, environment)?;
 
         // Exit as soon as account manager returns control.
         return Ok(());

validator_manager/src/lib.rs

@@ -30,7 +30,9 @@ pub fn run<'a, T: EthSpec>(
         .block_on_dangerous(
             async {
                 match matches.subcommand() {
-                    (validators::CMD, Some(matches)) => validators::cli_run(matches, &spec).await,
+                    (validators::CMD, Some(matches)) => {
+                        validators::cli_run::<T>(matches, &spec).await
+                    }
                     (unknown, _) => Err(format!(
                         "{} is not a valid {} command. See --help.",
                         unknown, CMD

validator_manager/src/validators/create_validators.rs

@@ -33,11 +33,6 @@ pub const DEPOSITS_FILENAME: &str = "deposits.json";
 
 const BEACON_NODE_HTTP_TIMEOUT: Duration = Duration::from_secs(2);
 
-struct ValidatorsAndDeposits {
-    validators: Vec<ValidatorSpecification>,
-    deposits: Option<Vec<StandardDepositDataJson>>,
-}
-
 pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
     App::new(CMD)
         .about(
@@ -183,88 +178,107 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
         )
 }
 
-pub async fn cli_run<'a>(matches: &'a ArgMatches<'a>, spec: &ChainSpec) -> Result<(), String> {
+struct Config {
-    let output_path: PathBuf = clap_utils::parse_required(matches, OUTPUT_PATH_FLAG)?;
+    output_path: PathBuf,
-
+    first_index: u32,
-    if !output_path.exists() {
+    count: u32,
-        fs::create_dir(&output_path)
+    deposit_gwei: u64,
-            .map_err(|e| format!("Failed to create {:?} directory: {:?}", output_path, e))?;
+    mnemonic_path: Option<PathBuf>,
-    } else if !output_path.is_dir() {
+    stdin_inputs: bool,
-        return Err(format!("{:?} must be a directory", output_path));
+    disable_deposits: bool,
+    specify_voting_keystore_password: bool,
+    eth1_withdrawal_address: Option<Address>,
+    builder_proposals: bool,
+    fee_recipient: Option<Address>,
+    gas_limit: Option<u64>,
+    bn_url: Option<SensitiveUrl>,
 }
 
-    let validators_path = output_path.join(VALIDATORS_FILENAME);
+impl Config {
-    if validators_path.exists() {
+    fn from_cli(matches: &ArgMatches, spec: &ChainSpec) -> Result<Self, String> {
-        return Err(format!(
+        Ok(Self {
-            "{:?} already exists, refusing to overwrite",
+            output_path: clap_utils::parse_required(matches, OUTPUT_PATH_FLAG)?,
-            validators_path
+            deposit_gwei: clap_utils::parse_optional(matches, DEPOSIT_GWEI_FLAG)?
-        ));
+                .unwrap_or(spec.max_effective_balance),
+            first_index: clap_utils::parse_required(matches, FIRST_INDEX_FLAG)?,
+            count: clap_utils::parse_required(matches, COUNT_FLAG)?,
+            mnemonic_path: clap_utils::parse_optional(matches, MNEMONIC_FLAG)?,
+            stdin_inputs: cfg!(windows) || matches.is_present(STDIN_INPUTS_FLAG),
+            disable_deposits: matches.is_present(DISABLE_DEPOSITS_FLAG),
+            specify_voting_keystore_password: matches
+                .is_present(SPECIFY_VOTING_KEYSTORE_PASSWORD_FLAG),
+            eth1_withdrawal_address: clap_utils::parse_optional(
+                matches,
+                ETH1_WITHDRAWAL_ADDRESS_FLAG,
+            )?,
+            builder_proposals: matches.is_present(BUILDER_PROPOSALS_FLAG),
+            fee_recipient: clap_utils::parse_optional(matches, FEE_RECIPIENT_FLAG)?,
+            gas_limit: clap_utils::parse_optional(matches, GAS_LIMIT_FLAG)?,
+            bn_url: clap_utils::parse_optional(matches, BEACON_NODE_FLAG)?,
+        })
     }
-    let deposits_path = output_path.join(DEPOSITS_FILENAME);
-    if deposits_path.exists() {
-        return Err(format!(
-            "{:?} already exists, refusing to overwrite",
-            deposits_path
-        ));
 }
 
-    let validators_and_deposits = build_validator_spec_from_cli(matches, spec).await?;
+struct ValidatorsAndDeposits {
-
+    validators: Vec<ValidatorSpecification>,
-    eprintln!("Keystore generation complete");
+    deposits: Option<Vec<StandardDepositDataJson>>,
-
-    write_to_json_file(&validators_path, &validators_and_deposits.validators)?;
-
-    if let Some(deposits) = &validators_and_deposits.deposits {
-        write_to_json_file(&deposits_path, deposits)?;
 }
 
-    Ok(())
+impl ValidatorsAndDeposits {
-}
+    async fn new<'a, T: EthSpec>(config: Config, spec: &ChainSpec) -> Result<Self, String> {
-
+        let Config {
-fn write_to_json_file<P: AsRef<Path>, S: Serialize>(path: P, contents: &S) -> Result<(), String> {
+            // The output path is handled upstream.
-    eprintln!("Writing {:?}", path.as_ref());
+            output_path: _,
-    let mut file = fs::OpenOptions::new()
+            first_index,
-        .write(true)
+            count,
-        .create_new(true)
+            deposit_gwei,
-        .open(&path)
+            mnemonic_path,
-        .map_err(|e| format!("Failed to open {:?}: {:?}", path.as_ref(), e))?;
+            stdin_inputs,
-    serde_json::to_writer(&mut file, contents)
+            disable_deposits,
-        .map_err(|e| format!("Failed to write JSON to {:?}: {:?}", path.as_ref(), e))
+            specify_voting_keystore_password,
-}
+            eth1_withdrawal_address,
-
+            builder_proposals,
-async fn build_validator_spec_from_cli<'a>(
+            fee_recipient,
-    matches: &'a ArgMatches<'a>,
+            gas_limit,
-    spec: &ChainSpec,
+            bn_url,
-) -> Result<ValidatorsAndDeposits, String> {
+        } = config;
-    let deposit_gwei = clap_utils::parse_optional(matches, DEPOSIT_GWEI_FLAG)?
-        .unwrap_or(spec.max_effective_balance);
-    let first_index: u32 = clap_utils::parse_required(matches, FIRST_INDEX_FLAG)?;
-    let count: u32 = clap_utils::parse_required(matches, COUNT_FLAG)?;
-    let mnemonic_path: Option<PathBuf> = clap_utils::parse_optional(matches, MNEMONIC_FLAG)?;
-    let stdin_inputs = cfg!(windows) || matches.is_present(STDIN_INPUTS_FLAG);
-    let disable_deposits = matches.is_present(DISABLE_DEPOSITS_FLAG);
-    let specify_voting_keystore_password =
-        matches.is_present(SPECIFY_VOTING_KEYSTORE_PASSWORD_FLAG);
-    let eth1_withdrawal_address: Option<Address> =
-        clap_utils::parse_optional(matches, ETH1_WITHDRAWAL_ADDRESS_FLAG)?;
-    let builder_proposals = matches.is_present(BUILDER_PROPOSALS_FLAG);
-    let fee_recipient: Option<Address> = clap_utils::parse_optional(matches, FEE_RECIPIENT_FLAG)?;
-    let gas_limit: Option<u64> = clap_utils::parse_optional(matches, GAS_LIMIT_FLAG)?;
-    let bn_url: Option<SensitiveUrl> = clap_utils::parse_optional(matches, BEACON_NODE_FLAG)?;
 
         let bn_http_client = if let Some(bn_url) = bn_url {
             let bn_http_client =
                 BeaconNodeHttpClient::new(bn_url, Timeouts::set_all(BEACON_NODE_HTTP_TIMEOUT));
 
+            /*
+             * Print the version of the remote beacon node.
+             */
             let version = bn_http_client
                 .get_node_version()
                 .await
                 .map_err(|e| format!("Failed to test connection to beacon node: {:?}", e))?
                 .data
                 .version;
-
             eprintln!("Connected to beacon node running version {}", version);
 
+            /*
+             * Attempt to ensure that the beacon node is on the same network.
+             */
+            let bn_config = bn_http_client
+                .get_config_spec::<types::Config>()
+                .await
+                .map_err(|e| format!("Failed to get spec from beacon node: {:?}", e))?
+                .data;
+            if let Some(config_name) = &bn_config.config_name {
+                eprintln!("Beacon node is on {} network", config_name)
+            }
+            let bn_spec = bn_config
+                .apply_to_chain_spec::<T>(&T::default_spec())
+                .ok_or("Beacon node appears to be on an incorrect network")?;
+            if bn_spec.genesis_fork_version != spec.genesis_fork_version {
+                if let Some(config_name) = bn_spec.config_name {
+                    eprintln!("Beacon node is on {} network", config_name)
+                }
+                return Err("Beacon node appears to be on the wrong network".to_string());
+            }
+
             Some(bn_http_client)
         } else {
             None
@@ -328,7 +342,9 @@ async fn build_validator_spec_from_cli<'a>(
             let voting_keystore = keystores.voting;
             let voting_public_key = voting_keystore
                 .public_key()
-            .ok_or_else(|| format!("Validator keystore at index {} is missing a public key", i))?
+                .ok_or_else(|| {
+                    format!("Validator keystore at index {} is missing a public key", i)
+                })?
                 .into();
 
             // If the user has provided a beacon node URL, check that the validator doesn't already
@@ -350,7 +366,7 @@ async fn build_validator_spec_from_cli<'a>(
                         ))?
                     }
                     Ok(None) => eprintln!(
-                    "Validator {:?} was not found in the beacon chain",
+                        "{:?} was not found in the beacon chain",
                         voting_public_key
                     ),
                     Err(e) => {
@@ -377,9 +393,8 @@ async fn build_validator_spec_from_cli<'a>(
                     ));
                 }
 
-            let withdrawal_credentials = if let Some(eth1_withdrawal_address) =
+                let withdrawal_credentials =
-                eth1_withdrawal_address
+                    if let Some(eth1_withdrawal_address) = eth1_withdrawal_address {
-            {
                         WithdrawalCredentials::eth1(eth1_withdrawal_address, spec)
                     } else {
                         // Decrypt the withdrawal keystore so withdrawal credentials can be created. It's
@@ -389,7 +404,9 @@ async fn build_validator_spec_from_cli<'a>(
                         let withdrawal_keypair = keystores
                             .withdrawal
                             .decrypt_keypair(withdrawal_keystore_password.as_ref())
-                    .map_err(|e| format!("Failed to decrypt withdrawal keystore {}: {:?}", i, e))?;
+                            .map_err(|e| {
+                                format!("Failed to decrypt withdrawal keystore {}: {:?}", i, e)
+                            })?;
                         WithdrawalCredentials::bls(&withdrawal_keypair.pk, spec)
                     };
 
@@ -416,13 +433,76 @@ async fn build_validator_spec_from_cli<'a>(
                 enabled: Some(true),
             };
 
-        eprintln!("{}/{}: {:?}", i.saturating_add(1), count, voting_public_key);
+            eprintln!(
+                "Completed {}/{}: {:?}",
+                i.saturating_add(1),
+                count,
+                voting_public_key
+            );
 
             validators.push(validator);
         }
 
-    Ok(ValidatorsAndDeposits {
+        Ok(Self {
             validators,
             deposits,
         })
     }
+}
+
+pub async fn cli_run<'a, T: EthSpec>(
+    matches: &'a ArgMatches<'a>,
+    spec: &ChainSpec,
+) -> Result<(), String> {
+    let config = Config::from_cli(matches, spec)?;
+    run::<T>(config, spec).await
+}
+
+async fn run<'a, T: EthSpec>(config: Config, spec: &ChainSpec) -> Result<(), String> {
+    let output_path = config.output_path.clone();
+
+    if !output_path.exists() {
+        fs::create_dir(&output_path)
+            .map_err(|e| format!("Failed to create {:?} directory: {:?}", output_path, e))?;
+    } else if !output_path.is_dir() {
+        return Err(format!("{:?} must be a directory", output_path));
+    }
+
+    let validators_path = output_path.join(VALIDATORS_FILENAME);
+    if validators_path.exists() {
+        return Err(format!(
+            "{:?} already exists, refusing to overwrite",
+            validators_path
+        ));
+    }
+    let deposits_path = output_path.join(DEPOSITS_FILENAME);
+    if deposits_path.exists() {
+        return Err(format!(
+            "{:?} already exists, refusing to overwrite",
+            deposits_path
+        ));
+    }
+
+    let validators_and_deposits = ValidatorsAndDeposits::new::<T>(config, spec).await?;
+
+    eprintln!("Keystore generation complete");
+
+    write_to_json_file(&validators_path, &validators_and_deposits.validators)?;
+
+    if let Some(deposits) = &validators_and_deposits.deposits {
+        write_to_json_file(&deposits_path, deposits)?;
+    }
+
+    Ok(())
+}
+
+fn write_to_json_file<P: AsRef<Path>, S: Serialize>(path: P, contents: &S) -> Result<(), String> {
+    eprintln!("Writing {:?}", path.as_ref());
+    let mut file = fs::OpenOptions::new()
+        .write(true)
+        .create_new(true)
+        .open(&path)
+        .map_err(|e| format!("Failed to open {:?}: {:?}", path.as_ref(), e))?;
+    serde_json::to_writer(&mut file, contents)
+        .map_err(|e| format!("Failed to write JSON to {:?}: {:?}", path.as_ref(), e))
+}

validator_manager/src/validators/mod.rs

@@ -3,7 +3,7 @@ pub mod create_validators;
 pub mod import_validators;
 
 use clap::{App, ArgMatches};
-use types::ChainSpec;
+use types::{ChainSpec, EthSpec};
 
 pub const CMD: &str = "validators";
 
@@ -14,9 +14,14 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
         .subcommand(import_validators::cli_app())
 }
 
-pub async fn cli_run<'a>(matches: &'a ArgMatches<'a>, spec: &ChainSpec) -> Result<(), String> {
+pub async fn cli_run<'a, T: EthSpec>(
+    matches: &'a ArgMatches<'a>,
+    spec: &ChainSpec,
+) -> Result<(), String> {
     match matches.subcommand() {
-        (create_validators::CMD, Some(matches)) => create_validators::cli_run(matches, spec).await,
+        (create_validators::CMD, Some(matches)) => {
+            create_validators::cli_run::<T>(matches, spec).await
+        }
         (import_validators::CMD, Some(matches)) => import_validators::cli_run(matches).await,
         (unknown, _) => Err(format!(
             "{} does not have a {} command. See --help",