Revamp nomination pool reward scheme (#11669)

* make pool roles optional * undo lock file changes? * add migration * add the ability for pools to chill themselves * boilerplate of tests * somewhat stable, but I think I found another bug as well * Fix it all * Add more more sophisticated test + capture one more bug. * Update frame/staking/src/lib.rs * reduce the diff a little bit * add some test for the slashing bug * cleanup * fix lock file? * Fix * fmt * Update frame/nomination-pools/src/lib.rs Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Update frame/nomination-pools/src/lib.rs Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Update frame/nomination-pools/src/lib.rs Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Update frame/nomination-pools/src/mock.rs Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Fix build * fix some fishy tests.. * add one last integrity check for MinCreateBond * remove bad assertion -- needs to be dealt with later * nits * fix tests and add benchmarks for chill * remove stuff * fix benchmarks * cargo run --quiet --profile=production --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark pallet --chain=dev --steps=50 --repeat=20 --pallet=pallet_nomination_pools --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/nomination-pools/src/weights.rs --template=./.maintain/frame-weight-template.hbs * remove defensive * first working version * bring back all tests * ALL new tests work now * cleanup * make sure benchmarks and all work * cargo run --quiet --profile=production --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark pallet --chain=dev --steps=50 --repeat=20 --pallet=pallet_nomination_pools --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/nomination-pools/src/weights.rs --template=./.maintain/frame-weight-template.hbs * round of self-review, make arithmetic safe * fix warn * add migration code * Fix doc * add precision notes * make arithmetic fallible * fix node runtime * a lot of precision tests and notes and stuff * document MaxPOintsToBalance better * :round of self-review * fmt * fix some comments * Fix proportional slashing logic * Update frame/nomination-pools/src/tests.rs Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Update frame/nomination-pools/src/tests.rs Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Update frame/nomination-pools/src/lib.rs Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * track poinst in migration * fix * fmt * fix migration * remove event read * Apply suggestions from code review * Update frame/staking/src/lib.rs Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> * Update frame/nomination-pools/src/lib.rs Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> * Update frame/nomination-pools/src/lib.rs Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> * update * fmt * fmt * add one last test * fmt Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> Co-authored-by: Parity Bot <admin@parity.io>
2026-06-13 19:51:05 +00:00 · 2022-07-13 13:49:20 +01:00
parent 5d96c0a0ea
commit f8d4b99917
10 changed files with 1991 additions and 963 deletions
@@ -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(())
+		}
+	}
+}