Multi-Block-Migrations, poll hook and new System callbacks

[ggwpez]

This MR is the merge of paritytech/substrate#14414 and paritytech/substrate#14275. It implements RFC#13, closes #198.

---

This Merge request introduces three major topicals:

1. Multi-Block-Migrations
2. New pallet poll hook for periodic service work
3. Replacement hooks for on_initialize and on_finalize in cases where poll cannot be used

and some more general changes to FRAME.
The changes for each topical span over multiple crates. They are listed in topical order below.

1.) Multi-Block-Migrations

Multi-Block-Migrations are facilitated by creating pallet_migrations and configuring System::Config::MultiBlockMigrator to point to it. Executive picks this up and triggers one step of the migrations pallet per block.
The chain is in lockdown mode for as long as an MBM is ongoing. Executive does this by polling MultiBlockMigrator::ongoing and not allowing any transaction in a block, if true.

A MBM is defined through trait SteppedMigration. A condensed version looks like this:

/// A migration that can proceed in multiple steps.
pub trait SteppedMigration {
	type Cursor: FullCodec + MaxEncodedLen;
	type Identifier: FullCodec + MaxEncodedLen;

	fn id() -> Self::Identifier;

	fn max_steps() -> Option<u32>;

	fn step(
		cursor: Option<Self::Cursor>,
		meter: &mut WeightMeter,
	) -> Result<Option<Self::Cursor>, SteppedMigrationError>;
}

pallet_migrations can be configured with an aggregated tuple of these migrations. It then starts to migrate them one-by-one on the next runtime upgrade.
Two things are important here:

• 1. Doing another runtime upgrade while MBMs are ongoing is not a good idea and can lead to messed up state.
• 2. Pallet Migrations MUST BE CONFIGURED IN System::Config, otherwise it is not used.

The pallet supports an UpgradeStatusHandler that can be used to notify external logic of upgrade start/finish (for example to pause XCM dispatch).

Error recovery is very limited in the case that a migration errors or times out (exceeds its max_steps). Currently the runtime dev can decide in FailedMigrationHandler::failed how to handle this. One follow-up would be to pair this with the SafeMode pallet and enact safe mode when an upgrade fails, to allow governance to rescue the chain. This is currently not possible, since governance is not Mandatory.

Runtime API

• Core: initialize_block now returns ExtrinsicInclusionMode to inform the Block Author whether they can push transactions.

Integration

Add it to your runtime implementation of Core and BlockBuilder:

diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs
@@ impl_runtime_apis! {
	impl sp_block_builder::Core<Block> for Runtime {
-		fn initialize_block(header: &<Block as BlockT>::Header) {
+		fn initialize_block(header: &<Block as BlockT>::Header) -> ExtrinsicInclusionMode {
			Executive::initialize_block(header)
		}

		...
	}

2.) poll hook

A new pallet hook is introduced: poll. Poll is intended to replace mostly all usage of on_initialize.
The reason for this is that any code that can be called from on_initialize cannot be migrated through an MBM. Currently there is no way to statically check this; the implication is to use on_initialize as rarely as possible.
Failing to do so can result in broken storage invariants.

The implementation of the poll hook depends on the Runtime API changes that are explained above.

3.) Hard-Deadline callbacks

Three new callbacks are introduced and configured on System::Config: PreInherents, PostInherents and PostTransactions.
These hooks are meant as replacement for on_initialize and on_finalize in cases where the code that runs cannot be moved to poll.
The reason for this is to make the usage of HD-code (hard deadline) more explicit - again to prevent broken invariants by MBMs.

4.) FRAME (general changes)

frame_system pallet

A new memorize storage item InherentsApplied is added. It is used by executive to track whether inherents have already been applied. Executive and can then execute the MBMs directly between inherents and transactions.

The Config gets five new items:

• SingleBlockMigrations this is the new way of configuring migrations that run in a single block. Previously they were defined as last generic argument of Executive. This shift is brings all central configuration about migrations closer into view of the developer (migrations that are configured in Executive will still work for now but is deprecated).
• MultiBlockMigrator this can be configured to an engine that drives MBMs. One example would be the pallet_migrations. Note that this is only the engine; the exact MBMs are injected into the engine.
• PreInherents a callback that executes after on_initialize but before inherents.
• PostInherents a callback that executes after all inherents ran (including MBMs and poll).
• PostTransactions in symmetry to PreInherents, this one is called before on_finalize but after all transactions.

A sane default is to set all of these to (). Example diff suitable for any chain:

@@ impl frame_system::Config for Test {
 	type MaxConsumers = ConstU32<16>;
+	type SingleBlockMigrations = ();
+	type MultiBlockMigrator = ();
+	type PreInherents = ();
+	type PostInherents = ();
+	type PostTransactions = ();
 }

An overview of how the block execution now looks like is here. The same graph is also in the rust doc.

Inherent Order

Moved to #2154

---

TODO

• Check that try-runtime still works
• Ensure backwards compatibility with old Runtime APIs
• Consume weight correctly
• Cleanup

[noot0xnoot]

Really great pallet docs 👌

Just a few small comments

[noot0xnoot]

Executive now additionally takes an MultiStepMigrator trait that defaults to ().
IT IS PARAMOUNT TO SET THIS TO THE MIGRATIONS PALLET WHEN YOU DEPLOY IT.

Can we enforce this somehow?

[ggwpez]

Executive now additionally takes an MultiStepMigrator trait that defaults to ().
IT IS PARAMOUNT TO SET THIS TO THE MIGRATIONS PALLET WHEN YOU DEPLOY IT.

Can we enforce this somehow?

I think so. I will try to add a check into the integrity_test, otherwise this is a huge footgun.

[nazar-pc]

I think I see the confusion. The runtime is not wrong, the dev runtime generated by gemini-3h-2024-mar-29 release is similar to what our live chain uses, I suggested to generate chain spec from it intentionally to check compatibility with new client. Upgraded client with main branch using ac2f3efb476ee3f5ac6bafefb458e9be158adb7f should still be able to work with older runtime that doesn't have new APIs, this is what versioning exists for. But looks like versioning is not checked properly somewhere in block authoring code and it fails to produce a block due to result decoding error.

[nazar-pc]

@ggwpez I think I see where the bug is.

This:

	#[api_version(5)]
	pub trait Core {
		/// Returns the version of the runtime.
		fn version() -> RuntimeVersion;
		/// Execute the given block.
		fn execute_block(block: Block);
		/// Initialize a block with the given header.
		#[changed_in(5)]
		#[renamed("initialise_block", 2)]
		fn initialize_block(header: &<Block as BlockT>::Header);
		/// Initialize a block with the given header and return the runtime executive mode.
		fn initialize_block(header: &<Block as BlockT>::Header) -> ExtrinsicInclusionMode;
	}

And this:

		let extrinsic_inclusion_mode = if core_version >= 5 {
			api.initialize_block(parent_hash, &header)?
		} else {
			#[allow(deprecated)]
			api.initialize_block_before_version_5(parent_hash, &header)?;
			ExtrinsicInclusionMode::AllExtrinsics
		};

Are both probably wrong. I checked and old runtime's version was already at version 5, so if you're making breaking changes to API you should have bumped version to 6 and checked core_version >= 6.

Otherwise since my old runtime that returns () is already at version 5, the whole thing inevitably breaks.

[nazar-pc]

Found it, it was caused by this custom patch in our fork of Substrate that bumped the version 😞 : https://github.com/substrate/polkadot-sdk/commit/447bbc765020674614e9ac982163f7e11e5b03ea

[tmcgroul]

hi there from Subsquid team! we've discovered that this pr introduces event pointing to non-existing extrinsics which is a "crime" in context of data indexing :)
here is a hack that i came up with to handle multi-block migrations - subsquid/squid-sdk@cbae638
i’m wondering if there was another way to implement this feature and similar features in the future. would actual extrinsic be an option? smth like Migrations.progress or Migrations.action. i also believe that a new phase would help to distinguish migration and non-migration events.

[ggwpez]

Hey i personally use Subsquid, nice work!

What is inconsistency that this introduces? You mean an event references an extrinsicIndex but there is no extrinsic?

[tmcgroul]

You mean an event references an extrinsicIndex but there is no extrinsic?

yes, exactly this

[ggwpez]

Originally we wanted to merge this right after: #3666
I have to double-check that it fixes what you describe, but my memory is that it should.

[tmcgroul]

got it! thanks for the inside