[FRAME] Simple multi block migrations

[ggwpez]

Partial paritytech/polkadot-sdk#198 (on_poll in #14279).

pallet-migrations

This pallet takes a configuration of migrations Vec<Box<dyn SteppedMigration<…>>> and executes these in order on_initialize. The execution starts on each runtime upgrade. The unique identifiers of each executed migration are stored to prevent accidental re-execution of old ones.
While migrations are executed, only Mandatory extrinsics and inherents are allowed to execute. This is accomplished by implementing MultiStepMigrator for the pallet and querying it in frame-executive before each non-mandatory TX. Concretely this means that these transactions will panic and make a block un-importable. The BlockBuilder respects this limitation and retains all extrinsics in the pool while migrations are ongoing. Once the MBMs are done, these extrinsics will start to be included again. From a user's perspective this just results in an increased time until the TX is included.

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 UpgradeStatusHandler::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.

frame-executive + BlockBuilder

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

The block builder is aware of the new after_inherents function and uses it to figure out the MBM state.
As comparison the old and new logic (changes with +):

Current order:
	on_runtime_upgrade
	System::initialize
	on_initialize
	apply inherents
	apply extrinsics
	on_idle
	on_finalize

New order:
	on_runtime_upgrade
	System::initialize
	on_initialize
	apply inherents
+	after_inherents:
+		progress mbms
+		poll TBD
+	If not MBM:
+		apply extrinsics
+		on_idle
	on_finalize

Runtime API

• BlockBuilder: Added after_inherents to progress ongoing MBMs and the poll hook TBD in FRAME: Add new poll hook + various other improvements to pallet hooks #14279.

frame-support

• SteppedMigration: A multi-step migration.
• ExtrinsicSuspenderQuery: Implemented by the migrations pallet and used by frame-executive to check TX suspension.
• UpgradeStatusHandler: React to upgrade start, completion and failure.
• FailedUpgradeHandling: Decide how to handle a failed upgrade. Either ForceUnstuck or KeepStuck.

TODO

• Consume weight correctly
• Cleanup

[kianenigma]

Vec is also fine here, but I have usually seen types like [8u; 32] being used for such cases.

[ggwpez]

I also made it generic. But with the new config defaults, we could add some sane default like this 🤔

[kianenigma]

So far everything LGTM.

Can fn poll() be done in a separate PR? I presume yes, and perhaps it would be good for someone to start that already.

One important topic related to https://github.com/paritytech/substrate/pull/14275/files#r1211972428 is also tx-pool validation. @ggwpez and I discussed if fn validate_transaction should also be aware of transaction suspension, and signal the pool that everything is "temporarily" invalid.

[liamaharon]

@kianenigma am I right to assume we'll want to make it easy to test these with try-runtime-cli? Ideally should be as easy as on-runtime-upgrade, maybe make it auto-mine blocks until all multi-block migrations are complete & provide a hook that runs afterwards?

[xlc]

Does this pauses user tx and incoming XCM etc? because otherwise it will be bad.

[ggwpez]

Does this pauses user tx and incoming XCM etc? because otherwise it will be bad.

It pauses non-mandatory extrinsics in frame-executive by checking a flag. For XCM we probably need a suspension and resume hook additionally.

But now that Im thinking about this again; this would make it impossible to rescue if one of the migrations errors, since governance requires things like the referenda pallet.
So maybe in the case that something errors, it could be put into SafeMode to re-allow few extrinsics.

[kianenigma]

@kianenigma am I right to assume we'll want to make it easy to test these with try-runtime-cli? Ideally should be as easy as on-runtime-upgrade, maybe make it auto-mine blocks until all multi-block migrations are complete & provide a hook that runs afterwards?

Good to think about it early, and yes I think it is possible with little effort. I believe follow-chain will already do everything needed to test an MBM?

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-benches
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3092555

[paritytech-cicd-pr]

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-check-benches
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3092556

[kianenigma]

Is this more or less ready for final review?

[ggwpez]

Still first need to merge #14414. I will ping you there for review when its ready.