Remove the deprecated CheckInherents logic

cumulus/pallets/aura-ext/src/lib.rs

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

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

@@ -31,14 +31,12 @@ mod keywords {
 struct Input {
 	runtime: Path,
 	block_executor: Path,
-	check_inherents: Option<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,
@@ -67,7 +65,7 @@ impl Parse for Input {
 			} 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)?;
+				return Err(Error::new(input.span(), "`CheckInherents` is not supported anymore!"));
 			} else {
 				return Err(lookahead.error())
 			}
@@ -76,7 +74,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,
 		})
 	}
 }
@@ -92,7 +89,7 @@ fn crate_() -> Result<Ident, Error> {
 
 #[proc_macro]
 pub fn register_validate_block(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
-	let Input { runtime, block_executor, check_inherents } = match syn::parse(input) {
+	let Input { runtime, block_executor } = match syn::parse(input) {
 		Ok(t) => t,
 		Err(e) => return e.into_compile_error().into(),
 	};
@@ -102,17 +99,6 @@ pub fn register_validate_block(input: proc_macro::TokenStream) -> proc_macro::To
 		Err(e) => return e.into_compile_error().into(),
 	};
 
-	let check_inherents = match check_inherents {
-		Some(_check_inherents) => {
-			quote::quote! { #_check_inherents }
-		},
-		None => {
-			quote::quote! {
-				#crate_::DummyCheckInherents<<#runtime as #crate_::validate_block::GetRuntimeBlockType>::RuntimeBlock>
-			}
-		},
-	};
-
 	if cfg!(not(feature = "std")) {
 		quote::quote! {
 			#[doc(hidden)]
@@ -139,7 +125,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_primitives::write_result(&res)

cumulus/pallets/parachain-system/src/lib.rs

@@ -55,7 +55,7 @@ use polkadot_parachain_primitives::primitives::RelayChainBlockNumber;
 use polkadot_runtime_parachains::{FeeTracker, GetMinFeeFactor};
 use scale_info::TypeInfo;
 use sp_runtime::{
-	traits::{Block as BlockT, BlockNumberProvider, Hash},
+	traits::{BlockNumberProvider, Hash},
 	FixedU128, RuntimeDebug, SaturatedConversion,
 };
 use xcm::{latest::XcmHash, VersionedLocation, VersionedXcm, MAX_XCM_DECODE_DEPTH};
@@ -96,12 +96,10 @@ pub use consensus_hook::{ConsensusHook, ExpectParentIncluded};
 /// ```
 ///     struct BlockExecutor;
 ///     struct Runtime;
-///     struct CheckInherents;
 ///
 ///     cumulus_pallet_parachain_system::register_validate_block! {
 ///         Runtime = Runtime,
 ///         BlockExecutor = Executive,
-///         CheckInherents = CheckInherents,
 ///     }
 ///
 /// # fn main() {}
@@ -592,6 +590,10 @@ pub mod pallet {
 				collator_peer_id: _,
 			} = data;
 
+			// Ensure that the validation data is correct when run in the context of
+			// `validate_block`.
+			Self::validate_validation_data(&vfp);
+
 			// Check that the associated relay chain block number is as expected.
 			T::CheckAssociatedRelayNumber::check_associated_relay_number(
 				vfp.relay_parent_number,
@@ -1087,6 +1089,27 @@ impl<T: Config> GetChannelInfo for Pallet<T> {
 }
 
 impl<T: Config> Pallet<T> {
+	/// Validates the given [`PersistedValidationData`] against the data from the relay chain.
+	///
+	/// This function will actually only do the checks when running inside the context of
+	/// `validate_block`, as it needs to access the data passed by the relay chain.
+	fn validate_validation_data(validation_data: &PersistedValidationData) {
+		validate_block::with_validation_params(|params| {
+			assert_eq!(
+				params.parent_head, validation_data.parent_head.0,
+				"Parent head doesn't match"
+			);
+			assert_eq!(
+				params.relay_parent_number, validation_data.relay_parent_number,
+				"Relay parent number doesn't match",
+			);
+			assert_eq!(
+				params.relay_parent_storage_root, validation_data.relay_parent_storage_root,
+				"Relay parent storage root doesn't match",
+			);
+		});
+	}
+
 	/// The bandwidth limit per block that applies when receiving messages from the relay chain via
 	/// DMP or XCMP.
 	///
@@ -1752,34 +1775,6 @@ impl<T: Config> polkadot_runtime_parachains::EnsureForParachain for Pallet<T> {
 	}
 }
 
-/// Something that can check the inherents of a block.
-#[deprecated(note = "This trait is deprecated and will be removed by September 2024. \
-		Consider switching to `cumulus-pallet-parachain-system::ConsensusHook`")]
-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;
-}
-
-/// Struct that always returns `Ok` on inherents check, needed for backwards-compatibility.
-#[doc(hidden)]
-pub struct DummyCheckInherents<Block>(core::marker::PhantomData<Block>);
-
-#[allow(deprecated)]
-impl<Block: BlockT> CheckInherents<Block> for DummyCheckInherents<Block> {
-	fn check_inherents(
-		_: &Block,
-		_: &RelayChainStateProof,
-	) -> frame_support::inherent::CheckInherentsResult {
-		sp_inherents::CheckInherentsResult::new()
-	}
-}
-
 /// Something that should be informed about system related events.
 ///
 /// This includes events like [`on_validation_data`](Self::on_validation_data) that is being

cumulus/pallets/parachain-system/src/validate_block/implementation.rs

@@ -16,26 +16,26 @@
 
 //! The actual implementation of the validate block functionality.
 
-use super::{trie_cache, trie_recorder, MemoryOptimizedValidationParams};
-use crate::parachain_inherent::BasicParachainInherentData;
+use super::{
+	run_with_validation_params, trie_cache, trie_recorder, MemoryOptimizedValidationParams,
+	ValidationParams,
+};
 use alloc::vec::Vec;
 use codec::{Decode, Encode};
 use cumulus_primitives_core::{
-	relay_chain::{Hash as RHash, UMPSignal, UMP_SEPARATOR},
-	ClaimQueueOffset, CoreSelector, ParachainBlockData, PersistedValidationData,
+	relay_chain::{UMPSignal, UMP_SEPARATOR},
+	ClaimQueueOffset, CoreSelector, ParachainBlockData,
 };
 use frame_support::{
 	traits::{ExecuteBlock, Get, IsSubType},
 	BoundedVec,
 };
-use polkadot_parachain_primitives::primitives::{
-	HeadData, RelayChainBlockNumber, ValidationResult,
-};
+use polkadot_parachain_primitives::primitives::{HeadData, ValidationResult};
 use sp_core::storage::{ChildInfo, StateVersion};
 use sp_externalities::{set_and_run_with_externalities, Externalities};
 use sp_io::{hashing::blake2_128, KillStorageResult};
 use sp_runtime::traits::{
-	Block as BlockT, ExtrinsicCall, ExtrinsicLike, Hash as HashT, HashingFor, Header as HeaderT,
+	Block as BlockT, ExtrinsicCall, Hash as HashT, HashingFor, Header as HeaderT,
 };
 use sp_state_machine::OverlayedChanges;
 use sp_trie::{HashDBT, ProofSizeProvider, EMPTY_PREFIX};
@@ -78,12 +78,7 @@ environmental::environmental!(recorder: trait ProofSizeProvider);
 /// end we return back the [`ValidationResult`] with all the required information for the validator.
 #[doc(hidden)]
 #[allow(deprecated)]
-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: parachain_head,
@@ -168,6 +163,11 @@ where
 	let mut head_data = None;
 	let mut new_validation_code = None;
 	let num_blocks = blocks.len();
+	let mut validation_params = ValidationParams {
+		parent_head: parachain_head,
+		relay_parent_number,
+		relay_parent_storage_root,
+	};
 
 	// Create the db
 	let mut db = match proof.to_memory_db(Some(parent_header.state_root())) {
@@ -205,55 +205,21 @@ where
 		let mut overlay = OverlayedChanges::default();
 
 		parent_header = block.header().clone();
-		let inherent_data = extract_parachain_inherent_data(&block);
-
-		validate_validation_data(
-			&inherent_data.validation_data,
-			relay_parent_number,
-			relay_parent_storage_root,
-			&parachain_head,
-		);
-
-		// We don't need the recorder or the overlay in here.
-		run_with_externalities_and_recorder::<B, _, _>(
-			&backend,
-			&mut Default::default(),
-			&mut Default::default(),
-			|| {
-				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");
-
-				#[allow(deprecated)]
-				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_and_recorder::<B, _, _>(
-			&execute_backend,
-			// Here is the only place where we want to use the recorder.
-			// We want to ensure that we not accidentally read something from the proof, that was
-			// not yet read and thus, alter the proof size. Otherwise we end up with mismatches in
-			// later blocks.
-			&mut execute_recorder,
-			&mut overlay,
-			|| {
-				E::execute_block(block);
-			},
-		);
+		run_with_validation_params(&mut validation_params, || {
+			run_with_externalities_and_recorder::<B, _, _>(
+				&execute_backend,
+				// Here is the only place where we want to use the recorder.
+				// We want to ensure that we not accidentally read something from the proof, that
+				// was not yet read and thus, alter the proof size. Otherwise, we end up with
+				// mismatches in later blocks.
+				&mut execute_recorder,
+				&mut overlay,
+				|| {
+					E::execute_block(block);
+				},
+			);
+		});
 
 		run_with_externalities_and_recorder::<B, _, _>(
 			&backend,
@@ -382,45 +348,6 @@ where
 	}
 }
 
-/// Extract the [`BasicParachainInherentData`].
-fn extract_parachain_inherent_data<B: BlockT, PSC: crate::Config>(
-	block: &B,
-) -> &BasicParachainInherentData
-where
-	B::Extrinsic: ExtrinsicCall,
-	<B::Extrinsic as ExtrinsicCall>::Call: IsSubType<crate::Call<PSC>>,
-{
-	block
-		.extrinsics()
-		.iter()
-		// Inherents are at the front of the block and are unsigned.
-		.take_while(|e| e.is_bare())
-		.filter_map(|e| e.call().is_sub_type())
-		.find_map(|c| match c {
-			crate::Call::set_validation_data { data: validation_data, .. } => Some(validation_data),
-			_ => None,
-		})
-		.expect("Could not find `set_validation_data` inherent")
-}
-
-/// Validate the given [`PersistedValidationData`] against the [`MemoryOptimizedValidationParams`].
-fn validate_validation_data(
-	validation_data: &PersistedValidationData,
-	relay_parent_number: RelayChainBlockNumber,
-	relay_parent_storage_root: RHash,
-	parent_head: &[u8],
-) {
-	assert_eq!(parent_head, validation_data.parent_head.0, "Parent head doesn't match");
-	assert_eq!(
-		relay_parent_number, validation_data.relay_parent_number,
-		"Relay parent number doesn't match",
-	);
-	assert_eq!(
-		relay_parent_storage_root, validation_data.relay_parent_storage_root,
-		"Relay parent storage root doesn't match",
-	);
-}
-
 /// Build a seed from the head data of the parachain block.
 ///
 /// Uses both the relay parent storage root and the hash of the blocks

cumulus/pallets/parachain-system/src/validate_block/mod.rs

@@ -68,3 +68,31 @@ pub struct MemoryOptimizedValidationParams {
 	pub relay_parent_number: cumulus_primitives_core::relay_chain::BlockNumber,
 	pub relay_parent_storage_root: cumulus_primitives_core::relay_chain::Hash,
 }
+
+/// The validation params passed to `validate_block`.
+///
+/// Used by the pallet to validate the validation data passed by the collator.
+pub struct ValidationParams {
+	pub parent_head: bytes::Bytes,
+	pub relay_parent_number: cumulus_primitives_core::relay_chain::BlockNumber,
+	pub relay_parent_storage_root: cumulus_primitives_core::relay_chain::Hash,
+}
+
+// Stores the [`ValidationParams`] when running `execute_block` inside of `validate_block`.
+//
+// The pallet uses the params to verify the `ValidationData` coming from the collator.
+environmental::environmental!(validation_params: ValidationParams);
+
+/// Run `function` with the given `validation_params` available in its context.
+#[cfg(not(feature = "std"))]
+fn run_with_validation_params(validation_params: &mut ValidationParams, function: impl FnOnce()) {
+	validation_params::using(validation_params, function)
+}
+
+/// Run `function` with access to [`ValidationParams`].
+///
+/// `function` will only be executed in the `validate_block` context, as otherwise the validation
+/// parameters are not set.
+pub(crate) fn with_validation_params(function: impl FnOnce(&mut ValidationParams)) {
+	validation_params::with(function);
+}