From 5150cfb00bbb7842fbffd791e84cee7b4d5b73ca Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Fri, 27 Mar 2020 21:11:16 +0400 Subject: [PATCH 1/3] validate ElectionParameters at genesis --- node/src/chain_spec.rs | 24 +++++----- runtime-modules/governance/src/election.rs | 44 ++++++++++++------- .../governance/src/election_params.rs | 5 ++- runtime/src/lib.rs | 1 + 4 files changed, 45 insertions(+), 29 deletions(-) diff --git a/node/src/chain_spec.rs b/node/src/chain_spec.rs index 5bc2015558..99271c8c83 100644 --- a/node/src/chain_spec.rs +++ b/node/src/chain_spec.rs @@ -18,9 +18,9 @@ use node_runtime::{ versioned_store::InputValidationLengthConstraint as VsInputValidation, ActorsConfig, AuthorityDiscoveryConfig, BabeConfig, Balance, BalancesConfig, ContentWorkingGroupConfig, CouncilConfig, CouncilElectionConfig, DataObjectStorageRegistryConfig, - DataObjectTypeRegistryConfig, GrandpaConfig, ImOnlineConfig, IndicesConfig, MembersConfig, - Perbill, ProposalsConfig, SessionConfig, SessionKeys, Signature, StakerStatus, StakingConfig, - SudoConfig, SystemConfig, VersionedStoreConfig, DAYS, WASM_BINARY, + DataObjectTypeRegistryConfig, ElectionParameters, GrandpaConfig, ImOnlineConfig, IndicesConfig, + MembersConfig, Perbill, ProposalsConfig, SessionConfig, SessionKeys, Signature, StakerStatus, + StakingConfig, SudoConfig, SystemConfig, VersionedStoreConfig, DAYS, WASM_BINARY, }; pub use node_runtime::{AccountId, GenesisConfig}; use primitives::{sr25519, Pair, Public}; @@ -235,14 +235,16 @@ pub fn testnet_genesis( }), election: Some(CouncilElectionConfig { auto_start: true, - announcing_period: 3 * DAYS, - voting_period: 1 * DAYS, - revealing_period: 1 * DAYS, - council_size: 12, - candidacy_limit: 25, - min_council_stake: 10 * DOLLARS, - new_term_duration: 14 * DAYS, - min_voting_stake: 1 * DOLLARS, + election_parameters: ElectionParameters { + announcing_period: 3 * DAYS, + voting_period: 1 * DAYS, + revealing_period: 1 * DAYS, + council_size: 12, + candidacy_limit: 25, + min_council_stake: 10 * DOLLARS, + new_term_duration: 14 * DAYS, + min_voting_stake: 1 * DOLLARS, + }, }), proposals: Some(ProposalsConfig { approval_quorum: 66, diff --git a/runtime-modules/governance/src/election.rs b/runtime-modules/governance/src/election.rs index bcb4bf4185..b969c15785 100644 --- a/runtime-modules/governance/src/election.rs +++ b/runtime-modules/governance/src/election.rs @@ -126,14 +126,21 @@ decl_storage! { // If both candidacy limit and council size are zero then all applicant become council members // since no filtering occurs at end of announcing stage. // We only guard against these edge cases in the set_election_parameters() call. - AnnouncingPeriod get(announcing_period) config(): T::BlockNumber = T::BlockNumber::from(100); - VotingPeriod get(voting_period) config(): T::BlockNumber = T::BlockNumber::from(100); - RevealingPeriod get(revealing_period) config(): T::BlockNumber = T::BlockNumber::from(100); - CouncilSize get(council_size) config(): u32 = 10; - CandidacyLimit get (candidacy_limit) config(): u32 = 20; - MinCouncilStake get(min_council_stake) config(): BalanceOf = BalanceOf::::from(100); - NewTermDuration get(new_term_duration) config(): T::BlockNumber = T::BlockNumber::from(1000); - MinVotingStake get(min_voting_stake) config(): BalanceOf = BalanceOf::::from(10); + AnnouncingPeriod get(announcing_period): T::BlockNumber; + VotingPeriod get(voting_period): T::BlockNumber; + RevealingPeriod get(revealing_period): T::BlockNumber; + CouncilSize get(council_size): u32; + CandidacyLimit get (candidacy_limit): u32; + MinCouncilStake get(min_council_stake): BalanceOf; + NewTermDuration get(new_term_duration): T::BlockNumber; + MinVotingStake get(min_voting_stake): BalanceOf; + } + add_extra_genesis { + config(election_parameters): ElectionParameters, T::BlockNumber>; + build(|config: &GenesisConfig| { + config.election_parameters.ensure_valid().expect("Invalid Election Parameters"); + Module::::set_verified_election_parameters(config.election_parameters); + }); } } @@ -731,6 +738,17 @@ impl Module { Ok(()) } + + fn set_verified_election_parameters(params: ElectionParameters, T::BlockNumber>) { + >::put(params.announcing_period); + >::put(params.voting_period); + >::put(params.revealing_period); + >::put(params.min_council_stake); + >::put(params.new_term_duration); + CouncilSize::put(params.council_size); + CandidacyLimit::put(params.candidacy_limit); + >::put(params.min_voting_stake); + } } decl_module! { @@ -825,15 +843,7 @@ decl_module! { ensure_root(origin)?; ensure!(!Self::is_election_running(), MSG_CANNOT_CHANGE_PARAMS_DURING_ELECTION); params.ensure_valid()?; - - >::put(params.announcing_period); - >::put(params.voting_period); - >::put(params.revealing_period); - >::put(params.min_council_stake); - >::put(params.new_term_duration); - CouncilSize::put(params.council_size); - CandidacyLimit::put(params.candidacy_limit); - >::put(params.min_voting_stake); + Self::set_verified_election_parameters(params); } fn force_stop_election(origin) { diff --git a/runtime-modules/governance/src/election_params.rs b/runtime-modules/governance/src/election_params.rs index 99d4404b49..f5050860bd 100644 --- a/runtime-modules/governance/src/election_params.rs +++ b/runtime-modules/governance/src/election_params.rs @@ -1,4 +1,6 @@ use codec::{Decode, Encode}; +#[cfg(feature = "std")] +use serde::{Deserialize, Serialize}; use sr_primitives::traits::Zero; use srml_support::{dispatch::Result, ensure}; @@ -8,7 +10,8 @@ pub static MSG_CANDIDACY_LIMIT_WAS_LOWER_THAN_COUNCIL_SIZE: &str = "CandidacyWasLessThanCouncilSize"; /// Combined Election parameters, as argument for set_election_parameters -#[derive(Clone, Copy, Encode, Decode, Default, PartialEq, Debug)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +#[derive(Clone, Copy, Encode, Decode, Default, PartialEq)] pub struct ElectionParameters { pub announcing_period: BlockNumber, pub voting_period: BlockNumber, diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index bf1581d52f..a2a1739202 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -396,6 +396,7 @@ impl finality_tracker::Trait for Runtime { } pub use forum; +pub use governance::election_params::ElectionParameters; use governance::{council, election, proposals}; use membership::members; use storage::{data_directory, data_object_storage_registry, data_object_type_registry}; From 61aad966748175ed621484e94cbb690f438acb6e Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Fri, 27 Mar 2020 21:20:40 +0400 Subject: [PATCH 2/3] council election parameters derive Debug --- runtime-modules/governance/src/election_params.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime-modules/governance/src/election_params.rs b/runtime-modules/governance/src/election_params.rs index f5050860bd..f34646e608 100644 --- a/runtime-modules/governance/src/election_params.rs +++ b/runtime-modules/governance/src/election_params.rs @@ -10,8 +10,8 @@ pub static MSG_CANDIDACY_LIMIT_WAS_LOWER_THAN_COUNCIL_SIZE: &str = "CandidacyWasLessThanCouncilSize"; /// Combined Election parameters, as argument for set_election_parameters -#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] -#[derive(Clone, Copy, Encode, Decode, Default, PartialEq)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +#[derive(Clone, Copy, Encode, Decode, Default, PartialEq, Debug)] pub struct ElectionParameters { pub announcing_period: BlockNumber, pub voting_period: BlockNumber, From 01df9ce39d9b87deebc7e5f0ad328741914f4aa2 Mon Sep 17 00:00:00 2001 From: Mokhtar Naamani Date: Sat, 28 Mar 2020 10:05:33 +0400 Subject: [PATCH 3/3] election parameters: doc --- runtime-modules/governance/src/election.rs | 43 +++++++++++++++------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/runtime-modules/governance/src/election.rs b/runtime-modules/governance/src/election.rs index b969c15785..d5a91b784a 100644 --- a/runtime-modules/governance/src/election.rs +++ b/runtime-modules/governance/src/election.rs @@ -1,3 +1,26 @@ +//! Council Elections Manager +//! +//! # Election Parameters: +//! We don't currently handle zero periods, zero council term, zero council size and candidacy +//! limit in any special way. The behaviour in such cases: +//! +//! - Setting any period to 0 will mean the election getting stuck in that stage, until force changing +//! the state. +//! +//! - Council Size of 0 - no limit to size of council, all applicants that move beyond +//! announcing stage would become council members, so effectively the candidacy limit will +//! be the size of the council, voting and revealing have no impact on final results. +//! +//! - If candidacy limit is zero and council size > 0, council_size number of applicants will reach the voting stage. +//! and become council members, voting will have no impact on final results. +//! +//! - If both candidacy limit and council size are zero then all applicant become council members +//! since no filtering occurs at end of announcing stage. +//! +//! We only guard against these edge cases in the [`set_election_parameters`] call. +//! +//! [`set_election_parameters`]: struct.Module.html#method.set_election_parameters + use rstd::prelude::*; use srml_support::traits::{Currency, ReservableCurrency}; use srml_support::{decl_event, decl_module, decl_storage, dispatch::Result, ensure}; @@ -113,19 +136,6 @@ decl_storage! { // Should we replace all the individual values with a single ElectionParameters type? // Having them individually makes it more flexible to add and remove new parameters in future // without dealing with migration issues. - - // We don't currently handle zero periods, zero council term, zero council size and candidacy - // limit in any special way. The behaviour in such cases: - // Setting any period to 0 will mean the election getting stuck in that stage, until force changing - // the state. - // Council Size of 0 - no limit to size of council, all applicants that move beyond - // announcing stage would become council members, so effectively the candidacy limit will - // be the size of the council, voting and revealing have no impact on final results. - // If candidacy limit is zero and council size > 0, council_size number of applicants will reach the voting stage. - // and become council members, voting will have no impact on final results. - // If both candidacy limit and council size are zero then all applicant become council members - // since no filtering occurs at end of announcing stage. - // We only guard against these edge cases in the set_election_parameters() call. AnnouncingPeriod get(announcing_period): T::BlockNumber; VotingPeriod get(voting_period): T::BlockNumber; RevealingPeriod get(revealing_period): T::BlockNumber; @@ -839,7 +849,12 @@ decl_module! { >::put(ElectionStage::Voting(ends_at)); } - fn set_election_parameters(origin, params: ElectionParameters, T::BlockNumber>) { + /// Sets new election parameters. Some combination of parameters that are not desirable, so + /// the parameters are checked for validity. + /// The call will fail if an election is in progress. If a council is not being elected for some + /// reaon after multiple rounds, force_stop_election() can be called to stop elections and followed by + /// set_election_parameters(). + pub fn set_election_parameters(origin, params: ElectionParameters, T::BlockNumber>) { ensure_root(origin)?; ensure!(!Self::is_election_running(), MSG_CANNOT_CHANGE_PARAMS_DURING_ELECTION); params.ensure_valid()?;