Update Treasury to Support Relay Chain Block Number Provider (#3970)

The goal of this PR is to have the treasury pallet work on a parachain
which does not produce blocks on a regular schedule, thus can use the
relay chain as a block provider.

Because blocks are not produced regularly, we cannot make the assumption
that block number increases monotonically, and thus have new logic to
handle multiple spend periods passing between blocks.

---------

Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Muharem <[email protected]>

cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs

@@ -333,4 +333,5 @@ impl pallet_treasury::Config<FellowshipTreasuryInstance> for Runtime {
 		sp_core::ConstU8<1>,
 		ConstU32<1000>,
 	>;
+	type BlockNumberProvider = crate::System;
 }

polkadot/runtime/common/src/impls.rs

@@ -366,6 +366,7 @@ mod tests {
 		type Paymaster = PayFromAccount<Balances, TreasuryAccount>;
 		type BalanceConverter = UnityAssetBalanceConversion;
 		type PayoutPeriod = ConstU64<0>;
+		type BlockNumberProvider = System;
 		#[cfg(feature = "runtime-benchmarks")]
 		type BenchmarkHelper = ();
 	}

polkadot/runtime/rococo/src/lib.rs

@@ -543,6 +543,7 @@ impl pallet_treasury::Config for Runtime {
 		AssetRate,
 	>;
 	type PayoutPeriod = PayoutSpendPeriod;
+	type BlockNumberProvider = System;
 	#[cfg(feature = "runtime-benchmarks")]
 	type BenchmarkHelper = polkadot_runtime_common::impls::benchmarks::TreasuryArguments;
 }

polkadot/runtime/westend/src/lib.rs

@@ -823,6 +823,7 @@ impl pallet_treasury::Config for Runtime {
 		AssetRate,
 	>;
 	type PayoutPeriod = PayoutSpendPeriod;
+	type BlockNumberProvider = System;
 	#[cfg(feature = "runtime-benchmarks")]
 	type BenchmarkHelper = polkadot_runtime_common::impls::benchmarks::TreasuryArguments;
 }

prdoc/pr_3970.prdoc

@@ -0,0 +1,17 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: Update Treasury to Support Block Number Provider
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      The goal of this PR is to have the treasury pallet work on a parachain which does not produce blocks on a regular schedule, thus can use the relay chain as a block provider. Because blocks are not produced regularly, we cannot make the assumption that block number increases monotonically, and thus have new logic to handle multiple spend periods passing between blocks. To migrate existing treasury implementations, simply add `type BlockNumberProvider = System` to have the same behavior as before.
+
+crates: 
+- name: pallet-treasury
+  bump: major
+- name: pallet-bounties
+  bump: minor
+- name: pallet-child-bounties
+  bump: minor

substrate/bin/node/runtime/src/lib.rs

@@ -1266,6 +1266,7 @@ impl pallet_treasury::Config for Runtime {
 	type Paymaster = PayAssetFromAccount<Assets, TreasuryAccount>;
 	type BalanceConverter = AssetRate;
 	type PayoutPeriod = SpendPayoutPeriod;
+	type BlockNumberProvider = System;
 	#[cfg(feature = "runtime-benchmarks")]
 	type BenchmarkHelper = ();
 }

substrate/frame/bounties/src/benchmarking.rs

@@ -26,13 +26,17 @@ use frame_benchmarking::v1::{
 	account, benchmarks_instance_pallet, whitelisted_caller, BenchmarkError,
 };
 use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
-use sp_runtime::traits::Bounded;
+use sp_runtime::traits::{BlockNumberProvider, Bounded};
 
 use crate::Pallet as Bounties;
 use pallet_treasury::Pallet as Treasury;
 
 const SEED: u32 = 0;
 
+fn set_block_number<T: Config<I>, I: 'static>(n: BlockNumberFor<T>) {
+	<T as pallet_treasury::Config<I>>::BlockNumberProvider::set_block_number(n);
+}
+
 // Create bounties that are approved for use in `on_initialize`.
 fn create_approved_bounties<T: Config<I>, I: 'static>(n: u32) -> Result<(), BenchmarkError> {
 	for i in 0..n {
@@ -78,7 +82,8 @@ fn create_bounty<T: Config<I>, I: 'static>(
 	let approve_origin =
 		T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
 	Bounties::<T, I>::approve_bounty(approve_origin.clone(), bounty_id)?;
-	Treasury::<T, I>::on_initialize(BlockNumberFor::<T>::zero());
+	set_block_number::<T, I>(T::SpendPeriod::get());
+	Treasury::<T, I>::on_initialize(frame_system::Pallet::<T>::block_number());
 	Bounties::<T, I>::propose_curator(approve_origin, bounty_id, curator_lookup.clone(), fee)?;
 	Bounties::<T, I>::accept_curator(RawOrigin::Signed(curator).into(), bounty_id)?;
 	Ok((curator_lookup, bounty_id))
@@ -116,16 +121,17 @@ benchmarks_instance_pallet! {
 		let bounty_id = BountyCount::<T, I>::get() - 1;
 		let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
 		Bounties::<T, I>::approve_bounty(approve_origin.clone(), bounty_id)?;
-		Treasury::<T, I>::on_initialize(BlockNumberFor::<T>::zero());
+		set_block_number::<T, I>(T::SpendPeriod::get());
+		Treasury::<T, I>::on_initialize(frame_system::Pallet::<T>::block_number());
 	}: _<T::RuntimeOrigin>(approve_origin, bounty_id, curator_lookup, fee)
 
 	// Worst case when curator is inactive and any sender unassigns the curator.
 	unassign_curator {
 		setup_pot_account::<T, I>();
 		let (curator_lookup, bounty_id) = create_bounty::<T, I>()?;
-		Treasury::<T, I>::on_initialize(BlockNumberFor::<T>::zero());
+		Treasury::<T, I>::on_initialize(frame_system::Pallet::<T>::block_number());
 		let bounty_id = BountyCount::<T, I>::get() - 1;
-		frame_system::Pallet::<T>::set_block_number(T::BountyUpdatePeriod::get() + 2u32.into());
+		set_block_number::<T, I>(T::SpendPeriod::get() + T::BountyUpdatePeriod::get() + 2u32.into());
 		let caller = whitelisted_caller();
 	}: _(RawOrigin::Signed(caller), bounty_id)
 
@@ -137,14 +143,15 @@ benchmarks_instance_pallet! {
 		let bounty_id = BountyCount::<T, I>::get() - 1;
 		let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
 		Bounties::<T, I>::approve_bounty(approve_origin.clone(), bounty_id)?;
-		Treasury::<T, I>::on_initialize(BlockNumberFor::<T>::zero());
+		set_block_number::<T, I>(T::SpendPeriod::get());
+		Treasury::<T, I>::on_initialize(frame_system::Pallet::<T>::block_number());
 		Bounties::<T, I>::propose_curator(approve_origin, bounty_id, curator_lookup, fee)?;
 	}: _(RawOrigin::Signed(curator), bounty_id)
 
 	award_bounty {
 		setup_pot_account::<T, I>();
 		let (curator_lookup, bounty_id) = create_bounty::<T, I>()?;
-		Treasury::<T, I>::on_initialize(BlockNumberFor::<T>::zero());
+		Treasury::<T, I>::on_initialize(frame_system::Pallet::<T>::block_number());
 
 		let bounty_id = BountyCount::<T, I>::get() - 1;
 		let curator = T::Lookup::lookup(curator_lookup).map_err(<&str>::from)?;
@@ -155,7 +162,7 @@ benchmarks_instance_pallet! {
 	claim_bounty {
 		setup_pot_account::<T, I>();
 		let (curator_lookup, bounty_id) = create_bounty::<T, I>()?;
-		Treasury::<T, I>::on_initialize(BlockNumberFor::<T>::zero());
+		Treasury::<T, I>::on_initialize(frame_system::Pallet::<T>::block_number());
 
 		let bounty_id = BountyCount::<T, I>::get() - 1;
 		let curator = T::Lookup::lookup(curator_lookup).map_err(<&str>::from)?;
@@ -164,7 +171,7 @@ benchmarks_instance_pallet! {
 		let beneficiary = T::Lookup::unlookup(beneficiary_account.clone());
 		Bounties::<T, I>::award_bounty(RawOrigin::Signed(curator.clone()).into(), bounty_id, beneficiary)?;
 
-		frame_system::Pallet::<T>::set_block_number(T::BountyDepositPayoutDelay::get() + 1u32.into());
+		set_block_number::<T, I>(T::SpendPeriod::get() + T::BountyDepositPayoutDelay::get() + 1u32.into());
 		ensure!(T::Currency::free_balance(&beneficiary_account).is_zero(), "Beneficiary already has balance");
 
 	}: _(RawOrigin::Signed(curator), bounty_id)
@@ -184,7 +191,7 @@ benchmarks_instance_pallet! {
 	close_bounty_active {
 		setup_pot_account::<T, I>();
 		let (curator_lookup, bounty_id) = create_bounty::<T, I>()?;
-		Treasury::<T, I>::on_initialize(BlockNumberFor::<T>::zero());
+		Treasury::<T, I>::on_initialize(frame_system::Pallet::<T>::block_number());
 		let bounty_id = BountyCount::<T, I>::get() - 1;
 		let approve_origin =
 			T::RejectOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
@@ -196,7 +203,7 @@ benchmarks_instance_pallet! {
 	extend_bounty_expiry {
 		setup_pot_account::<T, I>();
 		let (curator_lookup, bounty_id) = create_bounty::<T, I>()?;
-		Treasury::<T, I>::on_initialize(BlockNumberFor::<T>::zero());
+		Treasury::<T, I>::on_initialize(frame_system::Pallet::<T>::block_number());
 
 		let bounty_id = BountyCount::<T, I>::get() - 1;
 		let curator = T::Lookup::lookup(curator_lookup).map_err(<&str>::from)?;

substrate/frame/bounties/src/lib.rs

@@ -96,7 +96,7 @@ use frame_support::traits::{
 };
 
 use sp_runtime::{
-	traits::{AccountIdConversion, BadOrigin, Saturating, StaticLookup, Zero},
+	traits::{AccountIdConversion, BadOrigin, BlockNumberProvider, Saturating, StaticLookup, Zero},
 	DispatchResult, Permill, RuntimeDebug,
 };
 
@@ -488,7 +488,7 @@ pub mod pallet {
 								// If the sender is not the curator, and the curator is inactive,
 								// slash the curator.
 								if sender != *curator {
-									let block_number = frame_system::Pallet::<T>::block_number();
+									let block_number = Self::treasury_block_number();
 									if *update_due < block_number {
 										slash_curator(curator, &mut bounty.curator_deposit);
 									// Continue to change bounty status below...
@@ -552,8 +552,8 @@ pub mod pallet {
 						T::Currency::reserve(curator, deposit)?;
 						bounty.curator_deposit = deposit;
 
-						let update_due = frame_system::Pallet::<T>::block_number() +
-							T::BountyUpdatePeriod::get();
+						let update_due =
+							Self::treasury_block_number() + T::BountyUpdatePeriod::get();
 						bounty.status =
 							BountyStatus::Active { curator: curator.clone(), update_due };
 
@@ -607,8 +607,7 @@ pub mod pallet {
 				bounty.status = BountyStatus::PendingPayout {
 					curator: signer,
 					beneficiary: beneficiary.clone(),
-					unlock_at: frame_system::Pallet::<T>::block_number() +
-						T::BountyDepositPayoutDelay::get(),
+					unlock_at: Self::treasury_block_number() + T::BountyDepositPayoutDelay::get(),
 				};
 
 				Ok(())
@@ -639,10 +638,7 @@ pub mod pallet {
 				if let BountyStatus::PendingPayout { curator, beneficiary, unlock_at } =
 					bounty.status
 				{
-					ensure!(
-						frame_system::Pallet::<T>::block_number() >= unlock_at,
-						Error::<T, I>::Premature
-					);
+					ensure!(Self::treasury_block_number() >= unlock_at, Error::<T, I>::Premature);
 					let bounty_account = Self::bounty_account_id(bounty_id);
 					let balance = T::Currency::free_balance(&bounty_account);
 					let fee = bounty.fee.min(balance); // just to be safe
@@ -795,7 +791,7 @@ pub mod pallet {
 				match bounty.status {
 					BountyStatus::Active { ref curator, ref mut update_due } => {
 						ensure!(*curator == signer, Error::<T, I>::RequireCurator);
-						*update_due = (frame_system::Pallet::<T>::block_number() +
+						*update_due = (Self::treasury_block_number() +
 							T::BountyUpdatePeriod::get())
 						.max(*update_due);
 					},
@@ -860,6 +856,14 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 }
 
 impl<T: Config<I>, I: 'static> Pallet<T, I> {
+	/// Get the block number used in the treasury pallet.
+	///
+	/// It may be configured to use the relay chain block number on a parachain.
+	pub fn treasury_block_number() -> BlockNumberFor<T> {
+		<T as pallet_treasury::Config<I>>::BlockNumberProvider::current_block_number()
+	}
+
+	/// Calculate the deposit required for a curator.
 	pub fn calculate_curator_deposit(fee: &BalanceOf<T, I>) -> BalanceOf<T, I> {
 		let mut deposit = T::CuratorDepositMultiplier::get() * *fee;