Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Treasury to Support Relay Chain Block Number Provider #3970

Open
wants to merge 37 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
d7678db
add block number provider
shawntabrizi Apr 3, 2024
a8b649d
logic for handling multiple spend periods passed
shawntabrizi Apr 3, 2024
757c7f4
update for new associated type
shawntabrizi Apr 3, 2024
0f74c0c
add and fix tests
shawntabrizi Apr 3, 2024
2b1bd39
Merge branch 'master' into shawntabrizi-treasury
shawntabrizi Apr 3, 2024
b0cc89d
fix bug!
shawntabrizi Apr 3, 2024
f01f0e6
introduce `update_last_spend_period`
shawntabrizi Apr 4, 2024
904dd96
remove test feature flag
shawntabrizi Apr 4, 2024
1add90c
Update substrate/frame/treasury/src/lib.rs
shawntabrizi Apr 10, 2024
7ee9a4b
Merge branch 'master' into shawntabrizi-treasury
shawntabrizi Apr 10, 2024
a45e506
Update mod.rs
shawntabrizi Apr 10, 2024
6a0becc
Merge branch 'master' into shawntabrizi-treasury
shawntabrizi Apr 11, 2024
ad5d05a
Merge branch 'master' into shawntabrizi-treasury
bkchr Apr 15, 2024
64ceef4
Create pr_3970.prdoc
shawntabrizi Apr 15, 2024
e3a06a8
Update substrate/frame/treasury/src/lib.rs
bkchr Apr 15, 2024
e61e0f2
Merge branch 'master' into shawntabrizi-treasury
shawntabrizi Apr 15, 2024
d0027d1
Update pr_3970.prdoc
shawntabrizi Apr 15, 2024
ea02217
Merge branch 'shawntabrizi-treasury' of https://github.com/shawntabri…
shawntabrizi Apr 15, 2024
874db2a
Update mod.rs
shawntabrizi Apr 16, 2024
e155ee6
fix bounties test
shawntabrizi Apr 16, 2024
53a1a2e
update child bounties tests for consistency
shawntabrizi Apr 16, 2024
b1f3407
Merge branch 'master' into shawntabrizi-treasury
shawntabrizi Apr 16, 2024
be1d7d0
Merge branch 'master' into shawntabrizi-treasury
bkchr Apr 20, 2024
aceda46
Merge remote-tracking branch 'upstream/master' into shawntabrizi-trea…
shawntabrizi Apr 28, 2024
e1f25b3
more fixes
shawntabrizi Apr 28, 2024
cc80522
fix `on_initialize(0)` which is invalid
shawntabrizi Apr 28, 2024
b383500
fix up child bounties for block number
shawntabrizi Apr 28, 2024
bf6b8d2
choose a more specific name
shawntabrizi Apr 28, 2024
ee8ce2b
Merge branch 'master' into shawntabrizi-treasury
shawntabrizi Apr 28, 2024
93418c5
Merge remote-tracking branch 'upstream/master' into shawntabrizi-trea…
shawntabrizi Apr 30, 2024
bac943d
Merge remote-tracking branch 'upstream/master' into shawntabrizi-trea…
shawntabrizi Apr 30, 2024
a4f2732
Merge branch 'master' into shawntabrizi-treasury
shawntabrizi May 3, 2024
554eac1
Merge branch 'master' into shawntabrizi-treasury
shawntabrizi May 6, 2024
2ef0ce3
Update prdoc/pr_3970.prdoc
shawntabrizi May 7, 2024
fa68d86
Merge branch 'master' into shawntabrizi-treasury
shawntabrizi May 13, 2024
b90bf4a
Merge branch 'master' into shawntabrizi-treasury
muharem Jun 24, 2024
471cb2b
Merge branch 'master' into shawntabrizi-treasury
shawntabrizi Jun 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 46 additions & 16 deletions substrate/frame/treasury/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,11 @@ use codec::{Decode, Encode, MaxEncodedLen};
use scale_info::TypeInfo;

use sp_runtime::{
traits::{AccountIdConversion, CheckedAdd, Saturating, StaticLookup, Zero},
Permill, RuntimeDebug,
traits::{
AccountIdConversion, BlockNumberProvider, CheckedAdd, One, Saturating, StaticLookup,
UniqueSaturatedInto, Zero,
},
PerThing, Permill, RuntimeDebug,
};
use sp_std::{collections::btree_map::BTreeMap, prelude::*};

Expand All @@ -98,6 +101,7 @@ use frame_support::{
weights::Weight,
PalletId,
};
use frame_system::pallet_prelude::BlockNumberFor;

pub use pallet::*;
pub use weights::WeightInfo;
Expand Down Expand Up @@ -289,6 +293,9 @@ pub mod pallet {
/// Helper type for benchmarks.
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper: ArgumentsFactory<Self::AssetKind, Self::Beneficiary>;

/// Provider for the block number.
shawntabrizi marked this conversation as resolved.
Show resolved Hide resolved
type BlockNumberProvider: BlockNumberProvider<BlockNumber = BlockNumberFor<Self>>;
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
}

/// Number of proposals that have been made.
Expand Down Expand Up @@ -339,6 +346,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 = ()> {
Expand Down Expand Up @@ -437,7 +448,7 @@ 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(n: BlockNumberFor<T>) -> Weight {
let pot = Self::pot();
let deactivated = Deactivated::<T, I>::get();
if pot != deactivated {
Expand All @@ -451,17 +462,27 @@ 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 =
// AUDIT REVIEW TODO! Needs to handle migration story for this better.
LastSpendPeriod::<T, I>::get().unwrap_or(BlockNumberFor::<T>::zero());
Copy link
Member Author

@shawntabrizi shawntabrizi Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call out here

probably needs migration, but perhaps there is a way to avoid it with some clever logic?

else the first time this logic runs, it will execute a ton of burns at once.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// AUDIT REVIEW TODO! Needs to handle migration story for this better.
LastSpendPeriod::<T, I>::get().unwrap_or(BlockNumberFor::<T>::zero());
LastSpendPeriod::<T, I>::get().unwrap_or_else(|| calculate_last_spend_period());

in that way no migration is needed if the block number provider is the local block number

but we will need a migration when switch the block number provider

Copy link
Member Author

@shawntabrizi shawntabrizi Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but we will need a migration when switch the block number provider

Seems unavoidable

Otherwise good suggestion. Done.

let blocks_since_last_spend_period = n.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 = n.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(())
}
Expand Down Expand Up @@ -734,7 +755,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);
Expand Down Expand Up @@ -812,7 +833,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!(
Expand Down Expand Up @@ -858,7 +879,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.
Expand Down Expand Up @@ -942,7 +963,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();
Expand Down Expand Up @@ -994,10 +1019,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);
Expand Down
1 change: 1 addition & 0 deletions substrate/frame/treasury/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,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 = ();
}
Expand Down