Skip to content

Commit

Permalink
Allow StakingAdmin to set min_commission (paritytech#13018)
Browse files Browse the repository at this point in the history
* staking admin can set min commission

* ".git/.scripts/bench-bot.sh" pallet dev pallet_staking

* fmt

* fix for pr comments

Co-authored-by: command-bot <>
  • Loading branch information
Ank4n authored and ltfschoen committed Feb 22, 2023
1 parent 65d7e69 commit 3b2c199
Show file tree
Hide file tree
Showing 14 changed files with 286 additions and 201 deletions.
2 changes: 1 addition & 1 deletion bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ impl pallet_staking::Config for Runtime {
type BondingDuration = BondingDuration;
type SlashDeferDuration = SlashDeferDuration;
/// A super-majority of the council can cancel the slash.
type SlashCancelOrigin = EitherOfDiverse<
type AdminOrigin = EitherOfDiverse<
EnsureRoot<AccountId>,
pallet_collective::EnsureProportionAtLeast<AccountId, CouncilCollective, 3, 4>,
>;
Expand Down
2 changes: 1 addition & 1 deletion frame/babe/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ impl pallet_staking::Config for Test {
type SessionsPerEra = SessionsPerEra;
type BondingDuration = BondingDuration;
type SlashDeferDuration = SlashDeferDuration;
type SlashCancelOrigin = frame_system::EnsureRoot<Self::AccountId>;
type AdminOrigin = frame_system::EnsureRoot<Self::AccountId>;
type SessionInterface = Self;
type UnixTime = pallet_timestamp::Pallet<Test>;
type EraPayout = pallet_staking::ConvertCurve<RewardCurve>;
Expand Down
2 changes: 1 addition & 1 deletion frame/fast-unstake/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl pallet_staking::Config for Runtime {
type Reward = ();
type SessionsPerEra = ();
type SlashDeferDuration = ();
type SlashCancelOrigin = frame_system::EnsureRoot<Self::AccountId>;
type AdminOrigin = frame_system::EnsureRoot<Self::AccountId>;
type BondingDuration = BondingDuration;
type SessionInterface = ();
type EraPayout = pallet_staking::ConvertCurve<RewardCurve>;
Expand Down
2 changes: 1 addition & 1 deletion frame/grandpa/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ impl pallet_staking::Config for Test {
type SessionsPerEra = SessionsPerEra;
type BondingDuration = BondingDuration;
type SlashDeferDuration = ();
type SlashCancelOrigin = frame_system::EnsureRoot<Self::AccountId>;
type AdminOrigin = frame_system::EnsureRoot<Self::AccountId>;
type SessionInterface = Self;
type UnixTime = pallet_timestamp::Pallet<Test>;
type EraPayout = pallet_staking::ConvertCurve<RewardCurve>;
Expand Down
2 changes: 1 addition & 1 deletion frame/nomination-pools/benchmarking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl pallet_staking::Config for Runtime {
type Reward = ();
type SessionsPerEra = ();
type SlashDeferDuration = ();
type SlashCancelOrigin = frame_system::EnsureRoot<Self::AccountId>;
type AdminOrigin = frame_system::EnsureRoot<Self::AccountId>;
type BondingDuration = ConstU32<3>;
type SessionInterface = ();
type EraPayout = pallet_staking::ConvertCurve<RewardCurve>;
Expand Down
2 changes: 1 addition & 1 deletion frame/nomination-pools/test-staking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ impl pallet_staking::Config for Runtime {
type Reward = ();
type SessionsPerEra = ();
type SlashDeferDuration = ();
type SlashCancelOrigin = frame_system::EnsureRoot<Self::AccountId>;
type AdminOrigin = frame_system::EnsureRoot<Self::AccountId>;
type BondingDuration = BondingDuration;
type SessionInterface = ();
type EraPayout = pallet_staking::ConvertCurve<RewardCurve>;
Expand Down
2 changes: 1 addition & 1 deletion frame/offences/benchmarking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ impl pallet_staking::Config for Test {
type Reward = ();
type SessionsPerEra = ();
type SlashDeferDuration = ();
type SlashCancelOrigin = frame_system::EnsureRoot<Self::AccountId>;
type AdminOrigin = frame_system::EnsureRoot<Self::AccountId>;
type BondingDuration = ();
type SessionInterface = Self;
type EraPayout = pallet_staking::ConvertCurve<RewardCurve>;
Expand Down
2 changes: 1 addition & 1 deletion frame/root-offences/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ impl pallet_staking::Config for Test {
type Reward = ();
type SessionsPerEra = SessionsPerEra;
type SlashDeferDuration = SlashDeferDuration;
type SlashCancelOrigin = frame_system::EnsureRoot<Self::AccountId>;
type AdminOrigin = frame_system::EnsureRoot<Self::AccountId>;
type BondingDuration = BondingDuration;
type SessionInterface = Self;
type EraPayout = pallet_staking::ConvertCurve<RewardCurve>;
Expand Down
2 changes: 1 addition & 1 deletion frame/session/benchmarking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ impl pallet_staking::Config for Test {
type Reward = ();
type SessionsPerEra = ();
type SlashDeferDuration = ();
type SlashCancelOrigin = frame_system::EnsureRoot<Self::AccountId>;
type AdminOrigin = frame_system::EnsureRoot<Self::AccountId>;
type BondingDuration = ();
type SessionInterface = Self;
type EraPayout = pallet_staking::ConvertCurve<RewardCurve>;
Expand Down
7 changes: 7 additions & 0 deletions frame/staking/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -922,6 +922,13 @@ benchmarks! {
);
}

set_min_commission {
let min_commission = Perbill::max_value();
}: _(RawOrigin::Root, min_commission)
verify {
assert_eq!(MinCommission::<T>::get(), Perbill::from_percent(100));
}

impl_benchmark_test_suite!(
Staking,
crate::mock::ExtBuilder::default().has_stakers(true),
Expand Down
14 changes: 10 additions & 4 deletions frame/staking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@
use crate::{self as pallet_staking, *};
use frame_election_provider_support::{onchain, SequentialPhragmen, VoteWeight};
use frame_support::{
assert_ok, parameter_types,
assert_ok, ord_parameter_types, parameter_types,
traits::{
ConstU32, ConstU64, Currency, FindAuthor, GenesisBuild, Get, Hooks, Imbalance,
OnUnbalanced, OneSessionHandler,
ConstU32, ConstU64, Currency, EitherOfDiverse, FindAuthor, GenesisBuild, Get, Hooks,
Imbalance, OnUnbalanced, OneSessionHandler,
},
weights::constants::RocksDbWeight,
};
use frame_system::{EnsureRoot, EnsureSignedBy};
use sp_core::H256;
use sp_io;
use sp_runtime::{
Expand Down Expand Up @@ -292,7 +293,7 @@ impl crate::pallet::pallet::Config for Test {
type Reward = MockReward;
type SessionsPerEra = SessionsPerEra;
type SlashDeferDuration = SlashDeferDuration;
type SlashCancelOrigin = frame_system::EnsureRoot<Self::AccountId>;
type AdminOrigin = EnsureOneOrRoot;
type BondingDuration = BondingDuration;
type SessionInterface = Self;
type EraPayout = ConvertCurve<RewardCurve>;
Expand Down Expand Up @@ -797,6 +798,11 @@ pub(crate) fn staking_events() -> Vec<crate::Event<Test>> {
parameter_types! {
static StakingEventsIndex: usize = 0;
}
ord_parameter_types! {
pub const One: u64 = 1;
}

type EnsureOneOrRoot = EitherOfDiverse<EnsureRoot<AccountId>, EnsureSignedBy<One, AccountId>>;

pub(crate) fn staking_events_since_last_call() -> Vec<crate::Event<Test>> {
let all: Vec<_> = System::events()
Expand Down
23 changes: 18 additions & 5 deletions frame/staking/src/pallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,10 @@ pub mod pallet {
#[pallet::constant]
type SlashDeferDuration: Get<EraIndex>;

/// The origin which can cancel a deferred slash. Root can always do this.
type SlashCancelOrigin: EnsureOrigin<Self::RuntimeOrigin>;
/// The origin which can manage less critical staking parameters that does not require root.
///
/// Supported actions: (1) cancel deferred slash, (2) set minimum commission.
type AdminOrigin: EnsureOrigin<Self::RuntimeOrigin>;

/// Interface for interacting with a session pallet.
type SessionInterface: SessionInterface<Self::AccountId>;
Expand Down Expand Up @@ -1452,7 +1454,7 @@ pub mod pallet {

/// Cancel enactment of a deferred slash.
///
/// Can be called by the `T::SlashCancelOrigin`.
/// Can be called by the `T::AdminOrigin`.
///
/// Parameters: era and indices of the slashes for that era to kill.
#[pallet::call_index(17)]
Expand All @@ -1462,7 +1464,7 @@ pub mod pallet {
era: EraIndex,
slash_indices: Vec<u32>,
) -> DispatchResult {
T::SlashCancelOrigin::ensure_origin(origin)?;
T::AdminOrigin::ensure_origin(origin)?;

ensure!(!slash_indices.is_empty(), Error::<T>::EmptyTargets);
ensure!(is_sorted_and_unique(&slash_indices), Error::<T>::NotSortedAndUnique);
Expand Down Expand Up @@ -1683,7 +1685,6 @@ pub mod pallet {
config_op_exp!(MinCommission<T>, min_commission);
Ok(())
}

/// Declare a `controller` to stop participating as either a validator or nominator.
///
/// Effects will be felt at the beginning of the next era.
Expand Down Expand Up @@ -1792,6 +1793,18 @@ pub mod pallet {
})?;
Ok(())
}

/// Sets the minimum amount of commission that each validators must maintain.
///
/// This call has lower privilege requirements than `set_staking_config` and can be called
/// by the `T::AdminOrigin`. Root can always call this.
#[pallet::call_index(25)]
#[pallet::weight(T::WeightInfo::set_min_commission())]
pub fn set_min_commission(origin: OriginFor<T>, new: Perbill) -> DispatchResult {
T::AdminOrigin::ensure_origin(origin)?;
MinCommission::<T>::put(new);
Ok(())
}
}
}

Expand Down
40 changes: 40 additions & 0 deletions frame/staking/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5726,6 +5726,46 @@ fn scale_validator_count_errors() {
})
}

#[test]
fn set_min_commission_works_with_admin_origin() {
ExtBuilder::default().build_and_execute(|| {
// no minimum commission set initially
assert_eq!(MinCommission::<Test>::get(), Zero::zero());

// root can set min commission
assert_ok!(Staking::set_min_commission(RuntimeOrigin::root(), Perbill::from_percent(10)));

assert_eq!(MinCommission::<Test>::get(), Perbill::from_percent(10));

// Non privileged origin can not set min_commission
assert_noop!(
Staking::set_min_commission(RuntimeOrigin::signed(2), Perbill::from_percent(15)),
BadOrigin
);

// Admin Origin can set min commission
assert_ok!(Staking::set_min_commission(
RuntimeOrigin::signed(1),
Perbill::from_percent(15),
));

// setting commission below min_commission fails
assert_noop!(
Staking::validate(
RuntimeOrigin::signed(10),
ValidatorPrefs { commission: Perbill::from_percent(14), blocked: false }
),
Error::<Test>::CommissionTooLow
);

// setting commission >= min_commission works
assert_ok!(Staking::validate(
RuntimeOrigin::signed(10),
ValidatorPrefs { commission: Perbill::from_percent(15), blocked: false }
));
})
}

mod staking_interface {
use frame_support::storage::with_storage_layer;
use sp_staking::StakingInterface;
Expand Down
Loading

0 comments on commit 3b2c199

Please sign in to comment.