Skip to content

Commit

Permalink
Removes constraint in BlockNumberProvider from treasury (#6522)
Browse files Browse the repository at this point in the history
#3970 updated the
treasury pallet to support relay chain block number provider. However,
it added a constraint to the BlockNumberProvider to have the same block
number type as frame_system:

```rust
type BlockNumberProvider: BlockNumberProvider<BlockNumber = BlockNumberFor<Self>>;
```

This PR removes that constraint as suggested by @gui1117
  • Loading branch information
gupnik authored Nov 21, 2024
1 parent 70b6c7b commit a872278
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 34 deletions.
18 changes: 18 additions & 0 deletions prdoc/pr_6522.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
title: Removes constraint in BlockNumberProvider from treasury

doc:
- audience: Runtime Dev
description: |-
https://github.com/paritytech/polkadot-sdk/pull/3970 updated the treasury pallet to support
relay chain block number provider. However, it added a constraint to the `BlockNumberProvider`
trait to have the same block number type as `frame_system`:

```rust
type BlockNumberProvider: BlockNumberProvider<BlockNumber = BlockNumberFor<Self>>;
```

This PR removes that constraint and allows the treasury pallet to use any block number type.

crates:
- name: pallet-treasury
bump: major
6 changes: 3 additions & 3 deletions substrate/frame/bounties/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ use alloc::{vec, vec::Vec};
use frame_benchmarking::v1::{
account, benchmarks_instance_pallet, whitelisted_caller, BenchmarkError,
};
use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
use frame_system::{pallet_prelude::BlockNumberFor as SystemBlockNumberFor, RawOrigin};
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>) {
fn set_block_number<T: Config<I>, I: 'static>(n: BlockNumberFor<T, I>) {
<T as pallet_treasury::Config<I>>::BlockNumberProvider::set_block_number(n);
}

Expand Down Expand Up @@ -132,7 +132,7 @@ benchmarks_instance_pallet! {
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
let bounty_id = BountyCount::<T, I>::get() - 1;
let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Treasury::<T, I>::on_initialize(BlockNumberFor::<T>::zero());
Treasury::<T, I>::on_initialize(SystemBlockNumberFor::<T>::zero());
}: _<T::RuntimeOrigin>(approve_origin, bounty_id, curator_lookup, fee)
verify {
assert_last_event::<T, I>(
Expand Down
19 changes: 12 additions & 7 deletions substrate/frame/bounties/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ use sp_runtime::{
use frame_support::{dispatch::DispatchResultWithPostInfo, traits::EnsureOrigin};

use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;
use frame_system::pallet_prelude::{
ensure_signed, BlockNumberFor as SystemBlockNumberFor, OriginFor,
};
use scale_info::TypeInfo;
pub use weights::WeightInfo;

Expand All @@ -120,6 +122,9 @@ pub type BountyIndex = u32;

type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source;

type BlockNumberFor<T, I = ()> =
<<T as pallet_treasury::Config<I>>::BlockNumberProvider as BlockNumberProvider>::BlockNumber;

/// A bounty proposal.
#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
pub struct Bounty<AccountId, Balance, BlockNumber> {
Expand Down Expand Up @@ -213,11 +218,11 @@ pub mod pallet {

/// The delay period for which a bounty beneficiary need to wait before claim the payout.
#[pallet::constant]
type BountyDepositPayoutDelay: Get<BlockNumberFor<Self>>;
type BountyDepositPayoutDelay: Get<BlockNumberFor<Self, I>>;

/// Bounty duration in blocks.
#[pallet::constant]
type BountyUpdatePeriod: Get<BlockNumberFor<Self>>;
type BountyUpdatePeriod: Get<BlockNumberFor<Self, I>>;

/// The curator deposit is calculated as a percentage of the curator fee.
///
Expand Down Expand Up @@ -326,7 +331,7 @@ pub mod pallet {
_,
Twox64Concat,
BountyIndex,
Bounty<T::AccountId, BalanceOf<T, I>, BlockNumberFor<T>>,
Bounty<T::AccountId, BalanceOf<T, I>, BlockNumberFor<T, I>>,
>;

/// The description of each bounty.
Expand Down Expand Up @@ -876,9 +881,9 @@ pub mod pallet {
}

#[pallet::hooks]
impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> {
impl<T: Config<I>, I: 'static> Hooks<SystemBlockNumberFor<T>> for Pallet<T, I> {
#[cfg(feature = "try-runtime")]
fn try_state(_n: BlockNumberFor<T>) -> Result<(), sp_runtime::TryRuntimeError> {
fn try_state(_n: SystemBlockNumberFor<T>) -> Result<(), sp_runtime::TryRuntimeError> {
Self::do_try_state()
}
}
Expand Down Expand Up @@ -928,7 +933,7 @@ 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> {
pub fn treasury_block_number() -> BlockNumberFor<T, I> {
<T as pallet_treasury::Config<I>>::BlockNumberProvider::current_block_number()
}

Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/child-bounties/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
use alloc::vec;
use frame_benchmarking::{v2::*, BenchmarkError};
use frame_support::ensure;
use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
use frame_system::RawOrigin;
use pallet_bounties::Pallet as Bounties;
use pallet_treasury::Pallet as Treasury;
use sp_runtime::traits::BlockNumberProvider;
Expand Down
8 changes: 6 additions & 2 deletions substrate/frame/child-bounties/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@ use sp_runtime::{
};

use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;
use frame_system::pallet_prelude::{
ensure_signed, BlockNumberFor as SystemBlockNumberFor, OriginFor,
};
use pallet_bounties::BountyStatus;
use scale_info::TypeInfo;
pub use weights::WeightInfo;
Expand All @@ -90,6 +92,8 @@ type BalanceOf<T> = pallet_treasury::BalanceOf<T>;
type BountiesError<T> = pallet_bounties::Error<T>;
type BountyIndex = pallet_bounties::BountyIndex;
type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source;
type BlockNumberFor<T> =
<<T as pallet_treasury::Config>::BlockNumberProvider as BlockNumberProvider>::BlockNumber;

/// A child bounty proposal.
#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
Expand Down Expand Up @@ -810,7 +814,7 @@ pub mod pallet {
}

#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
impl<T: Config> Hooks<SystemBlockNumberFor<T>> for Pallet<T> {
fn integrity_test() {
let parent_bounty_id: BountyIndex = 1;
let child_bounty_id: BountyIndex = 2;
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/treasury/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ mod benchmarks {
None,
);

let valid_from = frame_system::Pallet::<T>::block_number();
let valid_from = T::BlockNumberProvider::current_block_number();
let expire_at = valid_from.saturating_add(T::PayoutPeriod::get());
assert_last_event::<T, I>(
Event::AssetSpendApproved {
Expand Down
40 changes: 21 additions & 19 deletions substrate/frame/treasury/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ use frame_support::{
weights::Weight,
BoundedVec, PalletId,
};
use frame_system::pallet_prelude::BlockNumberFor;
use frame_system::pallet_prelude::BlockNumberFor as SystemBlockNumberFor;

pub use pallet::*;
pub use weights::WeightInfo;
Expand All @@ -122,6 +122,8 @@ pub type NegativeImbalanceOf<T, I = ()> = <<T as Config<I>>::Currency as Currenc
>>::NegativeImbalance;
type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source;
type BeneficiaryLookupOf<T, I> = <<T as Config<I>>::BeneficiaryLookup as StaticLookup>::Source;
pub type BlockNumberFor<T, I = ()> =
<<T as Config<I>>::BlockNumberProvider as BlockNumberProvider>::BlockNumber;

/// A trait to allow the Treasury Pallet to spend it's funds for other purposes.
/// There is an expectation that the implementer of this trait will correctly manage
Expand Down Expand Up @@ -202,7 +204,7 @@ pub mod pallet {
pallet_prelude::*,
traits::tokens::{ConversionFromAssetBalance, PaymentStatus},
};
use frame_system::pallet_prelude::*;
use frame_system::pallet_prelude::{ensure_signed, OriginFor};

#[pallet::pallet]
pub struct Pallet<T, I = ()>(PhantomData<(T, I)>);
Expand All @@ -221,7 +223,7 @@ pub mod pallet {

/// Period between successive spends.
#[pallet::constant]
type SpendPeriod: Get<BlockNumberFor<Self>>;
type SpendPeriod: Get<BlockNumberFor<Self, I>>;

/// Percentage of spare funds (if any) that are burnt per spend period.
#[pallet::constant]
Expand Down Expand Up @@ -277,14 +279,14 @@ pub mod pallet {

/// The period during which an approved treasury spend has to be claimed.
#[pallet::constant]
type PayoutPeriod: Get<BlockNumberFor<Self>>;
type PayoutPeriod: Get<BlockNumberFor<Self, I>>;

/// Helper type for benchmarks.
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper: ArgumentsFactory<Self::AssetKind, Self::Beneficiary>;

/// Provider for the block number. Normally this is the `frame_system` pallet.
type BlockNumberProvider: BlockNumberProvider<BlockNumber = BlockNumberFor<Self>>;
type BlockNumberProvider: BlockNumberProvider;
}

/// DEPRECATED: associated with `spend_local` call and will be removed in May 2025.
Expand Down Expand Up @@ -335,15 +337,15 @@ pub mod pallet {
T::AssetKind,
AssetBalanceOf<T, I>,
T::Beneficiary,
BlockNumberFor<T>,
BlockNumberFor<T, I>,
<T::Paymaster as Pay>::Id,
>,
OptionQuery,
>;

/// The blocknumber for the last triggered spend period.
#[pallet::storage]
pub(crate) type LastSpendPeriod<T, I = ()> = StorageValue<_, BlockNumberFor<T>, OptionQuery>;
pub(crate) type LastSpendPeriod<T, I = ()> = StorageValue<_, BlockNumberFor<T, I>, OptionQuery>;

#[pallet::genesis_config]
#[derive(frame_support::DefaultNoBound)]
Expand Down Expand Up @@ -391,8 +393,8 @@ pub mod pallet {
asset_kind: T::AssetKind,
amount: AssetBalanceOf<T, I>,
beneficiary: T::Beneficiary,
valid_from: BlockNumberFor<T>,
expire_at: BlockNumberFor<T>,
valid_from: BlockNumberFor<T, I>,
expire_at: BlockNumberFor<T, I>,
},
/// An approved spend was voided.
AssetSpendVoided { index: SpendIndex },
Expand Down Expand Up @@ -434,10 +436,10 @@ pub mod pallet {
}

#[pallet::hooks]
impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> {
impl<T: Config<I>, I: 'static> Hooks<SystemBlockNumberFor<T>> for Pallet<T, I> {
/// ## Complexity
/// - `O(A)` where `A` is the number of approvals
fn on_initialize(_do_not_use_local_block_number: BlockNumberFor<T>) -> Weight {
fn on_initialize(_do_not_use_local_block_number: SystemBlockNumberFor<T>) -> Weight {
let block_number = T::BlockNumberProvider::current_block_number();
let pot = Self::pot();
let deactivated = Deactivated::<T, I>::get();
Expand All @@ -458,23 +460,23 @@ pub mod pallet {
// empty.
.unwrap_or_else(|| Self::update_last_spend_period());
let blocks_since_last_spend_period = block_number.saturating_sub(last_spend_period);
let safe_spend_period = T::SpendPeriod::get().max(BlockNumberFor::<T>::one());
let safe_spend_period = T::SpendPeriod::get().max(BlockNumberFor::<T, I>::one());

// Safe because of `max(1)` above.
let (spend_periods_passed, extra_blocks) = (
blocks_since_last_spend_period / safe_spend_period,
blocks_since_last_spend_period % safe_spend_period,
);
let new_last_spend_period = block_number.saturating_sub(extra_blocks);
if spend_periods_passed > BlockNumberFor::<T>::zero() {
if spend_periods_passed > BlockNumberFor::<T, I>::zero() {
Self::spend_funds(spend_periods_passed, new_last_spend_period)
} else {
Weight::zero()
}
}

#[cfg(feature = "try-runtime")]
fn try_state(_: BlockNumberFor<T>) -> Result<(), sp_runtime::TryRuntimeError> {
fn try_state(_: SystemBlockNumberFor<T>) -> Result<(), sp_runtime::TryRuntimeError> {
Self::do_try_state()?;
Ok(())
}
Expand Down Expand Up @@ -638,7 +640,7 @@ pub mod pallet {
asset_kind: Box<T::AssetKind>,
#[pallet::compact] amount: AssetBalanceOf<T, I>,
beneficiary: Box<BeneficiaryLookupOf<T, I>>,
valid_from: Option<BlockNumberFor<T>>,
valid_from: Option<BlockNumberFor<T, I>>,
) -> DispatchResult {
let max_amount = T::SpendOrigin::ensure_origin(origin)?;
let beneficiary = T::BeneficiaryLookup::lookup(*beneficiary)?;
Expand Down Expand Up @@ -844,9 +846,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
// Backfill the `LastSpendPeriod` storage, assuming that no configuration has changed
// since introducing this code. Used specifically for a migration-less switch to populate
// `LastSpendPeriod`.
fn update_last_spend_period() -> BlockNumberFor<T> {
fn update_last_spend_period() -> BlockNumberFor<T, I> {
let block_number = T::BlockNumberProvider::current_block_number();
let spend_period = T::SpendPeriod::get().max(BlockNumberFor::<T>::one());
let spend_period = T::SpendPeriod::get().max(BlockNumberFor::<T, I>::one());
let time_since_last_spend = block_number % spend_period;
// If it happens that this logic runs directly on a spend period block, we need to backdate
// to the last spend period so a spend still occurs this block.
Expand Down Expand Up @@ -889,8 +891,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {

/// Spend some money! returns number of approvals before spend.
pub fn spend_funds(
spend_periods_passed: BlockNumberFor<T>,
new_last_spend_period: BlockNumberFor<T>,
spend_periods_passed: BlockNumberFor<T, I>,
new_last_spend_period: BlockNumberFor<T, I>,
) -> Weight {
LastSpendPeriod::<T, I>::put(new_last_spend_period);
let mut total_weight = Weight::zero();
Expand Down
3 changes: 2 additions & 1 deletion substrate/primitives/runtime/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2349,7 +2349,8 @@ pub trait BlockNumberProvider {
+ TypeInfo
+ Debug
+ MaxEncodedLen
+ Copy;
+ Copy
+ EncodeLike;

/// Returns the current block number.
///
Expand Down

0 comments on commit a872278

Please sign in to comment.