Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Patch practical usability issues with Society #4651

Merged
merged 4 commits into from
Jan 17, 2020
Merged

Patch practical usability issues with Society #4651

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6ae2e86
Add `max_members` to `found`, add society genesis for Substrate node
shawntabrizi Jan 16, 2020
c69d56f
Update test
shawntabrizi Jan 16, 2020
f374b3a
Use `Option<bool>` rather than `Option<()>`
shawntabrizi Jan 17, 2020
707feaa
Update from feedback
shawntabrizi Jan 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions bin/node/cli/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this dependency?


# node-specific dependencies
node-runtime = { version = "2.0.0", path = "../runtime" }
Expand Down
9 changes: 7 additions & 2 deletions bin/node/cli/src/chain_spec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double check this number

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pot should probably just be zero.

max_members: 999,
})
}
}

Expand Down
3 changes: 1 addition & 2 deletions bin/node/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<T>},
Society: pallet_society::{Module, Call, Storage, Event<T>},
Society: pallet_society::{Module, Call, Storage, Event<T>, Config<T>},
Recovery: pallet_recovery::{Module, Call, Storage, Event<T>},
}
);
Expand Down
1 change: 1 addition & 0 deletions bin/node/testing/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
7 changes: 6 additions & 1 deletion bin/node/testing/src/genesis.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
}),
}
}
5 changes: 4 additions & 1 deletion frame/society/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
/// # <weight>
/// - Two storage mutates to set `Head` and `Founder`. O(1)
Expand All @@ -813,10 +814,12 @@ decl_module! {
/// Total Complexity: O(1)
/// # </weight>
#[weight = SimpleDispatchInfo::FixedNormal(10_000)]
fn found(origin, founder: T::AccountId) {
fn found(origin, founder: T::AccountId, max_members: u32) {
gavofyork marked this conversation as resolved.
Show resolved Hide resolved
T::FounderSetOrigin::ensure_origin(origin)?;
ensure!(!<Head<T, I>>::exists(), Error::<T, I>::AlreadyFounded);
ensure!(max_members > 1, Error::<T, I>::MaxMembers);
// This should never fail in the context of this function...
<MaxMembers<I>>::put(max_members);
Self::add_member(&founder)?;
<Head<T, I>>::put(&founder);
<Founder<T, I>>::put(&founder);
Expand Down
1 change: 0 additions & 1 deletion frame/society/src/mock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 12 additions & 5 deletions frame/society/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,29 @@ 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), 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);
// 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), Error::<Test, _>::AlreadyFounded);
assert_noop!(Society::found(Origin::signed(1), 20, 100), Error::<Test, _>::AlreadyFounded);
});
}

Expand Down