From 76c0d583cd7cf0bd01976a2c7f647b071233423b Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Wed, 15 Mar 2023 18:04:28 -0300 Subject: [PATCH] Make Pay trait from salaries pallet more generic (#13609) * Make Pay trait from salaries pallet more generic * Rename and add missing * Update frame/support/src/traits/tokens/pay.rs * Update pay.rs * Update pay.rs * Update pay.rs * Add better documentation for the AssetKind associated type --------- Co-authored-by: Gavin Wood Co-authored-by: parity-processbot <> --- bin/node/runtime/src/lib.rs | 6 +- frame/salary/src/lib.rs | 74 ++---------------- frame/salary/src/tests.rs | 11 ++- frame/support/src/traits/tokens.rs | 2 + frame/support/src/traits/tokens/pay.rs | 103 +++++++++++++++++++++++++ 5 files changed, 122 insertions(+), 74 deletions(-) create mode 100644 frame/support/src/traits/tokens/pay.rs diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 5d1dea02e8256..9f4ad2ed26474 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -33,7 +33,7 @@ use frame_support::{ parameter_types, traits::{ fungible::ItemOf, - tokens::{nonfungibles_v2::Inspect, GetSalary}, + tokens::{nonfungibles_v2::Inspect, GetSalary, PayFromAccount}, AsEnsureOriginWithArg, ConstBool, ConstU128, ConstU16, ConstU32, Currency, EitherOfDiverse, EqualPrivilegeOnly, Everything, Imbalance, InstanceFilter, KeyOwnerProofSystem, LockIdentifier, Nothing, OnUnbalanced, U128CurrencyToVote, WithdrawReasons, @@ -56,7 +56,7 @@ use pallet_election_provider_multi_phase::SolutionAccuracyOf; use pallet_im_online::sr25519::AuthorityId as ImOnlineId; use pallet_nfts::PalletFeatures; use pallet_nis::WithMaximumOf; -use pallet_session::historical::{self as pallet_session_historical}; +use pallet_session::historical as pallet_session_historical; pub use pallet_transaction_payment::{CurrencyAdapter, Multiplier, TargetedFeeAdjustment}; use pallet_transaction_payment::{FeeDetails, RuntimeDispatchInfo}; use sp_api::impl_runtime_apis; @@ -1567,7 +1567,7 @@ impl GetSalary for SalaryForRank { impl pallet_salary::Config for Runtime { type WeightInfo = (); type RuntimeEvent = RuntimeEvent; - type Paymaster = pallet_salary::PayFromAccount; + type Paymaster = PayFromAccount; type Members = RankedCollective; type Salary = SalaryForRank; type RegistrationPeriod = ConstU32<200>; diff --git a/frame/salary/src/lib.rs b/frame/salary/src/lib.rs index 6f9e63271cc2f..0b2b4b47d8b70 100644 --- a/frame/salary/src/lib.rs +++ b/frame/salary/src/lib.rs @@ -20,18 +20,17 @@ #![cfg_attr(not(feature = "std"), no_std)] #![recursion_limit = "128"] -use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; +use codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; use sp_arithmetic::traits::{Saturating, Zero}; -use sp_core::TypedGet; use sp_runtime::Perbill; -use sp_std::{fmt::Debug, marker::PhantomData, prelude::*}; +use sp_std::{marker::PhantomData, prelude::*}; use frame_support::{ dispatch::DispatchResultWithPostInfo, ensure, traits::{ - tokens::{fungible, Balance, GetSalary}, + tokens::{GetSalary, Pay, PaymentStatus}, RankedMembers, }, RuntimeDebug, @@ -50,67 +49,6 @@ pub use weights::WeightInfo; /// Payroll cycle. pub type Cycle = u32; -/// Status for making a payment via the `Pay::pay` trait function. -#[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] -pub enum PaymentStatus { - /// Payment is in progress. Nothing to report yet. - InProgress, - /// Payment status is unknowable. It will never be reported successful or failed. - Unknown, - /// Payment happened successfully. - Success, - /// Payment failed. It may safely be retried. - Failure, -} - -/// Can be implemented by `PayFromAccount` using a `fungible` impl, but can also be implemented with -/// XCM/MultiAsset and made generic over assets. -pub trait Pay { - /// The type by which we measure units of the currency in which we make payments. - type Balance: Balance; - /// The type by which we identify the individuals to whom a payment may be made. - type AccountId; - /// An identifier given to an individual payment. - type Id: FullCodec + MaxEncodedLen + TypeInfo + Clone + Eq + PartialEq + Debug + Copy; - /// Make a payment and return an identifier for later evaluation of success in some off-chain - /// mechanism (likely an event, but possibly not on this chain). - fn pay(who: &Self::AccountId, amount: Self::Balance) -> Result; - /// Check how a payment has proceeded. `id` must have been a previously returned by `pay` for - /// the result of this call to be meaningful. - fn check_payment(id: Self::Id) -> PaymentStatus; - /// Ensure that a call to pay with the given parameters will be successful if done immediately - /// after this call. Used in benchmarking code. - #[cfg(feature = "runtime-benchmarks")] - fn ensure_successful(who: &Self::AccountId, amount: Self::Balance); - /// Ensure that a call to `check_payment` with the given parameters will return either `Success` - /// or `Failure`. - #[cfg(feature = "runtime-benchmarks")] - fn ensure_concluded(id: Self::Id); -} - -/// Simple implementation of `Pay` which makes a payment from a "pot" - i.e. a single account. -pub struct PayFromAccount(sp_std::marker::PhantomData<(F, A)>); -impl + fungible::Mutate> Pay - for PayFromAccount -{ - type Balance = F::Balance; - type AccountId = A::Type; - type Id = (); - fn pay(who: &Self::AccountId, amount: Self::Balance) -> Result { - >::transfer(&A::get(), who, amount, false).map_err(|_| ())?; - Ok(()) - } - fn check_payment(_: ()) -> PaymentStatus { - PaymentStatus::Success - } - #[cfg(feature = "runtime-benchmarks")] - fn ensure_successful(_: &Self::AccountId, amount: Self::Balance) { - >::mint_into(&A::get(), amount).unwrap(); - } - #[cfg(feature = "runtime-benchmarks")] - fn ensure_concluded(_: Self::Id) {} -} - /// The status of the pallet instance. #[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] pub struct StatusType { @@ -168,7 +106,7 @@ pub mod pallet { /// Means by which we can make payments to accounts. This also defines the currency and the /// balance which we use to denote that currency. - type Paymaster: Pay::AccountId>; + type Paymaster: Pay::AccountId, AssetKind = ()>; /// The current membership of payees. type Members: RankedMembers::AccountId>; @@ -498,8 +436,8 @@ pub mod pallet { claimant.last_active = status.cycle_index; - let id = - T::Paymaster::pay(&beneficiary, payout).map_err(|()| Error::::PayError)?; + let id = T::Paymaster::pay(&beneficiary, (), payout) + .map_err(|()| Error::::PayError)?; claimant.status = Attempted { registered, id, amount: payout }; diff --git a/frame/salary/src/tests.rs b/frame/salary/src/tests.rs index e54b9612b152c..1b7bc6cbb6df5 100644 --- a/frame/salary/src/tests.rs +++ b/frame/salary/src/tests.rs @@ -99,11 +99,16 @@ fn set_status(id: u64, s: PaymentStatus) { pub struct TestPay; impl Pay for TestPay { - type AccountId = u64; + type Beneficiary = u64; type Balance = u64; type Id = u64; + type AssetKind = (); - fn pay(who: &Self::AccountId, amount: Self::Balance) -> Result { + fn pay( + who: &Self::Beneficiary, + _: Self::AssetKind, + amount: Self::Balance, + ) -> Result { PAID.with(|paid| *paid.borrow_mut().entry(*who).or_default() += amount); Ok(LAST_ID.with(|lid| { let x = *lid.borrow(); @@ -115,7 +120,7 @@ impl Pay for TestPay { STATUS.with(|s| s.borrow().get(&id).cloned().unwrap_or(PaymentStatus::Unknown)) } #[cfg(feature = "runtime-benchmarks")] - fn ensure_successful(_: &Self::AccountId, _: Self::Balance) {} + fn ensure_successful(_: &Self::Beneficiary, _: Self::Balance) {} #[cfg(feature = "runtime-benchmarks")] fn ensure_concluded(id: Self::Id) { set_status(id, PaymentStatus::Failure) diff --git a/frame/support/src/traits/tokens.rs b/frame/support/src/traits/tokens.rs index e1a96f621bd4f..2d2bd63ada551 100644 --- a/frame/support/src/traits/tokens.rs +++ b/frame/support/src/traits/tokens.rs @@ -27,7 +27,9 @@ pub mod nonfungible_v2; pub mod nonfungibles; pub mod nonfungibles_v2; pub use imbalance::Imbalance; +pub mod pay; pub use misc::{ AssetId, Balance, BalanceConversion, BalanceStatus, ConvertRank, DepositConsequence, ExistenceRequirement, GetSalary, Locker, WithdrawConsequence, WithdrawReasons, }; +pub use pay::{Pay, PayFromAccount, PaymentStatus}; diff --git a/frame/support/src/traits/tokens/pay.rs b/frame/support/src/traits/tokens/pay.rs new file mode 100644 index 0000000000000..1c6a147b5f22e --- /dev/null +++ b/frame/support/src/traits/tokens/pay.rs @@ -0,0 +1,103 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! The Pay trait and associated types. + +use codec::{Decode, Encode, FullCodec, MaxEncodedLen}; +use scale_info::TypeInfo; +use sp_core::{RuntimeDebug, TypedGet}; +use sp_std::fmt::Debug; + +use super::{fungible, Balance}; + +/// Can be implemented by `PayFromAccount` using a `fungible` impl, but can also be implemented with +/// XCM/MultiAsset and made generic over assets. +pub trait Pay { + /// The type by which we measure units of the currency in which we make payments. + type Balance: Balance; + /// The type by which we identify the beneficiaries to whom a payment may be made. + type Beneficiary; + /// The type for the kinds of asset that are going to be paid. + /// + /// The unit type can be used here to indicate there's only one kind of asset to do payments + /// with. When implementing, it should be clear from the context what that asset is. + type AssetKind; + /// An identifier given to an individual payment. + type Id: FullCodec + MaxEncodedLen + TypeInfo + Clone + Eq + PartialEq + Debug + Copy; + /// Make a payment and return an identifier for later evaluation of success in some off-chain + /// mechanism (likely an event, but possibly not on this chain). + fn pay( + who: &Self::Beneficiary, + asset_kind: Self::AssetKind, + amount: Self::Balance, + ) -> Result; + /// Check how a payment has proceeded. `id` must have been previously returned by `pay` for + /// the result of this call to be meaningful. Once this returns anything other than + /// `InProgress` for some `id` it must return `Unknown` rather than the actual result + /// value. + fn check_payment(id: Self::Id) -> PaymentStatus; + /// Ensure that a call to pay with the given parameters will be successful if done immediately + /// after this call. Used in benchmarking code. + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(who: &Self::Beneficiary, amount: Self::Balance); + /// Ensure that a call to `check_payment` with the given parameters will return either `Success` + /// or `Failure`. + #[cfg(feature = "runtime-benchmarks")] + fn ensure_concluded(id: Self::Id); +} + +/// Status for making a payment via the `Pay::pay` trait function. +#[derive(Encode, Decode, Eq, PartialEq, Clone, TypeInfo, MaxEncodedLen, RuntimeDebug)] +pub enum PaymentStatus { + /// Payment is in progress. Nothing to report yet. + InProgress, + /// Payment status is unknowable. It may already have reported the result, or if not then + /// it will never be reported successful or failed. + Unknown, + /// Payment happened successfully. + Success, + /// Payment failed. It may safely be retried. + Failure, +} + +/// Simple implementation of `Pay` which makes a payment from a "pot" - i.e. a single account. +pub struct PayFromAccount(sp_std::marker::PhantomData<(F, A)>); +impl + fungible::Mutate> Pay + for PayFromAccount +{ + type Balance = F::Balance; + type Beneficiary = A::Type; + type AssetKind = (); + type Id = (); + fn pay( + who: &Self::Beneficiary, + _: Self::AssetKind, + amount: Self::Balance, + ) -> Result { + >::transfer(&A::get(), who, amount, false).map_err(|_| ())?; + Ok(()) + } + fn check_payment(_: ()) -> PaymentStatus { + PaymentStatus::Success + } + #[cfg(feature = "runtime-benchmarks")] + fn ensure_successful(_: &Self::Beneficiary, amount: Self::Balance) { + >::mint_into(&A::get(), amount).unwrap(); + } + #[cfg(feature = "runtime-benchmarks")] + fn ensure_concluded(_: Self::Id) {} +}