Merge branch 'millsecond-slots' into interop

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;
@@ -147,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_millis(spec.milliseconds_per_slot),
-        );
+        )
+        .ok_or_else(|| Error::SlotClockDidNotStart)?;
 
         Ok(Self {
             spec,
@@ -179,11 +181,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_millis(spec.milliseconds_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;
@@ -222,6 +225,15 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         Ok(())
     }
 
+    /// 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).
+    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`.
     ///
     /// Fails if any root in `roots` does not have a corresponding block.
@@ -332,10 +344,7 @@ 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() {
+        let present_slot = self.slot()?;
-            Ok(Some(slot)) => slot,
-            _ => return Err(Error::UnableToReadSlot),
-        };
 
         if self.state.read().slot < present_slot {
             let mut state = self.state.write();
@@ -375,26 +384,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().ok()?;
         let genesis_slot = self.spec.genesis_slot;
 
         if now < genesis_slot {
@@ -404,15 +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
@@ -845,9 +829,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             return Ok(BlockProcessingOutcome::GenesisBlock);
         }
 
-        let present_slot = self
+        let present_slot = self.slot()?;
-            .read_slot_clock()
-            .ok_or_else(|| Error::UnableToReadSlot)?;
 
         if block.slot > present_slot {
             return Ok(BlockProcessingOutcome::FutureSlot {
@@ -1010,8 +992,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()
-            .ok_or_else(|| BlockProductionError::UnableToReadSlot)?;
+            .map_err(|_| BlockProductionError::UnableToReadSlot)?;
 
         self.produce_block_on_state(state, slot, randao_reveal)
     }
@@ -1187,10 +1169,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() {
+            let present_slot = self.slot()?;
-                Ok(Some(slot)) => slot,
-                _ => return Err(Error::UnableToReadSlot),
-            };
 
             // If required, transition the new state to the present slot.
             for _ in state.slot.as_u64()..present_slot.as_u64() {

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),

beacon_node/beacon_chain/src/test_utils.rs

@@ -150,7 +150,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.slot().expect("should have a slot") - 1
+                }
                 BlockStrategy::ForkCanonicalChainAt { previous_slot, .. } => previous_slot,
             };
 
@@ -159,14 +161,14 @@ 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.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.slot().expect("should have a slot") < slot {
                 self.advance_slot();
             }
 

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();
 
@@ -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,11 +178,11 @@ 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
-                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)
             };
@@ -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;

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.slot()),
                 "requested" => req.count,
                 "returned" => blocks.len(),
             );

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()
+        .slot()
-        .ok_or_else(|| ApiError::ServerError("Unable to read slot clock".to_string()))?;
+        .map_err(|_| ApiError::ServerError("Unable to read slot clock".to_string()))?;
 
     // There are four scenarios when obtaining a state for a given slot:
     //

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,

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,

eth2/utils/slot_clock/src/lib.rs

@@ -5,24 +5,37 @@ mod metrics;
 mod system_time_slot_clock;
 mod testing_slot_clock;
 
-use std::time::Duration;
+use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
 
-pub use crate::system_time_slot_clock::{Error as SystemTimeSlotClockError, SystemTimeSlotClock};
+pub use crate::system_time_slot_clock::SystemTimeSlotClock;
-pub use crate::testing_slot_clock::{Error as TestingSlotClockError, TestingSlotClock};
+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 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);
 
-    /// Create a new `SlotClock`.
+        if duration_between_now_and_unix_epoch < duration_between_unix_epoch_and_genesis {
-    ///
+            None
-    /// Returns an Error if `slot_duration_seconds == 0`.
+        } else {
-    fn new(genesis_slot: Slot, genesis_seconds: u64, slot_duration_seconds: u64) -> Self;
+            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 present_slot(&self) -> Result<Option<Slot>, Self::Error>;
+    fn new(genesis_slot: Slot, genesis: Instant, slot_duration: Duration) -> Self;
 
-    fn duration_to_next_slot(&self) -> Result<Option<Duration>, Self::Error>;
+    fn now(&self) -> Option<Slot>;
 
-    fn slot_duration_millis(&self) -> u64;
+    fn duration_to_next_slot(&self) -> Option<Duration>;
+
+    fn slot_duration(&self) -> Duration;
 }

eth2/utils/slot_clock/src/metrics.rs

@@ -17,8 +17,8 @@ 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() {
-        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,
+    );
 }

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,
+    genesis: Instant,
-    slot_duration_seconds: u64,
+    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,
+            genesis,
-            slot_duration_seconds,
+            slot_duration,
         }
     }
 
-    fn present_slot(&self) -> Result<Option<Slot>, Error> {
+    fn now(&self) -> Option<Slot> {
-        if self.slot_duration_seconds == 0 {
+        let now = Instant::now();
-            return Err(Error::SlotDurationIsZero);
-        }
 
-        let syslot_time = SystemTime::now();
+        if now < self.genesis {
-        let duration_since_epoch = syslot_time.duration_since(SystemTime::UNIX_EPOCH)?;
+            None
-        let duration_since_genesis =
+        } else {
-            duration_since_epoch.checked_sub(Duration::from_secs(self.genesis_seconds));
+            let slot = Slot::from(
-
+                (now.duration_since(self.genesis).as_millis() / self.slot_duration.as_millis())
-        match duration_since_genesis {
+                    as u64,
-            None => Ok(None),
+            );
-            Some(d) => Ok(slot_from_duration(self.slot_duration_seconds, d)
+            Some(slot + self.genesis_slot)
-                .and_then(|s| Some(s + self.genesis_slot))),
         }
     }
 
-    fn duration_to_next_slot(&self) -> Result<Option<Duration>, Error> {
+    fn duration_to_next_slot(&self) -> Option<Duration> {
-        duration_to_next_slot(self.genesis_seconds, self.slot_duration_seconds)
+        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 {
+    fn slot_duration(&self) -> Duration {
-        self.slot_duration_seconds * 1000
+        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 prior_genesis =
-        let since_epoch = now.duration_since(SystemTime::UNIX_EPOCH).unwrap();
+            |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.now(), Some(Slot::new(0)));
 
-        let clock = SystemTimeSlotClock {
+        let clock =
+            SystemTimeSlotClock::new(genesis_slot, prior_genesis(5), Duration::from_secs(1));
+        assert_eq!(clock.now(), Some(Slot::new(5)));
+
+        let clock = SystemTimeSlotClock::new(
             genesis_slot,
-            genesis_seconds: genesis,
+            Instant::now() - Duration::from_millis(500),
-            slot_duration_seconds: slot_time,
+            Duration::from_secs(1),
-        };
+        );
-        assert_eq!(clock.present_slot().unwrap(), Some(Slot::new(89)));
+        assert_eq!(clock.now(), 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(),
+            Instant::now() - Duration::from_millis(1_500),
-            slot_duration_seconds: slot_time,
+            Duration::from_secs(1),
-        };
+        );
-        assert_eq!(clock.present_slot().unwrap(), Some(Slot::new(0)));
+        assert_eq!(clock.now(), Some(Slot::new(1)));
-
+        assert!(clock.duration_to_next_slot().unwrap() < Duration::from_millis(500));
-        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)));
     }
 
     #[test]
-    fn test_slot_from_duration() {
+    #[should_panic]
-        let slot_time = 100;
+    fn zero_seconds() {
-
+        SystemTimeSlotClock::new(Slot::new(0), Instant::now(), Duration::from_secs(0));
-        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))
-        );
     }
 
     #[test]
-    fn test_slot_from_duration_slot_time_zero() {
+    #[should_panic]
-        let slot_time = 0;
+    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);
+    #[test]
-        assert_eq!(slot_from_duration(slot_time, Duration::from_secs(10)), None);
+    #[should_panic]
-        assert_eq!(
+    fn less_than_one_millis() {
-            slot_from_duration(slot_time, Duration::from_secs(1000)),
+        SystemTimeSlotClock::new(Slot::new(0), Instant::now(), Duration::from_nanos(999));
-            None
-        );
     }
 }

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)]
+/// A slot clock where the slot is manually set instead of being determined by the system time.
-pub enum Error {}
+///
-
+/// Useful for testing scenarios.
-/// Determines the present slot based upon the present system time.
 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.now().unwrap().as_u64() + 1)
     }
 }
 
 impl SlotClock for TestingSlotClock {
-    type Error = Error;
+    fn new(genesis_slot: Slot, _genesis: Instant, _slot_duration: Duration) -> Self {
-
-    /// Create a new `TestingSlotClock` at `genesis_slot`.
-    fn new(genesis_slot: Slot, _genesis_seconds: u64, _slot_duration_seconds: u64) -> Self {
         TestingSlotClock {
             slot: RwLock::new(genesis_slot),
         }
     }
 
-    fn present_slot(&self) -> Result<Option<Slot>, Error> {
+    fn now(&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> {
+    fn duration_to_next_slot(&self) -> Option<Duration> {
-        Ok(Some(Duration::from_secs(1)))
+        Some(Duration::from_secs(1))
     }
 
-    fn slot_duration_millis(&self) -> u64 {
+    /// Always returns a slot duration of 0 seconds.
-        0
+    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), Instant::now(), Duration::from_secs(0));
-
+        assert_eq!(clock.now(), Some(Slot::new(10)));
-        let clock = TestingSlotClock::new(Slot::new(10), null, null);
-        assert_eq!(clock.present_slot(), Ok(Some(Slot::new(10))));
         clock.set_slot(123);
-        assert_eq!(clock.present_slot(), Ok(Some(Slot::new(123))));
+        assert_eq!(clock.now(), Some(Slot::new(123)));
     }
 }

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)

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,18 +158,18 @@ 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_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()
+        })?;
 
-        let current_slot = slot_clock
+        let current_slot = slot_clock.now().ok_or_else::<error_chain::Error, _>(|| {
-            .present_slot()
+            "Genesis has not yet occurred. Exiting.".into()
-            .map_err(ErrorKind::SlotClockError)?
+        })?;
-            .ok_or_else::<error_chain::Error, _>(|| {
-                "Genesis is not in the past. Exiting.".into()
-            })?;
 
         /* Generate the duties manager */
 
@@ -244,7 +243,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()
             })?;
@@ -252,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)
         };
@@ -291,15 +289,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() {
+        let current_slot = self
-            Err(e) => {
+            .slot_clock
-                error!(self.log, "SystemTimeError {:?}", e);
+            .now()
-                return Err("Could not read system time".into());
+            .ok_or_else::<error_chain::Error, _>(|| {
-            }
-            Ok(slot) => 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);