From 7bfe02be1cde9c08bb24b3b8ed5b0c5689d96327 Mon Sep 17 00:00:00 2001
From: Paul Hauner <paul@paulhauner.com>
Date: Thu, 29 Aug 2019 12:46:18 +1000
Subject: [PATCH 1/4] Refactor slot clock.

---
 eth2/utils/slot_clock/src/lib.rs              |  19 +-
 eth2/utils/slot_clock/src/metrics.rs          |   7 +-
 .../slot_clock/src/system_time_slot_clock.rs  | 191 +++++++-----------
 .../slot_clock/src/testing_slot_clock.rs      |  37 ++--
 4 files changed, 98 insertions(+), 156 deletions(-)

diff --git a/eth2/utils/slot_clock/src/lib.rs b/eth2/utils/slot_clock/src/lib.rs
index 871743c9e6..988f3d322c 100644
--- a/eth2/utils/slot_clock/src/lib.rs
+++ b/eth2/utils/slot_clock/src/lib.rs
@@ -5,24 +5,19 @@ mod metrics;
 mod system_time_slot_clock;
 mod testing_slot_clock;
 
-use std::time::Duration;
+use std::time::{Duration, Instant};
 
-pub use crate::system_time_slot_clock::{Error as SystemTimeSlotClockError, SystemTimeSlotClock};
-pub use crate::testing_slot_clock::{Error as TestingSlotClockError, TestingSlotClock};
+pub use crate::system_time_slot_clock::SystemTimeSlotClock;
+pub use crate::testing_slot_clock::TestingSlotClock;
 pub use metrics::scrape_for_metrics;
 pub use types::Slot;
 
 pub trait SlotClock: Send + Sync + Sized {
-    type Error;
+    fn new(genesis_slot: Slot, genesis: Instant, slot_duration: Duration) -> Self;
 
-    /// Create a new `SlotClock`.
-    ///
-    /// Returns an Error if `slot_duration_seconds == 0`.
-    fn new(genesis_slot: Slot, genesis_seconds: u64, slot_duration_seconds: u64) -> Self;
+    fn present_slot(&self) -> Option<Slot>;
 
-    fn present_slot(&self) -> Result<Option<Slot>, Self::Error>;
+    fn duration_to_next_slot(&self) -> Option<Duration>;
 
-    fn duration_to_next_slot(&self) -> Result<Option<Duration>, Self::Error>;
-
-    fn slot_duration_millis(&self) -> u64;
+    fn slot_duration(&self) -> Duration;
 }
diff --git a/eth2/utils/slot_clock/src/metrics.rs b/eth2/utils/slot_clock/src/metrics.rs
index e0d3923e00..1abd93c488 100644
--- a/eth2/utils/slot_clock/src/metrics.rs
+++ b/eth2/utils/slot_clock/src/metrics.rs
@@ -18,7 +18,7 @@ lazy_static! {
 /// Update the global metrics `DEFAULT_REGISTRY` with info from the slot clock.
 pub fn scrape_for_metrics<T: EthSpec, U: SlotClock>(clock: &U) {
     let present_slot = match clock.present_slot() {
-        Ok(Some(slot)) => slot,
+        Some(slot) => slot,
         _ => Slot::new(0),
     };
 
@@ -28,5 +28,8 @@ pub fn scrape_for_metrics<T: EthSpec, U: SlotClock>(clock: &U) {
         present_slot.epoch(T::slots_per_epoch()).as_u64() as i64,
     );
     set_gauge(&SLOTS_PER_EPOCH, T::slots_per_epoch() as i64);
-    set_gauge(&MILLISECONDS_PER_SLOT, clock.slot_duration_millis() as i64);
+    set_gauge(
+        &MILLISECONDS_PER_SLOT,
+        clock.slot_duration().as_millis() as i64,
+    );
 }
diff --git a/eth2/utils/slot_clock/src/system_time_slot_clock.rs b/eth2/utils/slot_clock/src/system_time_slot_clock.rs
index c493a8be83..88c9c0e63e 100644
--- a/eth2/utils/slot_clock/src/system_time_slot_clock.rs
+++ b/eth2/utils/slot_clock/src/system_time_slot_clock.rs
@@ -1,99 +1,68 @@
 use super::SlotClock;
-use std::time::{Duration, SystemTime};
+use std::time::{Duration, Instant};
 use types::Slot;
 
 pub use std::time::SystemTimeError;
 
-#[derive(Debug, PartialEq)]
-pub enum Error {
-    SlotDurationIsZero,
-    SystemTimeError(String),
-}
-
 /// Determines the present slot based upon the present system time.
 #[derive(Clone)]
 pub struct SystemTimeSlotClock {
     genesis_slot: Slot,
-    genesis_seconds: u64,
-    slot_duration_seconds: u64,
+    genesis: Instant,
+    slot_duration: Duration,
 }
 
 impl SlotClock for SystemTimeSlotClock {
-    type Error = Error;
+    fn new(genesis_slot: Slot, genesis: Instant, slot_duration: Duration) -> Self {
+        if slot_duration.as_millis() == 0 {
+            panic!("SystemTimeSlotClock cannot have a < 1ms slot duration.");
+        }
 
-    /// Create a new `SystemTimeSlotClock`.
-    ///
-    /// Returns an Error if `slot_duration_seconds == 0`.
-    fn new(genesis_slot: Slot, genesis_seconds: u64, slot_duration_seconds: u64) -> Self {
         Self {
             genesis_slot,
-            genesis_seconds,
-            slot_duration_seconds,
+            genesis,
+            slot_duration,
         }
     }
 
-    fn present_slot(&self) -> Result<Option<Slot>, Error> {
-        if self.slot_duration_seconds == 0 {
-            return Err(Error::SlotDurationIsZero);
-        }
+    fn present_slot(&self) -> Option<Slot> {
+        let now = Instant::now();
 
-        let syslot_time = SystemTime::now();
-        let duration_since_epoch = syslot_time.duration_since(SystemTime::UNIX_EPOCH)?;
-        let duration_since_genesis =
-            duration_since_epoch.checked_sub(Duration::from_secs(self.genesis_seconds));
-
-        match duration_since_genesis {
-            None => Ok(None),
-            Some(d) => Ok(slot_from_duration(self.slot_duration_seconds, d)
-                .and_then(|s| Some(s + self.genesis_slot))),
+        if now < self.genesis {
+            None
+        } else {
+            let slot = Slot::from(
+                (now.duration_since(self.genesis).as_millis() / self.slot_duration.as_millis())
+                    as u64,
+            );
+            Some(slot + self.genesis_slot)
         }
     }
 
-    fn duration_to_next_slot(&self) -> Result<Option<Duration>, Error> {
-        duration_to_next_slot(self.genesis_seconds, self.slot_duration_seconds)
+    fn duration_to_next_slot(&self) -> Option<Duration> {
+        let now = Instant::now();
+        if now < self.genesis {
+            None
+        } else {
+            let duration_since_genesis = now - self.genesis;
+            let millis_since_genesis = duration_since_genesis.as_millis();
+            let millis_per_slot = self.slot_duration.as_millis();
+
+            let current_slot = millis_since_genesis / millis_per_slot;
+            let next_slot = current_slot + 1;
+
+            let next_slot =
+                self.genesis + Duration::from_millis((next_slot * millis_per_slot) as u64);
+
+            Some(next_slot.duration_since(now))
+        }
     }
 
-    fn slot_duration_millis(&self) -> u64 {
-        self.slot_duration_seconds * 1000
+    fn slot_duration(&self) -> Duration {
+        self.slot_duration
     }
 }
 
-impl From<SystemTimeError> for Error {
-    fn from(e: SystemTimeError) -> Error {
-        Error::SystemTimeError(format!("{:?}", e))
-    }
-}
-
-fn slot_from_duration(slot_duration_seconds: u64, duration: Duration) -> Option<Slot> {
-    Some(Slot::new(
-        duration.as_secs().checked_div(slot_duration_seconds)?,
-    ))
-}
-// calculate the duration to the next slot
-fn duration_to_next_slot(
-    genesis_time: u64,
-    seconds_per_slot: u64,
-) -> Result<Option<Duration>, Error> {
-    let now = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH)?;
-    let genesis_time = Duration::from_secs(genesis_time);
-
-    if now < genesis_time {
-        return Ok(None);
-    }
-
-    let since_genesis = now - genesis_time;
-
-    let elapsed_slots = since_genesis.as_secs() / seconds_per_slot;
-
-    let next_slot_start_seconds = (elapsed_slots + 1)
-        .checked_mul(seconds_per_slot)
-        .expect("Next slot time should not overflow u64");
-
-    let time_to_next_slot = Duration::from_secs(next_slot_start_seconds) - since_genesis;
-
-    Ok(Some(time_to_next_slot))
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -104,71 +73,51 @@ mod tests {
      */
     #[test]
     fn test_slot_now() {
-        let slot_time = 100;
         let genesis_slot = Slot::new(0);
 
-        let now = SystemTime::now();
-        let since_epoch = now.duration_since(SystemTime::UNIX_EPOCH).unwrap();
+        let prior_genesis =
+            |seconds_prior: u64| Instant::now() - Duration::from_secs(seconds_prior);
 
-        let genesis = since_epoch.as_secs() - slot_time * 89;
+        let clock =
+            SystemTimeSlotClock::new(genesis_slot, prior_genesis(0), Duration::from_secs(1));
+        assert_eq!(clock.present_slot(), Some(Slot::new(0)));
 
-        let clock = SystemTimeSlotClock {
+        let clock =
+            SystemTimeSlotClock::new(genesis_slot, prior_genesis(5), Duration::from_secs(1));
+        assert_eq!(clock.present_slot(), Some(Slot::new(5)));
+
+        let clock = SystemTimeSlotClock::new(
             genesis_slot,
-            genesis_seconds: genesis,
-            slot_duration_seconds: slot_time,
-        };
-        assert_eq!(clock.present_slot().unwrap(), Some(Slot::new(89)));
+            Instant::now() - Duration::from_millis(500),
+            Duration::from_secs(1),
+        );
+        assert_eq!(clock.present_slot(), Some(Slot::new(0)));
+        assert!(clock.duration_to_next_slot().unwrap() < Duration::from_millis(500));
 
-        let clock = SystemTimeSlotClock {
+        let clock = SystemTimeSlotClock::new(
             genesis_slot,
-            genesis_seconds: since_epoch.as_secs(),
-            slot_duration_seconds: slot_time,
-        };
-        assert_eq!(clock.present_slot().unwrap(), Some(Slot::new(0)));
-
-        let clock = SystemTimeSlotClock {
-            genesis_slot,
-            genesis_seconds: since_epoch.as_secs() - slot_time * 42 - 5,
-            slot_duration_seconds: slot_time,
-        };
-        assert_eq!(clock.present_slot().unwrap(), Some(Slot::new(42)));
+            Instant::now() - Duration::from_millis(1_500),
+            Duration::from_secs(1),
+        );
+        assert_eq!(clock.present_slot(), Some(Slot::new(1)));
+        assert!(clock.duration_to_next_slot().unwrap() < Duration::from_millis(500));
     }
 
     #[test]
-    fn test_slot_from_duration() {
-        let slot_time = 100;
-
-        assert_eq!(
-            slot_from_duration(slot_time, Duration::from_secs(0)),
-            Some(Slot::new(0))
-        );
-        assert_eq!(
-            slot_from_duration(slot_time, Duration::from_secs(10)),
-            Some(Slot::new(0))
-        );
-        assert_eq!(
-            slot_from_duration(slot_time, Duration::from_secs(100)),
-            Some(Slot::new(1))
-        );
-        assert_eq!(
-            slot_from_duration(slot_time, Duration::from_secs(101)),
-            Some(Slot::new(1))
-        );
-        assert_eq!(
-            slot_from_duration(slot_time, Duration::from_secs(1000)),
-            Some(Slot::new(10))
-        );
+    #[should_panic]
+    fn zero_seconds() {
+        SystemTimeSlotClock::new(Slot::new(0), Instant::now(), Duration::from_secs(0));
     }
 
     #[test]
-    fn test_slot_from_duration_slot_time_zero() {
-        let slot_time = 0;
+    #[should_panic]
+    fn zero_millis() {
+        SystemTimeSlotClock::new(Slot::new(0), Instant::now(), Duration::from_millis(0));
+    }
 
-        assert_eq!(slot_from_duration(slot_time, Duration::from_secs(0)), None);
-        assert_eq!(slot_from_duration(slot_time, Duration::from_secs(10)), None);
-        assert_eq!(
-            slot_from_duration(slot_time, Duration::from_secs(1000)),
-            None
-        );
+    #[test]
+    #[should_panic]
+    fn less_than_one_millis() {
+        SystemTimeSlotClock::new(Slot::new(0), Instant::now(), Duration::from_nanos(999));
     }
 }
diff --git a/eth2/utils/slot_clock/src/testing_slot_clock.rs b/eth2/utils/slot_clock/src/testing_slot_clock.rs
index f741d3b87a..0b65b15694 100644
--- a/eth2/utils/slot_clock/src/testing_slot_clock.rs
+++ b/eth2/utils/slot_clock/src/testing_slot_clock.rs
@@ -1,12 +1,11 @@
 use super::SlotClock;
 use std::sync::RwLock;
-use std::time::Duration;
+use std::time::{Duration, Instant};
 use types::Slot;
 
-#[derive(Debug, PartialEq)]
-pub enum Error {}
-
-/// Determines the present slot based upon the present system time.
+/// A slot clock where the slot is manually set instead of being determined by the system time.
+///
+/// Useful for testing scenarios.
 pub struct TestingSlotClock {
     slot: RwLock<Slot>,
 }
@@ -17,32 +16,30 @@ impl TestingSlotClock {
     }
 
     pub fn advance_slot(&self) {
-        self.set_slot(self.present_slot().unwrap().unwrap().as_u64() + 1)
+        self.set_slot(self.present_slot().unwrap().as_u64() + 1)
     }
 }
 
 impl SlotClock for TestingSlotClock {
-    type Error = Error;
-
-    /// Create a new `TestingSlotClock` at `genesis_slot`.
-    fn new(genesis_slot: Slot, _genesis_seconds: u64, _slot_duration_seconds: u64) -> Self {
+    fn new(genesis_slot: Slot, _genesis: Instant, _slot_duration: Duration) -> Self {
         TestingSlotClock {
             slot: RwLock::new(genesis_slot),
         }
     }
 
-    fn present_slot(&self) -> Result<Option<Slot>, Error> {
+    fn present_slot(&self) -> Option<Slot> {
         let slot = *self.slot.read().expect("TestingSlotClock poisoned.");
-        Ok(Some(slot))
+        Some(slot)
     }
 
     /// Always returns a duration of 1 second.
-    fn duration_to_next_slot(&self) -> Result<Option<Duration>, Error> {
-        Ok(Some(Duration::from_secs(1)))
+    fn duration_to_next_slot(&self) -> Option<Duration> {
+        Some(Duration::from_secs(1))
     }
 
-    fn slot_duration_millis(&self) -> u64 {
-        0
+    /// Always returns a slot duration of 0 seconds.
+    fn slot_duration(&self) -> Duration {
+        Duration::from_secs(0)
     }
 }
 
@@ -52,11 +49,9 @@ mod tests {
 
     #[test]
     fn test_slot_now() {
-        let null = 0;
-
-        let clock = TestingSlotClock::new(Slot::new(10), null, null);
-        assert_eq!(clock.present_slot(), Ok(Some(Slot::new(10))));
+        let clock = TestingSlotClock::new(Slot::new(10), Instant::now(), Duration::from_secs(0));
+        assert_eq!(clock.present_slot(), Some(Slot::new(10)));
         clock.set_slot(123);
-        assert_eq!(clock.present_slot(), Ok(Some(Slot::new(123))));
+        assert_eq!(clock.present_slot(), Some(Slot::new(123)));
     }
 }

From bcd53a8b10f46488eb41eafb110cf8a5576de446 Mon Sep 17 00:00:00 2001
From: Paul Hauner <paul@paulhauner.com>
Date: Thu, 29 Aug 2019 13:25:55 +1000
Subject: [PATCH 2/4] Migrate codebase across to new SlotClock API

---
 beacon_node/beacon_chain/src/beacon_chain.rs | 61 ++++++++++----------
 beacon_node/beacon_chain/src/errors.rs       |  1 +
 beacon_node/beacon_chain/src/test_utils.rs   | 18 ++++--
 beacon_node/network/src/sync/simple_sync.rs  |  2 +-
 beacon_node/rest_api/src/helpers.rs          |  4 +-
 eth2/utils/slot_clock/src/lib.rs             | 20 ++++++-
 validator_client/src/error.rs                |  7 ---
 validator_client/src/service.rs              | 27 ++++-----
 8 files changed, 78 insertions(+), 62 deletions(-)

diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs
index 0fc71fe7b9..9283d22310 100644
--- a/beacon_node/beacon_chain/src/beacon_chain.rs
+++ b/beacon_node/beacon_chain/src/beacon_chain.rs
@@ -23,6 +23,7 @@ use state_processing::{
     per_slot_processing, BlockProcessingError,
 };
 use std::sync::Arc;
+use std::time::Duration;
 use store::iter::{BlockRootsIterator, StateRootsIterator};
 use store::{Error as DBError, Store};
 use tree_hash::TreeHash;
@@ -173,11 +174,12 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             Ok(Some(p)) => p,
         };
 
-        let slot_clock = T::SlotClock::new(
+        let slot_clock = T::SlotClock::from_eth2_genesis(
             spec.genesis_slot,
             p.state.genesis_time,
-            spec.seconds_per_slot,
-        );
+            Duration::from_secs(spec.seconds_per_slot),
+        )
+        .ok_or_else(|| Error::SlotClockDidNotStart)?;
 
         let last_finalized_root = p.canonical_head.beacon_state.finalized_checkpoint.root;
         let last_finalized_block = &p.canonical_head.beacon_block;
@@ -216,6 +218,20 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         Ok(())
     }
 
+    /// Reads the slot clock, returns `Err` if the slot is unavailable.
+    ///
+    /// The slot might be unavailable due to an error with the system clock, or if the present time
+    /// is before genesis (i.e., a negative slot).
+    ///
+    /// This is distinct to `present_slot`, which simply reads the latest state. If a
+    /// call to `read_slot_clock` results in a higher slot than a call to `present_slot`,
+    /// `self.state` should undergo per slot processing.
+    pub fn present_slot(&self) -> Result<Slot, Error> {
+        self.slot_clock
+            .present_slot()
+            .ok_or_else(|| Error::UnableToReadSlot)
+    }
+
     /// Returns the beacon block body for each beacon block root in `roots`.
     ///
     /// Fails if any root in `roots` does not have a corresponding block.
@@ -326,10 +342,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     pub fn catchup_state(&self) -> Result<(), Error> {
         let spec = &self.spec;
 
-        let present_slot = match self.slot_clock.present_slot() {
-            Ok(Some(slot)) => slot,
-            _ => return Err(Error::UnableToReadSlot),
-        };
+        let present_slot = self
+            .slot_clock
+            .present_slot()
+            .ok_or_else(|| Error::UnableToReadSlot)?;
 
         if self.state.read().slot < present_slot {
             let mut state = self.state.write();
@@ -369,26 +385,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         None
     }
 
-    /// Reads the slot clock, returns `None` if the slot is unavailable.
-    ///
-    /// The slot might be unavailable due to an error with the system clock, or if the present time
-    /// is before genesis (i.e., a negative slot).
-    ///
-    /// This is distinct to `present_slot`, which simply reads the latest state. If a
-    /// call to `read_slot_clock` results in a higher slot than a call to `present_slot`,
-    /// `self.state` should undergo per slot processing.
-    pub fn read_slot_clock(&self) -> Option<Slot> {
-        match self.slot_clock.present_slot() {
-            Ok(Some(some_slot)) => Some(some_slot),
-            Ok(None) => None,
-            _ => None,
-        }
-    }
-
     /// Reads the slot clock (see `self.read_slot_clock()` and returns the number of slots since
     /// genesis.
     pub fn slots_since_genesis(&self) -> Option<SlotHeight> {
-        let now = self.read_slot_clock()?;
+        let now = self.slot_clock.present_slot()?;
         let genesis_slot = self.spec.genesis_slot;
 
         if now < genesis_slot {
@@ -398,6 +398,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         }
     }
 
+    /*
     /// Returns slot of the present state.
     ///
     /// This is distinct to `read_slot_clock`, which reads from the actual system clock. If
@@ -406,6 +407,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     pub fn present_slot(&self) -> Slot {
         self.state.read().slot
     }
+    */
 
     /// Returns the block proposer for a given slot.
     ///
@@ -840,7 +842,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         }
 
         let present_slot = self
-            .read_slot_clock()
+            .slot_clock
+            .present_slot()
             .ok_or_else(|| Error::UnableToReadSlot)?;
 
         if block.slot > present_slot {
@@ -1004,7 +1007,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     ) -> Result<(BeaconBlock<T::EthSpec>, BeaconState<T::EthSpec>), BlockProductionError> {
         let state = self.state.read().clone();
         let slot = self
-            .read_slot_clock()
+            .slot_clock
+            .present_slot()
             .ok_or_else(|| BlockProductionError::UnableToReadSlot)?;
 
         self.produce_block_on_state(state, slot, randao_reveal)
@@ -1181,10 +1185,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         *self.state.write() = {
             let mut state = self.canonical_head.read().beacon_state.clone();
 
-            let present_slot = match self.slot_clock.present_slot() {
-                Ok(Some(slot)) => slot,
-                _ => return Err(Error::UnableToReadSlot),
-            };
+            let present_slot = self.present_slot()?;
 
             // If required, transition the new state to the present slot.
             for _ in state.slot.as_u64()..present_slot.as_u64() {
diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs
index 22df90397e..8541a0d0b3 100644
--- a/beacon_node/beacon_chain/src/errors.rs
+++ b/beacon_node/beacon_chain/src/errors.rs
@@ -25,6 +25,7 @@ pub enum BeaconChainError {
         previous_epoch: Epoch,
         new_epoch: Epoch,
     },
+    SlotClockDidNotStart,
     UnableToFindTargetRoot(Slot),
     BeaconStateError(BeaconStateError),
     DBInconsistent(String),
diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs
index 09f4749ea3..4d6e56b041 100644
--- a/beacon_node/beacon_chain/src/test_utils.rs
+++ b/beacon_node/beacon_chain/src/test_utils.rs
@@ -6,6 +6,7 @@ use slot_clock::TestingSlotClock;
 use state_processing::per_slot_processing;
 use std::marker::PhantomData;
 use std::sync::Arc;
+use std::time::Duration;
 use store::MemoryStore;
 use store::Store;
 use tree_hash::{SignedRoot, TreeHash};
@@ -115,11 +116,12 @@ where
         let log = builder.build().expect("logger should build");
 
         // Slot clock
-        let slot_clock = TestingSlotClock::new(
+        let slot_clock = TestingSlotClock::from_eth2_genesis(
             spec.genesis_slot,
             genesis_state.genesis_time,
-            spec.seconds_per_slot,
-        );
+            Duration::from_secs(spec.seconds_per_slot),
+        )
+        .expect("Slot clock should start");
 
         let chain = BeaconChain::from_genesis(
             store,
@@ -164,7 +166,9 @@ where
         let mut state = {
             // Determine the slot for the first block (or skipped block).
             let state_slot = match block_strategy {
-                BlockStrategy::OnCanonicalHead => self.chain.read_slot_clock().unwrap() - 1,
+                BlockStrategy::OnCanonicalHead => {
+                    self.chain.present_slot().expect("should have a slot") - 1
+                }
                 BlockStrategy::ForkCanonicalChainAt { previous_slot, .. } => previous_slot,
             };
 
@@ -173,14 +177,16 @@ where
 
         // Determine the first slot where a block should be built.
         let mut slot = match block_strategy {
-            BlockStrategy::OnCanonicalHead => self.chain.read_slot_clock().unwrap(),
+            BlockStrategy::OnCanonicalHead => {
+                self.chain.present_slot().expect("should have a slot")
+            }
             BlockStrategy::ForkCanonicalChainAt { first_slot, .. } => first_slot,
         };
 
         let mut head_block_root = None;
 
         for _ in 0..num_blocks {
-            while self.chain.read_slot_clock().expect("should have a slot") < slot {
+            while self.chain.present_slot().expect("should have a slot") < slot {
                 self.advance_slot();
             }
 
diff --git a/beacon_node/network/src/sync/simple_sync.rs b/beacon_node/network/src/sync/simple_sync.rs
index 573ac9dd1f..49196facc1 100644
--- a/beacon_node/network/src/sync/simple_sync.rs
+++ b/beacon_node/network/src/sync/simple_sync.rs
@@ -387,7 +387,7 @@ impl<T: BeaconChainTypes> SimpleSync<T> {
                 "peer" => format!("{:?}", peer_id),
                 "msg" => "Failed to return all requested hashes",
                 "start_slot" => req.start_slot,
-                "current_slot" => self.chain.present_slot(),
+                "current_slot" => format!("{:?}", self.chain.present_slot()),
                 "requested" => req.count,
                 "returned" => blocks.len(),
             );
diff --git a/beacon_node/rest_api/src/helpers.rs b/beacon_node/rest_api/src/helpers.rs
index 5365086df7..0f47200e9d 100644
--- a/beacon_node/rest_api/src/helpers.rs
+++ b/beacon_node/rest_api/src/helpers.rs
@@ -88,8 +88,8 @@ pub fn state_root_at_slot<T: BeaconChainTypes>(
 ) -> Result<Hash256, ApiError> {
     let head_state = &beacon_chain.head().beacon_state;
     let current_slot = beacon_chain
-        .read_slot_clock()
-        .ok_or_else(|| ApiError::ServerError("Unable to read slot clock".to_string()))?;
+        .present_slot()
+        .map_err(|_| ApiError::ServerError("Unable to read slot clock".to_string()))?;
 
     // There are four scenarios when obtaining a state for a given slot:
     //
diff --git a/eth2/utils/slot_clock/src/lib.rs b/eth2/utils/slot_clock/src/lib.rs
index 988f3d322c..5986191dc1 100644
--- a/eth2/utils/slot_clock/src/lib.rs
+++ b/eth2/utils/slot_clock/src/lib.rs
@@ -5,7 +5,7 @@ mod metrics;
 mod system_time_slot_clock;
 mod testing_slot_clock;
 
-use std::time::{Duration, Instant};
+use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
 
 pub use crate::system_time_slot_clock::SystemTimeSlotClock;
 pub use crate::testing_slot_clock::TestingSlotClock;
@@ -13,6 +13,24 @@ pub use metrics::scrape_for_metrics;
 pub use types::Slot;
 
 pub trait SlotClock: Send + Sync + Sized {
+    fn from_eth2_genesis(
+        genesis_slot: Slot,
+        genesis_seconds: u64,
+        slot_duration: Duration,
+    ) -> Option<Self> {
+        let duration_between_now_and_unix_epoch =
+            SystemTime::now().duration_since(UNIX_EPOCH).ok()?;
+        let duration_between_unix_epoch_and_genesis = Duration::from_secs(genesis_seconds);
+
+        if duration_between_now_and_unix_epoch < duration_between_unix_epoch_and_genesis {
+            None
+        } else {
+            let genesis_instant = Instant::now()
+                - (duration_between_now_and_unix_epoch - duration_between_unix_epoch_and_genesis);
+            Some(Self::new(genesis_slot, genesis_instant, slot_duration))
+        }
+    }
+
     fn new(genesis_slot: Slot, genesis: Instant, slot_duration: Duration) -> Self;
 
     fn present_slot(&self) -> Option<Slot>;
diff --git a/validator_client/src/error.rs b/validator_client/src/error.rs
index 97500f900b..e13f7ded51 100644
--- a/validator_client/src/error.rs
+++ b/validator_client/src/error.rs
@@ -1,16 +1,9 @@
-use slot_clock;
-
 use error_chain::error_chain;
 
 error_chain! {
    links  { }
 
    errors {
-    SlotClockError(e: slot_clock::SystemTimeSlotClockError) {
-        description("Error reading system time"),
-        display("SlotClockError: '{:?}'", e)
-    }
-
     SystemTimeError(t: String ) {
         description("Error reading system time"),
         display("SystemTimeError: '{}'", t)
diff --git a/validator_client/src/service.rs b/validator_client/src/service.rs
index 3ddb96e4c2..62a782da93 100644
--- a/validator_client/src/service.rs
+++ b/validator_client/src/service.rs
@@ -13,7 +13,6 @@ use crate::block_producer::{BeaconBlockGrpcClient, BlockProducer};
 use crate::config::Config as ValidatorConfig;
 use crate::duties::{BeaconNodeDuties, DutiesManager, EpochDutiesMap};
 use crate::error as error_chain;
-use crate::error::ErrorKind;
 use crate::signer::Signer;
 use bls::Keypair;
 use eth2_config::Eth2Config;
@@ -159,17 +158,19 @@ impl<B: BeaconNodeDuties + 'static, S: Signer + 'static, E: EthSpec> Service<B,
         };
 
         // build the validator slot clock
-        let slot_clock = SystemTimeSlotClock::new(
+        let slot_clock = SystemTimeSlotClock::from_eth2_genesis(
             genesis_slot,
             genesis_time,
-            eth2_config.spec.seconds_per_slot,
-        );
+            Duration::from_secs(eth2_config.spec.seconds_per_slot),
+        )
+        .ok_or_else::<error_chain::Error, _>(|| {
+            "Unable to start slot clock. Genesis may not have occurred yet. Exiting.".into()
+        })?;
 
         let current_slot = slot_clock
             .present_slot()
-            .map_err(ErrorKind::SlotClockError)?
             .ok_or_else::<error_chain::Error, _>(|| {
-                "Genesis is not in the past. Exiting.".into()
+                "Genesis has not yet occurred. Exiting.".into()
             })?;
 
         /* Generate the duties manager */
@@ -244,7 +245,6 @@ impl<B: BeaconNodeDuties + 'static, S: Signer + 'static, E: EthSpec> Service<B,
         let duration_to_next_slot = service
             .slot_clock
             .duration_to_next_slot()
-            .map_err(|e| format!("System clock error: {:?}", e))?
             .ok_or_else::<error_chain::Error, _>(|| {
                 "Genesis is not in the past. Exiting.".into()
             })?;
@@ -291,15 +291,12 @@ impl<B: BeaconNodeDuties + 'static, S: Signer + 'static, E: EthSpec> Service<B,
 
     /// Updates the known current slot and epoch.
     fn update_current_slot(&mut self) -> error_chain::Result<()> {
-        let current_slot = match self.slot_clock.present_slot() {
-            Err(e) => {
-                error!(self.log, "SystemTimeError {:?}", e);
-                return Err("Could not read system time".into());
-            }
-            Ok(slot) => slot.ok_or_else::<error_chain::Error, _>(|| {
+        let current_slot = self
+            .slot_clock
+            .present_slot()
+            .ok_or_else::<error_chain::Error, _>(|| {
                 "Genesis is not in the past. Exiting.".into()
-            })?,
-        };
+            })?;
 
         let current_epoch = current_slot.epoch(self.slots_per_epoch);
 

From 7d03806107db9c2f6ad0984682ae0d7f652fb563 Mon Sep 17 00:00:00 2001
From: Paul Hauner <paul@paulhauner.com>
Date: Thu, 29 Aug 2019 14:26:30 +1000
Subject: [PATCH 3/4] Upgrade codebase to new SlotClock API

---
 beacon_node/beacon_chain/src/beacon_chain.rs  | 39 +++++++------------
 beacon_node/beacon_chain/src/test_utils.rs    |  8 ++--
 beacon_node/client/src/lib.rs                 | 10 +++--
 beacon_node/network/src/sync/simple_sync.rs   |  2 +-
 beacon_node/rest_api/src/helpers.rs           |  2 +-
 .../builders/testing_beacon_state_builder.rs  |  4 +-
 eth2/utils/slot_clock/src/lib.rs              |  2 +-
 eth2/utils/slot_clock/src/metrics.rs          |  2 +-
 .../slot_clock/src/system_time_slot_clock.rs  | 10 ++---
 .../slot_clock/src/testing_slot_clock.rs      |  8 ++--
 validator_client/src/service.rs               | 10 ++---
 11 files changed, 43 insertions(+), 54 deletions(-)

diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs
index 9ad4d5414e..67e4646c6f 100644
--- a/beacon_node/beacon_chain/src/beacon_chain.rs
+++ b/beacon_node/beacon_chain/src/beacon_chain.rs
@@ -148,11 +148,12 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         );
 
         // Slot clock
-        let slot_clock = T::SlotClock::new(
+        let slot_clock = T::SlotClock::from_eth2_genesis(
             spec.genesis_slot,
             genesis_state.genesis_time,
-            spec.seconds_per_slot,
-        );
+            Duration::from_secs(spec.seconds_per_slot),
+        )
+        .ok_or_else(|| Error::SlotClockDidNotStart)?;
 
         Ok(Self {
             spec,
@@ -224,18 +225,13 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         Ok(())
     }
 
-    /// Reads the slot clock, returns `Err` if the slot is unavailable.
+    /// Returns the slot _right now_ according to `self.slot_clock`. Returns `Err` if the slot is
+    /// unavailable.
     ///
     /// The slot might be unavailable due to an error with the system clock, or if the present time
     /// is before genesis (i.e., a negative slot).
-    ///
-    /// This is distinct to `present_slot`, which simply reads the latest state. If a
-    /// call to `read_slot_clock` results in a higher slot than a call to `present_slot`,
-    /// `self.state` should undergo per slot processing.
-    pub fn present_slot(&self) -> Result<Slot, Error> {
-        self.slot_clock
-            .present_slot()
-            .ok_or_else(|| Error::UnableToReadSlot)
+    pub fn slot(&self) -> Result<Slot, Error> {
+        self.slot_clock.now().ok_or_else(|| Error::UnableToReadSlot)
     }
 
     /// Returns the beacon block body for each beacon block root in `roots`.
@@ -348,10 +344,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     pub fn catchup_state(&self) -> Result<(), Error> {
         let spec = &self.spec;
 
-        let present_slot = self
-            .slot_clock
-            .present_slot()
-            .ok_or_else(|| Error::UnableToReadSlot)?;
+        let present_slot = self.slot()?;
 
         if self.state.read().slot < present_slot {
             let mut state = self.state.write();
@@ -394,7 +387,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     /// Reads the slot clock (see `self.read_slot_clock()` and returns the number of slots since
     /// genesis.
     pub fn slots_since_genesis(&self) -> Option<SlotHeight> {
-        let now = self.slot_clock.present_slot()?;
+        let now = self.slot().ok()?;
         let genesis_slot = self.spec.genesis_slot;
 
         if now < genesis_slot {
@@ -847,10 +840,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             return Ok(BlockProcessingOutcome::GenesisBlock);
         }
 
-        let present_slot = self
-            .slot_clock
-            .present_slot()
-            .ok_or_else(|| Error::UnableToReadSlot)?;
+        let present_slot = self.slot()?;
 
         if block.slot > present_slot {
             return Ok(BlockProcessingOutcome::FutureSlot {
@@ -1013,9 +1003,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     ) -> Result<(BeaconBlock<T::EthSpec>, BeaconState<T::EthSpec>), BlockProductionError> {
         let state = self.state.read().clone();
         let slot = self
-            .slot_clock
-            .present_slot()
-            .ok_or_else(|| BlockProductionError::UnableToReadSlot)?;
+            .slot()
+            .map_err(|_| BlockProductionError::UnableToReadSlot)?;
 
         self.produce_block_on_state(state, slot, randao_reveal)
     }
@@ -1191,7 +1180,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         *self.state.write() = {
             let mut state = self.canonical_head.read().beacon_state.clone();
 
-            let present_slot = self.present_slot()?;
+            let present_slot = self.slot()?;
 
             // If required, transition the new state to the present slot.
             for _ in state.slot.as_u64()..present_slot.as_u64() {
diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs
index 6ab657b087..c45a22fd85 100644
--- a/beacon_node/beacon_chain/src/test_utils.rs
+++ b/beacon_node/beacon_chain/src/test_utils.rs
@@ -151,7 +151,7 @@ where
             // Determine the slot for the first block (or skipped block).
             let state_slot = match block_strategy {
                 BlockStrategy::OnCanonicalHead => {
-                    self.chain.present_slot().expect("should have a slot") - 1
+                    self.chain.slot().expect("should have a slot") - 1
                 }
                 BlockStrategy::ForkCanonicalChainAt { previous_slot, .. } => previous_slot,
             };
@@ -161,16 +161,14 @@ where
 
         // Determine the first slot where a block should be built.
         let mut slot = match block_strategy {
-            BlockStrategy::OnCanonicalHead => {
-                self.chain.present_slot().expect("should have a slot")
-            }
+            BlockStrategy::OnCanonicalHead => self.chain.slot().expect("should have a slot"),
             BlockStrategy::ForkCanonicalChainAt { first_slot, .. } => first_slot,
         };
 
         let mut head_block_root = None;
 
         for _ in 0..num_blocks {
-            while self.chain.present_slot().expect("should have a slot") < slot {
+            while self.chain.slot().expect("should have a slot") < slot {
                 self.advance_slot();
             }
 
diff --git a/beacon_node/client/src/lib.rs b/beacon_node/client/src/lib.rs
index 2612fd6489..004353d38b 100644
--- a/beacon_node/client/src/lib.rs
+++ b/beacon_node/client/src/lib.rs
@@ -114,7 +114,7 @@ where
                 .map_err(error::Error::from)?,
         );
 
-        if beacon_chain.read_slot_clock().is_none() {
+        if beacon_chain.slot().is_err() {
             panic!("Cannot start client before genesis!")
         }
 
@@ -124,7 +124,9 @@ where
         // blocks and we're basically useless.
         {
             let state_slot = beacon_chain.head().beacon_state.slot;
-            let wall_clock_slot = beacon_chain.read_slot_clock().unwrap();
+            let wall_clock_slot = beacon_chain
+                .slot()
+                .expect("Cannot start client before genesis");
             let slots_since_genesis = beacon_chain.slots_since_genesis().unwrap();
             info!(
                 log,
@@ -176,7 +178,7 @@ where
         };
 
         let (slot_timer_exit_signal, exit) = exit_future::signal();
-        if let Ok(Some(duration_to_next_slot)) = beacon_chain.slot_clock.duration_to_next_slot() {
+        if let Some(duration_to_next_slot) = beacon_chain.slot_clock.duration_to_next_slot() {
             // set up the validator work interval - start at next slot and proceed every slot
             let interval = {
                 // Set the interval to start at the next slot, and every slot after
@@ -223,7 +225,7 @@ impl<T: BeaconChainTypes> Drop for Client<T> {
 
 fn do_state_catchup<T: BeaconChainTypes>(chain: &Arc<BeaconChain<T>>, log: &slog::Logger) {
     // Only attempt to `catchup_state` if we can read the slot clock.
-    if let Some(current_slot) = chain.read_slot_clock() {
+    if let Ok(current_slot) = chain.slot() {
         let state_catchup_result = chain.catchup_state();
 
         let best_slot = chain.head().beacon_block.slot;
diff --git a/beacon_node/network/src/sync/simple_sync.rs b/beacon_node/network/src/sync/simple_sync.rs
index 49196facc1..d3ed2f3e4f 100644
--- a/beacon_node/network/src/sync/simple_sync.rs
+++ b/beacon_node/network/src/sync/simple_sync.rs
@@ -387,7 +387,7 @@ impl<T: BeaconChainTypes> SimpleSync<T> {
                 "peer" => format!("{:?}", peer_id),
                 "msg" => "Failed to return all requested hashes",
                 "start_slot" => req.start_slot,
-                "current_slot" => format!("{:?}", self.chain.present_slot()),
+                "current_slot" => format!("{:?}", self.chain.slot()),
                 "requested" => req.count,
                 "returned" => blocks.len(),
             );
diff --git a/beacon_node/rest_api/src/helpers.rs b/beacon_node/rest_api/src/helpers.rs
index 0f47200e9d..aeaf5ad6e8 100644
--- a/beacon_node/rest_api/src/helpers.rs
+++ b/beacon_node/rest_api/src/helpers.rs
@@ -88,7 +88,7 @@ pub fn state_root_at_slot<T: BeaconChainTypes>(
 ) -> Result<Hash256, ApiError> {
     let head_state = &beacon_chain.head().beacon_state;
     let current_slot = beacon_chain
-        .present_slot()
+        .slot()
         .map_err(|_| ApiError::ServerError("Unable to read slot clock".to_string()))?;
 
     // There are four scenarios when obtaining a state for a given slot:
diff --git a/eth2/types/src/test_utils/builders/testing_beacon_state_builder.rs b/eth2/types/src/test_utils/builders/testing_beacon_state_builder.rs
index 98f8409538..4f8a2d9240 100644
--- a/eth2/types/src/test_utils/builders/testing_beacon_state_builder.rs
+++ b/eth2/types/src/test_utils/builders/testing_beacon_state_builder.rs
@@ -123,8 +123,10 @@ impl<T: EthSpec> TestingBeaconStateBuilder<T> {
             .collect::<Vec<_>>()
             .into();
 
+        let genesis_time = 1567052589; // 29 August, 2019;
+
         let mut state = BeaconState::new(
-            spec.min_genesis_time,
+            genesis_time,
             Eth1Data {
                 deposit_root: Hash256::zero(),
                 deposit_count: 0,
diff --git a/eth2/utils/slot_clock/src/lib.rs b/eth2/utils/slot_clock/src/lib.rs
index 5986191dc1..fd3bf029be 100644
--- a/eth2/utils/slot_clock/src/lib.rs
+++ b/eth2/utils/slot_clock/src/lib.rs
@@ -33,7 +33,7 @@ pub trait SlotClock: Send + Sync + Sized {
 
     fn new(genesis_slot: Slot, genesis: Instant, slot_duration: Duration) -> Self;
 
-    fn present_slot(&self) -> Option<Slot>;
+    fn now(&self) -> Option<Slot>;
 
     fn duration_to_next_slot(&self) -> Option<Duration>;
 
diff --git a/eth2/utils/slot_clock/src/metrics.rs b/eth2/utils/slot_clock/src/metrics.rs
index 1abd93c488..d1de491d00 100644
--- a/eth2/utils/slot_clock/src/metrics.rs
+++ b/eth2/utils/slot_clock/src/metrics.rs
@@ -17,7 +17,7 @@ lazy_static! {
 
 /// Update the global metrics `DEFAULT_REGISTRY` with info from the slot clock.
 pub fn scrape_for_metrics<T: EthSpec, U: SlotClock>(clock: &U) {
-    let present_slot = match clock.present_slot() {
+    let present_slot = match clock.now() {
         Some(slot) => slot,
         _ => Slot::new(0),
     };
diff --git a/eth2/utils/slot_clock/src/system_time_slot_clock.rs b/eth2/utils/slot_clock/src/system_time_slot_clock.rs
index 88c9c0e63e..0d4a52ef64 100644
--- a/eth2/utils/slot_clock/src/system_time_slot_clock.rs
+++ b/eth2/utils/slot_clock/src/system_time_slot_clock.rs
@@ -25,7 +25,7 @@ impl SlotClock for SystemTimeSlotClock {
         }
     }
 
-    fn present_slot(&self) -> Option<Slot> {
+    fn now(&self) -> Option<Slot> {
         let now = Instant::now();
 
         if now < self.genesis {
@@ -80,18 +80,18 @@ mod tests {
 
         let clock =
             SystemTimeSlotClock::new(genesis_slot, prior_genesis(0), Duration::from_secs(1));
-        assert_eq!(clock.present_slot(), Some(Slot::new(0)));
+        assert_eq!(clock.now(), Some(Slot::new(0)));
 
         let clock =
             SystemTimeSlotClock::new(genesis_slot, prior_genesis(5), Duration::from_secs(1));
-        assert_eq!(clock.present_slot(), Some(Slot::new(5)));
+        assert_eq!(clock.now(), Some(Slot::new(5)));
 
         let clock = SystemTimeSlotClock::new(
             genesis_slot,
             Instant::now() - Duration::from_millis(500),
             Duration::from_secs(1),
         );
-        assert_eq!(clock.present_slot(), Some(Slot::new(0)));
+        assert_eq!(clock.now(), Some(Slot::new(0)));
         assert!(clock.duration_to_next_slot().unwrap() < Duration::from_millis(500));
 
         let clock = SystemTimeSlotClock::new(
@@ -99,7 +99,7 @@ mod tests {
             Instant::now() - Duration::from_millis(1_500),
             Duration::from_secs(1),
         );
-        assert_eq!(clock.present_slot(), Some(Slot::new(1)));
+        assert_eq!(clock.now(), Some(Slot::new(1)));
         assert!(clock.duration_to_next_slot().unwrap() < Duration::from_millis(500));
     }
 
diff --git a/eth2/utils/slot_clock/src/testing_slot_clock.rs b/eth2/utils/slot_clock/src/testing_slot_clock.rs
index 0b65b15694..d90cb157aa 100644
--- a/eth2/utils/slot_clock/src/testing_slot_clock.rs
+++ b/eth2/utils/slot_clock/src/testing_slot_clock.rs
@@ -16,7 +16,7 @@ impl TestingSlotClock {
     }
 
     pub fn advance_slot(&self) {
-        self.set_slot(self.present_slot().unwrap().as_u64() + 1)
+        self.set_slot(self.now().unwrap().as_u64() + 1)
     }
 }
 
@@ -27,7 +27,7 @@ impl SlotClock for TestingSlotClock {
         }
     }
 
-    fn present_slot(&self) -> Option<Slot> {
+    fn now(&self) -> Option<Slot> {
         let slot = *self.slot.read().expect("TestingSlotClock poisoned.");
         Some(slot)
     }
@@ -50,8 +50,8 @@ mod tests {
     #[test]
     fn test_slot_now() {
         let clock = TestingSlotClock::new(Slot::new(10), Instant::now(), Duration::from_secs(0));
-        assert_eq!(clock.present_slot(), Some(Slot::new(10)));
+        assert_eq!(clock.now(), Some(Slot::new(10)));
         clock.set_slot(123);
-        assert_eq!(clock.present_slot(), Some(Slot::new(123)));
+        assert_eq!(clock.now(), Some(Slot::new(123)));
     }
 }
diff --git a/validator_client/src/service.rs b/validator_client/src/service.rs
index 62a782da93..68a9132656 100644
--- a/validator_client/src/service.rs
+++ b/validator_client/src/service.rs
@@ -167,11 +167,9 @@ impl<B: BeaconNodeDuties + 'static, S: Signer + 'static, E: EthSpec> Service<B,
             "Unable to start slot clock. Genesis may not have occurred yet. Exiting.".into()
         })?;
 
-        let current_slot = slot_clock
-            .present_slot()
-            .ok_or_else::<error_chain::Error, _>(|| {
-                "Genesis has not yet occurred. Exiting.".into()
-            })?;
+        let current_slot = slot_clock.now().ok_or_else::<error_chain::Error, _>(|| {
+            "Genesis has not yet occurred. Exiting.".into()
+        })?;
 
         /* Generate the duties manager */
 
@@ -293,7 +291,7 @@ impl<B: BeaconNodeDuties + 'static, S: Signer + 'static, E: EthSpec> Service<B,
     fn update_current_slot(&mut self) -> error_chain::Result<()> {
         let current_slot = self
             .slot_clock
-            .present_slot()
+            .now()
             .ok_or_else::<error_chain::Error, _>(|| {
                 "Genesis is not in the past. Exiting.".into()
             })?;

From 8cfbe8bbfba1b0d9371455959a5260f9675f767c Mon Sep 17 00:00:00 2001
From: Paul Hauner <paul@paulhauner.com>
Date: Thu, 29 Aug 2019 14:32:21 +1000
Subject: [PATCH 4/4] Change seconds_per_slot to milliseconds_per_slot

---
 beacon_node/beacon_chain/src/beacon_chain.rs | 15 ++-------------
 beacon_node/client/src/lib.rs                |  4 ++--
 eth2/types/src/chain_spec.rs                 |  6 +++---
 validator_client/src/service.rs              |  4 ++--
 4 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs
index 67e4646c6f..fb2f8ea6a9 100644
--- a/beacon_node/beacon_chain/src/beacon_chain.rs
+++ b/beacon_node/beacon_chain/src/beacon_chain.rs
@@ -151,7 +151,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         let slot_clock = T::SlotClock::from_eth2_genesis(
             spec.genesis_slot,
             genesis_state.genesis_time,
-            Duration::from_secs(spec.seconds_per_slot),
+            Duration::from_millis(spec.milliseconds_per_slot),
         )
         .ok_or_else(|| Error::SlotClockDidNotStart)?;
 
@@ -184,7 +184,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         let slot_clock = T::SlotClock::from_eth2_genesis(
             spec.genesis_slot,
             p.state.genesis_time,
-            Duration::from_secs(spec.seconds_per_slot),
+            Duration::from_millis(spec.milliseconds_per_slot),
         )
         .ok_or_else(|| Error::SlotClockDidNotStart)?;
 
@@ -397,17 +397,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         }
     }
 
-    /*
-    /// Returns slot of the present state.
-    ///
-    /// This is distinct to `read_slot_clock`, which reads from the actual system clock. If
-    /// `self.state` has not been transitioned it is possible for the system clock to be on a
-    /// different slot to what is returned from this call.
-    pub fn present_slot(&self) -> Slot {
-        self.state.read().slot
-    }
-    */
-
     /// Returns the block proposer for a given slot.
     ///
     /// Information is read from the present `beacon_state` shuffling, only information from the
diff --git a/beacon_node/client/src/lib.rs b/beacon_node/client/src/lib.rs
index 004353d38b..67528e2f96 100644
--- a/beacon_node/client/src/lib.rs
+++ b/beacon_node/client/src/lib.rs
@@ -76,7 +76,7 @@ where
         executor: &TaskExecutor,
     ) -> error::Result<Self> {
         let store = Arc::new(store);
-        let seconds_per_slot = eth2_config.spec.seconds_per_slot;
+        let milliseconds_per_slot = eth2_config.spec.milliseconds_per_slot;
 
         let spec = &eth2_config.spec.clone();
 
@@ -182,7 +182,7 @@ where
             // set up the validator work interval - start at next slot and proceed every slot
             let interval = {
                 // Set the interval to start at the next slot, and every slot after
-                let slot_duration = Duration::from_secs(seconds_per_slot);
+                let slot_duration = Duration::from_millis(milliseconds_per_slot);
                 //TODO: Handle checked add correctly
                 Interval::new(Instant::now() + duration_to_next_slot, slot_duration)
             };
diff --git a/eth2/types/src/chain_spec.rs b/eth2/types/src/chain_spec.rs
index 9dec626d44..d59e0db0ac 100644
--- a/eth2/types/src/chain_spec.rs
+++ b/eth2/types/src/chain_spec.rs
@@ -58,7 +58,7 @@ pub struct ChainSpec {
     /*
      * Time parameters
      */
-    pub seconds_per_slot: u64,
+    pub milliseconds_per_slot: u64,
     pub min_attestation_inclusion_delay: u64,
     pub min_seed_lookahead: Epoch,
     pub activation_exit_delay: u64,
@@ -158,7 +158,7 @@ impl ChainSpec {
             /*
              * Time parameters
              */
-            seconds_per_slot: 6,
+            milliseconds_per_slot: 6_000,
             min_attestation_inclusion_delay: 1,
             min_seed_lookahead: Epoch::new(1),
             activation_exit_delay: 4,
@@ -221,7 +221,7 @@ impl ChainSpec {
         let boot_nodes = vec![];
 
         Self {
-            seconds_per_slot: 12,
+            milliseconds_per_slot: 12_000,
             target_committee_size: 4,
             shuffle_round_count: 10,
             network_id: 13,
diff --git a/validator_client/src/service.rs b/validator_client/src/service.rs
index 68a9132656..bd694668bf 100644
--- a/validator_client/src/service.rs
+++ b/validator_client/src/service.rs
@@ -161,7 +161,7 @@ impl<B: BeaconNodeDuties + 'static, S: Signer + 'static, E: EthSpec> Service<B,
         let slot_clock = SystemTimeSlotClock::from_eth2_genesis(
             genesis_slot,
             genesis_time,
-            Duration::from_secs(eth2_config.spec.seconds_per_slot),
+            Duration::from_millis(eth2_config.spec.milliseconds_per_slot),
         )
         .ok_or_else::<error_chain::Error, _>(|| {
             "Unable to start slot clock. Genesis may not have occurred yet. Exiting.".into()
@@ -250,7 +250,7 @@ impl<B: BeaconNodeDuties + 'static, S: Signer + 'static, E: EthSpec> Service<B,
         // set up the validator work interval - start at next slot and proceed every slot
         let interval = {
             // Set the interval to start at the next slot, and every slot after
-            let slot_duration = Duration::from_secs(service.spec.seconds_per_slot);
+            let slot_duration = Duration::from_millis(service.spec.milliseconds_per_slot);
             //TODO: Handle checked add correctly
             Interval::new(Instant::now() + duration_to_next_slot, slot_duration)
         };