Add BoundedVec to Treasury Pallet (paritytech#8665)

* bounded treasury approvals * update benchmarks * update configs * cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_treasury --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/treasury/src/weights.rs --template=./.maintain/frame-weight-template.hbs * fix weight param Co-authored-by: Parity Benchmarking Bot <[email protected]>
canyon-network · Apr 26, 2021 · 4cbeeca · 4cbeeca
1 parent 8060a43
commit 4cbeeca
Show file tree

Hide file tree

Showing 7 changed files with 66 additions and 27 deletions.
diff --git 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<Runtime>;
+	type MaxApprovals = MaxApprovals;
 }
 
 impl pallet_bounties::Config for Runtime {

diff --git 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
@@ -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
@@ -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/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<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/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::<Test, _>::TooManyApprovals);
+	});
+}
diff --git 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,32 +46,34 @@ 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;
 }
 
 /// Weights for pallet_treasury using the Substrate node and recommended hardware.
 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))