aura-ext: check slot in consensus hook and remove all CheckInherents logic (#2658)

* aura-ext: check slot in consensus hook

* convert relay chain slot

* Make relay chain slot duration generic

* use fixed velocity hook for pallets with aura

* purge timestamp inherent

* fix warning

* adjust runtime tests

* fix slots in tests

* Make `xcm-emulator` test pass for new consensus hook (#2722)

* add pallets on_initialize

* tests pass

* add AuraExt on_init

* ".git/.scripts/commands/fmt/fmt.sh"

---------

Co-authored-by: command-bot <>

---------

Co-authored-by: Ignacio Palacios <[email protected]>

Author: slumber

Cargo.lock

@@ -32,7 +32,6 @@ members = [
 	"parachain-template/runtime",
 	"primitives/core",
 	"primitives/parachain-inherent",
-	"primitives/timestamp",
 	"primitives/utility",
 	"polkadot-parachain",
 	"parachains/common",

Cargo.toml

@@ -13,6 +13,7 @@ scale-info = { version = "2.7.0", default-features = false, features = ["derive"
 frame-support = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" }
 frame-system = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" }
 pallet-aura = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" }
+pallet-timestamp= { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" }
 sp-application-crypto = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" }
 sp-consensus-aura = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" }
 sp-runtime = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" }

pallets/aura-ext/Cargo.toml

@@ -19,28 +19,51 @@
 //!
 //! The velocity `V` refers to the rate of block processing by the relay chain.
 
-use super::pallet;
+use super::{pallet, Aura};
 use cumulus_pallet_parachain_system::{
 	consensus_hook::{ConsensusHook, UnincludedSegmentCapacity},
 	relay_state_snapshot::RelayChainStateProof,
 };
 use frame_support::pallet_prelude::*;
+use sp_consensus_aura::{Slot, SlotDuration};
 use sp_std::{marker::PhantomData, num::NonZeroU32};
 
+const MILLIS_PER_SECOND: u64 = 1000;
+
 /// A consensus hook for a fixed block processing velocity and unincluded segment capacity.
-pub struct FixedVelocityConsensusHook<T, const V: u32, const C: u32>(PhantomData<T>);
+///
+/// Relay chain slot duration must be provided in seconds.
+pub struct FixedVelocityConsensusHook<
+	T,
+	const RELAY_CHAIN_SLOT_DURATION: u32,
+	const V: u32,
+	const C: u32,
+>(PhantomData<T>);
 
-impl<T: pallet::Config, const V: u32, const C: u32> ConsensusHook
-	for FixedVelocityConsensusHook<T, V, C>
+impl<T: pallet::Config, const RELAY_CHAIN_SLOT_DURATION: u32, const V: u32, const C: u32>
+	ConsensusHook for FixedVelocityConsensusHook<T, RELAY_CHAIN_SLOT_DURATION, V, C>
+where
+	<T as pallet_timestamp::Config>::Moment: Into<u64>,
 {
 	// Validates the number of authored blocks within the slot with respect to the `V + 1` limit.
-	fn on_state_proof(_state_proof: &RelayChainStateProof) -> (Weight, UnincludedSegmentCapacity) {
+	fn on_state_proof(state_proof: &RelayChainStateProof) -> (Weight, UnincludedSegmentCapacity) {
 		// Ensure velocity is non-zero.
 		let velocity = V.max(1);
+		let relay_chain_slot = state_proof.read_slot().expect("failed to read relay chain slot");
 
-		let authored = pallet::Pallet::<T>::slot_info()
-			.map(|(_slot, authored)| authored)
+		let (slot, authored) = pallet::Pallet::<T>::slot_info()
 			.expect("slot info is inserted on block initialization");
+
+		// Convert relay chain timestamp.
+		let relay_chain_timestamp = u64::from(RELAY_CHAIN_SLOT_DURATION)
+			.saturating_mul(*relay_chain_slot)
+			.saturating_mul(MILLIS_PER_SECOND);
+		let para_slot_duration = SlotDuration::from_millis(Aura::<T>::slot_duration().into());
+		let para_slot_from_relay =
+			Slot::from_timestamp(relay_chain_timestamp.into(), para_slot_duration);
+
+		// Perform checks.
+		assert_eq!(slot, para_slot_from_relay, "slot number mismatch");
 		if authored > velocity + 1 {
 			panic!("authored blocks limit is reached for the slot")
 		}

pallets/aura-ext/src/consensus_hook.rs

@@ -25,11 +25,9 @@
 //! ```
 //!# struct Runtime;
 //!# struct Executive;
-//!# struct CheckInherents;
 //! cumulus_pallet_parachain_system::register_validate_block! {
 //!     Runtime = Runtime,
 //!     BlockExecutor = cumulus_pallet_aura_ext::BlockExecutor::<Runtime, Executive>,
-//!     CheckInherents = CheckInherents,
 //! }
 //! ```
 

pallets/aura-ext/src/lib.rs

@@ -25,20 +25,17 @@ use syn::{
 mod keywords {
 	syn::custom_keyword!(Runtime);
 	syn::custom_keyword!(BlockExecutor);
-	syn::custom_keyword!(CheckInherents);
 }
 
 struct Input {
 	runtime: Path,
 	block_executor: Path,
-	check_inherents: Path,
 }
 
 impl Parse for Input {
 	fn parse(input: ParseStream) -> Result<Self, Error> {
 		let mut runtime = None;
 		let mut block_executor = None;
-		let mut check_inherents = None;
 
 		fn parse_inner<KW: Parse + Spanned>(
 			input: ParseStream,
@@ -59,15 +56,13 @@ impl Parse for Input {
 			}
 		}
 
-		while runtime.is_none() || block_executor.is_none() || check_inherents.is_none() {
+		while runtime.is_none() || block_executor.is_none() {
 			let lookahead = input.lookahead1();
 
 			if lookahead.peek(keywords::Runtime) {
 				parse_inner::<keywords::Runtime>(input, &mut runtime)?;
 			} else if lookahead.peek(keywords::BlockExecutor) {
 				parse_inner::<keywords::BlockExecutor>(input, &mut block_executor)?;
-			} else if lookahead.peek(keywords::CheckInherents) {
-				parse_inner::<keywords::CheckInherents>(input, &mut check_inherents)?;
 			} else {
 				return Err(lookahead.error())
 			}
@@ -81,7 +76,6 @@ impl Parse for Input {
 		Ok(Self {
 			runtime: runtime.expect("Everything is parsed before; qed"),
 			block_executor: block_executor.expect("Everything is parsed before; qed"),
-			check_inherents: check_inherents.expect("Everything is parsed before; qed"),
 		})
 	}
 }
@@ -97,7 +91,7 @@ fn crate_() -> Result<Ident, Error> {
 
 #[proc_macro]
 pub fn register_validate_block(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
-	let Input { runtime, check_inherents, block_executor } = match syn::parse(input) {
+	let Input { runtime, block_executor } = match syn::parse(input) {
 		Ok(t) => t,
 		Err(e) => return e.into_compile_error().into(),
 	};
@@ -133,7 +127,6 @@ pub fn register_validate_block(input: proc_macro::TokenStream) -> proc_macro::To
 						<#runtime as #crate_::validate_block::GetRuntimeBlockType>::RuntimeBlock,
 						#block_executor,
 						#runtime,
-						#check_inherents,
 					>(params);
 
 					#crate_::validate_block::polkadot_parachain::write_result(&res)

pallets/parachain-system/proc-macro/src/lib.rs

@@ -48,7 +48,7 @@ use frame_system::{ensure_none, ensure_root};
 use polkadot_parachain::primitives::RelayChainBlockNumber;
 use scale_info::TypeInfo;
 use sp_runtime::{
-	traits::{Block as BlockT, BlockNumberProvider, Hash},
+	traits::{BlockNumberProvider, Hash},
 	transaction_validity::{
 		InvalidTransaction, TransactionLongevity, TransactionSource, TransactionValidity,
 		ValidTransaction,
@@ -85,12 +85,10 @@ pub use consensus_hook::ConsensusHook;
 /// ```
 ///     struct BlockExecutor;
 ///     struct Runtime;
-///     struct CheckInherents;
 ///
 ///     cumulus_pallet_parachain_system::register_validate_block! {
 ///         Runtime = Runtime,
 ///         BlockExecutor = Executive,
-///         CheckInherents = CheckInherents,
 ///     }
 ///
 /// # fn main() {}
@@ -1387,18 +1385,6 @@ impl<T: Config> UpwardMessageSender for Pallet<T> {
 	}
 }
 
-/// Something that can check the inherents of a block.
-pub trait CheckInherents<Block: BlockT> {
-	/// Check all inherents of the block.
-	///
-	/// This function gets passed all the extrinsics of the block, so it is up to the callee to
-	/// identify the inherents. The `validation_data` can be used to access the
-	fn check_inherents(
-		block: &Block,
-		validation_data: &RelayChainStateProof,
-	) -> frame_support::inherent::CheckInherentsResult;
-}
-
 /// Something that should be informed about system related events.
 ///
 /// This includes events like [`on_validation_data`](Self::on_validation_data) that is being

pallets/parachain-system/src/lib.rs

@@ -61,20 +61,12 @@ fn with_externalities<F: FnOnce(&mut dyn Externalities) -> R, R>(f: F) -> R {
 /// we have the in-memory database that contains all the values from the state of the parachain
 /// that we require to verify the block.
 ///
-/// 5. We are going to run `check_inherents`. This is important to check stuff like the timestamp
-/// matching the real world time.
-///
-/// 6. The last step is to execute the entire block in the machinery we just have setup. Executing
+/// 5. The last step is to execute the entire block in the machinery we just have setup. Executing
 /// the blocks include running all transactions in the block against our in-memory database and
 /// ensuring that the final storage root matches the storage root in the header of the block. In the
 /// end we return back the [`ValidationResult`] with all the required information for the validator.
 #[doc(hidden)]
-pub fn validate_block<
-	B: BlockT,
-	E: ExecuteBlock<B>,
-	PSC: crate::Config,
-	CI: crate::CheckInherents<B>,
->(
+pub fn validate_block<B: BlockT, E: ExecuteBlock<B>, PSC: crate::Config>(
 	MemoryOptimizedValidationParams {
 		block_data,
 		parent_head,
@@ -158,27 +150,6 @@ where
 		sp_io::offchain_index::host_clear.replace_implementation(host_offchain_index_clear),
 	);
 
-	run_with_externalities::<B, _, _>(&backend, || {
-		let relay_chain_proof = crate::RelayChainStateProof::new(
-			PSC::SelfParaId::get(),
-			inherent_data.validation_data.relay_parent_storage_root,
-			inherent_data.relay_chain_state.clone(),
-		)
-		.expect("Invalid relay chain state proof");
-
-		let res = CI::check_inherents(&block, &relay_chain_proof);
-
-		if !res.ok() {
-			if log::log_enabled!(log::Level::Error) {
-				res.into_errors().for_each(|e| {
-					log::error!("Checking inherent with identifier `{:?}` failed", e.0)
-				});
-			}
-
-			panic!("Checking inherents failed");
-		}
-	});
-
 	run_with_externalities::<B, _, _>(&backend, || {
 		let head_data = HeadData(block.header().encode());