Merge pull request #858 from shamil-gadelshin/working_group_unstaking_period_bugfix

Fix ‘add_opening’ zero unstaking periods bug.

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

@@ -247,6 +247,18 @@ decl_error! {
 
         /// Working group size limit exceeded.
         MaxActiveWorkerNumberExceeded,
+
+        /// Invalid OpeningPolicyCommitment parameter:
+        /// fill_opening_failed_applicant_application_stake_unstaking_period should be non-zero.
+        FillOpeningFailedApplicantApplicationStakeUnstakingPeriodIsZero,
+
+        /// Invalid OpeningPolicyCommitment parameter:
+        /// fill_opening_failed_applicant_role_stake_unstaking_period should be non-zero.
+        FillOpeningFailedApplicantRoleStakeUnstakingPeriodIsZero,
+
+        /// Invalid OpeningPolicyCommitment parameter:
+        /// fill_opening_successful_applicant_application_stake_unstaking_period should be non-zero.
+        FillOpeningSuccessfulApplicantApplicationStakeUnstakingPeriodIsZero,
     }
 }
 

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

@@ -507,8 +507,7 @@ decl_module! {
         pub fn add_opening(
             origin,
             activate_at: hiring::ActivateOpeningAt<T::BlockNumber>,
-            commitment: OpeningPolicyCommitment<T::BlockNumber,
-            BalanceOf<T>>,
+            commitment: OpeningPolicyCommitment<T::BlockNumber, BalanceOf<T>>,
             human_readable_text: Vec<u8>,
             opening_type: OpeningType,
         ){
@@ -516,6 +515,8 @@ decl_module! {
 
             Self::ensure_opening_human_readable_text_is_valid(&human_readable_text)?;
 
+            Self::ensure_opening_policy_commitment_is_valid(&commitment)?;
+
 
             // Add opening
             // NB: This call can in principle fail, because the staking policies
@@ -989,6 +990,41 @@ decl_module! {
 // ****************** Ensures **********************
 
 impl<T: Trait<I>, I: Instance> Module<T, I> {
+    fn ensure_opening_policy_commitment_is_valid(
+        policy_commitment: &OpeningPolicyCommitment<T::BlockNumber, BalanceOf<T>>,
+    ) -> Result<(), Error> {
+        // check fill_opening unstaking periods
+
+        if let Some(unstaking_period) =
+            policy_commitment.fill_opening_failed_applicant_application_stake_unstaking_period
+        {
+            ensure!(
+                unstaking_period != Zero::zero(),
+                Error::FillOpeningFailedApplicantApplicationStakeUnstakingPeriodIsZero
+            );
+        }
+
+        if let Some(unstaking_period) =
+            policy_commitment.fill_opening_failed_applicant_role_stake_unstaking_period
+        {
+            ensure!(
+                unstaking_period != Zero::zero(),
+                Error::FillOpeningFailedApplicantRoleStakeUnstakingPeriodIsZero
+            );
+        }
+
+        if let Some(unstaking_period) =
+            policy_commitment.fill_opening_successful_applicant_application_stake_unstaking_period
+        {
+            ensure!(
+                unstaking_period != Zero::zero(),
+                Error::FillOpeningSuccessfulApplicantApplicationStakeUnstakingPeriodIsZero
+            );
+        }
+
+        Ok(())
+    }
+
     fn ensure_origin_for_opening_type(
         origin: T::Origin,
         opening_type: OpeningType,

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

@@ -56,6 +56,40 @@ fn hire_lead_fails_multiple_applications() {
     });
 }
 
+#[test]
+fn add_opening_fails_with_incorrect_unstaking_periods() {
+    build_test_externalities().execute_with(|| {
+        HireLeadFixture::default().hire_lead();
+
+        let add_opening_fixture =
+            AddWorkerOpeningFixture::default().with_policy_commitment(OpeningPolicyCommitment {
+                fill_opening_failed_applicant_role_stake_unstaking_period: Some(0),
+                ..OpeningPolicyCommitment::default()
+            });
+        add_opening_fixture.call_and_assert(Err(
+            Error::FillOpeningFailedApplicantRoleStakeUnstakingPeriodIsZero,
+        ));
+
+        let add_opening_fixture =
+            AddWorkerOpeningFixture::default().with_policy_commitment(OpeningPolicyCommitment {
+                fill_opening_failed_applicant_application_stake_unstaking_period: Some(0),
+                ..OpeningPolicyCommitment::default()
+            });
+        add_opening_fixture.call_and_assert(Err(
+            Error::FillOpeningFailedApplicantApplicationStakeUnstakingPeriodIsZero,
+        ));
+
+        let add_opening_fixture =
+            AddWorkerOpeningFixture::default().with_policy_commitment(OpeningPolicyCommitment {
+                fill_opening_successful_applicant_application_stake_unstaking_period: Some(0),
+                ..OpeningPolicyCommitment::default()
+            });
+        add_opening_fixture.call_and_assert(Err(
+            Error::FillOpeningSuccessfulApplicantApplicationStakeUnstakingPeriodIsZero,
+        ));
+    });
+}
+
 #[test]
 fn add_worker_opening_succeeds() {
     build_test_externalities().execute_with(|| {