Revert "Fix order of hook execution (#10043)"

This reverts commit d36550a.

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

@@ -323,7 +323,7 @@ pub type Executive = frame_executive::Executive<
 	Block,
 	frame_system::ChainContext<Runtime>,
 	Runtime,
-	AllPalletsWithSystem,
+	AllPallets,
 >;
 
 impl_runtime_apis! {

bin/node/runtime/src/lib.rs

@@ -1268,8 +1268,6 @@ construct_runtime!(
 		Utility: pallet_utility,
 		Babe: pallet_babe,
 		Timestamp: pallet_timestamp,
-		// Authorship must be before session in order to note author in the correct session and era
-		// for im-online and staking.
 		Authorship: pallet_authorship,
 		Indices: pallet_indices,
 		Balances: pallet_balances,
@@ -1347,7 +1345,7 @@ pub type Executive = frame_executive::Executive<
 	Block,
 	frame_system::ChainContext<Runtime>,
 	Runtime,
-	AllPalletsWithSystem,
+	AllPallets,
 	pallet_bags_list::migrations::CheckCounterPrefix<Runtime>,
 >;
 

frame/executive/src/lib.rs

@@ -59,7 +59,7 @@
 //! # type Context = frame_system::ChainContext<Runtime>;
 //! # pub type Block = generic::Block<Header, UncheckedExtrinsic>;
 //! # pub type Balances = u64;
-//! # pub type AllPalletsWithSystem = u64;
+//! # pub type AllPallets = u64;
 //! # pub enum Runtime {};
 //! # use sp_runtime::transaction_validity::{
 //! #    TransactionValidity, UnknownTransaction, TransactionSource,
@@ -73,7 +73,7 @@
 //! #     }
 //! # }
 //! /// Executive: handles dispatch to the various modules.
-//! pub type Executive = executive::Executive<Runtime, Block, Context, Runtime, AllPalletsWithSystem>;
+//! pub type Executive = executive::Executive<Runtime, Block, Context, Runtime, AllPallets>;
 //! ```
 //!
 //! ### Custom `OnRuntimeUpgrade` logic
@@ -90,7 +90,7 @@
 //! # type Context = frame_system::ChainContext<Runtime>;
 //! # pub type Block = generic::Block<Header, UncheckedExtrinsic>;
 //! # pub type Balances = u64;
-//! # pub type AllPalletsWithSystem = u64;
+//! # pub type AllPallets = u64;
 //! # pub enum Runtime {};
 //! # use sp_runtime::transaction_validity::{
 //! #    TransactionValidity, UnknownTransaction, TransactionSource,
@@ -111,7 +111,7 @@
 //!     }
 //! }
 //!
-//! pub type Executive = executive::Executive<Runtime, Block, Context, Runtime, AllPalletsWithSystem, CustomOnRuntimeUpgrade>;
+//! pub type Executive = executive::Executive<Runtime, Block, Context, Runtime, AllPallets, CustomOnRuntimeUpgrade>;
 //! ```
 
 #![cfg_attr(not(feature = "std"), no_std)]
@@ -147,41 +147,26 @@ pub type OriginOf<E, C> = <CallOf<E, C> as Dispatchable>::Origin;
 /// - `Block`: The block type of the runtime
 /// - `Context`: The context that is used when checking an extrinsic.
 /// - `UnsignedValidator`: The unsigned transaction validator of the runtime.
-/// - `AllPalletsWithSystem`: Tuple that contains all pallets including frame system pallet. Will be
-///   used to call hooks e.g. `on_initialize`.
+/// - `AllPallets`: Tuple that contains all modules. Will be used to call e.g. `on_initialize`.
 /// - `OnRuntimeUpgrade`: Custom logic that should be called after a runtime upgrade. Modules are
-///   already called by `AllPalletsWithSystem`. It will be called before all modules will be called.
-pub struct Executive<
-	System,
-	Block,
-	Context,
-	UnsignedValidator,
-	AllPalletsWithSystem,
-	OnRuntimeUpgrade = (),
->(
-	PhantomData<(
-		System,
-		Block,
-		Context,
-		UnsignedValidator,
-		AllPalletsWithSystem,
-		OnRuntimeUpgrade,
-	)>,
+///   already called by `AllPallets`. It will be called before all modules will be called.
+pub struct Executive<System, Block, Context, UnsignedValidator, AllPallets, OnRuntimeUpgrade = ()>(
+	PhantomData<(System, Block, Context, UnsignedValidator, AllPallets, OnRuntimeUpgrade)>,
 );
 
 impl<
 		System: frame_system::Config + EnsureInherentsAreFirst<Block>,
 		Block: traits::Block<Header = System::Header, Hash = System::Hash>,
 		Context: Default,
 		UnsignedValidator,
-		AllPalletsWithSystem: OnRuntimeUpgrade
+		AllPallets: OnRuntimeUpgrade
 			+ OnInitialize<System::BlockNumber>
 			+ OnIdle<System::BlockNumber>
 			+ OnFinalize<System::BlockNumber>
 			+ OffchainWorker<System::BlockNumber>,
 		COnRuntimeUpgrade: OnRuntimeUpgrade,
 	> ExecuteBlock<Block>
-	for Executive<System, Block, Context, UnsignedValidator, AllPalletsWithSystem, COnRuntimeUpgrade>
+	for Executive<System, Block, Context, UnsignedValidator, AllPallets, COnRuntimeUpgrade>
 where
 	Block::Extrinsic: Checkable<Context> + Codec,
 	CheckedOf<Block::Extrinsic, Context>: Applyable + GetDispatchInfo,
@@ -196,7 +181,7 @@ where
 			Block,
 			Context,
 			UnsignedValidator,
-			AllPalletsWithSystem,
+			AllPallets,
 			COnRuntimeUpgrade,
 		>::execute_block(block);
 	}
@@ -207,13 +192,13 @@ impl<
 		Block: traits::Block<Header = System::Header, Hash = System::Hash>,
 		Context: Default,
 		UnsignedValidator,
-		AllPalletsWithSystem: OnRuntimeUpgrade
+		AllPallets: OnRuntimeUpgrade
 			+ OnInitialize<System::BlockNumber>
 			+ OnIdle<System::BlockNumber>
 			+ OnFinalize<System::BlockNumber>
 			+ OffchainWorker<System::BlockNumber>,
 		COnRuntimeUpgrade: OnRuntimeUpgrade,
-	> Executive<System, Block, Context, UnsignedValidator, AllPalletsWithSystem, COnRuntimeUpgrade>
+	> Executive<System, Block, Context, UnsignedValidator, AllPallets, COnRuntimeUpgrade>
 where
 	Block::Extrinsic: Checkable<Context> + Codec,
 	CheckedOf<Block::Extrinsic, Context>: Applyable + GetDispatchInfo,
@@ -224,7 +209,14 @@ where
 {
 	/// Execute all `OnRuntimeUpgrade` of this runtime, and return the aggregate weight.
 	pub fn execute_on_runtime_upgrade() -> frame_support::weights::Weight {
-		<(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::on_runtime_upgrade()
+		let mut weight = 0;
+		weight = weight.saturating_add(COnRuntimeUpgrade::on_runtime_upgrade());
+		weight = weight.saturating_add(
+			<frame_system::Pallet<System> as OnRuntimeUpgrade>::on_runtime_upgrade(),
+		);
+		weight = weight.saturating_add(<AllPallets as OnRuntimeUpgrade>::on_runtime_upgrade());
+
+		weight
 	}
 
 	/// Execute given block, but don't do any of the `final_checks`.
@@ -263,10 +255,19 @@ where
 	/// This should only be used for testing.
 	#[cfg(feature = "try-runtime")]
 	pub fn try_runtime_upgrade() -> Result<frame_support::weights::Weight, &'static str> {
-		<(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::pre_upgrade().unwrap();
+		<
+			(frame_system::Pallet::<System>, COnRuntimeUpgrade, AllPallets)
+			as
+			OnRuntimeUpgrade
+		>::pre_upgrade().unwrap();
+
 		let weight = Self::execute_on_runtime_upgrade();
 
-		<(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::post_upgrade().unwrap();
+		<
+			(frame_system::Pallet::<System>, COnRuntimeUpgrade, AllPallets)
+			as
+			OnRuntimeUpgrade
+		>::post_upgrade().unwrap();
 
 		Ok(weight)
 	}
@@ -304,9 +305,12 @@ where
 			digest,
 			frame_system::InitKind::Full,
 		);
-		weight = weight.saturating_add(<AllPalletsWithSystem as OnInitialize<
+		weight = weight.saturating_add(<frame_system::Pallet<System> as OnInitialize<
 			System::BlockNumber,
 		>>::on_initialize(*block_number));
+		weight = weight.saturating_add(
+			<AllPallets as OnInitialize<System::BlockNumber>>::on_initialize(*block_number),
+		);
 		weight = weight.saturating_add(
 			<System::BlockWeights as frame_support::traits::Get<_>>::get().base_block,
 		);
@@ -411,20 +415,30 @@ where
 	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;
-		let remaining_weight = max_weight.saturating_sub(weight.total());
+		let mut remaining_weight = max_weight.saturating_sub(weight.total());
 
 		if remaining_weight > 0 {
-			let used_weight = <AllPalletsWithSystem as OnIdle<System::BlockNumber>>::on_idle(
+			let mut used_weight =
+				<frame_system::Pallet<System> as OnIdle<System::BlockNumber>>::on_idle(
+					block_number,
+					remaining_weight,
+				);
+			remaining_weight = remaining_weight.saturating_sub(used_weight);
+			used_weight = <AllPallets as OnIdle<System::BlockNumber>>::on_idle(
 				block_number,
 				remaining_weight,
-			);
+			)
+			.saturating_add(used_weight);
 			<frame_system::Pallet<System>>::register_extra_weight_unchecked(
 				used_weight,
 				DispatchClass::Mandatory,
 			);
 		}
 
-		<AllPalletsWithSystem as OnFinalize<System::BlockNumber>>::on_finalize(block_number);
+		<frame_system::Pallet<System> as OnFinalize<System::BlockNumber>>::on_finalize(
+			block_number,
+		);
+		<AllPallets as OnFinalize<System::BlockNumber>>::on_finalize(block_number);
 	}
 
 	/// Apply extrinsic outside of the block execution function.
@@ -553,9 +567,7 @@ where
 		// as well.
 		frame_system::BlockHash::<System>::insert(header.number(), header.hash());
 
-		<AllPalletsWithSystem as OffchainWorker<System::BlockNumber>>::offchain_worker(
-			*header.number(),
-		)
+		<AllPallets as OffchainWorker<System::BlockNumber>>::offchain_worker(*header.number())
 	}
 }
 
@@ -837,7 +849,7 @@ mod tests {
 		Block<TestXt>,
 		ChainContext<Runtime>,
 		Runtime,
-		AllPalletsWithSystem,
+		AllPallets,
 		CustomOnRuntimeUpgrade,
 	>;
 
@@ -1348,19 +1360,23 @@ mod tests {
 			));
 
 			// All weights that show up in the `initialize_block_impl`
+			let frame_system_upgrade_weight = frame_system::Pallet::<Runtime>::on_runtime_upgrade();
 			let custom_runtime_upgrade_weight = CustomOnRuntimeUpgrade::on_runtime_upgrade();
-			let runtime_upgrade_weight =
-				<AllPalletsWithSystem as OnRuntimeUpgrade>::on_runtime_upgrade();
+			let runtime_upgrade_weight = <AllPallets as OnRuntimeUpgrade>::on_runtime_upgrade();
+			let frame_system_on_initialize_weight =
+				frame_system::Pallet::<Runtime>::on_initialize(block_number);
 			let on_initialize_weight =
-				<AllPalletsWithSystem as OnInitialize<u64>>::on_initialize(block_number);
+				<AllPallets as OnInitialize<u64>>::on_initialize(block_number);
 			let base_block_weight =
 				<Runtime as frame_system::Config>::BlockWeights::get().base_block;
 
 			// Weights are recorded correctly
 			assert_eq!(
 				frame_system::Pallet::<Runtime>::block_weight().total(),
-				custom_runtime_upgrade_weight +
+				frame_system_upgrade_weight +
+					custom_runtime_upgrade_weight +
 					runtime_upgrade_weight +
+					frame_system_on_initialize_weight +
 					on_initialize_weight + base_block_weight,
 			);
 		});

frame/support/procedural/src/construct_runtime/mod.rs

@@ -304,82 +304,35 @@ fn decl_all_pallets<'a>(
 		types.extend(type_decl);
 		names.push(&pallet_declaration.name);
 	}
-
-	// Make nested tuple structure like:
-	// `((FirstPallet, (SecondPallet, ( ... , LastPallet) ... ))))`
+	// Make nested tuple structure like (((Babe, Consensus), Grandpa), ...)
 	// But ignore the system pallet.
-	let all_pallets_without_system = names
+	let all_pallets = names
 		.iter()
 		.filter(|n| **n != SYSTEM_PALLET_NAME)
-		.rev()
 		.fold(TokenStream2::default(), |combined, name| quote!((#name, #combined)));
 
-	// Make nested tuple structure like:
-	// `((FirstPallet, (SecondPallet, ( ... , LastPallet) ... ))))`
 	let all_pallets_with_system = names
 		.iter()
-		.rev()
-		.fold(TokenStream2::default(), |combined, name| quote!((#name, #combined)));
-
-	// Make nested tuple structure like:
-	// `((LastPallet, (SecondLastPallet, ( ... , FirstPallet) ... ))))`
-	// But ignore the system pallet.
-	let all_pallets_without_system_reversed = names
-		.iter()
-		.filter(|n| **n != SYSTEM_PALLET_NAME)
 		.fold(TokenStream2::default(), |combined, name| quote!((#name, #combined)));
 
-	// Make nested tuple structure like:
-	// `((LastPallet, (SecondLastPallet, ( ... , FirstPallet) ... ))))`
-	let all_pallets_with_system_reversed = names
-		.iter()
-		.fold(TokenStream2::default(), |combined, name| quote!((#name, #combined)));
-
-	let system_pallet = match names.iter().find(|n| **n == SYSTEM_PALLET_NAME) {
-		Some(name) => name,
-		None =>
-			return syn::Error::new(
-				proc_macro2::Span::call_site(),
-				"`System` pallet declaration is missing. \
-				 Please add this line: `System: frame_system::{Pallet, Call, Storage, Config, Event<T>},`",
-			)
-			.into_compile_error(),
-	};
-
 	quote!(
 		#types
 
 		/// All pallets included in the runtime as a nested tuple of types.
-		#[deprecated(note = "The type definition has changed from representing all pallets \
-			excluding system, in reversed order to become the representation of all pallets \
-			including system pallet in regular order. For this reason it is encouraged to use \
-			explicitly one of `AllPalletsWithSystem`, `AllPalletsWithoutSystem`, \
-			`AllPalletsWithSystemReversed`, `AllPalletsWithoutSystemReversed`. \
-			Note that the type `frame_executive::Executive` expects one of `AllPalletsWithSystem` \
-			, `AllPalletsWithSystemReversed`, `AllPalletsReversedWithSystemFirst`. More details in \
-			https://github.com/paritytech/substrate/pull/10043")]
-		pub type AllPallets = AllPalletsWithSystem;
-
+		/// Excludes the System pallet.
+		pub type AllPallets = ( #all_pallets );
 		/// All pallets included in the runtime as a nested tuple of types.
 		pub type AllPalletsWithSystem = ( #all_pallets_with_system );
 
-		/// All pallets included in the runtime as a nested tuple of types.
-		/// Excludes the System pallet.
-		pub type AllPalletsWithoutSystem = ( #all_pallets_without_system );
-
-		/// All pallets included in the runtime as a nested tuple of types in reversed order.
+		/// All modules included in the runtime as a nested tuple of types.
 		/// Excludes the System pallet.
-		pub type AllPalletsWithoutSystemReversed = ( #all_pallets_without_system_reversed );
-
-		/// All pallets included in the runtime as a nested tuple of types in reversed order.
-		pub type AllPalletsWithSystemReversed = ( #all_pallets_with_system_reversed );
-
-		/// All pallets included in the runtime as a nested tuple of types in reversed order.
-		/// With the system pallet first.
-		pub type AllPalletsReversedWithSystemFirst = (
-			#system_pallet,
-			AllPalletsWithoutSystemReversed
-		);
+		#[deprecated(note = "use `AllPallets` instead")]
+		#[allow(dead_code)]
+		pub type AllModules = ( #all_pallets );
+		/// All modules included in the runtime as a nested tuple of types.
+		#[deprecated(note = "use `AllPalletsWithSystem` instead")]
+		#[allow(dead_code)]
+		pub type AllModulesWithSystem = ( #all_pallets_with_system );
 	)
 }