paritytech / substrate

Substrate: The platform for blockchain innovators
Apache License 2.0
8.39k stars 2.65k forks source link

[Improvement] Make sure try-runtime executes migrations and their pre/post checks sequentially. #12295

Closed ruseinov closed 2 years ago

ruseinov commented 2 years ago

The problem:

When we have several migrations for the same pallet supplied for the runtime - their pre-update checks are being executed before any of the migrations, which means that at least storage version checks are failing. This makes it impossible to test several migrations at a time, which sometimes is a necessity and requires manual intervention.

Code example:

pub type Executive = frame_executive::Executive<
    Runtime,
    Block,
    frame_system::ChainContext<Runtime>,
    Runtime,
    AllPalletsWithSystem,
    (
        pallet_nomination_pools::migration::v3::MigrateToV3<Runtime>,
        pallet_staking::migrations::v11::MigrateToV11<
            Runtime,
            VoterList,
            StakingMigrationV11OldPallet,
        >,
        pallet_staking::migrations::v12::MigrateToV12<Runtime>, // THIS will fail due to failing pre-check, because the actual storage version will still be pre-v11. 
    ),
>;

The solution: We need to somehow make sure that the execution is sequential:

  1. pre-upgrade-v11, upgrade-v11, post-upgrade-v11
  2. pre-upgrade-v12, upgrade-v12, post-upgrade-v12

This needs further investigation to see how to make the scenario possible.

cc @kianenigma

bkchr commented 2 years ago

The implementation of the OnRuntimeUpgrade trait for tuples could, if try-runtime feature is enabled, run pre_check and post_check before/after the on_runtime_upgrade call of a single tuple element. This should solve your problem here.

ruseinov commented 2 years ago

@bkchr yep, I'm going to investigate this today.