diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs index d9042f2a5568d402146196c7fb5a745f7213cb6b..0c9f428c1396bede97a67002d0554d98d62dbc39 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs @@ -40,7 +40,7 @@ use origins::pallet_origins::{ EnsureAmbassadorsVoice, EnsureAmbassadorsVoiceFrom, EnsureHeadAmbassadorsVoice, Origin, }; use sp_core::ConstU128; -use sp_runtime::traits::{CheckedReduceBy, ConstU16, ConvertToValue, Replace}; +use sp_runtime::traits::{CheckedReduceBy, ConstU16, ConvertToValue, Replace, ReplaceWithDefault}; use xcm::prelude::*; use xcm_builder::{AliasesIntoAccountId32, PayOverXcm}; @@ -108,8 +108,10 @@ pub type ExchangeOrigin = EitherOf<EnsureRootWithSuccess<AccountId, ConstU16<655 impl pallet_ranked_collective::Config<AmbassadorCollectiveInstance> for Runtime { type WeightInfo = weights::pallet_ranked_collective_ambassador_collective::WeightInfo<Runtime>; type RuntimeEvent = RuntimeEvent; + type AddOrigin = MapSuccess<Self::PromoteOrigin, ReplaceWithDefault<()>>; type PromoteOrigin = PromoteOrigin; type DemoteOrigin = DemoteOrigin; + type RemoveOrigin = Self::DemoteOrigin; type ExchangeOrigin = ExchangeOrigin; type Polls = AmbassadorReferenda; type MinRankOfClass = sp_runtime::traits::Identity; diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs index 6632d511f7c3cf42aab7e4e98085b6cddf15b717..3816d2ed848ed51740283ffea31e9f7e53c01f1a 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs @@ -113,12 +113,17 @@ impl pallet_ranked_collective::Config<FellowshipCollectiveInstance> for Runtime type WeightInfo = weights::pallet_ranked_collective_fellowship_collective::WeightInfo<Runtime>; type RuntimeEvent = RuntimeEvent; - #[cfg(not(feature = "runtime-benchmarks"))] // Promotions and the induction of new members are serviced by `FellowshipCore` pallet instance. - type PromoteOrigin = frame_system::EnsureNever<pallet_ranked_collective::Rank>; + #[cfg(not(feature = "runtime-benchmarks"))] + type AddOrigin = frame_system::EnsureNever<()>; #[cfg(feature = "runtime-benchmarks")] + type AddOrigin = frame_system::EnsureRoot<Self::AccountId>; + // The maximum value of `u16` set as a success value for the root to ensure the benchmarks will // pass. + #[cfg(not(feature = "runtime-benchmarks"))] + type PromoteOrigin = frame_system::EnsureNever<pallet_ranked_collective::Rank>; + #[cfg(feature = "runtime-benchmarks")] type PromoteOrigin = EnsureRootWithSuccess<Self::AccountId, ConstU16<65535>>; // Demotion is by any of: @@ -127,6 +132,7 @@ impl pallet_ranked_collective::Config<FellowshipCollectiveInstance> for Runtime // // The maximum value of `u16` set as a success value for the root to ensure the benchmarks will // pass. + type RemoveOrigin = Self::DemoteOrigin; type DemoteOrigin = EitherOf< EnsureRootWithSuccess<Self::AccountId, ConstU16<65535>>, MapSuccess< diff --git a/polkadot/runtime/rococo/src/governance/fellowship.rs b/polkadot/runtime/rococo/src/governance/fellowship.rs index 9ac47e008a2719c6f637b2c9a58b3a8ae75b4c65..a589b768afde2c0757e74a6535509228f1613a82 100644 --- a/polkadot/runtime/rococo/src/governance/fellowship.rs +++ b/polkadot/runtime/rococo/src/governance/fellowship.rs @@ -17,7 +17,7 @@ //! Elements of governance concerning the Rococo Fellowship. use frame_support::traits::{MapSuccess, TryMapSuccess}; -use sp_runtime::traits::{CheckedReduceBy, ConstU16, Replace}; +use sp_runtime::traits::{CheckedReduceBy, ConstU16, Replace, ReplaceWithDefault}; use super::*; use crate::{CENTS, DAYS}; @@ -315,6 +315,11 @@ pub type FellowshipCollectiveInstance = pallet_ranked_collective::Instance1; impl pallet_ranked_collective::Config<FellowshipCollectiveInstance> for Runtime { type WeightInfo = weights::pallet_ranked_collective::WeightInfo<Self>; type RuntimeEvent = RuntimeEvent; + // Adding is by any of: + // - Root. + // - the FellowshipAdmin origin. + // - a Fellowship origin. + type AddOrigin = MapSuccess<Self::PromoteOrigin, ReplaceWithDefault<()>>; // Promotion is by any of: // - Root can demote arbitrarily. // - the FellowshipAdmin origin (i.e. token holder referendum); @@ -326,6 +331,11 @@ impl pallet_ranked_collective::Config<FellowshipCollectiveInstance> for Runtime TryMapSuccess<origins::EnsureFellowship, CheckedReduceBy<ConstU16<1>>>, >, >; + // Removing is by any of: + // - Root can remove arbitrarily. + // - the FellowshipAdmin origin (i.e. token holder referendum); + // - a vote by the rank two above the current rank. + type RemoveOrigin = Self::DemoteOrigin; // Demotion is by any of: // - Root can demote arbitrarily. // - the FellowshipAdmin origin (i.e. token holder referendum); diff --git a/prdoc/pr_3212.prdoc b/prdoc/pr_3212.prdoc new file mode 100644 index 0000000000000000000000000000000000000000..a9de1e28cdfe2294948fb3c954b9b417c09d149b --- /dev/null +++ b/prdoc/pr_3212.prdoc @@ -0,0 +1,9 @@ +title: "Ranked collective introduce `Add` and `Remove` origins" + +doc: + - audience: Runtime Dev + description: | + Add two new origins to the ranked-collective pallet. One to add new members and one to remove members, named `AddOrigin` and `RemoveOrigin` respectively. + +crates: + - name: pallet-ranked-collective diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 02d084c6c62b524671276bfac8fffb03fb1744fb..515272cd99ab46e0ae64ecd62c26e053df13cde3 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -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>>; diff --git a/substrate/frame/core-fellowship/src/tests/integration.rs b/substrate/frame/core-fellowship/src/tests/integration.rs index 57f9cad3dcb36335824577f2a850fbbb53013337..6f177ba66db37fa791b06eb3f9b7c10998af0b98 100644 --- a/substrate/frame/core-fellowship/src/tests/integration.rs +++ b/substrate/frame/core-fellowship/src/tests/integration.rs @@ -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; @@ -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>>, diff --git a/substrate/frame/ranked-collective/src/benchmarking.rs b/substrate/frame/ranked-collective/src/benchmarking.rs index 332d8292e53f1c8bd936a3fd19e9bd59f52d884b..f093fd93e6e2b7b991de3f4b41e8c9b2004d47f6 100644 --- a/substrate/frame/ranked-collective/src/benchmarking.rs +++ b/substrate/frame/ranked-collective/src/benchmarking.rs @@ -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 { @@ -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 { @@ -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 { @@ -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"); diff --git a/substrate/frame/ranked-collective/src/lib.rs b/substrate/frame/ranked-collective/src/lib.rs index d0303fee0b3670adcb723ba652ec8b8b50919afa..65ea886acc4fa099af9496a015412a98b6150e2f 100644 --- a/substrate/frame/ranked-collective/src/lib.rs +++ b/substrate/frame/ranked-collective/src/lib.rs @@ -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. @@ -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)` @@ -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. @@ -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. /// @@ -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); diff --git a/substrate/frame/ranked-collective/src/tests.rs b/substrate/frame/ranked-collective/src/tests.rs index 58e21ded358147fe7b7546be2df9cced94fde089..c5fccd3f7724550fcc9ae12eabef6bf492c5abe4 100644 --- a/substrate/frame/ranked-collective/src/tests.rs +++ b/substrate/frame/ranked-collective/src/tests.rs @@ -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; @@ -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>>, diff --git a/substrate/frame/salary/src/tests/integration.rs b/substrate/frame/salary/src/tests/integration.rs index bf1e82b028c6da58e5705fae62b30b95a40e215c..a49b5637b8ae42b6618fe98b12ceba77f35fe89b 100644 --- a/substrate/frame/salary/src/tests/integration.rs +++ b/substrate/frame/salary/src/tests/integration.rs @@ -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, }; @@ -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>>, diff --git a/substrate/primitives/runtime/src/traits.rs b/substrate/primitives/runtime/src/traits.rs index 4213117334ee7860f0600af2e8033c24b550938b..caede5e2b59a7e3843c1ec966a02f2bbd86f2a84 100644 --- a/substrate/primitives/runtime/src/traits.rs +++ b/substrate/primitives/runtime/src/traits.rs @@ -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())