Convert timestamp to unit type wrapper (#8333)

The timestamp inherent type was up to now just a simple `u64`. This
worked, but doesn't give you that much guarantees at compile time about
the type. This pr changes that by converting this type to a unit type
wrapper, similar to what we have done for `Slot`.

This is required for some future pr that touches quite a lot of the
inherents stuff :)

Besides this unit wrapper type, this pr also moves the `OnTimestampSet`
trait to `frame_support::traits`.

substrate/Cargo.lock

@@ -4546,7 +4546,6 @@ dependencies = [
  "sp-io",
  "sp-runtime",
  "sp-std",
- "sp-timestamp",
 ]
 
 [[package]]
@@ -4611,7 +4610,6 @@ dependencies = [
  "sp-session",
  "sp-staking",
  "sp-std",
- "sp-timestamp",
 ]
 
 [[package]]
@@ -9042,7 +9040,6 @@ dependencies = [
 name = "sp-timestamp"
 version = "3.0.0"
 dependencies = [
- "impl-trait-for-tuples",
  "parity-scale-codec",
  "sp-api",
  "sp-inherents",

substrate/client/consensus/aura/src/lib.rs

@@ -54,7 +54,7 @@ use sp_api::ProvideRuntimeApi;
 use sp_core::crypto::Pair;
 use sp_keystore::{SyncCryptoStorePtr, SyncCryptoStore};
 use sp_inherents::{InherentDataProviders, InherentData};
-use sp_timestamp::{TimestampInherentData, InherentType as TimestampInherent};
+use sp_timestamp::TimestampInherentData;
 use sc_consensus_slots::{SlotInfo, SlotCompatible, StorageChanges, BackoffAuthoringBlocksStrategy};
 use sc_telemetry::TelemetryHandle;
 use sp_consensus_slots::Slot;
@@ -111,12 +111,12 @@ impl SlotCompatible for AuraSlotCompatible {
 	fn extract_timestamp_and_slot(
 		&self,
 		data: &InherentData,
-	) -> Result<(TimestampInherent, AuraInherent, std::time::Duration), sp_consensus::Error> {
+	) -> Result<(u64, AuraInherent, std::time::Duration), sp_consensus::Error> {
 		data.timestamp_inherent_data()
 			.and_then(|t| data.aura_inherent_data().map(|a| (t, a)))
 			.map_err(Into::into)
 			.map_err(sp_consensus::Error::InherentData)
-			.map(|(x, y)| (x, y, Default::default()))
+			.map(|(x, y)| (*x, y, Default::default()))
 	}
 }
 

substrate/client/consensus/babe/src/lib.rs

@@ -97,7 +97,7 @@ use sp_consensus::{
 	SelectChain, SlotData, import_queue::{Verifier, BasicQueue, DefaultImportQueue, CacheKeyId},
 };
 use sp_consensus_babe::inherents::BabeInherentData;
-use sp_timestamp::{TimestampInherentData, InherentType as TimestampInherent};
+use sp_timestamp::TimestampInherentData;
 use sc_client_api::{
 	backend::AuxStore, BlockchainEvents, ProvideUncles,
 };
@@ -919,13 +919,13 @@ impl SlotCompatible for TimeSource {
 	fn extract_timestamp_and_slot(
 		&self,
 		data: &InherentData,
-	) -> Result<(TimestampInherent, Slot, std::time::Duration), sp_consensus::Error> {
+	) -> Result<(u64, Slot, std::time::Duration), sp_consensus::Error> {
 		trace!(target: "babe", "extract timestamp");
 		data.timestamp_inherent_data()
 			.and_then(|t| data.babe_inherent_data().map(|a| (t, a)))
 			.map_err(Into::into)
 			.map_err(sp_consensus::Error::InherentData)
-			.map(|(x, y)| (x, y, self.0.lock().0.take().unwrap_or_default()))
+			.map(|(x, y)| (*x, y, self.0.lock().0.take().unwrap_or_default()))
 	}
 }
 

substrate/client/consensus/manual-seal/src/consensus/babe.rs

@@ -221,7 +221,7 @@ impl<B, C> ConsensusDataProvider<B> for BabeConsensusDataProvider<B, C>
 
 		if !has_authority {
 			log::info!(target: "manual-seal", "authority not found");
-			let slot = inherents.timestamp_inherent_data()? / self.config.slot_duration;
+			let slot = *inherents.timestamp_inherent_data()? / self.config.slot_duration;
 			// manually hard code epoch descriptor
 			epoch_descriptor = match epoch_descriptor {
 				ViableEpochDescriptor::Signaled(identifier, _header) => {
@@ -293,7 +293,10 @@ impl ProvideInherentData for SlotTimestampProvider {
 
 	fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), sp_inherents::Error> {
 		// we update the time here.
-		let duration: InherentType = self.time.fetch_add(self.slot_duration, atomic::Ordering::SeqCst);
+		let duration: InherentType = self.time.fetch_add(
+			self.slot_duration,
+			atomic::Ordering::SeqCst,
+		).into();
 		inherent_data.put_data(INHERENT_IDENTIFIER, &duration)?;
 		Ok(())
 	}

substrate/client/consensus/pow/src/lib.rs

@@ -352,7 +352,7 @@ impl<B, I, C, S, Algorithm, CAW> BlockImport<B> for PowBlockImport<B, I, C, S, A
 				check_block.clone(),
 				BlockId::Hash(parent_hash),
 				inherent_data,
-				timestamp_now
+				*timestamp_now,
 			)?;
 
 			block.body = Some(check_block.deconstruct().1);

substrate/frame/aura/Cargo.toml

@@ -22,7 +22,6 @@ sp-runtime = { version = "3.0.0", default-features = false, path = "../../primit
 frame-support = { version = "3.0.0", default-features = false, path = "../support" }
 sp-consensus-aura = { version = "0.9.0", path = "../../primitives/consensus/aura", default-features = false }
 frame-system = { version = "3.0.0", default-features = false, path = "../system" }
-sp-timestamp = { version = "3.0.0", default-features = false, path = "../../primitives/timestamp" }
 pallet-timestamp = { version = "3.0.0", default-features = false, path = "../timestamp" }
 
 [dev-dependencies]
@@ -42,7 +41,6 @@ std = [
 	"frame-support/std",
 	"sp-consensus-aura/std",
 	"frame-system/std",
-	"sp-timestamp/std",
 	"pallet-timestamp/std",
 ]
 try-runtime = ["frame-support/try-runtime"]

substrate/frame/aura/src/lib.rs

@@ -39,12 +39,13 @@
 
 use sp_std::prelude::*;
 use codec::{Encode, Decode};
-use frame_support::{Parameter, traits::{Get, FindAuthor, OneSessionHandler}, ConsensusEngineId};
+use frame_support::{
+	Parameter, traits::{Get, FindAuthor, OneSessionHandler, OnTimestampSet}, ConsensusEngineId,
+};
 use sp_runtime::{
 	RuntimeAppPublic,
 	traits::{SaturatedConversion, Saturating, Zero, Member, IsMember}, generic::DigestItem,
 };
-use sp_timestamp::OnTimestampSet;
 use sp_consensus_aura::{AURA_ENGINE_ID, ConsensusLog, AuthorityIndex, Slot};
 
 mod mock;

substrate/frame/babe/Cargo.toml

@@ -29,7 +29,6 @@ sp-runtime = { version = "3.0.0", default-features = false, path = "../../primit
 sp-session = { version = "3.0.0", default-features = false, path = "../../primitives/session" }
 sp-staking = { version = "3.0.0", default-features = false, path = "../../primitives/staking" }
 sp-std = { version = "3.0.0", default-features = false, path = "../../primitives/std" }
-sp-timestamp = { version = "3.0.0", default-features = false, path = "../../primitives/timestamp" }
 log = { version = "0.4.14", default-features = false }
 
 [dev-dependencies]
@@ -59,7 +58,6 @@ std = [
 	"sp-session/std",
 	"sp-staking/std",
 	"sp-std/std",
-	"sp-timestamp/std",
 	"log/std",
 ]
 runtime-benchmarks = ["frame-benchmarking"]

substrate/frame/babe/src/lib.rs

@@ -25,7 +25,7 @@ use codec::{Decode, Encode};
 use frame_support::{
 	decl_error, decl_module, decl_storage,
 	dispatch::DispatchResultWithPostInfo,
-	traits::{FindAuthor, Get, KeyOwnerProofSystem, OneSessionHandler},
+	traits::{FindAuthor, Get, KeyOwnerProofSystem, OneSessionHandler, OnTimestampSet},
 	weights::{Pays, Weight},
 	Parameter,
 };
@@ -38,7 +38,6 @@ use sp_runtime::{
 };
 use sp_session::{GetSessionNumber, GetValidatorCount};
 use sp_std::prelude::*;
-use sp_timestamp::OnTimestampSet;
 
 use sp_consensus_babe::{
 	digests::{NextConfigDescriptor, NextEpochDescriptor, PreDigest},

substrate/frame/support/src/traits.rs

@@ -2291,6 +2291,13 @@ pub trait ExecuteBlock<Block: BlockT> {
 	fn execute_block(block: Block);
 }
 
+/// A trait which is called when the timestamp is set in the runtime.
+#[impl_trait_for_tuples::impl_for_tuples(30)]
+pub trait OnTimestampSet<Moment> {
+	/// Called when the timestamp is set.
+	fn on_timestamp_set(moment: Moment);
+}
+
 #[cfg(test)]
 mod tests {
 	use super::*;

substrate/frame/timestamp/src/lib.rs

@@ -97,7 +97,7 @@ pub mod weights;
 
 use sp_std::{result, cmp};
 use sp_inherents::InherentData;
-use frame_support::traits::{Time, UnixTime};
+use frame_support::traits::{Time, UnixTime, OnTimestampSet};
 use sp_runtime::{
 	RuntimeString,
 	traits::{
@@ -106,7 +106,6 @@ use sp_runtime::{
 };
 use sp_timestamp::{
 	InherentError, INHERENT_IDENTIFIER, InherentType,
-	OnTimestampSet,
 };
 pub use weights::WeightInfo;
 
@@ -214,16 +213,17 @@ pub mod pallet {
 		const INHERENT_IDENTIFIER: InherentIdentifier = INHERENT_IDENTIFIER;
 
 		fn create_inherent(data: &InherentData) -> Option<Self::Call> {
-			let data: T::Moment = extract_inherent_data(data)
+			let inherent_data = extract_inherent_data(data)
-				.expect("Gets and decodes timestamp inherent data")
+				.expect("Gets and decodes timestamp inherent data");
-				.saturated_into();
+			let data = (*inherent_data).saturated_into::<T::Moment>();
 
 			let next_time = cmp::max(data, Self::now() + T::MinimumPeriod::get());
 			Some(Call::set(next_time.into()))
 		}
 
 		fn check_inherent(call: &Self::Call, data: &InherentData) -> result::Result<(), Self::Error> {
-			const MAX_TIMESTAMP_DRIFT_MILLIS: u64 = 30 * 1000;
+			const MAX_TIMESTAMP_DRIFT_MILLIS: sp_timestamp::Timestamp =
+				sp_timestamp::Timestamp::new(30 * 1000);
 
 			let t: u64 = match call {
 				Call::set(ref t) => t.clone().saturated_into::<u64>(),
@@ -233,10 +233,10 @@ pub mod pallet {
 			let data = extract_inherent_data(data).map_err(|e| InherentError::Other(e))?;
 
 			let minimum = (Self::now() + T::MinimumPeriod::get()).saturated_into::<u64>();
-			if t > data + MAX_TIMESTAMP_DRIFT_MILLIS {
+			if t > *(data + MAX_TIMESTAMP_DRIFT_MILLIS) {
 				Err(InherentError::Other("Timestamp too far in future to accept".into()))
 			} else if t < minimum {
-				Err(InherentError::ValidAtTimestamp(minimum))
+				Err(InherentError::ValidAtTimestamp(minimum.into()))
 			} else {
 				Ok(())
 			}

substrate/primitives/consensus/aura/src/inherents.rs

@@ -88,7 +88,7 @@ impl ProvideInherentData for InherentDataProvider {
 		use sp_timestamp::TimestampInherentData;
 
 		let timestamp = inherent_data.timestamp_inherent_data()?;
-		let slot = timestamp / self.slot_duration;
+		let slot = *timestamp / self.slot_duration;
 		inherent_data.put_data(INHERENT_IDENTIFIER, &slot)
 	}
 

substrate/primitives/consensus/babe/src/inherents.rs

@@ -83,7 +83,7 @@ impl ProvideInherentData for InherentDataProvider {
 
 	fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), Error> {
 		let timestamp = inherent_data.timestamp_inherent_data()?;
-		let slot = timestamp / self.slot_duration;
+		let slot = *timestamp / self.slot_duration;
 		inherent_data.put_data(INHERENT_IDENTIFIER, &slot)
 	}
 

substrate/primitives/std/with_std.rs

@@ -37,6 +37,7 @@ pub use std::sync;
 pub use std::result;
 pub use std::slice;
 pub use std::str;
+pub use core::time;
 pub use std::vec;
 
 pub mod collections {

substrate/primitives/std/without_std.rs

@@ -39,6 +39,7 @@ pub use core::result;
 pub use core::slice;
 // Allow interpreting vectors of bytes as strings, but not constructing them.
 pub use core::str;
+pub use core::time;
 // We are trying to avoid certain things here, such as `core::string`
 // (if you need `String` you are probably doing something wrong, since
 // runtime doesn't require anything human readable).

substrate/primitives/timestamp/Cargo.toml

@@ -18,7 +18,6 @@ sp-std = { version = "3.0.0", default-features = false, path = "../std" }
 sp-runtime = { version = "3.0.0", default-features = false, path = "../runtime" }
 codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] }
 sp-inherents = { version = "3.0.0", default-features = false, path = "../inherents" }
-impl-trait-for-tuples = "0.2.1"
 wasm-timer = { version = "0.2", optional = true }
 
 [features]

substrate/primitives/timestamp/src/lib.rs

@@ -19,9 +19,7 @@
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
-use codec::Encode;
+use codec::{Encode, Decode};
-#[cfg(feature = "std")]
-use codec::Decode;
 #[cfg(feature = "std")]
 use sp_inherents::ProvideInherentData;
 use sp_inherents::{InherentIdentifier, IsFatalError, InherentData};
@@ -30,8 +28,83 @@ use sp_runtime::RuntimeString;
 
 /// The identifier for the `timestamp` inherent.
 pub const INHERENT_IDENTIFIER: InherentIdentifier = *b"timstap0";
+
 /// The type of the inherent.
-pub type InherentType = u64;
+pub type InherentType = Timestamp;
+
+/// Unit type wrapper that represents a timestamp.
+///
+/// Such a timestamp is the time since the UNIX_EPOCH in milliseconds at a given point in time.
+#[derive(Debug, Encode, Decode, Eq, Clone, Copy, Default, Ord)]
+pub struct Timestamp(u64);
+
+impl Timestamp {
+	/// Create new `Self`.
+	pub const fn new(inner: u64) -> Self {
+		Self(inner)
+	}
+}
+
+impl sp_std::ops::Deref for Timestamp {
+	type Target = u64;
+
+	fn deref(&self) -> &Self::Target {
+		&self.0
+	}
+}
+
+impl core::ops::Add for Timestamp {
+	type Output = Self;
+
+	fn add(self, other: Self) -> Self {
+		Self(self.0 + other.0)
+	}
+}
+
+impl core::ops::Add<u64> for Timestamp {
+	type Output = Self;
+
+	fn add(self, other: u64) -> Self {
+		Self(self.0 + other)
+	}
+}
+
+impl<T: Into<u64> + Copy> core::cmp::PartialEq<T> for Timestamp {
+	fn eq(&self, eq: &T) -> bool {
+		self.0 == (*eq).into()
+	}
+}
+
+impl<T: Into<u64> + Copy> core::cmp::PartialOrd<T> for Timestamp {
+	fn partial_cmp(&self, other: &T) -> Option<core::cmp::Ordering> {
+		self.0.partial_cmp(&(*other).into())
+	}
+}
+
+#[cfg(feature = "std")]
+impl std::fmt::Display for Timestamp {
+	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+		write!(f, "{}", self.0)
+	}
+}
+
+impl From<u64> for Timestamp {
+	fn from(timestamp: u64) -> Self {
+		Timestamp(timestamp)
+	}
+}
+
+impl From<Timestamp> for u64 {
+	fn from(timestamp: Timestamp) -> u64 {
+		timestamp.0
+	}
+}
+
+impl From<sp_std::time::Duration> for Timestamp {
+	fn from(duration: sp_std::time::Duration) -> Self {
+		Timestamp(duration.as_millis() as u64)
+	}
+}
 
 /// Errors that can occur while checking the timestamp inherent.
 #[derive(Encode, sp_runtime::RuntimeDebug)]
@@ -99,8 +172,7 @@ impl ProvideInherentData for InherentDataProvider {
 			.map_err(|_| {
 				"Current time is before unix epoch".into()
 			}).and_then(|d| {
-				let duration: InherentType = d.as_millis() as u64;
+				inherent_data.put_data(INHERENT_IDENTIFIER, &InherentType::from(d))
-				inherent_data.put_data(INHERENT_IDENTIFIER, &duration)
 			})
 	}
 
@@ -109,9 +181,3 @@ impl ProvideInherentData for InherentDataProvider {
 	}
 }
 
-
-/// A trait which is called when the timestamp is set.
-#[impl_trait_for_tuples::impl_for_tuples(30)]
-pub trait OnTimestampSet<Moment> {
-	fn on_timestamp_set(moment: Moment);
-}