diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 648bbff633046d402766b9e1424ae839d49a3184..2eaea18f2a625e1798596bb666f1f65e8c867fc1 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/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<Runtime>; + type MaxApprovals = MaxApprovals; } impl pallet_bounties::Config for Runtime { diff --git a/substrate/frame/bounties/src/tests.rs b/substrate/frame/bounties/src/tests.rs index b202e4da3e8479758bb01c22ab2ff743bcf7af6b..e90b1f565a4c9d808625a33775aeabcaa0ba2d17 100644 --- a/substrate/frame/bounties/src/tests.rs +++ b/substrate/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/substrate/frame/tips/src/tests.rs b/substrate/frame/tips/src/tests.rs index cb0d4e6c47b4220e77b3e6c3034577b9c4b892c7..3b11e105c6d06db8c6a40984ae874c3797f6ecc8 100644 --- a/substrate/frame/tips/src/tests.rs +++ b/substrate/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/substrate/frame/treasury/src/benchmarking.rs b/substrate/frame/treasury/src/benchmarking.rs index 119516fe2741a118a5d7a69865f22a51fca2347e..64ecbebe0bff9c061f494f5f33dcc3f26e20ebf4 100644 --- a/substrate/frame/treasury/src/benchmarking.rs +++ b/substrate/frame/treasury/src/benchmarking.rs @@ -55,7 +55,7 @@ fn create_approved_proposals<T: Config<I>, I: Instance>(n: u32) -> Result<(), &' let proposal_id = <ProposalCount<I>>::get() - 1; Treasury::<T, I>::approve_proposal(RawOrigin::Root.into(), proposal_id)?; } - ensure!(<Approvals<I>>::get().len() == n as usize, "Not all approved"); + ensure!(<Approvals<T, I>>::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::<T, _>(p)?; let (caller, value, beneficiary_lookup) = setup_proposal::<T, _>(SEED); Treasury::<T, _>::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::<T, _>(); create_approved_proposals::<T, _>(p)?; }: { diff --git a/substrate/frame/treasury/src/lib.rs b/substrate/frame/treasury/src/lib.rs index 7de193dd69848674c966068a80ed33829c995b26..473a570a8725660607c899f7c41f387f1c9e7b83 100644 --- a/substrate/frame/treasury/src/lib.rs +++ b/substrate/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<I=DefaultInstance>: frame_system::Config { /// Runtime hooks to external pallet using treasury to compute spend funds. type SpendFunds: SpendFunds<Self, I>; + + /// The maximum number of approvals that can wait in the spending queue. + type MaxApprovals: Get<u32>; } /// A trait to allow the Treasury Pallet to spend it's funds for other purposes. @@ -180,7 +186,7 @@ decl_storage! { => Option<Proposal<T::AccountId, BalanceOf<T, I>>>; /// Proposal indices that have been approved but not yet awarded. - pub Approvals get(fn approvals): Vec<ProposalIndex>; + pub Approvals get(fn approvals): BoundedVec<ProposalIndex, T::MaxApprovals>; } 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> - #[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!(<Proposals<T, I>>::contains_key(proposal_id), Error::<T, I>::InvalidIndex); - Approvals::<I>::append(proposal_id); + Approvals::<T, I>::try_append(proposal_id).map_err(|_| Error::<T, I>::TooManyApprovals)?; } /// # <weight> @@ -365,7 +373,7 @@ impl<T: Config<I>, I: Instance> Module<T, I> { let mut missed_any = false; let mut imbalance = <PositiveImbalanceOf<T, I>>::zero(); - let proposals_len = Approvals::<I>::mutate(|v| { + let proposals_len = Approvals::<T, I>::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/substrate/frame/treasury/src/tests.rs b/substrate/frame/treasury/src/tests.rs index 3ff9d63b1096a8119c8ebfab0c57d31c9fc73791..cb6d4903a5732449cc8d4c5440e8fde0594eb32e 100644 --- a/substrate/frame/treasury/src/tests.rs +++ b/substrate/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::<Test, _>::TooManyApprovals); + }); +} diff --git a/substrate/frame/treasury/src/weights.rs b/substrate/frame/treasury/src/weights.rs index b8a5625bf062425f7737ce709d32ae6156f4e504..9d627f1c287e29bfbbbb52f146406597b3a2ef34 100644 --- a/substrate/frame/treasury/src/weights.rs +++ b/substrate/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<T>(PhantomData<T>); impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> { 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<T: frame_system::Config> WeightInfo for SubstrateWeight<T> { // 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))