Skip to content

Commit

Permalink
[Deprecation] deprecate treasury spend_local call and related items (
Browse files Browse the repository at this point in the history
…#6169)

Resolves #5930

`spend_local` from `treasury` pallet and associated types are
deprecated. `spend_local` was being used before with native currency in
the treasury.

This PR provides a documentation on how to migrate to the `spend` call
instead.

### Migration

#### For users who were using only `spend_local` before

To replace `spend_local` functionality configure `Paymaster` pallet
configuration to be `PayFromAccount` and configure `AssetKind` to be
`()` and use `spend` call instead.
This way `spend` call will function as deprecated `spend_local`.

Example:
```
impl pallet_treasury::Config for Runtime {
    ..
    type AssetKind = ();
    type Paymaster = PayFromAccount<Self::Currency, TreasuryAccount>;
    // convert balance 1:1 ratio with native currency
    type BalanceConverter = UnityAssetBalanceConversion;
    ..
}
```

#### For users who were already using `spend` with all other assets,
except the native asset

Use `NativeOrWithId` type for `AssetKind` and have a `UnionOf` for
native and non-native assets, then use that with `PayAssetFromAccount`.

Example from `kitchensink-runtime`:
```
// Union of native currency and assets
pub type NativeAndAssets =
    UnionOf<Balances, Assets, NativeFromLeft, NativeOrWithId<u32>, AccountId>;

impl pallet_treasury::Config for Runtime {
    ..
    type AssetKind = NativeOrWithId<u32>;
    type Paymaster = PayAssetFromAccount<NativeAndAssets, TreasuryAccount>;
    type BalanceConverter = AssetRate;
    ..
}

// AssetRate pallet configuration
impl pallet_asset_rate::Config for Runtime {
    ..
    type Currency = Balances;
    type AssetKind = NativeOrWithId<u32>;
    ..
}
```

---------

Co-authored-by: DavidK <davidk@parity.io>
Co-authored-by: Muharem <ismailov.m.h@gmail.com>
  • Loading branch information
3 people authored Nov 5, 2024
1 parent 74ec1ee commit 7725890
Show file tree
Hide file tree
Showing 10 changed files with 213 additions and 27 deletions.
63 changes: 63 additions & 0 deletions prdoc/pr_6169.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
title: "[Deprecation] deprecate treasury `spend_local` call and related items"

doc:
- audience: Runtime Dev
description: |
Deprecates `spend_local` from the treasury pallet and items associated with it.

### Migration

#### For users who were using only `spend_local` before

To replace `spend_local` functionality configure `Paymaster` pallet configuration to be `PayFromAccount` and configure `AssetKind` to be `()` and use `spend` call instead.
This way `spend` call will function as deprecated `spend_local`.

Example:
```
impl pallet_treasury::Config for Runtime {
..
type AssetKind = ();
type Paymaster = PayFromAccount<Self::Currency, TreasuryAccount>;
// convert balance 1:1 ratio with native currency
type BalanceConverter = UnityAssetBalanceConversion;
..
}
```

#### For users who were already using `spend` with all other assets, except the native asset

Use `NativeOrWithId` type for `AssetKind` and have a `UnionOf` for native and non-native assets, then use that with `PayAssetFromAccount`.

Example from `kitchensink-runtime`:
```
// Union of native currency and assets
pub type NativeAndAssets =
UnionOf<Balances, Assets, NativeFromLeft, NativeOrWithId<u32>, AccountId>;

impl pallet_treasury::Config for Runtime {
..
type AssetKind = NativeOrWithId<u32>;
type Paymaster = PayAssetFromAccount<NativeAndAssets, TreasuryAccount>;
type BalanceConverter = AssetRate;
..
}

// AssetRate pallet configuration
impl pallet_asset_rate::Config for Runtime {
..
type Currency = Balances;
type AssetKind = NativeOrWithId<u32>;
..
}
```


crates:
- name: pallet-treasury
bump: patch
- name: pallet-bounties
bump: patch
- name: pallet-child-bounties
bump: patch
- name: pallet-tips
bump: patch
47 changes: 42 additions & 5 deletions substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@

extern crate alloc;

#[cfg(feature = "runtime-benchmarks")]
use pallet_asset_rate::AssetKindFactory;
#[cfg(feature = "runtime-benchmarks")]
use pallet_treasury::ArgumentsFactory;
#[cfg(feature = "runtime-benchmarks")]
use polkadot_sdk::sp_core::crypto::FromEntropy;

use polkadot_sdk::*;

use alloc::{vec, vec::Vec};
Expand Down Expand Up @@ -267,6 +274,36 @@ impl Contains<RuntimeCallNameOf<Runtime>> for TxPauseWhitelistedCalls {
}
}

#[cfg(feature = "runtime-benchmarks")]
pub struct AssetRateArguments;
#[cfg(feature = "runtime-benchmarks")]
impl AssetKindFactory<NativeOrWithId<u32>> for AssetRateArguments {
fn create_asset_kind(seed: u32) -> NativeOrWithId<u32> {
if seed % 2 > 0 {
NativeOrWithId::Native
} else {
NativeOrWithId::WithId(seed / 2)
}
}
}

#[cfg(feature = "runtime-benchmarks")]
pub struct PalletTreasuryArguments;
#[cfg(feature = "runtime-benchmarks")]
impl ArgumentsFactory<NativeOrWithId<u32>, AccountId> for PalletTreasuryArguments {
fn create_asset_kind(seed: u32) -> NativeOrWithId<u32> {
if seed % 2 > 0 {
NativeOrWithId::Native
} else {
NativeOrWithId::WithId(seed / 2)
}
}

fn create_beneficiary(seed: [u8; 32]) -> AccountId {
AccountId::from_entropy(&mut seed.as_slice()).unwrap()
}
}

impl pallet_tx_pause::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
type RuntimeCall = RuntimeCall;
Expand Down Expand Up @@ -1260,27 +1297,27 @@ impl pallet_treasury::Config for Runtime {
type WeightInfo = pallet_treasury::weights::SubstrateWeight<Runtime>;
type MaxApprovals = MaxApprovals;
type SpendOrigin = EnsureWithSuccess<EnsureRoot<AccountId>, AccountId, MaxBalance>;
type AssetKind = u32;
type AssetKind = NativeOrWithId<u32>;
type Beneficiary = AccountId;
type BeneficiaryLookup = Indices;
type Paymaster = PayAssetFromAccount<Assets, TreasuryAccount>;
type Paymaster = PayAssetFromAccount<NativeAndAssets, TreasuryAccount>;
type BalanceConverter = AssetRate;
type PayoutPeriod = SpendPayoutPeriod;
type BlockNumberProvider = System;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
type BenchmarkHelper = PalletTreasuryArguments;
}

impl pallet_asset_rate::Config for Runtime {
type CreateOrigin = EnsureRoot<AccountId>;
type RemoveOrigin = EnsureRoot<AccountId>;
type UpdateOrigin = EnsureRoot<AccountId>;
type Currency = Balances;
type AssetKind = u32;
type AssetKind = NativeOrWithId<u32>;
type RuntimeEvent = RuntimeEvent;
type WeightInfo = pallet_asset_rate::weights::SubstrateWeight<Runtime>;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
type BenchmarkHelper = AssetRateArguments;
}

parameter_types! {
Expand Down
1 change: 1 addition & 0 deletions substrate/frame/bounties/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@ pub mod pallet {

/// Bounty indices that have been approved but not yet funded.
#[pallet::storage]
#[allow(deprecated)]
pub type BountyApprovals<T: Config<I>, I: 'static = ()> =
StorageValue<_, BoundedVec<BountyIndex, T::MaxApprovals>, ValueQuery>;

Expand Down
7 changes: 7 additions & 0 deletions substrate/frame/bounties/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ fn expect_events(e: Vec<BountiesEvent<Test>>) {
}

#[test]
#[allow(deprecated)]
fn genesis_config_works() {
ExtBuilder::default().build_and_execute(|| {
assert_eq!(Treasury::pot(), 0);
Expand All @@ -248,6 +249,7 @@ fn minting_works() {
}

#[test]
#[allow(deprecated)]
fn accepted_spend_proposal_ignored_outside_spend_period() {
ExtBuilder::default().build_and_execute(|| {
Balances::make_free_balance_be(&Treasury::account_id(), 101);
Expand All @@ -274,6 +276,7 @@ fn unused_pot_should_diminish() {
}

#[test]
#[allow(deprecated)]
fn accepted_spend_proposal_enacted_on_spend_period() {
ExtBuilder::default().build_and_execute(|| {
Balances::make_free_balance_be(&Treasury::account_id(), 101);
Expand All @@ -288,6 +291,7 @@ fn accepted_spend_proposal_enacted_on_spend_period() {
}

#[test]
#[allow(deprecated)]
fn pot_underflow_should_not_diminish() {
ExtBuilder::default().build_and_execute(|| {
Balances::make_free_balance_be(&Treasury::account_id(), 101);
Expand All @@ -308,6 +312,7 @@ fn pot_underflow_should_not_diminish() {
// Treasury account doesn't get deleted if amount approved to spend is all its free balance.
// i.e. pot should not include existential deposit needed for account survival.
#[test]
#[allow(deprecated)]
fn treasury_account_doesnt_get_deleted() {
ExtBuilder::default().build_and_execute(|| {
Balances::make_free_balance_be(&Treasury::account_id(), 101);
Expand All @@ -330,6 +335,7 @@ fn treasury_account_doesnt_get_deleted() {
// In case treasury account is not existing then it works fine.
// This is useful for chain that will just update runtime.
#[test]
#[allow(deprecated)]
fn inexistent_account_works() {
let mut t = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();
pallet_balances::GenesisConfig::<Test> { balances: vec![(0, 100), (1, 99), (2, 1)] }
Expand Down Expand Up @@ -423,6 +429,7 @@ fn propose_bounty_validation_works() {
}

#[test]
#[allow(deprecated)]
fn close_bounty_works() {
ExtBuilder::default().build_and_execute(|| {
Balances::make_free_balance_be(&Treasury::account_id(), 101);
Expand Down
1 change: 1 addition & 0 deletions substrate/frame/child-bounties/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ fn last_event() -> ChildBountiesEvent<Test> {
}

#[test]
#[allow(deprecated)]
fn genesis_config_works() {
new_test_ext().execute_with(|| {
assert_eq!(Treasury::pot(), 0);
Expand Down
1 change: 1 addition & 0 deletions substrate/frame/tips/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ fn last_event() -> TipEvent<Test> {
}

#[test]
#[allow(deprecated)]
fn genesis_config_works() {
build_and_execute(|| {
assert_eq!(Treasury::pot(), 0);
Expand Down
6 changes: 4 additions & 2 deletions substrate/frame/treasury/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ fn create_approved_proposals<T: Config<I>, I: 'static>(n: u32) -> Result<(), &'s
for i in 0..n {
let (_, value, lookup) = setup_proposal::<T, I>(i);

#[allow(deprecated)]
if let Ok(origin) = &spender {
Treasury::<T, I>::spend_local(origin.clone(), value, lookup)?;
}
Expand Down Expand Up @@ -136,6 +137,7 @@ mod benchmarks {
let (spend_exists, proposal_id) =
if let Ok(origin) = T::SpendOrigin::try_successful_origin() {
let (_, value, beneficiary_lookup) = setup_proposal::<T, _>(SEED);
#[allow(deprecated)]
Treasury::<T, _>::spend_local(origin, value, beneficiary_lookup)?;
let proposal_id = ProposalCount::<T, _>::get() - 1;

Expand All @@ -149,8 +151,8 @@ mod benchmarks {

#[block]
{
let res =
Treasury::<T, _>::remove_approval(reject_origin as T::RuntimeOrigin, proposal_id);
#[allow(deprecated)]
let res = Treasury::<T, _>::remove_approval(reject_origin as T::RuntimeOrigin, proposal_id);

if spend_exists {
assert_ok!(res);
Expand Down
38 changes: 38 additions & 0 deletions substrate/frame/treasury/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,9 @@ pub mod pallet {
/// Runtime hooks to external pallet using treasury to compute spend funds.
type SpendFunds: SpendFunds<Self, I>;

/// DEPRECATED: associated with `spend_local` call and will be removed in May 2025.
/// Refer to </~https://github.com/paritytech/polkadot-sdk/pull/5961> for migration to `spend`.
///
/// The maximum number of approvals that can wait in the spending queue.
///
/// NOTE: This parameter is also used within the Bounties Pallet extension if enabled.
Expand Down Expand Up @@ -284,10 +287,16 @@ pub mod pallet {
type BlockNumberProvider: BlockNumberProvider<BlockNumber = BlockNumberFor<Self>>;
}

/// DEPRECATED: associated with `spend_local` call and will be removed in May 2025.
/// Refer to </~https://github.com/paritytech/polkadot-sdk/pull/5961> for migration to `spend`.
///
/// Number of proposals that have been made.
#[pallet::storage]
pub type ProposalCount<T, I = ()> = StorageValue<_, ProposalIndex, ValueQuery>;

/// DEPRECATED: associated with `spend_local` call and will be removed in May 2025.
/// Refer to </~https://github.com/paritytech/polkadot-sdk/pull/5961> for migration to `spend`.
///
/// Proposals that have been made.
#[pallet::storage]
pub type Proposals<T: Config<I>, I: 'static = ()> = StorageMap<
Expand All @@ -303,6 +312,9 @@ pub mod pallet {
pub type Deactivated<T: Config<I>, I: 'static = ()> =
StorageValue<_, BalanceOf<T, I>, ValueQuery>;

/// DEPRECATED: associated with `spend_local` call and will be removed in May 2025.
/// Refer to </~https://github.com/paritytech/polkadot-sdk/pull/5961> for migration to `spend`.
///
/// Proposal indices that have been approved but not yet awarded.
#[pallet::storage]
pub type Approvals<T: Config<I>, I: 'static = ()> =
Expand Down Expand Up @@ -494,6 +506,10 @@ pub mod pallet {
/// Emits [`Event::SpendApproved`] if successful.
#[pallet::call_index(3)]
#[pallet::weight(T::WeightInfo::spend_local())]
#[deprecated(
note = "The `spend_local` call will be removed by May 2025. Migrate to the new flow and use the `spend` call."
)]
#[allow(deprecated)]
pub fn spend_local(
origin: OriginFor<T>,
#[pallet::compact] amount: BalanceOf<T, I>,
Expand Down Expand Up @@ -523,7 +539,9 @@ pub mod pallet {
.unwrap_or(Ok(()))?;

let beneficiary = T::Lookup::lookup(beneficiary)?;
#[allow(deprecated)]
let proposal_index = ProposalCount::<T, I>::get();
#[allow(deprecated)]
Approvals::<T, I>::try_append(proposal_index)
.map_err(|_| Error::<T, I>::TooManyApprovals)?;
let proposal = Proposal {
Expand All @@ -532,7 +550,9 @@ pub mod pallet {
beneficiary: beneficiary.clone(),
bond: Default::default(),
};
#[allow(deprecated)]
Proposals::<T, I>::insert(proposal_index, proposal);
#[allow(deprecated)]
ProposalCount::<T, I>::put(proposal_index + 1);

Self::deposit_event(Event::SpendApproved { proposal_index, amount, beneficiary });
Expand Down Expand Up @@ -562,12 +582,17 @@ pub mod pallet {
/// in the first place.
#[pallet::call_index(4)]
#[pallet::weight((T::WeightInfo::remove_approval(), DispatchClass::Operational))]
#[deprecated(
note = "The `remove_approval` call will be removed by May 2025. It associated with the deprecated `spend_local` call."
)]
#[allow(deprecated)]
pub fn remove_approval(
origin: OriginFor<T>,
#[pallet::compact] proposal_id: ProposalIndex,
) -> DispatchResult {
T::RejectOrigin::ensure_origin(origin)?;

#[allow(deprecated)]
Approvals::<T, I>::try_mutate(|v| -> DispatchResult {
if let Some(index) = v.iter().position(|x| x == &proposal_id) {
v.remove(index);
Expand Down Expand Up @@ -836,16 +861,28 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
}

/// Public function to proposal_count storage.
#[deprecated(
note = "This function will be removed by May 2025. Configure pallet to use PayFromAccount for Paymaster type instead"
)]
pub fn proposal_count() -> ProposalIndex {
#[allow(deprecated)]
ProposalCount::<T, I>::get()
}

/// Public function to proposals storage.
#[deprecated(
note = "This function will be removed by May 2025. Configure pallet to use PayFromAccount for Paymaster type instead"
)]
pub fn proposals(index: ProposalIndex) -> Option<Proposal<T::AccountId, BalanceOf<T, I>>> {
#[allow(deprecated)]
Proposals::<T, I>::get(index)
}

/// Public function to approvals storage.
#[deprecated(
note = "This function will be removed by May 2025. Configure pallet to use PayFromAccount for Paymaster type instead"
)]
#[allow(deprecated)]
pub fn approvals() -> BoundedVec<ProposalIndex, T::MaxApprovals> {
Approvals::<T, I>::get()
}
Expand All @@ -864,6 +901,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {

let mut missed_any = false;
let mut imbalance = PositiveImbalanceOf::<T, I>::zero();
#[allow(deprecated)]
let proposals_len = Approvals::<T, I>::mutate(|v| {
let proposals_approvals_len = v.len() as u32;
v.retain(|&index| {
Expand Down
2 changes: 2 additions & 0 deletions substrate/frame/treasury/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,13 @@ pub mod cleanup_proposals {
{
fn on_runtime_upgrade() -> frame_support::weights::Weight {
let mut approval_index = BTreeSet::new();
#[allow(deprecated)]
for approval in Approvals::<T, I>::get().iter() {
approval_index.insert(*approval);
}

let mut proposals_processed = 0;
#[allow(deprecated)]
for (proposal_index, p) in Proposals::<T, I>::iter() {
if !approval_index.contains(&proposal_index) {
let err_amount = T::Currency::unreserve(&p.proposer, p.bond);
Expand Down
Loading

0 comments on commit 7725890

Please sign in to comment.