Skip to content

Commit

Permalink
Ranked collective Add+Remove origins (paritytech#3212)
Browse files Browse the repository at this point in the history
Superseeds paritytech#1245  

This PR is a migration of the
paritytech/substrate#14577.

The PR added associated types (`AddOrigin` & `RemoveOrigin`) to
`Config`. It allows you to decouple types and areas of responsibility,
since at the moment the same types are responsible for adding and
promoting(removing and demoting). This will improve the flexibility of
the pallet configuration.

```
/// The origin required to add a member.
type AddOrigin: EnsureOrigin<Self::RuntimeOrigin, Success = ()>;

/// The origin required to remove a member. The success value indicates the
/// maximum rank *from which* the removal may be.
type RemoveOrigin: EnsureOrigin<Self::RuntimeOrigin, Success = Rank>;
```
To achieve the backward compatibility, the users of the pallet can use
the old type via the new morph:

```
type AddOrigin = MapSuccess<Self::PromoteOrigin, Ignore>;
type RemoveOrigin = Self::DemoteOrigin;
```

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: PraetorP <praetorian281@gmail.com>
Co-authored-by: Pavel Orlov <45266194+PraetorP@users.noreply.github.com>
  • Loading branch information
3 people authored Feb 6, 2024
1 parent 5e0cfe8 commit 6ad2d32
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 32 deletions.
2 changes: 2 additions & 0 deletions substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,8 @@ impl pallet_referenda::Config<pallet_referenda::Instance2> for Runtime {
impl pallet_ranked_collective::Config for Runtime {
type WeightInfo = pallet_ranked_collective::weights::SubstrateWeight<Self>;
type RuntimeEvent = RuntimeEvent;
type AddOrigin = EnsureRoot<AccountId>;
type RemoveOrigin = Self::DemoteOrigin;
type PromoteOrigin = EnsureRootWithSuccess<AccountId, ConstU16<65535>>;
type DemoteOrigin = EnsureRootWithSuccess<AccountId, ConstU16<65535>>;
type ExchangeOrigin = EnsureRootWithSuccess<AccountId, ConstU16<65535>>;
Expand Down
4 changes: 3 additions & 1 deletion substrate/frame/core-fellowship/src/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use frame_system::EnsureSignedBy;
use pallet_ranked_collective::{EnsureRanked, Geometric, Rank, TallyOf, Votes};
use sp_core::Get;
use sp_runtime::{
traits::{Convert, ReduceBy, TryMorphInto},
traits::{Convert, ReduceBy, ReplaceWithDefault, TryMorphInto},
BuildStorage, DispatchError,
};
type Class = Rank;
Expand Down Expand Up @@ -137,12 +137,14 @@ impl pallet_ranked_collective::Config for Test {
// Members can promote up to the rank of 2 below them.
MapSuccess<EnsureRanked<Test, (), 2>, ReduceBy<ConstU16<2>>>,
>;
type AddOrigin = MapSuccess<Self::PromoteOrigin, ReplaceWithDefault<()>>;
type DemoteOrigin = EitherOf<
// Root can demote arbitrarily.
frame_system::EnsureRootWithSuccess<Self::AccountId, ConstU16<65535>>,
// Members can demote up to the rank of 3 below them.
MapSuccess<EnsureRanked<Test, (), 3>, ReduceBy<ConstU16<3>>>,
>;
type RemoveOrigin = Self::DemoteOrigin;
type ExchangeOrigin = EitherOf<
// Root can exchange arbitrarily.
frame_system::EnsureRootWithSuccess<Self::AccountId, ConstU16<65535>>,
Expand Down
26 changes: 7 additions & 19 deletions substrate/frame/ranked-collective/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ fn make_member<T: Config<I>, I: 'static>(rank: Rank) -> T::AccountId {
let who = account::<T::AccountId>("member", MemberCount::<T, I>::get(0), SEED);
let who_lookup = T::Lookup::unlookup(who.clone());
assert_ok!(Pallet::<T, I>::add_member(
T::PromoteOrigin::try_successful_origin()
.expect("PromoteOrigin has no successful origin required for the benchmark"),
T::AddOrigin::try_successful_origin()
.expect("AddOrigin has no successful origin required for the benchmark"),
who_lookup.clone(),
));
for _ in 0..rank {
Expand All @@ -60,7 +60,7 @@ benchmarks_instance_pallet! {
let who = account::<T::AccountId>("member", 0, SEED);
let who_lookup = T::Lookup::unlookup(who.clone());
let origin =
T::PromoteOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
T::AddOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
let call = Call::<T, I>::add_member { who: who_lookup };
}: { call.dispatch_bypass_filter(origin)? }
verify {
Expand All @@ -77,7 +77,7 @@ benchmarks_instance_pallet! {
let last = make_member::<T, I>(rank);
let last_index = (0..=rank).map(|r| IdToIndex::<T, I>::get(r, &last).unwrap()).collect::<Vec<_>>();
let origin =
T::DemoteOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
T::RemoveOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
let call = Call::<T, I>::remove_member { who: who_lookup, min_rank: rank };
}: { call.dispatch_bypass_filter(origin)? }
verify {
Expand Down Expand Up @@ -125,23 +125,11 @@ benchmarks_instance_pallet! {
}

vote {
let caller: T::AccountId = whitelisted_caller();
let caller_lookup = T::Lookup::unlookup(caller.clone());
assert_ok!(Pallet::<T, I>::add_member(
T::PromoteOrigin::try_successful_origin()
.expect("PromoteOrigin has no successful origin required for the benchmark"),
caller_lookup.clone(),
));
// Create a poll
let class = T::Polls::classes().into_iter().next().unwrap();
let rank = T::MinRankOfClass::convert(class.clone());
for _ in 0..rank {
assert_ok!(Pallet::<T, I>::promote_member(
T::PromoteOrigin::try_successful_origin()
.expect("PromoteOrigin has no successful origin required for the benchmark"),
caller_lookup.clone(),
));
}

let caller = make_member::<T, I>(rank);
let caller_lookup = T::Lookup::unlookup(caller.clone());

let poll = T::Polls::create_ongoing(class).expect("Must always be able to create a poll for rank 0");

Expand Down
27 changes: 17 additions & 10 deletions substrate/frame/ranked-collective/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,12 +393,20 @@ pub mod pallet {
type RuntimeEvent: From<Event<Self, I>>
+ IsType<<Self as frame_system::Config>::RuntimeEvent>;

/// The origin required to add or promote a mmember. The success value indicates the
/// The origin required to add a member.
type AddOrigin: EnsureOrigin<Self::RuntimeOrigin>;

/// The origin required to remove a member.
///
/// The success value indicates the maximum rank *from which* the removal may be.
type RemoveOrigin: EnsureOrigin<Self::RuntimeOrigin, Success = Rank>;

/// The origin required to promote a member. The success value indicates the
/// maximum rank *to which* the promotion may be.
type PromoteOrigin: EnsureOrigin<Self::RuntimeOrigin, Success = Rank>;

/// The origin required to demote or remove a member. The success value indicates the
/// maximum rank *from which* the demotion/removal may be.
/// The origin required to demote a member. The success value indicates the
/// maximum rank *from which* the demotion may be.
type DemoteOrigin: EnsureOrigin<Self::RuntimeOrigin, Success = Rank>;

/// The origin that can swap the account of a member.
Expand Down Expand Up @@ -510,22 +518,21 @@ pub mod pallet {
impl<T: Config<I>, I: 'static> Pallet<T, I> {
/// Introduce a new member.
///
/// - `origin`: Must be the `AdminOrigin`.
/// - `origin`: Must be the `AddOrigin`.
/// - `who`: Account of non-member which will become a member.
/// - `rank`: The rank to give the new member.
///
/// Weight: `O(1)`
#[pallet::call_index(0)]
#[pallet::weight(T::WeightInfo::add_member())]
pub fn add_member(origin: OriginFor<T>, who: AccountIdLookupOf<T>) -> DispatchResult {
let _ = T::PromoteOrigin::ensure_origin(origin)?;
T::AddOrigin::ensure_origin(origin)?;
let who = T::Lookup::lookup(who)?;
Self::do_add_member(who, true)
}

/// Increment the rank of an existing member by one.
///
/// - `origin`: Must be the `AdminOrigin`.
/// - `origin`: Must be the `PromoteOrigin`.
/// - `who`: Account of existing member.
///
/// Weight: `O(1)`
Expand All @@ -540,7 +547,7 @@ pub mod pallet {
/// Decrement the rank of an existing member by one. If the member is already at rank zero,
/// then they are removed entirely.
///
/// - `origin`: Must be the `AdminOrigin`.
/// - `origin`: Must be the `DemoteOrigin`.
/// - `who`: Account of existing member of rank greater than zero.
///
/// Weight: `O(1)`, less if the member's index is highest in its rank.
Expand All @@ -554,7 +561,7 @@ pub mod pallet {

/// Remove the member entirely.
///
/// - `origin`: Must be the `AdminOrigin`.
/// - `origin`: Must be the `RemoveOrigin`.
/// - `who`: Account of existing member of rank greater than zero.
/// - `min_rank`: The rank of the member or greater.
///
Expand All @@ -566,7 +573,7 @@ pub mod pallet {
who: AccountIdLookupOf<T>,
min_rank: Rank,
) -> DispatchResultWithPostInfo {
let max_rank = T::DemoteOrigin::ensure_origin(origin)?;
let max_rank = T::RemoveOrigin::ensure_origin(origin)?;
let who = T::Lookup::lookup(who)?;
let MemberRecord { rank, .. } = Self::ensure_member(&who)?;
ensure!(min_rank >= rank, Error::<T, I>::InvalidWitness);
Expand Down
7 changes: 6 additions & 1 deletion substrate/frame/ranked-collective/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ use frame_support::{
traits::{ConstU16, EitherOf, MapSuccess, Polling},
};
use sp_core::Get;
use sp_runtime::{traits::ReduceBy, BuildStorage};
use sp_runtime::{
traits::{ReduceBy, ReplaceWithDefault},
BuildStorage,
};

use super::*;
use crate as pallet_ranked_collective;
Expand Down Expand Up @@ -152,6 +155,8 @@ parameter_types! {
impl Config for Test {
type WeightInfo = ();
type RuntimeEvent = RuntimeEvent;
type AddOrigin = MapSuccess<Self::PromoteOrigin, ReplaceWithDefault<()>>;
type RemoveOrigin = Self::DemoteOrigin;
type PromoteOrigin = EitherOf<
// Root can promote arbitrarily.
frame_system::EnsureRootWithSuccess<Self::AccountId, ConstU16<65535>>,
Expand Down
4 changes: 3 additions & 1 deletion substrate/frame/salary/src/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use frame_support::{
use pallet_ranked_collective::{EnsureRanked, Geometric, TallyOf, Votes};
use sp_core::{ConstU16, Get};
use sp_runtime::{
traits::{Convert, ReduceBy},
traits::{Convert, ReduceBy, ReplaceWithDefault},
BuildStorage, DispatchError,
};

Expand Down Expand Up @@ -162,12 +162,14 @@ impl pallet_ranked_collective::Config for Test {
// Members can promote up to the rank of 2 below them.
MapSuccess<EnsureRanked<Test, (), 2>, ReduceBy<ConstU16<2>>>,
>;
type AddOrigin = MapSuccess<Self::PromoteOrigin, ReplaceWithDefault<()>>;
type DemoteOrigin = EitherOf<
// Root can demote arbitrarily.
frame_system::EnsureRootWithSuccess<Self::AccountId, ConstU16<65535>>,
// Members can demote up to the rank of 3 below them.
MapSuccess<EnsureRanked<Test, (), 3>, ReduceBy<ConstU16<3>>>,
>;
type RemoveOrigin = Self::DemoteOrigin;
type ExchangeOrigin = EitherOf<
// Root can exchange arbitrarily.
frame_system::EnsureRootWithSuccess<Self::AccountId, ConstU16<65535>>,
Expand Down
3 changes: 3 additions & 0 deletions substrate/primitives/runtime/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,9 @@ morph_types! {
/// Morpher to disregard the source value and replace with another.
pub type Replace<V: TypedGet> = |_| -> V::Type { V::get() };

/// Morpher to disregard the source value and replace with the default of `V`.
pub type ReplaceWithDefault<V: Default> = |_| -> V { Default::default() };

/// Mutator which reduces a scalar by a particular amount.
pub type ReduceBy<N: TypedGet> = |r: N::Type| -> N::Type {
r.checked_sub(&N::get()).unwrap_or(Zero::zero())
Expand Down

0 comments on commit 6ad2d32

Please sign in to comment.