From 5131069d0b514028e3d18fa0119291135ff4a029 Mon Sep 17 00:00:00 2001 From: Ankan Date: Sun, 31 Mar 2024 17:15:15 +0200 Subject: [PATCH] some comments --- substrate/frame/nomination-pools/src/lib.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 9ef052409d171..cca74b4bfa014 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -547,6 +547,9 @@ impl PoolMember { /// Total balance of the member, both active and unbonding. /// Doesn't mutate state. + /// + /// Worst case, iterates over [`TotalUnbondingPools`] member unbonding pools to calculate member + /// balance. fn total_balance(&self) -> BalanceOf { let maybe_pool = BondedPool::::get(self.pool_id); if maybe_pool.is_none() { @@ -2211,9 +2214,13 @@ pub mod pallet { /// /// # Note /// - /// If the target is the depositor, the pool will be destroyed. + /// - If the target is the depositor, the pool will be destroyed. + /// - If the pool has any pending slash, we also try to slash the member before letting them + /// withdraw. This calculation can be expensive and is only defensive. In ideal scenario, + /// pool slashes must have been already applied via permissionless [`Call::apply_slash`]. #[pallet::call_index(5)] #[pallet::weight( + // fixme(ank4n): fix weight taking into account potential slashing. T::WeightInfo::withdraw_unbonded_kill(*num_slashing_spans) )] pub fn withdraw_unbonded( @@ -2808,7 +2815,7 @@ pub mod pallet { /// Apply a pending slash on a member. #[pallet::call_index(23)] - // FIXME(ank4n): fix weight info. + // FIXME(ank4n): fix weight. Depends on unbonding pool count for member_account. #[pallet::weight(T::WeightInfo::set_commission_claim_permission())] pub fn apply_slash( origin: OriginFor, @@ -2915,7 +2922,7 @@ impl Pallet { ); // NOTE: Defensively force set balance to zero. T::Currency::set_balance(&reward_account, Zero::zero()); - // fixme(ank4n): Can't really do this with delegated staking? + // With `DelegateStake` strategy, this won't do anything. T::Currency::set_balance(&bonded_pool.bonded_account(), Zero::zero()); Self::deposit_event(Event::::Destroyed { pool_id: bonded_pool.id });