Merge pull request #4 from mnaamani/fill-opening-bug-fix

Fix use of mint in working groups

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

@@ -1466,6 +1466,23 @@ decl_module! {
             // Ensure curator opening exists
             let (curator_opening, _) = Self::ensure_curator_opening_exists(&curator_opening_id)?;
 
+            // Ensure a mint exists if lead is providing a reward for positions being filled
+            let create_reward_settings = if let Some(policy) = reward_policy {
+                // A reward will need to be created so ensure our configured mint exists
+                let mint_id = Self::mint();
+
+                // Technically this is a bug-check and should not be here.
+                ensure!(<minting::Mints<T>>::exists(mint_id), MSG_FILL_CURATOR_OPENING_MINT_DOES_NOT_EXIST);
+
+                // Make sure valid parameters are selected for next payment at block number
+                ensure!(policy.next_payment_at_block > <system::Module<T>>::block_number(), MSG_FILL_CURATOR_OPENING_INVALID_NEXT_PAYMENT_BLOCK);
+
+                // The verified reward settings to use
+                Some((mint_id, policy))
+            } else {
+                None
+            };
+
             // Make iterator over successful curator application
             let successful_iter = successful_curator_application_ids
                                     .iter()
@@ -1516,21 +1533,6 @@ decl_module! {
                 )
             )?;
 
-            let create_reward_settings = if let Some(policy) = reward_policy {
-                // A reward will need to be created so ensure our configured mint exists
-                let mint_id = Self::mint();
-
-                ensure!(<minting::Mints<T>>::exists(mint_id), MSG_FILL_CURATOR_OPENING_MINT_DOES_NOT_EXIST);
-
-                // Make sure valid parameters are selected for next payment at block number
-                ensure!(policy.next_payment_at_block > <system::Module<T>>::block_number(), MSG_FILL_CURATOR_OPENING_INVALID_NEXT_PAYMENT_BLOCK);
-
-                // The verified reward settings to use
-                Some((mint_id, policy))
-            } else {
-                None
-            };
-
             //
             // == MUTATION SAFE ==
             //
@@ -1979,6 +1981,8 @@ decl_module! {
         ) {
             ensure_root(origin)?;
 
+            ensure!(<Mint<T>>::exists(), MSG_FILL_CURATOR_OPENING_MINT_DOES_NOT_EXIST);
+
             let mint_id = Self::mint();
 
             // Mint must exist - it is set at genesis

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

@@ -230,9 +230,6 @@ decl_error! {
         /// Slash amount should be greater than zero.
         StakingErrorSlashAmountShouldBeGreaterThanZero,
 
-        /// Working group mint is not set.
-        WorkingGroupMintIsNotSet,
-
         /// Cannot find mint in the minting module.
         CannotFindMint,
 

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

@@ -745,6 +745,24 @@ decl_module! {
             // Ensure worker opening exists
             let (worker_opening, _) = Self::ensure_worker_opening_exists(&worker_opening_id)?;
 
+            // Ensure a mint exists if lead is providing a reward for positions being filled
+            let create_reward_settings = if let Some(policy) = reward_policy {
+                // A reward will need to be created so ensure our configured mint exists
+                let mint_id = Self::mint();
+
+                // Technically this is a bug-check and should not be here.
+                ensure!(<minting::Mints<T>>::exists(mint_id), Error::FillWorkerOpeningMintDoesNotExist);
+
+                // Make sure valid parameters are selected for next payment at block number
+                ensure!(policy.next_payment_at_block > <system::Module<T>>::block_number(),
+                    Error::FillWorkerOpeningInvalidNextPaymentBlock);
+
+                // The verified reward settings to use
+                Some((mint_id, policy))
+            } else {
+                None
+            };
+
             // Make iterator over successful worker application
             let successful_iter = successful_worker_application_ids
                                     .iter()
@@ -783,22 +801,6 @@ decl_module! {
                 )
             )?;
 
-            let create_reward_settings = if let Some(policy) = reward_policy {
-                // A reward will need to be created so ensure our configured mint exists
-                let mint_id = Self::mint();
-
-                ensure!(<minting::Mints<T>>::exists(mint_id), Error::FillWorkerOpeningMintDoesNotExist);
-
-                // Make sure valid parameters are selected for next payment at block number
-                ensure!(policy.next_payment_at_block > <system::Module<T>>::block_number(),
-                    Error::FillWorkerOpeningInvalidNextPaymentBlock);
-
-                // The verified reward settings to use
-                Some((mint_id, policy))
-            } else {
-                None
-            };
-
             //
             // == MUTATION SAFE ==
             //
@@ -965,10 +967,9 @@ decl_module! {
         ) {
             ensure_root(origin)?;
 
-            ensure!(<Mint<T, I>>::exists(), Error::WorkingGroupMintIsNotSet);
-
             let mint_id = Self::mint();
 
+            // Technically this is a bug-check and should not be here.
             ensure!(<minting::Mints<T>>::exists(mint_id), Error::CannotFindMint);
 
             // Mint must exist - it is set at genesis or migration.

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

@@ -219,13 +219,6 @@ impl UnsetLeadFixture {
     }
 }
 
-pub fn remove_mint() {
-    let mint_id = <crate::Mint<Test, TestWorkingGroupInstance>>::get();
-    <crate::Mint<Test, TestWorkingGroupInstance>>::kill();
-
-    <minting::Module<Test>>::remove_mint(mint_id);
-}
-
 pub fn set_mint_id(mint_id: u64) {
     <crate::Mint<Test, TestWorkingGroupInstance>>::put(mint_id);
 }

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

@@ -916,24 +916,12 @@ fn fill_worker_opening_fails_with_invalid_reward_policy() {
             FillWorkerOpeningFixture::default_for_ids(opening_id, vec![application_id])
                 .with_reward_policy(RewardPolicy {
                     amount_per_payout: 10000,
-                    next_payment_at_block: 100,
-                    payout_interval: None,
-                });
-
-        remove_mint(); //removes default mintx
-        fill_worker_opening_fixture.call_and_assert(Err(Error::FillWorkerOpeningMintDoesNotExist));
-
-        set_mint_id(22);
-
-        let fill_worker_opening_fixture =
-            FillWorkerOpeningFixture::default_for_ids(opening_id, vec![application_id])
-                .with_reward_policy(RewardPolicy {
-                    amount_per_payout: 10000,
+                    // Invalid next payment at block zero
                     next_payment_at_block: 0,
                     payout_interval: None,
                 });
         fill_worker_opening_fixture
-            .call_and_assert(Err(Error::FullWorkerOpeningOpeningNotInReviewPeriodStage));
+            .call_and_assert(Err(Error::FillWorkerOpeningInvalidNextPaymentBlock));
     });
 }
 
@@ -1588,18 +1576,6 @@ fn set_working_group_mint_capacity_succeeds() {
     });
 }
 
-#[test]
-fn set_working_group_mint_capacity_fails_with_not_set_working_group_mint() {
-    build_test_externalities().execute_with(|| {
-        remove_mint(); //removes default mint
-
-        let capacity = 15000;
-        let result = TestWorkingGroup::set_mint_capacity(RawOrigin::Root.into(), capacity);
-
-        assert_eq!(result, Err(Error::WorkingGroupMintIsNotSet));
-    });
-}
-
 #[test]
 fn set_working_group_mint_capacity_fails_with_mint_not_found() {
     build_test_externalities().execute_with(|| {