OnInitialize must run in order of declaration in construct_runtime (not in reversed as currently)

[gui1117]

Currently OnInitialize is run in the reversed order of their declaration in construct_runtime.

construct_runtime expand AllModules to:

((last, (last-1, ..., (first,))))

and on_initialize for tuple is implemented using first and then second element.

The fix is:

diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs
index d7529cd27..27fd4c3de 100644
--- a/frame/support/procedural/src/construct_runtime/mod.rs
+++ b/frame/support/procedural/src/construct_runtime/mod.rs
@@ -359,10 +359,11 @@ fn decl_all_modules<'a>(
                types.extend(type_decl);
                names.push(&module_declaration.name);
        }
-       // Make nested tuple structure like (((Babe, Consensus), Grandpa), ...)
+       // Make nested tuple structure like ((FirstModule, (SecondModule, (..., (LastModule)...))))
        // But ignore the system module.
        let all_modules = names.iter()
                .filter(|n| **n != SYSTEM_MODULE_NAME)
+               .rev()
                .fold(TokenStream2::default(), |combined, name| quote!((#name, #combined)));
 
        quote!(

But it needs tests and to make sure it doesn't break anything in kusama/polkadot.

Maybe this should be fixed with a flag in construct_runtime so it is opt-in for other chains.

[gui1117]

maybe this should be into substrate 2.0

[gui1117]

maybe one solution is to fix it and allow to keep old behavior with an attribute in construct_runtime:

construct_runtime!(
#[set_all_module_reversed_order]
	pub enum Runtime where
		Block = Block,
		NodeBlock = node_primitives::Block,
		UncheckedExtrinsic = UncheckedExtrinsic
	{
...
})

[gavofyork]

I believe two of the consensus pallets' on_initialize is sensitive to their ordering.

[apopiak]

IMO we will want to have the same order for all hooks, including on_runtime_upgrade. (And some runtime upgrades are sensitive to the order.)
Maybe we will want on_finalize in reverse order.

[gui1117]

maybe we can use some on_finalize_requirement and on_initialize_requirement in hooks, those would return the required pallet to be executed before.

enum HookRequirement {
  PalletIndex(u32),
  Misc([u8; 8]),
}

impl Hooks for Pallet {
  fn on_finalize_requirement() -> Vec<HookRequirement> { ... }
}

[stale]

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

[gui1117]

issue is still relevant