From 2871253905e3fe31ab190e2003339dcd0ffa729c Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 20 Apr 2020 11:58:39 +1000 Subject: [PATCH] Do not override spec defaults via default flags (#1022) --- lcli/src/main.rs | 13 +++------- lcli/src/new_testnet.rs | 54 ++++++++++++++++++----------------------- 2 files changed, 26 insertions(+), 41 deletions(-) diff --git a/lcli/src/main.rs b/lcli/src/main.rs index 7abd4b44a..c7e54badf 100644 --- a/lcli/src/main.rs +++ b/lcli/src/main.rs @@ -251,7 +251,8 @@ fn main() { .subcommand( SubCommand::with_name("new-testnet") .about( - "Produce a new testnet directory.", + "Produce a new testnet directory. If any of the optional flags are not + supplied the values will remain the default for the --spec flag", ) .arg( Arg::with_name("min-genesis-time") @@ -266,7 +267,6 @@ fn main() { .long("min-genesis-active-validator-count") .value_name("INTEGER") .takes_value(true) - .default_value("16384") .help("The number of validators required to trigger eth2 genesis."), ) .arg( @@ -274,7 +274,6 @@ fn main() { .long("min-genesis-delay") .value_name("SECONDS") .takes_value(true) - .default_value("3600") // 10 minutes .help("The delay between sufficient eth1 deposits and eth2 genesis."), ) .arg( @@ -282,7 +281,6 @@ fn main() { .long("min-deposit-amount") .value_name("GWEI") .takes_value(true) - .default_value("100000000") // 0.1 Eth .help("The minimum permitted deposit amount."), ) .arg( @@ -290,7 +288,6 @@ fn main() { .long("max-effective-balance") .value_name("GWEI") .takes_value(true) - .default_value("3200000000") // 3.2 Eth .help("The amount required to become a validator."), ) .arg( @@ -298,7 +295,6 @@ fn main() { .long("effective-balance-increment") .value_name("GWEI") .takes_value(true) - .default_value("100000000") // 0.1 Eth .help("The steps in effective balance calculation."), ) .arg( @@ -306,7 +302,6 @@ fn main() { .long("ejection-balance") .value_name("GWEI") .takes_value(true) - .default_value("1600000000") // 1.6 Eth .help("The balance at which a validator gets ejected."), ) .arg( @@ -314,7 +309,6 @@ fn main() { .long("eth1-follow-distance") .value_name("ETH1_BLOCKS") .takes_value(true) - .default_value("16") .help("The distance to follow behind the eth1 chain head."), ) .arg( @@ -322,7 +316,6 @@ fn main() { .long("genesis-fork-version") .value_name("HEX") .takes_value(true) - .default_value("0x00000000") .help("Used to avoid reply attacks between testnets. Recommended to set to non-default."), ) @@ -331,7 +324,7 @@ fn main() { .long("deposit-contract-address") .value_name("ETH1_ADDRESS") .takes_value(true) - .default_value("0x0000000000000000000000000000000000000000") + .required(true) .help("The address of the deposit contract."), ) .arg( diff --git a/lcli/src/new_testnet.rs b/lcli/src/new_testnet.rs index 7050defa2..44eb1ab65 100644 --- a/lcli/src/new_testnet.rs +++ b/lcli/src/new_testnet.rs @@ -4,7 +4,6 @@ use clap_utils::{ }; use eth2_testnet_config::Eth2TestnetConfig; use std::path::PathBuf; -use std::time::{SystemTime, UNIX_EPOCH}; use types::{Address, EthSpec, YamlConfig}; pub fn run(matches: &ArgMatches) -> Result<(), String> { @@ -13,18 +12,8 @@ pub fn run(matches: &ArgMatches) -> Result<(), String> { "testnet-dir", PathBuf::from(".lighthouse/testnet"), )?; - let min_genesis_time = parse_optional(matches, "min-genesis-time")?; - let min_genesis_delay = parse_required(matches, "min-genesis-delay")?; - let min_genesis_active_validator_count = - parse_required(matches, "min-genesis-active-validator-count")?; - let min_deposit_amount = parse_required(matches, "min-deposit-amount")?; - let max_effective_balance = clap_utils::parse_required(matches, "max-effective-balance")?; - let effective_balance_increment = parse_required(matches, "effective-balance-increment")?; - let ejection_balance = parse_required(matches, "ejection-balance")?; - let eth1_follow_distance = parse_required(matches, "eth1-follow-distance")?; - let deposit_contract_deploy_block = parse_required(matches, "deposit-contract-deploy-block")?; - let genesis_fork_version = parse_ssz_optional::<[u8; 4]>(matches, "genesis-fork-version")?; let deposit_contract_address: Address = parse_required(matches, "deposit-contract-address")?; + let deposit_contract_deploy_block = parse_required(matches, "deposit-contract-deploy-block")?; if testnet_dir_path.exists() { return Err(format!( @@ -34,19 +23,29 @@ pub fn run(matches: &ArgMatches) -> Result<(), String> { } let mut spec = T::default_spec(); - if let Some(time) = min_genesis_time { - spec.min_genesis_time = time; - } else { - spec.min_genesis_time = time_now()?; + + // Update the spec value if the flag was defined. Otherwise, leave it as the default. + macro_rules! maybe_update { + ($flag: tt, $var: ident) => { + if let Some(val) = parse_optional(matches, $flag)? { + spec.$var = val + } + }; } - spec.min_deposit_amount = min_deposit_amount; - spec.min_genesis_active_validator_count = min_genesis_active_validator_count; - spec.max_effective_balance = max_effective_balance; - spec.effective_balance_increment = effective_balance_increment; - spec.ejection_balance = ejection_balance; - spec.eth1_follow_distance = eth1_follow_distance; - spec.min_genesis_delay = min_genesis_delay; - if let Some(v) = genesis_fork_version { + + maybe_update!("min-genesis-time", min_genesis_time); + maybe_update!("min-deposit-amount", min_deposit_amount); + maybe_update!( + "min-genesis-active-validator-count", + min_genesis_active_validator_count + ); + maybe_update!("max-effective-balance", max_effective_balance); + maybe_update!("effective-balance-increment", effective_balance_increment); + maybe_update!("ejection-balance", ejection_balance); + maybe_update!("eth1-follow_distance", eth1_follow_distance); + maybe_update!("min-genesis-delay", min_genesis_delay); + + if let Some(v) = parse_ssz_optional(matches, "genesis-fork-version")? { spec.genesis_fork_version = v; } @@ -60,10 +59,3 @@ pub fn run(matches: &ArgMatches) -> Result<(), String> { testnet.write_to_file(testnet_dir_path) } - -pub fn time_now() -> Result { - SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|duration| duration.as_secs()) - .map_err(|e| format!("Unable to get time: {:?}", e)) -}