diff --git a/srml/council/src/lib.rs b/srml/council/src/lib.rs index a13eb7e28067a..321b06f814bf2 100644 --- a/srml/council/src/lib.rs +++ b/srml/council/src/lib.rs @@ -14,7 +14,217 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -//! Council system: Handles the voting in and maintenance of council members. +//! # Council Module +//! +//! The Council module provides tools to manage the council and proposals. The main components are: +//! +//! - **Council Seats:** Election of councillors. +//! - [`seats::Trait`](./seats/trait.Trait.html) +//! - [`Call`](./seats/enum.Call.html) +//! - [`Module`](./seats/struct.Module.html) +//! - **Council Motions:** Voting as a body to dispatch calls from the `Council` origin. +//! - [`motions::Trait`](./motions/trait.Trait.html) +//! - [`Call`](./motions/enum.Call.html) +//! - [`Module`](./motions/struct.Module.html) +//! - **Council Voting:** Proposals sent to the [Democracy module](../srml_democracy/index.html) for referenda. +//! - [`voting::Trait`](./voting/trait.Trait.html) +//! - [`Call`](./voting/enum.Call.html) +//! - [`Module`](./voting/struct.Module.html) +//! +//! ## Overview +//! +//! The Council module provides functionality to handle: +//! +//! - Voting in and maintenance of council members. +//! - Proposing, vetoing, and passing of motions. +//! +//! The council is an on-chain entity comprised of a set of account IDs, with the role of representing +//! passive stakeholders. Its primary tasks are to propose sensible referenda and thwart any uncontroversially +//! dangerous or malicious referenda. +//! +//! ### Terminology +//! +//! #### Council Motions (motions.rs) +//! +//! _Motions_ handle internal proposals that are only proposed and voted upon by _councillors_. +//! Each proposal has a minimum threshold of yay votes that it needs to gain to be enacted. +//! +//! - **Council motion:** A mechanism used to enact a proposal. +//! - **Proposal:** A submission by a councillor. An initial vote of yay from that councillor is applied. +//! - **Vote:** A vote of yay or nay from a councillor on a single proposal. Councillors may change their vote but a +//! duplicate vote will return an error. +//! +//! Upon each vote, if the threshold is reached, the proposal is dispatched from the `Council` origin. Similarly, +//! if the number of nay votes is high enough such that it could not pass even if all other councillors +//! (including those who have not voted) voted yay, the proposal is dropped. +//! +//! Note that a council motion has a special origin type, [`seats::Origin`](./motions/enum.Origin.html), that limits +//! which calls can be effectively dispatched. +//! +//! #### Council Voting (voting.rs) +//! +//! _Voting_ handles councillor proposing and voting. Unlike motions, if a proposal is approved, +//! it is elevated to the [Democracy module](../srml_democracy/index.html) as a referendum. +//! +//! - **Proposal validity:** A council proposal is valid when it's unique, hasn't yet been vetoed, and +//! when the proposing councillor's term doesn't expire before the block number when the proposal's voting period ends. +//! A proposal is a generic type that can be _dispatched_ (similar to variants of the `Call` enum in each module). +//! - **Proposal postponement:** Councillors may postpone a council proposal from being approved or rejected. +//! Postponement is equivalent to a veto, which only lasts for the cooloff period. +//! - **Cooloff period:** Period, in blocks, for which a veto is in effect. +//! - **Referendum:** The means of public voting on a proposal. +//! - **Veto:** A council member may veto any council proposal that exists. A vetoed proposal that's valid is set +//! aside for a cooloff period. The vetoer cannot re-veto or propose the proposal again until the veto expires. +//! - **Elevation:** A referendum can be elevated from the council to a referendum. This means it has +//! been passed to the Democracy module for a public vote. +//! - **Referendum cancellation:** At the end of a given block we cancel all elevated referenda whose voting period +//! ends at that block and where the outcome of the vote tally was a unanimous vote to cancel the referendum. +//! - **Voting process to elevate a proposal:** At the end of a given block we tally votes for expiring referenda. +//! Referenda that are passed (yay votes are greater than nay votes plus abstainers) are sent to the Democracy +//! module for a public referendum. If there are no nay votes (abstention is acceptable), then the proposal is +//! tabled immediately. Otherwise, there will be a delay period. If the vote is unanimous, then the public +//! referendum will require a vote threshold of supermajority against to prevent it. Otherwise, +//! it is a simple majority vote. See [`VoteThreshold`](../srml_democracy/enum.VoteThreshold.html) in the +//! Democracy module for more details on how votes are approved. +//! +//! As opposed to motions, proposals executed through the Democracy module have the +//! root origin, which gives them the highest privilege. +//! +//! #### Council Seats (seats.rs) +//! +//! _Seats_ handles the selection of councillors. The selection of council seats is a circulating procedure, +//! regularly performing approval voting to accept a new council member and remove those whose period has ended. +//! Each tally (round of voting) is divided into two time periods: +//! +//! - **Voting period:** In which any stakeholder can vote for any of the council candidates. +//! - **Presentation period:** In which voting is no longer allowed, and stakeholders can _present_ a candidate +//! and claim that a particular candidate has won a seat. +//! +//! A tally is scheduled to execute based on the number of desired and free seats in the council. +//! +//! Both operations have associated bonds and fees that need to be paid based on the complexity of the transaction. +//! See [`set_approvals`](./seats/enum.Call.html#variant.set_approvals) and +//! [`submit_candidacy`](./seats/enum.Call.html#variant.submit_candidacy) for more information. +//! +//! Upon the end of the presentation period, the leaderboard is finalized and sorted based on the approval +//! weight of the _presented_ candidates. +//! Based on the configurations of the module, `N` top candidates in the leaderboard are immediately recognized +//! as councillors (where `N` is `desired_seats - active_council.len()`) and runners-up are kept in +//! the leaderboard as carry for the next tally to compete again. Similarly, councillors whose term has ended +//! are also removed. +//! +//! - **Vote index**: A counter indicating the number of tally rounds already applied. +//! - **Desired seats:** The number of seats on the council. +//! - **Candidacy bond:** Bond required to be a candidate. The bond is returned to all candidates at the end +//! of the tally if they have won the tally (i.e. have become a councillor). +//! - **Leaderboard:** A list of candidates and their respective votes in an election, ordered low to high. +//! - **Presentation:** The act of proposing a candidate for insertion into the leaderboard. Presenting is +//! `O(|number_of_voters|)`, so the presenter must be slashable and will be slashed for duplicate or invalid +//! presentations. Presentation is only allowed during the "presentation period," after voting has closed. +//! - **Voting bond:** Bond required to be permitted to vote. Must be held because many voting operations affect +//! storage. The bond is held to discourage abuse. +//! - **Voting:** Process of inserting approval votes into storage. Can be called by anyone, given they submit +//! an appropriate list of approvals. A bond is reserved from a voter until they retract or get reported. +//! - **Inactive voter**: A voter whose approvals are now invalid. Such voters can be _reaped_ by other voters +//! after an `inactivity_grace_period` has passed from their last known activity. +//! - **Reaping process:** Voters may propose the removal of inactive voters, as explained above. If the claim is not +//! valid, the _reaper_ will be slashed and removed as a voter. Otherwise, the _reported_ voter is removed. A voter +//! always gets his or her bond back upon being removed (either through _reaping_ or _retracting_). +//! +//! ### Goals +//! +//! The Council module in Substrate is designed to make the following possible: +//! +//! - Create council proposals by councillors using the council motion mechanism. +//! - Validate council proposals. +//! - Tally votes of council proposals by councillors during the proposal's voting period. +//! - Veto (postpone) council proposals for a cooloff period through abstention by councillors. +//! - Elevate council proposals to start a referendum. +//! - Execute referenda once their vote tally reaches the vote threshold level of approval. +//! - Manage candidacy, including voting, term expiration, and punishment. +//! +//! ## Interface +//! +//! ### Dispatchable Functions +//! +//! The dispatchable functions in the Council module provide the functionality that councillors need. +//! See the `Call` enums from the Motions, Seats, and Voting modules for details on dispatchable functions. +//! +//! ### Public Functions +//! +//! The public functions provide the functionality for other modules to interact with the Council module. +//! See the `Module` structs from the Motions, Seats, and Voting modules for details on public functions. +//! +//! ## Usage +//! +//! ### Council Seats: Councillor Election Procedure +//! +//! A Council seat vote can proceed as follows: +//! +//! 1. Candidates submit themselves for candidacy. +//! 2. Voting occurs. +//! 3. Voting period ends and presentation period begins. +//! 4. Candidates are presented for the leaderboard. +//! 5. Presentation period ends, votes are tallied, and new council takes effect. +//! 6. Candidate list is cleared (except for a defined number of carries) and vote index increased. +//! +//! ### Council Votes: Proposal Elevation Procedure +//! +//! A council motion elevation would proceed as follows: +//! +//! 1. A councillor makes a proposal. +//! 2. Other councillors vote yay or nay or abstain. +//! 3. At the end of the voting period, the votes are tallied. +//! 4. If it has passed, then it will be sent to the Democracy module with the vote threshold parameter. +//! +//! ### Example +//! +//! This code snippet uses the `is_councillor` public function to check if the calling user +//! is an active councillor before proceeding with additional runtime logic. +//! +//! ``` +//! use srml_support::{decl_module, ensure, dispatch::Result}; +//! use system::ensure_signed; +//! use srml_council::motions; +//! +//! pub trait Trait: motions::Trait + system::Trait {} +//! +//! decl_module! { +//! pub struct Module for enum Call where origin: ::Origin { +//! +//! pub fn privileged_function(origin) -> Result { +//! // Get the sender AccountId +//! let sender = ensure_signed(origin)?; +//! +//! // Check they are an active councillor +//! ensure!(>::is_councillor(&sender), +//! "Must be a councillor to call this function"); +//! +//! // Do some privileged operation here... +//! +//! // Return `Ok` at the end +//! Ok(()) +//! } +//! } +//! } +//! # fn main() { } +//! ``` +//! +//! ## Genesis Config +//! +//! The Council module depends on the `GenesisConfig`. +//! +//! - [Seats](./seats/struct.GenesisConfig.html) +//! - [Voting](./voting/struct.GenesisConfig.html) +//! +//! ## Related Modules +//! +//! - [Democracy](../srml_democracy/index.html) +//! - [Staking](../srml_staking/index.html) +//! +//! ## References +//! +//! - [Polkadot wiki](https://wiki.polkadot.network/en/latest/polkadot/learn/governance/) on governance. #![cfg_attr(not(feature = "std"), no_std)] @@ -56,7 +266,7 @@ mod tests { } } - // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. + // Workaround for https://github.com/rust-lang/rust/issues/26925. Remove when sorted. #[derive(Clone, Eq, PartialEq, Debug)] pub struct Test; impl system::Trait for Test { diff --git a/srml/council/src/motions.rs b/srml/council/src/motions.rs index 3bbe463780c46..ec269c41e14c7 100644 --- a/srml/council/src/motions.rs +++ b/srml/council/src/motions.rs @@ -39,7 +39,7 @@ pub trait Trait: CouncilTrait { type Event: From> + Into<::Event>; } -/// Origin for the council module. +/// Origin for the Council module. #[derive(PartialEq, Eq, Clone)] #[cfg_attr(feature = "std", derive(Debug))] pub enum Origin { @@ -49,10 +49,10 @@ pub enum Origin { decl_event!( pub enum Event where ::Hash, ::AccountId { - /// A motion (given hash) has been proposed (by given account) with a threshold (given u32). + /// A motion (given hash) has been proposed (by given account) with a threshold (given `u32`). Proposed(AccountId, ProposalIndex, Hash, u32), /// A motion (given hash) has been voted on by given account, leaving - /// a tally (yes votes and no votes given as u32s respectively). + /// a tally (yes votes and no votes given as `u32`s respectively). Voted(AccountId, Hash, bool, u32, u32), /// A motion was approved by the required threshold. Approved(Hash), @@ -66,6 +66,13 @@ decl_event!( decl_module! { pub struct Module for enum Call where origin: ::Origin { fn deposit_event() = default; + + /// Make a proposal. `threshold` indicates the number of yes-votes needed for the proposal + /// to execute. + /// + /// The proposal must be unique. + /// + /// The dispatch origin of this call must be signed by a _councillor_. fn propose(origin, #[compact] threshold: u32, proposal: Box<::Proposal>) { let who = ensure_signed(origin)?; @@ -89,6 +96,16 @@ decl_module! { } } + /// Vote on a proposal. + /// + /// The proposal hash and the index of the vote must be correctly provided. + /// A voter can change their vote from yes to no, but duplicate votes will raise an error. + /// + /// Each submitted vote _may_ cause the proposal to be executed, if the required + /// threshold is reached. Similarly, enough no-votes can cause the proposal to be + /// removed from storage. + /// + /// The dispatch origin of this call must be signed by a _councillor_. fn vote(origin, proposal: T::Hash, #[compact] index: ProposalIndex, approve: bool) { let who = ensure_signed(origin)?; @@ -158,8 +175,8 @@ decl_storage! { /// The (hashes of) the active proposals. pub Proposals get(proposals): Vec; /// Actual proposal for a given hash, if it's current. - pub ProposalOf get(proposal_of): map T::Hash => Option< ::Proposal >; - /// Votes for a given proposal: (required_yes_votes, yes_voters, no_voters). + pub ProposalOf get(proposal_of): map T::Hash => Option<::Proposal>; + /// Votes for a given proposal: (Proposal index, required number of yes votes, yes voters, no voters). pub Voting get(voting): map T::Hash => Option<(ProposalIndex, u32, Vec, Vec)>; /// Proposals so far. pub ProposalCount get(proposal_count): u32; @@ -170,6 +187,7 @@ decl_storage! { } impl Module { + /// Returns true if `who` is an active councillor. pub fn is_councillor(who: &T::AccountId) -> bool { >::active_council().iter() .any(|&(ref a, _)| a == who) @@ -187,6 +205,7 @@ pub fn ensure_council_members(o: OuterOrigin, n: u32) -> result::Re } } +/// Phantom struct to ensure that an `origin` is a councillor. pub struct EnsureMembers(::rstd::marker::PhantomData); impl EnsureOrigin for EnsureMembers where O: Into> @@ -207,6 +226,10 @@ mod tests { use system::{EventRecord, Phase}; use hex_literal::{hex, hex_impl}; + fn set_balance_proposal(value: u64) -> Call { + Call::Balances(balances::Call::set_balance(42, value.into(), 0)) + } + #[test] fn motions_basic_environment_works() { with_externalities(&mut new_test_ext(true), || { @@ -216,10 +239,6 @@ mod tests { }); } - fn set_balance_proposal(value: u64) -> Call { - Call::Balances(balances::Call::set_balance(42, value.into(), 0)) - } - #[test] fn motions_propose_works() { with_externalities(&mut new_test_ext(true), || { @@ -234,7 +253,14 @@ mod tests { assert_eq!(System::events(), vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Proposed(1, 0, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), 3)) + event: OuterEvent::motions( + RawEvent::Proposed( + 1, + 0, + hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), + 3 + ) + ), } ]); }); @@ -245,7 +271,10 @@ mod tests { with_externalities(&mut new_test_ext(true), || { System::set_block_number(1); let proposal = set_balance_proposal(42); - assert_noop!(CouncilMotions::propose(Origin::signed(42), 3, Box::new(proposal.clone())), "proposer not on council"); + assert_noop!( + CouncilMotions::propose(Origin::signed(42), 3, Box::new(proposal.clone())), + "proposer not on council" + ); }); } @@ -287,11 +316,26 @@ mod tests { assert_eq!(System::events(), vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Proposed(1, 0, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), 2)) + event: OuterEvent::motions( + RawEvent::Proposed( + 1, + 0, + hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), + 2 + ) + ), }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Voted(1, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), false, 0, 1)) + event: OuterEvent::motions( + RawEvent::Voted( + 1, + hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), + false, + 0, + 1 + ) + ), } ]); }); @@ -309,15 +353,34 @@ mod tests { assert_eq!(System::events(), vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Proposed(1, 0, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), 3)) + event: OuterEvent::motions( + RawEvent::Proposed( + 1, + 0, + hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), + 3 + ) + ), }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Voted(2, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), false, 1, 1)) + event: OuterEvent::motions( + RawEvent::Voted( + 2, + hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), + false, + 1, + 1 + ) + ), }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Disapproved(hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into())) + event: OuterEvent::motions( + RawEvent::Disapproved( + hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into() + ) + ), } ]); }); @@ -327,6 +390,7 @@ mod tests { fn motions_approval_works() { with_externalities(&mut new_test_ext(true), || { System::set_block_number(1); + assert_eq!(Balances::free_balance(&42), 0); let proposal = set_balance_proposal(42); let hash: H256 = proposal.blake2_256().into(); assert_ok!(CouncilMotions::propose(Origin::signed(1), 2, Box::new(proposal.clone()))); @@ -335,19 +399,43 @@ mod tests { assert_eq!(System::events(), vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Proposed(1, 0, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), 2)) + event: OuterEvent::motions( + RawEvent::Proposed( + 1, + 0, + hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), + 2 + ) + ), }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Voted(2, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), true, 2, 0)) + event: OuterEvent::motions( + RawEvent::Voted( + 2, + hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), + true, + 2, + 0 + ) + ), }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Approved(hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into())) + event: OuterEvent::motions( + RawEvent::Approved( + hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into() + ) + ), }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Executed(hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), false)) + event: OuterEvent::motions( + RawEvent::Executed( + hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), + false + ) + ), } ]); }); diff --git a/srml/council/src/seats.rs b/srml/council/src/seats.rs index 02c3b222c82a7..3d96ea430fd99 100644 --- a/srml/council/src/seats.rs +++ b/srml/council/src/seats.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -//! Council system: Handles the voting in and maintenance of council members. +//! Council member seat candidacy, voting, approval, and maintenance. use rstd::prelude::*; use primitives::traits::{Zero, One, As, StaticLookup}; @@ -41,9 +41,11 @@ use system::{self, ensure_signed}; // public operations: // - express approvals (you pay in a "voter" bond the first time you do this; O(1); one extra DB entry, one DB change) // - remove active voter (you get your "voter" bond back; O(1); one fewer DB entry, one DB change) -// - remove inactive voter (either you or the target is removed; if the target, you get their "voter" bond back; O(1); one fewer DB entry, one DB change) +// - remove inactive voter (either you or the target is removed; if the target, you get their "voter" bond back; O(1); +// one fewer DB entry, one DB change) // - submit candidacy (you pay a "candidate" bond; O(1); one extra DB entry, two DB changes) -// - present winner/runner-up (you may pay a "presentation" bond of O(voters) if the presentation is invalid; O(voters) compute; ) +// - present winner/runner-up (you may pay a "presentation" bond of O(voters) if the presentation is invalid; +// O(voters) compute; ) // protected operations: // - remove candidacy (remove all votes for a candidate) (one fewer DB entry, two DB changes) @@ -62,7 +64,7 @@ use system::{self, ensure_signed}; // for B blocks following, there's a counting period whereby each of the candidates that believe // they fall in the top K+C voted can present themselves. they get the total stake -// recorded (based on the snapshot); an ordered list is maintained (the leaderboard). Noone may +// recorded (based on the snapshot); an ordered list is maintained (the leaderboard). No one may // present themselves that, if elected, would result in being included twice on the council // (important since existing councillors will have their approval votes as it may be that they // don't get removed), nor if existing presenters would mean they're not in the top K+C. @@ -82,6 +84,7 @@ use system::{self, ensure_signed}; use srml_support::decl_module; +/// Index for the total number of vote tallies that have happened or are in progress. pub type VoteIndex = u32; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; @@ -101,31 +104,31 @@ decl_storage! { trait Store for Module as Council { // parameters - /// How much should be locked up in order to submit one's candidacy. + /// Amount that must be locked up in order to submit one's candidacy. pub CandidacyBond get(candidacy_bond) config(): BalanceOf = BalanceOf::::sa(9); - /// How much should be locked up in order to be able to submit votes. + /// Amount that must be locked up in order to be able to submit votes. pub VotingBond get(voting_bond) config(voter_bond): BalanceOf; /// The punishment, per voter, if you provide an invalid presentation. pub PresentSlashPerVoter get(present_slash_per_voter) config(): BalanceOf = BalanceOf::::sa(1); - /// How many runners-up should have their approvals persist until the next vote. + /// Number of runners-up who should have their approvals persist until the next vote. pub CarryCount get(carry_count) config(): u32 = 2; - /// How long to give each top candidate to present themselves after the vote ends. + /// Number of blocks to give each top candidate to present themselves after the vote ends. pub PresentationDuration get(presentation_duration) config(): T::BlockNumber = T::BlockNumber::sa(1000); - /// How many vote indexes need to go by after a target voter's last vote before they can be reaped if their - /// approvals are moot. + /// Number of vote indices that need to go by after a target voter's last vote before they can + /// be reaped if their approvals are moot. pub InactiveGracePeriod get(inactivity_grace_period) config(inactive_grace_period): VoteIndex = 1; /// How often (in blocks) to check for new votes. pub VotingPeriod get(voting_period) config(approval_voting_period): T::BlockNumber = T::BlockNumber::sa(1000); - /// How long each position is active for. + /// How long (in blocks) each position is active for. pub TermDuration get(term_duration) config(): T::BlockNumber = T::BlockNumber::sa(5); /// Number of accounts that should be sitting on the council. pub DesiredSeats get(desired_seats) config(): u32; // permanent state (always relevant, changes only at the finalization of voting) /// The current council. When there's a vote going on, this should still be used for executive - /// matters. The block number (second element in the tuple) is the block that their position is - /// active until (calculated by the sum of the block number when the council member was elected - /// and their term duration). + /// matters. The block number is the block that the associated account ID's position is + /// active until (calculated by the sum of the block number when the council member was + /// elected and their term duration). pub ActiveCouncil get(active_council) config(): Vec<(T::AccountId, T::BlockNumber)>; /// The total number of votes that have happened or are in progress. pub VoteCount get(vote_index): VoteIndex; @@ -134,8 +137,8 @@ decl_storage! { /// A list of votes for each voter, respecting the last cleared vote index that this voter was /// last active at. pub ApprovalsOf get(approvals_of): map T::AccountId => Vec; - /// The vote index and list slot that the candidate `who` was registered or `None` if they are not - /// currently registered. + /// The vote index and list slot in which the candidate account ID was registered or `None` if + /// they are not currently registered. pub RegisterInfoOf get(candidate_reg_info): map T::AccountId => Option<(VoteIndex, u32)>; /// The last cleared vote index that this voter was last active at. pub LastActiveOf get(voter_last_active): map T::AccountId => Option; @@ -143,10 +146,12 @@ decl_storage! { pub Voters get(voters): Vec; /// The present candidate list. pub Candidates get(candidates): Vec; // has holes + /// Number of candidates. pub CandidateCount get(candidate_count): u32; // temporary state (only relevant during finalization/presentation) - /// The accounts holding the seats that will become free on the next tally. + /// The accounts holding the seats that will become free on the next tally. Tuple of the block number + /// at which the next tally will occur, the number of seats, and a list of the account IDs. pub NextFinalize get(next_finalize): Option<(T::BlockNumber, u32, Vec)>; /// The stakes as they were at the point that the vote ended. pub SnapshotedStakes get(snapshoted_stakes): Vec>; @@ -157,9 +162,9 @@ decl_storage! { decl_event!( pub enum Event where ::AccountId { - /// reaped voter, reaper + /// A voter has been reaped. The tuple corresponds to the reaped voter and reaper, respectively. VoterReaped(AccountId, AccountId), - /// slashed reaper + /// A reaper has been slashed. BadReaperSlashed(AccountId), /// A tally (for approval votes of council seat(s)) has started. TallyStarted(u32), @@ -219,7 +224,8 @@ decl_module! { .any(|(&appr, addr)| appr && *addr != T::AccountId::default() && - Self::candidate_reg_info(addr).map_or(false, |x| x.0 <= last_active)/*defensive only: all items in candidates list are registered*/ + /*defensive only: all items in candidates list are registered*/ + Self::candidate_reg_info(addr).map_or(false, |x| x.0 <= last_active) ); Self::remove_voter( @@ -284,9 +290,10 @@ decl_module! { >::put(count as u32 + 1); } - /// Claim that `signed` is one of the top Self::carry_count() + current_vote().1 candidates. - /// Only works if the `block_number >= current_vote().0` and `< current_vote().0 + presentation_duration()`` - /// `signed` should have at least + /// Present a candidate to be inserted into the leaderboard. + /// + /// The presenter (`origin`) must be slashable and will be slashed in the cases + /// of an incorrect total or a duplicate presentation. fn present_winner( origin, candidate: ::Source, @@ -302,7 +309,10 @@ decl_module! { let stakes = Self::snapshoted_stakes(); let voters = Self::voters(); let bad_presentation_punishment = Self::present_slash_per_voter() * BalanceOf::::sa(voters.len() as u64); - ensure!(T::Currency::can_slash(&who, bad_presentation_punishment), "presenter must have sufficient slashable funds"); + ensure!( + T::Currency::can_slash(&who, bad_presentation_punishment), + "presenter must have sufficient slashable funds" + ); let mut leaderboard = Self::leaderboard().ok_or("leaderboard must exist while present phase active")?; ensure!(total > leaderboard[0].0, "candidate not worthy of leaderboard"); @@ -339,15 +349,15 @@ decl_module! { } } - /// Set the desired member count; if lower than the current count, then seats will not be up - /// election when they expire. If more, then a new vote will be started if one is not already - /// in progress. + /// Set the desired member count; if less than or equal to the number of seats to be retained, + /// then seats will not be up for election when they expire. If more, then a new vote will be + /// started if one is not already in progress. fn set_desired_seats(#[compact] count: u32) { >::put(count); } /// Remove a particular member. A tally will happen instantly (if not already in a presentation - /// period) to fill the seat if removal means that the desired members are not met. + /// period) to fill the seat if removal means that the desired number of members is not met. /// This is effective immediately. fn remove_member(who: ::Source) { let who = T::Lookup::lookup(who)?; @@ -358,14 +368,12 @@ decl_module! { >::put(new_council); } - /// Set the presentation duration. If there is currently a vote being presented for, will - /// invoke `finalize_vote`. + /// Set the presentation duration (number of blocks). fn set_presentation_duration(#[compact] count: T::BlockNumber) { >::put(count); } - /// Set the presentation duration. If there is current a vote being presented for, will - /// invoke `finalize_vote`. + /// Set the term duration (number of blocks). fn set_term_duration(#[compact] count: T::BlockNumber) { >::put(count); } @@ -382,17 +390,17 @@ decl_module! { impl Module { // exposed immutables. - /// True if we're currently in a presentation period. + /// Returns true if we're currently in a presentation period. pub fn presentation_active() -> bool { >::exists() } - /// If `who` a candidate at the moment? + /// Returns true if `who` is a candidate at the moment. pub fn is_a_candidate(who: &T::AccountId) -> bool { >::exists(who) } - /// Determine the block that a vote can happen on which is no less than `n`. + /// Determine the block that a vote can happen on, which is no less than `n`. pub fn next_vote_from(n: T::BlockNumber) -> T::BlockNumber { let voting_period = Self::voting_period(); (n + voting_period - One::one()) / voting_period * voting_period @@ -429,7 +437,7 @@ impl Module { } // Private - /// Check there's nothing to do this block + /// Check that there's nothing to do this block. fn end_block(block_number: T::BlockNumber) -> Result { if (block_number % Self::voting_period()).is_zero() { if let Some(number) = Self::next_tally() { @@ -446,28 +454,24 @@ impl Module { Ok(()) } - /// Remove a voter from the system. Trusts that Self::voters()[index] != voter. - /// - /// ASSERTS: MUST NOT BE CALLED DURING THE PRESENTATION PERIOD! + /// Remove a voter from the system. Trusts that `Self::voters()[index] != voter`. fn remove_voter(voter: &T::AccountId, index: usize, mut voters: Vec) { - // Indicative only: - //assert!(!Self::presentation_active()); >::put({ voters.swap_remove(index); voters }); >::remove(voter); >::remove(voter); } - // Actually do the voting. + /// Actually do the voting. fn do_set_approvals(who: T::AccountId, votes: Vec, index: VoteIndex) -> Result { let candidates = Self::candidates(); ensure!(!Self::presentation_active(), "no approval changes during presentation period"); ensure!(index == Self::vote_index(), "incorrect vote index"); - ensure!(!candidates.is_empty(), "amount of candidates to receive approval votes should be non-zero"); + ensure!(!candidates.is_empty(), "number of candidates to receive approval votes should be non-zero"); // Prevent a vote from voters that provide a list of votes that exceeds the candidates length // since otherwise an attacker may be able to submit a very long list of `votes` that far exceeds // the amount of candidates and waste more computation than a reasonable voting bond would cover. - ensure!(candidates.len() >= votes.len(), "amount of candidate approval votes cannot exceed amount of candidates"); + ensure!(candidates.len() >= votes.len(), "number of candidate approval votes cannot exceed number of candidates"); if !>::exists(&who) { // not yet a voter - deduct bond. @@ -482,7 +486,7 @@ impl Module { Ok(()) } - /// Close the voting, snapshot the staking and the number of seats that are actually up for grabs. + /// Close the voting. Snapshot the staking and the number of seats that are actually up for grabs. fn start_tally() { let active_council = Self::active_council(); let desired_seats = Self::desired_seats() as usize; @@ -505,10 +509,11 @@ impl Module { } } - /// Finalize the vote, removing each of the `removals` and inserting `seats` of the most approved - /// candidates in their place. If the total council members is less than the desired membership + /// Finalize the vote, removing each of the expiring member seats and inserting seats of the most approved + /// candidates in their place. If the total number of council members is less than the desired membership, /// a new vote is started. - /// Clears all presented candidates, returning the bond of the elected ones. + /// + /// Clears all presented candidates, returning the bonds of the elected ones. fn finalize_tally() -> Result { >::kill(); let (_, coming, expiring): (T::BlockNumber, u32, Vec) = @@ -796,7 +801,10 @@ mod tests { assert_eq!(Council::candidates().len(), 0); - assert_noop!(Council::set_approvals(Origin::signed(4), vec![], 0), "amount of candidates to receive approval votes should be non-zero"); + assert_noop!( + Council::set_approvals(Origin::signed(4), vec![], 0), + "number of candidates to receive approval votes should be non-zero" + ); }); } @@ -808,7 +816,10 @@ mod tests { assert_ok!(Council::submit_candidacy(Origin::signed(5), 0)); assert_eq!(Council::candidates().len(), 1); - assert_noop!(Council::set_approvals(Origin::signed(4), vec![true, true], 0), "amount of candidate approval votes cannot exceed amount of candidates"); + assert_noop!(Council::set_approvals( + Origin::signed(4), vec![true, true], 0), + "number of candidate approval votes cannot exceed number of candidates" + ); }); } @@ -953,7 +964,10 @@ mod tests { assert_ok!(Council::end_block(System::block_number())); System::set_block_number(6); - assert_noop!(Council::present_winner(Origin::signed(4), 2, 0, 0), "stake deposited to present winner and be added to leaderboard should be non-zero"); + assert_noop!( + Council::present_winner(Origin::signed(4), 2, 0, 0), + "stake deposited to present winner and be added to leaderboard should be non-zero" + ); }); } @@ -1032,7 +1046,10 @@ mod tests { assert_ok!(Council::end_block(System::block_number())); System::set_block_number(10); - assert_noop!(Council::present_winner(Origin::signed(4), 2, 20, 1), "candidate must not form a duplicated member if elected"); + assert_noop!( + Council::present_winner(Origin::signed(4), 2, 20, 1), + "candidate must not form a duplicated member if elected" + ); }); } @@ -1278,7 +1295,10 @@ mod tests { with_externalities(&mut new_test_ext(false), || { System::set_block_number(4); assert!(!Council::presentation_active()); - assert_noop!(Council::present_winner(Origin::signed(5), 5, 1, 0), "cannot present outside of presentation period"); + assert_noop!( + Council::present_winner(Origin::signed(5), 5, 1, 0), + "cannot present outside of presentation period" + ); }); } @@ -1312,7 +1332,10 @@ mod tests { System::set_block_number(6); assert_eq!(Balances::free_balance(&1), 1); assert_eq!(Balances::reserved_balance(&1), 9); - assert_noop!(Council::present_winner(Origin::signed(1), 1, 20, 0), "presenter must have sufficient slashable funds"); + assert_noop!( + Council::present_winner(Origin::signed(1), 1, 20, 0), + "presenter must have sufficient slashable funds" + ); }); } diff --git a/srml/council/src/voting.rs b/srml/council/src/voting.rs index 92ffbea12e1a2..e33fe102e6ca8 100644 --- a/srml/council/src/voting.rs +++ b/srml/council/src/voting.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -//! Council voting system. +//! Council proposal voting, tallying, and vetoing. use rstd::prelude::*; use rstd::borrow::Borrow; @@ -32,14 +32,22 @@ pub trait Trait: CouncilTrait { decl_storage! { trait Store for Module as CouncilVoting { + /// Period (in blocks) that a veto is in effect. pub CooloffPeriod get(cooloff_period) config(): T::BlockNumber = T::BlockNumber::sa(1000); + /// Period (in blocks) that a vote is open for. pub VotingPeriod get(voting_period) config(): T::BlockNumber = T::BlockNumber::sa(3); - /// Number of blocks by which to delay enactment of successful, non-unanimous-council-instigated referendum proposals. + /// Number of blocks by which to delay enactment of successful, + /// non-unanimous, council-instigated referendum proposals. pub EnactDelayPeriod get(enact_delay_period) config(): T::BlockNumber = T::BlockNumber::sa(0); + /// A list of proposals by block number and proposal ID. pub Proposals get(proposals) build(|_| vec![]): Vec<(T::BlockNumber, T::Hash)>; // ordered by expiry. + /// Map from proposal ID to the proposal. pub ProposalOf get(proposal_of): map T::Hash => Option; + /// List of voters that have voted on a proposal ID. pub ProposalVoters get(proposal_voters): map T::Hash => Vec; + /// Map from a proposal ID and voter to the outcome of the vote. True indicates that it passed. pub CouncilVoteOf get(vote_of): map (T::Hash, T::AccountId) => Option; + /// A veto of a proposal. The veto has an expiry (block number) and a list of vetoers. pub VetoedProposal get(veto_of): map T::Hash => Option<(T::BlockNumber, Vec)>; } } @@ -48,7 +56,7 @@ decl_event!( pub enum Event where ::Hash { /// A voting tally has happened for a referendum cancellation vote. /// Last three are yes, no, abstain counts. - TallyCancelation(Hash, u32, u32, u32), + TallyCancellation(Hash, u32, u32, u32), /// A voting tally has happened for a referendum vote. /// Last three are yes, no, abstain counts. TallyReferendum(Hash, u32, u32, u32), @@ -59,6 +67,12 @@ decl_module! { pub struct Module for enum Call where origin: T::Origin { fn deposit_event() = default; + /// Make a proposal. + /// + /// A councillor vote of yay from `origin` is applied by default. + /// + /// The dispatch origin of this call must be signed by a _councillor_ by the time + /// the proposal is subject to voting. See [`voting_period`](./voting/struct.VotingPeriod.html). fn propose(origin, proposal: Box) { let who = ensure_signed(origin)?; @@ -80,6 +94,9 @@ decl_module! { >::insert((proposal_hash, who.clone()), true); } + /// Vote on a proposal. + /// + /// The dispatch origin of this call must be signed by a _councillor_. fn vote(origin, proposal: T::Hash, approve: bool) { let who = ensure_signed(origin)?; @@ -91,6 +108,16 @@ decl_module! { >::insert((proposal, who), approve); } + /// Veto a proposal. + /// + /// A proposal cannot be vetoed while in the cooloff period. + /// A proposal cannot be vetoed twice by the same account. + /// + /// The proposal information, including voters, is removed and only veto information is kept + /// to indicate when the proposal can be re-proposed, and to make sure that no single councillor + /// can veto it twice. + /// + /// The dispatch origin of this call must be signed by a _councillor_. fn veto(origin, proposal_hash: T::Hash) { let who = ensure_signed(origin)?; @@ -119,10 +146,12 @@ decl_module! { } } + /// Set the cooloff period. fn set_cooloff_period(#[compact] blocks: T::BlockNumber) { >::put(blocks); } + /// Set the voting period. fn set_voting_period(#[compact] blocks: T::BlockNumber) { >::put(blocks); } @@ -137,12 +166,14 @@ decl_module! { } impl Module { + /// Returns true if there are one or more vetos in effect on a proposal. pub fn is_vetoed>(proposal: B) -> bool { Self::veto_of(proposal.borrow()) .map(|(expiry, _): (T::BlockNumber, Vec)| >::block_number() < expiry) .unwrap_or(false) } + /// Returns true if `who` will still be a councillor at block `n`. pub fn will_still_be_councillor_at(who: &T::AccountId, n: T::BlockNumber) -> bool { >::active_council().iter() .find(|&&(ref a, _)| a == who) @@ -150,11 +181,13 @@ impl Module { .unwrap_or(false) } + /// Returns true if `who` is a councillor. pub fn is_councillor(who: &T::AccountId) -> bool { >::active_council().iter() .any(|&(ref a, _)| a == who) } + /// Tally the votes for a proposal. Tuple returned is yay, nay, and abstain votes, respectively. pub fn tally(proposal_hash: &T::Hash) -> (u32, u32, u32) { Self::generic_tally(proposal_hash, |w: &T::AccountId, p: &T::Hash| Self::vote_of((*p, w.clone()))) } @@ -168,11 +201,17 @@ impl Module { >::remove(proposal); } + /// Perform and return the tally result of a specific proposal. + /// + /// The returned tuple is: `(approves, rejects, abstainers)`. fn take_tally(proposal_hash: &T::Hash) -> (u32, u32, u32) { - Self::generic_tally(proposal_hash, |w: &T::AccountId, p: &T::Hash| >::take((*p, w.clone()))) + let vote_of = |w: &T::AccountId, p: &T::Hash| >::take((*p, w.clone())); + Self::generic_tally(proposal_hash, vote_of) } - fn generic_tally Option>(proposal_hash: &T::Hash, vote_of: F) -> (u32, u32, u32) { + fn generic_tally(proposal_hash: &T::Hash, vote_of: F) -> (u32, u32, u32) where + F: Fn(&T::AccountId, &T::Hash) -> Option + { let c = >::active_council(); let (approve, reject) = c.iter() .filter_map(|&(ref a, _)| vote_of(a, proposal_hash)) @@ -185,11 +224,15 @@ impl Module { >::put(p); } + /// Return the proposal that was supposed to be expired at block number `n`. + /// + /// The proposal is removed from storage, if any exists. fn take_proposal_if_expiring_at(n: T::BlockNumber) -> Option<(T::Proposal, T::Hash)> { let proposals = Self::proposals(); + // sorted from earliest to latest expiry. match proposals.first() { Some(&(expiry, hash)) if expiry == n => { - // yes this is horrible, but fixing it will need substantial work in storage. + // NOTE: yes this is horrible, but fixing it will need substantial work in storage. Self::set_proposals(&proposals[1..].to_vec()); >::take(hash).map(|p| (p, hash)) /* defensive only: all queued proposal hashes must have associated proposals*/ } @@ -197,16 +240,27 @@ impl Module { } } + /// Checks for the end of the voting period at the end of each block. + /// + /// Might end up: + /// + /// - cancelling a referendum, or + /// - enacting immediately or with a delay, + /// + /// based on the votes and the type of the proposal. fn end_block(now: T::BlockNumber) -> Result { while let Some((proposal, proposal_hash)) = Self::take_proposal_if_expiring_at(now) { let tally = Self::take_tally(&proposal_hash); + // If the proposal is to kill an already existing referendum `ref_index`. if let Some(&democracy::Call::cancel_referendum(ref_index)) = IsSubType::>::is_aux_sub_type(&proposal) { - Self::deposit_event(RawEvent::TallyCancelation(proposal_hash, tally.0, tally.1, tally.2)); + Self::deposit_event(RawEvent::TallyCancellation(proposal_hash, tally.0, tally.1, tally.2)); + // must have no nay and no abstains if let (_, 0, 0) = tally { >::internal_cancel_referendum(ref_index.into()); } } else { Self::deposit_event(RawEvent::TallyReferendum(proposal_hash.clone(), tally.0, tally.1, tally.2)); + // More yay than nay and abstain. Will be enacted either immediately or with a delay if tally.0 > tally.1 + tally.2 { Self::kill_veto_of(&proposal_hash); // If there were no nay-votes from the council, then it's weakly uncontroversial; we enact immediately. @@ -451,7 +505,10 @@ mod tests { System::set_block_number(2); assert_ok!(CouncilVoting::end_block(System::block_number())); assert_eq!(CouncilVoting::proposals().len(), 0); - assert_eq!(Democracy::active_referenda(), vec![(0, ReferendumInfo::new(5, proposal, VoteThreshold::SuperMajorityAgainst, 0))]); + assert_eq!( + Democracy::active_referenda(), + vec![(0, ReferendumInfo::new(5, proposal, VoteThreshold::SuperMajorityAgainst, 0))] + ); }); } @@ -469,7 +526,10 @@ mod tests { System::set_block_number(2); assert_ok!(CouncilVoting::end_block(System::block_number())); assert_eq!(CouncilVoting::proposals().len(), 0); - assert_eq!(Democracy::active_referenda(), vec![(0, ReferendumInfo::new(5, proposal, VoteThreshold::SimpleMajority, 0))]); + assert_eq!( + Democracy::active_referenda(), + vec![(0, ReferendumInfo::new(5, proposal, VoteThreshold::SimpleMajority, 0))] + ); }); } @@ -478,7 +538,10 @@ mod tests { with_externalities(&mut new_test_ext(true), || { System::set_block_number(1); let proposal = set_balance_proposal(42); - assert_noop!(CouncilVoting::propose(Origin::signed(4), Box::new(proposal)), "proposer would not be on council"); + assert_noop!( + CouncilVoting::propose(Origin::signed(4), Box::new(proposal)), + "proposer would not be on council" + ); }); } @@ -488,7 +551,10 @@ mod tests { System::set_block_number(1); let proposal = set_balance_proposal(42); assert_ok!(CouncilVoting::propose(Origin::signed(1), Box::new(proposal.clone()))); - assert_noop!(CouncilVoting::vote(Origin::signed(4), proposal.blake2_256().into(), true), "only councillors may vote on council proposals"); + assert_noop!( + CouncilVoting::vote(Origin::signed(4), proposal.blake2_256().into(), true), + "only councillors may vote on council proposals" + ); }); } }