Refactor the asset-conversion-tx-payment pallet (#14558)

* Code refactoring

* Fix imports

* Typo

* Update frame/asset-conversion/src/types.rs

Co-authored-by: joe petrowski <[email protected]>

* Sync docs

---------

Co-authored-by: parity-processbot <>
Co-authored-by: joe petrowski <[email protected]>

frame/asset-conversion/src/lib.rs

@@ -81,11 +81,10 @@ use sp_arithmetic::traits::Unsigned;
 use sp_runtime::{
  traits::{
  CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, Ensure, MaybeDisplay, TrailingZeroInput,
- Zero,
  },
  DispatchError,
 };
-use sp_std::vec;
+use sp_std::prelude::*;
 pub use types::*;
 pub use weights::WeightInfo;
 
@@ -111,7 +110,6 @@ pub mod pallet {
  traits::{IntegerSquareRoot, One, Zero},
  Saturating,
  };
- use sp_std::prelude::*;
 
  #[pallet::pallet]
  pub struct Pallet<T>(_);
@@ -148,7 +146,7 @@ pub mod pallet {
  type AssetId: AssetId + PartialOrd;
 
  /// Type that identifies either the native currency or a token class from `Assets`.
- type MultiAssetId: AssetId + Ord;
+ type MultiAssetId: AssetId + Ord + From<Self::AssetId>;
 
  /// Type to convert an `AssetId` into `MultiAssetId`.
  type MultiAssetIdConverter: MultiAssetIdConverter<Self::MultiAssetId, Self::AssetId>;
@@ -644,18 +642,14 @@ pub mod pallet {
  keep_alive: bool,
  ) -> DispatchResult {
  let sender = ensure_signed(origin)?;
-
- ensure!(
- amount_in > Zero::zero() && amount_out_min > Zero::zero(),
- Error::<T>::ZeroAmount
- );
- Self::validate_swap_path(&path)?;
-
- let amounts = Self::get_amounts_out(&amount_in, &path)?;
- let amount_out = *amounts.last().expect("Has always more than 1 element");
- ensure!(amount_out >= amount_out_min, Error::<T>::ProvidedMinimumNotSufficientForSwap);
-
- Self::do_swap(sender, &amounts, path, send_to, keep_alive)?;
+ Self::do_swap_exact_tokens_for_tokens(
+ sender,
+ path,
+ amount_in,
+ Some(amount_out_min),
+ send_to,
+ keep_alive,
+ )?;
  Ok(())
  }
 
@@ -676,23 +670,95 @@ pub mod pallet {
  keep_alive: bool,
  ) -> DispatchResult {
  let sender = ensure_signed(origin)?;
+ Self::do_swap_tokens_for_exact_tokens(
+ sender,
+ path,
+ amount_out,
+ Some(amount_in_max),
+ send_to,
+ keep_alive,
+ )?;
+ Ok(())
+ }
+ }
+
+ impl<T: Config> Pallet<T> {
+ /// Swap exactly `amount_in` of asset `path[0]` for asset `path[1]`.
+ /// If an `amount_out_min` is specified, it will return an error if it is unable to acquire
+ /// the amount desired.
+ ///
+ /// Withdraws the `path[0]` asset from `sender`, deposits the `path[1]` asset to `send_to`,
+ /// respecting `keep_alive`.
+ ///
+ /// If successful, returns the amount of `path[1]` acquired for the `amount_in`.
+ pub fn do_swap_exact_tokens_for_tokens(
+ sender: T::AccountId,
+ path: BoundedVec<T::MultiAssetId, T::MaxSwapPathLength>,
+ amount_in: T::AssetBalance,
+ amount_out_min: Option<T::AssetBalance>,
+ send_to: T::AccountId,
+ keep_alive: bool,
+ ) -> Result<T::AssetBalance, DispatchError> {
+ ensure!(amount_in > Zero::zero(), Error::<T>::ZeroAmount);
+ if let Some(amount_out_min) = amount_out_min {
+ ensure!(amount_out_min > Zero::zero(), Error::<T>::ZeroAmount);
+ }
+
+ Self::validate_swap_path(&path)?;
+
+ let amounts = Self::get_amounts_out(&amount_in, &path)?;
+ let amount_out =
+ *amounts.last().defensive_ok_or("get_amounts_out() returned an empty result")?;
+
+ if let Some(amount_out_min) = amount_out_min {
+ ensure!(
+ amount_out >= amount_out_min,
+ Error::<T>::ProvidedMinimumNotSufficientForSwap
+ );
+ }
+
+ Self::do_swap(sender, &amounts, path, send_to, keep_alive)?;
+ Ok(amount_out)
+ }
+
+ /// Take the `path[0]` asset and swap some amount for `amount_out` of the `path[1]`. If an
+ /// `amount_in_max` is specified, it will return an error if acquiring `amount_out` would be
+ /// too costly.
+ ///
+ /// Withdraws `path[0]` asset from `sender`, deposits the `path[1]` asset to `send_to`,
+ /// respecting `keep_alive`.
+ ///
+ /// If successful returns the amount of the `path[0]` taken to provide `path[1]`.
+ pub fn do_swap_tokens_for_exact_tokens(
+ sender: T::AccountId,
+ path: BoundedVec<T::MultiAssetId, T::MaxSwapPathLength>,
+ amount_out: T::AssetBalance,
+ amount_in_max: Option<T::AssetBalance>,
+ send_to: T::AccountId,
+ keep_alive: bool,
+ ) -> Result<T::AssetBalance, DispatchError> {
+ ensure!(amount_out > Zero::zero(), Error::<T>::ZeroAmount);
+ if let Some(amount_in_max) = amount_in_max {
+ ensure!(amount_in_max > Zero::zero(), Error::<T>::ZeroAmount);
+ }
 
- ensure!(
- amount_out > Zero::zero() && amount_in_max > Zero::zero(),
- Error::<T>::ZeroAmount
- );
  Self::validate_swap_path(&path)?;
 
  let amounts = Self::get_amounts_in(&amount_out, &path)?;
- let amount_in = *amounts.first().expect("Always has more than one element");
- ensure!(amount_in <= amount_in_max, Error::<T>::ProvidedMaximumNotSufficientForSwap);
+ let amount_in =
+ *amounts.first().defensive_ok_or("get_amounts_in() returned an empty result")?;
+
+ if let Some(amount_in_max) = amount_in_max {
+ ensure!(
+ amount_in <= amount_in_max,
+ Error::<T>::ProvidedMaximumNotSufficientForSwap
+ );
+ }
 
  Self::do_swap(sender, &amounts, path, send_to, keep_alive)?;
- Ok(())
+ Ok(amount_in)
  }
- }
 
- impl<T: Config> Pallet<T> {
  /// Transfer an `amount` of `asset_id`, respecting the `keep_alive` requirements.
  fn transfer(
  asset_id: &T::MultiAssetId,
@@ -749,6 +815,13 @@ pub mod pallet {
  .map_err(|_| Error::<T>::Overflow)
  }
 
+ /// Convert a `HigherPrecisionBalance` type to an `AssetBalance`.
+ pub(crate) fn convert_hpb_to_asset_balance(
+ amount: T::HigherPrecisionBalance,
+ ) -> Result<T::AssetBalance, Error<T>> {
+ amount.try_into().map_err(|_| Error::<T>::Overflow)
+ }
+
  /// Swap assets along a `path`, depositing in `send_to`.
  pub(crate) fn do_swap(
  sender: T::AccountId,
@@ -1123,92 +1196,47 @@ pub mod pallet {
  }
 }
 
-impl<T: Config>
- frame_support::traits::tokens::fungibles::SwapNative<
- T::RuntimeOrigin,
- T::AccountId,
- T::Balance,
- T::AssetBalance,
- T::AssetId,
- > for Pallet<T>
-where
- <T as pallet::Config>::Currency:
- frame_support::traits::tokens::fungible::Inspect<<T as frame_system::Config>::AccountId>,
-{
- /// 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(
+impl<T: Config> Swap<T::AccountId, T::HigherPrecisionBalance, T::MultiAssetId> for Pallet<T> {
+ fn swap_exact_tokens_for_tokens(
  sender: T::AccountId,
- asset_id: T::AssetId,
- amount_out: T::Balance,
- amount_in_max: Option<T::AssetBalance>,
+ path: Vec<T::MultiAssetId>,
+ amount_in: T::HigherPrecisionBalance,
+ amount_out_min: Option<T::HigherPrecisionBalance>,
  send_to: T::AccountId,
  keep_alive: bool,
- ) -> Result<T::AssetBalance, DispatchError> {
- ensure!(amount_out > Zero::zero(), Error::<T>::ZeroAmount);
- if let Some(amount_in_max) = amount_in_max {
- ensure!(amount_in_max > Zero::zero(), Error::<T>::ZeroAmount);
- }
- let mut path = sp_std::vec::Vec::new();
- path.push(T::MultiAssetIdConverter::into_multiasset_id(&asset_id));
- path.push(T::MultiAssetIdConverter::get_native());
- let path = path.try_into().unwrap();
-
- // convert `amount_out` from native balance type, to asset balance type
- 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)?;
- let amount_in =
- *amounts.first().defensive_ok_or("get_amounts_in() returned an empty result")?;
- if let Some(amount_in_max) = amount_in_max {
- ensure!(amount_in <= amount_in_max, Error::<T>::ProvidedMaximumNotSufficientForSwap);
- }
-
- Self::do_swap(sender, &amounts, path, send_to, keep_alive)?;
- Ok(amount_in)
+ ) -> Result<T::HigherPrecisionBalance, DispatchError> {
+ let path = path.try_into().map_err(|_| Error::<T>::PathError)?;
+ let amount_out_min = amount_out_min.map(Self::convert_hpb_to_asset_balance).transpose()?;
+ let amount_out = Self::do_swap_exact_tokens_for_tokens(
+ sender,
+ path,
+ Self::convert_hpb_to_asset_balance(amount_in)?,
+ amount_out_min,
+ send_to,
+ keep_alive,
+ )?;
+ Ok(amount_out.into())
  }
 
- /// 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(
+ fn swap_tokens_for_exact_tokens(
  sender: T::AccountId,
- asset_id: T::AssetId,
- amount_in: T::Balance,
- amount_out_min: Option<T::AssetBalance>,
+ path: Vec<T::MultiAssetId>,
+ amount_out: T::HigherPrecisionBalance,
+ amount_in_max: Option<T::HigherPrecisionBalance>,
  send_to: T::AccountId,
  keep_alive: bool,
- ) -> Result<T::AssetBalance, DispatchError> {
- ensure!(amount_in > Zero::zero(), Error::<T>::ZeroAmount);
- if let Some(amount_out_min) = amount_out_min {
- ensure!(amount_out_min > Zero::zero(), Error::<T>::ZeroAmount);
- }
- let mut path = sp_std::vec::Vec::new();
- path.push(T::MultiAssetIdConverter::get_native());
- path.push(T::MultiAssetIdConverter::into_multiasset_id(&asset_id));
- let path = path.try_into().expect(
- "`MaxSwapPathLength` is ensured by to be greater than 2; pushed only twice; qed",
- );
-
- // convert `amount_in` from native balance type, to asset balance type
- 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)?;
- let amount_out =
- *amounts.last().defensive_ok_or("get_amounts_out() returned an empty result")?;
- if let Some(amount_out_min) = amount_out_min {
- ensure!(amount_out >= amount_out_min, Error::<T>::ProvidedMaximumNotSufficientForSwap);
- }
-
- Self::do_swap(sender, &amounts, path, send_to, keep_alive)?;
- Ok(amount_out)
+ ) -> Result<T::HigherPrecisionBalance, DispatchError> {
+ let path = path.try_into().map_err(|_| Error::<T>::PathError)?;
+ let amount_in_max = amount_in_max.map(Self::convert_hpb_to_asset_balance).transpose()?;
+ let amount_in = Self::do_swap_tokens_for_exact_tokens(
+ sender,
+ path,
+ Self::convert_hpb_to_asset_balance(amount_out)?,
+ amount_in_max,
+ send_to,
+ keep_alive,
+ )?;
+ Ok(amount_in.into())
  }
 }
 

frame/asset-conversion/src/types.rs

@@ -33,7 +33,7 @@ pub struct PoolInfo<PoolAssetId> {
 
 /// A trait that converts between a MultiAssetId and either the native currency or an AssetId.
 pub trait MultiAssetIdConverter<MultiAssetId, AssetId> {
- /// Returns the MultiAssetId reperesenting the native currency of the chain.
+ /// Returns the MultiAssetId representing the native currency of the chain.
  fn get_native() -> MultiAssetId;
 
  /// Returns true if the given MultiAssetId is the native currency.
@@ -42,7 +42,7 @@ pub trait MultiAssetIdConverter<MultiAssetId, AssetId> {
  /// If it's not native, returns the AssetId for the given MultiAssetId.
  fn try_convert(asset: &MultiAssetId) -> Result<AssetId, ()>;
 
- /// Wrapps an AssetId as a MultiAssetId.
+ /// Wraps an AssetId as a MultiAssetId.
  fn into_multiasset_id(asset: &AssetId) -> MultiAssetId;
 }
 
@@ -76,6 +76,12 @@ where
  Asset(AssetId),
 }
 
+impl<AssetId: Ord> From<AssetId> for NativeOrAssetId<AssetId> {
+ fn from(asset: AssetId) -> Self {
+ Self::Asset(asset)
+ }
+}
+
 impl<AssetId: Ord> Ord for NativeOrAssetId<AssetId> {
  fn cmp(&self, other: &Self) -> Ordering {
  match (self, other) {
@@ -126,3 +132,40 @@ impl<AssetId: Ord + Clone> MultiAssetIdConverter<NativeOrAssetId<AssetId>, Asset
  NativeOrAssetId::Asset((*asset).clone())
  }
 }
+
+/// Trait for providing methods to swap between the various asset classes.
+pub trait Swap<AccountId, Balance, MultiAssetId> {
+ /// Swap exactly `amount_in` of asset `path[0]` for asset `path[1]`.
+ /// If an `amount_out_min` is specified, it will return an error if it is unable to acquire
+ /// the amount desired.
+ ///
+ /// Withdraws the `path[0]` asset from `sender`, deposits the `path[1]` asset to `send_to`,
+ /// respecting `keep_alive`.
+ ///
+ /// If successful, returns the amount of `path[1]` acquired for the `amount_in`.
+ fn swap_exact_tokens_for_tokens(
+ sender: AccountId,
+ path: Vec<MultiAssetId>,
+ amount_in: Balance,
+ amount_out_min: Option<Balance>,
+ send_to: AccountId,
+ keep_alive: bool,
+ ) -> Result<Balance, DispatchError>;
+
+ /// Take the `path[0]` asset and swap some amount for `amount_out` of the `path[1]`. If an
+ /// `amount_in_max` is specified, it will return an error if acquiring `amount_out` would be
+ /// too costly.
+ ///
+ /// Withdraws `path[0]` asset from `sender`, deposits `path[1]` asset to `send_to`,
+ /// respecting `keep_alive`.
+ ///
+ /// If successful returns the amount of the `path[0]` taken to provide `path[1]`.
+ fn swap_tokens_for_exact_tokens(
+ sender: AccountId,
+ path: Vec<MultiAssetId>,
+ amount_out: Balance,
+ amount_in_max: Option<Balance>,
+ send_to: AccountId,
+ keep_alive: bool,
+ ) -> Result<Balance, DispatchError>;
+}

frame/support/src/traits/tokens/fungibles/mod.rs

@@ -36,5 +36,5 @@ pub use hold::{
 pub use imbalance::{Credit, Debt, HandleImbalanceDrop, Imbalance};
 pub use lifetime::{Create, Destroy};
 pub use regular::{
- Balanced, DecreaseIssuance, Dust, IncreaseIssuance, Inspect, Mutate, SwapNative, Unbalanced,
+ Balanced, DecreaseIssuance, Dust, IncreaseIssuance, Inspect, Mutate, Unbalanced,
 };