From b6493d5e2400234ce7148e3a400d6663c3f0af89 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 1 Mar 2022 22:56:47 +0000 Subject: [PATCH] Enforce Optimistic Sync Conditions & CLI Tests (v2) (#3050) ## Description This PR adds a single, trivial commit (f5d2b27d78349d5a675a2615eba42cc9ae708094) atop #2986 to resolve a tests compile error. The original author (@ethDreamer) is AFK so I'm getting this one merged :relaxed: Please see #2986 for more information about the other, significant changes in this PR. Co-authored-by: Mark Mackey Co-authored-by: ethDreamer <37123614+ethDreamer@users.noreply.github.com> --- Cargo.lock | 7 ++ .../beacon_chain/src/block_verification.rs | 25 +++++ .../beacon_chain/src/execution_payload.rs | 26 +++-- .../beacon_processor/worker/gossip_methods.rs | 1 + boot_node/Cargo.toml | 1 + boot_node/src/lib.rs | 18 ++- common/clap_utils/Cargo.toml | 4 + common/clap_utils/src/lib.rs | 33 ++++++ common/sensitive_url/src/lib.rs | 2 +- consensus/fork_choice/src/fork_choice.rs | 48 ++++++++ .../src/proto_array_fork_choice.rs | 11 +- consensus/types/src/chain_spec.rs | 18 +++ lighthouse/Cargo.toml | 2 + lighthouse/src/main.rs | 41 ++++--- lighthouse/tests/beacon_node.rs | 104 +++++++++++++++++- lighthouse/tests/exec.rs | 56 ++++++++-- 16 files changed, 348 insertions(+), 49 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 611cdf57a..49f0e7e4f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -513,6 +513,7 @@ dependencies = [ "serde", "serde_derive", "serde_json", + "serde_yaml", "slog", "slog-async", "slog-scope", @@ -729,6 +730,10 @@ dependencies = [ "eth2_ssz", "ethereum-types 0.12.1", "hex", + "serde", + "serde_json", + "serde_yaml", + "types", ] [[package]] @@ -3339,8 +3344,10 @@ dependencies = [ "lighthouse_network", "lighthouse_version", "malloc_utils", + "sensitive_url", "serde", "serde_json", + "serde_yaml", "slashing_protection", "slog", "sloggers", diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 4ac587fd7..866e1e338 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -312,6 +312,8 @@ pub enum ExecutionPayloadError { /// /// The peer is not necessarily invalid. PoWParentMissing(ExecutionBlockHash), + /// The execution node is syncing but we fail the conditions for optimistic sync + UnverifiedNonOptimisticCandidate, } impl From for ExecutionPayloadError { @@ -1128,6 +1130,29 @@ impl<'a, T: BeaconChainTypes> FullyVerifiedBlock<'a, T> { // `randao` may change. let payload_verification_status = notify_new_payload(chain, &state, block.message())?; + // If the payload did not validate or invalidate the block, check to see if this block is + // valid for optimistic import. + if payload_verification_status == PayloadVerificationStatus::NotVerified { + let current_slot = chain + .slot_clock + .now() + .ok_or(BeaconChainError::UnableToReadSlot)?; + + if !chain + .fork_choice + .read() + .is_optimistic_candidate_block( + current_slot, + block.slot(), + &block.parent_root(), + &chain.spec, + ) + .map_err(BeaconChainError::from)? + { + return Err(ExecutionPayloadError::UnverifiedNonOptimisticCandidate.into()); + } + } + // If the block is sufficiently recent, notify the validator monitor. if let Some(slot) = chain.slot_clock.now() { let epoch = slot.epoch(T::EthSpec::slots_per_epoch()); diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index 30a0d2b19..a36154619 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -141,13 +141,25 @@ pub fn validate_merge_block( } .into()), None => { - debug!( - chain.log, - "Optimistically accepting terminal block"; - "block_hash" => ?execution_payload.parent_hash, - "msg" => "the terminal block/parent was unavailable" - ); - Ok(()) + let current_slot = chain + .slot_clock + .now() + .ok_or(BeaconChainError::UnableToReadSlot)?; + // Check the optimistic sync conditions. Note that because this is the merge block, + // the justified checkpoint can't have execution enabled so we only need to check the + // current slot is at least SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY ahead of the block + // https://github.com/ethereum/consensus-specs/blob/v1.1.9/sync/optimistic.md#when-to-optimistically-import-blocks + if block.slot() + chain.spec.safe_slots_to_import_optimistically <= current_slot { + debug!( + chain.log, + "Optimistically accepting terminal block"; + "block_hash" => ?execution_payload.parent_hash, + "msg" => "the terminal block/parent was unavailable" + ); + Ok(()) + } else { + Err(ExecutionPayloadError::UnverifiedNonOptimisticCandidate.into()) + } } } } diff --git a/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs b/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs index 72cb3a7ee..bb85d063e 100644 --- a/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs +++ b/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs @@ -772,6 +772,7 @@ impl Worker { } // TODO(merge): reconsider peer scoring for this event. Err(e @BlockError::ExecutionPayloadError(ExecutionPayloadError::RequestFailed(_))) + | Err(e @ BlockError::ExecutionPayloadError(ExecutionPayloadError::UnverifiedNonOptimisticCandidate)) | Err(e @BlockError::ExecutionPayloadError(ExecutionPayloadError::NoExecutionConnection)) => { debug!(self.log, "Could not verify block for gossip, ignoring the block"; "error" => %e); diff --git a/boot_node/Cargo.toml b/boot_node/Cargo.toml index 8c89ab2e4..be58ae616 100644 --- a/boot_node/Cargo.toml +++ b/boot_node/Cargo.toml @@ -23,4 +23,5 @@ hex = "0.4.2" serde = "1.0.116" serde_derive = "1.0.116" serde_json = "1.0.66" +serde_yaml = "0.8.13" eth2_network_config = { path = "../common/eth2_network_config" } diff --git a/boot_node/src/lib.rs b/boot_node/src/lib.rs index 6b933013f..f4391f987 100644 --- a/boot_node/src/lib.rs +++ b/boot_node/src/lib.rs @@ -3,8 +3,6 @@ use clap::ArgMatches; use slog::{o, Drain, Level, Logger}; use eth2_network_config::Eth2NetworkConfig; -use std::fs::File; -use std::path::PathBuf; mod cli; pub mod config; mod server; @@ -86,15 +84,13 @@ fn main( // parse the CLI args into a useable config let config: BootNodeConfig = BootNodeConfig::new(bn_matches, eth2_network_config)?; - // Dump config if `dump-config` flag is set - let dump_config = clap_utils::parse_optional::(lh_matches, "dump-config")?; - if let Some(dump_path) = dump_config { - let config_sz = BootNodeConfigSerialization::from_config_ref(&config); - let mut file = File::create(dump_path) - .map_err(|e| format!("Failed to create dumped config: {:?}", e))?; - serde_json::to_writer(&mut file, &config_sz) - .map_err(|e| format!("Error serializing config: {:?}", e))?; - } + // Dump configs if `dump-config` or `dump-chain-config` flags are set + let config_sz = BootNodeConfigSerialization::from_config_ref(&config); + clap_utils::check_dump_configs::<_, T>( + lh_matches, + &config_sz, + ð2_network_config.chain_spec::()?, + )?; // Run the boot node if !lh_matches.is_present("immediate-shutdown") { diff --git a/common/clap_utils/Cargo.toml b/common/clap_utils/Cargo.toml index 0aa35b233..b370eb082 100644 --- a/common/clap_utils/Cargo.toml +++ b/common/clap_utils/Cargo.toml @@ -13,3 +13,7 @@ dirs = "3.0.1" eth2_network_config = { path = "../eth2_network_config" } eth2_ssz = "0.4.1" ethereum-types = "0.12.1" +serde = "1.0.116" +serde_json = "1.0.59" +serde_yaml = "0.8.13" +types = { path = "../../consensus/types"} diff --git a/common/clap_utils/src/lib.rs b/common/clap_utils/src/lib.rs index 3dd42f2a9..1ebd2b174 100644 --- a/common/clap_utils/src/lib.rs +++ b/common/clap_utils/src/lib.rs @@ -6,6 +6,7 @@ use ethereum_types::U256 as Uint256; use ssz::Decode; use std::path::PathBuf; use std::str::FromStr; +use types::{ChainSpec, Config, EthSpec}; pub mod flags; @@ -52,6 +53,12 @@ pub fn get_eth2_network_config(cli_args: &ArgMatches) -> Result( }) .transpose() } + +/// Writes configs to file if `dump-config` or `dump-chain-config` flags are set +pub fn check_dump_configs( + matches: &ArgMatches, + config: S, + spec: &ChainSpec, +) -> Result<(), String> +where + S: serde::Serialize, + E: EthSpec, +{ + if let Some(dump_path) = parse_optional::(matches, "dump-config")? { + let mut file = std::fs::File::create(dump_path) + .map_err(|e| format!("Failed to open file for writing config: {:?}", e))?; + serde_json::to_writer(&mut file, &config) + .map_err(|e| format!("Error serializing config: {:?}", e))?; + } + if let Some(dump_path) = parse_optional::(matches, "dump-chain-config")? { + let chain_config = Config::from_chain_spec::(spec); + let mut file = std::fs::File::create(dump_path) + .map_err(|e| format!("Failed to open file for writing chain config: {:?}", e))?; + serde_yaml::to_writer(&mut file, &chain_config) + .map_err(|e| format!("Error serializing config: {:?}", e))?; + } + Ok(()) +} diff --git a/common/sensitive_url/src/lib.rs b/common/sensitive_url/src/lib.rs index b7e620485..7a3cbae20 100644 --- a/common/sensitive_url/src/lib.rs +++ b/common/sensitive_url/src/lib.rs @@ -10,7 +10,7 @@ pub enum SensitiveError { } // Wrapper around Url which provides a custom `Display` implementation to protect user secrets. -#[derive(Clone)] +#[derive(Clone, PartialEq)] pub struct SensitiveUrl { pub full: Url, pub redacted: String, diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index 9f98dadf3..cf3e80603 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -928,6 +928,54 @@ where .is_descendant(self.fc_store.finalized_checkpoint().root, block_root) } + /// Returns `Ok(false)` if a block is not viable to be imported optimistically. + /// + /// ## Notes + /// + /// Equivalent to the function with the same name in the optimistic sync specs: + /// + /// https://github.com/ethereum/consensus-specs/blob/dev/sync/optimistic.md#helpers + pub fn is_optimistic_candidate_block( + &self, + current_slot: Slot, + block_slot: Slot, + block_parent_root: &Hash256, + spec: &ChainSpec, + ) -> Result> { + // If the block is sufficiently old, import it. + if block_slot + spec.safe_slots_to_import_optimistically <= current_slot { + return Ok(true); + } + + // If the justified block has execution enabled, then optimistically import any block. + if self + .get_justified_block()? + .execution_status + .is_execution_enabled() + { + return Ok(true); + } + + // If the block has an ancestor with a verified parent, import this block. + // + // TODO: This condition is not yet merged into the spec. See: + // + // https://github.com/ethereum/consensus-specs/pull/2841 + // + // ## Note + // + // If `block_parent_root` is unknown this iter will always return `None`. + if self + .proto_array + .iter_nodes(block_parent_root) + .any(|node| node.execution_status.is_valid()) + { + return Ok(true); + } + + Ok(false) + } + /// Return the current finalized checkpoint. pub fn finalized_checkpoint(&self) -> Checkpoint { *self.fc_store.finalized_checkpoint() diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 1f5b997f6..2c1341be9 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -1,5 +1,5 @@ use crate::error::Error; -use crate::proto_array::{ProposerBoost, ProtoArray}; +use crate::proto_array::{Iter, ProposerBoost, ProtoArray}; use crate::ssz_container::SszContainer; use serde_derive::{Deserialize, Serialize}; use ssz::{Decode, Encode}; @@ -40,6 +40,10 @@ pub enum ExecutionStatus { } impl ExecutionStatus { + pub fn is_execution_enabled(&self) -> bool { + !matches!(self, ExecutionStatus::Irrelevant(_)) + } + pub fn irrelevant() -> Self { ExecutionStatus::Irrelevant(false) } @@ -341,6 +345,11 @@ impl ProtoArrayForkChoice { } } + /// See `ProtoArray::iter_nodes` + pub fn iter_nodes<'a>(&'a self, block_root: &Hash256) -> Iter<'a> { + self.proto_array.iter_nodes(block_root) + } + pub fn as_bytes(&self) -> Vec { SszContainer::from(self).as_ssz_bytes() } diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 29c67808c..1ea34eafc 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -146,6 +146,7 @@ pub struct ChainSpec { pub terminal_total_difficulty: Uint256, pub terminal_block_hash: ExecutionBlockHash, pub terminal_block_hash_activation_epoch: Epoch, + pub safe_slots_to_import_optimistically: u64, /* * Networking @@ -551,6 +552,7 @@ impl ChainSpec { .expect("addition does not overflow"), terminal_block_hash: ExecutionBlockHash::zero(), terminal_block_hash_activation_epoch: Epoch::new(u64::MAX), + safe_slots_to_import_optimistically: 128u64, /* * Network specific @@ -748,6 +750,7 @@ impl ChainSpec { .expect("addition does not overflow"), terminal_block_hash: ExecutionBlockHash::zero(), terminal_block_hash_activation_epoch: Epoch::new(u64::MAX), + safe_slots_to_import_optimistically: 128u64, /* * Network specific @@ -791,6 +794,9 @@ pub struct Config { // TODO(merge): remove this default #[serde(default = "default_terminal_block_hash_activation_epoch")] pub terminal_block_hash_activation_epoch: Epoch, + // TODO(merge): remove this default + #[serde(default = "default_safe_slots_to_import_optimistically")] + pub safe_slots_to_import_optimistically: u64, #[serde(with = "eth2_serde_utils::quoted_u64")] min_genesis_active_validator_count: u64, @@ -878,6 +884,10 @@ fn default_terminal_block_hash_activation_epoch() -> Epoch { Epoch::new(u64::MAX) } +fn default_safe_slots_to_import_optimistically() -> u64 { + 128u64 +} + impl Default for Config { fn default() -> Self { let chain_spec = MainnetEthSpec::default_spec(); @@ -935,6 +945,7 @@ impl Config { terminal_total_difficulty: spec.terminal_total_difficulty, terminal_block_hash: spec.terminal_block_hash, terminal_block_hash_activation_epoch: spec.terminal_block_hash_activation_epoch, + safe_slots_to_import_optimistically: spec.safe_slots_to_import_optimistically, min_genesis_active_validator_count: spec.min_genesis_active_validator_count, min_genesis_time: spec.min_genesis_time, @@ -985,6 +996,7 @@ impl Config { terminal_total_difficulty, terminal_block_hash, terminal_block_hash_activation_epoch, + safe_slots_to_import_optimistically, min_genesis_active_validator_count, min_genesis_time, genesis_fork_version, @@ -1040,6 +1052,7 @@ impl Config { terminal_total_difficulty, terminal_block_hash, terminal_block_hash_activation_epoch, + safe_slots_to_import_optimistically, ..chain_spec.clone() }) } @@ -1227,6 +1240,7 @@ mod yaml_tests { #TERMINAL_TOTAL_DIFFICULTY: 115792089237316195423570985008687907853269984665640564039457584007913129638911 #TERMINAL_BLOCK_HASH: 0x0000000000000000000000000000000000000000000000000000000000000001 #TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH: 18446744073709551614 + #SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY: 2 MIN_GENESIS_ACTIVE_VALIDATOR_COUNT: 16384 MIN_GENESIS_TIME: 1606824000 GENESIS_FORK_VERSION: 0x00000000 @@ -1266,6 +1280,10 @@ mod yaml_tests { chain_spec.terminal_block_hash_activation_epoch, default_terminal_block_hash_activation_epoch() ); + assert_eq!( + chain_spec.safe_slots_to_import_optimistically, + default_safe_slots_to_import_optimistically() + ); assert_eq!( chain_spec.bellatrix_fork_epoch, diff --git a/lighthouse/Cargo.toml b/lighthouse/Cargo.toml index 0b4b38b58..dc9baafb0 100644 --- a/lighthouse/Cargo.toml +++ b/lighthouse/Cargo.toml @@ -41,6 +41,7 @@ lighthouse_metrics = { path = "../common/lighthouse_metrics" } lazy_static = "1.4.0" serde = { version = "1.0.116", features = ["derive"] } serde_json = "1.0.59" +serde_yaml = "0.8.13" task_executor = { path = "../common/task_executor" } malloc_utils = { path = "../common/malloc_utils" } directory = { path = "../common/directory" } @@ -51,6 +52,7 @@ tempfile = "3.1.0" validator_dir = { path = "../common/validator_dir" } slashing_protection = { path = "../validator_client/slashing_protection" } lighthouse_network = { path = "../beacon_node/lighthouse_network" } +sensitive_url = { path = "../common/sensitive_url" } [[test]] name = "lighthouse_tests" diff --git a/lighthouse/src/main.rs b/lighthouse/src/main.rs index 2f04b95ca..b60f3404c 100644 --- a/lighthouse/src/main.rs +++ b/lighthouse/src/main.rs @@ -13,7 +13,6 @@ use eth2_network_config::{Eth2NetworkConfig, DEFAULT_HARDCODED_NETWORK, HARDCODE use lighthouse_version::VERSION; use malloc_utils::configure_memory_allocator; use slog::{crit, info, warn}; -use std::fs::File; use std::path::PathBuf; use std::process::exit; use task_executor::ShutdownReason; @@ -193,6 +192,14 @@ fn main() { .takes_value(true) .global(true) ) + .arg( + Arg::with_name("dump-chain-config") + .long("dump-chain-config") + .hidden(true) + .help("Dumps the chain config to a desired location. Used for testing only.") + .takes_value(true) + .global(true) + ) .arg( Arg::with_name("immediate-shutdown") .long("immediate-shutdown") @@ -251,6 +258,19 @@ fn main() { .takes_value(true) .global(true) ) + .arg( + Arg::with_name("safe-slots-to-import-optimistically") + .long("safe-slots-to-import-optimistically") + .value_name("INTEGER") + .help("Used to coordinate manual overrides of the SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY \ + parameter. This flag should only be used if the user has a clear understanding \ + that the broad Ethereum community has elected to override this parameter in the event \ + of an attack at the PoS transition block. Incorrect use of this flag can cause your \ + node to possibly accept an invalid chain or sync more slowly. Be extremely careful with \ + this flag.") + .takes_value(true) + .global(true) + ) .subcommand(beacon_node::cli_app()) .subcommand(boot_node::cli_app()) .subcommand(validator_client::cli_app()) @@ -481,14 +501,8 @@ fn run( let executor = context.executor.clone(); let config = beacon_node::get_config::(matches, &context)?; let shutdown_flag = matches.is_present("immediate-shutdown"); - if let Some(dump_path) = clap_utils::parse_optional::(matches, "dump-config")? - { - let mut file = File::create(dump_path) - .map_err(|e| format!("Failed to create dumped config: {:?}", e))?; - serde_json::to_writer(&mut file, &config) - .map_err(|e| format!("Error serializing config: {:?}", e))?; - }; - + // Dump configs if `dump-config` or `dump-chain-config` flags are set + clap_utils::check_dump_configs::<_, E>(matches, &config, &context.eth2_config.spec)?; executor.clone().spawn( async move { if let Err(e) = ProductionBeaconNode::new(context.clone(), config).await { @@ -514,13 +528,8 @@ fn run( let config = validator_client::Config::from_cli(matches, context.log()) .map_err(|e| format!("Unable to initialize validator config: {}", e))?; let shutdown_flag = matches.is_present("immediate-shutdown"); - if let Some(dump_path) = clap_utils::parse_optional::(matches, "dump-config")? - { - let mut file = File::create(dump_path) - .map_err(|e| format!("Failed to create dumped config: {:?}", e))?; - serde_json::to_writer(&mut file, &config) - .map_err(|e| format!("Error serializing config: {:?}", e))?; - }; + // Dump configs if `dump-config` or `dump-chain-config` flags are set + clap_utils::check_dump_configs::<_, E>(matches, &config, &context.eth2_config.spec)?; if !shutdown_flag { executor.clone().spawn( async move { diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index 37c435945..7de201bc3 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -10,7 +10,7 @@ use std::process::Command; use std::str::FromStr; use std::string::ToString; use tempfile::TempDir; -use types::{Address, Checkpoint, Epoch, Hash256}; +use types::{Address, Checkpoint, Epoch, ExecutionBlockHash, Hash256, MainnetEthSpec}; use unused_port::{unused_tcp_port, unused_udp_port}; const DEFAULT_ETH1_ENDPOINT: &str = "http://localhost:8545/"; @@ -206,7 +206,35 @@ fn eth1_purge_cache_flag() { .with_config(|config| assert!(config.eth1.purge_cache)); } -// Tests for Merge flags. +// Tests for Bellatrix flags. +#[test] +fn merge_flag() { + CommandLineTest::new() + .flag("merge", None) + .run_with_zero_port() + .with_config(|config| assert!(config.execution_endpoints.is_some())); +} +#[test] +fn merge_execution_endpoints_flag() { + use sensitive_url::SensitiveUrl; + let urls = vec!["http://sigp.io/no-way:1337", "http://infura.not_real:4242"]; + let endpoints = urls + .iter() + .map(|s| SensitiveUrl::parse(s).unwrap()) + .collect::>(); + let mut endpoint_arg = urls[0].to_string(); + for url in urls.into_iter().skip(1) { + endpoint_arg.push(','); + endpoint_arg.push_str(url); + } + // this is way better but intersperse is still a nightly feature :/ + // let endpoint_arg: String = urls.into_iter().intersperse(",").collect(); + CommandLineTest::new() + .flag("merge", None) + .flag("execution-endpoints", Some(&endpoint_arg)) + .run_with_zero_port() + .with_config(|config| assert_eq!(config.execution_endpoints.as_ref(), Some(&endpoints))); +} #[test] fn merge_fee_recipient_flag() { CommandLineTest::new() @@ -223,6 +251,62 @@ fn merge_fee_recipient_flag() { ) }); } +#[test] +fn terminal_total_difficulty_override_flag() { + use beacon_node::beacon_chain::types::Uint256; + CommandLineTest::new() + .flag("terminal-total-difficulty-override", Some("1337424242")) + .run_with_zero_port() + .with_spec::(|spec| { + assert_eq!(spec.terminal_total_difficulty, Uint256::from(1337424242)) + }); +} +#[test] +fn terminal_block_hash_and_activation_epoch_override_flags() { + CommandLineTest::new() + .flag("terminal-block-hash-epoch-override", Some("1337")) + .flag( + "terminal-block-hash-override", + Some("0x4242424242424242424242424242424242424242424242424242424242424242"), + ) + .run_with_zero_port() + .with_spec::(|spec| { + assert_eq!( + spec.terminal_block_hash, + ExecutionBlockHash::from_str( + "0x4242424242424242424242424242424242424242424242424242424242424242" + ) + .unwrap() + ); + assert_eq!(spec.terminal_block_hash_activation_epoch, 1337); + }); +} +#[test] +#[should_panic] +fn terminal_block_hash_missing_activation_epoch() { + CommandLineTest::new() + .flag( + "terminal-block-hash-override", + Some("0x4242424242424242424242424242424242424242424242424242424242424242"), + ) + .run_with_zero_port(); +} +#[test] +#[should_panic] +fn epoch_override_missing_terminal_block_hash() { + CommandLineTest::new() + .flag("terminal-block-hash-epoch-override", Some("1337")) + .run_with_zero_port(); +} +#[test] +fn safe_slots_to_import_optimistically_flag() { + CommandLineTest::new() + .flag("safe-slots-to-import-optimistically", Some("421337")) + .run_with_zero_port() + .with_spec::(|spec| { + assert_eq!(spec.safe_slots_to_import_optimistically, 421337) + }); +} // Tests for Network flags. #[test] @@ -410,6 +494,15 @@ fn zero_ports_flag() { assert_eq!(config.http_metrics.listen_port, 0); }); } +#[test] +fn network_load_flag() { + CommandLineTest::new() + .flag("network-load", Some("4")) + .run_with_zero_port() + .with_config(|config| { + assert_eq!(config.network.network_load, 4); + }); +} // Tests for ENR flags. #[test] @@ -527,6 +620,13 @@ fn http_allow_origin_all_flag() { .with_config(|config| assert_eq!(config.http_api.allow_origin, Some("*".to_string()))); } #[test] +fn http_allow_sync_stalled_flag() { + CommandLineTest::new() + .flag("http-allow-sync-stalled", None) + .run_with_zero_port() + .with_config(|config| assert_eq!(config.http_api.allow_sync_stalled, true)); +} +#[test] fn http_tls_flags() { let dir = TempDir::new().expect("Unable to create temporary directory"); CommandLineTest::new() diff --git a/lighthouse/tests/exec.rs b/lighthouse/tests/exec.rs index 9526a1caf..61e0677ca 100644 --- a/lighthouse/tests/exec.rs +++ b/lighthouse/tests/exec.rs @@ -5,6 +5,7 @@ use std::path::PathBuf; use std::process::{Command, Output}; use std::str::from_utf8; use tempfile::TempDir; +use types::{ChainSpec, Config, EthSpec}; pub trait CommandLineTestExec { type Config: DeserializeOwned; @@ -23,19 +24,22 @@ pub trait CommandLineTestExec { /// Executes the `Command` returned by `Self::cmd_mut` with temporary data directory, dumps /// the configuration and shuts down immediately. /// - /// Options `--datadir`, `--dump-config` and `--immediate-shutdown` must not be set on the - /// command. + /// Options `--datadir`, `--dump-config`, `--dump-chain-config` and `--immediate-shutdown` must + /// not be set on the command. fn run(&mut self) -> CompletedTest { // Setup temp directory. let tmp_dir = TempDir::new().expect("Unable to create temporary directory"); let tmp_config_path: PathBuf = tmp_dir.path().join("config.json"); + let tmp_chain_config_path: PathBuf = tmp_dir.path().join("chain_spec.yaml"); - // Add args --datadir --dump-config --immediate-shutdown + // Add args --datadir --dump-config --dump-chain-config --immediate-shutdown let cmd = self.cmd_mut(); cmd.arg("--datadir") .arg(tmp_dir.path().as_os_str()) - .arg(format!("--{}", "--dump-config")) + .arg(format!("--{}", "dump-config")) .arg(tmp_config_path.as_os_str()) + .arg(format!("--{}", "dump-chain-config")) + .arg(tmp_chain_config_path.as_os_str()) .arg("--immediate-shutdown"); // Run the command. @@ -47,23 +51,32 @@ pub trait CommandLineTestExec { // Grab the config. let config_file = File::open(tmp_config_path).expect("Unable to open dumped config"); let config: Self::Config = from_reader(config_file).expect("Unable to deserialize config"); + // Grab the chain config. + let spec_file = + File::open(tmp_chain_config_path).expect("Unable to open dumped chain spec"); + let chain_config: Config = + serde_yaml::from_reader(spec_file).expect("Unable to deserialize config"); - CompletedTest::new(config, tmp_dir) + CompletedTest::new(config, chain_config, tmp_dir) } /// Executes the `Command` returned by `Self::cmd_mut` dumps the configuration and /// shuts down immediately. /// - /// Options `--dump-config` and `--immediate-shutdown` must not be set on the command. + /// Options `--dump-config`, `--dump-chain-config` and `--immediate-shutdown` must not be set on + /// the command. fn run_with_no_datadir(&mut self) -> CompletedTest { // Setup temp directory. let tmp_dir = TempDir::new().expect("Unable to create temporary directory"); let tmp_config_path: PathBuf = tmp_dir.path().join("config.json"); + let tmp_chain_config_path: PathBuf = tmp_dir.path().join("chain_spec.yaml"); - // Add args --datadir --dump-config --immediate-shutdown + // Add args --datadir --dump-config --dump-chain-config --immediate-shutdown let cmd = self.cmd_mut(); - cmd.arg(format!("--{}", "--dump-config")) + cmd.arg(format!("--{}", "dump-config")) .arg(tmp_config_path.as_os_str()) + .arg(format!("--{}", "dump-chain-config")) + .arg(tmp_chain_config_path.as_os_str()) .arg("--immediate-shutdown"); // Run the command. @@ -75,8 +88,13 @@ pub trait CommandLineTestExec { // Grab the config. let config_file = File::open(tmp_config_path).expect("Unable to open dumped config"); let config: Self::Config = from_reader(config_file).expect("Unable to deserialize config"); + // Grab the chain config. + let spec_file = + File::open(tmp_chain_config_path).expect("Unable to open dumped chain spec"); + let chain_config: Config = + serde_yaml::from_reader(spec_file).expect("Unable to deserialize config"); - CompletedTest::new(config, tmp_dir) + CompletedTest::new(config, chain_config, tmp_dir) } } @@ -95,19 +113,35 @@ fn output_result(cmd: &mut Command) -> Result { pub struct CompletedTest { config: C, + chain_config: Config, dir: TempDir, } impl CompletedTest { - fn new(config: C, dir: TempDir) -> Self { - CompletedTest { config, dir } + fn new(config: C, chain_config: Config, dir: TempDir) -> Self { + CompletedTest { + config, + chain_config, + dir, + } } pub fn with_config(self, func: F) { func(&self.config); } + pub fn with_spec(self, func: F) { + let spec = ChainSpec::from_config::(&self.chain_config).unwrap(); + func(spec); + } + pub fn with_config_and_dir(self, func: F) { func(&self.config, &self.dir); } + + #[allow(dead_code)] + pub fn with_config_and_spec(self, func: F) { + let spec = ChainSpec::from_config::(&self.chain_config).unwrap(); + func(&self.config, spec); + } }