Make Staking pallet using a proper Time module.

bin/node/runtime/src/lib.rs

@@ -274,7 +274,8 @@ parameter_types! {
 
 impl pallet_staking::Trait for Runtime {
 	type Currency = Balances;
-	type Time = Timestamp;
+	type UnixTime = Timestamp;
+	type DeprecatedTime = Timestamp;
 	type CurrencyToVote = CurrencyToVoteHandler;
 	type RewardRemainder = Treasury;
 	type Event = Event;

frame/staking/src/lib.rs

@@ -266,7 +266,7 @@ use frame_support::{
 	dispatch::DispatchResult,
 	traits::{
 		Currency, LockIdentifier, LockableCurrency,
-		WithdrawReasons, OnUnbalanced, Imbalance, Get, Time
+		WithdrawReasons, OnUnbalanced, Imbalance, Get, UnixTime, Time,
 	}
 };
 use pallet_session::historical::SessionManager;
@@ -301,14 +301,14 @@ pub type RewardPoint = u32;
 
 /// Information regarding the active era (era in used in session).
 #[derive(Encode, Decode, RuntimeDebug)]
-pub struct ActiveEraInfo<Moment> {
+pub struct ActiveEraInfo {
 	/// Index of era.
 	index: EraIndex,
-	/// Moment of start
+	/// Moment of start expresed as millisecond from `$UNIX_EPOCH`.
 	///
 	/// Start can be none if start hasn't been set for the era yet,
 	/// Start is set on the first on_finalize of the era to guarantee usage of `Time`.
-	start: Option<Moment>,
+	start: Option<u64>,
 }
 
 /// Reward points of an era. Used to split era total payout between validators.
@@ -565,7 +565,6 @@ type PositiveImbalanceOf<T> =
 	<<T as Trait>::Currency as Currency<<T as frame_system::Trait>::AccountId>>::PositiveImbalance;
 type NegativeImbalanceOf<T> =
 	<<T as Trait>::Currency as Currency<<T as frame_system::Trait>::AccountId>>::NegativeImbalance;
-type MomentOf<T> = <<T as Trait>::Time as Time>::Moment;
 
 /// Means for interacting with a specialized version of the `session` trait.
 ///
@@ -614,7 +613,7 @@ pub trait Trait: frame_system::Trait {
 	///
 	/// It is guaranteed to start being called from the first `on_finalize`. Thus value at genesis
 	/// is not used.
-	type Time: Time;
+	type UnixTime: UnixTime;
 
 	/// Convert a balance into a number used for election calculation.
 	/// This must fit into a `u64` but is allowed to be sensibly lossy.
@@ -660,6 +659,11 @@ pub trait Trait: frame_system::Trait {
 	/// For each validator only the `$MaxNominatorRewardedPerValidator` biggest stakers can claim
 	/// their reward. This used to limit the i/o cost for the nominator payout.
 	type MaxNominatorRewardedPerValidator: Get<u32>;
+
+	/// Deprecated Time.
+	///
+	/// Deprecated associated type wrongly used to query Time in millisecond.
+	type DeprecatedTime: Time;
 }
 
 /// Mode of era-forcing.
@@ -687,6 +691,7 @@ impl Default for Forcing {
 enum Releases {
 	V1_0_0,
 	V2_0_0,
+	V3_0_0,
 }
 
 impl Default for Releases {
@@ -747,7 +752,7 @@ decl_storage! {
 		///
 		/// The active era is the era currently rewarded.
 		/// Validator set of this era must be equal to `SessionInterface::validators`.
-		pub ActiveEra get(fn active_era): Option<ActiveEraInfo<MomentOf<T>>>;
+		pub ActiveEra get(fn active_era): Option<ActiveEraInfo>;
 
 		/// The session index at which the era start for the last `HISTORY_DEPTH` eras
 		pub ErasStartSessionIndex get(fn eras_start_session_index):
@@ -953,8 +958,9 @@ decl_module! {
 			// Set the start of the first era.
 			if let Some(mut active_era) = Self::active_era() {
 				if active_era.start.is_none() {
-					active_era.start = Some(T::Time::now());
-					<ActiveEra<T>>::put(active_era);
+					let now_as_millis_u64 = T::UnixTime::now().as_millis().saturated_into::<u64>();
+					active_era.start = Some(now_as_millis_u64);
+					ActiveEra::put(active_era);
 				}
 			}
 		}
@@ -1674,7 +1680,7 @@ impl<T: Trait> Module<T> {
 	/// * reset `active_era.start`,
 	/// * update `BondedEras` and apply slashes.
 	fn start_era(start_session: SessionIndex) {
-		let active_era = <ActiveEra<T>>::mutate(|active_era| {
+		let active_era = ActiveEra::mutate(|active_era| {
 			let new_index = active_era.as_ref().map(|info| info.index + 1).unwrap_or(0);
 			*active_era = Some(ActiveEraInfo {
 				index: new_index,
@@ -1712,12 +1718,12 @@ impl<T: Trait> Module<T> {
 	}
 
 	/// Compute payout for era.
-	fn end_era(active_era: ActiveEraInfo<MomentOf<T>>, _session_index: SessionIndex) {
+	fn end_era(active_era: ActiveEraInfo, _session_index: SessionIndex) {
 		// Note: active_era_start can be None if end era is called during genesis config.
 		if let Some(active_era_start) = active_era.start {
-			let now = T::Time::now();
+			let now_as_millis_u64 = T::UnixTime::now().as_millis().saturated_into::<u64>();
 
-			let era_duration = now - active_era_start;
+			let era_duration = now_as_millis_u64 - active_era_start;
 			let (total_payout, _max_payout) = inflation::compute_total_payout(
 				&T::RewardCurve::get(),
 				Self::eras_total_stake(&active_era.index),

frame/staking/src/migration/deprecated.rs

@@ -16,11 +16,25 @@
 
 /// Deprecated storages used for migration from v1.0.0 to v2.0.0 only.
 
-use crate::{Trait, BalanceOf, MomentOf, SessionIndex, Exposure, UnlockChunk};
+use crate::{Trait, BalanceOf, SessionIndex, Exposure, UnlockChunk, EraIndex};
 use codec::{Encode, Decode, HasCompact};
-use frame_support::{decl_module, decl_storage};
+use frame_support::{decl_module, decl_storage, traits::Time};
 use sp_std::prelude::*;
 
+pub type MomentOf<T> = <<T as Trait>::DeprecatedTime as Time>::Moment;
+
+/// Information regarding the active era (era in used in session).
+#[derive(Encode, Decode)]
+pub struct ActiveEraInfo<Moment> {
+	/// Index of era.
+	pub index: EraIndex,
+	/// Moment of start
+	///
+	/// Start can be none if start hasn't been set for the era yet,
+	/// Start is set on the first on_finalize of the era to guarantee usage of `Time`.
+	pub start: Option<Moment>,
+}
+
 /// Reward points of an era. Used to split era total payout between validators.
 #[derive(Encode, Decode, Default)]
 pub struct EraPoints {
@@ -69,5 +83,13 @@ decl_storage! {
 
 		/// Old upgrade flag.
 		pub IsUpgraded: bool;
+
+		/// Old active era information.
+		///
+		/// The active era information, it holds index and start.
+		///
+		/// The active era is the era currently rewarded.
+		/// Validator set of this era must be equal to `SessionInterface::validators`.
+		pub ActiveEra get(fn active_era): Option<ActiveEraInfo<MomentOf<T>>>;
 	}
 }

frame/staking/src/migration/mod.rs

@@ -14,13 +14,46 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
-//! Update storage from v1.0.0 to v2.0.0
-//!
-//! In old version the staking module has several issue about handling session delay, the
-//! current era was always considered the active one.
-//!
-//! After the migration the current era will still be considered the active one for the era of
-//! the upgrade. And the delay issue will be fixed when planning the next era.
+use super::*;
+mod deprecated;
+#[cfg(test)]
+mod tests;
+#[cfg(test)]
+mod test_upgrade_from_master_dataset;
+
+pub fn on_runtime_upgrade<T: Trait>() {
+	match StorageVersion::get() {
+		Releases::V3_0_0 => return,
+		Releases::V2_0_0 => upgrade_v2_to_v3::<T>(),
+		Releases::V1_0_0 => upgrade_v1_to_v2::<T>(),
+	}
+}
+
+/// Update storage from v2.0.0 to v3.0.0
+///
+/// Update ActiveEra storage.
+///
+/// It reset the start of the active era to now because there is no guarantee that the
+/// DeprecatedTime implementation was time since unix epoch.
+fn upgrade_v2_to_v3<T: Trait>() {
+	if let Some(old_active_era) = deprecated::ActiveEra::<T>::get() {
+		let now_as_millis_u64 = T::UnixTime::now().as_millis().saturated_into::<u64>();
+		<Module<T> as Store>::ActiveEra::put(ActiveEraInfo {
+			index: old_active_era.index,
+			start: Some(now_as_millis_u64),
+		});
+		// Note: we don't kill deprecated storage because both the deprecated one and the current
+		// one are at the same storage value.
+	}
+}
+
+/// Update storage from v1.0.0 to v2.0.0
+///
+/// In old version the staking module has several issue about handling session delay, the
+/// current era was always considered the active one.
+///
+/// After the migration the current era will still be considered the active one for the era of
+/// the upgrade. And the delay issue will be fixed when planning the next era.
 // * create:
 //   * ActiveEraStart
 //   * ErasRewardPoints
@@ -38,29 +71,14 @@
 //   * CurrentEraStart
 //   * CurrentEraStartSessionIndex
 //   * CurrentEraPointsEarned
-
-use super::*;
-mod deprecated;
-#[cfg(test)]
-mod tests;
-#[cfg(test)]
-mod test_upgrade_from_master_dataset;
-
-pub fn on_runtime_upgrade<T: Trait>() {
-	match StorageVersion::get() {
-		Releases::V2_0_0 => return,
-		Releases::V1_0_0 => upgrade_v1_to_v2::<T>(),
-	}
-}
-
 fn upgrade_v1_to_v2<T: Trait>() {
 	deprecated::IsUpgraded::kill();
 
 	let current_era_start_index = deprecated::CurrentEraStartSessionIndex::get();
 	let current_era = <Module<T> as Store>::CurrentEra::get().unwrap_or(0);
 	let current_era_start = deprecated::CurrentEraStart::<T>::get();
 	<Module<T> as Store>::ErasStartSessionIndex::insert(current_era, current_era_start_index);
-	<Module<T> as Store>::ActiveEra::put(ActiveEraInfo {
+	deprecated::ActiveEra::<T>::put(deprecated::ActiveEraInfo {
 		index: current_era,
 		start: Some(current_era_start),
 	});

frame/staking/src/migration/tests.rs

@@ -4,9 +4,10 @@ use frame_support::storage::migration::*;
 use sp_core::hashing::blake2_256;
 use super::test_upgrade_from_master_dataset;
 use sp_runtime::traits::OnRuntimeUpgrade;
+use crate::migration::deprecated;
 
 #[test]
-fn upgrade_works() {
+fn upgrade_from_v1_to_v2_works() {
 	ExtBuilder::default().build().execute_with(|| {
 		start_era(3);
 
@@ -15,7 +16,7 @@ fn upgrade_works() {
 		// Insert fake data to check the migration
 		put_storage_value::<Vec<AccountId>>(b"Staking", b"CurrentElected", b"", vec![21, 31]);
 		put_storage_value::<SessionIndex>(b"Staking", b"CurrentEraStartSessionIndex", b"", 5);
-		put_storage_value::<MomentOf<Test>>(b"Staking", b"CurrentEraStart", b"", 777);
+		put_storage_value::<deprecated::MomentOf<Test>>(b"Staking", b"CurrentEraStart", b"", 777);
 		put_storage_value(
 			b"Staking", b"Stakers", &blake2_256(&11u64.encode()),
 			Exposure::<AccountId, Balance> {
@@ -57,8 +58,8 @@ fn upgrade_works() {
 			total: 12,
 			individual: vec![(21, 2), (31, 10)].into_iter().collect(),
 		});
-		assert_eq!(<Staking as Store>::ActiveEra::get().unwrap().index, 3);
-		assert_eq!(<Staking as Store>::ActiveEra::get().unwrap().start, Some(777));
+		assert_eq!(deprecated::ActiveEra::<Test>::get().unwrap().index, 3);
+		assert_eq!(deprecated::ActiveEra::<Test>::get().unwrap().start, Some(777));
 		assert_eq!(<Staking as Store>::CurrentEra::get().unwrap(), 3);
 		assert_eq!(<Staking as Store>::ErasStakers::get(3, 11), Exposure {
 			total: 0,
@@ -98,7 +99,7 @@ fn upgrade_works() {
 
 // Test that an upgrade from previous test environment works.
 #[test]
-fn test_upgrade_from_master_works() {
+fn test_upgrade_from_v1_to_v2_from_master_works() {
 	let data_sets = &[
 		test_upgrade_from_master_dataset::_0,
 		test_upgrade_from_master_dataset::_1,
@@ -139,7 +140,7 @@ fn test_upgrade_from_master_works() {
 			assert!(<Staking as Store>::StorageVersion::get() == Releases::V2_0_0);
 
 			// Check ActiveEra and CurrentEra
-			let active_era = Staking::active_era().unwrap().index;
+			let active_era = deprecated::ActiveEra::<Test>::get().unwrap().index;
 			let current_era = Staking::current_era().unwrap();
 			assert!(current_era == active_era);
 			assert!(current_era == old_current_era);
@@ -218,3 +219,21 @@ fn test_upgrade_from_master_works() {
 		});
 	}
 }
+
+#[test]
+fn upgrade_from_v2_to_v3() {
+	ExtBuilder::default().build().execute_with(|| {
+		Timestamp::set_timestamp(51);
+
+		deprecated::ActiveEra::<Test>::put(deprecated::ActiveEraInfo {
+			index: 10,
+			start: Some(100),
+		});
+		StorageVersion::put(Releases::V2_0_0);
+
+		Staking::on_runtime_upgrade();
+
+		assert_eq!(ActiveEra::get().unwrap().index, 10);
+		assert_eq!(ActiveEra::get().unwrap().start, Some(51));
+	});
+}

frame/staking/src/mock.rs

@@ -202,8 +202,9 @@ parameter_types! {
 	pub const MaxNominatorRewardedPerValidator: u32 = 64;
 }
 impl Trait for Test {
-	type Currency = pallet_balances::Module<Self>;
-	type Time = pallet_timestamp::Module<Self>;
+	type Currency = Balances;
+	type UnixTime = Timestamp;
+	type DeprecatedTime = Timestamp;
 	type CurrencyToVote = CurrencyToVoteHandler;
 	type RewardRemainder = ();
 	type Event = ();

frame/support/src/traits.rs

@@ -785,6 +785,12 @@ pub trait Time {
 	fn now() -> Self::Moment;
 }
 
+/// Trait to deal with unix time.
+pub trait UnixTime {
+	/// Return duration since `SystemTime::UNIX_EPOCH`.
+	fn now() -> core::time::Duration;
+}
+
 impl WithdrawReasons {
 	/// Choose all variants except for `one`.
 	///

frame/timestamp/src/lib.rs

@@ -95,15 +95,17 @@ mod benchmarking;
 
 use sp_std::{result, cmp};
 use sp_inherents::{ProvideInherent, InherentData, InherentIdentifier};
-use frame_support::{Parameter, decl_storage, decl_module};
-use frame_support::traits::{Time, Get};
+use frame_support::{
+	Parameter, decl_storage, decl_module, debug,
+	traits::{Time, UnixTime, Get},
+	weights::SimpleDispatchInfo,
+};
 use sp_runtime::{
 	RuntimeString,
 	traits::{
 		AtLeast32Bit, Zero, SaturatedConversion, Scale
 	}
 };
-use frame_support::weights::SimpleDispatchInfo;
 use frame_system::ensure_none;
 use sp_timestamp::{
 	InherentError, INHERENT_IDENTIFIER, InherentType,
@@ -239,6 +241,23 @@ impl<T: Trait> Time for Module<T> {
 	}
 }
 
+/// Before the timestamp inherent is applied, it returns the time of previous block.
+///
+/// On genesis the time returned is not valid.
+impl<T: Trait> UnixTime for Module<T> {
+	fn now() -> core::time::Duration {
+		// now is duration since unix epoch in millisecond as documented in
+		// `sp_timestamp::InherentDataProvider`.
+		let now = Self::now();
+		if now == T::Moment::zero() {
+			debug::error!(
+				"`pallet_timestamp::UnixTime::now` is called at genesis, invalid value returned: 0"
+			);
+		}
+		core::time::Duration::from_millis(now.saturated_into::<u64>())
+	}
+}
+
 #[cfg(test)]
 mod tests {
 	use super::*;

primitives/timestamp/src/lib.rs

@@ -77,6 +77,7 @@ impl TimestampInherentData for InherentData {
 	}
 }
 
+/// Provide duration since unix epoch in millisecond for timestamp inherent.
 #[cfg(feature = "std")]
 pub struct InherentDataProvider;