Add review fixes

- remove worker stages
- remove worker exit summary
- remove worker after exiting
- remove unstate()
- add member_id to the worker

runtime-modules/bureaucracy/src/errors.rs

@@ -71,24 +71,12 @@ decl_error! {
         /// Worker exit rationale text is too short.
         WorkerExitRationaleTextTooShort,
 
-        /// Unstaker does not exist.
-        UnstakerDoesNotExist,
-
-        /// Only workers can unstake.
-        OnlyWorkersCanUnstake,
-
-        /// Worker must be in unstaking stage.
-        WorkerIsNotUnstaking,
-
         /// Signer is not worker role account.
         SignerIsNotWorkerRoleAccount,
 
         /// Worker has no recurring reward.
         WorkerHasNoReward,
 
-        /// Worker is not active.
-        WorkerIsNotActive,
-
         /// Worker does not exist.
         WorkerDoesNotExist,
 

runtime-modules/bureaucracy/src/lib.rs

@@ -51,14 +51,14 @@ use srml_support::traits::{Currency, ExistenceRequirement, WithdrawReasons};
 use srml_support::{decl_event, decl_module, decl_storage, ensure};
 use system::{ensure_root, ensure_signed, RawOrigin};
 
-use crate::types::{WorkerExitInitiationOrigin, WorkerExitSummary, WorkingGroupUnstaker};
+use crate::types::WorkerExitInitiationOrigin;
 use common::constraints::InputValidationLengthConstraint;
 use errors::WrappedError;
 
 pub use errors::Error;
 pub use types::{
     Lead, OpeningPolicyCommitment, RewardPolicy, Worker, WorkerApplication, WorkerOpening,
-    WorkerRoleStage, WorkerRoleStakeProfile,
+    WorkerRoleStakeProfile,
 };
 
 //TODO: initialize a mint!
@@ -136,6 +136,7 @@ type WorkerOf<T> = Worker<
     <T as recurringrewards::Trait>::RewardRelationshipId,
     <T as stake::Trait>::StakeId,
     <T as system::Trait>::BlockNumber,
+    MemberId<T>,
 >;
 
 /// The _Bureaucracy_ main _Trait_
@@ -162,6 +163,7 @@ decl_event!(
         WorkerOpeningId = WorkerOpeningId<T>,
         WorkerApplicationId = WorkerApplicationId<T>,
         WorkerApplicationIdToWorkerIdMap = WorkerApplicationIdToWorkerIdMap<T>,
+        RationaleText = Vec<u8>,
     {
         /// Emits on setting the leader.
         /// Params:
@@ -178,17 +180,14 @@ decl_event!(
         /// Emits on terminating the worker.
         /// Params:
         /// - worker id.
-        TerminatedWorker(WorkerId),
+        /// - termination rationale text
+        TerminatedWorker(WorkerId, RationaleText),
 
         /// Emits on exiting the worker.
         /// Params:
         /// - worker id.
-        WorkerExited(WorkerId),
-
-        /// Emits on unstaking the worker.
-        /// Params:
-        /// - worker id.
-        WorkerUnstaking(WorkerId),
+        /// - exit rationale text
+        WorkerExited(WorkerId, RationaleText),
 
         /// Emits on updating the role account of the worker.
         /// Params:
@@ -273,9 +272,6 @@ decl_storage! {
         /// Next identifier for new worker.
         pub NextWorkerId get(next_worker_id) : WorkerId<T>;
 
-        /// Recover worker by the role stake which is currently unstaking.
-        pub UnstakerByStakeId get(unstaker_by_stake_id) : linked_map StakeId<T> => WorkingGroupUnstaker<MemberId<T>, WorkerId<T>>;
-
         /// Worker exit rationale text length limits.
         pub WorkerExitRationaleText get(worker_exit_rationale_text) : InputValidationLengthConstraint;
     }
@@ -302,7 +298,9 @@ decl_module! {
                 role_account_id: role_account_id.clone(),
             };
 
-            // mutation
+            //
+            // == MUTATION SAFE ==
+            //
 
             // Update current lead
             <CurrentLead<T, I>>::put(new_lead);
@@ -315,39 +313,35 @@ decl_module! {
         pub fn unset_lead(origin) {
             ensure_root(origin)?;
 
-            // Ensure there is a lead set
-            let lead = <CurrentLead<T, I>>::get();
+            let lead = Self::ensure_lead_is_set()?;
 
-            if lead.is_none() {
-                return Err(Error::CurrentLeadNotSet);
-            }
-
-            // mutation
+            //
+            // == MUTATION SAFE ==
+            //
 
             // Update current lead
             <CurrentLead<T, I>>::kill();
 
-            // Unwrap lead
-            if let Some(lead) = lead {
-                // Trigger event
-                Self::deposit_event(RawEvent::LeaderUnset(lead.member_id, lead.role_account_id));
-            }
+            Self::deposit_event(RawEvent::LeaderUnset(lead.member_id, lead.role_account_id));
         }
 
         /// Update the associated role account of the active worker.
         pub fn update_worker_role_account(
             origin,
-            member_id: T::MemberId,
             worker_id: WorkerId<T>,
             new_role_account_id: T::AccountId
         ) {
+            // Ensuring worker actually exists
+            let worker = Self::ensure_worker_exists(&worker_id)?;
 
             // Ensure that origin is signed by member with given id.
             ensure_on_wrapped_error!(
-                membership::members::Module::<T>::ensure_member_controller_account_signed(origin, &member_id)
+                membership::members::Module::<T>::ensure_member_controller_account_signed(origin, &worker.member_id)
             )?;
 
-            // mutation
+            //
+            // == MUTATION SAFE ==
+            //
 
             // Update role account
             WorkerById::<T, I>::mutate(worker_id, |worker| {
@@ -365,12 +359,14 @@ decl_module! {
             new_reward_account_id: T::AccountId
         ) {
             // Ensure there is a signer which matches role account of worker corresponding to provided id.
-            let worker = Self::ensure_active_worker_signed(origin, &worker_id)?;
+            let worker = Self::ensure_worker_signed(origin, &worker_id)?;
 
             // Ensure the worker actually has a recurring reward
             let relationship_id = Self::ensure_worker_has_recurring_reward(&worker)?;
 
-            // mutation
+            //
+            // == MUTATION SAFE ==
+            //
 
             // Update only the reward account.
             ensure_on_wrapped_error!(
@@ -393,9 +389,11 @@ decl_module! {
             rationale_text: Vec<u8>
         ) {
             // Ensure there is a signer which matches role account of worker corresponding to provided id.
-            let active_worker = Self::ensure_active_worker_signed(origin, &worker_id)?;
+            let active_worker = Self::ensure_worker_signed(origin, &worker_id)?;
 
-            // mutation
+            //
+            // == MUTATION SAFE ==
+            //
 
             Self::deactivate_worker(
                 &worker_id,
@@ -415,13 +413,15 @@ decl_module! {
             // Ensure lead is set and is origin signer
             Self::ensure_origin_is_set_lead(origin)?;
 
-            // Ensuring worker actually exists and is active
-            let worker = Self::ensure_active_worker_exists(&worker_id)?;
+            // Ensuring worker actually exists
+            let worker = Self::ensure_worker_exists(&worker_id)?;
 
             // Ensure rationale text is valid
             Self::ensure_worker_exit_rationale_text_is_valid(&rationale_text)?;
 
-            // mutation
+            //
+            // == MUTATION SAFE ==
+            //
 
             Self::deactivate_worker(
                 &worker_id,
@@ -431,58 +431,6 @@ decl_module! {
             )?;
         }
 
-        /// Unstake using provided stake_id. Has no side effects if stake_id is not relevant
-        /// to this module.
-        pub fn unstake(origin, stake_id: StakeId<T>) {
-            ensure_root(origin)?;
-
-            // Ignore if unstaked doesn't exist
-            if !<UnstakerByStakeId<T, I>>::exists(stake_id) {
-                return Ok(());
-            }
-
-            // Unstaker must be in this group
-            let unstaker = Self::ensure_unstaker_exists(&stake_id)?;
-
-            // Get worker doing the unstaking,
-            // currently the only possible unstaker in this module.
-            let worker_id = if let WorkingGroupUnstaker::Worker(worker_id) = unstaker {
-                worker_id
-            } else {
-                return Err(Error::OnlyWorkersCanUnstake);
-            };
-
-            let unstaking_worker = Self::ensure_worker_exists(&worker_id)?;
-
-            // Update stage of worker
-            let worker_exit_summary =
-                if let WorkerRoleStage::Unstaking(summary) = unstaking_worker.stage {
-                    summary
-                } else {
-                    return Err(Error::WorkerIsNotUnstaking);
-                };
-
-            let new_worker = Worker {
-                stage: WorkerRoleStage::Exited(worker_exit_summary.clone()),
-                ..unstaking_worker
-            };
-
-            // mutation
-
-            WorkerById::<T, I>::insert(worker_id, new_worker);
-
-            // Remove from unstaker
-            UnstakerByStakeId::<T, I>::remove(stake_id);
-
-            // Trigger event
-            let event = match worker_exit_summary.origin {
-                WorkerExitInitiationOrigin::Lead => RawEvent::TerminatedWorker(worker_id),
-                WorkerExitInitiationOrigin::Worker => RawEvent::WorkerExited(worker_id),
-            };
-
-            Self::deposit_event(event);
-        }
-
         // ****************** Hiring flow **********************
 
          /// Add an opening for a worker role.
@@ -504,7 +452,9 @@ decl_module! {
 
             let policy_commitment = commitment.clone();
 
-            // mutation
+            //
+            // == MUTATION SAFE ==
+            //
 
             let opening_id = ensure_on_wrapped_error!(
                 hiring::Module::<T>::add_opening(
@@ -549,7 +499,9 @@ decl_module! {
             // Attempt to begin accepting applications
             // NB: Combined ensure check and mutation in hiring module
 
-            // mutation
+            //
+            // == MUTATION SAFE ==
+            //
 
             ensure_on_wrapped_error!(
                 hiring::Module::<T>::begin_accepting_applications(worker_opening.opening_id)
@@ -606,7 +558,9 @@ decl_module! {
                 member_id
             )?;
 
-            // mutation
+            //
+            // == MUTATION SAFE ==
+            //
 
             // Make imbalances for staking
             let opt_role_stake_imbalance = Self::make_stake_opt_imbalance(&opt_role_stake_balance, &source_account);
@@ -663,6 +617,10 @@ decl_module! {
                 Error::OriginIsNotApplicant
             );
 
+            //
+            // == MUTATION SAFE ==
+            //
+
             // Attempt to deactivate application
             // NB: Combined ensure check and mutation in hiring module
             ensure_on_wrapped_error!(
@@ -673,7 +631,6 @@ decl_module! {
                 )
             )?;
 
-            // mutation
 
             // Trigger event
             Self::deposit_event(RawEvent::WorkerApplicationWithdrawn(worker_application_id));
@@ -701,7 +658,9 @@ decl_module! {
                 )
             )?;
 
-            // mutation
+            //
+            // == MUTATION SAFE ==
+            //
 
             // Trigger event
             Self::deposit_event(RawEvent::WorkerApplicationTerminated(worker_application_id));
@@ -719,14 +678,16 @@ decl_module! {
             // this working group, not something else.
             let (worker_opening, _opening) = Self::ensure_worker_opening_exists(&worker_opening_id)?;
 
+            //
+            // == MUTATION SAFE ==
+            //
+
             // Attempt to begin review of applications
             // NB: Combined ensure check and mutation in hiring module
             ensure_on_wrapped_error!(
                 hiring::Module::<T>::begin_review(worker_opening.opening_id)
                 )?;
 
-            // mutation
-
             // Trigger event
             Self::deposit_event(RawEvent::BeganWorkerApplicationReview(worker_opening_id));
         }
@@ -798,7 +759,9 @@ decl_module! {
                 None
             };
 
-            // mutation
+            //
+            // == MUTATION SAFE ==
+            //
 
             let mut worker_application_id_to_worker_id = BTreeMap::new();
 
@@ -854,10 +817,10 @@ decl_module! {
 
                 // Construct worker
                 let worker = Worker::new(
-                    &(successful_worker_application.role_account),
-                   &reward_relationship,
-                   &stake_profile,
-                   &WorkerRoleStage::Active,
+                    &successful_worker_application.member_id,
+                    &successful_worker_application.role_account,
+                    &reward_relationship,
+                    &stake_profile,
                 );
 
                 // Store worker
@@ -909,6 +872,16 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
         Ok(())
     }
 
+    fn ensure_lead_is_set() -> Result<Lead<MemberId<T>, T::AccountId>, Error> {
+        let lead = <CurrentLead<T, I>>::get();
+
+        if let Some(lead) = lead {
+            Ok(lead)
+        } else {
+            Err(Error::CurrentLeadNotSet)
+        }
+    }
+
     fn ensure_opening_human_readable_text_is_valid(text: &[u8]) -> Result<(), Error> {
         <OpeningHumanReadableText<I>>::get()
             .ensure_valid(
@@ -1049,7 +1022,7 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
         Ok((worker_application, *worker_application_id, worker_opening))
     }
 
-    fn ensure_active_worker_signed(
+    fn ensure_worker_signed(
         origin: T::Origin,
         worker_id: &WorkerId<T>,
     ) -> Result<WorkerOf<T>, Error> {
@@ -1057,7 +1030,7 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
         let signer_account = ensure_signed(origin)?;
 
         // Ensure that id corresponds to active worker
-        let worker = Self::ensure_active_worker_exists(&worker_id)?;
+        let worker = Self::ensure_worker_exists(&worker_id)?;
 
         // Ensure that signer is actually role account of worker
         ensure!(
@@ -1068,22 +1041,6 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
         Ok(worker)
     }
 
-    fn ensure_active_worker_exists(worker_id: &WorkerId<T>) -> Result<WorkerOf<T>, Error> {
-        // Ensuring worker actually exists
-        let worker = Self::ensure_worker_exists(worker_id)?;
-
-        // Ensure worker is still active
-        ensure!(
-            match worker.stage {
-                WorkerRoleStage::Active => true,
-                _ => false,
-            },
-            Error::WorkerIsNotActive
-        );
-
-        Ok(worker)
-    }
-
     fn ensure_worker_exists(worker_id: &WorkerId<T>) -> Result<WorkerOf<T>, Error> {
         ensure!(
             WorkerById::<T, I>::exists(worker_id),
@@ -1119,61 +1076,33 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
                 .map_err(|_| Error::RelationshipMustExist)?;
         }; // else: Did not deactivate, there was no reward relationship!
 
-        // When the worker is staked, unstaking must first be initiated,
-        // otherwise they can be terminated right away.
-
-        // Create exit summary for this termination
-        let current_block = <system::Module<T>>::block_number();
-        let worker_exit_summary =
-            WorkerExitSummary::new(exit_initiation_origin, &current_block, rationale_text);
-
-        // Determine new worker stage and event to emit
-        let (new_worker_stage, unstake_directions, event) =
-            if let Some(ref stake_profile) = worker.role_stake_profile {
-                // Determine unstaknig period based on who initiated deactivation
-                let unstaking_period = match worker_exit_summary.origin {
-                    WorkerExitInitiationOrigin::Lead => stake_profile.termination_unstaking_period,
-                    WorkerExitInitiationOrigin::Worker => stake_profile.exit_unstaking_period,
-                };
-
-                (
-                    WorkerRoleStage::Unstaking(worker_exit_summary),
-                    Some((stake_profile.stake_id, unstaking_period)),
-                    RawEvent::WorkerUnstaking(*worker_id),
-                )
-            } else {
-                (
-                    WorkerRoleStage::Exited(worker_exit_summary.clone()),
-                    None,
-                    match worker_exit_summary.origin {
-                        WorkerExitInitiationOrigin::Lead => RawEvent::TerminatedWorker(*worker_id),
-                        WorkerExitInitiationOrigin::Worker => RawEvent::WorkerExited(*worker_id),
-                    },
-                )
+        // Unstake if stake profile exists
+        if let Some(ref stake_profile) = worker.role_stake_profile {
+            // Determine unstaknig period based on who initiated deactivation
+            let unstaking_period = match exit_initiation_origin {
+                WorkerExitInitiationOrigin::Lead => stake_profile.termination_unstaking_period,
+                WorkerExitInitiationOrigin::Worker => stake_profile.exit_unstaking_period,
             };
 
-        // Update worker
-        let new_worker = Worker {
-            stage: new_worker_stage,
-            ..worker.clone()
-        };
-
-        WorkerById::<T, I>::insert(worker_id, new_worker);
-
-        // Unstake if directions provided
-        if let Some(directions) = unstake_directions {
-            // Keep track of worker unstaking
-            let unstaker = WorkingGroupUnstaker::Worker(*worker_id);
-            UnstakerByStakeId::<T, I>::insert(directions.0, unstaker);
-
             // Unstake
             ensure_on_wrapped_error!(stake::Module::<T>::initiate_unstaking(
-                &directions.0,
-                directions.1
+                &stake_profile.stake_id,
+                unstaking_period
             ))?;
         }
 
-        // Trigger event
+        WorkerById::<T, I>::remove(worker_id);
+
+        // Trigger the event
+        let event = match exit_initiation_origin {
+            WorkerExitInitiationOrigin::Lead => {
+                RawEvent::TerminatedWorker(*worker_id, rationale_text.to_vec())
+            }
+            WorkerExitInitiationOrigin::Worker => {
+                RawEvent::WorkerExited(*worker_id, rationale_text.to_vec())
+            }
+        };
+
         Self::deposit_event(event);
 
         Ok(())
@@ -1188,17 +1117,4 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
             )
             .map_err(|e| e.into())
     }
-
-    fn ensure_unstaker_exists(
-        stake_id: &StakeId<T>,
-    ) -> Result<WorkingGroupUnstaker<MemberId<T>, WorkerId<T>>, Error> {
-        ensure!(
-            UnstakerByStakeId::<T, I>::exists(stake_id),
-            Error::UnstakerDoesNotExist
-        );
-
-        let unstaker = UnstakerByStakeId::<T, I>::get(stake_id);
-
-        Ok(unstaker)
-    }
 }

runtime-modules/bureaucracy/src/tests/mod.rs

@@ -2,9 +2,8 @@ mod mock;
 
 use crate::tests::mock::Test;
 use crate::types::{
-    Lead, OpeningPolicyCommitment, RewardPolicy, Worker, WorkerApplication,
-    WorkerExitInitiationOrigin, WorkerExitSummary, WorkerOpening, WorkerRoleStage,
-    WorkerRoleStakeProfile, WorkingGroupUnstaker,
+    Lead, OpeningPolicyCommitment, RewardPolicy, Worker, WorkerApplication, WorkerOpening,
+    WorkerRoleStakeProfile,
 };
 use crate::Error;
 use crate::{Instance1, RawEvent};
@@ -14,51 +13,6 @@ use srml_support::{StorageLinkedMap, StorageValue};
 use std::collections::{BTreeMap, BTreeSet};
 use system::{EventRecord, Phase, RawOrigin};
 
-struct UnstakeFixture {
-    origin: RawOrigin<u64>,
-    worker_id: u64,
-    stake_id: u64,
-}
-
-impl UnstakeFixture {
-    fn default() -> Self {
-        Self::default_for_worker_id(0)
-    }
-    fn default_for_worker_id(worker_id: u64) -> Self {
-        UnstakeFixture {
-            origin: RawOrigin::Root,
-            worker_id,
-            stake_id: 0,
-        }
-    }
-    fn with_origin(self, origin: RawOrigin<u64>) -> Self {
-        UnstakeFixture { origin, ..self }
-    }
-
-    fn call_and_assert(&self, expected_result: Result<(), Error>) {
-        let stake_existed =
-            <crate::UnstakerByStakeId<Test, crate::Instance1>>::exists(self.stake_id);
-
-        let actual_result = Bureaucracy1::unstake(self.origin.clone().into(), self.stake_id);
-        assert_eq!(actual_result, expected_result);
-
-        if stake_existed && actual_result.is_ok() {
-            assert!(!<crate::UnstakerByStakeId<Test, crate::Instance1>>::exists(
-                self.stake_id
-            ));
-
-            let worker = Bureaucracy1::worker_by_id(self.worker_id);
-            let expected_worker_stage = WorkerRoleStage::Exited(WorkerExitSummary {
-                origin: WorkerExitInitiationOrigin::Worker,
-                initiated_at_block_number: 1,
-                rationale_text: b"rationale_text".to_vec(),
-            });
-
-            assert_eq!(worker.stage, expected_worker_stage);
-        }
-    }
-}
-
 struct TerminateWorkerRoleFixture {
     worker_id: u64,
     origin: RawOrigin<u64>,
@@ -87,17 +41,8 @@ impl TerminateWorkerRoleFixture {
     }
 
     fn call_and_assert(&self, expected_result: Result<(), Error>) {
-        self.call_and_assert_impl(expected_result, false);
-    }
-
-    fn call_and_assert_with_unstaking(&self, expected_result: Result<(), Error>) {
-        self.call_and_assert_impl(expected_result, true);
-    }
-
-    fn call_and_assert_impl(&self, expected_result: Result<(), Error>, expect_unstaking: bool) {
         <crate::WorkerExitRationaleText<Instance1>>::put(self.constraint.clone());
 
-        let rationale_text = b"rationale_text".to_vec();
         let actual_result = Bureaucracy1::terminate_worker_role(
             self.origin.clone().into(),
             self.worker_id,
@@ -106,30 +51,9 @@ impl TerminateWorkerRoleFixture {
         assert_eq!(actual_result, expected_result);
 
         if actual_result.is_ok() {
-            let worker = Bureaucracy1::worker_by_id(self.worker_id);
-
-            let worker_exit_summary = WorkerExitSummary {
-                origin: WorkerExitInitiationOrigin::Lead,
-                initiated_at_block_number: 1,
-                rationale_text,
-            };
-
-            let expected_worker_stage = if expect_unstaking {
-                WorkerRoleStage::Unstaking(worker_exit_summary)
-            } else {
-                WorkerRoleStage::Exited(worker_exit_summary)
-            };
-
-            assert_eq!(worker.stage, expected_worker_stage);
-
-            let stake_id = 0;
-            if expect_unstaking {
-                assert!(<crate::UnstakerByStakeId<Test, crate::Instance1>>::exists(
-                    stake_id
-                ));
-            } else {
-                assert!(!<crate::UnstakerByStakeId<Test, crate::Instance1>>::exists(
-                    stake_id
+            if actual_result.is_ok() {
+                assert!(!<crate::WorkerById<Test, crate::Instance1>>::exists(
+                    self.worker_id
                 ));
             }
         }
@@ -153,14 +77,6 @@ impl LeaveWorkerRoleFixture {
     }
 
     fn call_and_assert(&self, expected_result: Result<(), Error>) {
-        self.call_and_assert_impl(expected_result, false);
-    }
-
-    fn call_and_assert_with_unstaking(&self, expected_result: Result<(), Error>) {
-        self.call_and_assert_impl(expected_result, true);
-    }
-
-    fn call_and_assert_impl(&self, expected_result: Result<(), Error>, expect_unstaking: bool) {
         let rationale_text = b"rationale_text".to_vec();
         let actual_result = Bureaucracy1::leave_worker_role(
             self.origin.clone().into(),
@@ -170,32 +86,9 @@ impl LeaveWorkerRoleFixture {
         assert_eq!(actual_result, expected_result);
 
         if actual_result.is_ok() {
-            let worker = Bureaucracy1::worker_by_id(self.worker_id);
-
-            let worker_exit_summary = WorkerExitSummary {
-                origin: WorkerExitInitiationOrigin::Worker,
-                initiated_at_block_number: 1,
-                rationale_text,
-            };
-
-            let expected_worker_stage = if expect_unstaking {
-                WorkerRoleStage::Unstaking(worker_exit_summary)
-            } else {
-                WorkerRoleStage::Exited(worker_exit_summary)
-            };
-
-            assert_eq!(worker.stage, expected_worker_stage);
-
-            let stake_id = 0;
-            if expect_unstaking {
-                assert!(<crate::UnstakerByStakeId<Test, crate::Instance1>>::exists(
-                    stake_id
-                ));
-            } else {
-                assert!(!<crate::UnstakerByStakeId<Test, crate::Instance1>>::exists(
-                    stake_id
-                ));
-            }
+            assert!(!<crate::WorkerById<Test, crate::Instance1>>::exists(
+                self.worker_id
+            ));
         }
     }
 }
@@ -231,16 +124,14 @@ impl UpdateWorkerRewardAccountFixture {
 }
 
 struct UpdateWorkerRoleAccountFixture {
-    member_id: u64,
     worker_id: u64,
     new_role_account_id: u64,
     origin: RawOrigin<u64>,
 }
 
 impl UpdateWorkerRoleAccountFixture {
-    fn default_with_ids(member_id: u64, worker_id: u64, new_role_account_id: u64) -> Self {
+    fn default_with_ids(worker_id: u64, new_role_account_id: u64) -> Self {
         UpdateWorkerRoleAccountFixture {
-            member_id,
             worker_id,
             new_role_account_id,
             origin: RawOrigin::Signed(1),
@@ -253,7 +144,6 @@ impl UpdateWorkerRoleAccountFixture {
     fn call_and_assert(&self, expected_result: Result<(), Error>) {
         let actual_result = Bureaucracy1::update_worker_role_account(
             self.origin.clone().into(),
-            self.member_id,
             self.worker_id,
             self.new_role_account_id,
         );
@@ -292,6 +182,7 @@ struct FillWorkerOpeningFixture {
     successful_worker_application_ids: BTreeSet<u64>,
     role_account: u64,
     reward_policy: Option<RewardPolicy<u64, u64>>,
+    member_id: u64,
 }
 
 impl FillWorkerOpeningFixture {
@@ -304,6 +195,7 @@ impl FillWorkerOpeningFixture {
             successful_worker_application_ids: application_ids,
             role_account: 1,
             reward_policy: None,
+            member_id: 1,
         }
     }
 
@@ -359,7 +251,7 @@ impl FillWorkerOpeningFixture {
                 role_account: self.role_account,
                 reward_relationship,
                 role_stake_profile,
-                stage: WorkerRoleStage::Active,
+                member_id: self.member_id,
             };
 
             let actual_worker = Bureaucracy1::worker_by_id(worker_id);
@@ -684,6 +576,7 @@ impl EventFixture {
                 u64,
                 u64,
                 std::collections::BTreeMap<u64, u64>,
+                Vec<u8>,
                 crate::Instance1,
             >,
         >,
@@ -717,6 +610,7 @@ impl EventFixture {
             u64,
             u64,
             std::collections::BTreeMap<u64, u64>,
+            Vec<u8>,
             crate::Instance1,
         >,
     ) {
@@ -1690,11 +1584,10 @@ fn set_lead_fails_with_invalid_origin() {
 fn update_worker_role_account_succeeds() {
     build_test_externalities().execute_with(|| {
         let new_account_id = 10;
-        let member_id = 1;
         let worker_id = fill_default_worker_position();
 
         let update_worker_account_fixture =
-            UpdateWorkerRoleAccountFixture::default_with_ids(member_id, worker_id, new_account_id);
+            UpdateWorkerRoleAccountFixture::default_with_ids(worker_id, new_account_id);
 
         update_worker_account_fixture.call_and_assert(Ok(()));
 
@@ -1705,21 +1598,13 @@ fn update_worker_role_account_succeeds() {
     });
 }
 
-#[test]
-fn update_worker_role_account_fails_with_membership_error() {
-    build_test_externalities().execute_with(|| {
-        let update_worker_account_fixture =
-            UpdateWorkerRoleAccountFixture::default_with_ids(1, 1, 1);
-
-        update_worker_account_fixture.call_and_assert(Err(Error::MembershipInvalidMemberId));
-    });
-}
-
 #[test]
 fn update_worker_role_account_fails_with_invalid_origin() {
     build_test_externalities().execute_with(|| {
+        let worker_id = fill_default_worker_position();
         let update_worker_account_fixture =
-            UpdateWorkerRoleAccountFixture::default_with_ids(1, 1, 1).with_origin(RawOrigin::None);
+            UpdateWorkerRoleAccountFixture::default_with_ids(worker_id, 1)
+                .with_origin(RawOrigin::None);
 
         update_worker_account_fixture.call_and_assert(Err(Error::MembershipUnsignedOrigin));
     });
@@ -1865,28 +1750,6 @@ fn update_worker_reward_account_fails_with_invalid_worker_id() {
     });
 }
 
-#[test]
-fn update_worker_reward_account_fails_with_inactive_worker() {
-    build_test_externalities().execute_with(|| {
-        let lead_account_id = 1;
-        let worker_id = fill_default_worker_position();
-
-        let mut worker = Bureaucracy1::worker_by_id(worker_id);
-        worker.stage = WorkerRoleStage::Exited(WorkerExitSummary {
-            origin: WorkerExitInitiationOrigin::Lead,
-            initiated_at_block_number: 333,
-            rationale_text: Vec::new(),
-        });
-
-        <crate::WorkerById<Test, crate::Instance1>>::insert(worker_id, worker);
-
-        let update_worker_account_fixture =
-            UpdateWorkerRewardAccountFixture::default_with_ids(worker_id, lead_account_id);
-
-        update_worker_account_fixture.call_and_assert(Err(Error::WorkerIsNotActive));
-    });
-}
-
 #[test]
 fn update_worker_reward_account_fails_with_no_recurring_reward() {
     build_test_externalities().execute_with(|| {
@@ -1909,7 +1772,10 @@ fn leave_worker_role_succeeds() {
 
         leave_worker_role_fixture.call_and_assert(Ok(()));
 
-        EventFixture::assert_last_crate_event(RawEvent::WorkerExited(worker_id));
+        EventFixture::assert_last_crate_event(RawEvent::WorkerExited(
+            worker_id,
+            b"rationale_text".to_vec(),
+        ));
     });
 }
 
@@ -1948,26 +1814,6 @@ fn leave_worker_role_fails_with_invalid_worker_id() {
     });
 }
 
-#[test]
-fn leave_worker_role_fails_with_inactive_worker() {
-    build_test_externalities().execute_with(|| {
-        let worker_id = fill_default_worker_position();
-
-        let mut worker = Bureaucracy1::worker_by_id(worker_id);
-        worker.stage = WorkerRoleStage::Exited(WorkerExitSummary {
-            origin: WorkerExitInitiationOrigin::Lead,
-            initiated_at_block_number: 333,
-            rationale_text: Vec::new(),
-        });
-
-        <crate::WorkerById<Test, crate::Instance1>>::insert(worker_id, worker);
-
-        let leave_worker_role_fixture = LeaveWorkerRoleFixture::default_for_worker_id(worker_id);
-
-        leave_worker_role_fixture.call_and_assert(Err(Error::WorkerIsNotActive));
-    });
-}
-
 #[test]
 fn leave_worker_role_fails_with_invalid_recurring_reward_relationships() {
     build_test_externalities().execute_with(|| {
@@ -1991,9 +1837,12 @@ fn leave_worker_role_succeeds_with_stakes() {
 
         let leave_worker_role_fixture = LeaveWorkerRoleFixture::default_for_worker_id(worker_id);
 
-        leave_worker_role_fixture.call_and_assert_with_unstaking(Ok(()));
+        leave_worker_role_fixture.call_and_assert(Ok(()));
 
-        EventFixture::assert_last_crate_event(RawEvent::WorkerUnstaking(worker_id));
+        EventFixture::assert_last_crate_event(RawEvent::WorkerExited(
+            worker_id,
+            b"rationale_text".to_vec(),
+        ));
     });
 }
 
@@ -2005,9 +1854,12 @@ fn terminate_worker_role_succeeds_with_stakes() {
         let terminate_worker_role_fixture =
             TerminateWorkerRoleFixture::default_for_worker_id(worker_id);
 
-        terminate_worker_role_fixture.call_and_assert_with_unstaking(Ok(()));
+        terminate_worker_role_fixture.call_and_assert(Ok(()));
 
-        EventFixture::assert_last_crate_event(RawEvent::WorkerUnstaking(worker_id));
+        EventFixture::assert_last_crate_event(RawEvent::TerminatedWorker(
+            worker_id,
+            b"rationale_text".to_vec(),
+        ));
     });
 }
 
@@ -2021,7 +1873,10 @@ fn terminate_worker_role_succeeds() {
 
         terminate_worker_role_fixture.call_and_assert(Ok(()));
 
-        EventFixture::assert_last_crate_event(RawEvent::TerminatedWorker(worker_id));
+        EventFixture::assert_last_crate_event(RawEvent::TerminatedWorker(
+            worker_id,
+            b"rationale_text".to_vec(),
+        ));
     });
 }
 
@@ -2066,72 +1921,3 @@ fn terminate_worker_role_fails_with_invalid_origin() {
         terminate_worker_role_fixture.call_and_assert(Err(Error::Other("RequireSignedOrigin")));
     });
 }
-
-#[test]
-fn unstake_fails_with_invalid_origin() {
-    build_test_externalities().execute_with(|| {
-        let unstake_fixture = UnstakeFixture::default().with_origin(RawOrigin::None);
-
-        unstake_fixture.call_and_assert(Err(Error::Other("RequireRootOrigin")));
-    });
-}
-
-#[test]
-fn unstake_succeeds_with_invalid_stake_id() {
-    build_test_externalities().execute_with(|| {
-        let unstake_fixture = UnstakeFixture::default();
-
-        unstake_fixture.call_and_assert(Ok(()));
-    });
-}
-
-#[test]
-fn unstake_succeeds() {
-    build_test_externalities().execute_with(|| {
-        let worker_id = fill_worker_position_with_stake(100);
-
-        let leave_worker_role_fixture = LeaveWorkerRoleFixture::default_for_worker_id(worker_id);
-        leave_worker_role_fixture.call_and_assert_with_unstaking(Ok(()));
-
-        let unstake_fixture = UnstakeFixture::default();
-        unstake_fixture.call_and_assert(Ok(()));
-
-        EventFixture::assert_last_crate_event(RawEvent::WorkerExited(worker_id));
-    });
-}
-
-#[test]
-fn unstake_fails_with_invalid_unstaker() {
-    build_test_externalities().execute_with(|| {
-        let worker_id = fill_worker_position_with_stake(100);
-
-        let leave_worker_role_fixture = LeaveWorkerRoleFixture::default_for_worker_id(worker_id);
-        leave_worker_role_fixture.call_and_assert_with_unstaking(Ok(()));
-
-        let stake_id = 0;
-        <crate::UnstakerByStakeId<Test, crate::Instance1>>::insert(
-            stake_id,
-            WorkingGroupUnstaker::Lead(1),
-        );
-
-        let unstake_fixture = UnstakeFixture::default();
-        unstake_fixture.call_and_assert(Err(Error::OnlyWorkersCanUnstake));
-    });
-}
-
-#[test]
-fn unstake_fails_with_non_unstaking_worker() {
-    build_test_externalities().execute_with(|| {
-        let worker_id = fill_worker_position_with_stake(100);
-
-        let leave_worker_role_fixture = LeaveWorkerRoleFixture::default_for_worker_id(worker_id);
-        leave_worker_role_fixture.call_and_assert_with_unstaking(Ok(()));
-
-        let mut worker = Bureaucracy1::worker_by_id(worker_id);
-        worker.stage = WorkerRoleStage::Active;
-        <crate::WorkerById<Test, crate::Instance1>>::insert(worker_id, worker);
-
-        let unstake_fixture = UnstakeFixture::default();
-        unstake_fixture.call_and_assert(Err(Error::WorkerIsNotUnstaking));
-    });
-}

runtime-modules/bureaucracy/src/types.rs

@@ -1,9 +1,7 @@
 #![warn(missing_docs)]
 
 use codec::{Decode, Encode};
-use rstd::borrow::ToOwned;
 use rstd::collections::btree_set::BTreeSet;
-use rstd::vec::Vec;
 
 #[cfg(feature = "std")]
 use serde::{Deserialize, Serialize};
@@ -134,10 +132,10 @@ impl<AccountId: Clone, WorkerOpeningId: Clone, MemberId: Clone, ApplicationId: C
         application_id: &ApplicationId,
     ) -> Self {
         WorkerApplication {
-            role_account: (*role_account).clone(),
-            worker_opening_id: (*worker_opening_id).clone(),
-            member_id: (*member_id).clone(),
-            application_id: (*application_id).clone(),
+            role_account: role_account.clone(),
+            worker_opening_id: worker_opening_id.clone(),
+            member_id: member_id.clone(),
+            application_id: application_id.clone(),
         }
     }
 }
@@ -164,9 +162,9 @@ impl<StakeId: Clone, BlockNumber: Clone> WorkerRoleStakeProfile<StakeId, BlockNu
         exit_unstaking_period: &Option<BlockNumber>,
     ) -> Self {
         Self {
-            stake_id: (*stake_id).clone(),
-            termination_unstaking_period: (*termination_unstaking_period).clone(),
-            exit_unstaking_period: (*exit_unstaking_period).clone(),
+            stake_id: stake_id.clone(),
+            termination_unstaking_period: termination_unstaking_period.clone(),
+            exit_unstaking_period: exit_unstaking_period.clone(),
         }
     }
 }
@@ -175,82 +173,37 @@ impl<StakeId: Clone, BlockNumber: Clone> WorkerRoleStakeProfile<StakeId, BlockNu
 /// This role can be staked, have reward and be inducted through the hiring module.
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
 #[derive(Encode, Decode, Default, Debug, Clone, PartialEq)]
-pub struct Worker<AccountId, RewardRelationshipId, StakeId, BlockNumber> {
+pub struct Worker<AccountId, RewardRelationshipId, StakeId, BlockNumber, MemberId> {
+    /// Member id related to the worker
+    pub member_id: MemberId,
     /// Account used to authenticate in this role.
     pub role_account: AccountId,
     /// Whether the role has recurring reward, and if so an identifier for this.
     pub reward_relationship: Option<RewardRelationshipId>,
     /// When set, describes role stake of worker.
     pub role_stake_profile: Option<WorkerRoleStakeProfile<StakeId, BlockNumber>>,
-    /// The stage of this worker in the working group.
-    pub stage: WorkerRoleStage<BlockNumber>,
 }
 
-impl<AccountId: Clone, RewardRelationshipId: Clone, StakeId: Clone, BlockNumber: Clone>
-    Worker<AccountId, RewardRelationshipId, StakeId, BlockNumber>
+impl<
+        AccountId: Clone,
+        RewardRelationshipId: Clone,
+        StakeId: Clone,
+        BlockNumber: Clone,
+        MemberId: Clone,
+    > Worker<AccountId, RewardRelationshipId, StakeId, BlockNumber, MemberId>
 {
     /// Creates a new _Worker_ using parameters.
     pub fn new(
+        member_id: &MemberId,
         role_account: &AccountId,
         reward_relationship: &Option<RewardRelationshipId>,
         role_stake_profile: &Option<WorkerRoleStakeProfile<StakeId, BlockNumber>>,
-        stage: &WorkerRoleStage<BlockNumber>,
     ) -> Self {
         Worker {
-            role_account: (*role_account).clone(),
-            reward_relationship: (*reward_relationship).clone(),
-            role_stake_profile: (*role_stake_profile).clone(),
-            stage: (*stage).clone(),
-        }
-    }
-}
-
-/// The stage of the involvement of a curator in the working group.
-#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
-#[derive(Encode, Decode, Debug, Clone, PartialEq)]
-pub enum WorkerRoleStage<BlockNumber> {
-    /// Currently active.
-    Active,
-
-    /// Currently unstaking.
-    Unstaking(WorkerExitSummary<BlockNumber>),
-
-    /// No longer active and unstaked.
-    Exited(WorkerExitSummary<BlockNumber>),
-}
-
-/// Must be default constructible because it indirectly is a value in a storage map.
-/// ***SHOULD NEVER ACTUALLY GET CALLED, IS REQUIRED TO DUE BAD STORAGE MODEL IN SUBSTRATE***
-impl<BlockNumber> Default for WorkerRoleStage<BlockNumber> {
-    fn default() -> Self {
-        WorkerRoleStage::Active
-    }
-}
-
-/// The exit stage of a curators involvement in the working group.
-#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
-#[derive(Encode, Decode, Debug, Clone, PartialEq)]
-pub struct WorkerExitSummary<BlockNumber> {
-    /// Origin for exit.
-    pub origin: WorkerExitInitiationOrigin,
-
-    /// When exit was initiated.
-    pub initiated_at_block_number: BlockNumber,
-
-    /// Explainer for why exit was initiated.
-    pub rationale_text: Vec<u8>,
-}
-
-impl<BlockNumber: Clone> WorkerExitSummary<BlockNumber> {
-    pub fn new(
-        origin: &WorkerExitInitiationOrigin,
-        initiated_at_block_number: &BlockNumber,
-        rationale_text: &[u8],
-    ) -> Self {
-        WorkerExitSummary {
-            origin: (*origin).clone(),
-            initiated_at_block_number: (*initiated_at_block_number).clone(),
-            rationale_text: rationale_text.to_owned(),
+            member_id: member_id.clone(),
+            role_account: role_account.clone(),
+            reward_relationship: reward_relationship.clone(),
+            role_stake_profile: role_stake_profile.clone(),
         }
     }
 }
@@ -278,22 +231,3 @@ pub struct RewardPolicy<Balance, BlockNumber> {
     /// Optional payout interval.
     pub payout_interval: Option<BlockNumber>,
 }
-
-/// Represents a possible unstaker in working group.
-#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
-#[derive(Encode, Decode, Debug, Eq, PartialEq, Clone, PartialOrd)]
-pub enum WorkingGroupUnstaker<MemberId, WorkerId> {
-    /// Lead unstaker.
-    Lead(MemberId),
-
-    /// Worker unstaker.
-    Worker(WorkerId),
-}
-
-/// Must be default constructable because it indirectly is a value in a storage map.
-/// ***SHOULD NEVER ACTUALLY GET CALLED, IS REQUIRED TO DUE BAD STORAGE MODEL IN SUBSTRATE***
-impl<MemberId: Default, WorkerId> Default for WorkingGroupUnstaker<MemberId, WorkerId> {
-    fn default() -> Self {
-        Self::Lead(MemberId::default())
-    }
-}