Skip to content

Commit

Permalink
Drop ED Requirement for Transaction Payment with Exchangeable Asset (p…
Browse files Browse the repository at this point in the history
…olkadot-fellows#310)

Drop the Existential Deposit (ED) requirement for the asset amount
exchangeable for the fee asset (DOT/KSM) during transaction payments.

Currently, every swap during transaction payment, processed with asset
`A` for native asset, must be for an amount greater than the ED of a
native asset if the user lacks a native asset account. Since fees are
typically smaller, the current implementation necessitates additional
swaps to meet the ED during `pre_dispatch`, with refunds for excess ED
swap occurring during `post_dispatch`. Further details can be found
[here](https://github.com/paritytech/polkadot-sdk/blob/115c2477eb287df55107cd95594100ba395ed239/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs#L115).

This setup presents an issue where a user is unable to transfer their
entire balance and close the account. Instead, the user must transfer
slightly less of asset `A` to ensure there is enough not only for the
fee payment but also some extra to meet the ED requirement for their
native account during `pre_dispatch`. In some cases during
`post_dispatch`, the user will have the excess ED swapped back to asset
`A`, while in other cases, it may not be sufficient to meet the ED
requirement for asset `A`, leading it being left in the user's
'temporary' native asset account.
Example:
https://assethub-polkadot.subscan.io/extrinsic/6204018-9?event=6204018-79

Given the importance of this scenario for CEX, I propose a solution that
does not entail breaking changes to the pallets' API and open PR to the
runtimes without waiting for new polkadot-sdk version. Additionally, I
have opened a draft PR with these types in their respective pallets in
FRAME, where they have been tested against existing tests for types
implementing the same contract. PR -
paritytech/polkadot-sdk#4455

Target implementation with breaking changes:
paritytech/polkadot-sdk#4488

---------

Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
  • Loading branch information
muharem and joepetrowski authored May 19, 2024
1 parent 2f8a10e commit b6f99f2
Show file tree
Hide file tree
Showing 5 changed files with 611 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- Update the fellowship salary budget amount in alignment with the Fellowship Salary [RFC](https://github.com/polkadot-fellows/RFCs/pull/50) ([polkadot-fellows/runtimes#121](https://github.com/polkadot-fellows/runtimes/pull/121))
- Set up an account ID for the local root location on Polkadot Collectives ([polkadot-fellows/runtimes#125](https://github.com/polkadot-fellows/runtimes/pull/125))
- Increase confirmation period for treasury spend tracks on Polkadot & Kusama ([polkadot-fellows/runtimes#119](https://github.com/polkadot-fellows/runtimes/pull/119))
- Drop ED requirement for transaction payments with an exchangeable asset ([polkadot-fellows/runtimes#310](https://github.com/polkadot-fellows/runtimes/pull/310))

### Added

Expand Down
303 changes: 303 additions & 0 deletions system-parachains/asset-hubs/asset-hub-kusama/src/impls.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,303 @@
// Copyright (C) Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: Apache-2.0

// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use crate::*;

// TODO: move implementations to the polkadot-sdk.
pub mod tx_payment {
use super::*;
use core::marker::PhantomData;
use frame_support::{
ensure,
pallet_prelude::{InvalidTransaction, TransactionValidityError},
traits::{
fungibles::{Balanced as FungiblesBalanced, Inspect as FungiblesInspect},
tokens::{Fortitude, Precision, Preservation},
Defensive, OnUnbalanced, SameOrOther,
},
};
use pallet_asset_conversion::{Pallet as AssetConversion, SwapCredit};
use pallet_asset_conversion_tx_payment::OnChargeAssetTransaction;
use pallet_transaction_payment::OnChargeTransaction;
use sp_core::Get;
use sp_runtime::{
traits::{DispatchInfoOf, PostDispatchInfoOf, Zero},
Saturating,
};

/// Implements [`OnChargeTransaction`] for [`pallet_transaction_payment`], where the asset class
/// used to pay the fee is defined with the `A` type parameter (eg. KSM location) and accessed
/// via the type implementing the [`frame_support::traits::fungibles`] trait.
///
/// This implementation with the `fungibles` trait is necessary to set up
/// [`pallet_asset_conversion_tx_payment`] with the [`SwapCreditAdapter`] type. For both types,
/// the credit types they handle must be the same, therefore they must be credits of
/// `fungibles`.
pub struct FungiblesAdapter<F, A, OU>(PhantomData<(F, A, OU)>);

impl<T, F, A, OU> OnChargeTransaction<T> for FungiblesAdapter<F, A, OU>
where
T: pallet_transaction_payment::Config,
F: fungibles::Balanced<T::AccountId>,
A: Get<F::AssetId>,
OU: OnUnbalanced<fungibles::Credit<T::AccountId, F>>,
{
type LiquidityInfo = Option<fungibles::Credit<T::AccountId, F>>;
type Balance = F::Balance;

fn withdraw_fee(
who: &<T>::AccountId,
_call: &<T>::RuntimeCall,
_dispatch_info: &DispatchInfoOf<<T>::RuntimeCall>,
fee: Self::Balance,
_tip: Self::Balance,
) -> Result<Self::LiquidityInfo, TransactionValidityError> {
if fee.is_zero() {
return Ok(None)
}

match F::withdraw(
A::get(),
who,
fee,
Precision::Exact,
Preservation::Preserve,
Fortitude::Polite,
) {
Ok(imbalance) => Ok(Some(imbalance)),
Err(_) => Err(InvalidTransaction::Payment.into()),
}
}

fn correct_and_deposit_fee(
who: &<T>::AccountId,
_dispatch_info: &DispatchInfoOf<<T>::RuntimeCall>,
_post_info: &PostDispatchInfoOf<<T>::RuntimeCall>,
corrected_fee: Self::Balance,
_tip: Self::Balance,
already_withdrawn: Self::LiquidityInfo,
) -> Result<(), TransactionValidityError> {
let Some(paid) = already_withdrawn else {
return Ok(());
};
// Make sure the credit is in desired asset id.
ensure!(paid.asset() == A::get(), InvalidTransaction::Payment);
// Calculate how much refund we should return.
let refund_amount = paid.peek().saturating_sub(corrected_fee);
// Refund to the the account that paid the fees if it was not removed by the
// dispatched function. If fails for any reason (eg. ED requirement is not met) no
// refund given.
let refund_debt =
if F::total_balance(A::get(), who).is_zero() || refund_amount.is_zero() {
fungibles::Debt::<T::AccountId, F>::zero(A::get())
} else {
F::deposit(A::get(), who, refund_amount, Precision::BestEffort)
.unwrap_or_else(|_| fungibles::Debt::<T::AccountId, F>::zero(A::get()))
};
// Merge the imbalance caused by paying the fees and refunding parts of it again.
let adjusted_paid: fungibles::Credit<T::AccountId, F> =
match paid.offset(refund_debt).defensive_proof("credits should be identical") {
Ok(SameOrOther::Same(credit)) => credit,
// Paid amount is fully refunded.
Ok(SameOrOther::None) => fungibles::Credit::<T::AccountId, F>::zero(A::get()),
// Should never fail as at this point the asset id is always valid and the
// refund amount is not greater than paid amount.
_ => return Err(InvalidTransaction::Payment.into()),
};
// No separation for simplicity.
// In our case the fees and the tips are deposited to the same pot.
// We cannot call [`OnUnbalanced::on_unbalanceds`] since fungibles credit does not
// implement `Imbalanced` trait.
OU::on_unbalanced(adjusted_paid);
Ok(())
}
}

type LiquidityInfoOf<T> =
<<T as pallet_transaction_payment::Config>::OnChargeTransaction as OnChargeTransaction<
T,
>>::LiquidityInfo;

/// Implements [`OnChargeAssetTransaction`] for [`pallet_asset_conversion_tx_payment`], where
/// the asset class used to pay the fee is defined with the `A` type parameter (eg. DOT
/// location) and accessed via the type implementing the [`frame_support::traits::fungibles`]
/// trait.
pub struct SwapCreditAdapter<A, S>(PhantomData<(A, S)>);
impl<A, S, T> OnChargeAssetTransaction<T> for SwapCreditAdapter<A, S>
where
A: Get<S::AssetKind>,
S: SwapCredit<
T::AccountId,
Balance = T::Balance,
AssetKind = T::AssetKind,
Credit = fungibles::Credit<T::AccountId, T::Assets>,
>,

T: pallet_asset_conversion_tx_payment::Config,
T::Fungibles:
fungibles::Inspect<T::AccountId, Balance = T::Balance, AssetId = T::AssetKind>,
T::OnChargeTransaction:
OnChargeTransaction<T, Balance = T::Balance, LiquidityInfo = Option<S::Credit>>,
{
type AssetId = T::AssetKind;
type Balance = T::Balance;
type LiquidityInfo = T::Balance;

fn withdraw_fee(
who: &<T>::AccountId,
_call: &<T>::RuntimeCall,
_dispatch_info: &DispatchInfoOf<<T>::RuntimeCall>,
asset_id: Self::AssetId,
fee: Self::Balance,
_tip: Self::Balance,
) -> Result<(LiquidityInfoOf<T>, Self::LiquidityInfo, T::Balance), TransactionValidityError>
{
let asset_fee = AssetConversion::<T>::quote_price_tokens_for_exact_tokens(
asset_id.clone(),
A::get(),
fee,
true,
)
.ok_or(InvalidTransaction::Payment)?;

let asset_fee_credit = T::Assets::withdraw(
asset_id.clone(),
who,
asset_fee,
Precision::Exact,
Preservation::Preserve,
Fortitude::Polite,
)
.map_err(|_| TransactionValidityError::from(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, _)) => {
// The swap should not error since the price quote was successful.
let _ = T::Assets::resolve(who, credit_in).defensive();
return Err(InvalidTransaction::Payment.into())
},
};

// Should be always zero since the exact price was quoted before.
ensure!(change.peek().is_zero(), InvalidTransaction::Payment);

Ok((Some(fee_credit), fee, asset_fee))
}
fn correct_and_deposit_fee(
who: &<T>::AccountId,
dispatch_info: &DispatchInfoOf<<T>::RuntimeCall>,
post_info: &PostDispatchInfoOf<<T>::RuntimeCall>,
corrected_fee: Self::Balance,
tip: Self::Balance,
fee_paid: LiquidityInfoOf<T>,
_received_exchanged: Self::LiquidityInfo,
asset_id: Self::AssetId,
initial_asset_consumed: T::Balance,
) -> Result<T::Balance, TransactionValidityError> {
let Some(fee_paid) = fee_paid else {
return Ok(Zero::zero());
};
// Try to refund if the fee paid is more than the corrected fee and the account was not
// removed by the dispatched function.
let (fee, fee_in_asset) = if fee_paid.peek() > corrected_fee &&
!T::Assets::total_balance(asset_id.clone(), who).is_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 =
AssetConversion::<T>::quote_price_exact_tokens_for_tokens(
A::get(),
asset_id.clone(),
refund_amount,
true,
)
// 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 T::Assets::deposit(
asset_id.clone(),
who,
refund_asset_amount,
Precision::BestEffort,
) {
Ok(debt) => debt,
// No refund given since it cannot be deposited.
Err(_) => fungibles::Debt::<T::AccountId, T::Assets>::zero(asset_id.clone()),
};

if debt.peek().is_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))
},
// The error should not occur since swap was quoted before.
Err((refund, _)) => {
match T::Assets::settle(who, debt, Preservation::Expendable) {
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()),
};
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.
TransactionValidityError::from(InvalidTransaction::Payment)
})?;
(fee_paid, initial_asset_consumed)
},
}
}
} else {
(fee_paid, initial_asset_consumed)
};

// Refund is already processed.
let corrected_fee = fee.peek();
// Deposit fee.
T::OnChargeTransaction::correct_and_deposit_fee(
who,
dispatch_info,
post_info,
corrected_fee,
tip,
Some(fee),
)
.map(|_| fee_in_asset)
}
}
}
16 changes: 10 additions & 6 deletions system-parachains/asset-hubs/asset-hub-kusama/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#[cfg(feature = "std")]
include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs"));

mod impls;
mod weights;
pub mod xcm_config;

Expand Down Expand Up @@ -69,11 +70,10 @@ use frame_system::{
limits::{BlockLength, BlockWeights},
EnsureRoot, EnsureSigned, EnsureSignedBy,
};
use pallet_asset_conversion_tx_payment::AssetConversionAdapter;
use pallet_nfts::PalletFeatures;
use parachains_common::{
impls::DealWithFees, message_queue::*, AccountId, AssetIdForTrustBackedAssets, AuraId, Balance,
BlockNumber, Hash, Header, Nonce, Signature,
message_queue::*, AccountId, AssetIdForTrustBackedAssets, AuraId, Balance, BlockNumber, Hash,
Header, Nonce, Signature,
};
use sp_runtime::RuntimeDebug;
pub use system_parachains_constants::SLOT_DURATION;
Expand Down Expand Up @@ -254,12 +254,16 @@ impl pallet_vesting::Config for Runtime {
parameter_types! {
/// Relay Chain `TransactionByteFee` / 10
pub const TransactionByteFee: Balance = system_parachains_constants::kusama::fee::TRANSACTION_BYTE_FEE;
pub StakingPot: AccountId = CollatorSelection::account_id();
}

impl pallet_transaction_payment::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
type OnChargeTransaction =
pallet_transaction_payment::CurrencyAdapter<Balances, DealWithFees<Runtime>>;
type OnChargeTransaction = impls::tx_payment::FungiblesAdapter<
NativeAndAssets,
KsmLocationV3,
ResolveAssetTo<StakingPot, NativeAndAssets>,
>;
type WeightToFee = WeightToFee;
type LengthToFee = ConstantMultiplier<Balance, TransactionByteFee>;
type FeeMultiplierUpdate = SlowAdjustingFeeUpdate<Self>;
Expand Down Expand Up @@ -807,7 +811,7 @@ impl pallet_asset_conversion_tx_payment::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
type Fungibles = LocalAndForeignAssets;
type OnChargeAssetTransaction =
AssetConversionAdapter<Balances, AssetConversion, KsmLocationV3>;
impls::tx_payment::SwapCreditAdapter<KsmLocationV3, AssetConversion>;
}

parameter_types! {
Expand Down
Loading

0 comments on commit b6f99f2

Please sign in to comment.