runtime: working-group: add rationale to leave and terminate role

runtime-modules/proposals/codex/src/types.rs

@@ -100,7 +100,7 @@ pub struct TerminateRoleParameters<WorkerId, Balance> {
     pub worker_id: WorkerId,
 
     /// Terminate role slash penalty.
-    pub penalty: Option<Penalty<Balance>>,
+    pub penalty: Option<Balance>,
 
     /// Defines working group with the open position.
     pub working_group: WorkingGroup,

runtime-modules/working-group/src/benchmarking.rs

@@ -303,13 +303,18 @@ benchmarks_instance! {
         let mut worker_id = Zero::zero();
         for id in application_account_id {
             worker_id += One::one();
-            WorkingGroup::<T, _>::leave_role(RawOrigin::Signed(id).into(), worker_id).unwrap();
+            WorkingGroup::<T, _>::leave_role(
+                    RawOrigin::Signed(id).into(),
+                    worker_id,
+                    Some(vec![0u8]),
+                ).unwrap();
         }
 
         // Worst case scenario one of the leaving workers is the lead
         WorkingGroup::<T, _>::leave_role(
             RawOrigin::Signed(lead_id).into(),
-            lead_worker_id
+            lead_worker_id,
+            Some(vec![0u8]),
         ).unwrap();
 
         for i in 1..successful_application_ids.len() {
@@ -717,11 +722,12 @@ benchmarks_instance! {
         // To be able to pay unpaid reward
         let current_budget = BalanceOf::<T>::max_value();
         WorkingGroup::<T, _>::set_budget(RawOrigin::Root.into(), current_budget).unwrap();
-        let penalty = Penalty {
-            slashing_text: vec![0u8; i.try_into().unwrap()],
-            slashing_amount: One::one(),
-        };
-    }: terminate_role(RawOrigin::Signed(lead_id.clone()), worker_id, Some(penalty))
+    }: terminate_role(
+            RawOrigin::Signed(lead_id.clone()),
+            worker_id,
+            Some(One::one()),
+            Some(vec![0u8; i.try_into().unwrap()])
+        )
     verify {
         assert!(!WorkerById::<T, I>::contains_key(worker_id), "Worker not terminated");
         assert_last_event::<T, I>(RawEvent::TerminatedWorker(worker_id).into());
@@ -735,11 +741,12 @@ benchmarks_instance! {
         let current_budget = BalanceOf::<T>::max_value();
         // To be able to pay unpaid reward
         WorkingGroup::<T, _>::set_budget(RawOrigin::Root.into(), current_budget).unwrap();
-        let penalty = Penalty {
-            slashing_text: vec![0u8; i.try_into().unwrap()],
-            slashing_amount: One::one(),
-        };
-    }: terminate_role(RawOrigin::Root, lead_worker_id, Some(penalty))
+    }: terminate_role(
+            RawOrigin::Root,
+            lead_worker_id,
+            Some(One::one()),
+            Some(vec![0u8; i.try_into().unwrap()])
+        )
     verify {
         assert!(!WorkerById::<T, I>::contains_key(lead_worker_id), "Worker not terminated");
         assert_last_event::<T, I>(RawEvent::TerminatedLeader(lead_worker_id).into());
@@ -901,6 +908,7 @@ benchmarks_instance! {
 
     // This is always worse than leave_role_immediatly
     leave_role_immediatly {
+        let i in 0 .. MAX_BYTES;
         // Worst case scenario there is a lead(this requires **always** more steps)
         // could separate into new branch to tighten weight
         // Also, workers without stake can leave immediatly
@@ -913,7 +921,11 @@ benchmarks_instance! {
             BalanceOf::<T>::max_value()
         ).unwrap();
 
-    }: leave_role(RawOrigin::Signed(caller_id), lead_worker_id)
+    }: leave_role(
+            RawOrigin::Signed(caller_id),
+            lead_worker_id,
+            Some(vec![0u8; i.try_into().unwrap()])
+        )
     verify {
         assert!(!WorkerById::<T, I>::contains_key(lead_worker_id), "Worker hasn't left");
         assert_last_event::<T, I>(RawEvent::WorkerExited(lead_worker_id).into());
@@ -923,6 +935,7 @@ benchmarks_instance! {
     // but since it's so obviously a different branch I think it's a good idea
     // to leave this branch and use tha max between these 2
     leave_role_later {
+        let i in 0 .. MAX_BYTES;
         // Workers with stake can't leave immediatly
         let (caller_id, caller_worker_id) = insert_a_worker::<T, I>(
             StakingRole::WithStakes,
@@ -930,7 +943,11 @@ benchmarks_instance! {
             0,
             None
         );
-    }: leave_role(RawOrigin::Signed(caller_id), caller_worker_id)
+    }: leave_role(
+            RawOrigin::Signed(caller_id),
+            caller_worker_id,
+            Some(vec![0u8; i.try_into().unwrap()])
+        )
     verify {
         assert_eq!(
             WorkingGroup::<T, _>::worker_by_id(caller_worker_id).started_leaving_at,

runtime-modules/working-group/src/lib.rs

@@ -549,6 +549,7 @@ decl_module! {
         pub fn leave_role(
             origin,
             worker_id: WorkerId<T>,
+            _rationale: Option<Vec<u8>>
         ) {
             // Ensure there is a signer which matches role account of worker corresponding to provided id.
             let worker = checks::ensure_worker_signed::<T, I>(origin, &worker_id)?;
@@ -579,11 +580,14 @@ decl_module! {
         /// - DB:
         ///    - O(1) doesn't depend on the state or parameters
         /// # </weight>
-        #[weight = Module::<T, I>::terminate_role_weight(&penalty)]
+        // Note: We use separate penalty and rationale instead of Penalty struct to be able to have
+        // one set and not the other
+        #[weight = Module::<T, I>::terminate_role_weight(&_rationale)]
         pub fn terminate_role(
             origin,
             worker_id: WorkerId<T>,
-            penalty: Option<Penalty<BalanceOf<T>>>,
+            penalty: Option<BalanceOf<T>>,
+            _rationale: Option<Vec<u8>>,
         ) {
             // Ensure lead is set or it is the council terminating the leader.
             let is_sudo = checks::ensure_origin_for_worker_operation::<T,I>(origin, worker_id)?;
@@ -605,7 +609,7 @@ decl_module! {
 
             if let Some(penalty) = penalty {
                 if let Some(staking_account_id) = worker.staking_account_id.clone() {
-                    Self::slash(worker_id, &staking_account_id, Some(penalty.slashing_amount));
+                    Self::slash(worker_id, &staking_account_id, Some(penalty));
                 }
             }
 
@@ -1045,17 +1049,17 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
     }
 
     // Calculate weights for terminate_role
-    fn terminate_role_weight(penalty: &Option<Penalty<BalanceOf<T>>>) -> Weight {
+    fn terminate_role_weight(penalty: &Option<Vec<u8>>) -> Weight {
         WeightInfoWorkingGroup::<T, I>::terminate_role_lead(
             penalty
                 .as_ref()
-                .map(|penalty| penalty.slashing_text.len().saturated_into())
+                .map(|penalty| penalty.len().saturated_into())
                 .unwrap_or_default(),
         )
         .max(WeightInfoWorkingGroup::<T, I>::terminate_role_worker(
             penalty
                 .as_ref()
-                .map(|penalty| penalty.slashing_text.len().saturated_into())
+                .map(|penalty| penalty.len().saturated_into())
                 .unwrap_or_default(),
         ))
     }

runtime-modules/working-group/src/tests/fixtures.rs

@@ -515,7 +515,7 @@ impl LeaveWorkerRoleFixture {
 
     pub fn call_and_assert(&self, expected_result: DispatchResult) {
         let actual_result =
-            TestWorkingGroup::leave_role(self.origin.clone().into(), self.worker_id);
+            TestWorkingGroup::leave_role(self.origin.clone().into(), self.worker_id, None);
         assert_eq!(actual_result, expected_result);
 
         if actual_result.is_ok() {
@@ -541,7 +541,8 @@ impl LeaveWorkerRoleFixture {
 pub struct TerminateWorkerRoleFixture {
     worker_id: u64,
     origin: RawOrigin<u64>,
-    penalty: Option<Penalty<u64>>,
+    penalty: Option<u64>,
+    rationale: Option<Vec<u8>>,
 }
 
 impl TerminateWorkerRoleFixture {
@@ -550,13 +551,14 @@ impl TerminateWorkerRoleFixture {
             worker_id,
             origin: RawOrigin::Signed(1),
             penalty: None,
+            rationale: None,
         }
     }
     pub fn with_origin(self, origin: RawOrigin<u64>) -> Self {
         Self { origin, ..self }
     }
 
-    pub fn with_penalty(self, penalty: Option<Penalty<u64>>) -> Self {
+    pub fn with_penalty(self, penalty: Option<u64>) -> Self {
         Self { penalty, ..self }
     }
 
@@ -565,6 +567,7 @@ impl TerminateWorkerRoleFixture {
             self.origin.clone().into(),
             self.worker_id,
             self.penalty.clone(),
+            self.rationale.clone(),
         );
         assert_eq!(actual_result, expected_result);
 

runtime-modules/working-group/src/tests/mod.rs

@@ -1134,11 +1134,6 @@ fn terminate_worker_with_slashing_succeeds() {
             leaving_unstaking_period: 10,
         });
 
-        let penalty = Penalty {
-            slashing_amount: stake,
-            slashing_text: Vec::new(),
-        };
-
         increase_total_balance_issuance_using_account_id(account_id, total_balance);
 
         let worker_id = HireRegularWorkerFixture::default()
@@ -1148,8 +1143,7 @@ fn terminate_worker_with_slashing_succeeds() {
         assert_eq!(Balances::usable_balance(&account_id), total_balance - stake);
 
         let terminate_worker_role_fixture =
-            TerminateWorkerRoleFixture::default_for_worker_id(worker_id)
-                .with_penalty(Some(penalty));
+            TerminateWorkerRoleFixture::default_for_worker_id(worker_id).with_penalty(Some(stake));
 
         terminate_worker_role_fixture.call_and_assert(Ok(()));
 
@@ -1164,11 +1158,6 @@ fn terminate_worker_with_slashing_fails_with_no_staking_account() {
         let total_balance = 300;
         let stake = 200;
 
-        let penalty = Penalty {
-            slashing_amount: stake,
-            slashing_text: Vec::new(),
-        };
-
         increase_total_balance_issuance_using_account_id(account_id, total_balance);
 
         let worker_id = HiringWorkflow::default()
@@ -1177,8 +1166,7 @@ fn terminate_worker_with_slashing_fails_with_no_staking_account() {
             .unwrap();
 
         let terminate_worker_role_fixture =
-            TerminateWorkerRoleFixture::default_for_worker_id(worker_id)
-                .with_penalty(Some(penalty));
+            TerminateWorkerRoleFixture::default_for_worker_id(worker_id).with_penalty(Some(stake));
 
         terminate_worker_role_fixture.call_and_assert(Err(
             Error::<Test, DefaultInstance>::CannotChangeStakeWithoutStakingAccount.into(),

runtime/src/integration/proposals/proposal_encoder.rs

@@ -174,6 +174,7 @@ where
         working_group::Call::<T, I>::terminate_role(
             terminate_role_params.worker_id,
             terminate_role_params.penalty,
+            None, // The rationale is given in the proposal description
         )
     }
 

runtime/src/tests/proposals_integration/working_group_proposals.rs

@@ -293,7 +293,7 @@ fn terminate_role(
     member_id: MemberId,
     account_id: [u8; 32],
     leader_worker_id: u64,
-    penalty: Option<Penalty<Balance>>,
+    penalty: Option<u128>,
     sequence_number: u32, // action sequence number to align with other actions
     working_group: WorkingGroup,
 ) {
@@ -314,7 +314,7 @@ fn terminate_role(
             ProposalDetails::TerminateWorkingGroupLeaderRole(
                 proposals_codex::TerminateRoleParameters {
                     worker_id: leader_worker_id,
-                    penalty: penalty.clone(),
+                    penalty,
                     working_group,
                 },
             ),
@@ -1161,10 +1161,7 @@ fn run_create_terminate_group_leader_role_proposal_with_slashing_execution_succe
             member_id,
             account_id,
             leader_worker_id.into(),
-            Some(Penalty {
-                slashing_amount: stake_amount.into(),
-                slashing_text: Vec::new(),
-            }),
+            Some(stake_amount),
             4,
             working_group,
         );