diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 66c5f786f1e25..fbee831a7217d 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -749,6 +749,10 @@ impl Commission { None => None, Some((commission, payee)) => { ensure!(!self.throttling(commission), Error::::CommissionChangeThrottled); + ensure!( + commission <= &GlobalMaxCommission::::get().unwrap_or(Bounded::max_value()), + Error::::CommissionExceedsGlobalMaximum + ); ensure!( self.max.map_or(true, |m| commission <= &m), Error::::CommissionExceedsMaximum @@ -773,6 +777,10 @@ impl Commission { /// updated to the new maximum. This will also register a `throttle_from` update. /// A `PoolCommissionUpdated` event is triggered if `current.0` is updated. fn try_update_max(&mut self, pool_id: PoolId, new_max: Perbill) -> DispatchResult { + ensure!( + new_max <= GlobalMaxCommission::::get().unwrap_or(Bounded::max_value()), + Error::::CommissionExceedsGlobalMaximum + ); if let Some(old) = self.max.as_mut() { if new_max > *old { return Err(Error::::MaxCommissionRestricted.into()) @@ -1824,6 +1832,8 @@ pub mod pallet { MaxCommissionRestricted, /// The supplied commission exceeds the max allowed commission. CommissionExceedsMaximum, + /// The supplied commission exceeds global maximum commission. + CommissionExceedsGlobalMaximum, /// Not enough blocks have surpassed since the last commission update. CommissionChangeThrottled, /// The submitted changes to commission change rate are not allowed. diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index 49a0596587706..ac8fa5c4dbc0c 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -5915,6 +5915,16 @@ mod commission { Error::::DoesNotHavePermission ); + // Cannot set max commission above GlobalMaxCommission + assert_noop!( + Pools::set_commission_max( + RuntimeOrigin::signed(900), + 1, + Perbill::from_percent(100) + ), + Error::::CommissionExceedsGlobalMaximum + ); + // Set a max commission commission pool 1 to 80% assert_ok!(Pools::set_commission_max( RuntimeOrigin::signed(900), @@ -6591,7 +6601,7 @@ mod commission { } #[test] - fn global_max_prevents_100_percent_commission_payout() { + fn global_max_caps_max_commission_payout() { ExtBuilder::default().build_and_execute(|| { // Note: GlobalMaxCommission is set at 90%. @@ -6601,24 +6611,31 @@ mod commission { // top up the commission payee account to existential deposit let _ = Balances::deposit_creating(&2, 5); - // Set a commission pool 1 to 100%, with a payee set to `2` - assert_ok!(Pools::set_commission( - RuntimeOrigin::signed(900), - bonded_pool.id, - Some((Perbill::from_percent(100), 2)), - )); + // Set a commission pool 1 to 100% fails. + assert_noop!( + Pools::set_commission( + RuntimeOrigin::signed(900), + bonded_pool.id, + Some((Perbill::from_percent(100), 2)), + ), + Error::::CommissionExceedsGlobalMaximum + ); assert_eq!( pool_events_since_last_call(), vec![ Event::Created { depositor: 10, pool_id: 1 }, Event::Bonded { member: 10, pool_id: 1, bonded: 10, joined: true }, - Event::PoolCommissionUpdated { - pool_id: 1, - current: Some((Perbill::from_percent(100), 2)) - } ] ); + // Set pool commission to 90% and then set global max commission to 80%. + assert_ok!(Pools::set_commission( + RuntimeOrigin::signed(900), + bonded_pool.id, + Some((Perbill::from_percent(90), 2)), + )); + GlobalMaxCommission::::set(Some(Perbill::from_percent(80))); + // The pool earns 10 points deposit_rewards(10); @@ -6630,11 +6647,17 @@ mod commission { &mut reward_pool )); - // Confirm the commission was only 9 points out of 10 points, and the payout was 1 out - // of 10 points, reflecting the 90% global max commission. + // Confirm the commission was only 8 points out of 10 points, and the payout was 2 out + // of 10 points, reflecting the 80% global max commission. assert_eq!( pool_events_since_last_call(), - vec![Event::PaidOut { member: 10, pool_id: 1, payout: 1 },] + vec![ + Event::PoolCommissionUpdated { + pool_id: 1, + current: Some((Perbill::from_percent(90), 2)) + }, + Event::PaidOut { member: 10, pool_id: 1, payout: 2 }, + ] ); }) }