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 4616cfc0991c83c6e818adec59bb6298766fc2bd..ae56a3f93831b67d91b4f442c06ceeef7f8f1dbe 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 0acce3fe620e07687e1032897ead11958cbf1f40..c935dbd51247a4ecf8d6f2316188446679aef12e 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<Native, NativeAndAssets, AssetConversion, DealWithFee, DealWithTip>; + SwapAssetAdapter<Native, NativeAndAssets, AssetConversion, DealWithFee, DealWithTip>; } 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 a7f216387331eba64fc4aa7808076a25e2fd7761..6147eb15076d510fa4473c9e9786cc38177934d0 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<BalanceOf<T>, 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::<T::AccountId, F>::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).