Add OnPostInherent Hook (try 2)

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

@@ -366,6 +366,10 @@ impl_runtime_apis! {
 		) -> sp_inherents::CheckInherentsResult {
 			data.check_extrinsics(&block)
 		}
+
+		fn on_post_inherent() {
+			Executive::on_post_inherent();
+		}
 	}
 
 	impl sp_transaction_pool::runtime_api::TaggedTransactionQueue<Block> for Runtime {

bin/node/runtime/src/lib.rs

@@ -1379,6 +1379,10 @@ impl_runtime_apis! {
 		fn check_inherents(block: Block, data: InherentData) -> CheckInherentsResult {
 			data.check_extrinsics(&block)
 		}
+
+		fn on_post_inherent() {
+			Executive::on_post_inherent();
+		}
 	}
 
 	impl sp_transaction_pool::runtime_api::TaggedTransactionQueue<Block> for Runtime {

client/basic-authorship/src/basic_authorship.rs

@@ -335,6 +335,9 @@ where
 			}
 		}
 
+		// Inherents have been processed, so run any on post inherent logic from the runtime.
+		block_builder.on_post_inherent()?;
+
 		// proceed with transactions
 		// We calculate soft deadline used only in case we start skipping transactions.
 		let now = (self.now)();

client/block-builder/src/lib.rs

@@ -289,6 +289,15 @@ where
 			size
 		}
 	}
+
+	/// Execute any logic after processing all inherents in the block.
+	pub fn on_post_inherent(&self) -> Result<(), Error> {
+		let block_id = &self.block_id;
+
+		self.api
+			.on_post_inherent_with_context(&block_id, ExecutionContext::BlockConstruction)?;
+		Ok(())
+	}
 }
 
 #[cfg(test)]

frame/executive/src/lib.rs

@@ -121,7 +121,7 @@ use frame_support::{
 	dispatch::PostDispatchInfo,
 	traits::{
 		EnsureInherentsAreFirst, ExecuteBlock, OffchainWorker, OnFinalize, OnIdle, OnInitialize,
-		OnRuntimeUpgrade,
+		OnPostInherent, OnRuntimeUpgrade,
 	},
 	weights::{DispatchClass, DispatchInfo, GetDispatchInfo},
 };
@@ -162,6 +162,7 @@ impl<
 		UnsignedValidator,
 		AllPallets: OnRuntimeUpgrade
 			+ OnInitialize<System::BlockNumber>
+			+ OnPostInherent<System::BlockNumber>
 			+ OnIdle<System::BlockNumber>
 			+ OnFinalize<System::BlockNumber>
 			+ OffchainWorker<System::BlockNumber>,
@@ -195,6 +196,7 @@ impl<
 		UnsignedValidator,
 		AllPallets: OnRuntimeUpgrade
 			+ OnInitialize<System::BlockNumber>
+			+ OnPostInherent<System::BlockNumber>
 			+ OnIdle<System::BlockNumber>
 			+ OnFinalize<System::BlockNumber>
 			+ OffchainWorker<System::BlockNumber>,
@@ -226,11 +228,15 @@ where
 	#[cfg(feature = "try-runtime")]
 	pub fn execute_block_no_check(block: Block) -> frame_support::weights::Weight {
 		Self::initialize_block(block.header());
-		Self::initial_checks(&block);
+		let maybe_first_non_inherent_index = Self::initial_checks(&block);
 
 		let (header, extrinsics) = block.deconstruct();
 
-		Self::execute_extrinsics_with_book_keeping(extrinsics, *header.number());
+		Self::execute_extrinsics_with_book_keeping(
+			extrinsics,
+			*header.number(),
+			maybe_first_non_inherent_index,
+		);
 
 		// do some of the checks that would normally happen in `final_checks`, but definitely skip
 		// the state root check.
@@ -338,7 +344,9 @@ where
 		}
 	}
 
-	fn initial_checks(block: &Block) {
+	/// Checks the block is valid, and returns the index of the first non-inherent in the block if
+	/// there is one.
+	fn initial_checks(block: &Block) -> Option<u32> {
 		sp_tracing::enter_span!(sp_tracing::Level::TRACE, "initial_checks");
 		let header = block.header();
 
@@ -351,8 +359,9 @@ where
 			"Parent hash should be valid.",
 		);
 
-		if let Err(i) = System::ensure_inherents_are_first(block) {
-			panic!("Invalid inherent position for extrinsic at index {}", i);
+		match System::ensure_inherents_are_first(block) {
+			Ok(i) => i,
+			Err(i) => panic!("Invalid inherent position for extrinsic at index {}", i),
 		}
 	}
 
@@ -365,13 +374,17 @@ where
 			Self::initialize_block(block.header());
 
 			// any initial checks
-			Self::initial_checks(&block);
+			let maybe_first_non_inherent_index = Self::initial_checks(&block);
 
 			let signature_batching = sp_runtime::SignatureBatching::start();
 
 			// execute extrinsics
 			let (header, extrinsics) = block.deconstruct();
-			Self::execute_extrinsics_with_book_keeping(extrinsics, *header.number());
+			Self::execute_extrinsics_with_book_keeping(
+				extrinsics,
+				*header.number(),
+				maybe_first_non_inherent_index,
+			);
 
 			if !signature_batching.verify() {
 				panic!("Signature verification failed.");
@@ -386,8 +399,17 @@ where
 	fn execute_extrinsics_with_book_keeping(
 		extrinsics: Vec<Block::Extrinsic>,
 		block_number: NumberFor<Block>,
+		maybe_first_non_inherent_index: Option<u32>,
 	) {
-		extrinsics.into_iter().for_each(|e| {
+		if maybe_first_non_inherent_index.is_none() {
+			Self::on_post_inherent()
+		}
+		extrinsics.into_iter().enumerate().for_each(|(i, e)| {
+			if let Some(first_non_inherent_index) = maybe_first_non_inherent_index {
+				if first_non_inherent_index == i as u32 {
+					Self::on_post_inherent()
+				}
+			}
 			if let Err(e) = Self::apply_extrinsic(e) {
 				let err: &'static str = e.into();
 				panic!("{}", err)
@@ -413,6 +435,21 @@ where
 		<frame_system::Pallet<System>>::finalize()
 	}
 
+	pub fn on_post_inherent() {
+		let block_number = <frame_system::Pallet<System>>::block_number();
+		let mut weight = 0;
+		weight = weight.saturating_add(<frame_system::Pallet<System> as OnPostInherent<
+			System::BlockNumber,
+		>>::on_post_inherent(block_number));
+		weight = weight.saturating_add(
+			<AllPallets as OnPostInherent<System::BlockNumber>>::on_post_inherent(block_number),
+		);
+		<frame_system::Pallet<System>>::register_extra_weight_unchecked(
+			weight,
+			DispatchClass::Mandatory,
+		);
+	}
+
 	fn idle_and_finalize_hook(block_number: NumberFor<Block>) {
 		let weight = <frame_system::Pallet<System>>::block_weight();
 		let max_weight = <System::BlockWeights as frame_support::traits::Get<_>>::get().max_block;
@@ -618,12 +655,12 @@ mod tests {
 			// one with block number arg and one without
 			fn on_initialize(n: T::BlockNumber) -> Weight {
 				println!("on_initialize({})", n);
-				175
+				123
 			}
 
 			fn on_idle(n: T::BlockNumber, remaining_weight: Weight) -> Weight {
 				println!("on_idle{}, {})", n, remaining_weight);
-				175
+				234
 			}
 
 			fn on_finalize(n: T::BlockNumber) {
@@ -638,6 +675,11 @@ mod tests {
 			fn offchain_worker(n: T::BlockNumber) {
 				assert_eq!(T::BlockNumber::from(1u32), n);
 			}
+
+			fn on_post_inherent(n: T::BlockNumber) -> Weight {
+				println!("on_post_inherent({})", n);
+				345
+			}
 		}
 
 		#[pallet::call]
@@ -916,7 +958,7 @@ mod tests {
 					parent_hash: [69u8; 32].into(),
 					number: 1,
 					state_root: hex!(
-						"1039e1a4bd0cf5deefe65f313577e70169c41c7773d6acf31ca8d671397559f5"
+						"16626995740d9ac5dd5729222b02c90fa28283f6120ae3b4864f8cad6c95c384"
 					)
 					.into(),
 					extrinsics_root: hex!(
@@ -1003,7 +1045,7 @@ mod tests {
 		let encoded_len = encoded.len() as Weight;
 		// on_initialize weight + base block execution weight
 		let block_weights = <Runtime as frame_system::Config>::BlockWeights::get();
-		let base_block_weight = 175 + block_weights.base_block;
+		let base_block_weight = 123 + block_weights.base_block;
 		let limit = block_weights.get(DispatchClass::Normal).max_total.unwrap() - base_block_weight;
 		let num_to_exhaust_block = limit / (encoded_len + 5);
 		t.execute_with(|| {
@@ -1060,7 +1102,7 @@ mod tests {
 		t.execute_with(|| {
 			// Block execution weight + on_initialize weight from custom module
 			let base_block_weight =
-				175 + <Runtime as frame_system::Config>::BlockWeights::get().base_block;
+				123 + <Runtime as frame_system::Config>::BlockWeights::get().base_block;
 
 			Executive::initialize_block(&Header::new(
 				1,
@@ -1186,11 +1228,19 @@ mod tests {
 	fn block_hooks_weight_is_stored() {
 		new_test_ext(1).execute_with(|| {
 			Executive::initialize_block(&Header::new_from_number(1));
+			Executive::on_post_inherent();
 			Executive::finalize_block();
 			// NOTE: might need updates over time if new weights are introduced.
-			// For now it only accounts for the base block execution weight and
-			// the `on_initialize` weight defined in the custom test module.
-			assert_eq!(<frame_system::Pallet<Runtime>>::block_weight().total(), 175 + 175 + 10);
+			// For now it only accounts for:
+			// * base block execution: 10
+			// * on_initialize:        123
+			// * on_idle:              234
+			// * on_post_inherent:     345
+			// defined in the custom test module.
+			assert_eq!(
+				<frame_system::Pallet<Runtime>>::block_weight().total(),
+				10 + 123 + 234 + 345
+			);
 		})
 	}
 
@@ -1317,6 +1367,8 @@ mod tests {
 				Digest::default(),
 			));
 
+			Executive::on_post_inherent();
+
 			Executive::apply_extrinsic(xt.clone()).unwrap().unwrap();
 
 			Executive::finalize_block()
@@ -1416,6 +1468,8 @@ mod tests {
 				Digest::default(),
 			));
 
+			Executive::on_post_inherent();
+
 			Executive::apply_extrinsic(xt.clone()).unwrap().unwrap();
 
 			Executive::finalize_block()
@@ -1448,6 +1502,8 @@ mod tests {
 			Executive::apply_extrinsic(xt1.clone()).unwrap().unwrap();
 			Executive::apply_extrinsic(xt2.clone()).unwrap().unwrap();
 
+			Executive::on_post_inherent();
+
 			Executive::finalize_block()
 		});
 
@@ -1472,6 +1528,9 @@ mod tests {
 			));
 
 			Executive::apply_extrinsic(xt1.clone()).unwrap().unwrap();
+
+			Executive::on_post_inherent();
+
 			Executive::apply_extrinsic(xt2.clone()).unwrap().unwrap();
 
 			Executive::finalize_block()

frame/support/procedural/src/construct_runtime/expand/inherent.rs

@@ -162,12 +162,12 @@ pub fn expand_outer_inherent(
 		}
 
 		impl #scrate::traits::EnsureInherentsAreFirst<#block> for #runtime {
-			fn ensure_inherents_are_first(block: &#block) -> Result<(), u32> {
+			fn ensure_inherents_are_first(block: &#block) -> Result<Option<u32>, u32> {
 				use #scrate::inherent::ProvideInherent;
 				use #scrate::traits::{IsSubType, ExtrinsicCall};
 				use #scrate::sp_runtime::traits::Block as _;
 
-				let mut first_signed_observed = false;
+				let mut first_non_inherent_observed = None;
 
 				for (i, xt) in block.extrinsics().iter().enumerate() {
 					let is_signed = #scrate::inherent::Extrinsic::is_signed(xt).unwrap_or(false);
@@ -188,16 +188,16 @@ pub fn expand_outer_inherent(
 						is_inherent
 					};
 
-					if !is_inherent {
-						first_signed_observed = true;
+					if !is_inherent && first_non_inherent_observed.is_none() {
+						first_non_inherent_observed = Some(i as u32);
 					}
 
-					if first_signed_observed && is_inherent {
+					if first_non_inherent_observed.is_some() && is_inherent {
 						return Err(i as u32)
 					}
 				}
 
-				Ok(())
+				Ok(first_non_inherent_observed)
 			}
 		}
 	}

frame/support/procedural/src/pallet/expand/hooks.rs

@@ -123,6 +123,24 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream {
 			}
 		}
 
+		impl<#type_impl_gen>
+			#frame_support::traits::OnPostInherent<<T as #frame_system::Config>::BlockNumber>
+			for #pallet_ident<#type_use_gen> #where_clause
+		{
+			fn on_post_inherent(
+				n: <T as #frame_system::Config>::BlockNumber
+			) -> #frame_support::weights::Weight {
+				#frame_support::sp_tracing::enter_span!(
+					#frame_support::sp_tracing::trace_span!("on_post_inherent")
+				);
+				<
+					Self as #frame_support::traits::Hooks<
+						<T as #frame_system::Config>::BlockNumber
+					>
+				>::on_post_inherent(n)
+			}
+		}
+
 		impl<#type_impl_gen>
 			#frame_support::traits::OnRuntimeUpgrade
 			for #pallet_ident<#type_use_gen> #where_clause

frame/support/src/dispatch.rs

@@ -1489,6 +1489,11 @@ macro_rules! decl_module {
 				{ $( $impl )* }
 			}
 		}
+		// `on_post_inherent` is not supported with `decl_module`, so we do an empty impl.
+		impl<$trait_instance: $system::Config + $trait_name$(<I>, $instance: $instantiable)?>
+			$crate::traits::OnPostInherent<<$trait_instance as $system::Config>::BlockNumber>
+			for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )*
+		{}
 	};
 
 	(@impl_on_initialize
@@ -1506,6 +1511,11 @@ macro_rules! decl_module {
 				{ $( $impl )* }
 			}
 		}
+		// `on_post_inherent` is not supported with `decl_module`, so we do an empty impl.
+		impl<$trait_instance: $system::Config + $trait_name$(<I>, $instance: $instantiable)?>
+			$crate::traits::OnPostInherent<<$trait_instance as $system::Config>::BlockNumber>
+			for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )*
+		{}
 	};
 
 	(@impl_on_initialize
@@ -1517,6 +1527,11 @@ macro_rules! decl_module {
 			$crate::traits::OnInitialize<<$trait_instance as $system::Config>::BlockNumber>
 			for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )*
 		{}
+		// `on_post_inherent` is not supported with `decl_module`, so we do an empty impl.
+		impl<$trait_instance: $system::Config + $trait_name$(<I>, $instance: $instantiable)?>
+			$crate::traits::OnPostInherent<<$trait_instance as $system::Config>::BlockNumber>
+			for $module<$trait_instance$(, $instance)?> where $( $other_where_bounds )*
+		{}
 	};
 
 	(@impl_on_runtime_upgrade
@@ -2506,6 +2521,9 @@ macro_rules! __check_reserved_fn_name {
 	(on_initialize $( $rest:ident )*) => {
 		$crate::__check_reserved_fn_name!(@compile_error on_initialize);
 	};
+	(on_post_inherent $( $rest:ident )*) => {
+		$crate::__check_reserved_fn_name!(@compile_error on_post_inherent);
+	};
 	(on_runtime_upgrade $( $rest:ident )*) => {
 		$crate::__check_reserved_fn_name!(@compile_error on_runtime_upgrade);
 	};