Nomination Pool Commission (#13128)

* + nomination pool commission

* fmt

* use register_update()

* Update frame/nomination-pools/src/lib.rs

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

* Update frame/nomination-pools/src/lib.rs

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

* fmt

* amend comments

* + test for set_commission

* fix

* Update frame/nomination-pools/fuzzer/src/call.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* rm comment

* use PalletError

* some feedback item amendments

* update weights

* revert PalletError stuff

* ".git/.scripts/commands/fmt/fmt.sh"

* make pool_events_since_last_call more modular

* fmt

* fix call indexes + test

* add payout teste

* add event to max_commisson updating current

* begin refactor

* some debugging

* update

* more tests

* rewardpol not working

* commission refactor

* pending rewards returns commission

* fmt

* add claim_commission call

* + claim_commission

* fix benchmarks

* weight 0 for now

* + claim_commission benchmark

* fmt

* apply commission to benchmarks

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_nomination_pools

* ".git/.scripts/commands/fmt/fmt.sh"

* clippy

* + pending

* add RewardPool.total_rewards_acounted

* fixes

* println

* more logs

* Fix plus cleanups

* fix assert

* tidy up

* tests work + tidy up

* rm unused

* clippy fix

* persist reward_pool update

* claim_commission_works tests

* .

* some test formatting

* add high level docs

* add calls

* docs

* rename

* rename

* docs

* rename

* fmt

* use matches!

* Update frame/nomination-pools/src/lib.rs

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

* Update frame/nomination-pools/src/lib.rs

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

* Update frame/nomination-pools/src/tests.rs

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

* comment

* Update frame/nomination-pools/src/lib.rs

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>

* .

* weights order

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_nomination_pools

* use from_parts

* comment

* ".git/.scripts/commands/fmt/fmt.sh"

* revert clippy suggestions on old migrations

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_nomination_pools

* add InitialGlobalMaxCommission

* fix migration

* reward counter comments & explanations

* format

* add commission implementation note

* fmt

* revert InitialGlobalMaxCommission

* global max commission migration generic

* text

* 100% commission no payout test

* add commission_accumulates_on_multiple_rewards

* non-zero fuzzer GlobalMaxCommission

* add last_recorded_total_payouts_needs_commission

* commission event fix + claim commission test

---------

Co-authored-by: Gonçalo Pestana <g6pestana@gmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: command-bot <>
Co-authored-by: Bastian Köcher <info@kchr.de>

substrate/frame/nomination-pools/src/migration.rs

@@ -52,9 +52,12 @@ pub mod v1 {
 
 	impl<T: Config> OldBondedPoolInner<T> {
 		fn migrate_to_v1(self) -> BondedPoolInner<T> {
+			// Note: `commission` field not introduced to `BondedPoolInner` until
+			// migration 4.
 			BondedPoolInner {
-				member_counter: self.member_counter,
 				points: self.points,
+				commission: Commission::default(),
+				member_counter: self.member_counter,
 				state: self.state,
 				roles: self.roles.migrate_to_v1(),
 			}
@@ -307,6 +310,8 @@ pub mod v2 {
 						last_recorded_reward_counter: Zero::zero(),
 						last_recorded_total_payouts: Zero::zero(),
 						total_rewards_claimed: Zero::zero(),
+						total_commission_claimed: Zero::zero(),
+						total_commission_pending: Zero::zero(),
 					})
 				},
 			);
@@ -449,3 +454,97 @@ pub mod v3 {
 		}
 	}
 }
+
+pub mod v4 {
+	use super::*;
+
+	#[derive(Decode)]
+	pub struct OldBondedPoolInner<T: Config> {
+		pub points: BalanceOf<T>,
+		pub state: PoolState,
+		pub member_counter: u32,
+		pub roles: PoolRoles<T::AccountId>,
+	}
+
+	impl<T: Config> OldBondedPoolInner<T> {
+		fn migrate_to_v4(self) -> BondedPoolInner<T> {
+			BondedPoolInner {
+				commission: Commission::default(),
+				member_counter: self.member_counter,
+				points: self.points,
+				state: self.state,
+				roles: self.roles,
+			}
+		}
+	}
+
+	/// This migration adds a `commission` field to every `BondedPoolInner`, if
+	/// any.
+	pub struct MigrateToV4<T, U>(sp_std::marker::PhantomData<(T, U)>);
+	impl<T: Config, U: Get<Perbill>> OnRuntimeUpgrade for MigrateToV4<T, U> {
+		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 == 4 && onchain == 3 {
+				let initial_global_max_commission = U::get();
+				GlobalMaxCommission::<T>::set(Some(initial_global_max_commission));
+				log!(
+					info,
+					"Set initial global max commission to {:?}.",
+					initial_global_max_commission
+				);
+
+				let mut translated = 0u64;
+				BondedPools::<T>::translate::<OldBondedPoolInner<T>, _>(|_key, old_value| {
+					translated.saturating_inc();
+					Some(old_value.migrate_to_v4())
+				});
+
+				current.put::<Pallet<T>>();
+				log!(info, "Upgraded {} pools, storage to version {:?}", translated, current);
+
+				// reads: translated + onchain version.
+				// writes: translated + current.put + initial global commission.
+				T::DbWeight::get().reads_writes(translated + 1, translated + 2)
+			} else {
+				log!(info, "Migration did not execute. This probably should be removed");
+				T::DbWeight::get().reads(1)
+			}
+		}
+
+		#[cfg(feature = "try-runtime")]
+		fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
+			ensure!(
+				Pallet::<T>::current_storage_version() > Pallet::<T>::on_chain_storage_version(),
+				"the on_chain version is equal or more than the current one"
+			);
+			Ok(Vec::new())
+		}
+
+		#[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)| inner.commission.current.is_none() &&
+					inner.commission.max.is_none() &&
+					inner.commission.change_rate.is_none() &&
+					inner.commission.throttle_from.is_none()),
+				"a commission value has been incorrectly set"
+			);
+			ensure!(
+				GlobalMaxCommission::<T>::get() == Some(U::get()),
+				"global maximum commission error"
+			);
+			ensure!(Pallet::<T>::on_chain_storage_version() == 4, "wrong storage version");
+			Ok(())
+		}
+	}
+}

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

@@ -118,8 +118,8 @@ impl sp_staking::StakingInterface for StakingMock {
 
 	fn stake(who: &Self::AccountId) -> Result<Stake<Self>, DispatchError> {
 		match (
-			UnbondingBalanceMap::get().get(who).map(|v| *v),
-			BondedBalanceMap::get().get(who).map(|v| *v),
+			UnbondingBalanceMap::get().get(who).copied(),
+			BondedBalanceMap::get().get(who).copied(),
 		) {
 			(None, None) => Err(DispatchError::Other("balance not found")),
 			(Some(v), None) => Ok(Stake { total: v, active: 0, stash: *who }),
@@ -251,11 +251,17 @@ pub struct ExtBuilder {
 	members: Vec<(AccountId, Balance)>,
 	max_members: Option<u32>,
 	max_members_per_pool: Option<u32>,
+	global_max_commission: Option<Perbill>,
 }
 
 impl Default for ExtBuilder {
 	fn default() -> Self {
-		Self { members: Default::default(), max_members: Some(4), max_members_per_pool: Some(3) }
+		Self {
+			members: Default::default(),
+			max_members: Some(4),
+			max_members_per_pool: Some(3),
+			global_max_commission: Some(Perbill::from_percent(90)),
+		}
 	}
 }
 
@@ -297,6 +303,11 @@ impl ExtBuilder {
 		self
 	}
 
+	pub fn global_max_commission(mut self, commission: Option<Perbill>) -> Self {
+		self.global_max_commission = commission;
+		self
+	}
+
 	pub fn build(self) -> sp_io::TestExternalities {
 		sp_tracing::try_init_simple();
 		let mut storage =
@@ -308,6 +319,7 @@ impl ExtBuilder {
 			max_pools: Some(2),
 			max_members_per_pool: self.max_members_per_pool,
 			max_members: self.max_members,
+			global_max_commission: self.global_max_commission,
 		}
 		.assimilate_storage(&mut storage);
 
@@ -332,7 +344,7 @@ impl ExtBuilder {
 		ext
 	}
 
-	pub fn build_and_execute(self, test: impl FnOnce() -> ()) {
+	pub fn build_and_execute(self, test: impl FnOnce()) {
 		self.build().execute_with(|| {
 			test();
 			Pools::do_try_state(CheckLevel::get()).unwrap();
@@ -354,6 +366,23 @@ parameter_types! {
 	storage BalancesEvents: u32 = 0;
 }
 
+/// Helper to run a specified amount of blocks.
+pub fn run_blocks(n: u64) {
+	let current_block = System::block_number();
+	run_to_block(n + current_block);
+}
+
+/// Helper to run to a specific block.
+pub fn run_to_block(n: u64) {
+	let current_block = System::block_number();
+	assert!(n > current_block);
+	while System::block_number() < n {
+		Pools::on_finalize(System::block_number());
+		System::set_block_number(System::block_number() + 1);
+		Pools::on_initialize(System::block_number());
+	}
+}
+
 /// All events of this pallet.
 pub fn pool_events_since_last_call() -> Vec<super::Event<Runtime>> {
 	let events = System::events()
@@ -380,7 +409,7 @@ pub fn balances_events_since_last_call() -> Vec<pallet_balances::Event<Runtime>>
 
 /// Same as `fully_unbond`, in permissioned setting.
 pub fn fully_unbond_permissioned(member: AccountId) -> DispatchResult {
-	let points = PoolMembers::<Runtime>::get(&member)
+	let points = PoolMembers::<Runtime>::get(member)
 		.map(|d| d.active_points())
 		.unwrap_or_default();
 	Pools::unbond(RuntimeOrigin::signed(member), member, points)