Skip to content

Add set_election_parameters() replacing individual calls #250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
shamil-gadelshin merged 4 commits into from
Mar 27, 2020
Merged

Add set_election_parameters() replacing individual calls #250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0ad43e5
council election: set_election_parameters() replacing individual call…
mnaamani Mar 26, 2020
34b149a
bump runtime spec to 10, bump node to v2.1.4
mnaamani Mar 26, 2020
f97d3c8
fix compiler warning
mnaamani Mar 26, 2020
6623d58
Merge branch 'development' into council-set-election-parameters
mnaamani Mar 26, 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
4 changes: 2 additions & 2 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.

2 changes: 1 addition & 1 deletion node/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ authors = ['Joystream']
build = 'build.rs'
edition = '2018'
name = 'joystream-node'
version = '2.1.3'
version = '2.1.4'
default-run = "joystream-node"

[[bin]]
Expand Down
130 changes: 82 additions & 48 deletions runtime-modules/governance/src/election.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use super::sealed_vote::SealedVote;
use super::stake::Stake;

use super::council;
use crate::election_params::ElectionParameters;
pub use common::currency::{BalanceOf, GovernanceCurrency};

pub trait Trait:
Expand All @@ -24,6 +25,8 @@ pub trait Trait:
type CouncilElected: CouncilElected<Seats<Self::AccountId, BalanceOf<Self>>, Self::BlockNumber>;
}

pub static MSG_CANNOT_CHANGE_PARAMS_DURING_ELECTION: &str = "CannotChangeParamsDuringElection";

#[derive(Clone, Copy, Encode, Decode)]
pub enum ElectionStage<BlockNumber> {
Announcing(BlockNumber),
Expand Down Expand Up @@ -106,8 +109,23 @@ decl_storage! {
// TODO value type of this map looks scary, is there any way to simplify the notation?
Votes get(votes): map T::Hash => SealedVote<T::AccountId, ElectionStake<T>, T::Hash, T::AccountId>;

// Current Election Parameters - default "zero" values are not meaningful. Running an election without
// settings reasonable values is a bad idea. Parameters can be set in the TriggerElection hook.
// Current Election Parameters.
// 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) 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);
Expand Down Expand Up @@ -195,7 +213,7 @@ impl<T: Trait> Module<T> {

// Take snapshot of seat and backing stakes of an existing council
// Its important to note that the election system takes ownership of these stakes, and is responsible
// to return any unused stake to original owners and the end of the election.
// to return any unused stake to original owners at the end of the election.
Self::initialize_transferable_stakes(current_council);

Self::deposit_event(RawEvent::ElectionStarted());
Expand Down Expand Up @@ -803,52 +821,19 @@ decl_module! {
<Stage<T>>::put(ElectionStage::Voting(ends_at));
}

fn set_param_announcing_period(origin, period: T::BlockNumber) {
ensure_root(origin)?;
ensure!(!Self::is_election_running(), "cannot change params during election");
ensure!(!period.is_zero(), "period cannot be zero");
<AnnouncingPeriod<T>>::put(period);
}
fn set_param_voting_period(origin, period: T::BlockNumber) {
ensure_root(origin)?;
ensure!(!Self::is_election_running(), "cannot change params during election");
ensure!(!period.is_zero(), "period cannot be zero");
<VotingPeriod<T>>::put(period);
}
fn set_param_revealing_period(origin, period: T::BlockNumber) {
ensure_root(origin)?;
ensure!(!Self::is_election_running(), "cannot change params during election");
ensure!(!period.is_zero(), "period cannot be zero");
<RevealingPeriod<T>>::put(period);
}
fn set_param_min_council_stake(origin, amount: BalanceOf<T>) {
ensure_root(origin)?;
ensure!(!Self::is_election_running(), "cannot change params during election");
<MinCouncilStake<T>>::put(amount);
}
fn set_param_new_term_duration(origin, duration: T::BlockNumber) {
fn set_election_parameters(origin, params: ElectionParameters<BalanceOf<T>, T::BlockNumber>) {
ensure_root(origin)?;
ensure!(!Self::is_election_running(), "cannot change params during election");
ensure!(!duration.is_zero(), "new term duration cannot be zero");
<NewTermDuration<T>>::put(duration);
}
fn set_param_council_size(origin, council_size: u32) {
ensure_root(origin)?;
ensure!(!Self::is_election_running(), "cannot change params during election");
ensure!(council_size > 0, "council size cannot be zero");
ensure!(council_size <= Self::candidacy_limit(), "council size cannot greater than candidacy limit");
CouncilSize::put(council_size);
}
fn set_param_candidacy_limit(origin, limit: u32) {
ensure_root(origin)?;
ensure!(!Self::is_election_running(), "cannot change params during election");
ensure!(limit >= Self::council_size(), "candidacy limit cannot be less than council size");
CandidacyLimit::put(limit);
}
fn set_param_min_voting_stake(origin, amount: BalanceOf<T>) {
ensure_root(origin)?;
ensure!(!Self::is_election_running(), "cannot change params during election");
<MinVotingStake<T>>::put(amount);
ensure!(!Self::is_election_running(), MSG_CANNOT_CHANGE_PARAMS_DURING_ELECTION);
params.ensure_valid()?;

<AnnouncingPeriod<T>>::put(params.announcing_period);
<VotingPeriod<T>>::put(params.voting_period);
<RevealingPeriod<T>>::put(params.revealing_period);
<MinCouncilStake<T>>::put(params.min_council_stake);
<NewTermDuration<T>>::put(params.new_term_duration);
CouncilSize::put(params.council_size);
CandidacyLimit::put(params.candidacy_limit);
<MinVotingStake<T>>::put(params.min_voting_stake);
}

fn force_stop_election(origin) {
Expand Down Expand Up @@ -2042,4 +2027,53 @@ mod tests {
assert_ok!(Election::start_election(vec![]));
});
}

#[test]
fn setting_election_parameters() {
initial_test_ext().execute_with(|| {
let default_parameters: ElectionParameters<u64, u64> = ElectionParameters::default();
// default all zeros is invalid
assert!(default_parameters.ensure_valid().is_err());

let new_parameters = ElectionParameters {
announcing_period: 1,
voting_period: 2,
revealing_period: 3,
council_size: 4,
candidacy_limit: 5,
min_voting_stake: 6,
min_council_stake: 7,
new_term_duration: 8,
};

assert_ok!(Election::set_election_parameters(
Origin::ROOT,
new_parameters
));

assert_eq!(
<AnnouncingPeriod<Test>>::get(),
new_parameters.announcing_period
);
assert_eq!(<VotingPeriod<Test>>::get(), new_parameters.voting_period);
assert_eq!(
<RevealingPeriod<Test>>::get(),
new_parameters.revealing_period
);
assert_eq!(
<MinCouncilStake<Test>>::get(),
new_parameters.min_council_stake
);
assert_eq!(
<NewTermDuration<Test>>::get(),
new_parameters.new_term_duration
);
assert_eq!(CouncilSize::get(), new_parameters.council_size);
assert_eq!(CandidacyLimit::get(), new_parameters.candidacy_limit);
assert_eq!(
<MinVotingStake<Test>>::get(),
new_parameters.min_voting_stake
);
});
}
}
45 changes: 45 additions & 0 deletions runtime-modules/governance/src/election_params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
use codec::{Decode, Encode};
use sr_primitives::traits::Zero;
use srml_support::{dispatch::Result, ensure};

pub static MSG_PERIOD_CANNOT_BE_ZERO: &str = "PeriodCannotBeZero";
pub static MSG_COUNCIL_SIZE_CANNOT_BE_ZERO: &str = "CouncilSizeCannotBeZero";
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)]
pub struct ElectionParameters<Balance, BlockNumber> {
pub announcing_period: BlockNumber,
pub voting_period: BlockNumber,
pub revealing_period: BlockNumber,
pub council_size: u32,
pub candidacy_limit: u32,
pub new_term_duration: BlockNumber,
pub min_council_stake: Balance,
pub min_voting_stake: Balance,
}

impl<Balance, BlockNumber: PartialOrd + Zero> ElectionParameters<Balance, BlockNumber> {
pub fn ensure_valid(&self) -> Result {
self.ensure_periods_are_valid()?;
self.ensure_council_size_and_candidacy_limit_are_valid()?;
Ok(())
}

fn ensure_periods_are_valid(&self) -> Result {
ensure!(!self.announcing_period.is_zero(), MSG_PERIOD_CANNOT_BE_ZERO);
ensure!(!self.voting_period.is_zero(), MSG_PERIOD_CANNOT_BE_ZERO);
ensure!(!self.revealing_period.is_zero(), MSG_PERIOD_CANNOT_BE_ZERO);
Ok(())
}

fn ensure_council_size_and_candidacy_limit_are_valid(&self) -> Result {
ensure!(self.council_size > 0, MSG_COUNCIL_SIZE_CANNOT_BE_ZERO);
ensure!(
self.council_size <= self.candidacy_limit,
MSG_CANDIDACY_LIMIT_WAS_LOWER_THAN_COUNCIL_SIZE
);
Ok(())
}
}
1 change: 1 addition & 0 deletions runtime-modules/governance/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

pub mod council;
pub mod election;
pub mod election_params;
pub mod proposals;

mod sealed_vote;
Expand Down
2 changes: 1 addition & 1 deletion runtime/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ edition = '2018'
name = 'joystream-node-runtime'
# Follow convention: https://github.com/Joystream/substrate-runtime-joystream/issues/1
# {Authoring}.{Spec}.{Impl} of the RuntimeVersion
version = '6.9.0'
version = '6.10.0'

[features]
default = ['std']
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("joystream-node"),
impl_name: create_runtime_str!("joystream-node"),
authoring_version: 6,
spec_version: 9,
spec_version: 10,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
};
Expand Down