Skip to content

Commit a4e66dd

Browse files
gupnikKrayt78
authored andcommitted
Removes constraint in BlockNumberProvider from treasury (paritytech#6522)
paritytech#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
1 parent b8febac commit a4e66dd

8 files changed

Lines changed: 64 additions & 34 deletions

File tree

prdoc/pr_6522.prdoc

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
title: Removes constraint in BlockNumberProvider from treasury
2+
3+
doc:
4+
- audience: Runtime Dev
5+
description: |-
6+
https://github.com/paritytech/polkadot-sdk/pull/3970 updated the treasury pallet to support
7+
relay chain block number provider. However, it added a constraint to the `BlockNumberProvider`
8+
trait to have the same block number type as `frame_system`:
9+
10+
```rust
11+
type BlockNumberProvider: BlockNumberProvider<BlockNumber = BlockNumberFor<Self>>;
12+
```
13+
14+
This PR removes that constraint and allows the treasury pallet to use any block number type.
15+
16+
crates:
17+
- name: pallet-treasury
18+
bump: major

substrate/frame/bounties/src/benchmarking.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,15 @@ use alloc::{vec, vec::Vec};
2525
use frame_benchmarking::v1::{
2626
account, benchmarks_instance_pallet, whitelisted_caller, BenchmarkError,
2727
};
28-
use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
28+
use frame_system::{pallet_prelude::BlockNumberFor as SystemBlockNumberFor, RawOrigin};
2929
use sp_runtime::traits::{BlockNumberProvider, Bounded};
3030

3131
use crate::Pallet as Bounties;
3232
use pallet_treasury::Pallet as Treasury;
3333

3434
const SEED: u32 = 0;
3535

36-
fn set_block_number<T: Config<I>, I: 'static>(n: BlockNumberFor<T>) {
36+
fn set_block_number<T: Config<I>, I: 'static>(n: BlockNumberFor<T, I>) {
3737
<T as pallet_treasury::Config<I>>::BlockNumberProvider::set_block_number(n);
3838
}
3939

@@ -132,7 +132,7 @@ benchmarks_instance_pallet! {
132132
Bounties::<T, I>::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?;
133133
let bounty_id = BountyCount::<T, I>::get() - 1;
134134
let approve_origin = T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
135-
Treasury::<T, I>::on_initialize(BlockNumberFor::<T>::zero());
135+
Treasury::<T, I>::on_initialize(SystemBlockNumberFor::<T>::zero());
136136
}: _<T::RuntimeOrigin>(approve_origin, bounty_id, curator_lookup, fee)
137137
verify {
138138
assert_last_event::<T, I>(

substrate/frame/bounties/src/lib.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,9 @@ use sp_runtime::{
105105
use frame_support::{dispatch::DispatchResultWithPostInfo, traits::EnsureOrigin};
106106

107107
use frame_support::pallet_prelude::*;
108-
use frame_system::pallet_prelude::*;
108+
use frame_system::pallet_prelude::{
109+
ensure_signed, BlockNumberFor as SystemBlockNumberFor, OriginFor,
110+
};
109111
use scale_info::TypeInfo;
110112
pub use weights::WeightInfo;
111113

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

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

125+
type BlockNumberFor<T, I = ()> =
126+
<<T as pallet_treasury::Config<I>>::BlockNumberProvider as BlockNumberProvider>::BlockNumber;
127+
123128
/// A bounty proposal.
124129
#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
125130
pub struct Bounty<AccountId, Balance, BlockNumber> {
@@ -213,11 +218,11 @@ pub mod pallet {
213218

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

218223
/// Bounty duration in blocks.
219224
#[pallet::constant]
220-
type BountyUpdatePeriod: Get<BlockNumberFor<Self>>;
225+
type BountyUpdatePeriod: Get<BlockNumberFor<Self, I>>;
221226

222227
/// The curator deposit is calculated as a percentage of the curator fee.
223228
///
@@ -326,7 +331,7 @@ pub mod pallet {
326331
_,
327332
Twox64Concat,
328333
BountyIndex,
329-
Bounty<T::AccountId, BalanceOf<T, I>, BlockNumberFor<T>>,
334+
Bounty<T::AccountId, BalanceOf<T, I>, BlockNumberFor<T, I>>,
330335
>;
331336

332337
/// The description of each bounty.
@@ -876,9 +881,9 @@ pub mod pallet {
876881
}
877882

878883
#[pallet::hooks]
879-
impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> {
884+
impl<T: Config<I>, I: 'static> Hooks<SystemBlockNumberFor<T>> for Pallet<T, I> {
880885
#[cfg(feature = "try-runtime")]
881-
fn try_state(_n: BlockNumberFor<T>) -> Result<(), sp_runtime::TryRuntimeError> {
886+
fn try_state(_n: SystemBlockNumberFor<T>) -> Result<(), sp_runtime::TryRuntimeError> {
882887
Self::do_try_state()
883888
}
884889
}
@@ -928,7 +933,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
928933
/// Get the block number used in the treasury pallet.
929934
///
930935
/// It may be configured to use the relay chain block number on a parachain.
931-
pub fn treasury_block_number() -> BlockNumberFor<T> {
936+
pub fn treasury_block_number() -> BlockNumberFor<T, I> {
932937
<T as pallet_treasury::Config<I>>::BlockNumberProvider::current_block_number()
933938
}
934939

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
use alloc::vec;
2323
use frame_benchmarking::{v2::*, BenchmarkError};
2424
use frame_support::ensure;
25-
use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
25+
use frame_system::RawOrigin;
2626
use pallet_bounties::Pallet as Bounties;
2727
use pallet_treasury::Pallet as Treasury;
2828
use sp_runtime::traits::BlockNumberProvider;

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,9 @@ use sp_runtime::{
7979
};
8080

8181
use frame_support::pallet_prelude::*;
82-
use frame_system::pallet_prelude::*;
82+
use frame_system::pallet_prelude::{
83+
ensure_signed, BlockNumberFor as SystemBlockNumberFor, OriginFor,
84+
};
8385
use pallet_bounties::BountyStatus;
8486
use scale_info::TypeInfo;
8587
pub use weights::WeightInfo;
@@ -90,6 +92,8 @@ type BalanceOf<T> = pallet_treasury::BalanceOf<T>;
9092
type BountiesError<T> = pallet_bounties::Error<T>;
9193
type BountyIndex = pallet_bounties::BountyIndex;
9294
type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source;
95+
type BlockNumberFor<T> =
96+
<<T as pallet_treasury::Config>::BlockNumberProvider as BlockNumberProvider>::BlockNumber;
9397

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

812816
#[pallet::hooks]
813-
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
817+
impl<T: Config> Hooks<SystemBlockNumberFor<T>> for Pallet<T> {
814818
fn integrity_test() {
815819
let parent_bounty_id: BountyIndex = 1;
816820
let child_bounty_id: BountyIndex = 2;

substrate/frame/treasury/src/benchmarking.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ mod benchmarks {
198198
None,
199199
);
200200

201-
let valid_from = frame_system::Pallet::<T>::block_number();
201+
let valid_from = T::BlockNumberProvider::current_block_number();
202202
let expire_at = valid_from.saturating_add(T::PayoutPeriod::get());
203203
assert_last_event::<T, I>(
204204
Event::AssetSpendApproved {

substrate/frame/treasury/src/lib.rs

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ use frame_support::{
106106
weights::Weight,
107107
BoundedVec, PalletId,
108108
};
109-
use frame_system::pallet_prelude::BlockNumberFor;
109+
use frame_system::pallet_prelude::BlockNumberFor as SystemBlockNumberFor;
110110

111111
pub use pallet::*;
112112
pub use weights::WeightInfo;
@@ -122,6 +122,8 @@ pub type NegativeImbalanceOf<T, I = ()> = <<T as Config<I>>::Currency as Currenc
122122
>>::NegativeImbalance;
123123
type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source;
124124
type BeneficiaryLookupOf<T, I> = <<T as Config<I>>::BeneficiaryLookup as StaticLookup>::Source;
125+
pub type BlockNumberFor<T, I = ()> =
126+
<<T as Config<I>>::BlockNumberProvider as BlockNumberProvider>::BlockNumber;
125127

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

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

222224
/// Period between successive spends.
223225
#[pallet::constant]
224-
type SpendPeriod: Get<BlockNumberFor<Self>>;
226+
type SpendPeriod: Get<BlockNumberFor<Self, I>>;
225227

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

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

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

286288
/// Provider for the block number. Normally this is the `frame_system` pallet.
287-
type BlockNumberProvider: BlockNumberProvider<BlockNumber = BlockNumberFor<Self>>;
289+
type BlockNumberProvider: BlockNumberProvider;
288290
}
289291

290292
/// DEPRECATED: associated with `spend_local` call and will be removed in May 2025.
@@ -335,15 +337,15 @@ pub mod pallet {
335337
T::AssetKind,
336338
AssetBalanceOf<T, I>,
337339
T::Beneficiary,
338-
BlockNumberFor<T>,
340+
BlockNumberFor<T, I>,
339341
<T::Paymaster as Pay>::Id,
340342
>,
341343
OptionQuery,
342344
>;
343345

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

348350
#[pallet::genesis_config]
349351
#[derive(frame_support::DefaultNoBound)]
@@ -391,8 +393,8 @@ pub mod pallet {
391393
asset_kind: T::AssetKind,
392394
amount: AssetBalanceOf<T, I>,
393395
beneficiary: T::Beneficiary,
394-
valid_from: BlockNumberFor<T>,
395-
expire_at: BlockNumberFor<T>,
396+
valid_from: BlockNumberFor<T, I>,
397+
expire_at: BlockNumberFor<T, I>,
396398
},
397399
/// An approved spend was voided.
398400
AssetSpendVoided { index: SpendIndex },
@@ -434,10 +436,10 @@ pub mod pallet {
434436
}
435437

436438
#[pallet::hooks]
437-
impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> {
439+
impl<T: Config<I>, I: 'static> Hooks<SystemBlockNumberFor<T>> for Pallet<T, I> {
438440
/// ## Complexity
439441
/// - `O(A)` where `A` is the number of approvals
440-
fn on_initialize(_do_not_use_local_block_number: BlockNumberFor<T>) -> Weight {
442+
fn on_initialize(_do_not_use_local_block_number: SystemBlockNumberFor<T>) -> Weight {
441443
let block_number = T::BlockNumberProvider::current_block_number();
442444
let pot = Self::pot();
443445
let deactivated = Deactivated::<T, I>::get();
@@ -458,23 +460,23 @@ pub mod pallet {
458460
// empty.
459461
.unwrap_or_else(|| Self::update_last_spend_period());
460462
let blocks_since_last_spend_period = block_number.saturating_sub(last_spend_period);
461-
let safe_spend_period = T::SpendPeriod::get().max(BlockNumberFor::<T>::one());
463+
let safe_spend_period = T::SpendPeriod::get().max(BlockNumberFor::<T, I>::one());
462464

463465
// Safe because of `max(1)` above.
464466
let (spend_periods_passed, extra_blocks) = (
465467
blocks_since_last_spend_period / safe_spend_period,
466468
blocks_since_last_spend_period % safe_spend_period,
467469
);
468470
let new_last_spend_period = block_number.saturating_sub(extra_blocks);
469-
if spend_periods_passed > BlockNumberFor::<T>::zero() {
471+
if spend_periods_passed > BlockNumberFor::<T, I>::zero() {
470472
Self::spend_funds(spend_periods_passed, new_last_spend_period)
471473
} else {
472474
Weight::zero()
473475
}
474476
}
475477

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

890892
/// Spend some money! returns number of approvals before spend.
891893
pub fn spend_funds(
892-
spend_periods_passed: BlockNumberFor<T>,
893-
new_last_spend_period: BlockNumberFor<T>,
894+
spend_periods_passed: BlockNumberFor<T, I>,
895+
new_last_spend_period: BlockNumberFor<T, I>,
894896
) -> Weight {
895897
LastSpendPeriod::<T, I>::put(new_last_spend_period);
896898
let mut total_weight = Weight::zero();

substrate/primitives/runtime/src/traits/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2349,7 +2349,8 @@ pub trait BlockNumberProvider {
23492349
+ TypeInfo
23502350
+ Debug
23512351
+ MaxEncodedLen
2352-
+ Copy;
2352+
+ Copy
2353+
+ EncodeLike;
23532354

23542355
/// Returns the current block number.
23552356
///

0 commit comments

Comments
 (0)