cli fixes

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)

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)

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::<usize>().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::<Vec<_>>().join(", "))),
-                    }
-                })
+                // TODO
+                // .value_parser(|len: &str| {
+                //     match len.parse::<usize>().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::<Vec<_>>().join(", "))),
+                //     }
+                // })
                 .default_value("24"),
         )
 }

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(

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::<bool>("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.

common/clap_utils/src/lib.rs

@@ -123,7 +123,8 @@ where
     <T as FromStr>::Err: std::fmt::Display,
 {
     matches
-        .get_one::<String>(name)
+        .try_get_one::<String>(name)
+        .map_err(|e| format!("Unable to parse {}: {}", name, e))?
         .map(|val| {
             val.parse()
                 .map_err(|e| format!("Unable to parse {}: {}", name, e))

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

lighthouse/tests/validator_client.rs

@@ -593,23 +593,26 @@ 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()
         .with_config(|config| {
             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);

lighthouse/tests/validator_manager.rs

@@ -55,7 +55,12 @@ impl<T> CommandLineTest<T> {
     }
 
     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"),

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")

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")?;

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