Introduce on_runtime_upgrade similarly to on_initialize but executed before.

[gui1117]

With the recent introduction of StorageValue::translate a new pattern appeared in on_initialize:

fn on_initialize() {
    if new_runtime.get() == true {
        MyStorageValue::translate(...);
        new_runtime.put(false);
    }
}

The error proneness is that on_initialize are executed in the order declaration in construct_runtime. Thus if one upper module in on_initialize call your module and use this storage, then the value is still the old encoded one. And thus can leads to very error.

To solve this we can introduce a new function: on_runtime_upgrade_initialize which will be call before all on_initialize.
With this the code to check is just the code contained in on_runtime_upgrade_initialize of other module which logic is easier to audit.

cc @shawntabrizi

[gui1117]

note: even if we execute on_runtime_upgrade just before on_initialize of next block we still have some logic executing in between for validating transaction:

such logic is regular validate transaction but also the per-pallet defined signed extensions, this signed extensions logic could fail.

Maybe the validate_transaction could return an error with transaction_validity::UnknownTransaction::CannotLookUp on runtime upgrade.

Either on all transaction but that means having an empty block after each upgrade (which looks fine) or only for transaction that have some signed extension define in module.

EDIT: having an empty block after an update looks very fine to me actually, or making the queue aware that transactions invalid for this block should be kept.

[gui1117]

note: the staking module has update its storage one time, you can see an example of a migration code in frame/staking/src/migration.rs

This kind of transition work for staking but in case of SignedExtensions this has one caveat the validation of transaction in the queue could panic, thus probably having them removed from the queue. Or the signed extension validation should do the storage migration, but then this validation function can cost a lot.

[shawntabrizi]

Expanding on this idea, it is very possible that post-runtime-upgrade functions may want to execute logic which would take more than a single block.

In magical dream land, on_runtime_upgrade would be special such that it could handle complex logic which could take multiple blocks, and Substrate would be able to correctly handle this.

For example: Let's imagine we want to update the balances module of a live chain such that the balance type is u256 rather than u128 or something like that.

With thousands, maybe millions of accounts, a storage upgrade (#4555) would not complete in just one block.

But, if a runtime developer could simply write in the on_runtime_upgrade: "Upgrade all the balances", and magically, it should be able to batch the storage migration over multiple blocks.

As long as there is still some on_runtime_upgrade logic to be run, the blockchain will not accept any other extrinsics or runtime tasks, but the blockchain will not halt or stop producing blocks.

[gui1117]

we could also think as non-blocking migration, like we create a new storages and do as such:

• insert value: remove value from storage1 and insert new value in storage2
• remove: remove value from both storages
• get: get value from storage2 if none then get value from storage1 translate it and insert it to storage2 and return it.
• and maybe at initialization of each block we translate the nth first value of storage1

[shawntabrizi]

Yes, a lazy migration pattern would also be very useful.

[bkchr]

How would you know that the item was already migrated?

[shawntabrizi]

You would be able to check storage2(item).exists(), right?

[bkchr]

Why would the key change? And if we had 2 keys, we would need to check both keys until the end of the universe, which also requires to have one extra storage access, each time.

[gui1117]

my idea for lazy_migration was to do this as well:

and maybe at initialization of each block we translate the nth first value of storage1

Then after some block the entire storage1 will be empty thus we no longer need to check it.

It would be a performance regression for only the time of the migration.