Fix todos and tests

validator_client/src/beacon_node_fallback.rs

@@ -23,7 +23,7 @@ use std::marker::PhantomData;
 use std::sync::Arc;
 use std::time::{Duration, Instant};
 use tokio::{sync::RwLock, time::sleep};
-use types::{ChainSpec, Config as ConfigSpec, EthSpec};
+use types::{ChainSpec, Config as ConfigSpec, EthSpec, Slot};
 
 /// Message emitted when the VC detects the BN is using a different spec.
 const UPDATE_REQUIRED_LOG_HINT: &str = "this VC or the remote BN may need updating";
@@ -37,6 +37,10 @@ const UPDATE_REQUIRED_LOG_HINT: &str = "this VC or the remote BN may need updati
 /// having the correct nodes up and running prior to the start of the slot.
 const SLOT_LOOKAHEAD: Duration = Duration::from_secs(2);
 
+/// If the beacon node slot_clock is within 1 slot, this is deemed acceptable. Otherwise the node
+/// will be marked as CandidateError::TimeDiscrepancy.
+const FUTURE_SLOT_TOLERANCE: Slot = Slot::new(1);
+
 // Configuration for the Beacon Node fallback.
 #[derive(Copy, Clone, Debug, Default, Serialize, Deserialize)]
 pub struct Config {
@@ -213,6 +217,13 @@ impl<E: EthSpec> CandidateBeaconNode<E> {
         if let Some(slot_clock) = slot_clock {
             match check_node_health(&self.beacon_node, log).await {
                 Ok((head, is_optimistic, el_offline)) => {
+                    let slot_clock_head = slot_clock.now().ok_or(CandidateError::Uninitialized)?;
+
+                    if head > slot_clock_head + FUTURE_SLOT_TOLERANCE {
+                        return Err(CandidateError::TimeDiscrepancy);
+                    }
+                    let sync_distance = slot_clock_head.saturating_sub(head);
+
                     // Currently ExecutionEngineHealth is solely determined by online status.
                     let execution_status = if el_offline {
                         ExecutionEngineHealth::Unhealthy
@@ -228,15 +239,13 @@ impl<E: EthSpec> CandidateBeaconNode<E> {
 
                     let new_health = BeaconNodeHealth::from_status(
                         self.id,
+                        sync_distance,
                         head,
                         optimistic_status,
                         execution_status,
                         distance_tiers,
-                        slot_clock,
                     );
 
-                    // TODO(mac): Set metric here.
-
                     *self.health.write() = Ok(new_health);
                     Ok(())
                 }

validator_client/src/beacon_node_health.rs

@@ -1,18 +1,17 @@
 use crate::beacon_node_fallback::Config;
 use serde_derive::{Deserialize, Serialize};
-use slot_clock::SlotClock;
 use std::cmp::Ordering;
 use std::fmt::{Debug, Display, Formatter};
 use types::Slot;
 
-// Sync distances between 0 and DEFAULT_SYNC_TOLERANCE are considered `synced`.
-// Sync distance tiers are determined by the different modifiers.
-//
-// The default range is the following:
-// Synced: 0..=8
-// Small: 9..=16
-// Medium: 17..=64
-// Large: 65..
+/// Sync distances between 0 and DEFAULT_SYNC_TOLERANCE are considered `synced`.
+/// Sync distance tiers are determined by the different modifiers.
+///
+/// The default range is the following:
+/// Synced: 0..=8
+/// Small: 9..=16
+/// Medium: 17..=64
+/// Large: 65..
 const DEFAULT_SYNC_TOLERANCE: Slot = Slot::new(8);
 const DEFAULT_SMALL_SYNC_DISTANCE_MODIFIER: Slot = Slot::new(8);
 const DEFAULT_MEDIUM_SYNC_DISTANCE_MODIFIER: Slot = Slot::new(48);
@@ -195,15 +194,14 @@ impl PartialOrd for BeaconNodeHealth {
 }
 
 impl BeaconNodeHealth {
-    pub fn from_status<T: SlotClock>(
+    pub fn from_status(
         id: usize,
+        sync_distance: Slot,
         head: Slot,
         optimistic_status: IsOptimistic,
         execution_status: ExecutionEngineHealth,
         distance_tiers: &BeaconNodeSyncDistanceTiers,
-        slot_clock: &T,
     ) -> Self {
-        let sync_distance = BeaconNodeHealth::compute_sync_distance(head, slot_clock);
         let health_tier = BeaconNodeHealth::compute_health_tier(
             sync_distance,
             optimistic_status,
@@ -228,14 +226,6 @@ impl BeaconNodeHealth {
         self.health_tier
     }
 
-    fn compute_sync_distance<T: SlotClock>(head: Slot, slot_clock: &T) -> SyncDistance {
-        // TODO(mac) May be worth distinguishing between nodes that are ahead of the `slot_clock`.
-        slot_clock
-            .now()
-            .map(|head_slot| head_slot.saturating_sub(head))
-            .unwrap_or(Slot::max_value())
-    }
-
     fn compute_health_tier(
         sync_distance: SyncDistance,
         optimistic_status: IsOptimistic,
@@ -306,20 +296,13 @@ mod tests {
         SyncDistanceTier,
     };
     use crate::beacon_node_fallback::Config;
-    use slot_clock::{SlotClock, TestingSlotClock};
-    use std::time::Duration;
     use types::Slot;
 
     #[test]
     fn all_possible_health_tiers() {
-        let current_head = Slot::new(64);
-
         let config = Config::default();
         let beacon_node_sync_distance_tiers = BeaconNodeSyncDistanceTiers::from_config(&config);
 
-        let slot_clock =
-            TestingSlotClock::new(current_head, Duration::from_secs(0), Duration::from_secs(1));
-
         let mut health_vec = vec![];
 
         for head_slot in 0..=64 {
@@ -327,11 +310,11 @@ mod tests {
                 for ee_health in &[Healthy, Unhealthy] {
                     let health = BeaconNodeHealth::from_status(
                         0,
+                        Slot::new(0),
                         Slot::new(head_slot),
                         *optimistic_status,
                         *ee_health,
                         &beacon_node_sync_distance_tiers,
-                        &slot_clock,
                     );
                     health_vec.push(health);
                 }

validator_client/src/http_api/tests.rs

@@ -110,6 +110,7 @@ impl ApiTester {
         let context = Arc::new(Context {
             task_executor: test_runtime.task_executor.clone(),
             api_secret,
+            block_service: None,
             validator_dir: Some(validator_dir.path().into()),
             secrets_dir: Some(secrets_dir.path().into()),
             validator_store: Some(validator_store.clone()),

validator_client/src/notifier.rs

@@ -84,7 +84,6 @@ async fn notify<T: SlotClock + 'static, E: EthSpec>(
         set_gauge(&http_metrics::metrics::ETH2_FALLBACK_CONNECTED, 0);
     }
 
-    // TODO(mac) Store all connected node info into metrics.
     for info in candidate_info {
         if let Some(health) = info.health {
             debug!(