From fa52407856c11ee138a32f2ba744f774fac984d5 Mon Sep 17 00:00:00 2001
From: Shawn Tabrizi <shawntabrizi@gmail.com>
Date: Fri, 1 Nov 2024 06:30:26 -0700
Subject: [PATCH] Update Treasury to Support Relay Chain Block Number Provider
(#3970)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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 <git@kchr.de>
Co-authored-by: Muharem <ismailov.m.h@gmail.com>
---
.../collectives-westend/src/fellowship/mod.rs | 1 +
polkadot/runtime/common/src/impls.rs | 1 +
polkadot/runtime/rococo/src/lib.rs | 1 +
polkadot/runtime/westend/src/lib.rs | 1 +
prdoc/pr_3970.prdoc | 17 +++
substrate/bin/node/runtime/src/lib.rs | 1 +
substrate/frame/bounties/src/benchmarking.rs | 29 ++--
substrate/frame/bounties/src/lib.rs | 26 ++--
substrate/frame/bounties/src/tests.rs | 120 ++++++---------
.../frame/child-bounties/src/benchmarking.rs | 16 +-
substrate/frame/child-bounties/src/lib.rs | 18 ++-
substrate/frame/child-bounties/src/tests.rs | 139 +++++++-----------
substrate/frame/tips/src/tests.rs | 2 +
substrate/frame/treasury/src/lib.rs | 84 +++++++++--
substrate/frame/treasury/src/tests.rs | 66 +++++++--
15 files changed, 302 insertions(+), 220 deletions(-)
create mode 100644 prdoc/pr_3970.prdoc
diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs
index 942e0c294dd..1e8212cf6ac 100644
--- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/fellowship/mod.rs
+++ b/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;
}
diff --git a/polkadot/runtime/common/src/impls.rs b/polkadot/runtime/common/src/impls.rs
index b6a93cf5368..2f79d223d3c 100644
--- a/polkadot/runtime/common/src/impls.rs
+++ b/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 = ();
}
diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs
index 5bcde612cf1..e02a28353d2 100644
--- a/polkadot/runtime/rococo/src/lib.rs
+++ b/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;
}
diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs
index 970ef531899..251d92b03bb 100644
--- a/polkadot/runtime/westend/src/lib.rs
+++ b/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;
}
diff --git a/prdoc/pr_3970.prdoc b/prdoc/pr_3970.prdoc
new file mode 100644
index 00000000000..5c20e744478
--- /dev/null
+++ b/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
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index 12e8dc3e507..7712d8ba954 100644
--- a/substrate/bin/node/runtime/src/lib.rs
+++ b/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 = ();
}
diff --git a/substrate/frame/bounties/src/benchmarking.rs b/substrate/frame/bounties/src/benchmarking.rs
index de93ba5c4ce..6fa60e6938b 100644
--- a/substrate/frame/bounties/src/benchmarking.rs
+++ b/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)?;
diff --git a/substrate/frame/bounties/src/lib.rs b/substrate/frame/bounties/src/lib.rs
index e30d6fa2d14..9d5931f4a60 100644
--- a/substrate/frame/bounties/src/lib.rs
+++ b/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;
diff --git a/substrate/frame/bounties/src/tests.rs b/substrate/frame/bounties/src/tests.rs
index c152391d807..37bcadddae5 100644
--- a/substrate/frame/bounties/src/tests.rs
+++ b/substrate/frame/bounties/src/tests.rs
@@ -40,6 +40,12 @@ use super::Event as BountiesEvent;
type Block = frame_system::mocking::MockBlock<Test>;
+// This function directly jumps to a block number, and calls `on_initialize`.
+fn go_to_block(n: u64) {
+ <Test as pallet_treasury::Config>::BlockNumberProvider::set_block_number(n);
+ <Treasury as OnInitialize<u64>>::on_initialize(n);
+}
+
frame_support::construct_runtime!(
pub enum Test
{
@@ -98,6 +104,7 @@ impl pallet_treasury::Config for Test {
type Paymaster = PayFromAccount<Balances, TreasuryAccount>;
type BalanceConverter = UnityAssetBalanceConversion;
type PayoutPeriod = ConstU64<10>;
+ type BlockNumberProvider = System;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
}
@@ -120,6 +127,7 @@ impl pallet_treasury::Config<Instance1> for Test {
type Paymaster = PayFromAccount<Balances, TreasuryInstance1Account>;
type BalanceConverter = UnityAssetBalanceConversion;
type PayoutPeriod = ConstU64<10>;
+ type BlockNumberProvider = System;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
}
@@ -186,7 +194,9 @@ impl ExtBuilder {
.build_storage()
.unwrap()
.into();
- ext.execute_with(|| System::set_block_number(1));
+ ext.execute_with(|| {
+ <Test as pallet_treasury::Config>::BlockNumberProvider::set_block_number(1)
+ });
ext
}
@@ -232,7 +242,7 @@ fn accepted_spend_proposal_ignored_outside_spend_period() {
assert_ok!({ Treasury::spend_local(RuntimeOrigin::root(), 100, 3) });
- <Treasury as OnInitialize<u64>>::on_initialize(1);
+ go_to_block(1);
assert_eq!(Balances::free_balance(3), 0);
assert_eq!(Treasury::pot(), 100);
});
@@ -245,7 +255,7 @@ fn unused_pot_should_diminish() {
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(pallet_balances::TotalIssuance::<Test>::get(), init_total_issuance + 100);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_eq!(Treasury::pot(), 50);
assert_eq!(pallet_balances::TotalIssuance::<Test>::get(), init_total_issuance + 50);
});
@@ -259,7 +269,7 @@ fn accepted_spend_proposal_enacted_on_spend_period() {
assert_ok!({ Treasury::spend_local(RuntimeOrigin::root(), 100, 3) });
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_eq!(Balances::free_balance(3), 100);
assert_eq!(Treasury::pot(), 0);
});
@@ -273,11 +283,11 @@ fn pot_underflow_should_not_diminish() {
assert_ok!({ Treasury::spend_local(RuntimeOrigin::root(), 150, 3) });
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_eq!(Treasury::pot(), 100); // Pot hasn't changed
assert_ok!(Balances::deposit_into_existing(&Treasury::account_id(), 100));
- <Treasury as OnInitialize<u64>>::on_initialize(4);
+ go_to_block(4);
assert_eq!(Balances::free_balance(3), 150); // Fund has been spent
assert_eq!(Treasury::pot(), 25); // Pot has finally changed
});
@@ -294,12 +304,12 @@ fn treasury_account_doesnt_get_deleted() {
assert_ok!({ Treasury::spend_local(RuntimeOrigin::root(), treasury_balance, 3) });
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_eq!(Treasury::pot(), 100); // Pot hasn't changed
assert_ok!({ Treasury::spend_local(RuntimeOrigin::root(), Treasury::pot(), 3) });
- <Treasury as OnInitialize<u64>>::on_initialize(4);
+ go_to_block(4);
assert_eq!(Treasury::pot(), 0); // Pot is emptied
assert_eq!(Balances::free_balance(Treasury::account_id()), 1); // but the account is still there
});
@@ -322,7 +332,8 @@ fn inexistent_account_works() {
assert_ok!({ Treasury::spend_local(RuntimeOrigin::root(), 99, 3) });
assert_ok!({ Treasury::spend_local(RuntimeOrigin::root(), 1, 3) });
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
+
assert_eq!(Treasury::pot(), 0); // Pot hasn't changed
assert_eq!(Balances::free_balance(3), 0); // Balance of `3` hasn't changed
@@ -330,7 +341,7 @@ fn inexistent_account_works() {
assert_eq!(Treasury::pot(), 99); // Pot now contains funds
assert_eq!(Balances::free_balance(Treasury::account_id()), 100); // Account does exist
- <Treasury as OnInitialize<u64>>::on_initialize(4);
+ go_to_block(4);
assert_eq!(Treasury::pot(), 0); // Pot has changed
assert_eq!(Balances::free_balance(3), 99); // Balance of `3` has changed
@@ -340,8 +351,6 @@ fn inexistent_account_works() {
#[test]
fn propose_bounty_works() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
-
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Treasury::pot(), 100);
@@ -377,8 +386,6 @@ fn propose_bounty_works() {
#[test]
fn propose_bounty_validation_works() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
-
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Treasury::pot(), 100);
@@ -406,7 +413,6 @@ fn propose_bounty_validation_works() {
#[test]
fn close_bounty_works() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_noop!(Bounties::close_bounty(RuntimeOrigin::root(), 0), Error::<Test>::InvalidIndex);
@@ -431,7 +437,6 @@ fn close_bounty_works() {
#[test]
fn approve_bounty_works() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_noop!(
Bounties::approve_bounty(RuntimeOrigin::root(), 0),
@@ -466,7 +471,7 @@ fn approve_bounty_works() {
assert_eq!(Balances::reserved_balance(0), deposit);
assert_eq!(Balances::free_balance(0), 100 - deposit);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
// return deposit
assert_eq!(Balances::reserved_balance(0), 0);
@@ -492,7 +497,6 @@ fn approve_bounty_works() {
#[test]
fn assign_curator_works() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_noop!(
@@ -504,8 +508,7 @@ fn assign_curator_works() {
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_noop!(
Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, 50),
@@ -562,14 +565,12 @@ fn assign_curator_works() {
#[test]
fn unassign_curator_works() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
let fee = 4;
@@ -615,15 +616,13 @@ fn unassign_curator_works() {
#[test]
fn award_and_claim_bounty_works() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
Balances::make_free_balance_be(&4, 10);
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
let fee = 4;
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, fee));
@@ -653,8 +652,7 @@ fn award_and_claim_bounty_works() {
assert_noop!(Bounties::claim_bounty(RuntimeOrigin::signed(1), 0), Error::<Test>::Premature);
- System::set_block_number(5);
- <Treasury as OnInitialize<u64>>::on_initialize(5);
+ go_to_block(5);
assert_ok!(Balances::transfer_allow_death(
RuntimeOrigin::signed(0),
@@ -682,23 +680,20 @@ fn award_and_claim_bounty_works() {
#[test]
fn claim_handles_high_fee() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
Balances::make_free_balance_be(&4, 30);
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, 49));
assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(4), 0));
assert_ok!(Bounties::award_bounty(RuntimeOrigin::signed(4), 0, 3));
- System::set_block_number(5);
- <Treasury as OnInitialize<u64>>::on_initialize(5);
+ go_to_block(5);
// make fee > balance
let res = Balances::slash(&Bounties::bounty_account_id(0), 10);
@@ -723,16 +718,13 @@ fn claim_handles_high_fee() {
#[test]
fn cancel_and_refund() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
-
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Balances::transfer_allow_death(
RuntimeOrigin::signed(0),
@@ -766,14 +758,12 @@ fn cancel_and_refund() {
#[test]
fn award_and_cancel() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 0, 10));
assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(0), 0));
@@ -809,14 +799,12 @@ fn award_and_cancel() {
#[test]
fn expire_and_unassign() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 1, 10));
assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(1), 0));
@@ -824,16 +812,14 @@ fn expire_and_unassign() {
assert_eq!(Balances::free_balance(1), 93);
assert_eq!(Balances::reserved_balance(1), 5);
- System::set_block_number(22);
- <Treasury as OnInitialize<u64>>::on_initialize(22);
+ go_to_block(22);
assert_noop!(
Bounties::unassign_curator(RuntimeOrigin::signed(0), 0),
Error::<Test>::Premature
);
- System::set_block_number(23);
- <Treasury as OnInitialize<u64>>::on_initialize(23);
+ go_to_block(23);
assert_ok!(Bounties::unassign_curator(RuntimeOrigin::signed(0), 0));
@@ -857,7 +843,6 @@ fn expire_and_unassign() {
#[test]
fn extend_expiry() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
Balances::make_free_balance_be(&4, 10);
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
@@ -869,8 +854,7 @@ fn extend_expiry() {
Error::<Test>::UnexpectedStatus
);
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, 10));
assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(4), 0));
@@ -878,8 +862,7 @@ fn extend_expiry() {
assert_eq!(Balances::free_balance(4), 5);
assert_eq!(Balances::reserved_balance(4), 5);
- System::set_block_number(10);
- <Treasury as OnInitialize<u64>>::on_initialize(10);
+ go_to_block(10);
assert_noop!(
Bounties::extend_bounty_expiry(RuntimeOrigin::signed(0), 0, Vec::new()),
@@ -913,8 +896,7 @@ fn extend_expiry() {
}
);
- System::set_block_number(25);
- <Treasury as OnInitialize<u64>>::on_initialize(25);
+ go_to_block(25);
assert_noop!(
Bounties::unassign_curator(RuntimeOrigin::signed(0), 0),
@@ -993,13 +975,11 @@ fn genesis_funding_works() {
#[test]
fn unassign_curator_self() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 1, 10));
assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(1), 0));
@@ -1007,8 +987,7 @@ fn unassign_curator_self() {
assert_eq!(Balances::free_balance(1), 93);
assert_eq!(Balances::reserved_balance(1), 5);
- System::set_block_number(8);
- <Treasury as OnInitialize<u64>>::on_initialize(8);
+ go_to_block(8);
assert_ok!(Bounties::unassign_curator(RuntimeOrigin::signed(1), 0));
@@ -1040,7 +1019,6 @@ fn accept_curator_handles_different_deposit_calculations() {
let value = 88;
let fee = 42;
- 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:
@@ -1048,8 +1026,7 @@ fn accept_curator_handles_different_deposit_calculations() {
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), value, b"12345".to_vec()));
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), bounty_index));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), bounty_index, user, fee));
assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(user), bounty_index));
@@ -1070,8 +1047,7 @@ fn accept_curator_handles_different_deposit_calculations() {
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), value, b"12345".to_vec()));
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), bounty_index));
- System::set_block_number(4);
- <Treasury as OnInitialize<u64>>::on_initialize(4);
+ go_to_block(4);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), bounty_index, user, fee));
assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(user), bounty_index));
@@ -1096,8 +1072,7 @@ fn accept_curator_handles_different_deposit_calculations() {
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), value, b"12345".to_vec()));
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), bounty_index));
- System::set_block_number(6);
- <Treasury as OnInitialize<u64>>::on_initialize(6);
+ go_to_block(6);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), bounty_index, user, fee));
assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(user), bounty_index));
@@ -1114,7 +1089,6 @@ fn approve_bounty_works_second_instance() {
// Set burn to 0 to make tracking funds easier.
Burn::set(Permill::from_percent(0));
- System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
Balances::make_free_balance_be(&Treasury1::account_id(), 201);
assert_eq!(Balances::free_balance(&Treasury::account_id()), 101);
@@ -1122,7 +1096,7 @@ fn approve_bounty_works_second_instance() {
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);
+ go_to_block(2);
<Treasury1 as OnInitialize<u64>>::on_initialize(2);
// Bounties 1 is funded... but from where?
@@ -1137,8 +1111,6 @@ fn approve_bounty_works_second_instance() {
#[test]
fn approve_bounty_insufficient_spend_limit_errors() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
-
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Treasury::pot(), 100);
@@ -1155,8 +1127,6 @@ fn approve_bounty_insufficient_spend_limit_errors() {
#[test]
fn approve_bounty_instance1_insufficient_spend_limit_errors() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
-
Balances::make_free_balance_be(&Treasury1::account_id(), 101);
assert_eq!(Treasury1::pot(), 100);
@@ -1173,7 +1143,6 @@ fn approve_bounty_instance1_insufficient_spend_limit_errors() {
#[test]
fn propose_curator_insufficient_spend_limit_errors() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
// Temporarily set a larger spend limit;
@@ -1181,8 +1150,7 @@ fn propose_curator_insufficient_spend_limit_errors() {
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);
+ go_to_block(2);
SpendLimit::set(50);
// 51 will not work since the limit is 50.
@@ -1196,7 +1164,6 @@ fn propose_curator_insufficient_spend_limit_errors() {
#[test]
fn propose_curator_instance1_insufficient_spend_limit_errors() {
ExtBuilder::default().build_and_execute(|| {
- System::set_block_number(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
// Temporarily set a larger spend limit;
@@ -1204,7 +1171,6 @@ fn propose_curator_instance1_insufficient_spend_limit_errors() {
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);
diff --git a/substrate/frame/child-bounties/src/benchmarking.rs b/substrate/frame/child-bounties/src/benchmarking.rs
index b1f6370f334..68e99e21a45 100644
--- a/substrate/frame/child-bounties/src/benchmarking.rs
+++ b/substrate/frame/child-bounties/src/benchmarking.rs
@@ -25,6 +25,7 @@ use alloc::{vec, vec::Vec};
use frame_benchmarking::v1::{account, benchmarks, whitelisted_caller, BenchmarkError};
use frame_system::{pallet_prelude::BlockNumberFor, RawOrigin};
+use sp_runtime::traits::BlockNumberProvider;
use crate::Pallet as ChildBounties;
use pallet_bounties::Pallet as Bounties;
@@ -56,6 +57,10 @@ struct BenchmarkChildBounty<T: Config> {
reason: Vec<u8>,
}
+fn set_block_number<T: Config>(n: BlockNumberFor<T>) {
+ <T as pallet_treasury::Config>::BlockNumberProvider::set_block_number(n);
+}
+
fn setup_bounty<T: Config>(
user: u32,
description: u32,
@@ -116,7 +121,8 @@ fn activate_bounty<T: Config>(
let approve_origin =
T::SpendOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Bounties::<T>::approve_bounty(approve_origin, child_bounty_setup.bounty_id)?;
- Treasury::<T>::on_initialize(BlockNumberFor::<T>::zero());
+ set_block_number::<T>(T::SpendPeriod::get());
+ Treasury::<T>::on_initialize(frame_system::Pallet::<T>::block_number());
Bounties::<T>::propose_curator(
RawOrigin::Root.into(),
child_bounty_setup.bounty_id,
@@ -231,8 +237,8 @@ benchmarks! {
unassign_curator {
setup_pot_account::<T>();
let bounty_setup = activate_child_bounty::<T>(0, T::MaximumReasonLength::get())?;
- Treasury::<T>::on_initialize(BlockNumberFor::<T>::zero());
- frame_system::Pallet::<T>::set_block_number(T::BountyUpdatePeriod::get() + 1u32.into());
+ 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();
}: _(RawOrigin::Signed(caller), bounty_setup.bounty_id,
bounty_setup.child_bounty_id)
@@ -268,7 +274,7 @@ benchmarks! {
let beneficiary_account: T::AccountId = account("beneficiary", 0, SEED);
let beneficiary = T::Lookup::unlookup(beneficiary_account.clone());
- frame_system::Pallet::<T>::set_block_number(T::BountyDepositPayoutDelay::get());
+ set_block_number::<T>(T::SpendPeriod::get() + T::BountyDepositPayoutDelay::get());
ensure!(T::Currency::free_balance(&beneficiary_account).is_zero(),
"Beneficiary already has balance.");
@@ -305,7 +311,7 @@ benchmarks! {
close_child_bounty_active {
setup_pot_account::<T>();
let bounty_setup = activate_child_bounty::<T>(0, T::MaximumReasonLength::get())?;
- Treasury::<T>::on_initialize(BlockNumberFor::<T>::zero());
+ Treasury::<T>::on_initialize(frame_system::Pallet::<T>::block_number());
}: close_child_bounty(RawOrigin::Root, bounty_setup.bounty_id, bounty_setup.child_bounty_id)
verify {
assert_last_event::<T>(Event::Canceled {
diff --git a/substrate/frame/child-bounties/src/lib.rs b/substrate/frame/child-bounties/src/lib.rs
index 660a30ca5d2..1e970b6ae67 100644
--- a/substrate/frame/child-bounties/src/lib.rs
+++ b/substrate/frame/child-bounties/src/lib.rs
@@ -67,7 +67,10 @@ use frame_support::traits::{
};
use sp_runtime::{
- traits::{AccountIdConversion, BadOrigin, CheckedSub, Saturating, StaticLookup, Zero},
+ traits::{
+ AccountIdConversion, BadOrigin, BlockNumberProvider, CheckedSub, Saturating, StaticLookup,
+ Zero,
+ },
DispatchResult, RuntimeDebug,
};
@@ -523,7 +526,7 @@ pub mod pallet {
let (parent_curator, update_due) =
Self::ensure_bounty_active(parent_bounty_id)?;
if sender == parent_curator ||
- update_due < frame_system::Pallet::<T>::block_number()
+ update_due < Self::treasury_block_number()
{
// Slash the child-bounty curator if
// + the call is made by the parent bounty curator.
@@ -602,7 +605,7 @@ pub mod pallet {
child_bounty.status = ChildBountyStatus::PendingPayout {
curator: signer,
beneficiary: beneficiary.clone(),
- unlock_at: frame_system::Pallet::<T>::block_number() +
+ unlock_at: Self::treasury_block_number() +
T::BountyDepositPayoutDelay::get(),
};
Ok(())
@@ -664,7 +667,7 @@ pub mod pallet {
// Ensure block number is elapsed for processing the
// claim.
ensure!(
- frame_system::Pallet::<T>::block_number() >= *unlock_at,
+ Self::treasury_block_number() >= *unlock_at,
BountiesError::<T>::Premature,
);
@@ -772,6 +775,13 @@ pub mod pallet {
}
impl<T: Config> Pallet<T> {
+ /// 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>::BlockNumberProvider::current_block_number()
+ }
+
// This function will calculate the deposit of a curator.
fn calculate_curator_deposit(
parent_curator: &T::AccountId,
diff --git a/substrate/frame/child-bounties/src/tests.rs b/substrate/frame/child-bounties/src/tests.rs
index 125844fa70e..96d01b03560 100644
--- a/substrate/frame/child-bounties/src/tests.rs
+++ b/substrate/frame/child-bounties/src/tests.rs
@@ -42,6 +42,12 @@ use super::Event as ChildBountiesEvent;
type Block = frame_system::mocking::MockBlock<Test>;
type BountiesError = pallet_bounties::Error<Test>;
+// This function directly jumps to a block number, and calls `on_initialize`.
+fn go_to_block(n: u64) {
+ <Test as pallet_treasury::Config>::BlockNumberProvider::set_block_number(n);
+ <Treasury as OnInitialize<u64>>::on_initialize(n);
+}
+
frame_support::construct_runtime!(
pub enum Test
{
@@ -98,6 +104,7 @@ impl pallet_treasury::Config for Test {
type Paymaster = PayFromAccount<Balances, TreasuryAccount>;
type BalanceConverter = UnityAssetBalanceConversion;
type PayoutPeriod = ConstU64<10>;
+ type BlockNumberProvider = System;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
}
@@ -184,15 +191,14 @@ fn add_child_bounty() {
// Curator, child-bounty curator & beneficiary.
// Make the parent bounty.
- System::set_block_number(1);
+ go_to_block(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
let fee = 8;
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, fee));
@@ -278,7 +284,7 @@ fn child_bounty_assign_curator() {
// 3, Test for DB state of `ChildBounties`.
// Make the parent bounty.
- System::set_block_number(1);
+ go_to_block(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
Balances::make_free_balance_be(&4, 101);
Balances::make_free_balance_be(&8, 101);
@@ -287,8 +293,7 @@ fn child_bounty_assign_curator() {
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
let fee = 4;
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, fee));
@@ -383,7 +388,7 @@ fn child_bounty_assign_curator() {
fn award_claim_child_bounty() {
new_test_ext().execute_with(|| {
// Make the parent bounty.
- System::set_block_number(1);
+ go_to_block(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Balances::free_balance(Treasury::account_id()), 101);
assert_eq!(Balances::reserved_balance(Treasury::account_id()), 0);
@@ -396,8 +401,7 @@ fn award_claim_child_bounty() {
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, 6));
assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(4), 0));
@@ -449,7 +453,7 @@ fn award_claim_child_bounty() {
BountiesError::Premature
);
- System::set_block_number(9);
+ go_to_block(9);
assert_ok!(ChildBounties::claim_child_bounty(RuntimeOrigin::signed(7), 0, 0));
@@ -474,7 +478,7 @@ fn award_claim_child_bounty() {
fn close_child_bounty_added() {
new_test_ext().execute_with(|| {
// Make the parent bounty.
- System::set_block_number(1);
+ go_to_block(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Balances::free_balance(Treasury::account_id()), 101);
assert_eq!(Balances::reserved_balance(Treasury::account_id()), 0);
@@ -487,8 +491,7 @@ fn close_child_bounty_added() {
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, 6));
@@ -504,7 +507,7 @@ fn close_child_bounty_added() {
assert_eq!(last_event(), ChildBountiesEvent::Added { index: 0, child_index: 0 });
- System::set_block_number(4);
+ go_to_block(4);
// Close child-bounty.
// Wrong origin.
@@ -531,7 +534,7 @@ fn close_child_bounty_added() {
fn close_child_bounty_active() {
new_test_ext().execute_with(|| {
// Make the parent bounty.
- System::set_block_number(1);
+ go_to_block(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Balances::free_balance(Treasury::account_id()), 101);
assert_eq!(Balances::reserved_balance(Treasury::account_id()), 0);
@@ -544,8 +547,7 @@ fn close_child_bounty_active() {
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, 6));
@@ -589,7 +591,7 @@ fn close_child_bounty_active() {
fn close_child_bounty_pending() {
new_test_ext().execute_with(|| {
// Make the parent bounty.
- System::set_block_number(1);
+ go_to_block(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Balances::free_balance(Treasury::account_id()), 101);
assert_eq!(Balances::reserved_balance(Treasury::account_id()), 0);
@@ -602,8 +604,7 @@ fn close_child_bounty_pending() {
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
let parent_fee = 6;
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, parent_fee));
@@ -650,7 +651,7 @@ fn close_child_bounty_pending() {
fn child_bounty_added_unassign_curator() {
new_test_ext().execute_with(|| {
// Make the parent bounty.
- System::set_block_number(1);
+ go_to_block(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Balances::free_balance(Treasury::account_id()), 101);
assert_eq!(Balances::reserved_balance(Treasury::account_id()), 0);
@@ -663,8 +664,7 @@ fn child_bounty_added_unassign_curator() {
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, 6));
@@ -692,7 +692,7 @@ fn child_bounty_added_unassign_curator() {
fn child_bounty_curator_proposed_unassign_curator() {
new_test_ext().execute_with(|| {
// Make the parent bounty.
- System::set_block_number(1);
+ go_to_block(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Balances::free_balance(Treasury::account_id()), 101);
assert_eq!(Balances::reserved_balance(Treasury::account_id()), 0);
@@ -705,8 +705,7 @@ fn child_bounty_curator_proposed_unassign_curator() {
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, 6));
@@ -767,7 +766,7 @@ fn child_bounty_active_unassign_curator() {
// bounty. Unassign from random account. Should slash.
new_test_ext().execute_with(|| {
// Make the parent bounty.
- System::set_block_number(1);
+ go_to_block(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Balances::free_balance(Treasury::account_id()), 101);
assert_eq!(Balances::reserved_balance(Treasury::account_id()), 0);
@@ -782,8 +781,7 @@ fn child_bounty_active_unassign_curator() {
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, 6));
assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(4), 0));
@@ -797,8 +795,7 @@ fn child_bounty_active_unassign_curator() {
));
assert_eq!(last_event(), ChildBountiesEvent::Added { index: 0, child_index: 0 });
- System::set_block_number(3);
- <Treasury as OnInitialize<u64>>::on_initialize(3);
+ go_to_block(3);
// Propose and accept curator for child-bounty.
let fee = 6;
@@ -817,8 +814,7 @@ fn child_bounty_active_unassign_curator() {
}
);
- System::set_block_number(4);
- <Treasury as OnInitialize<u64>>::on_initialize(4);
+ go_to_block(4);
// Unassign curator - from reject origin.
assert_ok!(ChildBounties::unassign_curator(RuntimeOrigin::root(), 0, 0));
@@ -856,8 +852,7 @@ fn child_bounty_active_unassign_curator() {
}
);
- System::set_block_number(5);
- <Treasury as OnInitialize<u64>>::on_initialize(5);
+ go_to_block(5);
// Unassign curator again - from parent curator.
assert_ok!(ChildBounties::unassign_curator(RuntimeOrigin::signed(4), 0, 0));
@@ -893,8 +888,7 @@ fn child_bounty_active_unassign_curator() {
}
);
- System::set_block_number(6);
- <Treasury as OnInitialize<u64>>::on_initialize(6);
+ go_to_block(6);
// Unassign curator again - from child-bounty curator.
assert_ok!(ChildBounties::unassign_curator(RuntimeOrigin::signed(6), 0, 0));
@@ -932,8 +926,7 @@ fn child_bounty_active_unassign_curator() {
}
);
- System::set_block_number(7);
- <Treasury as OnInitialize<u64>>::on_initialize(7);
+ go_to_block(7);
// Unassign curator again - from non curator; non reject origin; some random guy.
// Bounty update period is not yet complete.
@@ -942,8 +935,7 @@ fn child_bounty_active_unassign_curator() {
BountiesError::Premature
);
- System::set_block_number(20);
- <Treasury as OnInitialize<u64>>::on_initialize(20);
+ go_to_block(20);
// Unassign child curator from random account after inactivity.
assert_ok!(ChildBounties::unassign_curator(RuntimeOrigin::signed(3), 0, 0));
@@ -972,7 +964,7 @@ fn parent_bounty_inactive_unassign_curator_child_bounty() {
// This can happen when the curator of parent bounty has been unassigned.
new_test_ext().execute_with(|| {
// Make the parent bounty.
- System::set_block_number(1);
+ go_to_block(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Balances::free_balance(Treasury::account_id()), 101);
assert_eq!(Balances::reserved_balance(Treasury::account_id()), 0);
@@ -987,8 +979,7 @@ fn parent_bounty_inactive_unassign_curator_child_bounty() {
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, 6));
assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(4), 0));
@@ -1002,8 +993,7 @@ fn parent_bounty_inactive_unassign_curator_child_bounty() {
));
assert_eq!(last_event(), ChildBountiesEvent::Added { index: 0, child_index: 0 });
- System::set_block_number(3);
- <Treasury as OnInitialize<u64>>::on_initialize(3);
+ go_to_block(3);
// Propose and accept curator for child-bounty.
let fee = 8;
@@ -1022,14 +1012,12 @@ fn parent_bounty_inactive_unassign_curator_child_bounty() {
}
);
- System::set_block_number(4);
- <Treasury as OnInitialize<u64>>::on_initialize(4);
+ go_to_block(4);
// Unassign parent bounty curator.
assert_ok!(Bounties::unassign_curator(RuntimeOrigin::root(), 0));
- System::set_block_number(5);
- <Treasury as OnInitialize<u64>>::on_initialize(5);
+ go_to_block(5);
// Try unassign child-bounty curator - from non curator; non reject
// origin; some random guy. Bounty update period is not yet complete.
@@ -1057,15 +1045,13 @@ fn parent_bounty_inactive_unassign_curator_child_bounty() {
assert_eq!(Balances::free_balance(8), 101 - expected_child_deposit);
assert_eq!(Balances::reserved_balance(8), 0); // slashed
- System::set_block_number(6);
- <Treasury as OnInitialize<u64>>::on_initialize(6);
+ go_to_block(6);
// Propose and accept curator for parent-bounty again.
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 5, 6));
assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(5), 0));
- System::set_block_number(7);
- <Treasury as OnInitialize<u64>>::on_initialize(7);
+ go_to_block(7);
// Propose and accept curator for child-bounty again.
let fee = 2;
@@ -1084,8 +1070,7 @@ fn parent_bounty_inactive_unassign_curator_child_bounty() {
}
);
- System::set_block_number(8);
- <Treasury as OnInitialize<u64>>::on_initialize(8);
+ go_to_block(8);
assert_noop!(
ChildBounties::unassign_curator(RuntimeOrigin::signed(3), 0, 0),
@@ -1095,8 +1080,7 @@ fn parent_bounty_inactive_unassign_curator_child_bounty() {
// Unassign parent bounty curator again.
assert_ok!(Bounties::unassign_curator(RuntimeOrigin::signed(5), 0));
- System::set_block_number(9);
- <Treasury as OnInitialize<u64>>::on_initialize(9);
+ go_to_block(9);
// Unassign curator again - from parent curator.
assert_ok!(ChildBounties::unassign_curator(RuntimeOrigin::signed(7), 0, 0));
@@ -1123,7 +1107,7 @@ fn parent_bounty_inactive_unassign_curator_child_bounty() {
fn close_parent_with_child_bounty() {
new_test_ext().execute_with(|| {
// Make the parent bounty.
- System::set_block_number(1);
+ go_to_block(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Balances::free_balance(Treasury::account_id()), 101);
assert_eq!(Balances::reserved_balance(Treasury::account_id()), 0);
@@ -1142,8 +1126,7 @@ fn close_parent_with_child_bounty() {
Error::<Test>::ParentBountyNotActive
);
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, 6));
assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(4), 0));
@@ -1157,8 +1140,7 @@ fn close_parent_with_child_bounty() {
));
assert_eq!(last_event(), ChildBountiesEvent::Added { index: 0, child_index: 0 });
- System::set_block_number(4);
- <Treasury as OnInitialize<u64>>::on_initialize(4);
+ go_to_block(4);
// Try close parent-bounty.
// Child bounty active, can't close parent.
@@ -1167,8 +1149,6 @@ fn close_parent_with_child_bounty() {
BountiesError::HasActiveChildBounty
);
- System::set_block_number(2);
-
// Close child-bounty.
assert_ok!(ChildBounties::close_child_bounty(RuntimeOrigin::root(), 0, 0));
@@ -1187,7 +1167,7 @@ fn children_curator_fee_calculation_test() {
// from parent bounty fee when claiming bounties.
new_test_ext().execute_with(|| {
// Make the parent bounty.
- System::set_block_number(1);
+ go_to_block(1);
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Balances::free_balance(Treasury::account_id()), 101);
assert_eq!(Balances::reserved_balance(Treasury::account_id()), 0);
@@ -1199,8 +1179,7 @@ fn children_curator_fee_calculation_test() {
assert_ok!(Bounties::propose_bounty(RuntimeOrigin::signed(0), 50, b"12345".to_vec()));
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), 0));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(RuntimeOrigin::root(), 0, 4, 6));
assert_ok!(Bounties::accept_curator(RuntimeOrigin::signed(4), 0));
@@ -1214,8 +1193,7 @@ fn children_curator_fee_calculation_test() {
));
assert_eq!(last_event(), ChildBountiesEvent::Added { index: 0, child_index: 0 });
- System::set_block_number(4);
- <Treasury as OnInitialize<u64>>::on_initialize(4);
+ go_to_block(4);
let fee = 6;
@@ -1245,7 +1223,7 @@ fn children_curator_fee_calculation_test() {
}
);
- System::set_block_number(9);
+ go_to_block(9);
// Claim child-bounty.
assert_ok!(ChildBounties::claim_child_bounty(RuntimeOrigin::signed(7), 0, 0));
@@ -1256,7 +1234,7 @@ fn children_curator_fee_calculation_test() {
// Award the parent bounty.
assert_ok!(Bounties::award_bounty(RuntimeOrigin::signed(4), 0, 9));
- System::set_block_number(15);
+ go_to_block(15);
// Claim the parent bounty.
assert_ok!(Bounties::claim_bounty(RuntimeOrigin::signed(9), 0));
@@ -1282,7 +1260,7 @@ fn accept_curator_handles_different_deposit_calculations() {
let parent_value = 1_000_000;
let parent_fee = 10_000;
- System::set_block_number(1);
+ go_to_block(1);
Balances::make_free_balance_be(&Treasury::account_id(), parent_value * 3);
Balances::make_free_balance_be(&parent_curator, parent_fee * 100);
assert_ok!(Bounties::propose_bounty(
@@ -1292,8 +1270,7 @@ fn accept_curator_handles_different_deposit_calculations() {
));
assert_ok!(Bounties::approve_bounty(RuntimeOrigin::root(), parent_index));
- System::set_block_number(2);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_ok!(Bounties::propose_curator(
RuntimeOrigin::root(),
@@ -1319,8 +1296,7 @@ fn accept_curator_handles_different_deposit_calculations() {
child_value,
b"12345-p1".to_vec()
));
- System::set_block_number(3);
- <Treasury as OnInitialize<u64>>::on_initialize(3);
+ go_to_block(3);
assert_ok!(ChildBounties::propose_curator(
RuntimeOrigin::signed(parent_curator),
parent_index,
@@ -1354,8 +1330,7 @@ fn accept_curator_handles_different_deposit_calculations() {
child_value,
b"12345-p1".to_vec()
));
- System::set_block_number(4);
- <Treasury as OnInitialize<u64>>::on_initialize(4);
+ go_to_block(4);
assert_ok!(ChildBounties::propose_curator(
RuntimeOrigin::signed(parent_curator),
parent_index,
@@ -1387,8 +1362,7 @@ fn accept_curator_handles_different_deposit_calculations() {
child_value,
b"12345-p1".to_vec()
));
- System::set_block_number(5);
- <Treasury as OnInitialize<u64>>::on_initialize(5);
+ go_to_block(5);
assert_ok!(ChildBounties::propose_curator(
RuntimeOrigin::signed(parent_curator),
parent_index,
@@ -1423,8 +1397,7 @@ fn accept_curator_handles_different_deposit_calculations() {
child_value,
b"12345-p1".to_vec()
));
- System::set_block_number(5);
- <Treasury as OnInitialize<u64>>::on_initialize(5);
+ go_to_block(5);
assert_ok!(ChildBounties::propose_curator(
RuntimeOrigin::signed(parent_curator),
parent_index,
diff --git a/substrate/frame/tips/src/tests.rs b/substrate/frame/tips/src/tests.rs
index 7e4a9368ad0..f6f130b7e26 100644
--- a/substrate/frame/tips/src/tests.rs
+++ b/substrate/frame/tips/src/tests.rs
@@ -119,6 +119,7 @@ impl pallet_treasury::Config for Test {
type Paymaster = PayFromAccount<Balances, TreasuryAccount>;
type BalanceConverter = UnityAssetBalanceConversion;
type PayoutPeriod = ConstU64<10>;
+ type BlockNumberProvider = System;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
}
@@ -141,6 +142,7 @@ impl pallet_treasury::Config<Instance1> for Test {
type Paymaster = PayFromAccount<Balances, TreasuryInstance1Account>;
type BalanceConverter = UnityAssetBalanceConversion;
type PayoutPeriod = ConstU64<10>;
+ type BlockNumberProvider = System;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
}
diff --git a/substrate/frame/treasury/src/lib.rs b/substrate/frame/treasury/src/lib.rs
index ad74495ce09..b21a3694935 100644
--- a/substrate/frame/treasury/src/lib.rs
+++ b/substrate/frame/treasury/src/lib.rs
@@ -89,8 +89,11 @@ use scale_info::TypeInfo;
use alloc::{boxed::Box, collections::btree_map::BTreeMap};
use sp_runtime::{
- traits::{AccountIdConversion, CheckedAdd, Saturating, StaticLookup, Zero},
- Permill, RuntimeDebug,
+ traits::{
+ AccountIdConversion, BlockNumberProvider, CheckedAdd, One, Saturating, StaticLookup,
+ UniqueSaturatedInto, Zero,
+ },
+ PerThing, Permill, RuntimeDebug,
};
use frame_support::{
@@ -103,6 +106,7 @@ use frame_support::{
weights::Weight,
BoundedVec, PalletId,
};
+use frame_system::pallet_prelude::BlockNumberFor;
pub use pallet::*;
pub use weights::WeightInfo;
@@ -275,6 +279,9 @@ pub mod pallet {
/// 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>>;
}
/// Number of proposals that have been made.
@@ -322,6 +329,10 @@ pub mod pallet {
OptionQuery,
>;
+ /// The blocknumber for the last triggered spend period.
+ #[pallet::storage]
+ pub(crate) type LastSpendPeriod<T, I = ()> = StorageValue<_, BlockNumberFor<T>, OptionQuery>;
+
#[pallet::genesis_config]
#[derive(frame_support::DefaultNoBound)]
pub struct GenesisConfig<T: Config<I>, I: 'static = ()> {
@@ -414,7 +425,8 @@ pub mod pallet {
impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> {
/// ## Complexity
/// - `O(A)` where `A` is the number of approvals
- fn on_initialize(n: frame_system::pallet_prelude::BlockNumberFor<T>) -> Weight {
+ fn on_initialize(_do_not_use_local_block_number: BlockNumberFor<T>) -> Weight {
+ let block_number = T::BlockNumberProvider::current_block_number();
let pot = Self::pot();
let deactivated = Deactivated::<T, I>::get();
if pot != deactivated {
@@ -428,17 +440,29 @@ pub mod pallet {
}
// Check to see if we should spend some funds!
- if (n % T::SpendPeriod::get()).is_zero() {
- Self::spend_funds()
+ let last_spend_period = LastSpendPeriod::<T, I>::get()
+ // This unwrap should only occur one time on any blockchain.
+ // `update_last_spend_period` will populate the `LastSpendPeriod` storage if it is
+ // 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());
+
+ // 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() {
+ Self::spend_funds(spend_periods_passed, new_last_spend_period)
} else {
Weight::zero()
}
}
#[cfg(feature = "try-runtime")]
- fn try_state(
- _: frame_system::pallet_prelude::BlockNumberFor<T>,
- ) -> Result<(), sp_runtime::TryRuntimeError> {
+ fn try_state(_: BlockNumberFor<T>) -> Result<(), sp_runtime::TryRuntimeError> {
Self::do_try_state()?;
Ok(())
}
@@ -594,7 +618,7 @@ pub mod pallet {
let max_amount = T::SpendOrigin::ensure_origin(origin)?;
let beneficiary = T::BeneficiaryLookup::lookup(*beneficiary)?;
- let now = frame_system::Pallet::<T>::block_number();
+ let now = T::BlockNumberProvider::current_block_number();
let valid_from = valid_from.unwrap_or(now);
let expire_at = valid_from.saturating_add(T::PayoutPeriod::get());
ensure!(expire_at > now, Error::<T, I>::SpendExpired);
@@ -672,7 +696,7 @@ pub mod pallet {
pub fn payout(origin: OriginFor<T>, index: SpendIndex) -> DispatchResult {
ensure_signed(origin)?;
let mut spend = Spends::<T, I>::get(index).ok_or(Error::<T, I>::InvalidIndex)?;
- let now = frame_system::Pallet::<T>::block_number();
+ let now = T::BlockNumberProvider::current_block_number();
ensure!(now >= spend.valid_from, Error::<T, I>::EarlyPayout);
ensure!(spend.expire_at > now, Error::<T, I>::SpendExpired);
ensure!(
@@ -718,7 +742,7 @@ pub mod pallet {
ensure_signed(origin)?;
let mut spend = Spends::<T, I>::get(index).ok_or(Error::<T, I>::InvalidIndex)?;
- let now = frame_system::Pallet::<T>::block_number();
+ let now = T::BlockNumberProvider::current_block_number();
if now > spend.expire_at && !matches!(spend.status, State::Attempted { .. }) {
// spend has expired and no further status update is expected.
@@ -792,6 +816,25 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
T::PalletId::get().into_account_truncating()
}
+ // 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> {
+ let block_number = T::BlockNumberProvider::current_block_number();
+ let spend_period = T::SpendPeriod::get().max(BlockNumberFor::<T>::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.
+ let last_spend_period = if time_since_last_spend.is_zero() {
+ block_number.saturating_sub(spend_period)
+ } else {
+ // Otherwise, this is the last time we had a spend period.
+ block_number.saturating_sub(time_since_last_spend)
+ };
+ LastSpendPeriod::<T, I>::put(last_spend_period);
+ last_spend_period
+ }
+
/// Public function to proposal_count storage.
pub fn proposal_count() -> ProposalIndex {
ProposalCount::<T, I>::get()
@@ -808,7 +851,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}
/// Spend some money! returns number of approvals before spend.
- pub fn spend_funds() -> Weight {
+ pub fn spend_funds(
+ spend_periods_passed: BlockNumberFor<T>,
+ new_last_spend_period: BlockNumberFor<T>,
+ ) -> Weight {
+ LastSpendPeriod::<T, I>::put(new_last_spend_period);
let mut total_weight = Weight::zero();
let mut budget_remaining = Self::pot();
@@ -860,10 +907,15 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
&mut missed_any,
);
- if !missed_any {
- // burn some proportion of the remaining budget if we run a surplus.
- let burn = (T::Burn::get() * budget_remaining).min(budget_remaining);
- budget_remaining -= burn;
+ if !missed_any && !T::Burn::get().is_zero() {
+ // Get the amount of treasury that should be left after potentially multiple spend
+ // periods have passed.
+ let one_minus_burn = T::Burn::get().left_from_one();
+ let percent_left =
+ one_minus_burn.saturating_pow(spend_periods_passed.unique_saturated_into());
+ let new_budget_remaining = percent_left * budget_remaining;
+ let burn = budget_remaining.saturating_sub(new_budget_remaining);
+ budget_remaining = new_budget_remaining;
let (debit, credit) = T::Currency::pair(burn);
imbalance.subsume(debit);
diff --git a/substrate/frame/treasury/src/tests.rs b/substrate/frame/treasury/src/tests.rs
index 106bfb530a8..a99dd0dd444 100644
--- a/substrate/frame/treasury/src/tests.rs
+++ b/substrate/frame/treasury/src/tests.rs
@@ -97,6 +97,12 @@ fn set_status(id: u64, s: PaymentStatus) {
STATUS.with(|m| m.borrow_mut().insert(id, s));
}
+// This function directly jumps to a block number, and calls `on_initialize`.
+fn go_to_block(n: u64) {
+ <Test as Config>::BlockNumberProvider::set_block_number(n);
+ <Treasury as OnInitialize<u64>>::on_initialize(n);
+}
+
pub struct TestPay;
impl Pay for TestPay {
type Beneficiary = u128;
@@ -187,6 +193,7 @@ impl Config for Test {
type Paymaster = TestPay;
type BalanceConverter = MulBy<ConstU64<2>>;
type PayoutPeriod = SpendPayoutPeriod;
+ type BlockNumberProvider = System;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
}
@@ -268,7 +275,7 @@ fn spend_local_origin_permissioning_works() {
fn spend_local_origin_works() {
ExtBuilder::default().build().execute_with(|| {
// Check that accumulate works when we have Some value in Dummy already.
- Balances::make_free_balance_be(&Treasury::account_id(), 101);
+ Balances::make_free_balance_be(&Treasury::account_id(), 102);
// approve spend of some amount to beneficiary `6`.
assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(10), 5, 6));
assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(10), 5, 6));
@@ -278,12 +285,12 @@ fn spend_local_origin_works() {
assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(12), 20, 6));
assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(13), 50, 6));
// free balance of `6` is zero, spend period has not passed.
- <Treasury as OnInitialize<u64>>::on_initialize(1);
+ go_to_block(1);
assert_eq!(Balances::free_balance(6), 0);
// free balance of `6` is `100`, spend period has passed.
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_eq!(Balances::free_balance(6), 100);
- // `100` spent, `1` burned.
+ // `100` spent, `1` burned, `1` in ED.
assert_eq!(Treasury::pot(), 0);
});
}
@@ -304,7 +311,7 @@ fn accepted_spend_proposal_ignored_outside_spend_period() {
assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(14), 100, 3));
- <Treasury as OnInitialize<u64>>::on_initialize(1);
+ go_to_block(1);
assert_eq!(Balances::free_balance(3), 0);
assert_eq!(Treasury::pot(), 100);
});
@@ -317,7 +324,7 @@ fn unused_pot_should_diminish() {
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(pallet_balances::TotalIssuance::<Test>::get(), init_total_issuance + 100);
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_eq!(Treasury::pot(), 50);
assert_eq!(pallet_balances::TotalIssuance::<Test>::get(), init_total_issuance + 50);
});
@@ -331,7 +338,7 @@ fn accepted_spend_proposal_enacted_on_spend_period() {
assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(14), 100, 3));
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_eq!(Balances::free_balance(3), 100);
assert_eq!(Treasury::pot(), 0);
});
@@ -345,11 +352,11 @@ fn pot_underflow_should_not_diminish() {
assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(14), 150, 3));
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_eq!(Treasury::pot(), 100); // Pot hasn't changed
let _ = Balances::deposit_into_existing(&Treasury::account_id(), 100).unwrap();
- <Treasury as OnInitialize<u64>>::on_initialize(4);
+ go_to_block(4);
assert_eq!(Balances::free_balance(3), 150); // Fund has been spent
assert_eq!(Treasury::pot(), 25); // Pot has finally changed
});
@@ -366,12 +373,12 @@ fn treasury_account_doesnt_get_deleted() {
assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(14), treasury_balance, 3));
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
assert_eq!(Treasury::pot(), 100); // Pot hasn't changed
assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(14), Treasury::pot(), 3));
- <Treasury as OnInitialize<u64>>::on_initialize(4);
+ go_to_block(4);
assert_eq!(Treasury::pot(), 0); // Pot is emptied
assert_eq!(Balances::free_balance(Treasury::account_id()), 1); // but the account is still there
});
@@ -395,7 +402,8 @@ fn inexistent_account_works() {
assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(14), 99, 3));
assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(14), 1, 3));
- <Treasury as OnInitialize<u64>>::on_initialize(2);
+ go_to_block(2);
+
assert_eq!(Treasury::pot(), 0); // Pot hasn't changed
assert_eq!(Balances::free_balance(3), 0); // Balance of `3` hasn't changed
@@ -403,7 +411,7 @@ fn inexistent_account_works() {
assert_eq!(Treasury::pot(), 99); // Pot now contains funds
assert_eq!(Balances::free_balance(Treasury::account_id()), 100); // Account does exist
- <Treasury as OnInitialize<u64>>::on_initialize(4);
+ go_to_block(4);
assert_eq!(Treasury::pot(), 0); // Pot has changed
assert_eq!(Balances::free_balance(3), 99); // Balance of `3` has changed
@@ -936,3 +944,35 @@ fn try_state_spends_invariant_3_works() {
);
});
}
+
+#[test]
+fn multiple_spend_periods_work() {
+ ExtBuilder::default().build().execute_with(|| {
+ // Check that accumulate works when we have Some value in Dummy already.
+ // 100 will be spent, 1024 will be the burn amount, 1 for ED
+ Balances::make_free_balance_be(&Treasury::account_id(), 100 + 1024 + 1);
+ // approve spend of total amount 100 to beneficiary `6`.
+ assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(10), 5, 6));
+ assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(10), 5, 6));
+ assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(10), 5, 6));
+ assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(10), 5, 6));
+ assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(11), 10, 6));
+ assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(12), 20, 6));
+ assert_ok!(Treasury::spend_local(RuntimeOrigin::signed(13), 50, 6));
+ // free balance of `6` is zero, spend period has not passed.
+ go_to_block(1);
+ assert_eq!(Balances::free_balance(6), 0);
+ // free balance of `6` is `100`, spend period has passed.
+ go_to_block(2);
+ assert_eq!(Balances::free_balance(6), 100);
+ // `100` spent, 50% burned
+ assert_eq!(Treasury::pot(), 512);
+
+ // 3 more spends periods pass at once, and an extra block.
+ go_to_block(2 + (3 * 2) + 1);
+ // Pot should be reduced by 50% 3 times, so 1/8th the amount.
+ assert_eq!(Treasury::pot(), 64);
+ // Even though we are on block 9, the last spend period was block 8.
+ assert_eq!(LastSpendPeriod::<Test>::get(), Some(8));
+ });
+}
--
GitLab