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

[Deprecation] deprecate treasury spend_local call and related items #6169

Merged
merged 28 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
061d27f
Deprecate Gov_v1 flow components inside treasury pallet
Oct 9, 2024
74ed8e3
Formatting fixes
Oct 23, 2024
6acbeb4
prdoc fixes
Oct 23, 2024
05f5006
More fixes for deprecation messages
Oct 23, 2024
4151276
Allow deprecated dependency in bounties
Oct 24, 2024
8814ba2
Allow more deprecations in bounties tests
Oct 24, 2024
28888a5
Add deprecated to more pallets
Oct 24, 2024
52024b0
Correct version bumps in prdoc
Oct 24, 2024
804e796
Update substrate/frame/treasury/src/lib.rs
davidk-pt Oct 25, 2024
d7d8273
Update substrate/frame/treasury/src/lib.rs
davidk-pt Oct 25, 2024
cd7a9e5
Update substrate/frame/treasury/src/lib.rs
davidk-pt Oct 25, 2024
b24ef4d
Update prdoc/pr_6169.prdoc
davidk-pt Oct 25, 2024
dfe6a4a
Merge branch 'master' into davidk/treasury-deprecate-proposals
davidk-pt Oct 25, 2024
b912a47
Update substrate/frame/treasury/src/lib.rs
davidk-pt Oct 25, 2024
1a78b8f
Change kitchensink runtime to use native and assets for treasury
Oct 25, 2024
c22c7df
Improve prdoc with example
Oct 25, 2024
021fda5
Better markdown headings
Oct 25, 2024
4a35d37
Remove deprecated from items where warning cannot be hidden
Oct 25, 2024
37f3071
Fix allow deprecations for tests and benchmarks
Oct 25, 2024
82aae6b
Merge branch 'master' into davidk/treasury-deprecate-proposals
davidk-pt Oct 25, 2024
af300f7
Fix prdoc
Oct 25, 2024
fef1701
Add benchmarks arguments to kitchensink runtime
Oct 25, 2024
b9f2ee0
Add deprecation in documentation of the items
Oct 26, 2024
f11fa40
Merge branch 'master' into davidk/treasury-deprecate-proposals
davidk-pt Oct 26, 2024
7904975
Merge branch 'master' into davidk/treasury-deprecate-proposals
davidk-pt Nov 4, 2024
6304678
Merge branch 'master' into davidk/treasury-deprecate-proposals
davidk-pt Nov 5, 2024
96a5648
Formatting fixes
Nov 5, 2024
0a31dcc
Allow deprecated in tests
Nov 5, 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
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
davidk-pt marked this conversation as resolved.
Show resolved Hide resolved
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,26 +1297,26 @@ 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;
#[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 @@ -326,6 +326,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 @@ -209,6 +209,7 @@ fn last_event() -> BountiesEvent<Test> {
}

#[test]
#[allow(deprecated)]
fn genesis_config_works() {
ExtBuilder::default().build_and_execute(|| {
assert_eq!(Treasury::pot(), 0);
Expand All @@ -226,6 +227,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 @@ -252,6 +254,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 @@ -266,6 +269,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 @@ -286,6 +290,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 @@ -308,6 +313,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 @@ -404,6 +410,7 @@ fn propose_bounty_validation_works() {
}

#[test]
#[allow(deprecated)]
fn close_bounty_works() {
ExtBuilder::default().build_and_execute(|| {
System::set_block_number(1);
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 @@ -154,6 +154,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 @@ -207,6 +207,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 @@ -236,6 +236,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 @@ -277,10 +280,16 @@ pub mod pallet {
type BenchmarkHelper: ArgumentsFactory<Self::AssetKind, Self::Beneficiary>;
}

/// 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 @@ -296,6 +305,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.
davidk-pt marked this conversation as resolved.
Show resolved Hide resolved
/// 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 @@ -470,6 +482,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 @@ -499,7 +515,9 @@ pub mod pallet {
.unwrap_or(Ok(()))?;

let beneficiary = T::Lookup::lookup(beneficiary)?;
#[allow(deprecated)]
davidk-pt marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -508,7 +526,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 @@ -538,12 +558,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 @@ -793,16 +818,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 @@ -817,6 +854,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
Loading