update Members and Roles traits

src/governance/mock.rs

@@ -3,7 +3,7 @@
 use rstd::prelude::*;
 pub use super::{election, council, proposals, GovernanceCurrency};
 pub use system;
-use crate::traits;
+use crate::traits::{Members};
 
 pub use primitives::{H256, Blake2Hasher};
 pub use runtime_primitives::{
@@ -19,13 +19,19 @@ impl_outer_origin! {
 }
 
 pub struct MockMembership {}
-impl<T: system::Trait> traits::Members<T> for MockMembership {
+impl<T: system::Trait> Members<T> for MockMembership {
     type Id = u32;
+    fn is_active_member(who: &T::AccountId) -> bool {
+        // all accounts are considered members.
+        // There is currently no test coverage for non-members.
+        // Should add some coverage, and update this method to reflect which accounts are or are not members
+        true
+    }
+    fn lookup_member_id(account_id: &T::AccountId) -> Result<Self::Id, &'static str> {
+        Err("not implemented!")
+    }
 }
 
-// default trait implementation - any account is not a member
-// impl<T: system::Trait> traits::IsActiveMember<T> for () {}
-
 // For testing the module, we construct most of a mock runtime. This means
 // first constructing a configuration type (`Test`) which `impl`s each of the
 // configuration traits of modules we want to use.

src/governance/proposals.rs

@@ -1,4 +1,4 @@
-use srml_support::{StorageValue, StorageMap, dispatch::Result, decl_module, decl_event, decl_storage, ensure};
+use srml_support::{StorageValue, StorageMap, dispatch, decl_module, decl_event, decl_storage, ensure};
 use srml_support::traits::{Currency};
 use primitives::{storage::well_known_keys};
 use runtime_primitives::traits::{As, Hash, Zero};
@@ -368,7 +368,7 @@ impl<T: Trait> Module<T> {
         Self::current_block() >= proposed_at + Self::voting_period()
     }
 
-    fn _process_vote(voter: T::AccountId, proposal_id: u32, vote: VoteKind) -> Result {
+    fn _process_vote(voter: T::AccountId, proposal_id: u32, vote: VoteKind) -> dispatch::Result {
         let new_vote = (voter.clone(), vote.clone());
         if <VotesByProposal<T>>::exists(proposal_id) {
             // Append a new vote to other votes on this proposal:
@@ -382,7 +382,7 @@ impl<T: Trait> Module<T> {
         Ok(())
     }
 
-    fn end_block(now: T::BlockNumber) -> Result {
+    fn end_block(now: T::BlockNumber) -> dispatch::Result {
 
         // TODO refactor this method
 
@@ -395,7 +395,7 @@ impl<T: Trait> Module<T> {
     }
 
     /// Get the voters for the current proposal.
-    pub fn tally(/* proposal_id: u32 */) -> Result {
+    pub fn tally(/* proposal_id: u32 */) -> dispatch::Result {
 
         let councilors: u32 = Self::councilors_count();
         let quorum: u32 = Self::approval_quorum_seats();
@@ -475,7 +475,7 @@ impl<T: Trait> Module<T> {
     }
 
     /// Updates proposal status and removes proposal from active ids.
-    fn _update_proposal_status(proposal_id: u32, new_status: ProposalStatus) -> Result {
+    fn _update_proposal_status(proposal_id: u32, new_status: ProposalStatus) -> dispatch::Result {
         let all_active_ids = Self::active_proposal_ids();
         let all_len = all_active_ids.len();
         let other_active_ids: Vec<u32> = all_active_ids
@@ -503,7 +503,7 @@ impl<T: Trait> Module<T> {
     }
 
     /// Slash a proposal. The staked deposit will be slashed.
-    fn _slash_proposal(proposal_id: u32) -> Result {
+    fn _slash_proposal(proposal_id: u32) -> dispatch::Result {
         let proposal = Self::proposals(proposal_id);
 
         // Slash proposer's stake:
@@ -513,7 +513,7 @@ impl<T: Trait> Module<T> {
     }
 
     /// Reject a proposal. The staked deposit will be returned to a proposer.
-    fn _reject_proposal(proposal_id: u32) -> Result {
+    fn _reject_proposal(proposal_id: u32) -> dispatch::Result {
         let proposal = Self::proposals(proposal_id);
         let proposer = proposal.proposer;
 
@@ -529,7 +529,7 @@ impl<T: Trait> Module<T> {
     }
 
     /// Approve a proposal. The staked deposit will be returned.
-    fn _approve_proposal(proposal_id: u32) -> Result {
+    fn _approve_proposal(proposal_id: u32) -> dispatch::Result {
         let proposal = Self::proposals(proposal_id);
         let wasm_code = Self::wasm_code_by_hash(proposal.wasm_hash);
 
@@ -621,6 +621,15 @@ mod tests {
     pub struct MockMembership {}
     impl<T: system::Trait> Members<T> for MockMembership {
         type Id = u32;
+        fn is_active_member(who: &T::AccountId) -> bool {
+            // all accounts are considered members.
+            // There is currently no test coverage for non-members.
+            // Should add some coverage, and update this method to reflect which accounts are or are not members
+            true
+        }
+        fn lookup_member_id(account_id: &T::AccountId) -> Result<Self::Id, &'static str> {
+            Err("not implemented!")
+        }
     }
 
     type System = system::Module<Test>;
@@ -714,7 +723,7 @@ mod tests {
         b"Proposal Wasm Code".to_vec()
     }
 
-    fn _create_default_proposal() -> Result {
+    fn _create_default_proposal() -> dispatch::Result {
         _create_proposal(None, None, None, None, None)
     }
 
@@ -724,7 +733,7 @@ mod tests {
         name: Option<Vec<u8>>,
         description: Option<Vec<u8>>,
         wasm_code: Option<Vec<u8>>
-    ) -> Result {
+    ) -> dispatch::Result {
         Proposals::create_proposal(
             Origin::signed(origin.unwrap_or(PROPOSER1)),
             stake.unwrap_or(min_stake()),

src/lib.rs

@@ -237,6 +237,7 @@ impl membership::registry::Trait for Runtime {
 	type MemberId = u64;
 	type PaidTermId = u64;
 	type SubscriptionId = u64;
+	type Roles = Actors;
 }
 
 impl migration::Trait for Runtime {

src/membership/mock.rs

@@ -74,6 +74,7 @@ impl registry::Trait for Test {
     type MemberId = u32;
     type PaidTermId = u32;
     type SubscriptionId = u32;
+    type Roles = ();
 }
 
 pub struct ExtBuilder {

src/membership/registry.rs

@@ -9,7 +9,7 @@ use runtime_primitives::traits::{Zero, SimpleArithmetic, As, Member, MaybeSerial
 use system::{self, ensure_signed};
 use crate::governance::{GovernanceCurrency, BalanceOf };
 use {timestamp};
-use crate::traits;
+use crate::traits::{Members, Roles};
 
 pub trait Trait: system::Trait + GovernanceCurrency + timestamp::Trait {
     type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>;
@@ -22,6 +22,8 @@ pub trait Trait: system::Trait + GovernanceCurrency + timestamp::Trait {
 
     type SubscriptionId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy
         + As<usize> + As<u64> + MaybeSerializeDebug + PartialEq;
+
+    type Roles: Roles<Self>;
 }
 
 const DEFAULT_FIRST_MEMBER_ID: u64 = 1;
@@ -173,7 +175,7 @@ impl<T: Trait> Module<T> {
     }
 }
 
-impl<T: Trait> traits::Members<T> for Module<T> {
+impl<T: Trait> Members<T> for Module<T> {
     type Id = T::MemberId;
 
     fn is_active_member(who: &T::AccountId) -> bool {
@@ -206,6 +208,7 @@ decl_module! {
             Self::ensure_not_member(&who)?;
 
             // ensure account is not in a bonded role
+            ensure!(!T::Roles::is_role_key(&who), "role key cannot be used for membership");
 
             // ensure paid_terms_id is active
             let terms = Self::ensure_active_terms_id(paid_terms_id)?;
@@ -280,6 +283,7 @@ decl_module! {
             Self::ensure_not_member(&new_member)?;
 
             // ensure account is not in a bonded role
+            ensure!(!T::Roles::is_role_key(&new_member), "role key cannot be used for membership");
 
             let user_info = Self::check_user_registration_info(user_info)?;
 

src/roles/actors.rs

@@ -10,7 +10,7 @@ use system::{self, ensure_signed};
 use crate::governance::{GovernanceCurrency, BalanceOf };
 use crate::membership::registry;
 
-use crate::traits::{Members};
+use crate::traits::{Members, Roles};
 
 #[derive(Encode, Decode, Copy, Clone, Eq, PartialEq, Debug)]
 pub enum Role {
@@ -85,6 +85,12 @@ impl<T: Trait> Module<T> {
     }
 }
 
+impl<T: Trait> Roles<T> for Module<T> {
+    fn is_role_key(account_id: &T::AccountId) -> bool {
+        Self::actors(account_id).is_some()
+    }
+}
+
 decl_module! {
     pub struct Module<T: Trait> for enum Call where origin: T::Origin {
         fn deposit_event<T>() = default;

src/traits.rs

@@ -5,15 +5,31 @@ use parity_codec::Codec;
 use srml_support::{Parameter};
 use runtime_primitives::traits::{SimpleArithmetic, As, Member, MaybeSerializeDebug};
 
+// Members
 pub trait Members<T: system::Trait> {
     type Id : Parameter + Member + SimpleArithmetic + Codec + Default + Copy
         + As<usize> + As<u64> + MaybeSerializeDebug + PartialEq;
 
+    fn is_active_member(account_id: &T::AccountId) -> bool;
+
+    fn lookup_member_id(account_id: &T::AccountId) -> Result<Self::Id, &'static str>;
+}
+
+impl<T: system::Trait> Members<T> for () {
+    type Id = u32;
     fn is_active_member(account_id: &T::AccountId) -> bool {
-        true
+        false
     }
-
     fn lookup_member_id(account_id: &T::AccountId) -> Result<Self::Id, &'static str> {
         Err("member not found")
     }
+}
+
+// Roles
+pub trait Roles<T: system::Trait> {
+    fn is_role_key(account_id: &T::AccountId) -> bool;
+}
+
+impl<T: system::Trait> Roles<T> for () {
+	fn is_role_key(_who: &T::AccountId) -> bool { false }
 }