Address PR review

prdoc/pr_7377.prdoc

@@ -6,7 +6,12 @@ title: Add missing events to nomination pool extrinsics
 doc:
   - audience: [Runtime Dev, Runtime User]
     description: |
-      Introduces events to extrinsics from `pallet_nomination_pools` that previous had none.
+      Introduces events to extrinsics from `pallet_nomination_pools` that previous had none:
+        - `set_metadata`
+        - `nominate`
+        - `chill`
+        - `set_configs`
+        - `set_claim_permission`
 
 crates:
 - name: pallet-nomination-pools

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

@@ -1869,13 +1869,13 @@ pub mod pallet {
 		MemberClaimPermissionUpdated { member: T::AccountId, permission: ClaimPermission },
 		/// A pool's metadata was updated.
 		MetadataUpdated { pool_id: PoolId, caller: T::AccountId },
-		/// A pool's nominating account (or the pool's root account) has nominated a validator set on behalf of the
-		/// pool.
+		/// A pool's nominating account (or the pool's root account) has nominated a validator set
+		/// on behalf of the pool.
 		PoolNominationMade { pool_id: PoolId, caller: T::AccountId },
-		/// A pool's nominator was chilled.
+		/// The pool is chilled i.e. no longer nominating.
 		PoolNominatorChilled { pool_id: PoolId, caller: T::AccountId },
 		/// Global parameters regulating nomination pools have been updated.
-		GlobalNomPoolParamsUpdated {
+		GlobalParamsUpdated {
 			min_join_bond: BalanceOf<T>,
 			min_create_bond: BalanceOf<T>,
 			max_pools: Option<u32>,
@@ -2527,8 +2527,8 @@ pub mod pallet {
 		/// The dispatch origin of this call must be signed by the pool nominator or the pool
 		/// root role.
 		///
-		/// This directly forwards the call to the staking pallet, on behalf of the pool bonded
-		/// account.
+		/// This directly forwards the call to an implementation of `StakingInterface` (e.g.,
+		/// `pallet-staking`) through [`T::StakeAdapter`], on behalf of the bonded pool.
 		///
 		/// # Note
 		///
@@ -2556,12 +2556,9 @@ pub mod pallet {
 				Error::<T>::MinimumBondNotMet
 			);
 
-			let nominate_res = T::StakeAdapter::nominate(Pool::from(bonded_pool.bonded_account()), validators);
-			if let Ok(_) = nominate_res {
-				Self::deposit_event(Event::<T>::PoolNominationMade { pool_id, caller: who })
-			}
-
-			return nominate_res
+			T::StakeAdapter::nominate(Pool::from(bonded_pool.bonded_account()), validators).map(
+				|_| Self::deposit_event(Event::<T>::PoolNominationMade { pool_id, caller: who }),
+			);
 		}
 
 		/// Set a new state for the pool.
@@ -2672,7 +2669,7 @@ pub mod pallet {
 			config_op_exp!(MaxPoolMembersPerPool::<T>, max_members_per_pool);
 			config_op_exp!(GlobalMaxCommission::<T>, global_max_commission);
 
-			Self::deposit_event(Event::<T>::GlobalNomPoolParamsUpdated {
+			Self::deposit_event(Event::<T>::GlobalParamsUpdated {
 				min_join_bond: MinJoinBond::<T>::get(),
 				min_create_bond: MinCreateBond::<T>::get(),
 				max_pools: MaxPools::<T>::get(),
@@ -2745,6 +2742,9 @@ pub mod pallet {
 		/// The dispatch origin of this call can be signed by the pool nominator or the pool
 		/// root role, same as [`Pallet::nominate`].
 		///
+		/// This directly forwards the call to an implementation of `StakingInterface` (e.g.,
+		/// `pallet-staking`) through [`T::StakeAdapter`], on behalf of the bonded pool.
+		///
 		/// Under certain conditions, this call can be dispatched permissionlessly (i.e. by any
 		/// account).
 		///
@@ -2754,8 +2754,6 @@ pub mod pallet {
 		///
 		/// # Conditions for permissioned dispatch:
 		/// * The caller is the pool's nominator or root.
-		/// This directly forwards the call to the staking pallet, on behalf of the pool's bonded
-		/// account.
 		#[pallet::call_index(13)]
 		#[pallet::weight(T::WeightInfo::chill())]
 		pub fn chill(origin: OriginFor<T>, pool_id: PoolId) -> DispatchResult {
@@ -2774,12 +2772,9 @@ pub mod pallet {
 				ensure!(bonded_pool.can_nominate(&who), Error::<T>::NotNominator);
 			}
 
-			let chill_res = T::StakeAdapter::chill(Pool::from(bonded_pool.bonded_account()));
-			if let Ok(()) = chill_res {
-				Self::deposit_event(Event::<T>::PoolNominatorChilled { pool_id, caller: who });
-			}
-
-			chill_res
+			T::StakeAdapter::chill(Pool::from(bonded_pool.bonded_account())).map(|_| {
+				Self::deposit_event(Event::<T>::PoolNominatorChilled { pool_id, caller: who })
+			});
 		}
 
 		/// `origin` bonds funds from `extra` for some pool member `member` into their respective
@@ -2964,11 +2959,14 @@ pub mod pallet {
 		/// trigger the process of claiming the pool's commission.
 		///
 		/// If the pool has set its `CommissionClaimPermission` to `Account(acc)`, then only
-		/// accounts `acc` and the pool's root account may call this extrinsic on behalf of the
-		/// pool.
+		/// accounts
+		/// * `acc`, and
+		/// * the pool's root account
+		///
+		/// may call this extrinsic on behalf of the pool.
 		///
-		/// Pending commission is paid out and added to total claimed commission`. Total pending
-		/// commission is reset to zero.
+		/// Pending commissions are paid out and added to the total claimed commission.
+		/// The total pending commission is reset to zero.
 		#[pallet::call_index(20)]
 		#[pallet::weight(T::WeightInfo::claim_commission())]
 		pub fn claim_commission(origin: OriginFor<T>, pool_id: PoolId) -> DispatchResult {

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

@@ -5134,16 +5134,14 @@ mod set_configs {
 			assert_eq!(GlobalMaxCommission::<Runtime>::get(), Some(Perbill::from_percent(6)));
 
 			// Check events
-			System::assert_last_event(tests::RuntimeEvent::Pools(
-				Event::GlobalNomPoolParamsUpdated {
-					min_join_bond: 1,
-					min_create_bond: 2,
-					max_pools: Some(3),
-					max_members: Some(4),
-					max_members_per_pool: Some(5),
-					global_max_commission: Some(Perbill::from_percent(6)),
-				},
-			));
+			System::assert_last_event(tests::RuntimeEvent::Pools(Event::GlobalParamsUpdated {
+				min_join_bond: 1,
+				min_create_bond: 2,
+				max_pools: Some(3),
+				max_members: Some(4),
+				max_members_per_pool: Some(5),
+				global_max_commission: Some(Perbill::from_percent(6)),
+			}));
 
 			// Noop does nothing
 			assert_ok!(Pools::set_configs(
@@ -5163,16 +5161,14 @@ mod set_configs {
 			assert_eq!(MaxPoolMembersPerPool::<Runtime>::get(), Some(5));
 			assert_eq!(GlobalMaxCommission::<Runtime>::get(), Some(Perbill::from_percent(6)));
 
-			System::assert_last_event(tests::RuntimeEvent::Pools(
-				Event::GlobalNomPoolParamsUpdated {
-					min_join_bond: 1,
-					min_create_bond: 2,
-					max_pools: Some(3),
-					max_members: Some(4),
-					max_members_per_pool: Some(5),
-					global_max_commission: Some(Perbill::from_percent(6)),
-				},
-			));
+			System::assert_last_event(tests::RuntimeEvent::Pools(Event::GlobalParamsUpdated {
+				min_join_bond: 1,
+				min_create_bond: 2,
+				max_pools: Some(3),
+				max_members: Some(4),
+				max_members_per_pool: Some(5),
+				global_max_commission: Some(Perbill::from_percent(6)),
+			}));
 
 			// Removing works
 			assert_ok!(Pools::set_configs(
@@ -5191,16 +5187,14 @@ mod set_configs {
 			assert_eq!(MaxPoolMembersPerPool::<Runtime>::get(), None);
 			assert_eq!(GlobalMaxCommission::<Runtime>::get(), None);
 
-			System::assert_last_event(tests::RuntimeEvent::Pools(
-				Event::GlobalNomPoolParamsUpdated {
-					min_join_bond: 0,
-					min_create_bond: 0,
-					max_pools: None,
-					max_members: None,
-					max_members_per_pool: None,
-					global_max_commission: None,
-				},
-			));
+			System::assert_last_event(tests::RuntimeEvent::Pools(Event::GlobalParamsUpdated {
+				min_join_bond: 0,
+				min_create_bond: 0,
+				max_pools: None,
+				max_members: None,
+				max_members_per_pool: None,
+				global_max_commission: None,
+			}));
 		});
 	}
 }