[FRAME] Make core-fellowship ans salary work for swapped members (#…

…3156)

Fixup for #2587 to make
the `core-fellowship` crate work with swapped members.

Adds a `MemberSwappedHandler` to the `ranked-collective` pallet that are
implemented by `core-fellowship+salary`.
There is are exhaustive tests
[here](https://github.com/paritytech/polkadot-sdk/blob/72aa7ac17a0e5b16faab5d2992aa2db2e01b05d0/substrate/frame/core-fellowship/src/tests/integration.rs#L338)
and
[here](https://github.com/paritytech/polkadot-sdk/blob/ab3cdb05a5ebc1ff841f8dda67edef0ea40bbba5/substrate/frame/salary/src/tests/integration.rs#L224)
to check that adding member `1` is equivalent to adding member `0` and
then swapping.

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

cumulus/parachains/runtimes/collectives/collectives-westend/src/ambassador/mod.rs

@@ -113,7 +113,10 @@ impl pallet_ranked_collective::Config<AmbassadorCollectiveInstance> for Runtime
 	type ExchangeOrigin = ExchangeOrigin;
 	type Polls = AmbassadorReferenda;
 	type MinRankOfClass = sp_runtime::traits::Identity;
+	type MemberSwappedHandler = (crate::AmbassadorCore, crate::AmbassadorSalary);
 	type VoteWeight = pallet_ranked_collective::Linear;
+	#[cfg(feature = "runtime-benchmarks")]
+	type BenchmarkSetup = (crate::AmbassadorCore, crate::AmbassadorSalary);
 }
 
 parameter_types! {

cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs

@@ -141,7 +141,10 @@ impl pallet_ranked_collective::Config<FellowshipCollectiveInstance> for Runtime
 		EitherOf<EnsureRootWithSuccess<Self::AccountId, ConstU16<65535>>, Fellows>;
 	type Polls = FellowshipReferenda;
 	type MinRankOfClass = tracks::MinRankOfClass;
+	type MemberSwappedHandler = (crate::FellowshipCore, crate::FellowshipSalary);
 	type VoteWeight = pallet_ranked_collective::Geometric;
+	#[cfg(feature = "runtime-benchmarks")]
+	type BenchmarkSetup = (crate::FellowshipCore, crate::FellowshipSalary);
 }
 
 pub type FellowshipCoreInstance = pallet_core_fellowship::Instance1;

polkadot/runtime/rococo/src/governance/fellowship.rs

@@ -344,5 +344,8 @@ impl pallet_ranked_collective::Config<FellowshipCollectiveInstance> for Runtime
 		EitherOf<EnsureRootWithSuccess<Self::AccountId, ConstU16<65535>>, Fellows>;
 	type Polls = FellowshipReferenda;
 	type MinRankOfClass = sp_runtime::traits::Identity;
+	type MemberSwappedHandler = ();
 	type VoteWeight = pallet_ranked_collective::Geometric;
+	#[cfg(feature = "runtime-benchmarks")]
+	type BenchmarkSetup = ();
 }

prdoc/pr_3156.prdoc

@@ -0,0 +1,13 @@
+title: "Adapt core-fellowship and salary pallets for swapped members"
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      The ranked-collective pallet got the ability to swap members but the core-fellowship and
+      salary pallets would not be notified of this change. This MR adds `MemberSwappedHandler` to
+      the collective that is implemented by both pallets.
+
+crates:
+  - name: "pallet-ranked-collective"
+  - name: "pallet-core-fellowship"
+  - name: "pallet-salary"

substrate/bin/node/runtime/src/lib.rs

@@ -1019,6 +1019,9 @@ impl pallet_ranked_collective::Config for Runtime {
 	type Polls = RankedPolls;
 	type MinRankOfClass = traits::Identity;
 	type VoteWeight = pallet_ranked_collective::Geometric;
+	type MemberSwappedHandler = (CoreFellowship, Salary);
+	#[cfg(feature = "runtime-benchmarks")]
+	type BenchmarkSetup = (CoreFellowship, Salary);
 }
 
 impl pallet_remark::Config for Runtime {

substrate/frame/core-fellowship/Cargo.toml

@@ -27,15 +27,18 @@ sp-core = { path = "../../primitives/core", default-features = false }
 sp-io = { path = "../../primitives/io", default-features = false }
 sp-runtime = { path = "../../primitives/runtime", default-features = false }
 sp-std = { path = "../../primitives/std", default-features = false }
+pallet-ranked-collective = { path = "../ranked-collective", default-features = false, optional = true }
 
 [features]
 default = ["std"]
 std = [
 	"codec/std",
 	"frame-benchmarking?/std",
+	"frame-support/experimental",
 	"frame-support/std",
 	"frame-system/std",
 	"log/std",
+	"pallet-ranked-collective/std",
 	"scale-info/std",
 	"sp-arithmetic/std",
 	"sp-core/std",
@@ -47,10 +50,12 @@ runtime-benchmarks = [
 	"frame-benchmarking/runtime-benchmarks",
 	"frame-support/runtime-benchmarks",
 	"frame-system/runtime-benchmarks",
+	"pallet-ranked-collective/runtime-benchmarks",
 	"sp-runtime/runtime-benchmarks",
 ]
 try-runtime = [
 	"frame-support/try-runtime",
 	"frame-system/try-runtime",
+	"pallet-ranked-collective?/try-runtime",
 	"sp-runtime/try-runtime",
 ]

substrate/frame/core-fellowship/src/benchmarking.rs

@@ -227,7 +227,7 @@ mod benchmarks {
 
 	impl_benchmark_test_suite! {
 		CoreFellowship,
-		crate::tests::new_test_ext(),
-		crate::tests::Test,
+		crate::tests::unit::new_test_ext(),
+		crate::tests::unit::Test,
 	}
 }

substrate/frame/core-fellowship/src/lib.rs

@@ -65,10 +65,12 @@ use sp_runtime::RuntimeDebug;
 use sp_std::{marker::PhantomData, prelude::*};
 
 use frame_support::{
+	defensive,
 	dispatch::DispatchResultWithPostInfo,
 	ensure, impl_ensure_origin_with_arg_ignoring_arg,
 	traits::{
 		tokens::Balance as BalanceTrait, EnsureOrigin, EnsureOriginWithArg, Get, RankedMembers,
+		RankedMembersSwapHandler,
 	},
 	BoundedVec,
 };
@@ -249,6 +251,8 @@ pub mod pallet {
 		},
 		/// Pre-ranked account has been inducted at their current rank.
 		Imported { who: T::AccountId, rank: RankOf<T, I> },
+		/// A member had its AccountId swapped.
+		Swapped { who: T::AccountId, new_who: T::AccountId },
 	}
 
 	#[pallet::error]
@@ -603,3 +607,38 @@ impl_ensure_origin_with_arg_ignoring_arg! {
 		EnsureOriginWithArg<T::RuntimeOrigin, A> for EnsureInducted<T, I, MIN_RANK>
 	{}
 }
+
+impl<T: Config<I>, I: 'static> RankedMembersSwapHandler<T::AccountId, u16> for Pallet<T, I> {
+	fn swapped(old: &T::AccountId, new: &T::AccountId, _rank: u16) {
+		if old == new {
+			defensive!("Should not try to swap with self");
+			return
+		}
+		if !Member::<T, I>::contains_key(old) {
+			defensive!("Should not try to swap non-member");
+			return
+		}
+		if Member::<T, I>::contains_key(new) {
+			defensive!("Should not try to overwrite existing member");
+			return
+		}
+
+		if let Some(member) = Member::<T, I>::take(old) {
+			Member::<T, I>::insert(new, member);
+		}
+		if let Some(we) = MemberEvidence::<T, I>::take(old) {
+			MemberEvidence::<T, I>::insert(new, we);
+		}
+
+		Self::deposit_event(Event::<T, I>::Swapped { who: old.clone(), new_who: new.clone() });
+	}
+}
+
+#[cfg(feature = "runtime-benchmarks")]
+impl<T: Config<I>, I: 'static>
+	pallet_ranked_collective::BenchmarkSetup<<T as frame_system::Config>::AccountId> for Pallet<T, I>
+{
+	fn ensure_member(who: &<T as frame_system::Config>::AccountId) {
+		Self::import(frame_system::RawOrigin::Signed(who.clone()).into()).unwrap();
+	}
+}