[core-fellowship] Add permissionless import_member

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

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

@@ -260,6 +260,23 @@ mod benchmarks {
 		Ok(())
 	}
 
+	#[benchmark]
+	fn import_member() -> Result<(), BenchmarkError> {
+		let member = account("member", 0, SEED);
+		let sender = account("sender", 0, SEED);
+
+		T::Members::induct(&member)?;
+		T::Members::promote(&member)?;
+
+		assert!(!Member::<T, I>::contains_key(&member));
+
+		#[extrinsic_call]
+		_(RawOrigin::Signed(sender), member.clone());
+
+		assert!(Member::<T, I>::contains_key(&member));
+		Ok(())
+	}
+
 	#[benchmark]
 	fn approve() -> Result<(), BenchmarkError> {
 		let member = make_member::<T, I>(1)?;

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

@@ -585,6 +585,7 @@ pub mod pallet {
 			Ok(if replaced { Pays::Yes } else { Pays::No }.into())
 		}
 
+		/// DEPRECATED: Use `import_member` instead.
 		/// Introduce an already-ranked individual of the collective into this pallet. The rank may
 		/// still be zero.
 		///
@@ -596,17 +597,29 @@ pub mod pallet {
 		#[pallet::call_index(8)]
 		pub fn import(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
 			let who = ensure_signed(origin)?;
-			ensure!(!Member::<T, I>::contains_key(&who), Error::<T, I>::AlreadyInducted);
-			let rank = T::Members::rank_of(&who).ok_or(Error::<T, I>::Unranked)?;
+			Self::do_import(who)?;
 
-			let now = frame_system::Pallet::<T>::block_number();
-			Member::<T, I>::insert(
-				&who,
-				MemberStatus { is_active: true, last_promotion: 0u32.into(), last_proof: now },
-			);
-			Self::deposit_event(Event::<T, I>::Imported { who, rank });
+			Ok(Pays::No.into()) // Successful imports are free
+		}
 
-			Ok(Pays::No.into())
+		/// Introduce an already-ranked individual of the collective into this pallet. The rank may
+		/// still be zero. Can be called by anyone on any collective member - including the sender.
+		///
+		/// This resets `last_proof` to the current block and `last_promotion` will be set to zero,
+		/// thereby delaying any automatic demotion but allowing immediate promotion.
+		///
+		/// - `origin`: A signed origin of a ranked, but not tracked, account.
+		/// - `who`: The account ID of the collective member to be inducted.
+		#[pallet::weight(T::WeightInfo::set_partial_params())]
+		#[pallet::call_index(11)]
+		pub fn import_member(
+			origin: OriginFor<T>,
+			who: T::AccountId,
+		) -> DispatchResultWithPostInfo {
+			ensure_signed(origin)?;
+			Self::do_import(who)?;
+
+			Ok(Pays::No.into()) // Successful imports are free
 		}
 
 		/// Set the parameters partially.
@@ -661,6 +674,21 @@ pub mod pallet {
 				}
 			}
 		}
+
+		pub(crate) fn do_import(who: T::AccountId) -> DispatchResult {
+			ensure!(!Member::<T, I>::contains_key(&who), Error::<T, I>::AlreadyInducted);
+			let rank = T::Members::rank_of(&who).ok_or(Error::<T, I>::Unranked)?;
+
+			let now = frame_system::Pallet::<T>::block_number();
+			Member::<T, I>::insert(
+				&who,
+				MemberStatus { is_active: true, last_promotion: 0u32.into(), last_proof: now },
+			);
+			Self::deposit_event(Event::<T, I>::Imported { who, rank });
+
+			Ok(())
+		}
+
 		/// Convert a rank into a `0..RANK_COUNT` index suitable for the arrays in Params.
 		///
 		/// Rank 1 becomes index 0, rank `RANK_COUNT` becomes index `RANK_COUNT - 1`. Any rank not

substrate/frame/core-fellowship/src/tests/integration.rs

@@ -18,7 +18,7 @@
 //! Integration test together with the ranked-collective pallet.
 
 use frame_support::{
-	assert_noop, assert_ok, derive_impl, hypothetically, ord_parameter_types,
+	assert_noop, assert_ok, derive_impl, hypothetically, hypothetically_ok, ord_parameter_types,
 	pallet_prelude::Weight,
 	parameter_types,
 	traits::{ConstU16, EitherOf, IsInVec, MapSuccess, NoOpPoll, TryMapSuccess},
@@ -170,6 +170,37 @@ fn evidence(e: u32) -> Evidence<Test, ()> {
 		.expect("Static length matches")
 }
 
+#[test]
+fn import_simple_works() {
+	new_test_ext().execute_with(|| {
+		for i in 0u16..9 {
+			let acc = i as u64;
+
+			// Does not work yet
+			assert_noop!(CoreFellowship::import(signed(acc)), Error::<Test>::Unranked);
+			assert_noop!(
+				CoreFellowship::import_member(signed(acc + 1), acc),
+				Error::<Test>::Unranked
+			);
+
+			assert_ok!(Club::add_member(RuntimeOrigin::root(), acc));
+			promote_n_times(acc, i);
+
+			hypothetically_ok!(CoreFellowship::import(signed(acc)));
+			hypothetically_ok!(CoreFellowship::import_member(signed(acc), acc));
+			// Works from other accounts
+			assert_ok!(CoreFellowship::import_member(signed(acc + 1), acc));
+
+			// Does not work again
+			assert_noop!(CoreFellowship::import(signed(acc)), Error::<Test>::AlreadyInducted);
+			assert_noop!(
+				CoreFellowship::import_member(signed(acc + 1), acc),
+				Error::<Test>::AlreadyInducted
+			);
+		}
+	});
+}
+
 #[test]
 fn swap_simple_works() {
 	new_test_ext().execute_with(|| {
@@ -178,7 +209,8 @@ fn swap_simple_works() {
 
 			assert_ok!(Club::add_member(RuntimeOrigin::root(), acc));
 			promote_n_times(acc, i);
-			assert_ok!(CoreFellowship::import(signed(acc)));
+			hypothetically_ok!(CoreFellowship::import(signed(acc)));
+			assert_ok!(CoreFellowship::import_member(signed(acc), acc));
 
 			// Swapping normally works:
 			assert_ok!(Club::exchange_member(RuntimeOrigin::root(), acc, acc + 10));

substrate/frame/core-fellowship/src/tests/unit.rs

@@ -222,6 +222,66 @@ fn set_partial_params_works() {
 	});
 }
 
+#[test]
+fn import_member_works() {
+	new_test_ext().execute_with(|| {
+		assert_noop!(CoreFellowship::import_member(signed(0), 0), Error::<Test>::Unranked);
+		assert_noop!(CoreFellowship::import(signed(0)), Error::<Test>::Unranked);
+
+		// Make induction work:
+		set_rank(0, 1);
+		assert!(!Member::<Test>::contains_key(0), "not yet imported");
+
+		// `import_member` can be used to induct ourselves:
+		hypothetically!({
+			assert_ok!(CoreFellowship::import_member(signed(0), 0));
+			assert!(Member::<Test>::contains_key(0), "got imported");
+
+			// Twice does not work:
+			assert_noop!(
+				CoreFellowship::import_member(signed(0), 0),
+				Error::<Test>::AlreadyInducted
+			);
+			assert_noop!(CoreFellowship::import(signed(0)), Error::<Test>::AlreadyInducted);
+		});
+
+		// But we could have also used `import`:
+		hypothetically!({
+			assert_ok!(CoreFellowship::import(signed(0)));
+			assert!(Member::<Test>::contains_key(0), "got imported");
+
+			// Twice does not work:
+			assert_noop!(
+				CoreFellowship::import_member(signed(0), 0),
+				Error::<Test>::AlreadyInducted
+			);
+			assert_noop!(CoreFellowship::import(signed(0)), Error::<Test>::AlreadyInducted);
+		});
+	});
+}
+
+#[test]
+fn import_member_same_as_import() {
+	new_test_ext().execute_with(|| {
+		for rank in 0..=9 {
+			set_rank(0, rank);
+
+			let import_root = hypothetically!({
+				assert_ok!(CoreFellowship::import(signed(0)));
+				sp_io::storage::root(sp_runtime::StateVersion::V1)
+			});
+
+			let import_member_root = hypothetically!({
+				assert_ok!(CoreFellowship::import_member(signed(1), 0));
+				sp_io::storage::root(sp_runtime::StateVersion::V1)
+			});
+
+			// `import` and `import_member` do exactly the same thing.
+			assert_eq!(import_root, import_member_root);
+		}
+	});
+}
+
 #[test]
 fn induct_works() {
 	new_test_ext().execute_with(|| {