pallet-bounties: allow bounties to never expire

prdoc/pr_7723.prdoc

@@ -0,0 +1,14 @@
+title: '[pallet-bounties] Allow bounties to never expire'
+doc:
+- audience: Runtime Dev
+  description: |
+    Refactored the `update_due` calculation to use `saturating_add`, allowing bounties to remain active   
+    indefinitely without requiring `extend_bounty_expiry` and preventing automatic curator slashing for  
+    inactivity. Previously, setting `BountyUpdatePeriod` to a large value, such as `BlockNumber::max_value()`,  
+    could cause an overflow. 
+
+crates:
+- name: pallet-bounties
+  bump: patch
+- name: pallet-child-bounties
+  bump: patch

substrate/frame/bounties/src/benchmarking.rs

@@ -146,15 +146,26 @@ benchmarks_instance_pallet! {
 		);
 	}
 
-	// Worst case when curator is inactive and any sender unassigns the curator.
+	// Worst case when curator is inactive and any sender unassigns the curator,
+	// or if `BountyUpdatePeriod` is large enough and `RejectOrigin` executes the call.
 	unassign_curator {
 		setup_pot_account::<T, I>();
 		let (curator_lookup, bounty_id) = create_bounty::<T, I>()?;
 		Treasury::<T, I>::on_initialize(frame_system::Pallet::<T>::block_number());
 		let bounty_id = BountyCount::<T, I>::get() - 1;
-		set_block_number::<T, I>(T::SpendPeriod::get() + T::BountyUpdatePeriod::get() + 2u32.into());
-		let caller = whitelisted_caller();
-	}: _(RawOrigin::Signed(caller), bounty_id)
+		let bounty_update_period = T::BountyUpdatePeriod::get();
+		let inactivity_timeout = T::SpendPeriod::get().saturating_add(bounty_update_period);
+		set_block_number::<T, I>(inactivity_timeout.saturating_add(2u32.into()));
+
+		// If `BountyUpdatePeriod` overflows the inactivity timeout the benchmark still executes the slash
+		let origin = if Pallet::<T, I>::treasury_block_number() <= inactivity_timeout {
+			let curator = T::Lookup::lookup(curator_lookup).map_err(<&str>::from)?;
+			T::RejectOrigin::try_successful_origin().unwrap_or_else(|_| RawOrigin::Signed(curator).into())
+		} else {
+			let caller = whitelisted_caller();
+			RawOrigin::Signed(caller).into()
+		};
+	}: _<T::RuntimeOrigin>(origin, bounty_id)
 
 	accept_curator {
 		setup_pot_account::<T, I>();

substrate/frame/bounties/src/lib.rs

@@ -221,7 +221,12 @@ pub mod pallet {
 		#[pallet::constant]
 		type BountyDepositPayoutDelay: Get<BlockNumberFor<Self, I>>;
 
-		/// Bounty duration in blocks.
+		/// The time limit for a curator to act before a bounty expires.
+		///
+		/// The period that starts when a curator is approved, during which they must execute or
+		/// update the bounty via `extend_bounty_expiry`. If missed, the bounty expires, and the
+		/// curator may be slashed. If `BlockNumberFor::MAX`, bounties stay active indefinitely,
+		/// removing the need for `extend_bounty_expiry`.
 		#[pallet::constant]
 		type BountyUpdatePeriod: Get<BlockNumberFor<Self, I>>;
 
@@ -578,8 +583,8 @@ pub mod pallet {
 						T::Currency::reserve(curator, deposit)?;
 						bounty.curator_deposit = deposit;
 
-						let update_due =
-							Self::treasury_block_number() + T::BountyUpdatePeriod::get();
+						let update_due = Self::treasury_block_number()
+							.saturating_add(T::BountyUpdatePeriod::get());
 						bounty.status =
 							BountyStatus::Active { curator: curator.clone(), update_due };
 
@@ -820,9 +825,9 @@ pub mod pallet {
 				match bounty.status {
 					BountyStatus::Active { ref curator, ref mut update_due } => {
 						ensure!(*curator == signer, Error::<T, I>::RequireCurator);
-						*update_due = (Self::treasury_block_number() +
-							T::BountyUpdatePeriod::get())
-						.max(*update_due);
+						*update_due = Self::treasury_block_number()
+							.saturating_add(T::BountyUpdatePeriod::get())
+							.max(*update_due);
 					},
 					_ => return Err(Error::<T, I>::UnexpectedStatus.into()),
 				}

substrate/frame/bounties/src/tests.rs

@@ -137,14 +137,14 @@ parameter_types! {
 	pub const CuratorDepositMultiplier: Permill = Permill::from_percent(50);
 	pub const CuratorDepositMax: Balance = 1_000;
 	pub const CuratorDepositMin: Balance = 3;
-
+	pub static BountyUpdatePeriod: u64 = 20;
 }
 
 impl Config for Test {
 	type RuntimeEvent = RuntimeEvent;
 	type BountyDepositBase = ConstU64<80>;
 	type BountyDepositPayoutDelay = ConstU64<3>;
-	type BountyUpdatePeriod = ConstU64<20>;
+	type BountyUpdatePeriod = BountyUpdatePeriod;
 	type CuratorDepositMultiplier = CuratorDepositMultiplier;
 	type CuratorDepositMax = CuratorDepositMax;
 	type CuratorDepositMin = CuratorDepositMin;
@@ -160,7 +160,7 @@ impl Config<Instance1> for Test {
 	type RuntimeEvent = RuntimeEvent;
 	type BountyDepositBase = ConstU64<80>;
 	type BountyDepositPayoutDelay = ConstU64<3>;
-	type BountyUpdatePeriod = ConstU64<20>;
+	type BountyUpdatePeriod = BountyUpdatePeriod;
 	type CuratorDepositMultiplier = CuratorDepositMultiplier;
 	type CuratorDepositMax = CuratorDepositMax;
 	type CuratorDepositMin = CuratorDepositMin;
@@ -1416,3 +1416,68 @@ fn approve_bounty_with_curator_proposed_unassign_works() {
 		assert_eq!(last_event(), BountiesEvent::CuratorUnassigned { bounty_id: 0 });
 	});
 }
+
+#[test]
+fn accept_curator_sets_update_due_correctly() {
+	ExtBuilder::default().build_and_execute(|| {
+		// Given (BountyUpdatePeriod = 20)
+		let bounty_id = 0;
+		let proposer = 0;
+		let fee = 10;
+		let curator = 4;
+		Balances::make_free_balance_be(&Treasury::account_id(), 101);
+		Balances::make_free_balance_be(&curator, 12);
+		assert_ok!(Bounties::propose_bounty(
+			RuntimeOrigin::signed(proposer),
+			50,
+			b"12345".to_vec()
+		));
+		assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
+		go_to_block(4);
+		assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), bounty_id, curator, fee));
+
+		// When
+		assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(curator), bounty_id));
+
+		// Then
+		assert_eq!(
+			pallet_bounties::Bounties::<Test>::get(bounty_id).unwrap().status,
+			BountyStatus::Active { curator, update_due: 24 }
+		);
+
+		// Given (BountyUpdatePeriod = BlockNumber::max_value())
+		BountyUpdatePeriod::set(BlockNumberFor::<Test>::max_value());
+		Balances::make_free_balance_be(&Treasury1::account_id(), 101);
+		assert_ok!(Bounties1::propose_bounty(
+			RuntimeOrigin::signed(proposer),
+			50,
+			b"12345".to_vec()
+		));
+		assert_ok!(Bounties1::approve_bounty(RuntimeOrigin::root(), bounty_id));
+		go_to_block(6);
+		<Treasury1 as OnInitialize<u64>>::on_initialize(6);
+		assert_ok!(Bounties1::propose_curator(RuntimeOrigin::root(), bounty_id, curator, fee));
+
+		// When
+		assert_ok!(Bounties1::accept_curator(RuntimeOrigin::signed(curator), bounty_id));
+
+		// Then
+		assert_eq!(
+			pallet_bounties::Bounties::<Test, Instance1>::get(bounty_id).unwrap().status,
+			BountyStatus::Active { curator, update_due: BlockNumberFor::<Test>::max_value() }
+		);
+
+		// When
+		assert_ok!(Bounties1::extend_bounty_expiry(
+			RuntimeOrigin::signed(curator),
+			bounty_id,
+			Vec::new()
+		));
+
+		// Then
+		assert_eq!(
+			pallet_bounties::Bounties::<Test, Instance1>::get(bounty_id).unwrap().status,
+			BountyStatus::Active { curator, update_due: BlockNumberFor::<Test>::max_value() }
+		);
+	});
+}

substrate/frame/child-bounties/src/benchmarking.rs

@@ -267,17 +267,31 @@ mod benchmarks {
 		Ok(())
 	}
 
-	// Worst case when curator is inactive and any sender un-assigns the curator.
+	// Worst case when curator is inactive and any sender un-assigns the curator,
+	// or if `BountyUpdatePeriod` is large enough and `RejectOrigin` executes the call.
 	#[benchmark]
 	fn unassign_curator() -> Result<(), BenchmarkError> {
 		setup_pot_account::<T>();
 		let bounty_setup = activate_child_bounty::<T>(0, T::MaximumReasonLength::get())?;
 		Treasury::<T>::on_initialize(frame_system::Pallet::<T>::block_number());
-		set_block_number::<T>(T::SpendPeriod::get() + T::BountyUpdatePeriod::get() + 1u32.into());
-		let caller = whitelisted_caller();
+		let bounty_update_period = T::BountyUpdatePeriod::get();
+		let inactivity_timeout = T::SpendPeriod::get().saturating_add(bounty_update_period);
+		set_block_number::<T>(inactivity_timeout.saturating_add(1u32.into()));
+
+		// If `BountyUpdatePeriod` overflows the inactivity timeout the benchmark still
+		// executes the slash
+		let origin: T::RuntimeOrigin = if Pallet::<T>::treasury_block_number() <= inactivity_timeout
+		{
+			let child_curator = bounty_setup.child_curator;
+			T::RejectOrigin::try_successful_origin()
+				.unwrap_or_else(|_| RawOrigin::Signed(child_curator).into())
+		} else {
+			let caller = whitelisted_caller();
+			RawOrigin::Signed(caller).into()
+		};
 
 		#[extrinsic_call]
-		_(RawOrigin::Signed(caller), bounty_setup.bounty_id, bounty_setup.child_bounty_id);
+		_(origin as T::RuntimeOrigin, bounty_setup.bounty_id, bounty_setup.child_bounty_id);
 
 		Ok(())
 	}