diff --git a/lighthouse/tests/validator_manager.rs b/lighthouse/tests/validator_manager.rs index a0737032d4..3ce64fe70e 100644 --- a/lighthouse/tests/validator_manager.rs +++ b/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 { cmd: Command, config_path: PathBuf, @@ -93,6 +98,12 @@ impl CommandLineTest { } } +impl CommandLineTest { + 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); + }); +} diff --git a/validator_manager/src/validators/create_validators.rs b/validator_manager/src/validators/create_validators.rs index 6d8d1b5a42..01c1bc71e5 100644 --- a/validator_manager/src/validators/create_validators.rs +++ b/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
, - pub builder_proposals: bool, + pub builder_proposals: Option, pub fee_recipient: Option
, pub gas_limit: Option, pub bn_url: Option, @@ -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), - enabled: Some(true), + builder_proposals, + // 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!( diff --git a/validator_manager/src/validators/mod.rs b/validator_manager/src/validators/mod.rs index 4e502647da..eeb0a02962 100644 --- a/validator_manager/src/validators/mod.rs +++ b/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 diff --git a/validator_manager/src/validators/move_validators.rs b/validator_manager/src/validators/move_validators.rs index f12c22f0cd..9ccb0286e6 100644 --- a/validator_manager/src/validators/move_validators.rs +++ b/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, pub fee_recipient: Option
, pub gas_limit: Option, } @@ -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), - enabled: Some(true), + builder_proposals, + // 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,