From 33fd17061e439bbb3aca3bdacf02a08770f62814 Mon Sep 17 00:00:00 2001
From: Francisco Aguirre <franciscoaguirreperez@gmail.com>
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 <gavin@parity.io>
Co-authored-by: parity-processbot <>
---
 substrate/bin/node/runtime/src/lib.rs         |   6 +-
 substrate/frame/salary/src/lib.rs             |  74 +------------
 substrate/frame/salary/src/tests.rs           |  11 +-
 substrate/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 substrate/frame/support/src/traits/tokens/pay.rs

diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index 5d1dea02e82..9f4ad2ed264 100644
--- a/substrate/bin/node/runtime/src/lib.rs
+++ b/substrate/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<u16, AccountId, Balance> for SalaryForRank {
 impl pallet_salary::Config for Runtime {
 	type WeightInfo = ();
 	type RuntimeEvent = RuntimeEvent;
-	type Paymaster = pallet_salary::PayFromAccount<Balances, TreasuryAccount>;
+	type Paymaster = PayFromAccount<Balances, TreasuryAccount>;
 	type Members = RankedCollective;
 	type Salary = SalaryForRank;
 	type RegistrationPeriod = ConstU32<200>;
diff --git a/substrate/frame/salary/src/lib.rs b/substrate/frame/salary/src/lib.rs
index 6f9e63271cc..0b2b4b47d8b 100644
--- a/substrate/frame/salary/src/lib.rs
+++ b/substrate/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<Self::Id, ()>;
-	/// 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<F, A>(sp_std::marker::PhantomData<(F, A)>);
-impl<A: TypedGet, F: fungible::Transfer<A::Type> + fungible::Mutate<A::Type>> Pay
-	for PayFromAccount<F, A>
-{
-	type Balance = F::Balance;
-	type AccountId = A::Type;
-	type Id = ();
-	fn pay(who: &Self::AccountId, amount: Self::Balance) -> Result<Self::Id, ()> {
-		<F as fungible::Transfer<_>>::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) {
-		<F as fungible::Mutate<_>>::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<CycleIndex, BlockNumber, Balance> {
@@ -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 = <Self as frame_system::Config>::AccountId>;
+		type Paymaster: Pay<Beneficiary = <Self as frame_system::Config>::AccountId, AssetKind = ()>;
 
 		/// The current membership of payees.
 		type Members: RankedMembers<AccountId = <Self as frame_system::Config>::AccountId>;
@@ -498,8 +436,8 @@ pub mod pallet {
 
 			claimant.last_active = status.cycle_index;
 
-			let id =
-				T::Paymaster::pay(&beneficiary, payout).map_err(|()| Error::<T, I>::PayError)?;
+			let id = T::Paymaster::pay(&beneficiary, (), payout)
+				.map_err(|()| Error::<T, I>::PayError)?;
 
 			claimant.status = Attempted { registered, id, amount: payout };
 
diff --git a/substrate/frame/salary/src/tests.rs b/substrate/frame/salary/src/tests.rs
index e54b9612b15..1b7bc6cbb6d 100644
--- a/substrate/frame/salary/src/tests.rs
+++ b/substrate/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<Self::Id, ()> {
+	fn pay(
+		who: &Self::Beneficiary,
+		_: Self::AssetKind,
+		amount: Self::Balance,
+	) -> Result<Self::Id, ()> {
 		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/substrate/frame/support/src/traits/tokens.rs b/substrate/frame/support/src/traits/tokens.rs
index e1a96f621bd..2d2bd63ada5 100644
--- a/substrate/frame/support/src/traits/tokens.rs
+++ b/substrate/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/substrate/frame/support/src/traits/tokens/pay.rs b/substrate/frame/support/src/traits/tokens/pay.rs
new file mode 100644
index 00000000000..1c6a147b5f2
--- /dev/null
+++ b/substrate/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<Self::Id, ()>;
+	/// 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<F, A>(sp_std::marker::PhantomData<(F, A)>);
+impl<A: TypedGet, F: fungible::Transfer<A::Type> + fungible::Mutate<A::Type>> Pay
+	for PayFromAccount<F, A>
+{
+	type Balance = F::Balance;
+	type Beneficiary = A::Type;
+	type AssetKind = ();
+	type Id = ();
+	fn pay(
+		who: &Self::Beneficiary,
+		_: Self::AssetKind,
+		amount: Self::Balance,
+	) -> Result<Self::Id, ()> {
+		<F as fungible::Transfer<_>>::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) {
+		<F as fungible::Mutate<_>>::mint_into(&A::get(), amount).unwrap();
+	}
+	#[cfg(feature = "runtime-benchmarks")]
+	fn ensure_concluded(_: Self::Id) {}
+}
-- 
GitLab