From 9a27b53e8e0523e0c01df11abd492fc16af5a5bf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Silva?=
 <123550+andresilva@users.noreply.github.com>
Date: Sun, 31 Dec 2023 18:12:36 +0000
Subject: [PATCH] core-fellowship: allow infinite demotion period (#2828)

---
 .gitlab/pipeline/test.yml                     |  7 +++---
 .../frame/core-fellowship/src/benchmarking.rs | 17 ++++++++++++++
 substrate/frame/core-fellowship/src/lib.rs    |  5 +++++
 substrate/frame/core-fellowship/src/tests.rs  | 22 +++++++++++++++++++
 4 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/.gitlab/pipeline/test.yml b/.gitlab/pipeline/test.yml
index bbe9b612bc3..359d5b4dbcd 100644
--- a/.gitlab/pipeline/test.yml
+++ b/.gitlab/pipeline/test.yml
@@ -238,6 +238,8 @@ test-deterministic-wasm:
 
 cargo-check-benches:
   stage: test
+  artifacts:
+    expire_in: 10 days
   variables:
     CI_JOB_NAME: "cargo-check-benches"
   extends:
@@ -303,13 +305,10 @@ node-bench-regression-guard:
       artifacts: true
   variables:
     CI_IMAGE: "paritytech/node-bench-regression-guard:latest"
-    # current git limit is 20, set to 100 to avoid failures (gitlab removes old artifacts)
-    GIT_DEPTH: 100
-    GIT_STRATEGY: fetch
   before_script: [""]
   script:
     - if [ $(ls -la artifacts/benches/ | grep master | wc -l) == 0 ]; then
-      echo "Couldn't find master artifacts, consider increasing GIT_LIMIT variable";
+      echo "Couldn't find master artifacts";
       exit 1;
       fi
     - echo "------- IMPORTANT -------"
diff --git a/substrate/frame/core-fellowship/src/benchmarking.rs b/substrate/frame/core-fellowship/src/benchmarking.rs
index ea0b5c6d449..a3c410fac0a 100644
--- a/substrate/frame/core-fellowship/src/benchmarking.rs
+++ b/substrate/frame/core-fellowship/src/benchmarking.rs
@@ -53,6 +53,19 @@ mod benchmarks {
 		Ok(member)
 	}
 
+	fn set_benchmark_params<T: Config<I>, I: 'static>() -> Result<(), BenchmarkError> {
+		let params = ParamsType {
+			active_salary: [100u32.into(); 9],
+			passive_salary: [10u32.into(); 9],
+			demotion_period: [100u32.into(); 9],
+			min_promotion_period: [100u32.into(); 9],
+			offboard_timeout: 1u32.into(),
+		};
+
+		CoreFellowship::<T, I>::set_params(RawOrigin::Root.into(), Box::new(params))?;
+		Ok(())
+	}
+
 	#[benchmark]
 	fn set_params() -> Result<(), BenchmarkError> {
 		let params = ParamsType {
@@ -72,6 +85,8 @@ mod benchmarks {
 
 	#[benchmark]
 	fn bump_offboard() -> Result<(), BenchmarkError> {
+		set_benchmark_params::<T, I>()?;
+
 		let member = make_member::<T, I>(0)?;
 
 		// Set it to the max value to ensure that any possible auto-demotion period has passed.
@@ -89,6 +104,8 @@ mod benchmarks {
 
 	#[benchmark]
 	fn bump_demote() -> Result<(), BenchmarkError> {
+		set_benchmark_params::<T, I>()?;
+
 		let member = make_member::<T, I>(2)?;
 
 		// Set it to the max value to ensure that any possible auto-demotion period has passed.
diff --git a/substrate/frame/core-fellowship/src/lib.rs b/substrate/frame/core-fellowship/src/lib.rs
index 1aa53cf08d1..2042f68e714 100644
--- a/substrate/frame/core-fellowship/src/lib.rs
+++ b/substrate/frame/core-fellowship/src/lib.rs
@@ -298,6 +298,11 @@ pub mod pallet {
 				let rank_index = Self::rank_to_index(rank).ok_or(Error::<T, I>::InvalidRank)?;
 				params.demotion_period[rank_index]
 			};
+
+			if demotion_period.is_zero() {
+				return Err(Error::<T, I>::NothingDoing.into())
+			}
+
 			let demotion_block = member.last_proof.saturating_add(demotion_period);
 
 			// Ensure enough time has passed.
diff --git a/substrate/frame/core-fellowship/src/tests.rs b/substrate/frame/core-fellowship/src/tests.rs
index 9ac381ab7f5..c9098f2171f 100644
--- a/substrate/frame/core-fellowship/src/tests.rs
+++ b/substrate/frame/core-fellowship/src/tests.rs
@@ -306,6 +306,28 @@ fn offboard_works() {
 	});
 }
 
+#[test]
+fn infinite_demotion_period_works() {
+	new_test_ext().execute_with(|| {
+		let params = ParamsType {
+			active_salary: [10; 9],
+			passive_salary: [10; 9],
+			min_promotion_period: [10; 9],
+			demotion_period: [0; 9],
+			offboard_timeout: 0,
+		};
+		assert_ok!(CoreFellowship::set_params(signed(1), Box::new(params)));
+
+		set_rank(0, 0);
+		assert_ok!(CoreFellowship::import(signed(0)));
+		set_rank(1, 1);
+		assert_ok!(CoreFellowship::import(signed(1)));
+
+		assert_noop!(CoreFellowship::bump(signed(0), 0), Error::<Test>::NothingDoing);
+		assert_noop!(CoreFellowship::bump(signed(0), 1), Error::<Test>::NothingDoing);
+	});
+}
+
 #[test]
 fn proof_postpones_auto_demote() {
 	new_test_ext().execute_with(|| {
-- 
GitLab