Fix nomination pools unbonding logic

bin/node/runtime/src/lib.rs

@@ -68,7 +68,7 @@ use sp_runtime::{
 		SaturatedConversion, StaticLookup,
 	},
 	transaction_validity::{TransactionPriority, TransactionSource, TransactionValidity},
-	ApplyExtrinsicResult, FixedPointNumber, Perbill, Percent, Permill, Perquintill,
+	ApplyExtrinsicResult, FixedPointNumber, FixedU128, Perbill, Percent, Permill, Perquintill,
 };
 use sp_std::prelude::*;
 #[cfg(any(feature = "std", test))]
@@ -730,7 +730,7 @@ impl pallet_bags_list::Config for Runtime {
 parameter_types! {
 	pub const PostUnbondPoolsWindow: u32 = 4;
 	pub const NominationPoolsPalletId: PalletId = PalletId(*b"py/nopls");
-	pub const MinPointsToBalance: u32 = 10;
+	pub const MaxPointsToBalance: u8 = 10;
 }
 
 use sp_runtime::traits::Convert;
@@ -751,14 +751,16 @@ impl pallet_nomination_pools::Config for Runtime {
 	type WeightInfo = ();
 	type Event = Event;
 	type Currency = Balances;
+	type CurrencyBalance = Balance;
+	type RewardCounter = FixedU128;
 	type BalanceToU256 = BalanceToU256;
 	type U256ToBalance = U256ToBalance;
 	type StakingInterface = pallet_staking::Pallet<Self>;
 	type PostUnbondingPoolsWindow = PostUnbondPoolsWindow;
 	type MaxMetadataLen = ConstU32<256>;
 	type MaxUnbonding = ConstU32<8>;
 	type PalletId = NominationPoolsPalletId;
-	type MinPointsToBalance = MinPointsToBalance;
+	type MaxPointsToBalance = MaxPointsToBalance;
 }
 
 parameter_types! {
@@ -1675,6 +1677,7 @@ pub type Executive = frame_executive::Executive<
 	frame_system::ChainContext<Runtime>,
 	Runtime,
 	AllPalletsWithSystem,
+	pallet_nomination_pools::migration::v2::MigrateToV2<Runtime>,
 >;
 
 /// MMR helper types.

frame/nomination-pools/benchmarking/src/mock.rs

@@ -17,7 +17,10 @@
 
 use frame_election_provider_support::VoteWeight;
 use frame_support::{pallet_prelude::*, parameter_types, traits::ConstU64, PalletId};
-use sp_runtime::traits::{Convert, IdentityLookup};
+use sp_runtime::{
+	traits::{Convert, IdentityLookup},
+	FixedU128,
+};
 
 type AccountId = u128;
 type AccountIndex = u32;
@@ -144,21 +147,23 @@ impl Convert<sp_core::U256, Balance> for U256ToBalance {
 parameter_types! {
 	pub static PostUnbondingPoolsWindow: u32 = 10;
 	pub const PoolsPalletId: PalletId = PalletId(*b"py/nopls");
-	pub const MinPointsToBalance: u32 = 10;
+	pub const MaxPointsToBalance: u8 = 10;
 }
 
 impl pallet_nomination_pools::Config for Runtime {
 	type Event = Event;
 	type WeightInfo = ();
 	type Currency = Balances;
+	type CurrencyBalance = Balance;
+	type RewardCounter = FixedU128;
 	type BalanceToU256 = BalanceToU256;
 	type U256ToBalance = U256ToBalance;
 	type StakingInterface = Staking;
 	type PostUnbondingPoolsWindow = PostUnbondingPoolsWindow;
 	type MaxMetadataLen = ConstU32<256>;
 	type MaxUnbonding = ConstU32<8>;
 	type PalletId = PoolsPalletId;
-	type MinPointsToBalance = MinPointsToBalance;
+	type MaxPointsToBalance = MaxPointsToBalance;
 }
 
 impl crate::Config for Runtime {}

frame/nomination-pools/src/migration.rs

@@ -16,11 +16,12 @@
 // limitations under the License.
 
 use super::*;
+use crate::log;
+use frame_support::traits::OnRuntimeUpgrade;
+use sp_std::collections::btree_map::BTreeMap;
 
 pub mod v1 {
 	use super::*;
-	use crate::log;
-	use frame_support::traits::OnRuntimeUpgrade;
 
 	#[derive(Decode)]
 	pub struct OldPoolRoles<AccountId> {
@@ -103,3 +104,282 @@ pub mod v1 {
 		}
 	}
 }
+
+pub mod v2 {
+	use super::*;
+	use sp_runtime::Perbill;
+
+	#[test]
+	fn migration_assumption_is_correct() {
+		// this migrations cleans all the reward accounts to contain exactly ed, and all members
+		// having no claimable rewards. In this state, all fields of the `RewardPool` and
+		// `member.last_recorded_reward_counter` are all zero.
+		use crate::mock::*;
+		ExtBuilder::default().build_and_execute(|| {
+			let join = |x| {
+				Balances::make_free_balance_be(&x, Balances::minimum_balance() + 10);
+				frame_support::assert_ok!(Pools::join(Origin::signed(x), 10, 1));
+			};
+
+			assert_eq!(BondedPool::<Runtime>::get(1).unwrap().points, 10);
+			assert_eq!(
+				RewardPools::<Runtime>::get(1).unwrap(),
+				RewardPool { ..Default::default() }
+			);
+			assert_eq!(
+				PoolMembers::<Runtime>::get(10).unwrap().last_recorded_reward_counter,
+				Zero::zero()
+			);
+
+			join(20);
+			assert_eq!(BondedPool::<Runtime>::get(1).unwrap().points, 20);
+			assert_eq!(
+				RewardPools::<Runtime>::get(1).unwrap(),
+				RewardPool { ..Default::default() }
+			);
+			assert_eq!(
+				PoolMembers::<Runtime>::get(10).unwrap().last_recorded_reward_counter,
+				Zero::zero()
+			);
+			assert_eq!(
+				PoolMembers::<Runtime>::get(20).unwrap().last_recorded_reward_counter,
+				Zero::zero()
+			);
+
+			join(30);
+			assert_eq!(BondedPool::<Runtime>::get(1).unwrap().points, 30);
+			assert_eq!(
+				RewardPools::<Runtime>::get(1).unwrap(),
+				RewardPool { ..Default::default() }
+			);
+			assert_eq!(
+				PoolMembers::<Runtime>::get(10).unwrap().last_recorded_reward_counter,
+				Zero::zero()
+			);
+			assert_eq!(
+				PoolMembers::<Runtime>::get(20).unwrap().last_recorded_reward_counter,
+				Zero::zero()
+			);
+			assert_eq!(
+				PoolMembers::<Runtime>::get(30).unwrap().last_recorded_reward_counter,
+				Zero::zero()
+			);
+		});
+	}
+
+	#[derive(Decode)]
+	pub struct OldRewardPool<B> {
+		pub balance: B,
+		pub total_earnings: B,
+		pub points: U256,
+	}
+
+	#[derive(Decode)]
+	pub struct OldPoolMember<T: Config> {
+		pub pool_id: PoolId,
+		pub points: BalanceOf<T>,
+		pub reward_pool_total_earnings: BalanceOf<T>,
+		pub unbonding_eras: BoundedBTreeMap<EraIndex, BalanceOf<T>, T::MaxUnbonding>,
+	}
+
+	/// Migrate the pool reward scheme to the new version, as per
+	/// <https://github.com/paritytech/substrate/pull/11669.>.
+	pub struct MigrateToV2<T>(sp_std::marker::PhantomData<T>);
+	impl<T: Config> MigrateToV2<T> {
+		fn run(current: StorageVersion) -> Weight {
+			let mut reward_pools_translated = 0u64;
+			let mut members_translated = 0u64;
+			// just for logging.
+			let mut total_value_locked = BalanceOf::<T>::zero();
+			let mut total_points_locked = BalanceOf::<T>::zero();
+
+			// store each member of the pool, with their active points. In the process, migrate
+			// their data as well.
+			let mut temp_members = BTreeMap::<PoolId, Vec<(T::AccountId, BalanceOf<T>)>>::new();
+			PoolMembers::<T>::translate::<OldPoolMember<T>, _>(|key, old_member| {
+				let id = old_member.pool_id;
+				temp_members.entry(id).or_default().push((key, old_member.points));
+
+				total_points_locked += old_member.points;
+				members_translated += 1;
+				Some(PoolMember::<T> {
+					last_recorded_reward_counter: Zero::zero(),
+					pool_id: old_member.pool_id,
+					points: old_member.points,
+					unbonding_eras: old_member.unbonding_eras,
+				})
+			});
+
+			// translate all reward pools. In the process, do the last payout as well.
+			RewardPools::<T>::translate::<OldRewardPool<BalanceOf<T>>, _>(
+				|id, _old_reward_pool| {
+					// each pool should have at least one member.
+					let members = match temp_members.get(&id) {
+						Some(x) => x,
+						None => {
+							log!(error, "pool {} has no member! deleting it..", id);
+							return None
+						},
+					};
+					let bonded_pool = match BondedPools::<T>::get(id) {
+						Some(x) => x,
+						None => {
+							log!(error, "pool {} has no bonded pool! deleting it..", id);
+							return None
+						},
+					};
+
+					let accumulated_reward = RewardPool::<T>::current_balance(id);
+					let reward_account = Pallet::<T>::create_reward_account(id);
+					let mut sum_paid_out = BalanceOf::<T>::zero();
+
+					members
+						.into_iter()
+						.filter_map(|(who, points)| {
+							let bonded_pool = match BondedPool::<T>::get(id) {
+								Some(x) => x,
+								None => {
+									log!(error, "pool {} for member {:?} does not exist!", id, who);
+									return None
+								},
+							};
+
+							total_value_locked += bonded_pool.points_to_balance(points.clone());
+							let portion = Perbill::from_rational(*points, bonded_pool.points);
+							let last_claim = portion * accumulated_reward;
+
+							log!(
+								debug,
+								"{:?} has {:?} ({:?}) of pool {} with total reward of {:?}",
+								who,
+								portion,
+								last_claim,
+								id,
+								accumulated_reward
+							);
+
+							if last_claim.is_zero() {
+								None
+							} else {
+								Some((who, last_claim))
+							}
+						})
+						.for_each(|(who, last_claim)| {
+							let outcome = T::Currency::transfer(
+								&reward_account,
+								&who,
+								last_claim,
+								ExistenceRequirement::KeepAlive,
+							);
+
+							if let Err(reason) = outcome {
+								log!(warn, "last reward claim failed due to {:?}", reason,);
+							} else {
+								sum_paid_out = sum_paid_out.saturating_add(last_claim);
+							}
+
+							Pallet::<T>::deposit_event(Event::<T>::PaidOut {
+								member: who.clone(),
+								pool_id: id,
+								payout: last_claim,
+							});
+						});
+
+					// this can only be because of rounding down, or because the person we
+					// wanted to pay their reward to could not accept it (dust).
+					let leftover = accumulated_reward.saturating_sub(sum_paid_out);
+					if !leftover.is_zero() {
+						// pay it all to depositor.
+						let o = T::Currency::transfer(
+							&reward_account,
+							&bonded_pool.roles.depositor,
+							leftover,
+							ExistenceRequirement::KeepAlive,
+						);
+						log!(warn, "paying {:?} leftover to the depositor: {:?}", leftover, o);
+					}
+
+					// finally, migrate the reward pool.
+					reward_pools_translated += 1;
+
+					Some(RewardPool {
+						last_recorded_reward_counter: Zero::zero(),
+						last_recorded_total_payouts: Zero::zero(),
+						total_rewards_claimed: Zero::zero(),
+					})
+				},
+			);
+
+			log!(
+				info,
+				"Upgraded {} members, {} reward pools, TVL {:?} TPL {:?}, storage to version {:?}",
+				members_translated,
+				reward_pools_translated,
+				total_value_locked,
+				total_points_locked,
+				current
+			);
+			current.put::<Pallet<T>>();
+			T::DbWeight::get().reads_writes(members_translated + 1, reward_pools_translated + 1)
+		}
+	}
+
+	impl<T: Config> OnRuntimeUpgrade for MigrateToV2<T> {
+		fn on_runtime_upgrade() -> Weight {
+			let current = Pallet::<T>::current_storage_version();
+			let onchain = Pallet::<T>::on_chain_storage_version();
+
+			log!(
+				info,
+				"Running migration with current storage version {:?} / onchain {:?}",
+				current,
+				onchain
+			);
+
+			if current == 2 && onchain == 1 {
+				Self::run(current)
+			} else {
+				log!(info, "MigrateToV2 did not executed. This probably should be removed");
+				T::DbWeight::get().reads(1)
+			}
+		}
+
+		#[cfg(feature = "try-runtime")]
+		fn pre_upgrade() -> Result<(), &'static str> {
+			// all reward accounts must have more than ED.
+			RewardPools::<T>::iter().for_each(|(id, _)| {
+				assert!(
+					T::Currency::free_balance(&Pallet::<T>::create_reward_account(id)) >=
+						T::Currency::minimum_balance()
+				)
+			});
+
+			Ok(())
+		}
+
+		#[cfg(feature = "try-runtime")]
+		fn post_upgrade() -> Result<(), &'static str> {
+			// new version must be set.
+			assert_eq!(Pallet::<T>::on_chain_storage_version(), 2);
+
+			// no reward or bonded pool has been skipped.
+			assert_eq!(RewardPools::<T>::iter().count() as u32, RewardPools::<T>::count());
+			assert_eq!(BondedPools::<T>::iter().count() as u32, BondedPools::<T>::count());
+
+			// all reward pools must have exactly ED in them. This means no reward can be claimed,
+			// and that setting reward counters all over the board to zero will work henceforth.
+			RewardPools::<T>::iter().for_each(|(id, _)| {
+				assert_eq!(
+					RewardPool::<T>::current_balance(id),
+					Zero::zero(),
+					"reward pool({}) balance is {:?}",
+					id,
+					RewardPool::<T>::current_balance(id)
+				);
+			});
+
+			log!(info, "post upgrade hook for MigrateToV2 executed.");
+			Ok(())
+		}
+	}
+}