diff --git a/Cargo.lock b/Cargo.lock index 8621e62937..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", @@ -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", @@ -5122,6 +5125,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", @@ -5191,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/Cargo.toml b/runtime-modules/proposals/codex/Cargo.toml index a1186abc73..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', @@ -23,6 +24,7 @@ std = [ 'membership/std', 'governance/std', 'mint/std', + 'roles/std', ] @@ -83,6 +85,18 @@ 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' +package = 'sr-io' +rev = 'c37bb08535c49a12320af7facfd555ce05cce2e8' + [dependencies.stake] default_features = false package = 'substrate-stake-module' @@ -123,11 +137,10 @@ default_features = false package = 'substrate-content-working-group-module' path = '../../content-working-group' -[dev-dependencies.runtime-io] +[dependencies.roles] default_features = false -git = 'https://github.com/paritytech/substrate.git' -package = 'sr-io' -rev = 'c37bb08535c49a12320af7facfd555ce05cce2e8' +package = 'substrate-roles-module' +path = '../../roles' [dev-dependencies.hiring] default_features = false @@ -153,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 0ed555210e..d56c47466d 100644 --- a/runtime-modules/proposals/codex/src/lib.rs +++ b/runtime-modules/proposals/codex/src/lib.rs @@ -1,24 +1,27 @@ //! # 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. +//! 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) //! - [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) +//! - [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 @@ -46,16 +49,27 @@ 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::convert::TryInto; use rstd::prelude::*; use rstd::str::from_utf8; use rstd::vec::Vec; -use sr_primitives::traits::Zero; +use runtime_io::blake2_256; +use sr_primitives::traits::SaturatedConversion; +use sr_primitives::traits::{One, Zero}; +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}; 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 @@ -64,6 +78,8 @@ pub trait Trait: + membership::members::Trait + governance::election::Trait + content_working_group::Trait + + roles::actors::Trait + + staking::Trait { /// Defines max allowed text proposal length. type TextProposalMaxLength: Get; @@ -83,6 +99,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< @@ -115,10 +134,73 @@ decl_error! { RuntimeProposalIsEmpty, /// Invalid balance value for the spending proposal - SpendingProposalZeroBalance, + InvalidSpendingProposalBalance, + + /// Invalid validator count for the 'set validator count' proposal + InvalidValidatorCount, /// 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 - 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, + + /// 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 'set lead proposal' parameter - proposed lead cannot be a councilor + InvalidSetLeadParameterCannotBeCouncilor } } @@ -158,6 +240,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 +274,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,10 +284,12 @@ decl_module! { stake_balance, proposal_code.encode(), proposal_parameters, + ProposalDetails::Text(text), )?; } - /// 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, @@ -208,10 +302,12 @@ 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); - let proposal_parameters = proposal_types::parameters::upgrade_runtime::(); + let proposal_parameters = proposal_types::parameters::runtime_upgrade_proposal::(); Self::create_proposal( origin, @@ -221,6 +317,7 @@ decl_module! { stake_balance, proposal_code.encode(), proposal_parameters, + ProposalDetails::RuntimeUpgrade(wasm_hash.to_vec()), )?; } @@ -236,8 +333,10 @@ decl_module! { ) { election_parameters.ensure_valid()?; + Self::ensure_council_election_parameters_valid(&election_parameters)?; + 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,13 +349,13 @@ decl_module! { stake_balance, proposal_code.encode(), proposal_parameters, + ProposalDetails::SetElectionParameters(election_parameters), )?; } - - /// 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( + /// 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( origin, member_id: MemberId, title: Vec, @@ -264,11 +363,20 @@ 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_council_mint_capacity(mint_balance); + >::set_mint_capacity(mint_balance.clone()); let proposal_parameters = - proposal_types::parameters::set_council_mint_capacity_proposal::(); + proposal_types::parameters::set_content_working_group_mint_capacity_proposal::(); Self::create_proposal( origin, @@ -278,24 +386,42 @@ decl_module! { stake_balance, proposal_code.encode(), proposal_parameters, + ProposalDetails::SetContentWorkingGroupMintCapacity(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( + /// Create 'Spending' proposal type. + /// This proposal uses `spend_from_council_mint()` extrinsic from the `governance::council` module. + pub fn create_spending_proposal( origin, member_id: MemberId, title: Vec, description: Vec, stake_balance: Option>, - mint_balance: BalanceOfMint, + balance: BalanceOfMint, + destination: T::AccountId, ) { - let proposal_code = - >::set_mint_capacity(mint_balance); + 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(), + destination.clone() + ); let proposal_parameters = - proposal_types::parameters::set_content_working_group_mint_capacity_proposal::(); + proposal_types::parameters::spending_proposal::(); Self::create_proposal( origin, @@ -305,27 +431,34 @@ decl_module! { stake_balance, proposal_code.encode(), proposal_parameters, + ProposalDetails::Spending(balance, destination), )?; } - /// Create 'Spending' proposal type. - /// This proposal uses `spend_from_council_mint()` extrinsic from the `governance::council` module. - pub fn create_spending_proposal( + + /// Create 'Set lead' proposal type. + /// This proposal uses `replace_lead()` extrinsic from the `content_working_group` module. + pub fn create_set_lead_proposal( origin, member_id: MemberId, title: Vec, description: Vec, stake_balance: Option>, - balance: BalanceOfMint, - destination: T::AccountId, + new_lead: Option<(T::MemberId, T::AccountId)> ) { - ensure!(balance != BalanceOfMint::::zero(), Error::SpendingProposalZeroBalance); + if let Some(lead) = new_lead.clone() { + let account_id = lead.1; + ensure!( + !>::is_councilor(&account_id), + Error::InvalidSetLeadParameterCannotBeCouncilor + ); + } let proposal_code = - >::spend_from_council_mint(balance, destination); + >::replace_lead(new_lead.clone()); let proposal_parameters = - proposal_types::parameters::spending_proposal::(); + proposal_types::parameters::set_lead_proposal::(); Self::create_proposal( origin, @@ -335,25 +468,63 @@ decl_module! { stake_balance, proposal_code.encode(), proposal_parameters, + ProposalDetails::SetLead(new_lead), )?; } + /// 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::(); - /// Create 'Set lead' proposal type. - /// This proposal uses `replace_lead()` extrinsic from the `content_working_group` module. - pub fn create_set_lead_proposal( + Self::create_proposal( + origin, + member_id, + title, + description, + stake_balance, + proposal_code.encode(), + proposal_parameters, + ProposalDetails::EvictStorageProvider(actor_account), + )?; + } + + /// 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_lead: Option<(T::MemberId, T::AccountId)> + new_validator_count: u32, ) { + ensure!( + new_validator_count >= >::minimum_validator_count(), + Error::InvalidValidatorCount + ); + + ensure!( + new_validator_count <= 1000, // max validator count + Error::InvalidValidatorCount + ); + let proposal_code = - >::replace_lead(new_lead); + >::set_validator_count(new_validator_count); let proposal_parameters = - proposal_types::parameters::set_lead_proposal::(); + proposal_types::parameters::set_validator_count_proposal::(); Self::create_proposal( origin, @@ -363,6 +534,39 @@ decl_module! { stake_balance, proposal_code.encode(), proposal_parameters, + ProposalDetails::SetValidatorCount(new_validator_count), + )?; + } + + /// 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> + ) { + Self::ensure_storage_role_parameters_valid(&role_parameters)?; + + 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), )?; } @@ -434,6 +638,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())?; @@ -445,7 +656,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())?; @@ -461,7 +672,233 @@ impl Module { )?; >::insert(proposal_id, discussion_thread_id); + >::insert(proposal_id, proposal_details); + + 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 <= 5, + Error::InvalidStorageRoleParameterMinActors + ); + + ensure!( + role_parameters.max_actors >= 5, + Error::InvalidStorageRoleParameterMaxActors + ); + + ensure!( + role_parameters.max_actors < 100, + Error::InvalidStorageRoleParameterMaxActors + ); + + ensure!( + role_parameters.reward_period >= T::BlockNumber::from(600), + Error::InvalidStorageRoleParameterRewardPeriod + ); + + ensure!( + 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), + Error::InvalidStorageRoleParameterUnbondingPeriod + ); + + ensure!( + role_parameters.unbonding_period <= T::BlockNumber::from(28800), + Error::InvalidStorageRoleParameterUnbondingPeriod + ); + + ensure!( + role_parameters.min_service_period >= T::BlockNumber::from(600), + Error::InvalidStorageRoleParameterMinServicePeriod + ); + + ensure!( + role_parameters.min_service_period <= T::BlockNumber::from(28800), + Error::InvalidStorageRoleParameterMinServicePeriod + ); + + ensure!( + role_parameters.startup_grace_period >= T::BlockNumber::from(600), + 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 + pub(crate) fn ensure_council_election_parameters_valid( + election_parameters: &ElectionParameters, T::BlockNumber>, + ) -> Result<(), Error> { + ensure!( + election_parameters.council_size >= 4, + Error::InvalidCouncilElectionParameterCouncilSize + ); + + ensure!( + election_parameters.council_size <= 20, + Error::InvalidCouncilElectionParameterCouncilSize + ); + + ensure!( + election_parameters.candidacy_limit >= 25, + 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/mod.rs b/runtime-modules/proposals/codex/src/proposal_types/mod.rs index ee65d7af17..e9e7bc93dc 100644 --- a/runtime-modules/proposals/codex/src/proposal_types/mod.rs +++ b/runtime-modules/proposals/codex/src/proposal_types/mod.rs @@ -1,101 +1,49 @@ -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; +use roles::actors::RoleParameters; - // 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 content working group mint capacity` proposal + SetContentWorkingGroupMintCapacity(MintedBalance), + + /// AccountId for the `evict storage provider` proposal + EvictStorageProvider(AccountId), + + /// Validator count for the `set validator count` proposal + SetValidatorCount(u32), + + /// Role parameters for the `set storage role parameters` proposal + SetStorageRoleParameters(RoleParameters), +} - // 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..031c17de09 --- /dev/null +++ b/runtime-modules/proposals/codex/src/proposal_types/parameters.rs @@ -0,0 +1,156 @@ +use crate::{get_required_stake_by_fraction, BalanceOf, ProposalParameters}; + +// 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(72000u32), + grace_period: T::BlockNumber::from(72000u32), + approval_quorum_percentage: 80, + approval_threshold_percentage: 100, + slashing_quorum_percentage: 60, + slashing_threshold_percentage: 80, + 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(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)), + } +} + +// Proposal parameters for the 'Set Election Parameters' proposal +pub(crate) fn set_election_parameters_proposal( +) -> ProposalParameters> { + ProposalParameters { + 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(get_required_stake_by_fraction::(75, 10000)), + } +} + +// 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(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)), + } +} + +// Proposal parameters for the 'Spending' proposal +pub(crate) fn spending_proposal( +) -> ProposalParameters> { + ProposalParameters { + voting_period: T::BlockNumber::from(72000u32), + grace_period: T::BlockNumber::from(14400u32), + 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 'Set lead' proposal +pub(crate) fn set_lead_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 'Evict storage provider' proposal +pub(crate) fn evict_storage_provider_proposal( +) -> ProposalParameters> { + ProposalParameters { + 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 storage role parameters' proposal +pub(crate) fn set_storage_role_parameters_proposal( +) -> ProposalParameters> { + ProposalParameters { + 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)), + } +} + +#[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/mock.rs b/runtime-modules/proposals/codex/src/tests/mock.rs index 37a958aa68..7b80011871 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 {} @@ -120,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) } } @@ -187,6 +192,59 @@ 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) {} +} + +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 71d691ecd2..d66127363f 100644 --- a/runtime-modules/proposals/codex/src/tests/mod.rs +++ b/runtime-modules/proposals/codex/src/tests/mod.rs @@ -5,11 +5,26 @@ use srml_support::traits::Currency; use srml_support::StorageMap; use system::RawOrigin; -use crate::{BalanceOf, Error}; -use mock::*; +use crate::{BalanceOf, Error, ProposalDetails}; 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) { + increase_total_balance_issuance_using_account_id(999, balance); +} + +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 ProposalTestFixture where InsufficientRightsCall: Fn() -> DispatchResult, @@ -22,6 +37,7 @@ where invalid_stake_call: InvalidStakeCall, successful_call: SuccessfulCall, proposal_parameters: ProposalParameters, + proposal_details: ProposalDetails, } impl @@ -42,7 +58,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) { @@ -59,6 +78,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) { @@ -71,6 +94,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( @@ -108,11 +133,12 @@ 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(), ) }, proposal_parameters: crate::proposal_types::parameters::text_proposal::(), + proposal_details: ProposalDetails::Text(b"text".to_vec()), }; proposal_fixture.check_all(); }); @@ -153,6 +179,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( @@ -190,11 +218,12 @@ 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(), ) }, - 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(); }); @@ -235,26 +264,17 @@ fn create_upgrade_runtime_proposal_codex_call_fails_with_incorrect_wasm_size() { #[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, - }; + increase_total_balance_issuance(500000); 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: || { @@ -264,7 +284,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: || { @@ -274,7 +294,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: || { @@ -283,43 +303,195 @@ fn create_set_election_parameters_proposal_common_checks_succeed() { 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), - election_parameters.clone(), + Some(>::from(3750u32)), + get_valid_election_parameters(), ) }, proposal_parameters: crate::proposal_types::parameters::set_election_parameters_proposal::(), + 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(3750u32)), + election_parameters, + ), + Err(error) + ); +} + +fn get_valid_election_parameters() -> ElectionParameters { + ElectionParameters { + announcing_period: 14400, + voting_period: 14400, + revealing_period: 14400, + council_size: 4, + 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); + increase_total_balance_issuance_using_account_id(1, 500000); + + let mut election_parameters = get_valid_election_parameters(); + election_parameters.council_size = 2; + assert_failed_election_parameters_call( + election_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( + election_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( + election_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( + election_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( + election_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( + election_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( + election_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_working_group_mint_capacity_proposal_fails_with_invalid_parameters() { + initial_test_ext().execute_with(|| { + increase_total_balance_issuance(500000); assert_eq!( - ProposalCodex::create_set_election_parameters_proposal( + ProposalCodex::create_set_content_working_group_mint_capacity_proposal( RawOrigin::Signed(1).into(), 1, b"title".to_vec(), b"body".to_vec(), - required_stake, - ElectionParameters::default(), + Some(>::from(1250u32)), + 5001, ), - Err(Error::Other("PeriodCannotBeZero")) + Err(Error::InvalidStorageWorkingGroupMintCapacity) ); }); } #[test] -fn create_set_council_mint_capacity_proposal_common_checks_succeed() { +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_council_mint_capacity_proposal( + ProposalCodex::create_set_content_working_group_mint_capacity_proposal( RawOrigin::None.into(), 1, b"title".to_vec(), @@ -329,7 +501,7 @@ fn create_set_council_mint_capacity_proposal_common_checks_succeed() { ) }, empty_stake_call: || { - ProposalCodex::create_set_council_mint_capacity_proposal( + ProposalCodex::create_set_content_working_group_mint_capacity_proposal( RawOrigin::Signed(1).into(), 1, b"title".to_vec(), @@ -339,7 +511,7 @@ fn create_set_council_mint_capacity_proposal_common_checks_succeed() { ) }, invalid_stake_call: || { - ProposalCodex::create_set_council_mint_capacity_proposal( + ProposalCodex::create_set_content_working_group_mint_capacity_proposal( RawOrigin::Signed(1).into(), 1, b"title".to_vec(), @@ -349,190 +521,534 @@ fn create_set_council_mint_capacity_proposal_common_checks_succeed() { ) }, successful_call: || { - ProposalCodex::create_set_council_mint_capacity_proposal( + ProposalCodex::create_set_content_working_group_mint_capacity_proposal( RawOrigin::Signed(1).into(), 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), - 0, + Some(>::from(1250u32)), + 10, ) }, - proposal_parameters: - crate::proposal_types::parameters::set_council_mint_capacity_proposal::(), + proposal_parameters: crate::proposal_types::parameters::set_content_working_group_mint_capacity_proposal::(), + proposal_details: ProposalDetails::SetContentWorkingGroupMintCapacity(10), }; proposal_fixture.check_all(); }); } #[test] -fn create_set_content_working_group_mint_capacity_proposal_common_checks_succeed() { +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_set_content_working_group_mint_capacity_proposal( + ProposalCodex::create_spending_proposal( RawOrigin::None.into(), 1, b"title".to_vec(), b"body".to_vec(), None, - 0, + 20, + 10, ) }, empty_stake_call: || { - ProposalCodex::create_set_content_working_group_mint_capacity_proposal( + ProposalCodex::create_spending_proposal( RawOrigin::Signed(1).into(), 1, b"title".to_vec(), b"body".to_vec(), None, - 0, + 20, + 10, ) }, invalid_stake_call: || { - ProposalCodex::create_set_content_working_group_mint_capacity_proposal( + ProposalCodex::create_spending_proposal( RawOrigin::Signed(1).into(), 1, b"title".to_vec(), b"body".to_vec(), Some(>::from(5000u32)), - 0, + 20, + 10, ) }, successful_call: || { - ProposalCodex::create_set_content_working_group_mint_capacity_proposal( + ProposalCodex::create_spending_proposal( RawOrigin::Signed(1).into(), 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), - 0, + Some(>::from(1250u32)), + 100, + 2, ) }, - proposal_parameters: crate::proposal_types::parameters::set_content_working_group_mint_capacity_proposal::(), + proposal_parameters: crate::proposal_types::parameters::spending_proposal::(), + proposal_details: ProposalDetails::Spending(100, 2), }; proposal_fixture.check_all(); }); } #[test] -fn create_spending_proposal_common_checks_succeed() { +fn create_spending_proposal_call_fails_with_incorrect_balance() { initial_test_ext().execute_with(|| { + increase_total_balance_issuance_using_account_id(500000, 1); + + assert_eq!( + ProposalCodex::create_spending_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(1250u32)), + 0, + 2, + ), + 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(1, 500000); + + 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) + ); + }); +} + +#[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_spending_proposal( + ProposalCodex::create_set_lead_proposal( RawOrigin::None.into(), 1, b"title".to_vec(), b"body".to_vec(), None, - 20, - 10, + Some((20, 10)), ) }, empty_stake_call: || { - ProposalCodex::create_spending_proposal( + ProposalCodex::create_set_lead_proposal( RawOrigin::Signed(1).into(), 1, b"title".to_vec(), b"body".to_vec(), None, - 20, - 10, + Some((20, 10)), ) }, invalid_stake_call: || { - ProposalCodex::create_spending_proposal( + ProposalCodex::create_set_lead_proposal( RawOrigin::Signed(1).into(), 1, b"title".to_vec(), b"body".to_vec(), Some(>::from(5000u32)), - 20, - 10, + Some((20, 10)), ) }, successful_call: || { - ProposalCodex::create_spending_proposal( + ProposalCodex::create_set_lead_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(1250u32)), + Some((20, 10)), + ) + }, + proposal_parameters: crate::proposal_types::parameters::set_lead_proposal::(), + proposal_details: ProposalDetails::SetLead(Some((20, 10))), + }; + proposal_fixture.check_all(); + }); +} + +#[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( + 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)), - 100, - 2, + 1, ) }, - proposal_parameters: crate::proposal_types::parameters::spending_proposal::(), + proposal_parameters: crate::proposal_types::parameters::evict_storage_provider_proposal::(), + proposal_details: ProposalDetails::EvictStorageProvider(1), }; proposal_fixture.check_all(); }); } #[test] -fn create_spending_proposal_call_fails_with_incorrect_balance() { +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( + RawOrigin::None.into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + None, + 4, + ) + }, + empty_stake_call: || { + ProposalCodex::create_set_validator_count_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + None, + 4, + ) + }, + 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)), + 4, + ) + }, + successful_call: || { + ProposalCodex::create_set_validator_count_proposal( + RawOrigin::Signed(1).into(), + 1, + b"title".to_vec(), + b"body".to_vec(), + Some(>::from(1250u32)), + 4, + ) + }, + proposal_parameters: crate::proposal_types::parameters::set_validator_count_proposal::< + Test, + >(), + 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_spending_proposal( + ProposalCodex::create_set_validator_count_proposal( RawOrigin::Signed(1).into(), 1, b"title".to_vec(), b"body".to_vec(), Some(>::from(500u32)), - 0, - 2, + 3, + ), + 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::SpendingProposalZeroBalance) + Err(Error::InvalidValidatorCount) ); }); } #[test] -fn create_set_lead_proposal_common_checks_succeed() { +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_lead_proposal( + ProposalCodex::create_set_storage_role_parameters_proposal( RawOrigin::None.into(), 1, b"title".to_vec(), b"body".to_vec(), None, - Some((20, 10)), + RoleParameters::default(), ) }, empty_stake_call: || { - ProposalCodex::create_set_lead_proposal( + ProposalCodex::create_set_storage_role_parameters_proposal( RawOrigin::Signed(1).into(), 1, b"title".to_vec(), b"body".to_vec(), None, - Some((20, 10)), + RoleParameters::default(), ) }, invalid_stake_call: || { - ProposalCodex::create_set_lead_proposal( + ProposalCodex::create_set_storage_role_parameters_proposal( RawOrigin::Signed(1).into(), 1, b"title".to_vec(), b"body".to_vec(), Some(>::from(5000u32)), - Some((20, 10)), + RoleParameters::default(), ) }, successful_call: || { - ProposalCodex::create_set_lead_proposal( + ProposalCodex::create_set_storage_role_parameters_proposal( RawOrigin::Signed(1).into(), 1, b"title".to_vec(), b"body".to_vec(), - Some(>::from(500u32)), - Some((20, 10)), + Some(>::from(1250u32)), + RoleParameters::default(), ) }, - proposal_parameters: crate::proposal_types::parameters::set_lead_proposal::(), + proposal_parameters: + crate::proposal_types::parameters::set_storage_role_parameters_proposal::(), + proposal_details: ProposalDetails::SetStorageRoleParameters(RoleParameters::default()), }; 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(|| { + increase_total_balance_issuance(500000); + + let mut role_parameters = RoleParameters::default(); + role_parameters.min_actors = 6; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterMinActors, + ); + + role_parameters = RoleParameters::default(); + role_parameters.max_actors = 4; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterMaxActors, + ); + + role_parameters = RoleParameters::default(); + 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 = 599; + assert_failed_set_storage_parameters_call( + role_parameters, + Error::InvalidStorageRoleParameterBondingPeriod, + ); + + role_parameters = RoleParameters::default(); + 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 = 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 = 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, + ); + }); +} diff --git a/runtime-modules/proposals/discussion/src/lib.rs b/runtime-modules/proposals/discussion/src/lib.rs index 7820307efc..19ff49ba0d 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. +//! It 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 an 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, @@ -168,7 +199,7 @@ decl_module! { ) { T::PostAuthorOriginValidator::ensure_actor_origin( origin, - post_author_id.clone(), + post_author_id, )?; ensure!(>::exists(thread_id), Error::ThreadDoesntExist); @@ -187,7 +218,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, }; @@ -208,7 +239,7 @@ decl_module! { ){ T::PostAuthorOriginValidator::ensure_actor_origin( origin, - post_author_id.clone(), + post_author_id, )?; ensure!(>::exists(thread_id), Error::ThreadDoesntExist); @@ -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, &title)?; let next_thread_count_value = Self::thread_count() + 1; let new_thread_id = next_thread_count_value; @@ -261,11 +287,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 @@ -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!( @@ -307,7 +319,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(), @@ -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/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 19975197f9..733f83dd3e 100644 --- a/runtime-modules/proposals/engine/src/lib.rs +++ b/runtime-modules/proposals/engine/src/lib.rs @@ -1,18 +1,107 @@ -//! 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. +//! The 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 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. A 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). +//! 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. //! +//! ### 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. +//! - _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 an interface for the staking. +//! +//! 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 +//! - [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, 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, +//! ) -> 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 = >::executable_proposal().encode(); +//! +//! >::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)] @@ -128,6 +217,7 @@ decl_event!( ); decl_error! { + /// Engine module predefined errors pub enum Error { /// Proposal cannot have an empty title" EmptyTitleProvided, @@ -229,7 +319,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); @@ -239,7 +329,7 @@ decl_module! { let did_not_vote_before = !>::exists( proposal_id, - voter_id.clone(), + voter_id, ); ensure!(did_not_vote_before, Error::AlreadyVoted); @@ -249,7 +339,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)); } @@ -257,7 +347,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); @@ -278,11 +368,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 @@ -358,7 +451,7 @@ impl Module { parameters, title, description, - proposer_id: proposer_id.clone(), + proposer_id, status: ProposalStatus::Active(stake_data), voting_results: VotingResults::default(), }; @@ -464,11 +557,12 @@ 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| { proposal.reset_proposal(); + >::remove_prefix(&proposal_id); }); }); } @@ -508,6 +602,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); @@ -548,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()); @@ -604,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 df2e49a9bf..5cdc3ec0e0 100644 --- a/runtime-modules/proposals/engine/src/tests/mod.rs +++ b/runtime-modules/proposals/engine/src/tests/mod.rs @@ -6,12 +6,20 @@ 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}; 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,6 +842,79 @@ fn proposal_execution_postponed_because_of_grace_period() { }); } +#[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(|| { @@ -1339,6 +1435,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 +1467,115 @@ fn proposal_reset_succeeds() { slashes: 0, } ); + + // whole double map prefix was removed (should return default value) + assert_eq!( + >::get(&proposal_id, &2), + VoteKind::default() + ); + }); +} + +#[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/mod.rs b/runtime-modules/proposals/engine/src/types/mod.rs index 4ced301e21..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; @@ -222,7 +223,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; @@ -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 @@ -369,6 +370,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 +674,120 @@ 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, + slashing_threshold_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 { + approval_quorum_percentage: 63, + 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 { + slashing_threshold_percentage: 63, + 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, + approval_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()); + } } 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 + ); + } +} 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 diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 8a9da4e4cc..2eea0aca46 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; }