substrate icon indicating copy to clipboard operation
substrate copied to clipboard

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

Open ruseinov opened this issue 3 years ago • 2 comments

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

ruseinov avatar Sep 19 '22 10:09 ruseinov

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.

bkchr avatar Sep 20 '22 21:09 bkchr

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

ruseinov avatar Sep 21 '22 09:09 ruseinov