refactor key management

src/governance/election.rs

@@ -158,7 +158,7 @@ impl<T: Trait> Module<T> {
 
     fn can_participate(sender: &T::AccountId) -> bool {
         !T::Currency::free_balance(sender).is_zero()
-            && <membership::members::Module<T>>::is_active_member(sender)
+            && <membership::members::Module<T>>::is_member_account(sender)
     }
 
     // PUBLIC IMMUTABLES

src/governance/proposals.rs

@@ -232,7 +232,7 @@ decl_module! {
         ) {
 
             let proposer = ensure_signed(origin)?;
-            ensure!(Self::can_participate(proposer.clone()), MSG_ONLY_MEMBERS_CAN_PROPOSE);
+            ensure!(Self::can_participate(&proposer), MSG_ONLY_MEMBERS_CAN_PROPOSE);
             ensure!(stake >= Self::min_stake(), MSG_STAKE_IS_TOO_LOW);
 
             ensure!(!name.is_empty(), MSG_EMPTY_NAME_PROVIDED);
@@ -357,9 +357,9 @@ impl<T: Trait> Module<T> {
         <system::Module<T>>::block_number()
     }
 
-    fn can_participate(sender: T::AccountId) -> bool {
-        !T::Currency::free_balance(&sender).is_zero()
-            && <membership::members::Module<T>>::is_active_member(&sender)
+    fn can_participate(sender: &T::AccountId) -> bool {
+        !T::Currency::free_balance(sender).is_zero()
+            && <membership::members::Module<T>>::is_member_account(sender)
     }
 
     fn is_councilor(sender: &T::AccountId) -> bool {

src/lib.rs

@@ -492,14 +492,13 @@ pub struct ShimMembershipRegistry {}
 
 impl forum::ForumUserRegistry<AccountId> for ShimMembershipRegistry {
     fn get_forum_user(id: &AccountId) -> Option<forum::ForumUser<AccountId>> {
-        if let Some(_profile) = members::Module::<Runtime>::get_profile_by_primary_account(id) {
-            // For now the profile is not used for anything,
+        if members::Module::<Runtime>::is_member_account(id) {
+            // For now we don't retreive the members profile since it is not used for anything,
             // but in the future we may need it to read out more
             // information possibly required to construct a
             // ForumUser.
 
             // Now convert member profile to a forum user
-
             Some(forum::ForumUser { id: id.clone() })
         } else {
             None

src/membership/members.rs

@@ -73,6 +73,7 @@ pub struct Profile<T: Trait> {
     pub entry: EntryMethod<T>,
     pub suspended: bool,
     pub subscription: Option<T::SubscriptionId>,
+    pub root_account: T::AccountId,
     pub controller_account: T::AccountId,
     pub roles: ActorInRoleSet,
 }
@@ -123,20 +124,17 @@ decl_storage! {
         /// total number of members created. MemberIds start at Zero.
         pub MembersCreated get(members_created) : T::MemberId;
 
-        /// Mapping of member ids to their corresponding primary account_id
-        pub AccountIdByMemberId get(account_id_by_member_id) : map T::MemberId => T::AccountId;
-
-        /// Mapping of members' primary account ids to their member id.
-        pub MemberIdByAccountId get(member_id_by_account_id) : map T::AccountId => Option<T::MemberId>;
-
-        /// Mapping of members' controller account ids to their member id.
-        pub MemberIdByControllerAccountId get(member_id_by_controller_account_id) : map T::AccountId => Option<T::MemberId>;
-
         /// Mapping of member's id to their membership profile
         pub MemberProfile get(member_profile) : map T::MemberId => Option<Profile<T>>;
 
+        /// Mapping of a root account id to vector of member ids it controls
+        pub MemberIdsByRootAccountId get(member_ids_by_root_account_id) : map T::AccountId => Vec<T::MemberId>;
+
+        /// Mapping of a controller account id to vector of member ids it controls
+        pub MemberIdsByControllerAccountId get(member_ids_by_controller_account_id) : map T::AccountId => Vec<T::MemberId>;
+
         /// Registered unique handles and their mapping to their owner
-        pub Handles get(handles) : map Vec<u8> => Option<T::MemberId>;
+        pub Handles get(handles) : map Vec<u8> => T::MemberId;
 
         /// Next paid membership terms id
         pub NextPaidMembershipTermsId get(next_paid_membership_terms_id) : T::PaidTermId = T::PaidTermId::from(FIRST_PAID_TERMS_ID);
@@ -202,8 +200,8 @@ decl_event! {
         MemberUpdatedAboutText(MemberId),
         MemberUpdatedAvatar(MemberId),
         MemberUpdatedHandle(MemberId),
-        MemberSetPrimaryKey(MemberId, AccountId),
-        MemberSetControllerKey(MemberId, AccountId),
+        MemberSetRootAccount(MemberId, AccountId),
+        MemberSetControllerAccount(MemberId, AccountId),
         MemberRegisteredRole(MemberId, RoleId, ActorId),
         MemberUnregisteredRole(MemberId, RoleId, ActorId),
     }
@@ -220,12 +218,6 @@ decl_module! {
             // make sure we are accepting new memberships
             ensure!(Self::new_memberships_allowed(), "new members not allowed");
 
-            // ensure key not associated with an existing membership
-            Self::ensure_not_member(&who)?;
-
-            // TODO: ensure account doesn't have any locks, is not a signing account of another member
-            // what other restrictions should we apply if any at all?
-
             // ensure paid_terms_id is active
             let terms = Self::ensure_active_terms_id(paid_terms_id)?;
 
@@ -244,31 +236,46 @@ decl_module! {
         }
 
         /// Change member's about text
-        pub fn change_member_about_text(origin, text: Vec<u8>) {
-            let controller = ensure_signed(origin)?;
-            let member_id = Self::ensure_is_member_controller_account(&controller)?;
+        pub fn change_member_about_text(origin, member_id: T::MemberId, text: Vec<u8>) {
+            let sender = ensure_signed(origin)?;
+
+            let profile = Self::ensure_profile(member_id)?;
+
+            ensure!(profile.controller_account == sender, "only controller account can update member about text");
+
             Self::_change_member_about_text(member_id, &text)?;
         }
 
         /// Change member's avatar
-        pub fn change_member_avatar(origin, uri: Vec<u8>) {
-            let controller = ensure_signed(origin)?;
-            let member_id = Self::ensure_is_member_controller_account(&controller)?;
+        pub fn change_member_avatar(origin, member_id: T::MemberId, uri: Vec<u8>) {
+            let sender = ensure_signed(origin)?;
+
+            let profile = Self::ensure_profile(member_id)?;
+
+            ensure!(profile.controller_account == sender, "only controller account can update member avatar");
+
             Self::_change_member_avatar(member_id, &uri)?;
         }
 
         /// Change member's handle. Will ensure new handle is unique and old one will be available
         /// for other members to use.
-        pub fn change_member_handle(origin, handle: Vec<u8>) {
-            let controller = ensure_signed(origin)?;
-            let member_id = Self::ensure_is_member_controller_account(&controller)?;
+        pub fn change_member_handle(origin, member_id: T::MemberId, handle: Vec<u8>) {
+            let sender = ensure_signed(origin)?;
+
+            let profile = Self::ensure_profile(member_id)?;
+
+            ensure!(profile.controller_account == sender, "only controller account can update member handle");
+
             Self::_change_member_handle(member_id, handle)?;
         }
 
         /// Update member's all or some of handle, avatar and about text.
-        pub fn update_profile(origin, user_info: UserInfo) {
-            let controller = ensure_signed(origin)?;
-            let member_id = Self::ensure_is_member_controller_account(&controller)?;
+        pub fn update_profile(origin, member_id: T::MemberId, user_info: UserInfo) {
+            let sender = ensure_signed(origin)?;
+
+            let profile = Self::ensure_profile(member_id)?;
+
+            ensure!(profile.controller_account == sender, "only controller account can update member info");
 
             if let Some(uri) = user_info.avatar_uri {
                 Self::_change_member_avatar(member_id, &uri)?;
@@ -281,39 +288,52 @@ decl_module! {
             }
         }
 
-        pub fn set_controller_key(origin, controller: T::AccountId) {
-            let master = ensure_signed(origin)?;
-            let member_id = Self::ensure_is_member_primary_account(&master)?;
+        pub fn set_controller_account(origin, member_id: T::MemberId, new_controller_account: T::AccountId) {
+            let sender = ensure_signed(origin)?;
 
             let mut profile = Self::ensure_profile(member_id)?;
 
-            // ensure new key is not used by someone else (master or controller)
-            ensure!(!<MemberIdByControllerAccountId<T>>::exists(&controller), "account already paired with member");
-            ensure!(!<MemberIdByAccountId<T>>::exists(&controller), "account already paired with member");
+            ensure!(profile.root_account == sender, "only root account can set new controller account");
+
+            // only update if new_controller_account is different than current one
+            if profile.controller_account != new_controller_account {
+                <MemberIdsByControllerAccountId<T>>::mutate(&profile.controller_account, |ids| {
+                    ids.retain(|id| *id != member_id);
+                });
 
-            <MemberIdByControllerAccountId<T>>::remove(&profile.controller_account);
-            <MemberIdByControllerAccountId<T>>::insert(&controller, member_id);
-            profile.controller_account = controller.clone();
-            <MemberProfile<T>>::insert(member_id, profile);
-            Self::deposit_event(RawEvent::MemberSetControllerKey(member_id, controller));
+                <MemberIdsByControllerAccountId<T>>::mutate(&new_controller_account, |ids| {
+                    ids.push(member_id);
+                });
+
+                profile.controller_account = new_controller_account.clone();
+                <MemberProfile<T>>::insert(member_id, profile);
+                Self::deposit_event(RawEvent::MemberSetControllerAccount(member_id, new_controller_account));
+            }
         }
 
-        pub fn set_primary_key(origin, new_primary: T::AccountId) {
-            let old_primary = ensure_signed(origin)?;
-            let member_id = Self::ensure_is_member_primary_account(&old_primary)?;
+        pub fn set_root_account(origin, member_id: T::MemberId, new_root_account: T::AccountId) {
+            let sender = ensure_signed(origin)?;
+
+            let mut profile = Self::ensure_profile(member_id)?;
+
+            ensure!(profile.root_account == sender, "only root account can set new root account");
 
-            // ensure new key not is used by someone else (master or controller)
-            ensure!(!<MemberIdByControllerAccountId<T>>::exists(&new_primary), "account already paired with member");
-            ensure!(!<MemberIdByAccountId<T>>::exists(&new_primary), "account already paired with member");
+            // only update if new root account is different than current one
+            if profile.root_account != new_root_account {
+                <MemberIdsByRootAccountId<T>>::mutate(&profile.root_account, |ids| {
+                    ids.retain(|id| *id != member_id);
+                });
 
-            // update mappings
-            <AccountIdByMemberId<T>>::insert(member_id, new_primary.clone());
-            <MemberIdByAccountId<T>>::remove(&old_primary);
-            <MemberIdByAccountId<T>>::insert(&new_primary, member_id);
-            Self::deposit_event(RawEvent::MemberSetPrimaryKey(member_id, new_primary));
+                <MemberIdsByRootAccountId<T>>::mutate(&new_root_account, |ids| {
+                    ids.push(member_id);
+                });
+
+                profile.root_account = new_root_account.clone();
+                Self::deposit_event(RawEvent::MemberSetRootAccount(member_id, new_root_account));
+            }
         }
 
-        pub fn add_screened_member(origin, new_member: T::AccountId, user_info: UserInfo) {
+        pub fn add_screened_member(origin, new_member_account: T::AccountId, user_info: UserInfo) {
             // ensure sender is screening authority
             let sender = ensure_signed(origin)?;
 
@@ -327,20 +347,14 @@ decl_module! {
             // make sure we are accepting new memberships
             ensure!(Self::new_memberships_allowed(), "new members not allowed");
 
-            // ensure key not associated with an existing membership
-            Self::ensure_not_member(&new_member)?;
-
-            // TODO: ensure account doesn't have any locks, is not a signing account of another member
-            // what other restrictions should we apply if any at all?
-
             let user_info = Self::check_user_registration_info(user_info)?;
 
             // ensure handle is not already registered
             Self::ensure_unique_handle(&user_info.handle)?;
 
-            let member_id = Self::insert_member(&new_member, &user_info, EntryMethod::Screening(sender));
+            let member_id = Self::insert_member(&new_member_account, &user_info, EntryMethod::Screening(sender));
 
-            Self::deposit_event(RawEvent::MemberRegistered(member_id, new_member.clone()));
+            Self::deposit_event(RawEvent::MemberRegistered(member_id, new_member_account));
         }
 
         pub fn set_screening_authority(origin, authority: T::AccountId) {
@@ -351,60 +365,49 @@ decl_module! {
 }
 
 impl<T: Trait> Module<T> {
-    pub fn is_active_member(who: &T::AccountId) -> bool {
-        match Self::ensure_is_member_primary_account(who)
-            .and_then(|member_id| Self::ensure_profile(member_id))
-        {
-            Ok(profile) => !profile.suspended,
-            Err(_err) => false,
-        }
+    /// Provided that the memberid exists return its profile. Returns error otherwise.
+    pub fn ensure_profile(id: T::MemberId) -> Result<Profile<T>, &'static str> {
+        Self::member_profile(&id).ok_or("member profile not found")
     }
 
-    pub fn primary_account_by_member_id(id: T::MemberId) -> Result<T::AccountId, &'static str> {
-        if <AccountIdByMemberId<T>>::exists(&id) {
-            Ok(Self::account_id_by_member_id(&id))
-        } else {
-            Err("member id doesn't exist")
-        }
+    /// Returns true if account is either a mmeber's root or controller account
+    pub fn is_member_account(who: &T::AccountId) -> bool {
+        <MemberIdsByRootAccountId<T>>::exists(who)
+            || <MemberIdsByControllerAccountId<T>>::exists(who)
     }
 
-    fn ensure_not_member(who: &T::AccountId) -> dispatch::Result {
-        ensure!(
-            !<MemberIdByAccountId<T>>::exists(who),
-            "account already associated with a membership"
-        );
-        Ok(())
-    }
-
-    pub fn ensure_is_member_primary_account(
-        who: &T::AccountId,
-    ) -> Result<T::MemberId, &'static str> {
-        let member_id =
-            Self::member_id_by_account_id(who).ok_or("no member id found for accountid")?;
-        Ok(member_id)
-    }
-
-    pub fn ensure_is_member_controller_account(
-        who: &T::AccountId,
-    ) -> Result<T::MemberId, &'static str> {
-        let member_id = Self::member_id_by_controller_account_id(who)
-            .ok_or("no member id found for accountid")?;
-        Ok(member_id)
-    }
-
-    fn ensure_profile(id: T::MemberId) -> Result<Profile<T>, &'static str> {
-        let profile = Self::member_profile(&id).ok_or("member profile not found")?;
-        Ok(profile)
-    }
-
-    pub fn get_profile_by_primary_account(id: &T::AccountId) -> Option<Profile<T>> {
-        if let Ok(member_id) = Self::ensure_is_member_primary_account(id) {
-            // This option _must_ be set
-            Self::member_profile(&member_id)
-        } else {
-            None
-        }
-    }
+    // pub fn primary_account_by_member_id(id: T::MemberId) -> Result<T::AccountId, &'static str> {
+    //     if <AccountIdByMemberId<T>>::exists(&id) {
+    //         Ok(Self::account_id_by_member_id(&id))
+    //     } else {
+    //         Err("member id doesn't exist")
+    //     }
+    // }
+
+    // pub fn ensure_is_member_primary_account(
+    //     who: &T::AccountId,
+    // ) -> Result<T::MemberId, &'static str> {
+    //     let member_id =
+    //         Self::member_id_by_account_id(who).ok_or("no member id found for accountid")?;
+    //     Ok(member_id)
+    // }
+
+    // pub fn ensure_is_member_controller_account(
+    //     who: &T::AccountId,
+    // ) -> Result<T::MemberId, &'static str> {
+    //     let member_id = Self::member_id_by_controller_account_id(who)
+    //         .ok_or("no member id found for accountid")?;
+    //     Ok(member_id)
+    // }
+
+    // pub fn get_profile_by_primary_account(id: &T::AccountId) -> Option<Profile<T>> {
+    //     if let Ok(member_id) = Self::ensure_is_member_primary_account(id) {
+    //         // This option _must_ be set
+    //         Self::member_profile(&member_id)
+    //     } else {
+    //         None
+    //     }
+    // }
 
     fn ensure_active_terms_id(
         terms_id: T::PaidTermId,
@@ -485,13 +488,18 @@ impl<T: Trait> Module<T> {
             entry: entry_method,
             suspended: false,
             subscription: None,
-            roles: ActorInRoleSet::new(),
+            root_account: who.clone(),
             controller_account: who.clone(),
+            roles: ActorInRoleSet::new(),
         };
 
-        <MemberIdByAccountId<T>>::insert(who.clone(), new_member_id);
-        <MemberIdByControllerAccountId<T>>::insert(who.clone(), new_member_id);
-        <AccountIdByMemberId<T>>::insert(new_member_id, who.clone());
+        <MemberIdsByRootAccountId<T>>::mutate(who, |ids| {
+            ids.push(new_member_id);
+        });
+        <MemberIdsByControllerAccountId<T>>::mutate(who, |ids| {
+            ids.push(new_member_id);
+        });
+
         <MemberProfile<T>>::insert(new_member_id, profile);
         <Handles<T>>::insert(user_info.handle.clone(), new_member_id);
         <MembersCreated<T>>::put(new_member_id + One::one());
@@ -529,15 +537,11 @@ impl<T: Trait> Module<T> {
         Ok(())
     }
 
-    // Checks if a member identified by their controller account occupies a role at least once
-    pub fn member_is_in_role(controller_account: &T::AccountId, role: Role) -> bool {
-        if let Ok(member_id) = Self::ensure_is_member_controller_account(controller_account) {
-            Self::ensure_profile(member_id)
-                .ok()
-                .map_or(false, |profile| profile.roles.has_role(role))
-        } else {
-            false
-        }
+    // Checks if a member identified by their member id occupies a role at least once
+    pub fn member_is_in_role(member_id: T::MemberId, role: Role) -> bool {
+        Self::ensure_profile(member_id)
+            .ok()
+            .map_or(false, |profile| profile.roles.has_role(role))
     }
 
     // policy across all roles is:
@@ -547,20 +551,22 @@ impl<T: Trait> Module<T> {
     // ** Note ** Role specific policies should be enforced by the client modules
     // this method should handle higher level policies
     pub fn can_register_role_on_member(
-        controller_account: &T::AccountId,
+        signing_account: &T::AccountId,
+        member_id: T::MemberId,
         role: Role,
     ) -> Result<(), &'static str> {
-        let member_id = Self::ensure_is_member_controller_account(controller_account)?;
+        let profile = Self::ensure_profile(member_id)?;
 
         ensure!(
-            !Self::member_is_in_role(controller_account, role),
-            "member already in role"
+            profile.controller_account == *signing_account,
+            "only member controller account can register role"
         );
 
         // ensure is active member
-        let profile = Self::ensure_profile(member_id)?;
         ensure!(!profile.suspended, "suspended members can't enter a role");
 
+        ensure!(!profile.roles.has_role(role), "member already in role");
+
         // Other possible checks:
         // How long the member has been registered
         // Minimum balance
@@ -570,14 +576,13 @@ impl<T: Trait> Module<T> {
     }
 
     pub fn register_role_on_member(
-        controller_account: &T::AccountId,
+        signing_account: &T::AccountId,
+        member_id: T::MemberId,
         role: Role,
         actor_id: ActorId,
     ) -> Result<(), &'static str> {
-        let member_id = Self::ensure_is_member_controller_account(controller_account)?;
-
         // policy check
-        Self::can_register_role_on_member(controller_account, role)?;
+        Self::can_register_role_on_member(signing_account, member_id, role)?;
 
         // guard against duplicate ActorInRole
         let actor_in_role = ActorInRole {
@@ -603,10 +608,18 @@ impl<T: Trait> Module<T> {
     }
 
     pub fn can_unregister_role_on_member(
-        controller_account: &T::AccountId,
+        signing_account: &T::AccountId,
+        member_id: T::MemberId,
         role: Role,
         actor_id: ActorId,
     ) -> Result<(), &'static str> {
+        let profile = Self::ensure_profile(member_id)?;
+
+        ensure!(
+            profile.controller_account == *signing_account,
+            "only member controller account can un-register role"
+        );
+
         let actor_in_role = ActorInRole {
             role_id: role.into(),
             actor_id,
@@ -615,8 +628,6 @@ impl<T: Trait> Module<T> {
             <MembershipIdByActorInRole<T>>::exists(&actor_in_role),
             "role actor not found"
         );
-
-        let member_id = Self::ensure_is_member_controller_account(controller_account)?;
         ensure!(
             <MembershipIdByActorInRole<T>>::get(&actor_in_role) == member_id,
             "role actor not for member"
@@ -625,13 +636,13 @@ impl<T: Trait> Module<T> {
     }
 
     pub fn unregister_role_on_member(
-        controller_account: &T::AccountId,
+        signing_account: &T::AccountId,
+        member_id: T::MemberId,
         role: Role,
         actor_id: ActorId,
     ) -> Result<(), &'static str> {
-        Self::can_unregister_role_on_member(controller_account, role, actor_id)?;
+        Self::can_unregister_role_on_member(signing_account, member_id, role, actor_id)?;
 
-        let member_id = Self::ensure_is_member_controller_account(controller_account)?;
         let mut profile = Self::ensure_profile(member_id)?;
 
         let actor_in_role = ActorInRole {

src/membership/tests.rs

@@ -9,6 +9,9 @@ fn assert_ok_unwrap<T>(value: Option<T>, err: &'static str) -> T {
     match value {
         None => {
             assert!(false, err);
+            // although this code path is not reached, we need to return correct type
+            // in match arm. Using panic! would remove this need, but assert! gives us better error in
+            // console output
             value.unwrap()
         }
         Some(v) => v,
@@ -99,15 +102,15 @@ fn buy_membership() {
             let initial_balance = DEFAULT_FEE + SURPLUS_BALANCE;
             set_alice_free_balance(initial_balance);
 
+            let next_member_id = Members::members_created();
+
             assert_ok!(buy_default_membership_as_alice());
 
-            let member_id = assert_ok_unwrap(
-                Members::member_id_by_account_id(&ALICE_ACCOUNT_ID),
-                "member id not assigned",
-            );
+            let member_ids = Members::member_ids_by_root_account_id(&ALICE_ACCOUNT_ID);
+            assert_eq!(member_ids, vec![next_member_id]);
 
             let profile = assert_ok_unwrap(
-                Members::member_profile(&member_id),
+                Members::member_profile(&next_member_id),
                 "member profile not created",
             );
 
@@ -120,8 +123,8 @@ fn buy_membership() {
             // controller account initially set to primary account
             assert_eq!(profile.controller_account, ALICE_ACCOUNT_ID);
             assert_eq!(
-                Members::member_id_by_controller_account_id(ALICE_ACCOUNT_ID),
-                Some(member_id)
+                Members::member_ids_by_controller_account_id(ALICE_ACCOUNT_ID),
+                vec![next_member_id]
             );
         },
     );
@@ -169,32 +172,6 @@ fn new_memberships_allowed_flag() {
     );
 }
 
-#[test]
-fn account_cannot_create_multiple_memberships() {
-    const DEFAULT_FEE: u64 = 500;
-    const SURPLUS_BALANCE: u64 = 500;
-
-    with_externalities(
-        &mut ExtBuilder::default()
-            .default_paid_membership_fee(DEFAULT_FEE)
-            .members(vec![ALICE_ACCOUNT_ID])
-            .build(),
-        || {
-            let initial_balance = DEFAULT_FEE + SURPLUS_BALANCE;
-            set_alice_free_balance(initial_balance);
-
-            // assert alice member exists
-            assert!(Members::member_id_by_account_id(&ALICE_ACCOUNT_ID).is_some());
-
-            // buying membership should fail...
-            assert_dispatch_error_message(
-                buy_default_membership_as_alice(),
-                "account already associated with a membership",
-            );
-        },
-    );
-}
-
 #[test]
 fn unique_handles() {
     const DEFAULT_FEE: u64 = 500;
@@ -233,20 +210,18 @@ fn update_profile() {
             let initial_balance = DEFAULT_FEE + SURPLUS_BALANCE;
             set_alice_free_balance(initial_balance);
 
+            let next_member_id = Members::members_created();
+
             assert_ok!(buy_default_membership_as_alice());
 
             assert_ok!(Members::update_profile(
                 Origin::signed(ALICE_ACCOUNT_ID),
+                next_member_id,
                 get_bob_info()
             ));
 
-            let member_id = assert_ok_unwrap(
-                Members::member_id_by_account_id(&ALICE_ACCOUNT_ID),
-                "member id not assigned",
-            );
-
             let profile = assert_ok_unwrap(
-                Members::member_profile(&member_id),
+                Members::member_profile(&next_member_id),
                 "member profile not created",
             );
 
@@ -263,19 +238,16 @@ fn add_screened_member() {
         let screening_authority = 5;
         <members::ScreeningAuthority<Test>>::put(&screening_authority);
 
+        let next_member_id = Members::members_created();
+
         assert_ok!(Members::add_screened_member(
             Origin::signed(screening_authority),
             ALICE_ACCOUNT_ID,
             get_alice_info()
         ));
 
-        let member_id = assert_ok_unwrap(
-            Members::member_id_by_account_id(&ALICE_ACCOUNT_ID),
-            "member id not assigned",
-        );
-
         let profile = assert_ok_unwrap(
-            Members::member_profile(&member_id),
+            Members::member_profile(&next_member_id),
             "member profile not created",
         );
 
@@ -299,16 +271,14 @@ fn set_controller_key() {
             .members(initial_members.to_vec())
             .build(),
         || {
-            assert_ok!(Members::set_controller_key(
+            let member_id = Members::member_ids_by_root_account_id(&ALICE_ACCOUNT_ID)[0];
+
+            assert_ok!(Members::set_controller_account(
                 Origin::signed(ALICE_ACCOUNT_ID),
+                member_id,
                 ALICE_CONTROLLER_ID
             ));
 
-            let member_id = assert_ok_unwrap(
-                Members::member_id_by_account_id(&ALICE_ACCOUNT_ID),
-                "member id not assigned",
-            );
-
             let profile = assert_ok_unwrap(
                 Members::member_profile(&member_id),
                 "member profile not created",
@@ -316,46 +286,37 @@ fn set_controller_key() {
 
             assert_eq!(profile.controller_account, ALICE_CONTROLLER_ID);
             assert_eq!(
-                Members::member_id_by_controller_account_id(ALICE_CONTROLLER_ID),
-                Some(member_id)
+                Members::member_ids_by_controller_account_id(&ALICE_CONTROLLER_ID),
+                vec![member_id]
             );
-            assert!(!<members::MemberIdByControllerAccountId<Test>>::exists(
-                &ALICE_ACCOUNT_ID
-            ));
+            assert!(Members::member_ids_by_controller_account_id(&ALICE_ACCOUNT_ID).is_empty());
         },
     );
 }
 
 #[test]
-fn set_primary_key() {
+fn set_root_account() {
     let initial_members = [ALICE_ACCOUNT_ID];
-    const ALICE_NEW_PRIMARY_KEY: u64 = 2;
+    const ALICE_NEW_ROOT_ACCOUNT: u64 = 2;
 
     with_externalities(
         &mut ExtBuilder::default()
             .members(initial_members.to_vec())
             .build(),
         || {
-            let member_id_1 = assert_ok_unwrap(
-                Members::member_id_by_account_id(&ALICE_ACCOUNT_ID),
-                "member id not found",
-            );
+            let member_id_1 = Members::member_ids_by_root_account_id(&ALICE_ACCOUNT_ID)[0];
 
-            assert_ok!(Members::set_primary_key(
+            assert_ok!(Members::set_root_account(
                 Origin::signed(ALICE_ACCOUNT_ID),
-                ALICE_NEW_PRIMARY_KEY
+                member_id_1,
+                ALICE_NEW_ROOT_ACCOUNT
             ));
 
-            let member_id_2 = assert_ok_unwrap(
-                Members::member_id_by_account_id(&ALICE_NEW_PRIMARY_KEY),
-                "member id not found",
-            );
+            let member_id_2 = Members::member_ids_by_root_account_id(&ALICE_NEW_ROOT_ACCOUNT)[0];
 
             assert_eq!(member_id_1, member_id_2);
-            assert_eq!(
-                Members::account_id_by_member_id(member_id_1),
-                ALICE_NEW_PRIMARY_KEY
-            );
+
+            assert!(Members::member_ids_by_root_account_id(&ALICE_ACCOUNT_ID).is_empty());
         },
     );
 }
@@ -370,29 +331,48 @@ fn registering_and_unregistering_roles_on_member() {
             .build(),
         || {
             const DUMMY_ACTOR_ID: u32 = 100;
+            let member_id_1 = Members::member_ids_by_root_account_id(&1)[0];
+            let member_id_2 = Members::member_ids_by_root_account_id(&2)[0];
 
             // no initial roles for member
-            assert!(!Members::member_is_in_role(&1, members::Role::Publisher));
+            assert!(!Members::member_is_in_role(
+                member_id_1,
+                members::Role::Publisher
+            ));
 
             // REGISTERING
 
             // successful registration
             assert_ok!(Members::register_role_on_member(
                 &1,
+                member_id_1,
                 members::Role::Publisher,
                 DUMMY_ACTOR_ID
             ));
-            assert!(Members::member_is_in_role(&1, members::Role::Publisher));
+            assert!(Members::member_is_in_role(
+                member_id_1,
+                members::Role::Publisher
+            ));
 
             // enter role a second time should fail
             assert_dispatch_error_message(
-                Members::register_role_on_member(&1, members::Role::Publisher, DUMMY_ACTOR_ID),
+                Members::register_role_on_member(
+                    &1,
+                    member_id_1,
+                    members::Role::Publisher,
+                    DUMMY_ACTOR_ID,
+                ),
                 "member already in role",
             );
 
             // registering another member in same role and actorid combination should fail
             assert_dispatch_error_message(
-                Members::register_role_on_member(&2, members::Role::Publisher, DUMMY_ACTOR_ID),
+                Members::register_role_on_member(
+                    &2,
+                    member_id_2,
+                    members::Role::Publisher,
+                    DUMMY_ACTOR_ID,
+                ),
                 "role actor already exists",
             );
 
@@ -400,23 +380,32 @@ fn registering_and_unregistering_roles_on_member() {
 
             // trying to unregister non existant actor role should fail
             assert_dispatch_error_message(
-                Members::unregister_role_on_member(&1, members::Role::Curator, 222),
+                Members::unregister_role_on_member(&1, member_id_1, members::Role::Curator, 222),
                 "role actor not found",
             );
 
             // trying to unregister actor role on wrong member should fail
             assert_dispatch_error_message(
-                Members::unregister_role_on_member(&2, members::Role::Publisher, DUMMY_ACTOR_ID),
+                Members::unregister_role_on_member(
+                    &2,
+                    member_id_2,
+                    members::Role::Publisher,
+                    DUMMY_ACTOR_ID,
+                ),
                 "role actor not for member",
             );
 
             // successfully unregister role
             assert_ok!(Members::unregister_role_on_member(
                 &1,
+                member_id_1,
                 members::Role::Publisher,
                 DUMMY_ACTOR_ID
             ));
-            assert!(!Members::member_is_in_role(&1, members::Role::Publisher));
+            assert!(!Members::member_is_in_role(
+                member_id_1,
+                members::Role::Publisher
+            ));
         },
     );
 }

src/roles/actors.rs

@@ -174,18 +174,6 @@ impl<T: Trait> Module<T> {
         Self::actor_by_account_id(role_key).ok_or("not role key")
     }
 
-    fn ensure_actor_is_member(
-        role_key: &T::AccountId,
-        member_id: MemberId<T>,
-    ) -> Result<Actor<T>, &'static str> {
-        let actor = Self::ensure_actor(role_key)?;
-        if actor.member_id == member_id {
-            Ok(actor)
-        } else {
-            Err("actor not owned by member")
-        }
-    }
-
     fn ensure_role_parameters(role: Role) -> Result<RoleParameters<T>, &'static str> {
         Self::parameters(role).ok_or("no parameters for role")
     }
@@ -287,9 +275,9 @@ decl_module! {
                                 if balance < params.min_stake {
                                     let _ = T::Currency::deposit_into_existing(&actor.account, params.reward);
                                 } else {
-                                    // otherwise it should go the the member account
-                                    if let Ok(member_account) = <membership::members::Module<T>>::primary_account_by_member_id(actor.member_id) {
-                                        let _ = T::Currency::deposit_into_existing(&member_account, params.reward);
+                                    // otherwise it should go the the member's root account
+                                    if let Some(profile) = <membership::members::Module<T>>::member_profile(&actor.member_id) {
+                                        let _ = T::Currency::deposit_into_existing(&profile.root_account, params.reward);
                                     }
                                 }
                             }
@@ -302,13 +290,14 @@ decl_module! {
         pub fn role_entry_request(origin, role: Role, member_id: MemberId<T>) {
             let sender = ensure_signed(origin)?;
 
-            ensure!(<membership::members::Module<T>>::ensure_is_member_primary_account(&sender).is_err(), "account is a member");
             ensure!(!Self::is_role_account(&sender), "account already used");
 
             ensure!(Self::is_role_available(role), "inactive role");
 
             let role_parameters = Self::ensure_role_parameters(role)?;
 
+            <membership::members::Module<T>>::ensure_profile(member_id)?;
+
             // pay (burn) entry fee - spam filter
             let fee = role_parameters.entry_request_fee;
             ensure!(T::Currency::can_slash(&sender, fee), "cannot pay role entry request fee");
@@ -324,16 +313,25 @@ decl_module! {
         /// Member activating entry request
         pub fn stake(origin, role: Role, actor_account: T::AccountId) {
             let sender = ensure_signed(origin)?;
-            let member_id = <membership::members::Module<T>>::ensure_is_member_primary_account(&sender)?;
+            ensure!(<membership::members::Module<T>>::is_member_account(&sender), "members only can accept storage entry request");
 
-            if !Self::role_entry_requests()
+            // get member ids from requests that are controller by origin
+            let ids = Self::role_entry_requests()
                 .iter()
-                .any(|request| request.0 == actor_account && request.1 == member_id && request.2 == role)
-            {
-                return Err("no role entry request matches");
-            }
+                .filter(|request| request.0 == actor_account && request.2 == role)
+                .map(|request| request.1)
+                .filter(|member_id|
+                    <membership::members::Module<T>>::ensure_profile(*member_id)
+                        .ok()
+                        .map_or(false, |profile| profile.root_account == sender || profile.controller_account == sender)
+                )
+                .collect::<Vec<_>>();
+
+            ensure!(!ids.is_empty(), "no role entry request matches");
+
+            // take first matching id
+            let member_id = ids[0];
 
-            ensure!(<membership::members::Module<T>>::ensure_is_member_primary_account(&actor_account).is_err(), "account is a member");
             ensure!(!Self::is_role_account(&actor_account), "account already used");
 
             // make sure role is still available
@@ -372,9 +370,10 @@ decl_module! {
 
         pub fn unstake(origin, actor_account: T::AccountId) {
             let sender = ensure_signed(origin)?;
-            let member_id = <membership::members::Module<T>>::ensure_is_member_primary_account(&sender)?;
+            let actor = Self::ensure_actor(&actor_account)?;
 
-            let actor = Self::ensure_actor_is_member(&actor_account, member_id)?;
+            let profile = <membership::members::Module<T>>::ensure_profile(actor.member_id)?;
+            ensure!(profile.root_account == sender || profile.controller_account == sender, "only member can unstake storage provider");
 
             let role_parameters = Self::ensure_role_parameters(actor.role)?;
 

src/roles/mock.rs

@@ -21,7 +21,7 @@ impl_outer_origin! {
 }
 
 pub fn alice_id() -> u32 {
-    Members::member_id_by_account_id(alice_account()).unwrap()
+    Members::member_ids_by_root_account_id(alice_account())[0]
 }
 pub fn alice_account() -> u64 {
     1

src/storage/data_directory.rs

@@ -161,7 +161,7 @@ decl_module! {
             ipfs_content_id: Vec<u8>
         ) {
             let who = ensure_signed(origin)?;
-            ensure!(<membership::members::Module<T>>::is_active_member(&who), MSG_CREATOR_MUST_BE_MEMBER);
+            ensure!(<membership::members::Module<T>>::is_member_account(&who), MSG_CREATOR_MUST_BE_MEMBER);
 
             ensure!(T::IsActiveDataObjectType::is_active_data_object_type(&type_id),
                 MSG_DO_TYPE_MUST_BE_ACTIVE);
@@ -211,7 +211,7 @@ decl_module! {
             update: ContentMetadataUpdate<T>
         ) {
             let who = ensure_signed(origin)?;
-            ensure!(<membership::members::Module<T>>::is_active_member(&who),
+            ensure!(<membership::members::Module<T>>::is_member_account(&who),
                 "Only active members can add content metadata");
 
             ensure!(!<MetadataByContentId<T>>::exists(&content_id),
@@ -248,7 +248,7 @@ decl_module! {
             let who = ensure_signed(origin)?;
 
             // Even if origin is an owner of metadata, they stil need to be an active member.
-            ensure!(<membership::members::Module<T>>::is_active_member(&who),
+            ensure!(<membership::members::Module<T>>::is_member_account(&who),
                 "Only active members can update content metadata");
 
             let has_updates = update.schema.is_some() || update.json.is_some();