From 24dc9482a9a2f8651b30bfbe6ab13df0db1be2a2 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Sun, 24 Nov 2019 12:23:07 +1100 Subject: [PATCH] Fix beacon_chain_sim, nitpicks --- tests/beacon_chain_sim/src/main.rs | 18 +++++++++++++++--- tests/node_test_rig/src/lib.rs | 28 ++++++++++++++++------------ validator_client/src/config.rs | 2 +- validator_client/src/lib.rs | 17 ++++------------- 4 files changed, 36 insertions(+), 29 deletions(-) diff --git a/tests/beacon_chain_sim/src/main.rs b/tests/beacon_chain_sim/src/main.rs index 0860416fa1..0853096d0d 100644 --- a/tests/beacon_chain_sim/src/main.rs +++ b/tests/beacon_chain_sim/src/main.rs @@ -1,5 +1,5 @@ use node_test_rig::{ - environment::{EnvironmentBuilder, RuntimeContext}, + environment::{Environment, EnvironmentBuilder, RuntimeContext}, testing_client_config, ClientConfig, ClientGenesis, LocalBeaconNode, LocalValidatorClient, ProductionClient, ValidatorConfig, }; @@ -64,10 +64,13 @@ fn simulation(num_nodes: usize, validators_per_node: usize) -> Result<(), String .spec .clone(); + let context = env.service_context(format!("validator_{}", i)); + let indices = (i * validators_per_node..(i + 1) * validators_per_node).collect::>(); new_validator_client( - env.service_context(format!("validator_{}", i)), + &mut env, + context, node, ValidatorConfig::default(), &indices, @@ -100,7 +103,10 @@ fn new_with_bootnode_via_enr( BeaconNode::production(context, config) } +// Note: this function will block until the validator can connect to the beaco node. It is +// recommended to ensure that the beacon node is running first. fn new_validator_client( + env: &mut Environment, context: RuntimeContext, beacon_node: &BeaconNode, base_config: ValidatorConfig, @@ -115,5 +121,11 @@ fn new_validator_client( config.http_server = format!("http://{}:{}", socket_addr.ip(), socket_addr.port()); - LocalValidatorClient::production_with_insecure_keypairs(context, config, keypair_indices) + env.runtime() + .block_on(LocalValidatorClient::production_with_insecure_keypairs( + context, + config, + keypair_indices, + )) + .expect("should start validator") } diff --git a/tests/node_test_rig/src/lib.rs b/tests/node_test_rig/src/lib.rs index c81034be51..d194801eed 100644 --- a/tests/node_test_rig/src/lib.rs +++ b/tests/node_test_rig/src/lib.rs @@ -106,7 +106,7 @@ impl LocalValidatorClient { context: RuntimeContext, mut config: ValidatorConfig, keypair_indices: &[usize], - ) -> Self { + ) -> impl Future { // Creates a temporary directory that will be deleted once this `TempDir` is dropped. let datadir = TempDir::new("lighthouse-beacon-node") .expect("should create temp directory for client datadir"); @@ -120,7 +120,10 @@ impl LocalValidatorClient { /// /// - The validator created is using the same types as the node we use in production. /// - It is recommended to use `production_with_insecure_keypairs` for testing. - pub fn production(context: RuntimeContext, config: ValidatorConfig) -> Self { + pub fn production( + context: RuntimeContext, + config: ValidatorConfig, + ) -> impl Future { // Creates a temporary directory that will be deleted once this `TempDir` is dropped. let datadir = TempDir::new("lighthouse-validator") .expect("should create temp directory for client datadir"); @@ -128,17 +131,18 @@ impl LocalValidatorClient { Self::new(context, config, datadir) } - fn new(context: RuntimeContext, mut config: ValidatorConfig, datadir: TempDir) -> Self { + fn new( + context: RuntimeContext, + mut config: ValidatorConfig, + datadir: TempDir, + ) -> impl Future { config.data_dir = datadir.path().into(); - let client = ProductionValidatorClient::new(context, config) - .wait() - .expect("should start validator client"); - - client - .start_service() - .expect("should start validator client"); - - Self { client, datadir } + ProductionValidatorClient::new(context, config).map(move |mut client| { + client + .start_service() + .expect("should start validator services"); + Self { client, datadir } + }) } } diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index 001f453de8..2e16752d67 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -72,7 +72,7 @@ impl Config { } } -/// Parses the `testnet` CLI subcommand, modifying the `config` based upon the parametes in +/// Parses the `testnet` CLI subcommand, modifying the `config` based upon the parameters in /// `cli_args`. fn process_testnet_subcommand(cli_args: &ArgMatches, mut config: Config) -> Result { config.key_source = match cli_args.subcommand() { diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index bdd373c33d..8ac42cf650 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -22,12 +22,10 @@ use futures::{ future::{self, loop_fn, Loop}, Future, IntoFuture, }; -use parking_lot::RwLock; use remote_beacon_node::RemoteBeaconNode; use slog::{error, info, Logger}; use slot_clock::SlotClock; use slot_clock::SystemTimeSlotClock; -use std::sync::Arc; use std::time::{Duration, Instant}; use tokio::timer::Delay; use types::EthSpec; @@ -39,14 +37,13 @@ const RETRY_DELAY: Duration = Duration::from_secs(2); /// The global timeout for HTTP requests to the beacon node. const HTTP_TIMEOUT: Duration = Duration::from_secs(12); -#[derive(Clone)] pub struct ProductionValidatorClient { context: RuntimeContext, duties_service: DutiesService, fork_service: ForkService, block_service: BlockService, attestation_service: AttestationService, - exit_signals: Arc>>, + exit_signals: Vec, } impl ProductionValidatorClient { @@ -188,39 +185,33 @@ impl ProductionValidatorClient { fork_service, block_service, attestation_service, - exit_signals: Arc::new(RwLock::new(vec![])), + exit_signals: vec![], }) }) } - pub fn start_service(&self) -> Result<(), String> { + pub fn start_service(&mut self) -> Result<(), String> { let duties_exit = self .duties_service .start_update_service(&self.context.eth2_config.spec) .map_err(|e| format!("Unable to start duties service: {}", e))?; - self.exit_signals.write().push(duties_exit); - let fork_exit = self .fork_service .start_update_service(&self.context.eth2_config.spec) .map_err(|e| format!("Unable to start fork service: {}", e))?; - self.exit_signals.write().push(fork_exit); - let block_exit = self .block_service .start_update_service(&self.context.eth2_config.spec) .map_err(|e| format!("Unable to start block service: {}", e))?; - self.exit_signals.write().push(block_exit); - let attestation_exit = self .attestation_service .start_update_service(&self.context.eth2_config.spec) .map_err(|e| format!("Unable to start attestation service: {}", e))?; - self.exit_signals.write().push(attestation_exit); + self.exit_signals = vec![duties_exit, fork_exit, block_exit, attestation_exit]; Ok(()) }