Patch practical usability issues with Society

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" }

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: 0,
+			max_members: 999,
+		})
 	}
 }
 

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<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>},
 	}
 );

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" }

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: 0,
+			max_members: 999,
+		}),
 	}
 }

frame/society/src/lib.rs

@@ -426,7 +426,7 @@ decl_storage! {
 		}): Vec<T::AccountId>;
 
 		/// 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<Bid<T::AccountId, BalanceOf<T, I>>>;
@@ -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)
@@ -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) {
 			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);
@@ -1423,7 +1426,7 @@ impl<T: Trait<I>, I: Instance> Module<T, I> {
 	/// Suspend a user, removing them from the member list.
 	fn suspend_member(who: &T::AccountId) {
 		if Self::remove_member(&who).is_ok() {
-			<SuspendedMembers<T, I>>::insert(who, ());
+			<SuspendedMembers<T, I>>::insert(who, true);
 			<Strikes<T, I>>::remove(who);
 			Self::deposit_event(RawEvent::MemberSuspended(who.clone()));
 		}

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;

frame/society/src/tests.rs

@@ -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);
 	});
 }
 
@@ -252,7 +259,7 @@ fn suspended_member_lifecycle_works() {
 		assert_ok!(Society::add_member(&20));
 		assert_eq!(<Members<Test>>::get(), vec![10, 20]);
 		assert_eq!(Strikes::<Test>::get(20), 0);
-		assert_eq!(<SuspendedMembers<Test>>::get(20), None);
+		assert_eq!(<SuspendedMembers<Test>>::get(20), false);
 
 		// Let's suspend account 20 by giving them 2 strikes by not voting
 		assert_ok!(Society::bid(Origin::signed(30), 0));
@@ -262,7 +269,7 @@ fn suspended_member_lifecycle_works() {
 		run_to_block(16);
 
 		// Strike 2 is accumulated, and 20 is suspended :(
-		assert_eq!(<SuspendedMembers<Test>>::get(20), Some(()));
+		assert_eq!(<SuspendedMembers<Test>>::get(20), true);
 		assert_eq!(<Members<Test>>::get(), vec![10]);
 
 		// Suspended members cannot get payout
@@ -275,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!(<SuspendedMembers<Test>>::get(20), None);
+		assert_eq!(<SuspendedMembers<Test>>::get(20), false);
 		assert_eq!(<Members<Test>>::get(), vec![10, 20]);
 
 		// Let's suspend them again, directly
 		Society::suspend_member(&20);
-		assert_eq!(<SuspendedMembers<Test>>::get(20), Some(()));
+		assert_eq!(<SuspendedMembers<Test>>::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!(<SuspendedMembers<Test>>::get(20), None);
+		assert_eq!(<SuspendedMembers<Test>>::get(20), false);
 		assert_eq!(<Members<Test>>::get(), vec![10]);
 		assert_eq!(<Payouts<Test>>::get(20), vec![]);
 	});
@@ -519,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::<Test>::get(50), 0);
-		assert_eq!(<SuspendedMembers<Test>>::get(50), Some(()));
+		assert_eq!(<SuspendedMembers<Test>>::get(50), true);
 		assert_eq!(Society::members(), vec![10, 80]);
 	});
 }
@@ -564,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), true);
 		// New defender is chosen
 		assert_eq!(Society::defender(), Some(40));
 	});
@@ -636,7 +643,7 @@ fn vouching_handles_removed_member_with_bid() {
 		assert_eq!(<Vouching<Test>>::get(20), Some(VouchingStatus::Vouching));
 		// Suspend that member
 		Society::suspend_member(&20);
-		assert_eq!(<SuspendedMembers<Test>>::get(20), Some(()));
+		assert_eq!(<SuspendedMembers<Test>>::get(20), true);
 		// Nothing changes yet
 		assert_eq!(<Bids<Test>>::get(), vec![create_bid(1000, 30, BidKind::Vouch(20, 100))]);
 		assert_eq!(<Vouching<Test>>::get(20), Some(VouchingStatus::Vouching));
@@ -663,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!(<SuspendedMembers<Test>>::get(20), Some(()));
+		assert_eq!(<SuspendedMembers<Test>>::get(20), true);
 		// Nothing changes yet
 		assert_eq!(Society::candidates(), vec![create_bid(1000, 30, BidKind::Vouch(20, 100))]);
 		assert_eq!(<Vouching<Test>>::get(20), Some(VouchingStatus::Vouching));