From 222aec2a2af7212fe5a7a635964cad3ab4f7b9f9 Mon Sep 17 00:00:00 2001 From: Patricio Napoli Date: Fri, 4 Aug 2023 17:17:35 -0300 Subject: [PATCH 1/7] initial tentative changes --- frame/asset-conversion/src/lib.rs | 67 +++++++++++++++++++++++++++++ frame/asset-conversion/src/types.rs | 7 +++ 2 files changed, 74 insertions(+) diff --git a/frame/asset-conversion/src/lib.rs b/frame/asset-conversion/src/lib.rs index f9aeeace11fe7..61216cabc61b8 100644 --- a/frame/asset-conversion/src/lib.rs +++ b/frame/asset-conversion/src/lib.rs @@ -72,6 +72,7 @@ use frame_support::{ ensure, traits::tokens::{AssetId, Balance}, }; +use frame_support::traits::fungibles::Credit; use frame_system::{ ensure_signed, pallet_prelude::{BlockNumberFor, OriginFor}, @@ -218,6 +219,9 @@ pub mod pallet { #[pallet::storage] pub type NextPoolAssetId = StorageValue<_, T::PoolAssetId, OptionQuery>; + /// Credit (negative imbalance) of an asset handled by this pallet. + pub type AssetCredit = Credit::AssetBalance>; + // Pallet's events. #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] @@ -288,6 +292,18 @@ pub mod pallet { /// The amount of the second asset that was received. amount_out: T::AssetBalance, }, + /// Assets have been converted from one to another using credits. + /// Both `SwapExactTokenForTokenCredit and `SwapTokenForExactTokenCredit` + /// will generate this event. + SwapCreditExecuted { + /// 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, + }, /// An amount has been transferred from one account to another. Transfer { /// The account that the assets were transferred from. @@ -741,6 +757,35 @@ pub mod pallet { Ok(amount_out) } + pub fn do_swap_exact_tokens_for_tokens_credit( + path: BoundedVec, + amount_in: AssetCredit, + amount_out_min: Option, + ) -> Result, DispatchError> { + ensure!(amount_in > Zero::zero(), Error::::ZeroAmount); + if let Some(amount_out_min) = amount_out_min { + ensure!(amount_out_min > Zero::zero(), Error::::ZeroAmount); + } + + Self::validate_swap_path(&path)?; + + let amounts = Self::get_amounts_out(&amount_in, &path)?; + let amount_out = + *amounts.last().defensive_ok_or("get_amounts_out() returned an empty result")?; + + let credit_amounts = amounts.iter().map(|x| x.into()).collect::>>(); + + if let Some(amount_out_min) = amount_out_min { + ensure!( + amount_out >= amount_out_min, + Error::::ProvidedMinimumNotSufficientForSwap + ); + } + + Self::do_swap_credit(&credit_amounts, path)?; + Ok(amount_out.into()) + } + /// 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. @@ -902,6 +947,13 @@ pub mod pallet { Ok(()) } + pub(crate) fn do_swap_credit( + amounts: &Vec>, + path: BoundedVec, + ) -> Result<(), DispatchError> { + Ok(()) + } + /// The account ID of the pool. /// /// This actually does computation. If you need to keep using it, then make sure you cache @@ -1256,6 +1308,21 @@ impl Swap f Ok(amount_out.into()) } + fn swap_exact_tokens_for_tokens_credit( + path: Vec, + amount_in: AssetCredit, + amount_out_min: Option, + ) -> Result, DispatchError> { + 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_credit( + path, + amount_in?, + amount_out_min, + )?; + Ok(amount_out.into()) + } + fn swap_tokens_for_exact_tokens( sender: T::AccountId, path: Vec, diff --git a/frame/asset-conversion/src/types.rs b/frame/asset-conversion/src/types.rs index 7cd9743ff04b8..ac501d1117648 100644 --- a/frame/asset-conversion/src/types.rs +++ b/frame/asset-conversion/src/types.rs @@ -19,6 +19,7 @@ use super::*; use codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; +use frame_support::traits::fungibles::Credit; use sp_std::{cmp::Ordering, marker::PhantomData}; pub(super) type PoolIdOf = (::MultiAssetId, ::MultiAssetId); @@ -98,6 +99,12 @@ pub trait Swap { keep_alive: bool, ) -> Result; + fn swap_exact_tokens_for_tokens_credit( + path: Vec, + amount_in: Credit, + amount_out_min: Option, + ) -> Result, DispatchError>; + /// Take the `path[0]` asset and swap some amount for `amount_out` of the `path[1]`. If an /// `amount_in_max` is specified, it will return an error if acquiring `amount_out` would be /// too costly. From aa00fc718db1fdf3f063112ca1e1c6d023cf9984 Mon Sep 17 00:00:00 2001 From: Patricio Napoli Date: Wed, 9 Aug 2023 02:22:04 -0300 Subject: [PATCH 2/7] more changes, changed to swap_exact_for_tokens to prevent overswapping --- frame/asset-conversion/src/lib.rs | 136 ++++++++++++++++++++-------- frame/asset-conversion/src/types.rs | 35 ++++++- 2 files changed, 127 insertions(+), 44 deletions(-) diff --git a/frame/asset-conversion/src/lib.rs b/frame/asset-conversion/src/lib.rs index 61216cabc61b8..0b8e11e2b2401 100644 --- a/frame/asset-conversion/src/lib.rs +++ b/frame/asset-conversion/src/lib.rs @@ -70,9 +70,11 @@ mod mock; use codec::Codec; use frame_support::{ ensure, - traits::tokens::{AssetId, Balance}, + traits::{ + fungibles::{Balanced, Credit}, + tokens::{AssetId, Balance}, + }, }; -use frame_support::traits::fungibles::Credit; use frame_system::{ ensure_signed, pallet_prelude::{BlockNumberFor, OriginFor}, @@ -219,9 +221,6 @@ pub mod pallet { #[pallet::storage] pub type NextPoolAssetId = StorageValue<_, T::PoolAssetId, OptionQuery>; - /// Credit (negative imbalance) of an asset handled by this pallet. - pub type AssetCredit = Credit::AssetBalance>; - // Pallet's events. #[pallet::event] #[pallet::generate_deposit(pub(super) fn deposit_event)] @@ -757,33 +756,25 @@ pub mod pallet { Ok(amount_out) } - pub fn do_swap_exact_tokens_for_tokens_credit( + pub fn do_swap_tokens_for_exact_tokens_credit( path: BoundedVec, - amount_in: AssetCredit, - amount_out_min: Option, - ) -> Result, DispatchError> { - ensure!(amount_in > Zero::zero(), Error::::ZeroAmount); - if let Some(amount_out_min) = amount_out_min { - ensure!(amount_out_min > Zero::zero(), Error::::ZeroAmount); - } + amount_in_max: T::AssetBalance, + amount_out: T::AssetBalance, + ) -> Result, DispatchError> { + ensure!(amount_out > Zero::zero(), Error::::ZeroAmount); + ensure!(amount_in_max > Zero::zero(), Error::::ZeroAmount); Self::validate_swap_path(&path)?; - let amounts = Self::get_amounts_out(&amount_in, &path)?; - let amount_out = - *amounts.last().defensive_ok_or("get_amounts_out() returned an empty result")?; + 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 credit_amounts = amounts.iter().map(|x| x.into()).collect::>>(); + ensure!(amount_in <= amount_in_max, Error::::ProvidedMaximumNotSufficientForSwap); - if let Some(amount_out_min) = amount_out_min { - ensure!( - amount_out >= amount_out_min, - Error::::ProvidedMinimumNotSufficientForSwap - ); - } - - Self::do_swap_credit(&credit_amounts, path)?; - Ok(amount_out.into()) + let out_credit = Self::do_swap_credit(&amounts, path)?; + let in_credit = (path[0].clone(), amount_in_max.saturating_sub(amount_in)?).into(); + Ok((in_credit, out_credit).into()) } /// Take the `path[0]` asset and swap some amount for `amount_out` of the `path[1]`. If an @@ -863,6 +854,33 @@ pub mod pallet { result } + /// Credit an `amount` of `asset_id` into `to` account. + fn credit( + asset_id: &T::MultiAssetId, + to: &T::AccountId, + amount: T::AssetBalance, + ) -> Result<(), DispatchError> { + let result = match T::MultiAssetIdConverter::try_convert(asset_id) { + MultiAssetIdConversionResult::Converted(asset_id) => { + let credit: Credit = (asset_id, amount).into(); + >::resolve(&to, credit)?; + + Ok(()) + }, + MultiAssetIdConversionResult::Native => { + let amount = Self::convert_asset_balance_to_native_balance(amount)?; + let credit: Credit = (asset_id, amount).into(); + >::resolve(&to, credit)?; + + Ok(()) + }, + MultiAssetIdConversionResult::Unsupported(_) => + Err(Error::::UnsupportedAsset.into()), + }; + + result + } + /// Convert a `Balance` type to an `AssetBalance`. pub(crate) fn convert_native_balance_to_asset_balance( amount: T::Balance, @@ -948,10 +966,48 @@ pub mod pallet { } pub(crate) fn do_swap_credit( - amounts: &Vec>, + amounts: &Vec, path: BoundedVec, - ) -> Result<(), DispatchError> { - Ok(()) + ) -> 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)?; + + // Pool account needs enough balance in asset 1 for the swap, but we don't want to + // actually transfer the funds, since that would require a sender account, so we + // instead imply an imbalance in the total issuance, which will be settled later. + Self::credit(asset1, &pool_account, *first_amount)?; + + 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 reserve = Self::get_balance(&pool_account, asset2)?; + let reserve_left = reserve.saturating_sub(*amount_out); + Self::validate_minimal_amount(reserve_left, asset2) + .map_err(|_| Error::::ReserveLeftLessThanMinimal)?; + + // TODO: + // Same as credit but use settle instead of resolve, and get a Credit<> out + Self::credit(asset2, &pool_account, *amount_out)?; + } + i.saturating_inc(); + } + // TODO: Emit a newly created event called SwapCreditExecuted + } else { + return Err(Error::::InvalidPath.into()) + } + // TODO: Return the Credit outstanding from the Settle + Ok((1, 2).into()) } /// The account ID of the pool. @@ -1246,7 +1302,9 @@ pub mod pallet { () ); } else { - let MultiAssetIdConversionResult::Converted(asset_id) = T::MultiAssetIdConverter::try_convert(asset) else { + let MultiAssetIdConversionResult::Converted(asset_id) = + T::MultiAssetIdConverter::try_convert(asset) + else { return Err(()) }; let minimal = T::Assets::minimum_balance(asset_id); @@ -1308,19 +1366,19 @@ impl Swap f Ok(amount_out.into()) } - fn swap_exact_tokens_for_tokens_credit( + fn swap_tokens_for_exact_tokens_credit( path: Vec, - amount_in: AssetCredit, - amount_out_min: Option, - ) -> Result, DispatchError> { + amount_in_max: T::HigherPrecisionBalance, + amount_out: T::HigherPrecisionBalance, + ) -> Result, DispatchError> { 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_credit( + let amount_in_max = Self::convert_hpb_to_asset_balance(amount_in_max)?; + let amount_in = Self::do_swap_tokens_for_exact_tokens_credit( path, - amount_in?, - amount_out_min, + amount_in_max, + Self::convert_hpb_to_asset_balance(amount_out)?, )?; - Ok(amount_out.into()) + Ok(amount_in.into()) } fn swap_tokens_for_exact_tokens( diff --git a/frame/asset-conversion/src/types.rs b/frame/asset-conversion/src/types.rs index ac501d1117648..51f2dc53b07c8 100644 --- a/frame/asset-conversion/src/types.rs +++ b/frame/asset-conversion/src/types.rs @@ -18,8 +18,8 @@ use super::*; use codec::{Decode, Encode, MaxEncodedLen}; -use scale_info::TypeInfo; use frame_support::traits::fungibles::Credit; +use scale_info::TypeInfo; use sp_std::{cmp::Ordering, marker::PhantomData}; pub(super) type PoolIdOf = (::MultiAssetId, ::MultiAssetId); @@ -80,6 +80,12 @@ where } } +/// Representation of the final credit imbalance after a swap. +pub struct SwapCredits( + Credit, // In asset + Credit, // Out asset +); + /// 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]`. @@ -99,11 +105,30 @@ pub trait Swap { keep_alive: bool, ) -> Result; - fn swap_exact_tokens_for_tokens_credit( + /// Swap `amount_in_max` of asset `path[0]` for asset `path[1]` declared in `amount_out`. + /// It will return an error if acquiring `amount_out` would be too costly. + /// + /// Thus it is on the RPC side to ensure that `amount_in_max` is enough to acquire `amount_out`. + /// + /// This method implies that the amount_in is an imbalance of the `path[0]` asset. + /// + /// Uses the `amount_in_max` imbalance to offset into the pool account. + /// + /// If successful, returns the amount of `path[1]` acquired for the `amount_in_max` + /// along with the `amount_in_max` as an imbalance. + /// They could be credited somewhere as the type implies, but can also be dropped. + /// + /// Note: This method effectively prevents overswapping, so that the + /// returned Credit.0 can then be directly refunded in the initial asset + /// without swapping back from the `path[1]` asset, saving us a bit of gas. + /// + /// `amount_in_max` is not optional due to the fact that it is a balance to be offset + /// (credited to the pool), and not an amount to be acquired from a sender. + fn swap_tokens_for_exact_tokens_credit( path: Vec, - amount_in: Credit, - amount_out_min: Option, - ) -> Result, DispatchError>; + amount_in_max: Balance, // Is there a benefit of changing this to Credit? + amount_out: Balance, + ) -> Result, DispatchError>; /// Take the `path[0]` asset and swap some amount for `amount_out` of the `path[1]`. If an /// `amount_in_max` is specified, it will return an error if acquiring `amount_out` would be From 2fb12f5a48dbe19e9c36fb7aadf2629f19d61113 Mon Sep 17 00:00:00 2001 From: Patricio Napoli Date: Sat, 12 Aug 2023 18:14:56 -0300 Subject: [PATCH 3/7] few fixes based on feedback, there still are some compile issues betwen T::HPB and fungibles::Balanced --- frame/asset-conversion/src/lib.rs | 125 ++++++++++++++++++---------- frame/asset-conversion/src/types.rs | 34 ++++---- 2 files changed, 99 insertions(+), 60 deletions(-) diff --git a/frame/asset-conversion/src/lib.rs b/frame/asset-conversion/src/lib.rs index 0b8e11e2b2401..6f9a247d3dc4c 100644 --- a/frame/asset-conversion/src/lib.rs +++ b/frame/asset-conversion/src/lib.rs @@ -71,7 +71,7 @@ use codec::Codec; use frame_support::{ ensure, traits::{ - fungibles::{Balanced, Credit}, + fungibles::{Balanced, Credit, Debt}, tokens::{AssetId, Balance}, }, }; @@ -104,7 +104,7 @@ pub mod pallet { Precision::Exact, Preservation::{Expendable, Preserve}, }, - AccountTouch, ContainsPair, + AccountTouch, ContainsPair, SameOrOther, }, BoundedBTreeSet, PalletId, }; @@ -349,6 +349,8 @@ pub mod pallet { AssetOneWithdrawalDidNotMeetMinimum, /// The minimal amount requirement for the second token in the pair wasn't met. AssetTwoWithdrawalDidNotMeetMinimum, + /// The asset for swap and the originating credit asset id do not match. + AssetCreditMismatch, /// Optimal calculated amount is less than desired. OptimalAmountLessThanDesired, /// Insufficient liquidity minted. @@ -758,11 +760,16 @@ pub mod pallet { pub fn do_swap_tokens_for_exact_tokens_credit( path: BoundedVec, - amount_in_max: T::AssetBalance, amount_out: T::AssetBalance, - ) -> Result, DispatchError> { + credit_in: Credit, + ) -> Result, DispatchError> + where + T::AssetBalance: Balanced, + (T::MultiAssetId, T::AssetBalance): Into> + + Into>, + { ensure!(amount_out > Zero::zero(), Error::::ZeroAmount); - ensure!(amount_in_max > Zero::zero(), Error::::ZeroAmount); + ensure!(credit_in.peek() > Zero::zero(), Error::::ZeroAmount); Self::validate_swap_path(&path)?; @@ -770,11 +777,20 @@ pub mod pallet { let amount_in = *amounts.first().defensive_ok_or("get_amounts_in() returned an empty result")?; - ensure!(amount_in <= amount_in_max, Error::::ProvidedMaximumNotSufficientForSwap); + ensure!(amount_in <= credit_in.peek(), Error::::ProvidedMaximumNotSufficientForSwap); let out_credit = Self::do_swap_credit(&amounts, path)?; - let in_credit = (path[0].clone(), amount_in_max.saturating_sub(amount_in)?).into(); - Ok((in_credit, out_credit).into()) + + let same_or_other = credit_in + .offset((path[0].clone(), amount_in).into()) + .map_err(|_| Error::::AssetCreditMismatch)?; + + let in_leftover = match same_or_other { + SameOrOther::Same(credit) => credit, + _ => unreachable!("credit_in.peek() <= amount_in"), + }; + + Ok(CreditPair { in_leftover, out_credit }) } /// Take the `path[0]` asset and swap some amount for `amount_out` of the `path[1]`. If an @@ -854,31 +870,38 @@ pub mod pallet { result } - /// Credit an `amount` of `asset_id` into `to` account. - fn credit( - asset_id: &T::MultiAssetId, + /// Credit an `amount` of `asset_id` in `to` account. + fn credit_resolve( + asset_id: T::MultiAssetId, to: &T::AccountId, amount: T::AssetBalance, - ) -> Result<(), DispatchError> { - let result = match T::MultiAssetIdConverter::try_convert(asset_id) { - MultiAssetIdConversionResult::Converted(asset_id) => { - let credit: Credit = (asset_id, amount).into(); - >::resolve(&to, credit)?; + ) -> Result<(), DispatchError> + where + T::AssetBalance: Balanced, + (T::MultiAssetId, T::AssetBalance): Into>, + { + let credit: Credit = (asset_id, amount).into(); - Ok(()) - }, - MultiAssetIdConversionResult::Native => { - let amount = Self::convert_asset_balance_to_native_balance(amount)?; - let credit: Credit = (asset_id, amount).into(); - >::resolve(&to, credit)?; + // Resolve is swallowing the inner Self::deposit DispatchError.. + T::AssetBalance::resolve(&to, credit).map_err(|_| Error::::Overflow)?; - Ok(()) - }, - MultiAssetIdConversionResult::Unsupported(_) => - Err(Error::::UnsupportedAsset.into()), - }; + Ok(()) + } - result + /// Remove an `amount` of `asset_id` `from` account and return it as a credit imbalance. + fn credit_settle( + asset_id: T::MultiAssetId, + from: &T::AccountId, + amount: T::AssetBalance, + ) -> Result, DispatchError> + where + T::AssetBalance: Balanced, + (T::MultiAssetId, T::AssetBalance): Into>, + { + let debt: Debt = (asset_id, amount).into(); + + // Settle is swallowing the inner Self::deposit DispatchError.. + Ok(T::AssetBalance::settle(&from, debt, Preserve).map_err(|_| Error::::Overflow)?) } /// Convert a `Balance` type to an `AssetBalance`. @@ -968,9 +991,15 @@ pub mod pallet { pub(crate) fn do_swap_credit( amounts: &Vec, path: BoundedVec, - ) -> Result, DispatchError> { + ) -> Result, DispatchError> + where + T::AssetBalance: Balanced, + (T::MultiAssetId, T::AssetBalance): Into> + + Into>, + { ensure!(amounts.len() > 1, Error::::CorrespondenceError); - if let Some([asset1, asset2]) = &path.get(0..2) { + + return 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. @@ -978,8 +1007,8 @@ pub mod pallet { // Pool account needs enough balance in asset 1 for the swap, but we don't want to // actually transfer the funds, since that would require a sender account, so we - // instead imply an imbalance in the total issuance, which will be settled later. - Self::credit(asset1, &pool_account, *first_amount)?; + // instead resolve the credit into the pool_account. + Self::credit_resolve(asset1.clone(), &pool_account, *first_amount)?; let mut i = 0; let path_len = path.len() as u32; @@ -996,18 +1025,18 @@ pub mod pallet { Self::validate_minimal_amount(reserve_left, asset2) .map_err(|_| Error::::ReserveLeftLessThanMinimal)?; - // TODO: // Same as credit but use settle instead of resolve, and get a Credit<> out - Self::credit(asset2, &pool_account, *amount_out)?; + // TODO: This will only work for path vectors of size 2 (no hops). + // TODO: Emit a newly created event called SwapCreditExecuted + return Ok(Self::credit_settle(asset2.clone(), &pool_account, *amount_out)?) } i.saturating_inc(); } - // TODO: Emit a newly created event called SwapCreditExecuted + + Err(Error::::InvalidPath.into()) } else { - return Err(Error::::InvalidPath.into()) + Err(Error::::InvalidPath.into()) } - // TODO: Return the Credit outstanding from the Settle - Ok((1, 2).into()) } /// The account ID of the pool. @@ -1368,17 +1397,25 @@ impl Swap f fn swap_tokens_for_exact_tokens_credit( path: Vec, - amount_in_max: T::HigherPrecisionBalance, amount_out: T::HigherPrecisionBalance, - ) -> Result, DispatchError> { + credit_in: Credit, + ) -> Result, DispatchError> + where + T::HigherPrecisionBalance: Balanced, + T::AssetBalance: Balanced, + (T::MultiAssetId, T::AssetBalance): + Into> + Into>, + { let path = path.try_into().map_err(|_| Error::::PathError)?; - let amount_in_max = Self::convert_hpb_to_asset_balance(amount_in_max)?; - let amount_in = Self::do_swap_tokens_for_exact_tokens_credit( + let amount_in_ab = Self::convert_hpb_to_asset_balance(credit_in.peek())?; + let amount_in: Credit = (path[0], amount_in_ab).into(); + let out = Self::do_swap_tokens_for_exact_tokens_credit( path, - amount_in_max, Self::convert_hpb_to_asset_balance(amount_out)?, + amount_in, )?; - Ok(amount_in.into()) + // TODO: convert out credits to HPB + Ok(out.into()) } fn swap_tokens_for_exact_tokens( diff --git a/frame/asset-conversion/src/types.rs b/frame/asset-conversion/src/types.rs index 51f2dc53b07c8..1527d59bc208e 100644 --- a/frame/asset-conversion/src/types.rs +++ b/frame/asset-conversion/src/types.rs @@ -80,11 +80,11 @@ where } } -/// Representation of the final credit imbalance after a swap. -pub struct SwapCredits( - Credit, // In asset - Credit, // Out asset -); +/// Representation of the final credit imbalance after a swap for exact. +pub struct CreditPair> { + pub in_leftover: Credit, + pub out_credit: Credit, +} /// Trait for providing methods to swap between the various asset classes. pub trait Swap { @@ -108,27 +108,29 @@ pub trait Swap { /// Swap `amount_in_max` of asset `path[0]` for asset `path[1]` declared in `amount_out`. /// It will return an error if acquiring `amount_out` would be too costly. /// - /// Thus it is on the RPC side to ensure that `amount_in_max` is enough to acquire `amount_out`. - /// - /// This method implies that the amount_in is an imbalance of the `path[0]` asset. + /// Thus it is on the RPC side to ensure that `amount_in` is enough to acquire `amount_out`. /// - /// Uses the `amount_in_max` imbalance to offset into the pool account. + /// Uses the `amount_in` imbalance to offset into the pool account. /// - /// If successful, returns the amount of `path[1]` acquired for the `amount_in_max` - /// along with the `amount_in_max` as an imbalance. + /// If successful, returns the amount of `path[1]` acquired for the `amount_in` + /// along with the leftovers from `amount_in` as an imbalance. /// They could be credited somewhere as the type implies, but can also be dropped. /// /// Note: This method effectively prevents overswapping, so that the - /// returned Credit.0 can then be directly refunded in the initial asset - /// without swapping back from the `path[1]` asset, saving us a bit of gas. + /// returned `CreditPair::in_leftover` can then be directly refunded in the initial asset + /// without swapping back from the `path[1]` asset. /// - /// `amount_in_max` is not optional due to the fact that it is a balance to be offset + /// `amount_in` is not optional due to the fact that it is a balance to be offset /// (credited to the pool), and not an amount to be acquired from a sender. + /// + /// If this function returns an error, no credit will be taken from amount_in, like a no-op. fn swap_tokens_for_exact_tokens_credit( path: Vec, - amount_in_max: Balance, // Is there a benefit of changing this to Credit? amount_out: Balance, - ) -> Result, DispatchError>; + credit_in: Credit, + ) -> Result, DispatchError> + where + Balance: Balanced; /// 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 From c097efb9cd8959ee9772c37cd12a58937f370bd2 Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 21 Aug 2023 12:16:21 +0200 Subject: [PATCH 4/7] draft --- frame/asset-conversion/src/lib.rs | 123 ++++++++++++++++++++++++++++ frame/asset-conversion/src/tests.rs | 6 +- frame/asset-conversion/src/types.rs | 14 ++++ 3 files changed, 142 insertions(+), 1 deletion(-) diff --git a/frame/asset-conversion/src/lib.rs b/frame/asset-conversion/src/lib.rs index 6f9a247d3dc4c..b3f74a0391875 100644 --- a/frame/asset-conversion/src/lib.rs +++ b/frame/asset-conversion/src/lib.rs @@ -793,6 +793,32 @@ pub mod pallet { Ok(CreditPair { in_leftover, out_credit }) } + pub fn do_swap_tokens_for_exact_tokens_credit2( + path: BoundedVec, + amount_out: T::AssetBalance, + credit_in: Credit, + ) -> Result< + (Credit, Credit), + DispatchError, + > { + ensure!(amount_out > Zero::zero(), Error::::ZeroAmount); + ensure!(credit_in.peek() > Zero::zero(), Error::::ZeroAmount); + + Self::validate_swap_path(&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")?; + + ensure!(amount_in <= credit_in.peek(), Error::::ProvidedMaximumNotSufficientForSwap); + + let (credit_in, credit_change) = credit_in.split(amount_in); + + let out_credit = Self::do_swap_credit2(credit_in, &amounts, path)?; + + Ok((credit_change, out_credit)) + } + /// 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. @@ -904,6 +930,26 @@ pub mod pallet { Ok(T::AssetBalance::settle(&from, debt, Preserve).map_err(|_| Error::::Overflow)?) } + /// TODO + fn credit_resolve( + asset_id: T::MultiAssetId, + to: &T::AccountId, + credit: Credit, + ) -> Result<(), DispatchError> { + // TODO similar to Self::transfer, use MultiAssetIdConverter to determine whether to use + // T::Assets or T:Currency + Ok(()) + } + + fn withdraw( + asset_id: T::MultiAssetId, + from: &T::AccountId, + amount: T::AssetBalance, + ) -> Result, DispatchError> { + // TODO similar to Self::transfer, use MultiAssetIdConverter to determine whether to use + // T::Assets or T:Currency + } + /// Convert a `Balance` type to an `AssetBalance`. pub(crate) fn convert_native_balance_to_asset_balance( amount: T::Balance, @@ -1039,6 +1085,66 @@ pub mod pallet { } } + pub(crate) fn do_swap_credit2( + credit_in: Credit, + amounts: &Vec, + path: BoundedVec, + ) -> 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)?; + + // TODO ensure credit_in.peak() == first_amount + // TODO ensure credit_asset_id == asset1 + Self::credit_resolve(asset1, &pool_account, credit_in)?; + + 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)?; + + // TODO copy past from original swap, please validate + let to = if i < path_len - 2 { + let asset3 = path.get((i + 2) as usize).ok_or(Error::::PathError)?; + Some(Self::get_pool_account(&Self::get_pool_id( + asset2.clone(), + asset3.clone(), + ))) + } else { + None + }; + + let reserve = Self::get_balance(&pool_account, asset2)?; + let reserve_left = reserve.saturating_sub(*amount_out); + Self::validate_minimal_amount(reserve_left, asset2) + .map_err(|_| Error::::ReserveLeftLessThanMinimal)?; + + if to.is_some() { + // TODO transfer as in original swap + // Self::transfer(asset2, &pool_account, &to, *amount_out, true)?; + } else { + // TODO let credit_out = Self::withdraw(asset2.clone(), &pool_account, + // *amount_out) + } + } + i.saturating_inc(); + } + + // TODO deposit event + } else { + Err(Error::::InvalidPath.into()) + } + } + /// The account ID of the pool. /// /// This actually does computation. If you need to keep using it, then make sure you cache @@ -1440,6 +1546,23 @@ impl Swap f } } +impl SwapCredit for Pallet { + // TODO fn swap_exact_tokens_for_tokens_credit() + + fn swap_tokens_for_exact_tokens_credit( + path: Vec, + amount_out: T::AssetBalance, + credit_in: Credit, + ) -> Result< + (Credit, Credit), + DispatchError, + > { + let path = path.try_into().map_err(|_| Error::::PathError)?; + let out = Self::do_swap_tokens_for_exact_tokens_credit2(path, amount_out, credit_in)?; + Ok(out) + } +} + 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/frame/asset-conversion/src/tests.rs b/frame/asset-conversion/src/tests.rs index 80faf5363b011..450a074ec3675 100644 --- a/frame/asset-conversion/src/tests.rs +++ b/frame/asset-conversion/src/tests.rs @@ -66,7 +66,11 @@ fn pool_assets() -> Vec { fn create_tokens(owner: u128, tokens: Vec>) { for token_id in tokens { - let MultiAssetIdConversionResult::Converted(asset_id) = NativeOrAssetIdConverter::try_convert(&token_id) else { unreachable!("invalid token") }; + 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)); } } diff --git a/frame/asset-conversion/src/types.rs b/frame/asset-conversion/src/types.rs index 1527d59bc208e..ebf4b2a595966 100644 --- a/frame/asset-conversion/src/types.rs +++ b/frame/asset-conversion/src/types.rs @@ -150,6 +150,20 @@ pub trait Swap { ) -> Result; } +pub trait SwapCredit { + fn swap_exact_tokens_for_tokens_credit( + path: Vec, + credit_in: Credit, + amount_out_min: Option, + ) -> Result, DispatchError>; + + fn swap_tokens_for_exact_tokens_credit( + path: Vec, + amount_out: Balance, + credit_in: Credit, + ) -> Result<(Credit, Credit), DispatchError>; +} + /// 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 725f1b70e187327891c197f803fee037dff5371c Mon Sep 17 00:00:00 2001 From: Patricio Napoli Date: Tue, 22 Aug 2023 02:02:19 -0300 Subject: [PATCH 5/7] development around new crediting - resolve/withdraw pending --- frame/asset-conversion/src/lib.rs | 315 +++++++++++++++------------- frame/asset-conversion/src/types.rs | 64 +++--- 2 files changed, 196 insertions(+), 183 deletions(-) diff --git a/frame/asset-conversion/src/lib.rs b/frame/asset-conversion/src/lib.rs index b3f74a0391875..0016ab5e4e7d8 100644 --- a/frame/asset-conversion/src/lib.rs +++ b/frame/asset-conversion/src/lib.rs @@ -762,45 +762,13 @@ pub mod pallet { path: BoundedVec, amount_out: T::AssetBalance, credit_in: Credit, - ) -> Result, DispatchError> - where - T::AssetBalance: Balanced, - (T::MultiAssetId, T::AssetBalance): Into> - + Into>, - { - ensure!(amount_out > Zero::zero(), Error::::ZeroAmount); - ensure!(credit_in.peek() > Zero::zero(), Error::::ZeroAmount); - - Self::validate_swap_path(&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")?; - - ensure!(amount_in <= credit_in.peek(), Error::::ProvidedMaximumNotSufficientForSwap); - - let out_credit = Self::do_swap_credit(&amounts, path)?; - - let same_or_other = credit_in - .offset((path[0].clone(), amount_in).into()) - .map_err(|_| Error::::AssetCreditMismatch)?; - - let in_leftover = match same_or_other { - SameOrOther::Same(credit) => credit, - _ => unreachable!("credit_in.peek() <= amount_in"), - }; - - Ok(CreditPair { in_leftover, out_credit }) - } - - pub fn do_swap_tokens_for_exact_tokens_credit2( - path: BoundedVec, - amount_out: T::AssetBalance, - credit_in: Credit, ) -> Result< (Credit, Credit), DispatchError, - > { + > + where + T::AssetBalance: Balanced, + { ensure!(amount_out > Zero::zero(), Error::::ZeroAmount); ensure!(credit_in.peek() > Zero::zero(), Error::::ZeroAmount); @@ -810,11 +778,11 @@ pub mod pallet { let amount_in = *amounts.first().defensive_ok_or("get_amounts_in() returned an empty result")?; - ensure!(amount_in <= credit_in.peek(), Error::::ProvidedMaximumNotSufficientForSwap); + ensure!(credit_in.peek() >= amount_in, Error::::ProvidedMaximumNotSufficientForSwap); let (credit_in, credit_change) = credit_in.split(amount_in); - let out_credit = Self::do_swap_credit2(credit_in, &amounts, path)?; + let out_credit = Self::do_swap_credit(credit_in, &amounts, path)?; Ok((credit_change, out_credit)) } @@ -896,56 +864,78 @@ pub mod pallet { result } - /// Credit an `amount` of `asset_id` in `to` account. + // /// Credit an `amount` of `asset_id` in `to` account. + // fn credit_resolve( + // asset_id: T::MultiAssetId, + // to: &T::AccountId, + // amount: T::AssetBalance, + // ) -> Result<(), DispatchError> + // where + // T::AssetBalance: Balanced, + // (T::MultiAssetId, T::AssetBalance): Into>, + // { + // let credit: Credit = (asset_id, amount).into(); + // + // // Resolve is swallowing the inner Self::deposit DispatchError.. + // T::AssetBalance::resolve(&to, credit).map_err(|_| Error::::Overflow)?; + // + // Ok(()) + // } + + // /// Remove an `amount` of `asset_id` `from` account and return it as a credit imbalance. + // fn credit_settle( + // asset_id: T::MultiAssetId, + // from: &T::AccountId, + // amount: T::AssetBalance, + // ) -> Result, DispatchError> + // where + // T::AssetBalance: Balanced, + // (T::MultiAssetId, T::AssetBalance): Into>, + // { + // let debt: Debt = (asset_id, amount).into(); + // + // // Settle is swallowing the inner Self::deposit DispatchError.. + // Ok(T::AssetBalance::settle(&from, debt, Preserve).map_err(|_| Error::::Overflow)?) + // } + + /// TODO fn credit_resolve( asset_id: T::MultiAssetId, to: &T::AccountId, - amount: T::AssetBalance, + credit: Credit, ) -> Result<(), DispatchError> where T::AssetBalance: Balanced, - (T::MultiAssetId, T::AssetBalance): Into>, + // T::AssetBalance: From::Balance> { - let credit: Credit = (asset_id, amount).into(); + // TODO similar to Self::transfer, use MultiAssetIdConverter to determine whether to use + // T::Assets or T:Currency - // Resolve is swallowing the inner Self::deposit DispatchError.. - T::AssetBalance::resolve(&to, credit).map_err(|_| Error::::Overflow)?; + let result = match T::MultiAssetIdConverter::try_convert(&asset_id) { + MultiAssetIdConversionResult::Converted(asset_id) => + T::AssetBalance::resolve(&to, credit).map_err(|_| Error::::Overflow)?, + MultiAssetIdConversionResult::Native => { + T::Currency::mint_into( + to, + Self::convert_asset_balance_to_native_balance(credit.peek())?, + )?; + Ok(()) + }, + MultiAssetIdConversionResult::Unsupported(_) => + Err(Error::::UnsupportedAsset.into()), + }; - Ok(()) + Ok(result) } - /// Remove an `amount` of `asset_id` `from` account and return it as a credit imbalance. - fn credit_settle( + fn withdraw( asset_id: T::MultiAssetId, from: &T::AccountId, amount: T::AssetBalance, ) -> Result, DispatchError> where T::AssetBalance: Balanced, - (T::MultiAssetId, T::AssetBalance): Into>, { - let debt: Debt = (asset_id, amount).into(); - - // Settle is swallowing the inner Self::deposit DispatchError.. - Ok(T::AssetBalance::settle(&from, debt, Preserve).map_err(|_| Error::::Overflow)?) - } - - /// TODO - fn credit_resolve( - asset_id: T::MultiAssetId, - to: &T::AccountId, - credit: Credit, - ) -> Result<(), DispatchError> { - // TODO similar to Self::transfer, use MultiAssetIdConverter to determine whether to use - // T::Assets or T:Currency - Ok(()) - } - - fn withdraw( - asset_id: T::MultiAssetId, - from: &T::AccountId, - amount: T::AssetBalance, - ) -> Result, DispatchError> { // TODO similar to Self::transfer, use MultiAssetIdConverter to determine whether to use // T::Assets or T:Currency } @@ -1034,73 +1024,80 @@ pub mod pallet { Ok(()) } + // pub(crate) fn do_swap_credit( + // amounts: &Vec, + // path: BoundedVec, + // ) -> Result, DispatchError> + // where + // T::AssetBalance: Balanced, + // (T::MultiAssetId, T::AssetBalance): Into> + // + Into>, + // { + // ensure!(amounts.len() > 1, Error::::CorrespondenceError); + // + // return 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)?; + // + // // Pool account needs enough balance in asset 1 for the swap, but we don't want to + // // actually transfer the funds, since that would require a sender account, so we + // // instead resolve the credit into the pool_account. + // Self::credit_resolve(asset1.clone(), &pool_account, *first_amount)?; + // + // 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 reserve = Self::get_balance(&pool_account, asset2)?; + // let reserve_left = reserve.saturating_sub(*amount_out); + // Self::validate_minimal_amount(reserve_left, asset2) + // .map_err(|_| Error::::ReserveLeftLessThanMinimal)?; + // + // // Same as credit but use settle instead of resolve, and get a Credit<> out + // // TODO: This will only work for path vectors of size 2 (no hops). + // // TODO: Emit a newly created event called SwapCreditExecuted + // return Ok(Self::credit_settle(asset2.clone(), &pool_account, *amount_out)?) + // } + // i.saturating_inc(); + // } + // + // Err(Error::::InvalidPath.into()) + // } else { + // Err(Error::::InvalidPath.into()) + // } + // } + pub(crate) fn do_swap_credit( + credit_in: Credit, amounts: &Vec, path: BoundedVec, ) -> Result, DispatchError> where T::AssetBalance: Balanced, - (T::MultiAssetId, T::AssetBalance): Into> - + Into>, { ensure!(amounts.len() > 1, Error::::CorrespondenceError); - return 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)?; - - // Pool account needs enough balance in asset 1 for the swap, but we don't want to - // actually transfer the funds, since that would require a sender account, so we - // instead resolve the credit into the pool_account. - Self::credit_resolve(asset1.clone(), &pool_account, *first_amount)?; - - 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 reserve = Self::get_balance(&pool_account, asset2)?; - let reserve_left = reserve.saturating_sub(*amount_out); - Self::validate_minimal_amount(reserve_left, asset2) - .map_err(|_| Error::::ReserveLeftLessThanMinimal)?; - - // Same as credit but use settle instead of resolve, and get a Credit<> out - // TODO: This will only work for path vectors of size 2 (no hops). - // TODO: Emit a newly created event called SwapCreditExecuted - return Ok(Self::credit_settle(asset2.clone(), &pool_account, *amount_out)?) - } - i.saturating_inc(); - } - - Err(Error::::InvalidPath.into()) - } else { - Err(Error::::InvalidPath.into()) - } - } - - pub(crate) fn do_swap_credit2( - credit_in: Credit, - amounts: &Vec, - path: BoundedVec, - ) -> 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)?; - // TODO ensure credit_in.peak() == first_amount - // TODO ensure credit_asset_id == asset1 - Self::credit_resolve(asset1, &pool_account, credit_in)?; + ensure!( + credit_in.peek() >= first_amount, + Error::::ProvidedMinimumNotSufficientForSwap + ); + ensure!(credit_in.asset() == asset1, Error::::AssetCreditMismatch); + + Self::credit_resolve(asset1.clone(), &pool_account, credit_in)?; let mut i = 0; let path_len = path.len() as u32; @@ -1112,7 +1109,6 @@ pub mod pallet { let amount_out = amounts.get((i + 1) as usize).ok_or(Error::::CorrespondenceError)?; - // TODO copy past from original swap, please validate let to = if i < path_len - 2 { let asset3 = path.get((i + 2) as usize).ok_or(Error::::PathError)?; Some(Self::get_pool_account(&Self::get_pool_id( @@ -1130,15 +1126,17 @@ pub mod pallet { if to.is_some() { // TODO transfer as in original swap - // Self::transfer(asset2, &pool_account, &to, *amount_out, true)?; + // Wouldn't this transfer between pool accounts? Shouldn't we do it + // with credits instead? Also, this implies more than one asset hop. + Self::transfer(asset2, &pool_account, &to.unwrap(), *amount_out, true)?; } else { - // TODO let credit_out = Self::withdraw(asset2.clone(), &pool_account, - // *amount_out) + return Ok(Self::withdraw(asset2.clone(), &pool_account, *amount_out)?) } } i.saturating_inc(); } + Err(Error::::InvalidPath.into()) // TODO deposit event } else { Err(Error::::InvalidPath.into()) @@ -1501,28 +1499,28 @@ impl Swap f Ok(amount_out.into()) } - fn swap_tokens_for_exact_tokens_credit( - path: Vec, - amount_out: T::HigherPrecisionBalance, - credit_in: Credit, - ) -> Result, DispatchError> - where - T::HigherPrecisionBalance: Balanced, - T::AssetBalance: Balanced, - (T::MultiAssetId, T::AssetBalance): - Into> + Into>, - { - let path = path.try_into().map_err(|_| Error::::PathError)?; - let amount_in_ab = Self::convert_hpb_to_asset_balance(credit_in.peek())?; - let amount_in: Credit = (path[0], amount_in_ab).into(); - let out = Self::do_swap_tokens_for_exact_tokens_credit( - path, - Self::convert_hpb_to_asset_balance(amount_out)?, - amount_in, - )?; - // TODO: convert out credits to HPB - Ok(out.into()) - } + // fn swap_tokens_for_exact_tokens_credit( + // path: Vec, + // amount_out: T::HigherPrecisionBalance, + // credit_in: Credit, + // ) -> Result, DispatchError> + // where + // T::HigherPrecisionBalance: Balanced, + // T::AssetBalance: Balanced, + // (T::MultiAssetId, T::AssetBalance): + // Into> + Into>, + // { + // let path = path.try_into().map_err(|_| Error::::PathError)?; + // let amount_in_ab = Self::convert_hpb_to_asset_balance(credit_in.peek())?; + // let amount_in: Credit = (path[0], amount_in_ab).into(); + // let out = Self::do_swap_tokens_for_exact_tokens_credit( + // path, + // Self::convert_hpb_to_asset_balance(amount_out)?, + // amount_in, + // )?; + // // TODO: convert out credits to HPB + // Ok(out.into()) + // } fn swap_tokens_for_exact_tokens( sender: T::AccountId, @@ -1546,8 +1544,23 @@ impl Swap f } } -impl SwapCredit for Pallet { - // TODO fn swap_exact_tokens_for_tokens_credit() +impl SwapCredit for Pallet +where + T::AssetBalance: Balanced, +{ + // fn swap_exact_tokens_for_tokens_credit( + // path: Vec, + // credit_in: Credit, + // amount_out_min: Option, + // ) -> Result, DispatchError> + // where + // T::AssetBalance: Balanced, + // { + // let path = path.try_into().map_err(|_| Error::::PathError)?; + // // TODO + // let out = Self::do_swap_tokens_for_exact_tokens_credit(path, amount_out_min, credit_in)?; + // Ok(out.0) + // } fn swap_tokens_for_exact_tokens_credit( path: Vec, @@ -1558,7 +1571,7 @@ impl SwapCredit for P DispatchError, > { let path = path.try_into().map_err(|_| Error::::PathError)?; - let out = Self::do_swap_tokens_for_exact_tokens_credit2(path, amount_out, credit_in)?; + let out = Self::do_swap_tokens_for_exact_tokens_credit(path, amount_out, credit_in)?; Ok(out) } } diff --git a/frame/asset-conversion/src/types.rs b/frame/asset-conversion/src/types.rs index ebf4b2a595966..5debcfc9c814a 100644 --- a/frame/asset-conversion/src/types.rs +++ b/frame/asset-conversion/src/types.rs @@ -105,32 +105,32 @@ pub trait Swap { keep_alive: bool, ) -> Result; - /// Swap `amount_in_max` of asset `path[0]` for asset `path[1]` declared in `amount_out`. - /// It will return an error if acquiring `amount_out` would be too costly. - /// - /// Thus it is on the RPC side to ensure that `amount_in` is enough to acquire `amount_out`. - /// - /// Uses the `amount_in` imbalance to offset into the pool account. - /// - /// If successful, returns the amount of `path[1]` acquired for the `amount_in` - /// along with the leftovers from `amount_in` as an imbalance. - /// They could be credited somewhere as the type implies, but can also be dropped. - /// - /// Note: This method effectively prevents overswapping, so that the - /// returned `CreditPair::in_leftover` can then be directly refunded in the initial asset - /// without swapping back from the `path[1]` asset. - /// - /// `amount_in` is not optional due to the fact that it is a balance to be offset - /// (credited to the pool), and not an amount to be acquired from a sender. - /// - /// If this function returns an error, no credit will be taken from amount_in, like a no-op. - fn swap_tokens_for_exact_tokens_credit( - path: Vec, - amount_out: Balance, - credit_in: Credit, - ) -> Result, DispatchError> - where - Balance: Balanced; + // /// Swap `amount_in_max` of asset `path[0]` for asset `path[1]` declared in `amount_out`. + // /// It will return an error if acquiring `amount_out` would be too costly. + // /// + // /// Thus it is on the RPC side to ensure that `amount_in` is enough to acquire `amount_out`. + // /// + // /// Uses the `amount_in` imbalance to offset into the pool account. + // /// + // /// If successful, returns the amount of `path[1]` acquired for the `amount_in` + // /// along with the leftovers from `amount_in` as an imbalance. + // /// They could be credited somewhere as the type implies, but can also be dropped. + // /// + // /// Note: This method effectively prevents overswapping, so that the + // /// returned `CreditPair::in_leftover` can then be directly refunded in the initial asset + // /// without swapping back from the `path[1]` asset. + // /// + // /// `amount_in` is not optional due to the fact that it is a balance to be offset + // /// (credited to the pool), and not an amount to be acquired from a sender. + // /// + // /// If this function returns an error, no credit will be taken from amount_in, like a no-op. + // fn swap_tokens_for_exact_tokens_credit( + // path: Vec, + // amount_out: Balance, + // credit_in: Credit, + // ) -> Result, DispatchError> + // where + // Balance: Balanced; /// 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 @@ -150,12 +150,12 @@ pub trait Swap { ) -> Result; } -pub trait SwapCredit { - fn swap_exact_tokens_for_tokens_credit( - path: Vec, - credit_in: Credit, - amount_out_min: Option, - ) -> Result, DispatchError>; +pub trait SwapCredit, MultiAssetId> { + // fn swap_exact_tokens_for_tokens_credit( + // path: Vec, + // credit_in: Credit, + // amount_out_min: Option, + // ) -> Result, DispatchError>; fn swap_tokens_for_exact_tokens_credit( path: Vec, From 2038feae45d005464a198941f1f2fab36726ac28 Mon Sep 17 00:00:00 2001 From: Patricio Napoli Date: Thu, 24 Aug 2023 01:43:49 -0300 Subject: [PATCH 6/7] added tentative bounds, pending correct type arguments between Inspect::balance and AssetBalance --- frame/asset-conversion/src/lib.rs | 28 ++++++++++++++++++++++------ frame/asset-conversion/src/types.rs | 4 ++-- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/frame/asset-conversion/src/lib.rs b/frame/asset-conversion/src/lib.rs index 0016ab5e4e7d8..1c9617b9619c8 100644 --- a/frame/asset-conversion/src/lib.rs +++ b/frame/asset-conversion/src/lib.rs @@ -906,7 +906,6 @@ pub mod pallet { ) -> Result<(), DispatchError> where T::AssetBalance: Balanced, - // T::AssetBalance: From::Balance> { // TODO similar to Self::transfer, use MultiAssetIdConverter to determine whether to use // T::Assets or T:Currency @@ -1081,7 +1080,20 @@ pub mod pallet { path: BoundedVec, ) -> Result, DispatchError> where + ::MultiAssetId: + Into< + <::AssetBalance as Inspect< + ::AccountId, + >>::AssetId, + >, T::AssetBalance: Balanced, + T::HigherPrecisionBalance: + From + + From< + <::AssetBalance as Inspect< + ::AccountId, + >>::Balance, + >, { ensure!(amounts.len() > 1, Error::::CorrespondenceError); @@ -1092,10 +1104,14 @@ pub mod pallet { let first_amount = amounts.first().ok_or(Error::::CorrespondenceError)?; ensure!( - credit_in.peek() >= first_amount, + T::HigherPrecisionBalance::from(credit_in.peek()) >= + T::HigherPrecisionBalance::from(first_amount.clone()), Error::::ProvidedMinimumNotSufficientForSwap ); - ensure!(credit_in.asset() == asset1, Error::::AssetCreditMismatch); + ensure!( + credit_in.asset() == asset1.clone().into(), + Error::::AssetCreditMismatch + ); Self::credit_resolve(asset1.clone(), &pool_account, credit_in)?; @@ -1499,7 +1515,7 @@ impl Swap f Ok(amount_out.into()) } - // fn swap_tokens_for_exact_tokens_credit( + // fn swap_tokens_for_exact_tokens( // path: Vec, // amount_out: T::HigherPrecisionBalance, // credit_in: Credit, @@ -1548,7 +1564,7 @@ impl SwapCredit for P where T::AssetBalance: Balanced, { - // fn swap_exact_tokens_for_tokens_credit( + // fn swap_exact_tokens_for_tokens( // path: Vec, // credit_in: Credit, // amount_out_min: Option, @@ -1562,7 +1578,7 @@ where // Ok(out.0) // } - fn swap_tokens_for_exact_tokens_credit( + fn swap_tokens_for_exact_tokens( path: Vec, amount_out: T::AssetBalance, credit_in: Credit, diff --git a/frame/asset-conversion/src/types.rs b/frame/asset-conversion/src/types.rs index 5debcfc9c814a..b7848cf39e66f 100644 --- a/frame/asset-conversion/src/types.rs +++ b/frame/asset-conversion/src/types.rs @@ -151,13 +151,13 @@ pub trait Swap { } pub trait SwapCredit, MultiAssetId> { - // fn swap_exact_tokens_for_tokens_credit( + // fn swap_exact_tokens_for_tokens( // path: Vec, // credit_in: Credit, // amount_out_min: Option, // ) -> Result, DispatchError>; - fn swap_tokens_for_exact_tokens_credit( + fn swap_tokens_for_exact_tokens( path: Vec, amount_out: Balance, credit_in: Credit, From 0aaf3ab3004ec4ccdf53cee8b23d28e6ce5818ad Mon Sep 17 00:00:00 2001 From: muharem Date: Thu, 24 Aug 2023 16:55:07 +0200 Subject: [PATCH 7/7] compiling version --- frame/asset-conversion/src/lib.rs | 181 ++++++++++++++-------------- frame/asset-conversion/src/types.rs | 32 +++-- 2 files changed, 115 insertions(+), 98 deletions(-) diff --git a/frame/asset-conversion/src/lib.rs b/frame/asset-conversion/src/lib.rs index 1c9617b9619c8..2d593b3312092 100644 --- a/frame/asset-conversion/src/lib.rs +++ b/frame/asset-conversion/src/lib.rs @@ -51,7 +51,7 @@ //! http://localhost:9933/ //! ``` //! (This can be run against the kitchen sync node in the `node` folder of this repo.) -#![deny(missing_docs)] +// #![deny(missing_docs)] #![cfg_attr(not(feature = "std"), no_std)] use frame_support::traits::{DefensiveOption, Incrementable}; @@ -71,7 +71,11 @@ use codec::Codec; use frame_support::{ ensure, traits::{ - fungibles::{Balanced, Credit, Debt}, + fungible::{ + Balanced as BalancedFungible, Credit as CreditFungible, Inspect as InspectFungible, + Mutate as MutateFungible, + }, + fungibles::{Balanced, Create, Credit, Debt, Inspect, Mutate}, tokens::{AssetId, Balance}, }, }; @@ -97,8 +101,6 @@ pub mod pallet { use frame_support::{ pallet_prelude::*, traits::{ - fungible::{Inspect as InspectFungible, Mutate as MutateFungible}, - fungibles::{Create, Inspect, Mutate}, tokens::{ Fortitude::Polite, Precision::Exact, @@ -124,7 +126,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; @@ -162,7 +165,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. @@ -758,31 +762,31 @@ pub mod pallet { Ok(amount_out) } - pub fn do_swap_tokens_for_exact_tokens_credit( + pub fn do_swap_tokens_for_exact_native_tokens_credit( path: BoundedVec, - amount_out: T::AssetBalance, - credit_in: Credit, + amount_out: T::Balance, + credit_in: Credit, ) -> Result< - (Credit, Credit), + (Credit, CreditFungible), DispatchError, - > - where - T::AssetBalance: Balanced, - { + > { ensure!(amount_out > Zero::zero(), Error::::ZeroAmount); ensure!(credit_in.peek() > Zero::zero(), Error::::ZeroAmount); Self::validate_swap_path(&path)?; - let amounts = Self::get_amounts_in(&amount_out, &path)?; + let amounts = Self::get_amounts_in( + &Self::convert_native_balance_to_asset_balance(amount_out)?, + &path, + )?; let amount_in = *amounts.first().defensive_ok_or("get_amounts_in() returned an empty result")?; - ensure!(credit_in.peek() >= amount_in, Error::::ProvidedMaximumNotSufficientForSwap); + ensure!(credit_in.peek() == amount_in, Error::::ProvidedMaximumNotSufficientForSwap); let (credit_in, credit_change) = credit_in.split(amount_in); - let out_credit = Self::do_swap_credit(credit_in, &amounts, path)?; + let out_credit = Self::do_swap_native_credit(credit_in, &amounts, path)?; Ok((credit_change, out_credit)) } @@ -898,46 +902,46 @@ pub mod pallet { // Ok(T::AssetBalance::settle(&from, debt, Preserve).map_err(|_| Error::::Overflow)?) // } - /// TODO - fn credit_resolve( - asset_id: T::MultiAssetId, - to: &T::AccountId, - credit: Credit, - ) -> Result<(), DispatchError> - where - T::AssetBalance: Balanced, - { - // TODO similar to Self::transfer, use MultiAssetIdConverter to determine whether to use - // T::Assets or T:Currency - - let result = match T::MultiAssetIdConverter::try_convert(&asset_id) { - MultiAssetIdConversionResult::Converted(asset_id) => - T::AssetBalance::resolve(&to, credit).map_err(|_| Error::::Overflow)?, - MultiAssetIdConversionResult::Native => { - T::Currency::mint_into( - to, - Self::convert_asset_balance_to_native_balance(credit.peek())?, - )?; - Ok(()) - }, - MultiAssetIdConversionResult::Unsupported(_) => - Err(Error::::UnsupportedAsset.into()), - }; - - Ok(result) - } + // /// TODO + // fn credit_resolve( + // asset_id: T::MultiAssetId, + // to: &T::AccountId, + // credit: Credit, + // ) -> Result<(), DispatchError> + // where + // T::AssetBalance: Balanced, + // { + // // TODO similar to Self::transfer, use MultiAssetIdConverter to determine whether to use + // // T::Assets or T:Currency + + // let result = match T::MultiAssetIdConverter::try_convert(&asset_id) { + // MultiAssetIdConversionResult::Converted(asset_id) => + // T::AssetBalance::resolve(&to, credit).map_err(|_| Error::::Overflow)?, + // MultiAssetIdConversionResult::Native => { + // T::Currency::mint_into( + // to, + // Self::convert_asset_balance_to_native_balance(credit.peek())?, + // )?; + // Ok(()) + // }, + // MultiAssetIdConversionResult::Unsupported(_) => + // Err(Error::::UnsupportedAsset.into()), + // }; + + // Ok(result) + // } - fn withdraw( - asset_id: T::MultiAssetId, - from: &T::AccountId, - amount: T::AssetBalance, - ) -> Result, DispatchError> - where - T::AssetBalance: Balanced, - { - // TODO similar to Self::transfer, use MultiAssetIdConverter to determine whether to use - // T::Assets or T:Currency - } + // fn withdraw( + // asset_id: T::MultiAssetId, + // from: &T::AccountId, + // amount: T::AssetBalance, + // ) -> Result, DispatchError> + // where + // T::AssetBalance: Balanced, + // { + // // TODO similar to Self::transfer, use MultiAssetIdConverter to determine whether to use + // // T::Assets or T:Currency + // } /// Convert a `Balance` type to an `AssetBalance`. pub(crate) fn convert_native_balance_to_asset_balance( @@ -1074,27 +1078,11 @@ pub mod pallet { // } // } - pub(crate) fn do_swap_credit( - credit_in: Credit, + pub(crate) fn do_swap_native_credit( + credit_in: Credit, amounts: &Vec, path: BoundedVec, - ) -> Result, DispatchError> - where - ::MultiAssetId: - Into< - <::AssetBalance as Inspect< - ::AccountId, - >>::AssetId, - >, - T::AssetBalance: Balanced, - T::HigherPrecisionBalance: - From - + From< - <::AssetBalance as Inspect< - ::AccountId, - >>::Balance, - >, - { + ) -> Result, DispatchError> { ensure!(amounts.len() > 1, Error::::CorrespondenceError); if let Some([asset1, asset2]) = &path.get(0..2) { @@ -1104,16 +1092,17 @@ pub mod pallet { let first_amount = amounts.first().ok_or(Error::::CorrespondenceError)?; ensure!( - T::HigherPrecisionBalance::from(credit_in.peek()) >= - T::HigherPrecisionBalance::from(first_amount.clone()), + credit_in.peek() == *first_amount, Error::::ProvidedMinimumNotSufficientForSwap ); - ensure!( - credit_in.asset() == asset1.clone().into(), - Error::::AssetCreditMismatch - ); + // TODO MultiAssetIdConverter::try_convert(asset1) and assert + // ensure!( + // credit_in.asset() == asset1.clone().into(), + // Error::::AssetCreditMismatch + // ); - Self::credit_resolve(asset1.clone(), &pool_account, credit_in)?; + // TODO fails if returns back credit + T::Assets::resolve(&pool_account, credit_in); let mut i = 0; let path_len = path.len() as u32; @@ -1146,7 +1135,18 @@ pub mod pallet { // with credits instead? Also, this implies more than one asset hop. Self::transfer(asset2, &pool_account, &to.unwrap(), *amount_out, true)?; } else { - return Ok(Self::withdraw(asset2.clone(), &pool_account, *amount_out)?) + return match T::MultiAssetIdConverter::try_convert(asset2) { + MultiAssetIdConversionResult::Native => + // TODO review arguments (Exact, Expendable, Polite). + T::Currency::withdraw( + &pool_account, + Self::convert_asset_balance_to_native_balance(*amount_out)?, + Exact, + Expendable, + Polite, + ), + _ => Err(Error::::InvalidPath.into()), + } } } i.saturating_inc(); @@ -1560,9 +1560,8 @@ impl Swap f } } -impl SwapCredit for Pallet -where - T::AssetBalance: Balanced, +impl SwapCredit + for Pallet { // fn swap_exact_tokens_for_tokens( // path: Vec, @@ -1578,16 +1577,16 @@ where // Ok(out.0) // } - fn swap_tokens_for_exact_tokens( + fn swap_tokens_for_exact_native_tokens( path: Vec, - amount_out: T::AssetBalance, - credit_in: Credit, + amount_out: T::Balance, + credit_in: Credit, ) -> Result< - (Credit, Credit), + (Credit, CreditFungible), DispatchError, > { let path = path.try_into().map_err(|_| Error::::PathError)?; - let out = Self::do_swap_tokens_for_exact_tokens_credit(path, amount_out, credit_in)?; + let out = Self::do_swap_tokens_for_exact_native_tokens_credit(path, amount_out, credit_in)?; Ok(out) } } diff --git a/frame/asset-conversion/src/types.rs b/frame/asset-conversion/src/types.rs index b7848cf39e66f..f0955dfd9ee2d 100644 --- a/frame/asset-conversion/src/types.rs +++ b/frame/asset-conversion/src/types.rs @@ -19,6 +19,7 @@ use super::*; use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::traits::fungibles::Credit; +use frame_system::Account; use scale_info::TypeInfo; use sp_std::{cmp::Ordering, marker::PhantomData}; @@ -150,18 +151,35 @@ pub trait Swap { ) -> Result; } -pub trait SwapCredit, MultiAssetId> { - // fn swap_exact_tokens_for_tokens( +pub trait SwapCredit +where + Fungible: BalancedFungible, + Fungibles: Balanced, +{ + // fn swap_exact_native_tokens_for_tokens( // path: Vec, - // credit_in: Credit, + // credit_in: CreditFungible, + // amount_out_min: Option, + // ) -> Result, DispatchError>; + + // fn swap_native_tokens_for_exact_tokens( + // path: Vec, + // amount_out: AssetBalance, + // credit_in: CreditFungible, + // ) -> Result<(CreditFungible, Credit), + // DispatchError>; + + // fn swap_exact_tokens_for_native_tokens( + // path: Vec, + // credit_in: Credit, // amount_out_min: Option, - // ) -> Result, DispatchError>; + // ) -> Result, DispatchError>; - fn swap_tokens_for_exact_tokens( + fn swap_tokens_for_exact_native_tokens( path: Vec, amount_out: Balance, - credit_in: Credit, - ) -> Result<(Credit, Credit), DispatchError>; + credit_in: Credit, + ) -> Result<(Credit, CreditFungible), DispatchError>; } /// An implementation of MultiAssetId that can be either Native or an asset.