From 3606f0447d4dcc22f614a7532335a7d9e5bf4cb4 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 18 Mar 2020 11:51:29 +1100 Subject: [PATCH] Merge latest from branch `clock-disparity` * Account for genesis time * Use checked mul * Account for genesis slot * Change API * Refactor "duration to..." functions --- eth2/utils/slot_clock/src/lib.rs | 14 + .../utils/slot_clock/src/manual_slot_clock.rs | 242 +++++++++++++++--- .../slot_clock/src/system_time_slot_clock.rs | 4 + 3 files changed, 219 insertions(+), 41 deletions(-) diff --git a/eth2/utils/slot_clock/src/lib.rs b/eth2/utils/slot_clock/src/lib.rs index 0990ec141..e9566a9a0 100644 --- a/eth2/utils/slot_clock/src/lib.rs +++ b/eth2/utils/slot_clock/src/lib.rs @@ -40,4 +40,18 @@ pub trait SlotClock: Send + Sync + Sized { /// Returns the duration until the first slot of the next epoch. fn duration_to_next_epoch(&self, slots_per_epoch: u64) -> Option; + + /// Returns the first slot to be returned at the genesis time. + fn genesis_slot(&self) -> Slot; + + /// Returns the slot if the internal clock were advanced by `duration`. + fn now_with_future_tolerance(&self, tolerance: Duration) -> Option { + self.slot_of(self.now_duration()?.checked_add(tolerance)?) + } + + /// Returns the slot if the internal clock were reversed by `duration`. + fn now_with_past_tolerance(&self, tolerance: Duration) -> Option { + self.slot_of(self.now_duration()?.checked_sub(tolerance)?) + .or_else(|| Some(self.genesis_slot())) + } } diff --git a/eth2/utils/slot_clock/src/manual_slot_clock.rs b/eth2/utils/slot_clock/src/manual_slot_clock.rs index 69109f49b..fc0e27e3a 100644 --- a/eth2/utils/slot_clock/src/manual_slot_clock.rs +++ b/eth2/utils/slot_clock/src/manual_slot_clock.rs @@ -41,58 +41,47 @@ impl ManualSlotClock { self.set_slot(self.now().unwrap().as_u64() + 1) } + /// Returns the duration between UNIX epoch and the start of `slot`. + pub fn start_of(&self, slot: Slot) -> Option { + let slot = slot + .as_u64() + .checked_sub(self.genesis_slot.as_u64())? + .try_into() + .ok()?; + let unadjusted_slot_duration = self.slot_duration.checked_mul(slot)?; + + self.genesis_duration.checked_add(unadjusted_slot_duration) + } + + /// Returns the duration from `now` until the start of `slot`. + /// + /// Will return `None` if `now` is later than the start of `slot`. + pub fn duration_to_slot(&self, slot: Slot, now: Duration) -> Option { + self.start_of(slot)?.checked_sub(now) + } + + /// Returns the duration between `now` and the start of the next slot. pub fn duration_to_next_slot_from(&self, now: Duration) -> Option { - let genesis = self.genesis_duration; - - let slot_start = |slot: Slot| -> Duration { - let slot = slot.as_u64() as u32; - genesis + slot * self.slot_duration - }; - - if now >= genesis { - Some( - slot_start(self.slot_of(now)? + 1) - .checked_sub(now) - .expect("The next slot cannot start before now"), - ) + if now < self.genesis_duration { + self.genesis_duration.checked_sub(now) } else { - Some( - genesis - .checked_sub(now) - .expect("Control flow ensures genesis is greater than or equal to now"), - ) + self.duration_to_slot(self.slot_of(now)? + 1, now) } } + /// Returns the duration between `now` and the start of the next epoch. pub fn duration_to_next_epoch_from( &self, now: Duration, slots_per_epoch: u64, ) -> Option { - let genesis = self.genesis_duration; - - let slot_start = |slot: Slot| -> Duration { - let slot = slot.as_u64() as u32; - genesis + slot * self.slot_duration - }; - - let epoch_start_slot = self - .now() - .map(|slot| slot.epoch(slots_per_epoch)) - .map(|epoch| (epoch + 1).start_slot(slots_per_epoch))?; - - if now >= genesis { - Some( - slot_start(epoch_start_slot) - .checked_sub(now) - .expect("The next epoch cannot start before now"), - ) + if now < self.genesis_duration { + self.genesis_duration.checked_sub(now) } else { - Some( - genesis - .checked_sub(now) - .expect("Control flow ensures genesis is greater than or equal to now"), - ) + let next_epoch_start_slot = + (self.slot_of(now)?.epoch(slots_per_epoch) + 1).start_slot(slots_per_epoch); + + self.duration_to_slot(next_epoch_start_slot, now) } } } @@ -145,6 +134,10 @@ impl SlotClock for ManualSlotClock { fn slot_duration(&self) -> Duration { self.slot_duration } + + fn genesis_slot(&self) -> Slot { + self.genesis_slot + } } #[cfg(test)] @@ -162,4 +155,171 @@ mod tests { clock.set_slot(123); assert_eq!(clock.now(), Some(Slot::new(123))); } + + #[test] + fn start_of() { + // Genesis slot and genesis duration 0. + let clock = + ManualSlotClock::new(Slot::new(0), Duration::from_secs(0), Duration::from_secs(1)); + assert_eq!(clock.start_of(Slot::new(0)), Some(Duration::from_secs(0))); + assert_eq!(clock.start_of(Slot::new(1)), Some(Duration::from_secs(1))); + assert_eq!(clock.start_of(Slot::new(2)), Some(Duration::from_secs(2))); + + // Genesis slot 1 and genesis duration 10. + let clock = ManualSlotClock::new( + Slot::new(0), + Duration::from_secs(10), + Duration::from_secs(1), + ); + assert_eq!(clock.start_of(Slot::new(0)), Some(Duration::from_secs(10))); + assert_eq!(clock.start_of(Slot::new(1)), Some(Duration::from_secs(11))); + assert_eq!(clock.start_of(Slot::new(2)), Some(Duration::from_secs(12))); + + // Genesis slot 1 and genesis duration 0. + let clock = + ManualSlotClock::new(Slot::new(1), Duration::from_secs(0), Duration::from_secs(1)); + assert_eq!(clock.start_of(Slot::new(0)), None); + assert_eq!(clock.start_of(Slot::new(1)), Some(Duration::from_secs(0))); + assert_eq!(clock.start_of(Slot::new(2)), Some(Duration::from_secs(1))); + + // Genesis slot 1 and genesis duration 10. + let clock = ManualSlotClock::new( + Slot::new(1), + Duration::from_secs(10), + Duration::from_secs(1), + ); + assert_eq!(clock.start_of(Slot::new(0)), None); + assert_eq!(clock.start_of(Slot::new(1)), Some(Duration::from_secs(10))); + assert_eq!(clock.start_of(Slot::new(2)), Some(Duration::from_secs(11))); + } + + #[test] + fn test_duration_to_next_slot() { + let slot_duration = Duration::from_secs(1); + + // Genesis time is now. + let clock = ManualSlotClock::new(Slot::new(0), Duration::from_secs(0), slot_duration); + *clock.current_time.write() = Duration::from_secs(0); + assert_eq!(clock.duration_to_next_slot(), Some(Duration::from_secs(1))); + + // Genesis time is in the future. + let clock = ManualSlotClock::new(Slot::new(0), Duration::from_secs(10), slot_duration); + *clock.current_time.write() = Duration::from_secs(0); + assert_eq!(clock.duration_to_next_slot(), Some(Duration::from_secs(10))); + + // Genesis time is in the past. + let clock = ManualSlotClock::new(Slot::new(0), Duration::from_secs(0), slot_duration); + *clock.current_time.write() = Duration::from_secs(10); + assert_eq!(clock.duration_to_next_slot(), Some(Duration::from_secs(1))); + } + + #[test] + fn test_duration_to_next_epoch() { + let slot_duration = Duration::from_secs(1); + let slots_per_epoch = 32; + + // Genesis time is now. + let clock = ManualSlotClock::new(Slot::new(0), Duration::from_secs(0), slot_duration); + *clock.current_time.write() = Duration::from_secs(0); + assert_eq!( + clock.duration_to_next_epoch(slots_per_epoch), + Some(Duration::from_secs(32)) + ); + + // Genesis time is in the future. + let clock = ManualSlotClock::new(Slot::new(0), Duration::from_secs(10), slot_duration); + *clock.current_time.write() = Duration::from_secs(0); + assert_eq!( + clock.duration_to_next_epoch(slots_per_epoch), + Some(Duration::from_secs(10)) + ); + + // Genesis time is in the past. + let clock = ManualSlotClock::new(Slot::new(0), Duration::from_secs(0), slot_duration); + *clock.current_time.write() = Duration::from_secs(10); + assert_eq!( + clock.duration_to_next_epoch(slots_per_epoch), + Some(Duration::from_secs(22)) + ); + + // Genesis time is in the past. + let clock = ManualSlotClock::new( + Slot::new(0), + Duration::from_secs(0), + Duration::from_secs(12), + ); + *clock.current_time.write() = Duration::from_secs(72_333); + assert!(clock.duration_to_next_epoch(slots_per_epoch).is_some(),); + } + + #[test] + fn test_tolerance() { + let clock = ManualSlotClock::new( + Slot::new(0), + Duration::from_secs(10), + Duration::from_secs(1), + ); + + // Set clock to the 0'th slot. + *clock.current_time.write() = Duration::from_secs(10); + assert_eq!( + clock + .now_with_future_tolerance(Duration::from_secs(0)) + .unwrap(), + Slot::new(0), + "future tolerance of zero should return current slot" + ); + assert_eq!( + clock + .now_with_past_tolerance(Duration::from_secs(0)) + .unwrap(), + Slot::new(0), + "past tolerance of zero should return current slot" + ); + assert_eq!( + clock + .now_with_future_tolerance(Duration::from_millis(10)) + .unwrap(), + Slot::new(0), + "insignificant future tolerance should return current slot" + ); + assert_eq!( + clock + .now_with_past_tolerance(Duration::from_millis(10)) + .unwrap(), + Slot::new(0), + "past tolerance that precedes genesis should return genesis slot" + ); + + // Set clock to part-way through the 1st slot. + *clock.current_time.write() = Duration::from_millis(11_200); + assert_eq!( + clock + .now_with_future_tolerance(Duration::from_secs(0)) + .unwrap(), + Slot::new(1), + "future tolerance of zero should return current slot" + ); + assert_eq!( + clock + .now_with_past_tolerance(Duration::from_secs(0)) + .unwrap(), + Slot::new(1), + "past tolerance of zero should return current slot" + ); + assert_eq!( + clock + .now_with_future_tolerance(Duration::from_millis(800)) + .unwrap(), + Slot::new(2), + "significant future tolerance should return next slot" + ); + assert_eq!( + clock + .now_with_past_tolerance(Duration::from_millis(201)) + .unwrap(), + Slot::new(0), + "significant past tolerance should return previous slot" + ); + } } 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 a66b11265..81a5e5912 100644 --- a/eth2/utils/slot_clock/src/system_time_slot_clock.rs +++ b/eth2/utils/slot_clock/src/system_time_slot_clock.rs @@ -43,6 +43,10 @@ impl SlotClock for SystemTimeSlotClock { fn slot_duration(&self) -> Duration { self.clock.slot_duration() } + + fn genesis_slot(&self) -> Slot { + self.clock.genesis_slot() + } } #[cfg(test)]