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 BeaconChain { 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 BeaconChain { 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 { + 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 BeaconChain { 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 BeaconChain { 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 { - 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 { - 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 BeaconChain { } } + /* /// 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 BeaconChain { pub fn present_slot(&self) -> Slot { self.state.read().slot } + */ /// Returns the block proposer for a given slot. /// @@ -840,7 +842,8 @@ impl BeaconChain { } 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 BeaconChain { ) -> Result<(BeaconBlock, BeaconState), 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 BeaconChain { *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 SimpleSync { "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( ) -> Result { 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 { + 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; 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 Service(|| { + "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::(|| { - "Genesis is not in the past. Exiting.".into() + "Genesis has not yet occurred. Exiting.".into() })?; /* Generate the duties manager */ @@ -244,7 +245,6 @@ impl Service(|| { "Genesis is not in the past. Exiting.".into() })?; @@ -291,15 +291,12 @@ impl Service 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::(|| { + let current_slot = self + .slot_clock + .present_slot() + .ok_or_else::(|| { "Genesis is not in the past. Exiting.".into() - })?, - }; + })?; let current_epoch = current_slot.epoch(self.slots_per_epoch);