From 55f2072437504f855b230d80392ac9587cdcfc56 Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 16 May 2024 17:18:31 +0200 Subject: [PATCH 01/16] refactoring and swap credit --- .../asset-conversion-tx-payment/src/lib.rs | 173 ++++------- .../asset-conversion-tx-payment/src/mock.rs | 32 +- .../src/payment.rs | 276 +++++++++++------- .../asset-conversion-tx-payment/src/tests.rs | 34 +-- .../frame/transaction-payment/src/lib.rs | 5 + 5 files changed, 268 insertions(+), 252 deletions(-) diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs index ed0ed56e6e07..4616cfc0991c 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs @@ -23,7 +23,7 @@ //! This pallet provides a `SignedExtension` with an optional `AssetId` that specifies the asset //! to be used for payment (defaulting to the native token on `None`). It expects an //! [`OnChargeAssetTransaction`] implementation analogous to [`pallet-transaction-payment`]. The -//! included [`AssetConversionAdapter`] (implementing [`OnChargeAssetTransaction`]) determines the +//! included [`SwapCreditAdapter`] (implementing [`OnChargeAssetTransaction`]) determines the //! fee amount by converting the fee calculated by [`pallet-transaction-payment`] in the native //! asset into the amount required of the specified asset. //! @@ -47,19 +47,14 @@ use sp_std::prelude::*; use codec::{Decode, Encode}; use frame_support::{ dispatch::{DispatchInfo, DispatchResult, PostDispatchInfo}, - traits::{ - fungibles::{Balanced, Inspect}, - IsType, - }, + traits::IsType, DefaultNoBound, }; use pallet_transaction_payment::OnChargeTransaction; use scale_info::TypeInfo; use sp_runtime::{ traits::{DispatchInfoOf, Dispatchable, PostDispatchInfoOf, SignedExtension, Zero}, - transaction_validity::{ - InvalidTransaction, TransactionValidity, TransactionValidityError, ValidTransaction, - }, + transaction_validity::{TransactionValidity, TransactionValidityError, ValidTransaction}, }; #[cfg(test)] @@ -71,30 +66,23 @@ mod payment; use frame_support::traits::tokens::AssetId; pub use payment::*; +/// Type alias for Asset IDs in their interaction with `OnChargeAssetTransaction`. +pub(crate) type AssetIdOf = + <::OnChargeAssetTransaction as OnChargeAssetTransaction>::AssetId; + +/// Balance type alias for balances of the chain's native asset. +pub(crate) type BalanceOf = as OnChargeTransaction>::Balance; + /// Type aliases used for interaction with `OnChargeTransaction`. pub(crate) type OnChargeTransactionOf = ::OnChargeTransaction; -/// Balance type alias for balances of the chain's native asset. -pub(crate) type BalanceOf = as OnChargeTransaction>::Balance; -/// Liquidity info type alias. -pub(crate) type LiquidityInfoOf = - as OnChargeTransaction>::LiquidityInfo; -/// Balance type alias for balances of assets that implement the `fungibles` trait. -pub(crate) type AssetBalanceOf = - <::Fungibles as Inspect<::AccountId>>::Balance; -/// Type alias for Asset IDs. -pub(crate) type AssetIdOf = - <::Fungibles as Inspect<::AccountId>>::AssetId; +/// Liquidity info type alias for the chain's native asset. +pub(crate) type NativeLiquidityInfoOf = + as OnChargeTransaction>::LiquidityInfo; -/// Type alias for the interaction of balances with `OnChargeAssetTransaction`. -pub(crate) type ChargeAssetBalanceOf = - <::OnChargeAssetTransaction as OnChargeAssetTransaction>::Balance; -/// Type alias for Asset IDs in their interaction with `OnChargeAssetTransaction`. -pub(crate) type ChargeAssetIdOf = - <::OnChargeAssetTransaction as OnChargeAssetTransaction>::AssetId; -/// Liquidity info type alias for interaction with `OnChargeAssetTransaction`. -pub(crate) type ChargeAssetLiquidityOf = +/// Liquidity info type alias for the chain's assets. +pub(crate) type AssetLiquidityInfoOf = <::OnChargeAssetTransaction as OnChargeAssetTransaction>::LiquidityInfo; /// Used to pass the initial payment info from pre- to post-dispatch. @@ -104,9 +92,9 @@ pub enum InitialPayment { #[default] Nothing, /// The initial fee was paid in the native currency. - Native(LiquidityInfoOf), + Native(NativeLiquidityInfoOf), /// The initial fee was paid in an asset. - Asset((LiquidityInfoOf, BalanceOf, AssetBalanceOf)), + Asset((AssetIdOf, AssetLiquidityInfoOf)), } pub use pallet::*; @@ -116,15 +104,11 @@ pub mod pallet { use super::*; #[pallet::config] - pub trait Config: - frame_system::Config + pallet_transaction_payment::Config + pallet_asset_conversion::Config - { + pub trait Config: frame_system::Config + pallet_transaction_payment::Config { /// The overarching event type. type RuntimeEvent: From> + IsType<::RuntimeEvent>; - /// The fungibles instance used to pay for transactions in assets. - type Fungibles: Balanced; /// The actual transaction charging logic that charges the fees. - type OnChargeAssetTransaction: OnChargeAssetTransaction; + type OnChargeAssetTransaction: OnChargeAssetTransaction>; } #[pallet::pallet] @@ -137,9 +121,9 @@ pub mod pallet { /// has been paid by `who` in an asset `asset_id`. AssetTxFeePaid { who: T::AccountId, - actual_fee: AssetBalanceOf, + actual_fee: BalanceOf, tip: BalanceOf, - asset_id: ChargeAssetIdOf, + asset_id: AssetIdOf, }, /// A swap of the refund in native currency back to asset failed. AssetRefundFailed { native_amount_kept: BalanceOf }, @@ -157,23 +141,22 @@ pub mod pallet { pub struct ChargeAssetTxPayment { #[codec(compact)] tip: BalanceOf, - asset_id: Option>, + asset_id: Option>, } impl ChargeAssetTxPayment where T::RuntimeCall: Dispatchable, - AssetBalanceOf: Send + Sync, - BalanceOf: Send + Sync + Into> + From>, - ChargeAssetIdOf: Send + Sync, + BalanceOf: Send + Sync, + AssetIdOf: Send + Sync, { /// Utility constructor. Used only in client/factory code. - pub fn from(tip: BalanceOf, asset_id: Option>) -> Self { + pub fn from(tip: BalanceOf, asset_id: Option>) -> Self { Self { tip, asset_id } } - /// Fee withdrawal logic that dispatches to either `OnChargeAssetTransaction` or - /// `OnChargeTransaction`. + /// Fee withdrawal logic that dispatches to either [`Config::OnChargeAssetTransaction`] or + /// [`pallet_transaction_payment::Config::OnChargeTransaction`]. fn withdraw_fee( &self, who: &T::AccountId, @@ -191,25 +174,13 @@ where call, info, asset_id.clone(), - fee.into(), - self.tip.into(), + fee, + self.tip, ) - .map(|(used_for_fee, received_exchanged, asset_consumed)| { - ( - fee, - InitialPayment::Asset(( - used_for_fee.into(), - received_exchanged.into(), - asset_consumed.into(), - )), - ) - }) + .map(|payment| (fee, InitialPayment::Asset((asset_id.clone(), payment)))) } else { - as OnChargeTransaction>::withdraw_fee( - who, call, info, fee, self.tip, - ) - .map(|i| (fee, InitialPayment::Native(i))) - .map_err(|_| -> TransactionValidityError { InvalidTransaction::Payment.into() }) + T::OnChargeTransaction::withdraw_fee(who, call, info, fee, self.tip) + .map(|payment| (fee, InitialPayment::Native(payment))) } } } @@ -228,14 +199,8 @@ impl sp_std::fmt::Debug for ChargeAssetTxPayment { impl SignedExtension for ChargeAssetTxPayment where T::RuntimeCall: Dispatchable, - AssetBalanceOf: Send + Sync, - BalanceOf: Send - + Sync - + From - + Into> - + Into> - + From>, - ChargeAssetIdOf: Send + Sync, + BalanceOf: Send + Sync, + AssetIdOf: Send + Sync, { const IDENTIFIER: &'static str = "ChargeAssetTxPayment"; type AccountId = T::AccountId; @@ -248,11 +213,9 @@ where Self::AccountId, // imbalance resulting from withdrawing the fee InitialPayment, - // asset_id for the transaction payment - Option>, ); - fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> { + fn additional_signed(&self) -> Result<(), TransactionValidityError> { Ok(()) } @@ -277,7 +240,7 @@ where len: usize, ) -> Result { let (_fee, initial_payment) = self.withdraw_fee(who, call, info, len)?; - Ok((self.tip, who.clone(), initial_payment, self.asset_id)) + Ok((self.tip, who.clone(), initial_payment)) } fn post_dispatch( @@ -285,53 +248,45 @@ where info: &DispatchInfoOf, post_info: &PostDispatchInfoOf, len: usize, - result: &DispatchResult, + _result: &DispatchResult, ) -> Result<(), TransactionValidityError> { - if let Some((tip, who, initial_payment, asset_id)) = pre { + if let Some((tip, who, initial_payment)) = pre { match initial_payment { InitialPayment::Native(already_withdrawn) => { - debug_assert!( - asset_id.is_none(), - "For that payment type the `asset_id` should be None" + let actual_fee = pallet_transaction_payment::Pallet::::compute_actual_fee( + len as u32, info, post_info, tip, ); - pallet_transaction_payment::ChargeTransactionPayment::::post_dispatch( - Some((tip, who, already_withdrawn)), + T::OnChargeTransaction::correct_and_deposit_fee( + &who, info, post_info, - len, - result, + actual_fee, + tip, + already_withdrawn, )?; - }, - InitialPayment::Asset(already_withdrawn) => { - debug_assert!( - asset_id.is_some(), - "For that payment type the `asset_id` should be set" + pallet_transaction_payment::Pallet::::deposit_fee_paid_event( + who, actual_fee, tip, ); + }, + InitialPayment::Asset((asset_id, already_withdrawn)) => { let actual_fee = pallet_transaction_payment::Pallet::::compute_actual_fee( len as u32, info, post_info, tip, ); - - if let Some(asset_id) = asset_id { - let (used_for_fee, received_exchanged, asset_consumed) = already_withdrawn; - let converted_fee = T::OnChargeAssetTransaction::correct_and_deposit_fee( - &who, - info, - post_info, - actual_fee.into(), - tip.into(), - used_for_fee.into(), - received_exchanged.into(), - asset_id.clone(), - asset_consumed.into(), - )?; - - Pallet::::deposit_event(Event::::AssetTxFeePaid { - who, - actual_fee: converted_fee, - tip, - asset_id, - }); - } + let converted_fee = T::OnChargeAssetTransaction::correct_and_deposit_fee( + &who, + info, + post_info, + actual_fee, + tip, + asset_id.clone(), + already_withdrawn, + )?; + Pallet::::deposit_event(Event::::AssetTxFeePaid { + who, + actual_fee: converted_fee, + tip, + asset_id, + }); }, InitialPayment::Nothing => { // `actual_fee` should be zero here for any signed extrinsic. It would be diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs index 9a2b22b81709..0acce3fe620e 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs @@ -24,7 +24,7 @@ use frame_support::{ pallet_prelude::*, parameter_types, traits::{ - fungible, + fungible, fungibles, tokens::{ fungible::{NativeFromLeft, NativeOrWithId, UnionOf}, imbalance::ResolveAssetTo, @@ -156,13 +156,9 @@ parameter_types! { } pub struct DealWithFees; -impl OnUnbalanced::AccountId, Balances>> - for DealWithFees -{ +impl OnUnbalanced> for DealWithFees { fn on_unbalanceds( - mut fees_then_tips: impl Iterator< - Item = fungible::Credit<::AccountId, Balances>, - >, + mut fees_then_tips: impl Iterator>, ) { if let Some(fees) = fees_then_tips.next() { FeeUnbalancedAmount::mutate(|a| *a += fees.peek()); @@ -173,6 +169,20 @@ impl OnUnbalanced::AccountId, } } +pub struct DealWithFee; +impl OnUnbalanced> for DealWithFee { + fn on_nonzero_unbalanced(fee: fungibles::Credit) { + FeeUnbalancedAmount::mutate(|a| *a += fee.peek()); + } +} + +pub struct DealWithTip; +impl OnUnbalanced> for DealWithTip { + fn on_nonzero_unbalanced(tip: fungibles::Credit) { + TipUnbalancedAmount::mutate(|a| *a += tip.peek()); + } +} + #[derive_impl(pallet_transaction_payment::config_preludes::TestDefaultConfig)] impl pallet_transaction_payment::Config for Runtime { type RuntimeEvent = RuntimeEvent; @@ -249,12 +259,14 @@ pub type PoolIdToAccountId = pallet_asset_conversion::AccountIdConverter< (NativeOrWithId, NativeOrWithId), >; +type NativeAndAssets = UnionOf, AccountId>; + impl pallet_asset_conversion::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Balance = Balance; type HigherPrecisionBalance = u128; type AssetKind = NativeOrWithId; - type Assets = UnionOf, AccountId>; + type Assets = NativeAndAssets; type PoolId = (Self::AssetKind, Self::AssetKind); type PoolLocator = Chain< WithFirstAsset, PoolIdToAccountId>, @@ -278,6 +290,6 @@ impl pallet_asset_conversion::Config for Runtime { impl Config for Runtime { type RuntimeEvent = RuntimeEvent; - type Fungibles = Assets; - type OnChargeAssetTransaction = AssetConversionAdapter; + type OnChargeAssetTransaction = + SwapCreditAdapter; } diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs index f2f2c57bb376..a7f216387331 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs @@ -19,10 +19,14 @@ use crate::Config; use frame_support::{ ensure, - traits::{fungible::Inspect, tokens::Balance}, + traits::{ + fungibles, + tokens::{Balance, Fortitude, Precision, Preservation}, + Defensive, OnUnbalanced, SameOrOther, + }, unsigned::TransactionValidityError, }; -use pallet_asset_conversion::Swap; +use pallet_asset_conversion::{QuotePrice, SwapCredit}; use sp_runtime::{ traits::{DispatchInfoOf, Get, PostDispatchInfoOf, Zero}, transaction_validity::InvalidTransaction, @@ -49,150 +53,200 @@ pub trait OnChargeAssetTransaction { asset_id: Self::AssetId, fee: Self::Balance, tip: Self::Balance, - ) -> Result< - (LiquidityInfoOf, Self::LiquidityInfo, AssetBalanceOf), - TransactionValidityError, - >; + ) -> Result; /// Refund any overpaid fees and deposit the corrected amount. /// The actual fee gets calculated once the transaction is executed. /// /// Note: The `fee` already includes the `tip`. /// - /// Returns the fee and tip in the asset used for payment as (fee, tip). + /// Returns the amount of `asset_id` that was used for the payment. fn correct_and_deposit_fee( who: &T::AccountId, dispatch_info: &DispatchInfoOf, post_info: &PostDispatchInfoOf, corrected_fee: Self::Balance, tip: Self::Balance, - fee_paid: LiquidityInfoOf, - received_exchanged: Self::LiquidityInfo, asset_id: Self::AssetId, - initial_asset_consumed: AssetBalanceOf, - ) -> Result, TransactionValidityError>; + already_withdraw: Self::LiquidityInfo, + ) -> Result, TransactionValidityError>; } -/// Implements the asset transaction for a balance to asset converter (implementing [`Swap`]). +/// Means to withdraw, correct and deposit fees in the asset accepted by the system. /// -/// The converter is given the complete fee in terms of the asset used for the transaction. -pub struct AssetConversionAdapter(PhantomData<(C, CON, N)>); +/// The type uses the [`SwapCredit`] implementation to swap the asset used by a user for the fee +/// payment for the asset accepted as a fee payment be the system. +/// +/// Parameters: +/// - `A`: The asset identifier that system accepts as a fee payment (eg. native asset). +/// - `F`: The fungibles registry that can handle assets provided by user and the `A` asset. +/// - `S`: The swap implementation that can swap assets provided by user for the `A` asset. +/// - `OUF`: The handler for the fee payment. +/// - `OUT`: The handler for the tip payment. +/// - `T`: The pallet's configuration. +pub struct SwapAssetAdapter(PhantomData<(A, F, S, OUF, OUT)>); -/// Default implementation for a runtime instantiating this pallet, an asset to native swapper. -impl OnChargeAssetTransaction for AssetConversionAdapter +impl OnChargeAssetTransaction for SwapAssetAdapter where - N: Get, + A: Get, + F: fungibles::Balanced>, + S: SwapCredit< + T::AccountId, + Balance = BalanceOf, + AssetKind = F::AssetId, + Credit = fungibles::Credit, + > + QuotePrice, AssetKind = F::AssetId>, + OUF: OnUnbalanced>, + OUT: OnUnbalanced>, T: Config, - C: Inspect<::AccountId>, - CON: Swap, AssetKind = T::AssetKind>, - BalanceOf: Into>, - T::AssetKind: From>, - BalanceOf: IsType<::AccountId>>::Balance>, { + type AssetId = F::AssetId; type Balance = BalanceOf; - type AssetId = AssetIdOf; - type LiquidityInfo = BalanceOf; + type LiquidityInfo = (fungibles::Credit, BalanceOf); - /// Swap & withdraw the predicted fee from the transaction origin. - /// - /// Note: The `fee` already includes the `tip`. - /// - /// Returns the total amount in native currency received by exchanging the `asset_id` and the - /// amount in native currency used to pay the fee. fn withdraw_fee( who: &T::AccountId, - call: &T::RuntimeCall, - info: &DispatchInfoOf, + _call: &T::RuntimeCall, + _dispatch_info: &DispatchInfoOf<::RuntimeCall>, asset_id: Self::AssetId, - fee: BalanceOf, - tip: BalanceOf, - ) -> Result< - (LiquidityInfoOf, Self::LiquidityInfo, AssetBalanceOf), - TransactionValidityError, - > { - // convert the asset into native currency - let ed = C::minimum_balance(); - let native_asset_required = - if C::balance(&who) >= ed.saturating_add(fee.into()) { fee } else { fee + ed.into() }; - - let asset_consumed = CON::swap_tokens_for_exact_tokens( - who.clone(), - vec![asset_id.into(), N::get()], - native_asset_required, - None, - who.clone(), - true, + fee: Self::Balance, + _tip: Self::Balance, + ) -> Result { + if asset_id == A::get() { + // The `asset_id` is the target asset, we do not need to swap. + let fee_credit = F::withdraw( + asset_id.clone(), + who, + fee, + Precision::Exact, + Preservation::Preserve, + Fortitude::Polite, + ) + .map_err(|_| InvalidTransaction::Payment)?; + + return Ok((fee_credit, fee)); + } + + // Quote the amount of the `asset_id` needed to pay the fee in the asset `A`. + let asset_fee = + S::quote_price_tokens_for_exact_tokens(asset_id.clone(), A::get(), fee, true) + .ok_or(InvalidTransaction::Payment)?; + + // Withdraw the `asset_id` credit for the swap. + let asset_fee_credit = F::withdraw( + asset_id.clone(), + who, + asset_fee, + Precision::Exact, + Preservation::Preserve, + Fortitude::Polite, ) - .map_err(|_| TransactionValidityError::from(InvalidTransaction::Payment))?; + .map_err(|_| InvalidTransaction::Payment)?; + + let (fee_credit, change) = match S::swap_tokens_for_exact_tokens( + vec![asset_id, A::get()], + asset_fee_credit, + fee, + ) { + Ok((fee_credit, change)) => (fee_credit, change), + Err((credit_in, _)) => { + let _ = F::resolve(who, credit_in).defensive(); + return Err(InvalidTransaction::Payment.into()) + }, + }; - ensure!(asset_consumed > Zero::zero(), InvalidTransaction::Payment); + // Since the exact price for `fee` has been quoted, the change should be zero. + ensure!(change.peek() == Zero::zero(), InvalidTransaction::Payment); - // charge the fee in native currency - ::withdraw_fee(who, call, info, fee, tip) - .map(|r| (r, native_asset_required, asset_consumed.into())) + Ok((fee_credit, asset_fee)) } - /// Correct the fee and swap the refund back to asset. - /// - /// Note: The `corrected_fee` already includes the `tip`. - /// Note: Is the ED wasn't needed, the `received_exchanged` will be equal to `fee_paid`, or - /// `fee_paid + ed` otherwise. fn correct_and_deposit_fee( who: &T::AccountId, - dispatch_info: &DispatchInfoOf, - post_info: &PostDispatchInfoOf, - corrected_fee: BalanceOf, - tip: BalanceOf, - fee_paid: LiquidityInfoOf, - received_exchanged: Self::LiquidityInfo, + _dispatch_info: &DispatchInfoOf<::RuntimeCall>, + _post_info: &PostDispatchInfoOf<::RuntimeCall>, + corrected_fee: Self::Balance, + tip: Self::Balance, asset_id: Self::AssetId, - initial_asset_consumed: AssetBalanceOf, - ) -> Result, TransactionValidityError> { - // Refund the native asset to the account that paid the fees (`who`). - // The `who` account will receive the "fee_paid - corrected_fee" refund. - ::correct_and_deposit_fee( - who, - dispatch_info, - post_info, - corrected_fee, - tip, - fee_paid, - )?; - - // calculate the refund in native asset, to swap back to the desired `asset_id` - let swap_back = received_exchanged.saturating_sub(corrected_fee); - let mut asset_refund = Zero::zero(); - if !swap_back.is_zero() { - // If this fails, the account might have dropped below the existential balance or there - // is not enough liquidity left in the pool. In that case we don't throw an error and - // the account will keep the native currency. - match CON::swap_exact_tokens_for_tokens( - who.clone(), // we already deposited the native to `who` - vec![ - N::get(), // we provide the native - asset_id.into(), // we want asset_id back - ], - swap_back, // amount of the native asset to convert to `asset_id` - None, // no minimum amount back - who.clone(), // we will refund to `who` - false, // no need to keep alive + already_withdrawn: Self::LiquidityInfo, + ) -> Result, TransactionValidityError> { + let (fee_paid, initial_asset_consumed) = already_withdrawn; + // Try to refund if the fee paid is more than the corrected fee and the account was not + // removed by the dispatched function. + let (adjusted_paid, fee_in_asset) = if fee_paid.peek() > corrected_fee && + F::total_balance(asset_id.clone(), who) > Zero::zero() + { + let refund_amount = fee_paid.peek().saturating_sub(corrected_fee); + // Check if the refund amount can be swapped back into the asset used by `who` for fee + // payment. + let refund_asset_amount = S::quote_price_exact_tokens_for_tokens( + A::get(), + asset_id.clone(), + refund_amount, + true, ) - .ok() - { - Some(acquired) => { - asset_refund = acquired - .try_into() - .map_err(|_| TransactionValidityError::from(InvalidTransaction::Payment))?; - }, - None => { - Pallet::::deposit_event(Event::::AssetRefundFailed { - native_amount_kept: swap_back, - }); - }, + // No refund given if it cannot be swapped back. + .unwrap_or(Zero::zero()); + + // Deposit the refund before the swap to ensure it can be processed. + let debt = match F::deposit( + asset_id.clone(), + &who, + refund_asset_amount, + Precision::BestEffort, + ) { + Ok(debt) => debt, + // No refund given since it cannot be deposited. + Err(_) => fungibles::Debt::::zero(asset_id.clone()), + }; + + if debt.peek() == Zero::zero() { + // No refund given. + (fee_paid, initial_asset_consumed) + } else { + let (refund, fee_paid) = fee_paid.split(refund_amount); + match S::swap_exact_tokens_for_tokens( + vec![A::get(), asset_id], + refund, + Some(refund_asset_amount), + ) { + Ok(refund_asset) => { + match refund_asset.offset(debt) { + Ok(SameOrOther::None) => {}, + // This arm should never be reached, as the amount of `debt` is + // expected to be exactly equal to the amount of `refund_asset` credit. + _ => return Err(InvalidTransaction::Payment.into()), + }; + ( + fee_paid, + initial_asset_consumed.saturating_sub(refund_asset_amount.into()), + ) + }, + // The error should not occur since swap was quoted before. + Err((refund, _)) => { + match F::settle(who, debt, Preservation::Expendable) { + Ok(dust) => + ensure!(dust.peek() == Zero::zero(), InvalidTransaction::Payment), + // The error should not occur as the `debt` was just withdrawn above. + Err(_) => return Err(InvalidTransaction::Payment.into()), + }; + let fee_paid = fee_paid.merge(refund).map_err(|_| { + // The error should never occur since `fee_paid` and `refund` are + // credits of the same asset. + InvalidTransaction::Payment + })?; + (fee_paid, initial_asset_consumed) + }, + } } - } + } else { + (fee_paid, initial_asset_consumed) + }; - let actual_paid = initial_asset_consumed.saturating_sub(asset_refund); - Ok(actual_paid) + // Handle the imbalance (fee and tip separately). + let (tip, fee) = adjusted_paid.split(tip); + OUF::on_unbalanced(fee); + OUT::on_unbalanced(tip); + Ok(fee_in_asset) } } diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs index aa2f26f3a6a8..f8e25276ad3c 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs @@ -239,7 +239,7 @@ fn transaction_payment_in_asset_possible() { let fee_in_asset = input_quote.unwrap(); assert_eq!(Assets::balance(asset_id, caller), balance); - let pre = ChargeAssetTxPayment::::from(0, Some(asset_id)) + let pre = ChargeAssetTxPayment::::from(0, Some(asset_id.into())) .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_5), len) .unwrap(); // assert that native balance is not used @@ -297,7 +297,7 @@ fn transaction_payment_in_asset_fails_if_no_pool_for_that_asset() { assert_eq!(Assets::balance(asset_id, caller), balance); let len = 10; - let pre = ChargeAssetTxPayment::::from(0, Some(asset_id)).pre_dispatch( + let pre = ChargeAssetTxPayment::::from(0, Some(asset_id.into())).pre_dispatch( &caller, CALL, &info_from_weight(WEIGHT_5), @@ -352,7 +352,7 @@ fn transaction_payment_without_fee() { assert_eq!(input_quote, Some(201)); let fee_in_asset = input_quote.unwrap(); - let pre = ChargeAssetTxPayment::::from(0, Some(asset_id)) + let pre = ChargeAssetTxPayment::::from(0, Some(asset_id.into())) .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_5), len) .unwrap(); @@ -429,7 +429,7 @@ fn asset_transaction_payment_with_tip_and_refund() { assert_eq!(input_quote, Some(1206)); let fee_in_asset = input_quote.unwrap(); - let pre = ChargeAssetTxPayment::::from(tip, Some(asset_id)) + let pre = ChargeAssetTxPayment::::from(tip, Some(asset_id.into())) .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_100), len) .unwrap(); assert_eq!(Assets::balance(asset_id, caller), balance - fee_in_asset); @@ -512,31 +512,21 @@ fn payment_from_account_with_only_assets() { let len = 10; let fee_in_native = base_weight + weight + len as u64; - let ed = Balances::minimum_balance(); let fee_in_asset = AssetConversion::quote_price_tokens_for_exact_tokens( NativeOrWithId::WithId(asset_id), NativeOrWithId::Native, - fee_in_native + ed, + fee_in_native, true, ) .unwrap(); - assert_eq!(fee_in_asset, 301); + assert_eq!(fee_in_asset, 201); - let pre = ChargeAssetTxPayment::::from(0, Some(asset_id)) + let pre = ChargeAssetTxPayment::::from(0, Some(asset_id.into())) .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_5), len) .unwrap(); - assert_eq!(Balances::free_balance(caller), ed); // check that fee was charged in the given asset assert_eq!(Assets::balance(asset_id, caller), balance - fee_in_asset); - let refund = AssetConversion::quote_price_exact_tokens_for_tokens( - NativeOrWithId::Native, - NativeOrWithId::WithId(asset_id), - ed, - true, - ) - .unwrap(); - assert_ok!(ChargeAssetTxPayment::::post_dispatch( Some(pre), &info_from_weight(WEIGHT_5), @@ -544,7 +534,7 @@ fn payment_from_account_with_only_assets() { len, &Ok(()) )); - assert_eq!(Assets::balance(asset_id, caller), balance - fee_in_asset + refund); + assert_eq!(Assets::balance(asset_id, caller), balance - fee_in_asset); assert_eq!(Balances::free_balance(caller), 0); assert_eq!(TipUnbalancedAmount::get(), 0); @@ -587,7 +577,7 @@ fn converted_fee_is_never_zero_if_input_fee_is_not() { // there will be no conversion when the fee is zero { - let pre = ChargeAssetTxPayment::::from(0, Some(asset_id)) + let pre = ChargeAssetTxPayment::::from(0, Some(asset_id.into())) .pre_dispatch(&caller, CALL, &info_from_pays(Pays::No), len) .unwrap(); // `Pays::No` implies there are no fees @@ -613,7 +603,7 @@ fn converted_fee_is_never_zero_if_input_fee_is_not() { ) .unwrap(); - let pre = ChargeAssetTxPayment::::from(0, Some(asset_id)) + let pre = ChargeAssetTxPayment::::from(0, Some(asset_id.into())) .pre_dispatch(&caller, CALL, &info_from_weight(Weight::from_parts(weight, 0)), len) .unwrap(); assert_eq!(Assets::balance(asset_id, caller), balance - fee_in_asset); @@ -663,14 +653,14 @@ fn post_dispatch_fee_is_zero_if_pre_dispatch_fee_is_zero() { // calculated fee is greater than 0 assert!(fee > 0); - let pre = ChargeAssetTxPayment::::from(0, Some(asset_id)) + let pre = ChargeAssetTxPayment::::from(0, Some(asset_id.into())) .pre_dispatch(&caller, CALL, &info_from_pays(Pays::No), len) .unwrap(); // `Pays::No` implies no pre-dispatch fees assert_eq!(Assets::balance(asset_id, caller), balance); - let (_tip, _who, initial_payment, _asset_id) = ⪯ + let (_tip, _who, initial_payment) = ⪯ let not_paying = match initial_payment { &InitialPayment::Nothing => true, _ => false, diff --git a/substrate/frame/transaction-payment/src/lib.rs b/substrate/frame/transaction-payment/src/lib.rs index 0e440ee4e9ff..81bca437bea5 100644 --- a/substrate/frame/transaction-payment/src/lib.rs +++ b/substrate/frame/transaction-payment/src/lib.rs @@ -664,6 +664,11 @@ impl Pallet { let capped_weight = weight.min(T::BlockWeights::get().max_block); T::WeightToFee::weight_to_fee(&capped_weight) } + + /// Deposit the [`Event::TransactionFeePaid`] event. + pub fn deposit_fee_paid_event(who: T::AccountId, actual_fee: BalanceOf, tip: BalanceOf) { + Self::deposit_event(Event::TransactionFeePaid { who, actual_fee, tip }); + } } impl Convert> for Pallet From dcfa93ec60a8cc44efa63a230aaa002027cc384d Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 16 May 2024 17:18:52 +0200 Subject: [PATCH 02/16] integration of new api --- .../assets/asset-hub-rococo/src/lib.rs | 16 ++++-- .../assets/asset-hub-westend/src/lib.rs | 16 ++++-- substrate/bin/node/runtime/src/lib.rs | 29 ++++------- substrate/frame/asset-conversion/src/swap.rs | 52 +++++++++++++++++++ 4 files changed, 87 insertions(+), 26 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index 0df9b65c714d..6f9a559938cb 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -69,7 +69,7 @@ use frame_system::{ limits::{BlockLength, BlockWeights}, EnsureRoot, EnsureSigned, EnsureSignedBy, }; -use pallet_asset_conversion_tx_payment::AssetConversionAdapter; +use pallet_asset_conversion_tx_payment::SwapAssetAdapter; use pallet_nfts::PalletFeatures; use parachains_common::{ impls::DealWithFees, @@ -766,11 +766,19 @@ impl pallet_collator_selection::Config for Runtime { type WeightInfo = weights::pallet_collator_selection::WeightInfo; } +parameter_types! { + pub StakingPot: AccountId = CollatorSelection::account_id(); +} + impl pallet_asset_conversion_tx_payment::Config for Runtime { type RuntimeEvent = RuntimeEvent; - type Fungibles = LocalAndForeignAssets; - type OnChargeAssetTransaction = - AssetConversionAdapter; + type OnChargeAssetTransaction = SwapAssetAdapter< + TokenLocationV3, + NativeAndAssets, + AssetConversion, + ResolveAssetTo, + ResolveAssetTo, + >; } parameter_types! { diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index b5c3ed5053c4..a08651efc2b8 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -52,7 +52,7 @@ use frame_system::{ limits::{BlockLength, BlockWeights}, EnsureRoot, EnsureSigned, EnsureSignedBy, }; -use pallet_asset_conversion_tx_payment::AssetConversionAdapter; +use pallet_asset_conversion_tx_payment::SwapAssetAdapter; use pallet_nfts::{DestroyWitness, PalletFeatures}; use pallet_xcm::EnsureXcm; use parachains_common::{ @@ -758,11 +758,19 @@ impl pallet_collator_selection::Config for Runtime { type WeightInfo = weights::pallet_collator_selection::WeightInfo; } +parameter_types! { + pub StakingPot: AccountId = CollatorSelection::account_id(); +} + impl pallet_asset_conversion_tx_payment::Config for Runtime { type RuntimeEvent = RuntimeEvent; - type Fungibles = LocalAndForeignAssets; - type OnChargeAssetTransaction = - AssetConversionAdapter; + type OnChargeAssetTransaction = SwapAssetAdapter< + WestendLocationV3, + NativeAndAssets, + AssetConversion, + ResolveAssetTo, + ResolveAssetTo, + >; } parameter_types! { diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 017ee9100f9e..f61cab3bd96e 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -64,6 +64,7 @@ use frame_system::{ pub use node_primitives::{AccountId, Signature}; use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Moment, Nonce}; use pallet_asset_conversion::{AccountIdConverter, Ascending, Chain, WithFirstAsset}; +use pallet_asset_conversion_tx_payment::SwapAssetAdapter; use pallet_broker::{CoreAssignment, CoreIndex, CoretimeInterface, PartsOf57600}; use pallet_election_provider_multi_phase::{GeometricDepositBase, SolutionAccuracyOf}; use pallet_identity::legacy::IdentityInfo; @@ -119,7 +120,7 @@ pub use sp_runtime::BuildStorage; pub mod impls; #[cfg(not(feature = "runtime-benchmarks"))] use impls::AllianceIdentityVerifier; -use impls::{AllianceProposalProvider, Author, CreditToBlockAuthor}; +use impls::{AllianceProposalProvider, Author}; /// Constant values used within the runtime. pub mod constants; @@ -570,22 +571,14 @@ impl pallet_transaction_payment::Config for Runtime { >; } -impl pallet_asset_tx_payment::Config for Runtime { - type RuntimeEvent = RuntimeEvent; - type Fungibles = Assets; - type OnChargeAssetTransaction = pallet_asset_tx_payment::FungiblesAdapter< - pallet_assets::BalanceToAssetBalance, - CreditToBlockAuthor, - >; -} - impl pallet_asset_conversion_tx_payment::Config for Runtime { type RuntimeEvent = RuntimeEvent; - type Fungibles = Assets; - type OnChargeAssetTransaction = pallet_asset_conversion_tx_payment::AssetConversionAdapter< - Balances, - AssetConversion, + type OnChargeAssetTransaction = SwapAssetAdapter< Native, + NativeAndAssets, + AssetConversion, + ResolveAssetTo, + ResolveAssetTo, >; } @@ -1701,12 +1694,15 @@ parameter_types! { pub const Native: NativeOrWithId = NativeOrWithId::Native; } +pub type NativeAndAssets = + UnionOf, AccountId>; + impl pallet_asset_conversion::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Balance = u128; type HigherPrecisionBalance = sp_core::U256; type AssetKind = NativeOrWithId; - type Assets = UnionOf, AccountId>; + type Assets = NativeAndAssets; type PoolId = (Self::AssetKind, Self::AssetKind); type PoolLocator = Chain< WithFirstAsset< @@ -2281,9 +2277,6 @@ mod runtime { #[runtime::pallet_index(7)] pub type TransactionPayment = pallet_transaction_payment; - #[runtime::pallet_index(8)] - pub type AssetTxPayment = pallet_asset_tx_payment; - #[runtime::pallet_index(9)] pub type AssetConversionTxPayment = pallet_asset_conversion_tx_payment; diff --git a/substrate/frame/asset-conversion/src/swap.rs b/substrate/frame/asset-conversion/src/swap.rs index a6154e294147..1485e7166f30 100644 --- a/substrate/frame/asset-conversion/src/swap.rs +++ b/substrate/frame/asset-conversion/src/swap.rs @@ -112,6 +112,37 @@ pub trait SwapCredit { ) -> Result<(Self::Credit, Self::Credit), (Self::Credit, DispatchError)>; } +/// Trait providing methods to quote swap prices between asset classes. +/// +/// The quoted price is only guaranteed if no other swaps are made after the price is quoted and +/// before the target swap (e.g., the swap is made immediately within the same transaction). +pub trait QuotePrice { + /// Measurement units of the asset classes for pricing. + type Balance: Balance; + /// Type representing the kind of assets for which the price is being quoted. + type AssetKind; + /// Quotes the amount of `asset1` required to obtain the exact `amount` of `asset2`. + /// + /// If `include_fee` is set to `true`, the price will include the pool's fee. + /// If the pool does not exist or the swap cannot be made, `None` is returned. + fn quote_price_tokens_for_exact_tokens( + asset1: Self::AssetKind, + asset2: Self::AssetKind, + amount: Self::Balance, + include_fee: bool, + ) -> Option; + /// Quotes the amount of `asset2` resulting from swapping the exact `amount` of `asset1`. + /// + /// If `include_fee` is set to `true`, the price will include the pool's fee. + /// If the pool does not exist or the swap cannot be made, `None` is returned. + fn quote_price_exact_tokens_for_tokens( + asset1: Self::AssetKind, + asset2: Self::AssetKind, + amount: Self::Balance, + include_fee: bool, + ) -> Option; +} + impl Swap for Pallet { type Balance = T::Balance; type AssetKind = T::AssetKind; @@ -210,3 +241,24 @@ impl SwapCredit for Pallet { .map_err(|_| (Self::Credit::zero(credit_asset), DispatchError::Corruption))? } } + +impl QuotePrice for Pallet { + type Balance = T::Balance; + type AssetKind = T::AssetKind; + fn quote_price_exact_tokens_for_tokens( + asset1: Self::AssetKind, + asset2: Self::AssetKind, + amount: Self::Balance, + include_fee: bool, + ) -> Option { + Self::quote_price_exact_tokens_for_tokens(asset1, asset2, amount, include_fee) + } + fn quote_price_tokens_for_exact_tokens( + asset1: Self::AssetKind, + asset2: Self::AssetKind, + amount: Self::Balance, + include_fee: bool, + ) -> Option { + Self::quote_price_tokens_for_exact_tokens(asset1, asset2, amount, include_fee) + } +} From a4dbc71b3ba2b81e8534c750246d7e864ff2b845 Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 23 May 2024 15:38:15 +0200 Subject: [PATCH 03/16] style chages and logs --- .../asset-conversion-tx-payment/src/lib.rs | 2 +- .../asset-conversion-tx-payment/src/mock.rs | 2 +- .../src/payment.rs | 36 +++++++++++-------- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs index 4616cfc0991c..ae56a3f93831 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs @@ -23,7 +23,7 @@ //! This pallet provides a `SignedExtension` with an optional `AssetId` that specifies the asset //! to be used for payment (defaulting to the native token on `None`). It expects an //! [`OnChargeAssetTransaction`] implementation analogous to [`pallet-transaction-payment`]. The -//! included [`SwapCreditAdapter`] (implementing [`OnChargeAssetTransaction`]) determines the +//! included [`SwapAssetAdapter`] (implementing [`OnChargeAssetTransaction`]) determines the //! fee amount by converting the fee calculated by [`pallet-transaction-payment`] in the native //! asset into the amount required of the specified asset. //! diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs index 0acce3fe620e..c935dbd51247 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs @@ -291,5 +291,5 @@ impl pallet_asset_conversion::Config for Runtime { impl Config for Runtime { type RuntimeEvent = RuntimeEvent; type OnChargeAssetTransaction = - SwapCreditAdapter; + SwapAssetAdapter; } diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs index a7f216387331..6147eb15076d 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs @@ -18,7 +18,7 @@ use super::*; use crate::Config; use frame_support::{ - ensure, + defensive, ensure, traits::{ fungibles, tokens::{Balance, Fortitude, Precision, Preservation}, @@ -150,13 +150,14 @@ where ) { Ok((fee_credit, change)) => (fee_credit, change), Err((credit_in, _)) => { - let _ = F::resolve(who, credit_in).defensive(); + defensive!("Fee swap should pass for the quoted amount"); + let _ = F::resolve(who, credit_in).defensive_proof("Should resolve the credit"); return Err(InvalidTransaction::Payment.into()) }, }; // Since the exact price for `fee` has been quoted, the change should be zero. - ensure!(change.peek() == Zero::zero(), InvalidTransaction::Payment); + ensure!(change.peek().is_zero(), InvalidTransaction::Payment); Ok((fee_credit, asset_fee)) } @@ -171,12 +172,13 @@ where already_withdrawn: Self::LiquidityInfo, ) -> Result, TransactionValidityError> { let (fee_paid, initial_asset_consumed) = already_withdrawn; - // Try to refund if the fee paid is more than the corrected fee and the account was not - // removed by the dispatched function. - let (adjusted_paid, fee_in_asset) = if fee_paid.peek() > corrected_fee && - F::total_balance(asset_id.clone(), who) > Zero::zero() + let refund_amount = fee_paid.peek().saturating_sub(corrected_fee); + let (adjusted_paid, fee_in_asset) = if refund_amount.is_zero() || + F::total_balance(asset_id.clone(), who).is_zero() { - let refund_amount = fee_paid.peek().saturating_sub(corrected_fee); + // Nothing to refund or the account was removed be the dispatched function. + (fee_paid, initial_asset_consumed) + } else { // Check if the refund amount can be swapped back into the asset used by `who` for fee // payment. let refund_asset_amount = S::quote_price_exact_tokens_for_tokens( @@ -200,7 +202,7 @@ where Err(_) => fungibles::Debt::::zero(asset_id.clone()), }; - if debt.peek() == Zero::zero() { + if debt.peek().is_zero() { // No refund given. (fee_paid, initial_asset_consumed) } else { @@ -215,7 +217,10 @@ where Ok(SameOrOther::None) => {}, // This arm should never be reached, as the amount of `debt` is // expected to be exactly equal to the amount of `refund_asset` credit. - _ => return Err(InvalidTransaction::Payment.into()), + _ => { + defensive!("Debt should be equal to the refund credit"); + return Err(InvalidTransaction::Payment.into()) + }, }; ( fee_paid, @@ -224,11 +229,14 @@ where }, // The error should not occur since swap was quoted before. Err((refund, _)) => { + defensive!("Refund swap should pass for the quoted amount"); match F::settle(who, debt, Preservation::Expendable) { - Ok(dust) => - ensure!(dust.peek() == Zero::zero(), InvalidTransaction::Payment), + Ok(dust) => ensure!(dust.peek().is_zero(), InvalidTransaction::Payment), // The error should not occur as the `debt` was just withdrawn above. - Err(_) => return Err(InvalidTransaction::Payment.into()), + Err(_) => { + defensive!("Should settle the debt"); + return Err(InvalidTransaction::Payment.into()) + }, }; let fee_paid = fee_paid.merge(refund).map_err(|_| { // The error should never occur since `fee_paid` and `refund` are @@ -239,8 +247,6 @@ where }, } } - } else { - (fee_paid, initial_asset_consumed) }; // Handle the imbalance (fee and tip separately). From 62f474f5e6f08b24d66016118ce98030ee9dde4d Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 23 May 2024 16:30:32 +0200 Subject: [PATCH 04/16] pr doc --- prdoc/pr_4488.prdoc | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 prdoc/pr_4488.prdoc diff --git a/prdoc/pr_4488.prdoc b/prdoc/pr_4488.prdoc new file mode 100644 index 000000000000..06d7916160d6 --- /dev/null +++ b/prdoc/pr_4488.prdoc @@ -0,0 +1,12 @@ +title: "Tx Payment: drop ED requirements for tx payments with exchangeable asset" + +doc: + - audience: Runtime Dev + description: | + Drop the Existential Deposit requirement for the asset amount exchangeable for the fee asset (eg. DOT/KSM) during transaction payments. + + This achieved by using `SwapCredit` implementation of asset conversion, which works with imbalances and does not require a temporary balance account within the transaction payment. + +crates: + - name: pallet-asset-conversion-tx-payment + bump: major From 27bb4001234beb3d0e06839f07c8bef52c604880 Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 23 May 2024 16:33:05 +0200 Subject: [PATCH 05/16] pr doc style fix --- prdoc/pr_4488.prdoc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/prdoc/pr_4488.prdoc b/prdoc/pr_4488.prdoc index 06d7916160d6..d2760576e753 100644 --- a/prdoc/pr_4488.prdoc +++ b/prdoc/pr_4488.prdoc @@ -3,9 +3,11 @@ title: "Tx Payment: drop ED requirements for tx payments with exchangeable asset doc: - audience: Runtime Dev description: | - Drop the Existential Deposit requirement for the asset amount exchangeable for the fee asset (eg. DOT/KSM) during transaction payments. + Drop the Existential Deposit requirement for the asset amount exchangeable for the fee asset + (eg. DOT/KSM) during transaction payments. - This achieved by using `SwapCredit` implementation of asset conversion, which works with imbalances and does not require a temporary balance account within the transaction payment. + This achieved by using `SwapCredit` implementation of asset conversion, which works with + imbalances and does not require a temporary balance account within the transaction payment. crates: - name: pallet-asset-conversion-tx-payment From 16ef51572a2ff970d35150ec1869556b5f3b1fd1 Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 24 May 2024 11:22:02 +0200 Subject: [PATCH 06/16] handle: asset id is a native asset --- .../asset-conversion-tx-payment/src/lib.rs | 32 +++++++++----- .../asset-conversion-tx-payment/src/mock.rs | 1 + .../src/payment.rs | 33 +++++++++----- .../asset-conversion-tx-payment/src/tests.rs | 44 +++++++++++++++++++ 4 files changed, 88 insertions(+), 22 deletions(-) diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs index ae56a3f93831..168712e59909 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs @@ -66,10 +66,6 @@ mod payment; use frame_support::traits::tokens::AssetId; pub use payment::*; -/// Type alias for Asset IDs in their interaction with `OnChargeAssetTransaction`. -pub(crate) type AssetIdOf = - <::OnChargeAssetTransaction as OnChargeAssetTransaction>::AssetId; - /// Balance type alias for balances of the chain's native asset. pub(crate) type BalanceOf = as OnChargeTransaction>::Balance; @@ -94,7 +90,7 @@ pub enum InitialPayment { /// The initial fee was paid in the native currency. Native(NativeLiquidityInfoOf), /// The initial fee was paid in an asset. - Asset((AssetIdOf, AssetLiquidityInfoOf)), + Asset((T::AssetId, AssetLiquidityInfoOf)), } pub use pallet::*; @@ -107,8 +103,15 @@ pub mod pallet { pub trait Config: frame_system::Config + pallet_transaction_payment::Config { /// The overarching event type. type RuntimeEvent: From> + IsType<::RuntimeEvent>; + /// The asset ID type that can be used for transaction payments in addition to a + /// native asset. + type AssetId: AssetId; /// The actual transaction charging logic that charges the fees. - type OnChargeAssetTransaction: OnChargeAssetTransaction>; + type OnChargeAssetTransaction: OnChargeAssetTransaction< + Self, + Balance = BalanceOf, + AssetId = Self::AssetId, + >; } #[pallet::pallet] @@ -123,7 +126,7 @@ pub mod pallet { who: T::AccountId, actual_fee: BalanceOf, tip: BalanceOf, - asset_id: AssetIdOf, + asset_id: T::AssetId, }, /// A swap of the refund in native currency back to asset failed. AssetRefundFailed { native_amount_kept: BalanceOf }, @@ -131,27 +134,32 @@ pub mod pallet { } /// Require payment for transaction inclusion and optionally include a tip to gain additional -/// priority in the queue. Allows paying via both `Currency` as well as `fungibles::Balanced`. +/// priority in the queue. /// /// Wraps the transaction logic in [`pallet_transaction_payment`] and extends it with assets. /// An asset ID of `None` falls back to the underlying transaction payment logic via the native /// currency. +/// +/// Transaction payments are processed using different handlers based on the asset type: +/// - Payments with a native asset are charged by +/// [pallet_transaction_payment::Config::OnChargeTransaction]. +/// - Payments with other assets are charged by [Config::OnChargeAssetTransaction]. #[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] #[scale_info(skip_type_params(T))] pub struct ChargeAssetTxPayment { #[codec(compact)] tip: BalanceOf, - asset_id: Option>, + asset_id: Option, } impl ChargeAssetTxPayment where T::RuntimeCall: Dispatchable, BalanceOf: Send + Sync, - AssetIdOf: Send + Sync, + T::AssetId: Send + Sync, { /// Utility constructor. Used only in client/factory code. - pub fn from(tip: BalanceOf, asset_id: Option>) -> Self { + pub fn from(tip: BalanceOf, asset_id: Option) -> Self { Self { tip, asset_id } } @@ -200,7 +208,7 @@ impl SignedExtension for ChargeAssetTxPayment where T::RuntimeCall: Dispatchable, BalanceOf: Send + Sync, - AssetIdOf: Send + Sync, + T::AssetId: Send + Sync, { const IDENTIFIER: &'static str = "ChargeAssetTxPayment"; type AccountId = T::AccountId; diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs index c935dbd51247..0655ab8bf620 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs @@ -290,6 +290,7 @@ impl pallet_asset_conversion::Config for Runtime { impl Config for Runtime { type RuntimeEvent = RuntimeEvent; + type AssetId = NativeOrWithId; type OnChargeAssetTransaction = SwapAssetAdapter; } diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs index 6147eb15076d..43e0127af672 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs @@ -88,19 +88,19 @@ pub struct SwapAssetAdapter(PhantomData<(A, F, S, OUF, OUT)>) impl OnChargeAssetTransaction for SwapAssetAdapter where - A: Get, - F: fungibles::Balanced>, + A: Get, + F: fungibles::Balanced, AssetId = T::AssetId>, S: SwapCredit< T::AccountId, Balance = BalanceOf, - AssetKind = F::AssetId, + AssetKind = T::AssetId, Credit = fungibles::Credit, - > + QuotePrice, AssetKind = F::AssetId>, + > + QuotePrice, AssetKind = T::AssetId>, OUF: OnUnbalanced>, OUT: OnUnbalanced>, T: Config, { - type AssetId = F::AssetId; + type AssetId = T::AssetId; type Balance = BalanceOf; type LiquidityInfo = (fungibles::Credit, BalanceOf); @@ -173,11 +173,24 @@ where ) -> Result, TransactionValidityError> { let (fee_paid, initial_asset_consumed) = already_withdrawn; let refund_amount = fee_paid.peek().saturating_sub(corrected_fee); - let (adjusted_paid, fee_in_asset) = if refund_amount.is_zero() || + let (fee_in_asset, adjusted_paid) = if refund_amount.is_zero() || F::total_balance(asset_id.clone(), who).is_zero() { // Nothing to refund or the account was removed be the dispatched function. - (fee_paid, initial_asset_consumed) + (initial_asset_consumed, fee_paid) + } else if asset_id == A::get() { + // The `asset_id` is the target asset, we do not need to swap. + let (refund, fee_paid) = fee_paid.split(refund_amount); + if let Err(refund) = F::resolve(who, refund) { + let fee_paid = fee_paid.merge(refund).map_err(|_| { + // The error should never occur since `fee_paid` and `refund` are + // credits of the same asset. + InvalidTransaction::Payment + })?; + (initial_asset_consumed, fee_paid) + } else { + (fee_paid.peek().saturating_sub(refund_amount), fee_paid) + } } else { // Check if the refund amount can be swapped back into the asset used by `who` for fee // payment. @@ -204,7 +217,7 @@ where if debt.peek().is_zero() { // No refund given. - (fee_paid, initial_asset_consumed) + (initial_asset_consumed, fee_paid) } else { let (refund, fee_paid) = fee_paid.split(refund_amount); match S::swap_exact_tokens_for_tokens( @@ -223,8 +236,8 @@ where }, }; ( - fee_paid, initial_asset_consumed.saturating_sub(refund_asset_amount.into()), + fee_paid, ) }, // The error should not occur since swap was quoted before. @@ -243,7 +256,7 @@ where // credits of the same asset. InvalidTransaction::Payment })?; - (fee_paid, initial_asset_consumed) + (initial_asset_consumed, fee_paid) }, } } diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs index f8e25276ad3c..a2da459d9932 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs @@ -730,3 +730,47 @@ fn post_dispatch_fee_is_zero_if_unsigned_pre_dispatch_fee_is_zero() { assert_eq!(Assets::balance(asset_id, caller), balance); }); } + +#[test] +fn fee_with_native_asset_passed_with_id() { + let base_weight = 5; + let balance_factor = 100; + ExtBuilder::default() + .balance_factor(balance_factor) + .base_weight(Weight::from_parts(base_weight, 0)) + .build() + .execute_with(|| { + let caller = 1; + let caller_balance = 1000; + // native asset + let asset_id = NativeOrWithId::Native; + // assert that native balance is not necessary + assert_eq!(Balances::free_balance(caller), caller_balance); + + let tip = 10; + let weight = 100; + let len = 5; + let initial_fee = base_weight + weight + len as u64 + tip; + + let pre = ChargeAssetTxPayment::::from(tip, Some(asset_id.into())) + .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_100), len) + .unwrap(); + assert_eq!(Balances::free_balance(caller), caller_balance - initial_fee); + + let final_weight = 50; + let expected_fee = initial_fee - final_weight; + + assert_ok!(ChargeAssetTxPayment::::post_dispatch( + Some(pre), + &info_from_weight(WEIGHT_100), + &post_info_from_weight(WEIGHT_50), + len, + &Ok(()) + )); + + assert_eq!(Balances::free_balance(caller), caller_balance - expected_fee); + + assert_eq!(TipUnbalancedAmount::get(), tip); + assert_eq!(FeeUnbalancedAmount::get(), expected_fee - tip); + }); +} From 3a8e675e9f6f283514c00c14d3d1d90ed5bf59c0 Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 24 May 2024 11:36:58 +0200 Subject: [PATCH 07/16] fix integrations --- cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs | 1 + cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs | 1 + substrate/bin/node/runtime/src/lib.rs | 1 + 3 files changed, 3 insertions(+) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index 6f9a559938cb..094990beb8da 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -772,6 +772,7 @@ parameter_types! { impl pallet_asset_conversion_tx_payment::Config for Runtime { type RuntimeEvent = RuntimeEvent; + type AssetId = xcm::v3::Location; type OnChargeAssetTransaction = SwapAssetAdapter< TokenLocationV3, NativeAndAssets, diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index a08651efc2b8..37c75465e4fb 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -764,6 +764,7 @@ parameter_types! { impl pallet_asset_conversion_tx_payment::Config for Runtime { type RuntimeEvent = RuntimeEvent; + type AssetId = xcm::v3::Location; type OnChargeAssetTransaction = SwapAssetAdapter< WestendLocationV3, NativeAndAssets, diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index f61cab3bd96e..05ed997b2246 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -573,6 +573,7 @@ impl pallet_transaction_payment::Config for Runtime { impl pallet_asset_conversion_tx_payment::Config for Runtime { type RuntimeEvent = RuntimeEvent; + type AssetId = NativeOrWithId; type OnChargeAssetTransaction = SwapAssetAdapter< Native, NativeAndAssets, From e4f762b8fae681d60cf5c48aaaa99262502b7813 Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 24 May 2024 12:06:57 +0200 Subject: [PATCH 08/16] update pr doc --- prdoc/pr_4488.prdoc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/prdoc/pr_4488.prdoc b/prdoc/pr_4488.prdoc index d2760576e753..55f2998c462a 100644 --- a/prdoc/pr_4488.prdoc +++ b/prdoc/pr_4488.prdoc @@ -12,3 +12,11 @@ doc: crates: - name: pallet-asset-conversion-tx-payment bump: major + - name: pallet-transaction-payment + bump: patch + - name: pallet-asset-conversion + bump: patch + - name: asset-hub-rococo-runtime + bump: patch + - name: asset-hub-westend-runtime + bump: patch From 6a31c799838fee02cfcda5cb82ae6ff97bb65d90 Mon Sep 17 00:00:00 2001 From: Muharem Date: Mon, 27 May 2024 10:17:22 +0200 Subject: [PATCH 09/16] Apply suggestions from code review Co-authored-by: Oliver Tale-Yazdi --- .../transaction-payment/asset-conversion-tx-payment/src/lib.rs | 2 -- .../asset-conversion-tx-payment/src/payment.rs | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs index 168712e59909..1ee2662b5cfd 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs @@ -155,8 +155,6 @@ pub struct ChargeAssetTxPayment { impl ChargeAssetTxPayment where T::RuntimeCall: Dispatchable, - BalanceOf: Send + Sync, - T::AssetId: Send + Sync, { /// Utility constructor. Used only in client/factory code. pub fn from(tip: BalanceOf, asset_id: Option) -> Self { diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs index 43e0127af672..553697b16ceb 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs @@ -183,8 +183,7 @@ where let (refund, fee_paid) = fee_paid.split(refund_amount); if let Err(refund) = F::resolve(who, refund) { let fee_paid = fee_paid.merge(refund).map_err(|_| { - // The error should never occur since `fee_paid` and `refund` are - // credits of the same asset. + defensive!("`fee_paid` and `refund` are credits of the same asset."); InvalidTransaction::Payment })?; (initial_asset_consumed, fee_paid) From ed57577c0841ff29158da3dcdea5019d7bf7939f Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 27 May 2024 10:33:16 +0200 Subject: [PATCH 10/16] do not deposit zero amount --- .../asset-conversion-tx-payment/src/payment.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs index 553697b16ceb..a0099263f778 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs @@ -203,15 +203,15 @@ where .unwrap_or(Zero::zero()); // Deposit the refund before the swap to ensure it can be processed. - let debt = match F::deposit( - asset_id.clone(), - &who, - refund_asset_amount, - Precision::BestEffort, - ) { - Ok(debt) => debt, - // No refund given since it cannot be deposited. - Err(_) => fungibles::Debt::::zero(asset_id.clone()), + let debt = if !refund_asset_amount.is_zero() { + match F::deposit(asset_id.clone(), &who, refund_asset_amount, Precision::BestEffort) + { + Ok(debt) => debt, + // No refund given since it cannot be deposited. + Err(_) => fungibles::Debt::::zero(asset_id.clone()), + } + } else { + fungibles::Debt::::zero(asset_id.clone()) }; if debt.peek().is_zero() { From 165084de8977e05df1abf924b5e157b506ebbc5d Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 27 May 2024 10:35:29 +0200 Subject: [PATCH 11/16] style fix --- .../asset-conversion-tx-payment/src/payment.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs index a0099263f778..a6a8166b44cd 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs @@ -202,16 +202,16 @@ where // No refund given if it cannot be swapped back. .unwrap_or(Zero::zero()); - // Deposit the refund before the swap to ensure it can be processed. - let debt = if !refund_asset_amount.is_zero() { + let debt = if refund_asset_amount.is_zero() { + fungibles::Debt::::zero(asset_id.clone()) + } else { + // Deposit the refund before the swap to ensure it can be processed. match F::deposit(asset_id.clone(), &who, refund_asset_amount, Precision::BestEffort) { Ok(debt) => debt, // No refund given since it cannot be deposited. Err(_) => fungibles::Debt::::zero(asset_id.clone()), } - } else { - fungibles::Debt::::zero(asset_id.clone()) }; if debt.peek().is_zero() { From f305f093e3a9c16bc734f9efe267c3276d4ab6b1 Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 30 May 2024 16:43:15 +0200 Subject: [PATCH 12/16] test --- .../src/payment.rs | 10 +- .../asset-conversion-tx-payment/src/tests.rs | 93 +++++++++++++++++++ 2 files changed, 98 insertions(+), 5 deletions(-) diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs index a6a8166b44cd..1c614071e099 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs @@ -218,7 +218,7 @@ where // No refund given. (initial_asset_consumed, fee_paid) } else { - let (refund, fee_paid) = fee_paid.split(refund_amount); + let (refund, adjusted_paid) = fee_paid.split(refund_amount); match S::swap_exact_tokens_for_tokens( vec![A::get(), asset_id], refund, @@ -236,7 +236,7 @@ where }; ( initial_asset_consumed.saturating_sub(refund_asset_amount.into()), - fee_paid, + adjusted_paid, ) }, // The error should not occur since swap was quoted before. @@ -250,12 +250,12 @@ where return Err(InvalidTransaction::Payment.into()) }, }; - let fee_paid = fee_paid.merge(refund).map_err(|_| { - // The error should never occur since `fee_paid` and `refund` are + let adjusted_paid = adjusted_paid.merge(refund).map_err(|_| { + // The error should never occur since `adjusted_paid` and `refund` are // credits of the same asset. InvalidTransaction::Payment })?; - (initial_asset_consumed, fee_paid) + (initial_asset_consumed, adjusted_paid) }, } } diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs index a2da459d9932..d26ddf1a18f2 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs @@ -22,6 +22,7 @@ use frame_support::{ traits::{ fungible::{Inspect, NativeOrWithId}, fungibles::{Inspect as FungiblesInspect, Mutate}, + tokens::{Fortitude, Precision, Preservation}, }, weights::Weight, }; @@ -774,3 +775,95 @@ fn fee_with_native_asset_passed_with_id() { assert_eq!(FeeUnbalancedAmount::get(), expected_fee - tip); }); } + +#[test] +fn transfer_add_and_remove_account() { + let base_weight = 5; + let balance_factor = 100; + ExtBuilder::default() + .balance_factor(balance_factor) + .base_weight(Weight::from_parts(base_weight, 0)) + .build() + .execute_with(|| { + System::set_block_number(1); + + // create the asset + let asset_id = 1; + let min_balance = 2; + assert_ok!(Assets::force_create( + RuntimeOrigin::root(), + asset_id.into(), + 42, /* owner */ + true, /* is_sufficient */ + min_balance, + )); + + setup_lp(asset_id, balance_factor); + + // mint into the caller account + let caller = 222; + let beneficiary = ::Lookup::unlookup(caller); + let balance = 10000; + + assert_eq!(Balances::free_balance(caller), 0); + assert_ok!(Assets::mint_into(asset_id.into(), &beneficiary, balance)); + assert_eq!(Assets::balance(asset_id, caller), balance); + + let weight = 100; + let tip = 5; + let len = 10; + let fee_in_native = base_weight + weight + len as u64 + tip; + let input_quote = AssetConversion::quote_price_tokens_for_exact_tokens( + NativeOrWithId::WithId(asset_id), + NativeOrWithId::Native, + fee_in_native, + true, + ); + assert_eq!(input_quote, Some(1206)); + + let fee_in_asset = input_quote.unwrap(); + let pre = ChargeAssetTxPayment::::from(tip, Some(asset_id.into())) + .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_100), len) + .unwrap(); + + assert_eq!(Assets::balance(asset_id, &caller), balance - fee_in_asset); + + // remove caller account. + assert_ok!(Assets::burn_from( + asset_id, + &caller, + Assets::balance(asset_id, &caller), + Preservation::Expendable, + Precision::Exact, + Fortitude::Force + )); + + let final_weight = 50; + let final_fee_in_native = fee_in_native - final_weight - tip; + let token_refund = AssetConversion::quote_price_exact_tokens_for_tokens( + NativeOrWithId::Native, + NativeOrWithId::WithId(asset_id), + fee_in_native - final_fee_in_native - tip, + true, + ) + .unwrap(); + + // make sure the refund amount is enough to create the account. + assert!(token_refund >= min_balance); + + assert_ok!(ChargeAssetTxPayment::::post_dispatch( + Some(pre), + &info_from_weight(WEIGHT_100), + &post_info_from_weight(WEIGHT_50), + len, + &Ok(()) + )); + + // fee paid with no refund. + assert_eq!(TipUnbalancedAmount::get(), tip); + assert_eq!(FeeUnbalancedAmount::get(), fee_in_native - tip); + + // caller account removed. + assert_eq!(Assets::balance(asset_id, caller), 0); + }); +} From 9583ebb4642ebef3d34a6f81a6354129fee1805b Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 31 May 2024 10:35:09 +0200 Subject: [PATCH 13/16] test assert fix --- .../asset-conversion-tx-payment/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs index d26ddf1a18f2..aab657199533 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs @@ -819,7 +819,7 @@ fn transfer_add_and_remove_account() { fee_in_native, true, ); - assert_eq!(input_quote, Some(1206)); + assert!(!input_quote.unwrap().is_zero()); let fee_in_asset = input_quote.unwrap(); let pre = ChargeAssetTxPayment::::from(tip, Some(asset_id.into())) From 7b026fd3dea092495c36f3c74878d1999cf829ff Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 23 Jul 2024 17:42:15 +0200 Subject: [PATCH 14/16] use on_unbalanceds --- .../assets/asset-hub-rococo/src/lib.rs | 4 +-- .../assets/asset-hub-westend/src/lib.rs | 4 +-- substrate/bin/node/runtime/src/lib.rs | 1 - .../asset-conversion-tx-payment/src/mock.rs | 26 ++++++++++--------- .../src/payment.rs | 14 +++++----- 5 files changed, 24 insertions(+), 25 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index 86f7fbac4236..68045c2fd23a 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -542,7 +542,8 @@ impl InstanceFilter for ProxyType { RuntimeCall::Utility { .. } | RuntimeCall::Multisig { .. } | RuntimeCall::NftFractionalization { .. } | - RuntimeCall::Nfts { .. } | RuntimeCall::Uniques { .. } + RuntimeCall::Nfts { .. } | + RuntimeCall::Uniques { .. } ) }, ProxyType::AssetOwner => matches!( @@ -810,7 +811,6 @@ impl pallet_asset_conversion_tx_payment::Config for Runtime { NativeAndAssets, AssetConversion, ResolveAssetTo, - ResolveAssetTo, >; } diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index 8db1a598c33e..170d68f41c5b 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -536,7 +536,8 @@ impl InstanceFilter for ProxyType { RuntimeCall::Utility { .. } | RuntimeCall::Multisig { .. } | RuntimeCall::NftFractionalization { .. } | - RuntimeCall::Nfts { .. } | RuntimeCall::Uniques { .. } + RuntimeCall::Nfts { .. } | + RuntimeCall::Uniques { .. } ) }, ProxyType::AssetOwner => matches!( @@ -799,7 +800,6 @@ impl pallet_asset_conversion_tx_payment::Config for Runtime { NativeAndAssets, AssetConversion, ResolveAssetTo, - ResolveAssetTo, >; } diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 447264ffa519..5d046539b036 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -583,7 +583,6 @@ impl pallet_asset_conversion_tx_payment::Config for Runtime { NativeAndAssets, AssetConversion, ResolveAssetTo, - ResolveAssetTo, >; } diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs index dd7efc5d30bb..acfd43d0a7cb 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs @@ -145,17 +145,19 @@ impl OnUnbalanced::AccountId, } } -pub struct DealWithFee; -impl OnUnbalanced> for DealWithFee { - fn on_nonzero_unbalanced(fee: fungibles::Credit) { - FeeUnbalancedAmount::mutate(|a| *a += fee.peek()); - } -} - -pub struct DealWithTip; -impl OnUnbalanced> for DealWithTip { - fn on_nonzero_unbalanced(tip: fungibles::Credit) { - TipUnbalancedAmount::mutate(|a| *a += tip.peek()); +pub struct DealWithFungiblesFees; +impl OnUnbalanced> for DealWithFungiblesFees { + fn on_unbalanceds( + mut fees_then_tips: impl Iterator< + Item = fungibles::Credit<::AccountId, NativeAndAssets>, + >, + ) { + if let Some(fees) = fees_then_tips.next() { + FeeUnbalancedAmount::mutate(|a| *a += fees.peek()); + if let Some(tips) = fees_then_tips.next() { + TipUnbalancedAmount::mutate(|a| *a += tips.peek()); + } + } } } @@ -268,5 +270,5 @@ impl Config for Runtime { type RuntimeEvent = RuntimeEvent; type AssetId = NativeOrWithId; type OnChargeAssetTransaction = - SwapAssetAdapter; + SwapAssetAdapter; } diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs index d3c7786f0016..dc7faecd5608 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs @@ -82,12 +82,12 @@ pub trait OnChargeAssetTransaction { /// - `A`: The asset identifier that system accepts as a fee payment (eg. native asset). /// - `F`: The fungibles registry that can handle assets provided by user and the `A` asset. /// - `S`: The swap implementation that can swap assets provided by user for the `A` asset. -/// - `OUF`: The handler for the fee payment. -/// - `OUT`: The handler for the tip payment. +/// - OU: The handler for withdrawn `fee` and `tip`, passed in the respective order to +/// [OnUnbalanced::on_unbalanceds]. /// - `T`: The pallet's configuration. -pub struct SwapAssetAdapter(PhantomData<(A, F, S, OUF, OUT)>); +pub struct SwapAssetAdapter(PhantomData<(A, F, S, OU)>); -impl OnChargeAssetTransaction for SwapAssetAdapter +impl OnChargeAssetTransaction for SwapAssetAdapter where A: Get, F: fungibles::Balanced, AssetId = T::AssetId>, @@ -97,8 +97,7 @@ where AssetKind = T::AssetId, Credit = fungibles::Credit, > + QuotePrice, AssetKind = T::AssetId>, - OUF: OnUnbalanced>, - OUT: OnUnbalanced>, + OU: OnUnbalanced>, T: Config, { type AssetId = T::AssetId; @@ -264,8 +263,7 @@ where // Handle the imbalance (fee and tip separately). let (tip, fee) = adjusted_paid.split(tip); - OUF::on_unbalanced(fee); - OUT::on_unbalanced(tip); + OU::on_unbalanceds(Some(fee).into_iter().chain(Some(tip))); Ok(fee_in_asset) } } From 2b040351466e5b1533fbdea6ddb39de076f31176 Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 24 Jul 2024 11:30:11 +0200 Subject: [PATCH 15/16] fmt --- cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs | 3 +-- .../parachains/runtimes/assets/asset-hub-westend/src/lib.rs | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index 68045c2fd23a..83bc12cf9b41 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -542,8 +542,7 @@ impl InstanceFilter for ProxyType { RuntimeCall::Utility { .. } | RuntimeCall::Multisig { .. } | RuntimeCall::NftFractionalization { .. } | - RuntimeCall::Nfts { .. } | - RuntimeCall::Uniques { .. } + RuntimeCall::Nfts { .. } | RuntimeCall::Uniques { .. } ) }, ProxyType::AssetOwner => matches!( diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index 170d68f41c5b..2d9de07f251f 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -536,8 +536,7 @@ impl InstanceFilter for ProxyType { RuntimeCall::Utility { .. } | RuntimeCall::Multisig { .. } | RuntimeCall::NftFractionalization { .. } | - RuntimeCall::Nfts { .. } | - RuntimeCall::Uniques { .. } + RuntimeCall::Nfts { .. } | RuntimeCall::Uniques { .. } ) }, ProxyType::AssetOwner => matches!( From a3964fdbaf9b722ecc7ee090e571712ec0a8d1e1 Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 24 Jul 2024 15:14:32 +0200 Subject: [PATCH 16/16] update prdoc --- prdoc/pr_4488.prdoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/prdoc/pr_4488.prdoc b/prdoc/pr_4488.prdoc index 55f2998c462a..d0b6a877be6b 100644 --- a/prdoc/pr_4488.prdoc +++ b/prdoc/pr_4488.prdoc @@ -9,6 +9,9 @@ doc: This achieved by using `SwapCredit` implementation of asset conversion, which works with imbalances and does not require a temporary balance account within the transaction payment. + This is a breaking change for the `pallet-asset-conversion-tx-payment` pallet, use examples + from PR for the migration. + crates: - name: pallet-asset-conversion-tx-payment bump: major