Add tests for validator create

Cargo.lock

@@ -3626,6 +3626,7 @@ dependencies = [
  "env_logger 0.9.0",
  "environment",
  "eth1",
+ "eth2",
  "eth2_hashing",
  "eth2_network_config",
  "futures",

lighthouse/Cargo.toml

@@ -63,6 +63,7 @@ slashing_protection = { path = "../validator_client/slashing_protection" }
 lighthouse_network = { path = "../beacon_node/lighthouse_network" }
 sensitive_url = { path = "../common/sensitive_url" }
 eth1 = { path = "../beacon_node/eth1" }
+eth2 = { path = "../common/eth2" }
 
 [[test]]
 name = "lighthouse_tests"

lighthouse/tests/validator_manager.rs

@@ -1,15 +1,20 @@
+use eth2::SensitiveUrl;
 use serde::de::DeserializeOwned;
 use std::fs;
 use std::marker::PhantomData;
 use std::path::PathBuf;
-use std::process::Command;
+use std::process::{Command, Stdio};
+use std::str::FromStr;
 use tempfile::{tempdir, TempDir};
+use types::*;
 use validator_manager::validators::create_validators::CreateConfig;
 
+const EXAMPLE_ETH1_ADDRESS: &str = "0x00000000219ab540356cBB839Cbe05303d7705Fa";
+
 struct CommandLineTest<T> {
     cmd: Command,
-    dir: TempDir,
     config_path: PathBuf,
+    _dir: TempDir,
     _phantom: PhantomData<T>,
 }
 
@@ -18,13 +23,16 @@ impl<T> Default for CommandLineTest<T> {
         let dir = tempdir().unwrap();
         let config_path = dir.path().join("config.json");
         let mut cmd = Command::new(env!("CARGO_BIN_EXE_lighthouse"));
-        cmd.arg("--dump_config")
+        cmd.arg("--dump-config")
-            .arg(format!("{:?}", config_path))
+            .arg(config_path.as_os_str())
-            .arg("validator-manager");
+            .arg("validator-manager")
+            .stdin(Stdio::null())
+            .stdout(Stdio::null())
+            .stderr(Stdio::null());
         Self {
             cmd,
-            dir,
             config_path,
+            _dir: dir,
             _phantom: PhantomData,
         }
     }
@@ -38,26 +46,115 @@ impl<T> CommandLineTest<T> {
         }
         self
     }
+
+    fn run(mut cmd: Command, should_succeed: bool) {
+        let output = cmd.output().expect("process should complete");
+        if output.status.success() != should_succeed {
+            let stdout = String::from_utf8(output.stdout).unwrap();
+            let stderr = String::from_utf8(output.stderr).unwrap();
+            eprintln!("{}", stdout);
+            eprintln!("{}", stderr);
+            panic!(
+                "Command success was {} when expecting {}",
+                !should_succeed, should_succeed
+            );
+        }
+    }
 }
 
 impl<T: DeserializeOwned> CommandLineTest<T> {
-    fn assert_success<F: Fn(T)>(mut self, func: F) {
+    fn assert_success<F: Fn(T)>(self, func: F) {
-        let output = self.cmd.output().expect("should run command");
+        Self::run(self.cmd, true);
-        assert!(output.status.success(), "command should succeed");
-
         let contents = fs::read_to_string(self.config_path).unwrap();
         let config: T = serde_json::from_str(&contents).unwrap();
         func(config)
     }
+
+    fn assert_failed(self) {
+        Self::run(self.cmd, false);
+    }
 }
 
 impl CommandLineTest<CreateConfig> {
     fn validator_create() -> Self {
-        Self::default().flag("validator", None).flag("create", None)
+        Self::default()
+            .flag("validators", None)
+            .flag("create", None)
     }
 }
 
 #[test]
-pub fn validator_create_defaults() {
+pub fn validator_create_without_output_path() {
-    todo!()
+    CommandLineTest::validator_create().assert_failed();
+}
+
+#[test]
+pub fn validator_create_defaults() {
+    CommandLineTest::validator_create()
+        .flag("--output-path", Some("./meow"))
+        .flag("--count", Some("1"))
+        .assert_success(|config| {
+            let expected = CreateConfig {
+                output_path: PathBuf::from("./meow"),
+                first_index: 0,
+                count: 1,
+                deposit_gwei: MainnetEthSpec::default_spec().max_effective_balance,
+                mnemonic_path: None,
+                stdin_inputs: false,
+                disable_deposits: false,
+                specify_voting_keystore_password: false,
+                eth1_withdrawal_address: None,
+                builder_proposals: false,
+                fee_recipient: None,
+                gas_limit: None,
+                bn_url: None,
+            };
+            assert_eq!(expected, config);
+        });
+}
+
+#[test]
+pub fn validator_create_misc_flags_01() {
+    CommandLineTest::validator_create()
+        .flag("--output-path", Some("./meow"))
+        .flag("--deposit-gwei", Some("42"))
+        .flag("--first-index", Some("12"))
+        .flag("--count", Some("9"))
+        .flag("--mnemonic-path", Some("./woof"))
+        .flag("--stdin-inputs", None)
+        .flag("--specify-voting-keystore-password", None)
+        .flag("--eth1-withdrawal-address", Some(EXAMPLE_ETH1_ADDRESS))
+        .flag("--builder-proposals", None)
+        .flag("--suggested-fee-recipient", Some(EXAMPLE_ETH1_ADDRESS))
+        .flag("--gas-limit", Some("1337"))
+        .flag("--beacon-node", Some("http://localhost:1001"))
+        .assert_success(|config| {
+            let expected = CreateConfig {
+                output_path: PathBuf::from("./meow"),
+                first_index: 12,
+                count: 9,
+                deposit_gwei: 42,
+                mnemonic_path: Some(PathBuf::from("./woof")),
+                stdin_inputs: true,
+                disable_deposits: false,
+                specify_voting_keystore_password: true,
+                eth1_withdrawal_address: Some(Address::from_str(EXAMPLE_ETH1_ADDRESS).unwrap()),
+                builder_proposals: true,
+                fee_recipient: Some(Address::from_str(EXAMPLE_ETH1_ADDRESS).unwrap()),
+                gas_limit: Some(1337),
+                bn_url: Some(SensitiveUrl::parse("http://localhost:1001").unwrap()),
+            };
+            assert_eq!(expected, config);
+        });
+}
+
+#[test]
+pub fn validator_create_disable_deposits() {
+    CommandLineTest::validator_create()
+        .flag("--output-path", Some("./meow"))
+        .flag("--count", Some("1"))
+        .flag("--disable-deposits", None)
+        .assert_success(|config| {
+            assert_eq!(config.disable_deposits, true);
+        });
 }

validator_manager/src/lib.rs

@@ -11,29 +11,33 @@ pub mod validators;
 pub const CMD: &str = "validator_manager";
 
 /// This flag is on the top-level `lighthouse` binary.
-const DUMP_CONFIGS_FLAG: &str = "dump-configs";
+const DUMP_CONFIGS_FLAG: &str = "dump-config";
 
 /// Used only in testing, this allows a command to dump its configuration to a file and then exit
 /// successfully. This allows for testing how the CLI arguments translate to some configuration.
-pub enum DumpConfigs {
+pub enum DumpConfig {
     Disabled,
     Enabled(PathBuf),
 }
 
-impl DumpConfigs {
+impl DumpConfig {
     /// Returns `Ok(true)` if the configuration was successfully written to a file and the
     /// application should exit successfully without doing anything else.
     pub fn should_exit_early<T: Serialize>(&self, config: &T) -> Result<bool, String> {
         match self {
-            DumpConfigs::Disabled => Ok(false),
+            DumpConfig::Disabled => Ok(false),
-            DumpConfigs::Enabled(dump_path) => write_to_json_file(dump_path, config).map(|()| true),
+            DumpConfig::Enabled(dump_path) => {
+                dbg!(dump_path);
+                write_to_json_file(dump_path, config)?;
+                Ok(true)
+            }
         }
     }
 }
 
 pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
     App::new(CMD)
-        .visible_aliases(&["vm", CMD])
+        .visible_aliases(&["vm", "validator-manager", CMD])
         .about("Utilities for managing a Lighthouse validator client via the HTTP API.")
         .subcommand(validators::cli_app())
 }
@@ -45,9 +49,9 @@ pub fn run<'a, T: EthSpec>(
 ) -> Result<(), String> {
     let context = env.core_context();
     let spec = context.eth2_config.spec.clone();
-    let dump_configs = clap_utils::parse_optional(matches, DUMP_CONFIGS_FLAG)?
+    let dump_config = clap_utils::parse_optional(matches, DUMP_CONFIGS_FLAG)?
-        .map(DumpConfigs::Enabled)
+        .map(DumpConfig::Enabled)
-        .unwrap_or_else(|| DumpConfigs::Disabled);
+        .unwrap_or_else(|| DumpConfig::Disabled);
 
     context
         .executor
@@ -58,7 +62,7 @@ pub fn run<'a, T: EthSpec>(
             async {
                 match matches.subcommand() {
                     (validators::CMD, Some(matches)) => {
-                        validators::cli_run::<T>(matches, &spec, dump_configs).await
+                        validators::cli_run::<T>(matches, &spec, dump_config).await
                     }
                     (unknown, _) => Err(format!(
                         "{} is not a valid {} command. See --help.",

validator_manager/src/validators/create_validators.rs

@@ -1,5 +1,5 @@
 use super::common::*;
-use crate::DumpConfigs;
+use crate::DumpConfig;
 use account_utils::{random_password_string, read_mnemonic_from_cli, read_password_from_user};
 use clap::{App, Arg, ArgMatches};
 use eth2::{
@@ -51,7 +51,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                     "The path to a directory where the validator and (optionally) deposits \
                     files will be created. The directory will be created if it does not exist.",
                 )
-                .conflicts_with(DISABLE_DEPOSITS_FLAG)
                 .required(true)
                 .takes_value(true),
         )
@@ -100,7 +99,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
         .arg(
             Arg::with_name(DISABLE_DEPOSITS_FLAG)
                 .long(DISABLE_DEPOSITS_FLAG)
-                .value_name("PATH")
                 .help(
                     "When provided don't generate the deposits JSON file that is \
                     commonly used for submitting validator deposits via a web UI. \
@@ -111,8 +109,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
         .arg(
             Arg::with_name(SPECIFY_VOTING_KEYSTORE_PASSWORD_FLAG)
                 .long(SPECIFY_VOTING_KEYSTORE_PASSWORD_FLAG)
-                .value_name("STRING")
-                .takes_value(true)
                 .help(
                     "If present, the user will be prompted to enter the voting keystore \
                     password that will be used to encrypt the voting keystores. If this \
@@ -181,7 +177,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
 
 /// The CLI arguments are parsed into this struct before running the application. This step of
 /// indirection allows for testing the underlying logic without needing to parse CLI arguments.
-#[derive(Clone, PartialEq, Serialize, Deserialize)]
+#[derive(Clone, PartialEq, Debug, Serialize, Deserialize)]
 pub struct CreateConfig {
     pub output_path: PathBuf,
     pub first_index: u32,
@@ -461,10 +457,10 @@ impl ValidatorsAndDeposits {
 pub async fn cli_run<'a, T: EthSpec>(
     matches: &'a ArgMatches<'a>,
     spec: &ChainSpec,
-    dump_configs: DumpConfigs,
+    dump_config: DumpConfig,
 ) -> Result<(), String> {
     let config = CreateConfig::from_cli(matches, spec)?;
-    if dump_configs.should_exit_early(&config)? {
+    if dump_config.should_exit_early(&config)? {
         Ok(())
     } else {
         run::<T>(config, spec).await

validator_manager/src/validators/import_validators.rs

@@ -1,5 +1,5 @@
 use super::common::*;
-use crate::DumpConfigs;
+use crate::DumpConfig;
 use clap::{App, Arg, ArgMatches};
 use eth2::{
     lighthouse_vc::{
@@ -95,10 +95,10 @@ impl ImportConfig {
 
 pub async fn cli_run<'a>(
     matches: &'a ArgMatches<'a>,
-    dump_configs: DumpConfigs,
+    dump_config: DumpConfig,
 ) -> Result<(), String> {
     let config = ImportConfig::from_cli(matches)?;
-    if dump_configs.should_exit_early(&config)? {
+    if dump_config.should_exit_early(&config)? {
         Ok(())
     } else {
         run(config).await

validator_manager/src/validators/mod.rs

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