Browse Source

runtime: Fix proposals tests.

Shamil Gadelshin 4 years ago
parent
commit
4db66bbe4d

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

@@ -1,5 +1,7 @@
 #![cfg(test)]
 
+use frame_support::dispatch::DispatchResult;
+use frame_support::traits::LockIdentifier;
 use frame_support::{impl_outer_dispatch, impl_outer_origin, parameter_types};
 use sp_core::H256;
 use sp_runtime::curve::PiecewiseLinear;
@@ -12,7 +14,6 @@ use sp_staking::SessionIndex;
 pub use system;
 
 use crate::{ProposalDetailsOf, ProposalEncoder};
-use frame_support::dispatch::DispatchResult;
 use proposals_engine::VotersParameters;
 use sp_runtime::testing::TestXt;
 
@@ -79,6 +80,7 @@ parameter_types! {
     pub const TitleMaxLength: u32 = 100;
     pub const DescriptionMaxLength: u32 = 10000;
     pub const MaxActiveProposalLimit: u32 = 100;
+    pub const LockId: LockIdentifier = [2; 8];
 }
 
 impl proposals_engine::Trait for Test {
@@ -87,7 +89,7 @@ impl proposals_engine::Trait for Test {
     type VoterOriginValidator = ();
     type TotalVotersCounter = MockVotersParameters;
     type ProposalId = u32;
-    type StakingHandler = ();
+    type StakingHandler = proposals_engine::StakingManager<Test, LockId>;
     type CancellationFee = CancellationFee;
     type RejectionFee = RejectionFee;
     type TitleMaxLength = TitleMaxLength;
@@ -110,11 +112,7 @@ impl proposals_engine::StakingHandler<Test> for () {
         unimplemented!()
     }
 
-    fn decrease_stake(_account_id: &u64, _new_stake: u64) {
-        unimplemented!()
-    }
-
-    fn increase_stake(_account_id: &u64, _new_stake: u64) -> DispatchResult {
+    fn set_stake(_account_id: &u64, _new_stake: u64) -> DispatchResult {
         unimplemented!()
     }
 

+ 2 - 1
runtime-modules/proposals/engine/Cargo.toml

@@ -16,11 +16,11 @@ sp-runtime = { package = 'sp-runtime', default-features = false, git = 'https://
 membership = { package = 'pallet-membership', default-features = false, path = '../../membership'}
 stake = { package = 'pallet-stake', default-features = false, path = '../../stake'}
 common = { package = 'pallet-common', default-features = false, path = '../../common'}
+balances = { package = 'pallet-balances', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '00768a1f21a579c478fe5d4f51e1fa71f7db9fd4'}
 
 [dev-dependencies]
 sp-io = { package = 'sp-io', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '00768a1f21a579c478fe5d4f51e1fa71f7db9fd4'}
 sp-core = { package = 'sp-core', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '00768a1f21a579c478fe5d4f51e1fa71f7db9fd4'}
-balances = { package = 'pallet-balances', default-features = false, git = 'https://github.com/paritytech/substrate.git', rev = '00768a1f21a579c478fe5d4f51e1fa71f7db9fd4'}
 
 [features]
 default = ['std']
@@ -36,4 +36,5 @@ std = [
     'membership/std',
     'stake/std',
     'common/std',
+    'balances/std',
 ]

+ 109 - 2
runtime-modules/proposals/engine/src/lib.rs

@@ -132,7 +132,7 @@ mod tests;
 use codec::Decode;
 use frame_support::dispatch::{DispatchError, DispatchResult, UnfilteredDispatchable};
 use frame_support::storage::IterableStorageMap;
-use frame_support::traits::Get;
+use frame_support::traits::{Currency, Get, LockIdentifier, LockableCurrency, WithdrawReasons};
 use frame_support::{
     decl_error, decl_event, decl_module, decl_storage, ensure, print, Parameter, StorageDoubleMap,
 };
@@ -141,9 +141,12 @@ use sp_std::vec::Vec;
 use system::{ensure_root, RawOrigin};
 
 use common::origin::ActorOriginValidator;
+use frame_support::sp_std::marker::PhantomData;
 
 /// Proposals engine trait.
-pub trait Trait: system::Trait + pallet_timestamp::Trait + membership::Trait {
+pub trait Trait:
+    system::Trait + pallet_timestamp::Trait + membership::Trait + balances::Trait
+{
     /// Engine event type.
     type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>;
 
@@ -288,6 +291,9 @@ decl_error! {
 
         /// Exact execution block cannot be less than current_block.
         InvalidExactExecutionBlock,
+
+        /// There is not enough balance for a stake.
+        InsufficientBalanceForStake,
     }
 }
 
@@ -464,6 +470,14 @@ impl<T: Trait> Module<T> {
         let proposal_id = T::ProposalId::from(new_proposal_id);
 
         let stake_data = if let Some(stake_balance) = creation_params.stake_balance {
+            ensure!(
+                T::StakingHandler::is_enough_balance_for_stake(
+                    &creation_params.account_id,
+                    stake_balance
+                ),
+                Error::<T>::InsufficientBalanceForStake
+            );
+
             T::StakingHandler::lock(&creation_params.account_id, stake_balance);
 
             Some(ActiveStake {
@@ -845,3 +859,96 @@ type ProposalOf<T> = Proposal<
     BalanceOfCurrency<T>,
     <T as system::Trait>::AccountId,
 >;
+
+pub struct StakingManager<T: Trait, LockId: Get<LockIdentifier>> {
+    trait_marker: PhantomData<T>,
+    lock_id_marker: PhantomData<LockId>,
+}
+
+impl<T: Trait, LockId: Get<LockIdentifier>> StakingHandler<T> for StakingManager<T, LockId> {
+    fn lock(account_id: &T::AccountId, amount: BalanceOfCurrency<T>) {
+        <balances::Module<T>>::set_lock(LockId::get(), &account_id, amount, WithdrawReasons::all())
+    }
+
+    fn unlock(account_id: &T::AccountId) {
+        T::Currency::remove_lock(LockId::get(), &account_id);
+    }
+
+    fn slash(
+        account_id: &T::AccountId,
+        amount: Option<BalanceOfCurrency<T>>,
+    ) -> BalanceOfCurrency<T> {
+        let locks = <balances::Module<T>>::locks(&account_id);
+
+        let existing_lock = locks.iter().find(|lock| lock.id == LockId::get());
+
+        let mut actually_slashed_balance = Default::default();
+        if let Some(existing_lock) = existing_lock {
+            Self::unlock(&account_id);
+
+            let mut slashable_amount = existing_lock.amount;
+            if let Some(amount) = amount {
+                if existing_lock.amount > amount {
+                    let new_amount = existing_lock.amount - amount;
+                    Self::lock(&account_id, new_amount);
+
+                    slashable_amount = amount;
+                }
+            }
+
+            let _ = <balances::Module<T>>::slash(&account_id, slashable_amount);
+
+            actually_slashed_balance = slashable_amount
+        }
+
+        actually_slashed_balance
+    }
+
+    fn set_stake(account_id: &T::AccountId, new_stake: BalanceOfCurrency<T>) -> DispatchResult {
+        let current_stake = Self::current_stake(account_id);
+
+        //Unlock previous stake if its not zero.
+        if current_stake > Zero::zero() {
+            Self::unlock(account_id);
+        }
+
+        if !Self::is_enough_balance_for_stake(account_id, new_stake) {
+            //Restore previous stake if its not zero.
+            if current_stake > Zero::zero() {
+                Self::lock(account_id, current_stake);
+            }
+            return Err(DispatchError::Other("Not enough balance for a new stake."));
+        }
+
+        Self::lock(account_id, new_stake);
+
+        Ok(())
+    }
+
+    fn is_member_staking_account(_member_id: &MemberId<T>, _account_id: &T::AccountId) -> bool {
+        true
+    }
+
+    fn is_account_free_of_conflicting_stakes(account_id: &T::AccountId) -> bool {
+        let locks = <balances::Module<T>>::locks(&account_id);
+
+        let existing_lock = locks.iter().find(|lock| lock.id == LockId::get());
+
+        existing_lock.is_none()
+    }
+
+    fn is_enough_balance_for_stake(
+        account_id: &T::AccountId,
+        amount: BalanceOfCurrency<T>,
+    ) -> bool {
+        <balances::Module<T>>::usable_balance(account_id) >= amount
+    }
+
+    fn current_stake(account_id: &T::AccountId) -> BalanceOfCurrency<T> {
+        let locks = <balances::Module<T>>::locks(&account_id);
+
+        let existing_lock = locks.iter().find(|lock| lock.id == LockId::get());
+
+        existing_lock.map_or(Zero::zero(), |lock| lock.amount)
+    }
+}

+ 4 - 6
runtime-modules/proposals/engine/src/tests/mock/mod.rs

@@ -6,6 +6,7 @@
 
 #![cfg(test)]
 
+use frame_support::traits::LockIdentifier;
 use frame_support::{impl_outer_event, impl_outer_origin, parameter_types};
 use sp_core::H256;
 use sp_runtime::{
@@ -70,6 +71,7 @@ parameter_types! {
     pub const TitleMaxLength: u32 = 100;
     pub const DescriptionMaxLength: u32 = 10000;
     pub const MaxActiveProposalLimit: u32 = 100;
+    pub const LockId: LockIdentifier = [1; 8];
 }
 
 impl membership::Trait for Test {
@@ -86,7 +88,7 @@ impl crate::Trait for Test {
     type VoterOriginValidator = ();
     type TotalVotersCounter = ();
     type ProposalId = u32;
-    type StakingHandler = ();
+    type StakingHandler = crate::StakingManager<Test, LockId>;
     type CancellationFee = CancellationFee;
     type RejectionFee = RejectionFee;
     type TitleMaxLength = TitleMaxLength;
@@ -109,11 +111,7 @@ impl crate::StakingHandler<Test> for () {
         unimplemented!()
     }
 
-    fn decrease_stake(_account_id: &u64, _new_stake: u64) {
-        unimplemented!()
-    }
-
-    fn increase_stake(_account_id: &u64, _new_stake: u64) -> DispatchResult {
+    fn set_stake(_account_id: &u64, _new_stake: u64) -> DispatchResult {
         unimplemented!()
     }
 

+ 48 - 7
runtime-modules/proposals/engine/src/tests/mod.rs

@@ -1056,7 +1056,8 @@ fn create_dummy_proposal_fail_with_stake_on_empty_account() {
             .with_account_id(account_id)
             .with_stake(required_stake);
 
-        dummy_proposal.create_proposal_and_assert(Err(DispatchError::Other("InsufficientBalance")));
+        dummy_proposal
+            .create_proposal_and_assert(Err(Error::<Test>::InsufficientBalanceForStake.into()));
     });
 }
 
@@ -1118,13 +1119,13 @@ fn finalize_expired_proposal_and_check_stake_removing_with_balance_checks_succee
         );
 
         assert_eq!(
-            <Test as common::currency::GovernanceCurrency>::Currency::total_balance(&account_id),
+            <Test as common::currency::GovernanceCurrency>::Currency::usable_balance(&account_id),
             account_balance
         );
 
         let proposal_id = dummy_proposal.create_proposal_and_assert(Ok(1)).unwrap();
         assert_eq!(
-            <Test as common::currency::GovernanceCurrency>::Currency::total_balance(&account_id),
+            <Test as common::currency::GovernanceCurrency>::Currency::usable_balance(&account_id),
             account_balance - stake_amount
         );
 
@@ -1155,7 +1156,7 @@ fn finalize_expired_proposal_and_check_stake_removing_with_balance_checks_succee
 
         let rejection_fee = RejectionFee::get();
         assert_eq!(
-            <Test as common::currency::GovernanceCurrency>::Currency::total_balance(&account_id),
+            <Test as common::currency::GovernanceCurrency>::Currency::usable_balance(&account_id),
             account_balance - rejection_fee
         );
     });
@@ -1192,13 +1193,13 @@ fn proposal_cancellation_with_slashes_with_balance_checks_succeeds() {
         );
 
         assert_eq!(
-            <Test as common::currency::GovernanceCurrency>::Currency::total_balance(&account_id),
+            <Test as common::currency::GovernanceCurrency>::Currency::usable_balance(&account_id),
             account_balance
         );
 
         let proposal_id = dummy_proposal.create_proposal_and_assert(Ok(1)).unwrap();
         assert_eq!(
-            <Test as common::currency::GovernanceCurrency>::Currency::total_balance(&account_id),
+            <Test as common::currency::GovernanceCurrency>::Currency::usable_balance(&account_id),
             account_balance - stake_amount
         );
 
@@ -1241,12 +1242,47 @@ fn proposal_cancellation_with_slashes_with_balance_checks_succeeds() {
 #[test]
 fn proposal_slashing_succeeds() {
     initial_test_ext().execute_with(|| {
+        // to enable events
         let starting_block = 1;
         run_to_block_and_finalize(starting_block);
 
-        let dummy_proposal = DummyProposalFixture::default();
+        let account_id = 1;
+
+        let initial_balance = 100000;
+        increase_total_balance_issuance_using_account_id(account_id, initial_balance);
+
+        let stake_amount = 200;
+        let parameters = ProposalParameters {
+            voting_period: 3,
+            approval_quorum_percentage: 50,
+            approval_threshold_percentage: 60,
+            slashing_quorum_percentage: 60,
+            slashing_threshold_percentage: 60,
+            grace_period: 5,
+            required_stake: Some(stake_amount),
+        };
+        let dummy_proposal = DummyProposalFixture::default()
+            .with_parameters(parameters)
+            .with_account_id(account_id.clone())
+            .with_stake(stake_amount);
+
+        assert_eq!(
+            <Test as common::currency::GovernanceCurrency>::Currency::total_balance(&account_id),
+            initial_balance
+        );
+
         let proposal_id = dummy_proposal.create_proposal_and_assert(Ok(1)).unwrap();
 
+        assert_eq!(
+            <Test as common::currency::GovernanceCurrency>::Currency::total_balance(&account_id),
+            initial_balance
+        );
+
+        assert_eq!(
+            <Test as common::currency::GovernanceCurrency>::Currency::usable_balance(&account_id),
+            initial_balance - stake_amount
+        );
+
         let mut vote_generator = VoteGenerator::new(proposal_id);
         vote_generator.vote_and_assert_ok(VoteKind::Reject);
         vote_generator.vote_and_assert_ok(VoteKind::Slash);
@@ -1260,6 +1296,11 @@ fn proposal_slashing_succeeds() {
         assert!(!<ActiveProposalIds<Test>>::contains_key(proposal_id));
         assert!(!<crate::Proposals<Test>>::contains_key(proposal_id));
 
+        assert_eq!(
+            <Test as common::currency::GovernanceCurrency>::Currency::total_balance(&account_id),
+            initial_balance - stake_amount
+        );
+
         EventFixture::assert_last_crate_event(RawEvent::ProposalStatusUpdated(
             proposal_id,
             ProposalStatus::finalized(ProposalDecisionStatus::Slashed, starting_block),

+ 4 - 12
runtime-modules/proposals/engine/src/types/mod.rs

@@ -16,7 +16,6 @@ use sp_std::vec::Vec;
 
 mod proposal_statuses;
 
-use common::currency::GovernanceCurrency;
 pub use proposal_statuses::{
     ApprovedProposalStatus, FinalizationData, ProposalDecisionStatus, ProposalStatus,
 };
@@ -396,15 +395,12 @@ pub struct ProposalCreationParameters<BlockNumber, Balance, MemberId, AccountId>
 pub(crate) type MemberId<T> = <T as membership::Trait>::MemberId;
 
 /// Balance alias for GovernanceCurrency from `common` module. TODO: replace with BalanceOf
-pub type BalanceOfCurrency<T> =
-    <<T as common::currency::GovernanceCurrency>::Currency as Currency<
-        <T as system::Trait>::AccountId,
-    >>::Balance;
+pub type BalanceOfCurrency<T> = <T as balances::Trait>::Balance;
 
 /// Defines abstract staking handler to manage user stakes for different activities
 /// like adding a proposal. Implementation should use built-in LockableCurrency
 /// and LockIdentifier to lock balance consistently with pallet_staking.
-pub trait StakingHandler<T: system::Trait + membership::Trait + GovernanceCurrency> {
+pub trait StakingHandler<T: system::Trait + membership::Trait + balances::Trait> {
     /// Locks the specified balance on the account using specific lock identifier.
     fn lock(account_id: &T::AccountId, amount: BalanceOfCurrency<T>);
 
@@ -420,12 +416,8 @@ pub trait StakingHandler<T: system::Trait + membership::Trait + GovernanceCurren
         amount: Option<BalanceOfCurrency<T>>,
     ) -> BalanceOfCurrency<T>;
 
-    /// Decreases the stake for to a given amount.
-    fn decrease_stake(account_id: &T::AccountId, new_stake: BalanceOfCurrency<T>);
-
-    /// Increases the stake for to a given amount.
-    fn increase_stake(account_id: &T::AccountId, new_stake: BalanceOfCurrency<T>)
-        -> DispatchResult;
+    /// Sets the new stake to a given amount.
+    fn set_stake(account_id: &T::AccountId, new_stake: BalanceOfCurrency<T>) -> DispatchResult;
 
     /// Verifies that staking account bound to the member.
     fn is_member_staking_account(member_id: &MemberId<T>, account_id: &T::AccountId) -> bool;

+ 1 - 4
runtime-modules/proposals/engine/src/types/proposal_statuses.rs

@@ -111,10 +111,7 @@ pub enum ProposalDecisionStatus {
 
 #[cfg(test)]
 mod tests {
-    use crate::{
-         ApprovedProposalStatus, FinalizationData, ProposalDecisionStatus,
-        ProposalStatus,
-    };
+    use crate::{ApprovedProposalStatus, FinalizationData, ProposalDecisionStatus, ProposalStatus};
 
     #[test]
     fn approved_proposal_status_helper_succeeds() {

+ 3 - 41
runtime/src/lib.rs

@@ -20,7 +20,7 @@ mod runtime_api;
 #[cfg(test)]
 mod tests; // Runtime integration tests
 
-use frame_support::traits::KeyOwnerProofSystem;
+use frame_support::traits::{KeyOwnerProofSystem, LockIdentifier};
 use frame_support::weights::{
     constants::{BlockExecutionWeight, ExtrinsicBaseWeight, RocksDbWeight},
     Weight,
@@ -69,7 +69,6 @@ pub use content_directory;
 pub use content_directory::{
     HashedTextMaxLength, InputValidationLengthConstraint, MaxNumber, TextMaxLength, VecMaxLength,
 };
-use frame_support::dispatch::DispatchResult;
 
 /// This runtime version.
 pub const VERSION: RuntimeVersion = RuntimeVersion {
@@ -552,6 +551,7 @@ parameter_types! {
     pub const ProposalTitleMaxLength: u32 = 40;
     pub const ProposalDescriptionMaxLength: u32 = 3000;
     pub const ProposalMaxActiveProposalLimit: u32 = 5;
+    pub const LockId: LockIdentifier = [1; 8];
 }
 
 impl proposals_engine::Trait for Runtime {
@@ -560,7 +560,7 @@ impl proposals_engine::Trait for Runtime {
     type VoterOriginValidator = CouncilManager<Self>;
     type TotalVotersCounter = CouncilManager<Self>;
     type ProposalId = u32;
-    type StakingHandler = (); // !!!!!! REMOVE STUB
+    type StakingHandler = proposals_engine::StakingManager<Self, LockId>;
     type CancellationFee = ProposalCancellationFee;
     type RejectionFee = ProposalRejectionFee;
     type TitleMaxLength = ProposalTitleMaxLength;
@@ -570,44 +570,6 @@ impl proposals_engine::Trait for Runtime {
     type ProposalObserver = ProposalsCodex;
 }
 
-impl proposals_engine::StakingHandler<Runtime> for () {
-    fn lock(_account_id: &AccountId, _amount: Balance) {
-        unimplemented!()
-    }
-
-    fn unlock(_account_id: &AccountId) {
-        unimplemented!()
-    }
-
-    fn slash(_account_id: &AccountId, _amount: Option<Balance>) -> Balance {
-        unimplemented!()
-    }
-
-    fn decrease_stake(_account_id: &AccountId, _new_stake: Balance) {
-        unimplemented!()
-    }
-
-    fn increase_stake(_account_id: &AccountId, _new_stake: Balance) -> DispatchResult {
-        unimplemented!()
-    }
-
-    fn is_member_staking_account(_member_id: &MemberId, _account_id: &AccountId) -> bool {
-        unimplemented!()
-    }
-
-    fn is_account_free_of_conflicting_stakes(_account_id: &AccountId) -> bool {
-        unimplemented!()
-    }
-
-    fn is_enough_balance_for_stake(_account_id: &AccountId, _amount: Balance) -> bool {
-        unimplemented!()
-    }
-
-    fn current_stake(_account_id: &AccountId) -> Balance {
-        unimplemented!()
-    }
-}
-
 impl Default for Call {
     fn default() -> Self {
         panic!("shouldn't call default for Call");

+ 3 - 3
runtime/src/tests/proposals_integration/mod.rs

@@ -296,13 +296,13 @@ fn proposal_cancellation_with_slashes_with_balance_checks_succeeds() {
             <Runtime as stake::Trait>::Currency::deposit_creating(&account_id, account_balance);
 
         assert_eq!(
-            <Runtime as stake::Trait>::Currency::total_balance(&account_id),
+            <Runtime as stake::Trait>::Currency::usable_balance(&account_id),
             account_balance
         );
 
         let proposal_id = dummy_proposal.create_proposal_and_assert(Ok(1)).unwrap();
         assert_eq!(
-            <Runtime as stake::Trait>::Currency::total_balance(&account_id),
+            <Runtime as stake::Trait>::Currency::usable_balance(&account_id),
             account_balance - stake_amount
         );
 
@@ -330,7 +330,7 @@ fn proposal_cancellation_with_slashes_with_balance_checks_succeeds() {
 
         let cancellation_fee = ProposalCancellationFee::get() as u128;
         assert_eq!(
-            <Runtime as stake::Trait>::Currency::total_balance(&account_id),
+            <Runtime as stake::Trait>::Currency::usable_balance(&account_id),
             account_balance - cancellation_fee
         );
     });