Add move tests, "builder-proposals" takes bool

lighthouse/tests/validator_manager.rs

@@ -8,11 +8,16 @@ use std::str::FromStr;
 use tempfile::{tempdir, TempDir};
 use types::*;
 use validator_manager::validators::{
-    create_validators::CreateConfig, import_validators::ImportConfig,
+    create_validators::CreateConfig,
+    import_validators::ImportConfig,
+    move_validators::{MoveConfig, Validators},
 };
 
 const EXAMPLE_ETH1_ADDRESS: &str = "0x00000000219ab540356cBB839Cbe05303d7705Fa";
 
+const EXAMPLE_PUBKEY_0: &str = "0x933ad9491b62059dd065b560d256d8957a8c402cc6e8d8ee7290ae11e8f7329267a8811c397529dac52ae1342ba58c95";
+const EXAMPLE_PUBKEY_1: &str = "0xa1d1ad0714035353258038e964ae9675dc0252ee22cea896825c01458e1807bfad2f9969338798548d9858a571f7425c";
+
 struct CommandLineTest<T> {
     cmd: Command,
     config_path: PathBuf,
@@ -93,6 +98,12 @@ impl CommandLineTest<ImportConfig> {
     }
 }
 
+impl CommandLineTest<MoveConfig> {
+    fn validators_move() -> Self {
+        Self::default().flag("validators", None).flag("move", None)
+    }
+}
+
 #[test]
 pub fn validator_create_without_output_path() {
     CommandLineTest::validators_create().assert_failed();
@@ -114,7 +125,7 @@ pub fn validator_create_defaults() {
                 disable_deposits: false,
                 specify_voting_keystore_password: false,
                 eth1_withdrawal_address: None,
-                builder_proposals: false,
+                builder_proposals: None,
                 fee_recipient: None,
                 gas_limit: None,
                 bn_url: None,
@@ -134,7 +145,7 @@ pub fn validator_create_misc_flags() {
         .flag("--stdin-inputs", None)
         .flag("--specify-voting-keystore-password", None)
         .flag("--eth1-withdrawal-address", Some(EXAMPLE_ETH1_ADDRESS))
-        .flag("--builder-proposals", None)
+        .flag("--builder-proposals", Some("true"))
         .flag("--suggested-fee-recipient", Some(EXAMPLE_ETH1_ADDRESS))
         .flag("--gas-limit", Some("1337"))
         .flag("--beacon-node", Some("http://localhost:1001"))
@@ -149,7 +160,7 @@ pub fn validator_create_misc_flags() {
                 disable_deposits: false,
                 specify_voting_keystore_password: true,
                 eth1_withdrawal_address: Some(Address::from_str(EXAMPLE_ETH1_ADDRESS).unwrap()),
-                builder_proposals: true,
+                builder_proposals: Some(true),
                 fee_recipient: Some(Address::from_str(EXAMPLE_ETH1_ADDRESS).unwrap()),
                 gas_limit: Some(1337),
                 bn_url: Some(SensitiveUrl::parse("http://localhost:1001").unwrap()),
@@ -164,8 +175,10 @@ pub fn validator_create_disable_deposits() {
         .flag("--output-path", Some("./meow"))
         .flag("--count", Some("1"))
         .flag("--disable-deposits", None)
+        .flag("--builder-proposals", Some("false"))
         .assert_success(|config| {
             assert_eq!(config.disable_deposits, true);
+            assert_eq!(config.builder_proposals, Some(false));
         });
 }
 
@@ -215,3 +228,84 @@ pub fn validator_import_missing_validators_file() {
         .flag("--validator-client-token", Some("./token.json"))
         .assert_failed();
 }
+
+#[test]
+pub fn validator_move_defaults() {
+    CommandLineTest::validators_move()
+        .flag("--src-validator-client-url", Some("http://localhost:1"))
+        .flag("--src-validator-client-token", Some("./1.json"))
+        .flag("--dest-validator-client-url", Some("http://localhost:2"))
+        .flag("--dest-validator-client-token", Some("./2.json"))
+        .flag("--validators", Some("all"))
+        .assert_success(|config| {
+            let expected = MoveConfig {
+                src_vc_url: SensitiveUrl::parse("http://localhost:1").unwrap(),
+                src_vc_token_path: PathBuf::from("./1.json"),
+                dest_vc_url: SensitiveUrl::parse("http://localhost:2").unwrap(),
+                dest_vc_token_path: PathBuf::from("./2.json"),
+                validators: Validators::All,
+                builder_proposals: None,
+                fee_recipient: None,
+                gas_limit: None,
+            };
+            assert_eq!(expected, config);
+        });
+}
+
+#[test]
+pub fn validator_move_misc_flags_0() {
+    CommandLineTest::validators_move()
+        .flag("--src-validator-client-url", Some("http://localhost:1"))
+        .flag("--src-validator-client-token", Some("./1.json"))
+        .flag("--dest-validator-client-url", Some("http://localhost:2"))
+        .flag("--dest-validator-client-token", Some("./2.json"))
+        .flag(
+            "--validators",
+            Some(&format!("{},{}", EXAMPLE_PUBKEY_0, EXAMPLE_PUBKEY_1)),
+        )
+        .flag("--builder-proposals", Some("true"))
+        .flag("--suggested-fee-recipient", Some(EXAMPLE_ETH1_ADDRESS))
+        .flag("--gas-limit", Some("1337"))
+        .assert_success(|config| {
+            let expected = MoveConfig {
+                src_vc_url: SensitiveUrl::parse("http://localhost:1").unwrap(),
+                src_vc_token_path: PathBuf::from("./1.json"),
+                dest_vc_url: SensitiveUrl::parse("http://localhost:2").unwrap(),
+                dest_vc_token_path: PathBuf::from("./2.json"),
+                validators: Validators::Some(vec![
+                    PublicKeyBytes::from_str(EXAMPLE_PUBKEY_0).unwrap(),
+                    PublicKeyBytes::from_str(EXAMPLE_PUBKEY_1).unwrap(),
+                ]),
+                builder_proposals: Some(true),
+                fee_recipient: Some(Address::from_str(EXAMPLE_ETH1_ADDRESS).unwrap()),
+                gas_limit: Some(1337),
+            };
+            assert_eq!(expected, config);
+        });
+}
+
+#[test]
+pub fn validator_move_misc_flags_1() {
+    CommandLineTest::validators_move()
+        .flag("--src-validator-client-url", Some("http://localhost:1"))
+        .flag("--src-validator-client-token", Some("./1.json"))
+        .flag("--dest-validator-client-url", Some("http://localhost:2"))
+        .flag("--dest-validator-client-token", Some("./2.json"))
+        .flag("--validators", Some(&format!("{}", EXAMPLE_PUBKEY_0)))
+        .flag("--builder-proposals", Some("false"))
+        .assert_success(|config| {
+            let expected = MoveConfig {
+                src_vc_url: SensitiveUrl::parse("http://localhost:1").unwrap(),
+                src_vc_token_path: PathBuf::from("./1.json"),
+                dest_vc_url: SensitiveUrl::parse("http://localhost:2").unwrap(),
+                dest_vc_token_path: PathBuf::from("./2.json"),
+                validators: Validators::Some(vec![
+                    PublicKeyBytes::from_str(EXAMPLE_PUBKEY_0).unwrap()
+                ]),
+                builder_proposals: Some(false),
+                fee_recipient: None,
+                gas_limit: None,
+            };
+            assert_eq!(expected, config);
+        });
+}

validator_manager/src/validators/create_validators.rs

@@ -158,7 +158,9 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                     "When provided, all created validators will attempt to create \
                     blocks via builder rather than the local EL.",
                 )
-                .required(false),
+                .required(false)
+                .possible_values(&["true", "false"])
+                .takes_value(true),
         )
         .arg(
             Arg::with_name(BEACON_NODE_FLAG)
@@ -188,7 +190,7 @@ pub struct CreateConfig {
     pub disable_deposits: bool,
     pub specify_voting_keystore_password: bool,
     pub eth1_withdrawal_address: Option<Address>,
-    pub builder_proposals: bool,
+    pub builder_proposals: Option<bool>,
     pub fee_recipient: Option<Address>,
     pub gas_limit: Option<u64>,
     pub bn_url: Option<SensitiveUrl>,
@@ -211,7 +213,7 @@ impl CreateConfig {
                 matches,
                 ETH1_WITHDRAWAL_ADDRESS_FLAG,
             )?,
-            builder_proposals: matches.is_present(BUILDER_PROPOSALS_FLAG),
+            builder_proposals: clap_utils::parse_optional(matches, 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)?,
@@ -433,8 +435,11 @@ impl ValidatorsAndDeposits {
                 slashing_protection: None,
                 fee_recipient,
                 gas_limit,
-                builder_proposals: Some(builder_proposals),
+                builder_proposals,
-                enabled: Some(true),
+                // Allow the VC to choose a default "enabled" state. Since "enabled" is not part of
+                // the standard API, leaving this as `None` means we are not forced to use the
+                // non-standard API.
+                enabled: None,
             };
 
             eprintln!(

validator_manager/src/validators/mod.rs

@@ -14,6 +14,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
         .about("Provides commands for managing validators in a Lighthouse Validator Client.")
         .subcommand(create_validators::cli_app())
         .subcommand(import_validators::cli_app())
+        .subcommand(move_validators::cli_app())
 }
 
 pub async fn cli_run<'a, T: EthSpec>(
@@ -28,6 +29,9 @@ pub async fn cli_run<'a, T: EthSpec>(
         (import_validators::CMD, Some(matches)) => {
             import_validators::cli_run(matches, dump_config).await
         }
+        (move_validators::CMD, Some(matches)) => {
+            move_validators::cli_run(matches, dump_config).await
+        }
         (unknown, _) => Err(format!(
             "{} does not have a {} command. See --help",
             CMD, unknown

validator_manager/src/validators/move_validators.rs

@@ -124,7 +124,9 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                     "When provided, all created validators will attempt to create \
                     blocks via builder rather than the local EL.",
                 )
-                .required(false),
+                .required(false)
+                .possible_values(&["true", "false"])
+                .takes_value(true),
         )
 }
 
@@ -156,7 +158,7 @@ pub struct MoveConfig {
     pub dest_vc_url: SensitiveUrl,
     pub dest_vc_token_path: PathBuf,
     pub validators: Validators,
-    pub builder_proposals: bool,
+    pub builder_proposals: Option<bool>,
     pub fee_recipient: Option<Address>,
     pub gas_limit: Option<u64>,
 }
@@ -175,7 +177,7 @@ impl MoveConfig {
                 DEST_VALIDATOR_CLIENT_TOKEN_FLAG,
             )?,
             validators: clap_utils::parse_required(matches, VALIDATORS_FLAG)?,
-            builder_proposals: matches.is_present(BUILDER_PROPOSALS_FLAG),
+            builder_proposals: clap_utils::parse_optional(matches, BUILDER_PROPOSALS_FLAG)?,
             fee_recipient: clap_utils::parse_optional(matches, FEE_RECIPIENT_FLAG)?,
             gas_limit: clap_utils::parse_optional(matches, GAS_LIMIT_FLAG)?,
         })
@@ -388,8 +390,11 @@ async fn run<'a>(config: MoveConfig) -> Result<(), String> {
             slashing_protection: Some(InterchangeJsonStr(slashing_protection)),
             fee_recipient,
             gas_limit,
-            builder_proposals: Some(builder_proposals),
+            builder_proposals,
-            enabled: Some(true),
+            // Allow the VC to choose a default "enabled" state. Since "enabled" is not part of
+            // the standard API, leaving this as `None` means we are not forced to use the
+            // non-standard API.
+            enabled: None,
         };
 
         // We might as well just ignore validators that already exist on the destination machine,