From 5b86fce56b983b4060f39e0b443b90747517c80b Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 23 Aug 2022 12:09:02 +1000 Subject: [PATCH] Add tests for validator create --- Cargo.lock | 1 + lighthouse/Cargo.toml | 1 + lighthouse/tests/validator_manager.rs | 123 ++++++++++++++++-- validator_manager/src/lib.rs | 24 ++-- .../src/validators/create_validators.rs | 12 +- .../src/validators/import_validators.rs | 6 +- validator_manager/src/validators/mod.rs | 8 +- 7 files changed, 137 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5ed823ccd0..6740ba226d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3626,6 +3626,7 @@ dependencies = [ "env_logger 0.9.0", "environment", "eth1", + "eth2", "eth2_hashing", "eth2_network_config", "futures", diff --git a/lighthouse/Cargo.toml b/lighthouse/Cargo.toml index 3b0416cf0f..a9d49c59a5 100644 --- a/lighthouse/Cargo.toml +++ b/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" diff --git a/lighthouse/tests/validator_manager.rs b/lighthouse/tests/validator_manager.rs index d96f4c1a97..48eaa1a689 100644 --- a/lighthouse/tests/validator_manager.rs +++ b/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 { cmd: Command, - dir: TempDir, config_path: PathBuf, + _dir: TempDir, _phantom: PhantomData, } @@ -18,13 +23,16 @@ impl Default for CommandLineTest { 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") - .arg(format!("{:?}", config_path)) - .arg("validator-manager"); + cmd.arg("--dump-config") + .arg(config_path.as_os_str()) + .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 CommandLineTest { } 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 CommandLineTest { - fn assert_success(mut self, func: F) { - let output = self.cmd.output().expect("should run command"); - assert!(output.status.success(), "command should succeed"); - + fn assert_success(self, func: F) { + Self::run(self.cmd, true); 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 { 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() { - todo!() +pub fn validator_create_without_output_path() { + 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); + }); } diff --git a/validator_manager/src/lib.rs b/validator_manager/src/lib.rs index 439db1449b..4b52b8edb3 100644 --- a/validator_manager/src/lib.rs +++ b/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(&self, config: &T) -> Result { match self { - DumpConfigs::Disabled => Ok(false), - DumpConfigs::Enabled(dump_path) => write_to_json_file(dump_path, config).map(|()| true), + DumpConfig::Disabled => Ok(false), + 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)? - .map(DumpConfigs::Enabled) - .unwrap_or_else(|| DumpConfigs::Disabled); + let dump_config = clap_utils::parse_optional(matches, DUMP_CONFIGS_FLAG)? + .map(DumpConfig::Enabled) + .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::(matches, &spec, dump_configs).await + validators::cli_run::(matches, &spec, dump_config).await } (unknown, _) => Err(format!( "{} is not a valid {} command. See --help.", diff --git a/validator_manager/src/validators/create_validators.rs b/validator_manager/src/validators/create_validators.rs index ee73d02d2d..e2a0ec8f7a 100644 --- a/validator_manager/src/validators/create_validators.rs +++ b/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::(config, spec).await diff --git a/validator_manager/src/validators/import_validators.rs b/validator_manager/src/validators/import_validators.rs index 418b561b6c..851ee61ff3 100644 --- a/validator_manager/src/validators/import_validators.rs +++ b/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 diff --git a/validator_manager/src/validators/mod.rs b/validator_manager/src/validators/mod.rs index 1b6b769378..4933c87746 100644 --- a/validator_manager/src/validators/mod.rs +++ b/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::(matches, spec, dump_configs).await + create_validators::cli_run::(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",