diff --git a/Cargo.lock b/Cargo.lock index 0060543e01b27..f8978025a51ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4155,6 +4155,7 @@ dependencies = [ "sp-runtime", "sp-std", "sp-storage", + "substrate-test-utils", ] [[package]] diff --git a/bin/node/executor/tests/basic.rs b/bin/node/executor/tests/basic.rs index 47d5f3f3844e5..db6cb88bb3c7a 100644 --- a/bin/node/executor/tests/basic.rs +++ b/bin/node/executor/tests/basic.rs @@ -20,12 +20,10 @@ use frame_support::{ traits::Currency, weights::{GetDispatchInfo, DispatchInfo, DispatchClass, constants::ExtrinsicBaseWeight}, }; -use sp_core::{ - NeverNativeValue, map, traits::Externalities, storage::{well_known_keys, Storage}, -}; +use sp_core::{NeverNativeValue, traits::Externalities, storage::well_known_keys}; use sp_runtime::{ ApplyExtrinsicResult, Fixed128, - traits::{Hash as HashT, Convert, BlakeTwo256}, + traits::{Hash as HashT, Convert}, transaction_validity::InvalidTransaction, }; use pallet_contracts::ContractAddressFor; @@ -158,20 +156,13 @@ fn block_with_size(time: u64, nonce: u32, size: usize) -> (Vec, Hash) { #[test] fn panic_execution_with_foreign_code_gives_error() { - let mut t = TestExternalities::::new_with_code(BLOATY_CODE, Storage { - top: map![ - >::hashed_key_for(alice()) => { - (69u128, 0u8, 0u128, 0u128, 0u128).encode() - }, - >::hashed_key().to_vec() => { - 69_u128.encode() - }, - >::hashed_key_for(0) => { - vec![0u8; 32] - } - ], - children_default: map![], - }); + let mut t = new_test_ext(BLOATY_CODE, false); + t.insert( + >::hashed_key_for(alice()), + (69u128, 0u8, 0u128, 0u128, 0u128).encode() + ); + t.insert(>::hashed_key().to_vec(), 69_u128.encode()); + t.insert(>::hashed_key_for(0), vec![0u8; 32]); let r = executor_call:: _>( &mut t, @@ -194,20 +185,13 @@ fn panic_execution_with_foreign_code_gives_error() { #[test] fn bad_extrinsic_with_native_equivalent_code_gives_error() { - let mut t = TestExternalities::::new_with_code(COMPACT_CODE, Storage { - top: map![ - >::hashed_key_for(alice()) => { - (0u32, 0u8, 69u128, 0u128, 0u128, 0u128).encode() - }, - >::hashed_key().to_vec() => { - 69_u128.encode() - }, - >::hashed_key_for(0) => { - vec![0u8; 32] - } - ], - children_default: map![], - }); + let mut t = new_test_ext(COMPACT_CODE, false); + t.insert( + >::hashed_key_for(alice()), + (0u32, 0u8, 69u128, 0u128, 0u128, 0u128).encode() + ); + t.insert(>::hashed_key().to_vec(), 69_u128.encode()); + t.insert(>::hashed_key_for(0), vec![0u8; 32]); let r = executor_call:: _>( &mut t, @@ -230,18 +214,20 @@ fn bad_extrinsic_with_native_equivalent_code_gives_error() { #[test] fn successful_execution_with_native_equivalent_code_gives_ok() { - let mut t = TestExternalities::::new_with_code(COMPACT_CODE, Storage { - top: map![ - >::hashed_key_for(alice()) => { - (0u32, 0u8, 111 * DOLLARS, 0u128, 0u128, 0u128).encode() - }, - >::hashed_key().to_vec() => { - (111 * DOLLARS).encode() - }, - >::hashed_key_for(0) => vec![0u8; 32] - ], - children_default: map![], - }); + let mut t = new_test_ext(COMPACT_CODE, false); + t.insert( + >::hashed_key_for(alice()), + (0u32, 0u8, 111 * DOLLARS, 0u128, 0u128, 0u128).encode() + ); + t.insert( + >::hashed_key_for(bob()), + (0u32, 0u8, 0 * DOLLARS, 0u128, 0u128, 0u128).encode() + ); + t.insert( + >::hashed_key().to_vec(), + (111 * DOLLARS).encode() + ); + t.insert(>::hashed_key_for(0), vec![0u8; 32]); let r = executor_call:: _>( &mut t, @@ -272,18 +258,20 @@ fn successful_execution_with_native_equivalent_code_gives_ok() { #[test] fn successful_execution_with_foreign_code_gives_ok() { - let mut t = TestExternalities::::new_with_code(BLOATY_CODE, Storage { - top: map![ - >::hashed_key_for(alice()) => { - (0u32, 0u8, 111 * DOLLARS, 0u128, 0u128, 0u128).encode() - }, - >::hashed_key().to_vec() => { - (111 * DOLLARS).encode() - }, - >::hashed_key_for(0) => vec![0u8; 32] - ], - children_default: map![], - }); + let mut t = new_test_ext(BLOATY_CODE, false); + t.insert( + >::hashed_key_for(alice()), + (0u32, 0u8, 111 * DOLLARS, 0u128, 0u128, 0u128).encode() + ); + t.insert( + >::hashed_key_for(bob()), + (0u32, 0u8, 0 * DOLLARS, 0u128, 0u128, 0u128).encode() + ); + t.insert( + >::hashed_key().to_vec(), + (111 * DOLLARS).encode() + ); + t.insert(>::hashed_key_for(0), vec![0u8; 32]); let r = executor_call:: _>( &mut t, @@ -707,15 +695,13 @@ fn native_big_block_import_fails_on_fallback() { #[test] fn panic_execution_gives_error() { - let mut t = TestExternalities::::new_with_code(BLOATY_CODE, Storage { - top: map![ - >::hashed_key().to_vec() => { - 0_u128.encode() - }, - >::hashed_key_for(0) => vec![0u8; 32] - ], - children_default: map![], - }); + let mut t = new_test_ext(BLOATY_CODE, false); + t.insert( + >::hashed_key_for(alice()), + (0u32, 0u8, 0 * DOLLARS, 0u128, 0u128, 0u128).encode() + ); + t.insert(>::hashed_key().to_vec(), 0_u128.encode()); + t.insert(>::hashed_key_for(0), vec![0u8; 32]); let r = executor_call:: _>( &mut t, @@ -738,18 +724,20 @@ fn panic_execution_gives_error() { #[test] fn successful_execution_gives_ok() { - let mut t = TestExternalities::::new_with_code(COMPACT_CODE, Storage { - top: map![ - >::hashed_key_for(alice()) => { - (0u32, 0u8, 111 * DOLLARS, 0u128, 0u128, 0u128).encode() - }, - >::hashed_key().to_vec() => { - (111 * DOLLARS).encode() - }, - >::hashed_key_for(0) => vec![0u8; 32] - ], - children_default: map![], - }); + let mut t = new_test_ext(COMPACT_CODE, false); + t.insert( + >::hashed_key_for(alice()), + (0u32, 0u8, 111 * DOLLARS, 0u128, 0u128, 0u128).encode() + ); + t.insert( + >::hashed_key_for(bob()), + (0u32, 0u8, 0 * DOLLARS, 0u128, 0u128, 0u128).encode() + ); + t.insert( + >::hashed_key().to_vec(), + (111 * DOLLARS).encode() + ); + t.insert(>::hashed_key_for(0), vec![0u8; 32]); let r = executor_call:: _>( &mut t, @@ -759,7 +747,12 @@ fn successful_execution_gives_ok() { None, ).0; assert!(r.is_ok()); + t.execute_with(|| { + assert_eq!(Balances::total_balance(&alice()), 111 * DOLLARS); + }); + let fm = t.execute_with(TransactionPayment::next_fee_multiplier); + let r = executor_call:: _>( &mut t, "BlockBuilder_apply_extrinsic", diff --git a/bin/node/executor/tests/fees.rs b/bin/node/executor/tests/fees.rs index c7d6bb34cd5d9..504f6d8270f5f 100644 --- a/bin/node/executor/tests/fees.rs +++ b/bin/node/executor/tests/fees.rs @@ -20,8 +20,8 @@ use frame_support::{ traits::Currency, weights::{GetDispatchInfo, constants::ExtrinsicBaseWeight}, }; -use sp_core::{NeverNativeValue, map, storage::Storage}; -use sp_runtime::{Fixed128, Perbill, traits::{Convert, BlakeTwo256}}; +use sp_core::NeverNativeValue; +use sp_runtime::{Fixed128, Perbill, traits::Convert}; use node_runtime::{ CheckedExtrinsic, Call, Runtime, Balances, TransactionPayment, TransactionByteFee, WeightFeeCoefficient, @@ -130,21 +130,20 @@ fn transaction_fee_is_correct_ultimate() { // - 1 MILLICENTS in substrate node. // - 1 milli-dot based on current polkadot runtime. // (this baed on assigning 0.1 CENT to the cheapest tx with `weight = 100`) - let mut t = TestExternalities::::new_with_code(COMPACT_CODE, Storage { - top: map![ - >::hashed_key_for(alice()) => { - (0u32, 0u8, 100 * DOLLARS, 0 * DOLLARS, 0 * DOLLARS, 0 * DOLLARS).encode() - }, - >::hashed_key_for(bob()) => { - (0u32, 0u8, 10 * DOLLARS, 0 * DOLLARS, 0 * DOLLARS, 0 * DOLLARS).encode() - }, - >::hashed_key().to_vec() => { - (110 * DOLLARS).encode() - }, - >::hashed_key_for(0) => vec![0u8; 32] - ], - children_default: map![], - }); + let mut t = new_test_ext(COMPACT_CODE, false); + t.insert( + >::hashed_key_for(alice()), + (0u32, 0u8, 100 * DOLLARS, 0 * DOLLARS, 0 * DOLLARS, 0 * DOLLARS).encode() + ); + t.insert( + >::hashed_key_for(bob()), + (0u32, 0u8, 10 * DOLLARS, 0 * DOLLARS, 0 * DOLLARS, 0 * DOLLARS).encode() + ); + t.insert( + >::hashed_key().to_vec(), + (110 * DOLLARS).encode() + ); + t.insert(>::hashed_key_for(0), vec![0u8; 32]); let tip = 1_000_000; let xt = sign(CheckedExtrinsic { diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 1eec930dfbb72..7214d30d1bce0 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -342,6 +342,7 @@ parameter_types! { pub const CooloffPeriod: BlockNumber = 28 * 24 * 60 * MINUTES; // One cent: $10,000 / MB pub const PreimageByteDeposit: Balance = 1 * CENTS; + pub const MaxVotes: u32 = 100; } impl pallet_democracy::Trait for Runtime { @@ -374,6 +375,7 @@ impl pallet_democracy::Trait for Runtime { type PreimageByteDeposit = PreimageByteDeposit; type Slash = Treasury; type Scheduler = Scheduler; + type MaxVotes = MaxVotes; } parameter_types! { diff --git a/frame/democracy/Cargo.toml b/frame/democracy/Cargo.toml index 83caa671cee95..f0d2867f22492 100644 --- a/frame/democracy/Cargo.toml +++ b/frame/democracy/Cargo.toml @@ -26,6 +26,7 @@ sp-core = { version = "2.0.0-dev", path = "../../primitives/core" } pallet-balances = { version = "2.0.0-dev", path = "../balances" } pallet-scheduler = { version = "2.0.0-dev", path = "../scheduler" } sp-storage = { version = "2.0.0-dev", path = "../../primitives/storage" } +substrate-test-utils = { version = "2.0.0-dev", path = "../../test-utils" } hex-literal = "0.2.1" [features] @@ -43,5 +44,6 @@ std = [ runtime-benchmarks = [ "frame-benchmarking", "frame-system/runtime-benchmarks", + "frame-support/runtime-benchmarks", "sp-runtime/runtime-benchmarks", ] diff --git a/frame/democracy/src/benchmarking.rs b/frame/democracy/src/benchmarking.rs index d56113846545b..8b4485f46f493 100644 --- a/frame/democracy/src/benchmarking.rs +++ b/frame/democracy/src/benchmarking.rs @@ -33,7 +33,6 @@ const MAX_USERS: u32 = 1000; const MAX_REFERENDUMS: u32 = 100; const MAX_PROPOSALS: u32 = 100; const MAX_SECONDERS: u32 = 100; -const MAX_VETOERS: u32 = 100; const MAX_BYTES: u32 = 16_384; fn assert_last_event(generic_event: ::Event) { @@ -55,7 +54,11 @@ fn add_proposal(n: u32) -> Result { let value = T::MinimumDeposit::get(); let proposal_hash: T::Hash = T::Hashing::hash_of(&n); - Democracy::::propose(RawOrigin::Signed(other).into(), proposal_hash, value.into())?; + Democracy::::propose( + RawOrigin::Signed(other).into(), + proposal_hash, + value.into(), + )?; Ok(proposal_hash) } @@ -133,15 +136,15 @@ benchmarks! { // Create s existing "seconds" for i in 0 .. s { let seconder = funded_account::("seconder", i); - Democracy::::second(RawOrigin::Signed(seconder).into(), 0)?; + Democracy::::second(RawOrigin::Signed(seconder).into(), 0, u32::max_value())?; } let deposits = Democracy::::deposit_of(0).ok_or("Proposal not created")?; - assert_eq!(deposits.1.len(), (s + 1) as usize, "Seconds not recorded"); - }: _(RawOrigin::Signed(caller), 0) + assert_eq!(deposits.0.len(), (s + 1) as usize, "Seconds not recorded"); + }: _(RawOrigin::Signed(caller), 0, u32::max_value()) verify { let deposits = Democracy::::deposit_of(0).ok_or("Proposal not created")?; - assert_eq!(deposits.1.len(), (s + 2) as usize, "`second` benchmark did not work"); + assert_eq!(deposits.0.len(), (s + 2) as usize, "`second` benchmark did not work"); } vote_new { @@ -299,7 +302,7 @@ benchmarks! { // Worst case scenario, we external propose a previously blacklisted proposal external_propose { let p in 1 .. MAX_PROPOSALS; - let v in 1 .. MAX_VETOERS; + let v in 1 .. MAX_VETOERS as u32; let origin = T::ExternalOrigin::successful_origin(); let proposal_hash = T::Hashing::hash_of(&p); @@ -360,7 +363,7 @@ benchmarks! { veto_external { // Existing veto-ers - let v in 0 .. MAX_VETOERS; + let v in 0 .. MAX_VETOERS as u32; let proposal_hash: T::Hash = T::Hashing::hash_of(&v); @@ -684,7 +687,7 @@ benchmarks! { assert!(Preimages::::contains_key(proposal_hash)); let caller = funded_account::("caller", 0); - }: _(RawOrigin::Signed(caller), proposal_hash.clone()) + }: _(RawOrigin::Signed(caller), proposal_hash.clone(), u32::max_value()) verify { let proposal_hash = T::Hashing::hash(&encoded_proposal[..]); assert!(!Preimages::::contains_key(proposal_hash)); diff --git a/frame/democracy/src/lib.rs b/frame/democracy/src/lib.rs index a182907aba222..0136967ea7881 100644 --- a/frame/democracy/src/lib.rs +++ b/frame/democracy/src/lib.rs @@ -168,14 +168,16 @@ use sp_runtime::{ DispatchResult, DispatchError, RuntimeDebug, traits::{Zero, Hash, Dispatchable, Saturating}, }; -use codec::{Encode, Decode}; +use codec::{Encode, Decode, Input}; use frame_support::{ decl_module, decl_storage, decl_event, decl_error, ensure, Parameter, + storage::IterableStorageMap, weights::{Weight, DispatchClass}, traits::{ Currency, ReservableCurrency, LockableCurrency, WithdrawReason, LockIdentifier, Get, OnUnbalanced, BalanceStatus, schedule::Named as ScheduleNamed, EnsureOrigin - } + }, + dispatch::DispatchResultWithPostInfo, }; use frame_system::{self as system, ensure_signed, ensure_root}; @@ -196,6 +198,11 @@ pub mod benchmarking; const DEMOCRACY_ID: LockIdentifier = *b"democrac"; +/// The maximum number of vetoers on a single proposal used to compute Weight. +/// +/// NOTE: This is not enforced by any logic. +pub const MAX_VETOERS: Weight = 100; + /// A proposal index. pub type PropIndex = u32; @@ -264,6 +271,11 @@ pub trait Trait: frame_system::Trait + Sized { type CancellationOrigin: EnsureOrigin; /// Origin for anyone able to veto proposals. + /// + /// # Warning + /// + /// The number of Vetoers for a proposal must be small, extrinsics are weighted according to + /// [MAX_VETOERS](./const.MAX_VETOERS.html) type VetoOrigin: EnsureOrigin; /// Period in blocks where an external proposal may not be re-submitted after being vetoed. @@ -277,6 +289,12 @@ pub trait Trait: frame_system::Trait + Sized { /// The Scheduler. type Scheduler: ScheduleNamed; + + /// The maximum number of votes for an account. + /// + /// Also used to compute weight, an overly big value can + /// lead to extrinsic with very big weight: see `delegate` for instance. + type MaxVotes: Get; } #[derive(Clone, Encode, Decode, RuntimeDebug)] @@ -303,6 +321,14 @@ impl PreimageStatus as Democracy { // TODO: Refactor public proposal queue into its own pallet. @@ -313,7 +339,7 @@ decl_storage! { pub PublicProps get(fn public_props): Vec<(PropIndex, T::Hash, T::AccountId)>; /// Those who have locked a deposit. pub DepositOf get(fn deposit_of): - map hasher(twox_64_concat) PropIndex => Option<(BalanceOf, Vec)>; + map hasher(twox_64_concat) PropIndex => Option<(Vec, BalanceOf)>; /// Map of hashes to the proposal preimage, along with who registered it and their deposit. /// The block number is the block at which it was deposited. @@ -367,6 +393,11 @@ decl_storage! { /// Record of all proposals that have been subject to emergency cancellation. pub Cancellations: map hasher(identity) T::Hash => bool; + + /// Storage version of the pallet. + /// + /// New networks start with last version. + StorageVersion build(|_| Some(Releases::V1)): Option; } } @@ -491,6 +522,62 @@ decl_error! { InstantNotAllowed, /// Delegation to oneself makes no sense. Nonsense, + /// Invalid upper bound. + WrongUpperBound, + /// Maximum number of votes reached. + MaxVotesReached, + } +} + +/// Functions for calcuating the weight of some dispatchables. +mod weight_for { + use frame_support::{traits::Get, weights::Weight}; + use super::Trait; + + /// Calculate the weight for `delegate`. + /// - Db reads: 2*`VotingOf`, `balances locks` + /// - Db writes: 2*`VotingOf`, `balances locks` + /// - Db reads per votes: `ReferendumInfoOf` + /// - Db writes per votes: `ReferendumInfoOf` + /// - Base Weight: 65.78 + 8.229 * R µs + // NOTE: weight must cover an incorrect voting of origin with 100 votes. + pub(crate) fn delegate(votes: Weight) -> Weight { + T::DbWeight::get().reads_writes(votes.saturating_add(3), votes.saturating_add(3)) + .saturating_add(66_000_000) + .saturating_add(votes.saturating_mul(8_100_000)) + } + + /// Calculate the weight for `undelegate`. + /// - Db reads: 2*`VotingOf` + /// - Db writes: 2*`VotingOf` + /// - Db reads per votes: `ReferendumInfoOf` + /// - Db writes per votes: `ReferendumInfoOf` + /// - Base Weight: 33.29 + 8.104 * R µs + pub(crate) fn undelegate(votes: Weight) -> Weight { + T::DbWeight::get().reads_writes(votes.saturating_add(2), votes.saturating_add(2)) + .saturating_add(33_000_000) + .saturating_add(votes.saturating_mul(8_000_000)) + } + + /// Calculate the weight for `proxy_delegate`. + /// same as `delegate with additional: + /// - Db reads: `Proxy`, `proxy account` + /// - Db writes: `proxy account` + /// - Base Weight: 68.61 + 8.039 * R µs + pub(crate) fn proxy_delegate(votes: Weight) -> Weight { + T::DbWeight::get().reads_writes(votes.saturating_add(5), votes.saturating_add(4)) + .saturating_add(69_000_000) + .saturating_add(votes.saturating_mul(8_000_000)) + } + + /// Calculate the weight for `proxy_undelegate`. + /// same as `undelegate with additional: + /// Db reads: `Proxy` + /// Base Weight: 39 + 7.958 * R µs + pub(crate) fn proxy_undelegate(votes: Weight) -> Weight { + T::DbWeight::get().reads_writes(votes.saturating_add(3), votes.saturating_add(2)) + .saturating_add(40_000_000) + .saturating_add(votes.saturating_mul(8_000_000)) } } @@ -525,6 +612,22 @@ decl_module! { fn deposit_event() = default; + fn on_runtime_upgrade() -> Weight { + if let None = StorageVersion::get() { + StorageVersion::put(Releases::V1); + + DepositOf::::translate::< + (BalanceOf, Vec), _ + >(|_, (balance, accounts)| { + Some((accounts, balance)) + }); + + T::MaximumBlockWeight::get() + } else { + T::DbWeight::get().reads(1) + } + } + /// Propose a sensitive action to be taken. /// /// The dispatch origin of this call must be _Signed_ and the sender must @@ -536,22 +639,22 @@ decl_module! { /// Emits `Proposed`. /// /// # - /// - `O(P)` - /// - P is the number proposals in the `PublicProps` vec. - /// - Two DB changes, one DB entry. + /// - Complexity: `O(1)` + /// - Db reads: `PublicPropCount`, `PublicProps` + /// - Db writes: `PublicPropCount`, `PublicProps`, `DepositOf` + /// ------------------- + /// Base Weight: 42.58 + .127 * P µs with `P` the number of proposals `PublicProps` /// # - #[weight = 5_000_000_000] - fn propose(origin, - proposal_hash: T::Hash, - #[compact] value: BalanceOf - ) { + #[weight = 50_000_000 + T::DbWeight::get().reads_writes(2, 3)] + fn propose(origin, proposal_hash: T::Hash, #[compact] value: BalanceOf) { let who = ensure_signed(origin)?; ensure!(value >= T::MinimumDeposit::get(), Error::::ValueLow); + T::Currency::reserve(&who, value)?; let index = Self::public_prop_count(); PublicPropCount::put(index + 1); - >::insert(index, (value, &[&who][..])); + >::insert(index, (&[&who][..], value)); >::append((index, proposal_hash, who)); @@ -564,19 +667,30 @@ decl_module! { /// must have funds to cover the deposit, equal to the original deposit. /// /// - `proposal`: The index of the proposal to second. + /// - `seconds_upper_bound`: an upper bound on the current number of seconds on this + /// proposal. Extrinsic is weighted according to this value with no refund. /// /// # - /// - `O(S)`. - /// - S is the number of seconds a proposal already has. - /// - One DB entry. + /// - Complexity: `O(S)` where S is the number of seconds a proposal already has. + /// - Db reads: `DepositOf` + /// - Db writes: `DepositOf` + /// --------- + /// - Base Weight: 22.28 + .229 * S µs /// # - #[weight = 5_000_000_000] - fn second(origin, #[compact] proposal: PropIndex) { + #[weight = 23_000_000 + .saturating_add(230_000.saturating_mul(Weight::from(*seconds_upper_bound))) + .saturating_add(T::DbWeight::get().reads_writes(1, 1)) + ] + fn second(origin, #[compact] proposal: PropIndex, #[compact] seconds_upper_bound: u32) { let who = ensure_signed(origin)?; + + let seconds = Self::len_of_deposit_of(proposal) + .ok_or_else(|| Error::::ProposalMissing)?; + ensure!(seconds <= seconds_upper_bound, Error::::WrongUpperBound); let mut deposit = Self::deposit_of(proposal) .ok_or(Error::::ProposalMissing)?; - T::Currency::reserve(&who, deposit.0)?; - deposit.1.push(who); + T::Currency::reserve(&who, deposit.1)?; + deposit.0.push(who); >::insert(proposal, deposit); } @@ -589,11 +703,16 @@ decl_module! { /// - `vote`: The vote configuration. /// /// # - /// - `O(R)`. - /// - R is the number of referendums the voter has voted on. - /// - One DB change, one DB entry. + /// - Complexity: `O(R)` where R is the number of referendums the voter has voted on. + /// weight is charged as if maximum votes. + /// - Db reads: `ReferendumInfoOf`, `VotingOf`, `balances locks` + /// - Db writes: `ReferendumInfoOf`, `VotingOf`, `balances locks` + /// -------------------- + /// - Base Weight: + /// - Vote New: 49.24 + .333 * R µs + /// - Vote Existing: 49.94 + .343 * R µs /// # - #[weight = 200_000_000] + #[weight = 50_000_000 + 350_000 * Weight::from(T::MaxVotes::get()) + T::DbWeight::get().reads_writes(3, 3)] fn vote(origin, #[compact] ref_index: ReferendumIndex, vote: AccountVote>, @@ -611,10 +730,16 @@ decl_module! { /// - `vote`: The vote configuration. /// /// # - /// - `O(1)`. - /// - One DB change, one DB entry. + /// - Complexity: `O(R)` where R is the number of referendums the proxy has voted on. + /// weight is charged as if maximum votes. + /// - Db reads: `ReferendumInfoOf`, `VotingOf`, `balances locks`, `Proxy`, `proxy account` + /// - Db writes: `ReferendumInfoOf`, `VotingOf`, `balances locks` + /// ------------ + /// - Base Weight: + /// - Proxy Vote New: 54.35 + .344 * R µs + /// - Proxy Vote Existing: 54.35 + .35 * R µs /// # - #[weight = 200_000_000] + #[weight = 55_000_000 + 350_000 * Weight::from(T::MaxVotes::get()) + T::DbWeight::get().reads_writes(5, 3)] fn proxy_vote(origin, #[compact] ref_index: ReferendumIndex, vote: AccountVote>, @@ -632,9 +757,13 @@ decl_module! { /// -`ref_index`: The index of the referendum to cancel. /// /// # - /// - `O(1)`. + /// - Complexity: `O(1)`. + /// - Db reads: `ReferendumInfoOf`, `Cancellations` + /// - Db writes: `ReferendumInfoOf`, `Cancellations` + /// ------------- + /// - Base Weight: 34.25 µs /// # - #[weight = (500_000_000, DispatchClass::Operational)] + #[weight = (35_000_000 + T::DbWeight::get().reads_writes(2, 2), DispatchClass::Operational)] fn emergency_cancel(origin, ref_index: ReferendumIndex) { T::CancellationOrigin::ensure_origin(origin)?; @@ -654,10 +783,13 @@ decl_module! { /// - `proposal_hash`: The preimage hash of the proposal. /// /// # - /// - `O(1)`. - /// - One DB change. + /// - Complexity `O(V)` with V number of vetoers in the blacklist of proposal. + /// Decoding vec of length V. Charged as maximum + /// - Db reads: `NextExternal`, `Blacklist` + /// - Db writes: `NextExternal` + /// - Base Weight: 13.8 + .106 * V µs /// # - #[weight = 5_000_000_000] + #[weight = 15_000_000 + 110_000 * MAX_VETOERS + T::DbWeight::get().reads_writes(2, 1)] fn external_propose(origin, proposal_hash: T::Hash) { T::ExternalOrigin::ensure_origin(origin)?; ensure!(!>::exists(), Error::::DuplicateProposal); @@ -681,10 +813,11 @@ decl_module! { /// pre-scheduled `external_propose` call. /// /// # - /// - `O(1)`. - /// - One DB change. + /// - Complexity: `O(1)` + /// - Db write: `NextExternal` + /// - Base Weight: 3.065 µs /// # - #[weight = 5_000_000_000] + #[weight = 3_100_000 + T::DbWeight::get().writes(1)] fn external_propose_majority(origin, proposal_hash: T::Hash) { T::ExternalMajorityOrigin::ensure_origin(origin)?; >::put((proposal_hash, VoteThreshold::SimpleMajority)); @@ -701,10 +834,11 @@ decl_module! { /// pre-scheduled `external_propose` call. /// /// # - /// - `O(1)`. - /// - One DB change. + /// - Complexity: `O(1)` + /// - Db write: `NextExternal` + /// - Base Weight: 3.087 µs /// # - #[weight = 5_000_000_000] + #[weight = 3_100_000 + T::DbWeight::get().writes(1)] fn external_propose_default(origin, proposal_hash: T::Hash) { T::ExternalDefaultOrigin::ensure_origin(origin)?; >::put((proposal_hash, VoteThreshold::SuperMajorityAgainst)); @@ -725,11 +859,12 @@ decl_module! { /// Emits `Started`. /// /// # - /// - One DB clear. - /// - One DB change. - /// - One extra DB entry. + /// - Complexity: `O(1)` + /// - Db reads: `NextExternal`, `ReferendumCount` + /// - Db writes: `NextExternal`, `ReferendumCount`, `ReferendumInfoOf` + /// - Base Weight: 30.1 µs /// # - #[weight = 200_000_000] + #[weight = 30_000_000 + T::DbWeight::get().reads_writes(2, 3)] fn fast_track(origin, proposal_hash: T::Hash, voting_period: T::BlockNumber, @@ -774,13 +909,13 @@ decl_module! { /// Emits `Vetoed`. /// /// # - /// - Two DB entries. - /// - One DB clear. - /// - Performs a binary search on `existing_vetoers` which should not - /// be very large. - /// - O(log v), v is number of `existing_vetoers` + /// - Complexity: `O(V + log(V))` where V is number of `existing vetoers` + /// Performs a binary search on `existing_vetoers` which should not be very large. + /// - Db reads: `NextExternal`, `Blacklist` + /// - Db writes: `NextExternal`, `Blacklist` + /// - Base Weight: 29.87 + .188 * V µs /// # - #[weight = 200_000_000] + #[weight = 30_000_000 + 180_000 * MAX_VETOERS + T::DbWeight::get().reads_writes(2, 2)] fn veto_external(origin, proposal_hash: T::Hash) { let who = T::VetoOrigin::ensure_origin(origin)?; @@ -811,9 +946,11 @@ decl_module! { /// - `ref_index`: The index of the referendum to cancel. /// /// # - /// - `O(1)`. + /// - Complexity: `O(1)`. + /// - Db writes: `ReferendumInfoOf` + /// - Base Weight: 21.57 µs /// # - #[weight = (0, DispatchClass::Operational)] + #[weight = (22_000_000 + T::DbWeight::get().writes(1), DispatchClass::Operational)] fn cancel_referendum(origin, #[compact] ref_index: ReferendumIndex) { ensure_root(origin)?; Self::internal_cancel_referendum(ref_index); @@ -826,22 +963,24 @@ decl_module! { /// - `which`: The index of the referendum to cancel. /// /// # - /// - One DB change. - /// - O(d) where d is the items in the dispatch queue. + /// - `O(D)` where `D` is the items in the dispatch queue. Weighted as `D = 10`. + /// - Db reads: `scheduler lookup`, scheduler agenda` + /// - Db writes: `scheduler lookup`, scheduler agenda` + /// - Base Weight: 36.78 + 3.277 * D µs /// # - #[weight = (0, DispatchClass::Operational)] + #[weight = (68_000_000 + T::DbWeight::get().reads_writes(2, 2), DispatchClass::Operational)] fn cancel_queued(origin, which: ReferendumIndex) { ensure_root(origin)?; T::Scheduler::cancel_named((DEMOCRACY_ID, which).encode()) .map_err(|_| Error::::ProposalMissing)?; } + /// Weight: see `begin_block` fn on_initialize(n: T::BlockNumber) -> Weight { - if let Err(e) = Self::begin_block(n) { + Self::begin_block(n).unwrap_or_else(|e| { sp_runtime::print(e); - } - - 0 + 0 + }) } /// Specify a proxy that is already open to us. Called by the stash. @@ -853,9 +992,12 @@ decl_module! { /// - `proxy`: The account that will be activated as proxy. /// /// # - /// - One extra DB entry. + /// - Complexity: `O(1)` + /// - Db reads: `Proxy` + /// - Db writes: `Proxy` + /// - Base Weight: 7.972 µs /// # - #[weight = 100_000_000] + #[weight = 8_000_000 + T::DbWeight::get().reads_writes(1, 1)] fn activate_proxy(origin, proxy: T::AccountId) { let who = ensure_signed(origin)?; Proxy::::try_mutate(&proxy, |a| match a.take() { @@ -876,9 +1018,12 @@ decl_module! { /// The dispatch origin of this call must be _Signed_. /// /// # - /// - One DB clear. + /// - Complexity: `O(1)` + /// - Db reads: `Proxy`, `sender account` + /// - Db writes: `Proxy`, `sender account` + /// - Base Weight: 15.41 µs /// # - #[weight = 100_000_000] + #[weight = 16_000_000 + T::DbWeight::get().reads_writes(1, 1)] fn close_proxy(origin) { let who = ensure_signed(origin)?; Proxy::::mutate(&who, |a| { @@ -900,9 +1045,12 @@ decl_module! { /// - `proxy`: The account that will be deactivated as proxy. /// /// # - /// - One DB clear. + /// - Complexity: `O(1)` + /// - Db reads: `Proxy` + /// - Db writes: `Proxy` + /// - Base Weight: 8.03 µs /// # - #[weight = 100_000_000] + #[weight = 8_000_000 + T::DbWeight::get().reads_writes(1, 1)] fn deactivate_proxy(origin, proxy: T::AccountId) { let who = ensure_signed(origin)?; Proxy::::try_mutate(&proxy, |a| match a.take() { @@ -934,11 +1082,26 @@ decl_module! { /// Emits `Delegated`. /// /// # + /// - Complexity: `O(R)` where R is the number of referendums the voter delegating to has + /// voted on. Weight is charged as if maximum votes. + /// - Db reads: 2*`VotingOf`, `balances locks` + /// - Db writes: 2*`VotingOf`, `balances locks` + /// - Db reads per votes: `ReferendumInfoOf` + /// - Db writes per votes: `ReferendumInfoOf` + /// - Base Weight: 65.78 + 8.229 * R µs + // NOTE: weight must cover an incorrect voting of origin with 100 votes. /// # - #[weight = 500_000_000] - pub fn delegate(origin, to: T::AccountId, conviction: Conviction, balance: BalanceOf) { + #[weight = weight_for::delegate::(T::MaxVotes::get().into())] + pub fn delegate( + origin, + to: T::AccountId, + conviction: Conviction, + balance: BalanceOf + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; - Self::try_delegate(who, to, conviction, balance)?; + let votes = Self::try_delegate(who, to, conviction, balance)?; + + Ok(Some(weight_for::delegate::(votes.into())).into()) } /// Undelegate the voting power of the sending account. @@ -952,12 +1115,20 @@ decl_module! { /// Emits `Undelegated`. /// /// # - /// - O(1). + /// - Complexity: `O(R)` where R is the number of referendums the voter delegating to has + /// voted on. Weight is charged as if maximum votes. + /// - Db reads: 2*`VotingOf` + /// - Db writes: 2*`VotingOf` + /// - Db reads per votes: `ReferendumInfoOf` + /// - Db writes per votes: `ReferendumInfoOf` + /// - Base Weight: 33.29 + 8.104 * R µs + // NOTE: weight must cover an incorrect voting of origin with 100 votes. /// # - #[weight = 500_000_000] - fn undelegate(origin) { + #[weight = weight_for::undelegate::(T::MaxVotes::get().into())] + fn undelegate(origin) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; - Self::try_undelegate(who)?; + let votes = Self::try_undelegate(who)?; + Ok(Some(weight_for::undelegate::(votes.into())).into()) } /// Clears all public proposals. @@ -966,12 +1137,12 @@ decl_module! { /// /// # /// - `O(1)`. - /// - One DB clear. + /// - Db writes: `PublicProps` + /// - Base Weight: 2.505 µs /// # - #[weight = 0] + #[weight = 2_500_000 + T::DbWeight::get().writes(1)] fn clear_public_proposals(origin) { ensure_root(origin)?; - >::kill(); } @@ -985,10 +1156,13 @@ decl_module! { /// Emits `PreimageNoted`. /// /// # - /// - Dependent on the size of `encoded_proposal` but protected by a - /// required deposit. + /// - Complexity: `O(E)` with E size of `encoded_proposal` (protected by a required deposit). + /// - Db reads: `Preimages` + /// - Db writes: `Preimages` + /// - Base Weight: 37.93 + .004 * b µs /// # - #[weight = 100_000_000] + #[weight = 38_000_000 + 4_000 * Weight::from(encoded_proposal.len() as u32) + + T::DbWeight::get().reads_writes(1, 1)] fn note_preimage(origin, encoded_proposal: Vec) { let who = ensure_signed(origin)?; let proposal_hash = T::Hashing::hash(&encoded_proposal[..]); @@ -1021,12 +1195,17 @@ decl_module! { /// Emits `PreimageNoted`. /// /// # - /// - Dependent on the size of `encoded_proposal` and length of dispatch queue. + /// - Complexity: `O(E)` with E size of `encoded_proposal` (protected by a required deposit). + /// - Db reads: `Preimages` + /// - Db writes: `Preimages` + /// - Base Weight: 28.04 + .003 * b µs /// # - #[weight = 100_000_000] + #[weight = 28_000_000 + 3_000 * Weight::from(encoded_proposal.len() as u32) + + T::DbWeight::get().reads_writes(1, 1)] fn note_imminent_preimage(origin, encoded_proposal: Vec) { let who = ensure_signed(origin)?; let proposal_hash = T::Hashing::hash(&encoded_proposal[..]); + Self::check_pre_image_is_missing(proposal_hash)?; let status = Preimages::::get(&proposal_hash).ok_or(Error::::NotImminent)?; let expiry = status.to_missing_expiry().ok_or(Error::::DuplicatePreimage)?; @@ -1049,6 +1228,8 @@ decl_module! { /// The dispatch origin of this call must be _Signed_. /// /// - `proposal_hash`: The preimage hash of a proposal. + /// - `proposal_length_upper_bound`: an upper bound on length of the proposal. + /// Extrinsic is weighted according to this value with no refund. /// /// This will only work after `VotingPeriod` blocks from the time that the preimage was /// noted, if it's the same account doing it. If it's a different account, then it'll only @@ -1057,11 +1238,21 @@ decl_module! { /// Emits `PreimageReaped`. /// /// # - /// - One DB clear. + /// - Complexity: `O(D)` where D is length of proposal. + /// - Db reads: `Preimages` + /// - Db writes: `Preimages` + /// - Base Weight: 39.31 + .003 * b µs /// # - #[weight = 0] - fn reap_preimage(origin, proposal_hash: T::Hash) { + #[weight = (39_000_000 + T::DbWeight::get().reads_writes(1, 1)) + .saturating_add(3_000.saturating_mul(Weight::from(*proposal_len_upper_bound)))] + fn reap_preimage(origin, proposal_hash: T::Hash, #[compact] proposal_len_upper_bound: u32) { let who = ensure_signed(origin)?; + + ensure!( + Self::pre_image_data_len(proposal_hash)? <= proposal_len_upper_bound, + Error::::WrongUpperBound, + ); + let (provider, deposit, since, expiry) = >::get(&proposal_hash) .and_then(|m| match m { PreimageStatus::Available { provider, deposit, since, expiry, .. } @@ -1087,9 +1278,15 @@ decl_module! { /// - `target`: The account to remove the lock on. /// /// # - /// - `O(1)`. + /// - Complexity `O(R)` with R number of vote of target. + /// - Db reads: `VotingOf`, `balances locks`, `target account` + /// - Db writes: `VotingOf`, `balances locks`, `target account` + /// - Base Weight: + /// - Unlock Remove: 42.96 + .048 * R + /// - Unlock Set: 37.63 + .327 * R /// # - #[weight = 0] + #[weight = 43_000_000 + 330_000 * Weight::from(T::MaxVotes::get()) + + T::DbWeight::get().reads_writes(3, 3)] fn unlock(origin, target: T::AccountId) { ensure_signed(origin)?; Self::update_lock(&target); @@ -1106,9 +1303,12 @@ decl_module! { /// `close_proxy` must be called before the account can be destroyed. /// /// # - /// - One extra DB entry. + /// - Complexity: O(1) + /// - Db reads: `Proxy`, `proxy account` + /// - Db writes: `Proxy`, `proxy account` + /// - Base Weight: 14.86 µs /// # - #[weight = 100_000_000] + #[weight = 15_000_000 + T::DbWeight::get().reads_writes(2, 2)] fn open_proxy(origin, target: T::AccountId) { let who = ensure_signed(origin)?; Proxy::::mutate(&who, |a| { @@ -1146,8 +1346,12 @@ decl_module! { /// /// # /// - `O(R + log R)` where R is the number of referenda that `target` has voted on. + /// Weight is calculated for the maximum number of vote. + /// - Db reads: `ReferendumInfoOf`, `VotingOf` + /// - Db writes: `ReferendumInfoOf`, `VotingOf` + /// - Base Weight: 21.03 + .359 * R /// # - #[weight = 0] + #[weight = 21_000_000 + 360_000 * Weight::from(T::MaxVotes::get()) + T::DbWeight::get().reads_writes(2, 2)] fn remove_vote(origin, index: ReferendumIndex) -> DispatchResult { let who = ensure_signed(origin)?; Self::try_remove_vote(&who, index, UnvoteScope::Any) @@ -1168,8 +1372,12 @@ decl_module! { /// /// # /// - `O(R + log R)` where R is the number of referenda that `target` has voted on. + /// Weight is calculated for the maximum number of vote. + /// - Db reads: `ReferendumInfoOf`, `VotingOf` + /// - Db writes: `ReferendumInfoOf`, `VotingOf` + /// - Base Weight: 19.15 + .372 * R /// # - #[weight = 0] + #[weight = 19_000_000 + 370_000 * Weight::from(T::MaxVotes::get()) + T::DbWeight::get().reads_writes(2, 2)] fn remove_other_vote(origin, target: T::AccountId, index: ReferendumIndex) -> DispatchResult { let who = ensure_signed(origin)?; let scope = if target == who { UnvoteScope::Any } else { UnvoteScope::OnlyExpired }; @@ -1199,16 +1407,22 @@ decl_module! { /// Emits `Delegated`. /// /// # + /// same as `delegate with additional: + /// - Db reads: `Proxy`, `proxy account` + /// - Db writes: `proxy account` + /// - Base Weight: 68.61 + 8.039 * R µs /// # - #[weight = 500_000_000] + #[weight = weight_for::proxy_delegate::(T::MaxVotes::get().into())] pub fn proxy_delegate(origin, to: T::AccountId, conviction: Conviction, balance: BalanceOf, - ) { + ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let target = Self::proxy(who).and_then(|a| a.as_active()).ok_or(Error::::NotProxy)?; - Self::try_delegate(target, to, conviction, balance)?; + let votes = Self::try_delegate(target, to, conviction, balance)?; + + Ok(Some(weight_for::proxy_delegate::(votes.into())).into()) } /// Undelegate the voting power of a proxied account. @@ -1222,13 +1436,17 @@ decl_module! { /// Emits `Undelegated`. /// /// # - /// - O(1). + /// same as `undelegate with additional: + /// Db reads: `Proxy` + /// Base Weight: 39 + 7.958 * R µs /// # - #[weight = 500_000_000] - fn proxy_undelegate(origin) { + #[weight = weight_for::proxy_undelegate::(T::MaxVotes::get().into())] + fn proxy_undelegate(origin) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; let target = Self::proxy(who).and_then(|a| a.as_active()).ok_or(Error::::NotProxy)?; - Self::try_undelegate(target)?; + let votes = Self::try_undelegate(target)?; + + Ok(Some(weight_for::proxy_undelegate::(votes.into())).into()) } /// Remove a proxied vote for a referendum. @@ -1243,8 +1461,12 @@ decl_module! { /// /// # /// - `O(R + log R)` where R is the number of referenda that `target` has voted on. + /// Weight is calculated for the maximum number of vote. + /// - Db reads: `ReferendumInfoOf`, `VotingOf`, `Proxy` + /// - Db writes: `ReferendumInfoOf`, `VotingOf` + /// - Base Weight: 26.35 + .36 * R µs /// # - #[weight = 0] + #[weight = 26_000_000 + 360_000 * Weight::from(T::MaxVotes::get()) + T::DbWeight::get().reads_writes(2, 3)] fn proxy_remove_vote(origin, index: ReferendumIndex) -> DispatchResult { let who = ensure_signed(origin)?; let target = Self::proxy(who).and_then(|a| a.as_active()).ok_or(Error::::NotProxy)?; @@ -1266,7 +1488,7 @@ impl Module { /// Get the amount locked in support of `proposal`; `None` if proposal isn't a valid proposal /// index. pub fn backing_for(proposal: PropIndex) -> Option> { - Self::deposit_of(proposal).map(|(d, l)| d * (l.len() as u32).into()) + Self::deposit_of(proposal).map(|(l, d)| d * (l.len() as u32).into()) } /// Get all referenda ready for tally at block `n`. @@ -1275,7 +1497,14 @@ impl Module { ) -> Vec<(ReferendumIndex, ReferendumStatus>)> { let next = Self::lowest_unbaked(); let last = Self::referendum_count(); - (next..last).into_iter() + Self::maturing_referenda_at_inner(n, next..last) + } + + fn maturing_referenda_at_inner( + n: T::BlockNumber, + range: core::ops::Range, + ) -> Vec<(ReferendumIndex, ReferendumStatus>)> { + range.into_iter() .map(|i| (i, Self::referendum_info(i))) .filter_map(|(i, maybe_info)| match maybe_info { Some(ReferendumInfo::Ongoing(status)) => Some((i, status)), @@ -1352,7 +1581,10 @@ impl Module { } votes[i].1 = vote; } - Err(i) => votes.insert(i, (ref_index, vote)), + Err(i) => { + ensure!(votes.len() as u32 <= T::MaxVotes::get(), Error::::MaxVotesReached); + votes.insert(i, (ref_index, vote)); + } } // Shouldn't be possible to fail, but we handle it gracefully. status.tally.add(vote).ok_or(Error::::Overflow)?; @@ -1415,11 +1647,14 @@ impl Module { Ok(()) } - fn increase_upstream_delegation(who: &T::AccountId, amount: Delegations>) { + /// Return the number of votes for `who` + fn increase_upstream_delegation(who: &T::AccountId, amount: Delegations>) -> u32 { VotingOf::::mutate(who, |voting| match voting { - Voting::Delegating { delegations, .. } => + Voting::Delegating { delegations, .. } => { // We don't support second level delegating, so we don't need to do anything more. - *delegations = delegations.saturating_add(amount), + *delegations = delegations.saturating_add(amount); + 1 + }, Voting::Direct { votes, delegations, .. } => { *delegations = delegations.saturating_add(amount); for &(ref_index, account_vote) in votes.iter() { @@ -1431,15 +1666,19 @@ impl Module { ); } } + votes.len() as u32 } }) } - fn reduce_upstream_delegation(who: &T::AccountId, amount: Delegations>) { + /// Return the number of votes for `who` + fn reduce_upstream_delegation(who: &T::AccountId, amount: Delegations>) -> u32 { VotingOf::::mutate(who, |voting| match voting { - Voting::Delegating { delegations, .. } => - // We don't support second level delegating, so we don't need to do anything more. - *delegations = delegations.saturating_sub(amount), + Voting::Delegating { delegations, .. } => { + // We don't support second level delegating, so we don't need to do anything more. + *delegations = delegations.saturating_sub(amount); + 1 + } Voting::Direct { votes, delegations, .. } => { *delegations = delegations.saturating_sub(amount); for &(ref_index, account_vote) in votes.iter() { @@ -1451,20 +1690,23 @@ impl Module { ); } } + votes.len() as u32 } }) } /// Attempt to delegate `balance` times `conviction` of voting power from `who` to `target`. + /// + /// Return the upstream number of votes. fn try_delegate( who: T::AccountId, target: T::AccountId, conviction: Conviction, balance: BalanceOf, - ) -> DispatchResult { + ) -> Result { ensure!(who != target, Error::::Nonsense); ensure!(balance <= T::Currency::free_balance(&who), Error::::InsufficientFunds); - VotingOf::::try_mutate(&who, |voting| -> DispatchResult { + let votes = VotingOf::::try_mutate(&who, |voting| -> Result { let mut old = Voting::Delegating { balance, target: target.clone(), @@ -1485,7 +1727,7 @@ impl Module { voting.set_common(delegations, prior); } } - Self::increase_upstream_delegation(&target, conviction.votes(balance)); + let votes = Self::increase_upstream_delegation(&target, conviction.votes(balance)); // Extend the lock to `balance` (rather than setting it) since we don't know what other // votes are in place. T::Currency::extend_lock( @@ -1494,15 +1736,17 @@ impl Module { balance, WithdrawReason::Transfer.into() ); - Ok(()) + Ok(votes) })?; Self::deposit_event(Event::::Delegated(who, target)); - Ok(()) + Ok(votes) } /// Attempt to end the current delegation. - fn try_undelegate(who: T::AccountId) -> DispatchResult { - VotingOf::::try_mutate(&who, |voting| -> DispatchResult { + /// + /// Return the number of votes of upstream. + fn try_undelegate(who: T::AccountId) -> Result { + let votes = VotingOf::::try_mutate(&who, |voting| -> Result { let mut old = Voting::default(); sp_std::mem::swap(&mut old, voting); match old { @@ -1514,20 +1758,21 @@ impl Module { mut prior, } => { // remove any delegation votes to our current target. - Self::reduce_upstream_delegation(&target, conviction.votes(balance)); + let votes = Self::reduce_upstream_delegation(&target, conviction.votes(balance)); let now = system::Module::::block_number(); let lock_periods = conviction.lock_periods().into(); prior.accumulate(now + T::EnactmentPeriod::get() * lock_periods, balance); voting.set_common(delegations, prior); + + Ok(votes) } Voting::Direct { .. } => { - return Err(Error::::NotDelegating.into()) + Err(Error::::NotDelegating.into()) } } - Ok(()) })?; Self::deposit_event(Event::::Undelegated(who)); - Ok(()) + Ok(votes) } /// Rejig the lock on an account. It will never get more stringent (since that would indicate @@ -1597,7 +1842,7 @@ impl Module { let (prop_index, proposal, _) = public_props.swap_remove(winner_index); >::put(public_props); - if let Some((deposit, depositors)) = >::take(prop_index) { + if let Some((depositors, deposit)) = >::take(prop_index) { // refund depositors for d in &depositors { T::Currency::unreserve(d, deposit); @@ -1676,19 +1921,132 @@ impl Module { } /// Current era is ending; we should finish up any proposals. - fn begin_block(now: T::BlockNumber) -> DispatchResult { + /// + /// + /// # + /// If a referendum is launched or maturing take full block weight. Otherwise: + /// - Complexity: `O(R)` where `R` is the number of unbaked referenda. + /// - Db reads: `LastTabledWasExternal`, `NextExternal`, `PublicProps`, `account`, + /// `ReferendumCount`, `LowestUnbaked` + /// - Db writes: `PublicProps`, `account`, `ReferendumCount`, `DepositOf`, `ReferendumInfoOf` + /// - Db reads per R: `DepositOf`, `ReferendumInfoOf` + /// - Base Weight: 58.58 + 10.9 * R µs + /// # + fn begin_block(now: T::BlockNumber) -> Result { + let mut weight = 60_000_000 + T::DbWeight::get().reads_writes(6, 5); + // pick out another public referendum if it's time. if (now % T::LaunchPeriod::get()).is_zero() { // Errors come from the queue being empty. we don't really care about that, and even if // we did, there is nothing we can do here. let _ = Self::launch_next(now); + weight = T::MaximumBlockWeight::get(); } // tally up votes for any expiring referenda. - for (index, info) in Self::maturing_referenda_at(now).into_iter() { + let next = Self::lowest_unbaked(); + let last = Self::referendum_count(); + let r = Weight::from(last.saturating_sub(next)); + weight += 11_000_000 * r + T::DbWeight::get().reads(2 * r); + for (index, info) in Self::maturing_referenda_at_inner(now, next..last).into_iter() { let approved = Self::bake_referendum(now, index, info)?; ReferendumInfoOf::::insert(index, ReferendumInfo::Finished { end: now, approved }); + weight = T::MaximumBlockWeight::get(); + } + + Ok(weight) + } + + /// Reads the length of account in DepositOf without getting the complete value in the runtime. + /// + /// Return 0 if no deposit for this proposal. + fn len_of_deposit_of(proposal: PropIndex) -> Option { + // DepositOf first tuple element is a vec, decoding its len is equivalent to decode a + // `Compact`. + decode_compact_u32_at(&>::hashed_key_for(proposal)) + } + + /// Check that pre image exists and its value is variant `PreimageStatus::Missing`. + /// + /// This check is done without getting the complete value in the runtime to avoid copying a big + /// value in the runtime. + fn check_pre_image_is_missing(proposal_hash: T::Hash) -> DispatchResult { + // To decode the enum variant we only need the first byte. + let mut buf = [0u8; 1]; + let key = >::hashed_key_for(proposal_hash); + let bytes = match sp_io::storage::read(&key, &mut buf, 0) { + Some(bytes) => bytes, + None => return Err(Error::::NotImminent.into()), + }; + // The value may be smaller that 1 byte. + let mut input = &buf[0..buf.len().min(bytes as usize)]; + + match input.read_byte() { + Ok(0) => Ok(()), // PreimageStatus::Missing is variant 0 + Ok(1) => Err(Error::::DuplicatePreimage.into()), + _ => { + sp_runtime::print("Failed to decode `PreimageStatus` variant"); + Err(Error::::NotImminent.into()) + } + } + } + + /// Check that pre image exists, its value is variant `PreimageStatus::Available` and decode + /// the length of `data: Vec` fields. + /// + /// This check is done without getting the complete value in the runtime to avoid copying a big + /// value in the runtime. + /// + /// If the pre image is missing variant or doesn't exist then the error `PreimageMissing` is + /// returned. + fn pre_image_data_len(proposal_hash: T::Hash) -> Result { + // To decode the `data` field of Available variant we need: + // * one byte for the variant + // * at most 5 bytes to decode a `Compact` + let mut buf = [0u8; 6]; + let key = >::hashed_key_for(proposal_hash); + let bytes = match sp_io::storage::read(&key, &mut buf, 0) { + Some(bytes) => bytes, + None => return Err(Error::::PreimageMissing.into()), + }; + // The value may be smaller that 6 bytes. + let mut input = &buf[0..buf.len().min(bytes as usize)]; + + match input.read_byte() { + Ok(1) => (), // Check that input exists and is second variant. + Ok(0) => return Err(Error::::PreimageMissing.into()), + _ => { + sp_runtime::print("Failed to decode `PreimageStatus` variant"); + return Err(Error::::PreimageMissing.into()); + } + } + + // Decode the length of the vector. + let len = codec::Compact::::decode(&mut input).map_err(|_| { + sp_runtime::print("Failed to decode `PreimageStatus` variant"); + DispatchError::from(Error::::PreimageMissing) + })?.0; + + Ok(len) + } +} + +/// Decode `Compact` from the trie at given key. +fn decode_compact_u32_at(key: &[u8]) -> Option { + // `Compact` takes at most 5 bytes. + let mut buf = [0u8; 5]; + let bytes = match sp_io::storage::read(&key, &mut buf, 0) { + Some(bytes) => bytes, + None => return None, + }; + // The value may be smaller than 5 bytes. + let mut input = &buf[0..buf.len().min(bytes as usize)]; + match codec::Compact::::decode(&mut input) { + Ok(c) => Some(c.0), + Err(_) => { + sp_runtime::print("Failed to decode compact u32 at:"); + sp_runtime::print(key); + None } - Ok(()) } } diff --git a/frame/democracy/src/tests.rs b/frame/democracy/src/tests.rs index a835a0ff6ee1e..0c8bda86d62c7 100644 --- a/frame/democracy/src/tests.rs +++ b/frame/democracy/src/tests.rs @@ -41,6 +41,8 @@ mod proxying; mod public_proposals; mod scheduling; mod voting; +mod migration; +mod decoders; const AYE: Vote = Vote { aye: true, conviction: Conviction::None }; const NAY: Vote = Vote { aye: false, conviction: Conviction::None }; @@ -131,6 +133,7 @@ parameter_types! { pub const MinimumDeposit: u64 = 1; pub const EnactmentPeriod: u64 = 2; pub const CooloffPeriod: u64 = 2; + pub const MaxVotes: u32 = 100; } ord_parameter_types! { pub const One: u64 = 1; @@ -181,6 +184,7 @@ impl super::Trait for Test { type InstantOrigin = EnsureSignedBy; type InstantAllowed = InstantAllowed; type Scheduler = Scheduler; + type MaxVotes = MaxVotes; } pub fn new_test_ext() -> sp_io::TestExternalities { @@ -231,7 +235,7 @@ fn propose_set_balance(who: u64, value: u64, delay: u64) -> DispatchResult { Democracy::propose( Origin::signed(who), set_balance_proposal_hash(value), - delay + delay, ) } @@ -239,14 +243,14 @@ fn propose_set_balance_and_note(who: u64, value: u64, delay: u64) -> DispatchRes Democracy::propose( Origin::signed(who), set_balance_proposal_hash_and_note(value), - delay + delay, ) } fn next_block() { System::set_block_number(System::block_number() + 1); Scheduler::on_initialize(System::block_number()); - assert_eq!(Democracy::begin_block(System::block_number()), Ok(())); + assert!(Democracy::begin_block(System::block_number()).is_ok()); } fn fast_forward_to(n: u64) { diff --git a/frame/democracy/src/tests/decoders.rs b/frame/democracy/src/tests/decoders.rs new file mode 100644 index 0000000000000..6b8e661ca9fd9 --- /dev/null +++ b/frame/democracy/src/tests/decoders.rs @@ -0,0 +1,81 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! The for various partial storage decoders + +use super::*; +use frame_support::storage::{migration, StorageMap, unhashed}; + +#[test] +fn test_decode_compact_u32_at() { + new_test_ext().execute_with(|| { + let v = codec::Compact(u64::max_value()); + migration::put_storage_value(b"test", b"", &[], v); + assert_eq!(decode_compact_u32_at(b"test"), None); + + for v in vec![0, 10, u32::max_value()] { + let compact_v = codec::Compact(v); + unhashed::put(b"test", &compact_v); + assert_eq!(decode_compact_u32_at(b"test"), Some(v)); + } + + unhashed::kill(b"test"); + assert_eq!(decode_compact_u32_at(b"test"), None); + }) +} + +#[test] +fn len_of_deposit_of() { + new_test_ext().execute_with(|| { + for l in vec![0, 1, 200, 1000] { + let value: (Vec, u64) = ((0..l).map(|_| Default::default()).collect(), 3u64); + DepositOf::::insert(2, value); + assert_eq!(Democracy::len_of_deposit_of(2), Some(l)); + } + + DepositOf::::remove(2); + assert_eq!(Democracy::len_of_deposit_of(2), None); + }) +} + +#[test] +fn pre_image() { + new_test_ext().execute_with(|| { + let key = Default::default(); + let missing = PreimageStatus::Missing(0); + Preimages::::insert(key, missing); + assert!(Democracy::pre_image_data_len(key).is_err()); + assert_eq!(Democracy::check_pre_image_is_missing(key), Ok(())); + + Preimages::::remove(key); + assert!(Democracy::pre_image_data_len(key).is_err()); + assert!(Democracy::check_pre_image_is_missing(key).is_err()); + + for l in vec![0, 10, 100, 1000u32] { + let available = PreimageStatus::Available{ + data: (0..l).map(|i| i as u8).collect(), + provider: 0, + deposit: 0, + since: 0, + expiry: None, + }; + + Preimages::::insert(key, available); + assert_eq!(Democracy::pre_image_data_len(key), Ok(l)); + assert!(Democracy::check_pre_image_is_missing(key).is_err()); + } + }) +} diff --git a/frame/democracy/src/tests/migration.rs b/frame/democracy/src/tests/migration.rs new file mode 100644 index 0000000000000..cab8f7f5c936c --- /dev/null +++ b/frame/democracy/src/tests/migration.rs @@ -0,0 +1,45 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! The tests for migration. + +use super::*; +use frame_support::{storage::migration, Hashable, traits::OnRuntimeUpgrade}; +use substrate_test_utils::assert_eq_uvec; + +#[test] +fn migration() { + new_test_ext().execute_with(|| { + for i in 0..3 { + let k = i.twox_64_concat(); + let v: (BalanceOf, Vec) = (i * 1000, vec![i]); + migration::put_storage_value(b"Democracy", b"DepositOf", &k, v); + } + StorageVersion::kill(); + + Democracy::on_runtime_upgrade(); + + assert_eq!(StorageVersion::get(), Some(Releases::V1)); + assert_eq_uvec!( + DepositOf::::iter().collect::>(), + vec![ + (0, (vec![0u64], >::from(0u32))), + (1, (vec![1u64], >::from(1000u32))), + (2, (vec![2u64], >::from(2000u32))), + ] + ); + }) +} diff --git a/frame/democracy/src/tests/preimage.rs b/frame/democracy/src/tests/preimage.rs index 7d977b0ba83ac..ffa8b46790e13 100644 --- a/frame/democracy/src/tests/preimage.rs +++ b/frame/democracy/src/tests/preimage.rs @@ -76,11 +76,11 @@ fn preimage_deposit_should_be_reapable_earlier_by_owner() { next_block(); assert_noop!( - Democracy::reap_preimage(Origin::signed(6), set_balance_proposal_hash(2)), + Democracy::reap_preimage(Origin::signed(6), set_balance_proposal_hash(2), u32::max_value()), Error::::TooEarly ); next_block(); - assert_ok!(Democracy::reap_preimage(Origin::signed(6), set_balance_proposal_hash(2))); + assert_ok!(Democracy::reap_preimage(Origin::signed(6), set_balance_proposal_hash(2), u32::max_value())); assert_eq!(Balances::free_balance(6), 60); assert_eq!(Balances::reserved_balance(6), 0); @@ -91,7 +91,7 @@ fn preimage_deposit_should_be_reapable_earlier_by_owner() { fn preimage_deposit_should_be_reapable() { new_test_ext().execute_with(|| { assert_noop!( - Democracy::reap_preimage(Origin::signed(5), set_balance_proposal_hash(2)), + Democracy::reap_preimage(Origin::signed(5), set_balance_proposal_hash(2), u32::max_value()), Error::::PreimageMissing ); @@ -103,12 +103,12 @@ fn preimage_deposit_should_be_reapable() { next_block(); next_block(); assert_noop!( - Democracy::reap_preimage(Origin::signed(5), set_balance_proposal_hash(2)), + Democracy::reap_preimage(Origin::signed(5), set_balance_proposal_hash(2), u32::max_value()), Error::::TooEarly ); next_block(); - assert_ok!(Democracy::reap_preimage(Origin::signed(5), set_balance_proposal_hash(2))); + assert_ok!(Democracy::reap_preimage(Origin::signed(5), set_balance_proposal_hash(2), u32::max_value())); assert_eq!(Balances::reserved_balance(6), 0); assert_eq!(Balances::free_balance(6), 48); assert_eq!(Balances::free_balance(5), 62); @@ -152,6 +152,6 @@ fn reaping_imminent_preimage_should_fail() { assert_ok!(Democracy::vote(Origin::signed(1), r, aye(1))); next_block(); next_block(); - assert_noop!(Democracy::reap_preimage(Origin::signed(6), h), Error::::Imminent); + assert_noop!(Democracy::reap_preimage(Origin::signed(6), h, u32::max_value()), Error::::Imminent); }); } diff --git a/frame/democracy/src/tests/public_proposals.rs b/frame/democracy/src/tests/public_proposals.rs index 04246e86f1da4..fc1499960aa35 100644 --- a/frame/democracy/src/tests/public_proposals.rs +++ b/frame/democracy/src/tests/public_proposals.rs @@ -34,10 +34,10 @@ fn backing_for_should_work() { fn deposit_for_proposals_should_be_taken() { new_test_ext().execute_with(|| { assert_ok!(propose_set_balance_and_note(1, 2, 5)); - assert_ok!(Democracy::second(Origin::signed(2), 0)); - assert_ok!(Democracy::second(Origin::signed(5), 0)); - assert_ok!(Democracy::second(Origin::signed(5), 0)); - assert_ok!(Democracy::second(Origin::signed(5), 0)); + assert_ok!(Democracy::second(Origin::signed(2), 0, u32::max_value())); + assert_ok!(Democracy::second(Origin::signed(5), 0, u32::max_value())); + assert_ok!(Democracy::second(Origin::signed(5), 0, u32::max_value())); + assert_ok!(Democracy::second(Origin::signed(5), 0, u32::max_value())); assert_eq!(Balances::free_balance(1), 5); assert_eq!(Balances::free_balance(2), 15); assert_eq!(Balances::free_balance(5), 35); @@ -48,10 +48,10 @@ fn deposit_for_proposals_should_be_taken() { fn deposit_for_proposals_should_be_returned() { new_test_ext().execute_with(|| { assert_ok!(propose_set_balance_and_note(1, 2, 5)); - assert_ok!(Democracy::second(Origin::signed(2), 0)); - assert_ok!(Democracy::second(Origin::signed(5), 0)); - assert_ok!(Democracy::second(Origin::signed(5), 0)); - assert_ok!(Democracy::second(Origin::signed(5), 0)); + assert_ok!(Democracy::second(Origin::signed(2), 0, u32::max_value())); + assert_ok!(Democracy::second(Origin::signed(5), 0, u32::max_value())); + assert_ok!(Democracy::second(Origin::signed(5), 0, u32::max_value())); + assert_ok!(Democracy::second(Origin::signed(5), 0, u32::max_value())); fast_forward_to(3); assert_eq!(Balances::free_balance(1), 10); assert_eq!(Balances::free_balance(2), 20); @@ -77,7 +77,21 @@ fn poor_proposer_should_not_work() { fn poor_seconder_should_not_work() { new_test_ext().execute_with(|| { assert_ok!(propose_set_balance_and_note(2, 2, 11)); - assert_noop!(Democracy::second(Origin::signed(1), 0), BalancesError::::InsufficientBalance); + assert_noop!( + Democracy::second(Origin::signed(1), 0, u32::max_value()), + BalancesError::::InsufficientBalance + ); + }); +} + +#[test] +fn invalid_seconds_upper_bound_should_not_work() { + new_test_ext().execute_with(|| { + assert_ok!(propose_set_balance_and_note(1, 2, 5)); + assert_noop!( + Democracy::second(Origin::signed(2), 0, 0), + Error::::WrongUpperBound + ); }); }