Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing events to nomination pool extrinsincs #7377

Merged
merged 18 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions prdoc/pr_7377.prdoc
Original file line number Diff line number Diff line change
@@ -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
rockbmb marked this conversation as resolved.
Show resolved Hide resolved

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
81 changes: 67 additions & 14 deletions substrate/frame/nomination-pools/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
//!
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1865,6 +1865,24 @@ pub mod pallet {
MinBalanceDeficitAdjusted { pool_id: PoolId, amount: BalanceOf<T> },
/// Claimed excess frozen ED of af the reward pool.
MinBalanceExcessAdjusted { pool_id: PoolId, amount: BalanceOf<T> },
/// 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 },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is adding caller useful here? Nothing against it, just trying to understand the rationale. Same for other events.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of set_metadata, both the pool's root and its bouncer can make that call - this is to disambiguate which did it, in case the two roles are performed by different accounts.

The same goes for the other calls: nominate/chill can be called by the pool's root/nominator.

/// 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.
rockbmb marked this conversation as resolved.
Show resolved Hide resolved
PoolNominatorChilled { pool_id: PoolId, caller: T::AccountId },
/// Global parameters regulating nomination pools have been updated.
GlobalNomPoolParamsUpdated {
rockbmb marked this conversation as resolved.
Show resolved Hide resolved
min_join_bond: BalanceOf<T>,
min_create_bond: BalanceOf<T>,
max_pools: Option<u32>,
max_members: Option<u32>,
max_members_per_pool: Option<u32>,
global_max_commission: Option<Perbill>,
},
}

#[pallet::error]
Expand Down Expand Up @@ -2509,13 +2527,13 @@ 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
rockbmb marked this conversation as resolved.
Show resolved Hide resolved
/// account.
///
/// # Note
///
/// In addition to a `root` or `nominator` role of `origin`, 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(
Expand All @@ -2538,7 +2556,12 @@ pub mod pallet {
Error::<T>::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::<T>::PoolNominationMade { pool_id, caller: who })
}

return nominate_res
rockbmb marked this conversation as resolved.
Show resolved Hide resolved
}

/// Set a new state for the pool.
Expand Down Expand Up @@ -2603,6 +2626,8 @@ pub mod pallet {

Metadata::<T>::mutate(pool_id, |pool_meta| *pool_meta = metadata);

Self::deposit_event(Event::<T>::MetadataUpdated { pool_id, caller: who });

Ok(())
}

Expand Down Expand Up @@ -2646,6 +2671,16 @@ pub mod pallet {
config_op_exp!(MaxPoolMembers::<T>, max_members);
config_op_exp!(MaxPoolMembersPerPool::<T>, max_members_per_pool);
config_op_exp!(GlobalMaxCommission::<T>, global_max_commission);

Self::deposit_event(Event::<T>::GlobalNomPoolParamsUpdated {
min_join_bond: MinJoinBond::<T>::get(),
min_create_bond: MinCreateBond::<T>::get(),
max_pools: MaxPools::<T>::get(),
max_members: MaxPoolMembers::<T>::get(),
max_members_per_pool: MaxPoolMembersPerPool::<T>::get(),
global_max_commission: GlobalMaxCommission::<T>::get(),
});

Ok(())
}

Expand Down Expand Up @@ -2714,12 +2749,12 @@ 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:
/// * 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
rockbmb marked this conversation as resolved.
Show resolved Hide resolved
/// account.
#[pallet::call_index(13)]
#[pallet::weight(T::WeightInfo::chill())]
Expand All @@ -2739,7 +2774,12 @@ pub mod pallet {
ensure!(bonded_pool.can_nominate(&who), Error::<T>::NotNominator);
}

T::StakeAdapter::chill(Pool::from(bonded_pool.bonded_account()))
let chill_res = T::StakeAdapter::chill(Pool::from(bonded_pool.bonded_account()));
rockbmb marked this conversation as resolved.
Show resolved Hide resolved
if let Ok(()) = chill_res {
Self::deposit_event(Event::<T>::PoolNominatorChilled { pool_id, caller: who });
}

chill_res
}

/// `origin` bonds funds from `extra` for some pool member `member` into their respective
Expand Down Expand Up @@ -2794,10 +2834,15 @@ pub mod pallet {
Error::<T>::NotMigrated
);

ClaimPermissions::<T>::mutate(who, |source| {
ClaimPermissions::<T>::mutate(who.clone(), |source| {
*source = permission;
});

Self::deposit_event(Event::<T>::MemberClaimPermissionUpdated {
member: who,
permission,
});

Ok(())
}

Expand Down Expand Up @@ -2913,9 +2958,17 @@ 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
rockbmb marked this conversation as resolved.
Show resolved Hide resolved
/// pool.
///
/// Pending commission is paid out and added to total claimed commission`. Total pending
rockbmb marked this conversation as resolved.
Show resolved Hide resolved
/// 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 {
Expand Down
Loading
Loading