Ranked collective Add+Remove origins

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;

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<

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);

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

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>>;

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>>,

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");
 

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);

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>>,

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>>,

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())