From 370ab13fe3aa9af075fd862daafd44f7ada99498 Mon Sep 17 00:00:00 2001 From: Albrecht <14820950+weichweich@users.noreply.github.com> Date: Mon, 10 Aug 2020 13:57:15 +0200 Subject: [PATCH 01/23] wip: setup types --- frame/transaction-payment/src/lib.rs | 160 ++++++++++++++------------- 1 file changed, 84 insertions(+), 76 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 244b4280ade08..7f75c347ec30f 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -33,7 +33,7 @@ #![cfg_attr(not(feature = "std"), no_std)] use sp_std::prelude::*; -use codec::{Encode, Decode}; +use codec::{Encode, Decode, FullCodec}; use frame_support::{ decl_storage, decl_module, traits::{Currency, Get, OnUnbalanced, ExistenceRequirement, WithdrawReason, Imbalance}, @@ -51,19 +51,15 @@ use sp_runtime::{ }, traits::{ Zero, Saturating, SignedExtension, SaturatedConversion, Convert, Dispatchable, - DispatchInfoOf, PostDispatchInfoOf, + DispatchInfoOf, PostDispatchInfoOf, AtLeast32BitUnsigned, MaybeSerializeDeserialize, }, }; use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo; +use std::fmt::Debug; /// Fee multiplier. pub type Multiplier = FixedU128; -type BalanceOf = - <::Currency as Currency<::AccountId>>::Balance; -type NegativeImbalanceOf = - <::Currency as Currency<::AccountId>>::NegativeImbalance; - /// A struct to update the weight multiplier per block. It implements `Convert`, meaning that it can convert the previous multiplier to the next one. This should /// be called on `on_finalize` of a block, prior to potentially cleaning the weight data from the @@ -212,20 +208,44 @@ impl Default for Releases { } } +pub trait OnChargeTransaction { + type WithdrawAmount; + + fn withdraw( + who: &T::AccountId, + fee: T::Balance, + tip: T::Balance, + ) -> Result<(T::Balance, Self::WithdrawAmount), TransactionValidityError>; + fn refund( + who: &T::AccountId, + amount: Self::WithdrawAmount, + refund: T::Balance, + ) -> Result<(Self::WithdrawAmount), TransactionValidityError>; +} + pub trait Trait: frame_system::Trait { /// The currency type in which fees will be paid. - type Currency: Currency + Send + Sync; + type Balance: AtLeast32BitUnsigned + + FullCodec + + Copy + + MaybeSerializeDeserialize + + Debug + + Default + + Send + + Sync; /// Handler for the unbalanced reduction when taking transaction fees. This is either one or /// two separate imbalances, the first is the transaction fee paid, the second is the tip paid, /// if any. - type OnTransactionPayment: OnUnbalanced>; + type OnTransactionPayment: OnUnbalanced<_>; + + type OnChargeTransaction: OnChargeTransaction; /// The fee to be paid for making a transaction; the per-byte portion. - type TransactionByteFee: Get>; + type TransactionByteFee: Get; /// Convert a weight value into a deductible fee based on the currency type. - type WeightToFee: WeightToFeePolynomial>; + type WeightToFee: WeightToFeePolynomial; /// Update the multiplier of the next block, based on the previous block's weight. type FeeMultiplierUpdate: MultiplierUpdate; @@ -242,10 +262,10 @@ decl_storage! { decl_module! { pub struct Module for enum Call where origin: T::Origin { /// The fee to be paid for making a transaction; the per-byte portion. - const TransactionByteFee: BalanceOf = T::TransactionByteFee::get(); + const TransactionByteFee: T::Balance = T::TransactionByteFee::get(); /// The polynomial that is applied in order to derive fee from weight. - const WeightToFee: Vec>> = + const WeightToFee: Vec> = T::WeightToFee::polynomial().to_vec(); fn on_finalize() { @@ -295,8 +315,9 @@ decl_module! { } } -impl Module where - BalanceOf: FixedPointOperand +impl Module +where + T::Balance: FixedPointOperand, { /// Query the data that we know about the fee of a given `call`. /// @@ -309,10 +330,10 @@ impl Module where pub fn query_info( unchecked_extrinsic: Extrinsic, len: u32, - ) -> RuntimeDispatchInfo> + ) -> RuntimeDispatchInfo where T: Send + Sync, - BalanceOf: Send + Sync, + T::Balance: Send + Sync, T::Call: Dispatchable, { // NOTE: we can actually make it understand `ChargeTransactionPayment`, but would be some @@ -349,12 +370,9 @@ impl Module where /// inclusion_fee = base_fee + len_fee + [targeted_fee_adjustment * weight_fee]; /// final_fee = inclusion_fee + tip; /// ``` - pub fn compute_fee( - len: u32, - info: &DispatchInfoOf, - tip: BalanceOf, - ) -> BalanceOf where - T::Call: Dispatchable, + pub fn compute_fee(len: u32, info: &DispatchInfoOf, tip: T::Balance) -> T::Balance + where + T::Call: Dispatchable, { Self::compute_fee_raw(len, info.weight, tip, info.pays_fee) } @@ -367,21 +385,17 @@ impl Module where len: u32, info: &DispatchInfoOf, post_info: &PostDispatchInfoOf, - tip: BalanceOf, - ) -> BalanceOf where - T::Call: Dispatchable, + tip: T::Balance, + ) -> T::Balance + where + T::Call: Dispatchable, { Self::compute_fee_raw(len, post_info.calc_actual_weight(info), tip, post_info.pays_fee(info)) } - fn compute_fee_raw( - len: u32, - weight: Weight, - tip: BalanceOf, - pays_fee: Pays, - ) -> BalanceOf { + fn compute_fee_raw(len: u32, weight: Weight, tip: T::Balance, pays_fee: Pays) -> T::Balance { if pays_fee == Pays::Yes { - let len = >::from(len); + let len = ::from(len); let per_byte = T::TransactionByteFee::get(); // length fee. this is not adjusted. @@ -403,7 +417,7 @@ impl Module where } } - fn weight_to_fee(weight: Weight) -> BalanceOf { + fn weight_to_fee(weight: Weight) -> T::Balance { // cap the weight to the maximum defined in runtime, otherwise it will be the // `Bounded` maximum of its data type, which is not desired. let capped_weight = weight.min(::MaximumBlockWeight::get()); @@ -411,16 +425,17 @@ impl Module where } } -impl Convert> for Module where +impl Convert for Module +where T: Trait, - BalanceOf: FixedPointOperand, + T::Balance: FixedPointOperand, { /// Compute the fee for the specified weight. /// /// This fee is already adjusted by the per block fee adjustment factor and is therefore the /// share that the weight contributes to the overall fee of a transaction. It is mainly /// for informational purposes and not used in the actual fee calculation. - fn convert(weight: Weight) -> BalanceOf { + fn convert(weight: Weight) -> T::Balance { NextFeeMultiplier::get().saturating_mul_int(Self::weight_to_fee(weight)) } } @@ -428,14 +443,15 @@ impl Convert> for Module where /// Require the transactor pay for themselves and maybe include a tip to gain additional priority /// in the queue. #[derive(Encode, Decode, Clone, Eq, PartialEq)] -pub struct ChargeTransactionPayment(#[codec(compact)] BalanceOf); +pub struct ChargeTransactionPayment(#[codec(compact)] T::Balance); -impl ChargeTransactionPayment where - T::Call: Dispatchable, - BalanceOf: Send + Sync + FixedPointOperand, +impl ChargeTransactionPayment +where + T::Call: Dispatchable, + T::Balance: Send + Sync + FixedPointOperand, { /// utility constructor. Used only in client/factory code. - pub fn from(fee: BalanceOf) -> Self { + pub fn from(fee: T::Balance) -> Self { Self(fee) } @@ -444,7 +460,13 @@ impl ChargeTransactionPayment where who: &T::AccountId, info: &DispatchInfoOf, len: usize, - ) -> Result<(BalanceOf, Option>), TransactionValidityError> { + ) -> Result< + ( + T::Balance, + Option<<::OnChargeTransaction as OnChargeTransaction>::WithdrawAmount>, + ), + TransactionValidityError, + > { let tip = self.0; let fee = Module::::compute_fee(len as u32, info, tip); @@ -453,19 +475,8 @@ impl ChargeTransactionPayment where return Ok((fee, None)); } - match T::Currency::withdraw( - who, - fee, - if tip.is_zero() { - WithdrawReason::TransactionPayment.into() - } else { - WithdrawReason::TransactionPayment | WithdrawReason::Tip - }, - ExistenceRequirement::KeepAlive, - ) { - Ok(imbalance) => Ok((fee, Some(imbalance))), - Err(_) => Err(InvalidTransaction::Payment.into()), - } + <::OnChargeTransaction as OnChargeTransaction>::withdraw(who, fee, tip) + .map(|(x, y)| (x, Some(y))) } } @@ -480,16 +491,24 @@ impl sp_std::fmt::Debug for ChargeTransactionPayment } } -impl SignedExtension for ChargeTransactionPayment where - BalanceOf: Send + Sync + From + FixedPointOperand, - T::Call: Dispatchable, +impl SignedExtension for ChargeTransactionPayment +where + T::Balance: Send + Sync + From + FixedPointOperand, + T::Call: Dispatchable, { const IDENTIFIER: &'static str = "ChargeTransactionPayment"; type AccountId = T::AccountId; type Call = T::Call; type AdditionalSigned = (); - type Pre = (BalanceOf, Self::AccountId, Option>, BalanceOf); - fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> { Ok(()) } + type Pre = ( + T::Balance, + Self::AccountId, + Option<<::OnChargeTransaction as OnChargeTransaction>::WithdrawAmount>, + T::Balance, + ); + fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> { + Ok(()) + } fn validate( &self, @@ -534,22 +553,11 @@ impl SignedExtension for ChargeTransactionPayment whe tip, ); let refund = fee.saturating_sub(actual_fee); - let actual_payment = match T::Currency::deposit_into_existing(&who, refund) { - Ok(refund_imbalance) => { - // The refund cannot be larger than the up front payed max weight. - // `PostDispatchInfo::calc_unspent` guards against such a case. - match payed.offset(refund_imbalance) { - Ok(actual_payment) => actual_payment, - Err(_) => return Err(InvalidTransaction::Payment.into()), - } - } - // We do not recreate the account using the refund. The up front payment - // is gone in that case. - Err(_) => payed, - }; + let actual_payment = T::OnChargeTransaction::refund(&who, imbalance, refund)?; let imbalances = actual_payment.split(tip); - T::OnTransactionPayment::on_unbalanceds(Some(imbalances.0).into_iter() - .chain(Some(imbalances.1))); + T::OnTransactionPayment::on_unbalanceds( + Some(imbalances.0).into_iter().chain(Some(imbalances.1)), + ); } Ok(()) } From 96137cb2922a8b95f484b2b6338dcbef82bfecde Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Mon, 10 Aug 2020 15:12:52 +0200 Subject: [PATCH 02/23] fix types --- bin/node-template/runtime/src/lib.rs | 6 +-- bin/node/runtime/src/lib.rs | 6 +-- frame/transaction-payment/src/lib.rs | 62 ++++++++++++++++++++++------ 3 files changed, 55 insertions(+), 19 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 06e34e4551673..334b5e7759963 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -6,7 +6,7 @@ #[cfg(feature = "std")] include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); -use sp_std::prelude::*; +use sp_std::{prelude::*, marker::PhantomData}; use sp_core::{crypto::KeyTypeId, OpaqueMetadata}; use sp_runtime::{ ApplyExtrinsicResult, generic, create_runtime_str, impl_opaque_keys, MultiSignature, @@ -245,8 +245,8 @@ parameter_types! { } impl pallet_transaction_payment::Trait for Runtime { - type Currency = Balances; - type OnTransactionPayment = (); + type Balance = Balance; + type OnChargeTransaction = (PhantomData>, PhantomData<()>); type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; type FeeMultiplierUpdate = (); diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index aa0ddfc61a7bb..62b3fc98412c9 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -22,7 +22,7 @@ // `construct_runtime!` does a lot of recursion and requires us to increase the limit to 256. #![recursion_limit="256"] -use sp_std::prelude::*; +use sp_std::{prelude::*, marker::PhantomData}; use frame_support::{ construct_runtime, parameter_types, debug, RuntimeDebug, @@ -336,8 +336,8 @@ parameter_types! { } impl pallet_transaction_payment::Trait for Runtime { - type Currency = Balances; - type OnTransactionPayment = DealWithFees; + type Balance = Balance; + type OnChargeTransaction = (PhantomData>, PhantomData); type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; type FeeMultiplierUpdate = diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 7f75c347ec30f..4ba0282387a80 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -32,7 +32,7 @@ #![cfg_attr(not(feature = "std"), no_std)] -use sp_std::prelude::*; +use sp_std::{prelude::*, fmt::Debug, marker::PhantomData}; use codec::{Encode, Decode, FullCodec}; use frame_support::{ decl_storage, decl_module, @@ -55,11 +55,13 @@ use sp_runtime::{ }, }; use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo; -use std::fmt::Debug; /// Fee multiplier. pub type Multiplier = FixedU128; +type NegativeImbalanceOf = + ::AccountId>>::NegativeImbalance; + /// A struct to update the weight multiplier per block. It implements `Convert`, meaning that it can convert the previous multiplier to the next one. This should /// be called on `on_finalize` of a block, prior to potentially cleaning the weight data from the @@ -216,11 +218,48 @@ pub trait OnChargeTransaction { fee: T::Balance, tip: T::Balance, ) -> Result<(T::Balance, Self::WithdrawAmount), TransactionValidityError>; + + fn refund( + who: &T::AccountId, + amount: Self::WithdrawAmount, + refund: T::Balance, + ) -> Result; + + fn deposit( + amount: Self::WithdrawAmount, + ); +} + +impl OnChargeTransaction for (PhantomData, PhantomData) +where + T: Trait, + C: Currency<::AccountId>, + OU: OnUnbalanced>, +{ + type WithdrawAmount = (); + + fn withdraw( + who: &T::AccountId, + fee: T::Balance, + tip: T::Balance, + ) -> Result<(T::Balance, Self::WithdrawAmount), TransactionValidityError> { + todo!() + } + fn refund( who: &T::AccountId, amount: Self::WithdrawAmount, refund: T::Balance, - ) -> Result<(Self::WithdrawAmount), TransactionValidityError>; + ) -> Result { + todo!() + } + + fn deposit( + amount: Self::WithdrawAmount, + ) { + todo!() + } + } pub trait Trait: frame_system::Trait { @@ -234,11 +273,11 @@ pub trait Trait: frame_system::Trait { + Send + Sync; - /// Handler for the unbalanced reduction when taking transaction fees. This is either one or - /// two separate imbalances, the first is the transaction fee paid, the second is the tip paid, - /// if any. - type OnTransactionPayment: OnUnbalanced<_>; - + /// Handler for withdrawing, refunding and depositing the transaction fee. + /// Transaction fees are withdrawen before the transaction is executed. + /// After the transaction was executed the transaction weight can be adjusted, depending on the used resources by + /// the transaction. If the transaction weight is lower than expected, parts of the transaction fee might be refunded. + /// In the end the fees can be deposited. type OnChargeTransaction: OnChargeTransaction; /// The fee to be paid for making a transaction; the per-byte portion. @@ -553,11 +592,8 @@ where tip, ); let refund = fee.saturating_sub(actual_fee); - let actual_payment = T::OnChargeTransaction::refund(&who, imbalance, refund)?; - let imbalances = actual_payment.split(tip); - T::OnTransactionPayment::on_unbalanceds( - Some(imbalances.0).into_iter().chain(Some(imbalances.1)), - ); + let actual_payment = T::OnChargeTransaction::refund(&who, payed, refund)?; + T::OnChargeTransaction::deposit(actual_payment); } Ok(()) } From 006ffcd6b240e21b89e0cdbc229f6f01a4ed2aac Mon Sep 17 00:00:00 2001 From: Albrecht <14820950+weichweich@users.noreply.github.com> Date: Tue, 11 Aug 2020 14:28:47 +0200 Subject: [PATCH 03/23] make tx payment pallet independent from balances --- frame/transaction-payment/src/lib.rs | 108 ++++++++++++++++++--------- 1 file changed, 72 insertions(+), 36 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 4ba0282387a80..ed4a4d16d187f 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -211,7 +211,7 @@ impl Default for Releases { } pub trait OnChargeTransaction { - type WithdrawAmount; + type WithdrawAmount: Default; fn withdraw( who: &T::AccountId, @@ -225,41 +225,80 @@ pub trait OnChargeTransaction { refund: T::Balance, ) -> Result; - fn deposit( + fn finalize( amount: Self::WithdrawAmount, ); } -impl OnChargeTransaction for (PhantomData, PhantomData) -where - T: Trait, - C: Currency<::AccountId>, +impl OnChargeTransaction for (PhantomData, PhantomData) +where + B: AtLeast32BitUnsigned + + FullCodec + + Copy + + MaybeSerializeDeserialize + + Debug + + Default + + Send + + Sync, + T: Trait, + T::TransactionByteFee: Get, + C: Currency<::AccountId, Balance=B>, + C::PositiveImbalance: Imbalance, + C::NegativeImbalance: Imbalance, OU: OnUnbalanced>, { - type WithdrawAmount = (); + type WithdrawAmount = Option>; fn withdraw( who: &T::AccountId, - fee: T::Balance, - tip: T::Balance, - ) -> Result<(T::Balance, Self::WithdrawAmount), TransactionValidityError> { - todo!() + fee: B, + tip: B, + ) -> Result<(B, Self::WithdrawAmount), TransactionValidityError> { + if fee.is_zero() { + return Ok((B::zero(), None)) + } + match C::withdraw( + who, + fee, + if tip.is_zero() { + WithdrawReason::TransactionPayment.into() + } else { + WithdrawReason::TransactionPayment | WithdrawReason::Tip + }, + ExistenceRequirement::KeepAlive, + ) { + Ok(imbalance) => Ok((fee, Some(imbalance))), + Err(_) => Err(InvalidTransaction::Payment.into()), + } } fn refund( who: &T::AccountId, amount: Self::WithdrawAmount, - refund: T::Balance, + refund: B, ) -> Result { - todo!() + if let Some(payed) = amount { + match C::deposit_into_existing(&who, refund) { + Ok(refund_imbalance) => + // The refund cannot be larger than the up front payed max weight. + // `PostDispatchInfo::calc_unspent` guards against such a case. + match payed.offset(refund_imbalance) { + Ok(actual_payment) => Ok(Some(actual_payment)), + Err(_) => return Err(InvalidTransaction::Payment.into()), + } // We do not recreate the account using the refund. The up front payment + // is gone in that case. + Err(_) => Ok(Some(payed)), + } + } else { + Ok(amount) + } } - fn deposit( - amount: Self::WithdrawAmount, - ) { - todo!() + fn finalize(amount: Self::WithdrawAmount) { + if let Some(_payed) = amount { + // pay out the MONEY! Call OU + } } - } pub trait Trait: frame_system::Trait { @@ -275,7 +314,7 @@ pub trait Trait: frame_system::Trait { /// Handler for withdrawing, refunding and depositing the transaction fee. /// Transaction fees are withdrawen before the transaction is executed. - /// After the transaction was executed the transaction weight can be adjusted, depending on the used resources by + /// After the transaction was executed the transaction weight can be adjusted, depending on the used resources by /// the transaction. If the transaction weight is lower than expected, parts of the transaction fee might be refunded. /// In the end the fees can be deposited. type OnChargeTransaction: OnChargeTransaction; @@ -502,7 +541,7 @@ where ) -> Result< ( T::Balance, - Option<<::OnChargeTransaction as OnChargeTransaction>::WithdrawAmount>, + <::OnChargeTransaction as OnChargeTransaction>::WithdrawAmount, ), TransactionValidityError, > { @@ -511,11 +550,10 @@ where // Only mess with balances if fee is not zero. if fee.is_zero() { - return Ok((fee, None)); + return Ok((fee, Default::default())); } <::OnChargeTransaction as OnChargeTransaction>::withdraw(who, fee, tip) - .map(|(x, y)| (x, Some(y))) } } @@ -542,7 +580,7 @@ where type Pre = ( T::Balance, Self::AccountId, - Option<<::OnChargeTransaction as OnChargeTransaction>::WithdrawAmount>, + <::OnChargeTransaction as OnChargeTransaction>::WithdrawAmount, T::Balance, ); fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> { @@ -584,17 +622,15 @@ where _result: &DispatchResult, ) -> Result<(), TransactionValidityError> { let (tip, who, imbalance, fee) = pre; - if let Some(payed) = imbalance { - let actual_fee = Module::::compute_actual_fee( - len as u32, - info, - post_info, - tip, - ); - let refund = fee.saturating_sub(actual_fee); - let actual_payment = T::OnChargeTransaction::refund(&who, payed, refund)?; - T::OnChargeTransaction::deposit(actual_payment); - } + let actual_fee = Module::::compute_actual_fee( + len as u32, + info, + post_info, + tip, + ); + let refund = fee.saturating_sub(actual_fee); + let actual_payment = T::OnChargeTransaction::refund(&who, imbalance, refund)?; + T::OnChargeTransaction::finalize(actual_payment); Ok(()) } } @@ -727,8 +763,8 @@ mod tests { } impl Trait for Runtime { - type Currency = pallet_balances::Module; - type OnTransactionPayment = (); + type Balance = u64; + type OnChargeTransaction = (PhantomData>, PhantomData<()>); type TransactionByteFee = TransactionByteFee; type WeightToFee = WeightToFee; type FeeMultiplierUpdate = (); From 052d9518e677d64da5cc3f224678014d95d9e268 Mon Sep 17 00:00:00 2001 From: Albrecht <14820950+weichweich@users.noreply.github.com> Date: Tue, 11 Aug 2020 15:51:37 +0200 Subject: [PATCH 04/23] fix dependent tests --- frame/balances/src/tests_composite.rs | 6 +++--- frame/balances/src/tests_local.rs | 6 +++--- frame/executive/src/lib.rs | 4 ++-- frame/transaction-payment/src/lib.rs | 11 +++++++---- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/frame/balances/src/tests_composite.rs b/frame/balances/src/tests_composite.rs index 8e764112ba24c..d334b30ff246a 100644 --- a/frame/balances/src/tests_composite.rs +++ b/frame/balances/src/tests_composite.rs @@ -29,7 +29,7 @@ use sp_io; use frame_support::{impl_outer_origin, impl_outer_event, parameter_types}; use frame_support::traits::Get; use frame_support::weights::{Weight, DispatchInfo, IdentityFee}; -use std::cell::RefCell; +use std::{marker::PhantomData, cell::RefCell}; use crate::{GenesisConfig, Module, Trait, decl_tests, tests::CallWithDispatchInfo}; use frame_system as system; @@ -97,8 +97,8 @@ parameter_types! { pub const TransactionByteFee: u64 = 1; } impl pallet_transaction_payment::Trait for Test { - type Currency = Module; - type OnTransactionPayment = (); + type Balance = u64; + type OnChargeTransaction = (PhantomData>, PhantomData<()>); type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; type FeeMultiplierUpdate = (); diff --git a/frame/balances/src/tests_local.rs b/frame/balances/src/tests_local.rs index 86abc2b6044ce..d0f7320beeceb 100644 --- a/frame/balances/src/tests_local.rs +++ b/frame/balances/src/tests_local.rs @@ -29,7 +29,7 @@ use sp_io; use frame_support::{impl_outer_origin, impl_outer_event, parameter_types}; use frame_support::traits::{Get, StorageMapShim}; use frame_support::weights::{Weight, DispatchInfo, IdentityFee}; -use std::cell::RefCell; +use std::{marker::PhantomData, cell::RefCell}; use crate::{GenesisConfig, Module, Trait, decl_tests, tests::CallWithDispatchInfo}; use frame_system as system; @@ -97,8 +97,8 @@ parameter_types! { pub const TransactionByteFee: u64 = 1; } impl pallet_transaction_payment::Trait for Test { - type Currency = Module; - type OnTransactionPayment = (); + type Balance = u64; + type OnChargeTransaction = (PhantomData>, PhantomData<()>); type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; type FeeMultiplierUpdate = (); diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 24dccf8b0b4a4..61005753e4d0e 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -591,8 +591,8 @@ mod tests { pub const TransactionByteFee: Balance = 0; } impl pallet_transaction_payment::Trait for Runtime { - type Currency = Balances; - type OnTransactionPayment = (); + type Balance = Balance; + type OnChargeTransaction = (PhantomData>, PhantomData<()>); type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; type FeeMultiplierUpdate = (); diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index ed4a4d16d187f..e86311a9f44e5 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -227,6 +227,7 @@ pub trait OnChargeTransaction { fn finalize( amount: Self::WithdrawAmount, + tip: T::Balance, ); } @@ -294,9 +295,11 @@ where } } - fn finalize(amount: Self::WithdrawAmount) { - if let Some(_payed) = amount { - // pay out the MONEY! Call OU + fn finalize(amount: Self::WithdrawAmount, tip: T::Balance) { + if let Some(payed) = amount { + let imbalances = payed.split(tip); + OU::on_unbalanceds(Some(imbalances.0).into_iter() + .chain(Some(imbalances.1))); } } } @@ -630,7 +633,7 @@ where ); let refund = fee.saturating_sub(actual_fee); let actual_payment = T::OnChargeTransaction::refund(&who, imbalance, refund)?; - T::OnChargeTransaction::finalize(actual_payment); + T::OnChargeTransaction::finalize(actual_payment, tip); Ok(()) } } From 78b503b37ae590af1d85700f03d6b691ea7656c9 Mon Sep 17 00:00:00 2001 From: Albrecht <14820950+weichweich@users.noreply.github.com> Date: Tue, 11 Aug 2020 16:07:07 +0200 Subject: [PATCH 05/23] comments --- frame/transaction-payment/src/lib.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index e86311a9f44e5..bff35a1c13ce0 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -210,27 +210,40 @@ impl Default for Releases { } } +/// Handle withdrawing, refunding and depositing of transaction fees. pub trait OnChargeTransaction { type WithdrawAmount: Default; + /// Withdraw the transaction fee from a specific account. The sender of the transaction may also indicate that they + /// want to pay a tip to the block author. + /// A tuple containing the deducted balance and some state information for handling the deducted amount. fn withdraw( who: &T::AccountId, fee: T::Balance, tip: T::Balance, ) -> Result<(T::Balance, Self::WithdrawAmount), TransactionValidityError>; + /// After executing the transaction, the fee may be adjusted to the actual cost. In that case parts of the fee may be refunded. + /// Refunding parts of the transaction fees should be handled in this method. + /// - `who` the origin of the transaction + /// - `amount` the original amount that was paid by the origin. + /// - `refund` the amount that should be refunded. fn refund( who: &T::AccountId, amount: Self::WithdrawAmount, refund: T::Balance, ) -> Result; + /// After the transaction was executed and the correct fees where collected, the resulting funds can be spend here. + /// - `amount` the state information about how much was actually deducted from the origin. + /// - `tip` the tip contained inside the amount fn finalize( amount: Self::WithdrawAmount, tip: T::Balance, ); } +/// Default implementation for a Currency and an OnUnbalanced handler. impl OnChargeTransaction for (PhantomData, PhantomData) where B: AtLeast32BitUnsigned From 9bb1b460867aa18bb8de2bd09024520597025b16 Mon Sep 17 00:00:00 2001 From: Albrecht <14820950+weichweich@users.noreply.github.com> Date: Thu, 13 Aug 2020 16:30:18 +0200 Subject: [PATCH 06/23] restructure a bit and include more info --- frame/transaction-payment/src/lib.rs | 131 +++++++++++++-------------- 1 file changed, 63 insertions(+), 68 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index bff35a1c13ce0..616bc7d8522ee 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -212,35 +212,31 @@ impl Default for Releases { /// Handle withdrawing, refunding and depositing of transaction fees. pub trait OnChargeTransaction { - type WithdrawAmount: Default; + type LiquidityInfo: Default; - /// Withdraw the transaction fee from a specific account. The sender of the transaction may also indicate that they - /// want to pay a tip to the block author. - /// A tuple containing the deducted balance and some state information for handling the deducted amount. - fn withdraw( + /// Ensure that the transaction fees will be paid. Returns information that are used to conclude the fee payment. + /// + fn ensure_liquidity( + dispatch_info: &DispatchInfoOf, + call: &T::Call, who: &T::AccountId, fee: T::Balance, tip: T::Balance, - ) -> Result<(T::Balance, Self::WithdrawAmount), TransactionValidityError>; - - /// After executing the transaction, the fee may be adjusted to the actual cost. In that case parts of the fee may be refunded. - /// Refunding parts of the transaction fees should be handled in this method. - /// - `who` the origin of the transaction - /// - `amount` the original amount that was paid by the origin. - /// - `refund` the amount that should be refunded. - fn refund( - who: &T::AccountId, - amount: Self::WithdrawAmount, - refund: T::Balance, - ) -> Result; + ) -> Result; /// After the transaction was executed and the correct fees where collected, the resulting funds can be spend here. - /// - `amount` the state information about how much was actually deducted from the origin. - /// - `tip` the tip contained inside the amount - fn finalize( - amount: Self::WithdrawAmount, + /// - `who` + /// - `fee` + /// - `tip` + /// - `info` + fn conclude_payment( + dispatch_info: &DispatchInfoOf, + post_info: &PostDispatchInfoOf, + who: &T::AccountId, + fee: T::Balance, tip: T::Balance, - ); + liquidity_info: Self::LiquidityInfo, + ) -> Result<(), TransactionValidityError>; } /// Default implementation for a Currency and an OnUnbalanced handler. @@ -261,15 +257,17 @@ where C::NegativeImbalance: Imbalance, OU: OnUnbalanced>, { - type WithdrawAmount = Option>; + type LiquidityInfo = Option>; - fn withdraw( + fn ensure_liquidity( + _info: &DispatchInfoOf, + _call: &T::Call, who: &T::AccountId, fee: B, tip: B, - ) -> Result<(B, Self::WithdrawAmount), TransactionValidityError> { + ) -> Result { if fee.is_zero() { - return Ok((B::zero(), None)) + return Ok(None) } match C::withdraw( who, @@ -281,39 +279,35 @@ where }, ExistenceRequirement::KeepAlive, ) { - Ok(imbalance) => Ok((fee, Some(imbalance))), + Ok(imbalance) => Ok(Some(imbalance)), Err(_) => Err(InvalidTransaction::Payment.into()), } } - fn refund( + fn conclude_payment( + _dispatch_info: &DispatchInfoOf, + _post_info: &PostDispatchInfoOf, who: &T::AccountId, - amount: Self::WithdrawAmount, - refund: B, - ) -> Result { - if let Some(payed) = amount { - match C::deposit_into_existing(&who, refund) { - Ok(refund_imbalance) => - // The refund cannot be larger than the up front payed max weight. - // `PostDispatchInfo::calc_unspent` guards against such a case. - match payed.offset(refund_imbalance) { - Ok(actual_payment) => Ok(Some(actual_payment)), - Err(_) => return Err(InvalidTransaction::Payment.into()), - } // We do not recreate the account using the refund. The up front payment - // is gone in that case. - Err(_) => Ok(Some(payed)), - } - } else { - Ok(amount) - } - } - - fn finalize(amount: Self::WithdrawAmount, tip: T::Balance) { - if let Some(payed) = amount { - let imbalances = payed.split(tip); + fee: B, + tip: B, + liquidity_info: Self::LiquidityInfo, + ) -> Result<(), TransactionValidityError> { + if let Some(paid) = liquidity_info { + // Calculate how much refund we should return + let refund_amount = paid.peek().saturating_sub(fee); + // refund to the the account that paid the fees. If this fails, the account might have dropped below the + // existential balance. In that case we don't refund anything. sorry. :( + let refund_imbalance = C::deposit_into_existing(&who, refund_amount) + .unwrap_or_else(|_| C::PositiveImbalance::zero()); + // merge the imbalance caused by paying the fees and refunding parts of it again. + let adjusted_paid = paid.offset(refund_imbalance) + .map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::Payment))?; + // Call someone else to handle the imbalance (fee and tip separately) + let imbalances = adjusted_paid.split(tip); OU::on_unbalanceds(Some(imbalances.0).into_iter() .chain(Some(imbalances.1))); } + Ok(()) } } @@ -549,15 +543,16 @@ where Self(fee) } - fn withdraw_fee( + fn ensure_fee( &self, - who: &T::AccountId, info: &DispatchInfoOf, + call: &T::Call, + who: &T::AccountId, len: usize, ) -> Result< ( T::Balance, - <::OnChargeTransaction as OnChargeTransaction>::WithdrawAmount, + <::OnChargeTransaction as OnChargeTransaction>::LiquidityInfo, ), TransactionValidityError, > { @@ -567,9 +562,10 @@ where // Only mess with balances if fee is not zero. if fee.is_zero() { return Ok((fee, Default::default())); + } else { + <::OnChargeTransaction as OnChargeTransaction>::ensure_liquidity(info, call, who, fee, tip) + .map(|i| (fee, i)) } - - <::OnChargeTransaction as OnChargeTransaction>::withdraw(who, fee, tip) } } @@ -594,10 +590,12 @@ where type Call = T::Call; type AdditionalSigned = (); type Pre = ( + // tip T::Balance, + // who paid the fee Self::AccountId, - <::OnChargeTransaction as OnChargeTransaction>::WithdrawAmount, - T::Balance, + // imbalance resulting from withdrawing the fee + <::OnChargeTransaction as OnChargeTransaction>::LiquidityInfo, ); fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> { Ok(()) @@ -606,12 +604,11 @@ where fn validate( &self, who: &Self::AccountId, - _call: &Self::Call, + call: &Self::Call, info: &DispatchInfoOf, len: usize, ) -> TransactionValidity { - let (fee, _) = self.withdraw_fee(who, info, len)?; - + let (fee, _) = self.ensure_fee(info, call, who, len)?; let mut r = ValidTransaction::default(); // NOTE: we probably want to maximize the _fee (of any type) per weight unit_ here, which // will be a bit more than setting the priority to tip. For now, this is enough. @@ -622,12 +619,12 @@ where fn pre_dispatch( self, who: &Self::AccountId, - _call: &Self::Call, + call: &Self::Call, info: &DispatchInfoOf, len: usize ) -> Result { - let (fee, imbalance) = self.withdraw_fee(who, info, len)?; - Ok((self.0, who.clone(), imbalance, fee)) + let (_fee, imbalance) = self.ensure_fee(info, call, who, len)?; + Ok((self.0, who.clone(), imbalance)) } fn post_dispatch( @@ -637,16 +634,14 @@ where len: usize, _result: &DispatchResult, ) -> Result<(), TransactionValidityError> { - let (tip, who, imbalance, fee) = pre; + let (tip, who, imbalance) = pre; let actual_fee = Module::::compute_actual_fee( len as u32, info, post_info, tip, ); - let refund = fee.saturating_sub(actual_fee); - let actual_payment = T::OnChargeTransaction::refund(&who, imbalance, refund)?; - T::OnChargeTransaction::finalize(actual_payment, tip); + T::OnChargeTransaction::conclude_payment(info, post_info, &who, actual_fee, tip, imbalance)?; Ok(()) } } From 1d08e13ac452dca034980ff58551ade4dbf53703 Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Mon, 17 Aug 2020 13:19:11 +0200 Subject: [PATCH 07/23] clean up ugly phantom --- bin/node-template/runtime/src/lib.rs | 5 +++-- bin/node/runtime/src/lib.rs | 6 +++--- frame/balances/src/tests_composite.rs | 5 +++-- frame/balances/src/tests_local.rs | 5 +++-- frame/executive/src/lib.rs | 3 ++- frame/transaction-payment/src/lib.rs | 6 ++++-- 6 files changed, 18 insertions(+), 12 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 334b5e7759963..7d01910916352 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -6,7 +6,7 @@ #[cfg(feature = "std")] include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); -use sp_std::{prelude::*, marker::PhantomData}; +use sp_std::prelude::*; use sp_core::{crypto::KeyTypeId, OpaqueMetadata}; use sp_runtime::{ ApplyExtrinsicResult, generic, create_runtime_str, impl_opaque_keys, MultiSignature, @@ -37,6 +37,7 @@ pub use frame_support::{ constants::{BlockExecutionWeight, ExtrinsicBaseWeight, RocksDbWeight, WEIGHT_PER_SECOND}, }, }; +use pallet_transaction_payment::CurrencyAdapter; /// Import the template pallet. pub use template; @@ -246,7 +247,7 @@ parameter_types! { impl pallet_transaction_payment::Trait for Runtime { type Balance = Balance; - type OnChargeTransaction = (PhantomData>, PhantomData<()>); + type OnChargeTransaction = CurrencyAdapter, ()>; type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; type FeeMultiplierUpdate = (); diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 62b3fc98412c9..5e07c59e7ccb1 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -22,7 +22,7 @@ // `construct_runtime!` does a lot of recursion and requires us to increase the limit to 256. #![recursion_limit="256"] -use sp_std::{prelude::*, marker::PhantomData}; +use sp_std::prelude::*; use frame_support::{ construct_runtime, parameter_types, debug, RuntimeDebug, @@ -61,7 +61,7 @@ use pallet_grandpa::fg_primitives; use pallet_im_online::sr25519::AuthorityId as ImOnlineId; use sp_authority_discovery::AuthorityId as AuthorityDiscoveryId; use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo; -pub use pallet_transaction_payment::{Multiplier, TargetedFeeAdjustment}; +pub use pallet_transaction_payment::{Multiplier, TargetedFeeAdjustment, CurrencyAdapter}; use pallet_contracts_rpc_runtime_api::ContractExecResult; use pallet_session::{historical as pallet_session_historical}; use sp_inherents::{InherentData, CheckInherentsResult}; @@ -337,7 +337,7 @@ parameter_types! { impl pallet_transaction_payment::Trait for Runtime { type Balance = Balance; - type OnChargeTransaction = (PhantomData>, PhantomData); + type OnChargeTransaction = CurrencyAdapter, DealWithFees>; type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; type FeeMultiplierUpdate = diff --git a/frame/balances/src/tests_composite.rs b/frame/balances/src/tests_composite.rs index d334b30ff246a..924e03f1cd8d3 100644 --- a/frame/balances/src/tests_composite.rs +++ b/frame/balances/src/tests_composite.rs @@ -29,7 +29,8 @@ use sp_io; use frame_support::{impl_outer_origin, impl_outer_event, parameter_types}; use frame_support::traits::Get; use frame_support::weights::{Weight, DispatchInfo, IdentityFee}; -use std::{marker::PhantomData, cell::RefCell}; +use pallet_transaction_payment::CurrencyAdapter; +use std::cell::RefCell; use crate::{GenesisConfig, Module, Trait, decl_tests, tests::CallWithDispatchInfo}; use frame_system as system; @@ -98,7 +99,7 @@ parameter_types! { } impl pallet_transaction_payment::Trait for Test { type Balance = u64; - type OnChargeTransaction = (PhantomData>, PhantomData<()>); + type OnChargeTransaction = CurrencyAdapter, ()>; type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; type FeeMultiplierUpdate = (); diff --git a/frame/balances/src/tests_local.rs b/frame/balances/src/tests_local.rs index d0f7320beeceb..fdc894716d6e2 100644 --- a/frame/balances/src/tests_local.rs +++ b/frame/balances/src/tests_local.rs @@ -29,8 +29,9 @@ use sp_io; use frame_support::{impl_outer_origin, impl_outer_event, parameter_types}; use frame_support::traits::{Get, StorageMapShim}; use frame_support::weights::{Weight, DispatchInfo, IdentityFee}; -use std::{marker::PhantomData, cell::RefCell}; +use std::cell::RefCell; use crate::{GenesisConfig, Module, Trait, decl_tests, tests::CallWithDispatchInfo}; +use pallet_transaction_payment::CurrencyAdapter; use frame_system as system; impl_outer_origin!{ @@ -98,7 +99,7 @@ parameter_types! { } impl pallet_transaction_payment::Trait for Test { type Balance = u64; - type OnChargeTransaction = (PhantomData>, PhantomData<()>); + type OnChargeTransaction = CurrencyAdapter, ()>; type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; type FeeMultiplierUpdate = (); diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 61005753e4d0e..1afc7a0e62ae1 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -465,6 +465,7 @@ mod tests { traits::{Currency, LockIdentifier, LockableCurrency, WithdrawReasons, WithdrawReason}, }; use frame_system::{self as system, Call as SystemCall, ChainContext, LastRuntimeUpgradeInfo}; + use pallet_transaction_payment::CurrencyAdapter; use pallet_balances::Call as BalancesCall; use hex_literal::hex; const TEST_KEY: &[u8] = &*b":test:key:"; @@ -592,7 +593,7 @@ mod tests { } impl pallet_transaction_payment::Trait for Runtime { type Balance = Balance; - type OnChargeTransaction = (PhantomData>, PhantomData<()>); + type OnChargeTransaction = CurrencyAdapter, ()>; type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; type FeeMultiplierUpdate = (); diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 616bc7d8522ee..a10ed43290417 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -239,8 +239,10 @@ pub trait OnChargeTransaction { ) -> Result<(), TransactionValidityError>; } +pub struct CurrencyAdapter(PhantomData, PhantomData); + /// Default implementation for a Currency and an OnUnbalanced handler. -impl OnChargeTransaction for (PhantomData, PhantomData) +impl OnChargeTransaction for CurrencyAdapter where B: AtLeast32BitUnsigned + FullCodec @@ -775,7 +777,7 @@ mod tests { impl Trait for Runtime { type Balance = u64; - type OnChargeTransaction = (PhantomData>, PhantomData<()>); + type OnChargeTransaction = CurrencyAdapter, ()>; type TransactionByteFee = TransactionByteFee; type WeightToFee = WeightToFee; type FeeMultiplierUpdate = (); From af9bda7466e3d8640a80bdf6866546d30a060568 Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Tue, 18 Aug 2020 11:23:32 +0200 Subject: [PATCH 08/23] reduce complexity --- frame/transaction-payment/src/lib.rs | 128 +++-------------------- frame/transaction-payment/src/payment.rs | 108 +++++++++++++++++++ 2 files changed, 122 insertions(+), 114 deletions(-) create mode 100644 frame/transaction-payment/src/payment.rs diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index a10ed43290417..f751071799722 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -29,14 +29,15 @@ //! - A means of updating the fee for the next block, via defining a multiplier, based on the //! final state of the chain at the end of the previous block. This can be configured via //! [`Trait::FeeMultiplierUpdate`] +//! - How the fees are paid via [`Trait::OnChargeTransaction`]. #![cfg_attr(not(feature = "std"), no_std)] -use sp_std::{prelude::*, fmt::Debug, marker::PhantomData}; +use sp_std::{prelude::*, fmt::Debug}; use codec::{Encode, Decode, FullCodec}; use frame_support::{ decl_storage, decl_module, - traits::{Currency, Get, OnUnbalanced, ExistenceRequirement, WithdrawReason, Imbalance}, + traits::{Currency, Get}, weights::{ Weight, DispatchInfo, PostDispatchInfo, GetDispatchInfo, Pays, WeightToFeePolynomial, WeightToFeeCoefficient, @@ -46,8 +47,7 @@ use frame_support::{ use sp_runtime::{ FixedU128, FixedPointNumber, FixedPointOperand, Perquintill, RuntimeDebug, transaction_validity::{ - TransactionPriority, ValidTransaction, InvalidTransaction, TransactionValidityError, - TransactionValidity, + TransactionPriority, ValidTransaction, TransactionValidityError, TransactionValidity, }, traits::{ Zero, Saturating, SignedExtension, SaturatedConversion, Convert, Dispatchable, @@ -56,6 +56,9 @@ use sp_runtime::{ }; use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo; +mod payment; +pub use payment::*; + /// Fee multiplier. pub type Multiplier = FixedU128; @@ -210,109 +213,6 @@ impl Default for Releases { } } -/// Handle withdrawing, refunding and depositing of transaction fees. -pub trait OnChargeTransaction { - type LiquidityInfo: Default; - - /// Ensure that the transaction fees will be paid. Returns information that are used to conclude the fee payment. - /// - fn ensure_liquidity( - dispatch_info: &DispatchInfoOf, - call: &T::Call, - who: &T::AccountId, - fee: T::Balance, - tip: T::Balance, - ) -> Result; - - /// After the transaction was executed and the correct fees where collected, the resulting funds can be spend here. - /// - `who` - /// - `fee` - /// - `tip` - /// - `info` - fn conclude_payment( - dispatch_info: &DispatchInfoOf, - post_info: &PostDispatchInfoOf, - who: &T::AccountId, - fee: T::Balance, - tip: T::Balance, - liquidity_info: Self::LiquidityInfo, - ) -> Result<(), TransactionValidityError>; -} - -pub struct CurrencyAdapter(PhantomData, PhantomData); - -/// Default implementation for a Currency and an OnUnbalanced handler. -impl OnChargeTransaction for CurrencyAdapter -where - B: AtLeast32BitUnsigned - + FullCodec - + Copy - + MaybeSerializeDeserialize - + Debug - + Default - + Send - + Sync, - T: Trait, - T::TransactionByteFee: Get, - C: Currency<::AccountId, Balance=B>, - C::PositiveImbalance: Imbalance, - C::NegativeImbalance: Imbalance, - OU: OnUnbalanced>, -{ - type LiquidityInfo = Option>; - - fn ensure_liquidity( - _info: &DispatchInfoOf, - _call: &T::Call, - who: &T::AccountId, - fee: B, - tip: B, - ) -> Result { - if fee.is_zero() { - return Ok(None) - } - match C::withdraw( - who, - fee, - if tip.is_zero() { - WithdrawReason::TransactionPayment.into() - } else { - WithdrawReason::TransactionPayment | WithdrawReason::Tip - }, - ExistenceRequirement::KeepAlive, - ) { - Ok(imbalance) => Ok(Some(imbalance)), - Err(_) => Err(InvalidTransaction::Payment.into()), - } - } - - fn conclude_payment( - _dispatch_info: &DispatchInfoOf, - _post_info: &PostDispatchInfoOf, - who: &T::AccountId, - fee: B, - tip: B, - liquidity_info: Self::LiquidityInfo, - ) -> Result<(), TransactionValidityError> { - if let Some(paid) = liquidity_info { - // Calculate how much refund we should return - let refund_amount = paid.peek().saturating_sub(fee); - // refund to the the account that paid the fees. If this fails, the account might have dropped below the - // existential balance. In that case we don't refund anything. sorry. :( - let refund_imbalance = C::deposit_into_existing(&who, refund_amount) - .unwrap_or_else(|_| C::PositiveImbalance::zero()); - // merge the imbalance caused by paying the fees and refunding parts of it again. - let adjusted_paid = paid.offset(refund_imbalance) - .map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::Payment))?; - // Call someone else to handle the imbalance (fee and tip separately) - let imbalances = adjusted_paid.split(tip); - OU::on_unbalanceds(Some(imbalances.0).into_iter() - .chain(Some(imbalances.1))); - } - Ok(()) - } -} - pub trait Trait: frame_system::Trait { /// The currency type in which fees will be paid. type Balance: AtLeast32BitUnsigned @@ -545,11 +445,11 @@ where Self(fee) } - fn ensure_fee( + fn withdraw_fee( &self, - info: &DispatchInfoOf, - call: &T::Call, who: &T::AccountId, + call: &T::Call, + info: &DispatchInfoOf, len: usize, ) -> Result< ( @@ -565,7 +465,7 @@ where if fee.is_zero() { return Ok((fee, Default::default())); } else { - <::OnChargeTransaction as OnChargeTransaction>::ensure_liquidity(info, call, who, fee, tip) + <::OnChargeTransaction as OnChargeTransaction>::withdraw_fee(who, call, info, fee, tip) .map(|i| (fee, i)) } } @@ -610,7 +510,7 @@ where info: &DispatchInfoOf, len: usize, ) -> TransactionValidity { - let (fee, _) = self.ensure_fee(info, call, who, len)?; + let (fee, _) = self.withdraw_fee(who, call, info, len)?; let mut r = ValidTransaction::default(); // NOTE: we probably want to maximize the _fee (of any type) per weight unit_ here, which // will be a bit more than setting the priority to tip. For now, this is enough. @@ -625,7 +525,7 @@ where info: &DispatchInfoOf, len: usize ) -> Result { - let (_fee, imbalance) = self.ensure_fee(info, call, who, len)?; + let (_fee, imbalance) = self.withdraw_fee(who, call, info, len)?; Ok((self.0, who.clone(), imbalance)) } @@ -643,7 +543,7 @@ where post_info, tip, ); - T::OnChargeTransaction::conclude_payment(info, post_info, &who, actual_fee, tip, imbalance)?; + T::OnChargeTransaction::deposit_fee(&who, info, post_info, actual_fee, tip, imbalance)?; Ok(()) } } diff --git a/frame/transaction-payment/src/payment.rs b/frame/transaction-payment/src/payment.rs new file mode 100644 index 0000000000000..3456fa6cba784 --- /dev/null +++ b/frame/transaction-payment/src/payment.rs @@ -0,0 +1,108 @@ +///! Traits and default implementation for paying transaction fees. +use crate::{NegativeImbalanceOf, Trait}; +use codec::FullCodec; +use frame_support::{ + traits::{Currency, ExistenceRequirement, Get, Imbalance, OnUnbalanced, WithdrawReason}, + unsigned::TransactionValidityError, +}; +use sp_runtime::{ + traits::{AtLeast32BitUnsigned, DispatchInfoOf, MaybeSerializeDeserialize, PostDispatchInfoOf}, + transaction_validity::InvalidTransaction, +}; +use sp_std::{fmt::Debug, marker::PhantomData}; + +/// Handle withdrawing, refunding and depositing of transaction fees. +pub trait OnChargeTransaction { + type LiquidityInfo: Default; + + /// Before the transaction is executed the payment of the transaction fees need to be secured. + fn withdraw_fee( + who: &T::AccountId, + call: &T::Call, + dispatch_info: &DispatchInfoOf, + fee: T::Balance, + tip: T::Balance, + ) -> Result; + + /// After the transaction was executed and the correct fees where collected, the resulting funds can be spend here. + fn deposit_fee( + who: &T::AccountId, + dispatch_info: &DispatchInfoOf, + post_info: &PostDispatchInfoOf, + fee: T::Balance, + tip: T::Balance, + liquidity_info: Self::LiquidityInfo, + ) -> Result<(), TransactionValidityError>; +} + +/// Implements the transaction payment for a Currency (eg. the pallet_balances) using an unbalance handler +/// (`[OnUnbalanced]`). +pub struct CurrencyAdapter(PhantomData, PhantomData); + +/// Default implementation for a Currency and an OnUnbalanced handler. +impl OnChargeTransaction for CurrencyAdapter +where + B: AtLeast32BitUnsigned + FullCodec + Copy + MaybeSerializeDeserialize + Debug + Default + Send + Sync, + T: Trait, + T::TransactionByteFee: Get, + C: Currency<::AccountId, Balance = B>, + C::PositiveImbalance: Imbalance, + C::NegativeImbalance: Imbalance, + OU: OnUnbalanced>, +{ + type LiquidityInfo = Option>; + + /// Withdraw the predicted fee from the transaction origin. + fn withdraw_fee( + who: &T::AccountId, + _call: &T::Call, + _info: &DispatchInfoOf, + fee: B, + tip: B, + ) -> Result { + if fee.is_zero() { + return Ok(None); + } + match C::withdraw( + who, + fee, + if tip.is_zero() { + WithdrawReason::TransactionPayment.into() + } else { + WithdrawReason::TransactionPayment | WithdrawReason::Tip + }, + ExistenceRequirement::KeepAlive, + ) { + Ok(imbalance) => Ok(Some(imbalance)), + Err(_) => Err(InvalidTransaction::Payment.into()), + } + } + + /// Hand the fee and the tip over to the `[OnUnbalanced]` implementation. Since the predicted fee might have been + /// too high, parts of the fee may be refunded. + fn deposit_fee( + who: &T::AccountId, + _dispatch_info: &DispatchInfoOf, + _post_info: &PostDispatchInfoOf, + fee: B, + tip: B, + liquidity_info: Self::LiquidityInfo, + ) -> Result<(), TransactionValidityError> { + if let Some(paid) = liquidity_info { + // Calculate how much refund we should return + let refund_amount = paid.peek().saturating_sub(fee); + // refund to the the account that paid the fees. If this fails, the account might have dropped below the + // existential balance. In that case we don't refund anything. sorry. :( + let refund_imbalance = + C::deposit_into_existing(&who, refund_amount).unwrap_or_else(|_| C::PositiveImbalance::zero()); + // merge the imbalance caused by paying the fees and refunding parts of it again. + let adjusted_paid = paid + .offset(refund_imbalance) + .map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::Payment))?; + // Call someone else to handle the imbalance (fee and tip separately) + let imbalances = adjusted_paid.split(tip); + OU::on_unbalanceds(Some(imbalances.0).into_iter().chain(Some(imbalances.1))); + } + Ok(()) + } +} From 96af556724f345fee88008c5ed94b6edfb7fca26 Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Tue, 18 Aug 2020 13:25:16 +0200 Subject: [PATCH 09/23] minor doc improvements --- frame/transaction-payment/src/payment.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/transaction-payment/src/payment.rs b/frame/transaction-payment/src/payment.rs index 3456fa6cba784..cc558f6d57c35 100644 --- a/frame/transaction-payment/src/payment.rs +++ b/frame/transaction-payment/src/payment.rs @@ -35,8 +35,8 @@ pub trait OnChargeTransaction { ) -> Result<(), TransactionValidityError>; } -/// Implements the transaction payment for a Currency (eg. the pallet_balances) using an unbalance handler -/// (`[OnUnbalanced]`). +/// Implements the transaction payment for a module implementing the `Currency` trait (eg. the pallet_balances) using an +/// unbalance handler ( implementing `OnUnbalanced`). pub struct CurrencyAdapter(PhantomData, PhantomData); /// Default implementation for a Currency and an OnUnbalanced handler. From 308c22038ee56dac681d9feb2395c1a3da4e14ad Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Tue, 18 Aug 2020 14:04:29 +0200 Subject: [PATCH 10/23] use shorthand --- bin/node-template/runtime/src/lib.rs | 2 +- bin/node/runtime/src/lib.rs | 2 +- frame/executive/src/lib.rs | 2 +- frame/transaction-payment/src/lib.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 7d01910916352..e88b79009b87b 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -247,7 +247,7 @@ parameter_types! { impl pallet_transaction_payment::Trait for Runtime { type Balance = Balance; - type OnChargeTransaction = CurrencyAdapter, ()>; + type OnChargeTransaction = CurrencyAdapter; type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; type FeeMultiplierUpdate = (); diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 5e07c59e7ccb1..e3d556a425552 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -337,7 +337,7 @@ parameter_types! { impl pallet_transaction_payment::Trait for Runtime { type Balance = Balance; - type OnChargeTransaction = CurrencyAdapter, DealWithFees>; + type OnChargeTransaction = CurrencyAdapter; type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; type FeeMultiplierUpdate = diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 1afc7a0e62ae1..b39352af34a4d 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -593,7 +593,7 @@ mod tests { } impl pallet_transaction_payment::Trait for Runtime { type Balance = Balance; - type OnChargeTransaction = CurrencyAdapter, ()>; + type OnChargeTransaction = CurrencyAdapter; type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; type FeeMultiplierUpdate = (); diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index f751071799722..bf58de1145f8e 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -677,7 +677,7 @@ mod tests { impl Trait for Runtime { type Balance = u64; - type OnChargeTransaction = CurrencyAdapter, ()>; + type OnChargeTransaction = CurrencyAdapter; type TransactionByteFee = TransactionByteFee; type WeightToFee = WeightToFee; type FeeMultiplierUpdate = (); From c7249043fe0aac802c5d3542db288456ed243f77 Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Tue, 18 Aug 2020 14:07:25 +0200 Subject: [PATCH 11/23] doc --- frame/transaction-payment/src/payment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/transaction-payment/src/payment.rs b/frame/transaction-payment/src/payment.rs index cc558f6d57c35..9c94ce097ccc6 100644 --- a/frame/transaction-payment/src/payment.rs +++ b/frame/transaction-payment/src/payment.rs @@ -92,7 +92,7 @@ where // Calculate how much refund we should return let refund_amount = paid.peek().saturating_sub(fee); // refund to the the account that paid the fees. If this fails, the account might have dropped below the - // existential balance. In that case we don't refund anything. sorry. :( + // existential balance. In that case we don't refund anything. let refund_imbalance = C::deposit_into_existing(&who, refund_amount).unwrap_or_else(|_| C::PositiveImbalance::zero()); // merge the imbalance caused by paying the fees and refunding parts of it again. From 99eb07925bfad5e5b7f4417972a88e3e1f1f055f Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Tue, 18 Aug 2020 14:31:33 +0200 Subject: [PATCH 12/23] fix line lenght and style --- frame/transaction-payment/src/lib.rs | 9 +++++---- frame/transaction-payment/src/payment.rs | 21 +++++++++++++-------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index bf58de1145f8e..69080f3727d96 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -226,9 +226,10 @@ pub trait Trait: frame_system::Trait { /// Handler for withdrawing, refunding and depositing the transaction fee. /// Transaction fees are withdrawen before the transaction is executed. - /// After the transaction was executed the transaction weight can be adjusted, depending on the used resources by - /// the transaction. If the transaction weight is lower than expected, parts of the transaction fee might be refunded. - /// In the end the fees can be deposited. + /// After the transaction was executed the transaction weight can be + /// adjusted, depending on the used resources by the transaction. If the + /// transaction weight is lower than expected, parts of the transaction fee + /// might be refunded. In the end the fees can be deposited. type OnChargeTransaction: OnChargeTransaction; /// The fee to be paid for making a transaction; the per-byte portion. @@ -463,7 +464,7 @@ where // Only mess with balances if fee is not zero. if fee.is_zero() { - return Ok((fee, Default::default())); + Ok((fee, Default::default())) } else { <::OnChargeTransaction as OnChargeTransaction>::withdraw_fee(who, call, info, fee, tip) .map(|i| (fee, i)) diff --git a/frame/transaction-payment/src/payment.rs b/frame/transaction-payment/src/payment.rs index 9c94ce097ccc6..2b36e9cc4e6e6 100644 --- a/frame/transaction-payment/src/payment.rs +++ b/frame/transaction-payment/src/payment.rs @@ -15,7 +15,8 @@ use sp_std::{fmt::Debug, marker::PhantomData}; pub trait OnChargeTransaction { type LiquidityInfo: Default; - /// Before the transaction is executed the payment of the transaction fees need to be secured. + /// Before the transaction is executed the payment of the transaction fees + /// need to be secured. fn withdraw_fee( who: &T::AccountId, call: &T::Call, @@ -24,7 +25,8 @@ pub trait OnChargeTransaction { tip: T::Balance, ) -> Result; - /// After the transaction was executed and the correct fees where collected, the resulting funds can be spend here. + /// After the transaction was executed and the correct fees where collected, + /// the resulting funds can be spend here. fn deposit_fee( who: &T::AccountId, dispatch_info: &DispatchInfoOf, @@ -35,8 +37,9 @@ pub trait OnChargeTransaction { ) -> Result<(), TransactionValidityError>; } -/// Implements the transaction payment for a module implementing the `Currency` trait (eg. the pallet_balances) using an -/// unbalance handler ( implementing `OnUnbalanced`). +/// Implements the transaction payment for a module implementing the `Currency` +/// trait (eg. the pallet_balances) using an unbalance handler (implementing +/// `OnUnbalanced`). pub struct CurrencyAdapter(PhantomData, PhantomData); /// Default implementation for a Currency and an OnUnbalanced handler. @@ -78,8 +81,9 @@ where } } - /// Hand the fee and the tip over to the `[OnUnbalanced]` implementation. Since the predicted fee might have been - /// too high, parts of the fee may be refunded. + /// Hand the fee and the tip over to the `[OnUnbalanced]` implementation. + /// Since the predicted fee might have been too high, parts of the fee may + /// be refunded. fn deposit_fee( who: &T::AccountId, _dispatch_info: &DispatchInfoOf, @@ -91,8 +95,9 @@ where if let Some(paid) = liquidity_info { // Calculate how much refund we should return let refund_amount = paid.peek().saturating_sub(fee); - // refund to the the account that paid the fees. If this fails, the account might have dropped below the - // existential balance. In that case we don't refund anything. + // refund to the the account that paid the fees. If this fails, the + // account might have dropped below the existential balance. In + // that case we don't refund anything. let refund_imbalance = C::deposit_into_existing(&who, refund_amount).unwrap_or_else(|_| C::PositiveImbalance::zero()); // merge the imbalance caused by paying the fees and refunding parts of it again. From d8713c1e2f5b1d413c54f84a491b800e1e00162c Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Fri, 2 Oct 2020 09:26:12 +0200 Subject: [PATCH 13/23] readd BalanceOf --- frame/transaction-payment/src/lib.rs | 50 +++++++++++++++------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 479e219a38535..c35e5b8a0de4e 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -62,6 +62,8 @@ pub use payment::*; /// Fee multiplier. pub type Multiplier = FixedU128; +type BalanceOf = + ::Balance; type NegativeImbalanceOf = ::AccountId>>::NegativeImbalance; @@ -233,10 +235,10 @@ pub trait Trait: frame_system::Trait { type OnChargeTransaction: OnChargeTransaction; /// The fee to be paid for making a transaction; the per-byte portion. - type TransactionByteFee: Get; + type TransactionByteFee: Get>; /// Convert a weight value into a deductible fee based on the currency type. - type WeightToFee: WeightToFeePolynomial; + type WeightToFee: WeightToFeePolynomial>; /// Update the multiplier of the next block, based on the previous block's weight. type FeeMultiplierUpdate: MultiplierUpdate; @@ -253,10 +255,10 @@ decl_storage! { decl_module! { pub struct Module for enum Call where origin: T::Origin { /// The fee to be paid for making a transaction; the per-byte portion. - const TransactionByteFee: T::Balance = T::TransactionByteFee::get(); + const TransactionByteFee: BalanceOf = T::TransactionByteFee::get(); /// The polynomial that is applied in order to derive fee from weight. - const WeightToFee: Vec> = + const WeightToFee: Vec>> = T::WeightToFee::polynomial().to_vec(); fn on_finalize() { @@ -308,7 +310,7 @@ decl_module! { impl Module where - T::Balance: FixedPointOperand, + BalanceOf: FixedPointOperand, { /// Query the data that we know about the fee of a given `call`. /// @@ -321,10 +323,10 @@ where pub fn query_info( unchecked_extrinsic: Extrinsic, len: u32, - ) -> RuntimeDispatchInfo + ) -> RuntimeDispatchInfo> where T: Send + Sync, - T::Balance: Send + Sync, + BalanceOf: Send + Sync, T::Call: Dispatchable, { // NOTE: we can actually make it understand `ChargeTransactionPayment`, but would be some @@ -361,7 +363,7 @@ where /// inclusion_fee = base_fee + len_fee + [targeted_fee_adjustment * weight_fee]; /// final_fee = inclusion_fee + tip; /// ``` - pub fn compute_fee(len: u32, info: &DispatchInfoOf, tip: T::Balance) -> T::Balance + pub fn compute_fee(len: u32, info: &DispatchInfoOf, tip: BalanceOf) -> BalanceOf where T::Call: Dispatchable, { @@ -376,17 +378,17 @@ where len: u32, info: &DispatchInfoOf, post_info: &PostDispatchInfoOf, - tip: T::Balance, - ) -> T::Balance + tip: BalanceOf, + ) -> BalanceOf where T::Call: Dispatchable, { Self::compute_fee_raw(len, post_info.calc_actual_weight(info), tip, post_info.pays_fee(info)) } - fn compute_fee_raw(len: u32, weight: Weight, tip: T::Balance, pays_fee: Pays) -> T::Balance { + fn compute_fee_raw(len: u32, weight: Weight, tip: BalanceOf, pays_fee: Pays) -> BalanceOf { if pays_fee == Pays::Yes { - let len = ::from(len); + let len = >::from(len); let per_byte = T::TransactionByteFee::get(); // length fee. this is not adjusted. @@ -408,7 +410,7 @@ where } } - fn weight_to_fee(weight: Weight) -> T::Balance { + fn weight_to_fee(weight: Weight) -> BalanceOf { // cap the weight to the maximum defined in runtime, otherwise it will be the // `Bounded` maximum of its data type, which is not desired. let capped_weight = weight.min(::MaximumBlockWeight::get()); @@ -416,17 +418,17 @@ where } } -impl Convert for Module +impl Convert> for Module where T: Trait, - T::Balance: FixedPointOperand, + BalanceOf: FixedPointOperand, { /// Compute the fee for the specified weight. /// /// This fee is already adjusted by the per block fee adjustment factor and is therefore the /// share that the weight contributes to the overall fee of a transaction. It is mainly /// for informational purposes and not used in the actual fee calculation. - fn convert(weight: Weight) -> T::Balance { + fn convert(weight: Weight) -> BalanceOf { NextFeeMultiplier::get().saturating_mul_int(Self::weight_to_fee(weight)) } } @@ -434,15 +436,15 @@ where /// Require the transactor pay for themselves and maybe include a tip to gain additional priority /// in the queue. #[derive(Encode, Decode, Clone, Eq, PartialEq)] -pub struct ChargeTransactionPayment(#[codec(compact)] T::Balance); +pub struct ChargeTransactionPayment(#[codec(compact)] BalanceOf); impl ChargeTransactionPayment where T::Call: Dispatchable, - T::Balance: Send + Sync + FixedPointOperand, + BalanceOf: Send + Sync + FixedPointOperand, { /// utility constructor. Used only in client/factory code. - pub fn from(fee: T::Balance) -> Self { + pub fn from(fee: BalanceOf) -> Self { Self(fee) } @@ -454,7 +456,7 @@ where len: usize, ) -> Result< ( - T::Balance, + BalanceOf, <::OnChargeTransaction as OnChargeTransaction>::LiquidityInfo, ), TransactionValidityError, @@ -481,10 +483,10 @@ where /// and the entire block weight `(1/1)`, its priority is `fee * min(1, 4) = fee * 1`. This means /// that the transaction which consumes more resources (either length or weight) with the same /// `fee` ends up having lower priority. - fn get_priority(len: usize, info: &DispatchInfoOf, final_fee: ::Balance) -> TransactionPriority { + fn get_priority(len: usize, info: &DispatchInfoOf, final_fee: BalanceOf) -> TransactionPriority { let weight_saturation = T::MaximumBlockWeight::get() / info.weight.max(1); let len_saturation = T::MaximumBlockLength::get() as u64 / (len as u64).max(1); - let coefficient: ::Balance = weight_saturation.min(len_saturation).saturated_into::<::Balance>(); + let coefficient: BalanceOf = weight_saturation.min(len_saturation).saturated_into::>(); final_fee.saturating_mul(coefficient).saturated_into::() } } @@ -502,7 +504,7 @@ impl sp_std::fmt::Debug for ChargeTransactionPayment impl SignedExtension for ChargeTransactionPayment where - T::Balance: Send + Sync + From + FixedPointOperand, + BalanceOf: Send + Sync + From + FixedPointOperand, T::Call: Dispatchable, { const IDENTIFIER: &'static str = "ChargeTransactionPayment"; @@ -511,7 +513,7 @@ where type AdditionalSigned = (); type Pre = ( // tip - T::Balance, + BalanceOf, // who paid the fee Self::AccountId, // imbalance resulting from withdrawing the fee From bfff36dfda57640bdf445a1367a0cf3ae9efd60b Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Fri, 2 Oct 2020 13:47:24 +0200 Subject: [PATCH 14/23] some clarifications and readability improvements --- frame/transaction-payment/src/payment.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/frame/transaction-payment/src/payment.rs b/frame/transaction-payment/src/payment.rs index 2b36e9cc4e6e6..58bd1e9e22c3a 100644 --- a/frame/transaction-payment/src/payment.rs +++ b/frame/transaction-payment/src/payment.rs @@ -56,6 +56,8 @@ where type LiquidityInfo = Option>; /// Withdraw the predicted fee from the transaction origin. + /// + /// Note: The `fee` already includes the `tip`. fn withdraw_fee( who: &T::AccountId, _call: &T::Call, @@ -66,14 +68,17 @@ where if fee.is_zero() { return Ok(None); } + + let withdraw_reason = if tip.is_zero() { + WithdrawReason::TransactionPayment.into() + } else { + WithdrawReason::TransactionPayment | WithdrawReason::Tip + }; + match C::withdraw( who, fee, - if tip.is_zero() { - WithdrawReason::TransactionPayment.into() - } else { - WithdrawReason::TransactionPayment | WithdrawReason::Tip - }, + withdraw_reason, ExistenceRequirement::KeepAlive, ) { Ok(imbalance) => Ok(Some(imbalance)), @@ -84,6 +89,8 @@ where /// Hand the fee and the tip over to the `[OnUnbalanced]` implementation. /// Since the predicted fee might have been too high, parts of the fee may /// be refunded. + /// + /// Note: The `fee` already includes the `tip`. fn deposit_fee( who: &T::AccountId, _dispatch_info: &DispatchInfoOf, From 88a48d06d267f7a04e6b8b6e87df7ad3bedef7cb Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Fri, 2 Oct 2020 15:13:42 +0200 Subject: [PATCH 15/23] move balance type to OnChargeTransaction --- bin/node-template/runtime/src/lib.rs | 1 - bin/node/runtime/src/lib.rs | 1 - frame/balances/src/tests_composite.rs | 1 - frame/balances/src/tests_local.rs | 1 - frame/executive/src/lib.rs | 1 - frame/transaction-payment/src/lib.rs | 19 +++---------- frame/transaction-payment/src/payment.rs | 36 +++++++++++++----------- 7 files changed, 24 insertions(+), 36 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 9359264177b62..6898d100f81f1 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -250,7 +250,6 @@ parameter_types! { } impl pallet_transaction_payment::Trait for Runtime { - type Balance = Balance; type OnChargeTransaction = CurrencyAdapter; type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 3af31c958f80f..836fdae75d6af 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -359,7 +359,6 @@ parameter_types! { } impl pallet_transaction_payment::Trait for Runtime { - type Balance = Balance; type OnChargeTransaction = CurrencyAdapter; type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; diff --git a/frame/balances/src/tests_composite.rs b/frame/balances/src/tests_composite.rs index 9f4f78597aa08..88b73b47273eb 100644 --- a/frame/balances/src/tests_composite.rs +++ b/frame/balances/src/tests_composite.rs @@ -98,7 +98,6 @@ parameter_types! { pub const TransactionByteFee: u64 = 1; } impl pallet_transaction_payment::Trait for Test { - type Balance = u64; type OnChargeTransaction = CurrencyAdapter, ()>; type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; diff --git a/frame/balances/src/tests_local.rs b/frame/balances/src/tests_local.rs index 7971fbef6f771..319fb3640b4c7 100644 --- a/frame/balances/src/tests_local.rs +++ b/frame/balances/src/tests_local.rs @@ -98,7 +98,6 @@ parameter_types! { pub const TransactionByteFee: u64 = 1; } impl pallet_transaction_payment::Trait for Test { - type Balance = u64; type OnChargeTransaction = CurrencyAdapter, ()>; type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 2d4b1d30da616..9f4adb7b8fa47 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -633,7 +633,6 @@ mod tests { pub const TransactionByteFee: Balance = 0; } impl pallet_transaction_payment::Trait for Runtime { - type Balance = Balance; type OnChargeTransaction = CurrencyAdapter; type TransactionByteFee = TransactionByteFee; type WeightToFee = IdentityFee; diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index c35e5b8a0de4e..cca2dda6883f2 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -33,8 +33,8 @@ #![cfg_attr(not(feature = "std"), no_std)] -use sp_std::{prelude::*, fmt::Debug}; -use codec::{Encode, Decode, FullCodec}; +use sp_std::prelude::*; +use codec::{Encode, Decode}; use frame_support::{ decl_storage, decl_module, traits::{Currency, Get}, @@ -51,7 +51,7 @@ use sp_runtime::{ }, traits::{ Zero, Saturating, SignedExtension, SaturatedConversion, Convert, Dispatchable, - DispatchInfoOf, PostDispatchInfoOf, AtLeast32BitUnsigned, MaybeSerializeDeserialize, + DispatchInfoOf, PostDispatchInfoOf, }, }; use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo; @@ -63,7 +63,7 @@ pub use payment::*; pub type Multiplier = FixedU128; type BalanceOf = - ::Balance; + <::OnChargeTransaction as OnChargeTransaction>::Balance; type NegativeImbalanceOf = ::AccountId>>::NegativeImbalance; @@ -216,16 +216,6 @@ impl Default for Releases { } pub trait Trait: frame_system::Trait { - /// The currency type in which fees will be paid. - type Balance: AtLeast32BitUnsigned - + FullCodec - + Copy - + MaybeSerializeDeserialize - + Debug - + Default - + Send - + Sync; - /// Handler for withdrawing, refunding and depositing the transaction fee. /// Transaction fees are withdrawen before the transaction is executed. /// After the transaction was executed the transaction weight can be @@ -696,7 +686,6 @@ mod tests { } impl Trait for Runtime { - type Balance = u64; type OnChargeTransaction = CurrencyAdapter; type TransactionByteFee = TransactionByteFee; type WeightToFee = WeightToFee; diff --git a/frame/transaction-payment/src/payment.rs b/frame/transaction-payment/src/payment.rs index 58bd1e9e22c3a..14e2a1c2ae803 100644 --- a/frame/transaction-payment/src/payment.rs +++ b/frame/transaction-payment/src/payment.rs @@ -6,13 +6,15 @@ use frame_support::{ unsigned::TransactionValidityError, }; use sp_runtime::{ - traits::{AtLeast32BitUnsigned, DispatchInfoOf, MaybeSerializeDeserialize, PostDispatchInfoOf}, + traits::{AtLeast32BitUnsigned, DispatchInfoOf, MaybeSerializeDeserialize, PostDispatchInfoOf, Saturating, Zero}, transaction_validity::InvalidTransaction, }; use sp_std::{fmt::Debug, marker::PhantomData}; /// Handle withdrawing, refunding and depositing of transaction fees. pub trait OnChargeTransaction { + /// The currency type in which fees will be paid. + type Balance: AtLeast32BitUnsigned + FullCodec + Copy + MaybeSerializeDeserialize + Debug + Default; type LiquidityInfo: Default; /// Before the transaction is executed the payment of the transaction fees @@ -21,8 +23,8 @@ pub trait OnChargeTransaction { who: &T::AccountId, call: &T::Call, dispatch_info: &DispatchInfoOf, - fee: T::Balance, - tip: T::Balance, + fee: Self::Balance, + tip: Self::Balance, ) -> Result; /// After the transaction was executed and the correct fees where collected, @@ -31,8 +33,8 @@ pub trait OnChargeTransaction { who: &T::AccountId, dispatch_info: &DispatchInfoOf, post_info: &PostDispatchInfoOf, - fee: T::Balance, - tip: T::Balance, + fee: Self::Balance, + tip: Self::Balance, liquidity_info: Self::LiquidityInfo, ) -> Result<(), TransactionValidityError>; } @@ -43,17 +45,19 @@ pub trait OnChargeTransaction { pub struct CurrencyAdapter(PhantomData, PhantomData); /// Default implementation for a Currency and an OnUnbalanced handler. -impl OnChargeTransaction for CurrencyAdapter +impl OnChargeTransaction for CurrencyAdapter where - B: AtLeast32BitUnsigned + FullCodec + Copy + MaybeSerializeDeserialize + Debug + Default + Send + Sync, - T: Trait, - T::TransactionByteFee: Get, - C: Currency<::AccountId, Balance = B>, - C::PositiveImbalance: Imbalance, - C::NegativeImbalance: Imbalance, + T: Trait, + T::TransactionByteFee: Get<::AccountId>>::Balance>, + C: Currency<::AccountId>, + C::PositiveImbalance: + Imbalance<::AccountId>>::Balance, Opposite = C::NegativeImbalance>, + C::NegativeImbalance: + Imbalance<::AccountId>>::Balance, Opposite = C::PositiveImbalance>, OU: OnUnbalanced>, { type LiquidityInfo = Option>; + type Balance = ::AccountId>>::Balance; /// Withdraw the predicted fee from the transaction origin. /// @@ -62,8 +66,8 @@ where who: &T::AccountId, _call: &T::Call, _info: &DispatchInfoOf, - fee: B, - tip: B, + fee: Self::Balance, + tip: Self::Balance, ) -> Result { if fee.is_zero() { return Ok(None); @@ -95,8 +99,8 @@ where who: &T::AccountId, _dispatch_info: &DispatchInfoOf, _post_info: &PostDispatchInfoOf, - fee: B, - tip: B, + fee: Self::Balance, + tip: Self::Balance, liquidity_info: Self::LiquidityInfo, ) -> Result<(), TransactionValidityError> { if let Some(paid) = liquidity_info { From b1fdb6cd859779f485ddeccd8ba34583a1f71049 Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Fri, 2 Oct 2020 15:22:10 +0200 Subject: [PATCH 16/23] remove noise --- frame/transaction-payment/src/lib.rs | 45 ++++++++++++++-------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index cca2dda6883f2..c240168af4b44 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -228,7 +228,7 @@ pub trait Trait: frame_system::Trait { type TransactionByteFee: Get>; /// Convert a weight value into a deductible fee based on the currency type. - type WeightToFee: WeightToFeePolynomial>; + type WeightToFee: WeightToFeePolynomial>; /// Update the multiplier of the next block, based on the previous block's weight. type FeeMultiplierUpdate: MultiplierUpdate; @@ -298,9 +298,8 @@ decl_module! { } } -impl Module -where - BalanceOf: FixedPointOperand, +impl Module where + BalanceOf: FixedPointOperand { /// Query the data that we know about the fee of a given `call`. /// @@ -353,9 +352,12 @@ where /// inclusion_fee = base_fee + len_fee + [targeted_fee_adjustment * weight_fee]; /// final_fee = inclusion_fee + tip; /// ``` - pub fn compute_fee(len: u32, info: &DispatchInfoOf, tip: BalanceOf) -> BalanceOf - where - T::Call: Dispatchable, + pub fn compute_fee( + len: u32, + info: &DispatchInfoOf, + tip: BalanceOf, + ) -> BalanceOf where + T::Call: Dispatchable, { Self::compute_fee_raw(len, info.weight, tip, info.pays_fee) } @@ -369,14 +371,18 @@ where info: &DispatchInfoOf, post_info: &PostDispatchInfoOf, tip: BalanceOf, - ) -> BalanceOf - where - T::Call: Dispatchable, + ) -> BalanceOf where + T::Call: Dispatchable, { Self::compute_fee_raw(len, post_info.calc_actual_weight(info), tip, post_info.pays_fee(info)) } - fn compute_fee_raw(len: u32, weight: Weight, tip: BalanceOf, pays_fee: Pays) -> BalanceOf { + fn compute_fee_raw( + len: u32, + weight: Weight, + tip: BalanceOf, + pays_fee: Pays, + ) -> BalanceOf { if pays_fee == Pays::Yes { let len = >::from(len); let per_byte = T::TransactionByteFee::get(); @@ -408,8 +414,7 @@ where } } -impl Convert> for Module -where +impl Convert> for Module where T: Trait, BalanceOf: FixedPointOperand, { @@ -428,9 +433,8 @@ where #[derive(Encode, Decode, Clone, Eq, PartialEq)] pub struct ChargeTransactionPayment(#[codec(compact)] BalanceOf); -impl ChargeTransactionPayment -where - T::Call: Dispatchable, +impl ChargeTransactionPayment where + T::Call: Dispatchable, BalanceOf: Send + Sync + FixedPointOperand, { /// utility constructor. Used only in client/factory code. @@ -492,10 +496,9 @@ impl sp_std::fmt::Debug for ChargeTransactionPayment } } -impl SignedExtension for ChargeTransactionPayment -where +impl SignedExtension for ChargeTransactionPayment where BalanceOf: Send + Sync + From + FixedPointOperand, - T::Call: Dispatchable, + T::Call: Dispatchable, { const IDENTIFIER: &'static str = "ChargeTransactionPayment"; type AccountId = T::AccountId; @@ -509,9 +512,7 @@ where // imbalance resulting from withdrawing the fee <::OnChargeTransaction as OnChargeTransaction>::LiquidityInfo, ); - fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> { - Ok(()) - } + fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> { Ok(()) } fn validate( &self, From 1794b5f06e9e9f47358f82cdfc94b444daaaeaf6 Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Fri, 2 Oct 2020 15:23:43 +0200 Subject: [PATCH 17/23] fix style --- frame/transaction-payment/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index c240168af4b44..bf2898f54ce9b 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -372,7 +372,7 @@ impl Module where post_info: &PostDispatchInfoOf, tip: BalanceOf, ) -> BalanceOf where - T::Call: Dispatchable, + T::Call: Dispatchable, { Self::compute_fee_raw(len, post_info.calc_actual_weight(info), tip, post_info.pays_fee(info)) } From ada1d062cf46c3864ec69235a280126cffa5ddf6 Mon Sep 17 00:00:00 2001 From: Albrecht <14820950+weichweich@users.noreply.github.com> Date: Mon, 5 Oct 2020 13:14:15 +0200 Subject: [PATCH 18/23] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit improved documentation Co-authored-by: Alexander Theißen --- frame/transaction-payment/src/lib.rs | 2 +- frame/transaction-payment/src/payment.rs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index bf2898f54ce9b..d583b5fbc720e 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -217,7 +217,7 @@ impl Default for Releases { pub trait Trait: frame_system::Trait { /// Handler for withdrawing, refunding and depositing the transaction fee. - /// Transaction fees are withdrawen before the transaction is executed. + /// Transaction fees are withdrawn before the transaction is executed. /// After the transaction was executed the transaction weight can be /// adjusted, depending on the used resources by the transaction. If the /// transaction weight is lower than expected, parts of the transaction fee diff --git a/frame/transaction-payment/src/payment.rs b/frame/transaction-payment/src/payment.rs index 14e2a1c2ae803..6a9b021a1465e 100644 --- a/frame/transaction-payment/src/payment.rs +++ b/frame/transaction-payment/src/payment.rs @@ -13,12 +13,14 @@ use sp_std::{fmt::Debug, marker::PhantomData}; /// Handle withdrawing, refunding and depositing of transaction fees. pub trait OnChargeTransaction { - /// The currency type in which fees will be paid. + /// The underlying integer type in which fees are calculated. type Balance: AtLeast32BitUnsigned + FullCodec + Copy + MaybeSerializeDeserialize + Debug + Default; type LiquidityInfo: Default; /// Before the transaction is executed the payment of the transaction fees /// need to be secured. + /// + /// Note: The `fee` already includes the `tip`. fn withdraw_fee( who: &T::AccountId, call: &T::Call, From 623b449e2ab1fcb7bd5c7fa1d871e99fdf0cff01 Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Mon, 5 Oct 2020 13:25:27 +0200 Subject: [PATCH 19/23] Improve naming and documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply suggestions from code review Co-authored-by: Alexander Theißen --- frame/transaction-payment/src/lib.rs | 2 +- frame/transaction-payment/src/payment.rs | 30 +++++++++++------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index d583b5fbc720e..1ce4984b7f836 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -553,7 +553,7 @@ impl SignedExtension for ChargeTransactionPayment whe post_info, tip, ); - T::OnChargeTransaction::deposit_fee(&who, info, post_info, actual_fee, tip, imbalance)?; + T::OnChargeTransaction::correct_and_deposit_fee(&who, info, post_info, actual_fee, tip, imbalance)?; Ok(()) } } diff --git a/frame/transaction-payment/src/payment.rs b/frame/transaction-payment/src/payment.rs index 6a9b021a1465e..528c7c153f27d 100644 --- a/frame/transaction-payment/src/payment.rs +++ b/frame/transaction-payment/src/payment.rs @@ -29,15 +29,18 @@ pub trait OnChargeTransaction { tip: Self::Balance, ) -> Result; - /// After the transaction was executed and the correct fees where collected, - /// the resulting funds can be spend here. - fn deposit_fee( + /// After the transaction was executed the actual fee can be calculated. + /// This function should refund any overpaid fees and optionally deposit + /// the corrected amount. + /// + /// Note: The `fee` already includes the `tip`. + fn correct_and_deposit_fee( who: &T::AccountId, dispatch_info: &DispatchInfoOf, post_info: &PostDispatchInfoOf, - fee: Self::Balance, + corrected_fee: Self::Balance, tip: Self::Balance, - liquidity_info: Self::LiquidityInfo, + already_withdrawn: Self::LiquidityInfo, ) -> Result<(), TransactionValidityError>; } @@ -81,12 +84,7 @@ where WithdrawReason::TransactionPayment | WithdrawReason::Tip }; - match C::withdraw( - who, - fee, - withdraw_reason, - ExistenceRequirement::KeepAlive, - ) { + match C::withdraw(who, fee, withdraw_reason, ExistenceRequirement::KeepAlive) { Ok(imbalance) => Ok(Some(imbalance)), Err(_) => Err(InvalidTransaction::Payment.into()), } @@ -97,17 +95,17 @@ where /// be refunded. /// /// Note: The `fee` already includes the `tip`. - fn deposit_fee( + fn correct_and_deposit_fee( who: &T::AccountId, _dispatch_info: &DispatchInfoOf, _post_info: &PostDispatchInfoOf, - fee: Self::Balance, + corrected_fee: Self::Balance, tip: Self::Balance, - liquidity_info: Self::LiquidityInfo, + already_withdrawn: Self::LiquidityInfo, ) -> Result<(), TransactionValidityError> { - if let Some(paid) = liquidity_info { + if let Some(paid) = already_withdrawn { // Calculate how much refund we should return - let refund_amount = paid.peek().saturating_sub(fee); + let refund_amount = paid.peek().saturating_sub(corrected_fee); // refund to the the account that paid the fees. If this fails, the // account might have dropped below the existential balance. In // that case we don't refund anything. From 7bad69c33b271b2305da1cba47e26cd9b19233bc Mon Sep 17 00:00:00 2001 From: Albrecht <14820950+weichweich@users.noreply.github.com> Date: Mon, 12 Oct 2020 21:39:59 +0200 Subject: [PATCH 20/23] Apply suggestions from code review Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- frame/transaction-payment/src/payment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/transaction-payment/src/payment.rs b/frame/transaction-payment/src/payment.rs index 528c7c153f27d..20e2883d4ed2e 100644 --- a/frame/transaction-payment/src/payment.rs +++ b/frame/transaction-payment/src/payment.rs @@ -47,7 +47,7 @@ pub trait OnChargeTransaction { /// Implements the transaction payment for a module implementing the `Currency` /// trait (eg. the pallet_balances) using an unbalance handler (implementing /// `OnUnbalanced`). -pub struct CurrencyAdapter(PhantomData, PhantomData); +pub struct CurrencyAdapter(PhantomData<(C, OU)>); /// Default implementation for a Currency and an OnUnbalanced handler. impl OnChargeTransaction for CurrencyAdapter From 2367a3edfed1e5c4e53aed4df222590619cde6f9 Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Fri, 30 Oct 2020 09:04:59 +0100 Subject: [PATCH 21/23] always call withdraw_fee --- frame/transaction-payment/src/lib.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 1ce4984b7f836..3dbb93e15f653 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -50,7 +50,7 @@ use sp_runtime::{ TransactionPriority, ValidTransaction, TransactionValidityError, TransactionValidity, }, traits::{ - Zero, Saturating, SignedExtension, SaturatedConversion, Convert, Dispatchable, + Saturating, SignedExtension, SaturatedConversion, Convert, Dispatchable, DispatchInfoOf, PostDispatchInfoOf, }, }; @@ -458,13 +458,8 @@ impl ChargeTransactionPayment where let tip = self.0; let fee = Module::::compute_fee(len as u32, info, tip); - // Only mess with balances if fee is not zero. - if fee.is_zero() { - Ok((fee, Default::default())) - } else { - <::OnChargeTransaction as OnChargeTransaction>::withdraw_fee(who, call, info, fee, tip) - .map(|i| (fee, i)) - } + <::OnChargeTransaction as OnChargeTransaction>::withdraw_fee(who, call, info, fee, tip) + .map(|i| (fee, i)) } /// Get an appropriate priority for a transaction with the given length and info. From e5f1c64db69871c6e5f89c438bf7aa548afd7a5b Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Fri, 30 Oct 2020 09:16:52 +0100 Subject: [PATCH 22/23] move NegativeImbalanceOf to payment module --- frame/transaction-payment/src/lib.rs | 2 -- frame/transaction-payment/src/payment.rs | 5 ++++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 3dbb93e15f653..b90baa9e50293 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -64,8 +64,6 @@ pub type Multiplier = FixedU128; type BalanceOf = <::OnChargeTransaction as OnChargeTransaction>::Balance; -type NegativeImbalanceOf = - ::AccountId>>::NegativeImbalance; /// A struct to update the weight multiplier per block. It implements `Convert`, meaning that it can convert the previous multiplier to the next one. This should diff --git a/frame/transaction-payment/src/payment.rs b/frame/transaction-payment/src/payment.rs index 5d992f528ed73..de39215b575be 100644 --- a/frame/transaction-payment/src/payment.rs +++ b/frame/transaction-payment/src/payment.rs @@ -1,5 +1,5 @@ ///! Traits and default implementation for paying transaction fees. -use crate::{NegativeImbalanceOf, Trait}; +use crate::Trait; use codec::FullCodec; use frame_support::{ traits::{Currency, ExistenceRequirement, Get, Imbalance, OnUnbalanced, WithdrawReasons}, @@ -11,6 +11,9 @@ use sp_runtime::{ }; use sp_std::{fmt::Debug, marker::PhantomData}; +type NegativeImbalanceOf = + ::AccountId>>::NegativeImbalance; + /// Handle withdrawing, refunding and depositing of transaction fees. pub trait OnChargeTransaction { /// The underlying integer type in which fees are calculated. From 1b73d2baba7b76772676b5ab63a9412770590cd3 Mon Sep 17 00:00:00 2001 From: weichweich <14820950+weichweich@users.noreply.github.com> Date: Fri, 30 Oct 2020 09:47:16 +0100 Subject: [PATCH 23/23] fix unused import --- frame/transaction-payment/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index b90baa9e50293..dd310c2639842 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -37,7 +37,7 @@ use sp_std::prelude::*; use codec::{Encode, Decode}; use frame_support::{ decl_storage, decl_module, - traits::{Currency, Get}, + traits::Get, weights::{ Weight, DispatchInfo, PostDispatchInfo, GetDispatchInfo, Pays, WeightToFeePolynomial, WeightToFeeCoefficient, @@ -561,6 +561,7 @@ mod tests { DispatchClass, DispatchInfo, PostDispatchInfo, GetDispatchInfo, Weight, WeightToFeePolynomial, WeightToFeeCoefficients, WeightToFeeCoefficient, }, + traits::Currency, }; use pallet_balances::Call as BalancesCall; use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo;