From 43376ab55f62dd70b33f030da5f46ff093d808c0 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 13 Mar 2019 10:22:42 +0200 Subject: [PATCH 01/33] adding membership module - storage --- src/lib.rs | 9 ++++ src/membership.rs | 109 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 src/membership.rs diff --git a/src/lib.rs b/src/lib.rs index 77adf189ae..8bf9712edc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,6 +17,7 @@ extern crate parity_codec_derive; pub mod governance; use governance::{election, council, proposals}; mod memo; +mod membership; use rstd::prelude::*; #[cfg(feature = "std")] @@ -224,6 +225,13 @@ impl memo::Trait for Runtime { type Event = Event; } +impl membership::Trait for Runtime { + type Event = Event; + type MemberId = u64; + type PaidTermId = u32; + type SubscriptionId = u32; +} + construct_runtime!( pub enum Runtime with Log(InternalLog: DigestItem) where Block = Block, @@ -244,6 +252,7 @@ construct_runtime!( CouncilElection: election::{Module, Call, Storage, Event, Config}, Council: council::{Module, Call, Storage, Event, Config}, Memo: memo::{Module, Call, Storage, Event}, + Membership: membership, } ); diff --git a/src/membership.rs b/src/membership.rs new file mode 100644 index 0000000000..ec4d9c6cb8 --- /dev/null +++ b/src/membership.rs @@ -0,0 +1,109 @@ +#![cfg_attr(not(feature = "std"), no_std)] + +use rstd::prelude::*; +use parity_codec::Codec; +use parity_codec_derive::{Encode, Decode}; +use srml_support::{StorageMap, dispatch::Result, decl_module, decl_storage, decl_event, ensure, Parameter}; +use srml_support::traits::{Currency}; +use runtime_primitives::traits::{Zero, SimpleArithmetic, As, Member, MaybeSerializeDebug}; +use system::{self, ensure_signed}; +use crate::governance::{GovernanceCurrency, BalanceOf }; + +pub trait Trait: system::Trait + GovernanceCurrency { + type Event: From> + Into<::Event>; + + type MemberId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As + As + MaybeSerializeDebug; + + type PaidTermId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As + As + MaybeSerializeDebug; + + type SubscriptionId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As + As + MaybeSerializeDebug; +} + +#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +#[derive(Encode, Decode)] +pub struct Profile { + id: T::MemberId, // is it necessary to have the id in the struct? + handle: u32, + avatarUri: Vec, + description: Vec, + added: T::BlockNumber, + entry: EntryMethod, + suspended: bool, + subscription: Option +} + +#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +#[derive(Encode, Decode)] +pub enum EntryMethod { + Paid(T::PaidTermId), + Screening(T::AccountId), +} + +#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +#[derive(Encode, Decode)] +pub struct PaidMembershipTerms { + /// Unique identifier - the term id + id: T::PaidTermId, // is it necessary to have id in the struct? + /// Quantity of native tokens which must be provably burned + fee: BalanceOf, + /// String of capped length describing human readable conditions which are being agreed upon + text: Vec +} + +// Start at 1001? instead to reserve first 1000 ids ? +const FIRST_MEMBER_ID: u64 = 1; +const INITIAL_PAID_TERMS_ID: u64 = 1; + +// TEST: do initial values and build methods get called for store items when the runtime is +// an upgrade? if not we need a differnet way to set them... +decl_storage! { + trait Store for Module as MembersipRegistry { + FirstMemberId get(first_member_id) : T::MemberId = T::MemberId::sa(FIRST_MEMBER_ID); + + /// Id to assign to next member that is added to the registry + NextMemberId get(next_member_id) : T::MemberId = T::MemberId::sa(FIRST_MEMBER_ID); + + /// Mapping of member ids to their corresponding accountid + MembersById get(members_by_id) : map T::MemberId => T::AccountId; + + /// Mapping of members' accountid to their member id + MemberByAccount get(members_by_account) : map T::AccountId => T::MemberId; + + /// Mapping of member's id to their membership profile + // Value is Option because it is not meaningful to have a Default value for Profile + MemberProfile get(member_profile_preview) : map T::MemberId => Option>; + + /// Next paid membership terms id - 1 reserved for initial terms, (avoid 0 -> default value) + NextPaidMembershipTermsId get(next_paid_membership_terms_id) : T::PaidTermId = T::PaidTermId::sa(2); + + /// Paid membership terms record + // Value is an Option because it is not meanigful to have a Default value for a PaidMembershipTerms + // build method should return a vector of tuple(key, value) + // will this method even be called for a runtime upgrade? + PaidMembershipTermsById get(paid_membership_terms_by_id) build(|_| vec![(T::PaidTermId::sa(INITIAL_PAID_TERMS_ID), Some(PaidMembershipTerms { + id: T::PaidTermId::sa(INITIAL_PAID_TERMS_ID), + fee: BalanceOf::::sa(100), + text: String::from("Basic Membership TOS").into_bytes() + }))]) : map T::PaidTermId => Option>; + + /// Active Paid membership terms + ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec = vec![T::PaidTermId::sa(INITIAL_PAID_TERMS_ID)]; + + /// Is the platform is accepting new members or not + PlatformAcceptingNewMemberships get(platform_accepting_new_memberships) : bool = true; + } +} + +decl_event! { + pub enum Event where + ::AccountId, + ::MemberId { + MemberAdded(MemberId, AccountId), + } +} + +decl_module! { + pub struct Module for enum Call where origin: T::Origin { + fn deposit_event() = default; + } +} From 216e694afcddeaab01f79d8a6f9f6e36b6d9aab7 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 13 Mar 2019 13:47:12 +0200 Subject: [PATCH 02/33] use default implicit value for paid terms instead of initializing map --- src/lib.rs | 4 ++-- src/membership.rs | 33 ++++++++++++++++++++------------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 8bf9712edc..08eba2b1c9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -88,7 +88,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("joystream-node"), impl_name: create_runtime_str!("joystream-node"), authoring_version: 3, - spec_version: 4, + spec_version: 5, impl_version: 0, apis: RUNTIME_API_VERSIONS, }; @@ -252,7 +252,7 @@ construct_runtime!( CouncilElection: election::{Module, Call, Storage, Event, Config}, Council: council::{Module, Call, Storage, Event, Config}, Memo: memo::{Module, Call, Storage, Event}, - Membership: membership, + Membership: membership::{Module, Call, Storage, Event}, } ); diff --git a/src/membership.rs b/src/membership.rs index ec4d9c6cb8..33243cb12c 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -19,6 +19,14 @@ pub trait Trait: system::Trait + GovernanceCurrency { type SubscriptionId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As + As + MaybeSerializeDebug; } +const FIRST_MEMBER_ID: u64 = 1; +const FIRST_PAID_TERMS_ID: u64 = 1; + +// Default "implicit" paid terms +const DEFAULT_PAID_TERM_ID: u64 = 0; +const DEFAULT_PAID_TERM_FEE: u64 = 100; +const DEFAULT_PAID_TERM_TEXT: &str = "Default Paid Term TOS..."; + #[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] #[derive(Encode, Decode)] pub struct Profile { @@ -50,9 +58,15 @@ pub struct PaidMembershipTerms { text: Vec } -// Start at 1001? instead to reserve first 1000 ids ? -const FIRST_MEMBER_ID: u64 = 1; -const INITIAL_PAID_TERMS_ID: u64 = 1; +impl Default for PaidMembershipTerms { + fn default() -> Self { + PaidMembershipTerms { + id: T::PaidTermId::sa(DEFAULT_PAID_TERM_ID), + fee: BalanceOf::::sa(DEFAULT_PAID_TERM_FEE), + text: DEFAULT_PAID_TERM_TEXT.as_bytes().to_vec() + } + } +} // TEST: do initial values and build methods get called for store items when the runtime is // an upgrade? if not we need a differnet way to set them... @@ -74,20 +88,13 @@ decl_storage! { MemberProfile get(member_profile_preview) : map T::MemberId => Option>; /// Next paid membership terms id - 1 reserved for initial terms, (avoid 0 -> default value) - NextPaidMembershipTermsId get(next_paid_membership_terms_id) : T::PaidTermId = T::PaidTermId::sa(2); + NextPaidMembershipTermsId get(next_paid_membership_terms_id) : T::PaidTermId = T::PaidTermId::sa(FIRST_PAID_TERMS_ID); /// Paid membership terms record - // Value is an Option because it is not meanigful to have a Default value for a PaidMembershipTerms - // build method should return a vector of tuple(key, value) - // will this method even be called for a runtime upgrade? - PaidMembershipTermsById get(paid_membership_terms_by_id) build(|_| vec![(T::PaidTermId::sa(INITIAL_PAID_TERMS_ID), Some(PaidMembershipTerms { - id: T::PaidTermId::sa(INITIAL_PAID_TERMS_ID), - fee: BalanceOf::::sa(100), - text: String::from("Basic Membership TOS").into_bytes() - }))]) : map T::PaidTermId => Option>; + PaidMembershipTermsById get(paid_membership_terms_by_id) : map T::PaidTermId => PaidMembershipTerms; /// Active Paid membership terms - ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec = vec![T::PaidTermId::sa(INITIAL_PAID_TERMS_ID)]; + ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec = vec![T::PaidTermId::sa(DEFAULT_PAID_TERM_ID)]; /// Is the platform is accepting new members or not PlatformAcceptingNewMemberships get(platform_accepting_new_memberships) : bool = true; From a7d4aac9f130d4e3da1c24b79f91d6c122255202 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 14 Mar 2019 02:47:51 +0200 Subject: [PATCH 03/33] add migration --- Cargo.toml | 4 +-- src/lib.rs | 2 +- src/membership.rs | 69 +++++++++++++++++++++++++++++++++++------------ 3 files changed, 55 insertions(+), 20 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9433296744..75080094ef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -75,11 +75,11 @@ rev = 'df5e65927780b323482e2e8b5031822f423a032d' [dependencies.parity-codec] default-features = false -version = '3.0' +version = '3.1' [dependencies.parity-codec-derive] default-features = false -version = '3.0' +version = '3.1' [dependencies.primitives] default_features = false diff --git a/src/lib.rs b/src/lib.rs index 08eba2b1c9..b7d70cb892 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -252,7 +252,7 @@ construct_runtime!( CouncilElection: election::{Module, Call, Storage, Event, Config}, Council: council::{Module, Call, Storage, Event, Config}, Memo: memo::{Module, Call, Storage, Event}, - Membership: membership::{Module, Call, Storage, Event}, + Membership: membership::{Module, Call, Storage, Event, Config}, } ); diff --git a/src/membership.rs b/src/membership.rs index 33243cb12c..01f7a418bf 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -3,11 +3,12 @@ use rstd::prelude::*; use parity_codec::Codec; use parity_codec_derive::{Encode, Decode}; -use srml_support::{StorageMap, dispatch::Result, decl_module, decl_storage, decl_event, ensure, Parameter}; +use srml_support::{StorageMap, StorageValue, dispatch::Result, decl_module, decl_storage, decl_event, ensure, Parameter}; use srml_support::traits::{Currency}; use runtime_primitives::traits::{Zero, SimpleArithmetic, As, Member, MaybeSerializeDebug}; use system::{self, ensure_signed}; use crate::governance::{GovernanceCurrency, BalanceOf }; +use runtime_io::print; pub trait Trait: system::Trait + GovernanceCurrency { type Event: From> + Into<::Event>; @@ -19,20 +20,20 @@ pub trait Trait: system::Trait + GovernanceCurrency { type SubscriptionId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As + As + MaybeSerializeDebug; } -const FIRST_MEMBER_ID: u64 = 1; +const DEFAULT_FIRST_MEMBER_ID: u64 = 1; const FIRST_PAID_TERMS_ID: u64 = 1; -// Default "implicit" paid terms +// Default paid membership terms const DEFAULT_PAID_TERM_ID: u64 = 0; -const DEFAULT_PAID_TERM_FEE: u64 = 100; +const DEFAULT_PAID_TERM_FEE: u64 = 100; // Can be overidden in genesis config const DEFAULT_PAID_TERM_TEXT: &str = "Default Paid Term TOS..."; -#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +//#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] #[derive(Encode, Decode)] pub struct Profile { - id: T::MemberId, // is it necessary to have the id in the struct? + id: T::MemberId, handle: u32, - avatarUri: Vec, + avatar_uri: Vec, description: Vec, added: T::BlockNumber, entry: EntryMethod, @@ -40,18 +41,18 @@ pub struct Profile { subscription: Option } -#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +//#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] #[derive(Encode, Decode)] pub enum EntryMethod { Paid(T::PaidTermId), Screening(T::AccountId), } -#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +//#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] #[derive(Encode, Decode)] pub struct PaidMembershipTerms { /// Unique identifier - the term id - id: T::PaidTermId, // is it necessary to have id in the struct? + id: T::PaidTermId, /// Quantity of native tokens which must be provably burned fee: BalanceOf, /// String of capped length describing human readable conditions which are being agreed upon @@ -68,14 +69,12 @@ impl Default for PaidMembershipTerms { } } -// TEST: do initial values and build methods get called for store items when the runtime is -// an upgrade? if not we need a differnet way to set them... decl_storage! { - trait Store for Module as MembersipRegistry { - FirstMemberId get(first_member_id) : T::MemberId = T::MemberId::sa(FIRST_MEMBER_ID); + trait Store for Module as Membership { + FirstMemberId get(first_member_id) config(first_member_id): T::MemberId = T::MemberId::sa(DEFAULT_FIRST_MEMBER_ID); /// Id to assign to next member that is added to the registry - NextMemberId get(next_member_id) : T::MemberId = T::MemberId::sa(FIRST_MEMBER_ID); + NextMemberId get(next_member_id) build(|config: &GenesisConfig| config.first_member_id): T::MemberId = T::MemberId::sa(DEFAULT_FIRST_MEMBER_ID); /// Mapping of member ids to their corresponding accountid MembersById get(members_by_id) : map T::MemberId => T::AccountId; @@ -87,17 +86,35 @@ decl_storage! { // Value is Option because it is not meaningful to have a Default value for Profile MemberProfile get(member_profile_preview) : map T::MemberId => Option>; - /// Next paid membership terms id - 1 reserved for initial terms, (avoid 0 -> default value) + /// Next paid membership terms id NextPaidMembershipTermsId get(next_paid_membership_terms_id) : T::PaidTermId = T::PaidTermId::sa(FIRST_PAID_TERMS_ID); /// Paid membership terms record - PaidMembershipTermsById get(paid_membership_terms_by_id) : map T::PaidTermId => PaidMembershipTerms; + // Remember to add _genesis_phantom_data: std::marker::PhantomData{} to membership + // genesis config if not providing config() or extra_genesis + PaidMembershipTermsById get(paid_membership_terms_by_id) build(|config: &GenesisConfig| { + // This method only gets called when initializing storage, and is + // compiled as native code. (Will be called when building `raw` chainspec) + // So it can't be relied upon to initialize storage for runtimes updates. + // Initialization for updated runtime is done in run_migration() + let mut terms: PaidMembershipTerms = Default::default(); + terms.fee = BalanceOf::::sa(config.default_paid_membership_fee); + vec![(terms.id, terms)] + }) : map T::PaidTermId => Option>; /// Active Paid membership terms ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec = vec![T::PaidTermId::sa(DEFAULT_PAID_TERM_ID)]; /// Is the platform is accepting new members or not PlatformAcceptingNewMemberships get(platform_accepting_new_memberships) : bool = true; + + /// If we should run a migration and initialize new storage values introduced in new runtime + // This will initialize to false if starting at genesis (because the build() method will run), + // for a runtime upgrade it will return the default value when reading the first time. + ShouldRunMigration get(should_run_migration) build(|_| false) : bool = true; + } + add_extra_genesis { + config(default_paid_membership_fee): u64; } } @@ -109,8 +126,26 @@ decl_event! { } } +impl Module { + fn run_migration() { + if Self::should_run_migration() { + let default_terms: PaidMembershipTerms = Default::default(); + >::insert(T::PaidTermId::sa(0), default_terms); + >::put(false); + } + } +} + decl_module! { pub struct Module for enum Call where origin: T::Origin { fn deposit_event() = default; + + fn on_initialise(_now: T::BlockNumber) { + Self::run_migration(); + } + + fn on_finalise(_now: T::BlockNumber) { + + } } } From 6923186c68dccc57c699b21e31d41af1141fc58f Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 14 Mar 2019 08:59:29 +0200 Subject: [PATCH 04/33] renaming, adding handles map --- src/membership.rs | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/membership.rs b/src/membership.rs index 01f7a418bf..903d6928f1 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -8,9 +8,10 @@ use srml_support::traits::{Currency}; use runtime_primitives::traits::{Zero, SimpleArithmetic, As, Member, MaybeSerializeDebug}; use system::{self, ensure_signed}; use crate::governance::{GovernanceCurrency, BalanceOf }; -use runtime_io::print; +//use runtime_io::print; +use {timestamp}; -pub trait Trait: system::Trait + GovernanceCurrency { +pub trait Trait: system::Trait + GovernanceCurrency + timestamp::Trait { type Event: From> + Into<::Event>; type MemberId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As + As + MaybeSerializeDebug; @@ -32,10 +33,11 @@ const DEFAULT_PAID_TERM_TEXT: &str = "Default Paid Term TOS..."; #[derive(Encode, Decode)] pub struct Profile { id: T::MemberId, - handle: u32, + handle: Vec, avatar_uri: Vec, - description: Vec, - added: T::BlockNumber, + about: Vec, + registered_at_block: T::BlockNumber, + registered_at_time: T::Moment, entry: EntryMethod, suspended: bool, subscription: Option @@ -71,21 +73,25 @@ impl Default for PaidMembershipTerms { decl_storage! { trait Store for Module as Membership { + /// MemberId's start at this value FirstMemberId get(first_member_id) config(first_member_id): T::MemberId = T::MemberId::sa(DEFAULT_FIRST_MEMBER_ID); - /// Id to assign to next member that is added to the registry + /// MemberId to assign to next member that is added to the registry NextMemberId get(next_member_id) build(|config: &GenesisConfig| config.first_member_id): T::MemberId = T::MemberId::sa(DEFAULT_FIRST_MEMBER_ID); /// Mapping of member ids to their corresponding accountid - MembersById get(members_by_id) : map T::MemberId => T::AccountId; + AccountIdByMemberId get(account_id_by_member_id) : map T::MemberId => T::AccountId; /// Mapping of members' accountid to their member id - MemberByAccount get(members_by_account) : map T::AccountId => T::MemberId; + MemberIdByAccountId get(member_id_by_account_id) : map T::AccountId => T::MemberId; /// Mapping of member's id to their membership profile // Value is Option because it is not meaningful to have a Default value for Profile MemberProfile get(member_profile_preview) : map T::MemberId => Option>; + /// Registered unique handles and their mapping to their owner + Handles get(handles) : map Vec => Option; + /// Next paid membership terms id NextPaidMembershipTermsId get(next_paid_membership_terms_id) : T::PaidTermId = T::PaidTermId::sa(FIRST_PAID_TERMS_ID); @@ -106,7 +112,7 @@ decl_storage! { ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec = vec![T::PaidTermId::sa(DEFAULT_PAID_TERM_ID)]; /// Is the platform is accepting new members or not - PlatformAcceptingNewMemberships get(platform_accepting_new_memberships) : bool = true; + NewMembershipsAllowed get(new_memberships_allowed) : bool = true; /// If we should run a migration and initialize new storage values introduced in new runtime // This will initialize to false if starting at genesis (because the build() method will run), @@ -122,7 +128,7 @@ decl_event! { pub enum Event where ::AccountId, ::MemberId { - MemberAdded(MemberId, AccountId), + MemberRegistered(MemberId, AccountId), } } @@ -147,5 +153,6 @@ decl_module! { fn on_finalise(_now: T::BlockNumber) { } + } } From 1237df8c8e984b14ae6b47bb65932cc0be80e784 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 14 Mar 2019 12:26:56 +0200 Subject: [PATCH 05/33] membership: implement buy_membership --- src/lib.rs | 4 +- src/membership.rs | 127 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 125 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b7d70cb892..b8b0ac8764 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -228,8 +228,8 @@ impl memo::Trait for Runtime { impl membership::Trait for Runtime { type Event = Event; type MemberId = u64; - type PaidTermId = u32; - type SubscriptionId = u32; + type PaidTermId = u64; + type SubscriptionId = u64; } construct_runtime!( diff --git a/src/membership.rs b/src/membership.rs index 903d6928f1..4e91b6255a 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -3,7 +3,7 @@ use rstd::prelude::*; use parity_codec::Codec; use parity_codec_derive::{Encode, Decode}; -use srml_support::{StorageMap, StorageValue, dispatch::Result, decl_module, decl_storage, decl_event, ensure, Parameter}; +use srml_support::{StorageMap, StorageValue, dispatch, decl_module, decl_storage, decl_event, ensure, Parameter}; use srml_support::traits::{Currency}; use runtime_primitives::traits::{Zero, SimpleArithmetic, As, Member, MaybeSerializeDebug}; use system::{self, ensure_signed}; @@ -14,11 +14,14 @@ use {timestamp}; pub trait Trait: system::Trait + GovernanceCurrency + timestamp::Trait { type Event: From> + Into<::Event>; - type MemberId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As + As + MaybeSerializeDebug; + type MemberId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + + As + As + MaybeSerializeDebug + PartialEq; - type PaidTermId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As + As + MaybeSerializeDebug; + type PaidTermId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + + As + As + MaybeSerializeDebug + PartialEq; - type SubscriptionId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As + As + MaybeSerializeDebug; + type SubscriptionId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + + As + As + MaybeSerializeDebug + PartialEq; } const DEFAULT_FIRST_MEMBER_ID: u64 = 1; @@ -29,8 +32,15 @@ const DEFAULT_PAID_TERM_ID: u64 = 0; const DEFAULT_PAID_TERM_FEE: u64 = 100; // Can be overidden in genesis config const DEFAULT_PAID_TERM_TEXT: &str = "Default Paid Term TOS..."; +// Default user info constraints +const DEFAULT_MIN_HANDLE_LENGTH: u32 = 4; +const DEFAULT_MAX_HANDLE_LENGTH: u32 = 20; +const DEFAULT_MAX_AVATAR_URI_LENGTH: u32 = 512; +const DEFAULT_MAX_ABOUT_TEXT_LENGTH: u32 = 1024; + //#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] #[derive(Encode, Decode)] +/// Stored information about a registered user pub struct Profile { id: T::MemberId, handle: Vec, @@ -43,6 +53,20 @@ pub struct Profile { subscription: Option } +#[derive(Clone, Debug, Encode, Decode, PartialEq)] +/// Structure used to batch user configurable profile information when registering or updating info +pub struct UserInfo { + handle: Option>, + avatar_uri: Option>, + about: Option>, +} + +struct CheckedUserInfo { + handle: Vec, + avatar_uri: Vec, + about: Vec, +} + //#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] #[derive(Encode, Decode)] pub enum EntryMethod { @@ -118,6 +142,13 @@ decl_storage! { // This will initialize to false if starting at genesis (because the build() method will run), // for a runtime upgrade it will return the default value when reading the first time. ShouldRunMigration get(should_run_migration) build(|_| false) : bool = true; + + // User Input Validation parameters - do these really need to be state variables + // I don't see a need to adjust these in future? + MinHandleLength get(min_handle_length) : u32 = DEFAULT_MIN_HANDLE_LENGTH; + MaxHandleLength get(max_handle_length) : u32 = DEFAULT_MAX_HANDLE_LENGTH; + MaxAvatarUriLength get(max_avatar_uri_length) : u32 = DEFAULT_MAX_AVATAR_URI_LENGTH; + MaxAboutTextLength get(max_about_text_length) : u32 = DEFAULT_MAX_ABOUT_TEXT_LENGTH; } add_extra_genesis { config(default_paid_membership_fee): u64; @@ -154,5 +185,93 @@ decl_module! { } + /// Non-members can buy membership + fn buy_membership(origin, paid_terms_id: T::PaidTermId, user_info: UserInfo) { + let who = ensure_signed(origin)?; + + // ensure key not associated with an existing membership + Self::ensure_not_member(&who)?; + + // ensure paid_terms_id is active + Self::ensure_terms_id_is_active(paid_terms_id)?; + + let terms = Self::paid_membership_terms_by_id(paid_terms_id).ok_or("paid membership term id does not exist")?; + + // ensure enough free balance to cover terms fees + ensure!(T::Currency::free_balance(&who) >= terms.fee, "not enough balance to buy membership"); + + let user_info = Self::check_user_info(user_info)?; + + let member_id = Self::insert_new_paid_member(&who, paid_terms_id, &user_info)?; + + Self::deposit_event(RawEvent::MemberRegistered(member_id, who.clone())); + } } } + +impl Module { + fn ensure_not_member(who: &T::AccountId) -> dispatch::Result { + ensure!(!>::exists(who), "Account already associated with a membership"); + Ok(()) + } + + fn ensure_terms_id_is_active(terms_id: T::PaidTermId) -> dispatch::Result { + let active_terms = Self::active_paid_membership_terms(); + ensure!(active_terms.iter().any(|&id| id == terms_id), "paid terms id not active"); + Ok(()) + } + + fn ensure_unique_handle(handle: &Vec ) -> dispatch::Result { + ensure!(!>::exists(handle), "handle already registered"); + Ok(()) + } + + /// Basic user input validation + fn check_user_info(user_info: UserInfo) -> Result { + // Handle is required during registration + let handle = user_info.handle.ok_or("handle must be provided during registration")?; + ensure!(handle.len() >= Self::min_handle_length() as usize, "handle too short"); + ensure!(handle.len() <= Self::max_handle_length() as usize, "handle too long"); + + let mut about = user_info.about.unwrap_or_default(); + about.truncate(Self::max_about_text_length() as usize); + + let mut avatar_uri = user_info.avatar_uri.unwrap_or_default(); + avatar_uri.truncate(Self::max_avatar_uri_length() as usize); + + Ok(CheckedUserInfo { + handle, + avatar_uri, + about, + }) + } + + // Mutating methods + + fn insert_new_paid_member(who: &T::AccountId, paid_terms_id: T::PaidTermId, user_info: &CheckedUserInfo) -> Result { + // ensure handle is not already registered + Self::ensure_unique_handle(&user_info.handle)?; + + let new_member_id = Self::next_member_id(); + + let profile: Profile = Profile { + id: new_member_id, + handle: user_info.handle.clone(), + avatar_uri: user_info.avatar_uri.clone(), + about: user_info.about.clone(), + registered_at_block: >::block_number(), + registered_at_time: >::now(), + entry: EntryMethod::Paid(paid_terms_id), + suspended: false, + subscription: None, + }; + + >::insert(who.clone(), new_member_id); + >::insert(new_member_id, who.clone()); + >::insert(new_member_id, profile); + >::insert(user_info.handle.clone(), new_member_id); + >::mutate(|n| { *n += T::MemberId::sa(1); }); + + Ok(new_member_id) + } +} \ No newline at end of file From 918868e1735e2dd6bbd0613032b920f1c1011d55 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 14 Mar 2019 13:15:48 +0200 Subject: [PATCH 06/33] make avatar_uri an Option instead of empty string to represent no uri --- src/membership.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/membership.rs b/src/membership.rs index 4e91b6255a..4b757b8cc6 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -44,7 +44,7 @@ const DEFAULT_MAX_ABOUT_TEXT_LENGTH: u32 = 1024; pub struct Profile { id: T::MemberId, handle: Vec, - avatar_uri: Vec, + avatar_uri: Option>, about: Vec, registered_at_block: T::BlockNumber, registered_at_time: T::Moment, @@ -63,7 +63,7 @@ pub struct UserInfo { struct CheckedUserInfo { handle: Vec, - avatar_uri: Vec, + avatar_uri: Option>, about: Vec, } @@ -236,8 +236,11 @@ impl Module { let mut about = user_info.about.unwrap_or_default(); about.truncate(Self::max_about_text_length() as usize); - let mut avatar_uri = user_info.avatar_uri.unwrap_or_default(); - avatar_uri.truncate(Self::max_avatar_uri_length() as usize); + let avatar_uri = user_info.avatar_uri.and_then(|uri: Vec| { + let mut uri = uri.clone(); + uri.truncate(Self::max_avatar_uri_length() as usize); + Some(uri) + }); Ok(CheckedUserInfo { handle, From cb1a3a7d69660fd414900709b2bc80166fb55bf4 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 14 Mar 2019 13:16:40 +0200 Subject: [PATCH 07/33] add simple buy membership method --- src/membership.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/membership.rs b/src/membership.rs index 4b757b8cc6..21644ba152 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -206,6 +206,15 @@ decl_module! { Self::deposit_event(RawEvent::MemberRegistered(member_id, who.clone())); } + + /// Buy the default membership (if it is active) and only provide handle - for testing + fn buy_default_membership_testing(origin, handle: Vec) { + Self::buy_membership(origin, T::PaidTermId::sa(0), UserInfo { + handle: Some(handle.clone()), + avatar_uri: None, + about: None + })?; + } } } From 6e202d9850ce3471fad54f2dc732fc36de1b8169 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 14 Mar 2019 13:33:43 +0200 Subject: [PATCH 08/33] burn fee when buying membership --- src/membership.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/membership.rs b/src/membership.rs index 21644ba152..03e38d6fe9 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -195,11 +195,6 @@ decl_module! { // ensure paid_terms_id is active Self::ensure_terms_id_is_active(paid_terms_id)?; - let terms = Self::paid_membership_terms_by_id(paid_terms_id).ok_or("paid membership term id does not exist")?; - - // ensure enough free balance to cover terms fees - ensure!(T::Currency::free_balance(&who) >= terms.fee, "not enough balance to buy membership"); - let user_info = Self::check_user_info(user_info)?; let member_id = Self::insert_new_paid_member(&who, paid_terms_id, &user_info)?; @@ -260,7 +255,12 @@ impl Module { // Mutating methods - fn insert_new_paid_member(who: &T::AccountId, paid_terms_id: T::PaidTermId, user_info: &CheckedUserInfo) -> Result { + fn insert_new_paid_member(who: &T::AccountId, paid_terms_id: T::PaidTermId, user_info: &CheckedUserInfo) -> Result { + let terms = Self::paid_membership_terms_by_id(paid_terms_id).ok_or("paid membership term id does not exist")?; + // ensure enough free balance to cover terms fees + ensure!(T::Currency::can_slash(&who, terms.fee), "not enough balance to buy membership"); + let _ = T::Currency::slash(&who, terms.fee); + // ensure handle is not already registered Self::ensure_unique_handle(&user_info.handle)?; From 54738e74815055501d573325ee17d04f788cd0ba Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 14 Mar 2019 13:58:20 +0200 Subject: [PATCH 09/33] keep extrinsic validation outside insert_new_paid_member --- src/membership.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/membership.rs b/src/membership.rs index 03e38d6fe9..0e8e57bc36 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -195,9 +195,17 @@ decl_module! { // ensure paid_terms_id is active Self::ensure_terms_id_is_active(paid_terms_id)?; + // ensure enough free balance to cover terms fees + let terms = Self::paid_membership_terms_by_id(paid_terms_id).ok_or("paid membership term id does not exist")?; + ensure!(T::Currency::can_slash(&who, terms.fee), "not enough balance to buy membership"); + let user_info = Self::check_user_info(user_info)?; - let member_id = Self::insert_new_paid_member(&who, paid_terms_id, &user_info)?; + // ensure handle is not already registered + Self::ensure_unique_handle(&user_info.handle)?; + + let _ = T::Currency::slash(&who, terms.fee); + let member_id = Self::insert_new_paid_member(&who, paid_terms_id, &user_info); Self::deposit_event(RawEvent::MemberRegistered(member_id, who.clone())); } @@ -255,15 +263,7 @@ impl Module { // Mutating methods - fn insert_new_paid_member(who: &T::AccountId, paid_terms_id: T::PaidTermId, user_info: &CheckedUserInfo) -> Result { - let terms = Self::paid_membership_terms_by_id(paid_terms_id).ok_or("paid membership term id does not exist")?; - // ensure enough free balance to cover terms fees - ensure!(T::Currency::can_slash(&who, terms.fee), "not enough balance to buy membership"); - let _ = T::Currency::slash(&who, terms.fee); - - // ensure handle is not already registered - Self::ensure_unique_handle(&user_info.handle)?; - + fn insert_new_paid_member(who: &T::AccountId, paid_terms_id: T::PaidTermId, user_info: &CheckedUserInfo) -> T::MemberId { let new_member_id = Self::next_member_id(); let profile: Profile = Profile { @@ -284,6 +284,6 @@ impl Module { >::insert(user_info.handle.clone(), new_member_id); >::mutate(|n| { *n += T::MemberId::sa(1); }); - Ok(new_member_id) + new_member_id } } \ No newline at end of file From e99cb532a1ad1e4d50eb48d1439f002150bfd1af Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 14 Mar 2019 17:22:02 +0200 Subject: [PATCH 10/33] update methods for profile --- src/membership.rs | 104 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 84 insertions(+), 20 deletions(-) diff --git a/src/membership.rs b/src/membership.rs index 0e8e57bc36..0f06a1c53c 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -44,7 +44,7 @@ const DEFAULT_MAX_ABOUT_TEXT_LENGTH: u32 = 1024; pub struct Profile { id: T::MemberId, handle: Vec, - avatar_uri: Option>, + avatar_uri: Vec, about: Vec, registered_at_block: T::BlockNumber, registered_at_time: T::Moment, @@ -63,7 +63,7 @@ pub struct UserInfo { struct CheckedUserInfo { handle: Vec, - avatar_uri: Option>, + avatar_uri: Vec, about: Vec, } @@ -107,11 +107,11 @@ decl_storage! { AccountIdByMemberId get(account_id_by_member_id) : map T::MemberId => T::AccountId; /// Mapping of members' accountid to their member id - MemberIdByAccountId get(member_id_by_account_id) : map T::AccountId => T::MemberId; + MemberIdByAccountId get(member_id_by_account_id) : map T::AccountId => Option; /// Mapping of member's id to their membership profile // Value is Option because it is not meaningful to have a Default value for Profile - MemberProfile get(member_profile_preview) : map T::MemberId => Option>; + MemberProfile get(member_profile) : map T::MemberId => Option>; /// Registered unique handles and their mapping to their owner Handles get(handles) : map Vec => Option; @@ -160,6 +160,8 @@ decl_event! { ::AccountId, ::MemberId { MemberRegistered(MemberId, AccountId), + MemberUpdatedAboutText(MemberId), + MemberUpdatedAvatar(MemberId), } } @@ -193,13 +195,12 @@ decl_module! { Self::ensure_not_member(&who)?; // ensure paid_terms_id is active - Self::ensure_terms_id_is_active(paid_terms_id)?; + let terms = Self::ensure_active_terms_id(paid_terms_id)?; // ensure enough free balance to cover terms fees - let terms = Self::paid_membership_terms_by_id(paid_terms_id).ok_or("paid membership term id does not exist")?; ensure!(T::Currency::can_slash(&who, terms.fee), "not enough balance to buy membership"); - let user_info = Self::check_user_info(user_info)?; + let user_info = Self::check_user_registration_info(user_info)?; // ensure handle is not already registered Self::ensure_unique_handle(&user_info.handle)?; @@ -210,6 +211,46 @@ decl_module! { Self::deposit_event(RawEvent::MemberRegistered(member_id, who.clone())); } + fn change_member_about_text(origin, text: Vec) { + let who = ensure_signed(origin)?; + + let mut profile = Self::ensure_has_profile(&who)?; + + let text = Self::validate_text(&text); + + profile.about = text; + + Self::deposit_event(RawEvent::MemberUpdatedAboutText(profile.id)); + >::insert(profile.id, profile); + } + + fn change_member_avatar(origin, uri: Vec) { + let who = ensure_signed(origin)?; + + let mut profile = Self::ensure_has_profile(&who)?; + + let uri = Self::validate_avatar(&uri); + + profile.avatar_uri = uri; + + Self::deposit_event(RawEvent::MemberUpdatedAvatar(profile.id)); + >::insert(profile.id, profile); + } + + /// Change member's handle. + fn change_member_handle(origin, handle: Vec) { + let who = ensure_signed(origin)?; + let mut profile = Self::ensure_has_profile(&who)?; + + Self::validate_handle(&handle)?; + Self::ensure_unique_handle(&handle)?; + + >::remove(&profile.handle); + >::insert(handle.clone(), profile.id); + profile.handle = handle; + >::insert(profile.id, profile); + } + /// Buy the default membership (if it is active) and only provide handle - for testing fn buy_default_membership_testing(origin, handle: Vec) { Self::buy_membership(origin, T::PaidTermId::sa(0), UserInfo { @@ -227,10 +268,22 @@ impl Module { Ok(()) } - fn ensure_terms_id_is_active(terms_id: T::PaidTermId) -> dispatch::Result { + fn ensure_is_member(who: &T::AccountId) -> Result { + let member_id = Self::member_id_by_account_id(who).ok_or("no member id found for accountid")?; + Ok(member_id) + } + + fn ensure_has_profile(who: &T::AccountId) -> Result, &'static str> { + let member_id = Self::ensure_is_member(who)?; + let profile = Self::member_profile(&member_id).ok_or("member profile not found")?; + Ok(profile) + } + + fn ensure_active_terms_id(terms_id: T::PaidTermId) -> Result, &'static str> { let active_terms = Self::active_paid_membership_terms(); ensure!(active_terms.iter().any(|&id| id == terms_id), "paid terms id not active"); - Ok(()) + let terms = Self::paid_membership_terms_by_id(terms_id).ok_or("paid membership term id does not exist")?; + Ok(terms) } fn ensure_unique_handle(handle: &Vec ) -> dispatch::Result { @@ -238,21 +291,32 @@ impl Module { Ok(()) } - /// Basic user input validation - fn check_user_info(user_info: UserInfo) -> Result { - // Handle is required during registration - let handle = user_info.handle.ok_or("handle must be provided during registration")?; + fn validate_handle(handle: &Vec) -> dispatch::Result { ensure!(handle.len() >= Self::min_handle_length() as usize, "handle too short"); ensure!(handle.len() <= Self::max_handle_length() as usize, "handle too long"); + Ok(()) + } - let mut about = user_info.about.unwrap_or_default(); - about.truncate(Self::max_about_text_length() as usize); + fn validate_text(text: &Vec) -> Vec { + let mut text = text.clone(); + text.truncate(Self::max_about_text_length() as usize); + text + } + + fn validate_avatar(uri: &Vec) -> Vec { + let mut uri = uri.clone(); + uri.truncate(Self::max_avatar_uri_length() as usize); + uri + } + + /// Basic user input validation + fn check_user_registration_info(user_info: UserInfo) -> Result { + // Handle is required during registration + let handle = user_info.handle.ok_or("handle must be provided during registration")?; + Self::validate_handle(&handle)?; - let avatar_uri = user_info.avatar_uri.and_then(|uri: Vec| { - let mut uri = uri.clone(); - uri.truncate(Self::max_avatar_uri_length() as usize); - Some(uri) - }); + let about = Self::validate_text(&user_info.about.unwrap_or_default()); + let avatar_uri = Self::validate_avatar(&user_info.avatar_uri.unwrap_or_default()); Ok(CheckedUserInfo { handle, From 6f46ad1cbf55af3a9f5a22e2d8f49cab358de1e4 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 14 Mar 2019 21:03:31 +0200 Subject: [PATCH 11/33] add method to change multiple profile user infos in one transaction --- src/membership.rs | 64 ++++++++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/src/membership.rs b/src/membership.rs index 0f06a1c53c..c4343749e9 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -213,42 +213,25 @@ decl_module! { fn change_member_about_text(origin, text: Vec) { let who = ensure_signed(origin)?; - - let mut profile = Self::ensure_has_profile(&who)?; - - let text = Self::validate_text(&text); - - profile.about = text; - - Self::deposit_event(RawEvent::MemberUpdatedAboutText(profile.id)); - >::insert(profile.id, profile); + Self::_change_member_about_text(&who, &text)?; } fn change_member_avatar(origin, uri: Vec) { let who = ensure_signed(origin)?; - - let mut profile = Self::ensure_has_profile(&who)?; - - let uri = Self::validate_avatar(&uri); - - profile.avatar_uri = uri; - - Self::deposit_event(RawEvent::MemberUpdatedAvatar(profile.id)); - >::insert(profile.id, profile); + Self::_change_member_avatar(&who, &uri)?; } /// Change member's handle. fn change_member_handle(origin, handle: Vec) { let who = ensure_signed(origin)?; - let mut profile = Self::ensure_has_profile(&who)?; - - Self::validate_handle(&handle)?; - Self::ensure_unique_handle(&handle)?; + Self::_change_member_handle(&who, handle)?; + } - >::remove(&profile.handle); - >::insert(handle.clone(), profile.id); - profile.handle = handle; - >::insert(profile.id, profile); + fn batch_change_member_profile(origin, user_info: UserInfo) { + let who = ensure_signed(origin)?; + user_info.avatar_uri.map(|uri| Self::_change_member_avatar(&who, &uri)).ok_or("uri not changed"); + user_info.about.map(|about| Self::_change_member_about_text(&who, &about)).ok_or("about text not changed"); + user_info.handle.map(|handle| Self::_change_member_handle(&who, handle)).ok_or("handle not changed"); } /// Buy the default membership (if it is active) and only provide handle - for testing @@ -350,4 +333,33 @@ impl Module { new_member_id } + + fn _change_member_about_text (who: &T::AccountId, text: &Vec) -> dispatch::Result { + let mut profile = Self::ensure_has_profile(&who)?; + let text = Self::validate_text(text); + profile.about = text; + Self::deposit_event(RawEvent::MemberUpdatedAboutText(profile.id)); + >::insert(profile.id, profile); + Ok(()) + } + + fn _change_member_avatar(who: &T::AccountId, uri: &Vec) -> dispatch::Result { + let mut profile = Self::ensure_has_profile(who)?; + let uri = Self::validate_avatar(uri); + profile.avatar_uri = uri; + Self::deposit_event(RawEvent::MemberUpdatedAvatar(profile.id)); + >::insert(profile.id, profile); + Ok(()) + } + + fn _change_member_handle(who: &T::AccountId, handle: Vec) -> dispatch::Result { + let mut profile = Self::ensure_has_profile(who)?; + Self::validate_handle(&handle)?; + Self::ensure_unique_handle(&handle)?; + >::remove(&profile.handle); + >::insert(handle.clone(), profile.id); + profile.handle = handle; + >::insert(profile.id, profile); + Ok(()) + } } \ No newline at end of file From ac0c928c7e670dfccb1889d4a43be4de230c85cc Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 14 Mar 2019 21:13:13 +0200 Subject: [PATCH 12/33] check allowed new members before accepting new members --- src/membership.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/membership.rs b/src/membership.rs index c4343749e9..13791f5af7 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -191,6 +191,9 @@ decl_module! { fn buy_membership(origin, paid_terms_id: T::PaidTermId, user_info: UserInfo) { let who = ensure_signed(origin)?; + // 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)?; From f095b673d829b1d78e330074a7d687bc686e0660 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 14 Mar 2019 23:52:06 +0200 Subject: [PATCH 13/33] handle runtime uptime by using spec_version --- src/membership.rs | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/membership.rs b/src/membership.rs index 13791f5af7..dac8740dc3 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -8,8 +8,9 @@ use srml_support::traits::{Currency}; use runtime_primitives::traits::{Zero, SimpleArithmetic, As, Member, MaybeSerializeDebug}; use system::{self, ensure_signed}; use crate::governance::{GovernanceCurrency, BalanceOf }; -//use runtime_io::print; +use runtime_io::print; use {timestamp}; +use crate::{VERSION}; pub trait Trait: system::Trait + GovernanceCurrency + timestamp::Trait { type Event: From> + Into<::Event>; @@ -97,6 +98,10 @@ impl Default for PaidMembershipTerms { decl_storage! { trait Store for Module as Membership { + /// Records at what runtime spec version the store was initialized. This allows the runtime + /// to know when to run initialize code if it was installed as an update. + StoreSpecVersion get(store_spec_version) build(|_| Some(VERSION.spec_version)) : Option; + /// MemberId's start at this value FirstMemberId get(first_member_id) config(first_member_id): T::MemberId = T::MemberId::sa(DEFAULT_FIRST_MEMBER_ID); @@ -138,11 +143,6 @@ decl_storage! { /// Is the platform is accepting new members or not NewMembershipsAllowed get(new_memberships_allowed) : bool = true; - /// If we should run a migration and initialize new storage values introduced in new runtime - // This will initialize to false if starting at genesis (because the build() method will run), - // for a runtime upgrade it will return the default value when reading the first time. - ShouldRunMigration get(should_run_migration) build(|_| false) : bool = true; - // User Input Validation parameters - do these really need to be state variables // I don't see a need to adjust these in future? MinHandleLength get(min_handle_length) : u32 = DEFAULT_MIN_HANDLE_LENGTH; @@ -166,11 +166,14 @@ decl_event! { } impl Module { - fn run_migration() { - if Self::should_run_migration() { + /// Initialization step that runs when the runtime is installed as a runtime upgrade + /// Remember to remove this code in next release, or guard the code with the spec version + // Given it is easy to forget to do this, we need a better arrangement. + fn runtime_initialization(spec_version: u32) { + if 5 == spec_version { + print("Running New Runtime Init Code"); let default_terms: PaidMembershipTerms = Default::default(); >::insert(T::PaidTermId::sa(0), default_terms); - >::put(false); } } } @@ -180,7 +183,11 @@ decl_module! { fn deposit_event() = default; fn on_initialise(_now: T::BlockNumber) { - Self::run_migration(); + if Self::store_spec_version().map_or(true, |spec_version| VERSION.spec_version > spec_version) { + // Mark store version with latest so we don't run initialization code again + >::put(VERSION.spec_version); + Self::runtime_initialization(VERSION.spec_version); + } } fn on_finalise(_now: T::BlockNumber) { From 1d63f676b99618eb80c6da8da3ed44e5d300d66c Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Fri, 15 Mar 2019 00:03:53 +0200 Subject: [PATCH 14/33] fix batch profile update --- src/membership.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/membership.rs b/src/membership.rs index dac8740dc3..29d5774c4d 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -239,14 +239,20 @@ decl_module! { fn batch_change_member_profile(origin, user_info: UserInfo) { let who = ensure_signed(origin)?; - user_info.avatar_uri.map(|uri| Self::_change_member_avatar(&who, &uri)).ok_or("uri not changed"); - user_info.about.map(|about| Self::_change_member_about_text(&who, &about)).ok_or("about text not changed"); - user_info.handle.map(|handle| Self::_change_member_handle(&who, handle)).ok_or("handle not changed"); + if let Some(uri) = user_info.avatar_uri { + Self::_change_member_avatar(&who, &uri)?; + } + if let Some(about) = user_info.about { + Self::_change_member_about_text(&who, &about)?; + } + if let Some(handle) = user_info.handle { + Self::_change_member_handle(&who, handle)?; + } } - /// Buy the default membership (if it is active) and only provide handle - for testing + /// Buy the default membership (if it is active) and only provide handle fn buy_default_membership_testing(origin, handle: Vec) { - Self::buy_membership(origin, T::PaidTermId::sa(0), UserInfo { + Self::buy_membership(origin, T::PaidTermId::sa(DEFAULT_PAID_TERM_ID), UserInfo { handle: Some(handle.clone()), avatar_uri: None, about: None From 7ae24d007d8856a5aa841dc42944a4efc3c8642f Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Sat, 16 Mar 2019 08:03:14 +0200 Subject: [PATCH 15/33] rename update profile method, and don't truncate avatar uri --- src/membership.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/membership.rs b/src/membership.rs index 29d5774c4d..6ee6a69586 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -162,6 +162,7 @@ decl_event! { MemberRegistered(MemberId, AccountId), MemberUpdatedAboutText(MemberId), MemberUpdatedAvatar(MemberId), + MemberUpdatedHandle(MemberId), } } @@ -237,7 +238,7 @@ decl_module! { Self::_change_member_handle(&who, handle)?; } - fn batch_change_member_profile(origin, user_info: UserInfo) { + fn update_profile(origin, user_info: UserInfo) { let who = ensure_signed(origin)?; if let Some(uri) = user_info.avatar_uri { Self::_change_member_avatar(&who, &uri)?; @@ -302,10 +303,9 @@ impl Module { text } - fn validate_avatar(uri: &Vec) -> Vec { - let mut uri = uri.clone(); - uri.truncate(Self::max_avatar_uri_length() as usize); - uri + fn validate_avatar(uri: &Vec) -> dispatch::Result { + ensure!(uri.len() <= Self::max_avatar_uri_length() as usize, "avatar uri too long"); + Ok(()) } /// Basic user input validation @@ -315,7 +315,8 @@ impl Module { Self::validate_handle(&handle)?; let about = Self::validate_text(&user_info.about.unwrap_or_default()); - let avatar_uri = Self::validate_avatar(&user_info.avatar_uri.unwrap_or_default()); + let avatar_uri = user_info.avatar_uri.unwrap_or_default(); + Self::validate_avatar(&avatar_uri)?; Ok(CheckedUserInfo { handle, @@ -361,8 +362,8 @@ impl Module { fn _change_member_avatar(who: &T::AccountId, uri: &Vec) -> dispatch::Result { let mut profile = Self::ensure_has_profile(who)?; - let uri = Self::validate_avatar(uri); - profile.avatar_uri = uri; + Self::validate_avatar(uri)?; + profile.avatar_uri = uri.clone(); Self::deposit_event(RawEvent::MemberUpdatedAvatar(profile.id)); >::insert(profile.id, profile); Ok(()) @@ -375,6 +376,7 @@ impl Module { >::remove(&profile.handle); >::insert(handle.clone(), profile.id); profile.handle = handle; + Self::deposit_event(RawEvent::MemberUpdatedHandle(profile.id)); >::insert(profile.id, profile); Ok(()) } From 2d3329add5d7e460ab85d9c98b421e962c494daf Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Sat, 16 Mar 2019 09:55:40 +0200 Subject: [PATCH 16/33] member sub accounts --- src/membership.rs | 120 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 94 insertions(+), 26 deletions(-) diff --git a/src/membership.rs b/src/membership.rs index 6ee6a69586..657e1fee01 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -51,7 +51,8 @@ pub struct Profile { registered_at_time: T::Moment, entry: EntryMethod, suspended: bool, - subscription: Option + subscription: Option, + sub_accounts: Vec } #[derive(Clone, Debug, Encode, Decode, PartialEq)] @@ -108,10 +109,11 @@ decl_storage! { /// MemberId to assign to next member that is added to the registry NextMemberId get(next_member_id) build(|config: &GenesisConfig| config.first_member_id): T::MemberId = T::MemberId::sa(DEFAULT_FIRST_MEMBER_ID); - /// Mapping of member ids to their corresponding accountid + /// Mapping of member ids to their corresponding primary accountid AccountIdByMemberId get(account_id_by_member_id) : map T::MemberId => T::AccountId; - /// Mapping of members' accountid to their member id + /// Mapping of members' accountid ids to their member id. + /// A member can have one primary account and multiple sub accounts associated with their profile MemberIdByAccountId get(member_id_by_account_id) : map T::AccountId => Option; /// Mapping of member's id to their membership profile @@ -163,6 +165,9 @@ decl_event! { MemberUpdatedAboutText(MemberId), MemberUpdatedAvatar(MemberId), MemberUpdatedHandle(MemberId), + MemberChangedPrimaryAccount(MemberId, AccountId), + MemberAddedSubAccount(MemberId, AccountId), + MemberRemovedSubAccount(MemberId, AccountId), } } @@ -224,30 +229,35 @@ decl_module! { fn change_member_about_text(origin, text: Vec) { let who = ensure_signed(origin)?; - Self::_change_member_about_text(&who, &text)?; + let member_id = Self::ensure_is_member_primary_account(who.clone())?; + Self::_change_member_about_text(member_id, &text)?; } fn change_member_avatar(origin, uri: Vec) { let who = ensure_signed(origin)?; - Self::_change_member_avatar(&who, &uri)?; + let member_id = Self::ensure_is_member_primary_account(who.clone())?; + Self::_change_member_avatar(member_id, &uri)?; } /// Change member's handle. fn change_member_handle(origin, handle: Vec) { let who = ensure_signed(origin)?; - Self::_change_member_handle(&who, handle)?; + let member_id = Self::ensure_is_member_primary_account(who.clone())?; + Self::_change_member_handle(member_id, handle)?; } fn update_profile(origin, user_info: UserInfo) { let who = ensure_signed(origin)?; + let member_id = Self::ensure_is_member_primary_account(who.clone())?; + if let Some(uri) = user_info.avatar_uri { - Self::_change_member_avatar(&who, &uri)?; + Self::_change_member_avatar(member_id, &uri)?; } if let Some(about) = user_info.about { - Self::_change_member_about_text(&who, &about)?; + Self::_change_member_about_text(member_id, &about)?; } if let Some(handle) = user_info.handle { - Self::_change_member_handle(&who, handle)?; + Self::_change_member_handle(member_id, handle)?; } } @@ -259,12 +269,64 @@ decl_module! { about: None })?; } + + fn change_member_primary_account(origin, new_primary_account: T::AccountId) { + let who = ensure_signed(origin)?; + + // only primary account can assign new primary account + let member_id = Self::ensure_is_member_primary_account(who.clone())?; + + // ensure new_primary_account isn't already associated with any existing member + Self::ensure_not_member(&new_primary_account)?; + + // update associated accounts + >::remove(&who); + >::insert(&new_primary_account, member_id); + + // update primary account + >::insert(member_id, new_primary_account.clone()); + Self::deposit_event(RawEvent::MemberChangedPrimaryAccount(member_id, new_primary_account)); + } + + fn add_member_sub_account(origin, sub_account: T::AccountId) { + let who = ensure_signed(origin)?; + // only primary account can manage sub accounts + let member_id = Self::ensure_is_member_primary_account(who.clone())?; + // ensure sub_account isn't already associated with any existing member + Self::ensure_not_member(&sub_account)?; + + let mut profile = Self::ensure_profile(member_id)?; + profile.sub_accounts.push(sub_account.clone()); + >::insert(member_id, profile); + Self::deposit_event(RawEvent::MemberAddedSubAccount(member_id, sub_account)); + } + + fn remove_member_sub_account(origin, sub_account: T::AccountId) { + let who = ensure_signed(origin)?; + // only primary account can manage sub accounts + let member_id = Self::ensure_is_member_primary_account(who.clone())?; + + // ensure account is a sub account + // primary account cannot be added as a subaccount in add_member_sub_account() + ensure!(who != sub_account, "not sub account"); + + // ensure sub_account is associated with member + let sub_account_member_id = Self::ensure_is_member(&sub_account)?; + ensure!(member_id == sub_account_member_id, "not member sub account"); + + let mut profile = Self::ensure_profile(member_id)?; + + profile.sub_accounts = profile.sub_accounts.into_iter().filter(|account| !(*account == sub_account)).collect(); + + >::insert(member_id, profile); + Self::deposit_event(RawEvent::MemberRemovedSubAccount(member_id, sub_account)); + } } } impl Module { fn ensure_not_member(who: &T::AccountId) -> dispatch::Result { - ensure!(!>::exists(who), "Account already associated with a membership"); + ensure!(!>::exists(who), "account already associated with a membership"); Ok(()) } @@ -273,9 +335,14 @@ impl Module { Ok(member_id) } - fn ensure_has_profile(who: &T::AccountId) -> Result, &'static str> { - let member_id = Self::ensure_is_member(who)?; - let profile = Self::member_profile(&member_id).ok_or("member profile not found")?; + fn ensure_is_member_primary_account(who: T::AccountId) -> Result { + let member_id = Self::ensure_is_member(&who)?; + ensure!(Self::account_id_by_member_id(member_id) == who, "not primary account"); + Ok(member_id) + } + + fn ensure_profile(id: T::MemberId) -> Result, &'static str> { + let profile = Self::member_profile(&id).ok_or("member profile not found")?; Ok(profile) } @@ -340,6 +407,7 @@ impl Module { entry: EntryMethod::Paid(paid_terms_id), suspended: false, subscription: None, + sub_accounts: vec![], }; >::insert(who.clone(), new_member_id); @@ -351,33 +419,33 @@ impl Module { new_member_id } - fn _change_member_about_text (who: &T::AccountId, text: &Vec) -> dispatch::Result { - let mut profile = Self::ensure_has_profile(&who)?; + fn _change_member_about_text (id: T::MemberId, text: &Vec) -> dispatch::Result { + let mut profile = Self::ensure_profile(id)?; let text = Self::validate_text(text); profile.about = text; - Self::deposit_event(RawEvent::MemberUpdatedAboutText(profile.id)); - >::insert(profile.id, profile); + Self::deposit_event(RawEvent::MemberUpdatedAboutText(id)); + >::insert(id, profile); Ok(()) } - fn _change_member_avatar(who: &T::AccountId, uri: &Vec) -> dispatch::Result { - let mut profile = Self::ensure_has_profile(who)?; + fn _change_member_avatar(id: T::MemberId, uri: &Vec) -> dispatch::Result { + let mut profile = Self::ensure_profile(id)?; Self::validate_avatar(uri)?; profile.avatar_uri = uri.clone(); - Self::deposit_event(RawEvent::MemberUpdatedAvatar(profile.id)); - >::insert(profile.id, profile); + Self::deposit_event(RawEvent::MemberUpdatedAvatar(id)); + >::insert(id, profile); Ok(()) } - fn _change_member_handle(who: &T::AccountId, handle: Vec) -> dispatch::Result { - let mut profile = Self::ensure_has_profile(who)?; + fn _change_member_handle(id: T::MemberId, handle: Vec) -> dispatch::Result { + let mut profile = Self::ensure_profile(id)?; Self::validate_handle(&handle)?; Self::ensure_unique_handle(&handle)?; >::remove(&profile.handle); - >::insert(handle.clone(), profile.id); + >::insert(handle.clone(), id); profile.handle = handle; - Self::deposit_event(RawEvent::MemberUpdatedHandle(profile.id)); - >::insert(profile.id, profile); + Self::deposit_event(RawEvent::MemberUpdatedHandle(id)); + >::insert(id, profile); Ok(()) } } \ No newline at end of file From da0a0721862db595a11ab57c229bf5bdb4cd4610 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Sat, 16 Mar 2019 10:56:02 +0200 Subject: [PATCH 17/33] add some notes on sub-accounts --- src/membership.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/membership.rs b/src/membership.rs index 657e1fee01..4891bc3efc 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -270,6 +270,14 @@ decl_module! { })?; } + // Notes: + // Ability to change primary account and add/remove sub accounts should be restricted + // to accounts that are not actively being used in other roles in the platform? + // Other roles in the platform should be associated directly with member_id instead of an account_id, + // but can still use a seprate account/key to sign extrinsics, have a seprate balance from primary account, + // and a way to limit damage from a compromised sub account. + // Maybe should also not be allowed if member is suspended? + // Main usecase for changing primary account is for identity recoverability fn change_member_primary_account(origin, new_primary_account: T::AccountId) { let who = ensure_signed(origin)?; From 31fd1de32b2af40f673a3f6c6afb25030f5eed62 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Sat, 16 Mar 2019 11:10:15 +0200 Subject: [PATCH 18/33] default min length of handle 5 chars --- src/membership.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/membership.rs b/src/membership.rs index 4891bc3efc..f08acebc04 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -34,7 +34,7 @@ const DEFAULT_PAID_TERM_FEE: u64 = 100; // Can be overidden in genesis config const DEFAULT_PAID_TERM_TEXT: &str = "Default Paid Term TOS..."; // Default user info constraints -const DEFAULT_MIN_HANDLE_LENGTH: u32 = 4; +const DEFAULT_MIN_HANDLE_LENGTH: u32 = 5; const DEFAULT_MAX_HANDLE_LENGTH: u32 = 20; const DEFAULT_MAX_AVATAR_URI_LENGTH: u32 = 512; const DEFAULT_MAX_ABOUT_TEXT_LENGTH: u32 = 1024; From 95e4cd38f9ac30753264d3ce5ae6416256c82fdc Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Sat, 16 Mar 2019 11:43:15 +0200 Subject: [PATCH 19/33] adjust avatar, handle and about text default max lengths --- src/membership.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/membership.rs b/src/membership.rs index f08acebc04..c0a6a559f9 100644 --- a/src/membership.rs +++ b/src/membership.rs @@ -35,9 +35,9 @@ const DEFAULT_PAID_TERM_TEXT: &str = "Default Paid Term TOS..."; // Default user info constraints const DEFAULT_MIN_HANDLE_LENGTH: u32 = 5; -const DEFAULT_MAX_HANDLE_LENGTH: u32 = 20; -const DEFAULT_MAX_AVATAR_URI_LENGTH: u32 = 512; -const DEFAULT_MAX_ABOUT_TEXT_LENGTH: u32 = 1024; +const DEFAULT_MAX_HANDLE_LENGTH: u32 = 40; +const DEFAULT_MAX_AVATAR_URI_LENGTH: u32 = 1024; +const DEFAULT_MAX_ABOUT_TEXT_LENGTH: u32 = 2048; //#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] #[derive(Encode, Decode)] From 478fbe0ed6f0abe6373041b714fa1df5b84b6454 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Tue, 19 Mar 2019 12:22:10 +0200 Subject: [PATCH 20/33] reorg membership into a module directory --- src/lib.rs | 5 +++-- src/membership/mod.rs | 3 +++ src/{membership.rs => membership/registry.rs} | 0 3 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 src/membership/mod.rs rename src/{membership.rs => membership/registry.rs} (100%) diff --git a/src/lib.rs b/src/lib.rs index b8b0ac8764..b4cf410acc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,6 +18,7 @@ pub mod governance; use governance::{election, council, proposals}; mod memo; mod membership; +use membership::registry; use rstd::prelude::*; #[cfg(feature = "std")] @@ -225,7 +226,7 @@ impl memo::Trait for Runtime { type Event = Event; } -impl membership::Trait for Runtime { +impl membership::registry::Trait for Runtime { type Event = Event; type MemberId = u64; type PaidTermId = u64; @@ -252,7 +253,7 @@ construct_runtime!( CouncilElection: election::{Module, Call, Storage, Event, Config}, Council: council::{Module, Call, Storage, Event, Config}, Memo: memo::{Module, Call, Storage, Event}, - Membership: membership::{Module, Call, Storage, Event, Config}, + Membership: registry::{Module, Call, Storage, Event, Config}, } ); diff --git a/src/membership/mod.rs b/src/membership/mod.rs new file mode 100644 index 0000000000..50578b0286 --- /dev/null +++ b/src/membership/mod.rs @@ -0,0 +1,3 @@ +#![cfg_attr(not(feature = "std"), no_std)] + +pub mod registry; \ No newline at end of file diff --git a/src/membership.rs b/src/membership/registry.rs similarity index 100% rename from src/membership.rs rename to src/membership/registry.rs From a227fa3a3c5ca77255da013c5502c8fde680035d Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Tue, 19 Mar 2019 14:37:33 +0200 Subject: [PATCH 21/33] add tests for membership --- src/membership/mock.rs | 82 +++++++++++++++++++++++++++++++++++++++++ src/membership/mod.rs | 5 ++- src/membership/tests.rs | 15 ++++++++ 3 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 src/membership/mock.rs create mode 100644 src/membership/tests.rs diff --git a/src/membership/mock.rs b/src/membership/mock.rs new file mode 100644 index 0000000000..ec509d607f --- /dev/null +++ b/src/membership/mock.rs @@ -0,0 +1,82 @@ +#![cfg(test)] + +use rstd::prelude::*; +pub use crate::governance::{GovernanceCurrency}; +pub use super::{registry}; +pub use system; + +pub use primitives::{H256, Blake2Hasher}; +pub use runtime_primitives::{ + BuildStorage, + traits::{BlakeTwo256, OnFinalise, IdentityLookup}, + testing::{Digest, DigestItem, Header, UintAuthorityId} +}; + +use srml_support::impl_outer_origin; + +impl_outer_origin! { + pub enum Origin for Test {} +} + +// 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. +#[derive(Clone, Eq, PartialEq)] +pub struct Test; +impl system::Trait for Test { + type Origin = Origin; + type Index = u64; + type BlockNumber = u64; + type Hash = H256; + type Hashing = BlakeTwo256; + type Digest = Digest; + type AccountId = u64; + type Header = Header; + type Event = (); + type Log = DigestItem; + type Lookup = IdentityLookup; +} +impl timestamp::Trait for Test { + type Moment = u64; + type OnTimestampSet = (); +} +impl consensus::Trait for Test { + type SessionKey = UintAuthorityId; + type InherentOfflineReport = (); + type Log = DigestItem; +} + +impl balances::Trait for Test { + type Event = (); + + /// The balance of an account. + type Balance = u32; + + /// A function which is invoked when the free-balance has fallen below the existential deposit and + /// has been reduced to zero. + /// + /// Gives a chance to clean up resources associated with the given account. + type OnFreeBalanceZero = (); + + /// Handler for when a new account is created. + type OnNewAccount = (); + + /// A function that returns true iff a given account can transfer its funds to another account. + type EnsureAccountLiquid = (); +} + +impl GovernanceCurrency for Test { + type Currency = balances::Module; +} + +// This function basically just builds a genesis storage key/value store according to +// our desired mockup. +pub fn initial_test_ext() -> runtime_io::TestExternalities { + let mut t = system::GenesisConfig::::default().build_storage().unwrap().0; + + runtime_io::TestExternalities::new(t) +} + +pub type System = system::Module; +pub type Balances = balances::Module; +pub type Membership = registry::Module; diff --git a/src/membership/mod.rs b/src/membership/mod.rs index 50578b0286..a07ceef62a 100644 --- a/src/membership/mod.rs +++ b/src/membership/mod.rs @@ -1,3 +1,6 @@ #![cfg_attr(not(feature = "std"), no_std)] -pub mod registry; \ No newline at end of file +pub mod registry; + +mod mock; +mod tests; \ No newline at end of file diff --git a/src/membership/tests.rs b/src/membership/tests.rs new file mode 100644 index 0000000000..498e3abd74 --- /dev/null +++ b/src/membership/tests.rs @@ -0,0 +1,15 @@ +#![cfg(test)] + +use super::*; +use super::mock::*; + +use parity_codec::Encode; +use runtime_io::with_externalities; +use srml_support::*; + +#[test] +fn test_setup() { + with_externalities(&mut initial_test_ext(), || { + assert!(false); + }); +} \ No newline at end of file From 27b784111c90eb1e342b3d12a8ecf2c96eba81b9 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 20 Mar 2019 09:10:35 +0200 Subject: [PATCH 22/33] membership tests --- src/membership/mock.rs | 43 ++++++++++-- src/membership/registry.rs | 93 +++++++++++------------- src/membership/tests.rs | 140 ++++++++++++++++++++++++++++++++++++- 3 files changed, 217 insertions(+), 59 deletions(-) diff --git a/src/membership/mock.rs b/src/membership/mock.rs index ec509d607f..89fb228341 100644 --- a/src/membership/mock.rs +++ b/src/membership/mock.rs @@ -69,12 +69,45 @@ impl GovernanceCurrency for Test { type Currency = balances::Module; } -// This function basically just builds a genesis storage key/value store according to -// our desired mockup. -pub fn initial_test_ext() -> runtime_io::TestExternalities { - let mut t = system::GenesisConfig::::default().build_storage().unwrap().0; +impl registry::Trait for Test { + type Event = (); + type MemberId = u32; + type PaidTermId = u32; + type SubscriptionId = u32; +} + +pub struct ExtBuilder { + first_member_id: u32, + default_paid_membership_fee: u32, +} +impl Default for ExtBuilder { + fn default() -> Self { + Self { + first_member_id: 1, + default_paid_membership_fee: 100, + } + } +} + +impl ExtBuilder { + pub fn first_member_id(mut self, first_member_id: u32) -> Self { + self.first_member_id = first_member_id; + self + } + pub fn default_paid_membership_fee(mut self, default_paid_membership_fee: u32) -> Self { + self.default_paid_membership_fee = default_paid_membership_fee; + self + } + pub fn build(self) -> runtime_io::TestExternalities { + let mut t = system::GenesisConfig::::default().build_storage().unwrap().0; + + t.extend(registry::GenesisConfig::{ + first_member_id: self.first_member_id, + default_paid_membership_fee: self.default_paid_membership_fee, + }.build_storage().unwrap().0); - runtime_io::TestExternalities::new(t) + t.into() + } } pub type System = system::Module; diff --git a/src/membership/registry.rs b/src/membership/registry.rs index c0a6a559f9..8263f1b710 100644 --- a/src/membership/registry.rs +++ b/src/membership/registry.rs @@ -43,24 +43,24 @@ const DEFAULT_MAX_ABOUT_TEXT_LENGTH: u32 = 2048; #[derive(Encode, Decode)] /// Stored information about a registered user pub struct Profile { - id: T::MemberId, - handle: Vec, - avatar_uri: Vec, - about: Vec, - registered_at_block: T::BlockNumber, - registered_at_time: T::Moment, - entry: EntryMethod, - suspended: bool, - subscription: Option, - sub_accounts: Vec + pub id: T::MemberId, + pub handle: Vec, + pub avatar_uri: Vec, + pub about: Vec, + pub registered_at_block: T::BlockNumber, + pub registered_at_time: T::Moment, + pub entry: EntryMethod, + pub suspended: bool, + pub subscription: Option, + pub sub_accounts: Vec } #[derive(Clone, Debug, Encode, Decode, PartialEq)] /// Structure used to batch user configurable profile information when registering or updating info pub struct UserInfo { - handle: Option>, - avatar_uri: Option>, - about: Option>, + pub handle: Option>, + pub avatar_uri: Option>, + pub about: Option>, } struct CheckedUserInfo { @@ -77,14 +77,14 @@ pub enum EntryMethod { } //#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] -#[derive(Encode, Decode)] +#[derive(Encode, Decode, Eq, PartialEq)] pub struct PaidMembershipTerms { /// Unique identifier - the term id - id: T::PaidTermId, + pub id: T::PaidTermId, /// Quantity of native tokens which must be provably burned - fee: BalanceOf, + pub fee: BalanceOf, /// String of capped length describing human readable conditions which are being agreed upon - text: Vec + pub text: Vec } impl Default for PaidMembershipTerms { @@ -101,59 +101,59 @@ decl_storage! { trait Store for Module as Membership { /// Records at what runtime spec version the store was initialized. This allows the runtime /// to know when to run initialize code if it was installed as an update. - StoreSpecVersion get(store_spec_version) build(|_| Some(VERSION.spec_version)) : Option; + pub StoreSpecVersion get(store_spec_version) build(|_| Some(VERSION.spec_version)) : Option; /// MemberId's start at this value - FirstMemberId get(first_member_id) config(first_member_id): T::MemberId = T::MemberId::sa(DEFAULT_FIRST_MEMBER_ID); + pub FirstMemberId get(first_member_id) config(first_member_id): T::MemberId = T::MemberId::sa(DEFAULT_FIRST_MEMBER_ID); /// MemberId to assign to next member that is added to the registry - NextMemberId get(next_member_id) build(|config: &GenesisConfig| config.first_member_id): T::MemberId = T::MemberId::sa(DEFAULT_FIRST_MEMBER_ID); + pub NextMemberId get(next_member_id) build(|config: &GenesisConfig| config.first_member_id): T::MemberId = T::MemberId::sa(DEFAULT_FIRST_MEMBER_ID); /// Mapping of member ids to their corresponding primary accountid - AccountIdByMemberId get(account_id_by_member_id) : map T::MemberId => T::AccountId; + pub AccountIdByMemberId get(account_id_by_member_id) : map T::MemberId => T::AccountId; /// Mapping of members' accountid ids to their member id. /// A member can have one primary account and multiple sub accounts associated with their profile - MemberIdByAccountId get(member_id_by_account_id) : map T::AccountId => Option; + pub MemberIdByAccountId get(member_id_by_account_id) : map T::AccountId => Option; /// Mapping of member's id to their membership profile // Value is Option because it is not meaningful to have a Default value for Profile - MemberProfile get(member_profile) : map T::MemberId => Option>; + pub MemberProfile get(member_profile) : map T::MemberId => Option>; /// Registered unique handles and their mapping to their owner - Handles get(handles) : map Vec => Option; + pub Handles get(handles) : map Vec => Option; /// Next paid membership terms id - NextPaidMembershipTermsId get(next_paid_membership_terms_id) : T::PaidTermId = T::PaidTermId::sa(FIRST_PAID_TERMS_ID); + pub NextPaidMembershipTermsId get(next_paid_membership_terms_id) : T::PaidTermId = T::PaidTermId::sa(FIRST_PAID_TERMS_ID); /// Paid membership terms record // Remember to add _genesis_phantom_data: std::marker::PhantomData{} to membership // genesis config if not providing config() or extra_genesis - PaidMembershipTermsById get(paid_membership_terms_by_id) build(|config: &GenesisConfig| { + pub PaidMembershipTermsById get(paid_membership_terms_by_id) build(|config: &GenesisConfig| { // This method only gets called when initializing storage, and is // compiled as native code. (Will be called when building `raw` chainspec) // So it can't be relied upon to initialize storage for runtimes updates. // Initialization for updated runtime is done in run_migration() let mut terms: PaidMembershipTerms = Default::default(); - terms.fee = BalanceOf::::sa(config.default_paid_membership_fee); + terms.fee = config.default_paid_membership_fee; vec![(terms.id, terms)] }) : map T::PaidTermId => Option>; /// Active Paid membership terms - ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec = vec![T::PaidTermId::sa(DEFAULT_PAID_TERM_ID)]; + pub ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec = vec![T::PaidTermId::sa(DEFAULT_PAID_TERM_ID)]; /// Is the platform is accepting new members or not - NewMembershipsAllowed get(new_memberships_allowed) : bool = true; + pub NewMembershipsAllowed get(new_memberships_allowed) : bool = true; // User Input Validation parameters - do these really need to be state variables // I don't see a need to adjust these in future? - MinHandleLength get(min_handle_length) : u32 = DEFAULT_MIN_HANDLE_LENGTH; - MaxHandleLength get(max_handle_length) : u32 = DEFAULT_MAX_HANDLE_LENGTH; - MaxAvatarUriLength get(max_avatar_uri_length) : u32 = DEFAULT_MAX_AVATAR_URI_LENGTH; - MaxAboutTextLength get(max_about_text_length) : u32 = DEFAULT_MAX_ABOUT_TEXT_LENGTH; + pub MinHandleLength get(min_handle_length) : u32 = DEFAULT_MIN_HANDLE_LENGTH; + pub MaxHandleLength get(max_handle_length) : u32 = DEFAULT_MAX_HANDLE_LENGTH; + pub MaxAvatarUriLength get(max_avatar_uri_length) : u32 = DEFAULT_MAX_AVATAR_URI_LENGTH; + pub MaxAboutTextLength get(max_about_text_length) : u32 = DEFAULT_MAX_ABOUT_TEXT_LENGTH; } add_extra_genesis { - config(default_paid_membership_fee): u64; + config(default_paid_membership_fee): BalanceOf; } } @@ -201,7 +201,7 @@ decl_module! { } /// Non-members can buy membership - fn buy_membership(origin, paid_terms_id: T::PaidTermId, user_info: UserInfo) { + pub fn buy_membership(origin, paid_terms_id: T::PaidTermId, user_info: UserInfo) { let who = ensure_signed(origin)?; // make sure we are accepting new memberships @@ -227,26 +227,26 @@ decl_module! { Self::deposit_event(RawEvent::MemberRegistered(member_id, who.clone())); } - fn change_member_about_text(origin, text: Vec) { + pub fn change_member_about_text(origin, text: Vec) { let who = ensure_signed(origin)?; let member_id = Self::ensure_is_member_primary_account(who.clone())?; Self::_change_member_about_text(member_id, &text)?; } - fn change_member_avatar(origin, uri: Vec) { + pub fn change_member_avatar(origin, uri: Vec) { let who = ensure_signed(origin)?; let member_id = Self::ensure_is_member_primary_account(who.clone())?; Self::_change_member_avatar(member_id, &uri)?; } /// Change member's handle. - fn change_member_handle(origin, handle: Vec) { + pub fn change_member_handle(origin, handle: Vec) { let who = ensure_signed(origin)?; let member_id = Self::ensure_is_member_primary_account(who.clone())?; Self::_change_member_handle(member_id, handle)?; } - fn update_profile(origin, user_info: UserInfo) { + pub fn update_profile(origin, user_info: UserInfo) { let who = ensure_signed(origin)?; let member_id = Self::ensure_is_member_primary_account(who.clone())?; @@ -261,15 +261,6 @@ decl_module! { } } - /// Buy the default membership (if it is active) and only provide handle - fn buy_default_membership_testing(origin, handle: Vec) { - Self::buy_membership(origin, T::PaidTermId::sa(DEFAULT_PAID_TERM_ID), UserInfo { - handle: Some(handle.clone()), - avatar_uri: None, - about: None - })?; - } - // Notes: // Ability to change primary account and add/remove sub accounts should be restricted // to accounts that are not actively being used in other roles in the platform? @@ -278,7 +269,7 @@ decl_module! { // and a way to limit damage from a compromised sub account. // Maybe should also not be allowed if member is suspended? // Main usecase for changing primary account is for identity recoverability - fn change_member_primary_account(origin, new_primary_account: T::AccountId) { + pub fn change_member_primary_account(origin, new_primary_account: T::AccountId) { let who = ensure_signed(origin)?; // only primary account can assign new primary account @@ -296,7 +287,7 @@ decl_module! { Self::deposit_event(RawEvent::MemberChangedPrimaryAccount(member_id, new_primary_account)); } - fn add_member_sub_account(origin, sub_account: T::AccountId) { + pub fn add_member_sub_account(origin, sub_account: T::AccountId) { let who = ensure_signed(origin)?; // only primary account can manage sub accounts let member_id = Self::ensure_is_member_primary_account(who.clone())?; @@ -309,7 +300,7 @@ decl_module! { Self::deposit_event(RawEvent::MemberAddedSubAccount(member_id, sub_account)); } - fn remove_member_sub_account(origin, sub_account: T::AccountId) { + pub fn remove_member_sub_account(origin, sub_account: T::AccountId) { let who = ensure_signed(origin)?; // only primary account can manage sub accounts let member_id = Self::ensure_is_member_primary_account(who.clone())?; diff --git a/src/membership/tests.rs b/src/membership/tests.rs index 498e3abd74..657a1bbc38 100644 --- a/src/membership/tests.rs +++ b/src/membership/tests.rs @@ -7,9 +7,143 @@ use parity_codec::Encode; use runtime_io::with_externalities; use srml_support::*; +fn assert_ok_unwrap(value: Option, err: &'static str) -> T { + match value { + None => { assert!(false, err); value.unwrap() }, + Some(v) => v + } +} + +fn get_alice_info() -> registry::UserInfo { + registry::UserInfo { + handle: Some(String::from("alice").as_bytes().to_vec()), + avatar_uri: Some(String::from("http://avatar-url.com/alice").as_bytes().to_vec()), + about: Some(String::from("my name is alice").as_bytes().to_vec()), + } +} + +const ALICE_ACCOUNT_ID: u64 = 1; +const DEFAULT_TERMS_ID: u32 = 0; + +fn buy_default_membership_as_alice() -> dispatch::Result { + Membership::buy_membership(Origin::signed(ALICE_ACCOUNT_ID), DEFAULT_TERMS_ID, get_alice_info()) +} + +fn set_alice_free_balance(balance: u32) { + Balances::set_free_balance(&ALICE_ACCOUNT_ID, balance); +} + +#[test] +fn initial_state() { + const DEFAULT_FEE: u32 = 500; + const DEFAULT_FIRST_ID: u32 = 1000; + + with_externalities(&mut ExtBuilder::default() + .default_paid_membership_fee(DEFAULT_FEE) + .first_member_id(DEFAULT_FIRST_ID).build(), || + { + assert_eq!(Membership::first_member_id(), DEFAULT_FIRST_ID); + assert_eq!(Membership::next_member_id(), DEFAULT_FIRST_ID); + + let default_terms = assert_ok_unwrap(Membership::paid_membership_terms_by_id(DEFAULT_TERMS_ID), "default terms not initialized"); + + assert_eq!(default_terms.id, DEFAULT_TERMS_ID); + assert_eq!(default_terms.fee, DEFAULT_FEE); + }); +} + #[test] -fn test_setup() { - with_externalities(&mut initial_test_ext(), || { - assert!(false); +fn buy_membership() { + const DEFAULT_FEE: u32 = 500; + const DEFAULT_FIRST_ID: u32 = 1000; + const SURPLUS_BALANCE: u32 = 500; + + with_externalities(&mut ExtBuilder::default() + .default_paid_membership_fee(DEFAULT_FEE) + .first_member_id(DEFAULT_FIRST_ID).build(), || + { + let initial_balance = DEFAULT_FEE + SURPLUS_BALANCE; + set_alice_free_balance(initial_balance); + + assert_ok!(buy_default_membership_as_alice()); + + let member_id = assert_ok_unwrap(Membership::member_id_by_account_id(&ALICE_ACCOUNT_ID), "member id not assigned"); + + let profile = assert_ok_unwrap(Membership::member_profile(&member_id), "member profile created"); + + assert_eq!(Some(profile.handle), get_alice_info().handle); + + assert_eq!(Balances::free_balance(&ALICE_ACCOUNT_ID), SURPLUS_BALANCE); + + }); +} + +#[test] +fn buy_membership_fails_without_enough_balance() { + const DEFAULT_FEE: u32 = 500; + + with_externalities(&mut ExtBuilder::default() + .default_paid_membership_fee(DEFAULT_FEE).build(), || + { + let initial_balance = DEFAULT_FEE - 1; + set_alice_free_balance(initial_balance); + + assert!(buy_default_membership_as_alice().is_err()); + }); +} + +#[test] +fn new_memberships_allowed_flag() { + const DEFAULT_FEE: u32 = 500; + + with_externalities(&mut ExtBuilder::default() + .default_paid_membership_fee(DEFAULT_FEE).build(), || + { + let initial_balance = DEFAULT_FEE + 1; + set_alice_free_balance(initial_balance); + + >::put(false); + + assert!(buy_default_membership_as_alice().is_err()); + }); +} + +#[test] +fn account_cannot_create_multiple_memberships() { + const DEFAULT_FEE: u32 = 500; + const SURPLUS_BALANCE: u32 = 500; + + with_externalities(&mut ExtBuilder::default() + .default_paid_membership_fee(DEFAULT_FEE).build(), || + { + let initial_balance = DEFAULT_FEE + SURPLUS_BALANCE; + set_alice_free_balance(initial_balance); + + // First time it works + assert_ok!(buy_default_membership_as_alice()); + + // second attempt should fail + assert!(buy_default_membership_as_alice().is_err()); + + }); +} + +#[test] +fn unique_handles() { + const DEFAULT_FEE: u32 = 500; + const SURPLUS_BALANCE: u32 = 500; + + with_externalities(&mut ExtBuilder::default() + .default_paid_membership_fee(DEFAULT_FEE).build(), || + { + let initial_balance = DEFAULT_FEE + SURPLUS_BALANCE; + set_alice_free_balance(initial_balance); + + // alice's handle already taken + >::insert(get_alice_info().handle.unwrap(), 1); + + // should not be allowed to buy membership with that handle + assert!(buy_default_membership_as_alice().is_err()); + }); } \ No newline at end of file From 5ba5f86d637a0b6c12b42bca7faa52445ea571ea Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 20 Mar 2019 09:46:04 +0200 Subject: [PATCH 23/33] membership: more tests --- src/membership/tests.rs | 74 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/src/membership/tests.rs b/src/membership/tests.rs index 657a1bbc38..e21bf7acb4 100644 --- a/src/membership/tests.rs +++ b/src/membership/tests.rs @@ -22,6 +22,14 @@ fn get_alice_info() -> registry::UserInfo { } } +fn get_bob_info() -> registry::UserInfo { + registry::UserInfo { + handle: Some(String::from("bobby").as_bytes().to_vec()), + avatar_uri: Some(String::from("http://avatar-url.com/bob").as_bytes().to_vec()), + about: Some(String::from("my name is bob").as_bytes().to_vec()), + } +} + const ALICE_ACCOUNT_ID: u64 = 1; const DEFAULT_TERMS_ID: u32 = 0; @@ -69,9 +77,11 @@ fn buy_membership() { let member_id = assert_ok_unwrap(Membership::member_id_by_account_id(&ALICE_ACCOUNT_ID), "member id not assigned"); - let profile = assert_ok_unwrap(Membership::member_profile(&member_id), "member profile created"); + let profile = assert_ok_unwrap(Membership::member_profile(&member_id), "member profile not created"); assert_eq!(Some(profile.handle), get_alice_info().handle); + assert_eq!(Some(profile.avatar_uri), get_alice_info().avatar_uri); + assert_eq!(Some(profile.about), get_alice_info().about); assert_eq!(Balances::free_balance(&ALICE_ACCOUNT_ID), SURPLUS_BALANCE); @@ -146,4 +156,64 @@ fn unique_handles() { assert!(buy_default_membership_as_alice().is_err()); }); -} \ No newline at end of file +} + +#[test] +fn update_profile() { + const DEFAULT_FEE: u32 = 500; + const SURPLUS_BALANCE: u32 = 500; + + with_externalities(&mut ExtBuilder::default() + .default_paid_membership_fee(DEFAULT_FEE).build(), || + { + let initial_balance = DEFAULT_FEE + SURPLUS_BALANCE; + set_alice_free_balance(initial_balance); + + assert_ok!(buy_default_membership_as_alice()); + + assert_ok!(Membership::update_profile(Origin::signed(ALICE_ACCOUNT_ID), get_bob_info())); + + let member_id = assert_ok_unwrap(Membership::member_id_by_account_id(&ALICE_ACCOUNT_ID), "member id not assigned"); + + let profile = assert_ok_unwrap(Membership::member_profile(&member_id), "member profile created"); + + assert_eq!(Some(profile.handle), get_bob_info().handle); + assert_eq!(Some(profile.avatar_uri), get_bob_info().avatar_uri); + assert_eq!(Some(profile.about), get_bob_info().about); + + }); +} + +#[test] +fn member_sub_accounts() { + with_externalities(&mut ExtBuilder::default().build(), || + { + let initial_balance = 20000; + set_alice_free_balance(initial_balance); + assert_ok!(buy_default_membership_as_alice()); + + let member_id = assert_ok_unwrap(Membership::member_id_by_account_id(&ALICE_ACCOUNT_ID), "member id not assigned"); + + const NEW_PRIMARY_ACCOUNT: u64 = 2; + + assert_ok!(Membership::change_member_primary_account(Origin::signed(ALICE_ACCOUNT_ID), NEW_PRIMARY_ACCOUNT)); + + // new account id should be associated with the member + assert_eq!(assert_ok_unwrap(Membership::member_id_by_account_id(&NEW_PRIMARY_ACCOUNT), "member id not found"), member_id); + + const NEW_SUB_ACCOUNT_1: u64 = 3; + const NEW_SUB_ACCOUNT_2: u64 = 4; + assert_ok!(Membership::add_member_sub_account(Origin::signed(NEW_PRIMARY_ACCOUNT), NEW_SUB_ACCOUNT_1)); + assert_ok!(Membership::add_member_sub_account(Origin::signed(NEW_PRIMARY_ACCOUNT), NEW_SUB_ACCOUNT_2)); + + let profile = assert_ok_unwrap(Membership::member_profile(&member_id), "member profile not found"); + assert_eq!(profile.sub_accounts, vec![NEW_SUB_ACCOUNT_1, NEW_SUB_ACCOUNT_2]); + + assert_ok!(Membership::remove_member_sub_account(Origin::signed(NEW_PRIMARY_ACCOUNT), NEW_SUB_ACCOUNT_1)); + + let profile = assert_ok_unwrap(Membership::member_profile(&member_id), "member profile not found"); + assert_eq!(profile.sub_accounts, vec![NEW_SUB_ACCOUNT_2]); + + }); +} + From 74531d2d6b3686a8bd9b14a1a7eee7b4369de368 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 20 Mar 2019 09:46:18 +0200 Subject: [PATCH 24/33] membership sub accounts, fix updating subaccounts --- src/membership/registry.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/membership/registry.rs b/src/membership/registry.rs index 8263f1b710..9596892be9 100644 --- a/src/membership/registry.rs +++ b/src/membership/registry.rs @@ -297,6 +297,7 @@ decl_module! { let mut profile = Self::ensure_profile(member_id)?; profile.sub_accounts.push(sub_account.clone()); >::insert(member_id, profile); + >::insert(&sub_account, member_id); Self::deposit_event(RawEvent::MemberAddedSubAccount(member_id, sub_account)); } @@ -318,6 +319,7 @@ decl_module! { profile.sub_accounts = profile.sub_accounts.into_iter().filter(|account| !(*account == sub_account)).collect(); >::insert(member_id, profile); + >::remove(&sub_account); Self::deposit_event(RawEvent::MemberRemovedSubAccount(member_id, sub_account)); } } From 3328650a397f5aae7654cb442929cce23d4c1d7e Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 20 Mar 2019 10:07:29 +0200 Subject: [PATCH 25/33] membership tests --- src/membership/tests.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/membership/tests.rs b/src/membership/tests.rs index e21bf7acb4..9c63658227 100644 --- a/src/membership/tests.rs +++ b/src/membership/tests.rs @@ -196,9 +196,13 @@ fn member_sub_accounts() { const NEW_PRIMARY_ACCOUNT: u64 = 2; + // only primary account can change + assert!(Membership::change_member_primary_account(Origin::signed(NEW_PRIMARY_ACCOUNT), NEW_PRIMARY_ACCOUNT).is_err()); + + // primary account should successfully change account assert_ok!(Membership::change_member_primary_account(Origin::signed(ALICE_ACCOUNT_ID), NEW_PRIMARY_ACCOUNT)); - // new account id should be associated with the member + // new account id should be associated with the same member assert_eq!(assert_ok_unwrap(Membership::member_id_by_account_id(&NEW_PRIMARY_ACCOUNT), "member id not found"), member_id); const NEW_SUB_ACCOUNT_1: u64 = 3; From bf2f5d51e6f56e9b710e2f92b62cf6a0e67ee716 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 20 Mar 2019 11:12:53 +0200 Subject: [PATCH 26/33] introduce IsActiveMember trait --- src/lib.rs | 5 +++-- src/membership/registry.rs | 12 ++++++++++++ src/traits.rs | 9 +++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 src/traits.rs diff --git a/src/lib.rs b/src/lib.rs index b4cf410acc..4e7fe9f8f9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,6 +19,7 @@ use governance::{election, council, proposals}; mod memo; mod membership; use membership::registry; +mod traits; use rstd::prelude::*; #[cfg(feature = "std")] @@ -26,7 +27,7 @@ use primitives::bytes; use primitives::{Ed25519AuthorityId, OpaqueMetadata}; use runtime_primitives::{ ApplyResult, transaction_validity::TransactionValidity, Ed25519Signature, generic, - traits::{self, Convert, BlakeTwo256, Block as BlockT, StaticLookup}, create_runtime_str + traits::{self as runtime_traits, Convert, BlakeTwo256, Block as BlockT, StaticLookup}, create_runtime_str }; use client::{ block_builder::api::{CheckInherentsResult, InherentData, self as block_builder_api}, @@ -69,7 +70,7 @@ pub mod opaque { #[derive(PartialEq, Eq, Clone, Default, Encode, Decode)] #[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] pub struct UncheckedExtrinsic(#[cfg_attr(feature = "std", serde(with="bytes"))] pub Vec); - impl traits::Extrinsic for UncheckedExtrinsic { + impl runtime_traits::Extrinsic for UncheckedExtrinsic { fn is_signed(&self) -> Option { None } diff --git a/src/membership/registry.rs b/src/membership/registry.rs index 9596892be9..90c6e786d7 100644 --- a/src/membership/registry.rs +++ b/src/membership/registry.rs @@ -11,6 +11,7 @@ use crate::governance::{GovernanceCurrency, BalanceOf }; use runtime_io::print; use {timestamp}; use crate::{VERSION}; +use crate::traits; pub trait Trait: system::Trait + GovernanceCurrency + timestamp::Trait { type Event: From> + Into<::Event>; @@ -184,6 +185,17 @@ impl Module { } } +impl traits::IsActiveMember for Module { + fn is_active_member(who: T::AccountId) -> bool { + match Self::ensure_is_member(&who) + .and_then(|member_id| Self::ensure_profile(member_id)) + { + Ok(profile) => !profile.suspended, + Err(err) => false + } + } +} + decl_module! { pub struct Module for enum Call where origin: T::Origin { fn deposit_event() = default; diff --git a/src/traits.rs b/src/traits.rs new file mode 100644 index 0000000000..949f7f56f4 --- /dev/null +++ b/src/traits.rs @@ -0,0 +1,9 @@ +#![cfg_attr(not(feature = "std"), no_std)] + +use system; + +pub trait IsActiveMember { + fn is_active_member(account_id: T::AccountId) -> bool { + false + } +} \ No newline at end of file From c8b70cc096bd2d5105a4c79d142890bd55dff098 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 20 Mar 2019 12:37:54 +0200 Subject: [PATCH 27/33] use IsActiveMember trait in election and runtime upgrade proposal modules --- src/governance/election.rs | 14 +++++++++----- src/governance/mock.rs | 18 ++++++++++++++---- src/governance/proposals.rs | 18 ++++++++++++++---- src/lib.rs | 2 ++ src/membership/registry.rs | 4 ++-- src/traits.rs | 2 +- 6 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/governance/election.rs b/src/governance/election.rs index 282247e59f..e6d0f109e7 100644 --- a/src/governance/election.rs +++ b/src/governance/election.rs @@ -17,10 +17,14 @@ use super::sealed_vote::SealedVote; pub use super::{ GovernanceCurrency, BalanceOf }; use super::council; +use crate::traits::{IsActiveMember}; + pub trait Trait: system::Trait + council::Trait + GovernanceCurrency { type Event: From> + Into<::Event>; type CouncilElected: CouncilElected>, Self::BlockNumber>; + + type IsActiveMember: IsActiveMember; } #[derive(Clone, Copy, Encode, Decode)] @@ -148,9 +152,9 @@ impl Module { >::block_number() + length } - // TODO This method should be moved to Membership module once it's created. - fn is_member(sender: T::AccountId) -> bool { - !T::Currency::free_balance(&sender).is_zero() + + fn can_participate(sender: &T::AccountId) -> bool { + !T::Currency::free_balance(sender).is_zero() && T::IsActiveMember::is_active_member(sender) } // PUBLIC IMMUTABLES @@ -651,7 +655,7 @@ decl_module! { // Member can make subsequent calls during announcing stage to increase their stake. fn apply(origin, stake: BalanceOf) { let sender = ensure_signed(origin)?; - ensure!(Self::is_member(sender.clone()), "Only members can apply to be on council"); + ensure!(Self::can_participate(&sender), "Only members can apply to be on council"); let stage = Self::stage(); ensure!(Self::stage().is_some(), "election not running"); @@ -674,7 +678,7 @@ decl_module! { fn vote(origin, commitment: T::Hash, stake: BalanceOf) { let sender = ensure_signed(origin)?; - ensure!(Self::is_member(sender.clone()), "Only members can vote for an applicant"); + ensure!(Self::can_participate(&sender), "Only members can vote for an applicant"); let stage = Self::stage(); ensure!(Self::stage().is_some(), "election not running"); diff --git a/src/governance/mock.rs b/src/governance/mock.rs index 5cb193eddc..ce5bb9bd4a 100644 --- a/src/governance/mock.rs +++ b/src/governance/mock.rs @@ -3,6 +3,7 @@ use rstd::prelude::*; pub use super::{election, council, proposals, GovernanceCurrency}; pub use system; +use crate::traits; pub use primitives::{H256, Blake2Hasher}; pub use runtime_primitives::{ @@ -17,6 +18,16 @@ impl_outer_origin! { pub enum Origin for Test {} } +pub struct AnyAccountIsMember {} +impl traits::IsActiveMember for AnyAccountIsMember { + fn is_active_member(who: &T::AccountId) -> bool { + true + } +} + +// default trait implementation - any account is not a member +// impl traits::IsActiveMember 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. @@ -53,10 +64,10 @@ impl election::Trait for Test { type Event = (); type CouncilElected = (Council,); + + type IsActiveMember = AnyAccountIsMember; } -impl proposals::Trait for Test { - type Event = (); -} + impl balances::Trait for Test { type Event = (); @@ -92,6 +103,5 @@ pub fn initial_test_ext() -> runtime_io::TestExternalities { pub type Election = election::Module; pub type Council = council::Module; -pub type Proposals = proposals::Module; pub type System = system::Module; pub type Balances = balances::Module; diff --git a/src/governance/proposals.rs b/src/governance/proposals.rs index 2a08fe19c9..58878dc9e9 100644 --- a/src/governance/proposals.rs +++ b/src/governance/proposals.rs @@ -8,6 +8,7 @@ use rstd::prelude::*; use super::council; pub use super::{ GovernanceCurrency, BalanceOf }; +use crate::traits::{IsActiveMember}; const DEFAULT_APPROVAL_QUORUM: u32 = 60; const DEFAULT_MIN_STAKE: u64 = 100; @@ -115,6 +116,8 @@ pub struct TallyResult { pub trait Trait: timestamp::Trait + council::Trait + GovernanceCurrency { /// The overarching event type. type Event: From> + Into<::Event>; + + type IsActiveMember: IsActiveMember; } decl_event!( @@ -221,7 +224,7 @@ decl_module! { ) { let proposer = ensure_signed(origin)?; - ensure!(Self::is_member(proposer.clone()), MSG_ONLY_MEMBERS_CAN_PROPOSE); + ensure!(Self::can_participate(proposer.clone()), MSG_ONLY_MEMBERS_CAN_PROPOSE); ensure!(stake >= Self::min_stake(), MSG_STAKE_IS_TOO_LOW); ensure!(!name.is_empty(), MSG_EMPTY_NAME_PROVIDED); @@ -345,9 +348,8 @@ impl Module { >::block_number() } - // TODO This method should be moved to Membership module once it's created. - fn is_member(sender: T::AccountId) -> bool { - !T::Currency::free_balance(&sender).is_zero() + fn can_participate(sender: T::AccountId) -> bool { + !T::Currency::free_balance(&sender).is_zero() && T::IsActiveMember::is_active_member(&sender) } fn is_councilor(sender: &T::AccountId) -> bool { @@ -613,6 +615,14 @@ mod tests { impl Trait for Test { type Event = (); + type IsActiveMember = AnyAccountIsMember; + } + + pub struct AnyAccountIsMember {} + impl IsActiveMember for AnyAccountIsMember { + fn is_active_member(who: &T::AccountId) -> bool { + true + } } type System = system::Module; diff --git a/src/lib.rs b/src/lib.rs index 4e7fe9f8f9..0320ce8d76 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -211,11 +211,13 @@ impl governance::GovernanceCurrency for Runtime { impl governance::proposals::Trait for Runtime { type Event = Event; + type IsActiveMember = Membership; } impl governance::election::Trait for Runtime { type Event = Event; type CouncilElected = (Council,); + type IsActiveMember = Membership; } impl governance::council::Trait for Runtime { diff --git a/src/membership/registry.rs b/src/membership/registry.rs index 90c6e786d7..dfd289fa1e 100644 --- a/src/membership/registry.rs +++ b/src/membership/registry.rs @@ -186,8 +186,8 @@ impl Module { } impl traits::IsActiveMember for Module { - fn is_active_member(who: T::AccountId) -> bool { - match Self::ensure_is_member(&who) + fn is_active_member(who: &T::AccountId) -> bool { + match Self::ensure_is_member(who) .and_then(|member_id| Self::ensure_profile(member_id)) { Ok(profile) => !profile.suspended, diff --git a/src/traits.rs b/src/traits.rs index 949f7f56f4..8b24b830da 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -3,7 +3,7 @@ use system; pub trait IsActiveMember { - fn is_active_member(account_id: T::AccountId) -> bool { + fn is_active_member(account_id: &T::AccountId) -> bool { false } } \ No newline at end of file From c7326e167d7aeeb7cd9fbc2d2df5841b6fd1aee5 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 20 Mar 2019 15:57:32 +0200 Subject: [PATCH 28/33] factor our migration to its own module --- src/lib.rs | 6 ++++ src/membership/registry.rs | 33 ++++--------------- src/migration.rs | 65 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 27 deletions(-) create mode 100644 src/migration.rs diff --git a/src/lib.rs b/src/lib.rs index 0320ce8d76..93e6ba1afc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,6 +20,7 @@ mod memo; mod membership; use membership::registry; mod traits; +mod migration; use rstd::prelude::*; #[cfg(feature = "std")] @@ -236,6 +237,10 @@ impl membership::registry::Trait for Runtime { type SubscriptionId = u64; } +impl migration::Trait for Runtime { + type Event = Event; +} + construct_runtime!( pub enum Runtime with Log(InternalLog: DigestItem) where Block = Block, @@ -257,6 +262,7 @@ construct_runtime!( Council: council::{Module, Call, Storage, Event, Config}, Memo: memo::{Module, Call, Storage, Event}, Membership: registry::{Module, Call, Storage, Event, Config}, + Migration: migration::{Module, Call, Storage, Event}, } ); diff --git a/src/membership/registry.rs b/src/membership/registry.rs index dfd289fa1e..1da6611fa4 100644 --- a/src/membership/registry.rs +++ b/src/membership/registry.rs @@ -8,9 +8,7 @@ use srml_support::traits::{Currency}; use runtime_primitives::traits::{Zero, SimpleArithmetic, As, Member, MaybeSerializeDebug}; use system::{self, ensure_signed}; use crate::governance::{GovernanceCurrency, BalanceOf }; -use runtime_io::print; use {timestamp}; -use crate::{VERSION}; use crate::traits; pub trait Trait: system::Trait + GovernanceCurrency + timestamp::Trait { @@ -100,10 +98,6 @@ impl Default for PaidMembershipTerms { decl_storage! { trait Store for Module as Membership { - /// Records at what runtime spec version the store was initialized. This allows the runtime - /// to know when to run initialize code if it was installed as an update. - pub StoreSpecVersion get(store_spec_version) build(|_| Some(VERSION.spec_version)) : Option; - /// MemberId's start at this value pub FirstMemberId get(first_member_id) config(first_member_id): T::MemberId = T::MemberId::sa(DEFAULT_FIRST_MEMBER_ID); @@ -172,16 +166,13 @@ decl_event! { } } +/// Initialization step that runs when the runtime is installed as a runtime upgrade +/// This will and should ONLY be called from the migration module that keeps track of +/// the store version! impl Module { - /// Initialization step that runs when the runtime is installed as a runtime upgrade - /// Remember to remove this code in next release, or guard the code with the spec version - // Given it is easy to forget to do this, we need a better arrangement. - fn runtime_initialization(spec_version: u32) { - if 5 == spec_version { - print("Running New Runtime Init Code"); - let default_terms: PaidMembershipTerms = Default::default(); - >::insert(T::PaidTermId::sa(0), default_terms); - } + pub fn initialize_storage() { + let default_terms: PaidMembershipTerms = Default::default(); + >::insert(default_terms.id, default_terms); } } @@ -200,18 +191,6 @@ decl_module! { pub struct Module for enum Call where origin: T::Origin { fn deposit_event() = default; - fn on_initialise(_now: T::BlockNumber) { - if Self::store_spec_version().map_or(true, |spec_version| VERSION.spec_version > spec_version) { - // Mark store version with latest so we don't run initialization code again - >::put(VERSION.spec_version); - Self::runtime_initialization(VERSION.spec_version); - } - } - - fn on_finalise(_now: T::BlockNumber) { - - } - /// Non-members can buy membership pub fn buy_membership(origin, paid_terms_id: T::PaidTermId, user_info: UserInfo) { let who = ensure_signed(origin)?; diff --git a/src/migration.rs b/src/migration.rs new file mode 100644 index 0000000000..fe25cd8a60 --- /dev/null +++ b/src/migration.rs @@ -0,0 +1,65 @@ +#![cfg_attr(not(feature = "std"), no_std)] + +use srml_support::{StorageValue, dispatch::Result, decl_module, decl_storage, decl_event, ensure}; +use system; +use rstd::prelude::*; +use runtime_io::print; +use crate::{VERSION}; +use crate::membership::registry; + +pub trait Trait: system::Trait + registry::Trait { + type Event: From> + Into<::Event>; +} + +decl_storage! { + trait Store for Module as Migration { + /// Records at what runtime spec version the store was initialized. This allows the runtime + /// to know when to run initialize code if it was installed as an update. + pub SpecVersion get(spec_version) build(|_| Some(VERSION.spec_version)) : Option; + } +} + +decl_event! { + pub enum Event where ::BlockNumber { + Migrated(BlockNumber, u32), + } +} + +// When preparing a new major runtime release version bump this value to match it and update +// the initialization code in runtime_initialization(). Because of the way substrate runs runtime code +// the runtime doesn't need to maintain any logic for old migrations. All knowledge about state of the chain and runtime +// prior to the new runtime taking over is implicit in the migration code implementation. If assumptions are incorrect +// behaviour is undefined. +const MIGRATION_FOR_SPEC_VERSION: u32 = 5; + +impl Module { + fn runtime_initialization() { + if VERSION.spec_version != MIGRATION_FOR_SPEC_VERSION { return } + + print("running runtime initializers"); + + >::initialize_storage(); + + // ... + // add initialization of other modules introduced in this runtime + // ... + + Self::deposit_event(RawEvent::Migrated(>::block_number(), VERSION.spec_version)); + } +} + +decl_module! { + pub struct Module for enum Call where origin: T::Origin { + fn deposit_event() = default; + + fn on_initialise(_now: T::BlockNumber) { + if Self::spec_version().map_or(true, |spec_version| VERSION.spec_version > spec_version) { + // mark store version with current version of the runtime + >::put(VERSION.spec_version); + + // run migrations and store initializers + Self::runtime_initialization(); + } + } + } +} From 53e54b6bbc67e552c14ddbb601ef8479a6d30c97 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Wed, 20 Mar 2019 16:04:58 +0200 Subject: [PATCH 29/33] additional comments on membership methods --- src/membership/registry.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/membership/registry.rs b/src/membership/registry.rs index 1da6611fa4..cf661a1e46 100644 --- a/src/membership/registry.rs +++ b/src/membership/registry.rs @@ -218,25 +218,29 @@ decl_module! { Self::deposit_event(RawEvent::MemberRegistered(member_id, who.clone())); } + /// Change member's about text pub fn change_member_about_text(origin, text: Vec) { let who = ensure_signed(origin)?; let member_id = Self::ensure_is_member_primary_account(who.clone())?; Self::_change_member_about_text(member_id, &text)?; } + /// Change member's avatar pub fn change_member_avatar(origin, uri: Vec) { let who = ensure_signed(origin)?; let member_id = Self::ensure_is_member_primary_account(who.clone())?; Self::_change_member_avatar(member_id, &uri)?; } - /// Change member's handle. + /// 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) { let who = ensure_signed(origin)?; let member_id = Self::ensure_is_member_primary_account(who.clone())?; 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 who = ensure_signed(origin)?; let member_id = Self::ensure_is_member_primary_account(who.clone())?; From 3fab73d456c4126024454110c99298a1c9430c95 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 21 Mar 2019 13:24:27 +0200 Subject: [PATCH 30/33] membership sub accounts - revert (move to staked roles) --- src/membership/registry.rs | 70 +------------------------------------- src/membership/tests.rs | 37 -------------------- 2 files changed, 1 insertion(+), 106 deletions(-) diff --git a/src/membership/registry.rs b/src/membership/registry.rs index cf661a1e46..0e2acfa97b 100644 --- a/src/membership/registry.rs +++ b/src/membership/registry.rs @@ -51,7 +51,6 @@ pub struct Profile { pub entry: EntryMethod, pub suspended: bool, pub subscription: Option, - pub sub_accounts: Vec } #[derive(Clone, Debug, Encode, Decode, PartialEq)] @@ -107,8 +106,7 @@ decl_storage! { /// Mapping of member ids to their corresponding primary accountid pub AccountIdByMemberId get(account_id_by_member_id) : map T::MemberId => T::AccountId; - /// Mapping of members' accountid ids to their member id. - /// A member can have one primary account and multiple sub accounts associated with their profile + /// Mapping of members' account ids to their member id. pub MemberIdByAccountId get(member_id_by_account_id) : map T::AccountId => Option; /// Mapping of member's id to their membership profile @@ -160,9 +158,6 @@ decl_event! { MemberUpdatedAboutText(MemberId), MemberUpdatedAvatar(MemberId), MemberUpdatedHandle(MemberId), - MemberChangedPrimaryAccount(MemberId, AccountId), - MemberAddedSubAccount(MemberId, AccountId), - MemberRemovedSubAccount(MemberId, AccountId), } } @@ -255,68 +250,6 @@ decl_module! { Self::_change_member_handle(member_id, handle)?; } } - - // Notes: - // Ability to change primary account and add/remove sub accounts should be restricted - // to accounts that are not actively being used in other roles in the platform? - // Other roles in the platform should be associated directly with member_id instead of an account_id, - // but can still use a seprate account/key to sign extrinsics, have a seprate balance from primary account, - // and a way to limit damage from a compromised sub account. - // Maybe should also not be allowed if member is suspended? - // Main usecase for changing primary account is for identity recoverability - pub fn change_member_primary_account(origin, new_primary_account: T::AccountId) { - let who = ensure_signed(origin)?; - - // only primary account can assign new primary account - let member_id = Self::ensure_is_member_primary_account(who.clone())?; - - // ensure new_primary_account isn't already associated with any existing member - Self::ensure_not_member(&new_primary_account)?; - - // update associated accounts - >::remove(&who); - >::insert(&new_primary_account, member_id); - - // update primary account - >::insert(member_id, new_primary_account.clone()); - Self::deposit_event(RawEvent::MemberChangedPrimaryAccount(member_id, new_primary_account)); - } - - pub fn add_member_sub_account(origin, sub_account: T::AccountId) { - let who = ensure_signed(origin)?; - // only primary account can manage sub accounts - let member_id = Self::ensure_is_member_primary_account(who.clone())?; - // ensure sub_account isn't already associated with any existing member - Self::ensure_not_member(&sub_account)?; - - let mut profile = Self::ensure_profile(member_id)?; - profile.sub_accounts.push(sub_account.clone()); - >::insert(member_id, profile); - >::insert(&sub_account, member_id); - Self::deposit_event(RawEvent::MemberAddedSubAccount(member_id, sub_account)); - } - - pub fn remove_member_sub_account(origin, sub_account: T::AccountId) { - let who = ensure_signed(origin)?; - // only primary account can manage sub accounts - let member_id = Self::ensure_is_member_primary_account(who.clone())?; - - // ensure account is a sub account - // primary account cannot be added as a subaccount in add_member_sub_account() - ensure!(who != sub_account, "not sub account"); - - // ensure sub_account is associated with member - let sub_account_member_id = Self::ensure_is_member(&sub_account)?; - ensure!(member_id == sub_account_member_id, "not member sub account"); - - let mut profile = Self::ensure_profile(member_id)?; - - profile.sub_accounts = profile.sub_accounts.into_iter().filter(|account| !(*account == sub_account)).collect(); - - >::insert(member_id, profile); - >::remove(&sub_account); - Self::deposit_event(RawEvent::MemberRemovedSubAccount(member_id, sub_account)); - } } } @@ -403,7 +336,6 @@ impl Module { entry: EntryMethod::Paid(paid_terms_id), suspended: false, subscription: None, - sub_accounts: vec![], }; >::insert(who.clone(), new_member_id); diff --git a/src/membership/tests.rs b/src/membership/tests.rs index 9c63658227..c4f842403c 100644 --- a/src/membership/tests.rs +++ b/src/membership/tests.rs @@ -184,40 +184,3 @@ fn update_profile() { }); } -#[test] -fn member_sub_accounts() { - with_externalities(&mut ExtBuilder::default().build(), || - { - let initial_balance = 20000; - set_alice_free_balance(initial_balance); - assert_ok!(buy_default_membership_as_alice()); - - let member_id = assert_ok_unwrap(Membership::member_id_by_account_id(&ALICE_ACCOUNT_ID), "member id not assigned"); - - const NEW_PRIMARY_ACCOUNT: u64 = 2; - - // only primary account can change - assert!(Membership::change_member_primary_account(Origin::signed(NEW_PRIMARY_ACCOUNT), NEW_PRIMARY_ACCOUNT).is_err()); - - // primary account should successfully change account - assert_ok!(Membership::change_member_primary_account(Origin::signed(ALICE_ACCOUNT_ID), NEW_PRIMARY_ACCOUNT)); - - // new account id should be associated with the same member - assert_eq!(assert_ok_unwrap(Membership::member_id_by_account_id(&NEW_PRIMARY_ACCOUNT), "member id not found"), member_id); - - const NEW_SUB_ACCOUNT_1: u64 = 3; - const NEW_SUB_ACCOUNT_2: u64 = 4; - assert_ok!(Membership::add_member_sub_account(Origin::signed(NEW_PRIMARY_ACCOUNT), NEW_SUB_ACCOUNT_1)); - assert_ok!(Membership::add_member_sub_account(Origin::signed(NEW_PRIMARY_ACCOUNT), NEW_SUB_ACCOUNT_2)); - - let profile = assert_ok_unwrap(Membership::member_profile(&member_id), "member profile not found"); - assert_eq!(profile.sub_accounts, vec![NEW_SUB_ACCOUNT_1, NEW_SUB_ACCOUNT_2]); - - assert_ok!(Membership::remove_member_sub_account(Origin::signed(NEW_PRIMARY_ACCOUNT), NEW_SUB_ACCOUNT_1)); - - let profile = assert_ok_unwrap(Membership::member_profile(&member_id), "member profile not found"); - assert_eq!(profile.sub_accounts, vec![NEW_SUB_ACCOUNT_2]); - - }); -} - From 9f07751ee02042ddce6646895cbf09db2527b2e6 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Thu, 21 Mar 2019 14:47:53 +0200 Subject: [PATCH 31/33] add member through screener --- src/membership/mock.rs | 2 +- src/membership/registry.rs | 59 +++++++++++++++++++++++++++++++++++++- src/membership/tests.rs | 20 +++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) diff --git a/src/membership/mock.rs b/src/membership/mock.rs index 89fb228341..c85efaf4c6 100644 --- a/src/membership/mock.rs +++ b/src/membership/mock.rs @@ -21,7 +21,7 @@ impl_outer_origin! { // 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. -#[derive(Clone, Eq, PartialEq)] +#[derive(Clone, Eq, PartialEq, Debug)] pub struct Test; impl system::Trait for Test { type Origin = Origin; diff --git a/src/membership/registry.rs b/src/membership/registry.rs index 0e2acfa97b..a088ce275a 100644 --- a/src/membership/registry.rs +++ b/src/membership/registry.rs @@ -68,7 +68,7 @@ struct CheckedUserInfo { } //#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] -#[derive(Encode, Decode)] +#[derive(Encode, Decode, Debug, PartialEq)] pub enum EntryMethod { Paid(T::PaidTermId), Screening(T::AccountId), @@ -138,6 +138,8 @@ decl_storage! { /// Is the platform is accepting new members or not pub NewMembershipsAllowed get(new_memberships_allowed) : bool = true; + pub ScreeningAuthority get(screening_authority) : Option; + // User Input Validation parameters - do these really need to be state variables // I don't see a need to adjust these in future? pub MinHandleLength get(min_handle_length) : u32 = DEFAULT_MIN_HANDLE_LENGTH; @@ -250,6 +252,37 @@ decl_module! { Self::_change_member_handle(member_id, handle)?; } } + + pub fn add_screened_member(origin, new_member: T::AccountId, user_info: UserInfo) { + // ensure sender is screening authority + let sender = ensure_signed(origin)?; + + if let Some(screening_authority) = Self::screening_authority() { + ensure!(sender == screening_authority, "not screener"); + } else { + // no screening authority defined. Cannot accept this request + return Err("no screening authority defined"); + } + + // 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)?; + + 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_new_screened_member(sender, &new_member, &user_info); + + Self::deposit_event(RawEvent::MemberRegistered(member_id, new_member.clone())); + } + + pub fn set_screening_authority(authority: T::AccountId) { + >::put(authority); + } } } @@ -347,6 +380,30 @@ impl Module { new_member_id } + fn insert_new_screened_member(authority: T::AccountId, who: &T::AccountId, user_info: &CheckedUserInfo) -> T::MemberId { + let new_member_id = Self::next_member_id(); + + let profile: Profile = Profile { + id: new_member_id, + handle: user_info.handle.clone(), + avatar_uri: user_info.avatar_uri.clone(), + about: user_info.about.clone(), + registered_at_block: >::block_number(), + registered_at_time: >::now(), + entry: EntryMethod::Screening(authority), + suspended: false, + subscription: None, + }; + + >::insert(who.clone(), new_member_id); + >::insert(new_member_id, who.clone()); + >::insert(new_member_id, profile); + >::insert(user_info.handle.clone(), new_member_id); + >::mutate(|n| { *n += T::MemberId::sa(1); }); + + new_member_id + } + fn _change_member_about_text (id: T::MemberId, text: &Vec) -> dispatch::Result { let mut profile = Self::ensure_profile(id)?; let text = Self::validate_text(text); diff --git a/src/membership/tests.rs b/src/membership/tests.rs index c4f842403c..0de04124a3 100644 --- a/src/membership/tests.rs +++ b/src/membership/tests.rs @@ -184,3 +184,23 @@ fn update_profile() { }); } +#[test] +fn add_screened_member() { + with_externalities(&mut ExtBuilder::default().build(), || + { + let screening_authority = 5; + >::put(&screening_authority); + + assert_ok!(Membership::add_screened_member(Origin::signed(screening_authority), ALICE_ACCOUNT_ID, get_alice_info())); + + let member_id = assert_ok_unwrap(Membership::member_id_by_account_id(&ALICE_ACCOUNT_ID), "member id not assigned"); + + let profile = assert_ok_unwrap(Membership::member_profile(&member_id), "member profile created"); + + assert_eq!(Some(profile.handle), get_alice_info().handle); + assert_eq!(Some(profile.avatar_uri), get_alice_info().avatar_uri); + assert_eq!(Some(profile.about), get_alice_info().about); + assert_eq!(registry::EntryMethod::Screening(screening_authority), profile.entry); + + }); +} From 3e43d9ed364803c70513e94d8860f156cfee1c11 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Mon, 25 Mar 2019 23:14:26 +0200 Subject: [PATCH 32/33] refactor membership: renamed registry module to members --- src/lib.rs | 10 +++--- src/membership/{registry.rs => members.rs} | 0 src/membership/mock.rs | 8 ++--- src/membership/mod.rs | 2 +- src/membership/tests.rs | 40 +++++++++++----------- src/migration.rs | 6 ++-- 6 files changed, 33 insertions(+), 33 deletions(-) rename src/membership/{registry.rs => members.rs} (100%) diff --git a/src/lib.rs b/src/lib.rs index 93e6ba1afc..99fef0a8a3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,7 +18,7 @@ pub mod governance; use governance::{election, council, proposals}; mod memo; mod membership; -use membership::registry; +use membership::members; mod traits; mod migration; @@ -212,13 +212,13 @@ impl governance::GovernanceCurrency for Runtime { impl governance::proposals::Trait for Runtime { type Event = Event; - type IsActiveMember = Membership; + type IsActiveMember = Members; } impl governance::election::Trait for Runtime { type Event = Event; type CouncilElected = (Council,); - type IsActiveMember = Membership; + type IsActiveMember = Members; } impl governance::council::Trait for Runtime { @@ -230,7 +230,7 @@ impl memo::Trait for Runtime { type Event = Event; } -impl membership::registry::Trait for Runtime { +impl members::Trait for Runtime { type Event = Event; type MemberId = u64; type PaidTermId = u64; @@ -261,7 +261,7 @@ construct_runtime!( CouncilElection: election::{Module, Call, Storage, Event, Config}, Council: council::{Module, Call, Storage, Event, Config}, Memo: memo::{Module, Call, Storage, Event}, - Membership: registry::{Module, Call, Storage, Event, Config}, + Members: members::{Module, Call, Storage, Event, Config}, Migration: migration::{Module, Call, Storage, Event}, } ); diff --git a/src/membership/registry.rs b/src/membership/members.rs similarity index 100% rename from src/membership/registry.rs rename to src/membership/members.rs diff --git a/src/membership/mock.rs b/src/membership/mock.rs index c85efaf4c6..b4e42c1be2 100644 --- a/src/membership/mock.rs +++ b/src/membership/mock.rs @@ -2,7 +2,7 @@ use rstd::prelude::*; pub use crate::governance::{GovernanceCurrency}; -pub use super::{registry}; +pub use super::{members}; pub use system; pub use primitives::{H256, Blake2Hasher}; @@ -69,7 +69,7 @@ impl GovernanceCurrency for Test { type Currency = balances::Module; } -impl registry::Trait for Test { +impl members::Trait for Test { type Event = (); type MemberId = u32; type PaidTermId = u32; @@ -101,7 +101,7 @@ impl ExtBuilder { pub fn build(self) -> runtime_io::TestExternalities { let mut t = system::GenesisConfig::::default().build_storage().unwrap().0; - t.extend(registry::GenesisConfig::{ + t.extend(members::GenesisConfig::{ first_member_id: self.first_member_id, default_paid_membership_fee: self.default_paid_membership_fee, }.build_storage().unwrap().0); @@ -112,4 +112,4 @@ impl ExtBuilder { pub type System = system::Module; pub type Balances = balances::Module; -pub type Membership = registry::Module; +pub type Members = members::Module; diff --git a/src/membership/mod.rs b/src/membership/mod.rs index a07ceef62a..35c272c7ad 100644 --- a/src/membership/mod.rs +++ b/src/membership/mod.rs @@ -1,6 +1,6 @@ #![cfg_attr(not(feature = "std"), no_std)] -pub mod registry; +pub mod members; mod mock; mod tests; \ No newline at end of file diff --git a/src/membership/tests.rs b/src/membership/tests.rs index 0de04124a3..5b159d433e 100644 --- a/src/membership/tests.rs +++ b/src/membership/tests.rs @@ -14,16 +14,16 @@ fn assert_ok_unwrap(value: Option, err: &'static str) -> T { } } -fn get_alice_info() -> registry::UserInfo { - registry::UserInfo { +fn get_alice_info() -> members::UserInfo { + members::UserInfo { handle: Some(String::from("alice").as_bytes().to_vec()), avatar_uri: Some(String::from("http://avatar-url.com/alice").as_bytes().to_vec()), about: Some(String::from("my name is alice").as_bytes().to_vec()), } } -fn get_bob_info() -> registry::UserInfo { - registry::UserInfo { +fn get_bob_info() -> members::UserInfo { + members::UserInfo { handle: Some(String::from("bobby").as_bytes().to_vec()), avatar_uri: Some(String::from("http://avatar-url.com/bob").as_bytes().to_vec()), about: Some(String::from("my name is bob").as_bytes().to_vec()), @@ -34,7 +34,7 @@ const ALICE_ACCOUNT_ID: u64 = 1; const DEFAULT_TERMS_ID: u32 = 0; fn buy_default_membership_as_alice() -> dispatch::Result { - Membership::buy_membership(Origin::signed(ALICE_ACCOUNT_ID), DEFAULT_TERMS_ID, get_alice_info()) + Members::buy_membership(Origin::signed(ALICE_ACCOUNT_ID), DEFAULT_TERMS_ID, get_alice_info()) } fn set_alice_free_balance(balance: u32) { @@ -50,10 +50,10 @@ fn initial_state() { .default_paid_membership_fee(DEFAULT_FEE) .first_member_id(DEFAULT_FIRST_ID).build(), || { - assert_eq!(Membership::first_member_id(), DEFAULT_FIRST_ID); - assert_eq!(Membership::next_member_id(), DEFAULT_FIRST_ID); + assert_eq!(Members::first_member_id(), DEFAULT_FIRST_ID); + assert_eq!(Members::next_member_id(), DEFAULT_FIRST_ID); - let default_terms = assert_ok_unwrap(Membership::paid_membership_terms_by_id(DEFAULT_TERMS_ID), "default terms not initialized"); + let default_terms = assert_ok_unwrap(Members::paid_membership_terms_by_id(DEFAULT_TERMS_ID), "default terms not initialized"); assert_eq!(default_terms.id, DEFAULT_TERMS_ID); assert_eq!(default_terms.fee, DEFAULT_FEE); @@ -75,9 +75,9 @@ fn buy_membership() { assert_ok!(buy_default_membership_as_alice()); - let member_id = assert_ok_unwrap(Membership::member_id_by_account_id(&ALICE_ACCOUNT_ID), "member id not assigned"); + let member_id = assert_ok_unwrap(Members::member_id_by_account_id(&ALICE_ACCOUNT_ID), "member id not assigned"); - let profile = assert_ok_unwrap(Membership::member_profile(&member_id), "member profile not created"); + let profile = assert_ok_unwrap(Members::member_profile(&member_id), "member profile not created"); assert_eq!(Some(profile.handle), get_alice_info().handle); assert_eq!(Some(profile.avatar_uri), get_alice_info().avatar_uri); @@ -112,7 +112,7 @@ fn new_memberships_allowed_flag() { let initial_balance = DEFAULT_FEE + 1; set_alice_free_balance(initial_balance); - >::put(false); + >::put(false); assert!(buy_default_membership_as_alice().is_err()); }); @@ -150,7 +150,7 @@ fn unique_handles() { set_alice_free_balance(initial_balance); // alice's handle already taken - >::insert(get_alice_info().handle.unwrap(), 1); + >::insert(get_alice_info().handle.unwrap(), 1); // should not be allowed to buy membership with that handle assert!(buy_default_membership_as_alice().is_err()); @@ -171,11 +171,11 @@ fn update_profile() { assert_ok!(buy_default_membership_as_alice()); - assert_ok!(Membership::update_profile(Origin::signed(ALICE_ACCOUNT_ID), get_bob_info())); + assert_ok!(Members::update_profile(Origin::signed(ALICE_ACCOUNT_ID), get_bob_info())); - let member_id = assert_ok_unwrap(Membership::member_id_by_account_id(&ALICE_ACCOUNT_ID), "member id not assigned"); + let member_id = assert_ok_unwrap(Members::member_id_by_account_id(&ALICE_ACCOUNT_ID), "member id not assigned"); - let profile = assert_ok_unwrap(Membership::member_profile(&member_id), "member profile created"); + let profile = assert_ok_unwrap(Members::member_profile(&member_id), "member profile created"); assert_eq!(Some(profile.handle), get_bob_info().handle); assert_eq!(Some(profile.avatar_uri), get_bob_info().avatar_uri); @@ -189,18 +189,18 @@ fn add_screened_member() { with_externalities(&mut ExtBuilder::default().build(), || { let screening_authority = 5; - >::put(&screening_authority); + >::put(&screening_authority); - assert_ok!(Membership::add_screened_member(Origin::signed(screening_authority), ALICE_ACCOUNT_ID, get_alice_info())); + assert_ok!(Members::add_screened_member(Origin::signed(screening_authority), ALICE_ACCOUNT_ID, get_alice_info())); - let member_id = assert_ok_unwrap(Membership::member_id_by_account_id(&ALICE_ACCOUNT_ID), "member id not assigned"); + let member_id = assert_ok_unwrap(Members::member_id_by_account_id(&ALICE_ACCOUNT_ID), "member id not assigned"); - let profile = assert_ok_unwrap(Membership::member_profile(&member_id), "member profile created"); + let profile = assert_ok_unwrap(Members::member_profile(&member_id), "member profile created"); assert_eq!(Some(profile.handle), get_alice_info().handle); assert_eq!(Some(profile.avatar_uri), get_alice_info().avatar_uri); assert_eq!(Some(profile.about), get_alice_info().about); - assert_eq!(registry::EntryMethod::Screening(screening_authority), profile.entry); + assert_eq!(members::EntryMethod::Screening(screening_authority), profile.entry); }); } diff --git a/src/migration.rs b/src/migration.rs index fe25cd8a60..d835e0a17d 100644 --- a/src/migration.rs +++ b/src/migration.rs @@ -5,9 +5,9 @@ use system; use rstd::prelude::*; use runtime_io::print; use crate::{VERSION}; -use crate::membership::registry; +use crate::membership::members; -pub trait Trait: system::Trait + registry::Trait { +pub trait Trait: system::Trait + members::Trait { type Event: From> + Into<::Event>; } @@ -38,7 +38,7 @@ impl Module { print("running runtime initializers"); - >::initialize_storage(); + >::initialize_storage(); // ... // add initialization of other modules introduced in this runtime From 75d835d4f02bb5579cf81c2b99fd6e1aa66f2b31 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Tue, 26 Mar 2019 00:12:36 +0200 Subject: [PATCH 33/33] members: refactor insert members method --- src/membership/members.rs | 33 ++++----------------------------- 1 file changed, 4 insertions(+), 29 deletions(-) diff --git a/src/membership/members.rs b/src/membership/members.rs index a088ce275a..68faed9917 100644 --- a/src/membership/members.rs +++ b/src/membership/members.rs @@ -210,7 +210,7 @@ decl_module! { Self::ensure_unique_handle(&user_info.handle)?; let _ = T::Currency::slash(&who, terms.fee); - let member_id = Self::insert_new_paid_member(&who, paid_terms_id, &user_info); + let member_id = Self::insert_member(&who, &user_info, EntryMethod::Paid(paid_terms_id)); Self::deposit_event(RawEvent::MemberRegistered(member_id, who.clone())); } @@ -275,7 +275,7 @@ decl_module! { // ensure handle is not already registered Self::ensure_unique_handle(&user_info.handle)?; - let member_id = Self::insert_new_screened_member(sender, &new_member, &user_info); + let member_id = Self::insert_member(&new_member, &user_info, EntryMethod::Screening(sender)); Self::deposit_event(RawEvent::MemberRegistered(member_id, new_member.clone())); } @@ -355,32 +355,7 @@ impl Module { } // Mutating methods - - fn insert_new_paid_member(who: &T::AccountId, paid_terms_id: T::PaidTermId, user_info: &CheckedUserInfo) -> T::MemberId { - let new_member_id = Self::next_member_id(); - - let profile: Profile = Profile { - id: new_member_id, - handle: user_info.handle.clone(), - avatar_uri: user_info.avatar_uri.clone(), - about: user_info.about.clone(), - registered_at_block: >::block_number(), - registered_at_time: >::now(), - entry: EntryMethod::Paid(paid_terms_id), - suspended: false, - subscription: None, - }; - - >::insert(who.clone(), new_member_id); - >::insert(new_member_id, who.clone()); - >::insert(new_member_id, profile); - >::insert(user_info.handle.clone(), new_member_id); - >::mutate(|n| { *n += T::MemberId::sa(1); }); - - new_member_id - } - - fn insert_new_screened_member(authority: T::AccountId, who: &T::AccountId, user_info: &CheckedUserInfo) -> T::MemberId { + fn insert_member(who: &T::AccountId, user_info: &CheckedUserInfo, entry_method: EntryMethod) -> T::MemberId { let new_member_id = Self::next_member_id(); let profile: Profile = Profile { @@ -390,7 +365,7 @@ impl Module { about: user_info.about.clone(), registered_at_block: >::block_number(), registered_at_time: >::now(), - entry: EntryMethod::Screening(authority), + entry: entry_method, suspended: false, subscription: None, };