Bring support for calling check_inherents (#490)

* Start

* More work

* Add proc-macro for `validate_block`

* Make everything compile

* Add some test

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

@@ -16,14 +16,12 @@
 
 //! The actual implementation of the validate block functionality.
 
-use frame_support::traits::ExecuteBlock;
-use sp_runtime::traits::{Block as BlockT, HashFor, Header as HeaderT, NumberFor};
+use frame_support::traits::{ExecuteBlock, ExtrinsicCall, IsSubType, Get};
+use sp_runtime::traits::{Block as BlockT, Extrinsic, HashFor, Header as HeaderT, NumberFor};
 
 use sp_io::KillChildStorageResult;
 use sp_std::prelude::*;
 
-use hash_db::{HashDB, EMPTY_PREFIX};
-
 use polkadot_parachain::primitives::{HeadData, ValidationParams, ValidationResult};
 
 use codec::{Decode, Encode};
@@ -32,12 +30,9 @@ use sp_core::storage::ChildInfo;
 use sp_externalities::{set_and_run_with_externalities, Externalities};
 use sp_trie::MemoryDB;
 
-type Ext<'a, B> = sp_state_machine::Ext<
-	'a,
-	HashFor<B>,
-	NumberFor<B>,
-	sp_state_machine::TrieBackend<MemoryDB<HashFor<B>>, HashFor<B>>,
->;
+type TrieBackend<B> = sp_state_machine::TrieBackend<MemoryDB<HashFor<B>>, HashFor<B>>;
+
+type Ext<'a, B> = sp_state_machine::Ext<'a, HashFor<B>, NumberFor<B>, TrieBackend<B>>;
 
 fn with_externalities<F: FnOnce(&mut dyn Externalities) -> R, R>(f: F) -> R {
 	sp_externalities::with_externalities(f).expect("Environmental externalities not set.")
@@ -45,9 +40,18 @@ fn with_externalities<F: FnOnce(&mut dyn Externalities) -> R, R>(f: F) -> R {
 
 /// Validate a given parachain block on a validator.
 #[doc(hidden)]
-pub fn validate_block<B: BlockT, E: ExecuteBlock<B>, PSC: crate::Config>(
+pub fn validate_block<
+	B: BlockT,
+	E: ExecuteBlock<B>,
+	PSC: crate::Config,
+	CI: crate::CheckInherents<B>,
+>(
 	params: ValidationParams,
-) -> ValidationResult {
+) -> ValidationResult
+where
+	B::Extrinsic: ExtrinsicCall,
+	<B::Extrinsic as Extrinsic>::Call: IsSubType<crate::Call<PSC>>,
+{
 	let block_data =
 		cumulus_primitives_core::ParachainBlockData::<B>::decode(&mut &params.block_data.0[..])
 			.expect("Invalid parachain block data");
@@ -77,9 +81,6 @@ pub fn validate_block<B: BlockT, E: ExecuteBlock<B>, PSC: crate::Config>(
 	};
 
 	let backend = sp_state_machine::TrieBackend::new(db, root);
-	let mut overlay = sp_state_machine::OverlayedChanges::default();
-	let mut cache = Default::default();
-	let mut ext = Ext::<B>::new(&mut overlay, &mut cache, &backend);
 
 	let _guard = (
 		// Replace storage calls with our own implementations
@@ -121,7 +122,38 @@ pub fn validate_block<B: BlockT, E: ExecuteBlock<B>, PSC: crate::Config>(
 		sp_io::offchain_index::host_clear.replace_implementation(host_offchain_index_clear),
 	);
 
-	set_and_run_with_externalities(&mut ext, || {
+	let inherent_data = block
+		.extrinsics()
+		.iter()
+		.filter_map(|e| e.call().is_sub_type())
+		.find_map(|c| match c {
+			crate::Call::set_validation_data(validation_data) => Some(validation_data.clone()),
+			_ => None,
+		})
+		.expect("Could not find `set_validation_data` inherent");
+
+	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.extrinsics(), &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, || {
 		super::set_and_run_with_validation_params(params, || {
 			E::execute_block(block);
 
@@ -143,6 +175,18 @@ pub fn validate_block<B: BlockT, E: ExecuteBlock<B>, PSC: crate::Config>(
 	})
 }
 
+/// Run the given closure with the externalities set.
+fn run_with_externalities<B: BlockT, R, F: FnOnce() -> R>(
+	backend: &TrieBackend<B>,
+	execute: F,
+) -> R {
+	let mut overlay = sp_state_machine::OverlayedChanges::default();
+	let mut cache = Default::default();
+	let mut ext = Ext::<B>::new(&mut overlay, &mut cache, backend);
+
+	set_and_run_with_externalities(&mut ext, || execute())
+}
+
 fn host_storage_read(key: &[u8], value_out: &mut [u8], value_offset: u32) -> Option<u32> {
 	match with_externalities(|ext| ext.storage(key)) {
 		Some(value) => {

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

@@ -49,64 +49,3 @@ pub(crate) fn with_validation_params<R>(f: impl FnOnce(&ValidationParams) -> R)
 fn set_and_run_with_validation_params<R>(mut params: ValidationParams, f: impl FnOnce() -> R) -> R {
 	VALIDATION_PARAMS::using(&mut params, f)
 }
-
-/// Register the `validate_block` function that is used by parachains to validate blocks on a
-/// validator.
-///
-/// Does *nothing* when `std` feature is enabled.
-///
-/// Expects as parameters the runtime and a block executor.
-///
-/// # Example
-///
-/// ```
-///     struct BlockExecutor;
-///     struct Runtime;
-///
-///     cumulus_pallet_parachain_system::register_validate_block!(Runtime, BlockExecutor);
-///
-/// # fn main() {}
-/// ```
-#[macro_export]
-macro_rules! register_validate_block {
-	($runtime:ty, $block_executor:ty $( , )? ) => {
-		$crate::register_validate_block_impl!($runtime, $block_executor);
-	};
-}
-
-/// The actual implementation of `register_validate_block` for `no_std`.
-#[cfg(not(feature = "std"))]
-#[doc(hidden)]
-#[macro_export]
-macro_rules! register_validate_block_impl {
-	($runtime:ty, $block_executor:ty) => {
-		#[doc(hidden)]
-		mod parachain_validate_block {
-			use super::*;
-
-			#[no_mangle]
-			unsafe fn validate_block(arguments: *const u8, arguments_len: usize) -> u64 {
-				let params = $crate::validate_block::polkadot_parachain::load_params(
-					arguments,
-					arguments_len,
-				);
-
-				let res = $crate::validate_block::implementation::validate_block::<
-					<$runtime as $crate::validate_block::GetRuntimeBlockType>::RuntimeBlock,
-					$block_executor,
-					$runtime,
-				>(params);
-
-				$crate::validate_block::polkadot_parachain::write_result(&res)
-			}
-		}
-	};
-}
-
-/// The actual implementation of `register_validate_block` for `std`.
-#[cfg(feature = "std")]
-#[doc(hidden)]
-#[macro_export]
-macro_rules! register_validate_block_impl {
-	($runtime:ty, $block_executor:ty) => {};
-}

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

@@ -94,8 +94,8 @@ fn build_block_with_witness(
 	client: &Client,
 	extra_extrinsics: Vec<UncheckedExtrinsic>,
 	parent_head: Header,
+	sproof_builder: RelayStateSproofBuilder,
 ) -> TestBlockData {
-	let sproof_builder = RelayStateSproofBuilder::default();
 	let (relay_parent_storage_root, _) = sproof_builder.clone().into_state_root_and_proof();
 	let block_id = BlockId::Hash(client.info().best_hash);
 	let mut validation_data = PersistedValidationData {
@@ -137,7 +137,7 @@ fn validate_block_no_extra_extrinsics() {
 		block,
 		witness,
 		validation_data,
-	} = build_block_with_witness(&client, vec![], parent_head.clone());
+	} = build_block_with_witness(&client, vec![], parent_head.clone(), Default::default());
 	let (header, extrinsics) = block.deconstruct();
 
 	let block_data = ParachainBlockData::new(header.clone(), extrinsics, witness);
@@ -167,7 +167,12 @@ fn validate_block_with_extra_extrinsics() {
 		block,
 		witness,
 		validation_data,
-	} = build_block_with_witness(&client, extra_extrinsics, parent_head.clone());
+	} = build_block_with_witness(
+		&client,
+		extra_extrinsics,
+		parent_head.clone(),
+		Default::default(),
+	);
 	let (header, extrinsics) = block.deconstruct();
 
 	let block_data = ParachainBlockData::new(header.clone(), extrinsics, witness);
@@ -192,7 +197,7 @@ fn validate_block_invalid_parent_hash() {
 		block,
 		witness,
 		validation_data,
-	} = build_block_with_witness(&client, vec![], parent_head.clone());
+	} = build_block_with_witness(&client, vec![], parent_head.clone(), Default::default());
 	let (mut header, extrinsics) = block.deconstruct();
 	header.set_parent_hash(Hash::from_low_u64_be(1));
 
@@ -212,18 +217,44 @@ fn validate_block_fails_on_invalid_validation_data() {
 
 	let (client, longest_chain) = create_test_client();
 	let parent_head = longest_chain.best_chain().expect("Best block exists");
-	let TestBlockData {
-		block,
-		witness,
-		..
-	} = build_block_with_witness(&client, vec![], parent_head.clone());
+	let TestBlockData { block, witness, .. } =
+		build_block_with_witness(&client, vec![], parent_head.clone(), Default::default());
 	let (header, extrinsics) = block.deconstruct();
 
 	let block_data = ParachainBlockData::new(header, extrinsics, witness);
-	call_validate_block(
+	call_validate_block(parent_head, block_data, Hash::random()).expect("Calls `validate_block`");
+}
+
+#[test]
+#[should_panic(expected = "Calls `validate_block`: Other(\"Trap: Trap { kind: Unreachable }\")")]
+fn check_inherent_fails_on_validate_block_as_expected() {
+	let _ = env_logger::try_init();
+
+	let (client, longest_chain) = create_test_client();
+	let parent_head = longest_chain.best_chain().expect("Best block exists");
+
+	let TestBlockData {
+		block,
+		witness,
+		validation_data,
+	} = build_block_with_witness(
+		&client,
+		vec![],
+		parent_head.clone(),
+		RelayStateSproofBuilder {
+			current_slot: 1337.into(),
+			..Default::default()
+		},
+	);
+	let (header, extrinsics) = block.deconstruct();
+
+	let block_data = ParachainBlockData::new(header.clone(), extrinsics, witness);
+
+	let res_header = call_validate_block(
 		parent_head,
 		block_data,
-		Hash::random(),
+		validation_data.relay_parent_storage_root,
 	)
 	.expect("Calls `validate_block`");
+	assert_eq!(header, res_header);
 }