Bounties use SpendOrigin

frame/bounties/src/benchmarking.rs

@@ -97,7 +97,7 @@ benchmarks_instance_pallet! {
 		let (caller, curator, fee, value, reason) = setup_bounty::<T, I>(0, T::MaximumReasonLength::get());
 		Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
 		let bounty_id = BountyCount::<T, I>::get() - 1;
-		let approve_origin = T::ApproveOrigin::successful_origin();
+		let approve_origin = T::SpendOrigin::successful_origin();
 	}: _<T::RuntimeOrigin>(approve_origin, bounty_id)
 
 	propose_curator {
@@ -106,10 +106,10 @@ benchmarks_instance_pallet! {
 		let curator_lookup = T::Lookup::unlookup(curator);
 		Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
 		let bounty_id = BountyCount::<T, I>::get() - 1;
-		let approve_origin = T::ApproveOrigin::successful_origin();
+		let approve_origin = T::SpendOrigin::successful_origin();
 		Bounties::<T, I>::approve_bounty(approve_origin, bounty_id)?;
 		Treasury::<T, I>::on_initialize(T::BlockNumber::zero());
-		let approve_origin = T::ApproveOrigin::successful_origin();
+		let approve_origin = T::SpendOrigin::successful_origin();
 	}: _<T::RuntimeOrigin>(approve_origin, bounty_id, curator_lookup, fee)
 
 	// Worst case when curator is inactive and any sender unassigns the curator.
@@ -128,7 +128,7 @@ benchmarks_instance_pallet! {
 		let curator_lookup = T::Lookup::unlookup(curator.clone());
 		Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
 		let bounty_id = BountyCount::<T, I>::get() - 1;
-		let approve_origin = T::ApproveOrigin::successful_origin();
+		let approve_origin = T::SpendOrigin::successful_origin();
 		Bounties::<T, I>::approve_bounty(approve_origin.clone(), bounty_id)?;
 		Treasury::<T, I>::on_initialize(T::BlockNumber::zero());
 		Bounties::<T, I>::propose_curator(approve_origin, bounty_id, curator_lookup, fee)?;

frame/bounties/src/lib.rs

@@ -357,10 +357,13 @@ pub mod pallet {
 			origin: OriginFor<T>,
 			#[pallet::compact] bounty_id: BountyIndex,
 		) -> DispatchResult {
-			T::ApproveOrigin::ensure_origin(origin)?;
-
+			let max_amount = T::SpendOrigin::ensure_origin(origin)?;
 			Bounties::<T, I>::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResult {
 				let mut bounty = maybe_bounty.as_mut().ok_or(Error::<T, I>::InvalidIndex)?;
+				ensure!(
+					bounty.value <= max_amount,
+					pallet_treasury::Error::<T, I>::InsufficientPermission
+				);
 				ensure!(bounty.status == BountyStatus::Proposed, Error::<T, I>::UnexpectedStatus);
 
 				bounty.status = BountyStatus::Approved;
@@ -387,11 +390,15 @@ pub mod pallet {
 			curator: AccountIdLookupOf<T>,
 			#[pallet::compact] fee: BalanceOf<T, I>,
 		) -> DispatchResult {
-			T::ApproveOrigin::ensure_origin(origin)?;
+			let max_amount = T::SpendOrigin::ensure_origin(origin)?;
 
 			let curator = T::Lookup::lookup(curator)?;
 			Bounties::<T, I>::try_mutate_exists(bounty_id, |maybe_bounty| -> DispatchResult {
 				let mut bounty = maybe_bounty.as_mut().ok_or(Error::<T, I>::InvalidIndex)?;
+				ensure!(
+					bounty.value <= max_amount,
+					pallet_treasury::Error::<T, I>::InsufficientPermission
+				);
 				match bounty.status {
 					BountyStatus::Funded => {},
 					_ => return Err(Error::<T, I>::UnexpectedStatus.into()),

frame/bounties/src/tests.rs

@@ -106,6 +106,8 @@ parameter_types! {
 	pub static Burn: Permill = Permill::from_percent(50);
 	pub const TreasuryPalletId: PalletId = PalletId(*b"py/trsry");
 	pub const TreasuryPalletId2: PalletId = PalletId(*b"py/trsr2");
+	pub static SpendLimit: Balance = u64::MAX;
+	pub static SpendLimit1: Balance = u64::MAX;
 }
 
 impl pallet_treasury::Config for Test {
@@ -124,7 +126,7 @@ impl pallet_treasury::Config for Test {
 	type WeightInfo = ();
 	type SpendFunds = Bounties;
 	type MaxApprovals = ConstU32<100>;
-	type SpendOrigin = frame_support::traits::NeverEnsureOrigin<u64>;
+	type SpendOrigin = frame_system::EnsureRootWithSuccess<Self::AccountId, SpendLimit>;
 }
 
 impl pallet_treasury::Config<Instance1> for Test {
@@ -143,7 +145,7 @@ impl pallet_treasury::Config<Instance1> for Test {
 	type WeightInfo = ();
 	type SpendFunds = Bounties1;
 	type MaxApprovals = ConstU32<100>;
-	type SpendOrigin = frame_support::traits::NeverEnsureOrigin<u64>;
+	type SpendOrigin = frame_system::EnsureRootWithSuccess<Self::AccountId, SpendLimit1>;
 }
 
 parameter_types! {
@@ -185,6 +187,7 @@ impl Config<Instance1> for Test {
 }
 
 type TreasuryError = pallet_treasury::Error<Test>;
+type TreasuryError1 = pallet_treasury::Error<Test, Instance1>;
 
 pub fn new_test_ext() -> sp_io::TestExternalities {
 	let mut ext: sp_io::TestExternalities = GenesisConfig {
@@ -1136,6 +1139,8 @@ fn accept_curator_handles_different_deposit_calculations() {
 		System::set_block_number(1);
 		Balances::make_free_balance_be(&Treasury::account_id(), 101);
 		Balances::make_free_balance_be(&user, 100);
+		// Allow for a larger spend limit:
+		SpendLimit::set(value);
 		assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), value, b"12345".to_vec()));
 		assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), bounty_index));
 
@@ -1182,6 +1187,8 @@ fn accept_curator_handles_different_deposit_calculations() {
 		Balances::make_free_balance_be(&user, starting_balance);
 		Balances::make_free_balance_be(&0, starting_balance);
 
+		// Allow for a larger spend limit:
+		SpendLimit::set(value);
 		assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), value, b"12345".to_vec()));
 		assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), bounty_index));
 
@@ -1209,16 +1216,98 @@ fn approve_bounty_works_second_instance() {
 		assert_eq!(Balances::free_balance(&Treasury::account_id()), 101);
 		assert_eq!(Balances::free_balance(&Treasury1::account_id()), 201);
 
-		assert_ok!(Bounties1::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
+		assert_ok!(Bounties1::propose_bounty(RuntimeOrigin::signed(0), 10, b"12345".to_vec()));
 		assert_ok!(Bounties1::approve_bounty(RuntimeOrigin::root(), 0));
 		<Treasury as OnInitialize<u64>>::on_initialize(2);
 		<Treasury1 as OnInitialize<u64>>::on_initialize(2);
 
 		// Bounties 1 is funded... but from where?
-		assert_eq!(Balances::free_balance(Bounties1::bounty_account_id(0)), 50);
+		assert_eq!(Balances::free_balance(Bounties1::bounty_account_id(0)), 10);
 		// Treasury 1 unchanged
 		assert_eq!(Balances::free_balance(&Treasury::account_id()), 101);
 		// Treasury 2 has funds removed
-		assert_eq!(Balances::free_balance(&Treasury1::account_id()), 201 - 50);
+		assert_eq!(Balances::free_balance(&Treasury1::account_id()), 201 - 10);
+	});
+}
+
+#[test]
+fn approve_bounty_insufficient_spend_limit_errors() {
+	new_test_ext().execute_with(|| {
+		System::set_block_number(1);
+
+		Balances::make_free_balance_be(&Treasury::account_id(), 101);
+		assert_eq!(Treasury::pot(), 100);
+
+		assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 51, b"123".to_vec()));
+		// 51 will not work since the limit is 50.
+		SpendLimit::set(50);
+		assert_noop!(
+			Bounties::approve_bounty(RuntimeOrigin::root(), 0),
+			TreasuryError::InsufficientPermission
+		);
+	});
+}
+
+#[test]
+fn approve_bounty_instance1_insufficient_spend_limit_errors() {
+	new_test_ext().execute_with(|| {
+		System::set_block_number(1);
+
+		Balances::make_free_balance_be(&Treasury1::account_id(), 101);
+		assert_eq!(Treasury1::pot(), 100);
+
+		assert_ok!(Bounties1::propose_bounty(RuntimeOrigin::signed(0), 51, b"123".to_vec()));
+		// 51 will not work since the limit is 50.
+		SpendLimit1::set(50);
+		assert_noop!(
+			Bounties1::approve_bounty(RuntimeOrigin::root(), 0),
+			TreasuryError1::InsufficientPermission
+		);
+	});
+}
+
+#[test]
+fn propose_curator_insufficient_spend_limit_errors() {
+	new_test_ext().execute_with(|| {
+		System::set_block_number(1);
+		Balances::make_free_balance_be(&Treasury::account_id(), 101);
+
+		// Temporarily set a larger spend limit;
+		SpendLimit::set(51);
+		assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 51, b"12345".to_vec()));
+		assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
+
+		System::set_block_number(2);
+		<Treasury as OnInitialize<u64>>::on_initialize(2);
+
+		SpendLimit::set(50);
+		// 51 will not work since the limit is 50.
+		assert_noop!(
+			Bounties::propose_curator(RuntimeOrigin::root(), 0, 0, 0),
+			TreasuryError::InsufficientPermission
+		);
+	});
+}
+
+#[test]
+fn propose_curator_instance1_insufficient_spend_limit_errors() {
+	new_test_ext().execute_with(|| {
+		System::set_block_number(1);
+		Balances::make_free_balance_be(&Treasury::account_id(), 101);
+
+		// Temporarily set a larger spend limit;
+		SpendLimit1::set(11);
+		assert_ok!(Bounties1::propose_bounty(RuntimeOrigin::signed(0), 11, b"12345".to_vec()));
+		assert_ok!(Bounties1::approve_bounty(RuntimeOrigin::root(), 0));
+
+		System::set_block_number(2);
+		<Treasury1 as OnInitialize<u64>>::on_initialize(2);
+
+		SpendLimit1::set(10);
+		// 11 will not work since the limit is 10.
+		assert_noop!(
+			Bounties1::propose_curator(RuntimeOrigin::root(), 0, 0, 0),
+			TreasuryError1::InsufficientPermission
+		);
 	});
 }