[FRAME Core] Simple Multi-block Migrations

[gavofyork]

Needed for non-trivial migrations on parachains, and helpful for such migration on Relay/solo chains.

This is just a simple strategy and will work in probably around 80% of circumstances. It's good for pallets which are not depended upon by code which fires in on_initialize.

Have a migrations pallet (not too dissimilar from that of Moonbeam). This is configured with a tuple of all migrations, which provides introspection and access to each migration which needs to be completed. Migrations look like calls/tasks: they have a possibly-pessimistic weight estimation and can return the actual weight consumed. They are always benchmarked. Migrations should be written to proceed "piecewise" (e.g. one unit at a time, rather than migrating all entries of a storage map all at once).

Each migration defines a type Cursor which is Default + FullCodec + MaxEncodeLen. The migration function accepts a Cursor value and returns an Option<Cursor> value which is None if the migration is complete. If Some then it is up to the migration pallet to ensure it is used in the next call to the migrate function, storing it in state if weight constraints deem it necessary to wait until the next block before resuming.

The migrations pallet should suspend all non-critical subsystems in on_runtime_upgrade. This includes XCM and transaction processing. It should only re-enable them when all migrations' cursors are None.

One new hook function should be added: fn poll() -> Weight. This is guaranteed to execute regularly but not necessarily every block, providing a softer version of on_initialize. All Substrate pallets should have their on_initialize/on_finalize functions checked; if they can be moved or adapted to poll, they should be. While on_initialize/on_finalize should be called within an incomplete migration, poll need not (and indeed, should not) be.

Long-term

Deprecate on_* hooks

on_initialize/on_finalize should be deprecated. Pallets which absolutely require per-block-execution hooks on hard-deadlines (HDH) or mandatory extrinsics (ME) should go through a System config trait item which can give them low-level access at the cost of requiring the System config trait to be explicitly configured to reference them. There should be a regular facility for soft-deadline execution (which will work fine in almost all use-cases of parachain teams) which is implemented using tasks.

Guarantees

We should mark all storage items so they fall into one of three groups:

1. Can be accessed during a hard-deadline-hook or mandatory extrinsic
2. Can be accessed during a multi-block-migration
3. Unavailable to both MBMs and HDHs/MIs (the default)

Ideally we could have some means of statically verifying that HDH code only accessed items marked as (1) and MBM code only accessed items marked as (2).

Unfortunately, this probably cannot be done statically (can it?) but at least could form a basis for code reviews and audit.

[athei]

I am currently implementing a multi block migration system for pallet-contracts (#13638) that uses a very similar design. Getting this done quickly is time critical. So I would still merge the contracts specific migration system. But I can generalize and move it into FRAME afterwards.

[kianenigma]

on_initialize/on_finalize should be deprecated. Pallets which absolutely require per-block-execution hooks on hard-deadlines should go through a System config trait item which can give them low-level access at the cost of requiring the System config trait to be explicitly configured to reference them. There should be a regular facility for soft-deadline execution (which will work fine in almost all use-cases of parachain teams) which is implemented using tasks.

My 2 cents is that we should not make using things like on_initialize too hard, as it is a major selling point of "why should I build a pallet instead of a contract". Moreover, most of the headache related to usage of these hooks is related to parachains, not solo-chains.

So while

System config trait item which can give them low-level access at the cost of requiring the System config trait to be explicitly configured to reference them

Doesn't sound too bad, I would consider keeping it a bit simpler, something some marker along the lines of unsafe fn in Rust.

[gavofyork]

Yes that's actually what I thought of first and I'd not be against a requirement to mark it #[allow(deprecated)] or something. But I figured we do want to strongly discourage usage since it not only makes the pallet itself un-multi-block-migratable, but it also makes any pallets it uses un-multi-block-migratable. In effect it's super toxic and I can imagine random ecosystem devs not familiar with this tradeoff just automatically using it out of short-sighted laziness, leading to chains using it unable to migrate their pallets (since the migration code would automatically assume multi-block).

While on_initialize is useful, I think there are enough other things (tasks, for a start) which Frame can provide to make it basically fine to reduce the visibility of.

[gavofyork]

Also, by making it more explicit, we solve one other notable issue with the use of on_initialize: ordering. Currently some pallets must be placed with lower indices than others in the construct_runtime macro which is less than great. If we insisted on explicit knitting of these pallets (probably via traits and config items) then we'd reduce the possibility of accidental mis-configuration.

[athei]

Okay bear with me as this might be shaving the yak. I can spin that out to another issue later but I just want to write it down here for now: In my prototype that I am building right now the migration introspection also includes the storage version it upgrades to. Right, now that allows me to check statically that the sequence of migrations makes sense (is in order without holes). It allows my to check dynamically (during try-runtime) that the sequence of migrations contains all the needed migrations for the current storage.

What if we emit the information about the migrations that a runtime contains into a custom section. We also need to emit the in-code version of each pallet. We can then check on set_code that it has all the required migrations. I think this will remove another a big pain point of migrations: Making sure that a needed migration wasn't removed.

Maybe we could even emit each migration into its own custom section and then make offline tooling only include the required migrations when uploading the runtime. Then we might not need to periodically remove them for size reasons.

[ggwpez]

Yes I think that is its own issue @athei

We have some things planned to avoid the issue of forgetting migrations:

• Versioned OnRuntimeUpgrade substrate#13107
• Improve handling of unset StorageVersion substrate#13417
• TryDecodeEntireState check for storage types and pallets substrate#13013

I dont get what you mean with "custom section". Do you think the mentioned issue above is not enough?

[athei]

I dont get what you mean with "custom section". Do you think the mentioned issue above is not enough?

With custom section I mean a section in a wasm file. We use that to store the runtime version for example. This would allow us to introspect a wasm file whether it contains all required migrations before deploying it. It would probably happen off-chain by tooling which then removes the sections before upload.

It is something that comes on top of paritytech/substrate#13107.

[juangirini]

I want to bring @pgherveou in the loop as he's currently working on paritytech/substrate#13638

[ggwpez]

Just thinking about whether per-pallet migrations could be come feasible again with MB migration (i dont think so):

Originally, the migrations were part of the pallets in on_runtime_upgrade. This was easy to use for the runtime developers,
since they dont have to configure anything and can thereby not miss migrations.
We stopped doing this since it could stall parachains that execute a slow pallet migration. Now with the MBM, we could go back to per-pallet migration because they won't stall parachains anymore. It could be done with static configuration via tuples or dynamic dispatch.

But for both approaches i see the biggest downside being the ever-increasing WASM size…