From 7d03806107db9c2f6ad0984682ae0d7f652fb563 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 29 Aug 2019 14:26:30 +1000 Subject: [PATCH] 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 9ad4d5414..67e4646c6 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -148,11 +148,12 @@ impl BeaconChain { ); // 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 BeaconChain { 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 { - self.slot_clock - .present_slot() - .ok_or_else(|| Error::UnableToReadSlot) + pub fn slot(&self) -> Result { + 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 BeaconChain { 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 BeaconChain { /// 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.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 BeaconChain { 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 BeaconChain { ) -> Result<(BeaconBlock, BeaconState), 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 BeaconChain { *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 6ab657b08..c45a22fd8 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 2612fd648..004353d38 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 Drop for Client { fn do_state_catchup(chain: &Arc>, 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 49196facc..d3ed2f3e4 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" => 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 0f47200e9..aeaf5ad6e 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( ) -> Result { 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 98f840953..4f8a2d924 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 TestingBeaconStateBuilder { .collect::>() .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 5986191dc..fd3bf029b 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; + fn now(&self) -> Option; fn duration_to_next_slot(&self) -> Option; diff --git a/eth2/utils/slot_clock/src/metrics.rs b/eth2/utils/slot_clock/src/metrics.rs index 1abd93c48..d1de491d0 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(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 88c9c0e63..0d4a52ef6 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 { + fn now(&self) -> Option { 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 0b65b1569..d90cb157a 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 { + fn now(&self) -> Option { 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 62a782da9..68a913265 100644 --- a/validator_client/src/service.rs +++ b/validator_client/src/service.rs @@ -167,11 +167,9 @@ impl Service(|| { - "Genesis has not yet occurred. Exiting.".into() - })?; + let current_slot = slot_clock.now().ok_or_else::(|| { + "Genesis has not yet occurred. Exiting.".into() + })?; /* Generate the duties manager */ @@ -293,7 +291,7 @@ impl Service error_chain::Result<()> { let current_slot = self .slot_clock - .present_slot() + .now() .ok_or_else::(|| { "Genesis is not in the past. Exiting.".into() })?;