diff --git a/frame/asset-conversion/src/lib.rs b/frame/asset-conversion/src/lib.rs index 2acd0c12bd733..c1a086151ad36 100644 --- a/frame/asset-conversion/src/lib.rs +++ b/frame/asset-conversion/src/lib.rs @@ -298,8 +298,7 @@ pub mod pallet { } #[pallet::error] - pub enum Error - { + pub enum Error { /// Provided assets are equal. EqualAssets, /// Pool already exists. @@ -351,6 +350,10 @@ pub mod pallet { PathError, /// The provided path must consists of unique assets. NonUniquePath, + /// Unable to find an element in an array/vec that should have one-to-one correspondence + /// with another. For example, an array of assets constituting a `path` should have a + /// corresponding array of `amounts` along the path. + CorrespondenceError, } /// Pallet's callable functions. @@ -695,6 +698,7 @@ pub mod pallet { } impl Pallet { + /// Transfer an `amount` of `asset_id`, respecting the `keep_alive` requirements. fn transfer( asset_id: &T::MultiAssetId, from: &T::AccountId, @@ -713,8 +717,13 @@ pub mod pallet { true => Preserve, false => Expendable, }; - let amount = Self::asset_to_native(amount)?; - Ok(Self::native_to_asset(T::Currency::transfer(from, to, amount, preservation)?)?) + let amount = Self::convert_asset_balance_to_native_balance(amount)?; + Ok(Self::convert_native_balance_to_asset_balance(T::Currency::transfer( + from, + to, + amount, + preservation, + )?)?) } else { T::Assets::transfer( T::MultiAssetIdConverter::try_convert(&asset_id) @@ -727,18 +736,25 @@ pub mod pallet { } } - pub(crate) fn native_to_asset(amount: T::Balance) -> Result> { + /// Convert a `Balance` type to an `AssetBalance`. + pub(crate) fn convert_native_balance_to_asset_balance( + amount: T::Balance, + ) -> Result> { T::HigherPrecisionBalance::from(amount) .try_into() .map_err(|_| Error::::Overflow) } - pub(crate) fn asset_to_native(amount: T::AssetBalance) -> Result> { + /// Convert an `AssetBalance` type to a `Balance`. + pub(crate) fn convert_asset_balance_to_native_balance( + amount: T::AssetBalance, + ) -> Result> { T::HigherPrecisionBalance::from(amount) .try_into() .map_err(|_| Error::::Overflow) } + /// Swap assets along a `path`, depositing in `send_to`. pub(crate) fn do_swap( sender: &T::AccountId, amounts: &Vec, @@ -749,7 +765,8 @@ pub mod pallet { if let Some([asset1, asset2]) = path.get(0..2) { let pool_id = Self::get_pool_id(asset1.clone(), asset2.clone()); let pool_account = Self::get_pool_account(&pool_id); - let first_amount = amounts.first().expect("Always has more than one element"); + // amounts should always a corresponding element to path. + let first_amount = amounts.first().ok_or(Error::::CorrespondenceError)?; Self::transfer(asset1, sender, &pool_account, *first_amount, keep_alive)?; @@ -761,7 +778,7 @@ pub mod pallet { let pool_account = Self::get_pool_account(&pool_id); let amount_out = - amounts.get((i + 1) as usize).ok_or(Error::::PathError)?; + amounts.get((i + 1) as usize).ok_or(Error::::CorrespondenceError)?; let to = if i < path_len - 2 { let asset3 = path.get((i + 2) as usize).ok_or(Error::::PathError)?; @@ -797,14 +814,16 @@ pub mod pallet { .expect("infinite length input; no invalid inputs for type; qed") } + /// Get the `owner`'s balance of `asset`, which could be the chain's native asset or another + /// fungible. Returns a value in the form of an `AssetBalance`. fn get_balance( owner: &T::AccountId, asset: &T::MultiAssetId, ) -> Result> { if T::MultiAssetIdConverter::is_native(asset) { - Self::native_to_asset(<::Currency>::reducible_balance( - owner, Expendable, Polite, - )) + Self::convert_native_balance_to_asset_balance( + <::Currency>::reducible_balance(owner, Expendable, Polite), + ) } else { Ok(<::Assets>::reducible_balance( T::MultiAssetIdConverter::try_convert(asset) @@ -845,6 +864,7 @@ pub mod pallet { Ok((balance1, balance2)) } + /// Leading to an amount at the end of a `path`, get the required amounts in. pub(crate) fn get_amounts_in( amount_out: &T::AssetBalance, path: &BoundedVec, @@ -864,6 +884,7 @@ pub mod pallet { Ok(amounts) } + /// Following an amount into a `path`, get the corresponding amounts out. pub(crate) fn get_amounts_out( amount_in: &T::AssetBalance, path: &BoundedVec, @@ -973,10 +994,10 @@ pub mod pallet { result.try_into().map_err(|_| Error::::Overflow) } - /// Calculates amount out + /// Calculates amount out. /// /// Given an input amount of an asset and pair reserves, returns the maximum output amount - /// of the other asset + /// of the other asset. pub fn get_amount_out( amount_in: &T::AssetBalance, reserve_in: &T::AssetBalance, @@ -1008,10 +1029,10 @@ pub mod pallet { result.try_into().map_err(|_| Error::::Overflow) } - /// Calculates amount in + /// Calculates amount in. /// /// Given an output amount of an asset and pair reserves, returns a required input amount - /// of the other asset + /// of the other asset. pub fn get_amount_in( amount_out: &T::AssetBalance, reserve_in: &T::AssetBalance, @@ -1050,6 +1071,7 @@ pub mod pallet { result.try_into().map_err(|_| Error::::Overflow) } + /// Ensure that a `value` meets the minimum balance requirements of an `asset` class. fn validate_minimal_amount( value: T::AssetBalance, asset: &T::MultiAssetId, @@ -1068,6 +1090,7 @@ pub mod pallet { Ok(()) } + /// Ensure that a path is valid. fn validate_swap_path( path: &BoundedVec, ) -> Result<(), DispatchError> { @@ -1107,7 +1130,11 @@ where ::Currency: frame_support::traits::tokens::fungible::Inspect<::AccountId>, { - // If successful returns the amount in. + /// Take an `asset_id` and swap some amount for `amount_out` of the chain's native asset. If an + /// `amount_in_max` is specified, it will return an error if acquiring `amount_out` would be + /// too costly. + /// + /// If successful returns the amount of the `asset_id` taken to provide `amount_out`. fn swap_tokens_for_exact_native( sender: T::AccountId, asset_id: T::AssetId, @@ -1126,7 +1153,7 @@ where let path = path.try_into().unwrap(); // convert `amount_out` from native balance type, to asset balance type - let amount_out = Self::native_to_asset(amount_out)?; + let amount_out = Self::convert_native_balance_to_asset_balance(amount_out)?; // calculate the amount we need to provide let amounts = Self::get_amounts_in(&amount_out, &path)?; @@ -1148,7 +1175,11 @@ where Ok(amount_in) } - // If successful returns the amount out. + /// Take an `asset_id` and swap `amount_in` of the chain's native asset for it. If an + /// `amount_out_min` is specified, it will return an error if it is unable to acquire the amount + /// desired. + /// + /// If successful, returns the amount of `asset_id` acquired for the `amount_in`. fn swap_exact_native_for_tokens( sender: T::AccountId, asset_id: T::AssetId, @@ -1167,7 +1198,7 @@ where let path = path.try_into().expect("MaxSwapPathLength must be greater than 1"); // convert `amount_in` from native balance type, to asset balance type - let amount_in = Self::native_to_asset(amount_in)?; + let amount_in = Self::convert_native_balance_to_asset_balance(amount_in)?; // calculate the amount we should receive let amounts = Self::get_amounts_out(&amount_in, &path)?; diff --git a/frame/support/src/traits/tokens/fungibles/regular.rs b/frame/support/src/traits/tokens/fungibles/regular.rs index eaa891e073a24..3df0a6b6d3def 100644 --- a/frame/support/src/traits/tokens/fungibles/regular.rs +++ b/frame/support/src/traits/tokens/fungibles/regular.rs @@ -584,10 +584,16 @@ pub trait Balanced: Inspect + Unbalanced { fn done_withdraw(_asset: Self::AssetId, _who: &AccountId, _amount: Self::Balance) {} } -/// Trait for providing methods to swap the native token to an asset and back. +/// Trait for providing methods to swap between the chain's native token and other asset classes. pub trait SwapNative { - /// Swaps the provided amount of the `asset_id` into exact amount of native. - /// If successful, returns the amount of `asset_id` consumed to provide `amount_out`. + /// Take an `asset_id` and swap some amount for `amount_out` of the chain's native asset. If an + /// `amount_in_max` is specified, it will return an error if acquiring `amount_out` would be + /// too costly. + /// + /// Withdraws `asset_id` from `sender`, deposits native asset to `send_to`, respecting + /// `keep_alive`. + /// + /// If successful returns the amount of the `asset_id` taken to provide `amount_out`. fn swap_tokens_for_exact_native( sender: AccountId, asset_id: AssetId, @@ -597,7 +603,13 @@ pub trait SwapNative { keep_alive: bool, ) -> Result; - /// Swaps the provided exact amount of the native currency into the amount of `asset_id`. + /// Take an `asset_id` and swap `amount_in` of the chain's native asset for it. If an + /// `amount_out_min` is specified, it will return an error if it is unable to acquire the amount + /// desired. + /// + /// Withdraws native asset from `sender`, deposits `asset_id` to `send_to`, respecting + /// `keep_alive`. + /// /// If successful, returns the amount of `asset_id` acquired for the `amount_in`. fn swap_exact_native_for_tokens( sender: AccountId, diff --git a/frame/transaction-payment/asset-conversion-tx-payment/Cargo.toml b/frame/transaction-payment/asset-conversion-tx-payment/Cargo.toml index cdccaffd62cdd..0e8b28facb578 100644 --- a/frame/transaction-payment/asset-conversion-tx-payment/Cargo.toml +++ b/frame/transaction-payment/asset-conversion-tx-payment/Cargo.toml @@ -6,7 +6,7 @@ edition = "2021" license = "Apache-2.0" homepage = "https://substrate.io" repository = "https://github.com/paritytech/substrate/" -description = "Pallet to manage transaction payments in assets by utilising the assets conversion pallet" +description = "Pallet to manage transaction payments in assets by converting them to native assets." readme = "README.md" [package.metadata.docs.rs] 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 b184dd67a5c4e..8c0b8fdfae55c 100644 --- a/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs +++ b/frame/transaction-payment/asset-conversion-tx-payment/src/lib.rs @@ -19,7 +19,7 @@ //! chain's native asset. //! //! ## Overview - +//! //! 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 @@ -27,11 +27,18 @@ //! fee amount by converting the fee calculated by [`pallet-transaction-payment`] in the native //! asset into the amount required of the specified asset. //! -//! ## Integration - -//! This pallet wraps FRAME's Transaction Payment pallet and functions as a replacement. This means -//! you should include both pallets in your `construct_runtime` macro, but only include this -//! pallet's [`SignedExtension`] ([`ChargeAssetTxPayment`]). +//! ## Pallet API +//! +//! This pallet does not have any dispatchable calls or storage. It wraps FRAME's Transaction +//! Payment pallet and functions as a replacement. This means you should include both pallets in +//! your `construct_runtime` macro, but only include this pallet's [`SignedExtension`] +//! ([`ChargeAssetTxPayment`]). +//! +//! ## Terminology +//! +//! - Native Asset or Native Currency: The asset that a chain considers native, as in its default +//! for transaction fee payment, deposits, inflation, etc. +//! - Other assets: Other assets that may exist on chain, for example under the Assets pallet. #![cfg_attr(not(feature = "std"), no_std)] @@ -67,28 +74,26 @@ pub use payment::*; /// Type aliases used for interaction with `OnChargeTransaction`. pub(crate) type OnChargeTransactionOf = ::OnChargeTransaction; -/// Balance type alias. +/// 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; -/// Type alias used for interaction with fungibles (assets). -/// Balance type alias. +/// Balance type alias for balances of assets that implement the `fungibles` trait. pub(crate) type AssetBalanceOf = <::Fungibles as Inspect<::AccountId>>::Balance; -/// Asset id type alias. +/// Type alias for Asset IDs. pub(crate) type AssetIdOf = <::Fungibles as Inspect<::AccountId>>::AssetId; -/// Type aliases used for interaction with `OnChargeAssetTransaction`. -/// Balance type alias. +/// Type alias for the interaction of balances with `OnChargeAssetTransaction`. pub(crate) type ChargeAssetBalanceOf = <::OnChargeAssetTransaction as OnChargeAssetTransaction>::Balance; -/// Asset id type alias. +/// Type alias for Asset IDs in their interaction with `OnChargeAssetTransaction`. pub(crate) type ChargeAssetIdOf = <::OnChargeAssetTransaction as OnChargeAssetTransaction>::AssetId; -/// Liquidity info type alias. +/// Liquidity info type alias for interaction with `OnChargeAssetTransaction`. pub(crate) type ChargeAssetLiquidityOf = <::OnChargeAssetTransaction as OnChargeAssetTransaction>::LiquidityInfo; 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 04fb51bad26fe..dddc2b09a63a3 100644 --- a/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs +++ b/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs @@ -135,7 +135,7 @@ where total_swapped: Self::LiquidityInfo, asset_id: Self::AssetId, ) -> Result<(), TransactionValidityError> { - // Refund to the account that paid the fees. + // Refund the native asset to the account that paid the fees (`who`). ::correct_and_deposit_fee( who, dispatch_info, @@ -145,18 +145,19 @@ where already_paid, )?; + // amount, in native asset, to swap back to the desired `asset_id` let swap_back = total_swapped.saturating_sub(corrected_fee); 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. CON::swap_exact_native_for_tokens( - who.clone(), - asset_id, - swap_back, - None, - who.clone(), - false, + who.clone(), // we already deposited the native to `who` + asset_id, // 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 ) .ok(); }