Fix race condition in VC block proposal service (#1282)

Closes #918
Closes #923

testing/simulator/src/checks.rs

@@ -126,3 +126,23 @@ async fn verify_validator_count<E: EthSpec>(
         Ok(())
     }
 }
+
+/// Verifies that there's been a block produced at every slot up to and including `slot`.
+pub async fn verify_full_block_production_up_to<E: EthSpec>(
+    network: LocalNetwork<E>,
+    slot: Slot,
+    slot_duration: Duration,
+) -> Result<(), String> {
+    slot_delay(slot, slot_duration).await;
+    let beacon_nodes = network.beacon_nodes.read();
+    let beacon_chain = beacon_nodes[0].client.beacon_chain().unwrap();
+    let chain_dump = beacon_chain.chain_dump().unwrap();
+    if chain_dump.len() != slot.as_usize() + 1 {
+        return Err(format!(
+            "There wasn't a block produced at every slot, got: {}, expected: {}",
+            chain_dump.len(),
+            slot.as_usize() + 1
+        ));
+    }
+    Ok(())
+}

testing/simulator/src/eth1_sim.rs

@@ -9,6 +9,7 @@ use node_test_rig::{
 use rayon::prelude::*;
 use std::net::{IpAddr, Ipv4Addr};
 use std::time::Duration;
+use types::{Epoch, EthSpec, MainnetEthSpec};
 
 pub fn run_eth1_sim(matches: &ArgMatches) -> Result<(), String> {
     let node_count = value_t!(matches, "nodes", usize).expect("missing nodes default");
@@ -146,9 +147,15 @@ pub fn run_eth1_sim(matches: &ArgMatches) -> Result<(), String> {
          * tests start at the right time. Whilst this is works well for now, it's subject to
          * breakage by changes to the VC.
          */
-        let (finalization, validator_count, onboarding) = futures::join!(
+        let (finalization, block_prod, validator_count, onboarding) = futures::join!(
             // Check that the chain finalizes at the first given opportunity.
             checks::verify_first_finalization(network.clone(), slot_duration),
+            // Check that a block is produced at every slot.
+            checks::verify_full_block_production_up_to(
+                network.clone(),
+                Epoch::new(4).start_slot(MainnetEthSpec::slots_per_epoch()),
+                slot_duration,
+            ),
             // Check that the chain starts with the expected validator count.
             checks::verify_initial_validator_count(
                 network.clone(),
@@ -164,6 +171,7 @@ pub fn run_eth1_sim(matches: &ArgMatches) -> Result<(), String> {
             )
         );
 
+        block_prod?;
         finalization?;
         validator_count?;
         onboarding?;

testing/simulator/src/local_network.rs

@@ -11,9 +11,9 @@ const BOOTNODE_PORT: u16 = 42424;
 
 /// Helper struct to reduce `Arc` usage.
 pub struct Inner<E: EthSpec> {
-    context: RuntimeContext<E>,
-    beacon_nodes: RwLock<Vec<LocalBeaconNode<E>>>,
-    validator_clients: RwLock<Vec<LocalValidatorClient<E>>>,
+    pub context: RuntimeContext<E>,
+    pub beacon_nodes: RwLock<Vec<LocalBeaconNode<E>>>,
+    pub validator_clients: RwLock<Vec<LocalValidatorClient<E>>>,
 }
 
 /// Represents a set of interconnected `LocalBeaconNode` and `LocalValidatorClient`.

testing/simulator/src/no_eth1_sim.rs

@@ -9,6 +9,7 @@ use rayon::prelude::*;
 use std::net::{IpAddr, Ipv4Addr};
 use std::time::{Duration, SystemTime, UNIX_EPOCH};
 use tokio::time::{delay_until, Instant};
+use types::{Epoch, EthSpec, MainnetEthSpec};
 
 pub fn run_no_eth1_sim(matches: &ArgMatches) -> Result<(), String> {
     let node_count = value_t!(matches, "nodes", usize).expect("missing nodes default");
@@ -121,8 +122,18 @@ pub fn run_no_eth1_sim(matches: &ArgMatches) -> Result<(), String> {
         let checks_fut = async {
             delay_until(genesis_instant).await;
 
-            // Check that the chain finalizes at the first given opportunity.
-            checks::verify_first_finalization(network.clone(), slot_duration).await?;
+            let (finalization, block_prod) = futures::join!(
+                // Check that the chain finalizes at the first given opportunity.
+                checks::verify_first_finalization(network.clone(), slot_duration),
+                // Check that a block is produced at every slot.
+                checks::verify_full_block_production_up_to(
+                    network.clone(),
+                    Epoch::new(4).start_slot(MainnetEthSpec::slots_per_epoch()),
+                    slot_duration,
+                )
+            );
+            finalization?;
+            block_prod?;
 
             Ok::<(), String>(())
         };