Introduce a Slot type (#7997)

* Introduce a `Slot` type

Instead of having some type definition that only was used in half of the
code or directly using `u64`, this adds a new unit type wrapper `Slot`.
This makes it especially easy for the outside api to know what type is
expected/returned.

* Change epoch duratioC

* rename all instances of slot number to slot

* Make the constructor private

Co-authored-by: André Silva <andrerfosilva@gmail.com>

substrate/frame/babe/src/equivocation.rs

@@ -36,7 +36,7 @@
 //!
 
 use frame_support::{debug, traits::KeyOwnerProofSystem};
-use sp_consensus_babe::{EquivocationProof, SlotNumber};
+use sp_consensus_babe::{EquivocationProof, Slot};
 use sp_runtime::transaction_validity::{
 	InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity,
 	TransactionValidityError, ValidTransaction,
@@ -63,7 +63,7 @@ pub trait HandleEquivocation<T: Config> {
 	) -> Result<(), OffenceError>;
 
 	/// Returns true if all of the offenders at the given time slot have already been reported.
-	fn is_known_offence(offenders: &[T::KeyOwnerIdentification], time_slot: &SlotNumber) -> bool;
+	fn is_known_offence(offenders: &[T::KeyOwnerIdentification], time_slot: &Slot) -> bool;
 
 	/// Create and dispatch an equivocation report extrinsic.
 	fn submit_unsigned_equivocation_report(
@@ -83,7 +83,7 @@ impl<T: Config> HandleEquivocation<T> for () {
 		Ok(())
 	}
 
-	fn is_known_offence(_offenders: &[T::KeyOwnerIdentification], _time_slot: &SlotNumber) -> bool {
+	fn is_known_offence(_offenders: &[T::KeyOwnerIdentification], _time_slot: &Slot) -> bool {
 		true
 	}
 
@@ -136,7 +136,7 @@ where
 		R::report_offence(reporters, offence)
 	}
 
-	fn is_known_offence(offenders: &[T::KeyOwnerIdentification], time_slot: &SlotNumber) -> bool {
+	fn is_known_offence(offenders: &[T::KeyOwnerIdentification], time_slot: &Slot) -> bool {
 		R::is_known_offence(offenders, time_slot)
 	}
 
@@ -187,7 +187,7 @@ impl<T: Config> frame_support::unsigned::ValidateUnsigned for Module<T> {
 				// Only one equivocation report for the same offender at the same slot.
 				.and_provides((
 					equivocation_proof.offender.clone(),
-					equivocation_proof.slot_number,
+					*equivocation_proof.slot,
 				))
 				// We don't propagate this. This can never be included on a remote node.
 				.propagate(false)
@@ -212,7 +212,7 @@ impl<T: Config> frame_support::unsigned::ValidateUnsigned for Module<T> {
 			// and if so then we can discard the report.
 			let is_known_offence = T::HandleEquivocation::is_known_offence(
 				&[offender],
-				&equivocation_proof.slot_number,
+				&equivocation_proof.slot,
 			);
 
 			if is_known_offence {
@@ -230,8 +230,8 @@ impl<T: Config> frame_support::unsigned::ValidateUnsigned for Module<T> {
 ///
 /// When a validator released two or more blocks at the same slot.
 pub struct BabeEquivocationOffence<FullIdentification> {
-	/// A babe slot number in which this incident happened.
-	pub slot: SlotNumber,
+	/// A babe slot in which this incident happened.
+	pub slot: Slot,
 	/// The session index in which the incident happened.
 	pub session_index: SessionIndex,
 	/// The size of the validator set at the time of the offence.
@@ -244,7 +244,7 @@ impl<FullIdentification: Clone> Offence<FullIdentification>
 	for BabeEquivocationOffence<FullIdentification>
 {
 	const ID: Kind = *b"babe:equivocatio";
-	type TimeSlot = SlotNumber;
+	type TimeSlot = Slot;
 
 	fn offenders(&self) -> Vec<FullIdentification> {
 		vec![self.offender.clone()]

substrate/frame/babe/src/lib.rs

@@ -43,7 +43,7 @@ use sp_timestamp::OnTimestampSet;
 use sp_consensus_babe::{
 	digests::{NextConfigDescriptor, NextEpochDescriptor, PreDigest},
 	inherents::{BabeInherentData, INHERENT_IDENTIFIER},
-	BabeAuthorityWeight, ConsensusLog, Epoch, EquivocationProof, SlotNumber, BABE_ENGINE_ID,
+	BabeAuthorityWeight, ConsensusLog, Epoch, EquivocationProof, Slot, BABE_ENGINE_ID,
 };
 use sp_consensus_vrf::schnorrkel;
 use sp_inherents::{InherentData, InherentIdentifier, MakeFatalError, ProvideInherent};
@@ -66,7 +66,7 @@ pub trait Config: pallet_timestamp::Config {
 	/// The amount of time, in slots, that each epoch should last.
 	/// NOTE: Currently it is not possible to change the epoch duration after
 	/// the chain has started. Attempting to do so will brick block production.
-	type EpochDuration: Get<SlotNumber>;
+	type EpochDuration: Get<u64>;
 
 	/// The expected average block time at which BABE should be creating
 	/// blocks. Since BABE is probabilistic it is not trivial to figure out
@@ -168,10 +168,10 @@ decl_storage! {
 
 		/// The slot at which the first epoch actually started. This is 0
 		/// until the first block of the chain.
-		pub GenesisSlot get(fn genesis_slot): u64;
+		pub GenesisSlot get(fn genesis_slot): Slot;
 
 		/// Current slot number.
-		pub CurrentSlot get(fn current_slot): u64;
+		pub CurrentSlot get(fn current_slot): Slot;
 
 		/// The epoch randomness for the *current* epoch.
 		///
@@ -403,7 +403,7 @@ impl<T: Config> Module<T> {
 		// so we don't rotate the epoch.
 		now != One::one() && {
 			let diff = CurrentSlot::get().saturating_sub(Self::current_epoch_start());
-			diff >= T::EpochDuration::get()
+			*diff >= T::EpochDuration::get()
 		}
 	}
 
@@ -424,7 +424,7 @@ impl<T: Config> Module<T> {
 	pub fn next_expected_epoch_change(now: T::BlockNumber) -> Option<T::BlockNumber> {
 		let next_slot = Self::current_epoch_start().saturating_add(T::EpochDuration::get());
 		next_slot
-			.checked_sub(CurrentSlot::get())
+			.checked_sub(*CurrentSlot::get())
 			.map(|slots_remaining| {
 				// This is a best effort guess. Drifts in the slot/block ratio will cause errors here.
 				let blocks_remaining: T::BlockNumber = slots_remaining.saturated_into();
@@ -490,10 +490,10 @@ impl<T: Config> Module<T> {
 		}
 	}
 
-	// finds the start slot of the current epoch. only guaranteed to
-	// give correct results after `do_initialize` of the first block
-	// in the chain (as its result is based off of `GenesisSlot`).
-	pub fn current_epoch_start() -> SlotNumber {
+	/// Finds the start slot of the current epoch. only guaranteed to
+	/// give correct results after `do_initialize` of the first block
+	/// in the chain (as its result is based off of `GenesisSlot`).
+	pub fn current_epoch_start() -> Slot {
 		Self::epoch_start(EpochIndex::get())
 	}
 
@@ -525,7 +525,7 @@ impl<T: Config> Module<T> {
 		}
 	}
 
-	fn epoch_start(epoch_index: u64) -> SlotNumber {
+	fn epoch_start(epoch_index: u64) -> Slot {
 		// (epoch_index * epoch_duration) + genesis_slot
 
 		const PROOF: &str = "slot number is u64; it should relate in some way to wall clock time; \
@@ -535,7 +535,7 @@ impl<T: Config> Module<T> {
 			.checked_mul(T::EpochDuration::get())
 			.expect(PROOF);
 
-		epoch_start.checked_add(GenesisSlot::get()).expect(PROOF)
+		epoch_start.checked_add(*GenesisSlot::get()).expect(PROOF).into()
 	}
 
 	fn deposit_consensus<U: Encode>(new: U) {
@@ -583,9 +583,9 @@ impl<T: Config> Module<T> {
 			// on the first non-zero block (i.e. block #1)
 			// this is where the first epoch (epoch #0) actually starts.
 			// we need to adjust internal storage accordingly.
-			if GenesisSlot::get() == 0 {
-				GenesisSlot::put(digest.slot_number());
-				debug_assert_ne!(GenesisSlot::get(), 0);
+			if *GenesisSlot::get() == 0 {
+				GenesisSlot::put(digest.slot());
+				debug_assert_ne!(*GenesisSlot::get(), 0);
 
 				// deposit a log because this is the first block in epoch #0
 				// we use the same values as genesis because we haven't collected any
@@ -599,11 +599,11 @@ impl<T: Config> Module<T> {
 			}
 
 			// the slot number of the current block being initialized
-			let current_slot = digest.slot_number();
+			let current_slot = digest.slot();
 
 			// how many slots were skipped between current and last block
 			let lateness = current_slot.saturating_sub(CurrentSlot::get() + 1);
-			let lateness = T::BlockNumber::from(lateness as u32);
+			let lateness = T::BlockNumber::from(*lateness as u32);
 
 			Lateness::<T>::put(lateness);
 			CurrentSlot::put(current_slot);
@@ -684,7 +684,7 @@ impl<T: Config> Module<T> {
 		key_owner_proof: T::KeyOwnerProof,
 	) -> DispatchResultWithPostInfo {
 		let offender = equivocation_proof.offender.clone();
-		let slot_number = equivocation_proof.slot_number;
+		let slot = equivocation_proof.slot;
 
 		// validate the equivocation proof
 		if !sp_consensus_babe::check_equivocation_proof(equivocation_proof) {
@@ -694,7 +694,7 @@ impl<T: Config> Module<T> {
 		let validator_set_count = key_owner_proof.validator_count();
 		let session_index = key_owner_proof.session();
 
-		let epoch_index = (slot_number.saturating_sub(GenesisSlot::get()) / T::EpochDuration::get())
+		let epoch_index = (*slot.saturating_sub(GenesisSlot::get()) / T::EpochDuration::get())
 			.saturated_into::<u32>();
 
 		// check that the slot number is consistent with the session index
@@ -709,7 +709,7 @@ impl<T: Config> Module<T> {
 			.ok_or(Error::<T>::InvalidKeyOwnershipProof)?;
 
 		let offence = BabeEquivocationOffence {
-			slot: slot_number,
+			slot,
 			validator_set_count,
 			offender,
 			session_index,
@@ -837,7 +837,7 @@ impl<T: Config> ProvideInherent for Module<T> {
 		let timestamp_based_slot = (timestamp / Self::slot_duration()).saturated_into::<u64>();
 		let seal_slot = data.babe_inherent_data()?;
 
-		if timestamp_based_slot == seal_slot {
+		if timestamp_based_slot == *seal_slot {
 			Ok(())
 		} else {
 			Err(sp_inherents::Error::from("timestamp set in block doesn't match slot in seal").into())

substrate/frame/babe/src/mock.rs

@@ -33,7 +33,7 @@ use frame_support::{
 };
 use sp_io;
 use sp_core::{H256, U256, crypto::{IsWrappedBy, KeyTypeId, Pair}};
-use sp_consensus_babe::{AuthorityId, AuthorityPair, SlotNumber};
+use sp_consensus_babe::{AuthorityId, AuthorityPair, Slot};
 use sp_consensus_vrf::schnorrkel::{VRFOutput, VRFProof};
 use sp_staking::SessionIndex;
 use pallet_staking::EraIndex;
@@ -255,14 +255,14 @@ pub fn go_to_block(n: u64, s: u64) {
 		System::parent_hash()
 	};
 
-	let pre_digest = make_secondary_plain_pre_digest(0, s);
+	let pre_digest = make_secondary_plain_pre_digest(0, s.into());
 
 	System::initialize(&n, &parent_hash, &pre_digest, InitKind::Full);
 	System::set_block_number(n);
 	Timestamp::set_timestamp(n);
 
 	if s > 1 {
-		CurrentSlot::put(s);
+		CurrentSlot::put(Slot::from(s));
 	}
 
 	System::on_initialize(n);
@@ -272,8 +272,8 @@ pub fn go_to_block(n: u64, s: u64) {
 
 /// Slots will grow accordingly to blocks
 pub fn progress_to_block(n: u64) {
-	let mut slot = Babe::current_slot() + 1;
-	for i in System::block_number()+1..=n {
+	let mut slot = u64::from(Babe::current_slot()) + 1;
+	for i in System::block_number() + 1 ..= n {
 		go_to_block(i, slot);
 		slot += 1;
 	}
@@ -294,14 +294,14 @@ pub fn start_era(era_index: EraIndex) {
 
 pub fn make_primary_pre_digest(
 	authority_index: sp_consensus_babe::AuthorityIndex,
-	slot_number: sp_consensus_babe::SlotNumber,
+	slot: sp_consensus_babe::Slot,
 	vrf_output: VRFOutput,
 	vrf_proof: VRFProof,
 ) -> Digest {
 	let digest_data = sp_consensus_babe::digests::PreDigest::Primary(
 		sp_consensus_babe::digests::PrimaryPreDigest {
 			authority_index,
-			slot_number,
+			slot,
 			vrf_output,
 			vrf_proof,
 		}
@@ -312,12 +312,12 @@ pub fn make_primary_pre_digest(
 
 pub fn make_secondary_plain_pre_digest(
 	authority_index: sp_consensus_babe::AuthorityIndex,
-	slot_number: sp_consensus_babe::SlotNumber,
+	slot: sp_consensus_babe::Slot,
 ) -> Digest {
 	let digest_data = sp_consensus_babe::digests::PreDigest::SecondaryPlain(
 		sp_consensus_babe::digests::SecondaryPlainPreDigest {
 			authority_index,
-			slot_number,
+			slot,
 		}
 	);
 	let log = DigestItem::PreRuntime(sp_consensus_babe::BABE_ENGINE_ID, digest_data.encode());
@@ -326,14 +326,14 @@ pub fn make_secondary_plain_pre_digest(
 
 pub fn make_secondary_vrf_pre_digest(
 	authority_index: sp_consensus_babe::AuthorityIndex,
-	slot_number: sp_consensus_babe::SlotNumber,
+	slot: sp_consensus_babe::Slot,
 	vrf_output: VRFOutput,
 	vrf_proof: VRFProof,
 ) -> Digest {
 	let digest_data = sp_consensus_babe::digests::PreDigest::SecondaryVRF(
 		sp_consensus_babe::digests::SecondaryVRFPreDigest {
 			authority_index,
-			slot_number,
+			slot,
 			vrf_output,
 			vrf_proof,
 		}
@@ -343,11 +343,11 @@ pub fn make_secondary_vrf_pre_digest(
 }
 
 pub fn make_vrf_output(
-	slot_number: u64,
+	slot: Slot,
 	pair: &sp_consensus_babe::AuthorityPair
 ) -> (VRFOutput, VRFProof, [u8; 32]) {
 	let pair = sp_core::sr25519::Pair::from_ref(pair).as_ref();
-	let transcript = sp_consensus_babe::make_transcript(&Babe::randomness(), slot_number, 0);
+	let transcript = sp_consensus_babe::make_transcript(&Babe::randomness(), slot, 0);
 	let vrf_inout = pair.vrf_sign(transcript);
 	let vrf_randomness: sp_consensus_vrf::schnorrkel::Randomness = vrf_inout.0
 		.make_bytes::<[u8; 32]>(&sp_consensus_babe::BABE_VRF_INOUT_CONTEXT);
@@ -435,7 +435,7 @@ pub fn new_test_ext_raw_authorities(authorities: Vec<AuthorityId>) -> sp_io::Tes
 pub fn generate_equivocation_proof(
 	offender_authority_index: u32,
 	offender_authority_pair: &AuthorityPair,
-	slot_number: SlotNumber,
+	slot: Slot,
 ) -> sp_consensus_babe::EquivocationProof<Header> {
 	use sp_consensus_babe::digests::CompatibleDigestItem;
 
@@ -444,7 +444,7 @@ pub fn generate_equivocation_proof(
 
 	let make_header = || {
 		let parent_hash = System::parent_hash();
-		let pre_digest = make_secondary_plain_pre_digest(offender_authority_index, slot_number);
+		let pre_digest = make_secondary_plain_pre_digest(offender_authority_index, slot);
 		System::initialize(&current_block, &parent_hash, &pre_digest, InitKind::Full);
 		System::set_block_number(current_block);
 		Timestamp::set_timestamp(current_block);
@@ -469,10 +469,10 @@ pub fn generate_equivocation_proof(
 	seal_header(&mut h2);
 
 	// restore previous runtime state
-	go_to_block(current_block, current_slot);
+	go_to_block(current_block, *current_slot);
 
 	sp_consensus_babe::EquivocationProof {
-		slot_number,
+		slot,
 		offender: offender_authority_pair.public(),
 		first_header: h1,
 		second_header: h2,

substrate/frame/babe/src/tests.rs

@@ -25,7 +25,7 @@ use frame_support::{
 };
 use mock::*;
 use pallet_session::ShouldEndSession;
-use sp_consensus_babe::AllowedSlots;
+use sp_consensus_babe::{AllowedSlots, Slot};
 use sp_core::crypto::Pair;
 
 const EMPTY_RANDOMNESS: [u8; 32] = [
@@ -62,7 +62,7 @@ fn first_block_epoch_zero_start() {
 	let (pairs, mut ext) = new_test_ext_with_pairs(4);
 
 	ext.execute_with(|| {
-		let genesis_slot = 100;
+		let genesis_slot = Slot::from(100);
 		let (vrf_output, vrf_proof, vrf_randomness) = make_vrf_output(genesis_slot, &pairs[0]);
 
 		let first_vrf = vrf_output;
@@ -73,7 +73,7 @@ fn first_block_epoch_zero_start() {
 			vrf_proof,
 		);
 
-		assert_eq!(Babe::genesis_slot(), 0);
+		assert_eq!(Babe::genesis_slot(), Slot::from(0));
 		System::initialize(
 			&1,
 			&Default::default(),
@@ -120,7 +120,7 @@ fn author_vrf_output_for_primary() {
 	let (pairs, mut ext) = new_test_ext_with_pairs(1);
 
 	ext.execute_with(|| {
-		let genesis_slot = 10;
+		let genesis_slot = Slot::from(10);
 		let (vrf_output, vrf_proof, vrf_randomness) = make_vrf_output(genesis_slot, &pairs[0]);
 		let primary_pre_digest = make_primary_pre_digest(0, genesis_slot, vrf_output, vrf_proof);
 
@@ -146,7 +146,7 @@ fn author_vrf_output_for_secondary_vrf() {
 	let (pairs, mut ext) = new_test_ext_with_pairs(1);
 
 	ext.execute_with(|| {
-		let genesis_slot = 10;
+		let genesis_slot = Slot::from(10);
 		let (vrf_output, vrf_proof, vrf_randomness) = make_vrf_output(genesis_slot, &pairs[0]);
 		let secondary_vrf_pre_digest = make_secondary_vrf_pre_digest(0, genesis_slot, vrf_output, vrf_proof);
 
@@ -170,7 +170,7 @@ fn author_vrf_output_for_secondary_vrf() {
 #[test]
 fn no_author_vrf_output_for_secondary_plain() {
 	new_test_ext(1).execute_with(|| {
-		let genesis_slot = 10;
+		let genesis_slot = Slot::from(10);
 		let secondary_plain_pre_digest = make_secondary_plain_pre_digest(0, genesis_slot);
 
 		System::initialize(
@@ -205,17 +205,17 @@ fn can_predict_next_epoch_change() {
 		assert_eq!(<Test as Config>::EpochDuration::get(), 3);
 		// this sets the genesis slot to 6;
 		go_to_block(1, 6);
-		assert_eq!(Babe::genesis_slot(), 6);
-		assert_eq!(Babe::current_slot(), 6);
+		assert_eq!(*Babe::genesis_slot(), 6);
+		assert_eq!(*Babe::current_slot(), 6);
 		assert_eq!(Babe::epoch_index(), 0);
 
 		progress_to_block(5);
 
 		assert_eq!(Babe::epoch_index(), 5 / 3);
-		assert_eq!(Babe::current_slot(), 10);
+		assert_eq!(*Babe::current_slot(), 10);
 
 		// next epoch change will be at
-		assert_eq!(Babe::current_epoch_start(), 9); // next change will be 12, 2 slots from now
+		assert_eq!(*Babe::current_epoch_start(), 9); // next change will be 12, 2 slots from now
 		assert_eq!(Babe::next_expected_epoch_change(System::block_number()), Some(5 + 2));
 	})
 }
@@ -226,8 +226,8 @@ fn can_enact_next_config() {
 		assert_eq!(<Test as Config>::EpochDuration::get(), 3);
 		// this sets the genesis slot to 6;
 		go_to_block(1, 6);
-		assert_eq!(Babe::genesis_slot(), 6);
-		assert_eq!(Babe::current_slot(), 6);
+		assert_eq!(*Babe::genesis_slot(), 6);
+		assert_eq!(*Babe::current_slot(), 6);
 		assert_eq!(Babe::epoch_index(), 0);
 		go_to_block(2, 7);
 
@@ -269,12 +269,12 @@ fn can_fetch_current_and_next_epoch_data() {
 
 		let current_epoch = Babe::current_epoch();
 		assert_eq!(current_epoch.epoch_index, 3);
-		assert_eq!(current_epoch.start_slot, 10);
+		assert_eq!(*current_epoch.start_slot, 10);
 		assert_eq!(current_epoch.authorities.len(), 5);
 
 		let next_epoch = Babe::next_epoch();
 		assert_eq!(next_epoch.epoch_index, 4);
-		assert_eq!(next_epoch.start_slot, 13);
+		assert_eq!(*next_epoch.start_slot, 13);
 		assert_eq!(next_epoch.authorities.len(), 5);
 
 		// the on-chain randomness should always change across epochs
@@ -572,7 +572,7 @@ fn report_equivocation_invalid_equivocation_proof() {
 			&offending_authority_pair,
 			CurrentSlot::get(),
 		);
-		equivocation_proof.slot_number = 0;
+		equivocation_proof.slot = Slot::from(0);
 		assert_invalid_equivocation(equivocation_proof.clone());
 
 		// different slot numbers in headers