From 34b0e7cc9abdbb39171e38b5ec79442649310a02 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Wed, 1 Apr 2020 19:11:01 +0300 Subject: [PATCH 01/23] Add codex ProposalDetails for the frontend --- runtime-modules/proposals/codex/Cargo.toml | 12 +- runtime-modules/proposals/codex/src/lib.rs | 49 +++++-- .../proposals/codex/src/proposal_types/mod.rs | 125 +++++------------- .../codex/src/proposal_types/parameters.rs | 98 ++++++++++++++ .../proposals/codex/src/tests/mod.rs | 19 ++- 5 files changed, 194 insertions(+), 109 deletions(-) create mode 100644 runtime-modules/proposals/codex/src/proposal_types/parameters.rs diff --git a/runtime-modules/proposals/codex/Cargo.toml b/runtime-modules/proposals/codex/Cargo.toml index a1186abc73..98899c176c 100644 --- a/runtime-modules/proposals/codex/Cargo.toml +++ b/runtime-modules/proposals/codex/Cargo.toml @@ -83,6 +83,12 @@ default-features = false git = 'https://github.com/paritytech/substrate.git' rev = 'c37bb08535c49a12320af7facfd555ce05cce2e8' +[dependencies.runtime-io] +default_features = false +git = 'https://github.com/paritytech/substrate.git' +package = 'sr-io' +rev = 'c37bb08535c49a12320af7facfd555ce05cce2e8' + [dependencies.stake] default_features = false package = 'substrate-stake-module' @@ -123,12 +129,6 @@ default_features = false package = 'substrate-content-working-group-module' path = '../../content-working-group' -[dev-dependencies.runtime-io] -default_features = false -git = 'https://github.com/paritytech/substrate.git' -package = 'sr-io' -rev = 'c37bb08535c49a12320af7facfd555ce05cce2e8' - [dev-dependencies.hiring] default_features = false package = 'substrate-hiring-module' diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index 0ed555210e..8671496ac7 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -9,7 +9,8 @@ //! During the proposal creation `codex` also create a discussion thread using the `discussion` //! proposals module. `Codex` uses predefined parameters (eg.:`voting_period`) for each proposal and //! encodes extrinsic calls from dependency modules in order to create proposals inside the `engine` -//! module. +//! module. For each proposal, [its crucial details](./enum.ProposalDetails.html) are saved to the +//! `ProposalDetailsByProposalId` map. //! //! ### Supported extrinsics (proposal types) //! - [create_text_proposal](./struct.Module.html#method.create_text_proposal) @@ -50,12 +51,15 @@ use rstd::clone::Clone; use rstd::prelude::*; use rstd::str::from_utf8; use rstd::vec::Vec; +use runtime_io::blake2_256; use sr_primitives::traits::Zero; use srml_support::dispatch::DispatchResult; use srml_support::traits::{Currency, Get}; use srml_support::{decl_error, decl_module, decl_storage, ensure, print}; use system::{ensure_root, RawOrigin}; +pub use proposal_types::ProposalDetails; + /// 'Proposals codex' substrate module Trait pub trait Trait: system::Trait @@ -158,6 +162,16 @@ decl_storage! { /// Map proposal id to its discussion thread id pub ThreadIdByProposalId get(fn thread_id_by_proposal_id): map T::ProposalId => T::ThreadId; + + /// Map proposal id to proposal details + pub ProposalDetailsByProposalId get(fn proposal_details_by_proposal_id): + map T::ProposalId => ProposalDetails< + BalanceOfMint, + BalanceOfGovernanceCurrency, + T::BlockNumber, + T::AccountId, + T::MemberId + >; } } @@ -182,7 +196,7 @@ decl_module! { let proposal_parameters = proposal_types::parameters::text_proposal::(); let proposal_code = - >::execute_text_proposal(title.clone(), description.clone(), text); + >::execute_text_proposal(title.clone(), description.clone(), text.clone()); Self::create_proposal( origin, @@ -192,6 +206,7 @@ decl_module! { stake_balance, proposal_code.encode(), proposal_parameters, + ProposalDetails::Text(text), )?; } @@ -208,6 +223,8 @@ decl_module! { ensure!(wasm.len() as u32 <= T::RuntimeUpgradeWasmProposalMaxLength::get(), Error::RuntimeProposalSizeExceeded); + let wasm_hash = blake2_256(&wasm); + let proposal_code = >::execute_runtime_upgrade_proposal(title.clone(), description.clone(), wasm); @@ -221,6 +238,7 @@ decl_module! { stake_balance, proposal_code.encode(), proposal_parameters, + ProposalDetails::RuntimeUpgrade(wasm_hash.to_vec()), )?; } @@ -237,7 +255,7 @@ decl_module! { election_parameters.ensure_valid()?; let proposal_code = - >::set_election_parameters(election_parameters); + >::set_election_parameters(election_parameters.clone()); let proposal_parameters = proposal_types::parameters::set_election_parameters_proposal::(); @@ -250,6 +268,7 @@ decl_module! { stake_balance, proposal_code.encode(), proposal_parameters, + ProposalDetails::SetElectionParameters(election_parameters), )?; } @@ -265,7 +284,7 @@ decl_module! { mint_balance: BalanceOfMint, ) { let proposal_code = - >::set_council_mint_capacity(mint_balance); + >::set_council_mint_capacity(mint_balance.clone()); let proposal_parameters = proposal_types::parameters::set_council_mint_capacity_proposal::(); @@ -278,6 +297,7 @@ decl_module! { stake_balance, proposal_code.encode(), proposal_parameters, + ProposalDetails::SetCouncilMintCapacity(mint_balance), )?; } @@ -292,7 +312,7 @@ decl_module! { mint_balance: BalanceOfMint, ) { let proposal_code = - >::set_mint_capacity(mint_balance); + >::set_mint_capacity(mint_balance.clone()); let proposal_parameters = proposal_types::parameters::set_content_working_group_mint_capacity_proposal::(); @@ -305,6 +325,7 @@ decl_module! { stake_balance, proposal_code.encode(), proposal_parameters, + ProposalDetails::SetContentWorkingGroupMintCapacity(mint_balance), )?; } @@ -321,8 +342,10 @@ decl_module! { ) { ensure!(balance != BalanceOfMint::::zero(), Error::SpendingProposalZeroBalance); - let proposal_code = - >::spend_from_council_mint(balance, destination); + let proposal_code = >::spend_from_council_mint( + balance.clone(), + destination.clone() + ); let proposal_parameters = proposal_types::parameters::spending_proposal::(); @@ -335,6 +358,7 @@ decl_module! { stake_balance, proposal_code.encode(), proposal_parameters, + ProposalDetails::Spending(balance, destination), )?; } @@ -350,7 +374,7 @@ decl_module! { new_lead: Option<(T::MemberId, T::AccountId)> ) { let proposal_code = - >::replace_lead(new_lead); + >::replace_lead(new_lead.clone()); let proposal_parameters = proposal_types::parameters::set_lead_proposal::(); @@ -363,6 +387,7 @@ decl_module! { stake_balance, proposal_code.encode(), proposal_parameters, + ProposalDetails::SetLead(new_lead), )?; } @@ -434,6 +459,13 @@ impl Module { stake_balance: Option>, proposal_code: Vec, proposal_parameters: ProposalParameters>, + proposal_details: ProposalDetails< + BalanceOfMint, + BalanceOfGovernanceCurrency, + T::BlockNumber, + T::AccountId, + T::MemberId, + >, ) -> DispatchResult { let account_id = T::MembershipOriginValidator::ensure_actor_origin(origin, member_id.clone())?; @@ -461,6 +493,7 @@ impl Module { )?; >::insert(proposal_id, discussion_thread_id); + >::insert(proposal_id, proposal_details); Ok(()) } diff --git a/runtime-modules/proposals/codex/src/proposal_types/mod.rs b/runtime-modules/proposals/codex/src/proposal_types/mod.rs index ee65d7af17..4c5fff4519 100644 --- a/runtime-modules/proposals/codex/src/proposal_types/mod.rs +++ b/runtime-modules/proposals/codex/src/proposal_types/mod.rs @@ -1,101 +1,42 @@ -pub(crate) mod parameters { - use crate::{BalanceOf, ProposalParameters}; +pub(crate) mod parameters; - // Proposal parameters for the upgrade runtime proposal - pub(crate) fn upgrade_runtime( - ) -> ProposalParameters> { - ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 80, - approval_threshold_percentage: 80, - slashing_quorum_percentage: 80, - slashing_threshold_percentage: 80, - required_stake: Some(>::from(50000u32)), - } - } +use codec::{Decode, Encode}; +use rstd::vec::Vec; +#[cfg(feature = "std")] +use serde::{Deserialize, Serialize}; - // Proposal parameters for the text proposal - pub(crate) fn text_proposal( - ) -> ProposalParameters> { - ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 80, - slashing_threshold_percentage: 82, - required_stake: Some(>::from(500u32)), - } - } +use crate::ElectionParameters; - // Proposal parameters for the 'Set Election Parameters' proposal - pub(crate) fn set_election_parameters_proposal( - ) -> ProposalParameters> { - ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 81, - slashing_threshold_percentage: 80, - required_stake: Some(>::from(500u32)), - } - } +/// Proposal details provide voters the information required for the perceived voting. +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +#[derive(Encode, Decode, Clone, PartialEq, Debug)] +pub enum ProposalDetails { + /// The text of the `text proposal` + Text(Vec), - // Proposal parameters for the 'Set council mint capacity' proposal - pub(crate) fn set_council_mint_capacity_proposal( - ) -> ProposalParameters> { - ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 81, - slashing_threshold_percentage: 84, - required_stake: Some(>::from(500u32)), - } - } + /// The hash of wasm code for the `runtime upgrade proposal` + RuntimeUpgrade(Vec), - // Proposal parameters for the 'Set content working group mint capacity' proposal - pub(crate) fn set_content_working_group_mint_capacity_proposal( - ) -> ProposalParameters> { - ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 81, - slashing_threshold_percentage: 85, - required_stake: Some(>::from(500u32)), - } - } + /// Election parameters for the `set election parameters proposal` + SetElectionParameters(ElectionParameters), - // Proposal parameters for the 'Spending' proposal - pub(crate) fn spending_proposal( - ) -> ProposalParameters> { - ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 84, - slashing_threshold_percentage: 85, - required_stake: Some(>::from(500u32)), - } - } + /// Balance and destination account for the `spending proposal` + Spending(MintedBalance, AccountId), + + /// New leader memberId and account_id for the `set lead proposal` + SetLead(Option<(MemberId, AccountId)>), + + /// Balance for the `set council mint capacity proposal` + SetCouncilMintCapacity(MintedBalance), + + /// Balance for the `set content working group mint capacity proposal` + SetContentWorkingGroupMintCapacity(MintedBalance), +} - // Proposal parameters for the 'Set lead' proposal - pub(crate) fn set_lead_proposal( - ) -> ProposalParameters> { - ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 81, - slashing_threshold_percentage: 86, - required_stake: Some(>::from(500u32)), - } +impl Default + for ProposalDetails +{ + fn default() -> Self { + ProposalDetails::Text(b"invalid proposal details".to_vec()) } } diff --git a/runtime-modules/proposals/codex/src/proposal_types/parameters.rs b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs new file mode 100644 index 0000000000..b9dd1bd4c9 --- /dev/null +++ b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs @@ -0,0 +1,98 @@ +use crate::{BalanceOf, ProposalParameters}; + +// Proposal parameters for the upgrade runtime proposal +pub(crate) fn upgrade_runtime() -> ProposalParameters> +{ + ProposalParameters { + voting_period: T::BlockNumber::from(50000u32), + grace_period: T::BlockNumber::from(10000u32), + approval_quorum_percentage: 80, + approval_threshold_percentage: 80, + slashing_quorum_percentage: 80, + slashing_threshold_percentage: 80, + required_stake: Some(>::from(50000u32)), + } +} + +// Proposal parameters for the text proposal +pub(crate) fn text_proposal() -> ProposalParameters> { + ProposalParameters { + voting_period: T::BlockNumber::from(50000u32), + grace_period: T::BlockNumber::from(10000u32), + approval_quorum_percentage: 40, + approval_threshold_percentage: 51, + slashing_quorum_percentage: 80, + slashing_threshold_percentage: 82, + required_stake: Some(>::from(500u32)), + } +} + +// Proposal parameters for the 'Set Election Parameters' proposal +pub(crate) fn set_election_parameters_proposal( +) -> ProposalParameters> { + ProposalParameters { + voting_period: T::BlockNumber::from(50000u32), + grace_period: T::BlockNumber::from(10000u32), + approval_quorum_percentage: 40, + approval_threshold_percentage: 51, + slashing_quorum_percentage: 81, + slashing_threshold_percentage: 80, + required_stake: Some(>::from(500u32)), + } +} + +// Proposal parameters for the 'Set council mint capacity' proposal +pub(crate) fn set_council_mint_capacity_proposal( +) -> ProposalParameters> { + ProposalParameters { + voting_period: T::BlockNumber::from(50000u32), + grace_period: T::BlockNumber::from(10000u32), + approval_quorum_percentage: 40, + approval_threshold_percentage: 51, + slashing_quorum_percentage: 81, + slashing_threshold_percentage: 84, + required_stake: Some(>::from(500u32)), + } +} + +// Proposal parameters for the 'Set content working group mint capacity' proposal +pub(crate) fn set_content_working_group_mint_capacity_proposal( +) -> ProposalParameters> { + ProposalParameters { + voting_period: T::BlockNumber::from(50000u32), + grace_period: T::BlockNumber::from(10000u32), + approval_quorum_percentage: 40, + approval_threshold_percentage: 51, + slashing_quorum_percentage: 81, + slashing_threshold_percentage: 85, + required_stake: Some(>::from(500u32)), + } +} + +// Proposal parameters for the 'Spending' proposal +pub(crate) fn spending_proposal( +) -> ProposalParameters> { + ProposalParameters { + voting_period: T::BlockNumber::from(50000u32), + grace_period: T::BlockNumber::from(10000u32), + approval_quorum_percentage: 40, + approval_threshold_percentage: 51, + slashing_quorum_percentage: 84, + slashing_threshold_percentage: 85, + required_stake: Some(>::from(500u32)), + } +} + +// Proposal parameters for the 'Set lead' proposal +pub(crate) fn set_lead_proposal( +) -> ProposalParameters> { + ProposalParameters { + voting_period: T::BlockNumber::from(50000u32), + grace_period: T::BlockNumber::from(10000u32), + approval_quorum_percentage: 40, + approval_threshold_percentage: 51, + slashing_quorum_percentage: 81, + slashing_threshold_percentage: 86, + required_stake: Some(>::from(500u32)), + } +} diff --git a/runtime-modules/proposals/codex/src/tests/mod.rs b/runtime-modules/proposals/codex/src/tests/mod.rs index 71d691ecd2..c82d34027b 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -5,9 +5,10 @@ use srml_support::traits::Currency; use srml_support::StorageMap; use system::RawOrigin; -use crate::{BalanceOf, Error}; +use crate::{BalanceOf, Error, ProposalDetails}; use mock::*; use proposal_engine::ProposalParameters; +use runtime_io::blake2_256; use srml_support::dispatch::DispatchResult; struct ProposalTestFixture @@ -22,6 +23,7 @@ where invalid_stake_call: InvalidStakeCall, successful_call: SuccessfulCall, proposal_parameters: ProposalParameters, + proposal_details: ProposalDetails, } impl @@ -59,6 +61,10 @@ where let proposal = ProposalsEngine::proposals(proposal_id); // check for correct proposal parameters assert_eq!(proposal.parameters, self.proposal_parameters); + + // proposal details was set + let details = >::get(proposal_id); + assert_eq!(details, self.proposal_details); } pub fn check_all(&self) { @@ -113,6 +119,7 @@ fn create_text_proposal_common_checks_succeed() { ) }, proposal_parameters: crate::proposal_types::parameters::text_proposal::(), + proposal_details: ProposalDetails::Text(b"text".to_vec()), }; proposal_fixture.check_all(); }); @@ -195,6 +202,7 @@ fn create_runtime_upgrade_common_checks_succeed() { ) }, proposal_parameters: crate::proposal_types::parameters::upgrade_runtime::(), + proposal_details: ProposalDetails::RuntimeUpgrade(blake2_256(b"wasm").to_vec()), }; proposal_fixture.check_all(); }); @@ -289,6 +297,7 @@ fn create_set_election_parameters_proposal_common_checks_succeed() { }, proposal_parameters: crate::proposal_types::parameters::set_election_parameters_proposal::(), + proposal_details: ProposalDetails::SetElectionParameters(election_parameters), }; proposal_fixture.check_all(); }); @@ -355,11 +364,12 @@ fn create_set_council_mint_capacity_proposal_common_checks_succeed() { b"title".to_vec(), b"body".to_vec(), Some(>::from(500u32)), - 0, + 10, ) }, proposal_parameters: crate::proposal_types::parameters::set_council_mint_capacity_proposal::(), + proposal_details: ProposalDetails::SetCouncilMintCapacity(10), }; proposal_fixture.check_all(); }); @@ -406,10 +416,11 @@ fn create_set_content_working_group_mint_capacity_proposal_common_checks_succeed b"title".to_vec(), b"body".to_vec(), Some(>::from(500u32)), - 0, + 10, ) }, proposal_parameters: crate::proposal_types::parameters::set_content_working_group_mint_capacity_proposal::(), + proposal_details: ProposalDetails::SetContentWorkingGroupMintCapacity(10), }; proposal_fixture.check_all(); }); @@ -464,6 +475,7 @@ fn create_spending_proposal_common_checks_succeed() { ) }, proposal_parameters: crate::proposal_types::parameters::spending_proposal::(), + proposal_details: ProposalDetails::Spending(100, 2), }; proposal_fixture.check_all(); }); @@ -532,6 +544,7 @@ fn create_set_lead_proposal_common_checks_succeed() { ) }, proposal_parameters: crate::proposal_types::parameters::set_lead_proposal::(), + proposal_details: ProposalDetails::SetLead(Some((20, 10))), }; proposal_fixture.check_all(); }); From 51cde50fb4c314a137abc5550222bd8c6a5b210d Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Thu, 2 Apr 2020 11:51:58 +0300 Subject: [PATCH 02/23] =?UTF-8?q?Add=20=E2=80=98create=5Fevict=5Fstorage?= =?UTF-8?q?=5Fprovider=5Fproposal=E2=80=99=20extrinsic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add ‘create_evict_storage_provider_proposal’ extrinsic to the codex module - add tests --- Cargo.lock | 1 + runtime-modules/proposals/codex/Cargo.toml | 6 +++ runtime-modules/proposals/codex/src/lib.rs | 32 ++++++++++- .../proposals/codex/src/proposal_types/mod.rs | 3 ++ .../codex/src/proposal_types/parameters.rs | 18 ++++++- .../proposals/codex/src/tests/mock.rs | 9 ++++ .../proposals/codex/src/tests/mod.rs | 53 ++++++++++++++++++- 7 files changed, 118 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8621e62937..536145fff1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5122,6 +5122,7 @@ dependencies = [ "substrate-proposals-discussion-module", "substrate-proposals-engine-module", "substrate-recurring-reward-module", + "substrate-roles-module", "substrate-stake-module", "substrate-token-mint-module", "substrate-versioned-store", diff --git a/runtime-modules/proposals/codex/Cargo.toml b/runtime-modules/proposals/codex/Cargo.toml index 98899c176c..d8a27c08c0 100644 --- a/runtime-modules/proposals/codex/Cargo.toml +++ b/runtime-modules/proposals/codex/Cargo.toml @@ -23,6 +23,7 @@ std = [ 'membership/std', 'governance/std', 'mint/std', + 'roles/std', ] @@ -129,6 +130,11 @@ default_features = false package = 'substrate-content-working-group-module' path = '../../content-working-group' +[dependencies.roles] +default_features = false +package = 'substrate-roles-module' +path = '../../roles' + [dev-dependencies.hiring] default_features = false package = 'substrate-hiring-module' diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index 8671496ac7..ff3db1f482 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -20,6 +20,7 @@ //! - [create_set_content_working_group_mint_capacity_proposal](./struct.Module.html#method.create_set_content_working_group_mint_capacity_proposal) //! - [create_spending_proposal](./struct.Module.html#method.create_spending_proposal) //! - [create_set_lead_proposal](./struct.Module.html#method.create_set_lead_proposal) +//! - [create_evict_storage_provider_proposal](./struct.Module.html#method.create_evict_storage_provider_proposal) //! //! ### Proposal implementations of this module //! - execute_text_proposal - prints the proposal to the log @@ -68,6 +69,7 @@ pub trait Trait: + membership::members::Trait + governance::election::Trait + content_working_group::Trait + + roles::actors::Trait { /// Defines max allowed text proposal length. type TextProposalMaxLength: Get; @@ -228,7 +230,7 @@ decl_module! { let proposal_code = >::execute_runtime_upgrade_proposal(title.clone(), description.clone(), wasm); - let proposal_parameters = proposal_types::parameters::upgrade_runtime::(); + let proposal_parameters = proposal_types::parameters::runtime_upgrade_proposal::(); Self::create_proposal( origin, @@ -391,6 +393,34 @@ decl_module! { )?; } + /// Create 'Evict storage provider' proposal type. + /// This proposal uses `remove_actor()` extrinsic from the `roles::actors` module. + pub fn create_evict_storage_provider_proposal( + origin, + member_id: MemberId, + title: Vec, + description: Vec, + stake_balance: Option>, + actor_account: T::AccountId, + ) { + let proposal_code = + >::remove_actor(actor_account.clone()); + + let proposal_parameters = + proposal_types::parameters::evict_storage_provider_proposal::(); + + Self::create_proposal( + origin, + member_id, + title, + description, + stake_balance, + proposal_code.encode(), + proposal_parameters, + ProposalDetails::EvictStorageProvider(actor_account), + )?; + } + // *************** Extrinsic to execute /// Text proposal extrinsic. Should be used as callable object to pass to the `engine` module. diff --git a/runtime-modules/proposals/codex/src/proposal_types/mod.rs b/runtime-modules/proposals/codex/src/proposal_types/mod.rs index 4c5fff4519..bf1eaadd2d 100644 --- a/runtime-modules/proposals/codex/src/proposal_types/mod.rs +++ b/runtime-modules/proposals/codex/src/proposal_types/mod.rs @@ -31,6 +31,9 @@ pub enum ProposalDetails Default diff --git a/runtime-modules/proposals/codex/src/proposal_types/parameters.rs b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs index b9dd1bd4c9..bd1162b7c5 100644 --- a/runtime-modules/proposals/codex/src/proposal_types/parameters.rs +++ b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs @@ -1,8 +1,8 @@ use crate::{BalanceOf, ProposalParameters}; // Proposal parameters for the upgrade runtime proposal -pub(crate) fn upgrade_runtime() -> ProposalParameters> -{ +pub(crate) fn runtime_upgrade_proposal( +) -> ProposalParameters> { ProposalParameters { voting_period: T::BlockNumber::from(50000u32), grace_period: T::BlockNumber::from(10000u32), @@ -96,3 +96,17 @@ pub(crate) fn set_lead_proposal( required_stake: Some(>::from(500u32)), } } + +// Proposal parameters for the 'Evict storage provider' proposal +pub(crate) fn evict_storage_provider_proposal( +) -> ProposalParameters> { + ProposalParameters { + voting_period: T::BlockNumber::from(50000u32), + grace_period: T::BlockNumber::from(10000u32), + approval_quorum_percentage: 40, + approval_threshold_percentage: 51, + slashing_quorum_percentage: 81, + slashing_threshold_percentage: 87, + required_stake: Some(>::from(500u32)), + } +} diff --git a/runtime-modules/proposals/codex/src/tests/mock.rs b/runtime-modules/proposals/codex/src/tests/mock.rs index 37a958aa68..4cc7ea64d1 100644 --- a/runtime-modules/proposals/codex/src/tests/mock.rs +++ b/runtime-modules/proposals/codex/src/tests/mock.rs @@ -187,6 +187,15 @@ impl hiring::Trait for Test { type StakeHandlerProvider = hiring::Module; } +impl roles::actors::Trait for Test { + type Event = (); + type OnActorRemoved = (); +} + +impl roles::actors::ActorRemoved for () { + fn actor_removed(_: &u64) {} +} + impl crate::Trait for Test { type TextProposalMaxLength = TextProposalMaxLength; type RuntimeUpgradeWasmProposalMaxLength = RuntimeUpgradeWasmProposalMaxLength; diff --git a/runtime-modules/proposals/codex/src/tests/mod.rs b/runtime-modules/proposals/codex/src/tests/mod.rs index c82d34027b..3f9d6e5259 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -201,7 +201,7 @@ fn create_runtime_upgrade_common_checks_succeed() { b"wasm".to_vec(), ) }, - proposal_parameters: crate::proposal_types::parameters::upgrade_runtime::(), + proposal_parameters: crate::proposal_types::parameters::runtime_upgrade_proposal::(), proposal_details: ProposalDetails::RuntimeUpgrade(blake2_256(b"wasm").to_vec()), }; proposal_fixture.check_all(); @@ -549,3 +549,54 @@ fn create_set_lead_proposal_common_checks_succeed() { proposal_fixture.check_all(); }); } + +#[test] +fn create_evict_storage_provider_common_checks_succeed() { + initial_test_ext().execute_with(|| { + let proposal_fixture = ProposalTestFixture { + insufficient_rights_call: || { + ProposalCodex::create_evict_storage_provider_proposal( + RawOrigin::None.into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + None, + 1, + ) + }, + empty_stake_call: || { + ProposalCodex::create_evict_storage_provider_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + None, + 1, + ) + }, + invalid_stake_call: || { + ProposalCodex::create_evict_storage_provider_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(5000u32)), + 1, + ) + }, + successful_call: || { + ProposalCodex::create_evict_storage_provider_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(500u32)), + 1, + ) + }, + proposal_parameters: crate::proposal_types::parameters::evict_storage_provider_proposal::(), + proposal_details: ProposalDetails::EvictStorageProvider(1), + }; + proposal_fixture.check_all(); + }); +} From b994ee7016aee00775a9ed120667348b599055d4 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Thu, 2 Apr 2020 13:37:57 +0300 Subject: [PATCH 03/23] =?UTF-8?q?Add=20=E2=80=98create=5Fset=5Fvalidator?= =?UTF-8?q?=5Fcount=5Fproposal=E2=80=99=20extrinsic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add ‘create_set_validator_count_proposal’ extrinsic to the codex module - add tests --- Cargo.lock | 3 + runtime-modules/proposals/codex/Cargo.toml | 21 +++++++ runtime-modules/proposals/codex/src/lib.rs | 30 ++++++++++ .../proposals/codex/src/proposal_types/mod.rs | 3 + .../codex/src/proposal_types/parameters.rs | 14 +++++ .../proposals/codex/src/tests/mock.rs | 55 +++++++++++++++++-- .../proposals/codex/src/tests/mod.rs | 55 ++++++++++++++++++- 7 files changed, 176 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 536145fff1..f2c844959b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5108,8 +5108,11 @@ dependencies = [ "serde", "sr-io", "sr-primitives", + "sr-staking-primitives", "sr-std", "srml-balances", + "srml-staking", + "srml-staking-reward-curve", "srml-support", "srml-system", "srml-timestamp", diff --git a/runtime-modules/proposals/codex/Cargo.toml b/runtime-modules/proposals/codex/Cargo.toml index d8a27c08c0..9369a6daf9 100644 --- a/runtime-modules/proposals/codex/Cargo.toml +++ b/runtime-modules/proposals/codex/Cargo.toml @@ -15,6 +15,7 @@ std = [ 'sr-primitives/std', 'system/std', 'timestamp/std', + 'staking/std', 'serde', 'proposal_engine/std', 'proposal_discussion/std', @@ -84,6 +85,12 @@ default-features = false git = 'https://github.com/paritytech/substrate.git' rev = 'c37bb08535c49a12320af7facfd555ce05cce2e8' +[dependencies.staking] +default_features = false +git = 'https://github.com/paritytech/substrate.git' +package = 'srml-staking' +rev = 'c37bb08535c49a12320af7facfd555ce05cce2e8' + [dependencies.runtime-io] default_features = false git = 'https://github.com/paritytech/substrate.git' @@ -159,3 +166,17 @@ path = '../../versioned-store-permissions' default_features = false package = 'substrate-recurring-reward-module' path = '../../recurring-reward' + +[dev-dependencies.sr-staking-primitives] +default_features = false +git = 'https://github.com/paritytech/substrate.git' +package = 'sr-staking-primitives' +rev = 'c37bb08535c49a12320af7facfd555ce05cce2e8' + +# don't rename the dependency it is causing some strange compiler error: +# https://github.com/rust-lang/rust/issues/64450 +[dev-dependencies.srml-staking-reward-curve] +package = 'srml-staking-reward-curve' +git = 'https://github.com/paritytech/substrate.git' +default_features = false +rev = 'c37bb08535c49a12320af7facfd555ce05cce2e8' \ No newline at end of file diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index ff3db1f482..ce0b4eb2fb 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -21,6 +21,7 @@ //! - [create_spending_proposal](./struct.Module.html#method.create_spending_proposal) //! - [create_set_lead_proposal](./struct.Module.html#method.create_set_lead_proposal) //! - [create_evict_storage_provider_proposal](./struct.Module.html#method.create_evict_storage_provider_proposal) +//! - [create_set_validator_count_proposal](./struct.Module.html#method.create_set_validator_count_proposal) //! //! ### Proposal implementations of this module //! - execute_text_proposal - prints the proposal to the log @@ -70,6 +71,7 @@ pub trait Trait: + governance::election::Trait + content_working_group::Trait + roles::actors::Trait + + staking::Trait { /// Defines max allowed text proposal length. type TextProposalMaxLength: Get; @@ -421,6 +423,34 @@ decl_module! { )?; } + /// Create 'Evict storage provider' proposal type. + /// This proposal uses `set_validator_count()` extrinsic from the Substrate `staking` module. + pub fn create_set_validator_count_proposal( + origin, + member_id: MemberId, + title: Vec, + description: Vec, + stake_balance: Option>, + new_validator_count: u32, + ) { + let proposal_code = + >::set_validator_count(new_validator_count); + + let proposal_parameters = + proposal_types::parameters::set_validator_count_proposal::(); + + Self::create_proposal( + origin, + member_id, + title, + description, + stake_balance, + proposal_code.encode(), + proposal_parameters, + ProposalDetails::SetValidatorCount(new_validator_count), + )?; + } + // *************** Extrinsic to execute /// Text proposal extrinsic. Should be used as callable object to pass to the `engine` module. diff --git a/runtime-modules/proposals/codex/src/proposal_types/mod.rs b/runtime-modules/proposals/codex/src/proposal_types/mod.rs index bf1eaadd2d..cbaa17c1bd 100644 --- a/runtime-modules/proposals/codex/src/proposal_types/mod.rs +++ b/runtime-modules/proposals/codex/src/proposal_types/mod.rs @@ -34,6 +34,9 @@ pub enum ProposalDetails Default diff --git a/runtime-modules/proposals/codex/src/proposal_types/parameters.rs b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs index bd1162b7c5..f4be5877a9 100644 --- a/runtime-modules/proposals/codex/src/proposal_types/parameters.rs +++ b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs @@ -110,3 +110,17 @@ pub(crate) fn evict_storage_provider_proposal( required_stake: Some(>::from(500u32)), } } + +// Proposal parameters for the 'Set validator count' proposal +pub(crate) fn set_validator_count_proposal( +) -> ProposalParameters> { + ProposalParameters { + voting_period: T::BlockNumber::from(50000u32), + grace_period: T::BlockNumber::from(10000u32), + approval_quorum_percentage: 40, + approval_threshold_percentage: 51, + slashing_quorum_percentage: 82, + slashing_threshold_percentage: 87, + required_stake: Some(>::from(500u32)), + } +} diff --git a/runtime-modules/proposals/codex/src/tests/mock.rs b/runtime-modules/proposals/codex/src/tests/mock.rs index 4cc7ea64d1..c911208c6a 100644 --- a/runtime-modules/proposals/codex/src/tests/mock.rs +++ b/runtime-modules/proposals/codex/src/tests/mock.rs @@ -1,17 +1,20 @@ #![cfg(test)] - -pub use system; +// srml_staking_reward_curve::build! - substrate macro produces a warning. +// TODO: remove after post-Rome substrate upgrade +#![allow(array_into_iter)] pub use primitives::{Blake2Hasher, H256}; +use proposal_engine::VotersParameters; +use sr_primitives::curve::PiecewiseLinear; pub use sr_primitives::{ testing::{Digest, DigestItem, Header, UintAuthorityId}, traits::{BlakeTwo256, Convert, IdentityLookup, OnFinalize}, weights::Weight, BuildStorage, DispatchError, Perbill, }; - -use proposal_engine::VotersParameters; +use sr_staking_primitives::SessionIndex; use srml_support::{impl_outer_dispatch, impl_outer_origin, parameter_types}; +pub use system; impl_outer_origin! { pub enum Origin for Test {} @@ -196,6 +199,50 @@ impl roles::actors::ActorRemoved for () { fn actor_removed(_: &u64) {} } +srml_staking_reward_curve::build! { + const I_NPOS: PiecewiseLinear<'static> = curve!( + min_inflation: 0_025_000, + max_inflation: 0_100_000, + ideal_stake: 0_500_000, + falloff: 0_050_000, + max_piece_count: 40, + test_precision: 0_005_000, + ); +} + +parameter_types! { + pub const SessionsPerEra: SessionIndex = 3; + pub const BondingDuration: staking::EraIndex = 3; + pub const RewardCurve: &'static PiecewiseLinear<'static> = &I_NPOS; +} +impl staking::Trait for Test { + type Currency = balances::Module; + type Time = timestamp::Module; + type CurrencyToVote = (); + type RewardRemainder = (); + type Event = (); + type Slash = (); + type Reward = (); + type SessionsPerEra = SessionsPerEra; + type BondingDuration = BondingDuration; + type SessionInterface = Self; + type RewardCurve = RewardCurve; +} + +impl staking::SessionInterface for Test { + fn disable_validator(_: &u64) -> Result { + unimplemented!() + } + + fn validators() -> Vec { + unimplemented!() + } + + fn prune_historical_up_to(_: u32) { + unimplemented!() + } +} + impl crate::Trait for Test { type TextProposalMaxLength = TextProposalMaxLength; type RuntimeUpgradeWasmProposalMaxLength = RuntimeUpgradeWasmProposalMaxLength; diff --git a/runtime-modules/proposals/codex/src/tests/mod.rs b/runtime-modules/proposals/codex/src/tests/mod.rs index 3f9d6e5259..5a0922f817 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -551,7 +551,7 @@ fn create_set_lead_proposal_common_checks_succeed() { } #[test] -fn create_evict_storage_provider_common_checks_succeed() { +fn create_evict_storage_provider_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { let proposal_fixture = ProposalTestFixture { insufficient_rights_call: || { @@ -600,3 +600,56 @@ fn create_evict_storage_provider_common_checks_succeed() { proposal_fixture.check_all(); }); } + +#[test] +fn create_set_validator_count_proposal_common_checks_succeed() { + initial_test_ext().execute_with(|| { + let proposal_fixture = ProposalTestFixture { + insufficient_rights_call: || { + ProposalCodex::create_set_validator_count_proposal( + RawOrigin::None.into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + None, + 1, + ) + }, + empty_stake_call: || { + ProposalCodex::create_set_validator_count_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + None, + 1, + ) + }, + invalid_stake_call: || { + ProposalCodex::create_set_validator_count_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(5000u32)), + 1, + ) + }, + successful_call: || { + ProposalCodex::create_set_validator_count_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(500u32)), + 1, + ) + }, + proposal_parameters: crate::proposal_types::parameters::set_validator_count_proposal::< + Test, + >(), + proposal_details: ProposalDetails::SetValidatorCount(1), + }; + proposal_fixture.check_all(); + }); +} From e77768af7c8b514c675fdd47dd66d8b267c1c149 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Thu, 2 Apr 2020 18:19:54 +0300 Subject: [PATCH 04/23] =?UTF-8?q?Add=20=E2=80=98create=5Fset=5Fstorage=5Fr?= =?UTF-8?q?ole=5Fparameters=5Fproposal=E2=80=99=20extrinsic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add ‘create_set_storage_role_parameters_proposal’ extrinsic - add tests --- Cargo.lock | 4 +- runtime-modules/membership/Cargo.toml | 2 +- runtime-modules/membership/src/role_types.rs | 4 ++ runtime-modules/proposals/codex/src/lib.rs | 32 +++++++++++ .../proposals/codex/src/proposal_types/mod.rs | 22 ++++---- .../codex/src/proposal_types/parameters.rs | 14 +++++ .../proposals/codex/src/tests/mod.rs | 53 +++++++++++++++++++ runtime-modules/roles/Cargo.toml | 2 +- runtime-modules/roles/src/actors.rs | 4 ++ 9 files changed, 124 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f2c844959b..8f24c1d188 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4912,7 +4912,7 @@ dependencies = [ [[package]] name = "substrate-membership-module" -version = "1.0.0" +version = "1.0.1" dependencies = [ "parity-scale-codec", "serde", @@ -5195,7 +5195,7 @@ dependencies = [ [[package]] name = "substrate-roles-module" -version = "1.0.0" +version = "1.0.1" dependencies = [ "parity-scale-codec", "serde", diff --git a/runtime-modules/membership/Cargo.toml b/runtime-modules/membership/Cargo.toml index 8fc8e9adc5..9961e03f1d 100644 --- a/runtime-modules/membership/Cargo.toml +++ b/runtime-modules/membership/Cargo.toml @@ -1,6 +1,6 @@ [package] name = 'substrate-membership-module' -version = '1.0.0' +version = '1.0.1' authors = ['Joystream contributors'] edition = '2018' diff --git a/runtime-modules/membership/src/role_types.rs b/runtime-modules/membership/src/role_types.rs index 803e048b8e..25dd8c9938 100644 --- a/runtime-modules/membership/src/role_types.rs +++ b/runtime-modules/membership/src/role_types.rs @@ -2,6 +2,10 @@ use codec::{Decode, Encode}; use rstd::collections::btree_set::BTreeSet; use rstd::prelude::*; +#[cfg(feature = "std")] +use serde::{Deserialize, Serialize}; + +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] #[derive(Encode, Decode, Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Debug)] pub enum Role { StorageProvider, diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index ce0b4eb2fb..aba319b2f1 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -22,6 +22,7 @@ //! - [create_set_lead_proposal](./struct.Module.html#method.create_set_lead_proposal) //! - [create_evict_storage_provider_proposal](./struct.Module.html#method.create_evict_storage_provider_proposal) //! - [create_set_validator_count_proposal](./struct.Module.html#method.create_set_validator_count_proposal) +//! - [create_set_storage_role_parameters_proposal](./struct.Module.html#method.create_set_storage_role_parameters_proposal) //! //! ### Proposal implementations of this module //! - execute_text_proposal - prints the proposal to the log @@ -49,6 +50,7 @@ use codec::Encode; use common::origin_validator::ActorOriginValidator; use governance::election_params::ElectionParameters; use proposal_engine::ProposalParameters; +use roles::actors::{Role, RoleParameters}; use rstd::clone::Clone; use rstd::prelude::*; use rstd::str::from_utf8; @@ -451,6 +453,36 @@ decl_module! { )?; } + /// Create 'Set storage roles parameters' proposal type. + /// This proposal uses `set_role_parameters()` extrinsic from the Substrate `roles::actors` module. + pub fn create_set_storage_role_parameters_proposal( + origin, + member_id: MemberId, + title: Vec, + description: Vec, + stake_balance: Option>, + role_parameters: RoleParameters, T::BlockNumber> + ) { + let proposal_code = >::set_role_parameters( + Role::StorageProvider, + role_parameters.clone() + ); + + let proposal_parameters = + proposal_types::parameters::set_storage_role_parameters_proposal::(); + + Self::create_proposal( + origin, + member_id, + title, + description, + stake_balance, + proposal_code.encode(), + proposal_parameters, + ProposalDetails::SetStorageRoleParameters(role_parameters), + )?; + } + // *************** Extrinsic to execute /// Text proposal extrinsic. Should be used as callable object to pass to the `engine` module. diff --git a/runtime-modules/proposals/codex/src/proposal_types/mod.rs b/runtime-modules/proposals/codex/src/proposal_types/mod.rs index cbaa17c1bd..43a7138903 100644 --- a/runtime-modules/proposals/codex/src/proposal_types/mod.rs +++ b/runtime-modules/proposals/codex/src/proposal_types/mod.rs @@ -6,37 +6,41 @@ use rstd::vec::Vec; use serde::{Deserialize, Serialize}; use crate::ElectionParameters; +use roles::actors::RoleParameters; /// Proposal details provide voters the information required for the perceived voting. #[cfg_attr(feature = "std", derive(Serialize, Deserialize))] #[derive(Encode, Decode, Clone, PartialEq, Debug)] pub enum ProposalDetails { - /// The text of the `text proposal` + /// The text of the `text` proposal Text(Vec), - /// The hash of wasm code for the `runtime upgrade proposal` + /// The hash of wasm code for the `runtime upgrade` proposal RuntimeUpgrade(Vec), - /// Election parameters for the `set election parameters proposal` + /// Election parameters for the `set election parameters` proposal SetElectionParameters(ElectionParameters), - /// Balance and destination account for the `spending proposal` + /// Balance and destination account for the `spending` proposal Spending(MintedBalance, AccountId), - /// New leader memberId and account_id for the `set lead proposal` + /// New leader memberId and account_id for the `set lead` proposal SetLead(Option<(MemberId, AccountId)>), - /// Balance for the `set council mint capacity proposal` + /// Balance for the `set council mint capacity` proposal SetCouncilMintCapacity(MintedBalance), - /// Balance for the `set content working group mint capacity proposal` + /// Balance for the `set content working group mint capacity` proposal SetContentWorkingGroupMintCapacity(MintedBalance), - /// AccountId for the `evict storage provider proposal` + /// AccountId for the `evict storage provider` proposal EvictStorageProvider(AccountId), - /// Validator count for the `set validator count proposal` + /// Validator count for the `set validator count` proposal SetValidatorCount(u32), + + /// Role parameters for the `set storage role parameters` proposal + SetStorageRoleParameters(RoleParameters), } impl Default diff --git a/runtime-modules/proposals/codex/src/proposal_types/parameters.rs b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs index f4be5877a9..bcbf9a4e11 100644 --- a/runtime-modules/proposals/codex/src/proposal_types/parameters.rs +++ b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs @@ -124,3 +124,17 @@ pub(crate) fn set_validator_count_proposal( required_stake: Some(>::from(500u32)), } } + +// Proposal parameters for the 'Set storage role parameters' proposal +pub(crate) fn set_storage_role_parameters_proposal( +) -> ProposalParameters> { + ProposalParameters { + voting_period: T::BlockNumber::from(50000u32), + grace_period: T::BlockNumber::from(10000u32), + approval_quorum_percentage: 40, + approval_threshold_percentage: 51, + slashing_quorum_percentage: 82, + slashing_threshold_percentage: 88, + required_stake: Some(>::from(500u32)), + } +} diff --git a/runtime-modules/proposals/codex/src/tests/mod.rs b/runtime-modules/proposals/codex/src/tests/mod.rs index 5a0922f817..95b2e4a459 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -8,6 +8,7 @@ use system::RawOrigin; use crate::{BalanceOf, Error, ProposalDetails}; use mock::*; use proposal_engine::ProposalParameters; +use roles::actors::RoleParameters; use runtime_io::blake2_256; use srml_support::dispatch::DispatchResult; @@ -653,3 +654,55 @@ fn create_set_validator_count_proposal_common_checks_succeed() { proposal_fixture.check_all(); }); } + +#[test] +fn create_set_storage_role_parameters_proposal_common_checks_succeed() { + initial_test_ext().execute_with(|| { + let proposal_fixture = ProposalTestFixture { + insufficient_rights_call: || { + ProposalCodex::create_set_storage_role_parameters_proposal( + RawOrigin::None.into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + None, + RoleParameters::default(), + ) + }, + empty_stake_call: || { + ProposalCodex::create_set_storage_role_parameters_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + None, + RoleParameters::default(), + ) + }, + invalid_stake_call: || { + ProposalCodex::create_set_storage_role_parameters_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(5000u32)), + RoleParameters::default(), + ) + }, + successful_call: || { + ProposalCodex::create_set_storage_role_parameters_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(500u32)), + RoleParameters::default(), + ) + }, + proposal_parameters: + crate::proposal_types::parameters::set_storage_role_parameters_proposal::(), + proposal_details: ProposalDetails::SetStorageRoleParameters(RoleParameters::default()), + }; + proposal_fixture.check_all(); + }); +} diff --git a/runtime-modules/roles/Cargo.toml b/runtime-modules/roles/Cargo.toml index 78f024c802..4b31781d2d 100644 --- a/runtime-modules/roles/Cargo.toml +++ b/runtime-modules/roles/Cargo.toml @@ -1,6 +1,6 @@ [package] name = 'substrate-roles-module' -version = '1.0.0' +version = '1.0.1' authors = ['Joystream contributors'] edition = '2018' diff --git a/runtime-modules/roles/src/actors.rs b/runtime-modules/roles/src/actors.rs index c702726168..d1771cd135 100644 --- a/runtime-modules/roles/src/actors.rs +++ b/runtime-modules/roles/src/actors.rs @@ -8,10 +8,14 @@ use srml_support::traits::{ use srml_support::{decl_event, decl_module, decl_storage, ensure}; use system::{self, ensure_root, ensure_signed}; +#[cfg(feature = "std")] +use serde::{Deserialize, Serialize}; + pub use membership::members::Role; const STAKING_ID: LockIdentifier = *b"role_stk"; +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] #[derive(Encode, Decode, Copy, Clone, Eq, PartialEq, Debug)] pub struct RoleParameters { // minium balance required to stake to enter a role From 6c01c09dc549f2bacce4294b1282d046a8cc7a56 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Fri, 3 Apr 2020 11:34:23 +0300 Subject: [PATCH 05/23] Fix reset_proposal() in the engine module - fix reset_proposal() in the engine module: clean VoteExistsByProposalByVoter double map - add tests --- runtime-modules/proposals/engine/src/lib.rs | 1 + runtime-modules/proposals/engine/src/tests/mod.rs | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/runtime-modules/proposals/engine/src/lib.rs b/runtime-modules/proposals/engine/src/lib.rs index 19975197f9..e4fae3789e 100644 --- a/runtime-modules/proposals/engine/src/lib.rs +++ b/runtime-modules/proposals/engine/src/lib.rs @@ -469,6 +469,7 @@ impl Module { >::enumerate().for_each(|(proposal_id, _)| { >::mutate(proposal_id, |proposal| { proposal.reset_proposal(); + >::remove_prefix(&proposal_id); }); }); } diff --git a/runtime-modules/proposals/engine/src/tests/mod.rs b/runtime-modules/proposals/engine/src/tests/mod.rs index df2e49a9bf..48fca36675 100644 --- a/runtime-modules/proposals/engine/src/tests/mod.rs +++ b/runtime-modules/proposals/engine/src/tests/mod.rs @@ -6,7 +6,7 @@ use mock::*; use codec::Encode; use rstd::rc::Rc; use sr_primitives::traits::{DispatchResult, OnFinalize, OnInitialize}; -use srml_support::{StorageMap, StorageValue}; +use srml_support::{StorageDoubleMap, StorageMap, StorageValue}; use system::RawOrigin; use system::{EventRecord, Phase}; @@ -1339,6 +1339,10 @@ fn proposal_reset_succeeds() { vote_generator.vote_and_assert_ok(VoteKind::Slash); assert!(>::exists(proposal_id)); + assert_eq!( + >::get(&proposal_id, &2), + VoteKind::Abstain + ); run_to_block_and_finalize(2); @@ -1367,5 +1371,11 @@ fn proposal_reset_succeeds() { slashes: 0, } ); + + // whole double map prefix was removed (should return default value) + assert_eq!( + >::get(&proposal_id, &2), + VoteKind::default() + ); }); } From 020f3c7adc1bbd2e6ae9f9df18df2ed309ba8c1b Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Fri, 3 Apr 2020 15:27:16 +0300 Subject: [PATCH 06/23] Change upgrade runtime proposal - change upgrade runtime proposal in the codex module: add allowed proposers whitelist - add tests --- runtime-modules/proposals/codex/src/lib.rs | 11 +++++++++++ .../proposals/codex/src/tests/mock.rs | 2 ++ .../proposals/codex/src/tests/mod.rs | 17 +++++++++++++++++ runtime/src/lib.rs | 2 ++ 4 files changed, 32 insertions(+) diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index aba319b2f1..8f6f1cca34 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -81,6 +81,9 @@ pub trait Trait: /// Defines max wasm code length of the runtime upgrade proposal. type RuntimeUpgradeWasmProposalMaxLength: Get; + /// Defines allowed proposers (by member id list) for the runtime upgrade proposal. + type RuntimeUpgradeProposalAllowedProposers: Get>>; + /// Validates member id and origin combination type MembershipOriginValidator: ActorOriginValidator< Self::Origin, @@ -124,6 +127,9 @@ decl_error! { /// Provided WASM code for the runtime upgrade proposal is empty RuntimeProposalIsEmpty, + /// Runtime upgrade proposal can be created only by hardcoded members + RuntimeProposalProposerNotInTheAllowedProposersList, + /// Invalid balance value for the spending proposal SpendingProposalZeroBalance, @@ -229,6 +235,11 @@ decl_module! { ensure!(wasm.len() as u32 <= T::RuntimeUpgradeWasmProposalMaxLength::get(), Error::RuntimeProposalSizeExceeded); + ensure!( + T::RuntimeUpgradeProposalAllowedProposers::get().contains(&member_id), + Error::RuntimeProposalProposerNotInTheAllowedProposersList + ); + let wasm_hash = blake2_256(&wasm); let proposal_code = diff --git a/runtime-modules/proposals/codex/src/tests/mock.rs b/runtime-modules/proposals/codex/src/tests/mock.rs index c911208c6a..28e5a3da32 100644 --- a/runtime-modules/proposals/codex/src/tests/mock.rs +++ b/runtime-modules/proposals/codex/src/tests/mock.rs @@ -156,6 +156,7 @@ impl VotersParameters for MockVotersParameters { parameter_types! { pub const TextProposalMaxLength: u32 = 20_000; pub const RuntimeUpgradeWasmProposalMaxLength: u32 = 20_000; + pub const RuntimeUpgradeProposalAllowedProposers: Vec = vec![1]; } impl governance::election::Trait for Test { @@ -246,6 +247,7 @@ impl staking::SessionInterface for Test { impl crate::Trait for Test { type TextProposalMaxLength = TextProposalMaxLength; type RuntimeUpgradeWasmProposalMaxLength = RuntimeUpgradeWasmProposalMaxLength; + type RuntimeUpgradeProposalAllowedProposers = RuntimeUpgradeProposalAllowedProposers; type MembershipOriginValidator = (); } diff --git a/runtime-modules/proposals/codex/src/tests/mod.rs b/runtime-modules/proposals/codex/src/tests/mod.rs index 95b2e4a459..d148eaecb8 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -241,6 +241,23 @@ fn create_upgrade_runtime_proposal_codex_call_fails_with_incorrect_wasm_size() { }); } +#[test] +fn create_upgrade_runtime_proposal_codex_call_fails_with_not_allowed_member_id() { + initial_test_ext().execute_with(|| { + assert_eq!( + ProposalCodex::create_runtime_upgrade_proposal( + RawOrigin::Signed(1).into(), + 110, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(50000u32)), + b"wasm".to_vec(), + ), + Err(Error::RuntimeProposalProposerNotInTheAllowedProposersList) + ); + }); +} + #[test] fn create_set_election_parameters_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 8a9da4e4cc..1d6af4371e 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -860,12 +860,14 @@ impl proposals_discussion::Trait for Runtime { parameter_types! { pub const TextProposalMaxLength: u32 = 60_000; pub const RuntimeUpgradeWasmProposalMaxLength: u32 = 2_000_000; + pub const RuntimeUpgradeProposalAllowedProposers: Vec = Vec::new(); //TODO set allowed members } impl proposals_codex::Trait for Runtime { type MembershipOriginValidator = MembershipOriginValidator; type TextProposalMaxLength = TextProposalMaxLength; type RuntimeUpgradeWasmProposalMaxLength = RuntimeUpgradeWasmProposalMaxLength; + type RuntimeUpgradeProposalAllowedProposers = RuntimeUpgradeProposalAllowedProposers; } construct_runtime!( From ab54a829a40c3dc7810bdec8e383001135ca5f3e Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Fri, 3 Apr 2020 19:11:50 +0300 Subject: [PATCH 07/23] Add some comments to proposal modules --- runtime-modules/proposals/codex/src/lib.rs | 6 +- .../proposals/discussion/src/lib.rs | 91 +++++++++++++------ runtime-modules/proposals/engine/src/lib.rs | 1 + 3 files changed, 66 insertions(+), 32 deletions(-) diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index 8f6f1cca34..4aec583bf4 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -222,7 +222,8 @@ decl_module! { )?; } - /// Create 'Runtime upgrade' proposal type. + /// Create 'Runtime upgrade' proposal type. Runtime upgrade can be initiated only by + /// members from the hardcoded list `RuntimeUpgradeProposalAllowedProposers` pub fn create_runtime_upgrade_proposal( origin, member_id: MemberId, @@ -289,7 +290,6 @@ decl_module! { )?; } - /// Create 'Set council mint capacity' proposal type. This proposal uses `set_mint_capacity()` /// extrinsic from the `governance::council` module. pub fn create_set_council_mint_capacity_proposal( @@ -580,7 +580,7 @@ impl Module { stake_balance, )?; - >::ensure_can_create_thread(&title, member_id.clone())?; + >::ensure_can_create_thread(member_id.clone(), &title)?; let discussion_thread_id = >::create_thread(member_id, title.clone())?; diff --git a/runtime-modules/proposals/discussion/src/lib.rs b/runtime-modules/proposals/discussion/src/lib.rs index 7820307efc..904552e70a 100644 --- a/runtime-modules/proposals/discussion/src/lib.rs +++ b/runtime-modules/proposals/discussion/src/lib.rs @@ -1,13 +1,43 @@ -//! Proposals discussion module for the Joystream platform. Version 2. -//! Contains discussion subsystem for the proposals engine. +//! # Proposals discussion module +//! Proposals `discussion` module for the Joystream platform. Version 2. +//! Contains discussion subsystem of the proposals. //! -//! Supported extrinsics: -//! - add_post - adds a post to existing discussion thread -//! - update_post - updates existing post +//! ## Overview //! -//! Public API: -//! - create_discussion - creates a discussion -//! - ensure_can_create_thread - ensures safe thread creation +//! The proposals discussion module is used by the codex module to provide a platform for discussions +//! about different proposals. It allows to create discussion threads and then add and update related +//! posts. +//! +//! ## Supported extrinsics +//! - [add_post](./struct.Module.html#method.add_post) - adds a post to existing discussion thread +//! - [update_post](./struct.Module.html#method.update_post) - updates existing post +//! +//! ## Public API methods +//! - [create_thread](./struct.Module.html#method.create_thread) - creates a discussion thread +//! - [ensure_can_create_thread](./struct.Module.html#method.ensure_can_create_thread) - ensures safe thread creation +//! +//! ## Usage +//! +//! ``` +//! use srml_support::{decl_module, dispatch::Result}; +//! use system::ensure_root; +//! use substrate_proposals_discussion_module::{self as discussions}; +//! +//! pub trait Trait: discussions::Trait + membership::members::Trait {} +//! +//! decl_module! { +//! pub struct Module for enum Call where origin: T::Origin { +//! pub fn create_discussion(origin, title: Vec, author_id : T::MemberId) -> Result { +//! ensure_root(origin)?; +//! >::ensure_can_create_thread(author_id, &title)?; +//! >::create_thread(author_id, title)?; +//! Ok(()) +//! } +//! } +//! } +//! # fn main() { } +//! ``` + //! // Ensure we're `no_std` when compiling for Wasm. @@ -84,6 +114,7 @@ pub trait Trait: system::Trait + membership::members::Trait { } decl_error! { + /// Discussion module predefined errors pub enum Error { /// Author should match the post creator NotAuthor, @@ -242,18 +273,13 @@ decl_module! { } impl Module { - // Wrapper-function over system::block_number() - fn current_block() -> T::BlockNumber { - >::block_number() - } - /// Create the discussion thread. Cannot add more threads than 'predefined limit = MaxThreadInARowNumber' /// times in a row by the same author. pub fn create_thread( thread_author_id: MemberId, title: Vec, ) -> Result { - Self::ensure_can_create_thread(&title, thread_author_id.clone())?; + Self::ensure_can_create_thread(thread_author_id.clone(), &title)?; let next_thread_count_value = Self::thread_count() + 1; let new_thread_id = next_thread_count_value; @@ -278,27 +304,13 @@ impl Module { Ok(thread_id) } - // returns incremented thread counter if last thread author equals with provided parameter - fn get_updated_thread_counter(author_id: MemberId) -> ThreadCounter> { - // if thread counter exists - if let Some(last_thread_author_counter) = Self::last_thread_author_counter() { - // if last(previous) author is the same as current author - if last_thread_author_counter.author_id == author_id { - return last_thread_author_counter.increment(); - } - } - - // else return new counter (set with 1 thread number) - ThreadCounter::new(author_id) - } - /// Ensures thread can be created. /// Checks: /// - title is valid /// - max thread in a row by the same author pub fn ensure_can_create_thread( - title: &[u8], thread_author_id: MemberId, + title: &[u8], ) -> DispatchResult { ensure!(!title.is_empty(), Error::EmptyTitleProvided); ensure!( @@ -317,3 +329,24 @@ impl Module { Ok(()) } } + +impl Module { + // Wrapper-function over system::block_number() + fn current_block() -> T::BlockNumber { + >::block_number() + } + + // returns incremented thread counter if last thread author equals with provided parameter + fn get_updated_thread_counter(author_id: MemberId) -> ThreadCounter> { + // if thread counter exists + if let Some(last_thread_author_counter) = Self::last_thread_author_counter() { + // if last(previous) author is the same as current author + if last_thread_author_counter.author_id == author_id { + return last_thread_author_counter.increment(); + } + } + + // else return new counter (set with 1 thread number) + ThreadCounter::new(author_id) + } +} diff --git a/runtime-modules/proposals/engine/src/lib.rs b/runtime-modules/proposals/engine/src/lib.rs index e4fae3789e..c9ec38f1a0 100644 --- a/runtime-modules/proposals/engine/src/lib.rs +++ b/runtime-modules/proposals/engine/src/lib.rs @@ -128,6 +128,7 @@ decl_event!( ); decl_error! { + /// Engine module predefined errors pub enum Error { /// Proposal cannot have an empty title" EmptyTitleProvided, From c27a869e1268a6f4fb0970ddd17dde1fb3314050 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Mon, 6 Apr 2020 15:32:24 +0300 Subject: [PATCH 08/23] Set min validator count for the proposals --- runtime-modules/proposals/codex/src/lib.rs | 11 ++++++++ .../proposals/codex/src/tests/mock.rs | 2 ++ .../proposals/codex/src/tests/mod.rs | 27 +++++++++++++++---- runtime/src/lib.rs | 2 ++ 4 files changed, 37 insertions(+), 5 deletions(-) diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index 4aec583bf4..e6bd1e4935 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -75,6 +75,9 @@ pub trait Trait: + roles::actors::Trait + staking::Trait { + /// Defines min allowed validator count for the 'Set validator count' proposal. + type SetValidatorCountProposalMinValidators: Get; + /// Defines max allowed text proposal length. type TextProposalMaxLength: Get; @@ -133,6 +136,9 @@ decl_error! { /// Invalid balance value for the spending proposal SpendingProposalZeroBalance, + /// Invalid validator count for the 'set validator count' proposal + LessThanMinValidatorCount, + /// Require root origin in extrinsics RequireRootOrigin, } @@ -446,6 +452,11 @@ decl_module! { stake_balance: Option>, new_validator_count: u32, ) { + ensure!( + new_validator_count >= T::SetValidatorCountProposalMinValidators::get(), + Error::LessThanMinValidatorCount + ); + let proposal_code = >::set_validator_count(new_validator_count); diff --git a/runtime-modules/proposals/codex/src/tests/mock.rs b/runtime-modules/proposals/codex/src/tests/mock.rs index 28e5a3da32..67f7bc16f3 100644 --- a/runtime-modules/proposals/codex/src/tests/mock.rs +++ b/runtime-modules/proposals/codex/src/tests/mock.rs @@ -155,6 +155,7 @@ impl VotersParameters for MockVotersParameters { parameter_types! { pub const TextProposalMaxLength: u32 = 20_000; + pub const SetValidatorCountProposalMinValidators: u32 = 4; pub const RuntimeUpgradeWasmProposalMaxLength: u32 = 20_000; pub const RuntimeUpgradeProposalAllowedProposers: Vec = vec![1]; } @@ -245,6 +246,7 @@ impl staking::SessionInterface for Test { } impl crate::Trait for Test { + type SetValidatorCountProposalMinValidators = SetValidatorCountProposalMinValidators; type TextProposalMaxLength = TextProposalMaxLength; type RuntimeUpgradeWasmProposalMaxLength = RuntimeUpgradeWasmProposalMaxLength; type RuntimeUpgradeProposalAllowedProposers = RuntimeUpgradeProposalAllowedProposers; diff --git a/runtime-modules/proposals/codex/src/tests/mod.rs b/runtime-modules/proposals/codex/src/tests/mod.rs index d148eaecb8..a0ce277cf2 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -630,7 +630,7 @@ fn create_set_validator_count_proposal_common_checks_succeed() { b"title".to_vec(), b"body".to_vec(), None, - 1, + 4, ) }, empty_stake_call: || { @@ -640,7 +640,7 @@ fn create_set_validator_count_proposal_common_checks_succeed() { b"title".to_vec(), b"body".to_vec(), None, - 1, + 4, ) }, invalid_stake_call: || { @@ -650,7 +650,7 @@ fn create_set_validator_count_proposal_common_checks_succeed() { b"title".to_vec(), b"body".to_vec(), Some(>::from(5000u32)), - 1, + 4, ) }, successful_call: || { @@ -660,18 +660,35 @@ fn create_set_validator_count_proposal_common_checks_succeed() { b"title".to_vec(), b"body".to_vec(), Some(>::from(500u32)), - 1, + 4, ) }, proposal_parameters: crate::proposal_types::parameters::set_validator_count_proposal::< Test, >(), - proposal_details: ProposalDetails::SetValidatorCount(1), + proposal_details: ProposalDetails::SetValidatorCount(4), }; proposal_fixture.check_all(); }); } +#[test] +fn create_set_validator_count_proposal_failed_with_invalid_validator_count() { + initial_test_ext().execute_with(|| { + assert_eq!( + ProposalCodex::create_set_validator_count_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(500u32)), + 3, + ), + Err(Error::LessThanMinValidatorCount) + ); + }); +} + #[test] fn create_set_storage_role_parameters_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 1d6af4371e..e8172f2b16 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -859,6 +859,7 @@ impl proposals_discussion::Trait for Runtime { parameter_types! { pub const TextProposalMaxLength: u32 = 60_000; + pub const SetValidatorCountProposalMinValidators: u32 = 4; pub const RuntimeUpgradeWasmProposalMaxLength: u32 = 2_000_000; pub const RuntimeUpgradeProposalAllowedProposers: Vec = Vec::new(); //TODO set allowed members } @@ -868,6 +869,7 @@ impl proposals_codex::Trait for Runtime { type TextProposalMaxLength = TextProposalMaxLength; type RuntimeUpgradeWasmProposalMaxLength = RuntimeUpgradeWasmProposalMaxLength; type RuntimeUpgradeProposalAllowedProposers = RuntimeUpgradeProposalAllowedProposers; + type SetValidatorCountProposalMinValidators = SetValidatorCountProposalMinValidators; } construct_runtime!( From aa793af1d932e1cee9e289b24113d8ff4ef32ad5 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Mon, 6 Apr 2020 18:48:13 +0300 Subject: [PATCH 09/23] Add ensure_storage_role_parameters_valid() to the codex --- runtime-modules/proposals/codex/src/lib.rs | 68 ++++++++++++++++++ .../proposals/codex/src/tests/mod.rs | 71 +++++++++++++++++++ 2 files changed, 139 insertions(+) diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index e6bd1e4935..1bd4a26909 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -141,6 +141,30 @@ decl_error! { /// Require root origin in extrinsics RequireRootOrigin, + + /// Invalid storage role parameter - min_actors + InvalidStorageRoleParameterMinActors, + + /// Invalid storage role parameter - max_actors + InvalidStorageRoleParameterMaxActors, + + /// Invalid storage role parameter - reward_period + InvalidStorageRoleParameterRewardPeriod, + + /// Invalid storage role parameter - bonding_period + InvalidStorageRoleParameterBondingPeriod, + + /// Invalid storage role parameter - unbonding_period + InvalidStorageRoleParameterUnbondingPeriod, + + /// Invalid storage role parameter - min_service_period + InvalidStorageRoleParameterMinServicePeriod, + + /// Invalid storage role parameter - min_service_period + InvalidStorageRoleParameterMinServicePeriod, + + /// Invalid storage role parameter - startup_grace_period + InvalidStorageRoleParameterStartupGracePeriod, } } @@ -485,6 +509,8 @@ decl_module! { stake_balance: Option>, role_parameters: RoleParameters, T::BlockNumber> ) { + Self::ensure_storage_role_parameters_valid(&role_parameters)?; + let proposal_code = >::set_role_parameters( Role::StorageProvider, role_parameters.clone() @@ -611,4 +637,46 @@ impl Module { Ok(()) } + + // validates storage role parameters for the 'Set storage role parameters' proposal + fn ensure_storage_role_parameters_valid( + role_parameters: &RoleParameters, T::BlockNumber>, + ) -> Result<(), Error> { + ensure!( + role_parameters.min_actors > 0, + Error::InvalidStorageRoleParameterMinActors + ); + + ensure!( + role_parameters.max_actors > 0, + Error::InvalidStorageRoleParameterMaxActors + ); + + ensure!( + role_parameters.reward_period == T::BlockNumber::from(600), + Error::InvalidStorageRoleParameterRewardPeriod + ); + + ensure!( + role_parameters.bonding_period == T::BlockNumber::from(600), + Error::InvalidStorageRoleParameterBondingPeriod + ); + + ensure!( + role_parameters.unbonding_period == T::BlockNumber::from(600), + Error::InvalidStorageRoleParameterUnbondingPeriod + ); + + ensure!( + role_parameters.min_service_period == T::BlockNumber::from(600), + Error::InvalidStorageRoleParameterMinServicePeriod + ); + + ensure!( + role_parameters.startup_grace_period >= T::BlockNumber::from(600), + Error::InvalidStorageRoleParameterStartupGracePeriod + ); + + Ok(()) + } } diff --git a/runtime-modules/proposals/codex/src/tests/mod.rs b/runtime-modules/proposals/codex/src/tests/mod.rs index a0ce277cf2..c84292ea06 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -740,3 +740,74 @@ fn create_set_storage_role_parameters_proposal_common_checks_succeed() { proposal_fixture.check_all(); }); } + +fn assert_failed_set_storage_parameters_call( + role_parameters: RoleParameters, + error: Error, +) { + assert_eq!( + ProposalCodex::create_set_storage_role_parameters_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(500u32)), + role_parameters, + ), + Err(error) + ); +} + +#[test] +fn create_set_storage_role_parameters_proposal_fails_with_invalid_parameters() { + initial_test_ext().execute_with(|| { + let mut role_parameters = RoleParameters::default(); + role_parameters.min_actors = 0; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterMinActors, + ); + + role_parameters = RoleParameters::default(); + role_parameters.max_actors = 0; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterMaxActors, + ); + + role_parameters = RoleParameters::default(); + role_parameters.reward_period = 700; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterRewardPeriod, + ); + + role_parameters = RoleParameters::default(); + role_parameters.bonding_period = 700; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterBondingPeriod, + ); + + role_parameters = RoleParameters::default(); + role_parameters.unbonding_period = 700; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterUnbondingPeriod, + ); + + role_parameters = RoleParameters::default(); + role_parameters.min_service_period = 700; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterMinServicePeriod, + ); + + role_parameters = RoleParameters::default(); + role_parameters.startup_grace_period = 500; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterStartupGracePeriod, + ); + }); +} From 96720b4042201c8d8daa2ae025a760e1605bd008 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Tue, 7 Apr 2020 12:56:19 +0300 Subject: [PATCH 10/23] Add ensure_council_election_parameters_valid() to the codex --- runtime-modules/proposals/codex/src/lib.rs | 79 ++++++++++- .../proposals/codex/src/tests/mock.rs | 6 +- .../proposals/codex/src/tests/mod.rs | 131 +++++++++++++----- 3 files changed, 177 insertions(+), 39 deletions(-) diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index 1bd4a26909..f7d997b6eb 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -56,7 +56,7 @@ use rstd::prelude::*; use rstd::str::from_utf8; use rstd::vec::Vec; use runtime_io::blake2_256; -use sr_primitives::traits::Zero; +use sr_primitives::traits::{One, Zero}; use srml_support::dispatch::DispatchResult; use srml_support::traits::{Currency, Get}; use srml_support::{decl_error, decl_module, decl_storage, ensure, print}; @@ -160,11 +160,33 @@ decl_error! { /// Invalid storage role parameter - min_service_period InvalidStorageRoleParameterMinServicePeriod, - /// Invalid storage role parameter - min_service_period - InvalidStorageRoleParameterMinServicePeriod, - /// Invalid storage role parameter - startup_grace_period InvalidStorageRoleParameterStartupGracePeriod, + + /// Invalid council election parameter - council_size + InvalidCouncilElectionParameterCouncilSize, + + /// Invalid council election parameter - candidacy-limit + InvalidCouncilElectionParameterCandidacyLimit, + + /// Invalid council election parameter - min-voting_stake + InvalidCouncilElectionParameterMinVotingStake, + + /// Invalid council election parameter - new_term_duration + InvalidCouncilElectionParameterNewTermDuration, + + /// Invalid council election parameter - min_council_stake + InvalidCouncilElectionParameterMinCouncilStake, + + /// Invalid council election parameter - revealing_period + InvalidCouncilElectionParameterRevealingPeriod, + + /// Invalid council election parameter - voting_period + InvalidCouncilElectionParameterVotingPeriod, + + /// Invalid council election parameter - announcing_period + InvalidCouncilElectionParameterAnnouncingPeriod, + } } @@ -300,7 +322,7 @@ decl_module! { stake_balance: Option>, election_parameters: ElectionParameters, T::BlockNumber>, ) { - election_parameters.ensure_valid()?; + Self::ensure_council_election_parameters_valid(&election_parameters)?; let proposal_code = >::set_election_parameters(election_parameters.clone()); @@ -679,4 +701,51 @@ impl Module { Ok(()) } + + // validates council election parameters for the 'Set election parameters' proposal + fn ensure_council_election_parameters_valid( + election_parameters: &ElectionParameters, T::BlockNumber>, + ) -> Result<(), Error> { + ensure!( + election_parameters.council_size >= 3, + Error::InvalidCouncilElectionParameterCouncilSize + ); + + ensure!( + election_parameters.candidacy_limit >= 25, + Error::InvalidCouncilElectionParameterCandidacyLimit + ); + + ensure!( + election_parameters.min_voting_stake >= >::one(), + Error::InvalidCouncilElectionParameterMinVotingStake + ); + + ensure!( + election_parameters.new_term_duration >= T::BlockNumber::from(14400), + Error::InvalidCouncilElectionParameterNewTermDuration + ); + + ensure!( + election_parameters.revealing_period >= T::BlockNumber::from(14400), + Error::InvalidCouncilElectionParameterRevealingPeriod + ); + + ensure!( + election_parameters.voting_period >= T::BlockNumber::from(14400), + Error::InvalidCouncilElectionParameterVotingPeriod + ); + + ensure!( + election_parameters.announcing_period >= T::BlockNumber::from(14400), + Error::InvalidCouncilElectionParameterAnnouncingPeriod + ); + + ensure!( + election_parameters.min_council_stake >= >::one(), + Error::InvalidCouncilElectionParameterMinCouncilStake + ); + + Ok(()) + } } diff --git a/runtime-modules/proposals/codex/src/tests/mock.rs b/runtime-modules/proposals/codex/src/tests/mock.rs index 67f7bc16f3..866e19d918 100644 --- a/runtime-modules/proposals/codex/src/tests/mock.rs +++ b/runtime-modules/proposals/codex/src/tests/mock.rs @@ -123,8 +123,10 @@ impl governance::council::Trait for Test { } impl common::origin_validator::ActorOriginValidator for () { - fn ensure_actor_origin(_: Origin, _: u64) -> Result { - Ok(1) + fn ensure_actor_origin(origin: Origin, _: u64) -> Result { + let account_id = system::ensure_signed(origin)?; + + Ok(account_id) } } diff --git a/runtime-modules/proposals/codex/src/tests/mod.rs b/runtime-modules/proposals/codex/src/tests/mod.rs index c84292ea06..1a54966096 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -45,7 +45,10 @@ where } fn check_call_for_insufficient_rights(&self) { - assert!((self.insufficient_rights_call)().is_err()); + assert_eq!( + (self.insufficient_rights_call)(), + Err(Error::Other("RequireSignedOrigin")) + ); } fn check_for_successful_call(&self) { @@ -261,26 +264,15 @@ fn create_upgrade_runtime_proposal_codex_call_fails_with_not_allowed_member_id() #[test] fn create_set_election_parameters_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { - let election_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, - }; - let proposal_fixture = ProposalTestFixture { insufficient_rights_call: || { ProposalCodex::create_set_election_parameters_proposal( - RawOrigin::Signed(1).into(), + RawOrigin::None.into(), 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), - ElectionParameters::default(), + None, + get_valid_election_parameters(), ) }, empty_stake_call: || { @@ -290,7 +282,7 @@ fn create_set_election_parameters_proposal_common_checks_succeed() { b"title".to_vec(), b"body".to_vec(), None, - election_parameters.clone(), + get_valid_election_parameters(), ) }, invalid_stake_call: || { @@ -300,7 +292,7 @@ fn create_set_election_parameters_proposal_common_checks_succeed() { b"title".to_vec(), b"body".to_vec(), Some(>::from(50000u32)), - election_parameters.clone(), + get_valid_election_parameters(), ) }, successful_call: || { @@ -310,33 +302,108 @@ fn create_set_election_parameters_proposal_common_checks_succeed() { b"title".to_vec(), b"body".to_vec(), Some(>::from(500u32)), - election_parameters.clone(), + get_valid_election_parameters(), ) }, proposal_parameters: crate::proposal_types::parameters::set_election_parameters_proposal::(), - proposal_details: ProposalDetails::SetElectionParameters(election_parameters), + proposal_details: ProposalDetails::SetElectionParameters( + get_valid_election_parameters(), + ), }; proposal_fixture.check_all(); }); } + +fn assert_failed_election_parameters_call( + election_parameters: ElectionParameters, + error: Error, +) { + assert_eq!( + ProposalCodex::create_set_election_parameters_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(500u32)), + election_parameters, + ), + Err(error) + ); +} + +fn get_valid_election_parameters() -> ElectionParameters { + ElectionParameters { + announcing_period: 14400, + voting_period: 14400, + revealing_period: 14400, + council_size: 3, + candidacy_limit: 25, + new_term_duration: 14400, + min_council_stake: 1, + min_voting_stake: 1, + } +} + #[test] fn create_set_election_parameters_call_fails_with_incorrect_parameters() { initial_test_ext().execute_with(|| { - let account_id = 1; - let required_stake = Some(>::from(500u32)); - let _imbalance = ::Currency::deposit_creating(&account_id, 50000); + let _imbalance = ::Currency::deposit_creating(&1, 50000); - assert_eq!( - ProposalCodex::create_set_election_parameters_proposal( - RawOrigin::Signed(1).into(), - 1, - b"title".to_vec(), - b"body".to_vec(), - required_stake, - ElectionParameters::default(), - ), - Err(Error::Other("PeriodCannotBeZero")) + let mut election_parameters = get_valid_election_parameters(); + election_parameters.council_size = 2; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterCouncilSize, + ); + + election_parameters = get_valid_election_parameters(); + election_parameters.candidacy_limit = 22; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterCandidacyLimit, + ); + + election_parameters = get_valid_election_parameters(); + election_parameters.min_voting_stake = 0; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterMinVotingStake, + ); + + election_parameters = get_valid_election_parameters(); + election_parameters.new_term_duration = 10000; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterNewTermDuration, + ); + + election_parameters = get_valid_election_parameters(); + election_parameters.min_council_stake = 0; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterMinCouncilStake, + ); + + election_parameters = get_valid_election_parameters(); + election_parameters.voting_period = 10000; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterVotingPeriod, + ); + + election_parameters = get_valid_election_parameters(); + election_parameters.revealing_period = 10000; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterRevealingPeriod, + ); + + election_parameters = get_valid_election_parameters(); + election_parameters.announcing_period = 10000; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterAnnouncingPeriod, ); }); } From e8dbdcb502af4739ccc2503b8a57a3de52702313 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Tue, 7 Apr 2020 19:14:09 +0300 Subject: [PATCH 11/23] Change min_validator_count() limit in the codex --- runtime-modules/proposals/codex/src/lib.rs | 5 +---- runtime-modules/proposals/codex/src/tests/mock.rs | 1 - runtime/src/lib.rs | 1 - 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index f7d997b6eb..1d57d1e0d1 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -75,9 +75,6 @@ pub trait Trait: + roles::actors::Trait + staking::Trait { - /// Defines min allowed validator count for the 'Set validator count' proposal. - type SetValidatorCountProposalMinValidators: Get; - /// Defines max allowed text proposal length. type TextProposalMaxLength: Get; @@ -499,7 +496,7 @@ decl_module! { new_validator_count: u32, ) { ensure!( - new_validator_count >= T::SetValidatorCountProposalMinValidators::get(), + new_validator_count >= >::minimum_validator_count(), Error::LessThanMinValidatorCount ); diff --git a/runtime-modules/proposals/codex/src/tests/mock.rs b/runtime-modules/proposals/codex/src/tests/mock.rs index 866e19d918..38c0b36f9b 100644 --- a/runtime-modules/proposals/codex/src/tests/mock.rs +++ b/runtime-modules/proposals/codex/src/tests/mock.rs @@ -157,7 +157,6 @@ impl VotersParameters for MockVotersParameters { parameter_types! { pub const TextProposalMaxLength: u32 = 20_000; - pub const SetValidatorCountProposalMinValidators: u32 = 4; pub const RuntimeUpgradeWasmProposalMaxLength: u32 = 20_000; pub const RuntimeUpgradeProposalAllowedProposers: Vec = vec![1]; } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index e8172f2b16..173f21bc52 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -859,7 +859,6 @@ impl proposals_discussion::Trait for Runtime { parameter_types! { pub const TextProposalMaxLength: u32 = 60_000; - pub const SetValidatorCountProposalMinValidators: u32 = 4; pub const RuntimeUpgradeWasmProposalMaxLength: u32 = 2_000_000; pub const RuntimeUpgradeProposalAllowedProposers: Vec = Vec::new(); //TODO set allowed members } From b67925747fe99b7fc471d22d7725a291183a40da Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Wed, 8 Apr 2020 19:17:23 +0300 Subject: [PATCH 12/23] Add veto for the pending execution proposal. --- .../proposals/codex/src/tests/mock.rs | 1 - .../proposals/codex/src/tests/mod.rs | 5 + runtime-modules/proposals/engine/src/lib.rs | 30 ++++- .../proposals/engine/src/tests/mod.rs | 113 ++++++++++++++++++ runtime/src/lib.rs | 1 - 5 files changed, 145 insertions(+), 5 deletions(-) diff --git a/runtime-modules/proposals/codex/src/tests/mock.rs b/runtime-modules/proposals/codex/src/tests/mock.rs index 38c0b36f9b..d3a8de4f29 100644 --- a/runtime-modules/proposals/codex/src/tests/mock.rs +++ b/runtime-modules/proposals/codex/src/tests/mock.rs @@ -247,7 +247,6 @@ impl staking::SessionInterface for Test { } impl crate::Trait for Test { - type SetValidatorCountProposalMinValidators = SetValidatorCountProposalMinValidators; type TextProposalMaxLength = TextProposalMaxLength; type RuntimeUpgradeWasmProposalMaxLength = RuntimeUpgradeWasmProposalMaxLength; type RuntimeUpgradeProposalAllowedProposers = RuntimeUpgradeProposalAllowedProposers; diff --git a/runtime-modules/proposals/codex/src/tests/mod.rs b/runtime-modules/proposals/codex/src/tests/mod.rs index 1a54966096..f6e1381105 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -828,6 +828,11 @@ fn assert_failed_set_storage_parameters_call( #[test] fn create_set_storage_role_parameters_proposal_fails_with_invalid_parameters() { initial_test_ext().execute_with(|| { + // { + // let _imbalance = ::Currency::deposit_creating(&44, 50000); + // } + // assert_eq!(Balances::total_issuance(), 0); + let mut role_parameters = RoleParameters::default(); role_parameters.min_actors = 0; assert_failed_set_storage_parameters_call( diff --git a/runtime-modules/proposals/engine/src/lib.rs b/runtime-modules/proposals/engine/src/lib.rs index c9ec38f1a0..66cb908866 100644 --- a/runtime-modules/proposals/engine/src/lib.rs +++ b/runtime-modules/proposals/engine/src/lib.rs @@ -279,11 +279,14 @@ decl_module! { ensure!(>::exists(proposal_id), Error::ProposalNotFound); let proposal = Self::proposals(proposal_id); - ensure!(matches!(proposal.status, ProposalStatus::Active{..}), Error::ProposalFinalized); - // mutation - Self::finalize_proposal(proposal_id, ProposalDecisionStatus::Vetoed); + if >::exists(proposal_id) { + Self::veto_pending_execution_proposal(proposal_id, proposal); + } else { + ensure!(matches!(proposal.status, ProposalStatus::Active{..}), Error::ProposalFinalized); + Self::finalize_proposal(proposal_id, ProposalDecisionStatus::Vetoed); + } } /// Block finalization. Perform voting period check, vote result tally, approved proposals @@ -510,6 +513,27 @@ impl Module { .collect() // compose output vector } + // Veto approved proposal during its grace period. Saves a new proposal status and removes + // proposal id from the 'PendingExecutionProposalIds' + fn veto_pending_execution_proposal(proposal_id: T::ProposalId, proposal: ProposalOf) { + >::remove(proposal_id); + + let vetoed_proposal_status = ProposalStatus::finalized( + ProposalDecisionStatus::Vetoed, + None, + None, + Self::current_block(), + ); + + >::insert( + proposal_id, + Proposal { + status: vetoed_proposal_status, + ..proposal + }, + ); + } + // Executes approved proposal code fn execute_proposal(approved_proposal: ApprovedProposal) { let proposal_code = Self::proposal_codes(approved_proposal.proposal_id); diff --git a/runtime-modules/proposals/engine/src/tests/mod.rs b/runtime-modules/proposals/engine/src/tests/mod.rs index 48fca36675..de6c645943 100644 --- a/runtime-modules/proposals/engine/src/tests/mod.rs +++ b/runtime-modules/proposals/engine/src/tests/mod.rs @@ -819,6 +819,119 @@ fn proposal_execution_postponed_because_of_grace_period() { }); } +/* + +#[test] +fn veto_proposal_succeeds() { + initial_test_ext().execute_with(|| { + // internal active proposal counter check + assert_eq!(::get(), 0); + + let parameters_fixture = ProposalParametersFixture::default(); + let dummy_proposal = + DummyProposalFixture::default().with_parameters(parameters_fixture.params()); + let proposal_id = dummy_proposal.create_proposal_and_assert(Ok(1)).unwrap(); + + // internal active proposal counter check + assert_eq!(::get(), 1); + + let veto_proposal = VetoProposalFixture::new(proposal_id); + veto_proposal.veto_and_assert(Ok(())); + + let proposal = >::get(proposal_id); + + assert_eq!( + proposal, + Proposal { + parameters: parameters_fixture.params(), + proposer_id: 1, + created_at: 1, + status: ProposalStatus::finalized_successfully(ProposalDecisionStatus::Vetoed, 1), + title: b"title".to_vec(), + description: b"description".to_vec(), + voting_results: VotingResults::default(), + } + ); + + // internal active proposal counter check + assert_eq!(::get(), 0); + }); +} +*/ + +#[test] +fn proposal_execution_vetoed_successfully_during_the_grace_period() { + initial_test_ext().execute_with(|| { + let parameters_fixture = ProposalParametersFixture::default().with_grace_period(2); + let dummy_proposal = + DummyProposalFixture::default().with_parameters(parameters_fixture.params()); + + let proposal_id = dummy_proposal.create_proposal_and_assert(Ok(1)).unwrap(); + + let mut vote_generator = VoteGenerator::new(proposal_id); + vote_generator.vote_and_assert_ok(VoteKind::Approve); + vote_generator.vote_and_assert_ok(VoteKind::Approve); + vote_generator.vote_and_assert_ok(VoteKind::Approve); + vote_generator.vote_and_assert_ok(VoteKind::Approve); + + run_to_block_and_finalize(1); + run_to_block_and_finalize(2); + + // check internal cache for proposal_id presense + assert!(>::enumerate() + .find(|(x, _)| *x == proposal_id) + .is_some()); + + let proposal = >::get(proposal_id); + + assert_eq!( + proposal, + Proposal { + parameters: parameters_fixture.params(), + proposer_id: 1, + created_at: 1, + status: ProposalStatus::approved(ApprovedProposalStatus::PendingExecution, 1), + title: b"title".to_vec(), + description: b"description".to_vec(), + voting_results: VotingResults { + abstentions: 0, + approvals: 4, + rejections: 0, + slashes: 0, + }, + } + ); + + let veto_proposal = VetoProposalFixture::new(proposal_id); + veto_proposal.veto_and_assert(Ok(())); + + let proposal = >::get(proposal_id); + + assert_eq!( + proposal, + Proposal { + parameters: parameters_fixture.params(), + proposer_id: 1, + created_at: 1, + status: ProposalStatus::finalized_successfully(ProposalDecisionStatus::Vetoed, 2), + title: b"title".to_vec(), + description: b"description".to_vec(), + voting_results: VotingResults { + abstentions: 0, + approvals: 4, + rejections: 0, + slashes: 0, + }, + } + ); + + // check internal cache for proposal_id presense + assert!(>::enumerate() + .find(|(x, _)| *x == proposal_id) + .is_none()); + }); +} + #[test] fn proposal_execution_succeeds_after_the_grace_period() { initial_test_ext().execute_with(|| { diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 173f21bc52..1d6af4371e 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -868,7 +868,6 @@ impl proposals_codex::Trait for Runtime { type TextProposalMaxLength = TextProposalMaxLength; type RuntimeUpgradeWasmProposalMaxLength = RuntimeUpgradeWasmProposalMaxLength; type RuntimeUpgradeProposalAllowedProposers = RuntimeUpgradeProposalAllowedProposers; - type SetValidatorCountProposalMinValidators = SetValidatorCountProposalMinValidators; } construct_runtime!( From f55dcf6070c619f7f74f861a3fe68b708da68f39 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Thu, 9 Apr 2020 18:26:07 +0300 Subject: [PATCH 13/23] Set general proposal parameters. --- runtime-modules/proposals/codex/src/lib.rs | 3 + .../codex/src/proposal_types/parameters.rs | 197 +++++++++++------- .../proposals/codex/src/tests/mod.rs | 56 +++-- 3 files changed, 166 insertions(+), 90 deletions(-) diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index 1d57d1e0d1..2f49464614 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -96,6 +96,9 @@ pub trait Trait: pub type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; +/// Currency alias for `stake` module +pub type CurrencyOf = ::Currency; + /// Balance alias for GovernanceCurrency from `common` module. TODO: replace with BalanceOf pub type BalanceOfGovernanceCurrency = <::Currency as Currency< diff --git a/runtime-modules/proposals/codex/src/proposal_types/parameters.rs b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs index bcbf9a4e11..e72f956fd3 100644 --- a/runtime-modules/proposals/codex/src/proposal_types/parameters.rs +++ b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs @@ -1,29 +1,62 @@ -use crate::{BalanceOf, ProposalParameters}; +use crate::{BalanceOf, CurrencyOf, ProposalParameters}; +use rstd::convert::TryInto; +use sr_primitives::traits::SaturatedConversion; +pub use sr_primitives::Perbill; +use srml_support::traits::Currency; + +// calculates required stake value using total issuance value and stake percentage. Truncates to +// lowest integer value. +fn get_required_stake_by_fraction( + numerator: u32, + denominator: u32, +) -> BalanceOf { + let total_issuance: u128 = >::total_issuance().try_into().unwrap_or(0) as u128; + let required_stake = + Perbill::from_rational_approximation(numerator, denominator) * total_issuance; + + let balance: BalanceOf = required_stake.saturated_into(); + + balance +} + +// Proposal parameters for the 'Set validator count' proposal +pub(crate) fn set_validator_count_proposal( +) -> ProposalParameters> { + ProposalParameters { + voting_period: T::BlockNumber::from(43200u32), + grace_period: T::BlockNumber::from(0u32), + approval_quorum_percentage: 66, + approval_threshold_percentage: 80, + slashing_quorum_percentage: 60, + slashing_threshold_percentage: 80, + required_stake: Some(get_required_stake_by_fraction::(25, 10000)), + } +} // Proposal parameters for the upgrade runtime proposal pub(crate) fn runtime_upgrade_proposal( ) -> ProposalParameters> { ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), + voting_period: T::BlockNumber::from(72000u32), + grace_period: T::BlockNumber::from(72000u32), approval_quorum_percentage: 80, - approval_threshold_percentage: 80, - slashing_quorum_percentage: 80, + approval_threshold_percentage: 100, + slashing_quorum_percentage: 60, slashing_threshold_percentage: 80, - required_stake: Some(>::from(50000u32)), + required_stake: Some(get_required_stake_by_fraction::(1, 100)), } } // Proposal parameters for the text proposal pub(crate) fn text_proposal() -> ProposalParameters> { ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 80, - slashing_threshold_percentage: 82, - required_stake: Some(>::from(500u32)), + voting_period: T::BlockNumber::from(72000u32), + grace_period: T::BlockNumber::from(0u32), + approval_quorum_percentage: 66, + approval_threshold_percentage: 80, + slashing_quorum_percentage: 60, + slashing_threshold_percentage: 80, + required_stake: Some(get_required_stake_by_fraction::(25, 10000)), } } @@ -31,13 +64,13 @@ pub(crate) fn text_proposal() -> ProposalParameters( ) -> ProposalParameters> { ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 81, + voting_period: T::BlockNumber::from(72000u32), + grace_period: T::BlockNumber::from(201601u32), + approval_quorum_percentage: 66, + approval_threshold_percentage: 80, + slashing_quorum_percentage: 60, slashing_threshold_percentage: 80, - required_stake: Some(>::from(500u32)), + required_stake: Some(get_required_stake_by_fraction::(75, 10000)), } } @@ -45,13 +78,13 @@ pub(crate) fn set_election_parameters_proposal( pub(crate) fn set_council_mint_capacity_proposal( ) -> ProposalParameters> { ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 81, - slashing_threshold_percentage: 84, - required_stake: Some(>::from(500u32)), + voting_period: T::BlockNumber::from(43200u32), + grace_period: T::BlockNumber::from(0u32), + approval_quorum_percentage: 66, + approval_threshold_percentage: 80, + slashing_quorum_percentage: 50, + slashing_threshold_percentage: 50, + required_stake: Some(get_required_stake_by_fraction::(25, 10000)), } } @@ -59,13 +92,13 @@ pub(crate) fn set_council_mint_capacity_proposal( pub(crate) fn set_content_working_group_mint_capacity_proposal( ) -> ProposalParameters> { ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 81, - slashing_threshold_percentage: 85, - required_stake: Some(>::from(500u32)), + voting_period: T::BlockNumber::from(43200u32), + grace_period: T::BlockNumber::from(0u32), + approval_quorum_percentage: 50, + approval_threshold_percentage: 75, + slashing_quorum_percentage: 60, + slashing_threshold_percentage: 80, + required_stake: Some(get_required_stake_by_fraction::(25, 10000)), } } @@ -73,13 +106,13 @@ pub(crate) fn set_content_working_group_mint_capacity_proposal( pub(crate) fn spending_proposal( ) -> ProposalParameters> { ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 84, - slashing_threshold_percentage: 85, - required_stake: Some(>::from(500u32)), + voting_period: T::BlockNumber::from(43200u32), + grace_period: T::BlockNumber::from(0u32), + approval_quorum_percentage: 66, + approval_threshold_percentage: 80, + slashing_quorum_percentage: 60, + slashing_threshold_percentage: 80, + required_stake: Some(get_required_stake_by_fraction::(25, 10000)), } } @@ -87,13 +120,13 @@ pub(crate) fn spending_proposal( pub(crate) fn set_lead_proposal( ) -> ProposalParameters> { ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 81, - slashing_threshold_percentage: 86, - required_stake: Some(>::from(500u32)), + voting_period: T::BlockNumber::from(43200u32), + grace_period: T::BlockNumber::from(0u32), + approval_quorum_percentage: 66, + approval_threshold_percentage: 80, + slashing_quorum_percentage: 60, + slashing_threshold_percentage: 80, + required_stake: Some(get_required_stake_by_fraction::(25, 10000)), } } @@ -101,40 +134,56 @@ pub(crate) fn set_lead_proposal( pub(crate) fn evict_storage_provider_proposal( ) -> ProposalParameters> { ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 81, - slashing_threshold_percentage: 87, - required_stake: Some(>::from(500u32)), + voting_period: T::BlockNumber::from(43200u32), + grace_period: T::BlockNumber::from(0u32), + approval_quorum_percentage: 50, + approval_threshold_percentage: 75, + slashing_quorum_percentage: 60, + slashing_threshold_percentage: 80, + required_stake: Some(get_required_stake_by_fraction::(1, 1000)), } } -// Proposal parameters for the 'Set validator count' proposal -pub(crate) fn set_validator_count_proposal( +// Proposal parameters for the 'Set storage role parameters' proposal +pub(crate) fn set_storage_role_parameters_proposal( ) -> ProposalParameters> { ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 82, - slashing_threshold_percentage: 87, - required_stake: Some(>::from(500u32)), + voting_period: T::BlockNumber::from(43200u32), + grace_period: T::BlockNumber::from(14400u32), + approval_quorum_percentage: 75, + approval_threshold_percentage: 80, + slashing_quorum_percentage: 60, + slashing_threshold_percentage: 80, + required_stake: Some(get_required_stake_by_fraction::(25, 10000)), } } -// Proposal parameters for the 'Set storage role parameters' proposal -pub(crate) fn set_storage_role_parameters_proposal( -) -> ProposalParameters> { - ProposalParameters { - voting_period: T::BlockNumber::from(50000u32), - grace_period: T::BlockNumber::from(10000u32), - approval_quorum_percentage: 40, - approval_threshold_percentage: 51, - slashing_quorum_percentage: 82, - slashing_threshold_percentage: 88, - required_stake: Some(>::from(500u32)), +#[cfg(test)] +mod test { + use crate::proposal_types::parameters::get_required_stake_by_fraction; + use crate::tests::{increase_total_balance_issuance, initial_test_ext, Test}; + + pub use sr_primitives::Perbill; + + #[test] + fn calculate_get_required_stake_by_fraction_with_zero_issuance() { + initial_test_ext() + .execute_with(|| assert_eq!(get_required_stake_by_fraction::(5, 7), 0)); + } + + #[test] + fn calculate_stake_by_percentage_for_defined_issuance_succeeds() { + initial_test_ext().execute_with(|| { + increase_total_balance_issuance(50000); + assert_eq!(get_required_stake_by_fraction::(1, 1000), 50) + }); + } + + #[test] + fn calculate_stake_by_percentage_for_defined_issuance_with_fraction_loss() { + initial_test_ext().execute_with(|| { + increase_total_balance_issuance(1111); + assert_eq!(get_required_stake_by_fraction::(3, 1000), 3); + }); } } diff --git a/runtime-modules/proposals/codex/src/tests/mod.rs b/runtime-modules/proposals/codex/src/tests/mod.rs index f6e1381105..c6f868c9c5 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -6,12 +6,21 @@ use srml_support::StorageMap; use system::RawOrigin; use crate::{BalanceOf, Error, ProposalDetails}; -use mock::*; use proposal_engine::ProposalParameters; use roles::actors::RoleParameters; use runtime_io::blake2_256; use srml_support::dispatch::DispatchResult; +pub use mock::*; + +pub(crate) fn increase_total_balance_issuance(balance: u64) { + let initial_balance = Balances::total_issuance(); + { + let _ = ::Currency::deposit_creating(&999, balance); + } + assert_eq!(Balances::total_issuance(), initial_balance + balance); +} + struct ProposalTestFixture where InsufficientRightsCall: Fn() -> DispatchResult, @@ -81,6 +90,8 @@ where #[test] fn create_text_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { + increase_total_balance_issuance(500000); + let proposal_fixture = ProposalTestFixture { insufficient_rights_call: || { ProposalCodex::create_text_proposal( @@ -118,7 +129,7 @@ fn create_text_proposal_common_checks_succeed() { 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), + Some(>::from(1250u32)), b"text".to_vec(), ) }, @@ -164,6 +175,8 @@ fn create_text_proposal_codex_call_fails_with_incorrect_text_size() { #[test] fn create_runtime_upgrade_common_checks_succeed() { initial_test_ext().execute_with(|| { + increase_total_balance_issuance(500000); + let proposal_fixture = ProposalTestFixture { insufficient_rights_call: || { ProposalCodex::create_runtime_upgrade_proposal( @@ -201,7 +214,7 @@ fn create_runtime_upgrade_common_checks_succeed() { 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(50000u32)), + Some(>::from(5000u32)), b"wasm".to_vec(), ) }, @@ -264,6 +277,8 @@ fn create_upgrade_runtime_proposal_codex_call_fails_with_not_allowed_member_id() #[test] fn create_set_election_parameters_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { + increase_total_balance_issuance(500000); + let proposal_fixture = ProposalTestFixture { insufficient_rights_call: || { ProposalCodex::create_set_election_parameters_proposal( @@ -301,7 +316,7 @@ fn create_set_election_parameters_proposal_common_checks_succeed() { 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), + Some(>::from(3750u32)), get_valid_election_parameters(), ) }, @@ -411,6 +426,8 @@ fn create_set_election_parameters_call_fails_with_incorrect_parameters() { #[test] fn create_set_council_mint_capacity_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { + increase_total_balance_issuance(500000); + let proposal_fixture = ProposalTestFixture { insufficient_rights_call: || { ProposalCodex::create_set_council_mint_capacity_proposal( @@ -438,7 +455,7 @@ fn create_set_council_mint_capacity_proposal_common_checks_succeed() { 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(5000u32)), + Some(>::from(150u32)), 0, ) }, @@ -448,7 +465,7 @@ fn create_set_council_mint_capacity_proposal_common_checks_succeed() { 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), + Some(>::from(1250u32)), 10, ) }, @@ -463,6 +480,8 @@ fn create_set_council_mint_capacity_proposal_common_checks_succeed() { #[test] fn create_set_content_working_group_mint_capacity_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { + increase_total_balance_issuance(500000); + let proposal_fixture = ProposalTestFixture { insufficient_rights_call: || { ProposalCodex::create_set_content_working_group_mint_capacity_proposal( @@ -500,7 +519,7 @@ fn create_set_content_working_group_mint_capacity_proposal_common_checks_succeed 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), + Some(>::from(1250u32)), 10, ) }, @@ -514,6 +533,8 @@ fn create_set_content_working_group_mint_capacity_proposal_common_checks_succeed #[test] fn create_spending_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { + increase_total_balance_issuance(500000); + let proposal_fixture = ProposalTestFixture { insufficient_rights_call: || { ProposalCodex::create_spending_proposal( @@ -554,7 +575,7 @@ fn create_spending_proposal_common_checks_succeed() { 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), + Some(>::from(1250u32)), 100, 2, ) @@ -587,6 +608,8 @@ fn create_spending_proposal_call_fails_with_incorrect_balance() { #[test] fn create_set_lead_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { + increase_total_balance_issuance(500000); + let proposal_fixture = ProposalTestFixture { insufficient_rights_call: || { ProposalCodex::create_set_lead_proposal( @@ -624,7 +647,7 @@ fn create_set_lead_proposal_common_checks_succeed() { 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), + Some(>::from(1250u32)), Some((20, 10)), ) }, @@ -638,6 +661,8 @@ fn create_set_lead_proposal_common_checks_succeed() { #[test] fn create_evict_storage_provider_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { + increase_total_balance_issuance(500000); + let proposal_fixture = ProposalTestFixture { insufficient_rights_call: || { ProposalCodex::create_evict_storage_provider_proposal( @@ -689,6 +714,8 @@ fn create_evict_storage_provider_proposal_common_checks_succeed() { #[test] fn create_set_validator_count_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { + increase_total_balance_issuance(500000); + let proposal_fixture = ProposalTestFixture { insufficient_rights_call: || { ProposalCodex::create_set_validator_count_proposal( @@ -726,7 +753,7 @@ fn create_set_validator_count_proposal_common_checks_succeed() { 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), + Some(>::from(1250u32)), 4, ) }, @@ -759,6 +786,8 @@ fn create_set_validator_count_proposal_failed_with_invalid_validator_count() { #[test] fn create_set_storage_role_parameters_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { + increase_total_balance_issuance(500000); + let proposal_fixture = ProposalTestFixture { insufficient_rights_call: || { ProposalCodex::create_set_storage_role_parameters_proposal( @@ -796,7 +825,7 @@ fn create_set_storage_role_parameters_proposal_common_checks_succeed() { 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), + Some(>::from(1250u32)), RoleParameters::default(), ) }, @@ -828,11 +857,6 @@ fn assert_failed_set_storage_parameters_call( #[test] fn create_set_storage_role_parameters_proposal_fails_with_invalid_parameters() { initial_test_ext().execute_with(|| { - // { - // let _imbalance = ::Currency::deposit_creating(&44, 50000); - // } - // assert_eq!(Balances::total_issuance(), 0); - let mut role_parameters = RoleParameters::default(); role_parameters.min_actors = 0; assert_failed_set_storage_parameters_call( From 1884aa0beb73a12b57551b4e8a7d6ff8b403e3f4 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Thu, 9 Apr 2020 18:47:25 +0300 Subject: [PATCH 14/23] Update proposals runtime parameters --- runtime/src/lib.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 1d6af4371e..d9dd2957f6 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -814,9 +814,9 @@ impl discovery::Trait for Runtime { parameter_types! { pub const ProposalCancellationFee: u64 = 5; pub const ProposalRejectionFee: u64 = 3; - pub const ProposalTitleMaxLength: u32 = 100; - pub const ProposalDescriptionMaxLength: u32 = 10000; - pub const ProposalMaxActiveProposalLimit: u32 = 100; + pub const ProposalTitleMaxLength: u32 = 40; + pub const ProposalDescriptionMaxLength: u32 = 3000; + pub const ProposalMaxActiveProposalLimit: u32 = 5; } impl proposals_engine::Trait for Runtime { @@ -840,10 +840,10 @@ impl Default for Call { } parameter_types! { - pub const ProposalMaxPostEditionNumber: u32 = 5; - pub const ProposalMaxThreadInARowNumber: u32 = 3; - pub const ProposalThreadTitleLengthLimit: u32 = 200; - pub const ProposalPostLengthLimit: u32 = 2000; + pub const ProposalMaxPostEditionNumber: u32 = 0; // post update is disabled + pub const ProposalMaxThreadInARowNumber: u32 = 100000; // will not be used + pub const ProposalThreadTitleLengthLimit: u32 = 40; + pub const ProposalPostLengthLimit: u32 = 1000; } impl proposals_discussion::Trait for Runtime { @@ -858,7 +858,7 @@ impl proposals_discussion::Trait for Runtime { } parameter_types! { - pub const TextProposalMaxLength: u32 = 60_000; + pub const TextProposalMaxLength: u32 = 5_000; pub const RuntimeUpgradeWasmProposalMaxLength: u32 = 2_000_000; pub const RuntimeUpgradeProposalAllowedProposers: Vec = Vec::new(); //TODO set allowed members } From dda832e0c5d628d7e33a31d838ff033641fd69e3 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Fri, 10 Apr 2020 14:39:13 +0300 Subject: [PATCH 15/23] Set proposal parameters - set proposal limits and introduce errors - update spending proposal parameters - introduce total issuance token percentage --- runtime-modules/proposals/codex/src/lib.rs | 233 +++++++++++++++- .../codex/src/proposal_types/parameters.rs | 25 +- .../proposals/codex/src/tests/mod.rs | 263 +++++++++++++++++- 3 files changed, 473 insertions(+), 48 deletions(-) diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index 2f49464614..6ac6006916 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -52,11 +52,14 @@ use governance::election_params::ElectionParameters; use proposal_engine::ProposalParameters; use roles::actors::{Role, RoleParameters}; use rstd::clone::Clone; +use rstd::convert::TryInto; use rstd::prelude::*; use rstd::str::from_utf8; use rstd::vec::Vec; use runtime_io::blake2_256; +use sr_primitives::traits::SaturatedConversion; use sr_primitives::traits::{One, Zero}; +pub use sr_primitives::Perbill; use srml_support::dispatch::DispatchResult; use srml_support::traits::{Currency, Get}; use srml_support::{decl_error, decl_module, decl_storage, ensure, print}; @@ -64,6 +67,10 @@ use system::{ensure_root, RawOrigin}; pub use proposal_types::ProposalDetails; +// Percentage of the total token issue as max mint balance value. Shared with spending +// proposal max balance percentage. +const COUNCIL_MINT_MAX_BALANCE_PERCENT: u32 = 2; + /// 'Proposals codex' substrate module Trait pub trait Trait: system::Trait @@ -134,10 +141,10 @@ decl_error! { RuntimeProposalProposerNotInTheAllowedProposersList, /// Invalid balance value for the spending proposal - SpendingProposalZeroBalance, + InvalidSpendingProposalBalance, /// Invalid validator count for the 'set validator count' proposal - LessThanMinValidatorCount, + InvalidValidatorCount, /// Require root origin in extrinsics RequireRootOrigin, @@ -187,6 +194,23 @@ decl_error! { /// Invalid council election parameter - announcing_period InvalidCouncilElectionParameterAnnouncingPeriod, + /// Invalid council election parameter - min_stake + InvalidStorageRoleParameterMinStake, + + /// Invalid council election parameter - reward + InvalidStorageRoleParameterReward, + + /// Invalid council election parameter - entry_request_fee + InvalidStorageRoleParameterEntryRequestFee, + + /// Invalid working group mint capacity parameter + InvalidStorageWorkingGroupMintCapacity, + + /// Invalid council mint capacity parameter + InvalidStorageCouncilMintCapacity, + + /// Invalid 'set lead proposal' parameter - proposed lead cannot be a councilor + InvalidSetLeadParameterCannotBeCouncilor } } @@ -322,6 +346,8 @@ decl_module! { stake_balance: Option>, election_parameters: ElectionParameters, T::BlockNumber>, ) { + election_parameters.ensure_valid()?; + Self::ensure_council_election_parameters_valid(&election_parameters)?; let proposal_code = @@ -352,6 +378,19 @@ decl_module! { stake_balance: Option>, mint_balance: BalanceOfMint, ) { + + let max_mint_capacity: u32 = get_required_stake_by_fraction::( + COUNCIL_MINT_MAX_BALANCE_PERCENT, + 100 + ) + .try_into() + .unwrap_or_default() as u32; + + ensure!( + mint_balance < >::from(max_mint_capacity), + Error::InvalidStorageCouncilMintCapacity + ); + let proposal_code = >::set_council_mint_capacity(mint_balance.clone()); @@ -380,6 +419,15 @@ decl_module! { stake_balance: Option>, mint_balance: BalanceOfMint, ) { + + let max_mint_capacity: u32 = get_required_stake_by_fraction::(1, 100) + .try_into() + .unwrap_or_default() as u32; + ensure!( + mint_balance < >::from(max_mint_capacity), + Error::InvalidStorageWorkingGroupMintCapacity + ); + let proposal_code = >::set_mint_capacity(mint_balance.clone()); @@ -409,7 +457,19 @@ decl_module! { balance: BalanceOfMint, destination: T::AccountId, ) { - ensure!(balance != BalanceOfMint::::zero(), Error::SpendingProposalZeroBalance); + ensure!(balance != BalanceOfMint::::zero(), Error::InvalidSpendingProposalBalance); + + let max_balance: u32 = get_required_stake_by_fraction::( + COUNCIL_MINT_MAX_BALANCE_PERCENT, + 100 + ) + .try_into() + .unwrap_or_default() as u32; + + ensure!( + balance < >::from(max_balance), + Error::InvalidSpendingProposalBalance + ); let proposal_code = >::spend_from_council_mint( balance.clone(), @@ -442,6 +502,14 @@ decl_module! { stake_balance: Option>, new_lead: Option<(T::MemberId, T::AccountId)> ) { + if let Some(lead) = new_lead.clone() { + let account_id = lead.1; + ensure!( + !>::is_councilor(&account_id), + Error::InvalidSetLeadParameterCannotBeCouncilor + ); + } + let proposal_code = >::replace_lead(new_lead.clone()); @@ -500,7 +568,12 @@ decl_module! { ) { ensure!( new_validator_count >= >::minimum_validator_count(), - Error::LessThanMinValidatorCount + Error::InvalidValidatorCount + ); + + ensure!( + new_validator_count <= 1000, // max validator count + Error::InvalidValidatorCount ); let proposal_code = @@ -665,32 +738,57 @@ impl Module { role_parameters: &RoleParameters, T::BlockNumber>, ) -> Result<(), Error> { ensure!( - role_parameters.min_actors > 0, + role_parameters.min_actors <= 5, Error::InvalidStorageRoleParameterMinActors ); ensure!( - role_parameters.max_actors > 0, + role_parameters.max_actors >= 5, + Error::InvalidStorageRoleParameterMaxActors + ); + + ensure!( + role_parameters.max_actors < 100, Error::InvalidStorageRoleParameterMaxActors ); ensure!( - role_parameters.reward_period == T::BlockNumber::from(600), + role_parameters.reward_period >= T::BlockNumber::from(600), Error::InvalidStorageRoleParameterRewardPeriod ); ensure!( - role_parameters.bonding_period == T::BlockNumber::from(600), + role_parameters.reward_period <= T::BlockNumber::from(3600), + Error::InvalidStorageRoleParameterRewardPeriod + ); + + ensure!( + role_parameters.bonding_period >= T::BlockNumber::from(600), + Error::InvalidStorageRoleParameterBondingPeriod + ); + + ensure!( + role_parameters.bonding_period <= T::BlockNumber::from(28800), Error::InvalidStorageRoleParameterBondingPeriod ); ensure!( - role_parameters.unbonding_period == T::BlockNumber::from(600), + role_parameters.unbonding_period >= T::BlockNumber::from(600), + Error::InvalidStorageRoleParameterUnbondingPeriod + ); + + ensure!( + role_parameters.unbonding_period <= T::BlockNumber::from(28800), Error::InvalidStorageRoleParameterUnbondingPeriod ); ensure!( - role_parameters.min_service_period == T::BlockNumber::from(600), + role_parameters.min_service_period >= T::BlockNumber::from(600), + Error::InvalidStorageRoleParameterMinServicePeriod + ); + + ensure!( + role_parameters.min_service_period <= T::BlockNumber::from(28800), Error::InvalidStorageRoleParameterMinServicePeriod ); @@ -699,15 +797,74 @@ impl Module { Error::InvalidStorageRoleParameterStartupGracePeriod ); + ensure!( + role_parameters.startup_grace_period <= T::BlockNumber::from(28800), + Error::InvalidStorageRoleParameterStartupGracePeriod + ); + + ensure!( + role_parameters.min_stake > >::from(0u32), + Error::InvalidStorageRoleParameterMinStake + ); + + let max_min_stake: u32 = get_required_stake_by_fraction::(1, 100) + .try_into() + .unwrap_or_default() as u32; + + ensure!( + role_parameters.min_stake < >::from(max_min_stake), + Error::InvalidStorageRoleParameterMinStake + ); + + ensure!( + role_parameters.entry_request_fee > >::from(0u32), + Error::InvalidStorageRoleParameterEntryRequestFee + ); + + let max_entry_request_fee: u32 = get_required_stake_by_fraction::(1, 100) + .try_into() + .unwrap_or_default() as u32; + + ensure!( + role_parameters.entry_request_fee + < >::from(max_entry_request_fee), + Error::InvalidStorageRoleParameterEntryRequestFee + ); + + ensure!( + role_parameters.reward > >::from(0u32), + Error::InvalidStorageRoleParameterReward + ); + + let max_reward: u32 = get_required_stake_by_fraction::(1, 1000) + .try_into() + .unwrap_or_default() as u32; + + ensure!( + role_parameters.reward < >::from(max_reward), + Error::InvalidStorageRoleParameterReward + ); + Ok(()) } + /* + entry_request_fee [tJOY] >0 <1% NA + * Not enforced by runtime. Should not be displayed in the UI, or at least grayed out. + ** Should not be displayed in the UI, or at least grayed out. + */ + // validates council election parameters for the 'Set election parameters' proposal - fn ensure_council_election_parameters_valid( + pub(crate) fn ensure_council_election_parameters_valid( election_parameters: &ElectionParameters, T::BlockNumber>, ) -> Result<(), Error> { ensure!( - election_parameters.council_size >= 3, + election_parameters.council_size >= 4, + Error::InvalidCouncilElectionParameterCouncilSize + ); + + ensure!( + election_parameters.council_size <= 20, Error::InvalidCouncilElectionParameterCouncilSize ); @@ -716,36 +873,88 @@ impl Module { Error::InvalidCouncilElectionParameterCandidacyLimit ); + ensure!( + election_parameters.candidacy_limit <= 100, + Error::InvalidCouncilElectionParameterCandidacyLimit + ); + ensure!( election_parameters.min_voting_stake >= >::one(), Error::InvalidCouncilElectionParameterMinVotingStake ); + ensure!( + election_parameters.min_voting_stake + <= >::from(100000u32), + Error::InvalidCouncilElectionParameterMinVotingStake + ); + ensure!( election_parameters.new_term_duration >= T::BlockNumber::from(14400), Error::InvalidCouncilElectionParameterNewTermDuration ); + ensure!( + election_parameters.new_term_duration <= T::BlockNumber::from(432000), + Error::InvalidCouncilElectionParameterNewTermDuration + ); + ensure!( election_parameters.revealing_period >= T::BlockNumber::from(14400), Error::InvalidCouncilElectionParameterRevealingPeriod ); + ensure!( + election_parameters.revealing_period <= T::BlockNumber::from(43200), + Error::InvalidCouncilElectionParameterRevealingPeriod + ); + ensure!( election_parameters.voting_period >= T::BlockNumber::from(14400), Error::InvalidCouncilElectionParameterVotingPeriod ); + ensure!( + election_parameters.voting_period <= T::BlockNumber::from(43200), + Error::InvalidCouncilElectionParameterVotingPeriod + ); + ensure!( election_parameters.announcing_period >= T::BlockNumber::from(14400), Error::InvalidCouncilElectionParameterAnnouncingPeriod ); + ensure!( + election_parameters.announcing_period <= T::BlockNumber::from(43200), + Error::InvalidCouncilElectionParameterAnnouncingPeriod + ); + ensure!( election_parameters.min_council_stake >= >::one(), Error::InvalidCouncilElectionParameterMinCouncilStake ); + ensure!( + election_parameters.min_council_stake + <= >::from(100000u32), + Error::InvalidCouncilElectionParameterMinCouncilStake + ); + Ok(()) } } + +// calculates required stake value using total issuance value and stake percentage. Truncates to +// lowest integer value. Value fraction is defined by numerator and denominator. +pub(crate) fn get_required_stake_by_fraction( + numerator: u32, + denominator: u32, +) -> BalanceOf { + let total_issuance: u128 = >::total_issuance().try_into().unwrap_or(0) as u128; + let required_stake = + Perbill::from_rational_approximation(numerator, denominator) * total_issuance; + + let balance: BalanceOf = required_stake.saturated_into(); + + balance +} diff --git a/runtime-modules/proposals/codex/src/proposal_types/parameters.rs b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs index e72f956fd3..28dcb35c9c 100644 --- a/runtime-modules/proposals/codex/src/proposal_types/parameters.rs +++ b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs @@ -1,23 +1,4 @@ -use crate::{BalanceOf, CurrencyOf, ProposalParameters}; -use rstd::convert::TryInto; -use sr_primitives::traits::SaturatedConversion; -pub use sr_primitives::Perbill; -use srml_support::traits::Currency; - -// calculates required stake value using total issuance value and stake percentage. Truncates to -// lowest integer value. -fn get_required_stake_by_fraction( - numerator: u32, - denominator: u32, -) -> BalanceOf { - let total_issuance: u128 = >::total_issuance().try_into().unwrap_or(0) as u128; - let required_stake = - Perbill::from_rational_approximation(numerator, denominator) * total_issuance; - - let balance: BalanceOf = required_stake.saturated_into(); - - balance -} +use crate::{get_required_stake_by_fraction, BalanceOf, ProposalParameters}; // Proposal parameters for the 'Set validator count' proposal pub(crate) fn set_validator_count_proposal( @@ -106,8 +87,8 @@ pub(crate) fn set_content_working_group_mint_capacity_proposal( pub(crate) fn spending_proposal( ) -> ProposalParameters> { ProposalParameters { - voting_period: T::BlockNumber::from(43200u32), - grace_period: T::BlockNumber::from(0u32), + voting_period: T::BlockNumber::from(72000u32), + grace_period: T::BlockNumber::from(14400u32), approval_quorum_percentage: 66, approval_threshold_percentage: 80, slashing_quorum_percentage: 60, diff --git a/runtime-modules/proposals/codex/src/tests/mod.rs b/runtime-modules/proposals/codex/src/tests/mod.rs index c6f868c9c5..61ef9ce629 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -14,9 +14,13 @@ use srml_support::dispatch::DispatchResult; pub use mock::*; pub(crate) fn increase_total_balance_issuance(balance: u64) { + increase_total_balance_issuance_using_account_id(balance, 999); +} + +pub(crate) fn increase_total_balance_issuance_using_account_id(balance: u64, account_id: u64) { let initial_balance = Balances::total_issuance(); { - let _ = ::Currency::deposit_creating(&999, balance); + let _ = ::Currency::deposit_creating(&account_id, balance); } assert_eq!(Balances::total_issuance(), initial_balance + balance); } @@ -340,7 +344,7 @@ fn assert_failed_election_parameters_call( 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), + Some(>::from(3750u32)), election_parameters, ), Err(error) @@ -352,7 +356,7 @@ fn get_valid_election_parameters() -> ElectionParameters { announcing_period: 14400, voting_period: 14400, revealing_period: 14400, - council_size: 3, + council_size: 4, candidacy_limit: 25, new_term_duration: 14400, min_council_stake: 1, @@ -363,7 +367,7 @@ fn get_valid_election_parameters() -> ElectionParameters { #[test] fn create_set_election_parameters_call_fails_with_incorrect_parameters() { initial_test_ext().execute_with(|| { - let _imbalance = ::Currency::deposit_creating(&1, 50000); + increase_total_balance_issuance_using_account_id(500000, 1); let mut election_parameters = get_valid_election_parameters(); election_parameters.council_size = 2; @@ -372,6 +376,12 @@ fn create_set_election_parameters_call_fails_with_incorrect_parameters() { Error::InvalidCouncilElectionParameterCouncilSize, ); + election_parameters.council_size = 21; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterCouncilSize, + ); + election_parameters = get_valid_election_parameters(); election_parameters.candidacy_limit = 22; assert_failed_election_parameters_call( @@ -379,6 +389,13 @@ fn create_set_election_parameters_call_fails_with_incorrect_parameters() { Error::InvalidCouncilElectionParameterCandidacyLimit, ); + election_parameters = get_valid_election_parameters(); + election_parameters.candidacy_limit = 122; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterCandidacyLimit, + ); + election_parameters = get_valid_election_parameters(); election_parameters.min_voting_stake = 0; assert_failed_election_parameters_call( @@ -386,6 +403,13 @@ fn create_set_election_parameters_call_fails_with_incorrect_parameters() { Error::InvalidCouncilElectionParameterMinVotingStake, ); + election_parameters = get_valid_election_parameters(); + election_parameters.min_voting_stake = 200000; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterMinVotingStake, + ); + election_parameters = get_valid_election_parameters(); election_parameters.new_term_duration = 10000; assert_failed_election_parameters_call( @@ -393,6 +417,13 @@ fn create_set_election_parameters_call_fails_with_incorrect_parameters() { Error::InvalidCouncilElectionParameterNewTermDuration, ); + election_parameters = get_valid_election_parameters(); + election_parameters.new_term_duration = 500000; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterNewTermDuration, + ); + election_parameters = get_valid_election_parameters(); election_parameters.min_council_stake = 0; assert_failed_election_parameters_call( @@ -400,6 +431,13 @@ fn create_set_election_parameters_call_fails_with_incorrect_parameters() { Error::InvalidCouncilElectionParameterMinCouncilStake, ); + election_parameters = get_valid_election_parameters(); + election_parameters.min_council_stake = 200000; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterMinCouncilStake, + ); + election_parameters = get_valid_election_parameters(); election_parameters.voting_period = 10000; assert_failed_election_parameters_call( @@ -407,6 +445,13 @@ fn create_set_election_parameters_call_fails_with_incorrect_parameters() { Error::InvalidCouncilElectionParameterVotingPeriod, ); + election_parameters = get_valid_election_parameters(); + election_parameters.voting_period = 50000; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterVotingPeriod, + ); + election_parameters = get_valid_election_parameters(); election_parameters.revealing_period = 10000; assert_failed_election_parameters_call( @@ -414,12 +459,45 @@ fn create_set_election_parameters_call_fails_with_incorrect_parameters() { Error::InvalidCouncilElectionParameterRevealingPeriod, ); + election_parameters = get_valid_election_parameters(); + election_parameters.revealing_period = 50000; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterRevealingPeriod, + ); + election_parameters = get_valid_election_parameters(); election_parameters.announcing_period = 10000; assert_failed_election_parameters_call( election_parameters, Error::InvalidCouncilElectionParameterAnnouncingPeriod, ); + + election_parameters = get_valid_election_parameters(); + election_parameters.announcing_period = 50000; + assert_failed_election_parameters_call( + election_parameters, + Error::InvalidCouncilElectionParameterAnnouncingPeriod, + ); + }); +} + +#[test] +fn create_set_council_mint_capacity_proposal_fails_with_invalid_parameters() { + initial_test_ext().execute_with(|| { + increase_total_balance_issuance(500000); + + assert_eq!( + ProposalCodex::create_set_council_mint_capacity_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(1250u32)), + 10001, + ), + Err(Error::InvalidStorageCouncilMintCapacity) + ); }); } @@ -477,6 +555,25 @@ fn create_set_council_mint_capacity_proposal_common_checks_succeed() { }); } +#[test] +fn create_working_groupd_mint_capacity_proposal_fails_with_invalid_parameters() { + initial_test_ext().execute_with(|| { + increase_total_balance_issuance(500000); + + assert_eq!( + ProposalCodex::create_set_content_working_group_mint_capacity_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(1250u32)), + 5001, + ), + Err(Error::InvalidStorageWorkingGroupMintCapacity) + ); + }); +} + #[test] fn create_set_content_working_group_mint_capacity_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { @@ -590,17 +687,58 @@ fn create_spending_proposal_common_checks_succeed() { #[test] fn create_spending_proposal_call_fails_with_incorrect_balance() { initial_test_ext().execute_with(|| { + increase_total_balance_issuance_using_account_id(1, 500000); + assert_eq!( ProposalCodex::create_spending_proposal( RawOrigin::Signed(1).into(), 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), + Some(>::from(1250u32)), 0, 2, ), - Err(Error::SpendingProposalZeroBalance) + Err(Error::InvalidSpendingProposalBalance) + ); + + assert_eq!( + ProposalCodex::create_spending_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(1250u32)), + 1001, + 2, + ), + Err(Error::InvalidSpendingProposalBalance) + ); + }); +} + +#[test] +fn create_set_lead_proposal_fails_with_proposed_councilor() { + initial_test_ext().execute_with(|| { + increase_total_balance_issuance_using_account_id(500000, 1); + + let lead_account_id = 20; + >::set_council( + RawOrigin::Root.into(), + vec![lead_account_id], + ) + .unwrap(); + + assert_eq!( + ProposalCodex::create_set_lead_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(1250u32)), + Some((20, lead_account_id)), + ), + Err(Error::InvalidSetLeadParameterCannotBeCouncilor) ); }); } @@ -778,7 +916,19 @@ fn create_set_validator_count_proposal_failed_with_invalid_validator_count() { Some(>::from(500u32)), 3, ), - Err(Error::LessThanMinValidatorCount) + Err(Error::InvalidValidatorCount) + ); + + assert_eq!( + ProposalCodex::create_set_validator_count_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(1001u32)), + 3, + ), + Err(Error::InvalidValidatorCount) ); }); } @@ -857,53 +1007,138 @@ fn assert_failed_set_storage_parameters_call( #[test] fn create_set_storage_role_parameters_proposal_fails_with_invalid_parameters() { initial_test_ext().execute_with(|| { + increase_total_balance_issuance(500000); + let mut role_parameters = RoleParameters::default(); - role_parameters.min_actors = 0; + role_parameters.min_actors = 6; assert_failed_set_storage_parameters_call( role_parameters, Error::InvalidStorageRoleParameterMinActors, ); role_parameters = RoleParameters::default(); - role_parameters.max_actors = 0; + role_parameters.max_actors = 4; assert_failed_set_storage_parameters_call( role_parameters, Error::InvalidStorageRoleParameterMaxActors, ); role_parameters = RoleParameters::default(); - role_parameters.reward_period = 700; + role_parameters.max_actors = 100; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterMaxActors, + ); + + role_parameters = RoleParameters::default(); + role_parameters.reward_period = 599; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterRewardPeriod, + ); + + role_parameters.reward_period = 28801; assert_failed_set_storage_parameters_call( role_parameters, Error::InvalidStorageRoleParameterRewardPeriod, ); role_parameters = RoleParameters::default(); - role_parameters.bonding_period = 700; + role_parameters.bonding_period = 599; assert_failed_set_storage_parameters_call( role_parameters, Error::InvalidStorageRoleParameterBondingPeriod, ); role_parameters = RoleParameters::default(); - role_parameters.unbonding_period = 700; + role_parameters.bonding_period = 28801; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterBondingPeriod, + ); + + role_parameters = RoleParameters::default(); + role_parameters.unbonding_period = 599; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterUnbondingPeriod, + ); + + role_parameters = RoleParameters::default(); + role_parameters.unbonding_period = 28801; assert_failed_set_storage_parameters_call( role_parameters, Error::InvalidStorageRoleParameterUnbondingPeriod, ); role_parameters = RoleParameters::default(); - role_parameters.min_service_period = 700; + role_parameters.min_service_period = 599; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterMinServicePeriod, + ); + + role_parameters = RoleParameters::default(); + role_parameters.min_service_period = 28801; assert_failed_set_storage_parameters_call( role_parameters, Error::InvalidStorageRoleParameterMinServicePeriod, ); role_parameters = RoleParameters::default(); - role_parameters.startup_grace_period = 500; + role_parameters.startup_grace_period = 599; assert_failed_set_storage_parameters_call( role_parameters, Error::InvalidStorageRoleParameterStartupGracePeriod, ); + + role_parameters = RoleParameters::default(); + role_parameters.startup_grace_period = 28801; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterStartupGracePeriod, + ); + + role_parameters = RoleParameters::default(); + role_parameters.min_stake = 0; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterMinStake, + ); + + role_parameters = RoleParameters::default(); + role_parameters.min_stake = 5001; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterMinStake, + ); + + role_parameters = RoleParameters::default(); + role_parameters.entry_request_fee = 0; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterEntryRequestFee, + ); + + role_parameters = RoleParameters::default(); + role_parameters.entry_request_fee = 5001; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterEntryRequestFee, + ); + + role_parameters = RoleParameters::default(); + role_parameters.reward = 0; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterReward, + ); + + role_parameters = RoleParameters::default(); + role_parameters.reward = 501; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterReward, + ); }); } From 61a8127f9c51e66ea21086aba6546a8dadec48fa Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Mon, 13 Apr 2020 10:56:05 +0300 Subject: [PATCH 16/23] Fix clippy comments --- .../proposals/discussion/src/lib.rs | 31 +++++++++---------- runtime-modules/proposals/engine/src/lib.rs | 10 +++--- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/runtime-modules/proposals/discussion/src/lib.rs b/runtime-modules/proposals/discussion/src/lib.rs index 904552e70a..c1ed9a5baf 100644 --- a/runtime-modules/proposals/discussion/src/lib.rs +++ b/runtime-modules/proposals/discussion/src/lib.rs @@ -26,16 +26,15 @@ //! pub trait Trait: discussions::Trait + membership::members::Trait {} //! //! decl_module! { -//! pub struct Module for enum Call where origin: T::Origin { -//! pub fn create_discussion(origin, title: Vec, author_id : T::MemberId) -> Result { -//! ensure_root(origin)?; -//! >::ensure_can_create_thread(author_id, &title)?; -//! >::create_thread(author_id, title)?; -//! Ok(()) -//! } -//! } +//! pub struct Module for enum Call where origin: T::Origin { +//! pub fn create_discussion(origin, title: Vec, author_id : T::MemberId) -> Result { +//! ensure_root(origin)?; +//! >::ensure_can_create_thread(author_id, &title)?; +//! >::create_thread(author_id, title)?; +//! Ok(()) +//! } +//! } //! } -//! # fn main() { } //! ``` //! @@ -199,7 +198,7 @@ decl_module! { ) { T::PostAuthorOriginValidator::ensure_actor_origin( origin, - post_author_id.clone(), + post_author_id, )?; ensure!(>::exists(thread_id), Error::ThreadDoesntExist); @@ -218,7 +217,7 @@ decl_module! { text, created_at: Self::current_block(), updated_at: Self::current_block(), - author_id: post_author_id.clone(), + author_id: post_author_id, edition_number : 0, thread_id, }; @@ -239,7 +238,7 @@ decl_module! { ){ T::PostAuthorOriginValidator::ensure_actor_origin( origin, - post_author_id.clone(), + post_author_id, )?; ensure!(>::exists(thread_id), Error::ThreadDoesntExist); @@ -279,7 +278,7 @@ impl Module { thread_author_id: MemberId, title: Vec, ) -> Result { - Self::ensure_can_create_thread(thread_author_id.clone(), &title)?; + Self::ensure_can_create_thread(thread_author_id, &title)?; let next_thread_count_value = Self::thread_count() + 1; let new_thread_id = next_thread_count_value; @@ -287,11 +286,11 @@ impl Module { let new_thread = Thread { title, created_at: Self::current_block(), - author_id: thread_author_id.clone(), + author_id: thread_author_id, }; // get new 'threads in a row' counter for the author - let current_thread_counter = Self::get_updated_thread_counter(thread_author_id.clone()); + let current_thread_counter = Self::get_updated_thread_counter(thread_author_id); // mutation @@ -319,7 +318,7 @@ impl Module { ); // get new 'threads in a row' counter for the author - let current_thread_counter = Self::get_updated_thread_counter(thread_author_id.clone()); + let current_thread_counter = Self::get_updated_thread_counter(thread_author_id); ensure!( current_thread_counter.counter as u32 <= T::MaxThreadInARowNumber::get(), diff --git a/runtime-modules/proposals/engine/src/lib.rs b/runtime-modules/proposals/engine/src/lib.rs index 66cb908866..25aa18ee7d 100644 --- a/runtime-modules/proposals/engine/src/lib.rs +++ b/runtime-modules/proposals/engine/src/lib.rs @@ -230,7 +230,7 @@ decl_module! { pub fn vote(origin, voter_id: MemberId, proposal_id: T::ProposalId, vote: VoteKind) { T::VoterOriginValidator::ensure_actor_origin( origin, - voter_id.clone(), + voter_id, )?; ensure!(>::exists(proposal_id), Error::ProposalNotFound); @@ -240,7 +240,7 @@ decl_module! { let did_not_vote_before = !>::exists( proposal_id, - voter_id.clone(), + voter_id, ); ensure!(did_not_vote_before, Error::AlreadyVoted); @@ -250,7 +250,7 @@ decl_module! { // mutation >::insert(proposal_id, proposal); - >::insert( proposal_id, voter_id.clone(), vote.clone()); + >::insert( proposal_id, voter_id, vote.clone()); Self::deposit_event(RawEvent::Voted(voter_id, proposal_id, vote)); } @@ -258,7 +258,7 @@ decl_module! { pub fn cancel_proposal(origin, proposer_id: MemberId, proposal_id: T::ProposalId) { T::ProposerOriginValidator::ensure_actor_origin( origin, - proposer_id.clone(), + proposer_id, )?; ensure!(>::exists(proposal_id), Error::ProposalNotFound); @@ -362,7 +362,7 @@ impl Module { parameters, title, description, - proposer_id: proposer_id.clone(), + proposer_id, status: ProposalStatus::Active(stake_data), voting_results: VotingResults::default(), }; From a9bbf47b9f34236e4372af6a9b5174cb9a70670d Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Mon, 13 Apr 2020 14:57:41 +0300 Subject: [PATCH 17/23] Add comments --- .../proposals/discussion/src/lib.rs | 1 + runtime-modules/proposals/engine/src/lib.rs | 98 ++++++++++++++++--- .../proposals/engine/src/types/mod.rs | 2 +- 3 files changed, 87 insertions(+), 14 deletions(-) diff --git a/runtime-modules/proposals/discussion/src/lib.rs b/runtime-modules/proposals/discussion/src/lib.rs index c1ed9a5baf..1726de8c22 100644 --- a/runtime-modules/proposals/discussion/src/lib.rs +++ b/runtime-modules/proposals/discussion/src/lib.rs @@ -35,6 +35,7 @@ //! } //! } //! } +//! # fn main() {} //! ``` //! diff --git a/runtime-modules/proposals/engine/src/lib.rs b/runtime-modules/proposals/engine/src/lib.rs index 25aa18ee7d..77b148837e 100644 --- a/runtime-modules/proposals/engine/src/lib.rs +++ b/runtime-modules/proposals/engine/src/lib.rs @@ -1,18 +1,90 @@ -//! Proposals engine module for the Joystream platform. Version 2. -//! Provides methods and extrinsics to create and vote for proposals, -//! inspired by Parity **Democracy module**. +//! # Proposals engine module +//! Proposals `engine` module for the Joystream platform. Version 2. +//! Main component of the proposals system. Provides methods and extrinsics to create and +//! vote for proposals, inspired by Parity **Democracy module**. //! -//! Supported extrinsics: -//! - vote - registers a vote for the proposal -//! - cancel_proposal - cancels the proposal (can be canceled only by owner) -//! - veto_proposal - vetoes the proposal +//! ## Overview +//! Proposals `engine` module provides abstract mechanism to work with proposals: creation, voting, +//! execution, canceling, etc. Proposal execution demands serialized _Dispatchable_ proposal code. +//! It could be any _Dispatchable_ + _Parameter_ type, but most likely it would be serialized (via +//! Parity _codec_ crate) extrisic call. Proposal stage can be described by its [status](./enum.ProposalStatus.html). //! -//! Public API: -//! - create_proposal - creates proposal using provided parameters -//! - ensure_create_proposal_parameters_are_valid - ensures that we can create the proposal -//! - refund_proposal_stake - a callback for StakingHandlerEvents -//! - reset_active_proposals - resets voting results for active proposals +//! ## Proposal lifecycle +//! When a proposal passes [checks](./struct.Module.html#method.ensure_create_proposal_parameters_are_valid) +//! for its [parameters](./struct.ProposalParameters.html) - it can be [created](./struct.Module.html#method.create_proposal). +//! Newly created proposal has _Active_ status. The proposal can be voted on or canceled during its +//! _voting period_. Votes can be [different](./enum.VoteKind.html). When proposal gets enough votes +//! to be slashed or approved or _voting period_ ends - the proposal becomes _Finalized_. If the proposal +//! got approved and _grace period_ passed - the `engine` module tries to execute the proposal. +//! The final [approved status](./enum.ApprovedProposalStatus.html) of the proposal defines +//! an overall proposal outcome. The proposal can also be [vetoed](./struct.Module.html#method.veto_proposal) +//! anytime before the proposal execution by the _sudo_. //! +//! ### Important abstract types to be implemented +//! Proposals `engine` module has several abstractions to be implemented in order to work correctly. +//! - _VoterOriginValidator_ - ensure valid voter identity. Voters should have permissions to vote: +//! they should be council members. +//! - [VotersParameters](./trait.VotersParameters.html) - defines total voter number, which is +//! the council size +//! - _ProposerOriginValidator_ - ensure valid proposer identity. Proposers should have permissions +//! to create a proposal: they should be members of the Joystream. +//! - [StakeHandlerProvider](./trait.StakeHandlerProvider.html) - defines interface for the staking. +//! +//! Full list of the abstractions can be found [here](./trait.Trait.html). +//! +//! ### Supported extrinsics +//! - [vote](./struct.Module.html#method.vote) - registers a vote for the proposal +//! - [cancel_proposal](./struct.Module.html#method.cancel_proposal) - cancels the proposal (can be canceled only by owner) +//! - [veto_proposal](./struct.Module.html#method.veto_proposal) - vetoes the proposal +//! +//! ### Public API +//! - [create_proposal](./struct.Module.html#method.create_proposal) - creates proposal using provided parameters +//! - [ensure_create_proposal_parameters_are_valid](./struct.Module.html#method.ensure_create_proposal_parameters_are_valid) - ensures that we can create the proposal +//! - [refund_proposal_stake](./struct.Module.html#method.refund_proposal_stake) - a callback for _StakingHandlerEvents_ +//! - [reset_active_proposals](./trait.Module.html#method.reset_active_proposals) - resets voting results for active proposals +//! +//! ## Usage +//! +//! ``` +//! use srml_support::{decl_module, dispatch::Result}; +//! use system::ensure_signed; +//! use substrate_proposals_engine_module::{self as engine, ProposalParameters}; +//! +//! pub trait Trait: engine::Trait + membership::members::Trait {} +//! +//! decl_module! { +//! pub struct Module for enum Call where origin: T::Origin { +//! pub fn create_spending_proposal( +//! origin, +//! proposer_id: T::MemberId, +//! ) -> Result { +//! let account_id = ensure_signed(origin)?; +//! let parameters = ProposalParameters::default(); +//! let title = b"Spending proposal".to_vec(); +//! let description = b"We need to spend some tokens to support the working group lead.".to_vec(); +//! let encoded_proposal_code = Vec::new(); +//! +//! >::ensure_create_proposal_parameters_are_valid( +//! ¶meters, +//! &title, +//! &description, +//! None +//! )?; +//! >::create_proposal( +//! account_id, +//! proposer_id, +//! parameters, +//! title, +//! description, +//! None, +//! encoded_proposal_code +//! )?; +//! Ok(()) +//! } +//! } +//! } +//! # fn main() {} +//! ``` // Ensure we're `no_std` when compiling for Wasm. #![cfg_attr(not(feature = "std"), no_std)] @@ -468,7 +540,7 @@ impl Module { } /// Resets voting results for active proposals. - /// Possible application - after the new council elections. + /// Possible application includes new council elections. pub fn reset_active_proposals() { >::enumerate().for_each(|(proposal_id, _)| { >::mutate(proposal_id, |proposal| { diff --git a/runtime-modules/proposals/engine/src/types/mod.rs b/runtime-modules/proposals/engine/src/types/mod.rs index 4ced301e21..a496fec53e 100644 --- a/runtime-modules/proposals/engine/src/types/mod.rs +++ b/runtime-modules/proposals/engine/src/types/mod.rs @@ -222,7 +222,7 @@ where } } -/// Provides data for voting. +/// Provides data for the voting. pub trait VotersParameters { /// Defines maximum voters count for the proposal fn total_voters_count() -> u32; From 74e1fe2edb84c26087239ce89439c1e53d5d8e30 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Mon, 13 Apr 2020 18:00:22 +0300 Subject: [PATCH 18/23] Update comments --- runtime-modules/proposals/codex/src/lib.rs | 6 +-- .../proposals/discussion/src/lib.rs | 4 +- runtime-modules/proposals/engine/src/lib.rs | 41 +++++++++++++------ 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index 6ac6006916..de61e5ce59 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -1,12 +1,12 @@ //! # Proposals codex module //! Proposals `codex` module for the Joystream platform. Version 2. -//! Component of the proposals system. Contains preset proposal types. +//! Component of the proposals system. It contains preset proposal types. //! //! ## Overview //! -//! The proposals codex module serves as facade and entry point of the proposals system. It uses +//! The proposals codex module serves as a facade and entry point of the proposals system. It uses //! proposals `engine` module to maintain a lifecycle of the proposal and to execute proposals. -//! During the proposal creation `codex` also create a discussion thread using the `discussion` +//! During the proposal creation, `codex` also create a discussion thread using the `discussion` //! proposals module. `Codex` uses predefined parameters (eg.:`voting_period`) for each proposal and //! encodes extrinsic calls from dependency modules in order to create proposals inside the `engine` //! module. For each proposal, [its crucial details](./enum.ProposalDetails.html) are saved to the diff --git a/runtime-modules/proposals/discussion/src/lib.rs b/runtime-modules/proposals/discussion/src/lib.rs index 1726de8c22..19ff49ba0d 100644 --- a/runtime-modules/proposals/discussion/src/lib.rs +++ b/runtime-modules/proposals/discussion/src/lib.rs @@ -1,6 +1,6 @@ //! # Proposals discussion module //! Proposals `discussion` module for the Joystream platform. Version 2. -//! Contains discussion subsystem of the proposals. +//! It contains discussion subsystem of the proposals. //! //! ## Overview //! @@ -9,7 +9,7 @@ //! posts. //! //! ## Supported extrinsics -//! - [add_post](./struct.Module.html#method.add_post) - adds a post to existing discussion thread +//! - [add_post](./struct.Module.html#method.add_post) - adds a post to an existing discussion thread //! - [update_post](./struct.Module.html#method.update_post) - updates existing post //! //! ## Public API methods diff --git a/runtime-modules/proposals/engine/src/lib.rs b/runtime-modules/proposals/engine/src/lib.rs index 77b148837e..28e4badd53 100644 --- a/runtime-modules/proposals/engine/src/lib.rs +++ b/runtime-modules/proposals/engine/src/lib.rs @@ -1,24 +1,35 @@ //! # Proposals engine module //! Proposals `engine` module for the Joystream platform. Version 2. -//! Main component of the proposals system. Provides methods and extrinsics to create and +//! The main component of the proposals system. Provides methods and extrinsics to create and //! vote for proposals, inspired by Parity **Democracy module**. //! //! ## Overview -//! Proposals `engine` module provides abstract mechanism to work with proposals: creation, voting, +//! Proposals `engine` module provides an abstract mechanism to work with proposals: creation, voting, //! execution, canceling, etc. Proposal execution demands serialized _Dispatchable_ proposal code. -//! It could be any _Dispatchable_ + _Parameter_ type, but most likely it would be serialized (via -//! Parity _codec_ crate) extrisic call. Proposal stage can be described by its [status](./enum.ProposalStatus.html). +//! It could be any _Dispatchable_ + _Parameter_ type, but most likely, it would be serialized (via +//! Parity _codec_ crate) extrisic call. A proposal stage can be described by its [status](./enum.ProposalStatus.html). //! //! ## Proposal lifecycle //! When a proposal passes [checks](./struct.Module.html#method.ensure_create_proposal_parameters_are_valid) //! for its [parameters](./struct.ProposalParameters.html) - it can be [created](./struct.Module.html#method.create_proposal). -//! Newly created proposal has _Active_ status. The proposal can be voted on or canceled during its -//! _voting period_. Votes can be [different](./enum.VoteKind.html). When proposal gets enough votes +//! The newly created proposal has _Active_ status. The proposal can be voted on or canceled during its +//! _voting period_. Votes can be [different](./enum.VoteKind.html). When the proposal gets enough votes //! to be slashed or approved or _voting period_ ends - the proposal becomes _Finalized_. If the proposal //! got approved and _grace period_ passed - the `engine` module tries to execute the proposal. //! The final [approved status](./enum.ApprovedProposalStatus.html) of the proposal defines -//! an overall proposal outcome. The proposal can also be [vetoed](./struct.Module.html#method.veto_proposal) +//! an overall proposal outcome. +//! +//! ### Notes +//! +//! - The proposal can be [vetoed](./struct.Module.html#method.veto_proposal) //! anytime before the proposal execution by the _sudo_. +//! - When the proposal is created with some stake - refunding on proposal finalization with +//! different statuses should be accomplished from the external handler from the _stake module_ +//! (_StakingEventsHandler_). Such a handler should call +//! [refund_proposal_stake](./struct.Module.html#method.refund_proposal_stake) callback function. +//! - If the _council_ got reelected during the proposal _voting period_ the external handler calls +//! [reset_active_proposals](./trait.Module.html#method.reset_active_proposals) function and +//! all voting results get cleared. //! //! ### Important abstract types to be implemented //! Proposals `engine` module has several abstractions to be implemented in order to work correctly. @@ -28,9 +39,9 @@ //! the council size //! - _ProposerOriginValidator_ - ensure valid proposer identity. Proposers should have permissions //! to create a proposal: they should be members of the Joystream. -//! - [StakeHandlerProvider](./trait.StakeHandlerProvider.html) - defines interface for the staking. +//! - [StakeHandlerProvider](./trait.StakeHandlerProvider.html) - defines an interface for the staking. //! -//! Full list of the abstractions can be found [here](./trait.Trait.html). +//! A full list of the abstractions can be found [here](./trait.Trait.html). //! //! ### Supported extrinsics //! - [vote](./struct.Module.html#method.vote) - registers a vote for the proposal @@ -46,14 +57,19 @@ //! ## Usage //! //! ``` -//! use srml_support::{decl_module, dispatch::Result}; +//! use srml_support::{decl_module, dispatch::Result, print}; //! use system::ensure_signed; +//! use codec::Encode; //! use substrate_proposals_engine_module::{self as engine, ProposalParameters}; //! //! pub trait Trait: engine::Trait + membership::members::Trait {} //! //! decl_module! { //! pub struct Module for enum Call where origin: T::Origin { +//! fn executable_proposal(origin) { +//! print("executed!"); +//! } +//! //! pub fn create_spending_proposal( //! origin, //! proposer_id: T::MemberId, @@ -61,8 +77,9 @@ //! let account_id = ensure_signed(origin)?; //! let parameters = ProposalParameters::default(); //! let title = b"Spending proposal".to_vec(); -//! let description = b"We need to spend some tokens to support the working group lead.".to_vec(); -//! let encoded_proposal_code = Vec::new(); +//! let description = b"We need to spend some tokens to support the working group lead." +//! .to_vec(); +//! let encoded_proposal_code = >::executable_proposal().encode(); //! //! >::ensure_create_proposal_parameters_are_valid( //! ¶meters, From 00872d6bd16d7663535697cd2c82142489bd752e Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Wed, 15 Apr 2020 11:49:49 +0300 Subject: [PATCH 19/23] Add missing tests --- .../proposals/codex/src/tests/mod.rs | 10 +- .../proposals/discussion/src/tests/mod.rs | 20 ++- .../proposals/discussion/src/types.rs | 29 +++ runtime-modules/proposals/engine/src/lib.rs | 5 +- .../proposals/engine/src/tests/mod.rs | 167 +++++++++++++----- .../engine/src/types/proposal_statuses.rs | 47 +++++ 6 files changed, 230 insertions(+), 48 deletions(-) diff --git a/runtime-modules/proposals/codex/src/tests/mod.rs b/runtime-modules/proposals/codex/src/tests/mod.rs index 61ef9ce629..6912c029fc 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -14,10 +14,10 @@ use srml_support::dispatch::DispatchResult; pub use mock::*; pub(crate) fn increase_total_balance_issuance(balance: u64) { - increase_total_balance_issuance_using_account_id(balance, 999); + increase_total_balance_issuance_using_account_id(999, balance); } -pub(crate) fn increase_total_balance_issuance_using_account_id(balance: u64, account_id: u64) { +pub(crate) fn increase_total_balance_issuance_using_account_id(account_id: u64, balance: u64) { let initial_balance = Balances::total_issuance(); { let _ = ::Currency::deposit_creating(&account_id, balance); @@ -367,7 +367,7 @@ fn get_valid_election_parameters() -> ElectionParameters { #[test] fn create_set_election_parameters_call_fails_with_incorrect_parameters() { initial_test_ext().execute_with(|| { - increase_total_balance_issuance_using_account_id(500000, 1); + increase_total_balance_issuance_using_account_id(1, 500000); let mut election_parameters = get_valid_election_parameters(); election_parameters.council_size = 2; @@ -687,7 +687,7 @@ fn create_spending_proposal_common_checks_succeed() { #[test] fn create_spending_proposal_call_fails_with_incorrect_balance() { initial_test_ext().execute_with(|| { - increase_total_balance_issuance_using_account_id(1, 500000); + increase_total_balance_issuance_using_account_id(500000, 1); assert_eq!( ProposalCodex::create_spending_proposal( @@ -720,7 +720,7 @@ fn create_spending_proposal_call_fails_with_incorrect_balance() { #[test] fn create_set_lead_proposal_fails_with_proposed_councilor() { initial_test_ext().execute_with(|| { - increase_total_balance_issuance_using_account_id(500000, 1); + increase_total_balance_issuance_using_account_id(1, 500000); let lead_account_id = 20; >::set_council( diff --git a/runtime-modules/proposals/discussion/src/tests/mod.rs b/runtime-modules/proposals/discussion/src/tests/mod.rs index a2f51c458a..ab5edeb7f8 100644 --- a/runtime-modules/proposals/discussion/src/tests/mod.rs +++ b/runtime-modules/proposals/discussion/src/tests/mod.rs @@ -214,7 +214,7 @@ fn update_post_call_succeeds() { } #[test] -fn update_post_call_failes_because_of_post_edition_limit() { +fn update_post_call_fails_because_of_post_edition_limit() { initial_test_ext().execute_with(|| { let discussion_fixture = DiscussionFixture::default(); @@ -235,7 +235,7 @@ fn update_post_call_failes_because_of_post_edition_limit() { } #[test] -fn update_post_call_failes_because_of_the_wrong_author() { +fn update_post_call_fails_because_of_the_wrong_author() { initial_test_ext().execute_with(|| { let discussion_fixture = DiscussionFixture::default(); @@ -399,3 +399,19 @@ fn add_discussion_thread_fails_because_of_max_thread_by_same_author_in_a_row_lim discussion_fixture.create_discussion_and_assert(Err(Error::MaxThreadInARowLimitExceeded)); }); } + +#[test] +fn discussion_thread_and_post_counters_are_valid() { + initial_test_ext().execute_with(|| { + let discussion_fixture = DiscussionFixture::default(); + let thread_id = discussion_fixture + .create_discussion_and_assert(Ok(1)) + .unwrap(); + + let mut post_fixture1 = PostFixture::default_for_thread(thread_id); + let _ = post_fixture1.add_post_and_assert(Ok(())).unwrap(); + + assert_eq!(Discussions::thread_count(), 1); + assert_eq!(Discussions::post_count(), 1); + }); +} diff --git a/runtime-modules/proposals/discussion/src/types.rs b/runtime-modules/proposals/discussion/src/types.rs index 18bd8a86fb..5b8add6e5c 100644 --- a/runtime-modules/proposals/discussion/src/types.rs +++ b/runtime-modules/proposals/discussion/src/types.rs @@ -71,3 +71,32 @@ impl ThreadCounter { } } } + +#[cfg(test)] +mod tests { + use crate::types::ThreadCounter; + + #[test] + fn thread_counter_increment_works() { + let test = ThreadCounter { + author_id: 56, + counter: 56, + }; + let expected = ThreadCounter { + author_id: 56, + counter: 57, + }; + + assert_eq!(expected, test.increment()); + } + + #[test] + fn thread_counter_new_works() { + let expected = ThreadCounter { + author_id: 56, + counter: 1, + }; + + assert_eq!(expected, ThreadCounter::new(56)); + } +} diff --git a/runtime-modules/proposals/engine/src/lib.rs b/runtime-modules/proposals/engine/src/lib.rs index 28e4badd53..733f83dd3e 100644 --- a/runtime-modules/proposals/engine/src/lib.rs +++ b/runtime-modules/proposals/engine/src/lib.rs @@ -663,7 +663,9 @@ impl Module { // - update proposal status fields (status, finalized_at) // - add to pending execution proposal cache if approved // - slash and unstake proposal stake if stake exists + // - decrease active proposal counter // - fire an event + // It prints an error message in case of an attempt to finalize the non-active proposal. fn finalize_proposal(proposal_id: T::ProposalId, decision_status: ProposalDecisionStatus) { Self::decrease_active_proposal_counter(); >::remove(&proposal_id.clone()); @@ -719,7 +721,8 @@ impl Module { } // Calculates required slash based on finalization ProposalDecisionStatus and proposal parameters. - fn calculate_slash_balance( + // Method visibility allows testing. + pub(crate) fn calculate_slash_balance( decision_status: &ProposalDecisionStatus, proposal_parameters: &ProposalParameters>, ) -> types::BalanceOf { diff --git a/runtime-modules/proposals/engine/src/tests/mod.rs b/runtime-modules/proposals/engine/src/tests/mod.rs index de6c645943..5cdc3ec0e0 100644 --- a/runtime-modules/proposals/engine/src/tests/mod.rs +++ b/runtime-modules/proposals/engine/src/tests/mod.rs @@ -12,6 +12,14 @@ use system::{EventRecord, Phase}; use srml_support::traits::Currency; +pub(crate) fn increase_total_balance_issuance_using_account_id(account_id: u64, balance: u64) { + let initial_balance = Balances::total_issuance(); + { + let _ = ::Currency::deposit_creating(&account_id, balance); + } + assert_eq!(Balances::total_issuance(), initial_balance + balance); +} + struct ProposalParametersFixture { parameters: ProposalParameters, } @@ -437,6 +445,9 @@ fn voting_results_calculation_succeeds() { #[test] fn rejected_voting_results_and_remove_proposal_id_from_active_succeeds() { initial_test_ext().execute_with(|| { + // internal active proposal counter check + assert_eq!(::get(), 0); + let dummy_proposal = DummyProposalFixture::default(); let proposal_id = dummy_proposal.create_proposal_and_assert(Ok(1)).unwrap(); @@ -448,6 +459,9 @@ fn rejected_voting_results_and_remove_proposal_id_from_active_succeeds() { assert!(>::exists(proposal_id)); + // internal active proposal counter check + assert_eq!(::get(), 1); + run_to_block_and_finalize(2); let proposal = >::get(proposal_id); @@ -467,6 +481,9 @@ fn rejected_voting_results_and_remove_proposal_id_from_active_succeeds() { ProposalStatus::finalized_successfully(ProposalDecisionStatus::Rejected, 1), ); assert!(!>::exists(proposal_id)); + + // internal active proposal counter check + assert_eq!(::get(), 0); }); } @@ -555,9 +572,15 @@ fn cancel_proposal_succeeds() { DummyProposalFixture::default().with_parameters(parameters_fixture.params()); let proposal_id = dummy_proposal.create_proposal_and_assert(Ok(1)).unwrap(); + // internal active proposal counter check + assert_eq!(::get(), 1); + let cancel_proposal = CancelProposalFixture::new(proposal_id); cancel_proposal.cancel_and_assert(Ok(())); + // internal active proposal counter check + assert_eq!(::get(), 0); + let proposal = >::get(proposal_id); assert_eq!( @@ -819,46 +842,6 @@ fn proposal_execution_postponed_because_of_grace_period() { }); } -/* - -#[test] -fn veto_proposal_succeeds() { - initial_test_ext().execute_with(|| { - // internal active proposal counter check - assert_eq!(::get(), 0); - - let parameters_fixture = ProposalParametersFixture::default(); - let dummy_proposal = - DummyProposalFixture::default().with_parameters(parameters_fixture.params()); - let proposal_id = dummy_proposal.create_proposal_and_assert(Ok(1)).unwrap(); - - // internal active proposal counter check - assert_eq!(::get(), 1); - - let veto_proposal = VetoProposalFixture::new(proposal_id); - veto_proposal.veto_and_assert(Ok(())); - - let proposal = >::get(proposal_id); - - assert_eq!( - proposal, - Proposal { - parameters: parameters_fixture.params(), - proposer_id: 1, - created_at: 1, - status: ProposalStatus::finalized_successfully(ProposalDecisionStatus::Vetoed, 1), - title: b"title".to_vec(), - description: b"description".to_vec(), - voting_results: VotingResults::default(), - } - ); - - // internal active proposal counter check - assert_eq!(::get(), 0); - }); -} -*/ - #[test] fn proposal_execution_vetoed_successfully_during_the_grace_period() { initial_test_ext().execute_with(|| { @@ -1492,3 +1475,107 @@ fn proposal_reset_succeeds() { ); }); } + +#[test] +fn proposal_counters_are_valid() { + initial_test_ext().execute_with(|| { + let mut dummy_proposal = DummyProposalFixture::default(); + let _ = dummy_proposal.create_proposal_and_assert(Ok(1)).unwrap(); + + dummy_proposal = DummyProposalFixture::default(); + let _ = dummy_proposal.create_proposal_and_assert(Ok(2)).unwrap(); + + dummy_proposal = DummyProposalFixture::default(); + let proposal_id = dummy_proposal.create_proposal_and_assert(Ok(3)).unwrap(); + + assert_eq!(ActiveProposalCount::get(), 3); + assert_eq!(ProposalCount::get(), 3); + + let cancel_proposal_fixture = CancelProposalFixture::new(proposal_id); + cancel_proposal_fixture.cancel_and_assert(Ok(())); + + assert_eq!(ActiveProposalCount::get(), 2); + assert_eq!(ProposalCount::get(), 3); + }); +} + +#[test] +fn proposal_stake_cache_is_valid() { + initial_test_ext().execute_with(|| { + increase_total_balance_issuance_using_account_id(1, 50000); + + let stake = 250u32; + let parameters = ProposalParametersFixture::default().with_required_stake(stake.into()); + let dummy_proposal = DummyProposalFixture::default() + .with_parameters(parameters.params()) + .with_stake(stake as u64); + + let proposal_id = dummy_proposal.create_proposal_and_assert(Ok(1)).unwrap(); + let expected_stake_id = 0; + assert_eq!( + >::get(&expected_stake_id), + proposal_id + ); + }); +} + +#[test] +fn slash_balance_is_calculated_correctly() { + initial_test_ext().execute_with(|| { + let vetoed_slash_balance = ProposalsEngine::calculate_slash_balance( + &ProposalDecisionStatus::Vetoed, + &ProposalParametersFixture::default().params(), + ); + + assert_eq!(vetoed_slash_balance, 0); + + let approved_slash_balance = ProposalsEngine::calculate_slash_balance( + &ProposalDecisionStatus::Approved(ApprovedProposalStatus::Executed), + &ProposalParametersFixture::default().params(), + ); + + assert_eq!(approved_slash_balance, 0); + + let rejection_fee = ::RejectionFee::get(); + + let rejected_slash_balance = ProposalsEngine::calculate_slash_balance( + &ProposalDecisionStatus::Rejected, + &ProposalParametersFixture::default().params(), + ); + + assert_eq!(rejected_slash_balance, rejection_fee); + + let expired_slash_balance = ProposalsEngine::calculate_slash_balance( + &ProposalDecisionStatus::Expired, + &ProposalParametersFixture::default().params(), + ); + + assert_eq!(expired_slash_balance, rejection_fee); + + let cancellation_fee = ::CancellationFee::get(); + + let cancellation_slash_balance = ProposalsEngine::calculate_slash_balance( + &ProposalDecisionStatus::Canceled, + &ProposalParametersFixture::default().params(), + ); + + assert_eq!(cancellation_slash_balance, cancellation_fee); + + let slash_balance_with_no_stake = ProposalsEngine::calculate_slash_balance( + &ProposalDecisionStatus::Slashed, + &ProposalParametersFixture::default().params(), + ); + + assert_eq!(slash_balance_with_no_stake, 0); + + let stake = 256; + let slash_balance_with_stake = ProposalsEngine::calculate_slash_balance( + &ProposalDecisionStatus::Slashed, + &ProposalParametersFixture::default() + .with_required_stake(stake) + .params(), + ); + + assert_eq!(slash_balance_with_stake, stake); + }); +} diff --git a/runtime-modules/proposals/engine/src/types/proposal_statuses.rs b/runtime-modules/proposals/engine/src/types/proposal_statuses.rs index 77ebf17926..4deb8b647d 100644 --- a/runtime-modules/proposals/engine/src/types/proposal_statuses.rs +++ b/runtime-modules/proposals/engine/src/types/proposal_statuses.rs @@ -148,3 +148,50 @@ pub enum ProposalDecisionStatus { /// must be no less than the quorum value for the given proposal type. Approved(ApprovedProposalStatus), } + +#[cfg(test)] +mod tests { + use crate::{ + ActiveStake, ApprovedProposalStatus, FinalizationData, ProposalDecisionStatus, + ProposalStatus, + }; + + #[test] + fn approved_proposal_status_helper_succeeds() { + let msg = "error"; + + assert_eq!( + ApprovedProposalStatus::failed_execution(&msg), + ApprovedProposalStatus::ExecutionFailed { + error: msg.as_bytes().to_vec() + } + ); + } + + #[test] + fn finalized_proposal_status_helper_succeeds() { + let msg = "error"; + let block_number = 20; + let stake = ActiveStake { + stake_id: 50, + source_account_id: 2, + }; + + let proposal_status = ProposalStatus::finalized( + ProposalDecisionStatus::Slashed, + Some(msg), + Some(stake), + block_number, + ); + + assert_eq!( + ProposalStatus::Finalized(FinalizationData { + proposal_status: ProposalDecisionStatus::Slashed, + finalized_at: block_number, + encoded_unstaking_error_due_to_broken_runtime: Some(msg.as_bytes().to_vec()), + stake_data_after_unstaking_error: Some(stake) + }), + proposal_status + ); + } +} From 1683ac460eac7811e1dc9a4da6c46c268b24a6e1 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Wed, 15 Apr 2020 12:25:03 +0300 Subject: [PATCH 20/23] Add tests for proposal status resolution --- .../proposals/engine/src/types/mod.rs | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/runtime-modules/proposals/engine/src/types/mod.rs b/runtime-modules/proposals/engine/src/types/mod.rs index a496fec53e..d023f494a1 100644 --- a/runtime-modules/proposals/engine/src/types/mod.rs +++ b/runtime-modules/proposals/engine/src/types/mod.rs @@ -369,6 +369,7 @@ pub(crate) struct ApprovedProposalData< #[cfg(test)] mod tests { + use crate::types::ProposalStatusResolution; use crate::*; // Alias introduced for simplicity of changing Proposal exact types. @@ -672,4 +673,116 @@ mod tests { Some(ProposalDecisionStatus::Slashed) ); } + + #[test] + fn proposal_status_resolution_approval_quorum_works_correctly() { + let no_approval_quorum_proposal: Proposal = Proposal { + parameters: ProposalParameters { + approval_quorum_percentage: 63, + ..ProposalParameters::default() + }, + ..Proposal::default() + }; + let no_approval_proposal_status_resolution = ProposalStatusResolution { + proposal: &no_approval_quorum_proposal, + now: 20, + votes_count: 314, + total_voters_count: 500, + approvals: 3, + slashes: 3, + }; + + assert!(!no_approval_proposal_status_resolution.is_approval_quorum_reached()); + + let approval_quorum_proposal_status_resolution = ProposalStatusResolution { + votes_count: 315, + ..no_approval_proposal_status_resolution + }; + + assert!(approval_quorum_proposal_status_resolution.is_approval_quorum_reached()); + } + + #[test] + fn proposal_status_resolution_slashing_quorum_works_correctly() { + let no_slashing_quorum_proposal: Proposal = Proposal { + parameters: ProposalParameters { + slashing_quorum_percentage: 63, + ..ProposalParameters::default() + }, + ..Proposal::default() + }; + let no_slashing_proposal_status_resolution = ProposalStatusResolution { + proposal: &no_slashing_quorum_proposal, + now: 20, + votes_count: 314, + total_voters_count: 500, + approvals: 3, + slashes: 3, + }; + + assert!(!no_slashing_proposal_status_resolution.is_slashing_quorum_reached()); + + let slashing_quorum_proposal_status_resolution = ProposalStatusResolution { + votes_count: 315, + ..no_slashing_proposal_status_resolution + }; + + assert!(slashing_quorum_proposal_status_resolution.is_slashing_quorum_reached()); + } + + #[test] + fn proposal_status_resolution_approval_threshold_works_correctly() { + let no_approval_threshold_proposal: Proposal = Proposal { + parameters: ProposalParameters { + approval_threshold_percentage: 63, + ..ProposalParameters::default() + }, + ..Proposal::default() + }; + let no_approval_proposal_status_resolution = ProposalStatusResolution { + proposal: &no_approval_threshold_proposal, + now: 20, + votes_count: 500, + total_voters_count: 600, + approvals: 314, + slashes: 3, + }; + + assert!(!no_approval_proposal_status_resolution.is_approval_threshold_reached()); + + let approval_threshold_proposal_status_resolution = ProposalStatusResolution { + approvals: 315, + ..no_approval_proposal_status_resolution + }; + + assert!(approval_threshold_proposal_status_resolution.is_approval_threshold_reached()); + } + + #[test] + fn proposal_status_resolution_slashing_threshold_works_correctly() { + let no_slashing_threshold_proposal: Proposal = Proposal { + parameters: ProposalParameters { + slashing_threshold_percentage: 63, + ..ProposalParameters::default() + }, + ..Proposal::default() + }; + let no_slashing_proposal_status_resolution = ProposalStatusResolution { + proposal: &no_slashing_threshold_proposal, + now: 20, + votes_count: 500, + total_voters_count: 600, + approvals: 3, + slashes: 314, + }; + + assert!(!no_slashing_proposal_status_resolution.is_slashing_threshold_reached()); + + let slashing_threshold_proposal_status_resolution = ProposalStatusResolution { + slashes: 315, + ..no_slashing_proposal_status_resolution + }; + + assert!(slashing_threshold_proposal_status_resolution.is_slashing_threshold_reached()); + } } From cb80ece0d7c66ece64dc5dba7529e6a1815f5998 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Wed, 15 Apr 2020 13:44:55 +0300 Subject: [PATCH 21/23] Migrate proposal votes calculations to Perbill - migrate engine module quorum & threshold calculations from float division to Substrate Perbill type - add tests --- runtime-modules/proposals/codex/src/lib.rs | 2 +- .../proposals/engine/src/types/mod.rs | 37 +++++++++++-------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index de61e5ce59..a1fd3656d7 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -59,7 +59,7 @@ use rstd::vec::Vec; use runtime_io::blake2_256; use sr_primitives::traits::SaturatedConversion; use sr_primitives::traits::{One, Zero}; -pub use sr_primitives::Perbill; +use sr_primitives::Perbill; use srml_support::dispatch::DispatchResult; use srml_support::traits::{Currency, Get}; use srml_support::{decl_error, decl_module, decl_storage, ensure, print}; diff --git a/runtime-modules/proposals/engine/src/types/mod.rs b/runtime-modules/proposals/engine/src/types/mod.rs index d023f494a1..d6ad1a8c73 100644 --- a/runtime-modules/proposals/engine/src/types/mod.rs +++ b/runtime-modules/proposals/engine/src/types/mod.rs @@ -10,6 +10,7 @@ use rstd::prelude::*; #[cfg(feature = "std")] use serde::{Deserialize, Serialize}; +use sr_primitives::Perbill; use srml_support::dispatch; use srml_support::traits::Currency; @@ -253,45 +254,45 @@ where // Approval quorum reached for the proposal. Compares predefined parameter with actual // votes sum divided by total possible votes count. pub fn is_approval_quorum_reached(&self) -> bool { - let actual_votes_fraction: f32 = self.votes_count as f32 / self.total_voters_count as f32; - + let actual_votes_fraction = + Perbill::from_rational_approximation(self.votes_count, self.total_voters_count); let approval_quorum_fraction = - self.proposal.parameters.approval_quorum_percentage as f32 / 100.0; + Perbill::from_percent(self.proposal.parameters.approval_quorum_percentage); - actual_votes_fraction >= approval_quorum_fraction + actual_votes_fraction.deconstruct() >= approval_quorum_fraction.deconstruct() } // Slashing quorum reached for the proposal. Compares predefined parameter with actual // votes sum divided by total possible votes count. pub fn is_slashing_quorum_reached(&self) -> bool { - let actual_votes_fraction: f32 = self.votes_count as f32 / self.total_voters_count as f32; - + let actual_votes_fraction = + Perbill::from_rational_approximation(self.votes_count, self.total_voters_count); let slashing_quorum_fraction = - self.proposal.parameters.slashing_quorum_percentage as f32 / 100.0; + Perbill::from_percent(self.proposal.parameters.slashing_quorum_percentage); - actual_votes_fraction >= slashing_quorum_fraction + actual_votes_fraction.deconstruct() >= slashing_quorum_fraction.deconstruct() } // Approval threshold reached for the proposal. Compares predefined parameter with 'approve' // votes sum divided by actual votes count. pub fn is_approval_threshold_reached(&self) -> bool { - let approval_votes_fraction: f32 = self.approvals as f32 / self.votes_count as f32; - + let approval_votes_fraction = + Perbill::from_rational_approximation(self.approvals, self.votes_count); let required_threshold_fraction = - self.proposal.parameters.approval_threshold_percentage as f32 / 100.0; + Perbill::from_percent(self.proposal.parameters.approval_threshold_percentage); - approval_votes_fraction >= required_threshold_fraction + approval_votes_fraction.deconstruct() >= required_threshold_fraction.deconstruct() } // Slashing threshold reached for the proposal. Compares predefined parameter with 'approve' // votes sum divided by actual votes count. pub fn is_slashing_threshold_reached(&self) -> bool { - let slashing_votes_fraction: f32 = self.slashes as f32 / self.votes_count as f32; - + let slashing_votes_fraction = + Perbill::from_rational_approximation(self.slashes, self.votes_count); let required_threshold_fraction = - self.proposal.parameters.slashing_threshold_percentage as f32 / 100.0; + Perbill::from_percent(self.proposal.parameters.slashing_threshold_percentage); - slashing_votes_fraction >= required_threshold_fraction + slashing_votes_fraction.deconstruct() >= required_threshold_fraction.deconstruct() } // All voters had voted @@ -679,6 +680,7 @@ mod tests { let no_approval_quorum_proposal: Proposal = Proposal { parameters: ProposalParameters { approval_quorum_percentage: 63, + slashing_threshold_percentage: 63, ..ProposalParameters::default() }, ..Proposal::default() @@ -706,6 +708,7 @@ mod tests { fn proposal_status_resolution_slashing_quorum_works_correctly() { let no_slashing_quorum_proposal: Proposal = Proposal { parameters: ProposalParameters { + approval_quorum_percentage: 63, slashing_quorum_percentage: 63, ..ProposalParameters::default() }, @@ -734,6 +737,7 @@ mod tests { fn proposal_status_resolution_approval_threshold_works_correctly() { let no_approval_threshold_proposal: Proposal = Proposal { parameters: ProposalParameters { + slashing_threshold_percentage: 63, approval_threshold_percentage: 63, ..ProposalParameters::default() }, @@ -763,6 +767,7 @@ mod tests { let no_slashing_threshold_proposal: Proposal = Proposal { parameters: ProposalParameters { slashing_threshold_percentage: 63, + approval_threshold_percentage: 63, ..ProposalParameters::default() }, ..Proposal::default() From ad3705b48c9a5896036b7ec72b12cebc442afb62 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Wed, 15 Apr 2020 17:35:09 +0300 Subject: [PATCH 22/23] Remove allowd member_id list for runtime upgrade proposal --- runtime-modules/proposals/codex/src/lib.rs | 11 ----------- .../proposals/codex/src/tests/mock.rs | 2 -- .../proposals/codex/src/tests/mod.rs | 17 ----------------- runtime/src/lib.rs | 2 -- 4 files changed, 32 deletions(-) diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index a1fd3656d7..5d3f59d98b 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -88,9 +88,6 @@ pub trait Trait: /// Defines max wasm code length of the runtime upgrade proposal. type RuntimeUpgradeWasmProposalMaxLength: Get; - /// Defines allowed proposers (by member id list) for the runtime upgrade proposal. - type RuntimeUpgradeProposalAllowedProposers: Get>>; - /// Validates member id and origin combination type MembershipOriginValidator: ActorOriginValidator< Self::Origin, @@ -137,9 +134,6 @@ decl_error! { /// Provided WASM code for the runtime upgrade proposal is empty RuntimeProposalIsEmpty, - /// Runtime upgrade proposal can be created only by hardcoded members - RuntimeProposalProposerNotInTheAllowedProposersList, - /// Invalid balance value for the spending proposal InvalidSpendingProposalBalance, @@ -312,11 +306,6 @@ decl_module! { ensure!(wasm.len() as u32 <= T::RuntimeUpgradeWasmProposalMaxLength::get(), Error::RuntimeProposalSizeExceeded); - ensure!( - T::RuntimeUpgradeProposalAllowedProposers::get().contains(&member_id), - Error::RuntimeProposalProposerNotInTheAllowedProposersList - ); - let wasm_hash = blake2_256(&wasm); let proposal_code = diff --git a/runtime-modules/proposals/codex/src/tests/mock.rs b/runtime-modules/proposals/codex/src/tests/mock.rs index d3a8de4f29..7b80011871 100644 --- a/runtime-modules/proposals/codex/src/tests/mock.rs +++ b/runtime-modules/proposals/codex/src/tests/mock.rs @@ -158,7 +158,6 @@ impl VotersParameters for MockVotersParameters { parameter_types! { pub const TextProposalMaxLength: u32 = 20_000; pub const RuntimeUpgradeWasmProposalMaxLength: u32 = 20_000; - pub const RuntimeUpgradeProposalAllowedProposers: Vec = vec![1]; } impl governance::election::Trait for Test { @@ -249,7 +248,6 @@ impl staking::SessionInterface for Test { impl crate::Trait for Test { type TextProposalMaxLength = TextProposalMaxLength; type RuntimeUpgradeWasmProposalMaxLength = RuntimeUpgradeWasmProposalMaxLength; - type RuntimeUpgradeProposalAllowedProposers = RuntimeUpgradeProposalAllowedProposers; type MembershipOriginValidator = (); } diff --git a/runtime-modules/proposals/codex/src/tests/mod.rs b/runtime-modules/proposals/codex/src/tests/mod.rs index 6912c029fc..29a3d67075 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -261,23 +261,6 @@ fn create_upgrade_runtime_proposal_codex_call_fails_with_incorrect_wasm_size() { }); } -#[test] -fn create_upgrade_runtime_proposal_codex_call_fails_with_not_allowed_member_id() { - initial_test_ext().execute_with(|| { - assert_eq!( - ProposalCodex::create_runtime_upgrade_proposal( - RawOrigin::Signed(1).into(), - 110, - b"title".to_vec(), - b"body".to_vec(), - Some(>::from(50000u32)), - b"wasm".to_vec(), - ), - Err(Error::RuntimeProposalProposerNotInTheAllowedProposersList) - ); - }); -} - #[test] fn create_set_election_parameters_proposal_common_checks_succeed() { initial_test_ext().execute_with(|| { diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index d9dd2957f6..2eea0aca46 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -860,14 +860,12 @@ impl proposals_discussion::Trait for Runtime { parameter_types! { pub const TextProposalMaxLength: u32 = 5_000; pub const RuntimeUpgradeWasmProposalMaxLength: u32 = 2_000_000; - pub const RuntimeUpgradeProposalAllowedProposers: Vec = Vec::new(); //TODO set allowed members } impl proposals_codex::Trait for Runtime { type MembershipOriginValidator = MembershipOriginValidator; type TextProposalMaxLength = TextProposalMaxLength; type RuntimeUpgradeWasmProposalMaxLength = RuntimeUpgradeWasmProposalMaxLength; - type RuntimeUpgradeProposalAllowedProposers = RuntimeUpgradeProposalAllowedProposers; } construct_runtime!( From c2c9cb39a603e394d8ebedc79d60b6de8eeb60ef Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Mon, 20 Apr 2020 12:41:41 +0300 Subject: [PATCH 23/23] =?UTF-8?q?Remove=20=E2=80=98set=20council=20mint=20?= =?UTF-8?q?capacity=E2=80=99=20proposal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - remove proposal from the codex - remove tests - remove parameters and proposal details --- runtime-modules/proposals/codex/src/lib.rs | 45 ----------- .../proposals/codex/src/proposal_types/mod.rs | 3 - .../codex/src/proposal_types/parameters.rs | 14 ---- .../proposals/codex/src/tests/mod.rs | 75 +------------------ 4 files changed, 1 insertion(+), 136 deletions(-) diff --git a/runtime-modules/proposals/codex/src/lib.rs b/runtime-modules/proposals/codex/src/lib.rs index 5d3f59d98b..d56c47466d 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -16,7 +16,6 @@ //! - [create_text_proposal](./struct.Module.html#method.create_text_proposal) //! - [create_runtime_upgrade_proposal](./struct.Module.html#method.create_runtime_upgrade_proposal) //! - [create_set_election_parameters_proposal](./struct.Module.html#method.create_set_election_parameters_proposal) -//! - [create_set_council_mint_capacity_proposal](./struct.Module.html#method.create_set_council_mint_capacity_proposal) //! - [create_set_content_working_group_mint_capacity_proposal](./struct.Module.html#method.create_set_content_working_group_mint_capacity_proposal) //! - [create_spending_proposal](./struct.Module.html#method.create_spending_proposal) //! - [create_set_lead_proposal](./struct.Module.html#method.create_set_lead_proposal) @@ -200,9 +199,6 @@ decl_error! { /// Invalid working group mint capacity parameter InvalidStorageWorkingGroupMintCapacity, - /// Invalid council mint capacity parameter - InvalidStorageCouncilMintCapacity, - /// Invalid 'set lead proposal' parameter - proposed lead cannot be a councilor InvalidSetLeadParameterCannotBeCouncilor } @@ -357,47 +353,6 @@ decl_module! { )?; } - /// Create 'Set council mint capacity' proposal type. This proposal uses `set_mint_capacity()` - /// extrinsic from the `governance::council` module. - pub fn create_set_council_mint_capacity_proposal( - origin, - member_id: MemberId, - title: Vec, - description: Vec, - stake_balance: Option>, - mint_balance: BalanceOfMint, - ) { - - let max_mint_capacity: u32 = get_required_stake_by_fraction::( - COUNCIL_MINT_MAX_BALANCE_PERCENT, - 100 - ) - .try_into() - .unwrap_or_default() as u32; - - ensure!( - mint_balance < >::from(max_mint_capacity), - Error::InvalidStorageCouncilMintCapacity - ); - - let proposal_code = - >::set_council_mint_capacity(mint_balance.clone()); - - let proposal_parameters = - proposal_types::parameters::set_council_mint_capacity_proposal::(); - - Self::create_proposal( - origin, - member_id, - title, - description, - stake_balance, - proposal_code.encode(), - proposal_parameters, - ProposalDetails::SetCouncilMintCapacity(mint_balance), - )?; - } - /// Create 'Set content working group mint capacity' proposal type. /// This proposal uses `set_mint_capacity()` extrinsic from the `content-working-group` module. pub fn create_set_content_working_group_mint_capacity_proposal( diff --git a/runtime-modules/proposals/codex/src/proposal_types/mod.rs b/runtime-modules/proposals/codex/src/proposal_types/mod.rs index 43a7138903..e9e7bc93dc 100644 --- a/runtime-modules/proposals/codex/src/proposal_types/mod.rs +++ b/runtime-modules/proposals/codex/src/proposal_types/mod.rs @@ -27,9 +27,6 @@ pub enum ProposalDetails), - /// Balance for the `set council mint capacity` proposal - SetCouncilMintCapacity(MintedBalance), - /// Balance for the `set content working group mint capacity` proposal SetContentWorkingGroupMintCapacity(MintedBalance), diff --git a/runtime-modules/proposals/codex/src/proposal_types/parameters.rs b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs index 28dcb35c9c..031c17de09 100644 --- a/runtime-modules/proposals/codex/src/proposal_types/parameters.rs +++ b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs @@ -55,20 +55,6 @@ pub(crate) fn set_election_parameters_proposal( } } -// Proposal parameters for the 'Set council mint capacity' proposal -pub(crate) fn set_council_mint_capacity_proposal( -) -> ProposalParameters> { - ProposalParameters { - voting_period: T::BlockNumber::from(43200u32), - grace_period: T::BlockNumber::from(0u32), - approval_quorum_percentage: 66, - approval_threshold_percentage: 80, - slashing_quorum_percentage: 50, - slashing_threshold_percentage: 50, - required_stake: Some(get_required_stake_by_fraction::(25, 10000)), - } -} - // Proposal parameters for the 'Set content working group mint capacity' proposal pub(crate) fn set_content_working_group_mint_capacity_proposal( ) -> ProposalParameters> { diff --git a/runtime-modules/proposals/codex/src/tests/mod.rs b/runtime-modules/proposals/codex/src/tests/mod.rs index 29a3d67075..d66127363f 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -466,80 +466,7 @@ fn create_set_election_parameters_call_fails_with_incorrect_parameters() { } #[test] -fn create_set_council_mint_capacity_proposal_fails_with_invalid_parameters() { - initial_test_ext().execute_with(|| { - increase_total_balance_issuance(500000); - - assert_eq!( - ProposalCodex::create_set_council_mint_capacity_proposal( - RawOrigin::Signed(1).into(), - 1, - b"title".to_vec(), - b"body".to_vec(), - Some(>::from(1250u32)), - 10001, - ), - Err(Error::InvalidStorageCouncilMintCapacity) - ); - }); -} - -#[test] -fn create_set_council_mint_capacity_proposal_common_checks_succeed() { - initial_test_ext().execute_with(|| { - increase_total_balance_issuance(500000); - - let proposal_fixture = ProposalTestFixture { - insufficient_rights_call: || { - ProposalCodex::create_set_council_mint_capacity_proposal( - RawOrigin::None.into(), - 1, - b"title".to_vec(), - b"body".to_vec(), - None, - 0, - ) - }, - empty_stake_call: || { - ProposalCodex::create_set_council_mint_capacity_proposal( - RawOrigin::Signed(1).into(), - 1, - b"title".to_vec(), - b"body".to_vec(), - None, - 0, - ) - }, - invalid_stake_call: || { - ProposalCodex::create_set_council_mint_capacity_proposal( - RawOrigin::Signed(1).into(), - 1, - b"title".to_vec(), - b"body".to_vec(), - Some(>::from(150u32)), - 0, - ) - }, - successful_call: || { - ProposalCodex::create_set_council_mint_capacity_proposal( - RawOrigin::Signed(1).into(), - 1, - b"title".to_vec(), - b"body".to_vec(), - Some(>::from(1250u32)), - 10, - ) - }, - proposal_parameters: - crate::proposal_types::parameters::set_council_mint_capacity_proposal::(), - proposal_details: ProposalDetails::SetCouncilMintCapacity(10), - }; - proposal_fixture.check_all(); - }); -} - -#[test] -fn create_working_groupd_mint_capacity_proposal_fails_with_invalid_parameters() { +fn create_working_group_mint_capacity_proposal_fails_with_invalid_parameters() { initial_test_ext().execute_with(|| { increase_total_balance_issuance(500000);