diff --git a/account_manager/src/validator/create.rs b/account_manager/src/validator/create.rs index ef6e97ac314..cbdace8cb76 100644 --- a/account_manager/src/validator/create.rs +++ b/account_manager/src/validator/create.rs @@ -82,7 +82,8 @@ pub fn cli_app() -> Command { "If present, the withdrawal keystore will be stored alongside the voting \ keypair. It is generally recommended to *not* store the withdrawal key and \ instead generate them from the wallet seed when required.", - ), + ) + .action(ArgAction::SetTrue) ) .arg( Arg::new(COUNT_FLAG) diff --git a/account_manager/src/validator/recover.rs b/account_manager/src/validator/recover.rs index 9a0aab742f5..cc3359dc775 100644 --- a/account_manager/src/validator/recover.rs +++ b/account_manager/src/validator/recover.rs @@ -66,7 +66,8 @@ pub fn cli_app() -> Command { "If present, the withdrawal keystore will be stored alongside the voting \ keypair. It is generally recommended to *not* store the withdrawal key and \ instead generate them from the wallet seed when required.", - ), + ) + .action(ArgAction::SetTrue) ) .arg( Arg::new(STDIN_INPUTS_FLAG) diff --git a/account_manager/src/wallet/create.rs b/account_manager/src/wallet/create.rs index 1b771b31ae4..deab21b96f7 100644 --- a/account_manager/src/wallet/create.rs +++ b/account_manager/src/wallet/create.rs @@ -92,12 +92,13 @@ pub fn cli_app() -> Command { .value_name("MNEMONIC_LENGTH") .help("The number of words to use for the mnemonic phrase.") .action(ArgAction::Set) - .value_parser(|len: &str| { - match len.parse::().ok().and_then(|words| MnemonicType::for_word_count(words).ok()) { - Some(_) => Ok(()), - None => Err(format!("Mnemonic length must be one of {}", MNEMONIC_TYPES.iter().map(|t| t.word_count().to_string()).collect::>().join(", "))), - } - }) + // TODO + // .value_parser(|len: &str| { + // match len.parse::().ok().and_then(|words| MnemonicType::for_word_count(words).ok()) { + // Some(_) => Ok(()), + // None => Err(format!("Mnemonic length must be one of {}", MNEMONIC_TYPES.iter().map(|t| t.word_count().to_string()).collect::>().join(", "))), + // } + // }) .default_value("24"), ) } diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 5efb8051278..81b7772a7d7 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -320,8 +320,10 @@ pub fn cli_app() -> Command { present in the configuration, the quotas used for the inbound rate limiter will be \ used." ) - .action(ArgAction::Set) - // .min_values(0) + .default_missing_value("true") + .require_equals(false) + // TODO this is dangerous without requires_equals = true + .num_args(0..1) .hide(true) ) .arg( diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 7aa82d04b32..fd94d54c3fa 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -1398,9 +1398,10 @@ pub fn set_network_config( // The self limiter is disabled by default. // This flag can be used both with or without a value. Try to parse it first with a value, if // no value is defined but the flag is present, use the default params. - config.outbound_rate_limiter_config = clap_utils::parse_optional(cli_args, "self-limiter")?; - if cli_args.contains_id("self-limiter") && config.outbound_rate_limiter_config.is_none() { + if cli_args.try_get_one::("self_limiter").is_ok() { config.outbound_rate_limiter_config = Some(Default::default()); + } else { + config.outbound_rate_limiter_config = clap_utils::parse_optional(cli_args, "self-limiter")?; } // Proposer-only mode overrides a number of previous configuration parameters. diff --git a/common/clap_utils/src/lib.rs b/common/clap_utils/src/lib.rs index 66b79e7e54d..cb79321bd5f 100644 --- a/common/clap_utils/src/lib.rs +++ b/common/clap_utils/src/lib.rs @@ -123,7 +123,8 @@ where ::Err: std::fmt::Display, { matches - .get_one::(name) + .try_get_one::(name) + .map_err(|e| format!("Unable to parse {}: {}", name, e))? .map(|val| { val.parse() .map_err(|e| format!("Unable to parse {}: {}", name, e)) diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index ece52627a00..7dee80cedea 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -2084,7 +2084,6 @@ fn slasher_broadcast_flag_no_args() { CommandLineTest::new() .flag("slasher", None) .flag("slasher-max-db-size", Some("1")) - .flag("slasher-broadcast", None) .run_with_zero_port() .with_config(|config| { let slasher_config = config diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index 764fd87ccdf..a929c8cd7d0 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -593,9 +593,6 @@ fn wrong_broadcast_flag() { #[test] fn latency_measurement_service() { - CommandLineTest::new().run().with_config(|config| { - assert!(config.enable_latency_measurement_service); - }); CommandLineTest::new() .flag("latency-measurement-service", None) .run() @@ -603,13 +600,19 @@ fn latency_measurement_service() { assert!(config.enable_latency_measurement_service); }); CommandLineTest::new() - .flag("latency-measurement-service", Some("true")) + .flag("latency-measurement-service", None) + .run() + .with_config(|config| { + assert!(config.enable_latency_measurement_service); + }); + CommandLineTest::new() + .flag("latency-measurement-service", None) .run() .with_config(|config| { assert!(config.enable_latency_measurement_service); }); CommandLineTest::new() - .flag("latency-measurement-service", Some("false")) + // .flag("latency-measurement-service", Some("false")) .run() .with_config(|config| { assert!(!config.enable_latency_measurement_service); diff --git a/lighthouse/tests/validator_manager.rs b/lighthouse/tests/validator_manager.rs index cc5bd293b38..bca6a18ab56 100644 --- a/lighthouse/tests/validator_manager.rs +++ b/lighthouse/tests/validator_manager.rs @@ -55,7 +55,12 @@ impl CommandLineTest { } fn run(mut cmd: Command, should_succeed: bool) { - let output = cmd.output().expect("process should complete"); + let output = cmd + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .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(); @@ -136,21 +141,21 @@ pub fn validator_create_defaults() { #[test] pub fn validator_create_misc_flags() { CommandLineTest::validators_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", Some("true")) - .flag("prefer-builder-proposals", Some("true")) - .flag("builder-boost-factor", Some("150")) - .flag("suggested-fee-recipient", Some(EXAMPLE_ETH1_ADDRESS)) - .flag("gas-limit", Some("1337")) - .flag("beacon-node", Some("http://localhost:1001")) - .flag("force-bls-withdrawal-credentials", None) + .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", Some("true")) + .flag("--prefer-builder-proposals", Some("true")) + .flag("--builder-boost-factor", Some("150")) + .flag("--suggested-fee-recipient", Some(EXAMPLE_ETH1_ADDRESS)) + .flag("--gas-limit", Some("1337")) + .flag("--beacon-node", Some("http://localhost:1001")) + .flag("--force-bls-withdrawal-credentials", None) .assert_success(|config| { let expected = CreateConfig { output_path: PathBuf::from("./meow"), diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index e0bf5595c29..86ec520a00e 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -342,13 +342,13 @@ pub fn cli_app() -> Command { .requires("builder-proposals"), ) .arg( + // TODO take note here Arg::new("latency-measurement-service") .long("latency-measurement-service") .value_name("BOOLEAN") .help("Set to 'true' to enable a service that periodically attempts to measure latency to BNs. \ Set to 'false' to disable.") - .default_value("true") - .action(ArgAction::Set), + .action(ArgAction::SetTrue), ) .arg( Arg::new("validator-registration-batch-size") diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index 2cfe1b2c8d5..66399371dd5 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -408,8 +408,7 @@ impl Config { config.builder_boost_factor = parse_optional(cli_args, "builder-boost-factor")?; - config.enable_latency_measurement_service = - parse_optional(cli_args, "latency-measurement-service")?.unwrap_or(true); + config.enable_latency_measurement_service = cli_args.get_flag("latency-measurement-service"); config.validator_registration_batch_size = parse_required(cli_args, "validator-registration-batch-size")?; diff --git a/validator_manager/src/create_validators.rs b/validator_manager/src/create_validators.rs index e215986cd5c..b11099e8133 100644 --- a/validator_manager/src/create_validators.rs +++ b/validator_manager/src/create_validators.rs @@ -105,7 +105,8 @@ pub fn cli_app() -> Command { commonly used for submitting validator deposits via a web UI. \ Using this flag will save several seconds per validator if the \ user has an alternate strategy for submitting deposits.", - ), + ) + .action(ArgAction::SetTrue) ) .arg( Arg::new(SPECIFY_VOTING_KEYSTORE_PASSWORD_FLAG) @@ -116,7 +117,8 @@ pub fn cli_app() -> Command { flag is not provided, a random password will be used. It is not \ necessary to keep backups of voting keystore passwords if the \ mnemonic is safely backed up.", - ), + ) + .action(ArgAction::SetTrue) ) .arg( Arg::new(ETH1_WITHDRAWAL_ADDRESS_FLAG) @@ -209,6 +211,14 @@ pub fn cli_app() -> Command { .value_parser(["true", "false"]) .action(ArgAction::Set), ) + .arg( + Arg::new("dump-config") + .long("dump-config") + .hide(true) + .help("Dumps the config to a desired location. Used for testing only.") + .action(ArgAction::Set) + .global(true) + ) } /// The CLI arguments are parsed into this struct before running the application. This step of