AURA: Switch to CurrentSlot instead of LastTimestamp

bin/node-template/runtime/src/lib.rs

@@ -273,7 +273,7 @@ construct_runtime!(
 		System: frame_system::{Module, Call, Config, Storage, Event<T>},
 		RandomnessCollectiveFlip: pallet_randomness_collective_flip::{Module, Call, Storage},
 		Timestamp: pallet_timestamp::{Module, Call, Storage, Inherent},
-		Aura: pallet_aura::{Module, Config<T>, Inherent},
+		Aura: pallet_aura::{Module, Config<T>},
 		Grandpa: pallet_grandpa::{Module, Call, Storage, Config, Event},
 		Balances: pallet_balances::{Module, Call, Storage, Config<T>, Event<T>},
 		TransactionPayment: pallet_transaction_payment::{Module, Storage},

frame/aura/Cargo.toml

@@ -15,7 +15,6 @@ targets = ["x86_64-unknown-linux-gnu"]
 [dependencies]
 sp-application-crypto = { version = "2.0.0", default-features = false, path = "../../primitives/application-crypto" }
 codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] }
-sp-inherents = { version = "2.0.0", default-features = false, path = "../../primitives/inherents" }
 sp-std = { version = "2.0.0", default-features = false, path = "../../primitives/std" }
 serde = { version = "1.0.101", optional = true }
 pallet-session = { version = "2.0.0", default-features = false, path = "../session" }
@@ -37,7 +36,6 @@ default = ["std"]
 std = [
 	"sp-application-crypto/std",
 	"codec/std",
-	"sp-inherents/std",
 	"sp-std/std",
 	"serde",
 	"sp-runtime/std",

frame/aura/src/lib.rs

@@ -34,32 +34,22 @@
 //!
 //! - [Timestamp](../pallet_timestamp/index.html): The Timestamp module is used in Aura to track
 //! consensus rounds (via `slots`).
-//!
-//! ## References
-//!
-//! If you're interested in hacking on this module, it is useful to understand the interaction with
-//! `substrate/primitives/inherents/src/lib.rs` and, specifically, the required implementation of
-//! [`ProvideInherent`](../sp_inherents/trait.ProvideInherent.html) and
-//! [`ProvideInherentData`](../sp_inherents/trait.ProvideInherentData.html) to create and check inherents.
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
-use sp_std::{result, prelude::*};
+use sp_std::prelude::*;
 use codec::{Encode, Decode};
 use frame_support::{Parameter, traits::{Get, FindAuthor, OneSessionHandler}, ConsensusEngineId};
 use sp_runtime::{
 	RuntimeAppPublic,
 	traits::{SaturatedConversion, Saturating, Zero, Member, IsMember}, generic::DigestItem,
 };
 use sp_timestamp::OnTimestampSet;
-use sp_inherents::{InherentIdentifier, InherentData, ProvideInherent, MakeFatalError};
-use sp_consensus_aura::{
-	AURA_ENGINE_ID, ConsensusLog, AuthorityIndex, Slot,
-	inherents::{INHERENT_IDENTIFIER, AuraInherentData},
-};
+use sp_consensus_aura::{AURA_ENGINE_ID, ConsensusLog, AuthorityIndex, Slot};
 
 mod mock;
 mod tests;
+pub mod migrations;
 
 pub use pallet::*;
 
@@ -79,7 +69,22 @@ pub mod pallet {
 	pub struct Pallet<T>(sp_std::marker::PhantomData<T>);
 
 	#[pallet::hooks]
-	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {}
+	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
+		fn on_initialize(_: T::BlockNumber) -> Weight {
+			if let Some(new_slot) = Self::current_slot_from_digests() {
+				let current_slot = CurrentSlot::<T>::get();
+
+				assert!(current_slot < new_slot, "Slot must increase");
+				CurrentSlot::<T>::put(new_slot);
+
+				// TODO [#3398] Generate offence report for all authorities that skipped their slots.
+
+				T::DbWeight::get().reads_writes(2, 1)
+			} else {
+				T::DbWeight::get().reads(1)
+			}
+		}
+	}
 
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {}
@@ -89,10 +94,12 @@ pub mod pallet {
 	#[pallet::getter(fn authorities)]
 	pub(super) type Authorities<T: Config> = StorageValue<_, Vec<T::AuthorityId>, ValueQuery>;
 
-	/// The last timestamp we have been notified of.
+	/// The current slot of this block.
+	///
+	/// This will be set in `on_initialize`.
 	#[pallet::storage]
-	#[pallet::getter(fn last_timestamp)]
-	pub(super) type LastTimestamp<T: Config> = StorageValue<_, T::Moment, ValueQuery>;
+	#[pallet::getter(fn current_slot)]
+	pub(super) type CurrentSlot<T: Config> = StorageValue<_, Slot, ValueQuery>;
 
 	#[pallet::genesis_config]
 	pub struct GenesisConfig<T: Config> {
@@ -132,6 +139,19 @@ impl<T: Config> Pallet<T> {
 		}
 	}
 
+	/// Get the current slot from the pre-runtime digests.
+	fn current_slot_from_digests() -> Option<Slot> {
+		let digest = frame_system::Pallet::<T>::digest();
+		let pre_runtime_digests = digest.logs.iter().filter_map(|d| d.as_pre_runtime());
+		for (id, mut data) in pre_runtime_digests {
+			if id == AURA_ENGINE_ID {
+				return Slot::decode(&mut data).ok();
+			}
+		}
+
+		None
+	}
+
 	/// Determine the Aura slot-duration based on the Timestamp module configuration.
 	pub fn slot_duration() -> T::Moment {
 		// we double the minimum block-period so each author can always propose within
@@ -224,49 +244,12 @@ impl<T: Config> IsMember<T::AuthorityId> for Pallet<T> {
 
 impl<T: Config> OnTimestampSet<T::Moment> for Pallet<T> {
 	fn on_timestamp_set(moment: T::Moment) {
-		let last = Self::last_timestamp();
-		LastTimestamp::<T>::put(moment);
-
-		if last.is_zero() {
-			return;
-		}
-
 		let slot_duration = Self::slot_duration();
 		assert!(!slot_duration.is_zero(), "Aura slot duration cannot be zero.");
 
-		let last_slot = last / slot_duration;
-		let cur_slot = moment / slot_duration;
-
-		assert!(last_slot < cur_slot, "Only one block may be authored per slot.");
-
-		// TODO [#3398] Generate offence report for all authorities that skipped their slots.
-	}
-}
-
-impl<T: Config> ProvideInherent for Pallet<T> {
-	type Call = pallet_timestamp::Call<T>;
-	type Error = MakeFatalError<sp_inherents::Error>;
-	const INHERENT_IDENTIFIER: InherentIdentifier = INHERENT_IDENTIFIER;
+		let timestamp_slot = moment / slot_duration;
+		let timestamp_slot = Slot::from(timestamp_slot.saturated_into::<u64>());
 
-	fn create_inherent(_: &InherentData) -> Option<Self::Call> {
-		None
-	}
-
-	/// Verify the validity of the inherent using the timestamp.
-	fn check_inherent(call: &Self::Call, data: &InherentData) -> result::Result<(), Self::Error> {
-		let timestamp = match call {
-			pallet_timestamp::Call::set(ref timestamp) => timestamp.clone(),
-			_ => return Ok(()),
-		};
-
-		let timestamp_based_slot = timestamp / Self::slot_duration();
-
-		let seal_slot = u64::from(data.aura_inherent_data()?).saturated_into();
-
-		if timestamp_based_slot == seal_slot {
-			Ok(())
-		} else {
-			Err(sp_inherents::Error::from("timestamp set in block doesn't match slot in seal").into())
-		}
+		assert!(CurrentSlot::<T>::get() == timestamp_slot, "Timestamp slot must match `CurrentSlot`");
 	}
 }

frame/aura/src/migrations.rs

@@ -0,0 +1,43 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2021 Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Migrations for the AURA pallet.
+
+use frame_support::{traits::Get, weights::Weight, pallet_prelude::*};
+
+struct __LastTimestamp<T>(sp_std::marker::PhantomData<T>);
+impl<T: RemoveLastTimestamp> frame_support::traits::StorageInstance for __LastTimestamp<T> {
+	fn pallet_prefix() -> &'static str { T::PalletPrefix::get() }
+	const STORAGE_PREFIX: &'static str = "LastTimestamp";
+}
+
+type LastTimestamp<T> = StorageValue<__LastTimestamp<T>, (), ValueQuery>;
+
+pub trait RemoveLastTimestamp: super::Config {
+	type PalletPrefix: Get<&'static str>;
+}
+
+/// Remove the `LastTimestamp` storage value.
+///
+/// This storage value was removed and replaced by `CurrentSlot`. As we only remove this storage
+/// value, it is safe to call this method multiple times.
+///
+/// This migration requires a type `T` that implements [`RemoveLastTimestamp`].
+pub fn remove_last_timestamp<T: RemoveLastTimestamp>() -> Weight {
+	LastTimestamp::<T>::kill();
+	T::DbWeight::get().writes(1)
+}

frame/aura/src/mock.rs

@@ -21,12 +21,8 @@
 
 use crate as pallet_aura;
 use sp_consensus_aura::ed25519::AuthorityId;
-use sp_runtime::{
-	traits::IdentityLookup,
-	testing::{Header, UintAuthorityId},
-};
+use sp_runtime::{traits::IdentityLookup, testing::{Header, UintAuthorityId}};
 use frame_support::{parameter_types, traits::GenesisBuild};
-use sp_io;
 use sp_core::H256;
 
 type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
@@ -40,7 +36,7 @@ frame_support::construct_runtime!(
 	{
 		System: frame_system::{Module, Call, Config, Storage, Event<T>},
 		Timestamp: pallet_timestamp::{Module, Call, Storage, Inherent},
-		Aura: pallet_aura::{Module, Call, Storage, Config<T>, Inherent},
+		Aura: pallet_aura::{Module, Call, Storage, Config<T>},
 	}
 );
 

frame/aura/src/tests.rs

@@ -24,7 +24,7 @@ use crate::mock::{Aura, new_test_ext};
 #[test]
 fn initial_values() {
 	new_test_ext(vec![0, 1, 2, 3]).execute_with(|| {
-		assert_eq!(Aura::last_timestamp(), 0u64);
+		assert_eq!(Aura::current_slot(), 0u64);
 		assert_eq!(Aura::authorities().len(), 4);
 	});
 }

primitives/consensus/aura/src/inherents.rs

@@ -48,6 +48,7 @@ impl AuraInherentData for InherentData {
 }
 
 /// Provides the slot duration inherent data for `Aura`.
+// TODO: Remove in the future. https://github.com/paritytech/substrate/issues/8029
 #[cfg(feature = "std")]
 pub struct InherentDataProvider {
 	slot_duration: u64,