From 48d01339059ac60dd3b5dfffbc230f7caa7bc504 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Tue, 28 Jan 2025 19:46:20 +0000 Subject: [PATCH 01/14] Fix some typos in comments --- substrate/frame/nomination-pools/src/lib.rs | 27 ++++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index dc82bf3a37c6..669ac6f1bf54 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -18,7 +18,7 @@ //! # Nomination Pools for Staking Delegation //! //! A pallet that allows members to delegate their stake to nominating pools. A nomination pool acts -//! as nominator and nominates validators on the members behalf. +//! as nominator and nominates validators on the members' behalf. //! //! # Index //! @@ -178,7 +178,7 @@ //! //! ### Pool Members //! -//! * In general, whenever a pool member changes their total point, the chain will automatically +//! * In general, whenever a pool member changes their total points, the chain will automatically //! claim all their pending rewards for them. This is not optional, and MUST happen for the reward //! calculation to remain correct (see the documentation of `bond` as an example). So, make sure //! you are warning your users about it. They might be surprised if they see that they bonded an @@ -2509,12 +2509,12 @@ pub mod pallet { /// The dispatch origin of this call must be signed by the pool nominator or the pool /// root role. /// - /// This directly forward the call to the staking pallet, on behalf of the pool bonded + /// This directly forwards the call to the staking pallet, on behalf of the pool bonded /// account. /// /// # Note /// - /// In addition to a `root` or `nominator` role of `origin`, pool's depositor needs to have + /// In addition to a `root` or `nominator` role of `origin`, the pool's depositor needs to have /// at least `depositor_min_bond` in the pool to start nominating. #[pallet::call_index(8)] #[pallet::weight(T::WeightInfo::nominate(validators.len() as u32))] @@ -2628,6 +2628,8 @@ pub mod pallet { max_members_per_pool: ConfigOp, global_max_commission: ConfigOp, ) -> DispatchResult { + // No event? + T::AdminOrigin::ensure_origin(origin)?; macro_rules! config_op_exp { @@ -2718,8 +2720,8 @@ pub mod pallet { /// are unable to unbond. /// /// # Conditions for permissioned dispatch: - /// * The caller has a nominator or root role of the pool. - /// This directly forward the call to the staking pallet, on behalf of the pool bonded + /// * 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())] @@ -2913,9 +2915,16 @@ pub mod pallet { /// Claim pending commission. /// - /// The dispatch origin of this call must be signed by the `root` role of the pool. Pending - /// commission is paid out and added to total claimed commission`. Total pending commission - /// is reset to zero. the current. + /// The `root` role of the pool is _always_ allowed to claim the pool's commission. + /// + /// If the pool has set `CommissionClaimPermission::Permissionless`, then any account can + /// 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. + /// + /// Pending commission is paid out and added to total claimed commission`. Total pending commission + /// is reset to zero. #[pallet::call_index(20)] #[pallet::weight(T::WeightInfo::claim_commission())] pub fn claim_commission(origin: OriginFor, pool_id: PoolId) -> DispatchResult { From 4438fcd3faa8f5483529b5a3467b9ca52418a351 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Wed, 29 Jan 2025 00:57:49 +0000 Subject: [PATCH 02/14] Add missing events to `pallet-nomination-pools` --- substrate/frame/nomination-pools/src/lib.rs | 45 ++++++++++++++++++--- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 669ac6f1bf54..8b6e6080c2b2 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -1865,6 +1865,24 @@ pub mod pallet { MinBalanceDeficitAdjusted { pool_id: PoolId, amount: BalanceOf }, /// Claimed excess frozen ED of af the reward pool. MinBalanceExcessAdjusted { pool_id: PoolId, amount: BalanceOf }, + /// A pool member's claim permission has been updated. + MemberClaimPermissionUpdated { + member: T::AccountId, + permission: ClaimPermission, + }, + /// A pool's metadata was updated. + MetadataUpdated { pool_id: PoolId, caller: T::AccountId }, + /// A pool's nominator was chilled. + PoolNominatorChilled { pool_id: PoolId, caller: T::AccountId }, + /// Global parameters regulating nomination pools have been updated. + GlobalNomPoolParamsUpdated { + min_join_bond: BalanceOf, + min_create_bond: BalanceOf, + max_pools: Option, + max_members_per_pool: Option, + max_members: Option, + global_max_commission: Option, + } } #[pallet::error] @@ -2603,6 +2621,8 @@ pub mod pallet { Metadata::::mutate(pool_id, |pool_meta| *pool_meta = metadata); + Self::deposit_event(Event::::MetadataUpdated { pool_id, caller: who }); + Ok(()) } @@ -2628,8 +2648,6 @@ pub mod pallet { max_members_per_pool: ConfigOp, global_max_commission: ConfigOp, ) -> DispatchResult { - // No event? - T::AdminOrigin::ensure_origin(origin)?; macro_rules! config_op_exp { @@ -2648,6 +2666,16 @@ pub mod pallet { config_op_exp!(MaxPoolMembers::, max_members); config_op_exp!(MaxPoolMembersPerPool::, max_members_per_pool); config_op_exp!(GlobalMaxCommission::, global_max_commission); + + Self::deposit_event(Event::::GlobalNomPoolParamsUpdated { + min_join_bond: MinJoinBond::::get(), + min_create_bond: MinCreateBond::::get(), + max_pools: MaxPools::::get(), + max_members: MaxPoolMembers::::get(), + max_members_per_pool: MaxPoolMembersPerPool::::get(), + global_max_commission: GlobalMaxCommission::::get(), + }); + Ok(()) } @@ -2716,7 +2744,7 @@ pub mod pallet { /// account). /// /// # Conditions for a permissionless dispatch: - /// * When pool depositor has less than `MinNominatorBond` staked, otherwise pool members + /// * When pool depositor has less than `MinNominatorBond` staked, otherwise pool members /// are unable to unbond. /// /// # Conditions for permissioned dispatch: @@ -2741,7 +2769,12 @@ pub mod pallet { ensure!(bonded_pool.can_nominate(&who), Error::::NotNominator); } - T::StakeAdapter::chill(Pool::from(bonded_pool.bonded_account())) + let chill_res = T::StakeAdapter::chill(Pool::from(bonded_pool.bonded_account())); + if let Ok(()) = chill_res { + Self::deposit_event(Event::::PoolNominatorChilled { pool_id, caller: who }); + } + + chill_res } /// `origin` bonds funds from `extra` for some pool member `member` into their respective @@ -2796,10 +2829,12 @@ pub mod pallet { Error::::NotMigrated ); - ClaimPermissions::::mutate(who, |source| { + ClaimPermissions::::mutate(who.clone(), |source| { *source = permission; }); + Self::deposit_event(Event::::MemberClaimPermissionUpdated { member: who, permission }); + Ok(()) } From 0783b20edebc4ff3925e19010c0d308ab5796090 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Wed, 29 Jan 2025 00:58:33 +0000 Subject: [PATCH 03/14] Run `fmt` --- substrate/frame/nomination-pools/src/lib.rs | 25 +++++++++++---------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 8b6e6080c2b2..5cd171e84731 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -1866,10 +1866,7 @@ pub mod pallet { /// Claimed excess frozen ED of af the reward pool. MinBalanceExcessAdjusted { pool_id: PoolId, amount: BalanceOf }, /// A pool member's claim permission has been updated. - MemberClaimPermissionUpdated { - member: T::AccountId, - permission: ClaimPermission, - }, + MemberClaimPermissionUpdated { member: T::AccountId, permission: ClaimPermission }, /// A pool's metadata was updated. MetadataUpdated { pool_id: PoolId, caller: T::AccountId }, /// A pool's nominator was chilled. @@ -1882,7 +1879,7 @@ pub mod pallet { max_members_per_pool: Option, max_members: Option, global_max_commission: Option, - } + }, } #[pallet::error] @@ -2532,8 +2529,8 @@ pub mod pallet { /// /// # Note /// - /// In addition to a `root` or `nominator` role of `origin`, the pool's depositor needs to have - /// at least `depositor_min_bond` in the pool to start nominating. + /// In addition to a `root` or `nominator` role of `origin`, the pool's depositor needs to + /// have at least `depositor_min_bond` in the pool to start nominating. #[pallet::call_index(8)] #[pallet::weight(T::WeightInfo::nominate(validators.len() as u32))] pub fn nominate( @@ -2833,7 +2830,10 @@ pub mod pallet { *source = permission; }); - Self::deposit_event(Event::::MemberClaimPermissionUpdated { member: who, permission }); + Self::deposit_event(Event::::MemberClaimPermissionUpdated { + member: who, + permission, + }); Ok(()) } @@ -2955,11 +2955,12 @@ pub mod pallet { /// If the pool has set `CommissionClaimPermission::Permissionless`, then any account can /// 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. + /// 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. /// - /// Pending commission is paid out and added to total claimed commission`. Total pending commission - /// is reset to zero. + /// Pending commission is paid out and added to total claimed commission`. Total pending + /// commission is reset to zero. #[pallet::call_index(20)] #[pallet::weight(T::WeightInfo::claim_commission())] pub fn claim_commission(origin: OriginFor, pool_id: PoolId) -> DispatchResult { From c4c7b5e37652b3c469d11f817f0b6afd0b28b95c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Wed, 29 Jan 2025 15:23:08 +0000 Subject: [PATCH 04/14] Fix tests for nomination pools pallet --- substrate/frame/nomination-pools/src/lib.rs | 2 +- substrate/frame/nomination-pools/src/tests.rs | 145 ++++++++++++++++-- 2 files changed, 137 insertions(+), 10 deletions(-) diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 5cd171e84731..986d7634a680 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -1876,8 +1876,8 @@ pub mod pallet { min_join_bond: BalanceOf, min_create_bond: BalanceOf, max_pools: Option, - max_members_per_pool: Option, max_members: Option, + max_members_per_pool: Option, global_max_commission: Option, }, } diff --git a/substrate/frame/nomination-pools/src/tests.rs b/substrate/frame/nomination-pools/src/tests.rs index c46638d2f8f7..7fa5f26bcd07 100644 --- a/substrate/frame/nomination-pools/src/tests.rs +++ b/substrate/frame/nomination-pools/src/tests.rs @@ -17,7 +17,7 @@ use super::*; use crate::{mock::*, Event}; -use frame_support::{assert_err, assert_noop, assert_ok, assert_storage_noop}; +use frame_support::{assert_err, assert_noop, assert_ok}; use pallet_balances::Event as BEvent; use sp_runtime::{ bounded_btree_map, @@ -661,6 +661,7 @@ mod join { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 11, pool_id: 1, bonded: 2, joined: true }, ] ); @@ -817,6 +818,7 @@ mod join { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 101, pool_id: 1, bonded: 100, joined: true }, Event::Bonded { member: 102, pool_id: 1, bonded: 100, joined: true } ] @@ -1089,6 +1091,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 11, pool_id: 1, bonded: 11, joined: true }, Event::Unbonded { member: 11, pool_id: 1, points: 11, balance: 11, era: 3 } ] @@ -1121,6 +1124,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::PoolCommissionUpdated { pool_id: 1, current: Some((Perbill::from_percent(75), 2)) @@ -1184,6 +1188,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::PaidOut { member: 10, pool_id: 1, payout: 5 } ] ); @@ -1259,6 +1264,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 40, pool_id: 1, bonded: 40, joined: true }, Event::Bonded { member: 50, pool_id: 1, bonded: 50, joined: true } ] @@ -1514,6 +1520,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 10, joined: true }, Event::PaidOut { member: 10, pool_id: 1, payout: 20 }, Event::PaidOut { member: 20, pool_id: 1, payout: 10 }, @@ -1557,6 +1564,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 10, joined: true }, Event::PaidOut { member: 10, pool_id: 1, payout: 3 + 3 }, Event::PaidOut { member: 20, pool_id: 1, payout: 3 }, @@ -1620,6 +1628,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 10, joined: true }, Event::Bonded { member: 30, pool_id: 1, bonded: 10, joined: true }, Event::PaidOut { member: 10, pool_id: 1, payout: 30 + 100 / 2 + 60 / 3 }, @@ -1721,6 +1730,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 20, joined: true }, Event::Bonded { member: 30, pool_id: 1, bonded: 10, joined: true }, Event::PaidOut { member: 10, pool_id: 1, payout: 10 }, @@ -1770,6 +1780,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 20, joined: true }, Event::PaidOut { member: 10, pool_id: 1, payout: 10 }, Event::PaidOut { member: 20, pool_id: 1, payout: 20 } @@ -1818,6 +1829,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 20, joined: true }, Event::Bonded { member: 30, pool_id: 1, bonded: 10, joined: true }, Event::PaidOut { member: 10, pool_id: 1, payout: 10 }, @@ -1884,6 +1896,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 20, joined: true }, Event::PaidOut { member: 10, pool_id: 1, payout: 10 } ] @@ -1983,6 +1996,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Created { depositor: 20, pool_id: 2 }, Event::Bonded { member: 20, pool_id: 2, bonded: 10, joined: true }, Event::Created { depositor: 30, pool_id: 3 }, @@ -2052,6 +2066,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 10, joined: true }, Event::Bonded { member: 30, pool_id: 1, bonded: 10, joined: true }, Event::Bonded { member: 40, pool_id: 1, bonded: 10, joined: true } @@ -2132,6 +2147,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 20, joined: true }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: false }, Event::PaidOut { member: 10, pool_id: 1, payout: 15 }, @@ -2277,6 +2293,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 20, joined: true }, Event::Bonded { member: 30, pool_id: 1, bonded: 20, joined: true }, Event::Unbonded { member: 20, pool_id: 1, balance: 10, points: 10, era: 3 }, @@ -2315,6 +2332,7 @@ mod claim_payout { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 20, joined: true }, Event::PaidOut { member: 10, pool_id: 1, payout: 13 }, Event::PaidOut { member: 20, pool_id: 1, payout: 26 } @@ -2385,6 +2403,7 @@ mod claim_payout { bonded: 1000000000000000, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, @@ -2584,6 +2603,7 @@ mod unbond { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 20, joined: true }, Event::PaidOut { member: 20, pool_id: 1, payout: 6 }, Event::Unbonded { member: 20, pool_id: 1, balance: 20, points: 20, era: 3 } @@ -2833,6 +2853,7 @@ mod unbond { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 40, pool_id: 1, bonded: 40, joined: true }, Event::Bonded { member: 550, pool_id: 1, bonded: 550, joined: true }, Event::PoolSlashed { pool_id: 1, balance: 100 }, @@ -2975,6 +2996,7 @@ mod unbond { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Unbonded { member: 10, pool_id: 1, points: 10, balance: 10, era: 9 } ] ); @@ -3009,6 +3031,7 @@ mod unbond { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 100, pool_id: 1, bonded: 100, joined: true }, Event::Bonded { member: 200, pool_id: 1, bonded: 200, joined: true }, Event::Unbonded { @@ -3102,6 +3125,7 @@ mod unbond { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 100, pool_id: 1, bonded: 100, joined: true }, Event::Unbonded { member: 100, pool_id: 1, points: 100, balance: 100, era: 3 } ] @@ -3258,6 +3282,7 @@ mod unbond { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Unbonded { member: 10, pool_id: 1, points: 1, balance: 1, era: 3 } ] ); @@ -3390,6 +3415,7 @@ mod unbond { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 20, joined: true }, Event::Unbonded { member: 20, pool_id: 1, points: 2, balance: 2, era: 3 }, Event::Unbonded { member: 20, pool_id: 1, points: 3, balance: 3, era: 4 }, @@ -3426,6 +3452,7 @@ mod unbond { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Unbonded { member: 10, pool_id: 1, points: 3, balance: 3, era: 3 } ] ); @@ -3467,6 +3494,7 @@ mod unbond { // 2/3 of ed, which is 20's share. Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 20, joined: true }, Event::PaidOut { member: 20, pool_id: 1, payout: 10 }, Event::Unbonded { member: 20, pool_id: 1, balance: 2, points: 2, era: 3 } @@ -3641,6 +3669,7 @@ mod withdraw_unbonded { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 40, pool_id: 1, bonded: 40, joined: true }, Event::Bonded { member: 550, pool_id: 1, bonded: 550, joined: true }, Event::Unbonded { @@ -3750,6 +3779,7 @@ mod withdraw_unbonded { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 40, pool_id: 1, bonded: 40, joined: true }, Event::Bonded { member: 550, pool_id: 1, bonded: 550, joined: true }, Event::PoolSlashed { pool_id: 1, balance: 300 }, @@ -3827,6 +3857,7 @@ mod withdraw_unbonded { pool_events_since_last_call(), vec![ Event::Unbonded { member: 10, pool_id: 1, points: 5, balance: 5, era: 6 }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Withdrawn { member: 10, pool_id: 1, points: 5, balance: 5 }, Event::MemberRemoved { pool_id: 1, member: 10, released_balance: 0 }, Event::Destroyed { pool_id: 1 } @@ -3915,6 +3946,7 @@ mod withdraw_unbonded { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 100, pool_id: 1, bonded: 100, joined: true }, Event::Bonded { member: 200, pool_id: 1, bonded: 200, joined: true }, Event::Unbonded { @@ -4008,6 +4040,7 @@ mod withdraw_unbonded { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 100, pool_id: 1, bonded: 100, joined: true }, Event::Unbonded { member: 100, pool_id: 1, points: 100, balance: 100, era: 3 }, Event::Withdrawn { member: 100, pool_id: 1, points: 100, balance: 100 }, @@ -4048,6 +4081,7 @@ mod withdraw_unbonded { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: false }, Event::Unbonded { member: 10, pool_id: 1, points: 6, balance: 6, era: 3 }, Event::Unbonded { member: 10, pool_id: 1, points: 1, balance: 1, era: 4 } @@ -4135,6 +4169,7 @@ mod withdraw_unbonded { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 11, pool_id: 1, bonded: 10, joined: true }, Event::Unbonded { member: 11, pool_id: 1, points: 6, balance: 6, era: 3 }, Event::Unbonded { member: 11, pool_id: 1, points: 1, balance: 1, era: 4 } @@ -4231,6 +4266,7 @@ mod withdraw_unbonded { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 100, pool_id: 1, bonded: 100, joined: true }, Event::Unbonded { member: 100, pool_id: 1, points: 75, balance: 75, era: 3 }, Event::Unbonded { member: 100, pool_id: 1, points: 25, balance: 25, era: 4 }, @@ -4469,6 +4505,7 @@ mod withdraw_unbonded { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: false }, Event::Unbonded { member: 10, pool_id: 1, balance: 7, points: 7, era: 3 }, Event::Unbonded { member: 10, pool_id: 1, balance: 3, points: 3, era: 4 }, @@ -4515,6 +4552,7 @@ mod withdraw_unbonded { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 20, joined: true }, Event::Unbonded { member: 20, pool_id: 1, balance: 20, points: 20, era: 4 }, ] @@ -4555,6 +4593,7 @@ mod withdraw_unbonded { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Unbonded { member: 10, pool_id: 1, balance: 10, points: 10, era: 4 }, ] ); @@ -4597,6 +4636,7 @@ mod withdraw_unbonded { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Unbonded { member: 10, pool_id: 1, balance: 10, points: 10, era: 4 }, ] ); @@ -4699,6 +4739,7 @@ mod create { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Created { depositor: 11, pool_id: 2 }, Event::Bonded { member: 11, pool_id: 2, bonded: 10, joined: true } ] @@ -4829,6 +4870,7 @@ fn set_claim_permission_works() { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 11, pool_id: 1, bonded: 2, joined: true }, ] ); @@ -4844,6 +4886,14 @@ fn set_claim_permission_works() { ClaimPermission::Permissioned )); + assert_eq!( + pool_events_since_last_call(), + vec![Event::MemberClaimPermissionUpdated { + member: 11, + permission: ClaimPermission::Permissioned + },] + ); + // then assert_eq!(ClaimPermissions::::get(11), ClaimPermission::Permissioned); }); @@ -4923,6 +4973,7 @@ mod set_state { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::StateChanged { pool_id: 1, new_state: PoolState::Blocked } ] ); @@ -4999,10 +5050,20 @@ mod set_metadata { assert_ok!(Pools::set_metadata(RuntimeOrigin::signed(900), 1, vec![1, 1])); assert_eq!(Metadata::::get(1), vec![1, 1]); + System::assert_last_event(tests::RuntimeEvent::Pools(Event::MetadataUpdated { + pool_id: 1, + caller: 900, + })); + // bouncer can set metadata assert_ok!(Pools::set_metadata(RuntimeOrigin::signed(902), 1, vec![2, 2])); assert_eq!(Metadata::::get(1), vec![2, 2]); + System::assert_last_event(tests::RuntimeEvent::Pools(Event::MetadataUpdated { + pool_id: 1, + caller: 902, + })); + // Depositor can't set metadata assert_noop!( Pools::set_metadata(RuntimeOrigin::signed(10), 1, vec![3, 3]), @@ -5061,8 +5122,20 @@ mod set_configs { assert_eq!(MaxPoolMembersPerPool::::get(), Some(5)); assert_eq!(GlobalMaxCommission::::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)), + }, + )); + // Noop does nothing - assert_storage_noop!(assert_ok!(Pools::set_configs( + assert_ok!(Pools::set_configs( RuntimeOrigin::signed(42), ConfigOp::Noop, ConfigOp::Noop, @@ -5070,7 +5143,25 @@ mod set_configs { ConfigOp::Noop, ConfigOp::Noop, ConfigOp::Noop, - ))); + )); + + assert_eq!(MinJoinBond::::get(), 1); + assert_eq!(MinCreateBond::::get(), 2); + assert_eq!(MaxPools::::get(), Some(3)); + assert_eq!(MaxPoolMembers::::get(), Some(4)); + assert_eq!(MaxPoolMembersPerPool::::get(), Some(5)); + assert_eq!(GlobalMaxCommission::::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)), + }, + )); // Removing works assert_ok!(Pools::set_configs( @@ -5088,6 +5179,17 @@ mod set_configs { assert_eq!(MaxPoolMembers::::get(), None); assert_eq!(MaxPoolMembersPerPool::::get(), None); assert_eq!(GlobalMaxCommission::::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, + }, + )); }); } } @@ -5120,6 +5222,7 @@ mod bond_extra { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: false } ] ); @@ -5168,6 +5271,7 @@ mod bond_extra { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::PaidOut { member: 10, pool_id: 1, payout: claimable_reward }, Event::Bonded { member: 10, @@ -5229,6 +5333,7 @@ mod bond_extra { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 20, pool_id: 1, bonded: 20, joined: true }, Event::PaidOut { member: 10, pool_id: 1, payout: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 1, joined: false }, @@ -5372,6 +5477,7 @@ mod update_roles { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::RolesUpdated { root: Some(5), bouncer: Some(7), nominator: Some(6) } ] ); @@ -5485,7 +5591,8 @@ mod reward_counter_precision { pool_id: 1, bonded: 1173908528796953165005, joined: true, - } + }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, ] ); @@ -5518,7 +5625,8 @@ mod reward_counter_precision { pool_events_since_last_call(), vec![ Event::Created { depositor: 10, pool_id: 1 }, - Event::Bonded { member: 10, pool_id: 1, bonded: 10000000000000, joined: true } + Event::Bonded { member: 10, pool_id: 1, bonded: 10000000000000, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, ] ); @@ -5557,7 +5665,8 @@ mod reward_counter_precision { pool_id: 1, bonded: 12_968_712_300_500_000_000, joined: true, - } + }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, ] ); @@ -5623,7 +5732,8 @@ mod reward_counter_precision { pool_id: 1, bonded: 12_968_712_300_500_000_000, joined: true, - } + }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, ] ); @@ -5658,7 +5768,8 @@ mod reward_counter_precision { pool_id: 1, bonded: 2500000000000000000, joined: true, - } + }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, ] ); @@ -5726,7 +5837,8 @@ mod reward_counter_precision { pool_id: 1, bonded: 2500000000000000000, joined: true, - } + }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, ] ); @@ -5794,6 +5906,7 @@ mod commission { vec![ Event::Created { depositor: 10, pool_id }, Event::Bonded { member: 10, pool_id, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::PoolCommissionUpdated { pool_id, current: Some((Perbill::from_percent(50), root)) @@ -6087,6 +6200,7 @@ mod commission { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::PoolCommissionUpdated { pool_id: 1, current: Some((Perbill::from_percent(10), 900)) @@ -6109,6 +6223,7 @@ mod commission { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, ] ); @@ -6396,6 +6511,7 @@ mod commission { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::PoolMaxCommissionUpdated { pool_id: 1, max_commission: Perbill::from_percent(80) @@ -6463,6 +6579,7 @@ mod commission { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::PoolCommissionChangeRateUpdated { pool_id: 1, change_rate: CommissionChangeRate { @@ -6623,6 +6740,7 @@ mod commission { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::PoolCommissionUpdated { pool_id: 1, current: Some((Perbill::from_percent(10), 900)) @@ -6759,6 +6877,7 @@ mod commission { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::PoolCommissionChangeRateUpdated { pool_id: 1, change_rate: CommissionChangeRate { @@ -6792,6 +6911,7 @@ mod commission { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::PoolCommissionUpdated { pool_id: 1, current: Some((Perbill::from_percent(33), 2)) @@ -6882,6 +7002,7 @@ mod commission { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::PoolCommissionUpdated { pool_id: 1, current: Some((Perbill::from_percent(10), 2)) @@ -6948,6 +7069,7 @@ mod commission { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::PoolCommissionUpdated { pool_id: 1, current: Some((Perbill::from_percent(10), 2)) @@ -7007,6 +7129,7 @@ mod commission { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::PoolCommissionUpdated { pool_id: 1, current: Some((Perbill::from_percent(100), 2)) @@ -7052,6 +7175,7 @@ mod commission { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, ] ); @@ -7112,6 +7236,7 @@ mod commission { vec![ Event::Created { depositor: 10, pool_id }, Event::Bonded { member: 10, pool_id, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::PoolCommissionUpdated { pool_id, current: Some((Perbill::from_percent(50), 900)) @@ -7294,6 +7419,7 @@ mod commission { vec![ Event::Created { depositor: 10, pool_id }, Event::Bonded { member: 10, pool_id, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, ] ); @@ -7349,6 +7475,7 @@ mod slash { vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, + Event::MetadataUpdated { pool_id: 1, caller: 900 }, Event::Bonded { member: 11, pool_id: 1, bonded: 2, joined: true }, ] ); From a9b0b8ba724b6127027676345c1e56f45f2f90a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Wed, 29 Jan 2025 16:05:11 +0000 Subject: [PATCH 05/14] Test that `chill` emits event --- substrate/frame/nomination-pools/src/tests.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/substrate/frame/nomination-pools/src/tests.rs b/substrate/frame/nomination-pools/src/tests.rs index 7fa5f26bcd07..c572f83952b8 100644 --- a/substrate/frame/nomination-pools/src/tests.rs +++ b/substrate/frame/nomination-pools/src/tests.rs @@ -7533,10 +7533,20 @@ mod chill { assert_ok!(Pools::chill(RuntimeOrigin::signed(900), 1)); assert_ok!(Pools::nominate(RuntimeOrigin::signed(900), 1, vec![31])); + // Check that chill now emits an event + System::assert_last_event(tests::RuntimeEvent::Pools(Event::PoolNominatorChilled { + pool_id: 1, + caller: 900, + })); + // nominator can chill and re-nominate assert_ok!(Pools::chill(RuntimeOrigin::signed(901), 1)); assert_ok!(Pools::nominate(RuntimeOrigin::signed(901), 1, vec![31])); + System::assert_last_event(tests::RuntimeEvent::Pools(Event::PoolNominatorChilled { + pool_id: 1, + caller: 901, + })); // if `depositor` stake is less than the `MinimumNominatorBond`, then this call // becomes permissionless; StakingMinBond::set(20); From 051a8b373542e6f471857c6aabd915fcb77f3785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Wed, 29 Jan 2025 16:11:32 +0000 Subject: [PATCH 06/14] Add PRDoc --- prdoc/pr_7377.prdoc | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 prdoc/pr_7377.prdoc diff --git a/prdoc/pr_7377.prdoc b/prdoc/pr_7377.prdoc new file mode 100644 index 000000000000..67305417c57a --- /dev/null +++ b/prdoc/pr_7377.prdoc @@ -0,0 +1,13 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +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. + +crates: +- name: pallet-nomination-pools + bump: patch From bb1144601d6c03854b069e379853fbefa670b010 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Thu, 30 Jan 2025 19:16:01 +0000 Subject: [PATCH 07/14] Add event for act of pool nomination --- substrate/frame/nomination-pools/src/lib.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 986d7634a680..a98581f2aa0b 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -1869,6 +1869,9 @@ 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. + PoolNominationMade { pool_id: PoolId, caller: T::AccountId }, /// A pool's nominator was chilled. PoolNominatorChilled { pool_id: PoolId, caller: T::AccountId }, /// Global parameters regulating nomination pools have been updated. @@ -2553,7 +2556,12 @@ pub mod pallet { Error::::MinimumBondNotMet ); - T::StakeAdapter::nominate(Pool::from(bonded_pool.bonded_account()), validators) + let nominate_res = T::StakeAdapter::nominate(Pool::from(bonded_pool.bonded_account()), validators); + if let Ok(_) = nominate_res { + Self::deposit_event(Event::::PoolNominationMade { pool_id, caller: who }) + } + + return nominate_res } /// Set a new state for the pool. From 61576efa34614bdd0dcc5361797a28b832415f7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Thu, 30 Jan 2025 19:34:44 +0000 Subject: [PATCH 08/14] Test nomination event --- substrate/frame/nomination-pools/src/tests.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/substrate/frame/nomination-pools/src/tests.rs b/substrate/frame/nomination-pools/src/tests.rs index c572f83952b8..df81160c51ae 100644 --- a/substrate/frame/nomination-pools/src/tests.rs +++ b/substrate/frame/nomination-pools/src/tests.rs @@ -4933,10 +4933,21 @@ mod nominate { assert_ok!(Pools::nominate(RuntimeOrigin::signed(900), 1, vec![21])); assert_eq!(Nominations::get().unwrap(), vec![21]); + // Check event + System::assert_last_event(tests::RuntimeEvent::Pools(Event::PoolNominationMade { + pool_id: 1, + caller: 900, + })); + // Nominator can nominate assert_ok!(Pools::nominate(RuntimeOrigin::signed(901), 1, vec![31])); assert_eq!(Nominations::get().unwrap(), vec![31]); + System::assert_last_event(tests::RuntimeEvent::Pools(Event::PoolNominationMade { + pool_id: 1, + caller: 901, + })); + // Can't nominate for a pool that doesn't exist assert_noop!( Pools::nominate(RuntimeOrigin::signed(902), 123, vec![21]), From 3604b70b40026e8b42e1db136c7e07fb6f4f87dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Fri, 31 Jan 2025 19:57:44 +0000 Subject: [PATCH 09/14] Address PR review --- prdoc/pr_7377.prdoc | 7 ++- substrate/frame/nomination-pools/src/lib.rs | 48 ++++++++--------- substrate/frame/nomination-pools/src/tests.rs | 54 +++++++++---------- 3 files changed, 53 insertions(+), 56 deletions(-) diff --git a/prdoc/pr_7377.prdoc b/prdoc/pr_7377.prdoc index 67305417c57a..6e7c4bc017af 100644 --- a/prdoc/pr_7377.prdoc +++ b/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 diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index a98581f2aa0b..438d432d878c 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/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, min_create_bond: BalanceOf, max_pools: Option, @@ -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::::MinimumBondNotMet ); - let nominate_res = T::StakeAdapter::nominate(Pool::from(bonded_pool.bonded_account()), validators); - if let Ok(_) = nominate_res { - Self::deposit_event(Event::::PoolNominationMade { pool_id, caller: who }) - } - - return nominate_res + T::StakeAdapter::nominate(Pool::from(bonded_pool.bonded_account()), validators).map( + |_| Self::deposit_event(Event::::PoolNominationMade { pool_id, caller: who }), + ); } /// Set a new state for the pool. @@ -2672,7 +2669,7 @@ pub mod pallet { config_op_exp!(MaxPoolMembersPerPool::, max_members_per_pool); config_op_exp!(GlobalMaxCommission::, global_max_commission); - Self::deposit_event(Event::::GlobalNomPoolParamsUpdated { + Self::deposit_event(Event::::GlobalParamsUpdated { min_join_bond: MinJoinBond::::get(), min_create_bond: MinCreateBond::::get(), max_pools: MaxPools::::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, pool_id: PoolId) -> DispatchResult { @@ -2774,12 +2772,9 @@ pub mod pallet { ensure!(bonded_pool.can_nominate(&who), Error::::NotNominator); } - let chill_res = T::StakeAdapter::chill(Pool::from(bonded_pool.bonded_account())); - if let Ok(()) = chill_res { - Self::deposit_event(Event::::PoolNominatorChilled { pool_id, caller: who }); - } - - chill_res + T::StakeAdapter::chill(Pool::from(bonded_pool.bonded_account())).map(|_| { + Self::deposit_event(Event::::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, pool_id: PoolId) -> DispatchResult { diff --git a/substrate/frame/nomination-pools/src/tests.rs b/substrate/frame/nomination-pools/src/tests.rs index df81160c51ae..3426d08057e5 100644 --- a/substrate/frame/nomination-pools/src/tests.rs +++ b/substrate/frame/nomination-pools/src/tests.rs @@ -5134,16 +5134,14 @@ mod set_configs { assert_eq!(GlobalMaxCommission::::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::::get(), Some(5)); assert_eq!(GlobalMaxCommission::::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::::get(), None); assert_eq!(GlobalMaxCommission::::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, + })); }); } } From 68741463a3e6f2335b48ace6fb2c1a84b28647b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Sat, 1 Feb 2025 01:32:17 +0000 Subject: [PATCH 10/14] Fix typos --- prdoc/pr_7377.prdoc | 2 +- substrate/frame/staking/src/pallet/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/prdoc/pr_7377.prdoc b/prdoc/pr_7377.prdoc index 6e7c4bc017af..d31a8bff4b94 100644 --- a/prdoc/pr_7377.prdoc +++ b/prdoc/pr_7377.prdoc @@ -6,7 +6,7 @@ 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 previously had none: - `set_metadata` - `nominate` - `chill` diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 7d5da9ea0c49..9d8914627397 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -1552,7 +1552,7 @@ pub mod pallet { let _ = ledger .set_payee(payee) - .defensive_proof("ledger was retrieved from storage, thus its bonded; qed.")?; + .defensive_proof("ledger was retrieved from storage, thus it's bonded; qed.")?; Ok(()) } From fcd7393a3f7570cdc8949fb56965ddaa67210ffa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Sat, 1 Feb 2025 01:40:36 +0000 Subject: [PATCH 11/14] Fix `nominate/chill` return values and tests --- substrate/frame/nomination-pools/src/lib.rs | 4 ++-- substrate/frame/nomination-pools/src/tests.rs | 15 +++++++++++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 438d432d878c..3700c04316cc 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -2558,7 +2558,7 @@ pub mod pallet { T::StakeAdapter::nominate(Pool::from(bonded_pool.bonded_account()), validators).map( |_| Self::deposit_event(Event::::PoolNominationMade { pool_id, caller: who }), - ); + ) } /// Set a new state for the pool. @@ -2774,7 +2774,7 @@ pub mod pallet { T::StakeAdapter::chill(Pool::from(bonded_pool.bonded_account())).map(|_| { Self::deposit_event(Event::::PoolNominatorChilled { pool_id, caller: who }) - }); + }) } /// `origin` bonds funds from `extra` for some pool member `member` into their respective diff --git a/substrate/frame/nomination-pools/src/tests.rs b/substrate/frame/nomination-pools/src/tests.rs index 3426d08057e5..e2922e22fa98 100644 --- a/substrate/frame/nomination-pools/src/tests.rs +++ b/substrate/frame/nomination-pools/src/tests.rs @@ -7536,22 +7536,29 @@ mod chill { // root can chill and re-nominate assert_ok!(Pools::chill(RuntimeOrigin::signed(900), 1)); - assert_ok!(Pools::nominate(RuntimeOrigin::signed(900), 1, vec![31])); - // Check that chill now emits an event System::assert_last_event(tests::RuntimeEvent::Pools(Event::PoolNominatorChilled { pool_id: 1, caller: 900, })); + assert_ok!(Pools::nominate(RuntimeOrigin::signed(900), 1, vec![31])); + System::assert_last_event(tests::RuntimeEvent::Pools(Event::PoolNominationMade { + pool_id: 1, + caller: 900, + })); // nominator can chill and re-nominate assert_ok!(Pools::chill(RuntimeOrigin::signed(901), 1)); - assert_ok!(Pools::nominate(RuntimeOrigin::signed(901), 1, vec![31])); - System::assert_last_event(tests::RuntimeEvent::Pools(Event::PoolNominatorChilled { pool_id: 1, caller: 901, })); + assert_ok!(Pools::nominate(RuntimeOrigin::signed(901), 1, vec![31])); + System::assert_last_event(tests::RuntimeEvent::Pools(Event::PoolNominationMade { + pool_id: 1, + caller: 901, + })); + // if `depositor` stake is less than the `MinimumNominatorBond`, then this call // becomes permissionless; StakingMinBond::set(20); From d8ee6baae32d05560bb72ed280d7946be47239d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Mon, 3 Feb 2025 17:20:56 +0000 Subject: [PATCH 12/14] Fix `StakeAdapter` link in nominate/chill docs --- substrate/frame/nomination-pools/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 3700c04316cc..04736e6c1aad 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -2528,7 +2528,7 @@ pub mod pallet { /// root role. /// /// This directly forwards the call to an implementation of `StakingInterface` (e.g., - /// `pallet-staking`) through [`T::StakeAdapter`], on behalf of the bonded pool. + /// `pallet-staking`) through [`Config::StakeAdapter`], on behalf of the bonded pool. /// /// # Note /// @@ -2743,7 +2743,7 @@ pub mod pallet { /// 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. + /// `pallet-staking`) through [`Config::StakeAdapter`], on behalf of the bonded pool. /// /// Under certain conditions, this call can be dispatched permissionlessly (i.e. by any /// account). From f9536a23488730315b19433a554a2d60310d57ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Tue, 4 Feb 2025 13:53:49 +0000 Subject: [PATCH 13/14] Fix PRDoc SemVer --- prdoc/pr_7377.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_7377.prdoc b/prdoc/pr_7377.prdoc index d31a8bff4b94..e0799c1a116e 100644 --- a/prdoc/pr_7377.prdoc +++ b/prdoc/pr_7377.prdoc @@ -15,4 +15,4 @@ doc: crates: - name: pallet-nomination-pools - bump: patch + bump: major From c647a53ab539b51742a2db4fdf65dc14fbe8e7a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Bald=C3=A9?= Date: Tue, 4 Feb 2025 15:15:27 +0000 Subject: [PATCH 14/14] Fix tests in `test-delegate-stake` --- prdoc/pr_7377.prdoc | 2 ++ .../nomination-pools/test-delegate-stake/src/lib.rs | 9 ++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/prdoc/pr_7377.prdoc b/prdoc/pr_7377.prdoc index e0799c1a116e..1dfa88099a16 100644 --- a/prdoc/pr_7377.prdoc +++ b/prdoc/pr_7377.prdoc @@ -16,3 +16,5 @@ doc: crates: - name: pallet-nomination-pools bump: major +- name: pallet-staking + bump: none \ No newline at end of file diff --git a/substrate/frame/nomination-pools/test-delegate-stake/src/lib.rs b/substrate/frame/nomination-pools/test-delegate-stake/src/lib.rs index 54783332aa3e..b43a41cd0f98 100644 --- a/substrate/frame/nomination-pools/test-delegate-stake/src/lib.rs +++ b/substrate/frame/nomination-pools/test-delegate-stake/src/lib.rs @@ -62,6 +62,7 @@ fn pool_lifecycle_e2e() { vec![ PoolsEvent::Created { depositor: 10, pool_id: 1 }, PoolsEvent::Bonded { member: 10, pool_id: 1, bonded: 50, joined: true }, + PoolsEvent::PoolNominationMade { pool_id: 1, caller: 10 }, ] ); @@ -180,7 +181,10 @@ fn pool_lifecycle_e2e() { ); assert_eq!( pool_events_since_last_call(), - vec![PoolsEvent::Unbonded { member: 10, pool_id: 1, points: 50, balance: 50, era: 6 }] + vec![ + PoolsEvent::PoolNominatorChilled { pool_id: 1, caller: 10 }, + PoolsEvent::Unbonded { member: 10, pool_id: 1, points: 50, balance: 50, era: 6 } + ] ); // waiting another bonding duration: @@ -225,6 +229,7 @@ fn pool_chill_e2e() { vec![ PoolsEvent::Created { depositor: 10, pool_id: 1 }, PoolsEvent::Bonded { member: 10, pool_id: 1, bonded: 50, joined: true }, + PoolsEvent::PoolNominationMade { pool_id: 1, caller: 10 }, ] ); @@ -968,6 +973,7 @@ fn pool_migration_e2e() { vec![ PoolsEvent::Created { depositor: 10, pool_id: 1 }, PoolsEvent::Bonded { member: 10, pool_id: 1, bonded: 50, joined: true }, + PoolsEvent::PoolNominationMade { pool_id: 1, caller: 10 } ] ); @@ -1252,6 +1258,7 @@ fn disable_pool_operations_on_non_migrated() { vec![ PoolsEvent::Created { depositor: 10, pool_id: 1 }, PoolsEvent::Bonded { member: 10, pool_id: 1, bonded: 50, joined: true }, + PoolsEvent::PoolNominationMade { pool_id: 1, caller: 10 } ] );