diff --git a/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs b/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs index efc7386ae8c00..4946104418be9 100644 --- a/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs +++ b/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs @@ -148,6 +148,17 @@ pub mod pallet { } } +/// Holds parameters for if paying the transaction fee in the non-native asset. +#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] +#[scale_info(skip_type_params(T))] +pub struct PayByAsset { + /// The asset id to pay the fee in rather than the native asset. + id: ChargeAssetIdOf, + /// Effectively sets the max slippage that the user is willing to accept while allowing the + /// transaction to go ahead. + max_fee: Option>, +} + /// 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`. /// @@ -159,7 +170,7 @@ pub mod pallet { pub struct ChargeAssetTxPayment { #[codec(compact)] tip: BalanceOf, - asset_id: Option>, + asset: Option>, } impl ChargeAssetTxPayment @@ -174,8 +185,8 @@ where ChargeAssetIdOf: Send + Sync, { /// Utility constructor. Used only in client/factory code. - pub fn from(tip: BalanceOf, asset_id: Option>) -> Self { - Self { tip, asset_id } + pub fn from(tip: BalanceOf, asset: Option>) -> Self { + Self { tip, asset } } /// Fee withdrawal logic that dispatches to either `OnChargeAssetTransaction` or @@ -191,12 +202,13 @@ where debug_assert!(self.tip <= fee, "tip should be included in the computed fee"); if fee.is_zero() { Ok((fee, InitialPayment::Nothing)) - } else if let Some(asset_id) = &self.asset_id { + } else if let Some(PayByAsset { id, max_fee, .. }) = &self.asset { T::OnChargeAssetTransaction::withdraw_fee( who, call, info, - asset_id.clone(), + id.clone(), + *max_fee, fee.into(), self.tip.into(), ) @@ -223,7 +235,7 @@ where impl sp_std::fmt::Debug for ChargeAssetTxPayment { #[cfg(feature = "std")] fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { - write!(f, "ChargeAssetTxPayment<{:?}, {:?}>", self.tip, self.asset_id.encode()) + write!(f, "ChargeAssetTxPayment<{:?}, {:?}>", self.tip, self.asset.encode()) } #[cfg(not(feature = "std"))] fn fmt(&self, _: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { @@ -256,7 +268,7 @@ where // imbalance resulting from withdrawing the fee InitialPayment, // asset_id for the transaction payment - Option>, + Option>, ); fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> { @@ -284,7 +296,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, self.asset)) } fn post_dispatch( @@ -318,7 +330,7 @@ where len as u32, info, post_info, tip, ); - if let Some(asset_id) = asset_id { + if let Some(PayByAsset { id, .. }) = asset_id { let (used_for_fee, received_exchanged, asset_consumed) = already_withdrawn; let converted_fee = T::OnChargeAssetTransaction::correct_and_deposit_fee( &who, @@ -328,7 +340,7 @@ where tip.into(), used_for_fee.into(), received_exchanged.into(), - asset_id.clone(), + id.clone(), asset_consumed.into(), )?; @@ -336,7 +348,7 @@ where who, actual_fee: converted_fee, tip, - asset_id, + asset_id: id, }); } }, diff --git a/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs b/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs index 0d090211d0352..6af6fdb7e447a 100644 --- a/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs +++ b/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs @@ -47,6 +47,7 @@ pub trait OnChargeAssetTransaction { call: &T::RuntimeCall, dispatch_info: &DispatchInfoOf, asset_id: Self::AssetId, + max_fee: Option>, fee: Self::Balance, tip: Self::Balance, ) -> Result< @@ -84,7 +85,8 @@ where T: Config, C: Inspect<::AccountId>, CON: Swap, - T::HigherPrecisionBalance: From> + TryInto>, + T::HigherPrecisionBalance: + From> + From> + TryInto>, T::MultiAssetId: From>, BalanceOf: IsType<::AccountId>>::Balance>, { @@ -103,6 +105,7 @@ where call: &T::RuntimeCall, info: &DispatchInfoOf, asset_id: Self::AssetId, + max_fee: Option>, fee: BalanceOf, tip: BalanceOf, ) -> Result< @@ -118,7 +121,7 @@ where who.clone(), vec![asset_id.into(), T::MultiAssetIdConverter::get_native()], T::HigherPrecisionBalance::from(native_asset_required), - None, + max_fee.map(|fee| fee.into()), who.clone(), true, ) @@ -177,7 +180,7 @@ where ], T::HigherPrecisionBalance::from(swap_back), /* amount of the native asset to * convert to `asset_id` */ - None, // no minimum amount back + None, // because rate will be similar to pre-dispatch who.clone(), // we will refund to `who` false, // no need to keep alive ) diff --git a/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs b/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs index 9e9b74a0ddb2e..197a3a3bdf1a8 100644 --- a/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs +++ b/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs @@ -181,7 +181,16 @@ fn transaction_payment_in_native_possible() { } #[test] -fn transaction_payment_in_asset_possible() { +fn transaction_payment_in_asset_possible_without_slippage_set() { + transaction_payment_in_asset_possible_with_slippage(None); +} + +#[test] +fn transaction_payment_in_asset_possible_with_slippage_set() { + transaction_payment_in_asset_possible_with_slippage(Some(201)); +} + +fn transaction_payment_in_asset_possible_with_slippage(max_fee: Option) { let base_weight = 5; let balance_factor = 100; ExtBuilder::default() @@ -225,9 +234,12 @@ 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)) - .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_5), len) - .unwrap(); + let pre = ChargeAssetTxPayment::::from( + 0, + Some(PayByAsset { id: asset_id, max_fee }), + ) + .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_5), len) + .unwrap(); // assert that native balance is not used assert_eq!(Balances::free_balance(caller), 10 * balance_factor); @@ -249,7 +261,7 @@ fn transaction_payment_in_asset_possible() { } #[test] -fn transaction_payment_in_asset_fails_if_no_pool_for_that_asset() { +fn transaction_payment_in_asset_possible_fails_due_to_hitting_max_slippage() { let base_weight = 5; let balance_factor = 100; ExtBuilder::default() @@ -277,12 +289,72 @@ 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( - &caller, - CALL, - &info_from_weight(WEIGHT_5), - len, + let tx_weight = 5; + + setup_lp(asset_id, balance_factor); + + let fee_in_native = base_weight + tx_weight + len as u64; + let input_quote = AssetConversion::quote_price_tokens_for_exact_tokens( + NativeOrAssetId::Asset(asset_id), + NativeOrAssetId::Native, + fee_in_native, + true, ); + assert_eq!(input_quote, Some(201)); + + assert_eq!(Assets::balance(asset_id, caller), balance); + + assert!(ChargeAssetTxPayment::::from( + 0, + Some(PayByAsset { id: asset_id, max_fee: Some(200) }), + ) + .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_5), len) + .is_err()); + // assert that native balance is not used + assert_eq!(Balances::free_balance(caller), 10 * balance_factor); + + // check that fee was not charged in the given asset + assert_eq!(Assets::balance(asset_id, caller), balance); + + assert_eq!(TipUnbalancedAmount::get(), 0); + assert_eq!(FeeUnbalancedAmount::get(), 0); + }); +} + +#[test] +fn transaction_payment_in_asset_fails_if_no_pool_for_that_asset() { + 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(|| { + // 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 + )); + + // mint into the caller account + let caller = 1; + let beneficiary = ::Lookup::unlookup(caller); + let balance = 1000; + + assert_ok!(Assets::mint_into(asset_id.into(), &beneficiary, balance)); + assert_eq!(Assets::balance(asset_id, caller), balance); + + let len = 10; + let pre = ChargeAssetTxPayment::::from( + 0, + Some(PayByAsset { id: asset_id, max_fee: None }), + ) + .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_5), len); // As there is no pool in the dex set up for this asset, conversion should fail. assert!(pre.is_err()); @@ -332,9 +404,12 @@ 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)) - .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_5), len) - .unwrap(); + let pre = ChargeAssetTxPayment::::from( + 0, + Some(PayByAsset { id: asset_id, max_fee: None }), + ) + .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_5), len) + .unwrap(); // assert that native balance is not used assert_eq!(Balances::free_balance(caller), 10 * balance_factor); @@ -407,9 +482,12 @@ 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)) - .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_100), len) - .unwrap(); + let pre = ChargeAssetTxPayment::::from( + tip, + Some(PayByAsset { id: asset_id, max_fee: None }), + ) + .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_100), len) + .unwrap(); assert_eq!(Assets::balance(asset_id, caller), balance - fee_in_asset); let final_weight = 50; @@ -488,9 +566,12 @@ fn payment_from_account_with_only_assets() { .unwrap(); assert_eq!(fee_in_asset, 301); - let pre = ChargeAssetTxPayment::::from(0, Some(asset_id)) - .pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_5), len) - .unwrap(); + let pre = ChargeAssetTxPayment::::from( + 0, + Some(PayByAsset { id: asset_id, max_fee: None }), + ) + .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); @@ -553,9 +634,12 @@ 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)) - .pre_dispatch(&caller, CALL, &info_from_pays(Pays::No), len) - .unwrap(); + let pre = ChargeAssetTxPayment::::from( + 0, + Some(PayByAsset { id: asset_id, max_fee: None }), + ) + .pre_dispatch(&caller, CALL, &info_from_pays(Pays::No), len) + .unwrap(); // `Pays::No` implies there are no fees assert_eq!(Assets::balance(asset_id, caller), balance); @@ -579,9 +663,12 @@ fn converted_fee_is_never_zero_if_input_fee_is_not() { ) .unwrap(); - let pre = ChargeAssetTxPayment::::from(0, Some(asset_id)) - .pre_dispatch(&caller, CALL, &info_from_weight(Weight::from_parts(weight, 0)), len) - .unwrap(); + let pre = ChargeAssetTxPayment::::from( + 0, + Some(PayByAsset { id: asset_id, max_fee: None }), + ) + .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); assert_ok!(ChargeAssetTxPayment::::post_dispatch( @@ -629,9 +716,12 @@ 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)) - .pre_dispatch(&caller, CALL, &info_from_pays(Pays::No), len) - .unwrap(); + let pre = ChargeAssetTxPayment::::from( + 0, + Some(PayByAsset { id: asset_id, max_fee: None }), + ) + .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);