From 6ae2e86be452a2bacba55404913ccb857b6d19ed Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 16 Jan 2020 20:42:29 +0100 Subject: [PATCH 1/4] Add `max_members` to `found`, add society genesis for Substrate node --- Cargo.lock | 2 ++ bin/node/cli/Cargo.toml | 1 + bin/node/cli/src/chain_spec.rs | 9 +++++++-- bin/node/runtime/src/lib.rs | 3 +-- bin/node/testing/Cargo.toml | 1 + bin/node/testing/src/genesis.rs | 7 ++++++- frame/society/src/lib.rs | 4 +++- frame/society/src/mock.rs | 1 - frame/society/src/tests.rs | 8 +++++--- 9 files changed, 26 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dd44111c15ee5..c3ffed2446cb6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3041,6 +3041,7 @@ dependencies = [ "pallet-contracts 2.0.0", "pallet-im-online 2.0.0", "pallet-indices 2.0.0", + "pallet-society 2.0.0", "pallet-timestamp 2.0.0", "pallet-transaction-payment 2.0.0", "parity-scale-codec 1.1.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3295,6 +3296,7 @@ dependencies = [ "pallet-grandpa 2.0.0", "pallet-indices 2.0.0", "pallet-session 2.0.0", + "pallet-society 2.0.0", "pallet-staking 2.0.0", "pallet-timestamp 2.0.0", "pallet-transaction-payment 2.0.0", diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index c81cf38a9c2ff..b0a2242f0e4a3 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -72,6 +72,7 @@ pallet-transaction-payment = { version = "2.0.0", path = "../../../frame/transac frame-support = { version = "2.0.0", default-features = false, path = "../../../frame/support" } pallet-im-online = { version = "2.0.0", default-features = false, path = "../../../frame/im-online" } pallet-authority-discovery = { version = "2.0.0", path = "../../../frame/authority-discovery" } +pallet-society = { version = "2.0.0", path = "../../../frame/society" } # node-specific dependencies node-runtime = { version = "2.0.0", path = "../runtime" } diff --git a/bin/node/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs index 26b1f6d2949e3..3214cffa16979 100644 --- a/bin/node/cli/src/chain_spec.rs +++ b/bin/node/cli/src/chain_spec.rs @@ -21,8 +21,8 @@ use sp_core::{Pair, Public, crypto::UncheckedInto, sr25519}; use serde::{Serialize, Deserialize}; use node_runtime::{ AuthorityDiscoveryConfig, BabeConfig, BalancesConfig, ContractsConfig, CouncilConfig, DemocracyConfig, - GrandpaConfig, ImOnlineConfig, IndicesConfig, SessionConfig, SessionKeys, StakerStatus, StakingConfig, SudoConfig, - SystemConfig, TechnicalCommitteeConfig, WASM_BINARY, + GrandpaConfig, ImOnlineConfig, IndicesConfig, SessionConfig, SessionKeys, StakerStatus, StakingConfig, + SocietyConfig, SudoConfig, SystemConfig, TechnicalCommitteeConfig, WASM_BINARY, }; use node_runtime::Block; use node_runtime::constants::currency::*; @@ -295,6 +295,11 @@ pub fn testnet_genesis( }), pallet_membership_Instance1: Some(Default::default()), pallet_treasury: Some(Default::default()), + pallet_society: Some(SocietyConfig { + members: endowed_accounts[0..3].to_vec(), + pot: 100000, + max_members: 999, + }) } } diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index e5e453fcaff04..d769be7b505d5 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -571,7 +571,6 @@ parameter_types! { pub const PeriodSpend: Balance = 500 * DOLLARS; pub const MaxLockDuration: BlockNumber = 36 * 30 * DAYS; pub const ChallengePeriod: BlockNumber = 7 * DAYS; - pub const MaxMembers: u32 = 999; } impl pallet_society::Trait for Runtime { @@ -621,7 +620,7 @@ construct_runtime!( Offences: pallet_offences::{Module, Call, Storage, Event}, RandomnessCollectiveFlip: pallet_randomness_collective_flip::{Module, Call, Storage}, Identity: pallet_identity::{Module, Call, Storage, Event}, - Society: pallet_society::{Module, Call, Storage, Event}, + Society: pallet_society::{Module, Call, Storage, Event, Config}, Recovery: pallet_recovery::{Module, Call, Storage, Event}, } ); diff --git a/bin/node/testing/Cargo.toml b/bin/node/testing/Cargo.toml index 2c868c5906a59..37e85452d5425 100644 --- a/bin/node/testing/Cargo.toml +++ b/bin/node/testing/Cargo.toml @@ -20,6 +20,7 @@ sp-core = { version = "2.0.0", path = "../../../primitives/core" } sp-io = { version = "2.0.0", path = "../../../primitives/io" } frame-support = { version = "2.0.0", path = "../../../frame/support" } pallet-session = { version = "2.0.0", path = "../../../frame/session" } +pallet-society = { version = "2.0.0", path = "../../../frame/society" } sp-runtime = { version = "2.0.0", path = "../../../primitives/runtime" } pallet-staking = { version = "2.0.0", path = "../../../frame/staking" } sc-executor = { version = "0.8", path = "../../../client/executor" } diff --git a/bin/node/testing/src/genesis.rs b/bin/node/testing/src/genesis.rs index 44dd79a7f438a..abedf74a5d6b2 100644 --- a/bin/node/testing/src/genesis.rs +++ b/bin/node/testing/src/genesis.rs @@ -20,7 +20,7 @@ use crate::keyring::*; use sp_keyring::{Ed25519Keyring, Sr25519Keyring}; use node_runtime::{ GenesisConfig, BalancesConfig, SessionConfig, StakingConfig, SystemConfig, - GrandpaConfig, IndicesConfig, ContractsConfig, WASM_BINARY, + GrandpaConfig, IndicesConfig, ContractsConfig, SocietyConfig, WASM_BINARY, }; use node_runtime::constants::currency::*; use sp_core::ChangesTrieConfiguration; @@ -96,5 +96,10 @@ pub fn config(support_changes_trie: bool, code: Option<&[u8]>) -> GenesisConfig pallet_membership_Instance1: Some(Default::default()), pallet_sudo: Some(Default::default()), pallet_treasury: Some(Default::default()), + pallet_society: Some(SocietyConfig { + members: vec![alice(), bob()], + pot: 100000, + max_members: 999, + }), } } diff --git a/frame/society/src/lib.rs b/frame/society/src/lib.rs index 8c99e5b81a8c6..17d24fea101d5 100644 --- a/frame/society/src/lib.rs +++ b/frame/society/src/lib.rs @@ -804,6 +804,7 @@ decl_module! { /// /// Parameters: /// - `founder` - The first member and head of the newly founded society. + /// - `max_members` - The initial max number of members for the society. /// /// # /// - Two storage mutates to set `Head` and `Founder`. O(1) @@ -813,13 +814,14 @@ decl_module! { /// Total Complexity: O(1) /// # #[weight = SimpleDispatchInfo::FixedNormal(10_000)] - fn found(origin, founder: T::AccountId) { + fn found(origin, founder: T::AccountId, max_members: u32) { T::FounderSetOrigin::ensure_origin(origin)?; ensure!(!>::exists(), Error::::AlreadyFounded); // This should never fail in the context of this function... Self::add_member(&founder)?; >::put(&founder); >::put(&founder); + >::put(max_members); Self::deposit_event(RawEvent::Founded(founder)); } /// Allow suspension judgement origin to make judgement on a suspended member. diff --git a/frame/society/src/mock.rs b/frame/society/src/mock.rs index 3ce8938f95302..b8f249f35ce39 100644 --- a/frame/society/src/mock.rs +++ b/frame/society/src/mock.rs @@ -44,7 +44,6 @@ parameter_types! { pub const PeriodSpend: u64 = 1000; pub const MaxLockDuration: u64 = 100; pub const ChallengePeriod: u64 = 8; - pub const MaxMembers: u32 = 100; pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; diff --git a/frame/society/src/tests.rs b/frame/society/src/tests.rs index 886363590d051..915d316833bf1 100644 --- a/frame/society/src/tests.rs +++ b/frame/society/src/tests.rs @@ -29,17 +29,19 @@ fn founding_works() { assert_eq!(Society::founder(), None); // Account 1 is set as the founder origin // Account 5 cannot start a society - assert_noop!(Society::found(Origin::signed(5), 20), BadOrigin); + assert_noop!(Society::found(Origin::signed(5), 20, 100), BadOrigin); // Account 1 can start a society, where 10 is the founding member - assert_ok!(Society::found(Origin::signed(1), 10)); + assert_ok!(Society::found(Origin::signed(1), 10, 100)); // Society members only include 10 assert_eq!(Society::members(), vec![10]); // 10 is the head of the society assert_eq!(Society::head(), Some(10)); // ...and also the founder assert_eq!(Society::founder(), Some(10)); + // 100 members max + assert_eq!(Society::max_members(), 100); // Cannot start another society - assert_noop!(Society::found(Origin::signed(1), 20), Error::::AlreadyFounded); + assert_noop!(Society::found(Origin::signed(1), 20, 100), Error::::AlreadyFounded); }); } From c69d56f856c4f75874fd52665acf47381a293006 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 16 Jan 2020 21:00:15 +0100 Subject: [PATCH 2/4] Update test --- frame/society/src/lib.rs | 3 ++- frame/society/src/tests.rs | 9 +++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/frame/society/src/lib.rs b/frame/society/src/lib.rs index 17d24fea101d5..99b7f8fead285 100644 --- a/frame/society/src/lib.rs +++ b/frame/society/src/lib.rs @@ -817,11 +817,12 @@ decl_module! { fn found(origin, founder: T::AccountId, max_members: u32) { T::FounderSetOrigin::ensure_origin(origin)?; ensure!(!>::exists(), Error::::AlreadyFounded); + ensure!(max_members > 1, Error::::MaxMembers); // This should never fail in the context of this function... + >::put(max_members); Self::add_member(&founder)?; >::put(&founder); >::put(&founder); - >::put(max_members); Self::deposit_event(RawEvent::Founded(founder)); } /// Allow suspension judgement origin to make judgement on a suspended member. diff --git a/frame/society/src/tests.rs b/frame/society/src/tests.rs index 915d316833bf1..bf04f422548df 100644 --- a/frame/society/src/tests.rs +++ b/frame/society/src/tests.rs @@ -24,9 +24,11 @@ use sp_runtime::traits::BadOrigin; #[test] fn founding_works() { - EnvBuilder::new().with_members(vec![]).execute(|| { - // No founder initially. + EnvBuilder::new().with_max_members(0).with_members(vec![]).execute(|| { + // Not set up initially. assert_eq!(Society::founder(), None); + assert_eq!(Society::max_members(), 0); + assert_eq!(Society::pot(), 0); // Account 1 is set as the founder origin // Account 5 cannot start a society assert_noop!(Society::found(Origin::signed(5), 20, 100), BadOrigin); @@ -40,6 +42,9 @@ fn founding_works() { assert_eq!(Society::founder(), Some(10)); // 100 members max assert_eq!(Society::max_members(), 100); + // Pot grows after first rotation period + run_to_block(4); + assert_eq!(Society::pot(), 1000); // Cannot start another society assert_noop!(Society::found(Origin::signed(1), 20, 100), Error::::AlreadyFounded); }); From f374b3aca31e30d614747fde1cd5aeb838a0f7a4 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Fri, 17 Jan 2020 11:02:37 +0100 Subject: [PATCH 3/4] Use `Option` rather than `Option<()>` --- frame/society/src/lib.rs | 4 ++-- frame/society/src/tests.rs | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/frame/society/src/lib.rs b/frame/society/src/lib.rs index 99b7f8fead285..64fbf991eed31 100644 --- a/frame/society/src/lib.rs +++ b/frame/society/src/lib.rs @@ -426,7 +426,7 @@ decl_storage! { }): Vec; /// The set of suspended members. - pub SuspendedMembers get(fn suspended_member): map T::AccountId => Option<()>; + pub SuspendedMembers get(fn suspended_member): map T::AccountId => Option; /// The current bids, stored ordered by the value of the bid. Bids: Vec>>; @@ -1426,7 +1426,7 @@ impl, I: Instance> Module { /// Suspend a user, removing them from the member list. fn suspend_member(who: &T::AccountId) { if Self::remove_member(&who).is_ok() { - >::insert(who, ()); + >::insert(who, true); >::remove(who); Self::deposit_event(RawEvent::MemberSuspended(who.clone())); } diff --git a/frame/society/src/tests.rs b/frame/society/src/tests.rs index bf04f422548df..d735d7c0aaa30 100644 --- a/frame/society/src/tests.rs +++ b/frame/society/src/tests.rs @@ -269,7 +269,7 @@ fn suspended_member_lifecycle_works() { run_to_block(16); // Strike 2 is accumulated, and 20 is suspended :( - assert_eq!(>::get(20), Some(())); + assert_eq!(>::get(20), Some(true)); assert_eq!(>::get(), vec![10]); // Suspended members cannot get payout @@ -287,7 +287,7 @@ fn suspended_member_lifecycle_works() { // Let's suspend them again, directly Society::suspend_member(&20); - assert_eq!(>::get(20), Some(())); + assert_eq!(>::get(20), Some(true)); // Suspension judgement origin does not forgive the suspended member assert_ok!(Society::judge_suspended_member(Origin::signed(2), 20, false)); // Cleaned up @@ -526,7 +526,7 @@ fn founder_and_head_cannot_be_removed() { assert_ok!(Society::vote(Origin::signed(50), 90, true)); run_to_block(64); assert_eq!(Strikes::::get(50), 0); - assert_eq!(>::get(50), Some(())); + assert_eq!(>::get(50), Some(true)); assert_eq!(Society::members(), vec![10, 80]); }); } @@ -571,7 +571,7 @@ fn challenges_work() { run_to_block(32); // 20 is suspended assert_eq!(Society::members(), vec![10, 30, 40]); - assert_eq!(Society::suspended_member(20), Some(())); + assert_eq!(Society::suspended_member(20), Some(true)); // New defender is chosen assert_eq!(Society::defender(), Some(40)); }); @@ -643,7 +643,7 @@ fn vouching_handles_removed_member_with_bid() { assert_eq!(>::get(20), Some(VouchingStatus::Vouching)); // Suspend that member Society::suspend_member(&20); - assert_eq!(>::get(20), Some(())); + assert_eq!(>::get(20), Some(true)); // Nothing changes yet assert_eq!(>::get(), vec![create_bid(1000, 30, BidKind::Vouch(20, 100))]); assert_eq!(>::get(20), Some(VouchingStatus::Vouching)); @@ -670,7 +670,7 @@ fn vouching_handles_removed_member_with_candidate() { assert_eq!(Society::candidates(), vec![create_bid(1000, 30, BidKind::Vouch(20, 100))]); // Suspend that member Society::suspend_member(&20); - assert_eq!(>::get(20), Some(())); + assert_eq!(>::get(20), Some(true)); // Nothing changes yet assert_eq!(Society::candidates(), vec![create_bid(1000, 30, BidKind::Vouch(20, 100))]); assert_eq!(>::get(20), Some(VouchingStatus::Vouching)); From 707feaadfd659b9981d7d82bc28a4ba60da099cc Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Fri, 17 Jan 2020 12:25:38 +0100 Subject: [PATCH 4/4] Update from feedback --- bin/node/cli/src/chain_spec.rs | 2 +- bin/node/testing/src/genesis.rs | 2 +- frame/society/src/lib.rs | 2 +- frame/society/src/tests.rs | 18 +++++++++--------- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/bin/node/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs index 3214cffa16979..b8ff948b01b24 100644 --- a/bin/node/cli/src/chain_spec.rs +++ b/bin/node/cli/src/chain_spec.rs @@ -297,7 +297,7 @@ pub fn testnet_genesis( pallet_treasury: Some(Default::default()), pallet_society: Some(SocietyConfig { members: endowed_accounts[0..3].to_vec(), - pot: 100000, + pot: 0, max_members: 999, }) } diff --git a/bin/node/testing/src/genesis.rs b/bin/node/testing/src/genesis.rs index abedf74a5d6b2..183515f27274b 100644 --- a/bin/node/testing/src/genesis.rs +++ b/bin/node/testing/src/genesis.rs @@ -98,7 +98,7 @@ pub fn config(support_changes_trie: bool, code: Option<&[u8]>) -> GenesisConfig pallet_treasury: Some(Default::default()), pallet_society: Some(SocietyConfig { members: vec![alice(), bob()], - pot: 100000, + pot: 0, max_members: 999, }), } diff --git a/frame/society/src/lib.rs b/frame/society/src/lib.rs index 64fbf991eed31..3bbd4705f5793 100644 --- a/frame/society/src/lib.rs +++ b/frame/society/src/lib.rs @@ -426,7 +426,7 @@ decl_storage! { }): Vec; /// The set of suspended members. - pub SuspendedMembers get(fn suspended_member): map T::AccountId => Option; + pub SuspendedMembers get(fn suspended_member): map T::AccountId => bool; /// The current bids, stored ordered by the value of the bid. Bids: Vec>>; diff --git a/frame/society/src/tests.rs b/frame/society/src/tests.rs index d735d7c0aaa30..cf899fb4e250a 100644 --- a/frame/society/src/tests.rs +++ b/frame/society/src/tests.rs @@ -259,7 +259,7 @@ fn suspended_member_lifecycle_works() { assert_ok!(Society::add_member(&20)); assert_eq!(>::get(), vec![10, 20]); assert_eq!(Strikes::::get(20), 0); - assert_eq!(>::get(20), None); + assert_eq!(>::get(20), false); // Let's suspend account 20 by giving them 2 strikes by not voting assert_ok!(Society::bid(Origin::signed(30), 0)); @@ -269,7 +269,7 @@ fn suspended_member_lifecycle_works() { run_to_block(16); // Strike 2 is accumulated, and 20 is suspended :( - assert_eq!(>::get(20), Some(true)); + assert_eq!(>::get(20), true); assert_eq!(>::get(), vec![10]); // Suspended members cannot get payout @@ -282,16 +282,16 @@ fn suspended_member_lifecycle_works() { // Suspension judgment origin can judge thee // Suspension judgement origin forgives the suspended member assert_ok!(Society::judge_suspended_member(Origin::signed(2), 20, true)); - assert_eq!(>::get(20), None); + assert_eq!(>::get(20), false); assert_eq!(>::get(), vec![10, 20]); // Let's suspend them again, directly Society::suspend_member(&20); - assert_eq!(>::get(20), Some(true)); + assert_eq!(>::get(20), true); // Suspension judgement origin does not forgive the suspended member assert_ok!(Society::judge_suspended_member(Origin::signed(2), 20, false)); // Cleaned up - assert_eq!(>::get(20), None); + assert_eq!(>::get(20), false); assert_eq!(>::get(), vec![10]); assert_eq!(>::get(20), vec![]); }); @@ -526,7 +526,7 @@ fn founder_and_head_cannot_be_removed() { assert_ok!(Society::vote(Origin::signed(50), 90, true)); run_to_block(64); assert_eq!(Strikes::::get(50), 0); - assert_eq!(>::get(50), Some(true)); + assert_eq!(>::get(50), true); assert_eq!(Society::members(), vec![10, 80]); }); } @@ -571,7 +571,7 @@ fn challenges_work() { run_to_block(32); // 20 is suspended assert_eq!(Society::members(), vec![10, 30, 40]); - assert_eq!(Society::suspended_member(20), Some(true)); + assert_eq!(Society::suspended_member(20), true); // New defender is chosen assert_eq!(Society::defender(), Some(40)); }); @@ -643,7 +643,7 @@ fn vouching_handles_removed_member_with_bid() { assert_eq!(>::get(20), Some(VouchingStatus::Vouching)); // Suspend that member Society::suspend_member(&20); - assert_eq!(>::get(20), Some(true)); + assert_eq!(>::get(20), true); // Nothing changes yet assert_eq!(>::get(), vec![create_bid(1000, 30, BidKind::Vouch(20, 100))]); assert_eq!(>::get(20), Some(VouchingStatus::Vouching)); @@ -670,7 +670,7 @@ fn vouching_handles_removed_member_with_candidate() { assert_eq!(Society::candidates(), vec![create_bid(1000, 30, BidKind::Vouch(20, 100))]); // Suspend that member Society::suspend_member(&20); - assert_eq!(>::get(20), Some(true)); + assert_eq!(>::get(20), true); // Nothing changes yet assert_eq!(Society::candidates(), vec![create_bid(1000, 30, BidKind::Vouch(20, 100))]); assert_eq!(>::get(20), Some(VouchingStatus::Vouching));