From 39935e96c255657f2d41125cbe4328a944bd2807 Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 15 Sep 2023 12:28:22 +0200 Subject: [PATCH 01/43] move swap trait and impl into separate module --- substrate/frame/asset-conversion/src/lib.rs | 62 ++--------- substrate/frame/asset-conversion/src/swap.rs | 101 ++++++++++++++++++ substrate/frame/asset-conversion/src/types.rs | 37 ------- 3 files changed, 110 insertions(+), 90 deletions(-) create mode 100644 substrate/frame/asset-conversion/src/swap.rs diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index 8d811473e861..a3f5159743ec 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -57,15 +57,18 @@ use frame_support::traits::{DefensiveOption, Incrementable}; #[cfg(feature = "runtime-benchmarks")] mod benchmarking; - -mod types; -pub mod weights; - +#[cfg(test)] +mod mock; +mod swap; #[cfg(test)] mod tests; +mod types; +pub mod weights; -#[cfg(test)] -mod mock; +pub use pallet::*; +pub use swap::Swap; +pub use types::*; +pub use weights::WeightInfo; use codec::Codec; use frame_support::{ @@ -76,7 +79,6 @@ use frame_system::{ ensure_signed, pallet_prelude::{BlockNumberFor, OriginFor}, }; -pub use pallet::*; use sp_arithmetic::traits::Unsigned; use sp_runtime::{ traits::{ @@ -85,8 +87,6 @@ use sp_runtime::{ DispatchError, }; use sp_std::prelude::*; -pub use types::*; -pub use weights::WeightInfo; #[frame_support::pallet] pub mod pallet { @@ -1238,50 +1238,6 @@ pub mod pallet { } } -impl Swap for Pallet { - fn swap_exact_tokens_for_tokens( - sender: T::AccountId, - path: Vec, - amount_in: T::HigherPrecisionBalance, - amount_out_min: Option, - send_to: T::AccountId, - keep_alive: bool, - ) -> Result { - let path = path.try_into().map_err(|_| Error::::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()) - } - - fn swap_tokens_for_exact_tokens( - sender: T::AccountId, - path: Vec, - amount_out: T::HigherPrecisionBalance, - amount_in_max: Option, - send_to: T::AccountId, - keep_alive: bool, - ) -> Result { - let path = path.try_into().map_err(|_| Error::::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()) - } -} - sp_api::decl_runtime_apis! { /// This runtime api allows people to query the size of the liquidity pools /// and quote prices for swaps. diff --git a/substrate/frame/asset-conversion/src/swap.rs b/substrate/frame/asset-conversion/src/swap.rs new file mode 100644 index 000000000000..7c4468d24b7c --- /dev/null +++ b/substrate/frame/asset-conversion/src/swap.rs @@ -0,0 +1,101 @@ +// This file is part of Substrate. + +// 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. + +//! Traits and implementations for swap between the various asset classes. + +use super::*; + +/// Trait for providing methods to swap between the various asset classes. +pub trait Swap { + /// 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, + amount_in: Balance, + amount_out_min: Option, + send_to: AccountId, + keep_alive: bool, + ) -> Result; + + /// 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, + amount_out: Balance, + amount_in_max: Option, + send_to: AccountId, + keep_alive: bool, + ) -> Result; +} + +impl Swap for Pallet { + fn swap_exact_tokens_for_tokens( + sender: T::AccountId, + path: Vec, + amount_in: T::HigherPrecisionBalance, + amount_out_min: Option, + send_to: T::AccountId, + keep_alive: bool, + ) -> Result { + let path = path.try_into().map_err(|_| Error::::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()) + } + + fn swap_tokens_for_exact_tokens( + sender: T::AccountId, + path: Vec, + amount_out: T::HigherPrecisionBalance, + amount_in_max: Option, + send_to: T::AccountId, + keep_alive: bool, + ) -> Result { + let path = path.try_into().map_err(|_| Error::::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()) + } +} diff --git a/substrate/frame/asset-conversion/src/types.rs b/substrate/frame/asset-conversion/src/types.rs index ffdc63ce0ce7..0bec61705c11 100644 --- a/substrate/frame/asset-conversion/src/types.rs +++ b/substrate/frame/asset-conversion/src/types.rs @@ -83,43 +83,6 @@ where } } -/// Trait for providing methods to swap between the various asset classes. -pub trait Swap { - /// 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, - amount_in: Balance, - amount_out_min: Option, - send_to: AccountId, - keep_alive: bool, - ) -> Result; - - /// 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, - amount_out: Balance, - amount_in_max: Option, - send_to: AccountId, - keep_alive: bool, - ) -> Result; -} - /// An implementation of MultiAssetId that can be either Native or an asset. #[derive(Decode, Encode, Default, MaxEncodedLen, TypeInfo, Clone, Copy, Debug)] pub enum NativeOrAssetId From d8ab7c104fe02b0aca89c43f82df2245b4cabec7 Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 15 Sep 2023 12:51:45 +0200 Subject: [PATCH 02/43] restructure imports --- substrate/frame/asset-conversion/src/lib.rs | 47 ++++++++------------- 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index a3f5159743ec..bdf25c175821 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -72,44 +72,33 @@ pub use weights::WeightInfo; use codec::Codec; use frame_support::{ - ensure, - traits::tokens::{AssetId, Balance}, -}; -use frame_system::{ - ensure_signed, - pallet_prelude::{BlockNumberFor, OriginFor}, + traits::{ + fungible::{Inspect as InspectFungible, Mutate as MutateFungible}, + fungibles::{Create, Inspect, Mutate}, + tokens::{ + AssetId, Balance, + Fortitude::Polite, + Precision::Exact, + Preservation::{Expendable, Preserve}, + }, + AccountTouch, ContainsPair, + }, + BoundedBTreeSet, PalletId, }; -use sp_arithmetic::traits::Unsigned; use sp_runtime::{ traits::{ - CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, Ensure, MaybeDisplay, TrailingZeroInput, + CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, Ensure, IntegerSquareRoot, MaybeDisplay, + One, TrailingZeroInput, Zero, }, - DispatchError, + DispatchError, Saturating, }; -use sp_std::prelude::*; #[frame_support::pallet] pub mod pallet { use super::*; - use frame_support::{ - pallet_prelude::*, - traits::{ - fungible::{Inspect as InspectFungible, Mutate as MutateFungible}, - fungibles::{Create, Inspect, Mutate}, - tokens::{ - Fortitude::Polite, - Precision::Exact, - Preservation::{Expendable, Preserve}, - }, - AccountTouch, ContainsPair, - }, - BoundedBTreeSet, PalletId, - }; - use sp_arithmetic::Permill; - use sp_runtime::{ - traits::{IntegerSquareRoot, One, Zero}, - Saturating, - }; + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + use sp_arithmetic::{traits::Unsigned, Permill}; #[pallet::pallet] pub struct Pallet(_); From 6d6cc181d861624f008303c640ed01ea4b2e7fa2 Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 20 Sep 2023 17:46:07 +0200 Subject: [PATCH 03/43] transfer swap --- substrate/frame/asset-conversion/src/lib.rs | 280 +++++++++++------- substrate/frame/asset-conversion/src/types.rs | 79 +++++ 2 files changed, 260 insertions(+), 99 deletions(-) diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index bdf25c175821..926c1790e871 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -53,7 +53,6 @@ //! (This can be run against the kitchen sync node in the `node` folder of this repo.) #![deny(missing_docs)] #![cfg_attr(not(feature = "std"), no_std)] -use frame_support::traits::{DefensiveOption, Incrementable}; #[cfg(feature = "runtime-benchmarks")] mod benchmarking; @@ -73,15 +72,18 @@ pub use weights::WeightInfo; use codec::Codec; use frame_support::{ traits::{ - fungible::{Inspect as InspectFungible, Mutate as MutateFungible}, - fungibles::{Create, Inspect, Mutate}, + fungible::{ + Balanced as BalancedFungible, Credit as CreditFungible, Inspect as InspectFungible, + Mutate as MutateFungible, + }, + fungibles::{Balanced, Create, Credit as CreditFungibles, Inspect, Mutate}, tokens::{ AssetId, Balance, Fortitude::Polite, Precision::Exact, Preservation::{Expendable, Preserve}, }, - AccountTouch, ContainsPair, + AccountTouch, ContainsPair, Imbalance, Incrementable, }, BoundedBTreeSet, PalletId, }; @@ -90,7 +92,7 @@ use sp_runtime::{ CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, Ensure, IntegerSquareRoot, MaybeDisplay, One, TrailingZeroInput, Zero, }, - DispatchError, Saturating, + BoundedVec, DispatchError, Saturating, }; #[frame_support::pallet] @@ -110,7 +112,8 @@ pub mod pallet { /// Currency type that this works on. type Currency: InspectFungible - + MutateFungible; + + MutateFungible + + BalancedFungible; /// The `Currency::Balance` type of the native currency. type Balance: Balance; @@ -151,7 +154,8 @@ pub mod pallet { type Assets: Inspect + Mutate + AccountTouch - + ContainsPair; + + ContainsPair + + Balanced; /// Registry for the lp tokens. Ideally only this pallet should have create permissions on /// the assets. @@ -350,10 +354,8 @@ pub mod pallet { NonUniquePath, /// It was not possible to get or increment the Id of the pool. IncorrectPoolAssetId, - /// 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, + /// Account cannot exist with the funds that would be given. + BelowMinimum, } #[pallet::hooks] @@ -702,6 +704,9 @@ pub mod pallet { /// respecting `keep_alive`. /// /// If successful, returns the amount of `path[1]` acquired for the `amount_in`. + /// + /// Note: + /// * this function might modify storage even if an error occurs. pub fn do_swap_exact_tokens_for_tokens( sender: T::AccountId, path: BoundedVec, @@ -716,11 +721,9 @@ pub mod pallet { } Self::validate_swap_path(&path)?; + let path = Self::balance_path_from_amount_in(amount_in, 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")?; - + let amount_out = path.last().map(|(_, a)| a.clone()).ok_or(Error::::InvalidPath)?; if let Some(amount_out_min) = amount_out_min { ensure!( amount_out >= amount_out_min, @@ -728,7 +731,15 @@ pub mod pallet { ); } - Self::do_swap(sender, &amounts, path, send_to, keep_alive)?; + Self::transfer_swap(&sender, &path, &send_to, keep_alive)?; + + Self::deposit_event(Event::SwapExecuted { + who: sender, + send_to, + amount_in, + amount_out, + path: BoundedVec::truncate_from(path.into_iter().map(|(a, _)| a).collect()), + }); Ok(amount_out) } @@ -740,6 +751,9 @@ pub mod pallet { /// respecting `keep_alive`. /// /// If successful returns the amount of the `path[0]` taken to provide `path[1]`. + /// + /// Note: + /// * this function might modify storage even if an error occurs. pub fn do_swap_tokens_for_exact_tokens( sender: T::AccountId, path: BoundedVec, @@ -754,11 +768,9 @@ pub mod pallet { } Self::validate_swap_path(&path)?; + let path = Self::balance_path_from_amount_out(amount_out, path)?; - let amounts = Self::get_amounts_in(&amount_out, &path)?; - let amount_in = - *amounts.first().defensive_ok_or("get_amounts_in() returned an empty result")?; - + let amount_in = path.first().map(|(_, a)| a.clone()).ok_or(Error::::InvalidPath)?; if let Some(amount_in_max) = amount_in_max { ensure!( amount_in <= amount_in_max, @@ -766,7 +778,16 @@ pub mod pallet { ); } - Self::do_swap(sender, &amounts, path, send_to, keep_alive)?; + Self::transfer_swap(&sender, &path, &send_to, keep_alive)?; + + Self::deposit_event(Event::SwapExecuted { + who: sender, + send_to, + amount_in, + amount_out, + path: BoundedVec::truncate_from(path.into_iter().map(|(a, _)| a).collect()), + }); + Ok(amount_in) } @@ -809,6 +830,40 @@ pub mod pallet { result } + /// The balance of `who` is increased in order to counter `credit`. If the whole of `credit` + /// cannot be countered, then nothing is changed and the original `credit` is returned in an + /// `Err`. + fn resolve(who: &T::AccountId, credit: Credit) -> Result<(), Credit> { + match credit { + Credit::Native(c) => T::Currency::resolve(who, c).map_err(|c| c.into()), + Credit::Asset(c) => T::Assets::resolve(who, c).map_err(|c| c.into()), + } + } + + /// Removes `value` balance of `asset` from `who` account if possible. + fn withdraw( + asset: &T::MultiAssetId, + who: &T::AccountId, + value: T::AssetBalance, + keep_alive: bool, + ) -> Result, DispatchError> { + let preservation = match keep_alive { + true => Preserve, + false => Expendable, + }; + match T::MultiAssetIdConverter::try_convert(asset) { + MultiAssetIdConversionResult::Converted(asset) => + T::Assets::withdraw(asset, who, value, Exact, preservation, Polite) + .map(|c| c.into()), + MultiAssetIdConversionResult::Native => { + let value = Self::convert_asset_balance_to_native_balance(value)?; + T::Currency::withdraw(who, value, Exact, preservation, Polite).map(|c| c.into()) + }, + MultiAssetIdConversionResult::Unsupported(_) => + Err(Error::::UnsupportedAsset.into()), + } + } + /// Convert a `Balance` type to an `AssetBalance`. pub(crate) fn convert_native_balance_to_asset_balance( amount: T::Balance, @@ -834,63 +889,80 @@ pub mod pallet { 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, - path: BoundedVec, - send_to: T::AccountId, + /// Swap assets along the `path`, withdrawing from `sender` and depositing in `send_to`. + /// + /// Note: + /// * this function might modify storage even if an error occurs. + /// * it's assumed that the provided `path` is valid. + fn transfer_swap( + sender: &T::AccountId, + path: &BalancePath, + send_to: &T::AccountId, keep_alive: bool, ) -> Result<(), DispatchError> { - ensure!(amounts.len() > 1, Error::::CorrespondenceError); - 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); - // amounts should always contain a corresponding element to path. - let first_amount = amounts.first().ok_or(Error::::CorrespondenceError)?; - - Self::transfer(asset1, &sender, &pool_account, *first_amount, keep_alive)?; - - let mut i = 0; - let path_len = path.len() as u32; - for assets_pair in path.windows(2) { - if let [asset1, asset2] = assets_pair { - let pool_id = Self::get_pool_id(asset1.clone(), asset2.clone()); - let pool_account = Self::get_pool_account(&pool_id); - - let amount_out = - 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)?; - Self::get_pool_account(&Self::get_pool_id( - asset2.clone(), - asset3.clone(), - )) - } else { - send_to.clone() - }; + let (asset_in, amount_in) = path.first().ok_or(Error::::InvalidPath)?; + let credit_in = Self::withdraw(asset_in, sender, *amount_in, keep_alive)?; - let reserve = Self::get_balance(&pool_account, asset2)?; + let credit_out = Self::credit_swap(credit_in, path).map_err(|(_, e)| e)?; + Self::resolve(send_to, credit_out).map_err(|_| Error::::BelowMinimum)?; + + Ok(()) + } + + /// Swap assets along the specified `path`, consuming `credit_in` and producing + /// `credit_out`. + /// + /// Note: + /// * this function might modify storage even if an error occurs. + /// * if an error occurs, `credit_in` is returned back. + /// * it's assumed that the provided `path` is valid and `credit_in` corresponds to the + /// first asset in the `path`. + fn credit_swap( + credit_in: Credit, + path: &BalancePath, + ) -> Result, (Credit, DispatchError)> { + let resolve_path = || -> Result, DispatchError> { + for pos in 0..=path.len() { + if let Some([(asset1, _), (asset2, amount_out)]) = path.get(pos..=pos + 1) { + let pool_from = Self::get_pool_account(&Self::get_pool_id( + asset1.clone(), + asset2.clone(), + )); + + let reserve = Self::get_balance(&pool_from, asset2)?; let reserve_left = reserve.saturating_sub(*amount_out); Self::validate_minimal_amount(reserve_left, asset2) .map_err(|_| Error::::ReserveLeftLessThanMinimal)?; - Self::transfer(asset2, &pool_account, &to, *amount_out, true)?; + if let Some((asset3, _)) = path.get(pos + 2) { + let pool_to = Self::get_pool_account(&Self::get_pool_id( + asset2.clone(), + asset3.clone(), + )); + Self::transfer(asset2, &pool_from, &pool_to, *amount_out, true)?; + } else { + let credit_out = Self::withdraw(asset2, &pool_from, *amount_out, true)?; + return Ok(credit_out) + } } - i.saturating_inc(); } - Self::deposit_event(Event::SwapExecuted { - who: sender, - send_to, - path, - amount_in: *first_amount, - amount_out: *amounts.last().expect("Always has more than 1 element"), - }); - } else { return Err(Error::::InvalidPath.into()) - } - Ok(()) + }; + + let credit_out = match resolve_path() { + Ok(c) => c, + Err(e) => return Err((credit_in, e)), + }; + + let pool_to = if let Some([(asset1, _), (asset2, _)]) = path.get(0..2) { + Self::get_pool_account(&Self::get_pool_id(asset1.clone(), asset2.clone())) + } else { + return Err((credit_in, Error::::InvalidPath.into())) + }; + + Self::resolve(&pool_to, credit_in).map_err(|c| (c, Error::::BelowMinimum.into()))?; + + Ok(credit_out) } /// The account ID of the pool. @@ -967,42 +1039,52 @@ pub mod pallet { } /// 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, - ) -> Result, DispatchError> { - let mut amounts: Vec = vec![*amount_out]; - - for assets_pair in path.windows(2).rev() { - if let [asset1, asset2] = assets_pair { - let (reserve_in, reserve_out) = Self::get_reserves(asset1, asset2)?; - let prev_amount = amounts.last().expect("Always has at least one element"); - let amount_in = Self::get_amount_in(prev_amount, &reserve_in, &reserve_out)?; - amounts.push(amount_in); - } + pub(crate) fn balance_path_from_amount_out( + amount_out: T::AssetBalance, + path: BoundedVec, + ) -> Result, DispatchError> { + let mut balance_path: BalancePath = Vec::with_capacity(path.len()); + let mut amount_out: T::AssetBalance = amount_out; + + let mut iter = path.into_iter().rev().peekable(); + while let Some(asset2) = iter.next() { + let asset1 = match iter.peek() { + Some(a) => a, + None => { + balance_path.push((asset2, amount_out)); + break + }, + }; + let (reserve_in, reserve_out) = Self::get_reserves(asset1, &asset2)?; + balance_path.push((asset2, amount_out)); + amount_out = Self::get_amount_in(&amount_out, &reserve_in, &reserve_out)?; } - - amounts.reverse(); - Ok(amounts) + balance_path.reverse(); + Ok(balance_path) } /// Following an amount into a `path`, get the corresponding amounts out. - pub(crate) fn get_amounts_out( - amount_in: &T::AssetBalance, - path: &BoundedVec, - ) -> Result, DispatchError> { - let mut amounts: Vec = vec![*amount_in]; - - for assets_pair in path.windows(2) { - if let [asset1, asset2] = assets_pair { - let (reserve_in, reserve_out) = Self::get_reserves(asset1, asset2)?; - let prev_amount = amounts.last().expect("Always has at least one element"); - let amount_out = Self::get_amount_out(prev_amount, &reserve_in, &reserve_out)?; - amounts.push(amount_out); - } + pub(crate) fn balance_path_from_amount_in( + amount_in: T::AssetBalance, + path: BoundedVec, + ) -> Result, DispatchError> { + let mut balance_path: BalancePath = Vec::with_capacity(path.len()); + let mut amount_out: T::AssetBalance = amount_in; + + let mut iter = path.into_iter().peekable(); + while let Some(asset1) = iter.next() { + let asset2 = match iter.peek() { + Some(a) => a, + None => { + balance_path.push((asset1, amount_out)); + break + }, + }; + let (reserve_in, reserve_out) = Self::get_reserves(&asset1, asset2)?; + balance_path.push((asset1, amount_out)); + amount_out = Self::get_amount_out(&amount_out, &reserve_in, &reserve_out)?; } - - Ok(amounts) + Ok(balance_path) } /// Used by the RPC service to provide current prices. diff --git a/substrate/frame/asset-conversion/src/types.rs b/substrate/frame/asset-conversion/src/types.rs index 0bec61705c11..175ac11e375f 100644 --- a/substrate/frame/asset-conversion/src/types.rs +++ b/substrate/frame/asset-conversion/src/types.rs @@ -27,6 +27,17 @@ use sp_std::{cmp::Ordering, marker::PhantomData}; /// migration. pub(super) type PoolIdOf = (::MultiAssetId, ::MultiAssetId); +/// Represents a swap path with associated amounts for input and output. +/// +/// Each pair of neighboring assets forms a pool. +/// +/// Example: +/// Given path [(asset1, amount_in), (asset2, amount_out2), (asset3, amount_out3)], can be resolved: +/// 1. `asset(asset1, amount_in)` take from `user` and move to the pool(asset1, asset2); +/// 2. `asset(asset2, amount_out2)` transfer from pool(asset1, asset2) to pool(asset2, asset3); +/// 3. `asset(asset3, amount_out3)` move from pool(asset2, asset3) to `user`. +pub(super) type BalancePath = Vec<(::MultiAssetId, ::AssetBalance)>; + /// Stores the lp_token asset id a particular pool has been assigned. #[derive(Decode, Encode, Default, PartialEq, Eq, MaxEncodedLen, TypeInfo)] pub struct PoolInfo { @@ -149,3 +160,71 @@ impl MultiAssetIdConverter, Asset } } } + +/// Credit of [Config::Currency]. +pub type NativeCredit = + CreditFungible<::AccountId, ::Currency>; + +/// Credit of [Config::Assets]. +pub type AssetCredit = + CreditFungibles<::AccountId, ::Assets>; + +/// Credit that can be either [`NativeCredit`] or [`AssetCredit`]. +pub enum Credit { + /// Native credit. + Native(NativeCredit), + /// Asset credit. + Asset(AssetCredit), +} + +impl From> for Credit { + fn from(value: NativeCredit) -> Self { + Credit::Native(value) + } +} + +impl From> for Credit { + fn from(value: AssetCredit) -> Self { + Credit::Asset(value) + } +} + +impl Credit { + /// Amount of `self`. + pub fn peek(&self) -> Result { + let amount = match self { + Credit::Native(c) => T::HigherPrecisionBalance::from(c.peek()) + .try_into() + .map_err(|_| Error::::Overflow)?, + Credit::Asset(c) => c.peek(), + }; + Ok(amount) + } + + /// Asset class of `self`. + pub fn asset(&self) -> T::MultiAssetId { + match self { + Credit::Native(_) => T::MultiAssetIdConverter::get_native(), + Credit::Asset(c) => c.asset().into(), + } + } + + /// Consume `self` and return two independent instances; the first is guaranteed to be at most + /// `amount` and the second will be the remainder. + pub fn split(self, amount: T::AssetBalance) -> Result<(Self, Self), (Self, DispatchError)> { + match self { + Credit::Native(c) => { + let amount = match T::HigherPrecisionBalance::from(amount).try_into() { + Ok(a) => a, + Err(_) => return Err((Self::Native(c), Error::::Overflow.into())), + }; + let (left, right) = c.split(amount); + Ok((left.into(), right.into())) + }, + Credit::Asset(c) => { + let (left, right) = c.split(amount); + Ok((left.into(), right.into())) + }, + } + } +} From a3552419b1c07689c524b3bc673990ea558a18c7 Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 22 Sep 2023 12:24:55 +0200 Subject: [PATCH 04/43] swap credit --- substrate/frame/asset-conversion/src/lib.rs | 205 ++++++++++++------ substrate/frame/asset-conversion/src/swap.rs | 67 ++++++ substrate/frame/asset-conversion/src/tests.rs | 6 +- substrate/frame/asset-conversion/src/types.rs | 13 +- 4 files changed, 220 insertions(+), 71 deletions(-) diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index 926c1790e871..80d2c6e09f46 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -65,7 +65,7 @@ mod types; pub mod weights; pub use pallet::*; -pub use swap::Swap; +pub use swap::*; pub use types::*; pub use weights::WeightInfo; @@ -276,24 +276,15 @@ pub mod pallet { who: T::AccountId, /// The account that the assets were transferred to. send_to: T::AccountId, - /// The route of asset ids that the swap went through. - /// E.g. A -> Dot -> B - path: BoundedVec, - /// The amount of the first asset that was swapped. - amount_in: T::AssetBalance, - /// The amount of the second asset that was received. - amount_out: T::AssetBalance, + /// The route of asset ids with amounts that the swap went through. + /// E.g. (A, amount_in) -> (Dot, amount_out) -> (B, amount_out) + path: BalancePath, }, - /// An amount has been transferred from one account to another. - Transfer { - /// The account that the assets were transferred from. - from: T::AccountId, - /// The account that the assets were transferred to. - to: T::AccountId, - /// The asset that was transferred. - asset: T::MultiAssetId, - /// The amount of the asset that was transferred. - amount: T::AssetBalance, + /// Assets have been converted from one to another. + CreditSwapExecuted { + /// The route of asset ids with amounts that the swap went through. + /// E.g. (A, amount_in) -> (Dot, amount_out) -> (B, amount_out) + path: BalancePath, }, } @@ -705,9 +696,10 @@ pub mod pallet { /// /// If successful, returns the amount of `path[1]` acquired for the `amount_in`. /// - /// Note: - /// * this function might modify storage even if an error occurs. - pub fn do_swap_exact_tokens_for_tokens( + /// WARNING: This may return an error after a partial storage mutation. It should be used + /// only inside a transactional storage context and an Err result must imply a storage + /// rollback. + pub(crate) fn do_swap_exact_tokens_for_tokens( sender: T::AccountId, path: BoundedVec, amount_in: T::AssetBalance, @@ -723,7 +715,7 @@ pub mod pallet { Self::validate_swap_path(&path)?; let path = Self::balance_path_from_amount_in(amount_in, path)?; - let amount_out = path.last().map(|(_, a)| a.clone()).ok_or(Error::::InvalidPath)?; + let amount_out = path.last().map(|(_, a)| *a).ok_or(Error::::InvalidPath)?; if let Some(amount_out_min) = amount_out_min { ensure!( amount_out >= amount_out_min, @@ -731,15 +723,9 @@ pub mod pallet { ); } - Self::transfer_swap(&sender, &path, &send_to, keep_alive)?; + Self::swap(&sender, &path, &send_to, keep_alive)?; - Self::deposit_event(Event::SwapExecuted { - who: sender, - send_to, - amount_in, - amount_out, - path: BoundedVec::truncate_from(path.into_iter().map(|(a, _)| a).collect()), - }); + Self::deposit_event(Event::SwapExecuted { who: sender, send_to, path }); Ok(amount_out) } @@ -752,9 +738,10 @@ pub mod pallet { /// /// If successful returns the amount of the `path[0]` taken to provide `path[1]`. /// - /// Note: - /// * this function might modify storage even if an error occurs. - pub fn do_swap_tokens_for_exact_tokens( + /// WARNING: This may return an error after a partial storage mutation. It should be used + /// only inside a transactional storage context and an Err result must imply a storage + /// rollback. + pub(crate) fn do_swap_tokens_for_exact_tokens( sender: T::AccountId, path: BoundedVec, amount_out: T::AssetBalance, @@ -770,7 +757,7 @@ pub mod pallet { Self::validate_swap_path(&path)?; let path = Self::balance_path_from_amount_out(amount_out, path)?; - let amount_in = path.first().map(|(_, a)| a.clone()).ok_or(Error::::InvalidPath)?; + let amount_in = path.first().map(|(_, a)| *a).ok_or(Error::::InvalidPath)?; if let Some(amount_in_max) = amount_in_max { ensure!( amount_in <= amount_in_max, @@ -778,19 +765,109 @@ pub mod pallet { ); } - Self::transfer_swap(&sender, &path, &send_to, keep_alive)?; + Self::swap(&sender, &path, &send_to, keep_alive)?; - Self::deposit_event(Event::SwapExecuted { - who: sender, - send_to, - amount_in, - amount_out, - path: BoundedVec::truncate_from(path.into_iter().map(|(a, _)| a).collect()), - }); + Self::deposit_event(Event::SwapExecuted { who: sender, send_to, path }); Ok(amount_in) } + /// Swap exactly `credit_in` of asset `path[0]` for asset `path[last]`. If `amount_out_min` + /// is provided and the swap can't achieve at least this amount, an error is returned. + /// + /// On a successful swap, the function returns the `credit_out` of `path[last]` obtained + /// from the `credit_in`. On failure, it returns an `Err` containing the original + /// `credit_in` and the associated error code. + /// + /// WARNING: This may return an error after a partial storage mutation. It should be used + /// only inside a transactional storage context and an Err result must imply a storage + /// rollback. + pub(crate) fn do_swap_exact_credit_tokens_for_tokens( + path: BoundedVec, + credit_in: Credit, + amount_out_min: Option, + ) -> Result, (Credit, DispatchError)> { + let amount_in = match credit_in.peek() { + Ok(a) => a, + Err(e) => return Err((credit_in, e)), + }; + let inspect_path = |credit_asset| { + ensure!(path.get(0).map_or(false, |a| *a == credit_asset), Error::::InvalidPath); + ensure!(!amount_in.is_zero(), Error::::ZeroAmount); + ensure!(amount_out_min.map_or(true, |a| !a.is_zero()), Error::::ZeroAmount); + + Self::validate_swap_path(&path)?; + let path = Self::balance_path_from_amount_in(amount_in, path)?; + + let amount_out = path.last().map(|(_, a)| *a).ok_or(Error::::InvalidPath)?; + ensure!( + amount_out_min.map_or(true, |a| amount_out >= a), + Error::::ProvidedMinimumNotSufficientForSwap + ); + Ok(path) + }; + let path = match inspect_path(credit_in.asset()) { + Ok(p) => p, + Err(e) => return Err((credit_in, e)), + }; + + let credit_out = Self::credit_swap(credit_in, &path)?; + + Self::deposit_event(Event::CreditSwapExecuted { path }); + + Ok(credit_out) + } + + /// Swaps a portion of `credit_in` of `path[0]` asset to obtain the desired `amount_out` of + /// the `path[last]` asset. The provided `credit_in` must be adequate to achieve the target + /// `amount_out`, or an error will occur. + /// + /// On success, the function returns a (`credit_out`, `credit_change`) tuple, where + /// `credit_out` represents the acquired amount of the `path[last]` asset, and + /// `credit_change` is the remaining portion from the `credit_in`. On failure, an `Err` with + /// the initial `credit_in` and error code is returned. + /// + /// WARNING: This may return an error after a partial storage mutation. It should be used + /// only inside a transactional storage context and an Err result must imply a storage + /// rollback. + pub(crate) fn do_swap_credit_tokens_for_exact_tokens( + path: BoundedVec, + credit_in: Credit, + amount_out: T::AssetBalance, + ) -> Result<(Credit, Credit), (Credit, DispatchError)> { + let amount_in_max = match credit_in.peek() { + Ok(a) => a, + Err(e) => return Err((credit_in, e)), + }; + let inspect_path = |credit_asset| { + ensure!(path.get(0).map_or(false, |a| a == &credit_asset), Error::::InvalidPath); + ensure!(amount_in_max > Zero::zero(), Error::::ZeroAmount); + ensure!(amount_out > Zero::zero(), Error::::ZeroAmount); + + Self::validate_swap_path(&path)?; + let path = Self::balance_path_from_amount_out(amount_out, path)?; + + let amount_in = path.first().map(|(_, a)| *a).ok_or(Error::::InvalidPath)?; + ensure!( + amount_in <= amount_in_max, + Error::::ProvidedMaximumNotSufficientForSwap + ); + + Ok((path, amount_in)) + }; + let (path, amount_in) = match inspect_path(credit_in.asset()) { + Ok((p, a)) => (p, a), + Err(e) => return Err((credit_in, e)), + }; + + let (credit_in, credit_change) = credit_in.split(amount_in)?; + let credit_out = Self::credit_swap(credit_in, &path)?; + + Self::deposit_event(Event::CreditSwapExecuted { path }); + + Ok((credit_out, credit_change)) + } + /// Transfer an `amount` of `asset_id`, respecting the `keep_alive` requirements. fn transfer( asset_id: &T::MultiAssetId, @@ -799,7 +876,7 @@ pub mod pallet { amount: T::AssetBalance, keep_alive: bool, ) -> Result { - let result = match T::MultiAssetIdConverter::try_convert(asset_id) { + match T::MultiAssetIdConverter::try_convert(asset_id) { MultiAssetIdConversionResult::Converted(asset_id) => T::Assets::transfer(asset_id, from, to, amount, Expendable), MultiAssetIdConversionResult::Native => { @@ -817,17 +894,7 @@ pub mod pallet { }, MultiAssetIdConversionResult::Unsupported(_) => Err(Error::::UnsupportedAsset.into()), - }; - - if result.is_ok() { - Self::deposit_event(Event::Transfer { - from: from.clone(), - to: to.clone(), - asset: (*asset_id).clone(), - amount, - }); } - result } /// The balance of `who` is increased in order to counter `credit`. If the whole of `credit` @@ -891,10 +958,12 @@ pub mod pallet { /// Swap assets along the `path`, withdrawing from `sender` and depositing in `send_to`. /// - /// Note: - /// * this function might modify storage even if an error occurs. - /// * it's assumed that the provided `path` is valid. - fn transfer_swap( + /// Note: It's assumed that the provided `path` is valid. + /// + /// WARNING: This may return an error after a partial storage mutation. It should be used + /// only inside a transactional storage context and an Err result must imply a storage + /// rollback. + fn swap( sender: &T::AccountId, path: &BalancePath, send_to: &T::AccountId, @@ -912,11 +981,14 @@ pub mod pallet { /// Swap assets along the specified `path`, consuming `credit_in` and producing /// `credit_out`. /// - /// Note: - /// * this function might modify storage even if an error occurs. - /// * if an error occurs, `credit_in` is returned back. - /// * it's assumed that the provided `path` is valid and `credit_in` corresponds to the - /// first asset in the `path`. + /// If an error occurs, `credit_in` is returned back. + /// + /// Note: It's assumed that the provided `path` is valid and `credit_in` corresponds to the + /// first asset in the `path`. + /// + /// WARNING: This may return an error after a partial storage mutation. It should be used + /// only inside a transactional storage context and an Err result must imply a storage + /// rollback. fn credit_swap( credit_in: Credit, path: &BalancePath, @@ -1044,22 +1116,23 @@ pub mod pallet { path: BoundedVec, ) -> Result, DispatchError> { let mut balance_path: BalancePath = Vec::with_capacity(path.len()); - let mut amount_out: T::AssetBalance = amount_out; + let mut amount_in: T::AssetBalance = amount_out; let mut iter = path.into_iter().rev().peekable(); while let Some(asset2) = iter.next() { let asset1 = match iter.peek() { Some(a) => a, None => { - balance_path.push((asset2, amount_out)); + balance_path.push((asset2, amount_in)); break }, }; let (reserve_in, reserve_out) = Self::get_reserves(asset1, &asset2)?; - balance_path.push((asset2, amount_out)); - amount_out = Self::get_amount_in(&amount_out, &reserve_in, &reserve_out)?; + balance_path.push((asset2, amount_in)); + amount_in = Self::get_amount_in(&amount_in, &reserve_in, &reserve_out)?; } balance_path.reverse(); + Ok(balance_path) } diff --git a/substrate/frame/asset-conversion/src/swap.rs b/substrate/frame/asset-conversion/src/swap.rs index 7c4468d24b7c..8724e99a1678 100644 --- a/substrate/frame/asset-conversion/src/swap.rs +++ b/substrate/frame/asset-conversion/src/swap.rs @@ -56,6 +56,43 @@ pub trait Swap { ) -> Result; } +/// Trait providing methods to swap between the various asset classes. +pub trait SwapCredit { + /// Measure units of the asset classes for swapping. + type Balance; + /// Kind of assets that are going to be swapped. + type MultiAssetId; + /// Credit implying a negative imbalance in the system that can be placed into an account or + /// alter the total supply. + type Credit; + + /// Swap exactly `credit_in` of asset `path[0]` for asset `path[last]`. If `amount_out_min` is + /// provided and the swap can't achieve at least this amount, an error is returned. + /// + /// On a successful swap, the function returns the `credit_out` of `path[last]` obtained from + /// the `credit_in`. On failure, it returns an `Err` containing the original `credit_in` and the + /// associated error code. + fn swap_exact_tokens_for_tokens( + path: Vec, + credit_in: Self::Credit, + amount_out_min: Option, + ) -> Result; + + /// Swaps a portion of `credit_in` of `path[0]` asset to obtain the desired `amount_out` of + /// the `path[last]` asset. The provided `credit_in` must be adequate to achieve the target + /// `amount_out`, or an error will occur. + /// + /// On success, the function returns a (`credit_out`, `credit_change`) tuple, where `credit_out` + /// represents the acquired amount of the `path[last]` asset, and `credit_change` is the + /// remaining portion from the `credit_in`. On failure, an `Err` with the initial `credit_in` + /// and error code is returned. + fn swap_tokens_for_exact_tokens( + path: Vec, + credit_in: Self::Credit, + amount_out: Self::Balance, + ) -> Result<(Self::Credit, Self::Credit), (Self::Credit, DispatchError)>; +} + impl Swap for Pallet { fn swap_exact_tokens_for_tokens( sender: T::AccountId, @@ -99,3 +136,33 @@ impl Swap f Ok(amount_in.into()) } } + +impl SwapCredit for Pallet { + type Balance = T::AssetBalance; + type MultiAssetId = T::MultiAssetId; + type Credit = Credit; + + fn swap_exact_tokens_for_tokens( + path: Vec, + credit_in: Self::Credit, + amount_out_min: Option, + ) -> Result { + let path = match path.try_into() { + Ok(p) => p, + Err(_) => return Err((credit_in, Error::::PathError.into())), + }; + Self::do_swap_exact_credit_tokens_for_tokens(path, credit_in, amount_out_min) + } + + fn swap_tokens_for_exact_tokens( + path: Vec, + credit_in: Self::Credit, + amount_out: Self::Balance, + ) -> Result<(Self::Credit, Self::Credit), (Self::Credit, DispatchError)> { + let path = match path.try_into() { + Ok(p) => p, + Err(_) => return Err((credit_in, Error::::PathError.into())), + }; + Self::do_swap_credit_tokens_for_exact_tokens(path, credit_in, amount_out) + } +} diff --git a/substrate/frame/asset-conversion/src/tests.rs b/substrate/frame/asset-conversion/src/tests.rs index 1c1267ab87b3..609d8b2316c1 100644 --- a/substrate/frame/asset-conversion/src/tests.rs +++ b/substrate/frame/asset-conversion/src/tests.rs @@ -948,9 +948,9 @@ fn can_swap_with_realistic_values() { assert!(events().contains(&Event::::SwapExecuted { who: user, send_to: user, - path: bvec![usd, dot], - amount_in: 10 * UNIT, // usd - amount_out: 1_993_980_120, // About 2 dot after div by UNIT. + path: bvec![(usd, 10 * UNIT), (dot, 1_993_980_120)], + // amount_in: 10 * UNIT, // usd + // amount_out: 1_993_980_120, // About 2 dot after div by UNIT. })); }); } diff --git a/substrate/frame/asset-conversion/src/types.rs b/substrate/frame/asset-conversion/src/types.rs index 175ac11e375f..7fe8e3d255db 100644 --- a/substrate/frame/asset-conversion/src/types.rs +++ b/substrate/frame/asset-conversion/src/types.rs @@ -27,7 +27,7 @@ use sp_std::{cmp::Ordering, marker::PhantomData}; /// migration. pub(super) type PoolIdOf = (::MultiAssetId, ::MultiAssetId); -/// Represents a swap path with associated amounts for input and output. +/// Represents a swap path with associated asset amounts for input and output. /// /// Each pair of neighboring assets forms a pool. /// @@ -162,14 +162,23 @@ impl MultiAssetIdConverter, Asset } /// Credit of [Config::Currency]. +/// +/// Implies a negative imbalance in the system that can be placed into an account or alter the total +/// supply. pub type NativeCredit = CreditFungible<::AccountId, ::Currency>; -/// Credit of [Config::Assets]. +/// Credit (aka negative imbalance) of [Config::Assets]. +/// +/// Implies a negative imbalance in the system that can be placed into an account or alter the total +/// supply. pub type AssetCredit = CreditFungibles<::AccountId, ::Assets>; /// Credit that can be either [`NativeCredit`] or [`AssetCredit`]. +/// +/// Implies a negative imbalance in the system that can be placed into an account or alter the total +/// supply. pub enum Credit { /// Native credit. Native(NativeCredit), From a78ac716d36ed075b4390b847cdafc106219c0bd Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 22 Sep 2023 13:15:24 +0200 Subject: [PATCH 05/43] with transaction --- substrate/frame/asset-conversion/src/lib.rs | 3 +- substrate/frame/asset-conversion/src/swap.rs | 65 ++++++++++++++----- substrate/frame/asset-conversion/src/tests.rs | 2 - substrate/frame/asset-conversion/src/types.rs | 5 ++ 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index 80d2c6e09f46..bf2476d8293f 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -71,6 +71,7 @@ pub use weights::WeightInfo; use codec::Codec; use frame_support::{ + storage::{with_storage_layer, with_transaction}, traits::{ fungible::{ Balanced as BalancedFungible, Credit as CreditFungible, Inspect as InspectFungible, @@ -92,7 +93,7 @@ use sp_runtime::{ CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, Ensure, IntegerSquareRoot, MaybeDisplay, One, TrailingZeroInput, Zero, }, - BoundedVec, DispatchError, Saturating, + BoundedVec, DispatchError, Saturating, TransactionOutcome, }; #[frame_support::pallet] diff --git a/substrate/frame/asset-conversion/src/swap.rs b/substrate/frame/asset-conversion/src/swap.rs index 8724e99a1678..e64fd39782d4 100644 --- a/substrate/frame/asset-conversion/src/swap.rs +++ b/substrate/frame/asset-conversion/src/swap.rs @@ -104,14 +104,16 @@ impl Swap f ) -> Result { let path = path.try_into().map_err(|_| Error::::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, - )?; + let amount_out = with_storage_layer(|| { + 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()) } @@ -125,14 +127,16 @@ impl Swap f ) -> Result { let path = path.try_into().map_err(|_| Error::::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, - )?; + let amount_in = with_storage_layer(|| { + 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()) } } @@ -151,7 +155,20 @@ impl SwapCredit for Pallet { Ok(p) => p, Err(_) => return Err((credit_in, Error::::PathError.into())), }; - Self::do_swap_exact_credit_tokens_for_tokens(path, credit_in, amount_out_min) + let transaction_res = + with_transaction(|| -> TransactionOutcome> { + let res = + Self::do_swap_exact_credit_tokens_for_tokens(path, credit_in, amount_out_min); + match &res { + Ok(_) => TransactionOutcome::Commit(Ok(res)), + Err(_) => TransactionOutcome::Rollback(Ok(res)), + } + }); + match transaction_res { + Ok(r) => r, + // should never happen, `with_transaction` above never returns an `Err` variant. + Err(_) => Err((Self::Credit::native_zero(), DispatchError::Corruption)), + } } fn swap_tokens_for_exact_tokens( @@ -163,6 +180,18 @@ impl SwapCredit for Pallet { Ok(p) => p, Err(_) => return Err((credit_in, Error::::PathError.into())), }; - Self::do_swap_credit_tokens_for_exact_tokens(path, credit_in, amount_out) + let transaction_res = + with_transaction(|| -> TransactionOutcome> { + let res = Self::do_swap_credit_tokens_for_exact_tokens(path, credit_in, amount_out); + match &res { + Ok(_) => TransactionOutcome::Commit(Ok(res)), + Err(_) => TransactionOutcome::Rollback(Ok(res)), + } + }); + match transaction_res { + Ok(r) => r, + // should never happen, `with_transaction` above never returns an `Err` variant. + Err(_) => Err((Self::Credit::native_zero(), DispatchError::Corruption)), + } } } diff --git a/substrate/frame/asset-conversion/src/tests.rs b/substrate/frame/asset-conversion/src/tests.rs index 609d8b2316c1..b5d56cbc7155 100644 --- a/substrate/frame/asset-conversion/src/tests.rs +++ b/substrate/frame/asset-conversion/src/tests.rs @@ -949,8 +949,6 @@ fn can_swap_with_realistic_values() { who: user, send_to: user, path: bvec![(usd, 10 * UNIT), (dot, 1_993_980_120)], - // amount_in: 10 * UNIT, // usd - // amount_out: 1_993_980_120, // About 2 dot after div by UNIT. })); }); } diff --git a/substrate/frame/asset-conversion/src/types.rs b/substrate/frame/asset-conversion/src/types.rs index 7fe8e3d255db..1b921a757a5a 100644 --- a/substrate/frame/asset-conversion/src/types.rs +++ b/substrate/frame/asset-conversion/src/types.rs @@ -199,6 +199,11 @@ impl From> for Credit { } impl Credit { + /// Create zero native credit. + pub fn native_zero() -> Self { + NativeCredit::::zero().into() + } + /// Amount of `self`. pub fn peek(&self) -> Result { let amount = match self { From 888c9fad09e2bacadf70dccd304c11c13691310b Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 22 Sep 2023 15:14:07 +0200 Subject: [PATCH 06/43] preservation --- substrate/frame/asset-conversion/src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index bf2476d8293f..d549fe04512c 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -877,14 +877,14 @@ pub mod pallet { amount: T::AssetBalance, keep_alive: bool, ) -> Result { + let preservation = match keep_alive { + true => Preserve, + false => Expendable, + }; match T::MultiAssetIdConverter::try_convert(asset_id) { MultiAssetIdConversionResult::Converted(asset_id) => - T::Assets::transfer(asset_id, from, to, amount, Expendable), + T::Assets::transfer(asset_id, from, to, amount, preservation), MultiAssetIdConversionResult::Native => { - let preservation = match keep_alive { - true => Preserve, - false => Expendable, - }; let amount = Self::convert_asset_balance_to_native_balance(amount)?; Ok(Self::convert_native_balance_to_asset_balance(T::Currency::transfer( from, @@ -1213,7 +1213,7 @@ pub mod pallet { reserve1: &T::AssetBalance, reserve2: &T::AssetBalance, ) -> Result> { - // amount * reserve2 / reserve1 + // (amount * reserve2) / reserve1 Self::mul_div(amount, reserve2, reserve1) } From 793b61253d51402df5f64d704ac41df8241cb611 Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 22 Sep 2023 16:25:49 +0200 Subject: [PATCH 07/43] add amounts to the event --- substrate/frame/asset-conversion/src/lib.rs | 34 +++++++++++++++---- substrate/frame/asset-conversion/src/tests.rs | 2 ++ 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index d549fe04512c..cbc2bb3b40ad 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -277,12 +277,20 @@ pub mod pallet { who: T::AccountId, /// The account that the assets were transferred to. send_to: T::AccountId, + /// The amount of the first asset that was swapped. + amount_in: T::AssetBalance, + /// The amount of the second asset that was received. + amount_out: T::AssetBalance, /// The route of asset ids with amounts that the swap went through. /// E.g. (A, amount_in) -> (Dot, amount_out) -> (B, amount_out) path: BalancePath, }, /// Assets have been converted from one to another. CreditSwapExecuted { + /// The amount of the first asset that was swapped. + amount_in: T::AssetBalance, + /// The amount of the second asset that was received. + amount_out: T::AssetBalance, /// The route of asset ids with amounts that the swap went through. /// E.g. (A, amount_in) -> (Dot, amount_out) -> (B, amount_out) path: BalancePath, @@ -726,7 +734,13 @@ pub mod pallet { Self::swap(&sender, &path, &send_to, keep_alive)?; - Self::deposit_event(Event::SwapExecuted { who: sender, send_to, path }); + Self::deposit_event(Event::SwapExecuted { + who: sender, + send_to, + amount_in, + amount_out, + path, + }); Ok(amount_out) } @@ -768,7 +782,13 @@ pub mod pallet { Self::swap(&sender, &path, &send_to, keep_alive)?; - Self::deposit_event(Event::SwapExecuted { who: sender, send_to, path }); + Self::deposit_event(Event::SwapExecuted { + who: sender, + send_to, + amount_in, + amount_out, + path, + }); Ok(amount_in) } @@ -805,16 +825,16 @@ pub mod pallet { amount_out_min.map_or(true, |a| amount_out >= a), Error::::ProvidedMinimumNotSufficientForSwap ); - Ok(path) + Ok((path, amount_out)) }; - let path = match inspect_path(credit_in.asset()) { - Ok(p) => p, + let (path, amount_out) = match inspect_path(credit_in.asset()) { + Ok((p, a)) => (p, a), Err(e) => return Err((credit_in, e)), }; let credit_out = Self::credit_swap(credit_in, &path)?; - Self::deposit_event(Event::CreditSwapExecuted { path }); + Self::deposit_event(Event::CreditSwapExecuted { amount_in, amount_out, path }); Ok(credit_out) } @@ -864,7 +884,7 @@ pub mod pallet { let (credit_in, credit_change) = credit_in.split(amount_in)?; let credit_out = Self::credit_swap(credit_in, &path)?; - Self::deposit_event(Event::CreditSwapExecuted { path }); + Self::deposit_event(Event::CreditSwapExecuted { amount_in, amount_out, path }); Ok((credit_out, credit_change)) } diff --git a/substrate/frame/asset-conversion/src/tests.rs b/substrate/frame/asset-conversion/src/tests.rs index b5d56cbc7155..639ecafb8ed7 100644 --- a/substrate/frame/asset-conversion/src/tests.rs +++ b/substrate/frame/asset-conversion/src/tests.rs @@ -948,6 +948,8 @@ fn can_swap_with_realistic_values() { assert!(events().contains(&Event::::SwapExecuted { who: user, send_to: user, + amount_in: 10 * UNIT, // usd + amount_out: 1_993_980_120, // About 2 dot after div by UNIT. path: bvec![(usd, 10 * UNIT), (dot, 1_993_980_120)], })); }); From cdaf50027ef83bb52b9b148584ef4eb941cd3c8a Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 25 Sep 2023 15:59:57 +0200 Subject: [PATCH 08/43] not expendable constraint --- substrate/frame/asset-conversion/src/lib.rs | 26 ++-- substrate/frame/asset-conversion/src/tests.rs | 144 +++++++++++++++++- 2 files changed, 154 insertions(+), 16 deletions(-) diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index cbc2bb3b40ad..37cdfbca5f89 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -93,7 +93,7 @@ use sp_runtime::{ CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, Ensure, IntegerSquareRoot, MaybeDisplay, One, TrailingZeroInput, Zero, }, - BoundedVec, DispatchError, Saturating, TransactionOutcome, + BoundedVec, DispatchError, Saturating, TokenError, TransactionOutcome, }; #[frame_support::pallet] @@ -940,11 +940,25 @@ pub mod pallet { false => Expendable, }; match T::MultiAssetIdConverter::try_convert(asset) { - MultiAssetIdConversionResult::Converted(asset) => + MultiAssetIdConversionResult::Converted(asset) => { + if preservation == Preserve { + // TODO drop the ensure! when this issue addressed + // https://github.com/paritytech/polkadot-sdk/issues/1698 + let free = + T::Assets::reducible_balance(asset.clone(), who, preservation, Polite); + ensure!(free >= value, TokenError::NotExpendable); + } T::Assets::withdraw(asset, who, value, Exact, preservation, Polite) - .map(|c| c.into()), + .map(|c| c.into()) + }, MultiAssetIdConversionResult::Native => { let value = Self::convert_asset_balance_to_native_balance(value)?; + if preservation == Preserve { + // TODO drop the ensure! when this issue addressed + // https://github.com/paritytech/polkadot-sdk/issues/1698 + let free = T::Currency::reducible_balance(who, preservation, Polite); + ensure!(free >= value, TokenError::NotExpendable); + } T::Currency::withdraw(who, value, Exact, preservation, Polite).map(|c| c.into()) }, MultiAssetIdConversionResult::Unsupported(_) => @@ -1021,12 +1035,6 @@ pub mod pallet { asset1.clone(), asset2.clone(), )); - - let reserve = Self::get_balance(&pool_from, asset2)?; - let reserve_left = reserve.saturating_sub(*amount_out); - Self::validate_minimal_amount(reserve_left, asset2) - .map_err(|_| Error::::ReserveLeftLessThanMinimal)?; - if let Some((asset3, _)) = path.get(pos + 2) { let pool_to = Self::get_pool_account(&Self::get_pool_id( asset2.clone(), diff --git a/substrate/frame/asset-conversion/src/tests.rs b/substrate/frame/asset-conversion/src/tests.rs index 639ecafb8ed7..86fb61edcb77 100644 --- a/substrate/frame/asset-conversion/src/tests.rs +++ b/substrate/frame/asset-conversion/src/tests.rs @@ -19,7 +19,7 @@ use crate::{mock::*, *}; use frame_support::{ assert_noop, assert_ok, instances::Instance1, - traits::{fungible::Inspect, fungibles::InspectEnumerable, Get}, + traits::{fungible, fungibles, fungibles::InspectEnumerable, Get}, }; use sp_arithmetic::Permill; use sp_runtime::{DispatchError, TokenError}; @@ -65,13 +65,17 @@ fn pool_assets() -> Vec { } fn create_tokens(owner: u128, tokens: Vec>) { + create_tokens_with_ed(owner, tokens, 1) +} + +fn create_tokens_with_ed(owner: u128, tokens: Vec>, ed: u128) { for token_id in tokens { let MultiAssetIdConversionResult::Converted(asset_id) = NativeOrAssetIdConverter::try_convert(&token_id) else { unreachable!("invalid token") }; - assert_ok!(Assets::force_create(RuntimeOrigin::root(), asset_id, owner, false, 1)); + assert_ok!(Assets::force_create(RuntimeOrigin::root(), asset_id, owner, false, ed)); } } @@ -444,8 +448,14 @@ fn can_remove_liquidity() { let lp_token = AssetConversion::get_next_pool_asset_id(); assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); - assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 10000000000)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 100000)); + let ed_token_1 = >::minimum_balance(); + let ed_token_2 = >::minimum_balance(2); + assert_ok!(Balances::force_set_balance( + RuntimeOrigin::root(), + user, + 10000000000 + ed_token_1 + )); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 100000 + ed_token_2)); assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), @@ -487,8 +497,8 @@ fn can_remove_liquidity() { assert_eq!(balance(pool_account, token_2), 10001); assert_eq!(pool_balance(pool_account, lp_token), 100); - assert_eq!(balance(user, token_1), 10000000000 - 1000000000 + 899991000); - assert_eq!(balance(user, token_2), 89999); + assert_eq!(balance(user, token_1), 10000000000 - 1000000000 + 899991000 + ed_token_1); + assert_eq!(balance(user, token_2), 89999 + ed_token_2); assert_eq!(pool_balance(user, lp_token), 0); }); } @@ -1050,7 +1060,7 @@ fn check_no_panic_when_try_swap_close_to_empty_pool() { user, false, ), - Error::::ReserveLeftLessThanMinimal + TokenError::NotExpendable, ); assert_ok!(AssetConversion::swap_tokens_for_exact_tokens( @@ -1308,6 +1318,7 @@ fn swap_when_existential_deposit_would_cause_reaping_but_keep_alive_set() { assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 101)); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user2, 10000 + ed)); assert_ok!(Assets::mint(RuntimeOrigin::signed(user2), 2, user2, 1000)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user2), 2, user, 2)); assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user2), @@ -1343,6 +1354,125 @@ fn swap_when_existential_deposit_would_cause_reaping_but_keep_alive_set() { ), DispatchError::Token(TokenError::NotExpendable) ); + + assert_noop!( + AssetConversion::swap_tokens_for_exact_tokens( + RuntimeOrigin::signed(user), + bvec![token_2, token_1], + 51, // amount_out + 2, // amount_in_max + user, + true, + ), + DispatchError::Token(TokenError::NotExpendable) + ); + + assert_noop!( + AssetConversion::swap_exact_tokens_for_tokens( + RuntimeOrigin::signed(user), + bvec![token_2, token_1], + 2, // amount_in + 1, // amount_out_min + user, + true, + ), + DispatchError::Token(TokenError::NotExpendable) + ); + }); +} + +#[test] +fn swap_when_existential_deposit_would_cause_reaping_pool_account() { + new_test_ext().execute_with(|| { + let user = 1; + let user2 = 2; + let token_1 = NativeOrAssetId::Native; + let token_2 = NativeOrAssetId::Asset(2); + let token_3 = NativeOrAssetId::Asset(3); + + let ed_assets = 100; + create_tokens_with_ed(user2, vec![token_2, token_3], ed_assets); + assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user2), token_1, token_2)); + assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user2), token_1, token_3)); + + let ed = get_ed(); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + ed)); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user2, 20000 + ed)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user2), 2, user2, 200 + ed_assets)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user2), 3, user2, 10000 + ed_assets)); + + assert_ok!(Assets::mint(RuntimeOrigin::signed(user2), 2, user, 400 + ed_assets)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user2), 3, user, 20000 + ed_assets)); + + assert_ok!(AssetConversion::add_liquidity( + RuntimeOrigin::signed(user2), + token_1, + token_2, + 10000, + 200, + 1, + 1, + user2, + )); + + assert_noop!( + AssetConversion::swap_tokens_for_exact_tokens( + RuntimeOrigin::signed(user), + bvec![token_1, token_2], + 110, // amount_out + 20000, // amount_in_max + user, + true, + ), + DispatchError::Token(TokenError::NotExpendable) + ); + + assert_noop!( + AssetConversion::swap_exact_tokens_for_tokens( + RuntimeOrigin::signed(user), + bvec![token_1, token_2], + 15000, // amount_in + 110, // amount_out_min + user, + true, + ), + DispatchError::Token(TokenError::NotExpendable) + ); + + assert_ok!(AssetConversion::add_liquidity( + RuntimeOrigin::signed(user2), + token_1, + token_3, + 200, + 10000, + 1, + 1, + user2, + )); + + assert_noop!( + AssetConversion::swap_tokens_for_exact_tokens( + RuntimeOrigin::signed(user), + bvec![token_3, token_1], + 110, // amount_out + 20000, // amount_in_max + user, + true, + ), + DispatchError::Token(TokenError::NotExpendable) + ); + + assert_noop!( + AssetConversion::swap_exact_tokens_for_tokens( + RuntimeOrigin::signed(user), + bvec![token_3, token_1], + 15000, // amount_in + 110, // amount_out_min + user, + true, + ), + DispatchError::Token(TokenError::NotExpendable) + ); }); } From e237dfc651de6d0a80977996b7657fe132df5123 Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 25 Sep 2023 17:15:17 +0200 Subject: [PATCH 09/43] vec import --- substrate/frame/asset-conversion/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index 37cdfbca5f89..bb084f143e50 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -95,6 +95,7 @@ use sp_runtime::{ }, BoundedVec, DispatchError, Saturating, TokenError, TransactionOutcome, }; +use sp_std::vec::Vec; #[frame_support::pallet] pub mod pallet { From 0c13c85c4eeb58ced65a5a21f27d748b624ce62c Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 25 Sep 2023 18:14:09 +0200 Subject: [PATCH 10/43] fix tests, preserve account --- .../asset-conversion-tx-payment/src/tests.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs index 9e9b74a0ddb2..cad301dc9599 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs @@ -19,7 +19,10 @@ use frame_support::{ assert_ok, dispatch::{DispatchInfo, PostDispatchInfo}, pallet_prelude::*, - traits::{fungible::Inspect, fungibles::Mutate}, + traits::{ + fungible::Inspect, + fungibles::{Inspect as FungiblesInspect, Mutate}, + }, weights::Weight, }; use frame_system as system; @@ -110,13 +113,19 @@ fn default_post_info() -> PostDispatchInfo { fn setup_lp(asset_id: u32, balance_factor: u64) { let lp_provider = 5; + let ed = Balances::minimum_balance(); + let ed_asset = Assets::minimum_balance(asset_id); assert_ok!(Balances::force_set_balance( RuntimeOrigin::root(), lp_provider, - 10_000 * balance_factor + 10_000 * balance_factor + ed, )); let lp_provider_account = ::Lookup::unlookup(lp_provider); - assert_ok!(Assets::mint_into(asset_id.into(), &lp_provider_account, 10_000 * balance_factor)); + assert_ok!(Assets::mint_into( + asset_id.into(), + &lp_provider_account, + 10_000 * balance_factor + ed_asset + )); let token_1 = NativeOrAssetId::Native; let token_2 = NativeOrAssetId::Asset(asset_id); From 24654b88c235dc0b9b7a4d150fbff83909785723 Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 26 Sep 2023 16:08:57 +0200 Subject: [PATCH 11/43] tests --- substrate/frame/asset-conversion/src/tests.rs | 86 ++++++++++++++++--- 1 file changed, 73 insertions(+), 13 deletions(-) diff --git a/substrate/frame/asset-conversion/src/tests.rs b/substrate/frame/asset-conversion/src/tests.rs index 86fb61edcb77..e3530ea3e206 100644 --- a/substrate/frame/asset-conversion/src/tests.rs +++ b/substrate/frame/asset-conversion/src/tests.rs @@ -1394,12 +1394,13 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { create_tokens_with_ed(user2, vec![token_2, token_3], ed_assets); assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user2), token_1, token_2)); assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user2), token_1, token_3)); + assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user2), token_2, token_3)); let ed = get_ed(); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + ed)); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user2, 20000 + ed)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user2), 2, user2, 200 + ed_assets)); - assert_ok!(Assets::mint(RuntimeOrigin::signed(user2), 3, user2, 10000 + ed_assets)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user2), 2, user2, 400 + ed_assets)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user2), 3, user2, 20000 + ed_assets)); assert_ok!(Assets::mint(RuntimeOrigin::signed(user2), 2, user, 400 + ed_assets)); assert_ok!(Assets::mint(RuntimeOrigin::signed(user2), 3, user, 20000 + ed_assets)); @@ -1415,6 +1416,29 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { user2, )); + assert_ok!(AssetConversion::add_liquidity( + RuntimeOrigin::signed(user2), + token_1, + token_3, + 200, + 10000, + 1, + 1, + user2, + )); + + assert_ok!(AssetConversion::add_liquidity( + RuntimeOrigin::signed(user2), + token_2, + token_3, + 200, + 10000, + 1, + 1, + user2, + )); + + // causes an account removal for asset token 2 assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), @@ -1427,6 +1451,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { DispatchError::Token(TokenError::NotExpendable) ); + // causes an account removal for asset token 2 assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), @@ -1439,17 +1464,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { DispatchError::Token(TokenError::NotExpendable) ); - assert_ok!(AssetConversion::add_liquidity( - RuntimeOrigin::signed(user2), - token_1, - token_3, - 200, - 10000, - 1, - 1, - user2, - )); - + // causes an account removal for native token 1 assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), @@ -1462,6 +1477,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { DispatchError::Token(TokenError::NotExpendable) ); + // causes an account removal for native token 1 assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), @@ -1473,6 +1489,50 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { ), DispatchError::Token(TokenError::NotExpendable) ); + + // causes an account removal for native token 1 locate in the middle of a swap path + let amount_in = AssetConversion::balance_path_from_amount_out( + 110, + vec![token_3, token_1].try_into().unwrap(), + ) + .unwrap() + .first() + .map(|(_, a)| a.clone()) + .unwrap(); + + assert_noop!( + AssetConversion::swap_exact_tokens_for_tokens( + RuntimeOrigin::signed(user), + bvec![token_3, token_1, token_2], + amount_in, // amount_in + 1, // amount_out_min + user, + true, + ), + DispatchError::Token(TokenError::NotExpendable) + ); + + // causes an account removal for asset token 2 locate in the middle of a swap path + let amount_in = AssetConversion::balance_path_from_amount_out( + 110, + vec![token_1, token_2].try_into().unwrap(), + ) + .unwrap() + .first() + .map(|(_, a)| a.clone()) + .unwrap(); + + assert_noop!( + AssetConversion::swap_exact_tokens_for_tokens( + RuntimeOrigin::signed(user), + bvec![token_1, token_2, token_3], + amount_in, // amount_in + 1, // amount_out_min + user, + true, + ), + DispatchError::Token(TokenError::NotExpendable) + ); }); } From e0bd8b71cbe2607c96f331d90216a2cc2eb96ebb Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 26 Sep 2023 16:15:33 +0200 Subject: [PATCH 12/43] max lenght of the swap path in swap trait --- substrate/frame/asset-conversion/src/lib.rs | 1 + substrate/frame/asset-conversion/src/swap.rs | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index bb084f143e50..a3fa1464dd52 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -88,6 +88,7 @@ use frame_support::{ }, BoundedBTreeSet, PalletId, }; +use sp_core::Get; use sp_runtime::{ traits::{ CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, Ensure, IntegerSquareRoot, MaybeDisplay, diff --git a/substrate/frame/asset-conversion/src/swap.rs b/substrate/frame/asset-conversion/src/swap.rs index e64fd39782d4..2bff86392b38 100644 --- a/substrate/frame/asset-conversion/src/swap.rs +++ b/substrate/frame/asset-conversion/src/swap.rs @@ -21,6 +21,9 @@ use super::*; /// Trait for providing methods to swap between the various asset classes. pub trait Swap { + /// Returns the upper limit on the length of the swap path. + fn max_path_len() -> u32; + /// 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. @@ -66,6 +69,9 @@ pub trait SwapCredit { /// alter the total supply. type Credit; + /// Returns the upper limit on the length of the swap path. + fn max_path_len() -> u32; + /// Swap exactly `credit_in` of asset `path[0]` for asset `path[last]`. If `amount_out_min` is /// provided and the swap can't achieve at least this amount, an error is returned. /// @@ -94,6 +100,10 @@ pub trait SwapCredit { } impl Swap for Pallet { + fn max_path_len() -> u32 { + T::MaxSwapPathLength::get() + } + fn swap_exact_tokens_for_tokens( sender: T::AccountId, path: Vec, @@ -146,6 +156,10 @@ impl SwapCredit for Pallet { type MultiAssetId = T::MultiAssetId; type Credit = Credit; + fn max_path_len() -> u32 { + T::MaxSwapPathLength::get() + } + fn swap_exact_tokens_for_tokens( path: Vec, credit_in: Self::Credit, From 80dbcab83f26e29ec6801c7d30d15f35a726e780 Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 26 Sep 2023 16:39:45 +0200 Subject: [PATCH 13/43] fix clippy --- substrate/frame/asset-conversion/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/asset-conversion/src/tests.rs b/substrate/frame/asset-conversion/src/tests.rs index e3530ea3e206..6503c4b7d8c0 100644 --- a/substrate/frame/asset-conversion/src/tests.rs +++ b/substrate/frame/asset-conversion/src/tests.rs @@ -1497,7 +1497,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { ) .unwrap() .first() - .map(|(_, a)| a.clone()) + .map(|(_, a)| *a) .unwrap(); assert_noop!( @@ -1519,7 +1519,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { ) .unwrap() .first() - .map(|(_, a)| a.clone()) + .map(|(_, a)| *a) .unwrap(); assert_noop!( From 847207be9dc286137dd68601f325debb343cb459 Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 26 Sep 2023 18:24:38 +0200 Subject: [PATCH 14/43] debug impls for imbalances --- .../src/traits/tokens/fungible/imbalance.rs | 22 +++++++++++-- .../src/traits/tokens/fungibles/imbalance.rs | 33 +++++++++++++++++-- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/substrate/frame/support/src/traits/tokens/fungible/imbalance.rs b/substrate/frame/support/src/traits/tokens/fungible/imbalance.rs index de85924a4de7..1357056d3a07 100644 --- a/substrate/frame/support/src/traits/tokens/fungible/imbalance.rs +++ b/substrate/frame/support/src/traits/tokens/fungible/imbalance.rs @@ -23,7 +23,7 @@ use crate::traits::{ misc::{SameOrOther, TryDrop}, tokens::Balance, }; -use sp_runtime::{traits::Zero, RuntimeDebug}; +use sp_runtime::traits::Zero; use sp_std::marker::PhantomData; /// Handler for when an imbalance gets dropped. This could handle either a credit (negative) or @@ -43,7 +43,7 @@ impl HandleImbalanceDrop for () { /// /// Importantly, it has a special `Drop` impl, and cannot be created outside of this module. #[must_use] -#[derive(RuntimeDebug, Eq, PartialEq)] +#[derive(Eq, PartialEq)] pub struct Imbalance< B: Balance, OnDrop: HandleImbalanceDrop, @@ -141,6 +141,24 @@ impl, OppositeOnDrop: HandleImbalance } } +#[cfg(any(feature = "std", feature = "force-debug"))] +impl, OppositeOnDrop: HandleImbalanceDrop> + sp_std::fmt::Debug for Imbalance +{ + fn fmt(&self, fmt: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { + fmt.debug_struct("Imbalance").field("amount", &self.amount).finish() + } +} + +#[cfg(all(not(feature = "std"), not(feature = "force-debug")))] +impl, OppositeOnDrop: HandleImbalanceDrop> + sp_std::fmt::Debug for Imbalance +{ + fn fmt(&self, fmt: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { + fmt.write_str("") + } +} + /// Imbalance implying that the total_issuance value is less than the sum of all account balances. pub type Debt = Imbalance< >::Balance, diff --git a/substrate/frame/support/src/traits/tokens/fungibles/imbalance.rs b/substrate/frame/support/src/traits/tokens/fungibles/imbalance.rs index 1668268ea2dc..17e22cf917e7 100644 --- a/substrate/frame/support/src/traits/tokens/fungibles/imbalance.rs +++ b/substrate/frame/support/src/traits/tokens/fungibles/imbalance.rs @@ -23,7 +23,7 @@ use crate::traits::{ misc::{SameOrOther, TryDrop}, tokens::{AssetId, Balance}, }; -use sp_runtime::{traits::Zero, RuntimeDebug}; +use sp_runtime::traits::Zero; use sp_std::marker::PhantomData; /// Handler for when an imbalance gets dropped. This could handle either a credit (negative) or @@ -38,7 +38,7 @@ pub trait HandleImbalanceDrop { /// /// Importantly, it has a special `Drop` impl, and cannot be created outside of this module. #[must_use] -#[derive(RuntimeDebug, Eq, PartialEq)] +#[derive(Eq, PartialEq)] pub struct Imbalance< A: AssetId, B: Balance, @@ -158,6 +158,35 @@ impl< } } +#[cfg(any(feature = "std", feature = "force-debug"))] +impl< + A: AssetId, + B: Balance, + OnDrop: HandleImbalanceDrop, + OppositeOnDrop: HandleImbalanceDrop, + > sp_std::fmt::Debug for Imbalance +{ + fn fmt(&self, fmt: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { + fmt.debug_struct("Imbalance") + .field("asset", &self.asset) + .field("amount", &self.amount) + .finish() + } +} + +#[cfg(all(not(feature = "std"), not(feature = "force-debug")))] +impl< + A: AssetId, + B: Balance, + OnDrop: HandleImbalanceDrop, + OppositeOnDrop: HandleImbalanceDrop, + > sp_std::fmt::Debug for Imbalance +{ + fn fmt(&self, fmt: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { + fmt.write_str("") + } +} + /// Imbalance implying that the total_issuance value is less than the sum of all account balances. pub type Debt = Imbalance< >::AssetId, From 53c699f3745927984e1729538cc738f0f6efdaa5 Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 28 Sep 2023 13:16:13 +0200 Subject: [PATCH 15/43] eq and partial_eq impls for imbalance --- .../src/traits/tokens/fungible/imbalance.rs | 20 ++++++++++++++-- .../src/traits/tokens/fungibles/imbalance.rs | 24 ++++++++++++++++++- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/substrate/frame/support/src/traits/tokens/fungible/imbalance.rs b/substrate/frame/support/src/traits/tokens/fungible/imbalance.rs index 1357056d3a07..49b0b506db09 100644 --- a/substrate/frame/support/src/traits/tokens/fungible/imbalance.rs +++ b/substrate/frame/support/src/traits/tokens/fungible/imbalance.rs @@ -43,7 +43,6 @@ impl HandleImbalanceDrop for () { /// /// Importantly, it has a special `Drop` impl, and cannot be created outside of this module. #[must_use] -#[derive(Eq, PartialEq)] pub struct Imbalance< B: Balance, OnDrop: HandleImbalanceDrop, @@ -141,12 +140,29 @@ impl, OppositeOnDrop: HandleImbalance } } +impl, OppositeOnDrop: HandleImbalanceDrop> PartialEq + for Imbalance +{ + fn eq(&self, other: &Self) -> bool { + self.amount.eq(&other.amount) + } +} + +impl, OppositeOnDrop: HandleImbalanceDrop> Eq + for Imbalance +{ +} + #[cfg(any(feature = "std", feature = "force-debug"))] impl, OppositeOnDrop: HandleImbalanceDrop> sp_std::fmt::Debug for Imbalance { fn fmt(&self, fmt: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { - fmt.debug_struct("Imbalance").field("amount", &self.amount).finish() + fmt.debug_struct("Imbalance") + .field("amount", &self.amount) + .field("OnDrop", &sp_std::any::type_name::()) + .field("OppositeOnDrop", &sp_std::any::type_name::()) + .finish() } } diff --git a/substrate/frame/support/src/traits/tokens/fungibles/imbalance.rs b/substrate/frame/support/src/traits/tokens/fungibles/imbalance.rs index 17e22cf917e7..c1880cf087a5 100644 --- a/substrate/frame/support/src/traits/tokens/fungibles/imbalance.rs +++ b/substrate/frame/support/src/traits/tokens/fungibles/imbalance.rs @@ -38,7 +38,6 @@ pub trait HandleImbalanceDrop { /// /// Importantly, it has a special `Drop` impl, and cannot be created outside of this module. #[must_use] -#[derive(Eq, PartialEq)] pub struct Imbalance< A: AssetId, B: Balance, @@ -158,6 +157,27 @@ impl< } } +impl< + A: AssetId, + B: Balance, + OnDrop: HandleImbalanceDrop, + OppositeOnDrop: HandleImbalanceDrop, + > PartialEq for Imbalance +{ + fn eq(&self, other: &Self) -> bool { + self.amount.eq(&other.amount) && self.asset.eq(&other.asset) + } +} + +impl< + A: AssetId, + B: Balance, + OnDrop: HandleImbalanceDrop, + OppositeOnDrop: HandleImbalanceDrop, + > Eq for Imbalance +{ +} + #[cfg(any(feature = "std", feature = "force-debug"))] impl< A: AssetId, @@ -170,6 +190,8 @@ impl< fmt.debug_struct("Imbalance") .field("asset", &self.asset) .field("amount", &self.amount) + .field("OnDrop", &sp_std::any::type_name::()) + .field("OppositeOnDrop", &sp_std::any::type_name::()) .finish() } } From 827ce1bf93049f74f0c53e619063be531443419b Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 28 Sep 2023 13:20:41 +0200 Subject: [PATCH 16/43] eq, partial_eq, runtime debug for credit --- substrate/frame/asset-conversion/src/lib.rs | 2 +- substrate/frame/asset-conversion/src/types.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index a3fa1464dd52..b1fd109aa8fe 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -94,7 +94,7 @@ use sp_runtime::{ CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, Ensure, IntegerSquareRoot, MaybeDisplay, One, TrailingZeroInput, Zero, }, - BoundedVec, DispatchError, Saturating, TokenError, TransactionOutcome, + BoundedVec, DispatchError, RuntimeDebug, Saturating, TokenError, TransactionOutcome, }; use sp_std::vec::Vec; diff --git a/substrate/frame/asset-conversion/src/types.rs b/substrate/frame/asset-conversion/src/types.rs index 1b921a757a5a..466faf434031 100644 --- a/substrate/frame/asset-conversion/src/types.rs +++ b/substrate/frame/asset-conversion/src/types.rs @@ -179,6 +179,7 @@ pub type AssetCredit = /// /// Implies a negative imbalance in the system that can be placed into an account or alter the total /// supply. +#[derive(RuntimeDebug, Eq, PartialEq)] pub enum Credit { /// Native credit. Native(NativeCredit), From d533acf69137a79c8fe79f84e6053f0b06f1bff0 Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 28 Sep 2023 13:21:10 +0200 Subject: [PATCH 17/43] tests --- substrate/frame/asset-conversion/src/tests.rs | 404 +++++++++++++++++- 1 file changed, 403 insertions(+), 1 deletion(-) diff --git a/substrate/frame/asset-conversion/src/tests.rs b/substrate/frame/asset-conversion/src/tests.rs index 6503c4b7d8c0..f86cb6c12d75 100644 --- a/substrate/frame/asset-conversion/src/tests.rs +++ b/substrate/frame/asset-conversion/src/tests.rs @@ -17,7 +17,7 @@ use crate::{mock::*, *}; use frame_support::{ - assert_noop, assert_ok, + assert_noop, assert_ok, assert_storage_noop, instances::Instance1, traits::{fungible, fungibles, fungibles::InspectEnumerable, Get}, }; @@ -1875,3 +1875,405 @@ fn cannot_block_pool_creation() { )); }); } + +#[test] +fn swap_transactional() { + new_test_ext().execute_with(|| { + let user = 1; + let user2 = 2; + let token_1 = NativeOrAssetId::Native; + let token_2 = NativeOrAssetId::Asset(2); + let token_3 = NativeOrAssetId::Asset(3); + + let asset_ed = 150; + create_tokens_with_ed(user, vec![token_2, token_3], asset_ed); + assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_3)); + + let ed = get_ed(); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + ed)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 3, user, 1000)); + + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user2, 20000 + ed)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user2, 1000)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 3, user2, 1000)); + + let liquidity1 = 10000; + let liquidity2 = 200; + + assert_ok!(AssetConversion::add_liquidity( + RuntimeOrigin::signed(user), + token_1, + token_2, + liquidity1, + liquidity2, + 1, + 1, + user, + )); + + assert_ok!(AssetConversion::add_liquidity( + RuntimeOrigin::signed(user), + token_1, + token_3, + liquidity1, + liquidity2, + 1, + 1, + user, + )); + + let pool_1 = AssetConversion::get_pool_account(&(token_1, token_2)); + let pool_2 = AssetConversion::get_pool_account(&(token_1, token_3)); + + assert_eq!(Balances::balance(&pool_1), liquidity1); + assert_eq!(Assets::balance(2, &pool_1), liquidity2); + assert_eq!(Balances::balance(&pool_2), liquidity1); + assert_eq!(Assets::balance(3, &pool_2), liquidity2); + + // the amount that would cause a transfer from the last pool in the path to fail + let expected_out = liquidity2 - asset_ed + 1; + let amount_in = AssetConversion::balance_path_from_amount_out( + expected_out, + vec![token_2, token_1, token_3].try_into().unwrap(), + ) + .unwrap() + .first() + .map(|(_, a)| *a) + .unwrap(); + + // swap credit with `swap_tokens_for_exact_tokens` transactional + let credit_in = Assets::issue(2, amount_in); + let credit_in_err_expected = Assets::issue(2, amount_in); + // avoiding drop of any credit, to assert any storage mutation from an actual call. + let error; + assert_storage_noop!( + error = >::swap_tokens_for_exact_tokens( + bvec![token_2, token_1, token_3], + credit_in.into(), + expected_out, + ) + .unwrap_err() + ); + assert_eq!(error, (credit_in_err_expected.into(), TokenError::NotExpendable.into())); + + // swap credit with `swap_exact_tokens_for_tokens` transactional + let credit_in = Assets::issue(2, amount_in); + let credit_in_err_expected = Assets::issue(2, amount_in); + // avoiding drop of any credit, to assert any storage mutation from an actual call. + let error; + assert_storage_noop!( + error = >::swap_exact_tokens_for_tokens( + bvec![token_2, token_1, token_3], + credit_in.into(), + Some(expected_out), + ) + .unwrap_err() + ); + assert_eq!(error, (credit_in_err_expected.into(), TokenError::NotExpendable.into())); + + // swap with `swap_exact_tokens_for_tokens` transactional + assert_noop!( + >::swap_exact_tokens_for_tokens( + user2, + bvec![token_2, token_1, token_3], + amount_in.into(), + Some(expected_out.into()), + user2, + true, + ), + TokenError::NotExpendable + ); + + // swap with `swap_exact_tokens_for_tokens` transactional + assert_noop!( + >::swap_tokens_for_exact_tokens( + user2, + bvec![token_2, token_1, token_3], + expected_out.into(), + Some(amount_in.into()), + user2, + true, + ), + TokenError::NotExpendable + ); + + assert_eq!(Balances::balance(&pool_1), liquidity1); + assert_eq!(Assets::balance(2, &pool_1), liquidity2); + assert_eq!(Balances::balance(&pool_2), liquidity1); + assert_eq!(Assets::balance(3, &pool_2), liquidity2); + }) +} + +#[test] +fn swap_credit_returns_change() { + new_test_ext().execute_with(|| { + let user = 1; + let token_1 = NativeOrAssetId::Native; + let token_2 = NativeOrAssetId::Asset(2); + + create_tokens(user, vec![token_2]); + assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + + let ed = get_ed(); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + ed)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); + + let liquidity1 = 10000; + let liquidity2 = 200; + + assert_ok!(AssetConversion::add_liquidity( + RuntimeOrigin::signed(user), + token_1, + token_2, + liquidity1, + liquidity2, + 1, + 1, + user, + )); + + let expected_change = Balances::issue(100); + let expected_credit_out = Assets::issue(2, 20); + + let amount_in_max = + AssetConversion::get_amount_in(&expected_credit_out.peek(), &liquidity1, &liquidity2) + .unwrap(); + + let credit_in = Balances::issue(amount_in_max + expected_change.peek()); + assert_ok!( + >::swap_tokens_for_exact_tokens( + bvec![token_1, token_2], + credit_in.into(), + expected_credit_out.peek(), + ), + (expected_credit_out.into(), expected_change.into()) + ); + }) +} + +#[test] +fn swap_credit_insufficient_amount_bounds() { + new_test_ext().execute_with(|| { + let user = 1; + let user2 = 2; + let token_1 = NativeOrAssetId::Native; + let token_2 = NativeOrAssetId::Asset(2); + + create_tokens(user, vec![token_2]); + assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + + let ed = get_ed(); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + ed)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); + + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user2, 20000 + ed)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user2, 1000)); + + let liquidity1 = 10000; + let liquidity2 = 200; + + assert_ok!(AssetConversion::add_liquidity( + RuntimeOrigin::signed(user), + token_1, + token_2, + liquidity1, + liquidity2, + 1, + 1, + user, + )); + + // provided `credit_in` is not sufficient to swap for desired `amount_out_min` + let amount_out_min = 20; + let amount_in = + AssetConversion::get_amount_in(&(amount_out_min - 1), &liquidity2, &liquidity1) + .unwrap(); + let credit_in = Balances::issue(amount_in); + let expected_credit_in = Balances::issue(amount_in); + let error = >::swap_exact_tokens_for_tokens( + bvec![token_1, token_2], + credit_in.into(), + Some(amount_out_min), + ) + .unwrap_err(); + assert_eq!( + error, + (expected_credit_in.into(), Error::::ProvidedMinimumNotSufficientForSwap.into()) + ); + + // provided `credit_in` is not sufficient to swap for desired `amount_out` + let amount_out = 20; + let amount_in_max = + AssetConversion::get_amount_in(&(amount_out - 1), &liquidity2, &liquidity1).unwrap(); + let credit_in = Balances::issue(amount_in_max); + let expected_credit_in = Balances::issue(amount_in_max); + let error = >::swap_tokens_for_exact_tokens( + bvec![token_1, token_2], + credit_in.into(), + amount_out, + ) + .unwrap_err(); + assert_eq!( + error, + (expected_credit_in.into(), Error::::ProvidedMaximumNotSufficientForSwap.into()) + ); + }) +} + +#[test] +fn swap_credit_zero_amount() { + new_test_ext().execute_with(|| { + let user = 1; + let user2 = 2; + let token_1 = NativeOrAssetId::Native; + let token_2 = NativeOrAssetId::Asset(2); + + create_tokens(user, vec![token_2]); + assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + + let ed = get_ed(); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + ed)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); + + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user2, 20000 + ed)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user2, 1000)); + + let liquidity1 = 10000; + let liquidity2 = 200; + + assert_ok!(AssetConversion::add_liquidity( + RuntimeOrigin::signed(user), + token_1, + token_2, + liquidity1, + liquidity2, + 1, + 1, + user, + )); + + // swap with zero credit fails for `swap_exact_tokens_for_tokens` + let credit_in = Credit::native_zero(); + let expected_credit_in = Credit::native_zero(); + let error = >::swap_exact_tokens_for_tokens( + bvec![token_1, token_2], + credit_in, + None, + ) + .unwrap_err(); + assert_eq!(error, (expected_credit_in, Error::::ZeroAmount.into())); + + // swap with zero credit fails for `swap_tokens_for_exact_tokens` + let credit_in = Credit::native_zero(); + let expected_credit_in = Credit::native_zero(); + let error = >::swap_tokens_for_exact_tokens( + bvec![token_1, token_2], + credit_in, + 10, + ) + .unwrap_err(); + assert_eq!(error, (expected_credit_in, Error::::ZeroAmount.into())); + + // swap with zero amount_out_min fails for `swap_exact_tokens_for_tokens` + let credit_in = Balances::issue(10); + let expected_credit_in = Balances::issue(10); + let error = >::swap_exact_tokens_for_tokens( + bvec![token_1, token_2], + credit_in.into(), + Some(0), + ) + .unwrap_err(); + assert_eq!(error, (expected_credit_in.into(), Error::::ZeroAmount.into())); + + // swap with zero amount_out fails with `swap_tokens_for_exact_tokens` fails + let credit_in = Balances::issue(10); + let expected_credit_in = Balances::issue(10); + let error = >::swap_tokens_for_exact_tokens( + bvec![token_1, token_2], + credit_in.into(), + 0, + ) + .unwrap_err(); + assert_eq!(error, (expected_credit_in.into(), Error::::ZeroAmount.into())); + }); +} + +#[test] +fn swap_credit_invalid_path() { + new_test_ext().execute_with(|| { + let user = 1; + let user2 = 2; + let token_1 = NativeOrAssetId::Native; + let token_2 = NativeOrAssetId::Asset(2); + + create_tokens(user, vec![token_2]); + assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + + let ed = get_ed(); + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + ed)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); + + assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user2, 20000 + ed)); + assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user2, 1000)); + + let liquidity1 = 10000; + let liquidity2 = 200; + + assert_ok!(AssetConversion::add_liquidity( + RuntimeOrigin::signed(user), + token_1, + token_2, + liquidity1, + liquidity2, + 1, + 1, + user, + )); + + // swap with credit_in.asset different from path[0] asset fails + let credit_in = Balances::issue(10); + let expected_credit_in = Balances::issue(10); + let error = >::swap_exact_tokens_for_tokens( + bvec![token_2, token_1], + credit_in.into(), + None, + ) + .unwrap_err(); + assert_eq!(error, (expected_credit_in.into(), Error::::InvalidPath.into())); + + // swap with credit_in.asset different from path[0] asset fails + let credit_in = Assets::issue(2, 10); + let expected_credit_in = Assets::issue(2, 10); + let error = >::swap_tokens_for_exact_tokens( + bvec![token_1, token_2], + credit_in.into(), + 10, + ) + .unwrap_err(); + assert_eq!(error, (expected_credit_in.into(), Error::::InvalidPath.into())); + + // swap with path.len < 2 fails + let credit_in = Balances::issue(10); + let expected_credit_in = Balances::issue(10); + let error = >::swap_exact_tokens_for_tokens( + bvec![token_2], + credit_in.into(), + None, + ) + .unwrap_err(); + assert_eq!(error, (expected_credit_in.into(), Error::::InvalidPath.into())); + + // swap with path.len < 2 fails + let credit_in = Assets::issue(2, 10); + let expected_credit_in = Assets::issue(2, 10); + let error = >::swap_tokens_for_exact_tokens( + bvec![], + credit_in.into(), + 10, + ) + .unwrap_err(); + assert_eq!(error, (expected_credit_in.into(), Error::::InvalidPath.into())); + }); +} From 69b17f81634ed41daf4f922ae7159e93961dc771 Mon Sep 17 00:00:00 2001 From: Muharem Date: Thu, 28 Sep 2023 13:24:07 +0200 Subject: [PATCH 18/43] Apply suggestions from code review Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> --- substrate/frame/asset-conversion/src/lib.rs | 4 ++-- substrate/frame/asset-conversion/src/swap.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index b1fd109aa8fe..33c761611e25 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -283,7 +283,7 @@ pub mod pallet { amount_in: T::AssetBalance, /// The amount of the second asset that was received. amount_out: T::AssetBalance, - /// The route of asset ids with amounts that the swap went through. + /// The route of asset IDs with amounts that the swap went through. /// E.g. (A, amount_in) -> (Dot, amount_out) -> (B, amount_out) path: BalancePath, }, @@ -293,7 +293,7 @@ pub mod pallet { amount_in: T::AssetBalance, /// The amount of the second asset that was received. amount_out: T::AssetBalance, - /// The route of asset ids with amounts that the swap went through. + /// The route of asset IDs with amounts that the swap went through. /// E.g. (A, amount_in) -> (Dot, amount_out) -> (B, amount_out) path: BalancePath, }, diff --git a/substrate/frame/asset-conversion/src/swap.rs b/substrate/frame/asset-conversion/src/swap.rs index 2bff86392b38..fed03363b497 100644 --- a/substrate/frame/asset-conversion/src/swap.rs +++ b/substrate/frame/asset-conversion/src/swap.rs @@ -24,7 +24,7 @@ pub trait Swap { /// Returns the upper limit on the length of the swap path. fn max_path_len() -> u32; - /// Swap exactly `amount_in` of asset `path[0]` for asset `path[1]`. + /// Swap exactly `amount_in` of asset `path[0]` for asset `path[last]`. /// If an `amount_out_min` is specified, it will return an error if it is unable to acquire /// the amount desired. /// From 28edff58cee8ef6026ad2273414d83fa51f89ed4 Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 28 Sep 2023 13:26:23 +0200 Subject: [PATCH 19/43] correct docs --- substrate/frame/asset-conversion/src/swap.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/substrate/frame/asset-conversion/src/swap.rs b/substrate/frame/asset-conversion/src/swap.rs index fed03363b497..ec1f03a461b1 100644 --- a/substrate/frame/asset-conversion/src/swap.rs +++ b/substrate/frame/asset-conversion/src/swap.rs @@ -28,10 +28,10 @@ pub trait Swap { /// 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`, + /// Withdraws the `path[0]` asset from `sender`, deposits the `path[last]` asset to `send_to`, /// respecting `keep_alive`. /// - /// If successful, returns the amount of `path[1]` acquired for the `amount_in`. + /// If successful, returns the amount of `path[last]` acquired for the `amount_in`. fn swap_exact_tokens_for_tokens( sender: AccountId, path: Vec, @@ -41,14 +41,14 @@ pub trait Swap { keep_alive: bool, ) -> Result; - /// Take the `path[0]` asset and swap some amount for `amount_out` of the `path[1]`. If an + /// Take the `path[0]` asset and swap some amount for `amount_out` of the `path[last]`. 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`, + /// Withdraws `path[0]` asset from `sender`, deposits `path[last]` asset to `send_to`, /// respecting `keep_alive`. /// - /// If successful returns the amount of the `path[0]` taken to provide `path[1]`. + /// If successful returns the amount of the `path[0]` taken to provide `path[last]`. fn swap_tokens_for_exact_tokens( sender: AccountId, path: Vec, From 697afa6b1ef1c2bfe4b774b648cd59dc0fb492ba Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 28 Sep 2023 13:32:50 +0200 Subject: [PATCH 20/43] dev comments --- substrate/frame/asset-conversion/src/swap.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/substrate/frame/asset-conversion/src/swap.rs b/substrate/frame/asset-conversion/src/swap.rs index ec1f03a461b1..e65ea08c36b5 100644 --- a/substrate/frame/asset-conversion/src/swap.rs +++ b/substrate/frame/asset-conversion/src/swap.rs @@ -175,6 +175,8 @@ impl SwapCredit for Pallet { Self::do_swap_exact_credit_tokens_for_tokens(path, credit_in, amount_out_min); match &res { Ok(_) => TransactionOutcome::Commit(Ok(res)), + // wrapping `res` with `Ok`, since our `Err` doesn't satisfy the + // `From` bound of the `with_transaction` function. Err(_) => TransactionOutcome::Rollback(Ok(res)), } }); @@ -199,6 +201,8 @@ impl SwapCredit for Pallet { let res = Self::do_swap_credit_tokens_for_exact_tokens(path, credit_in, amount_out); match &res { Ok(_) => TransactionOutcome::Commit(Ok(res)), + // wrapping `res` with `Ok`, since our `Err` doesn't satisfy the + // `From` bound of the `with_transaction` function. Err(_) => TransactionOutcome::Rollback(Ok(res)), } }); From 1c5022014cd21cc698406684209dc88edeb2aee4 Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 29 Sep 2023 17:50:54 +0200 Subject: [PATCH 21/43] use eq/partial_eq/runtime_debug derives with no bound version --- .../src/traits/tokens/fungible/imbalance.rs | 37 +------------ .../src/traits/tokens/fungibles/imbalance.rs | 54 +------------------ 2 files changed, 4 insertions(+), 87 deletions(-) diff --git a/substrate/frame/support/src/traits/tokens/fungible/imbalance.rs b/substrate/frame/support/src/traits/tokens/fungible/imbalance.rs index 49b0b506db09..32a63fd25b29 100644 --- a/substrate/frame/support/src/traits/tokens/fungible/imbalance.rs +++ b/substrate/frame/support/src/traits/tokens/fungible/imbalance.rs @@ -23,6 +23,7 @@ use crate::traits::{ misc::{SameOrOther, TryDrop}, tokens::Balance, }; +use frame_support_procedural::{EqNoBound, PartialEqNoBound, RuntimeDebugNoBound}; use sp_runtime::traits::Zero; use sp_std::marker::PhantomData; @@ -43,6 +44,7 @@ impl HandleImbalanceDrop for () { /// /// Importantly, it has a special `Drop` impl, and cannot be created outside of this module. #[must_use] +#[derive(EqNoBound, PartialEqNoBound, RuntimeDebugNoBound)] pub struct Imbalance< B: Balance, OnDrop: HandleImbalanceDrop, @@ -140,41 +142,6 @@ impl, OppositeOnDrop: HandleImbalance } } -impl, OppositeOnDrop: HandleImbalanceDrop> PartialEq - for Imbalance -{ - fn eq(&self, other: &Self) -> bool { - self.amount.eq(&other.amount) - } -} - -impl, OppositeOnDrop: HandleImbalanceDrop> Eq - for Imbalance -{ -} - -#[cfg(any(feature = "std", feature = "force-debug"))] -impl, OppositeOnDrop: HandleImbalanceDrop> - sp_std::fmt::Debug for Imbalance -{ - fn fmt(&self, fmt: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { - fmt.debug_struct("Imbalance") - .field("amount", &self.amount) - .field("OnDrop", &sp_std::any::type_name::()) - .field("OppositeOnDrop", &sp_std::any::type_name::()) - .finish() - } -} - -#[cfg(all(not(feature = "std"), not(feature = "force-debug")))] -impl, OppositeOnDrop: HandleImbalanceDrop> - sp_std::fmt::Debug for Imbalance -{ - fn fmt(&self, fmt: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { - fmt.write_str("") - } -} - /// Imbalance implying that the total_issuance value is less than the sum of all account balances. pub type Debt = Imbalance< >::Balance, diff --git a/substrate/frame/support/src/traits/tokens/fungibles/imbalance.rs b/substrate/frame/support/src/traits/tokens/fungibles/imbalance.rs index c1880cf087a5..9f660a5f1689 100644 --- a/substrate/frame/support/src/traits/tokens/fungibles/imbalance.rs +++ b/substrate/frame/support/src/traits/tokens/fungibles/imbalance.rs @@ -23,6 +23,7 @@ use crate::traits::{ misc::{SameOrOther, TryDrop}, tokens::{AssetId, Balance}, }; +use frame_support_procedural::{EqNoBound, PartialEqNoBound, RuntimeDebugNoBound}; use sp_runtime::traits::Zero; use sp_std::marker::PhantomData; @@ -38,6 +39,7 @@ pub trait HandleImbalanceDrop { /// /// Importantly, it has a special `Drop` impl, and cannot be created outside of this module. #[must_use] +#[derive(EqNoBound, PartialEqNoBound, RuntimeDebugNoBound)] pub struct Imbalance< A: AssetId, B: Balance, @@ -157,58 +159,6 @@ impl< } } -impl< - A: AssetId, - B: Balance, - OnDrop: HandleImbalanceDrop, - OppositeOnDrop: HandleImbalanceDrop, - > PartialEq for Imbalance -{ - fn eq(&self, other: &Self) -> bool { - self.amount.eq(&other.amount) && self.asset.eq(&other.asset) - } -} - -impl< - A: AssetId, - B: Balance, - OnDrop: HandleImbalanceDrop, - OppositeOnDrop: HandleImbalanceDrop, - > Eq for Imbalance -{ -} - -#[cfg(any(feature = "std", feature = "force-debug"))] -impl< - A: AssetId, - B: Balance, - OnDrop: HandleImbalanceDrop, - OppositeOnDrop: HandleImbalanceDrop, - > sp_std::fmt::Debug for Imbalance -{ - fn fmt(&self, fmt: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { - fmt.debug_struct("Imbalance") - .field("asset", &self.asset) - .field("amount", &self.amount) - .field("OnDrop", &sp_std::any::type_name::()) - .field("OppositeOnDrop", &sp_std::any::type_name::()) - .finish() - } -} - -#[cfg(all(not(feature = "std"), not(feature = "force-debug")))] -impl< - A: AssetId, - B: Balance, - OnDrop: HandleImbalanceDrop, - OppositeOnDrop: HandleImbalanceDrop, - > sp_std::fmt::Debug for Imbalance -{ - fn fmt(&self, fmt: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { - fmt.write_str("") - } -} - /// Imbalance implying that the total_issuance value is less than the sum of all account balances. pub type Debt = Imbalance< >::AssetId, From e7ffbb412cd62d47ecac5f2906f419755410df52 Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 5 Oct 2023 13:39:17 +0200 Subject: [PATCH 22/43] swap trait update --- substrate/frame/asset-conversion/src/swap.rs | 44 +++++++++++-------- substrate/frame/asset-conversion/src/tests.rs | 4 +- .../src/payment.rs | 2 +- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/substrate/frame/asset-conversion/src/swap.rs b/substrate/frame/asset-conversion/src/swap.rs index e65ea08c36b5..92cd4c8877a5 100644 --- a/substrate/frame/asset-conversion/src/swap.rs +++ b/substrate/frame/asset-conversion/src/swap.rs @@ -20,7 +20,12 @@ use super::*; /// Trait for providing methods to swap between the various asset classes. -pub trait Swap { +pub trait Swap { + /// Measure units of the asset classes for swapping. + type Balance; + /// Kind of assets that are going to be swapped. + type MultiAssetId; + /// Returns the upper limit on the length of the swap path. fn max_path_len() -> u32; @@ -34,12 +39,12 @@ pub trait Swap { /// If successful, returns the amount of `path[last]` acquired for the `amount_in`. fn swap_exact_tokens_for_tokens( sender: AccountId, - path: Vec, - amount_in: Balance, - amount_out_min: Option, + path: Vec, + amount_in: Self::Balance, + amount_out_min: Option, send_to: AccountId, keep_alive: bool, - ) -> Result; + ) -> Result; /// Take the `path[0]` asset and swap some amount for `amount_out` of the `path[last]`. If an /// `amount_in_max` is specified, it will return an error if acquiring `amount_out` would be @@ -51,12 +56,12 @@ pub trait Swap { /// If successful returns the amount of the `path[0]` taken to provide `path[last]`. fn swap_tokens_for_exact_tokens( sender: AccountId, - path: Vec, - amount_out: Balance, - amount_in_max: Option, + path: Vec, + amount_out: Self::Balance, + amount_in_max: Option, send_to: AccountId, keep_alive: bool, - ) -> Result; + ) -> Result; } /// Trait providing methods to swap between the various asset classes. @@ -99,19 +104,22 @@ pub trait SwapCredit { ) -> Result<(Self::Credit, Self::Credit), (Self::Credit, DispatchError)>; } -impl Swap for Pallet { +impl Swap for Pallet { + type Balance = T::HigherPrecisionBalance; + type MultiAssetId = T::MultiAssetId; + fn max_path_len() -> u32 { T::MaxSwapPathLength::get() } fn swap_exact_tokens_for_tokens( sender: T::AccountId, - path: Vec, - amount_in: T::HigherPrecisionBalance, - amount_out_min: Option, + path: Vec, + amount_in: Self::Balance, + amount_out_min: Option, send_to: T::AccountId, keep_alive: bool, - ) -> Result { + ) -> Result { let path = path.try_into().map_err(|_| Error::::PathError)?; let amount_out_min = amount_out_min.map(Self::convert_hpb_to_asset_balance).transpose()?; let amount_out = with_storage_layer(|| { @@ -129,12 +137,12 @@ impl Swap f fn swap_tokens_for_exact_tokens( sender: T::AccountId, - path: Vec, - amount_out: T::HigherPrecisionBalance, - amount_in_max: Option, + path: Vec, + amount_out: Self::Balance, + amount_in_max: Option, send_to: T::AccountId, keep_alive: bool, - ) -> Result { + ) -> Result { let path = path.try_into().map_err(|_| Error::::PathError)?; let amount_in_max = amount_in_max.map(Self::convert_hpb_to_asset_balance).transpose()?; let amount_in = with_storage_layer(|| { diff --git a/substrate/frame/asset-conversion/src/tests.rs b/substrate/frame/asset-conversion/src/tests.rs index f86cb6c12d75..05bdaf1018ed 100644 --- a/substrate/frame/asset-conversion/src/tests.rs +++ b/substrate/frame/asset-conversion/src/tests.rs @@ -1975,7 +1975,7 @@ fn swap_transactional() { // swap with `swap_exact_tokens_for_tokens` transactional assert_noop!( - >::swap_exact_tokens_for_tokens( + >::swap_exact_tokens_for_tokens( user2, bvec![token_2, token_1, token_3], amount_in.into(), @@ -1988,7 +1988,7 @@ fn swap_transactional() { // swap with `swap_exact_tokens_for_tokens` transactional assert_noop!( - >::swap_tokens_for_exact_tokens( + >::swap_tokens_for_exact_tokens( user2, bvec![token_2, token_1, token_3], expected_out.into(), diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs index 0d090211d035..ac13e121dd65 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs @@ -83,7 +83,7 @@ impl OnChargeAssetTransaction for AssetConversionAdapter where T: Config, C: Inspect<::AccountId>, - CON: Swap, + CON: Swap, T::HigherPrecisionBalance: From> + TryInto>, T::MultiAssetId: From>, BalanceOf: IsType<::AccountId>>::Balance>, From ee49f271e5187a3ade386066c14ca43332ad8734 Mon Sep 17 00:00:00 2001 From: muharem Date: Sat, 7 Oct 2023 16:23:36 +0200 Subject: [PATCH 23/43] box multi asset id --- substrate/frame/asset-conversion/src/lib.rs | 38 +- substrate/frame/asset-conversion/src/tests.rs | 475 ++++++++++++------ 2 files changed, 350 insertions(+), 163 deletions(-) diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index 33c761611e25..356b7f41e924 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -381,14 +381,14 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::create_pool())] pub fn create_pool( origin: OriginFor, - asset1: T::MultiAssetId, - asset2: T::MultiAssetId, + asset1: Box, + asset2: Box, ) -> DispatchResult { let sender = ensure_signed(origin)?; ensure!(asset1 != asset2, Error::::EqualAssets); // prepare pool_id - let pool_id = Self::get_pool_id(asset1, asset2); + let pool_id = Self::get_pool_id(*asset1, *asset2); ensure!(!Pools::::contains_key(&pool_id), Error::::PoolExists); let (asset1, asset2) = &pool_id; if !T::AllowMultiAssetPools::get() && !T::MultiAssetIdConverter::is_native(asset1) { @@ -459,8 +459,8 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::add_liquidity())] pub fn add_liquidity( origin: OriginFor, - asset1: T::MultiAssetId, - asset2: T::MultiAssetId, + asset1: Box, + asset2: Box, amount1_desired: T::AssetBalance, amount2_desired: T::AssetBalance, amount1_min: T::AssetBalance, @@ -469,10 +469,10 @@ pub mod pallet { ) -> DispatchResult { let sender = ensure_signed(origin)?; - let pool_id = Self::get_pool_id(asset1.clone(), asset2.clone()); + let pool_id = Self::get_pool_id(*asset1.clone(), *asset2); // swap params if needed let (amount1_desired, amount2_desired, amount1_min, amount2_min) = - if pool_id.0 == asset1 { + if pool_id.0 == *asset1 { (amount1_desired, amount2_desired, amount1_min, amount2_min) } else { (amount2_desired, amount1_desired, amount2_min, amount1_min) @@ -571,8 +571,8 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::remove_liquidity())] pub fn remove_liquidity( origin: OriginFor, - asset1: T::MultiAssetId, - asset2: T::MultiAssetId, + asset1: Box, + asset2: Box, lp_token_burn: T::AssetBalance, amount1_min_receive: T::AssetBalance, amount2_min_receive: T::AssetBalance, @@ -580,9 +580,9 @@ pub mod pallet { ) -> DispatchResult { let sender = ensure_signed(origin)?; - let pool_id = Self::get_pool_id(asset1.clone(), asset2.clone()); + let pool_id = Self::get_pool_id(*asset1.clone(), *asset2); // swap params if needed - let (amount1_min_receive, amount2_min_receive) = if pool_id.0 == asset1 { + let (amount1_min_receive, amount2_min_receive) = if pool_id.0 == *asset1 { (amount1_min_receive, amount2_min_receive) } else { (amount2_min_receive, amount1_min_receive) @@ -650,7 +650,7 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::swap_exact_tokens_for_tokens())] pub fn swap_exact_tokens_for_tokens( origin: OriginFor, - path: BoundedVec, + path: BoundedVec, T::MaxSwapPathLength>, amount_in: T::AssetBalance, amount_out_min: T::AssetBalance, send_to: T::AccountId, @@ -659,7 +659,11 @@ pub mod pallet { let sender = ensure_signed(origin)?; Self::do_swap_exact_tokens_for_tokens( sender, - path, + path.into_iter() + .map(|a| *a) + .collect::>() + .try_into() + .map_err(|_| Error::::InvalidPath)?, amount_in, Some(amount_out_min), send_to, @@ -678,7 +682,7 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::swap_tokens_for_exact_tokens())] pub fn swap_tokens_for_exact_tokens( origin: OriginFor, - path: BoundedVec, + path: BoundedVec, T::MaxSwapPathLength>, amount_out: T::AssetBalance, amount_in_max: T::AssetBalance, send_to: T::AccountId, @@ -687,7 +691,11 @@ pub mod pallet { let sender = ensure_signed(origin)?; Self::do_swap_tokens_for_exact_tokens( sender, - path, + path.into_iter() + .map(|a| *a) + .collect::>() + .try_into() + .map_err(|_| Error::::InvalidPath)?, amount_out, Some(amount_in_max), send_to, diff --git a/substrate/frame/asset-conversion/src/tests.rs b/substrate/frame/asset-conversion/src/tests.rs index 05bdaf1018ed..d6a041f98e0c 100644 --- a/substrate/frame/asset-conversion/src/tests.rs +++ b/substrate/frame/asset-conversion/src/tests.rs @@ -100,6 +100,12 @@ macro_rules! bvec { } } +macro_rules! bbvec { + ($( $x:ident ),*) => { + vec![$( Box::new( $x ), )*].try_into().unwrap() + } +} + #[test] fn check_pool_accounts_dont_collide() { use std::collections::HashSet; @@ -149,7 +155,11 @@ fn can_create_pool() { let lp_token = AssetConversion::get_next_pool_asset_id(); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 1000)); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_2, token_1)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_2), + Box::new(token_1) + )); let setup_fee = <::PoolSetupFee as Get<::Balance>>::get(); let pool_account = <::PoolSetupFeeReceiver as Get>::get(); @@ -174,24 +184,40 @@ fn can_create_pool() { assert_eq!(pool_assets(), vec![lp_token]); assert_noop!( - AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_1), + AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_1) + ), Error::::EqualAssets ); assert_noop!( - AssetConversion::create_pool(RuntimeOrigin::signed(user), token_2, token_2), + AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_2), + Box::new(token_2) + ), Error::::EqualAssets ); // validate we can create Asset(1)/Asset(2) pool let token_1 = NativeOrAssetId::Asset(1); create_tokens(user, vec![token_1]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); // validate we can force the first asset to be the Native currency only AllowMultiAssetPools::set(&false); let token_1 = NativeOrAssetId::Asset(3); assert_noop!( - AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2), + AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + ), Error::::PoolMustContainNativeCurrency ); }); @@ -207,19 +233,31 @@ fn create_same_pool_twice_should_fail() { create_tokens(user, vec![token_2]); let lp_token = AssetConversion::get_next_pool_asset_id(); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_2, token_1)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_2), + Box::new(token_1) + )); let expected_free = lp_token + 1; assert_eq!(expected_free, AssetConversion::get_next_pool_asset_id()); assert_noop!( - AssetConversion::create_pool(RuntimeOrigin::signed(user), token_2, token_1), + AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_2), + Box::new(token_1) + ), Error::::PoolExists ); assert_eq!(expected_free, AssetConversion::get_next_pool_asset_id()); // Try switching the same tokens around: assert_noop!( - AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2), + AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + ), Error::::PoolExists ); assert_eq!(expected_free, AssetConversion::get_next_pool_asset_id()); @@ -239,7 +277,11 @@ fn different_pools_should_have_different_lp_tokens() { create_tokens(user, vec![token_2, token_3]); let lp_token2_1 = AssetConversion::get_next_pool_asset_id(); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_2, token_1)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_2), + Box::new(token_1) + )); let lp_token3_1 = AssetConversion::get_next_pool_asset_id(); assert_eq!( @@ -252,7 +294,11 @@ fn different_pools_should_have_different_lp_tokens() { }] ); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_3, token_1)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_3), + Box::new(token_1) + )); assert_eq!( events(), [Event::::PoolCreated { @@ -277,9 +323,17 @@ fn can_add_liquidity() { create_tokens(user, vec![token_2, token_3]); let lp_token1 = AssetConversion::get_next_pool_asset_id(); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); let lp_token2 = AssetConversion::get_next_pool_asset_id(); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_3)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_3) + )); let ed = get_ed(); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 10000 * 2 + ed)); @@ -288,8 +342,8 @@ fn can_add_liquidity() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), 10000, 10, 10000, @@ -317,8 +371,8 @@ fn can_add_liquidity() { // try to pass the non-native - native assets, the result should be the same assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_3, - token_1, + Box::new(token_3), + Box::new(token_1), 10, 10000, 10, @@ -353,7 +407,11 @@ fn add_tiny_liquidity_leads_to_insufficient_liquidity_minted_error() { let token_2 = NativeOrAssetId::Asset(2); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 1000)); assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); @@ -361,8 +419,8 @@ fn add_tiny_liquidity_leads_to_insufficient_liquidity_minted_error() { assert_noop!( AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), 1, 1, 1, @@ -375,8 +433,8 @@ fn add_tiny_liquidity_leads_to_insufficient_liquidity_minted_error() { assert_noop!( AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), get_ed(), 1, 1, @@ -397,8 +455,16 @@ fn add_tiny_liquidity_directly_to_pool_address() { let token_3 = NativeOrAssetId::Asset(3); create_tokens(user, vec![token_2, token_3]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_3)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_3) + )); let ed = get_ed(); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 10000 * 2 + ed)); @@ -411,8 +477,8 @@ fn add_tiny_liquidity_directly_to_pool_address() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), 10000, 10, 10000, @@ -425,8 +491,8 @@ fn add_tiny_liquidity_directly_to_pool_address() { assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, pallet_account, 1)); assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_3, + Box::new(token_1), + Box::new(token_3), 10000, 10, 10000, @@ -446,7 +512,11 @@ fn can_remove_liquidity() { create_tokens(user, vec![token_2]); let lp_token = AssetConversion::get_next_pool_asset_id(); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); let ed_token_1 = >::minimum_balance(); let ed_token_2 = >::minimum_balance(2); @@ -459,8 +529,8 @@ fn can_remove_liquidity() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), 1000000000, 100000, 1000000000, @@ -473,8 +543,8 @@ fn can_remove_liquidity() { assert_ok!(AssetConversion::remove_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), total_lp_received, 0, 0, @@ -512,15 +582,19 @@ fn can_not_redeem_more_lp_tokens_than_were_minted() { let lp_token = AssetConversion::get_next_pool_asset_id(); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 10000 + get_ed())); assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), 10000, 10, 10000, @@ -534,8 +608,8 @@ fn can_not_redeem_more_lp_tokens_than_were_minted() { assert_noop!( AssetConversion::remove_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), 216 + 1, // Try and redeem 10 lp tokens while only 9 minted. 0, 0, @@ -554,15 +628,19 @@ fn can_quote_price() { let token_2 = NativeOrAssetId::Asset(2); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 100000)); assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), 10000, 200, 1, @@ -775,15 +853,19 @@ fn quote_price_exact_tokens_for_tokens_matches_execution() { let token_2 = NativeOrAssetId::Asset(2); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 100000)); assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), 10000, 200, 1, @@ -803,7 +885,7 @@ fn quote_price_exact_tokens_for_tokens_matches_execution() { assert_eq!(prior_dot_balance, balance(user2, token_1)); assert_ok!(AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user2), - bvec![token_2, token_1], + bbvec![token_2, token_1], amount, 1, user2, @@ -823,15 +905,19 @@ fn quote_price_tokens_for_exact_tokens_matches_execution() { let token_2 = NativeOrAssetId::Asset(2); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 100000)); assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), 10000, 200, 1, @@ -853,7 +939,7 @@ fn quote_price_tokens_for_exact_tokens_matches_execution() { assert_eq!(prior_asset_balance, balance(user2, token_2)); assert_ok!(AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user2), - bvec![token_2, token_1], + bbvec![token_2, token_1], amount, 1, user2, @@ -874,7 +960,11 @@ fn can_swap_with_native() { let pool_id = (token_1, token_2); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); let ed = get_ed(); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 10000 + ed)); @@ -885,8 +975,8 @@ fn can_swap_with_native() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), liquidity1, liquidity2, 1, @@ -902,7 +992,7 @@ fn can_swap_with_native() { assert_ok!(AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![token_2, token_1], + bbvec![token_2, token_1], input_amount, 1, user, @@ -924,7 +1014,11 @@ fn can_swap_with_realistic_values() { let dot = NativeOrAssetId::Native; let usd = NativeOrAssetId::Asset(2); create_tokens(user, vec![usd]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), dot, usd)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(dot), + Box::new(usd) + )); const UNIT: u128 = 1_000_000_000; @@ -935,8 +1029,8 @@ fn can_swap_with_realistic_values() { let liquidity_usd = 1_000_000 * UNIT; assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - dot, - usd, + Box::new(dot), + Box::new(usd), liquidity_dot, liquidity_usd, 1, @@ -948,7 +1042,7 @@ fn can_swap_with_realistic_values() { assert_ok!(AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![usd, dot], + bbvec![usd, dot], input_amount, 1, user, @@ -973,13 +1067,17 @@ fn can_not_swap_in_pool_with_no_liquidity_added_yet() { let token_2 = NativeOrAssetId::Asset(2); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); // Check can't swap an empty pool assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![token_2, token_1], + bbvec![token_2, token_1], 10, 1, user, @@ -1000,7 +1098,11 @@ fn check_no_panic_when_try_swap_close_to_empty_pool() { let lp_token = AssetConversion::get_next_pool_asset_id(); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); let ed = get_ed(); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 10000 + ed)); @@ -1011,8 +1113,8 @@ fn check_no_panic_when_try_swap_close_to_empty_pool() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), liquidity1, liquidity2, 1, @@ -1037,8 +1139,8 @@ fn check_no_panic_when_try_swap_close_to_empty_pool() { assert_ok!(AssetConversion::remove_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), lp_token_minted, 1, 1, @@ -1054,7 +1156,7 @@ fn check_no_panic_when_try_swap_close_to_empty_pool() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bvec![token_2, token_1], + bbvec![token_2, token_1], 708 - ed + 1, // amount_out 500, // amount_in_max user, @@ -1065,7 +1167,7 @@ fn check_no_panic_when_try_swap_close_to_empty_pool() { assert_ok!(AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bvec![token_2, token_1], + bbvec![token_2, token_1], 608, // amount_out 500, // amount_in_max user, @@ -1087,7 +1189,7 @@ fn check_no_panic_when_try_swap_close_to_empty_pool() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bvec![token_2, token_1], + bbvec![token_2, token_1], token_1_left - 1, // amount_out 1000, // amount_in_max user, @@ -1100,7 +1202,7 @@ fn check_no_panic_when_try_swap_close_to_empty_pool() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bvec![token_2, token_1], + bbvec![token_2, token_1], token_1_left, // amount_out 1000, // amount_in_max user, @@ -1119,7 +1221,11 @@ fn swap_should_not_work_if_too_much_slippage() { let token_2 = NativeOrAssetId::Asset(2); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 10000 + get_ed())); assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); @@ -1129,8 +1235,8 @@ fn swap_should_not_work_if_too_much_slippage() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), liquidity1, liquidity2, 1, @@ -1143,7 +1249,7 @@ fn swap_should_not_work_if_too_much_slippage() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![token_2, token_1], + bbvec![token_2, token_1], exchange_amount, // amount_in 4000, // amount_out_min user, @@ -1163,7 +1269,11 @@ fn can_swap_tokens_for_exact_tokens() { let pool_id = (token_1, token_2); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); let ed = get_ed(); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + ed)); @@ -1178,8 +1288,8 @@ fn can_swap_tokens_for_exact_tokens() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), liquidity1, liquidity2, 1, @@ -1194,7 +1304,7 @@ fn can_swap_tokens_for_exact_tokens() { assert_ok!(AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bvec![token_1, token_2], + bbvec![token_1, token_2], exchange_out, // amount_out 3500, // amount_in_max user, @@ -1225,7 +1335,11 @@ fn can_swap_tokens_for_exact_tokens_when_not_liquidity_provider() { let lp_token = AssetConversion::get_next_pool_asset_id(); create_tokens(user2, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user2), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user2), + Box::new(token_1), + Box::new(token_2) + )); let ed = get_ed(); let base1 = 10000; @@ -1245,8 +1359,8 @@ fn can_swap_tokens_for_exact_tokens_when_not_liquidity_provider() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user2), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), liquidity1, liquidity2, 1, @@ -1264,7 +1378,7 @@ fn can_swap_tokens_for_exact_tokens_when_not_liquidity_provider() { assert_ok!(AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bvec![token_1, token_2], + bbvec![token_1, token_2], exchange_out, // amount_out 3500, // amount_in_max user, @@ -1293,8 +1407,8 @@ fn can_swap_tokens_for_exact_tokens_when_not_liquidity_provider() { assert_ok!(AssetConversion::remove_liquidity( RuntimeOrigin::signed(user2), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), lp_token_minted, 0, 0, @@ -1312,7 +1426,11 @@ fn swap_when_existential_deposit_would_cause_reaping_but_keep_alive_set() { let token_2 = NativeOrAssetId::Asset(2); create_tokens(user2, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user2), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user2), + Box::new(token_1), + Box::new(token_2) + )); let ed = get_ed(); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 101)); @@ -1322,8 +1440,8 @@ fn swap_when_existential_deposit_would_cause_reaping_but_keep_alive_set() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user2), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), 10000, 200, 1, @@ -1334,7 +1452,7 @@ fn swap_when_existential_deposit_would_cause_reaping_but_keep_alive_set() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bvec![token_1, token_2], + bbvec![token_1, token_2], 1, // amount_out 101, // amount_in_max user, @@ -1346,7 +1464,7 @@ fn swap_when_existential_deposit_would_cause_reaping_but_keep_alive_set() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![token_1, token_2], + bbvec![token_1, token_2], 51, // amount_in 1, // amount_out_min user, @@ -1358,7 +1476,7 @@ fn swap_when_existential_deposit_would_cause_reaping_but_keep_alive_set() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bvec![token_2, token_1], + bbvec![token_2, token_1], 51, // amount_out 2, // amount_in_max user, @@ -1370,7 +1488,7 @@ fn swap_when_existential_deposit_would_cause_reaping_but_keep_alive_set() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![token_2, token_1], + bbvec![token_2, token_1], 2, // amount_in 1, // amount_out_min user, @@ -1392,9 +1510,21 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { let ed_assets = 100; create_tokens_with_ed(user2, vec![token_2, token_3], ed_assets); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user2), token_1, token_2)); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user2), token_1, token_3)); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user2), token_2, token_3)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user2), + Box::new(token_1), + Box::new(token_2) + )); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user2), + Box::new(token_1), + Box::new(token_3) + )); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user2), + Box::new(token_2), + Box::new(token_3) + )); let ed = get_ed(); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + ed)); @@ -1407,8 +1537,8 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user2), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), 10000, 200, 1, @@ -1418,8 +1548,8 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user2), - token_1, - token_3, + Box::new(token_1), + Box::new(token_3), 200, 10000, 1, @@ -1429,8 +1559,8 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user2), - token_2, - token_3, + Box::new(token_2), + Box::new(token_3), 200, 10000, 1, @@ -1442,7 +1572,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bvec![token_1, token_2], + bbvec![token_1, token_2], 110, // amount_out 20000, // amount_in_max user, @@ -1455,7 +1585,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![token_1, token_2], + bbvec![token_1, token_2], 15000, // amount_in 110, // amount_out_min user, @@ -1468,7 +1598,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bvec![token_3, token_1], + bbvec![token_3, token_1], 110, // amount_out 20000, // amount_in_max user, @@ -1481,7 +1611,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![token_3, token_1], + bbvec![token_3, token_1], 15000, // amount_in 110, // amount_out_min user, @@ -1503,7 +1633,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![token_3, token_1, token_2], + bbvec![token_3, token_1, token_2], amount_in, // amount_in 1, // amount_out_min user, @@ -1525,7 +1655,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![token_1, token_2, token_3], + bbvec![token_1, token_2, token_3], amount_in, // amount_in 1, // amount_out_min user, @@ -1544,7 +1674,11 @@ fn swap_tokens_for_exact_tokens_should_not_work_if_too_much_slippage() { let token_2 = NativeOrAssetId::Asset(2); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + get_ed())); assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 2, user, 1000)); @@ -1554,8 +1688,8 @@ fn swap_tokens_for_exact_tokens_should_not_work_if_too_much_slippage() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), liquidity1, liquidity2, 1, @@ -1568,7 +1702,7 @@ fn swap_tokens_for_exact_tokens_should_not_work_if_too_much_slippage() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bvec![token_1, token_2], + bbvec![token_1, token_2], exchange_out, // amount_out 50, // amount_in_max just greater than slippage. user, @@ -1588,8 +1722,16 @@ fn swap_exact_tokens_for_tokens_in_multi_hops() { let token_3 = NativeOrAssetId::Asset(3); create_tokens(user, vec![token_2, token_3]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_2, token_3)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_2), + Box::new(token_3) + )); let ed = get_ed(); let base1 = 10000; @@ -1604,8 +1746,8 @@ fn swap_exact_tokens_for_tokens_in_multi_hops() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), liquidity1, liquidity2, 1, @@ -1614,8 +1756,8 @@ fn swap_exact_tokens_for_tokens_in_multi_hops() { )); assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_2, - token_3, + Box::new(token_2), + Box::new(token_3), liquidity2, liquidity3, 1, @@ -1634,7 +1776,7 @@ fn swap_exact_tokens_for_tokens_in_multi_hops() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![token_1], + bbvec![token_1], input_amount, 80, user, @@ -1646,7 +1788,7 @@ fn swap_exact_tokens_for_tokens_in_multi_hops() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![token_1, token_2, token_3, token_2], + bbvec![token_1, token_2, token_3, token_2], input_amount, 80, user, @@ -1657,7 +1799,7 @@ fn swap_exact_tokens_for_tokens_in_multi_hops() { assert_ok!(AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![token_1, token_2, token_3], + bbvec![token_1, token_2, token_3], input_amount, // amount_in 80, // amount_out_min user, @@ -1687,8 +1829,16 @@ fn swap_tokens_for_exact_tokens_in_multi_hops() { let token_3 = NativeOrAssetId::Asset(3); create_tokens(user, vec![token_2, token_3]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_2, token_3)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_2), + Box::new(token_3) + )); let ed = get_ed(); let base1 = 10000; @@ -1703,8 +1853,8 @@ fn swap_tokens_for_exact_tokens_in_multi_hops() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), liquidity1, liquidity2, 1, @@ -1713,8 +1863,8 @@ fn swap_tokens_for_exact_tokens_in_multi_hops() { )); assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_2, - token_3, + Box::new(token_2), + Box::new(token_3), liquidity2, liquidity3, 1, @@ -1732,7 +1882,7 @@ fn swap_tokens_for_exact_tokens_in_multi_hops() { assert_ok!(AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bvec![token_1, token_2, token_3], + bbvec![token_1, token_2, token_3], exchange_out3, // amount_out 1000, // amount_in_max user, @@ -1758,6 +1908,7 @@ fn can_not_swap_same_asset() { new_test_ext().execute_with(|| { let user = 1; let token_1 = NativeOrAssetId::Asset(1); + let token_2 = NativeOrAssetId::Native; create_tokens(user, vec![token_1]); assert_ok!(Assets::mint(RuntimeOrigin::signed(user), 1, user, 1000)); @@ -1767,8 +1918,8 @@ fn can_not_swap_same_asset() { assert_noop!( AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_1, + Box::new(token_1), + Box::new(token_1), liquidity1, liquidity2, 1, @@ -1782,7 +1933,7 @@ fn can_not_swap_same_asset() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![token_1, token_1], + bbvec![token_1, token_1], exchange_amount, 1, user, @@ -1794,7 +1945,7 @@ fn can_not_swap_same_asset() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bvec![NativeOrAssetId::Native, NativeOrAssetId::Native], + bbvec![token_2, token_2], exchange_amount, 1, user, @@ -1854,7 +2005,11 @@ fn cannot_block_pool_creation() { // User can still create the pool create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); // User has to transfer one Asset(2) token to the pool account (otherwise add_liquidity will // fail with `AssetTwoDepositDidNotMeetMinimum`) @@ -1865,8 +2020,8 @@ fn cannot_block_pool_creation() { // add_liquidity shouldn't fail because of the number of consumers assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), 10000, 100, 10000, @@ -1887,8 +2042,16 @@ fn swap_transactional() { let asset_ed = 150; create_tokens_with_ed(user, vec![token_2, token_3], asset_ed); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_3)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_3) + )); let ed = get_ed(); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + ed)); @@ -1904,8 +2067,8 @@ fn swap_transactional() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), liquidity1, liquidity2, 1, @@ -1915,8 +2078,8 @@ fn swap_transactional() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_3, + Box::new(token_1), + Box::new(token_3), liquidity1, liquidity2, 1, @@ -2014,7 +2177,11 @@ fn swap_credit_returns_change() { let token_2 = NativeOrAssetId::Asset(2); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); let ed = get_ed(); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + ed)); @@ -2025,8 +2192,8 @@ fn swap_credit_returns_change() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), liquidity1, liquidity2, 1, @@ -2062,7 +2229,11 @@ fn swap_credit_insufficient_amount_bounds() { let token_2 = NativeOrAssetId::Asset(2); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); let ed = get_ed(); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + ed)); @@ -2076,8 +2247,8 @@ fn swap_credit_insufficient_amount_bounds() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), liquidity1, liquidity2, 1, @@ -2131,7 +2302,11 @@ fn swap_credit_zero_amount() { let token_2 = NativeOrAssetId::Asset(2); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); let ed = get_ed(); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + ed)); @@ -2145,8 +2320,8 @@ fn swap_credit_zero_amount() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), liquidity1, liquidity2, 1, @@ -2209,7 +2384,11 @@ fn swap_credit_invalid_path() { let token_2 = NativeOrAssetId::Asset(2); create_tokens(user, vec![token_2]); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(user), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(user), + Box::new(token_1), + Box::new(token_2) + )); let ed = get_ed(); assert_ok!(Balances::force_set_balance(RuntimeOrigin::root(), user, 20000 + ed)); @@ -2223,8 +2402,8 @@ fn swap_credit_invalid_path() { assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(user), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), liquidity1, liquidity2, 1, From ee9d2ebc0b86368f92568a0895ffd3f9add1a10d Mon Sep 17 00:00:00 2001 From: muharem Date: Sat, 7 Oct 2023 16:32:54 +0200 Subject: [PATCH 24/43] use vec instead bounded vec --- substrate/frame/asset-conversion/src/lib.rs | 44 ++++----- substrate/frame/asset-conversion/src/swap.rs | 10 -- substrate/frame/asset-conversion/src/tests.rs | 98 +++++++++---------- 3 files changed, 63 insertions(+), 89 deletions(-) diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index 356b7f41e924..37d304bf8494 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -86,7 +86,7 @@ use frame_support::{ }, AccountTouch, ContainsPair, Imbalance, Incrementable, }, - BoundedBTreeSet, PalletId, + PalletId, }; use sp_core::Get; use sp_runtime::{ @@ -94,9 +94,9 @@ use sp_runtime::{ CheckedAdd, CheckedDiv, CheckedMul, CheckedSub, Ensure, IntegerSquareRoot, MaybeDisplay, One, TrailingZeroInput, Zero, }, - BoundedVec, DispatchError, RuntimeDebug, Saturating, TokenError, TransactionOutcome, + DispatchError, RuntimeDebug, Saturating, TokenError, TransactionOutcome, }; -use sp_std::vec::Vec; +use sp_std::{collections::btree_set::BTreeSet, vec::Vec}; #[frame_support::pallet] pub mod pallet { @@ -650,7 +650,7 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::swap_exact_tokens_for_tokens())] pub fn swap_exact_tokens_for_tokens( origin: OriginFor, - path: BoundedVec, T::MaxSwapPathLength>, + path: Vec>, amount_in: T::AssetBalance, amount_out_min: T::AssetBalance, send_to: T::AccountId, @@ -659,11 +659,7 @@ pub mod pallet { let sender = ensure_signed(origin)?; Self::do_swap_exact_tokens_for_tokens( sender, - path.into_iter() - .map(|a| *a) - .collect::>() - .try_into() - .map_err(|_| Error::::InvalidPath)?, + path.into_iter().map(|a| *a).collect(), amount_in, Some(amount_out_min), send_to, @@ -682,7 +678,7 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::swap_tokens_for_exact_tokens())] pub fn swap_tokens_for_exact_tokens( origin: OriginFor, - path: BoundedVec, T::MaxSwapPathLength>, + path: Vec>, amount_out: T::AssetBalance, amount_in_max: T::AssetBalance, send_to: T::AccountId, @@ -691,11 +687,7 @@ pub mod pallet { let sender = ensure_signed(origin)?; Self::do_swap_tokens_for_exact_tokens( sender, - path.into_iter() - .map(|a| *a) - .collect::>() - .try_into() - .map_err(|_| Error::::InvalidPath)?, + path.into_iter().map(|a| *a).collect(), amount_out, Some(amount_in_max), send_to, @@ -720,7 +712,7 @@ pub mod pallet { /// rollback. pub(crate) fn do_swap_exact_tokens_for_tokens( sender: T::AccountId, - path: BoundedVec, + path: Vec, amount_in: T::AssetBalance, amount_out_min: Option, send_to: T::AccountId, @@ -768,7 +760,7 @@ pub mod pallet { /// rollback. pub(crate) fn do_swap_tokens_for_exact_tokens( sender: T::AccountId, - path: BoundedVec, + path: Vec, amount_out: T::AssetBalance, amount_in_max: Option, send_to: T::AccountId, @@ -814,7 +806,7 @@ pub mod pallet { /// only inside a transactional storage context and an Err result must imply a storage /// rollback. pub(crate) fn do_swap_exact_credit_tokens_for_tokens( - path: BoundedVec, + path: Vec, credit_in: Credit, amount_out_min: Option, ) -> Result, (Credit, DispatchError)> { @@ -862,7 +854,7 @@ pub mod pallet { /// only inside a transactional storage context and an Err result must imply a storage /// rollback. pub(crate) fn do_swap_credit_tokens_for_exact_tokens( - path: BoundedVec, + path: Vec, credit_in: Credit, amount_out: T::AssetBalance, ) -> Result<(Credit, Credit), (Credit, DispatchError)> { @@ -1152,7 +1144,7 @@ pub mod pallet { /// Leading to an amount at the end of a `path`, get the required amounts in. pub(crate) fn balance_path_from_amount_out( amount_out: T::AssetBalance, - path: BoundedVec, + path: Vec, ) -> Result, DispatchError> { let mut balance_path: BalancePath = Vec::with_capacity(path.len()); let mut amount_in: T::AssetBalance = amount_out; @@ -1178,7 +1170,7 @@ pub mod pallet { /// Following an amount into a `path`, get the corresponding amounts out. pub(crate) fn balance_path_from_amount_in( amount_in: T::AssetBalance, - path: BoundedVec, + path: Vec, ) -> Result, DispatchError> { let mut balance_path: BalancePath = Vec::with_capacity(path.len()); let mut amount_out: T::AssetBalance = amount_in; @@ -1391,18 +1383,16 @@ pub mod pallet { } /// Ensure that a path is valid. - fn validate_swap_path( - path: &BoundedVec, - ) -> Result<(), DispatchError> { + fn validate_swap_path(path: &Vec) -> Result<(), DispatchError> { ensure!(path.len() >= 2, Error::::InvalidPath); + ensure!(path.len() as u32 <= T::MaxSwapPathLength::get(), Error::::InvalidPath); // validate all the pools in the path are unique - let mut pools = BoundedBTreeSet::, T::MaxSwapPathLength>::new(); + let mut pools = BTreeSet::>::new(); for assets_pair in path.windows(2) { if let [asset1, asset2] = assets_pair { let pool_id = Self::get_pool_id(asset1.clone(), asset2.clone()); - let new_element = - pools.try_insert(pool_id).map_err(|_| Error::::Overflow)?; + let new_element = pools.insert(pool_id); if !new_element { return Err(Error::::NonUniquePath.into()) } diff --git a/substrate/frame/asset-conversion/src/swap.rs b/substrate/frame/asset-conversion/src/swap.rs index 92cd4c8877a5..a86b55d41a53 100644 --- a/substrate/frame/asset-conversion/src/swap.rs +++ b/substrate/frame/asset-conversion/src/swap.rs @@ -120,7 +120,6 @@ impl Swap for Pallet { send_to: T::AccountId, keep_alive: bool, ) -> Result { - let path = path.try_into().map_err(|_| Error::::PathError)?; let amount_out_min = amount_out_min.map(Self::convert_hpb_to_asset_balance).transpose()?; let amount_out = with_storage_layer(|| { Self::do_swap_exact_tokens_for_tokens( @@ -143,7 +142,6 @@ impl Swap for Pallet { send_to: T::AccountId, keep_alive: bool, ) -> Result { - let path = path.try_into().map_err(|_| Error::::PathError)?; let amount_in_max = amount_in_max.map(Self::convert_hpb_to_asset_balance).transpose()?; let amount_in = with_storage_layer(|| { Self::do_swap_tokens_for_exact_tokens( @@ -173,10 +171,6 @@ impl SwapCredit for Pallet { credit_in: Self::Credit, amount_out_min: Option, ) -> Result { - let path = match path.try_into() { - Ok(p) => p, - Err(_) => return Err((credit_in, Error::::PathError.into())), - }; let transaction_res = with_transaction(|| -> TransactionOutcome> { let res = @@ -200,10 +194,6 @@ impl SwapCredit for Pallet { credit_in: Self::Credit, amount_out: Self::Balance, ) -> Result<(Self::Credit, Self::Credit), (Self::Credit, DispatchError)> { - let path = match path.try_into() { - Ok(p) => p, - Err(_) => return Err((credit_in, Error::::PathError.into())), - }; let transaction_res = with_transaction(|| -> TransactionOutcome> { let res = Self::do_swap_credit_tokens_for_exact_tokens(path, credit_in, amount_out); diff --git a/substrate/frame/asset-conversion/src/tests.rs b/substrate/frame/asset-conversion/src/tests.rs index d6a041f98e0c..db6aca01dece 100644 --- a/substrate/frame/asset-conversion/src/tests.rs +++ b/substrate/frame/asset-conversion/src/tests.rs @@ -95,14 +95,8 @@ fn get_ed() -> u128 { } macro_rules! bvec { - ($( $x:tt )*) => { - vec![$( $x )*].try_into().unwrap() - } -} - -macro_rules! bbvec { ($( $x:ident ),*) => { - vec![$( Box::new( $x ), )*].try_into().unwrap() + vec![$( Box::new( $x ), )*] } } @@ -885,7 +879,7 @@ fn quote_price_exact_tokens_for_tokens_matches_execution() { assert_eq!(prior_dot_balance, balance(user2, token_1)); assert_ok!(AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user2), - bbvec![token_2, token_1], + bvec![token_2, token_1], amount, 1, user2, @@ -939,7 +933,7 @@ fn quote_price_tokens_for_exact_tokens_matches_execution() { assert_eq!(prior_asset_balance, balance(user2, token_2)); assert_ok!(AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user2), - bbvec![token_2, token_1], + bvec![token_2, token_1], amount, 1, user2, @@ -992,7 +986,7 @@ fn can_swap_with_native() { assert_ok!(AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![token_2, token_1], + bvec![token_2, token_1], input_amount, 1, user, @@ -1042,7 +1036,7 @@ fn can_swap_with_realistic_values() { assert_ok!(AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![usd, dot], + bvec![usd, dot], input_amount, 1, user, @@ -1054,7 +1048,7 @@ fn can_swap_with_realistic_values() { send_to: user, amount_in: 10 * UNIT, // usd amount_out: 1_993_980_120, // About 2 dot after div by UNIT. - path: bvec![(usd, 10 * UNIT), (dot, 1_993_980_120)], + path: vec![(usd, 10 * UNIT), (dot, 1_993_980_120)], })); }); } @@ -1077,7 +1071,7 @@ fn can_not_swap_in_pool_with_no_liquidity_added_yet() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![token_2, token_1], + bvec![token_2, token_1], 10, 1, user, @@ -1156,7 +1150,7 @@ fn check_no_panic_when_try_swap_close_to_empty_pool() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bbvec![token_2, token_1], + bvec![token_2, token_1], 708 - ed + 1, // amount_out 500, // amount_in_max user, @@ -1167,7 +1161,7 @@ fn check_no_panic_when_try_swap_close_to_empty_pool() { assert_ok!(AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bbvec![token_2, token_1], + bvec![token_2, token_1], 608, // amount_out 500, // amount_in_max user, @@ -1189,7 +1183,7 @@ fn check_no_panic_when_try_swap_close_to_empty_pool() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bbvec![token_2, token_1], + bvec![token_2, token_1], token_1_left - 1, // amount_out 1000, // amount_in_max user, @@ -1202,7 +1196,7 @@ fn check_no_panic_when_try_swap_close_to_empty_pool() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bbvec![token_2, token_1], + bvec![token_2, token_1], token_1_left, // amount_out 1000, // amount_in_max user, @@ -1249,7 +1243,7 @@ fn swap_should_not_work_if_too_much_slippage() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![token_2, token_1], + bvec![token_2, token_1], exchange_amount, // amount_in 4000, // amount_out_min user, @@ -1304,7 +1298,7 @@ fn can_swap_tokens_for_exact_tokens() { assert_ok!(AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bbvec![token_1, token_2], + bvec![token_1, token_2], exchange_out, // amount_out 3500, // amount_in_max user, @@ -1378,7 +1372,7 @@ fn can_swap_tokens_for_exact_tokens_when_not_liquidity_provider() { assert_ok!(AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bbvec![token_1, token_2], + bvec![token_1, token_2], exchange_out, // amount_out 3500, // amount_in_max user, @@ -1452,7 +1446,7 @@ fn swap_when_existential_deposit_would_cause_reaping_but_keep_alive_set() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bbvec![token_1, token_2], + bvec![token_1, token_2], 1, // amount_out 101, // amount_in_max user, @@ -1464,7 +1458,7 @@ fn swap_when_existential_deposit_would_cause_reaping_but_keep_alive_set() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![token_1, token_2], + bvec![token_1, token_2], 51, // amount_in 1, // amount_out_min user, @@ -1476,7 +1470,7 @@ fn swap_when_existential_deposit_would_cause_reaping_but_keep_alive_set() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bbvec![token_2, token_1], + bvec![token_2, token_1], 51, // amount_out 2, // amount_in_max user, @@ -1488,7 +1482,7 @@ fn swap_when_existential_deposit_would_cause_reaping_but_keep_alive_set() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![token_2, token_1], + bvec![token_2, token_1], 2, // amount_in 1, // amount_out_min user, @@ -1572,7 +1566,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bbvec![token_1, token_2], + bvec![token_1, token_2], 110, // amount_out 20000, // amount_in_max user, @@ -1585,7 +1579,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![token_1, token_2], + bvec![token_1, token_2], 15000, // amount_in 110, // amount_out_min user, @@ -1598,7 +1592,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bbvec![token_3, token_1], + bvec![token_3, token_1], 110, // amount_out 20000, // amount_in_max user, @@ -1611,7 +1605,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![token_3, token_1], + bvec![token_3, token_1], 15000, // amount_in 110, // amount_out_min user, @@ -1633,7 +1627,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![token_3, token_1, token_2], + bvec![token_3, token_1, token_2], amount_in, // amount_in 1, // amount_out_min user, @@ -1655,7 +1649,7 @@ fn swap_when_existential_deposit_would_cause_reaping_pool_account() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![token_1, token_2, token_3], + bvec![token_1, token_2, token_3], amount_in, // amount_in 1, // amount_out_min user, @@ -1702,7 +1696,7 @@ fn swap_tokens_for_exact_tokens_should_not_work_if_too_much_slippage() { assert_noop!( AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bbvec![token_1, token_2], + bvec![token_1, token_2], exchange_out, // amount_out 50, // amount_in_max just greater than slippage. user, @@ -1776,7 +1770,7 @@ fn swap_exact_tokens_for_tokens_in_multi_hops() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![token_1], + bvec![token_1], input_amount, 80, user, @@ -1788,7 +1782,7 @@ fn swap_exact_tokens_for_tokens_in_multi_hops() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![token_1, token_2, token_3, token_2], + bvec![token_1, token_2, token_3, token_2], input_amount, 80, user, @@ -1799,7 +1793,7 @@ fn swap_exact_tokens_for_tokens_in_multi_hops() { assert_ok!(AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![token_1, token_2, token_3], + bvec![token_1, token_2, token_3], input_amount, // amount_in 80, // amount_out_min user, @@ -1882,7 +1876,7 @@ fn swap_tokens_for_exact_tokens_in_multi_hops() { assert_ok!(AssetConversion::swap_tokens_for_exact_tokens( RuntimeOrigin::signed(user), - bbvec![token_1, token_2, token_3], + bvec![token_1, token_2, token_3], exchange_out3, // amount_out 1000, // amount_in_max user, @@ -1933,7 +1927,7 @@ fn can_not_swap_same_asset() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![token_1, token_1], + bvec![token_1, token_1], exchange_amount, 1, user, @@ -1945,7 +1939,7 @@ fn can_not_swap_same_asset() { assert_noop!( AssetConversion::swap_exact_tokens_for_tokens( RuntimeOrigin::signed(user), - bbvec![token_2, token_2], + bvec![token_2, token_2], exchange_amount, 1, user, @@ -2113,7 +2107,7 @@ fn swap_transactional() { let error; assert_storage_noop!( error = >::swap_tokens_for_exact_tokens( - bvec![token_2, token_1, token_3], + vec![token_2, token_1, token_3], credit_in.into(), expected_out, ) @@ -2128,7 +2122,7 @@ fn swap_transactional() { let error; assert_storage_noop!( error = >::swap_exact_tokens_for_tokens( - bvec![token_2, token_1, token_3], + vec![token_2, token_1, token_3], credit_in.into(), Some(expected_out), ) @@ -2140,7 +2134,7 @@ fn swap_transactional() { assert_noop!( >::swap_exact_tokens_for_tokens( user2, - bvec![token_2, token_1, token_3], + vec![token_2, token_1, token_3], amount_in.into(), Some(expected_out.into()), user2, @@ -2153,7 +2147,7 @@ fn swap_transactional() { assert_noop!( >::swap_tokens_for_exact_tokens( user2, - bvec![token_2, token_1, token_3], + vec![token_2, token_1, token_3], expected_out.into(), Some(amount_in.into()), user2, @@ -2211,7 +2205,7 @@ fn swap_credit_returns_change() { let credit_in = Balances::issue(amount_in_max + expected_change.peek()); assert_ok!( >::swap_tokens_for_exact_tokens( - bvec![token_1, token_2], + vec![token_1, token_2], credit_in.into(), expected_credit_out.peek(), ), @@ -2264,7 +2258,7 @@ fn swap_credit_insufficient_amount_bounds() { let credit_in = Balances::issue(amount_in); let expected_credit_in = Balances::issue(amount_in); let error = >::swap_exact_tokens_for_tokens( - bvec![token_1, token_2], + vec![token_1, token_2], credit_in.into(), Some(amount_out_min), ) @@ -2281,7 +2275,7 @@ fn swap_credit_insufficient_amount_bounds() { let credit_in = Balances::issue(amount_in_max); let expected_credit_in = Balances::issue(amount_in_max); let error = >::swap_tokens_for_exact_tokens( - bvec![token_1, token_2], + vec![token_1, token_2], credit_in.into(), amount_out, ) @@ -2333,7 +2327,7 @@ fn swap_credit_zero_amount() { let credit_in = Credit::native_zero(); let expected_credit_in = Credit::native_zero(); let error = >::swap_exact_tokens_for_tokens( - bvec![token_1, token_2], + vec![token_1, token_2], credit_in, None, ) @@ -2344,7 +2338,7 @@ fn swap_credit_zero_amount() { let credit_in = Credit::native_zero(); let expected_credit_in = Credit::native_zero(); let error = >::swap_tokens_for_exact_tokens( - bvec![token_1, token_2], + vec![token_1, token_2], credit_in, 10, ) @@ -2355,7 +2349,7 @@ fn swap_credit_zero_amount() { let credit_in = Balances::issue(10); let expected_credit_in = Balances::issue(10); let error = >::swap_exact_tokens_for_tokens( - bvec![token_1, token_2], + vec![token_1, token_2], credit_in.into(), Some(0), ) @@ -2366,7 +2360,7 @@ fn swap_credit_zero_amount() { let credit_in = Balances::issue(10); let expected_credit_in = Balances::issue(10); let error = >::swap_tokens_for_exact_tokens( - bvec![token_1, token_2], + vec![token_1, token_2], credit_in.into(), 0, ) @@ -2415,7 +2409,7 @@ fn swap_credit_invalid_path() { let credit_in = Balances::issue(10); let expected_credit_in = Balances::issue(10); let error = >::swap_exact_tokens_for_tokens( - bvec![token_2, token_1], + vec![token_2, token_1], credit_in.into(), None, ) @@ -2426,7 +2420,7 @@ fn swap_credit_invalid_path() { let credit_in = Assets::issue(2, 10); let expected_credit_in = Assets::issue(2, 10); let error = >::swap_tokens_for_exact_tokens( - bvec![token_1, token_2], + vec![token_1, token_2], credit_in.into(), 10, ) @@ -2437,7 +2431,7 @@ fn swap_credit_invalid_path() { let credit_in = Balances::issue(10); let expected_credit_in = Balances::issue(10); let error = >::swap_exact_tokens_for_tokens( - bvec![token_2], + vec![token_2], credit_in.into(), None, ) @@ -2448,7 +2442,7 @@ fn swap_credit_invalid_path() { let credit_in = Assets::issue(2, 10); let expected_credit_in = Assets::issue(2, 10); let error = >::swap_tokens_for_exact_tokens( - bvec![], + vec![], credit_in.into(), 10, ) From ced5e99159c6ce149b7865e68f5fcfb0ebc8a112 Mon Sep 17 00:00:00 2001 From: muharem Date: Sat, 7 Oct 2023 17:14:16 +0200 Subject: [PATCH 25/43] update to new api --- .../asset-hub-westend/src/tests/swap.rs | 56 +++++++++---------- .../assets/asset-hub-kusama/src/lib.rs | 10 ++-- .../assets/asset-hub-westend/src/lib.rs | 23 ++++---- .../common/src/local_and_foreign_assets.rs | 43 +++++++------- substrate/frame/asset-conversion/src/lib.rs | 2 +- .../asset-conversion-tx-payment/src/tests.rs | 10 +++- 6 files changed, 72 insertions(+), 72 deletions(-) diff --git a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/swap.rs b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/swap.rs index 7d1615c9e291..d56a4c7e4de6 100644 --- a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/swap.rs +++ b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/swap.rs @@ -17,11 +17,11 @@ use crate::*; #[test] fn swap_locally_on_chain_using_local_assets() { - let asset_native = Box::new(asset_hub_westend_runtime::xcm_config::WestendLocation::get()); - let asset_one = Box::new(MultiLocation { + let asset_native = asset_hub_westend_runtime::xcm_config::WestendLocation::get(); + let asset_one = MultiLocation { parents: 0, interior: X2(PalletInstance(ASSETS_PALLET_ID), GeneralIndex(ASSET_ID.into())), - }); + }; AssetHubWestend::execute_with(|| { type RuntimeEvent = ::RuntimeEvent; @@ -43,8 +43,8 @@ fn swap_locally_on_chain_using_local_assets() { assert_ok!(::AssetConversion::create_pool( ::RuntimeOrigin::signed(AssetHubWestendSender::get()), - asset_native.clone(), - asset_one.clone(), + Box::new(asset_native.clone()), + Box::new(asset_one.clone()), )); assert_expected_events!( @@ -56,8 +56,8 @@ fn swap_locally_on_chain_using_local_assets() { assert_ok!(::AssetConversion::add_liquidity( ::RuntimeOrigin::signed(AssetHubWestendSender::get()), - asset_native.clone(), - asset_one.clone(), + Box::new(asset_native.clone()), + Box::new(asset_one.clone()), 1_000_000_000_000, 2_000_000_000_000, 0, @@ -72,7 +72,7 @@ fn swap_locally_on_chain_using_local_assets() { ] ); - let path = BoundedVec::<_, _>::truncate_from(vec![asset_native.clone(), asset_one.clone()]); + let path = vec![Box::new(asset_native.clone()), Box::new(asset_one.clone())]; assert_ok!(::AssetConversion::swap_exact_tokens_for_tokens( ::RuntimeOrigin::signed(AssetHubWestendSender::get()), @@ -95,8 +95,8 @@ fn swap_locally_on_chain_using_local_assets() { assert_ok!(::AssetConversion::remove_liquidity( ::RuntimeOrigin::signed(AssetHubWestendSender::get()), - asset_native, - asset_one, + Box::new(asset_native), + Box::new(asset_one), 1414213562273 - 2_000_000_000, // all but the 2 EDs can't be retrieved. 0, 0, @@ -109,16 +109,16 @@ fn swap_locally_on_chain_using_local_assets() { fn swap_locally_on_chain_using_foreign_assets() { use frame_support::weights::WeightToFee; - let asset_native = Box::new(asset_hub_westend_runtime::xcm_config::WestendLocation::get()); + let asset_native = asset_hub_westend_runtime::xcm_config::WestendLocation::get(); - let foreign_asset1_at_asset_hub_westend = Box::new(MultiLocation { + let foreign_asset1_at_asset_hub_westend = MultiLocation { parents: 1, interior: X3( Parachain(PenpalWestendA::para_id().into()), PalletInstance(ASSETS_PALLET_ID), GeneralIndex(ASSET_ID.into()), ), - }); + }; let assets_para_destination: VersionedMultiLocation = MultiLocation { parents: 1, interior: X1(Parachain(AssetHubWestend::para_id().into())) } @@ -163,7 +163,7 @@ fn swap_locally_on_chain_using_foreign_assets() { ::Runtime, Instance2, >::create { - id: *foreign_asset1_at_asset_hub_westend, + id: foreign_asset1_at_asset_hub_westend, min_balance: 1000, admin: sov_penpal_on_asset_hub_westend.clone().into(), }) @@ -211,7 +211,7 @@ fn swap_locally_on_chain_using_foreign_assets() { // Receive XCM message in Assets Parachain AssetHubWestend::execute_with(|| { assert!(::ForeignAssets::asset_exists( - *foreign_asset1_at_asset_hub_westend + foreign_asset1_at_asset_hub_westend )); // 3: Mint foreign asset on asset_hub_westend: @@ -225,7 +225,7 @@ fn swap_locally_on_chain_using_foreign_assets() { ::RuntimeOrigin::signed( sov_penpal_on_asset_hub_westend.clone().into() ), - *foreign_asset1_at_asset_hub_westend, + foreign_asset1_at_asset_hub_westend, sov_penpal_on_asset_hub_westend.clone().into(), 3_000_000_000_000, )); @@ -240,8 +240,8 @@ fn swap_locally_on_chain_using_foreign_assets() { // 4. Create pool: assert_ok!(::AssetConversion::create_pool( ::RuntimeOrigin::signed(AssetHubWestendSender::get()), - asset_native.clone(), - foreign_asset1_at_asset_hub_westend.clone(), + Box::new(asset_native.clone()), + Box::new(foreign_asset1_at_asset_hub_westend.clone()), )); assert_expected_events!( @@ -256,8 +256,8 @@ fn swap_locally_on_chain_using_foreign_assets() { ::RuntimeOrigin::signed( sov_penpal_on_asset_hub_westend.clone() ), - asset_native.clone(), - foreign_asset1_at_asset_hub_westend.clone(), + Box::new(asset_native.clone()), + Box::new(foreign_asset1_at_asset_hub_westend.clone()), 1_000_000_000_000, 2_000_000_000_000, 0, @@ -275,10 +275,10 @@ fn swap_locally_on_chain_using_foreign_assets() { ); // 6. Swap! - let path = BoundedVec::<_, _>::truncate_from(vec![ - asset_native.clone(), - foreign_asset1_at_asset_hub_westend.clone(), - ]); + let path = vec![ + Box::new(asset_native.clone()), + Box::new(foreign_asset1_at_asset_hub_westend.clone()), + ]; assert_ok!(::AssetConversion::swap_exact_tokens_for_tokens( ::RuntimeOrigin::signed(AssetHubWestendSender::get()), @@ -304,8 +304,8 @@ fn swap_locally_on_chain_using_foreign_assets() { ::RuntimeOrigin::signed( sov_penpal_on_asset_hub_westend.clone() ), - asset_native, - foreign_asset1_at_asset_hub_westend, + Box::new(asset_native), + Box::new(foreign_asset1_at_asset_hub_westend), 1414213562273 - 2_000_000_000, // all but the 2 EDs can't be retrieved. 0, 0, @@ -316,7 +316,7 @@ fn swap_locally_on_chain_using_foreign_assets() { #[test] fn cannot_create_pool_from_pool_assets() { - let asset_native = Box::new(asset_hub_westend_runtime::xcm_config::WestendLocation::get()); + let asset_native = asset_hub_westend_runtime::xcm_config::WestendLocation::get(); let mut asset_one = asset_hub_westend_runtime::xcm_config::PoolAssetsPalletLocation::get(); asset_one.append_with(GeneralIndex(ASSET_ID.into())).expect("pool assets"); @@ -341,7 +341,7 @@ fn cannot_create_pool_from_pool_assets() { assert_matches::assert_matches!( ::AssetConversion::create_pool( ::RuntimeOrigin::signed(AssetHubWestendSender::get()), - asset_native.clone(), + Box::new(asset_native.clone()), Box::new(asset_one), ), Err(DispatchError::Module(ModuleError{index: _, error: _, message})) => assert_eq!(message, Some("UnsupportedAsset")) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/lib.rs index 40ce122112d2..efadf4869bf1 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/lib.rs @@ -347,7 +347,7 @@ impl pallet_asset_conversion::Config for Runtime { type PalletId = AssetConversionPalletId; type AllowMultiAssetPools = AllowMultiAssetPools; type MaxSwapPathLength = ConstU32<4>; - type MultiAssetId = Box; + type MultiAssetId = MultiLocation; type MultiAssetIdConverter = MultiLocationConverter; type MintMinLiquidity = ConstU128<100>; @@ -1031,16 +1031,16 @@ impl_runtime_apis! { Block, Balance, u128, - Box, + MultiLocation, > for Runtime { - fn quote_price_exact_tokens_for_tokens(asset1: Box, asset2: Box, amount: u128, include_fee: bool) -> Option { + fn quote_price_exact_tokens_for_tokens(asset1: MultiLocation, asset2: MultiLocation, amount: u128, include_fee: bool) -> Option { AssetConversion::quote_price_exact_tokens_for_tokens(asset1, asset2, amount, include_fee) } - fn quote_price_tokens_for_exact_tokens(asset1: Box, asset2: Box, amount: u128, include_fee: bool) -> Option { + fn quote_price_tokens_for_exact_tokens(asset1: MultiLocation, asset2: MultiLocation, amount: u128, include_fee: bool) -> Option { AssetConversion::quote_price_tokens_for_exact_tokens(asset1, asset2, amount, include_fee) } - fn get_reserves(asset1: Box, asset2: Box) -> Option<(Balance, Balance)> { + fn get_reserves(asset1: MultiLocation, asset2: MultiLocation) -> Option<(Balance, Balance)> { AssetConversion::get_reserves(&asset1, &asset2).ok() } } diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index 943332087627..9a27a71bb16a 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -323,7 +323,7 @@ impl pallet_asset_conversion::Config for Runtime { type PalletId = AssetConversionPalletId; type AllowMultiAssetPools = AllowMultiAssetPools; type MaxSwapPathLength = ConstU32<4>; - type MultiAssetId = Box; + type MultiAssetId = MultiLocation; type MultiAssetIdConverter = MultiLocationConverter; type MintMinLiquidity = ConstU128<100>; @@ -1080,18 +1080,18 @@ impl_runtime_apis! { Block, Balance, u128, - Box, + MultiLocation, > for Runtime { - fn quote_price_exact_tokens_for_tokens(asset1: Box, asset2: Box, amount: u128, include_fee: bool) -> Option { + fn quote_price_exact_tokens_for_tokens(asset1: MultiLocation, asset2: MultiLocation, amount: u128, include_fee: bool) -> Option { AssetConversion::quote_price_exact_tokens_for_tokens(asset1, asset2, amount, include_fee) } - fn quote_price_tokens_for_exact_tokens(asset1: Box, asset2: Box, amount: u128, include_fee: bool) -> Option { + fn quote_price_tokens_for_exact_tokens(asset1: MultiLocation, asset2: MultiLocation, amount: u128, include_fee: bool) -> Option { AssetConversion::quote_price_tokens_for_exact_tokens(asset1, asset2, amount, include_fee) } - fn get_reserves(asset1: Box, asset2: Box) -> Option<(Balance, Balance)> { + fn get_reserves(asset1: MultiLocation, asset2: MultiLocation) -> Option<(Balance, Balance)> { AssetConversion::get_reserves(&asset1, &asset2).ok() } } @@ -1428,10 +1428,7 @@ pub mod migrations { /// `MultiLocation { parents: 1, interior: Here }` pub struct NativeAssetParents0ToParents1Migration(sp_std::marker::PhantomData); impl< - T: pallet_asset_conversion::Config< - MultiAssetId = Box, - AssetId = MultiLocation, - >, + T: pallet_asset_conversion::Config, > OnRuntimeUpgrade for NativeAssetParents0ToParents1Migration where ::PoolAssetId: Into, @@ -1457,14 +1454,14 @@ pub mod migrations { pallet_asset_conversion::Pallet::::get_pool_account(&old_pool_id); reads.saturating_accrue(1); let pool_asset_id = pool_info.lp_token.clone(); - if old_pool_id.0.as_ref() != &invalid_native_asset { + if old_pool_id.0 != invalid_native_asset { // skip, if ok continue } // fix new account let new_pool_id = pallet_asset_conversion::Pallet::::get_pool_id( - Box::new(valid_native_asset), + valid_native_asset, old_pool_id.1.clone(), ); let new_pool_account = @@ -1504,10 +1501,10 @@ pub mod migrations { // move LocalOrForeignAssets let _ = T::Assets::transfer( - *old_pool_id.1.as_ref(), + old_pool_id.1, &old_pool_account, &new_pool_account, - T::Assets::balance(*old_pool_id.1.as_ref(), &old_pool_account), + T::Assets::balance(old_pool_id.1, &old_pool_account), Preservation::Expendable, ); reads.saturating_accrue(1); diff --git a/cumulus/parachains/runtimes/assets/common/src/local_and_foreign_assets.rs b/cumulus/parachains/runtimes/assets/common/src/local_and_foreign_assets.rs index 5c66d1cabe57..b0802d35de2d 100644 --- a/cumulus/parachains/runtimes/assets/common/src/local_and_foreign_assets.rs +++ b/cumulus/parachains/runtimes/assets/common/src/local_and_foreign_assets.rs @@ -23,37 +23,36 @@ use frame_support::traits::{ use pallet_asset_conversion::{MultiAssetIdConversionResult, MultiAssetIdConverter}; use parachains_common::AccountId; use sp_runtime::{traits::MaybeEquivalence, DispatchError, DispatchResult}; -use sp_std::{boxed::Box, marker::PhantomData}; +use sp_std::marker::PhantomData; use xcm::latest::MultiLocation; pub struct MultiLocationConverter, MultiLocationMatcher> { _phantom: PhantomData<(NativeAssetLocation, MultiLocationMatcher)>, } -impl - MultiAssetIdConverter, MultiLocation> +impl MultiAssetIdConverter for MultiLocationConverter where NativeAssetLocation: Get, MultiLocationMatcher: Contains, { - fn get_native() -> Box { - Box::new(NativeAssetLocation::get()) + fn get_native() -> MultiLocation { + NativeAssetLocation::get() } - fn is_native(asset_id: &Box) -> bool { + fn is_native(asset_id: &MultiLocation) -> bool { *asset_id == Self::get_native() } fn try_convert( - asset_id: &Box, - ) -> MultiAssetIdConversionResult, MultiLocation> { - if Self::is_native(&asset_id) { + asset_id: &MultiLocation, + ) -> MultiAssetIdConversionResult { + if Self::is_native(asset_id) { return MultiAssetIdConversionResult::Native } - if MultiLocationMatcher::contains(&asset_id) { - MultiAssetIdConversionResult::Converted(*asset_id.clone()) + if MultiLocationMatcher::contains(asset_id) { + MultiAssetIdConversionResult::Converted(asset_id.clone()) } else { MultiAssetIdConversionResult::Unsupported(asset_id.clone()) } @@ -442,27 +441,27 @@ mod tests { interior: X2(GlobalConsensus(ByGenesis([1; 32])), Parachain(2222)), }; - assert!(C::is_native(&Box::new(native_asset))); - assert!(!C::is_native(&Box::new(local_asset))); - assert!(!C::is_native(&Box::new(pool_asset))); - assert!(!C::is_native(&Box::new(foreign_asset1))); - assert!(!C::is_native(&Box::new(foreign_asset2))); + assert!(C::is_native(&native_asset)); + assert!(!C::is_native(&local_asset)); + assert!(!C::is_native(&pool_asset)); + assert!(!C::is_native(&foreign_asset1)); + assert!(!C::is_native(&foreign_asset2)); - assert_eq!(C::try_convert(&Box::new(native_asset)), MultiAssetIdConversionResult::Native); + assert_eq!(C::try_convert(&native_asset), MultiAssetIdConversionResult::Native); assert_eq!( - C::try_convert(&Box::new(local_asset)), + C::try_convert(&local_asset), MultiAssetIdConversionResult::Converted(local_asset) ); assert_eq!( - C::try_convert(&Box::new(pool_asset)), - MultiAssetIdConversionResult::Unsupported(Box::new(pool_asset)) + C::try_convert(&pool_asset), + MultiAssetIdConversionResult::Unsupported(pool_asset) ); assert_eq!( - C::try_convert(&Box::new(foreign_asset1)), + C::try_convert(&foreign_asset1), MultiAssetIdConversionResult::Converted(foreign_asset1) ); assert_eq!( - C::try_convert(&Box::new(foreign_asset2)), + C::try_convert(&foreign_asset2), MultiAssetIdConversionResult::Converted(foreign_asset2) ); } diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index 37d304bf8494..45dcdc0063fe 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -96,7 +96,7 @@ use sp_runtime::{ }, DispatchError, RuntimeDebug, Saturating, TokenError, TransactionOutcome, }; -use sp_std::{collections::btree_set::BTreeSet, vec::Vec}; +use sp_std::{boxed::Box, collections::btree_set::BTreeSet, vec::Vec}; #[frame_support::pallet] pub mod pallet { diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs index cad301dc9599..d50a05164235 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/tests.rs @@ -129,12 +129,16 @@ fn setup_lp(asset_id: u32, balance_factor: u64) { let token_1 = NativeOrAssetId::Native; let token_2 = NativeOrAssetId::Asset(asset_id); - assert_ok!(AssetConversion::create_pool(RuntimeOrigin::signed(lp_provider), token_1, token_2)); + assert_ok!(AssetConversion::create_pool( + RuntimeOrigin::signed(lp_provider), + Box::new(token_1), + Box::new(token_2) + )); assert_ok!(AssetConversion::add_liquidity( RuntimeOrigin::signed(lp_provider), - token_1, - token_2, + Box::new(token_1), + Box::new(token_2), 1_000 * balance_factor, // 1 desired 10_000 * balance_factor, // 2 desired 1, // 1 min From 7fee81e62599885202b6a7a95fd1a0a26848011f Mon Sep 17 00:00:00 2001 From: muharem Date: Sat, 7 Oct 2023 19:42:55 +0200 Subject: [PATCH 26/43] drop clone --- .../runtimes/assets/common/src/local_and_foreign_assets.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/common/src/local_and_foreign_assets.rs b/cumulus/parachains/runtimes/assets/common/src/local_and_foreign_assets.rs index b0802d35de2d..a8bafa5e8b9e 100644 --- a/cumulus/parachains/runtimes/assets/common/src/local_and_foreign_assets.rs +++ b/cumulus/parachains/runtimes/assets/common/src/local_and_foreign_assets.rs @@ -52,9 +52,9 @@ where } if MultiLocationMatcher::contains(asset_id) { - MultiAssetIdConversionResult::Converted(asset_id.clone()) + MultiAssetIdConversionResult::Converted(*asset_id) } else { - MultiAssetIdConversionResult::Unsupported(asset_id.clone()) + MultiAssetIdConversionResult::Unsupported(*asset_id) } } } From b63cd7349a60643feacce9f96e8c6013567e4788 Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 8 Oct 2023 15:09:07 +0200 Subject: [PATCH 27/43] box arguments and drop bounded vec from benches --- .../asset-conversion/src/benchmarking.rs | 112 ++++++++++++------ 1 file changed, 75 insertions(+), 37 deletions(-) diff --git a/substrate/frame/asset-conversion/src/benchmarking.rs b/substrate/frame/asset-conversion/src/benchmarking.rs index 87b541cd4744..72033c58c8b2 100644 --- a/substrate/frame/asset-conversion/src/benchmarking.rs +++ b/substrate/frame/asset-conversion/src/benchmarking.rs @@ -21,7 +21,6 @@ use super::*; use frame_benchmarking::{benchmarks, whitelisted_caller}; use frame_support::{ assert_ok, - storage::bounded_vec::BoundedVec, traits::{ fungible::{Inspect as InspectFungible, Mutate as MutateFungible, Unbalanced}, fungibles::{Create, Inspect, Mutate}, @@ -80,8 +79,8 @@ where assert_ok!(AssetConversion::::create_pool( SystemOrigin::Signed(caller.clone()).into(), - asset1.clone(), - asset2.clone() + Box::new(asset1.clone()), + Box::new(asset2.clone()) )); let lp_token = get_lp_token_id::(); @@ -110,7 +109,7 @@ benchmarks! { let asset1 = T::MultiAssetIdConverter::get_native(); let asset2 = T::BenchmarkHelper::multiasset_id(0); let (caller, _) = create_asset::(&asset2); - }: _(SystemOrigin::Signed(caller.clone()), asset1.clone(), asset2.clone()) + }: _(SystemOrigin::Signed(caller.clone()), Box::new(asset1.clone()), Box::new(asset2.clone())) verify { let lp_token = get_lp_token_id::(); let pool_id = (asset1.clone(), asset2.clone()); @@ -128,7 +127,7 @@ benchmarks! { let (lp_token, caller, _) = create_asset_and_pool::(&asset1, &asset2); let ed: u128 = T::Currency::minimum_balance().into(); let add_amount = 1000 + ed; - }: _(SystemOrigin::Signed(caller.clone()), asset1.clone(), asset2.clone(), add_amount.into(), 1000.into(), 0.into(), 0.into(), caller.clone()) + }: _(SystemOrigin::Signed(caller.clone()), Box::new(asset1.clone()), Box::new(asset2.clone()), add_amount.into(), 1000.into(), 0.into(), 0.into(), caller.clone()) verify { let pool_id = (asset1.clone(), asset2.clone()); let lp_minted = AssetConversion::::calc_lp_amount_for_zero_supply(&add_amount.into(), &1000.into()).unwrap().into(); @@ -157,8 +156,8 @@ benchmarks! { AssetConversion::::add_liquidity( SystemOrigin::Signed(caller.clone()).into(), - asset1.clone(), - asset2.clone(), + Box::new(asset1.clone()), + Box::new(asset2.clone()), add_amount.into(), 1000.into(), 0.into(), @@ -166,7 +165,7 @@ benchmarks! { caller.clone(), )?; let total_supply = >::total_issuance(lp_token.clone()); - }: _(SystemOrigin::Signed(caller.clone()), asset1, asset2, remove_lp_amount.into(), 0.into(), 0.into(), caller.clone()) + }: _(SystemOrigin::Signed(caller.clone()), Box::new(asset1), Box::new(asset2), remove_lp_amount.into(), 0.into(), 0.into(), caller.clone()) verify { let new_total_supply = >::total_issuance(lp_token.clone()); assert_eq!( @@ -185,8 +184,8 @@ benchmarks! { AssetConversion::::add_liquidity( SystemOrigin::Signed(caller.clone()).into(), - native.clone(), - asset1.clone(), + Box::new(native.clone()), + Box::new(asset1.clone()), (100 * ed).into(), 200.into(), 0.into(), @@ -199,29 +198,45 @@ benchmarks! { // if we only allow the native-asset pools, then the worst case scenario would be to swap // asset1-native-asset2 if !T::AllowMultiAssetPools::get() { - AssetConversion::::create_pool(SystemOrigin::Signed(caller.clone()).into(), native.clone(), asset2.clone())?; + AssetConversion::::create_pool( + SystemOrigin::Signed(caller.clone()).into(), + Box::new(native.clone()), + Box::new(asset2.clone()) + )?; AssetConversion::::add_liquidity( SystemOrigin::Signed(caller.clone()).into(), - native.clone(), - asset2.clone(), + Box::new(native.clone()), + Box::new(asset2.clone()), (500 * ed).into(), 1000.into(), 0.into(), 0.into(), caller.clone(), )?; - path = vec![asset1.clone(), native.clone(), asset2.clone()]; + path = vec![ + Box::new(asset1.clone()), + Box::new(native.clone()), + Box::new(asset2.clone()) + ]; swap_amount = 100.into(); } else { let asset3 = T::BenchmarkHelper::multiasset_id(3); - AssetConversion::::create_pool(SystemOrigin::Signed(caller.clone()).into(), asset1.clone(), asset2.clone())?; + AssetConversion::::create_pool( + SystemOrigin::Signed(caller.clone()).into(), + Box::new(asset1.clone()), + Box::new(asset2.clone()) + )?; let (_, _) = create_asset::(&asset3); - AssetConversion::::create_pool(SystemOrigin::Signed(caller.clone()).into(), asset2.clone(), asset3.clone())?; + AssetConversion::::create_pool( + SystemOrigin::Signed(caller.clone()).into(), + Box::new(asset2.clone()), + Box::new(asset3.clone()) + )?; AssetConversion::::add_liquidity( SystemOrigin::Signed(caller.clone()).into(), - asset1.clone(), - asset2.clone(), + Box::new(asset1.clone()), + Box::new(asset2.clone()), 200.into(), 2000.into(), 0.into(), @@ -230,19 +245,22 @@ benchmarks! { )?; AssetConversion::::add_liquidity( SystemOrigin::Signed(caller.clone()).into(), - asset2.clone(), - asset3.clone(), + Box::new(asset2.clone()), + Box::new(asset3.clone()), 2000.into(), 2000.into(), 0.into(), 0.into(), caller.clone(), )?; - path = vec![native.clone(), asset1.clone(), asset2.clone(), asset3.clone()]; + path = vec![ + Box::new(native.clone()), + Box::new(asset1.clone()), + Box::new(asset2.clone()), + Box::new(asset3.clone()) + ]; swap_amount = ed.into(); } - - let path: BoundedVec<_, T::MaxSwapPathLength> = BoundedVec::try_from(path).unwrap(); let native_balance = T::Currency::balance(&caller); let asset1_balance = T::Assets::balance(T::BenchmarkHelper::asset_id(1), &caller); }: _(SystemOrigin::Signed(caller.clone()), path, swap_amount, 1.into(), caller.clone(), false) @@ -266,8 +284,8 @@ benchmarks! { AssetConversion::::add_liquidity( SystemOrigin::Signed(caller.clone()).into(), - native.clone(), - asset1.clone(), + Box::new(native.clone()), + Box::new(asset1.clone()), (1000 * ed).into(), 500.into(), 0.into(), @@ -279,28 +297,44 @@ benchmarks! { // if we only allow the native-asset pools, then the worst case scenario would be to swap // asset1-native-asset2 if !T::AllowMultiAssetPools::get() { - AssetConversion::::create_pool(SystemOrigin::Signed(caller.clone()).into(), native.clone(), asset2.clone())?; + AssetConversion::::create_pool( + SystemOrigin::Signed(caller.clone()).into(), + Box::new(native.clone()), + Box::new(asset2.clone()) + )?; AssetConversion::::add_liquidity( SystemOrigin::Signed(caller.clone()).into(), - native.clone(), - asset2.clone(), + Box::new(native.clone()), + Box::new(asset2.clone()), (500 * ed).into(), 1000.into(), 0.into(), 0.into(), caller.clone(), )?; - path = vec![asset1.clone(), native.clone(), asset2.clone()]; + path = vec![ + Box::new(asset1.clone()), + Box::new(native.clone()), + Box::new(asset2.clone()) + ]; } else { - AssetConversion::::create_pool(SystemOrigin::Signed(caller.clone()).into(), asset1.clone(), asset2.clone())?; + AssetConversion::::create_pool( + SystemOrigin::Signed(caller.clone()).into(), + Box::new(asset1.clone()), + Box::new(asset2.clone()) + )?; let asset3 = T::BenchmarkHelper::multiasset_id(3); let (_, _) = create_asset::(&asset3); - AssetConversion::::create_pool(SystemOrigin::Signed(caller.clone()).into(), asset2.clone(), asset3.clone())?; + AssetConversion::::create_pool( + SystemOrigin::Signed(caller.clone()).into(), + Box::new(asset2.clone()), + Box::new(asset3.clone()) + )?; AssetConversion::::add_liquidity( SystemOrigin::Signed(caller.clone()).into(), - asset1.clone(), - asset2.clone(), + Box::new(asset1.clone()), + Box::new(asset2.clone()), 2000.into(), 2000.into(), 0.into(), @@ -309,18 +343,22 @@ benchmarks! { )?; AssetConversion::::add_liquidity( SystemOrigin::Signed(caller.clone()).into(), - asset2.clone(), - asset3.clone(), + Box::new(asset2.clone()), + Box::new(asset3.clone()), 2000.into(), 2000.into(), 0.into(), 0.into(), caller.clone(), )?; - path = vec![native.clone(), asset1.clone(), asset2.clone(), asset3.clone()]; + path = vec![ + Box::new(native.clone()), + Box::new(asset1.clone()), + Box::new(asset2.clone()), + Box::new(asset3.clone()) + ]; } - let path: BoundedVec<_, T::MaxSwapPathLength> = BoundedVec::try_from(path).unwrap(); let asset2_balance = T::Assets::balance(T::BenchmarkHelper::asset_id(2), &caller); let asset3_balance = T::Assets::balance(T::BenchmarkHelper::asset_id(3), &caller); }: _(SystemOrigin::Signed(caller.clone()), path.clone(), 100.into(), (1000 * ed).into(), caller.clone(), false) From 01bd4da4499bb97a8cb02a1ab24cd8a8274221b2 Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 8 Oct 2023 16:27:38 +0200 Subject: [PATCH 28/43] no box for benchmarks impls --- .../runtimes/assets/asset-hub-kusama/src/xcm_config.rs | 7 +++---- .../runtimes/assets/asset-hub-westend/src/xcm_config.rs | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/xcm_config.rs b/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/xcm_config.rs index 0c197598f889..e29f9b6dfcc6 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/xcm_config.rs @@ -604,8 +604,7 @@ pub struct BenchmarkMultiLocationConverter { } #[cfg(feature = "runtime-benchmarks")] -impl - pallet_asset_conversion::BenchmarkHelper> +impl pallet_asset_conversion::BenchmarkHelper for BenchmarkMultiLocationConverter where SelfParaId: Get, @@ -620,7 +619,7 @@ where ), } } - fn multiasset_id(asset_id: u32) -> sp_std::boxed::Box { - sp_std::boxed::Box::new(Self::asset_id(asset_id)) + fn multiasset_id(asset_id: u32) -> MultiLocation { + Self::asset_id(asset_id) } } diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs index 6981c290c98c..611a920cc039 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/xcm_config.rs @@ -602,8 +602,7 @@ pub struct BenchmarkMultiLocationConverter { } #[cfg(feature = "runtime-benchmarks")] -impl - pallet_asset_conversion::BenchmarkHelper> +impl pallet_asset_conversion::BenchmarkHelper for BenchmarkMultiLocationConverter where SelfParaId: Get, @@ -619,7 +618,7 @@ where } } - fn multiasset_id(asset_id: u32) -> sp_std::boxed::Box { - sp_std::boxed::Box::new(Self::asset_id(asset_id)) + fn multiasset_id(asset_id: u32) -> MultiLocation { + Self::asset_id(asset_id) } } From e2b99f96f5828f6e04a490b4700658a5e902ab92 Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 8 Oct 2023 17:59:29 +0200 Subject: [PATCH 29/43] single balance type for Currency and Assets --- .../assets/asset-hub-kusama/src/lib.rs | 6 +- .../assets/asset-hub-westend/src/lib.rs | 6 +- substrate/bin/node/runtime/src/lib.rs | 6 +- .../asset-conversion/src/benchmarking.rs | 5 +- substrate/frame/asset-conversion/src/lib.rs | 210 +++++++----------- substrate/frame/asset-conversion/src/mock.rs | 3 +- substrate/frame/asset-conversion/src/swap.rs | 10 +- substrate/frame/asset-conversion/src/types.rs | 23 +- .../asset-conversion-tx-payment/src/mock.rs | 1 - .../src/payment.rs | 15 +- 10 files changed, 104 insertions(+), 181 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/lib.rs index efadf4869bf1..123791710070 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-kusama/src/lib.rs @@ -330,7 +330,6 @@ impl pallet_asset_conversion::Config for Runtime { type Balance = Balance; type HigherPrecisionBalance = sp_core::U256; type Currency = Balances; - type AssetBalance = Balance; type AssetId = MultiLocation; type Assets = LocalAndForeignAssets< Assets, @@ -1030,14 +1029,13 @@ impl_runtime_apis! { impl pallet_asset_conversion::AssetConversionApi< Block, Balance, - u128, MultiLocation, > for Runtime { - fn quote_price_exact_tokens_for_tokens(asset1: MultiLocation, asset2: MultiLocation, amount: u128, include_fee: bool) -> Option { + fn quote_price_exact_tokens_for_tokens(asset1: MultiLocation, asset2: MultiLocation, amount: Balance, include_fee: bool) -> Option { AssetConversion::quote_price_exact_tokens_for_tokens(asset1, asset2, amount, include_fee) } - fn quote_price_tokens_for_exact_tokens(asset1: MultiLocation, asset2: MultiLocation, amount: u128, include_fee: bool) -> Option { + fn quote_price_tokens_for_exact_tokens(asset1: MultiLocation, asset2: MultiLocation, amount: Balance, include_fee: bool) -> Option { AssetConversion::quote_price_tokens_for_exact_tokens(asset1, asset2, amount, include_fee) } fn get_reserves(asset1: MultiLocation, asset2: MultiLocation) -> Option<(Balance, Balance)> { diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index 9a27a71bb16a..51f14bfef300 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -307,7 +307,6 @@ impl pallet_asset_conversion::Config for Runtime { type Balance = Balance; type HigherPrecisionBalance = sp_core::U256; type Currency = Balances; - type AssetBalance = Balance; type AssetId = MultiLocation; type Assets = LocalAndForeignAssets< Assets, @@ -1079,15 +1078,14 @@ impl_runtime_apis! { impl pallet_asset_conversion::AssetConversionApi< Block, Balance, - u128, MultiLocation, > for Runtime { - fn quote_price_exact_tokens_for_tokens(asset1: MultiLocation, asset2: MultiLocation, amount: u128, include_fee: bool) -> Option { + fn quote_price_exact_tokens_for_tokens(asset1: MultiLocation, asset2: MultiLocation, amount: Balance, include_fee: bool) -> Option { AssetConversion::quote_price_exact_tokens_for_tokens(asset1, asset2, amount, include_fee) } - fn quote_price_tokens_for_exact_tokens(asset1: MultiLocation, asset2: MultiLocation, amount: u128, include_fee: bool) -> Option { + fn quote_price_tokens_for_exact_tokens(asset1: MultiLocation, asset2: MultiLocation, amount: Balance, include_fee: bool) -> Option { AssetConversion::quote_price_tokens_for_exact_tokens(asset1, asset2, amount, include_fee) } diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index f018639b732e..2db3a3ed3cca 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1632,7 +1632,6 @@ parameter_types! { impl pallet_asset_conversion::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Currency = Balances; - type AssetBalance = ::Balance; type HigherPrecisionBalance = sp_core::U256; type Assets = Assets; type Balance = u128; @@ -2526,15 +2525,14 @@ impl_runtime_apis! { impl pallet_asset_conversion::AssetConversionApi< Block, Balance, - u128, NativeOrAssetId > for Runtime { - fn quote_price_exact_tokens_for_tokens(asset1: NativeOrAssetId, asset2: NativeOrAssetId, amount: u128, include_fee: bool) -> Option { + fn quote_price_exact_tokens_for_tokens(asset1: NativeOrAssetId, asset2: NativeOrAssetId, amount: Balance, include_fee: bool) -> Option { AssetConversion::quote_price_exact_tokens_for_tokens(asset1, asset2, amount, include_fee) } - fn quote_price_tokens_for_exact_tokens(asset1: NativeOrAssetId, asset2: NativeOrAssetId, amount: u128, include_fee: bool) -> Option { + fn quote_price_tokens_for_exact_tokens(asset1: NativeOrAssetId, asset2: NativeOrAssetId, amount: Balance, include_fee: bool) -> Option { AssetConversion::quote_price_tokens_for_exact_tokens(asset1, asset2, amount, include_fee) } diff --git a/substrate/frame/asset-conversion/src/benchmarking.rs b/substrate/frame/asset-conversion/src/benchmarking.rs index 72033c58c8b2..628953d0558f 100644 --- a/substrate/frame/asset-conversion/src/benchmarking.rs +++ b/substrate/frame/asset-conversion/src/benchmarking.rs @@ -48,7 +48,7 @@ where fn create_asset(asset: &T::MultiAssetId) -> (T::AccountId, AccountIdLookupOf) where - T::AssetBalance: From, + T::Balance: From, T::Currency: Unbalanced, T::Assets: Create + Mutate, { @@ -69,7 +69,7 @@ fn create_asset_and_pool( asset2: &T::MultiAssetId, ) -> (T::PoolAssetId, T::AccountId, AccountIdLookupOf) where - T::AssetBalance: From, + T::Balance: From, T::Currency: Unbalanced, T::Assets: Create + Mutate, T::PoolAssetId: Into, @@ -98,7 +98,6 @@ fn assert_last_event(generic_event: ::RuntimeEvent) { benchmarks! { where_clause { where - T::AssetBalance: From + Into, T::Currency: Unbalanced, T::Balance: From + Into, T::Assets: Create + Mutate, diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index 45dcdc0063fe..3e118996f8e9 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -118,21 +118,16 @@ pub mod pallet { + MutateFungible + BalancedFungible; - /// The `Currency::Balance` type of the native currency. + /// The type in which the assets for swapping are measured. type Balance: Balance; - /// The type used to describe the amount of fractions converted into assets. - type AssetBalance: Balance; - - /// A type used for conversions between `Balance` and `AssetBalance`. + /// A type used for calculations concerning the `Balance` type to avoid possible overflows. type HigherPrecisionBalance: IntegerSquareRoot + One + Ensure + Unsigned + From - + From + From - + TryInto + TryInto; /// Identifier for the class of non-native asset. @@ -154,7 +149,7 @@ pub mod pallet { type PoolAssetId: AssetId + PartialOrd + Incrementable + From; /// Registry for the assets. - type Assets: Inspect + type Assets: Inspect + Mutate + AccountTouch + ContainsPair @@ -162,7 +157,7 @@ pub mod pallet { /// Registry for the lp tokens. Ideally only this pallet should have create permissions on /// the assets. - type PoolAssets: Inspect + type PoolAssets: Inspect + Create + Mutate + AccountTouch; @@ -184,7 +179,7 @@ pub mod pallet { /// The minimum LP token amount that could be minted. Ameliorates rounding errors. #[pallet::constant] - type MintMinLiquidity: Get; + type MintMinLiquidity: Get; /// The max number of hops in a swap. #[pallet::constant] @@ -244,13 +239,13 @@ pub mod pallet { /// The pool id of the pool that the liquidity was added to. pool_id: PoolIdOf, /// The amount of the first asset that was added to the pool. - amount1_provided: T::AssetBalance, + amount1_provided: T::Balance, /// The amount of the second asset that was added to the pool. - amount2_provided: T::AssetBalance, + amount2_provided: T::Balance, /// The id of the lp token that was minted. lp_token: T::PoolAssetId, /// The amount of lp tokens that were minted of that id. - lp_token_minted: T::AssetBalance, + lp_token_minted: T::Balance, }, /// A successful call of the `RemoveLiquidity` extrinsic will create this event. @@ -262,13 +257,13 @@ pub mod pallet { /// The pool id that the liquidity was removed from. pool_id: PoolIdOf, /// The amount of the first asset that was removed from the pool. - amount1: T::AssetBalance, + amount1: T::Balance, /// The amount of the second asset that was removed from the pool. - amount2: T::AssetBalance, + amount2: T::Balance, /// The id of the lp token that was burned. lp_token: T::PoolAssetId, /// The amount of lp tokens that were burned of that id. - lp_token_burned: T::AssetBalance, + lp_token_burned: T::Balance, /// Liquidity withdrawal fee (%). withdrawal_fee: Permill, }, @@ -280,9 +275,9 @@ pub mod pallet { /// The account that the assets were transferred to. send_to: T::AccountId, /// The amount of the first asset that was swapped. - amount_in: T::AssetBalance, + amount_in: T::Balance, /// The amount of the second asset that was received. - amount_out: T::AssetBalance, + amount_out: T::Balance, /// The route of asset IDs with amounts that the swap went through. /// E.g. (A, amount_in) -> (Dot, amount_out) -> (B, amount_out) path: BalancePath, @@ -290,9 +285,9 @@ pub mod pallet { /// Assets have been converted from one to another. CreditSwapExecuted { /// The amount of the first asset that was swapped. - amount_in: T::AssetBalance, + amount_in: T::Balance, /// The amount of the second asset that was received. - amount_out: T::AssetBalance, + amount_out: T::Balance, /// The route of asset IDs with amounts that the swap went through. /// E.g. (A, amount_in) -> (Dot, amount_out) -> (B, amount_out) path: BalancePath, @@ -461,10 +456,10 @@ pub mod pallet { origin: OriginFor, asset1: Box, asset2: Box, - amount1_desired: T::AssetBalance, - amount2_desired: T::AssetBalance, - amount1_min: T::AssetBalance, - amount2_min: T::AssetBalance, + amount1_desired: T::Balance, + amount2_desired: T::Balance, + amount1_min: T::Balance, + amount2_min: T::Balance, mint_to: T::AccountId, ) -> DispatchResult { let sender = ensure_signed(origin)?; @@ -490,8 +485,8 @@ pub mod pallet { let reserve1 = Self::get_balance(&pool_account, asset1)?; let reserve2 = Self::get_balance(&pool_account, asset2)?; - let amount1: T::AssetBalance; - let amount2: T::AssetBalance; + let amount1: T::Balance; + let amount2: T::Balance; if reserve1.is_zero() || reserve2.is_zero() { amount1 = amount1_desired; amount2 = amount2_desired; @@ -530,7 +525,7 @@ pub mod pallet { let total_supply = T::PoolAssets::total_issuance(pool.lp_token.clone()); - let lp_token_amount: T::AssetBalance; + let lp_token_amount: T::Balance; if total_supply.is_zero() { lp_token_amount = Self::calc_lp_amount_for_zero_supply(&amount1, &amount2)?; T::PoolAssets::mint_into( @@ -573,9 +568,9 @@ pub mod pallet { origin: OriginFor, asset1: Box, asset2: Box, - lp_token_burn: T::AssetBalance, - amount1_min_receive: T::AssetBalance, - amount2_min_receive: T::AssetBalance, + lp_token_burn: T::Balance, + amount1_min_receive: T::Balance, + amount2_min_receive: T::Balance, withdraw_to: T::AccountId, ) -> DispatchResult { let sender = ensure_signed(origin)?; @@ -651,8 +646,8 @@ pub mod pallet { pub fn swap_exact_tokens_for_tokens( origin: OriginFor, path: Vec>, - amount_in: T::AssetBalance, - amount_out_min: T::AssetBalance, + amount_in: T::Balance, + amount_out_min: T::Balance, send_to: T::AccountId, keep_alive: bool, ) -> DispatchResult { @@ -679,8 +674,8 @@ pub mod pallet { pub fn swap_tokens_for_exact_tokens( origin: OriginFor, path: Vec>, - amount_out: T::AssetBalance, - amount_in_max: T::AssetBalance, + amount_out: T::Balance, + amount_in_max: T::Balance, send_to: T::AccountId, keep_alive: bool, ) -> DispatchResult { @@ -713,11 +708,11 @@ pub mod pallet { pub(crate) fn do_swap_exact_tokens_for_tokens( sender: T::AccountId, path: Vec, - amount_in: T::AssetBalance, - amount_out_min: Option, + amount_in: T::Balance, + amount_out_min: Option, send_to: T::AccountId, keep_alive: bool, - ) -> Result { + ) -> Result { ensure!(amount_in > Zero::zero(), Error::::ZeroAmount); if let Some(amount_out_min) = amount_out_min { ensure!(amount_out_min > Zero::zero(), Error::::ZeroAmount); @@ -761,11 +756,11 @@ pub mod pallet { pub(crate) fn do_swap_tokens_for_exact_tokens( sender: T::AccountId, path: Vec, - amount_out: T::AssetBalance, - amount_in_max: Option, + amount_out: T::Balance, + amount_in_max: Option, send_to: T::AccountId, keep_alive: bool, - ) -> Result { + ) -> Result { ensure!(amount_out > Zero::zero(), Error::::ZeroAmount); if let Some(amount_in_max) = amount_in_max { ensure!(amount_in_max > Zero::zero(), Error::::ZeroAmount); @@ -808,12 +803,9 @@ pub mod pallet { pub(crate) fn do_swap_exact_credit_tokens_for_tokens( path: Vec, credit_in: Credit, - amount_out_min: Option, + amount_out_min: Option, ) -> Result, (Credit, DispatchError)> { - let amount_in = match credit_in.peek() { - Ok(a) => a, - Err(e) => return Err((credit_in, e)), - }; + let amount_in = credit_in.peek(); let inspect_path = |credit_asset| { ensure!(path.get(0).map_or(false, |a| *a == credit_asset), Error::::InvalidPath); ensure!(!amount_in.is_zero(), Error::::ZeroAmount); @@ -856,12 +848,9 @@ pub mod pallet { pub(crate) fn do_swap_credit_tokens_for_exact_tokens( path: Vec, credit_in: Credit, - amount_out: T::AssetBalance, + amount_out: T::Balance, ) -> Result<(Credit, Credit), (Credit, DispatchError)> { - let amount_in_max = match credit_in.peek() { - Ok(a) => a, - Err(e) => return Err((credit_in, e)), - }; + let amount_in_max = credit_in.peek(); let inspect_path = |credit_asset| { ensure!(path.get(0).map_or(false, |a| a == &credit_asset), Error::::InvalidPath); ensure!(amount_in_max > Zero::zero(), Error::::ZeroAmount); @@ -883,7 +872,7 @@ pub mod pallet { Err(e) => return Err((credit_in, e)), }; - let (credit_in, credit_change) = credit_in.split(amount_in)?; + let (credit_in, credit_change) = credit_in.split(amount_in); let credit_out = Self::credit_swap(credit_in, &path)?; Self::deposit_event(Event::CreditSwapExecuted { amount_in, amount_out, path }); @@ -896,9 +885,9 @@ pub mod pallet { asset_id: &T::MultiAssetId, from: &T::AccountId, to: &T::AccountId, - amount: T::AssetBalance, + amount: T::Balance, keep_alive: bool, - ) -> Result { + ) -> Result { let preservation = match keep_alive { true => Preserve, false => Expendable, @@ -906,15 +895,8 @@ pub mod pallet { match T::MultiAssetIdConverter::try_convert(asset_id) { MultiAssetIdConversionResult::Converted(asset_id) => T::Assets::transfer(asset_id, from, to, amount, preservation), - MultiAssetIdConversionResult::Native => { - 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, - )?)?) - }, + MultiAssetIdConversionResult::Native => + Ok(T::Currency::transfer(from, to, amount, preservation)?), MultiAssetIdConversionResult::Unsupported(_) => Err(Error::::UnsupportedAsset.into()), } @@ -934,7 +916,7 @@ pub mod pallet { fn withdraw( asset: &T::MultiAssetId, who: &T::AccountId, - value: T::AssetBalance, + value: T::Balance, keep_alive: bool, ) -> Result, DispatchError> { let preservation = match keep_alive { @@ -954,7 +936,6 @@ pub mod pallet { .map(|c| c.into()) }, MultiAssetIdConversionResult::Native => { - let value = Self::convert_asset_balance_to_native_balance(value)?; if preservation == Preserve { // TODO drop the ensure! when this issue addressed // https://github.com/paritytech/polkadot-sdk/issues/1698 @@ -968,31 +949,6 @@ pub mod pallet { } } - /// 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) - } - - /// 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) - } - - /// Convert a `HigherPrecisionBalance` type to an `AssetBalance`. - pub(crate) fn convert_hpb_to_asset_balance( - amount: T::HigherPrecisionBalance, - ) -> Result> { - amount.try_into().map_err(|_| Error::::Overflow) - } - /// Swap assets along the `path`, withdrawing from `sender` and depositing in `send_to`. /// /// Note: It's assumed that the provided `path` is valid. @@ -1080,19 +1036,17 @@ pub mod pallet { } /// 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`. + /// fungible. Returns a value in the form of an `Balance`. fn get_balance( owner: &T::AccountId, asset: &T::MultiAssetId, - ) -> Result> { + ) -> Result> { match T::MultiAssetIdConverter::try_convert(asset) { MultiAssetIdConversionResult::Converted(asset_id) => Ok( <::Assets>::reducible_balance(asset_id, owner, Expendable, Polite), ), MultiAssetIdConversionResult::Native => - Self::convert_native_balance_to_asset_balance( - <::Currency>::reducible_balance(owner, Expendable, Polite), - ), + Ok(<::Currency>::reducible_balance(owner, Expendable, Polite)), MultiAssetIdConversionResult::Unsupported(_) => Err(Error::::UnsupportedAsset.into()), } @@ -1127,7 +1081,7 @@ pub mod pallet { pub fn get_reserves( asset1: &T::MultiAssetId, asset2: &T::MultiAssetId, - ) -> Result<(T::AssetBalance, T::AssetBalance), Error> { + ) -> Result<(T::Balance, T::Balance), Error> { let pool_id = Self::get_pool_id(asset1.clone(), asset2.clone()); let pool_account = Self::get_pool_account(&pool_id); @@ -1143,11 +1097,11 @@ pub mod pallet { /// Leading to an amount at the end of a `path`, get the required amounts in. pub(crate) fn balance_path_from_amount_out( - amount_out: T::AssetBalance, + amount_out: T::Balance, path: Vec, ) -> Result, DispatchError> { let mut balance_path: BalancePath = Vec::with_capacity(path.len()); - let mut amount_in: T::AssetBalance = amount_out; + let mut amount_in: T::Balance = amount_out; let mut iter = path.into_iter().rev().peekable(); while let Some(asset2) = iter.next() { @@ -1169,11 +1123,11 @@ pub mod pallet { /// Following an amount into a `path`, get the corresponding amounts out. pub(crate) fn balance_path_from_amount_in( - amount_in: T::AssetBalance, + amount_in: T::Balance, path: Vec, ) -> Result, DispatchError> { let mut balance_path: BalancePath = Vec::with_capacity(path.len()); - let mut amount_out: T::AssetBalance = amount_in; + let mut amount_out: T::Balance = amount_in; let mut iter = path.into_iter().peekable(); while let Some(asset1) = iter.next() { @@ -1195,9 +1149,9 @@ pub mod pallet { pub fn quote_price_exact_tokens_for_tokens( asset1: T::MultiAssetId, asset2: T::MultiAssetId, - amount: T::AssetBalance, + amount: T::Balance, include_fee: bool, - ) -> Option { + ) -> Option { let pool_id = Self::get_pool_id(asset1.clone(), asset2.clone()); let pool_account = Self::get_pool_account(&pool_id); @@ -1218,9 +1172,9 @@ pub mod pallet { pub fn quote_price_tokens_for_exact_tokens( asset1: T::MultiAssetId, asset2: T::MultiAssetId, - amount: T::AssetBalance, + amount: T::Balance, include_fee: bool, - ) -> Option { + ) -> Option { let pool_id = Self::get_pool_id(asset1.clone(), asset2.clone()); let pool_account = Self::get_pool_account(&pool_id); @@ -1239,18 +1193,18 @@ pub mod pallet { /// Calculates the optimal amount from the reserves. pub fn quote( - amount: &T::AssetBalance, - reserve1: &T::AssetBalance, - reserve2: &T::AssetBalance, - ) -> Result> { + amount: &T::Balance, + reserve1: &T::Balance, + reserve2: &T::Balance, + ) -> Result> { // (amount * reserve2) / reserve1 Self::mul_div(amount, reserve2, reserve1) } pub(super) fn calc_lp_amount_for_zero_supply( - amount1: &T::AssetBalance, - amount2: &T::AssetBalance, - ) -> Result> { + amount1: &T::Balance, + amount2: &T::Balance, + ) -> Result> { let amount1 = T::HigherPrecisionBalance::from(*amount1); let amount2 = T::HigherPrecisionBalance::from(*amount2); @@ -1264,11 +1218,7 @@ pub mod pallet { result.try_into().map_err(|_| Error::::Overflow) } - fn mul_div( - a: &T::AssetBalance, - b: &T::AssetBalance, - c: &T::AssetBalance, - ) -> Result> { + fn mul_div(a: &T::Balance, b: &T::Balance, c: &T::Balance) -> Result> { let a = T::HigherPrecisionBalance::from(*a); let b = T::HigherPrecisionBalance::from(*b); let c = T::HigherPrecisionBalance::from(*c); @@ -1287,10 +1237,10 @@ pub mod pallet { /// Given an input amount of an asset and pair reserves, returns the maximum output amount /// of the other asset. pub fn get_amount_out( - amount_in: &T::AssetBalance, - reserve_in: &T::AssetBalance, - reserve_out: &T::AssetBalance, - ) -> Result> { + amount_in: &T::Balance, + reserve_in: &T::Balance, + reserve_out: &T::Balance, + ) -> Result> { let amount_in = T::HigherPrecisionBalance::from(*amount_in); let reserve_in = T::HigherPrecisionBalance::from(*reserve_in); let reserve_out = T::HigherPrecisionBalance::from(*reserve_out); @@ -1322,10 +1272,10 @@ pub mod pallet { /// Given an output amount of an asset and pair reserves, returns a required input amount /// of the other asset. pub fn get_amount_in( - amount_out: &T::AssetBalance, - reserve_in: &T::AssetBalance, - reserve_out: &T::AssetBalance, - ) -> Result> { + amount_out: &T::Balance, + reserve_in: &T::Balance, + reserve_out: &T::Balance, + ) -> Result> { let amount_out = T::HigherPrecisionBalance::from(*amount_out); let reserve_in = T::HigherPrecisionBalance::from(*reserve_in); let reserve_out = T::HigherPrecisionBalance::from(*reserve_out); @@ -1360,10 +1310,7 @@ pub mod pallet { } /// Ensure that a `value` meets the minimum balance requirements of an `asset` class. - fn validate_minimal_amount( - value: T::AssetBalance, - asset: &T::MultiAssetId, - ) -> Result<(), ()> { + fn validate_minimal_amount(value: T::Balance, asset: &T::MultiAssetId) -> Result<(), ()> { if T::MultiAssetIdConverter::is_native(asset) { let ed = T::Currency::minimum_balance(); ensure!( @@ -1414,22 +1361,21 @@ pub mod pallet { sp_api::decl_runtime_apis! { /// This runtime api allows people to query the size of the liquidity pools /// and quote prices for swaps. - pub trait AssetConversionApi where - Balance: Codec + MaybeDisplay, - AssetBalance: frame_support::traits::tokens::Balance, + pub trait AssetConversionApi where + Balance: frame_support::traits::tokens::Balance + MaybeDisplay, AssetId: Codec { /// Provides a quote for [`Pallet::swap_tokens_for_exact_tokens`]. /// /// Note that the price may have changed by the time the transaction is executed. /// (Use `amount_in_max` to control slippage.) - fn quote_price_tokens_for_exact_tokens(asset1: AssetId, asset2: AssetId, amount: AssetBalance, include_fee: bool) -> Option; + fn quote_price_tokens_for_exact_tokens(asset1: AssetId, asset2: AssetId, amount: Balance, include_fee: bool) -> Option; /// Provides a quote for [`Pallet::swap_exact_tokens_for_tokens`]. /// /// Note that the price may have changed by the time the transaction is executed. /// (Use `amount_out_min` to control slippage.) - fn quote_price_exact_tokens_for_tokens(asset1: AssetId, asset2: AssetId, amount: AssetBalance, include_fee: bool) -> Option; + fn quote_price_exact_tokens_for_tokens(asset1: AssetId, asset2: AssetId, amount: Balance, include_fee: bool) -> Option; /// Returns the size of the liquidity pool for the given asset pair. fn get_reserves(asset1: AssetId, asset2: AssetId) -> Option<(Balance, Balance)>; diff --git a/substrate/frame/asset-conversion/src/mock.rs b/substrate/frame/asset-conversion/src/mock.rs index 3a19f39e7ca6..03949afd07e7 100644 --- a/substrate/frame/asset-conversion/src/mock.rs +++ b/substrate/frame/asset-conversion/src/mock.rs @@ -152,7 +152,6 @@ ord_parameter_types! { impl Config for Test { type RuntimeEvent = RuntimeEvent; type Currency = Balances; - type AssetBalance = ::Balance; type AssetId = u32; type PoolAssetId = u32; type Assets = Assets; @@ -167,7 +166,7 @@ impl Config for Test { type MaxSwapPathLength = ConstU32<4>; type MintMinLiquidity = ConstU128<100>; // 100 is good enough when the main currency has 12 decimals. - type Balance = u128; + type Balance = ::Balance; type HigherPrecisionBalance = sp_core::U256; type MultiAssetId = NativeOrAssetId; diff --git a/substrate/frame/asset-conversion/src/swap.rs b/substrate/frame/asset-conversion/src/swap.rs index a86b55d41a53..a760ffd10a65 100644 --- a/substrate/frame/asset-conversion/src/swap.rs +++ b/substrate/frame/asset-conversion/src/swap.rs @@ -105,7 +105,7 @@ pub trait SwapCredit { } impl Swap for Pallet { - type Balance = T::HigherPrecisionBalance; + type Balance = T::Balance; type MultiAssetId = T::MultiAssetId; fn max_path_len() -> u32 { @@ -120,12 +120,11 @@ impl Swap for Pallet { send_to: T::AccountId, keep_alive: bool, ) -> Result { - let amount_out_min = amount_out_min.map(Self::convert_hpb_to_asset_balance).transpose()?; let amount_out = with_storage_layer(|| { Self::do_swap_exact_tokens_for_tokens( sender, path, - Self::convert_hpb_to_asset_balance(amount_in)?, + amount_in, amount_out_min, send_to, keep_alive, @@ -142,12 +141,11 @@ impl Swap for Pallet { send_to: T::AccountId, keep_alive: bool, ) -> Result { - let amount_in_max = amount_in_max.map(Self::convert_hpb_to_asset_balance).transpose()?; let amount_in = with_storage_layer(|| { Self::do_swap_tokens_for_exact_tokens( sender, path, - Self::convert_hpb_to_asset_balance(amount_out)?, + amount_out, amount_in_max, send_to, keep_alive, @@ -158,7 +156,7 @@ impl Swap for Pallet { } impl SwapCredit for Pallet { - type Balance = T::AssetBalance; + type Balance = T::Balance; type MultiAssetId = T::MultiAssetId; type Credit = Credit; diff --git a/substrate/frame/asset-conversion/src/types.rs b/substrate/frame/asset-conversion/src/types.rs index 466faf434031..c2bc74c51226 100644 --- a/substrate/frame/asset-conversion/src/types.rs +++ b/substrate/frame/asset-conversion/src/types.rs @@ -36,7 +36,7 @@ pub(super) type PoolIdOf = (::MultiAssetId, ::Multi /// 1. `asset(asset1, amount_in)` take from `user` and move to the pool(asset1, asset2); /// 2. `asset(asset2, amount_out2)` transfer from pool(asset1, asset2) to pool(asset2, asset3); /// 3. `asset(asset3, amount_out3)` move from pool(asset2, asset3) to `user`. -pub(super) type BalancePath = Vec<(::MultiAssetId, ::AssetBalance)>; +pub(super) type BalancePath = Vec<(::MultiAssetId, ::Balance)>; /// Stores the lp_token asset id a particular pool has been assigned. #[derive(Decode, Encode, Default, PartialEq, Eq, MaxEncodedLen, TypeInfo)] @@ -206,14 +206,11 @@ impl Credit { } /// Amount of `self`. - pub fn peek(&self) -> Result { - let amount = match self { - Credit::Native(c) => T::HigherPrecisionBalance::from(c.peek()) - .try_into() - .map_err(|_| Error::::Overflow)?, + pub fn peek(&self) -> T::Balance { + match self { + Credit::Native(c) => c.peek(), Credit::Asset(c) => c.peek(), - }; - Ok(amount) + } } /// Asset class of `self`. @@ -226,19 +223,15 @@ impl Credit { /// Consume `self` and return two independent instances; the first is guaranteed to be at most /// `amount` and the second will be the remainder. - pub fn split(self, amount: T::AssetBalance) -> Result<(Self, Self), (Self, DispatchError)> { + pub fn split(self, amount: T::Balance) -> (Self, Self) { match self { Credit::Native(c) => { - let amount = match T::HigherPrecisionBalance::from(amount).try_into() { - Ok(a) => a, - Err(_) => return Err((Self::Native(c), Error::::Overflow.into())), - }; let (left, right) = c.split(amount); - Ok((left.into(), right.into())) + (left.into(), right.into()) }, Credit::Asset(c) => { let (left, right) = c.split(amount); - Ok((left.into(), right.into())) + (left.into(), right.into()) }, } } diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs index bfbe8b4178ce..0091861f85fc 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs @@ -235,7 +235,6 @@ ord_parameter_types! { impl pallet_asset_conversion::Config for Runtime { type RuntimeEvent = RuntimeEvent; type Currency = Balances; - type AssetBalance = ::Balance; type AssetId = u32; type PoolAssetId = u32; type Assets = Assets; diff --git a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs index ac13e121dd65..ccea9e55bbed 100644 --- a/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs +++ b/substrate/frame/transaction-payment/asset-conversion-tx-payment/src/payment.rs @@ -83,8 +83,8 @@ impl OnChargeAssetTransaction for AssetConversionAdapter where T: Config, C: Inspect<::AccountId>, - CON: Swap, - T::HigherPrecisionBalance: From> + TryInto>, + CON: Swap, MultiAssetId = T::MultiAssetId>, + BalanceOf: Into>, T::MultiAssetId: From>, BalanceOf: IsType<::AccountId>>::Balance>, { @@ -117,22 +117,18 @@ where let asset_consumed = CON::swap_tokens_for_exact_tokens( who.clone(), vec![asset_id.into(), T::MultiAssetIdConverter::get_native()], - T::HigherPrecisionBalance::from(native_asset_required), + native_asset_required, None, who.clone(), true, ) .map_err(|_| TransactionValidityError::from(InvalidTransaction::Payment))?; - let asset_consumed = asset_consumed - .try_into() - .map_err(|_| TransactionValidityError::from(InvalidTransaction::Payment))?; - ensure!(asset_consumed > Zero::zero(), InvalidTransaction::Payment); // charge the fee in native currency ::withdraw_fee(who, call, info, fee, tip) - .map(|r| (r, native_asset_required, asset_consumed)) + .map(|r| (r, native_asset_required, asset_consumed.into())) } /// Correct the fee and swap the refund back to asset. @@ -175,8 +171,7 @@ where T::MultiAssetIdConverter::get_native(), // we provide the native asset_id.into(), // we want asset_id back ], - T::HigherPrecisionBalance::from(swap_back), /* amount of the native asset to - * convert to `asset_id` */ + 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 From 37b2e34f6dd1ee334566507c753a73e991055f42 Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 8 Oct 2023 18:00:15 +0200 Subject: [PATCH 30/43] Balance trait bound on Balance generic type --- substrate/frame/asset-conversion/src/swap.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/asset-conversion/src/swap.rs b/substrate/frame/asset-conversion/src/swap.rs index a760ffd10a65..5dff3403fb83 100644 --- a/substrate/frame/asset-conversion/src/swap.rs +++ b/substrate/frame/asset-conversion/src/swap.rs @@ -22,7 +22,7 @@ use super::*; /// Trait for providing methods to swap between the various asset classes. pub trait Swap { /// Measure units of the asset classes for swapping. - type Balance; + type Balance: Balance; /// Kind of assets that are going to be swapped. type MultiAssetId; @@ -67,7 +67,7 @@ pub trait Swap { /// Trait providing methods to swap between the various asset classes. pub trait SwapCredit { /// Measure units of the asset classes for swapping. - type Balance; + type Balance: Balance; /// Kind of assets that are going to be swapped. type MultiAssetId; /// Credit implying a negative imbalance in the system that can be placed into an account or From d9bc83607f349dde5dc11aae2a8a0169f8a90247 Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 8 Oct 2023 19:13:26 +0200 Subject: [PATCH 31/43] try into for credit --- substrate/frame/asset-conversion/src/types.rs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/substrate/frame/asset-conversion/src/types.rs b/substrate/frame/asset-conversion/src/types.rs index c2bc74c51226..4e9ebf792ca2 100644 --- a/substrate/frame/asset-conversion/src/types.rs +++ b/substrate/frame/asset-conversion/src/types.rs @@ -161,6 +161,12 @@ impl MultiAssetIdConverter, Asset } } +/// Provides a way to retrieve an underlying value without changing the state of the object. +pub trait Inspectable { + /// Retrieves the underlying value without modifying it. + fn peek(&self) -> Value; +} + /// Credit of [Config::Currency]. /// /// Implies a negative imbalance in the system that can be placed into an account or alter the total @@ -199,6 +205,26 @@ impl From> for Credit { } } +impl TryInto> for Credit { + type Error = (); + fn try_into(self) -> Result, ()> { + match self { + Credit::Native(c) => Ok(c), + _ => Err(()), + } + } +} + +impl TryInto> for Credit { + type Error = (); + fn try_into(self) -> Result, ()> { + match self { + Credit::Asset(c) => Ok(c), + _ => Err(()), + } + } +} + impl Credit { /// Create zero native credit. pub fn native_zero() -> Self { From 01938649d7dc70d80ca864e05d7bd75d7d95a645 Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 8 Oct 2023 19:15:23 +0200 Subject: [PATCH 32/43] drop clone --- cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index 51f14bfef300..14170ed99af3 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -1460,7 +1460,7 @@ pub mod migrations { // fix new account let new_pool_id = pallet_asset_conversion::Pallet::::get_pool_id( valid_native_asset, - old_pool_id.1.clone(), + *old_pool_id.1, ); let new_pool_account = pallet_asset_conversion::Pallet::::get_pool_account(&new_pool_id); From ae19bb86901d1f777210265e81693d7fda2726c2 Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 8 Oct 2023 20:12:17 +0200 Subject: [PATCH 33/43] fix --- cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index 14170ed99af3..cd7fc226693c 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -1460,7 +1460,7 @@ pub mod migrations { // fix new account let new_pool_id = pallet_asset_conversion::Pallet::::get_pool_id( valid_native_asset, - *old_pool_id.1, + old_pool_id.1, ); let new_pool_account = pallet_asset_conversion::Pallet::::get_pool_account(&new_pool_id); From 23722774b973c6f19a4ab6606097cfc3163049e4 Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 9 Oct 2023 10:50:39 +0200 Subject: [PATCH 34/43] clippy fixes --- .../asset-hub-westend/src/tests/swap.rs | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/swap.rs b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/swap.rs index d56a4c7e4de6..feb30fa0347e 100644 --- a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/swap.rs +++ b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend/src/tests/swap.rs @@ -43,8 +43,8 @@ fn swap_locally_on_chain_using_local_assets() { assert_ok!(::AssetConversion::create_pool( ::RuntimeOrigin::signed(AssetHubWestendSender::get()), - Box::new(asset_native.clone()), - Box::new(asset_one.clone()), + Box::new(asset_native), + Box::new(asset_one), )); assert_expected_events!( @@ -56,8 +56,8 @@ fn swap_locally_on_chain_using_local_assets() { assert_ok!(::AssetConversion::add_liquidity( ::RuntimeOrigin::signed(AssetHubWestendSender::get()), - Box::new(asset_native.clone()), - Box::new(asset_one.clone()), + Box::new(asset_native), + Box::new(asset_one), 1_000_000_000_000, 2_000_000_000_000, 0, @@ -72,7 +72,7 @@ fn swap_locally_on_chain_using_local_assets() { ] ); - let path = vec![Box::new(asset_native.clone()), Box::new(asset_one.clone())]; + let path = vec![Box::new(asset_native), Box::new(asset_one)]; assert_ok!(::AssetConversion::swap_exact_tokens_for_tokens( ::RuntimeOrigin::signed(AssetHubWestendSender::get()), @@ -240,8 +240,8 @@ fn swap_locally_on_chain_using_foreign_assets() { // 4. Create pool: assert_ok!(::AssetConversion::create_pool( ::RuntimeOrigin::signed(AssetHubWestendSender::get()), - Box::new(asset_native.clone()), - Box::new(foreign_asset1_at_asset_hub_westend.clone()), + Box::new(asset_native), + Box::new(foreign_asset1_at_asset_hub_westend), )); assert_expected_events!( @@ -256,8 +256,8 @@ fn swap_locally_on_chain_using_foreign_assets() { ::RuntimeOrigin::signed( sov_penpal_on_asset_hub_westend.clone() ), - Box::new(asset_native.clone()), - Box::new(foreign_asset1_at_asset_hub_westend.clone()), + Box::new(asset_native), + Box::new(foreign_asset1_at_asset_hub_westend), 1_000_000_000_000, 2_000_000_000_000, 0, @@ -275,10 +275,7 @@ fn swap_locally_on_chain_using_foreign_assets() { ); // 6. Swap! - let path = vec![ - Box::new(asset_native.clone()), - Box::new(foreign_asset1_at_asset_hub_westend.clone()), - ]; + let path = vec![Box::new(asset_native), Box::new(foreign_asset1_at_asset_hub_westend)]; assert_ok!(::AssetConversion::swap_exact_tokens_for_tokens( ::RuntimeOrigin::signed(AssetHubWestendSender::get()), @@ -341,7 +338,7 @@ fn cannot_create_pool_from_pool_assets() { assert_matches::assert_matches!( ::AssetConversion::create_pool( ::RuntimeOrigin::signed(AssetHubWestendSender::get()), - Box::new(asset_native.clone()), + Box::new(asset_native), Box::new(asset_one), ), Err(DispatchError::Module(ModuleError{index: _, error: _, message})) => assert_eq!(message, Some("UnsupportedAsset")) From d1f37c781995eef02561f36a709f4bf554744e7b Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 9 Oct 2023 11:34:40 +0200 Subject: [PATCH 35/43] review fixes --- substrate/frame/asset-conversion/src/lib.rs | 8 +-- substrate/frame/asset-conversion/src/swap.rs | 53 ++++++++----------- substrate/frame/asset-conversion/src/types.rs | 5 +- 3 files changed, 28 insertions(+), 38 deletions(-) diff --git a/substrate/frame/asset-conversion/src/lib.rs b/substrate/frame/asset-conversion/src/lib.rs index 3e118996f8e9..e25725157c32 100644 --- a/substrate/frame/asset-conversion/src/lib.rs +++ b/substrate/frame/asset-conversion/src/lib.rs @@ -283,7 +283,7 @@ pub mod pallet { path: BalancePath, }, /// Assets have been converted from one to another. - CreditSwapExecuted { + SwapCreditExecuted { /// The amount of the first asset that was swapped. amount_in: T::Balance, /// The amount of the second asset that was received. @@ -351,7 +351,7 @@ pub mod pallet { NonUniquePath, /// It was not possible to get or increment the Id of the pool. IncorrectPoolAssetId, - /// Account cannot exist with the funds that would be given. + /// The destination account cannot exist with the swapped funds. BelowMinimum, } @@ -828,7 +828,7 @@ pub mod pallet { let credit_out = Self::credit_swap(credit_in, &path)?; - Self::deposit_event(Event::CreditSwapExecuted { amount_in, amount_out, path }); + Self::deposit_event(Event::SwapCreditExecuted { amount_in, amount_out, path }); Ok(credit_out) } @@ -875,7 +875,7 @@ pub mod pallet { let (credit_in, credit_change) = credit_in.split(amount_in); let credit_out = Self::credit_swap(credit_in, &path)?; - Self::deposit_event(Event::CreditSwapExecuted { amount_in, amount_out, path }); + Self::deposit_event(Event::SwapCreditExecuted { amount_in, amount_out, path }); Ok((credit_out, credit_change)) } diff --git a/substrate/frame/asset-conversion/src/swap.rs b/substrate/frame/asset-conversion/src/swap.rs index 5dff3403fb83..26b5e45ca84b 100644 --- a/substrate/frame/asset-conversion/src/swap.rs +++ b/substrate/frame/asset-conversion/src/swap.rs @@ -169,22 +169,17 @@ impl SwapCredit for Pallet { credit_in: Self::Credit, amount_out_min: Option, ) -> Result { - let transaction_res = - with_transaction(|| -> TransactionOutcome> { - let res = - Self::do_swap_exact_credit_tokens_for_tokens(path, credit_in, amount_out_min); - match &res { - Ok(_) => TransactionOutcome::Commit(Ok(res)), - // wrapping `res` with `Ok`, since our `Err` doesn't satisfy the - // `From` bound of the `with_transaction` function. - Err(_) => TransactionOutcome::Rollback(Ok(res)), - } - }); - match transaction_res { - Ok(r) => r, - // should never happen, `with_transaction` above never returns an `Err` variant. - Err(_) => Err((Self::Credit::native_zero(), DispatchError::Corruption)), - } + with_transaction(|| -> TransactionOutcome> { + let res = Self::do_swap_exact_credit_tokens_for_tokens(path, credit_in, amount_out_min); + match &res { + Ok(_) => TransactionOutcome::Commit(Ok(res)), + // wrapping `res` with `Ok`, since our `Err` doesn't satisfy the + // `From` bound of the `with_transaction` function. + Err(_) => TransactionOutcome::Rollback(Ok(res)), + } + }) + // should never map an error since `with_transaction` above never returns it. + .map_err(|_| (Self::Credit::native_zero(), DispatchError::Corruption))? } fn swap_tokens_for_exact_tokens( @@ -192,20 +187,16 @@ impl SwapCredit for Pallet { credit_in: Self::Credit, amount_out: Self::Balance, ) -> Result<(Self::Credit, Self::Credit), (Self::Credit, DispatchError)> { - let transaction_res = - with_transaction(|| -> TransactionOutcome> { - let res = Self::do_swap_credit_tokens_for_exact_tokens(path, credit_in, amount_out); - match &res { - Ok(_) => TransactionOutcome::Commit(Ok(res)), - // wrapping `res` with `Ok`, since our `Err` doesn't satisfy the - // `From` bound of the `with_transaction` function. - Err(_) => TransactionOutcome::Rollback(Ok(res)), - } - }); - match transaction_res { - Ok(r) => r, - // should never happen, `with_transaction` above never returns an `Err` variant. - Err(_) => Err((Self::Credit::native_zero(), DispatchError::Corruption)), - } + with_transaction(|| -> TransactionOutcome> { + let res = Self::do_swap_credit_tokens_for_exact_tokens(path, credit_in, amount_out); + match &res { + Ok(_) => TransactionOutcome::Commit(Ok(res)), + // wrapping `res` with `Ok`, since our `Err` doesn't satisfy the + // `From` bound of the `with_transaction` function. + Err(_) => TransactionOutcome::Rollback(Ok(res)), + } + }) + // should never map an error since `with_transaction` above never returns it. + .map_err(|_| (Self::Credit::native_zero(), DispatchError::Corruption))? } } diff --git a/substrate/frame/asset-conversion/src/types.rs b/substrate/frame/asset-conversion/src/types.rs index 4e9ebf792ca2..849b1fc9a1cd 100644 --- a/substrate/frame/asset-conversion/src/types.rs +++ b/substrate/frame/asset-conversion/src/types.rs @@ -27,9 +27,8 @@ use sp_std::{cmp::Ordering, marker::PhantomData}; /// migration. pub(super) type PoolIdOf = (::MultiAssetId, ::MultiAssetId); -/// Represents a swap path with associated asset amounts for input and output. -/// -/// Each pair of neighboring assets forms a pool. +/// Represents a swap path with associated asset amounts indicating how much of the asset needs to +/// be deposited to get the following asset's amount withdrawn (this is inclusive of fees). /// /// Example: /// Given path [(asset1, amount_in), (asset2, amount_out2), (asset3, amount_out3)], can be resolved: From 3a4935ec1a8e2b5244c886febc129069abec10e8 Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 9 Oct 2023 12:56:28 +0200 Subject: [PATCH 36/43] remove unused trait --- substrate/frame/asset-conversion/src/types.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/substrate/frame/asset-conversion/src/types.rs b/substrate/frame/asset-conversion/src/types.rs index 849b1fc9a1cd..d5ad8f590653 100644 --- a/substrate/frame/asset-conversion/src/types.rs +++ b/substrate/frame/asset-conversion/src/types.rs @@ -160,12 +160,6 @@ impl MultiAssetIdConverter, Asset } } -/// Provides a way to retrieve an underlying value without changing the state of the object. -pub trait Inspectable { - /// Retrieves the underlying value without modifying it. - fn peek(&self) -> Value; -} - /// Credit of [Config::Currency]. /// /// Implies a negative imbalance in the system that can be placed into an account or alter the total From 75a7b8a3b53a054653b68e54298fa59ede77f18e Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 10 Oct 2023 12:53:59 +0200 Subject: [PATCH 37/43] note in docs for swap traits --- substrate/frame/asset-conversion/src/swap.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/substrate/frame/asset-conversion/src/swap.rs b/substrate/frame/asset-conversion/src/swap.rs index 26b5e45ca84b..7f59b8f9b804 100644 --- a/substrate/frame/asset-conversion/src/swap.rs +++ b/substrate/frame/asset-conversion/src/swap.rs @@ -37,6 +37,8 @@ pub trait Swap { /// respecting `keep_alive`. /// /// If successful, returns the amount of `path[last]` acquired for the `amount_in`. + /// + /// This operation is expected to be atomic. fn swap_exact_tokens_for_tokens( sender: AccountId, path: Vec, @@ -54,6 +56,8 @@ pub trait Swap { /// respecting `keep_alive`. /// /// If successful returns the amount of the `path[0]` taken to provide `path[last]`. + /// + /// This operation is expected to be atomic. fn swap_tokens_for_exact_tokens( sender: AccountId, path: Vec, @@ -83,6 +87,8 @@ pub trait SwapCredit { /// On a successful swap, the function returns the `credit_out` of `path[last]` obtained from /// the `credit_in`. On failure, it returns an `Err` containing the original `credit_in` and the /// associated error code. + /// + /// This operation is expected to be atomic. fn swap_exact_tokens_for_tokens( path: Vec, credit_in: Self::Credit, @@ -97,6 +103,8 @@ pub trait SwapCredit { /// represents the acquired amount of the `path[last]` asset, and `credit_change` is the /// remaining portion from the `credit_in`. On failure, an `Err` with the initial `credit_in` /// and error code is returned. + /// + /// This operation is expected to be atomic. fn swap_tokens_for_exact_tokens( path: Vec, credit_in: Self::Credit, From ec2cf1d5085bcba7f9b6bf828eaae63551c2c6a1 Mon Sep 17 00:00:00 2001 From: muharem Date: Wed, 18 Oct 2023 17:21:53 +0200 Subject: [PATCH 38/43] impl on_nonzero_unbalanced instead on_unbalanced --- .../traits/tokens/imbalance/on_unbalanced.rs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/substrate/frame/support/src/traits/tokens/imbalance/on_unbalanced.rs b/substrate/frame/support/src/traits/tokens/imbalance/on_unbalanced.rs index 27bfe46e181e..4be6cec346b8 100644 --- a/substrate/frame/support/src/traits/tokens/imbalance/on_unbalanced.rs +++ b/substrate/frame/support/src/traits/tokens/imbalance/on_unbalanced.rs @@ -53,3 +53,29 @@ pub trait OnUnbalanced { } impl OnUnbalanced for () {} + +/// Resolves received asset credit to account `A`, implementing [`OnUnbalanced`]. +/// +/// Credits that cannot be resolved to account `A` are dropped. This may occur if the account for +/// address `A` does not exist and the existential deposit requirement is not met. +pub struct ResolveTo(PhantomData<(A, F)>); +impl> OnUnbalanced> + for ResolveTo +{ + fn on_nonzero_unbalanced(credit: fungible::Credit) { + let _ = F::resolve(&A::get(), credit).map_err(|c| drop(c)); + } +} + +/// Resolves received asset credit to account `A`, implementing [`OnUnbalanced`]. +/// +/// Credits that cannot be resolved to account `A` are dropped. This may occur if the account for +/// address `A` does not exist and the existential deposit requirement is not met. +pub struct ResolveAssetTo(PhantomData<(A, F)>); +impl> OnUnbalanced> + for ResolveAssetTo +{ + fn on_nonzero_unbalanced(credit: fungibles::Credit) { + let _ = F::resolve(&A::get(), credit).map_err(|c| drop(c)); + } +} From 2b13f27e951c6fb76326a467a4afa8e369cb8359 Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 24 Oct 2023 12:34:15 +0200 Subject: [PATCH 39/43] adjustments for rococo setup --- .../runtimes/assets/asset-hub-rococo/src/lib.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index 20b59368a5b5..b8487f2e8f9e 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -338,7 +338,6 @@ impl pallet_asset_conversion::Config for Runtime { type Balance = Balance; type HigherPrecisionBalance = sp_core::U256; type Currency = Balances; - type AssetBalance = Balance; type AssetId = MultiLocation; type Assets = LocalAndForeignAssets< Assets, @@ -355,7 +354,7 @@ impl pallet_asset_conversion::Config for Runtime { type PalletId = AssetConversionPalletId; type AllowMultiAssetPools = AllowMultiAssetPools; type MaxSwapPathLength = ConstU32<4>; - type MultiAssetId = Box; + type MultiAssetId = MultiLocation; type MultiAssetIdConverter = MultiLocationConverter; type MintMinLiquidity = ConstU128<100>; @@ -1111,17 +1110,18 @@ impl_runtime_apis! { impl pallet_asset_conversion::AssetConversionApi< Block, Balance, - u128, - Box, + MultiLocation, > for Runtime { - fn quote_price_exact_tokens_for_tokens(asset1: Box, asset2: Box, amount: u128, include_fee: bool) -> Option { + fn quote_price_exact_tokens_for_tokens(asset1: MultiLocation, asset2: MultiLocation, amount: Balance, include_fee: bool) -> Option { AssetConversion::quote_price_exact_tokens_for_tokens(asset1, asset2, amount, include_fee) } - fn quote_price_tokens_for_exact_tokens(asset1: Box, asset2: Box, amount: u128, include_fee: bool) -> Option { + + fn quote_price_tokens_for_exact_tokens(asset1: MultiLocation, asset2: MultiLocation, amount: Balance, include_fee: bool) -> Option { AssetConversion::quote_price_tokens_for_exact_tokens(asset1, asset2, amount, include_fee) } - fn get_reserves(asset1: Box, asset2: Box) -> Option<(Balance, Balance)> { + + fn get_reserves(asset1: MultiLocation, asset2: MultiLocation) -> Option<(Balance, Balance)> { AssetConversion::get_reserves(&asset1, &asset2).ok() } } From bbca52d63f72fccbe6d5e72ef7c4a0b5a092128f Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 24 Oct 2023 13:31:48 +0200 Subject: [PATCH 40/43] adjustments for rococo tests --- .../assets/asset-hub-rococo/src/tests/swap.rs | 55 +++++++++---------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/swap.rs b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/swap.rs index f9da0bf946ed..b16667eaeaec 100644 --- a/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/swap.rs +++ b/cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo/src/tests/swap.rs @@ -14,17 +14,17 @@ // limitations under the License. use crate::*; -use frame_support::{instances::Instance2, BoundedVec}; +use frame_support::instances::Instance2; use parachains_common::rococo::currency::EXISTENTIAL_DEPOSIT; use sp_runtime::{DispatchError, ModuleError}; #[test] fn swap_locally_on_chain_using_local_assets() { - let asset_native = Box::new(asset_hub_rococo_runtime::xcm_config::TokenLocation::get()); - let asset_one = Box::new(MultiLocation { + let asset_native = asset_hub_rococo_runtime::xcm_config::TokenLocation::get(); + let asset_one = MultiLocation { parents: 0, interior: X2(PalletInstance(ASSETS_PALLET_ID), GeneralIndex(ASSET_ID.into())), - }); + }; AssetHubRococo::execute_with(|| { type RuntimeEvent = ::RuntimeEvent; @@ -52,8 +52,8 @@ fn swap_locally_on_chain_using_local_assets() { assert_ok!(::AssetConversion::create_pool( ::RuntimeOrigin::signed(AssetHubRococoSender::get()), - asset_native.clone(), - asset_one.clone(), + Box::new(asset_native), + Box::new(asset_one), )); assert_expected_events!( @@ -65,8 +65,8 @@ fn swap_locally_on_chain_using_local_assets() { assert_ok!(::AssetConversion::add_liquidity( ::RuntimeOrigin::signed(AssetHubRococoSender::get()), - asset_native.clone(), - asset_one.clone(), + Box::new(asset_native), + Box::new(asset_one), 1_000_000_000_000, 2_000_000_000_000, 0, @@ -81,7 +81,7 @@ fn swap_locally_on_chain_using_local_assets() { ] ); - let path = BoundedVec::<_, _>::truncate_from(vec![asset_native.clone(), asset_one.clone()]); + let path = vec![Box::new(asset_native), Box::new(asset_one)]; assert_ok!( ::AssetConversion::swap_exact_tokens_for_tokens( @@ -106,8 +106,8 @@ fn swap_locally_on_chain_using_local_assets() { assert_ok!(::AssetConversion::remove_liquidity( ::RuntimeOrigin::signed(AssetHubRococoSender::get()), - asset_native, - asset_one, + Box::new(asset_native), + Box::new(asset_one), 1414213562273 - EXISTENTIAL_DEPOSIT * 2, // all but the 2 EDs can't be retrieved. 0, 0, @@ -120,16 +120,16 @@ fn swap_locally_on_chain_using_local_assets() { fn swap_locally_on_chain_using_foreign_assets() { use frame_support::weights::WeightToFee; - let asset_native = Box::new(asset_hub_rococo_runtime::xcm_config::TokenLocation::get()); + let asset_native = asset_hub_rococo_runtime::xcm_config::TokenLocation::get(); - let foreign_asset1_at_asset_hub_rococo = Box::new(MultiLocation { + let foreign_asset1_at_asset_hub_rococo = MultiLocation { parents: 1, interior: X3( Parachain(PenpalRococoA::para_id().into()), PalletInstance(ASSETS_PALLET_ID), GeneralIndex(ASSET_ID.into()), ), - }); + }; let assets_para_destination: VersionedMultiLocation = MultiLocation { parents: 1, interior: X1(Parachain(AssetHubRococo::para_id().into())) } @@ -175,7 +175,7 @@ fn swap_locally_on_chain_using_foreign_assets() { ::Runtime, Instance2, >::create { - id: *foreign_asset1_at_asset_hub_rococo, + id: foreign_asset1_at_asset_hub_rococo, min_balance: 1000, admin: sov_penpal_on_asset_hub_rococo.clone().into(), }) @@ -223,7 +223,7 @@ fn swap_locally_on_chain_using_foreign_assets() { // Receive XCM message in Assets Parachain AssetHubRococo::execute_with(|| { assert!(::ForeignAssets::asset_exists( - *foreign_asset1_at_asset_hub_rococo + foreign_asset1_at_asset_hub_rococo )); // 3: Mint foreign asset on asset_hub_rococo: @@ -237,7 +237,7 @@ fn swap_locally_on_chain_using_foreign_assets() { ::RuntimeOrigin::signed( sov_penpal_on_asset_hub_rococo.clone().into() ), - *foreign_asset1_at_asset_hub_rococo, + foreign_asset1_at_asset_hub_rococo, sov_penpal_on_asset_hub_rococo.clone().into(), 3_000_000_000_000, )); @@ -252,8 +252,8 @@ fn swap_locally_on_chain_using_foreign_assets() { // 4. Create pool: assert_ok!(::AssetConversion::create_pool( ::RuntimeOrigin::signed(AssetHubRococoSender::get()), - asset_native.clone(), - foreign_asset1_at_asset_hub_rococo.clone(), + Box::new(asset_native), + Box::new(foreign_asset1_at_asset_hub_rococo), )); assert_expected_events!( @@ -268,8 +268,8 @@ fn swap_locally_on_chain_using_foreign_assets() { ::RuntimeOrigin::signed( sov_penpal_on_asset_hub_rococo.clone() ), - asset_native.clone(), - foreign_asset1_at_asset_hub_rococo.clone(), + Box::new(asset_native), + Box::new(foreign_asset1_at_asset_hub_rococo), 1_000_000_000_000, 2_000_000_000_000, 0, @@ -287,10 +287,7 @@ fn swap_locally_on_chain_using_foreign_assets() { ); // 6. Swap! - let path = BoundedVec::<_, _>::truncate_from(vec![ - asset_native.clone(), - foreign_asset1_at_asset_hub_rococo.clone(), - ]); + let path = vec![Box::new(asset_native), Box::new(foreign_asset1_at_asset_hub_rococo)]; assert_ok!( ::AssetConversion::swap_exact_tokens_for_tokens( @@ -318,8 +315,8 @@ fn swap_locally_on_chain_using_foreign_assets() { ::RuntimeOrigin::signed( sov_penpal_on_asset_hub_rococo.clone() ), - asset_native, - foreign_asset1_at_asset_hub_rococo, + Box::new(asset_native), + Box::new(foreign_asset1_at_asset_hub_rococo), 1414213562273 - 2_000_000_000, // all but the 2 EDs can't be retrieved. 0, 0, @@ -330,7 +327,7 @@ fn swap_locally_on_chain_using_foreign_assets() { #[test] fn cannot_create_pool_from_pool_assets() { - let asset_native = Box::new(asset_hub_rococo_runtime::xcm_config::TokenLocation::get()); + let asset_native = asset_hub_rococo_runtime::xcm_config::TokenLocation::get(); let mut asset_one = asset_hub_rococo_runtime::xcm_config::PoolAssetsPalletLocation::get(); asset_one.append_with(GeneralIndex(ASSET_ID.into())).expect("pool assets"); @@ -355,7 +352,7 @@ fn cannot_create_pool_from_pool_assets() { assert_matches::assert_matches!( ::AssetConversion::create_pool( ::RuntimeOrigin::signed(AssetHubRococoSender::get()), - asset_native.clone(), + Box::new(asset_native), Box::new(asset_one), ), Err(DispatchError::Module(ModuleError{index: _, error: _, message})) => assert_eq!(message, Some("UnsupportedAsset")) From 66a66110b7bf80aaa9de2d0b5701040f68caf6d6 Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 24 Oct 2023 15:13:23 +0200 Subject: [PATCH 41/43] rococo benchmarks --- .../runtimes/assets/asset-hub-rococo/src/xcm_config.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs index ae4a275d43ac..b88de9efb09c 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/xcm_config.rs @@ -724,8 +724,7 @@ pub struct BenchmarkMultiLocationConverter { } #[cfg(feature = "runtime-benchmarks")] -impl - pallet_asset_conversion::BenchmarkHelper> +impl pallet_asset_conversion::BenchmarkHelper for BenchmarkMultiLocationConverter where SelfParaId: Get, @@ -740,8 +739,8 @@ where ), } } - fn multiasset_id(asset_id: u32) -> sp_std::boxed::Box { - sp_std::boxed::Box::new(Self::asset_id(asset_id)) + fn multiasset_id(asset_id: u32) -> MultiLocation { + Self::asset_id(asset_id) } } From f7b7a2c8b6e7c25ed2d622fb6bd38a6bcb9171b7 Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 19 Dec 2023 15:40:25 +0100 Subject: [PATCH 42/43] prdoc --- prdoc/pr_1677.prdoc | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 prdoc/pr_1677.prdoc diff --git a/prdoc/pr_1677.prdoc b/prdoc/pr_1677.prdoc new file mode 100644 index 000000000000..9c5bee386ae3 --- /dev/null +++ b/prdoc/pr_1677.prdoc @@ -0,0 +1,22 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: "pallet-asset-conversion: Swap Credit" + +doc: + - audience: Runtime Dev + description: | + Introduces a swap implementation that allows the exchange of a credit (aka Negative Imbalance) of one asset for a credit of another asset. + + This is particularly useful when a credit swap is required but may not have sufficient value to meet the ED constraint, hence cannot be deposited to temp account before. An example use case is when XCM fees are paid using an asset held in the XCM executor registry and has to be swapped for native currency. + + Additional Updates: + - encapsulates the existing `Swap` trait impl within a transactional context, since partial storage mutation is possible when an error occurs; + - supplied `Currency` and `Assets` impls must be implemented over the same `Balance` type, the `AssetBalance` generic type is dropped. This helps to avoid numerous type conversion and overflow cases. If those types are different it should be handled outside of the pallet; + - `Box` asset kind on a pallet level, unbox on a runtime level - here [why](https://substrate.stackexchange.com/questions/10039/boxed-argument-of-a-dispatchable/10103#10103); + - `path` uses `Vec` now, instead of `BoundedVec` since it is never used in PoV; + - removes the `Transfer` event due to it's redundancy with the events emitted by `fungible/s` implementations; + - modifies the `SwapExecuted` event type; + +crates: [ ] + From f24b49e49336f12c847a23a935fe6ff761022ea2 Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 19 Dec 2023 15:41:43 +0100 Subject: [PATCH 43/43] fix --- .../emulated/tests/assets/asset-hub-westend/src/tests/swap.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/swap.rs b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/swap.rs index 55959a48d13e..46a9b4252e13 100644 --- a/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/swap.rs +++ b/cumulus/parachains/integration-tests/emulated/tests/assets/asset-hub-westend/src/tests/swap.rs @@ -160,7 +160,6 @@ fn swap_locally_on_chain_using_foreign_assets() { ] ); - let foreign_asset_at_asset_hub_westend = foreign_asset_at_asset_hub_westend; // 4. Create pool: assert_ok!(::AssetConversion::create_pool( ::RuntimeOrigin::signed(AssetHubWestendSender::get()),