Browse Source

runtime: membership: Modify staking accounts.

- change staking account verification to the double map.
Shamil Gadelshin 4 years ago
parent
commit
d423eafbf7

+ 0 - 1
runtime-modules/governance/src/mock.rs

@@ -82,7 +82,6 @@ impl membership::Trait for Test {
     type DefaultMembershipPrice = DefaultMembershipPrice;
     type WorkingGroup = ();
     type DefaultInitialInvitationBalance = ();
-    type StakingAccountNumberLimit = ();
 }
 
 impl common::working_group::WorkingGroupIntegration<Test> for () {

+ 43 - 76
runtime-modules/membership/src/lib.rs

@@ -56,7 +56,6 @@ use sp_std::borrow::ToOwned;
 use sp_std::vec::Vec;
 
 use common::working_group::WorkingGroupIntegration;
-use sp_std::collections::btree_map::BTreeMap;
 
 // Balance type alias
 type BalanceOf<T> = <T as balances::Trait>::Balance;
@@ -75,9 +74,6 @@ pub trait Trait:
 
     /// Defines the default balance for the invited member.
     type DefaultInitialInvitationBalance: Get<BalanceOf<Self>>;
-
-    /// Defines the maximum staking account number.
-    type StakingAccountNumberLimit: Get<u32>;
 }
 
 // Default user info constraints
@@ -123,46 +119,6 @@ pub struct MembershipObject<AccountId: Ord> {
 
     /// Defines how many invitations this member has
     pub invites: u32,
-
-    /// Staking account IDs bound to the membership. Each account must be confirmed.
-    /// A map consists from staking account IDs and their confirmation flags.
-    pub staking_account_ids: BTreeMap<AccountId, bool>,
-}
-
-impl<AccountId: Ord> MembershipObject<AccountId> {
-    /// Add staking account id to the membership. Set its confirmation flag to false by default.
-    pub fn add_staking_account_candidate(&mut self, staking_account_id: AccountId) {
-        self.staking_account_ids.insert(staking_account_id, false);
-    }
-
-    /// Remove staking account id to the membership.
-    pub fn remove_staking_account(&mut self, staking_account_id: AccountId) {
-        self.staking_account_ids.remove(&staking_account_id);
-    }
-
-    /// Sets staking account confirmation flag as True. No effect on non-existing account.
-    pub fn confirm_staking_account(&mut self, staking_account_id: AccountId) {
-        if self.staking_account_exists(&staking_account_id) {
-            self.staking_account_ids.insert(staking_account_id, true);
-        }
-    }
-
-    /// Verifies existence of the staking account.
-    pub fn staking_account_exists(&self, staking_account_id: &AccountId) -> bool {
-        self.staking_account_ids.contains_key(&staking_account_id)
-    }
-
-    /// Verifies confirmation of the staking account.
-    pub fn staking_account_confirmed(&self, staking_account_id: &AccountId) -> bool {
-        let staking_account_confirmation = self.staking_account_ids.get(staking_account_id);
-
-        staking_account_confirmation.copied().unwrap_or(false)
-    }
-
-    /// Returns current staking account number.
-    pub fn staking_account_count(&self) -> u32 {
-        self.staking_account_ids.len() as u32
-    }
 }
 
 // Contains valid or default user details
@@ -271,17 +227,14 @@ decl_error! {
         /// Membership working group leader is not set.
         WorkingGroupLeaderNotSet,
 
-        /// Staking account for membership exists.
-        StakingAccountExists,
+        /// Staking account is registered for some member.
+        StakingAccountIsAlreadyRegistered,
 
         /// Staking account for membership doesn't exist.
         StakingAccountDoesntExist,
 
         /// Staking account has already been confirmed.
         StakingAccountAlreadyConfirmed,
-
-        /// Cannot add more staking account id.
-        MaximumStakingAccountNumberExceeded,
     }
 }
 
@@ -336,6 +289,10 @@ decl_storage! {
         /// Initial invitation balance for the invited member.
         pub InitialInvitationBalance get(fn initial_invitation_balance) : BalanceOf<T> =
             T::DefaultInitialInvitationBalance::get();
+
+        /// Double of a staking account id and member id to the confirmation status.
+        pub(crate) StakingAccountIdMemberStatus get(fn vote_by_proposal_by_voter): double_map
+            hasher(blake2_128_concat) T::AccountId, hasher(blake2_128_concat) T::MemberId => bool;
     }
     add_extra_genesis {
         config(members) : Vec<genesis::Member<T::MemberId, T::AccountId>>;
@@ -753,25 +710,18 @@ decl_module! {
             member_id: T::MemberId,
             staking_account_id: T::AccountId
         ) {
-            let membership = Self::ensure_member_controller_account_signed(origin, &member_id)?;
-
-            ensure!(
-                !membership.staking_account_exists(&staking_account_id),
-                Error::<T>::StakingAccountExists
-            );
+            Self::ensure_member_controller_account_signed(origin, &member_id)?;
 
             ensure!(
-                membership.staking_account_count() < T::StakingAccountNumberLimit::get(),
-                Error::<T>::MaximumStakingAccountNumberExceeded
+                !Self::staking_account_registered(&staking_account_id),
+                Error::<T>::StakingAccountIsAlreadyRegistered
             );
 
             //
             // == MUTATION SAFE ==
             //
 
-            <MembershipById<T>>::mutate(&member_id, |membership| {
-                membership.add_staking_account_candidate(staking_account_id.clone());
-            });
+            <StakingAccountIdMemberStatus<T>>::insert(staking_account_id.clone(), member_id, false);
 
             Self::deposit_event(RawEvent::StakingAccountAdded(member_id, staking_account_id));
         }
@@ -783,10 +733,10 @@ decl_module! {
             member_id: T::MemberId,
             staking_account_id: T::AccountId
         ) {
-            let membership = Self::ensure_member_controller_account_signed(origin, &member_id)?;
+            Self::ensure_member_controller_account_signed(origin, &member_id)?;
 
             ensure!(
-                membership.staking_account_exists(&staking_account_id),
+                Self::staking_account_registered_for_member(&staking_account_id, &member_id),
                 Error::<T>::StakingAccountDoesntExist
             );
 
@@ -794,9 +744,7 @@ decl_module! {
             // == MUTATION SAFE ==
             //
 
-            <MembershipById<T>>::mutate(&member_id, |membership| {
-                membership.remove_staking_account(staking_account_id.clone());
-            });
+            <StakingAccountIdMemberStatus<T>>::remove(staking_account_id.clone(), member_id);
 
             Self::deposit_event(RawEvent::StakingAccountRemoved(member_id, staking_account_id));
         }
@@ -809,15 +757,15 @@ decl_module! {
         ) {
             let staking_account_id = ensure_signed(origin)?;
 
-            let membership = Self::ensure_membership(member_id)?;
+            Self::ensure_membership(member_id)?;
 
             ensure!(
-                membership.staking_account_exists(&staking_account_id),
+                Self::staking_account_registered_for_member(&staking_account_id, &member_id),
                 Error::<T>::StakingAccountDoesntExist
             );
 
             ensure!(
-                !membership.staking_account_confirmed(&staking_account_id),
+                !Self::staking_account_confirmed(&staking_account_id, &member_id),
                 Error::<T>::StakingAccountAlreadyConfirmed
             );
 
@@ -825,9 +773,7 @@ decl_module! {
             // == MUTATION SAFE ==
             //
 
-            <MembershipById<T>>::mutate(&member_id, |membership| {
-                membership.confirm_staking_account(staking_account_id.clone());
-            });
+            <StakingAccountIdMemberStatus<T>>::insert(staking_account_id.clone(), member_id, true);
 
             Self::deposit_event(RawEvent::StakingAccountConfirmed(member_id, staking_account_id));
         }
@@ -950,7 +896,6 @@ impl<T: Trait> Module<T> {
             controller_account: controller_account.clone(),
             verified: false,
             invites: allowed_invites,
-            staking_account_ids: BTreeMap::new(),
         };
 
         <MemberIdsByRootAccountId<T>>::mutate(root_account, |ids| {
@@ -1005,6 +950,31 @@ impl<T: Trait> Module<T> {
 
         membership_fee.min(referral_cut)
     }
+
+    // Verifies registration of the staking account for ANY member.
+    fn staking_account_registered(staking_account_id: &T::AccountId) -> bool {
+        <StakingAccountIdMemberStatus<T>>::iter_prefix_values(staking_account_id).count() > 0
+    }
+
+    // Verifies registration of the staking account for SOME member.
+    fn staking_account_registered_for_member(
+        staking_account_id: &T::AccountId,
+        member_id: &T::MemberId,
+    ) -> bool {
+        <StakingAccountIdMemberStatus<T>>::contains_key(staking_account_id, member_id)
+    }
+
+    // Verifies confirmation of the staking account.
+    fn staking_account_confirmed(
+        staking_account_id: &T::AccountId,
+        member_id: &T::MemberId,
+    ) -> bool {
+        if !Self::staking_account_registered_for_member(staking_account_id, member_id) {
+            return false;
+        }
+
+        <StakingAccountIdMemberStatus<T>>::get(staking_account_id, member_id)
+    }
 }
 
 impl<T: Trait> common::StakingAccountValidator<T> for Module<T> {
@@ -1012,9 +982,6 @@ impl<T: Trait> common::StakingAccountValidator<T> for Module<T> {
         member_id: &common::MemberId<T>,
         account_id: &T::AccountId,
     ) -> bool {
-        Self::ensure_membership(*member_id)
-            .ok()
-            .map(|membership| membership.staking_account_confirmed(account_id))
-            .unwrap_or(false)
+        Self::staking_account_confirmed(account_id, member_id)
     }
 }

+ 13 - 19
runtime-modules/membership/src/tests/fixtures.rs

@@ -2,7 +2,7 @@ use super::mock::*;
 use crate::{BuyMembershipParameters, InviteMembershipParameters};
 use frame_support::dispatch::DispatchResult;
 use frame_support::traits::{OnFinalize, OnInitialize};
-use frame_support::StorageMap;
+use frame_support::{StorageDoubleMap, StorageMap};
 use frame_system::{EventRecord, Phase, RawOrigin};
 
 // Recommendation from Parity on testing on_finalize
@@ -572,8 +572,6 @@ impl Default for AddStakingAccountFixture {
 
 impl AddStakingAccountFixture {
     pub fn call_and_assert(&self, expected_result: DispatchResult) {
-        let old_membership = Membership::membership(self.member_id);
-
         let actual_result = Membership::add_staking_account_candidate(
             self.origin.clone().into(),
             self.member_id,
@@ -583,13 +581,10 @@ impl AddStakingAccountFixture {
         assert_eq!(expected_result, actual_result);
 
         if actual_result.is_ok() {
-            let new_membership = Membership::membership(self.member_id);
-
-            assert!(!new_membership.staking_account_confirmed(&self.staking_account_id));
-            assert_eq!(
-                new_membership.staking_account_count(),
-                old_membership.staking_account_count() + 1
-            );
+            assert!(<crate::StakingAccountIdMemberStatus<Test>>::contains_key(
+                &self.staking_account_id,
+                &self.member_id
+            ));
         }
     }
 
@@ -627,8 +622,6 @@ impl Default for RemoveStakingAccountFixture {
 
 impl RemoveStakingAccountFixture {
     pub fn call_and_assert(&self, expected_result: DispatchResult) {
-        let old_membership = Membership::membership(self.member_id);
-
         let actual_result = Membership::remove_staking_account(
             self.origin.clone().into(),
             self.member_id,
@@ -638,11 +631,10 @@ impl RemoveStakingAccountFixture {
         assert_eq!(expected_result, actual_result);
 
         if actual_result.is_ok() {
-            let new_membership = Membership::membership(self.member_id);
-            assert_eq!(
-                new_membership.staking_account_count(),
-                old_membership.staking_account_count() - 1
-            );
+            assert!(!<crate::StakingAccountIdMemberStatus<Test>>::contains_key(
+                &self.staking_account_id,
+                &self.member_id
+            ));
         }
     }
 
@@ -677,8 +669,10 @@ impl ConfirmStakingAccountFixture {
         assert_eq!(expected_result, actual_result);
 
         if actual_result.is_ok() {
-            let new_membership = Membership::membership(self.member_id);
-            assert!(new_membership.staking_account_confirmed(&BOB_ACCOUNT_ID));
+            assert!(<crate::StakingAccountIdMemberStatus<Test>>::get(
+                &BOB_ACCOUNT_ID,
+                &self.member_id
+            ));
         }
     }
 

+ 0 - 2
runtime-modules/membership/src/tests/mock.rs

@@ -104,7 +104,6 @@ parameter_types! {
     pub const MaxWorkerNumberLimit: u32 = 3;
     pub const LockId: LockIdentifier = [9; 8];
     pub const DefaultInitialInvitationBalance: u64 = 100;
-    pub const StakingAccountNumberLimit: u32 = 1;
 }
 
 impl working_group::Trait<MembershipWorkingGroupInstance> for Test {
@@ -227,7 +226,6 @@ impl Trait for Test {
     type DefaultMembershipPrice = DefaultMembershipPrice;
     type WorkingGroup = ();
     type DefaultInitialInvitationBalance = DefaultInitialInvitationBalance;
-    type StakingAccountNumberLimit = StakingAccountNumberLimit;
 }
 
 impl common::working_group::WorkingGroupIntegration<Test> for () {

+ 6 - 19
runtime-modules/membership/src/tests/mod.rs

@@ -9,7 +9,7 @@ use mock::*;
 
 use common::StakingAccountValidator;
 use frame_support::traits::{LockIdentifier, LockableCurrency, WithdrawReasons};
-use frame_support::{assert_ok, StorageMap, StorageValue};
+use frame_support::{assert_ok, StorageDoubleMap, StorageMap, StorageValue};
 use frame_system::RawOrigin;
 use sp_runtime::DispatchError;
 
@@ -785,22 +785,7 @@ fn add_staking_account_candidate_fails_with_duplicated_staking_account_id() {
         AddStakingAccountFixture::default().call_and_assert(Ok(()));
 
         AddStakingAccountFixture::default()
-            .call_and_assert(Err(Error::<Test>::StakingAccountExists.into()));
-    });
-}
-
-#[test]
-fn add_staking_account_candidate_fails_with_exceeding_account_limit() {
-    let initial_members = [(ALICE_MEMBER_ID, ALICE_ACCOUNT_ID)];
-
-    build_test_externalities_with_initial_members(initial_members.to_vec()).execute_with(|| {
-        AddStakingAccountFixture::default().call_and_assert(Ok(()));
-
-        AddStakingAccountFixture::default()
-            .with_staking_account_id(ALICE_ACCOUNT_ID)
-            .call_and_assert(Err(
-                Error::<Test>::MaximumStakingAccountNumberExceeded.into()
-            ));
+            .call_and_assert(Err(Error::<Test>::StakingAccountIsAlreadyRegistered.into()));
     });
 }
 
@@ -869,8 +854,10 @@ fn confirm_staking_account_succeeds() {
 
         ConfirmStakingAccountFixture::default().call_and_assert(Ok(()));
 
-        let membership = Membership::membership(ALICE_MEMBER_ID);
-        assert!(membership.staking_account_ids.get(&BOB_ACCOUNT_ID).unwrap());
+        assert!(<crate::StakingAccountIdMemberStatus<Test>>::get(
+            &BOB_ACCOUNT_ID,
+            &ALICE_MEMBER_ID
+        ));
 
         EventFixture::assert_last_crate_event(Event::<Test>::StakingAccountConfirmed(
             ALICE_MEMBER_ID,

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

@@ -54,7 +54,6 @@ impl membership::Trait for Test {
     type DefaultMembershipPrice = DefaultMembershipPrice;
     type WorkingGroup = ();
     type DefaultInitialInvitationBalance = ();
-    type StakingAccountNumberLimit = ();
 }
 
 impl common::working_group::WorkingGroupIntegration<Test> for () {

+ 0 - 1
runtime-modules/proposals/discussion/src/tests/mock.rs

@@ -84,7 +84,6 @@ impl membership::Trait for Test {
     type DefaultMembershipPrice = DefaultMembershipPrice;
     type WorkingGroup = ();
     type DefaultInitialInvitationBalance = ();
-    type StakingAccountNumberLimit = ();
 }
 
 impl common::working_group::WorkingGroupIntegration<Test> for () {

+ 0 - 1
runtime-modules/proposals/engine/src/tests/mock/mod.rs

@@ -92,7 +92,6 @@ impl membership::Trait for Test {
     type DefaultMembershipPrice = DefaultMembershipPrice;
     type WorkingGroup = ();
     type DefaultInitialInvitationBalance = ();
-    type StakingAccountNumberLimit = ();
 }
 
 impl common::working_group::WorkingGroupIntegration<Test> for () {

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

@@ -100,7 +100,6 @@ impl membership::Trait for Test {
     type DefaultMembershipPrice = DefaultMembershipPrice;
     type WorkingGroup = ();
     type DefaultInitialInvitationBalance = ();
-    type StakingAccountNumberLimit = ();
 }
 
 impl common::working_group::WorkingGroupIntegration<Test> for () {

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

@@ -287,7 +287,6 @@ impl membership::Trait for Test {
     type DefaultMembershipPrice = DefaultMembershipPrice;
     type WorkingGroup = ();
     type DefaultInitialInvitationBalance = ();
-    type StakingAccountNumberLimit = ();
 }
 
 impl common::working_group::WorkingGroupIntegration<Test> for () {

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

@@ -102,7 +102,6 @@ impl membership::Trait for Test {
     type DefaultMembershipPrice = DefaultMembershipPrice;
     type WorkingGroup = Module<Test>;
     type DefaultInitialInvitationBalance = ();
-    type StakingAccountNumberLimit = ();
 }
 
 pub type Balances = balances::Module<Test>;

+ 0 - 2
runtime/src/lib.rs

@@ -513,11 +513,9 @@ impl membership::Trait for Runtime {
     type DefaultMembershipPrice = DefaultMembershipPrice;
     type WorkingGroup = MembershipWorkingGroup;
     type DefaultInitialInvitationBalance = DefaultInitialInvitationBalance;
-    type StakingAccountNumberLimit = StakingAccountNumberLimit;
 }
 
 parameter_types! {
-    pub const StakingAccountNumberLimit: u32 = 50;
     pub const DefaultInitialInvitationBalance: Balance = 100;
     pub const MaxCategoryDepth: u64 = 5;
     pub const MaxSubcategories: u64 = 20;