diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 648bbff633046..2eaea18f2a625 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -718,6 +718,7 @@ parameter_types! { pub const MaximumReasonLength: u32 = 16384; pub const BountyCuratorDeposit: Permill = Permill::from_percent(50); pub const BountyValueMinimum: Balance = 5 * DOLLARS; + pub const MaxApprovals: u32 = 100; } impl pallet_treasury::Config for Runtime { @@ -742,6 +743,7 @@ impl pallet_treasury::Config for Runtime { type BurnDestination = (); type SpendFunds = Bounties; type WeightInfo = pallet_treasury::weights::SubstrateWeight; + type MaxApprovals = MaxApprovals; } impl pallet_bounties::Config for Runtime { diff --git a/frame/bounties/src/tests.rs b/frame/bounties/src/tests.rs index b202e4da3e847..e90b1f565a4c9 100644 --- a/frame/bounties/src/tests.rs +++ b/frame/bounties/src/tests.rs @@ -105,6 +105,7 @@ parameter_types! { pub const Burn: Permill = Permill::from_percent(50); pub const DataDepositPerByte: u64 = 1; pub const TreasuryPalletId: PalletId = PalletId(*b"py/trsry"); + pub const MaxApprovals: u32 = 100; } // impl pallet_treasury::Config for Test { impl pallet_treasury::Config for Test { @@ -121,6 +122,7 @@ impl pallet_treasury::Config for Test { type BurnDestination = (); // Just gets burned. type WeightInfo = (); type SpendFunds = Bounties; + type MaxApprovals = MaxApprovals; } parameter_types! { pub const BountyDepositBase: u64 = 80; diff --git a/frame/tips/src/tests.rs b/frame/tips/src/tests.rs index cb0d4e6c47b42..3b11e105c6d06 100644 --- a/frame/tips/src/tests.rs +++ b/frame/tips/src/tests.rs @@ -127,6 +127,7 @@ parameter_types! { pub const DataDepositPerByte: u64 = 1; pub const TreasuryPalletId: PalletId = PalletId(*b"py/trsry"); pub const MaximumReasonLength: u32 = 16384; + pub const MaxApprovals: u32 = 100; } impl pallet_treasury::Config for Test { type PalletId = TreasuryPalletId; @@ -142,6 +143,7 @@ impl pallet_treasury::Config for Test { type BurnDestination = (); // Just gets burned. type WeightInfo = (); type SpendFunds = (); + type MaxApprovals = MaxApprovals; } parameter_types! { pub const TipCountdown: u64 = 1; diff --git a/frame/treasury/src/benchmarking.rs b/frame/treasury/src/benchmarking.rs index 119516fe2741a..64ecbebe0bff9 100644 --- a/frame/treasury/src/benchmarking.rs +++ b/frame/treasury/src/benchmarking.rs @@ -55,7 +55,7 @@ fn create_approved_proposals, I: Instance>(n: u32) -> Result<(), &' let proposal_id = >::get() - 1; Treasury::::approve_proposal(RawOrigin::Root.into(), proposal_id)?; } - ensure!(>::get().len() == n as usize, "Not all approved"); + ensure!(>::get().len() == n as usize, "Not all approved"); Ok(()) } @@ -85,6 +85,8 @@ benchmarks_instance! { }: _(RawOrigin::Root, proposal_id) approve_proposal { + let p in 0 .. T::MaxApprovals::get() - 1; + create_approved_proposals::(p)?; let (caller, value, beneficiary_lookup) = setup_proposal::(SEED); Treasury::::propose_spend( RawOrigin::Signed(caller).into(), @@ -95,7 +97,7 @@ benchmarks_instance! { }: _(RawOrigin::Root, proposal_id) on_initialize_proposals { - let p in 0 .. 100; + let p in 0 .. T::MaxApprovals::get(); setup_pot_account::(); create_approved_proposals::(p)?; }: { diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index 7de193dd69848..473a570a87256 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -64,10 +64,13 @@ mod benchmarking; pub mod weights; use sp_std::prelude::*; -use frame_support::{decl_module, decl_storage, decl_event, ensure, print, decl_error, PalletId}; +use frame_support::{ + decl_module, decl_storage, decl_event, ensure, print, decl_error, + PalletId, BoundedVec, bounded_vec::TryAppendValue, +}; use frame_support::traits::{ Currency, Get, Imbalance, OnUnbalanced, ExistenceRequirement::KeepAlive, - ReservableCurrency, WithdrawReasons + ReservableCurrency, WithdrawReasons, }; use sp_runtime::{ Permill, RuntimeDebug, @@ -128,6 +131,9 @@ pub trait Config: frame_system::Config { /// Runtime hooks to external pallet using treasury to compute spend funds. type SpendFunds: SpendFunds; + + /// The maximum number of approvals that can wait in the spending queue. + type MaxApprovals: Get; } /// A trait to allow the Treasury Pallet to spend it's funds for other purposes. @@ -180,7 +186,7 @@ decl_storage! { => Option>>; /// Proposal indices that have been approved but not yet awarded. - pub Approvals get(fn approvals): Vec; + pub Approvals get(fn approvals): BoundedVec; } add_extra_genesis { build(|_config| { @@ -225,6 +231,8 @@ decl_error! { InsufficientProposersBalance, /// No proposal or bounty at that index. InvalidIndex, + /// Too many approvals in the queue. + TooManyApprovals, } } @@ -313,12 +321,12 @@ decl_module! { /// - DbReads: `Proposals`, `Approvals` /// - DbWrite: `Approvals` /// # - #[weight = (T::WeightInfo::approve_proposal(), DispatchClass::Operational)] + #[weight = (T::WeightInfo::approve_proposal(T::MaxApprovals::get()), DispatchClass::Operational)] pub fn approve_proposal(origin, #[compact] proposal_id: ProposalIndex) { T::ApproveOrigin::ensure_origin(origin)?; ensure!(>::contains_key(proposal_id), Error::::InvalidIndex); - Approvals::::append(proposal_id); + Approvals::::try_append(proposal_id).map_err(|_| Error::::TooManyApprovals)?; } /// # @@ -365,7 +373,7 @@ impl, I: Instance> Module { let mut missed_any = false; let mut imbalance = >::zero(); - let proposals_len = Approvals::::mutate(|v| { + let proposals_len = Approvals::::mutate(|v| { let proposals_approvals_len = v.len() as u32; v.retain(|&index| { // Should always be true, but shouldn't panic if false or we're screwed. diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 3ff9d63b1096a..cb6d4903a5732 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -102,6 +102,7 @@ parameter_types! { pub const BountyUpdatePeriod: u32 = 20; pub const BountyCuratorDeposit: Permill = Permill::from_percent(50); pub const BountyValueMinimum: u64 = 1; + pub const MaxApprovals: u32 = 100; } impl Config for Test { type PalletId = TreasuryPalletId; @@ -117,6 +118,7 @@ impl Config for Test { type BurnDestination = (); // Just gets burned. type WeightInfo = (); type SpendFunds = (); + type MaxApprovals = MaxApprovals; } pub fn new_test_ext() -> sp_io::TestExternalities { @@ -359,3 +361,20 @@ fn genesis_funding_works() { assert_eq!(Treasury::pot(), initial_funding - Balances::minimum_balance()); }); } + +#[test] +fn max_approvals_limited() { + new_test_ext().execute_with(|| { + Balances::make_free_balance_be(&Treasury::account_id(), u64::max_value()); + Balances::make_free_balance_be(&0, u64::max_value()); + + for _ in 0 .. MaxApprovals::get() { + assert_ok!(Treasury::propose_spend(Origin::signed(0), 100, 3)); + assert_ok!(Treasury::approve_proposal(Origin::root(), 0)); + } + + // One too many will fail + assert_ok!(Treasury::propose_spend(Origin::signed(0), 100, 3)); + assert_noop!(Treasury::approve_proposal(Origin::root(), 0), Error::::TooManyApprovals); + }); +} diff --git a/frame/treasury/src/weights.rs b/frame/treasury/src/weights.rs index b8a5625bf0624..9d627f1c287e2 100644 --- a/frame/treasury/src/weights.rs +++ b/frame/treasury/src/weights.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2020-2021 Parity Technologies (UK) Ltd. +// Copyright (C) 2021 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); @@ -17,12 +17,12 @@ //! Autogenerated weights for pallet_treasury //! -//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0 -//! DATE: 2020-12-16, STEPS: `[50, ]`, REPEAT: 20, LOW RANGE: [], HIGH RANGE: [] +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 3.0.0 +//! DATE: 2021-04-26, STEPS: `[50, ]`, REPEAT: 20, LOW RANGE: `[]`, HIGH RANGE: `[]` //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128 // Executed Command: -// ./target/release/substrate +// target/release/substrate // benchmark // --chain=dev // --steps=50 @@ -46,7 +46,7 @@ use sp_std::marker::PhantomData; pub trait WeightInfo { fn propose_spend() -> Weight; fn reject_proposal() -> Weight; - fn approve_proposal() -> Weight; + fn approve_proposal(p: u32, ) -> Weight; fn on_initialize_proposals(p: u32, ) -> Weight; } @@ -54,24 +54,26 @@ pub trait WeightInfo { pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { fn propose_spend() -> Weight { - (59_986_000 as Weight) + (45_393_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) } fn reject_proposal() -> Weight { - (48_300_000 as Weight) + (42_796_000 as Weight) .saturating_add(T::DbWeight::get().reads(2 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) } - fn approve_proposal() -> Weight { - (14_054_000 as Weight) + fn approve_proposal(p: u32, ) -> Weight { + (14_153_000 as Weight) + // Standard Error: 1_000 + .saturating_add((94_000 as Weight).saturating_mul(p as Weight)) .saturating_add(T::DbWeight::get().reads(2 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn on_initialize_proposals(p: u32, ) -> Weight { - (86_038_000 as Weight) - // Standard Error: 18_000 - .saturating_add((78_781_000 as Weight).saturating_mul(p as Weight)) + (51_633_000 as Weight) + // Standard Error: 42_000 + .saturating_add((65_705_000 as Weight).saturating_mul(p as Weight)) .saturating_add(T::DbWeight::get().reads(2 as Weight)) .saturating_add(T::DbWeight::get().reads((3 as Weight).saturating_mul(p as Weight))) .saturating_add(T::DbWeight::get().writes(2 as Weight)) @@ -82,24 +84,26 @@ impl WeightInfo for SubstrateWeight { // For backwards compatibility and tests impl WeightInfo for () { fn propose_spend() -> Weight { - (59_986_000 as Weight) + (45_393_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } fn reject_proposal() -> Weight { - (48_300_000 as Weight) + (42_796_000 as Weight) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } - fn approve_proposal() -> Weight { - (14_054_000 as Weight) + fn approve_proposal(p: u32, ) -> Weight { + (14_153_000 as Weight) + // Standard Error: 1_000 + .saturating_add((94_000 as Weight).saturating_mul(p as Weight)) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn on_initialize_proposals(p: u32, ) -> Weight { - (86_038_000 as Weight) - // Standard Error: 18_000 - .saturating_add((78_781_000 as Weight).saturating_mul(p as Weight)) + (51_633_000 as Weight) + // Standard Error: 42_000 + .saturating_add((65_705_000 as Weight).saturating_mul(p as Weight)) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) .saturating_add(RocksDbWeight::get().reads((3 as Weight).saturating_mul(p as Weight))) .saturating_add(RocksDbWeight::get().writes(2 as Weight))