Browse Source

Refix bug and restore Currency::minimum_balance() check in the hiring module

Shamil Gadelshin 4 years ago
parent
commit
448f69b50b

+ 3 - 3
Cargo.lock

@@ -4694,7 +4694,7 @@ dependencies = [
 
 [[package]]
 name = "substrate-content-working-group-module"
-version = "1.0.0"
+version = "1.0.1"
 dependencies = [
  "parity-scale-codec",
  "serde",
@@ -4857,7 +4857,7 @@ dependencies = [
 
 [[package]]
 name = "substrate-hiring-module"
-version = "1.0.1"
+version = "1.0.2"
 dependencies = [
  "hex-literal 0.1.4",
  "mockall",
@@ -5567,7 +5567,7 @@ dependencies = [
 
 [[package]]
 name = "substrate-working-group-module"
-version = "1.0.0"
+version = "1.0.1"
 dependencies = [
  "parity-scale-codec",
  "serde",

+ 1 - 1
runtime-modules/content-working-group/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = 'substrate-content-working-group-module'
-version = '1.0.0'
+version = '1.0.1'
 authors = ['Joystream contributors']
 edition = '2018'
 

+ 10 - 0
runtime-modules/content-working-group/src/lib.rs

@@ -239,6 +239,10 @@ pub static MSG_ORIGIN_IS_NIETHER_MEMBER_CONTROLLER_OR_ROOT: &str =
     "Origin must be controller or root account of member";
 pub static MSG_MEMBER_HAS_ACTIVE_APPLICATION_ON_OPENING: &str =
     "Member already has an active application on the opening";
+pub static MSG_ADD_CURATOR_OPENING_ROLE_STAKE_CANNOT_BE_ZERO: &str =
+    "Add curator opening role stake cannot be zero";
+pub static MSG_ADD_CURATOR_OPENING_APPLICATION_STAKE_CANNOT_BE_ZERO: &str =
+    "Add curator opening application stake cannot be zero";
 
 /// The exit stage of a lead involvement in the working group.
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
@@ -849,6 +853,12 @@ impl rstd::convert::From<WrappedError<hiring::AddOpeningError>> for &str {
             hiring::AddOpeningError::ApplicationRationingZeroMaxApplicants => {
                 MSG_ADD_CURATOR_OPENING_ZERO_MAX_APPLICANT_COUNT
             }
+            hiring::AddOpeningError::StakeAmountCannotBeZero(purpose) => match purpose {
+                hiring::StakePurpose::Role => MSG_ADD_CURATOR_OPENING_ROLE_STAKE_CANNOT_BE_ZERO,
+                hiring::StakePurpose::Application => {
+                    MSG_ADD_CURATOR_OPENING_APPLICATION_STAKE_CANNOT_BE_ZERO
+                }
+            },
         }
     }
 }

+ 0 - 4
runtime-modules/content-working-group/src/mock.rs

@@ -162,9 +162,6 @@ impl stake::Trait for Test {
     type SlashId = TestSlashId;
 }
 
-parameter_types! {
-    pub const MinimumStakeBalance: u64 = 1; // just non-zero
-}
 type TestOpeningId = u64;
 type TestApplicationId = u64;
 impl hiring::Trait for Test {
@@ -172,7 +169,6 @@ impl hiring::Trait for Test {
     type ApplicationId = TestApplicationId;
     type ApplicationDeactivatedHandler = ();
     type StakeHandlerProvider = hiring::Module<Self>;
-    type MinimumStakeBalance = MinimumStakeBalance;
 }
 
 impl versioned_store::Trait for Test {

+ 1 - 1
runtime-modules/hiring/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = 'substrate-hiring-module'
-version = '1.0.1'
+version = '1.0.2'
 authors = ['Joystream contributors']
 edition = '2018'
 

+ 3 - 0
runtime-modules/hiring/src/hiring/mod.rs

@@ -154,4 +154,7 @@ pub enum AddOpeningError {
     /// It is not possible to provide application rationing policy with zero
     /// 'max_active_applicants' parameter.
     ApplicationRationingZeroMaxApplicants,
+
+    /// It is not possible to stake zero.
+    StakeAmountCannotBeZero(StakePurpose),
 }

+ 0 - 43
runtime-modules/hiring/src/hiring/opening.rs

@@ -6,7 +6,6 @@ use rstd::vec::Vec;
 use codec::{Decode, Encode};
 #[cfg(feature = "std")]
 use serde::{Deserialize, Serialize};
-use srml_support::ensure;
 
 use crate::hiring;
 use crate::hiring::*;
@@ -148,48 +147,6 @@ where
             panic!("stage MUST be active")
         }
     }
-
-    /// Performs all necessary check before adding an opening
-    pub(crate) fn ensure_can_add_opening(
-        current_block_height: BlockNumber,
-        activate_at: ActivateOpeningAt<BlockNumber>,
-        minimum_stake_balance: Balance,
-        application_rationing_policy: Option<ApplicationRationingPolicy>,
-        application_staking_policy: Option<StakingPolicy<Balance, BlockNumber>>,
-        role_staking_policy: Option<StakingPolicy<Balance, BlockNumber>>,
-    ) -> Result<(), AddOpeningError> {
-        // Check that exact activation is actually in the future
-        ensure!(
-            match activate_at {
-                ActivateOpeningAt::ExactBlock(block_number) => block_number > current_block_height,
-                _ => true,
-            },
-            AddOpeningError::OpeningMustActivateInTheFuture
-        );
-
-        if let Some(app_rationing_policy) = application_rationing_policy {
-            ensure!(
-                app_rationing_policy.max_active_applicants > 0,
-                AddOpeningError::ApplicationRationingZeroMaxApplicants
-            );
-        }
-
-        // Check that staking amounts clear minimum balance required.
-        StakingPolicy::ensure_amount_valid_in_opt_staking_policy(
-            application_staking_policy,
-            minimum_stake_balance.clone(),
-            AddOpeningError::StakeAmountLessThanMinimumStakeBalance(StakePurpose::Application),
-        )?;
-
-        // Check that staking amounts clear minimum balance required.
-        StakingPolicy::ensure_amount_valid_in_opt_staking_policy(
-            role_staking_policy,
-            minimum_stake_balance,
-            AddOpeningError::StakeAmountLessThanMinimumStakeBalance(StakePurpose::Role),
-        )?;
-
-        Ok(())
-    }
 }
 
 /// The stage at which an `Opening` may be at.

+ 0 - 15
runtime-modules/hiring/src/hiring/staking_policy.rs

@@ -50,21 +50,6 @@ impl<Balance: PartialOrd + Clone, BlockNumber: Clone> StakingPolicy<Balance, Blo
             None
         }
     }
-
-    /// Ensures that optional staking policy prescribes value that clears minimum balance requirement
-    pub(crate) fn ensure_amount_valid_in_opt_staking_policy<Err>(
-        opt_staking_policy: Option<StakingPolicy<Balance, BlockNumber>>,
-        minimum_stake_balance: Balance,
-        error: Err,
-    ) -> Result<(), Err> {
-        if let Some(ref staking_policy) = opt_staking_policy {
-            if staking_policy.amount < minimum_stake_balance {
-                return Err(error);
-            }
-        }
-
-        Ok(())
-    }
 }
 
 /// Constraints around staking amount

+ 66 - 9
runtime-modules/hiring/src/lib.rs

@@ -31,7 +31,7 @@ use codec::Codec;
 use runtime_primitives::traits::Zero;
 use runtime_primitives::traits::{MaybeSerialize, Member, One, SimpleArithmetic};
 
-use srml_support::traits::{Currency, Get};
+use srml_support::traits::Currency;
 use srml_support::{decl_module, decl_storage, ensure, Parameter};
 
 use rstd::collections::btree_map::BTreeMap;
@@ -79,9 +79,6 @@ pub trait Trait: system::Trait + stake::Trait + Sized {
 
     /// Marker type for Stake module handler. Indicates that hiring module uses stake module mock.
     type StakeHandlerProvider: StakeHandlerProvider<Self>;
-
-    /// Defines min application or role stake balance.
-    type MinimumStakeBalance: Get<stake::BalanceOf<Self>>;
 }
 
 decl_storage! {
@@ -108,9 +105,6 @@ decl_module! {
     /// Main hiring module definition
     pub struct Module<T: Trait> for enum Call where origin: T::Origin {
 
-        /// Exports const - min application or role stake balance.
-        const MinimumStakeBalance: stake::BalanceOf<T> = T::MinimumStakeBalance::get();
-
         fn on_finalize(now: T::BlockNumber) {
 
             //
@@ -190,10 +184,10 @@ impl<T: Trait> Module<T> {
     ) -> Result<T::OpeningId, AddOpeningError> {
         let current_block_height = <system::Module<T>>::block_number();
 
-        Opening::<BalanceOf<T>, T::BlockNumber, T::ApplicationId>::ensure_can_add_opening(
+        Self::ensure_can_add_opening(
             current_block_height,
             activate_at.clone(),
-            T::MinimumStakeBalance::get(),
+            T::Currency::minimum_balance(),
             application_rationing_policy.clone(),
             application_staking_policy.clone(),
             role_staking_policy.clone(),
@@ -1412,6 +1406,69 @@ impl<T: Trait> Module<T> {
             None
         }
     }
+
+    /// Performs all necessary check before adding an opening
+    pub(crate) fn ensure_can_add_opening(
+        current_block_height: T::BlockNumber,
+        activate_at: ActivateOpeningAt<T::BlockNumber>,
+        minimum_stake_balance: BalanceOf<T>,
+        application_rationing_policy: Option<ApplicationRationingPolicy>,
+        application_staking_policy: Option<StakingPolicy<BalanceOf<T>, T::BlockNumber>>,
+        role_staking_policy: Option<StakingPolicy<BalanceOf<T>, T::BlockNumber>>,
+    ) -> Result<(), AddOpeningError> {
+        // Check that exact activation is actually in the future
+        ensure!(
+            match activate_at {
+                ActivateOpeningAt::ExactBlock(block_number) => block_number > current_block_height,
+                _ => true,
+            },
+            AddOpeningError::OpeningMustActivateInTheFuture
+        );
+
+        if let Some(app_rationing_policy) = application_rationing_policy {
+            ensure!(
+                app_rationing_policy.max_active_applicants > 0,
+                AddOpeningError::ApplicationRationingZeroMaxApplicants
+            );
+        }
+
+        // Check that staking amounts clear minimum balance required.
+        Self::ensure_amount_valid_in_opt_staking_policy(
+            application_staking_policy,
+            minimum_stake_balance,
+            StakePurpose::Application,
+        )?;
+
+        // Check that staking amounts clear minimum balance required.
+        Self::ensure_amount_valid_in_opt_staking_policy(
+            role_staking_policy,
+            minimum_stake_balance,
+            StakePurpose::Role,
+        )?;
+
+        Ok(())
+    }
+
+    /// Ensures that optional staking policy prescribes value that clears minimum balance requirement
+    pub(crate) fn ensure_amount_valid_in_opt_staking_policy(
+        opt_staking_policy: Option<StakingPolicy<BalanceOf<T>, T::BlockNumber>>,
+        minimum_stake_balance: BalanceOf<T>,
+        stake_purpose: StakePurpose,
+    ) -> Result<(), AddOpeningError> {
+        if let Some(ref staking_policy) = opt_staking_policy {
+            ensure!(
+                staking_policy.amount > Zero::zero(),
+                AddOpeningError::StakeAmountCannotBeZero(stake_purpose)
+            );
+
+            ensure!(
+                staking_policy.amount >= minimum_stake_balance,
+                AddOpeningError::StakeAmountLessThanMinimumStakeBalance(stake_purpose)
+            );
+        }
+
+        Ok(())
+    }
 }
 
 /*

+ 0 - 5
runtime-modules/hiring/src/mock.rs

@@ -79,10 +79,6 @@ impl balances::Trait for Test {
     type CreationFee = CreationFee;
 }
 
-parameter_types! {
-    pub const MinimumStakeBalance: u64 = 1; // just non-zero
-}
-
 impl Trait for Test {
     type OpeningId = u64;
 
@@ -91,7 +87,6 @@ impl Trait for Test {
     type ApplicationDeactivatedHandler = TestApplicationDeactivatedHandler;
 
     type StakeHandlerProvider = TestStakeHandlerProvider;
-    type MinimumStakeBalance = MinimumStakeBalance;
 }
 
 impl stake::Trait for Test {

+ 33 - 4
runtime-modules/hiring/src/test/public_api/add_opening.rs

@@ -1,6 +1,11 @@
-use crate::mock::*;
-use crate::test::*;
+use crate::mock::{build_test_externalities, Hiring, Test};
+use crate::test::{BlockNumber, OpeningId};
 use crate::StakingAmountLimitMode::Exact;
+use crate::*;
+use crate::{
+    ActivateOpeningAt, ActiveOpeningStage, AddOpeningError, ApplicationRationingPolicy, Opening,
+    OpeningStage, StakePurpose, StakingPolicy,
+};
 use rstd::collections::btree_set::BTreeSet;
 
 static FIRST_BLOCK_HEIGHT: <Test as system::Trait>::BlockNumber = 1;
@@ -143,7 +148,7 @@ fn add_opening_succeeds_or_fails_due_to_application_staking_policy() {
 
         opening_data.call_and_assert(Ok(0));
 
-        //Invalid stake amount
+        //Zero stake amount
         opening_data.application_staking_policy = Some(StakingPolicy {
             amount: 0,
             amount_mode: Exact,
@@ -151,6 +156,18 @@ fn add_opening_succeeds_or_fails_due_to_application_staking_policy() {
             review_period_expired_unstaking_period_length: None,
         });
 
+        opening_data.call_and_assert(Err(AddOpeningError::StakeAmountCannotBeZero(
+            StakePurpose::Application,
+        )));
+
+        //Invalid stake amount
+        opening_data.application_staking_policy = Some(StakingPolicy {
+            amount: 1,
+            amount_mode: Exact,
+            crowded_out_unstaking_period_length: None,
+            review_period_expired_unstaking_period_length: None,
+        });
+
         opening_data.call_and_assert(Err(
             AddOpeningError::StakeAmountLessThanMinimumStakeBalance(StakePurpose::Application),
         ));
@@ -171,7 +188,7 @@ fn add_opening_succeeds_or_fails_due_to_role_staking_policy() {
 
         opening_data.call_and_assert(Ok(0));
 
-        //Invalid stake amount
+        //Zero stake amount
         opening_data.role_staking_policy = Some(StakingPolicy {
             amount: 0,
             amount_mode: Exact,
@@ -179,6 +196,18 @@ fn add_opening_succeeds_or_fails_due_to_role_staking_policy() {
             review_period_expired_unstaking_period_length: None,
         });
 
+        opening_data.call_and_assert(Err(AddOpeningError::StakeAmountCannotBeZero(
+            StakePurpose::Role,
+        )));
+
+        //Invalid stake amount
+        opening_data.role_staking_policy = Some(StakingPolicy {
+            amount: 1,
+            amount_mode: Exact,
+            crowded_out_unstaking_period_length: None,
+            review_period_expired_unstaking_period_length: None,
+        });
+
         opening_data.call_and_assert(Err(
             AddOpeningError::StakeAmountLessThanMinimumStakeBalance(StakePurpose::Role),
         ));

+ 0 - 5
runtime-modules/proposals/codex/src/tests/mock.rs

@@ -186,16 +186,11 @@ impl versioned_store::Trait for Test {
     type Event = ();
 }
 
-parameter_types! {
-    pub const MinimumStakeBalance: u64 = 1; // just non-zero
-}
-
 impl hiring::Trait for Test {
     type OpeningId = u64;
     type ApplicationId = u64;
     type ApplicationDeactivatedHandler = ();
     type StakeHandlerProvider = hiring::Module<Self>;
-    type MinimumStakeBalance = MinimumStakeBalance;
 }
 
 srml_staking_reward_curve::build! {

+ 0 - 5
runtime-modules/service-discovery/src/mock.rs

@@ -79,16 +79,11 @@ impl Trait for Test {
     type Event = MetaEvent;
 }
 
-parameter_types! {
-    pub const MinimumStakeBalance: u64 = 1; // just non-zero
-}
-
 impl hiring::Trait for Test {
     type OpeningId = u64;
     type ApplicationId = u64;
     type ApplicationDeactivatedHandler = ();
     type StakeHandlerProvider = hiring::Module<Self>;
-    type MinimumStakeBalance = MinimumStakeBalance;
 }
 
 impl minting::Trait for Test {

+ 0 - 5
runtime-modules/storage/src/tests/mock.rs

@@ -216,16 +216,11 @@ impl recurringrewards::Trait for Test {
     type RewardRelationshipId = u64;
 }
 
-parameter_types! {
-    pub const MinimumStakeBalance: u64 = 1; // just non-zero
-}
-
 impl hiring::Trait for Test {
     type OpeningId = u64;
     type ApplicationId = u64;
     type ApplicationDeactivatedHandler = ();
     type StakeHandlerProvider = hiring::Module<Self>;
-    type MinimumStakeBalance = MinimumStakeBalance;
 }
 
 pub struct ExtBuilder {

+ 1 - 1
runtime-modules/working-group/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = 'substrate-working-group-module'
-version = '1.0.0'
+version = '1.0.1'
 authors = ['Joystream contributors']
 edition = '2018'
 

+ 12 - 0
runtime-modules/working-group/src/errors.rs

@@ -247,6 +247,12 @@ decl_error! {
 
         /// Working group size limit exceeded.
         MaxActiveWorkerNumberExceeded,
+
+        /// Add worker opening role stake cannot be zero.
+        AddWorkerOpeningRoleStakeCannotBeZero,
+
+        /// Add worker opening application stake cannot be zero.
+        AddWorkerOpeningApplicationStakeCannotBeZero,
     }
 }
 
@@ -305,6 +311,12 @@ impl rstd::convert::From<WrappedError<hiring::AddOpeningError>> for Error {
             hiring::AddOpeningError::ApplicationRationingZeroMaxApplicants => {
                 Error::AddWorkerOpeningZeroMaxApplicantCount
             }
+            hiring::AddOpeningError::StakeAmountCannotBeZero(purpose) => match purpose {
+                hiring::StakePurpose::Role => Error::AddWorkerOpeningRoleStakeCannotBeZero,
+                hiring::StakePurpose::Application => {
+                    Error::AddWorkerOpeningApplicationStakeCannotBeZero
+                }
+            },
         }
     }
 }

+ 0 - 5
runtime-modules/working-group/src/tests/mock.rs

@@ -67,16 +67,11 @@ impl system::Trait for Test {
     type Version = ();
 }
 
-parameter_types! {
-    pub const MinimumStakeBalance: u64 = 1; // just non-zero
-}
-
 impl hiring::Trait for Test {
     type OpeningId = u64;
     type ApplicationId = u64;
     type ApplicationDeactivatedHandler = ();
     type StakeHandlerProvider = hiring::Module<Self>;
-    type MinimumStakeBalance = MinimumStakeBalance;
 }
 
 impl minting::Trait for Test {

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

@@ -323,7 +323,7 @@ fn add_opening_fails_with_invalid_zero_application_stake() {
                 ..OpeningPolicyCommitment::default()
             });
         add_opening_fixture
-            .call_and_assert(Err(Error::AddWorkerOpeningAppliicationStakeLessThanMinimum));
+            .call_and_assert(Err(Error::AddWorkerOpeningApplicationStakeCannotBeZero));
     });
 }
 

+ 0 - 5
runtime/src/lib.rs

@@ -502,16 +502,11 @@ impl versioned_store_permissions::CreateClassPermissionsChecker<Runtime>
     }
 }
 
-parameter_types! {
-    pub const MinimumStakeBalance: Balance = 1; // just non-zero
-}
-
 impl hiring::Trait for Runtime {
     type OpeningId = u64;
     type ApplicationId = u64;
     type ApplicationDeactivatedHandler = (); // TODO - what needs to happen?
     type StakeHandlerProvider = hiring::Module<Self>;
-    type MinimumStakeBalance = MinimumStakeBalance;
 }
 
 impl minting::Trait for Runtime {