From ae2cf3250402f0643f5929f7cd0d14e3333a9bce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Mon, 16 Dec 2024 21:06:18 -0500 Subject: [PATCH 1/7] feat(staging-kusama-runtime): add dynamic parameters to ensure OpenGov can decide on Burn parameters for Kusama Treasury. --- relay/kusama/src/lib.rs | 58 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/relay/kusama/src/lib.rs b/relay/kusama/src/lib.rs index 7649d2df0f..e01459a7e8 100644 --- a/relay/kusama/src/lib.rs +++ b/relay/kusama/src/lib.rs @@ -25,7 +25,7 @@ extern crate alloc; use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ dynamic_params::{dynamic_pallet_params, dynamic_params}, - traits::EnsureOriginWithArg, + traits::{EnsureOrigin, EnsureOriginWithArg}, weights::constants::{WEIGHT_PROOF_SIZE_PER_KB, WEIGHT_REF_TIME_PER_MICROS}, }; use kusama_runtime_constants::system_parachain::coretime::TIMESLICE_PERIOD; @@ -641,6 +641,17 @@ impl pallet_bags_list::Config for Runtime { type Score = sp_npos_elections::VoteWeight; } +/// Parameters for the `pallet-treasury` burn destination mechanism. +#[derive(Debug, Encode, Decode, MaxEncodedLen, TypeInfo, Eq, PartialEq, Clone)] +pub struct TreasuryBurnParameters { + /// A fraction of the treasury budget funds that, instead of burning, should be destined to + /// some account. + fraction: Permill, + /// An account for which the surplus funds that otherwise would be burnt should be + /// destined to. + account: AccountId, +} + /// Dynamic params that can be adjusted at runtime. #[dynamic_params(RuntimeParameters, pallet_parameters::Parameters::)] pub mod dynamic_params { @@ -678,6 +689,16 @@ pub mod dynamic_params { #[codec(index = 4)] pub static UseAuctionSlots: bool = true; } + + /// Parameters used by `pallet-treasury` to handle the burn process. + #[dynamic_pallet_params] + #[codec(index = 0)] + pub mod treasury { + /// A structure that includes the fraction of treasury surplus to handle instead of + /// plainly burning. It is expected that these two values work together. + #[codec(index = 0)] + pub static BurnParameters: Option = None; + } } #[cfg(feature = "runtime-benchmarks")] @@ -703,6 +724,8 @@ impl EnsureOriginWithArg for DynamicParamet match key { Inflation(_) => frame_system::ensure_root(origin.clone()), + Treasury(_) => + EitherOf::, GeneralAdmin>::ensure_origin(origin.clone()), } .map_err(|_| origin) } @@ -828,7 +851,6 @@ parameter_types! { pub const ProposalBondMinimum: Balance = 2000 * CENTS; pub const ProposalBondMaximum: Balance = GRAND; pub const SpendPeriod: BlockNumber = 6 * DAYS; - pub const Burn: Permill = Permill::zero(); pub const TreasuryPalletId: PalletId = PalletId(*b"py/trsry"); pub const PayoutSpendPeriod: BlockNumber = 30 * DAYS; // The asset's interior location for the paying account. This is the Treasury @@ -845,14 +867,42 @@ parameter_types! { pub const MaxPeerInHeartbeats: u32 = 10_000; } +use frame_support::traits::{Currency, Imbalance, OnUnbalanced}; + +pub type BalancesNegativeImbalance = >::NegativeImbalance; +pub struct TreasuryBurnHandler; + +impl OnUnbalanced for TreasuryBurnHandler { + fn on_nonzero_unbalanced(amount: BalancesNegativeImbalance) { + if let Some(TreasuryBurnParameters { account, .. }) = + dynamic_params::treasury::BurnParameters::get() + { + let _numeric_amount = amount.peek(); + // Must resolve into existing but better to be safe. + let _ = Balances::resolve_creating(&account, amount); + } else { + // + <() as OnUnbalanced<_>>::on_nonzero_unbalanced(amount) + } + } +} + +impl Get for TreasuryBurnHandler { + fn get() -> Permill { + dynamic_params::treasury::BurnParameters::get() + .map(|TreasuryBurnParameters { fraction, .. }| fraction) + .unwrap_or(Permill::zero()) + } +} + impl pallet_treasury::Config for Runtime { type PalletId = TreasuryPalletId; type Currency = Balances; type RejectOrigin = EitherOfDiverse, Treasurer>; type RuntimeEvent = RuntimeEvent; type SpendPeriod = SpendPeriod; - type Burn = Burn; - type BurnDestination = (); + type Burn = TreasuryBurnHandler; + type BurnDestination = TreasuryBurnHandler; type MaxApprovals = MaxApprovals; type WeightInfo = weights::pallet_treasury::WeightInfo; type SpendFunds = Bounties; From db30521414cf682ac4e3277e2688904a54c4673a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Mon, 16 Dec 2024 21:21:51 -0500 Subject: [PATCH 2/7] fix: make clippy happy and update CHANGELOG.md --- CHANGELOG.md | 1 + relay/kusama/src/lib.rs | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0eff501227..f1568b27e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Changed - Kusama Treasury: remove funding to the Kappa Sigma Mu Society and disable burn ([polkadot-fellows/runtimes#507](https://github.com/polkadot-fellows/runtimes/pull/507)) +- Kusama Treasury: allow burn parameters to be set via OpenGov ([polkadot-fellows/runtimes#511](https://github.com/polkadot-fellows/runtimes/pull/511)) #### From [#490](https://github.com/polkadot-fellows/runtimes/pull/490) diff --git a/relay/kusama/src/lib.rs b/relay/kusama/src/lib.rs index e01459a7e8..816542fcd0 100644 --- a/relay/kusama/src/lib.rs +++ b/relay/kusama/src/lib.rs @@ -867,7 +867,7 @@ parameter_types! { pub const MaxPeerInHeartbeats: u32 = 10_000; } -use frame_support::traits::{Currency, Imbalance, OnUnbalanced}; +use frame_support::traits::{Currency, OnUnbalanced}; pub type BalancesNegativeImbalance = >::NegativeImbalance; pub struct TreasuryBurnHandler; @@ -877,11 +877,11 @@ impl OnUnbalanced for TreasuryBurnHandler { if let Some(TreasuryBurnParameters { account, .. }) = dynamic_params::treasury::BurnParameters::get() { - let _numeric_amount = amount.peek(); // Must resolve into existing but better to be safe. - let _ = Balances::resolve_creating(&account, amount); + Balances::resolve_creating(&account, amount); } else { - // + // If no account to destinate the funds to, just drop the + // imbalance. <() as OnUnbalanced<_>>::on_nonzero_unbalanced(amount) } } From 0b4323c915d601b1d2b67cec7e01bc8dab13201e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Tue, 17 Dec 2024 01:06:44 -0500 Subject: [PATCH 3/7] fix(staging-kusama-runtime): duplicated dynamic params codec for `mod treasury` --- relay/kusama/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relay/kusama/src/lib.rs b/relay/kusama/src/lib.rs index 816542fcd0..88a49f21fb 100644 --- a/relay/kusama/src/lib.rs +++ b/relay/kusama/src/lib.rs @@ -692,7 +692,7 @@ pub mod dynamic_params { /// Parameters used by `pallet-treasury` to handle the burn process. #[dynamic_pallet_params] - #[codec(index = 0)] + #[codec(index = 1)] pub mod treasury { /// A structure that includes the fraction of treasury surplus to handle instead of /// plainly burning. It is expected that these two values work together. From aab1692398205b51419575de8a9871d0206acadf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Tue, 14 Jan 2025 00:12:05 -0500 Subject: [PATCH 4/7] fix(staging-kusama-runtime): change `BurnParameters` as a couple of separate values, to be more ergonomic with `set_parameters` usage. Note: while `BurnDestinationAccount` is not as ergonomic since it comprises of an `Option` value, it's more ergonomic than comparing against an arbitrary `AccountId::from([0u8; 32])`. Note 2: The `BurnDestinationAccount` wrapper is unfortunately necessary, as explicitly stating `AccountId` failed. Need to report an issue on PSDK about this. --- relay/kusama/src/lib.rs | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/relay/kusama/src/lib.rs b/relay/kusama/src/lib.rs index 88a49f21fb..0d64d996b1 100644 --- a/relay/kusama/src/lib.rs +++ b/relay/kusama/src/lib.rs @@ -641,16 +641,8 @@ impl pallet_bags_list::Config for Runtime { type Score = sp_npos_elections::VoteWeight; } -/// Parameters for the `pallet-treasury` burn destination mechanism. -#[derive(Debug, Encode, Decode, MaxEncodedLen, TypeInfo, Eq, PartialEq, Clone)] -pub struct TreasuryBurnParameters { - /// A fraction of the treasury budget funds that, instead of burning, should be destined to - /// some account. - fraction: Permill, - /// An account for which the surplus funds that otherwise would be burnt should be - /// destined to. - account: AccountId, -} +#[derive(Default, MaxEncodedLen, Encode, Decode, TypeInfo, Clone, Eq, PartialEq, Debug)] +pub struct BurnDestinationAccount(pub Option); /// Dynamic params that can be adjusted at runtime. #[dynamic_params(RuntimeParameters, pallet_parameters::Parameters::)] @@ -694,10 +686,11 @@ pub mod dynamic_params { #[dynamic_pallet_params] #[codec(index = 1)] pub mod treasury { - /// A structure that includes the fraction of treasury surplus to handle instead of - /// plainly burning. It is expected that these two values work together. #[codec(index = 0)] - pub static BurnParameters: Option = None; + pub static BurnPortion: Permill = Permill::from_percent(0); + + #[codec(index = 1)] + pub static BurnDestination: BurnDestinationAccount = Default::default(); } } @@ -874,9 +867,11 @@ pub struct TreasuryBurnHandler; impl OnUnbalanced for TreasuryBurnHandler { fn on_nonzero_unbalanced(amount: BalancesNegativeImbalance) { - if let Some(TreasuryBurnParameters { account, .. }) = - dynamic_params::treasury::BurnParameters::get() - { + let portion = dynamic_params::treasury::BurnPortion::get(); + let account = dynamic_params::treasury::BurnDestination::get(); + + if !portion.is_zero() && account.0.is_some() { + let account = account.0.expect("given `account.0.is_some`; qed"); // Must resolve into existing but better to be safe. Balances::resolve_creating(&account, amount); } else { @@ -889,9 +884,13 @@ impl OnUnbalanced for TreasuryBurnHandler { impl Get for TreasuryBurnHandler { fn get() -> Permill { - dynamic_params::treasury::BurnParameters::get() - .map(|TreasuryBurnParameters { fraction, .. }| fraction) - .unwrap_or(Permill::zero()) + let account = dynamic_params::treasury::BurnDestination::get(); + + if account.0.is_some() { + dynamic_params::treasury::BurnPortion::get() + } else { + Permill::zero() + } } } From d24ea09bade9a81122badb3bd6e000568d3c8282 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Tue, 14 Jan 2025 00:12:34 -0500 Subject: [PATCH 5/7] chore(staging-kusama-runtime): add tests for `TreasuryBurnHandler`. --- relay/kusama/tests/treasury_burn_handler.rs | 166 ++++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 relay/kusama/tests/treasury_burn_handler.rs diff --git a/relay/kusama/tests/treasury_burn_handler.rs b/relay/kusama/tests/treasury_burn_handler.rs new file mode 100644 index 0000000000..810e9005e7 --- /dev/null +++ b/relay/kusama/tests/treasury_burn_handler.rs @@ -0,0 +1,166 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +//! TreasuryBurnHandler helper structure tests. +//! +//! Note: These tests emulate the effects of burning some amount on `pallet_treasury` via +//! [`OnUnbalanced`], not the behaviour itself. + +use frame_support::{ + parameter_types, + traits::{ + tokens::fungible::{Inspect, Mutate}, + Currency, ExistenceRequirement, Get, Imbalance, OnUnbalanced, WithdrawReasons, + }, +}; +use kusama_runtime_constants::currency::UNITS; +use polkadot_primitives::{AccountId, Balance}; +use sp_arithmetic::Permill; +use staging_kusama_runtime::{ + dynamic_params::treasury::{self, BurnDestination, BurnPortion}, + Balances, BurnDestinationAccount, Parameters, RuntimeOrigin, RuntimeParameters, Treasury, + TreasuryBurnHandler, +}; + +parameter_types! { + TreasuryAccount: AccountId = Treasury::account_id(); +} + +const BURN_DESTINATION_ACCOUNT: AccountId = AccountId::new([1u8; 32]); + +const TREASURY_AMOUNT: Balance = 10 * UNITS; +const SURPLUS: Balance = UNITS; + +fn test(pre: impl FnOnce(), test: impl FnOnce(Balance)) { + sp_io::TestExternalities::default().execute_with(|| { + pre(); + + Balances::set_balance(&TreasuryAccount::get(), TREASURY_AMOUNT); + + let amount_to_handle = TreasuryBurnHandler::get() * SURPLUS; + let burn = >::withdraw( + &TreasuryAccount::get(), + SURPLUS, + WithdrawReasons::RESERVE, + ExistenceRequirement::KeepAlive, + ) + .expect("withdrawing of `burn` is within balance limits; qed"); + + // Withdrawn surplus to burn it. + assert_eq!(Balances::balance(&TreasuryAccount::get()), TREASURY_AMOUNT - SURPLUS); + + let (credit, burn) = burn.split(amount_to_handle); + + // Burn amount that's not to handle. + <() as OnUnbalanced<_>>::on_unbalanced(burn); + + assert_eq!(Balances::total_issuance(), TREASURY_AMOUNT - (SURPLUS - amount_to_handle)); + + // Let's handle the `credit` + TreasuryBurnHandler::on_unbalanced(credit); + + test(amount_to_handle); + + // Only the amount to handle was transferred to the burn destination account + // let burn_destination_account = BurnDestination::get(); + let burn_destination_account = BURN_DESTINATION_ACCOUNT; + let burn_destination_account_balance = + >::total_balance(&burn_destination_account); + + assert_eq!(burn_destination_account_balance, amount_to_handle); + }) +} + +#[test] +fn on_burn_parameters_not_set_does_not_handle_burn() { + test( + || {}, + |amount_to_handle| { + // Amount to burn should be zero by default + assert_eq!(amount_to_handle, 0); + }, + ) +} + +#[test] +fn on_burn_portion_not_set_does_not_handle_burn() { + test( + || { + Parameters::set_parameter( + RuntimeOrigin::root(), + RuntimeParameters::Treasury(treasury::Parameters::BurnDestination( + BurnDestination, + Some(BurnDestinationAccount(Some(BURN_DESTINATION_ACCOUNT))), + )), + ) + .expect("parameters are set accordingly; qed"); + }, + |amount_to_handle| { + // Amount to burn should be zero by default + assert_eq!(amount_to_handle, 0); + }, + ) +} + +#[test] +fn on_burn_destination_not_set_does_not_handle_burn() { + let one_percent = Permill::from_percent(1); + test( + || { + Parameters::set_parameter( + RuntimeOrigin::root(), + RuntimeParameters::Treasury(treasury::Parameters::BurnPortion( + BurnPortion, + Some(one_percent), + )), + ) + .expect("parameters are set accordingly; qed"); + }, + |amount_to_handle| { + // Amount to burn should be zero by default + assert_eq!(amount_to_handle, 0); + }, + ) +} + +#[test] +fn on_burn_parameters_set_works() { + let one_percent = Permill::from_percent(1); + test( + || { + Parameters::set_parameter( + RuntimeOrigin::root(), + RuntimeParameters::Treasury(treasury::Parameters::BurnDestination( + BurnDestination, + Some(BurnDestinationAccount(Some(BURN_DESTINATION_ACCOUNT))), + )), + ) + .expect("parameters are set accordingly; qed"); + Parameters::set_parameter( + RuntimeOrigin::root(), + RuntimeParameters::Treasury(treasury::Parameters::BurnPortion( + BurnPortion, + Some(one_percent), + )), + ) + .expect("parameters are set accordingly; qed"); + }, + |amount_to_handle| { + // Amount to burn should be zero by default + assert_eq!(amount_to_handle, one_percent * SURPLUS); + }, + ) +} From ba95b60d73be4076099b123030e28fbfaf3d745f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Tue, 14 Jan 2025 07:47:40 -0500 Subject: [PATCH 6/7] feat(staging-kusama-runtime): simplify branching to check if burn handler should act. --- relay/kusama/src/lib.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/relay/kusama/src/lib.rs b/relay/kusama/src/lib.rs index 467ed3d60c..1c28da0e0f 100644 --- a/relay/kusama/src/lib.rs +++ b/relay/kusama/src/lib.rs @@ -644,6 +644,12 @@ impl pallet_bags_list::Config for Runtime { #[derive(Default, MaxEncodedLen, Encode, Decode, TypeInfo, Clone, Eq, PartialEq, Debug)] pub struct BurnDestinationAccount(pub Option); +impl BurnDestinationAccount { + pub fn is_set(&self) -> bool { + self.0.is_some() + } +} + /// Dynamic params that can be adjusted at runtime. #[dynamic_params(RuntimeParameters, pallet_parameters::Parameters::)] pub mod dynamic_params { @@ -868,10 +874,9 @@ pub struct TreasuryBurnHandler; impl OnUnbalanced for TreasuryBurnHandler { fn on_nonzero_unbalanced(amount: BalancesNegativeImbalance) { let portion = dynamic_params::treasury::BurnPortion::get(); - let account = dynamic_params::treasury::BurnDestination::get(); + let destination = dynamic_params::treasury::BurnDestination::get(); - if !portion.is_zero() && account.0.is_some() { - let account = account.0.expect("given `account.0.is_some`; qed"); + if let BurnDestinationAccount(Some(account)) = destination { // Must resolve into existing but better to be safe. Balances::resolve_creating(&account, amount); } else { @@ -884,9 +889,9 @@ impl OnUnbalanced for TreasuryBurnHandler { impl Get for TreasuryBurnHandler { fn get() -> Permill { - let account = dynamic_params::treasury::BurnDestination::get(); + let destination = dynamic_params::treasury::BurnDestination::get(); - if account.0.is_some() { + if destination.is_set() { dynamic_params::treasury::BurnPortion::get() } else { Permill::zero() From 063e3d4d695de4f074d8e3aec463fbeac8721410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20Dorado=20Su=C3=A1rez?= Date: Tue, 14 Jan 2025 09:39:23 -0500 Subject: [PATCH 7/7] fix(staging-kusama-runtime): unused variable --- relay/kusama/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/relay/kusama/src/lib.rs b/relay/kusama/src/lib.rs index 1c28da0e0f..042162caff 100644 --- a/relay/kusama/src/lib.rs +++ b/relay/kusama/src/lib.rs @@ -873,7 +873,6 @@ pub struct TreasuryBurnHandler; impl OnUnbalanced for TreasuryBurnHandler { fn on_nonzero_unbalanced(amount: BalancesNegativeImbalance) { - let portion = dynamic_params::treasury::BurnPortion::get(); let destination = dynamic_params::treasury::BurnDestination::get(); if let BurnDestinationAccount(Some(account)) = destination {