Repository: paritytech/substrate
File: frame/support/src/migrations.rs (Rust)

Code before:
	_marker: PhantomData<(From, To, Inner, Pallet, Weight)>,
}

/// Implementation of the `OnRuntimeUpgrade` trait for `VersionedRuntimeUpgrade`.
///
/// Primarily, it
/// 1. Logs a warning in `pre_upgrade` if the storage version specified in the pallet appears to be
/// misconfigured before returning `Inner::pre_upgrade`.
///
/// 2. Performs the actual runtime upgrade in `on_runtime_upgrade` only if the on-chain version of
/// the pallets storage matches `From`, and after the upgrade sets the on-chian storage to `To`. If
/// the versions do not match, it writes a log notifying the developer that the migration is a noop.
impl<
		From: Get<u16>,
		To: Get<u16>,
		Inner: OnRuntimeUpgrade,
		Pallet: GetStorageVersion<CurrentStorageVersion = StorageVersion> + PalletInfoAccess,
		DbWeight: Get<RuntimeDbWeight>,
	> OnRuntimeUpgrade for VersionedRuntimeUpgrade<From, To, Inner, Pallet, DbWeight>
{
	/// VersionedRuntimeUpgrade pre-upgrade checks.
	///
	/// Logs a warning if the storage version specified in the pallet appears to be misconfigured,
	/// before returning `Inner::pre_upgrade`.
	#[cfg(feature = "try-runtime")]
	fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
		let current_version = Pallet::current_storage_version();
		if current_version < To::get() {
			log::warn!(
				"Pallet '{}' appears to be misconfigured: its version ({:?}) is less than the on-chain version that will be set by this VersionedRuntimeUpgrade ({}). Please ensure the pallet version is set correctly.",
				Pallet::name(),
				current_version,
				To::get(),
			);
		}

		Inner::pre_upgrade()
	}

	/// Actually executes the versioned runtime upgrade.
	///
	/// First checks if the pallet's on-chain storage version matches the version of this upgrade.
	/// If it matches, it calls `Inner::on_runtime_upgrade`, increments the on-chain version, and
	/// returns the weight. If it does not match, it writes a log notifying the developer that the
	/// migration is a noop.
	fn on_runtime_upgrade() -> Weight {
		let on_chain_version = Pallet::on_chain_storage_version();
		if on_chain_version == From::get() {
			// Execute the migration
			let weight = Inner::on_runtime_upgrade();


Review comment (suggestion):
For readability purposes, I would consider moving `Pallet` and `DbWeight` to be associated types rather than generics and hardcoded. This might require you to add another trait. When a pallet is implementing this new trait, it will fix its `Pallet` and `DbWeight` (both of which can be obtained from `system`).

Then, in the runtime, when you are wrapping things in `VersionedRuntimeUpgrade`, you only see `(VersionedRuntimeUpgrade<1, 2, Foo>, VersionedRuntimeUpgrade<5, 6, Bar>)`. 

Possibly not worth it, especially as I am still critical of the fundamental approach. 

Code after:
}

/// A helper enum to wrap the pre_upgrade bytes like an Option before passing them to post_upgrade.
/// This enum is used rather than an Option to make the API clearer to the developer.
#[cfg(feature = "experimental")]
#[derive(codec::Encode, codec::Decode)]
pub enum VersionedPostUpgradeData {
	/// The migration ran, inner vec contains pre_upgrade data.
	MigrationExecuted(Vec<u8>),
	/// This migration is a noop, do not run post_upgrade checks.
	Noop,
}

/// Implementation of the `OnRuntimeUpgrade` trait for `VersionedRuntimeUpgrade`.
///
/// Its main function is to perform the runtime upgrade in `on_runtime_upgrade` only if the on-chain
/// version of the pallets storage matches `From`, and after the upgrade set the on-chain storage to
/// `To`. If the versions do not match, it writes a log notifying the developer that the migration
/// is a noop.
#[cfg(feature = "experimental")]
impl<
		const FROM: u16,
		const TO: u16,
		Inner: OnRuntimeUpgrade,
		Pallet: GetStorageVersion<CurrentStorageVersion = StorageVersion> + PalletInfoAccess,
		DbWeight: Get<RuntimeDbWeight>,
	> OnRuntimeUpgrade for VersionedRuntimeUpgrade<FROM, TO, Inner, Pallet, DbWeight>
{
	/// Executes pre_upgrade if the migration will run, and wraps the pre_upgrade bytes in
	/// [`VersionedPostUpgradeData`] before passing them to post_upgrade, so it knows whether the
	/// migration ran or not.
	#[cfg(feature = "try-runtime")]
	fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
		use codec::Encode;
		let on_chain_version = Pallet::on_chain_storage_version();
		if on_chain_version == FROM {
			Ok(VersionedPostUpgradeData::MigrationExecuted(Inner::pre_upgrade()?).encode())
		} else {
			Ok(VersionedPostUpgradeData::Noop.encode())
		}
	}

	/// Executes the versioned runtime upgrade.
	///
	/// First checks if the pallets on-chain storage version matches the version of this upgrade. If
	/// it matches, it calls `Inner::on_runtime_upgrade`, updates the on-chain version, and returns
	/// the weight. If it does not match, it writes a log notifying the developer that the migration
	/// is a noop.
	fn on_runtime_upgrade() -> Weight {
		let on_chain_version = Pallet::on_chain_storage_version();
		if on_chain_version == FROM {