Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Runtime: Treasury spends various asset kinds #7427

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open

Runtime: Treasury spends various asset kinds #7427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
07ab21f
multi-asset treasury
muharem Jun 26, 2023
6f78c24
weights
muharem Jul 2, 2023
5ba47db
benchmarks fix
muharem Jul 3, 2023
bd45a32
enable features for asset-rate pallet
muharem Jul 3, 2023
d15bacb
benchmarks impls and weights
muharem Jul 4, 2023
d98fcaa
fix runtime commont tests
muharem Jul 4, 2023
9632b1a
benchmarks fixes
muharem Jul 4, 2023
cac07bc
Merge remote-tracking branch 'origin/master' into muharem-multi-asset…
muharem Jul 4, 2023
9063841
void spend
muharem Jul 8, 2023
5439b9d
use reject origin
muharem Jul 8, 2023
137415e
fix type definition
muharem Jul 8, 2023
8595f44
Add missing features
ggwpez Jul 10, 2023
0bb118c
Merge remote-tracking branch 'origin/master' into muharem-multi-asset…
muharem Jul 12, 2023
1d48a86
new fn for LocatableAssetId type
muharem Jul 19, 2023
db809d5
Merge remote-tracking branch 'origin/master' into muharem-multi-asset…
muharem Jul 27, 2023
9bac3ac
fixes after master merge
muharem Jul 27, 2023
0653c73
Merge remote-tracking branch 'origin/master' into muharem-multi-asset…
Jul 30, 2023
778f084
pallet-treasury deps in Cargo.lock, docify and sp-core
muharem Jul 31, 2023
3348a45
Merge remote-tracking branch 'origin/master' into muharem-multi-asset…
Aug 1, 2023
7215bfc
add asset kind type
muharem Aug 2, 2023
7189d45
Merge remote-tracking branch 'origin/master' into muharem-multi-asset…
muharem Aug 2, 2023
9c0bf72
remove Balance type from asset-rate pallet config
muharem Aug 2, 2023
533fac0
asset kind rename
muharem Aug 2, 2023
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
2 changes: 2 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion runtime/common/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pallet-timestamp = { git = "https://github.com/paritytech/substrate", branch = "
pallet-vesting = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
pallet-transaction-payment = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
pallet-treasury = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
pallet-asset-rate = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
pallet-election-provider-multi-phase = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
frame-election-provider-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }

Expand All @@ -49,6 +50,7 @@ runtime-parachains = { package = "polkadot-runtime-parachains", path = "../parac

slot-range-helper = { path = "slot_range_helper", default-features = false }
xcm = { path = "../../xcm", default-features = false }
xcm-builder = { path = "../../xcm/xcm-builder", default-features = false }

[dev-dependencies]
hex-literal = "0.4.1"
Expand Down Expand Up @@ -97,7 +99,9 @@ std = [
"libsecp256k1/std",
"runtime-parachains/std",
"xcm/std",
"xcm-builder/std",
"sp-npos-elections/std",
"pallet-asset-rate/std",
]
runtime-benchmarks = [
"libsecp256k1/hmac",
Expand All @@ -107,7 +111,10 @@ runtime-benchmarks = [
"frame-system/runtime-benchmarks",
"runtime-parachains/runtime-benchmarks",
"pallet-babe/runtime-benchmarks",
"pallet-fast-unstake/runtime-benchmarks"
"pallet-fast-unstake/runtime-benchmarks",
"pallet-treasury/runtime-benchmarks",
"xcm-builder/runtime-benchmarks",
"pallet-asset-rate/runtime-benchmarks",
]
try-runtime = [
"runtime-parachains/try-runtime",
Expand All @@ -120,4 +127,5 @@ try-runtime = [
"pallet-transaction-payment/try-runtime",
"pallet-treasury/try-runtime",
"pallet-fast-unstake/try-runtime",
"pallet-asset-rate/try-runtime",
]
80 changes: 78 additions & 2 deletions runtime/common/src/impls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@

use crate::NegativeImbalance;
use frame_support::traits::{Currency, Imbalance, OnUnbalanced};
use parity_scale_codec::{Decode, Encode, MaxEncodedLen};
use primitives::Balance;
use sp_runtime::Perquintill;
use sp_runtime::{traits::Convert, Perquintill, RuntimeDebug};
use xcm::latest::{AssetId, MultiLocation};

/// Logic for the author to get a portion of fees.
pub struct ToAuthor<R>(sp_std::marker::PhantomData<R>);
Expand Down Expand Up @@ -98,13 +100,78 @@ pub fn era_payout(
(staking_payout, rest)
}

/// Simple struct which contains both an XCM `location` and `asset_id` to identify an asset which
/// exists on some chain.
#[derive(
Encode, Decode, Eq, PartialEq, Clone, RuntimeDebug, scale_info::TypeInfo, MaxEncodedLen,
)]
pub struct LocatableAssetId {
/// The (relative) location in which the asset ID is meaningful.
pub location: MultiLocation,
/// The asset's ID.
pub asset_id: AssetId,
}

impl LocatableAssetId {
pub fn new(location: MultiLocation, asset_id: AssetId) -> Self {
LocatableAssetId { location, asset_id }
}
}

/// Converts the [`LocatableAssetId`] to the [`xcm_builder::LocatableAssetId`].
pub struct LocatableAssetIdConverter;
impl Convert<LocatableAssetId, xcm_builder::LocatableAssetId> for LocatableAssetIdConverter {
fn convert(a: LocatableAssetId) -> xcm_builder::LocatableAssetId {
xcm_builder::LocatableAssetId { asset_id: a.asset_id, location: a.location }
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks silly, why can't we just directly reuse xcm_builder::LocatableAssetId instead of declaring a new LocatableAssetId here in this module?

Copy link
Contributor Author

@muharem muharem Aug 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I do not like that a change in an external library can change the signature of the spend dispatchable.
Where xcm_builder::LocatableAssetId is not part of XCM protocol, or meant to be used as a dispatchable's type parameter.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see why you have to prepare for this? If upstream changes, then you can think of creating a new type retaining the old fields, but this now is a bit overoptimized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the change to the dispatchable signature will invalidate all spend proposals which has been submitted before and still active.
and such a change might be not noticed (this we can address with a test).
but this why I made it this way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's a better idea: why not have the spend dispatchable directly take in Box<VersionedMultiLocation> and Box<VersionedAssetId> as parameters? This should have been the proper way of accepting versioned XCM parameters anyway.

Copy link
Contributor Author

@muharem muharem Aug 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I like the idea with versioned types, this is probably how we should do it.
We need to combine both type into one type, since it is meant for pallet_treasury::Config::AssetKind type parameter. This AssetKind can be just u32 for example.

It can be same as LocatableAssetId but with versioned versions of MultiLocation and AssetId,

pub struct LocatableAssetId {
	pub location: VersionedMultiLocation,
	pub asset_id: VersionedAssetId,
}

or i would better do something like

enum VersionedLocatableAsset {
   #[codec(index = 3)]
   V3{location: v3::MultiLocation, asset_id: v3::AssetId},
}

this way we can control which versions relevant right now specifically for treasury proposals.

Copy link
Contributor

@KiChjang KiChjang Aug 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, so what you seem to be saying is the parameters to the spend extrinsic is dependent on what the concrete configured type is for pallet_treasury::Config::AssetKind, so if it ever changes, then the signature of the spend extrinsic must also necessarily change. Given that's the case, why would we then allow AssetKind to be configurable? It would seem to me that we should instead make it not configurable, so as to preserve API compatibility across versions.

}
}

#[cfg(feature = "runtime-benchmarks")]
pub mod benchmarks {
use super::LocatableAssetId;
use pallet_asset_rate::AssetKindFactory;
use pallet_treasury::ArgumentsFactory as TreasuryArgumentsFactory;
use xcm::prelude::*;

/// Provides a factory method for the [`LocatableAssetId`].
/// The location of the asset is determined as a Parachain with an ID equal to the passed seed.
pub struct AssetRateArguments;
impl AssetKindFactory<LocatableAssetId> for AssetRateArguments {
fn create_asset_kind(seed: u32) -> LocatableAssetId {
LocatableAssetId {
location: MultiLocation::new(0, X1(Parachain(seed))),
asset_id: MultiLocation::new(
0,
X2(PalletInstance(seed.try_into().unwrap()), GeneralIndex(seed.into())),
)
.into(),
}
}
}

/// Provide factory methods for the [`LocatableAssetId`] and the `Beneficiary` of the [`MultiLocation`].
/// The location of the asset is determined as a Parachain with an ID equal to the passed seed.
pub struct TreasuryArguments;
impl TreasuryArgumentsFactory<LocatableAssetId, MultiLocation> for TreasuryArguments {
fn create_asset_kind(seed: u32) -> LocatableAssetId {
AssetRateArguments::create_asset_kind(seed)
}
fn create_beneficiary(seed: [u8; 32]) -> MultiLocation {
MultiLocation::new(0, X1(AccountId32 { network: None, id: seed }))
}
}
}

#[cfg(test)]
mod tests {
use super::*;
use frame_support::{
dispatch::DispatchClass,
parameter_types,
traits::{ConstU32, FindAuthor},
traits::{
tokens::{PayFromAccount, UnityAssetBalanceConversion},
ConstU32, FindAuthor,
},
weights::Weight,
PalletId,
};
Expand Down Expand Up @@ -189,6 +256,7 @@ mod tests {
parameter_types! {
pub const TreasuryPalletId: PalletId = PalletId(*b"py/trsry");
pub const MaxApprovals: u32 = 100;
pub TreasuryAccount: AccountId = Treasury::account_id();
}

impl pallet_treasury::Config for Test {
Expand All @@ -208,6 +276,14 @@ mod tests {
type MaxApprovals = MaxApprovals;
type WeightInfo = ();
type SpendOrigin = frame_support::traits::NeverEnsureOrigin<u64>;
type AssetKind = ();
type Beneficiary = Self::AccountId;
type BeneficiaryLookup = IdentityLookup<Self::AccountId>;
type Paymaster = PayFromAccount<Balances, TreasuryAccount>;
type BalanceConverter = UnityAssetBalanceConversion;
type PayoutPeriod = ConstU64<0>;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = ();
}

pub struct OneAuthor;
Expand Down
4 changes: 4 additions & 0 deletions runtime/kusama/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ pallet-whitelist = { git = "https://github.com/paritytech/substrate", branch = "
pallet-xcm = { path = "../../xcm/pallet-xcm", default-features = false }
pallet-xcm-benchmarks = { path = "../../xcm/pallet-xcm-benchmarks", default-features = false, optional = true }
frame-election-provider-support = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
pallet-asset-rate = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }

frame-benchmarking = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false, optional = true }
frame-try-runtime = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false, optional = true }
Expand Down Expand Up @@ -204,6 +205,7 @@ std = [
"xcm-executor/std",
"xcm-builder/std",
"frame-election-provider-support/std",
"pallet-asset-rate/std",
]
runtime-benchmarks = [
"runtime-common/runtime-benchmarks",
Expand Down Expand Up @@ -255,6 +257,7 @@ runtime-benchmarks = [
"pallet-bags-list/runtime-benchmarks",
"runtime-parachains/runtime-benchmarks",
"pallet-xcm-benchmarks/runtime-benchmarks",
"pallet-asset-rate/runtime-benchmarks",
]
try-runtime = [
"frame-executive/try-runtime",
Expand Down Expand Up @@ -302,6 +305,7 @@ try-runtime = [
"pallet-babe/try-runtime",
"pallet-xcm/try-runtime",
"runtime-common/try-runtime",
"pallet-asset-rate/try-runtime",
]
# When enabled, the runtime API will not be build.
#
Expand Down
51 changes: 45 additions & 6 deletions runtime/kusama/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ use primitives::{
PARACHAIN_KEY_TYPE_ID,
};
use runtime_common::{
auctions, claims, crowdloan, impl_runtime_weights, impls::DealWithFees, paras_registrar,
prod_or_fast, slots, BalanceToU256, BlockHashCount, BlockLength, CurrencyToVote,
SlowAdjustingFeeUpdate, U256ToBalance,
auctions, claims, crowdloan, impl_runtime_weights,
impls::{DealWithFees, LocatableAssetId, LocatableAssetIdConverter},
paras_registrar, prod_or_fast, slots, BalanceToU256, BlockHashCount, BlockLength,
CurrencyToVote, SlowAdjustingFeeUpdate, U256ToBalance,
};
use scale_info::TypeInfo;
use sp_std::{cmp::Ordering, collections::btree_map::BTreeMap, prelude::*};
Expand Down Expand Up @@ -74,8 +75,8 @@ use sp_mmr_primitives as mmr;
use sp_runtime::{
create_runtime_str, generic, impl_opaque_keys,
traits::{
AccountIdLookup, BlakeTwo256, Block as BlockT, ConvertInto, Extrinsic as ExtrinsicT,
OpaqueKeys, SaturatedConversion, Verify,
AccountIdLookup, BlakeTwo256, Block as BlockT, CloneIdentity, ConvertInto,
Extrinsic as ExtrinsicT, IdentityLookup, OpaqueKeys, SaturatedConversion, Verify,
},
transaction_validity::{TransactionPriority, TransactionSource, TransactionValidity},
ApplyExtrinsicResult, FixedU128, KeyTypeId, Perbill, Percent, Permill,
Expand All @@ -84,7 +85,8 @@ use sp_staking::SessionIndex;
#[cfg(any(feature = "std", test))]
use sp_version::NativeVersion;
use sp_version::RuntimeVersion;
use xcm::latest::Junction;
use xcm::latest::{InteriorMultiLocation, Junction, Junction::PalletInstance, MultiLocation};
use xcm_builder::PayOverXcm;

pub use frame_system::Call as SystemCall;
pub use pallet_balances::Call as BalancesCall;
Expand Down Expand Up @@ -613,6 +615,10 @@ parameter_types! {
pub const SpendPeriod: BlockNumber = 6 * DAYS;
pub const Burn: Permill = Permill::from_perthousand(2);
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
// pallet instance (which sits at index 18).
pub TreasuryInteriorLocation: InteriorMultiLocation = PalletInstance(18).into();
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of hardcoding 18 here, I propose that we fetch it programmatically using the PalletInfo trait.


pub const TipCountdown: BlockNumber = 1 * DAYS;
pub const TipFindersFee: Percent = Percent::from_percent(20);
Expand Down Expand Up @@ -641,6 +647,23 @@ impl pallet_treasury::Config for Runtime {
type WeightInfo = weights::pallet_treasury::WeightInfo<Runtime>;
type SpendFunds = Bounties;
type SpendOrigin = TreasurySpender;
type AssetKind = LocatableAssetId;
type Beneficiary = MultiLocation;
type BeneficiaryLookup = IdentityLookup<Self::Beneficiary>;
type Paymaster = PayOverXcm<
TreasuryInteriorLocation,
crate::xcm_config::XcmRouter,
crate::XcmPallet,
ConstU32<{ 6 * HOURS }>,
Self::Beneficiary,
Self::AssetKind,
LocatableAssetIdConverter,
CloneIdentity,
>;
type BalanceConverter = AssetRate;
type PayoutPeriod = PayoutSpendPeriod;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = runtime_common::impls::benchmarks::TreasuryArguments;
}

parameter_types! {
Expand Down Expand Up @@ -1344,6 +1367,18 @@ impl pallet_nomination_pools::Config for Runtime {
type MaxPointsToBalance = MaxPointsToBalance;
}

impl pallet_asset_rate::Config for Runtime {
type WeightInfo = weights::pallet_asset_rate::WeightInfo<Runtime>;
type RuntimeEvent = RuntimeEvent;
type CreateOrigin = EitherOfDiverse<EnsureRoot<AccountId>, Treasurer>;
type RemoveOrigin = EitherOfDiverse<EnsureRoot<AccountId>, Treasurer>;
type UpdateOrigin = EitherOfDiverse<EnsureRoot<AccountId>, Treasurer>;
type Currency = Balances;
type AssetKind = <Runtime as pallet_treasury::Config>::AssetKind;
#[cfg(feature = "runtime-benchmarks")]
type BenchmarkHelper = runtime_common::impls::benchmarks::AssetRateArguments;
}

parameter_types! {
// The deposit configuration for the singed migration. Specially if you want to allow any signed account to do the migration (see `SignedFilter`, these deposits should be high)
pub const MigrationSignedDepositPerItem: Balance = 1 * CENTS;
Expand Down Expand Up @@ -1456,6 +1491,9 @@ construct_runtime! {
// Fast unstake pallet: extension to staking.
FastUnstake: pallet_fast_unstake = 42,

// Asset rate.
AssetRate: pallet_asset_rate::{Pallet, Call, Storage, Event<T>} = 46,

// Parachains pallets. Start indices at 50 to leave room.
ParachainsOrigin: parachains_origin::{Pallet, Origin} = 50,
Configuration: parachains_configuration::{Pallet, Call, Storage, Config<T>} = 51,
Expand Down Expand Up @@ -1610,6 +1648,7 @@ mod benches {
[pallet_utility, Utility]
[pallet_vesting, Vesting]
[pallet_whitelist, Whitelist]
[pallet_asset_rate, AssetRate]
// XCM
[pallet_xcm, XcmPallet]
[pallet_xcm_benchmarks::fungible, pallet_xcm_benchmarks::fungible::Pallet::<Runtime>]
Expand Down
1 change: 1 addition & 0 deletions runtime/kusama/src/weights/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

pub mod frame_election_provider_support;
pub mod frame_system;
pub mod pallet_asset_rate;
pub mod pallet_bags_list;
pub mod pallet_balances;
pub mod pallet_balances_nis_counterpart_balances;
Expand Down
Loading