Repository: paritytech/substrate
File: frame/nomination-pools/src/migration.rs (Rust)

Code before:

		#[cfg(feature = "try-runtime")]
		fn post_upgrade(_: Vec<u8>) -> Result<(), &'static str> {
			// ensure all BondedPools items now contain an `inner.commission: Commission` field.
			ensure!(
				BondedPools::<T>::iter().all(|(_, inner)|
					// Check current
					(inner.commission.current.is_none() ||
					inner.commission.current.is_some()) &&
					// Check max
					(inner.commission.max.is_none() || inner.commission.max.is_some()) &&
					// Check change_rate
					(inner.commission.change_rate.is_none() ||
					inner.commission.change_rate.is_some()) &&
					// Check throttle_from
					(inner.commission.throttle_from.is_none() ||
					inner.commission.throttle_from.is_some())),
				"a commission value has not been set correctly"
			);
			ensure!(
				GlobalMaxCommission::<T>::get() == Some(U::get()),
				"global maximum commission error"
			);
			ensure!(
				Pallet::<T>::on_chain_storage_version() >= 4,
				"nomination-pools::migration::v4: wrong storage version"
			);
			Ok(())
		}
	}
}

pub mod v5 {
	use super::*;

	#[derive(Decode)]
	pub struct OldRewardPool<T: Config> {
		last_recorded_reward_counter: T::RewardCounter,
		last_recorded_total_payouts: BalanceOf<T>,
		total_rewards_claimed: BalanceOf<T>,
	}

	impl<T: Config> OldRewardPool<T> {
		fn migrate_to_v5(self) -> RewardPool<T> {
			RewardPool {
				last_recorded_reward_counter: self.last_recorded_reward_counter,
				last_recorded_total_payouts: self.last_recorded_total_payouts,
				total_rewards_claimed: self.total_rewards_claimed,
				total_commission_pending: Zero::zero(),
				total_commission_claimed: Zero::zero(),
			}

Review comment (bug):
This `ensure!` was failing on Rococo I guess because commission became no longer `None` for everything after people started using the pallet. 

But the checks seem kind of like a noop (checking is_some || is_none). Can we just remove them? 

Code after:
			}
		}

		#[cfg(feature = "try-runtime")]
		fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
			Ok(Vec::new())
		}

		#[cfg(feature = "try-runtime")]
		fn post_upgrade(_: Vec<u8>) -> Result<(), TryRuntimeError> {
			// ensure all BondedPools items now contain an `inner.commission: Commission` field.
			ensure!(
				BondedPools::<T>::iter().all(|(_, inner)|
					// Check current
					(inner.commission.current.is_none() ||
					inner.commission.current.is_some()) &&
					// Check max
					(inner.commission.max.is_none() || inner.commission.max.is_some()) &&
					// Check change_rate
					(inner.commission.change_rate.is_none() ||
					inner.commission.change_rate.is_some()) &&
					// Check throttle_from
					(inner.commission.throttle_from.is_none() ||
					inner.commission.throttle_from.is_some())),
				"a commission value has not been set correctly"
			);
			ensure!(
				GlobalMaxCommission::<T>::get() == Some(U::get()),
				"global maximum commission error"
			);
			ensure!(
				Pallet::<T>::on_chain_storage_version() >= 4,
				"nomination-pools::migration::v4: wrong storage version"
			);
			Ok(())
		}
	}
}

pub mod v5 {
	use super::*;

	#[derive(Decode)]
	pub struct OldRewardPool<T: Config> {
		last_recorded_reward_counter: T::RewardCounter,
		last_recorded_total_payouts: BalanceOf<T>,
		total_rewards_claimed: BalanceOf<T>,
	}

	impl<T: Config> OldRewardPool<T> {
		fn migrate_to_v5(self) -> RewardPool<T> {