Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

allow slippage to be set when pay by swap #14680

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 23 additions & 11 deletions frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T: Config> {
/// The asset id to pay the fee in rather than the native asset.
id: ChargeAssetIdOf<T>,
/// Effectively sets the max slippage that the user is willing to accept while allowing the
/// transaction to go ahead.
max_fee: Option<AssetBalanceOf<T>>,
}

/// 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`.
///
Expand All @@ -159,7 +170,7 @@ pub mod pallet {
pub struct ChargeAssetTxPayment<T: Config> {
#[codec(compact)]
tip: BalanceOf<T>,
asset_id: Option<ChargeAssetIdOf<T>>,
asset: Option<PayByAsset<T>>,
}

impl<T: Config> ChargeAssetTxPayment<T>
Expand All @@ -174,8 +185,8 @@ where
ChargeAssetIdOf<T>: Send + Sync,
{
/// Utility constructor. Used only in client/factory code.
pub fn from(tip: BalanceOf<T>, asset_id: Option<ChargeAssetIdOf<T>>) -> Self {
Self { tip, asset_id }
pub fn from(tip: BalanceOf<T>, asset: Option<PayByAsset<T>>) -> Self {
Self { tip, asset }
}

/// Fee withdrawal logic that dispatches to either `OnChargeAssetTransaction` or
Expand All @@ -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(),
)
Expand All @@ -223,7 +235,7 @@ where
impl<T: Config> sp_std::fmt::Debug for ChargeAssetTxPayment<T> {
#[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 {
Expand Down Expand Up @@ -256,7 +268,7 @@ where
// imbalance resulting from withdrawing the fee
InitialPayment<T>,
// asset_id for the transaction payment
Option<ChargeAssetIdOf<T>>,
Option<PayByAsset<T>>,
);

fn additional_signed(&self) -> sp_std::result::Result<(), TransactionValidityError> {
Expand Down Expand Up @@ -284,7 +296,7 @@ where
len: usize,
) -> Result<Self::Pre, TransactionValidityError> {
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(
Expand Down Expand Up @@ -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,
Expand All @@ -328,15 +340,15 @@ where
tip.into(),
used_for_fee.into(),
received_exchanged.into(),
asset_id.clone(),
id.clone(),
asset_consumed.into(),
)?;

Pallet::<T>::deposit_event(Event::<T>::AssetTxFeePaid {
who,
actual_fee: converted_fee,
tip,
asset_id,
asset_id: id,
});
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ pub trait OnChargeAssetTransaction<T: Config> {
call: &T::RuntimeCall,
dispatch_info: &DispatchInfoOf<T::RuntimeCall>,
asset_id: Self::AssetId,
max_fee: Option<AssetBalanceOf<T>>,
fee: Self::Balance,
tip: Self::Balance,
) -> Result<
Expand Down Expand Up @@ -84,7 +85,8 @@ where
T: Config,
C: Inspect<<T as frame_system::Config>::AccountId>,
CON: Swap<T::AccountId, T::HigherPrecisionBalance, T::MultiAssetId>,
T::HigherPrecisionBalance: From<BalanceOf<T>> + TryInto<AssetBalanceOf<T>>,
T::HigherPrecisionBalance:
From<BalanceOf<T>> + From<AssetBalanceOf<T>> + TryInto<AssetBalanceOf<T>>,
T::MultiAssetId: From<AssetIdOf<T>>,
BalanceOf<T>: IsType<<C as Inspect<<T as frame_system::Config>::AccountId>>::Balance>,
{
Expand All @@ -103,6 +105,7 @@ where
call: &T::RuntimeCall,
info: &DispatchInfoOf<T::RuntimeCall>,
asset_id: Self::AssetId,
max_fee: Option<AssetBalanceOf<T>>,
fee: BalanceOf<T>,
tip: BalanceOf<T>,
) -> Result<
Expand All @@ -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,
)
Expand Down Expand Up @@ -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
)
Expand Down
146 changes: 118 additions & 28 deletions frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u64>) {
let base_weight = 5;
let balance_factor = 100;
ExtBuilder::default()
Expand Down Expand Up @@ -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::<Runtime>::from(0, Some(asset_id))
.pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_5), len)
.unwrap();
let pre = ChargeAssetTxPayment::<Runtime>::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);

Expand All @@ -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()
Expand Down Expand Up @@ -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::<Runtime>::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::<Runtime>::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 = <Runtime as system::Config>::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::<Runtime>::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());
Expand Down Expand Up @@ -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::<Runtime>::from(0, Some(asset_id))
.pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_5), len)
.unwrap();
let pre = ChargeAssetTxPayment::<Runtime>::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);
Expand Down Expand Up @@ -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::<Runtime>::from(tip, Some(asset_id))
.pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_100), len)
.unwrap();
let pre = ChargeAssetTxPayment::<Runtime>::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;
Expand Down Expand Up @@ -488,9 +566,12 @@ fn payment_from_account_with_only_assets() {
.unwrap();
assert_eq!(fee_in_asset, 301);

let pre = ChargeAssetTxPayment::<Runtime>::from(0, Some(asset_id))
.pre_dispatch(&caller, CALL, &info_from_weight(WEIGHT_5), len)
.unwrap();
let pre = ChargeAssetTxPayment::<Runtime>::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);
Expand Down Expand Up @@ -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::<Runtime>::from(0, Some(asset_id))
.pre_dispatch(&caller, CALL, &info_from_pays(Pays::No), len)
.unwrap();
let pre = ChargeAssetTxPayment::<Runtime>::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);

Expand All @@ -579,9 +663,12 @@ fn converted_fee_is_never_zero_if_input_fee_is_not() {
)
.unwrap();

let pre = ChargeAssetTxPayment::<Runtime>::from(0, Some(asset_id))
.pre_dispatch(&caller, CALL, &info_from_weight(Weight::from_parts(weight, 0)), len)
.unwrap();
let pre = ChargeAssetTxPayment::<Runtime>::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::<Runtime>::post_dispatch(
Expand Down Expand Up @@ -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::<Runtime>::from(0, Some(asset_id))
.pre_dispatch(&caller, CALL, &info_from_pays(Pays::No), len)
.unwrap();
let pre = ChargeAssetTxPayment::<Runtime>::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);
Expand Down