Migrate codebase across to new SlotClock API

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() {
+        let present_slot = self
-            Ok(Some(slot)) => slot,
+            .slot_clock
-            _ => return Err(Error::UnableToReadSlot),
+            .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() {
+            let present_slot = self.present_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

@@ -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();
             }
 

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

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()
+        .present_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/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>;

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,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() {
+        let current_slot = self
-            Err(e) => {
+            .slot_clock
-                error!(self.log, "SystemTimeError {:?}", e);
+            .present_slot()
-                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);