Tidy sim, fix broken rest_api tests

beacon_node/rest_api/tests/test.rs

@@ -26,6 +26,16 @@ fn build_env() -> Environment<E> {
         .expect("environment should build")
 }
 
+fn build_node<E: EthSpec>(env: &mut Environment<E>) -> LocalBeaconNode<E> {
+    let context = env.core_context();
+    env.runtime()
+        .block_on(LocalBeaconNode::production(
+            context,
+            testing_client_config(),
+        ))
+        .expect("should block until node created")
+}
+
 /// Returns the randao reveal for the given slot (assuming the given `beacon_chain` uses
 /// deterministic keypairs).
 fn get_randao_reveal<T: BeaconChainTypes>(
@@ -64,7 +74,7 @@ fn validator_produce_attestation() {
 
     let spec = &E::default_spec();
 
-    let node = LocalBeaconNode::production(env.core_context(), testing_client_config());
+    let node = build_node(&mut env);
     let remote_node = node.remote_node().expect("should produce remote node");
 
     let beacon_chain = node
@@ -160,7 +170,7 @@ fn validator_duties_bulk() {
 
     let spec = &E::default_spec();
 
-    let node = LocalBeaconNode::production(env.core_context(), testing_client_config());
+    let node = build_node(&mut env);
     let remote_node = node.remote_node().expect("should produce remote node");
 
     let beacon_chain = node
@@ -197,7 +207,7 @@ fn validator_duties() {
 
     let spec = &E::default_spec();
 
-    let node = LocalBeaconNode::production(env.core_context(), testing_client_config());
+    let node = build_node(&mut env);
     let remote_node = node.remote_node().expect("should produce remote node");
 
     let beacon_chain = node
@@ -321,7 +331,7 @@ fn validator_block_post() {
         genesis_time: 13_371_337,
     };
 
-    let node = LocalBeaconNode::production(env.core_context(), config);
+    let node = build_node(&mut env);
     let remote_node = node.remote_node().expect("should produce remote node");
 
     let beacon_chain = node
@@ -387,7 +397,7 @@ fn validator_block_get() {
 
     let spec = &E::default_spec();
 
-    let node = LocalBeaconNode::production(env.core_context(), testing_client_config());
+    let node = build_node(&mut env);
     let remote_node = node.remote_node().expect("should produce remote node");
 
     let beacon_chain = node
@@ -425,7 +435,7 @@ fn validator_block_get() {
 fn beacon_state() {
     let mut env = build_env();
 
-    let node = LocalBeaconNode::production(env.core_context(), testing_client_config());
+    let node = build_node(&mut env);
     let remote_node = node.remote_node().expect("should produce remote node");
 
     let (state_by_slot, root) = env
@@ -469,7 +479,7 @@ fn beacon_state() {
 fn beacon_block() {
     let mut env = build_env();
 
-    let node = LocalBeaconNode::production(env.core_context(), testing_client_config());
+    let node = build_node(&mut env);
     let remote_node = node.remote_node().expect("should produce remote node");
 
     let (block_by_slot, root) = env
@@ -513,7 +523,7 @@ fn beacon_block() {
 fn genesis_time() {
     let mut env = build_env();
 
-    let node = LocalBeaconNode::production(env.core_context(), testing_client_config());
+    let node = build_node(&mut env);
     let remote_node = node.remote_node().expect("should produce remote node");
 
     let genesis_time = env
@@ -537,7 +547,7 @@ fn genesis_time() {
 fn fork() {
     let mut env = build_env();
 
-    let node = LocalBeaconNode::production(env.core_context(), testing_client_config());
+    let node = build_node(&mut env);
     let remote_node = node.remote_node().expect("should produce remote node");
 
     let fork = env
@@ -561,7 +571,7 @@ fn fork() {
 fn eth2_config() {
     let mut env = build_env();
 
-    let node = LocalBeaconNode::production(env.core_context(), testing_client_config());
+    let node = build_node(&mut env);
     let remote_node = node.remote_node().expect("should produce remote node");
 
     let eth2_config = env
@@ -585,7 +595,7 @@ fn eth2_config() {
 fn get_version() {
     let mut env = build_env();
 
-    let node = LocalBeaconNode::production(env.core_context(), testing_client_config());
+    let node = build_node(&mut env);
     let remote_node = node.remote_node().expect("should produce remote node");
 
     let version = env

tests/beacon_chain_sim/src/checks.rs

@@ -2,15 +2,31 @@ use crate::local_network::LocalNetwork;
 use futures::{stream, Future, IntoFuture, Stream};
 use std::time::{Duration, Instant};
 use tokio::timer::Delay;
-use types::{Epoch, EthSpec, Slot};
+use types::{Epoch, EthSpec, Slot, Unsigned};
 
+/// Checks that all of the validators have on-boarded by the start of the second eth1 voting
+/// period.
+pub fn verify_initial_validator_count<E: EthSpec>(
+    network: LocalNetwork<E>,
+    slot_duration: Duration,
+    initial_validator_count: usize,
+) -> impl Future<Item = (), Error = String> {
+    slot_delay(Slot::new(1), slot_duration)
+        .and_then(move |()| verify_validator_count(network, initial_validator_count))
+}
+
+/// Checks that all of the validators have on-boarded by the start of the second eth1 voting
+/// period.
 pub fn verify_validator_onboarding<E: EthSpec>(
     network: LocalNetwork<E>,
     slot_duration: Duration,
     expected_validator_count: usize,
 ) -> impl Future<Item = (), Error = String> {
-    slot_delay(Slot::new(32), slot_duration)
+    slot_delay(
-        .and_then(move |()| verify_validator_count(network, expected_validator_count))
+        Slot::new(E::SlotsPerEth1VotingPeriod::to_u64()),
+        slot_duration,
+    )
+    .and_then(move |()| verify_validator_count(network, expected_validator_count))
 }
 
 /// Checks that the chain has made the first possible finalization.
@@ -42,6 +58,8 @@ fn slot_delay(slots: Slot, slot_duration: Duration) -> impl Future<Item = (), Er
     Delay::new(Instant::now() + duration).map_err(|e| format!("Epoch delay failed: {:?}", e))
 }
 
+/// Verifies that all beacon nodes in the given network have a head state that has a finalized
+/// epoch of `epoch`.
 fn verify_all_finalized_at<E: EthSpec>(
     network: LocalNetwork<E>,
     epoch: Epoch,
@@ -75,6 +93,8 @@ fn verify_all_finalized_at<E: EthSpec>(
         })
 }
 
+/// Verifies that all beacon nodes in the given `network` have a head state that contains
+/// `expected_count` validators.
 fn verify_validator_count<E: EthSpec>(
     network: LocalNetwork<E>,
     expected_count: usize,

tests/beacon_chain_sim/src/local_network.rs

@@ -1,8 +1,7 @@
-use crate::{BeaconNode, ValidatorClient};
 use futures::{Future, IntoFuture};
 use node_test_rig::{
-    environment::RuntimeContext, ClientConfig, LocalValidatorClient, RemoteBeaconNode,
+    environment::RuntimeContext, ClientConfig, LocalBeaconNode, LocalValidatorClient,
-    ValidatorConfig,
+    RemoteBeaconNode, ValidatorConfig,
 };
 use parking_lot::RwLock;
 use std::ops::Deref;
@@ -11,8 +10,8 @@ use types::EthSpec;
 
 pub struct Inner<E: EthSpec> {
     context: RuntimeContext<E>,
-    beacon_nodes: RwLock<Vec<BeaconNode<E>>>,
+    beacon_nodes: RwLock<Vec<LocalBeaconNode<E>>>,
-    validator_clients: RwLock<Vec<ValidatorClient<E>>>,
+    validator_clients: RwLock<Vec<LocalValidatorClient<E>>>,
 }
 
 pub struct LocalNetwork<E: EthSpec> {
@@ -41,7 +40,7 @@ impl<E: EthSpec> LocalNetwork<E> {
         context: RuntimeContext<E>,
         beacon_config: ClientConfig,
     ) -> impl Future<Item = Self, Error = String> {
-        BeaconNode::production(context.service_context("boot_node".into()), beacon_config).map(
+        LocalBeaconNode::production(context.service_context("boot_node".into()), beacon_config).map(
             |beacon_node| Self {
                 inner: Arc::new(Inner {
                     context,
@@ -81,7 +80,7 @@ impl<E: EthSpec> LocalNetwork<E> {
 
         let index = self.beacon_nodes.read().len();
 
-        BeaconNode::production(
+        LocalBeaconNode::production(
             self.context.service_context(format!("node_{}", index)),
             beacon_config,
         )

tests/beacon_chain_sim/src/main.rs

@@ -1,28 +1,57 @@
+//! This crate provides a simluation that creates `n` beacon node and validator clients, each with
+//! `v` validators. A deposit contract is deployed at the start of the simulation using a local
+//! `ganache-cli` instance (you must have `ganache-cli` installed and avaliable on your path). All
+//! beacon nodes independently listen for genesis from the deposit contract, then start operating.
+//!
+//! As the simulation runs, there are checks made to ensure that all components are running
+//! correctly. If any of these checks fail, the simulation will exit immediately.
+//!
+//! By default, the simulation will end as soon as all checks have finished. It may be configured
+//! to run indefinitely by setting `end_after_checks = false`.
+//!
+//! ## Future works
+//!
+//! Presently all the beacon nodes and validator clients all log to stdout. Additionally, the
+//! simulation uses `println` to communicate some info. It might be nice if the nodes logged to
+//! easy-to-find files and stdout only contained info from the simulation.
+//!
+//! It would also be nice to add a CLI using `clap` so that the variables in `main()` can be
+//! changed without a recompile.
+
 mod checks;
 mod local_network;
 
 use eth1_test_rig::GanacheEth1Instance;
-use futures::{stream, Future, Stream};
+use futures::{future, stream, Future, Stream};
 use local_network::LocalNetwork;
 use node_test_rig::{
-    environment::EnvironmentBuilder, testing_client_config, ClientGenesis, LocalBeaconNode,
+    environment::EnvironmentBuilder, testing_client_config, ClientGenesis, ValidatorConfig,
-    LocalValidatorClient, ProductionClient, ValidatorConfig,
 };
 use std::time::{Duration, Instant};
 use tokio::timer::Interval;
 use types::MinimalEthSpec;
 
 pub type E = MinimalEthSpec;
-pub type BeaconNode<E> = LocalBeaconNode<ProductionClient<E>>;
-pub type ValidatorClient<E> = LocalValidatorClient<E>;
 
 fn main() {
     let nodes = 4;
     let validators_per_node = 20;
+    let log_level = "debug";
+    let speed_up_factor = 4;
+    let end_after_checks = true;
 
-    match async_sim(nodes, validators_per_node, 4) {
+    match async_sim(
+        nodes,
+        validators_per_node,
+        speed_up_factor,
+        log_level,
+        end_after_checks,
+    ) {
         Ok(()) => println!("Simulation exited successfully"),
-        Err(e) => eprintln!("Simulation exited with error: {}", e),
+        Err(e) => {
+            eprintln!("Simulation exited with error: {}", e);
+            std::process::exit(1)
+        }
     }
 }
 
@@ -30,9 +59,11 @@ fn async_sim(
     node_count: usize,
     validators_per_node: usize,
     speed_up_factor: u64,
+    log_level: &str,
+    end_after_checks: bool,
 ) -> Result<(), String> {
     let mut env = EnvironmentBuilder::minimal()
-        .async_logger("debug")?
+        .async_logger(log_level)?
         .multi_threaded_tokio_runtime()?
         .build()?;
 
@@ -47,13 +78,18 @@ fn async_sim(
     spec.min_genesis_active_validator_count = 64;
 
     let slot_duration = Duration::from_millis(spec.milliseconds_per_slot);
-    let validator_count = validators_per_node * node_count;
+    let initial_validator_count = spec.min_genesis_active_validator_count as usize;
+    let total_validator_count = validators_per_node * node_count;
     let deposit_amount = env.eth2_config.spec.max_effective_balance;
 
     let context = env.core_context();
     let executor = context.executor.clone();
 
     let future = GanacheEth1Instance::new()
+        /*
+         * Deploy the deposit contract, spawn tasks to keep creating new blocks and deposit
+         * validators.
+         */
         .map(move |ganache_eth1_instance| {
             let deposit_contract = ganache_eth1_instance.deposit_contract;
             let ganache = ganache_eth1_instance.ganache;
@@ -71,7 +107,7 @@ fn async_sim(
 
             // Submit deposits to the deposit contract.
             executor.spawn(
-                stream::unfold(0..validator_count, move |mut iter| {
+                stream::unfold(0..total_validator_count, move |mut iter| {
                     iter.next().map(|i| {
                         println!("Submitting deposit for validator {}...", i);
                         deposit_contract
@@ -84,9 +120,6 @@ fn async_sim(
                 .map_err(|e| eprintln!("Error submitting deposit: {}", e)),
             );
 
-            (deposit_contract_address, eth1_endpoint)
-        })
-        .map(move |(deposit_contract_address, eth1_endpoint)| {
             let mut beacon_config = testing_client_config();
 
             beacon_config.genesis = ClientGenesis::DepositContract;
@@ -100,10 +133,16 @@ fn async_sim(
 
             beacon_config
         })
+        /*
+         * Create a new `LocalNetwork` with one beacon node.
+         */
         .and_then(move |beacon_config| {
             LocalNetwork::new(context, beacon_config.clone())
                 .map(|network| (network, beacon_config))
         })
+        /*
+         * One by one, add beacon nodes to the network.
+         */
         .and_then(move |(network, beacon_config)| {
             let network_1 = network.clone();
 
@@ -117,9 +156,20 @@ fn async_sim(
             .collect()
             .map(|_| network)
         })
+        /*
+         * One by one, add validator clients to the network. Each validator client is attached to
+         * a single corresponding beacon node.
+         */
         .and_then(move |network| {
             let network_1 = network.clone();
 
+            // Note: presently the validator client future will only resolve once genesis time
+            // occurs. This is great for this scenario, but likely to change in the future.
+            //
+            // If the validator client future behaviour changes, we would need to add a new future
+            // that delays until genesis. Otherwise, all of the checks that start in the next
+            // future will start too early.
+
             stream::unfold(0..node_count, move |mut iter| {
                 iter.next().map(|i| {
                     let indices = (i * validators_per_node..(i + 1) * validators_per_node)
@@ -133,15 +183,46 @@ fn async_sim(
             .collect()
             .map(|_| network)
         })
+        /*
+         * Start the processes that will run checks on the network as it runs.
+         */
         .and_then(move |network| {
-            checks::verify_first_finalization(network.clone(), slot_duration)
+            // The `final_future` either completes immediately or never completes, depending on the value
+            // of `end_after_checks`.
+            let final_future: Box<dyn Future<Item = (), Error = String> + Send> =
+                if end_after_checks {
+                    Box::new(future::ok(()).map_err(|()| "".to_string()))
+                } else {
+                    Box::new(future::empty().map_err(|()| "".to_string()))
+                };
+
+            future::ok(())
+                // Check that the chain finalizes at the first given opportunity.
+                .join(checks::verify_first_finalization(
+                    network.clone(),
+                    slot_duration,
+                ))
+                // Check that the chain starts with the expected validator count.
+                .join(checks::verify_initial_validator_count(
+                    network.clone(),
+                    slot_duration,
+                    initial_validator_count,
+                ))
+                // Check that validators greater than `spec.min_genesis_active_validator_count` are
+                // onboarded at the first possible opportunity.
                 .join(checks::verify_validator_onboarding(
                     network.clone(),
                     slot_duration,
-                    validator_count,
+                    total_validator_count,
                 ))
+                // End now or run forever, depending on the `end_after_checks` flag.
+                .join(final_future)
                 .map(|_| network)
         })
+        /*
+         * End the simulation by dropping the network. This will kill all running beacon nodes and
+         * validator clients.
+         */
         .map(|network| {
             println!(
                 "Simulation complete. Finished with {} beacon nodes and {} validator clients",

tests/node_test_rig/src/lib.rs

@@ -2,7 +2,7 @@
 //!
 //! Intended to be used for testing and simulation purposes. Not for production.
 
-use beacon_node::{beacon_chain::BeaconChainTypes, Client, ProductionBeaconNode};
+use beacon_node::ProductionBeaconNode;
 use environment::RuntimeContext;
 use futures::Future;
 use std::path::PathBuf;
@@ -20,12 +20,12 @@ pub use validator_client::Config as ValidatorConfig;
 /// is _local_ to this process).
 ///
 /// Intended for use in testing and simulation. Not for production.
-pub struct LocalBeaconNode<T> {
+pub struct LocalBeaconNode<E: EthSpec> {
-    pub client: T,
+    pub client: ProductionClient<E>,
     pub datadir: TempDir,
 }
 
-impl<E: EthSpec> LocalBeaconNode<ProductionClient<E>> {
+impl<E: EthSpec> LocalBeaconNode<E> {
     /// Starts a new, production beacon node on the tokio runtime in the given `context`.
     ///
     /// The node created is using the same types as the node we use in production.
@@ -47,10 +47,10 @@ impl<E: EthSpec> LocalBeaconNode<ProductionClient<E>> {
     }
 }
 
-impl<T: BeaconChainTypes> LocalBeaconNode<Client<T>> {
+impl<E: EthSpec> LocalBeaconNode<E> {
     /// Returns a `RemoteBeaconNode` that can connect to `self`. Useful for testing the node as if
     /// it were external this process.
-    pub fn remote_node(&self) -> Result<RemoteBeaconNode<T::EthSpec>, String> {
+    pub fn remote_node(&self) -> Result<RemoteBeaconNode<E>, String> {
         let socket_addr = self
             .client
             .http_listen_addr()