From ee51e95183c9d3e95a81b007ff3fe420c20b0698 Mon Sep 17 00:00:00 2001 From: Martin Date: Thu, 6 Jul 2023 18:46:47 +0200 Subject: [PATCH 1/4] add remove liquidity --- math/src/stableswap/math.rs | 8 ++ pallets/stableswap/src/benchmarks.rs | 60 ++++----- pallets/stableswap/src/lib.rs | 107 +++++++++++++-- pallets/stableswap/src/tests/add_liquidity.rs | 56 ++++---- pallets/stableswap/src/tests/invariants.rs | 26 ++-- pallets/stableswap/src/tests/mock.rs | 4 +- .../stableswap/src/tests/remove_liquidity.rs | 126 ++++++++++++++---- pallets/stableswap/src/tests/trades.rs | 30 ++--- pallets/stableswap/src/types.rs | 2 +- 9 files changed, 292 insertions(+), 127 deletions(-) diff --git a/math/src/stableswap/math.rs b/math/src/stableswap/math.rs index 2d66d58ae..5c5aede68 100644 --- a/math/src/stableswap/math.rs +++ b/math/src/stableswap/math.rs @@ -110,6 +110,14 @@ pub fn calculate_shares( } } +pub fn calculate_withdraw_liquidity( + reserves: &[Balance], + shares: Balance, + share_asset_issuance: Balance, +) -> Option> { + None +} + /// Given amount of shares and asset reserves, calculate corresponding amount of selected asset to be withdrawn. pub fn calculate_withdraw_one_asset( reserves: &[Balance], diff --git a/pallets/stableswap/src/benchmarks.rs b/pallets/stableswap/src/benchmarks.rs index 446dd8744..7cf30dc6e 100644 --- a/pallets/stableswap/src/benchmarks.rs +++ b/pallets/stableswap/src/benchmarks.rs @@ -30,7 +30,7 @@ use sp_runtime::Permill; use hydradx_traits::Registry; -use crate::types::{AssetLiquidity, Balance}; +use crate::types::{AssetBalance, Balance}; // Stable benchmarks // Worst case scenarios in any stableswap calculations are scenarios where "math" does max number of iterations. @@ -71,8 +71,8 @@ benchmarks! { let initial_liquidity = 1_000_000_000_000_000u128; let liquidity_added = 300_000_000_000_000u128; - let mut initial: Vec> = vec![]; - let mut added_liquidity: Vec> = vec![]; + let mut initial: Vec> = vec![]; + let mut added_liquidity: Vec> = vec![]; let mut asset_ids: Vec = Vec::new() ; for idx in 0..MAX_ASSETS_IN_POOL { @@ -81,11 +81,11 @@ benchmarks! { asset_ids.push(asset_id); T::Currency::update_balance(asset_id, &caller, 1_000_000_000_000_000i128)?; T::Currency::update_balance(asset_id, &lp_provider, 1_000_000_000_000_000_000_000i128)?; - initial.push(AssetLiquidity{ + initial.push(AssetBalance{ asset_id, amount: initial_liquidity }); - added_liquidity.push(AssetLiquidity{ + added_liquidity.push(AssetBalance{ asset_id, amount: liquidity_added }); @@ -121,8 +121,8 @@ benchmarks! { let initial_liquidity = 1_000_000_000_000_000u128; let liquidity_added = 300_000_000_000_000u128; - let mut initial: Vec> = vec![]; - let mut added_liquidity: Vec> = vec![]; + let mut initial: Vec> = vec![]; + let mut added_liquidity: Vec> = vec![]; let mut asset_ids: Vec = Vec::new() ; for idx in 0..MAX_ASSETS_IN_POOL { @@ -131,11 +131,11 @@ benchmarks! { asset_ids.push(asset_id); T::Currency::update_balance(asset_id, &caller, 1_000_000_000_000_000i128)?; T::Currency::update_balance(asset_id, &lp_provider, liquidity_added as i128)?; - initial.push(AssetLiquidity{ + initial.push(AssetBalance{ asset_id, amount: initial_liquidity }); - added_liquidity.push(AssetLiquidity{ + added_liquidity.push(AssetBalance{ asset_id, amount: liquidity_added }); @@ -173,7 +173,7 @@ benchmarks! { let shares = T::Currency::free_balance(pool_id, &lp_provider); - }: _(RawOrigin::Signed(lp_provider.clone()), pool_id, asset_id_to_withdraw, shares) + }: _(RawOrigin::Signed(lp_provider.clone()), pool_id, asset_id_to_withdraw, shares, 0) verify { assert_eq!(T::Currency::free_balance(pool_id, &lp_provider), 0u128); assert_eq!(T::Currency::free_balance(asset_id_to_withdraw, &lp_provider), 1296846466078107); @@ -185,8 +185,8 @@ benchmarks! { let initial_liquidity = 1_000_000_000_000_000u128; let liquidity_added = 300_000_000_000_000u128; - let mut initial: Vec> = vec![]; - let mut added_liquidity: Vec> = vec![]; + let mut initial: Vec> = vec![]; + let mut added_liquidity: Vec> = vec![]; let mut asset_ids: Vec = Vec::new() ; for idx in 0..MAX_ASSETS_IN_POOL { @@ -195,11 +195,11 @@ benchmarks! { asset_ids.push(asset_id); T::Currency::update_balance(asset_id, &caller, 1_000_000_000_000_000i128)?; T::Currency::update_balance(asset_id, &lp_provider, 1_000_000_000_000_000_000_000i128)?; - initial.push(AssetLiquidity{ + initial.push(AssetBalance{ asset_id, amount: initial_liquidity }); - added_liquidity.push(AssetLiquidity{ + added_liquidity.push(AssetBalance{ asset_id, amount: liquidity_added }); @@ -256,8 +256,8 @@ benchmarks! { let initial_liquidity = 1_000_000_000_000_000u128; let liquidity_added = 300_000_000_000_000u128; - let mut initial: Vec> = vec![]; - let mut added_liquidity: Vec> = vec![]; + let mut initial: Vec> = vec![]; + let mut added_liquidity: Vec> = vec![]; let mut asset_ids: Vec = Vec::new() ; for idx in 0..MAX_ASSETS_IN_POOL { @@ -266,11 +266,11 @@ benchmarks! { asset_ids.push(asset_id); T::Currency::update_balance(asset_id, &caller, 1_000_000_000_000_000i128)?; T::Currency::update_balance(asset_id, &lp_provider, 1_000_000_000_000_000_000_000i128)?; - initial.push(AssetLiquidity{ + initial.push(AssetBalance{ asset_id, amount: initial_liquidity }); - added_liquidity.push(AssetLiquidity{ + added_liquidity.push(AssetBalance{ asset_id, amount: liquidity_added }); @@ -326,8 +326,8 @@ benchmarks! { let initial_liquidity = 1_000_000_000_000_000u128; let liquidity_added = 300_000_000_000_000u128; - let mut initial: Vec> = vec![]; - let mut added_liquidity: Vec> = vec![]; + let mut initial: Vec> = vec![]; + let mut added_liquidity: Vec> = vec![]; let mut asset_ids: Vec = Vec::new() ; for idx in 0..MAX_ASSETS_IN_POOL { @@ -336,11 +336,11 @@ benchmarks! { asset_ids.push(asset_id); T::Currency::update_balance(asset_id, &caller, 1_000_000_000_000_000i128)?; T::Currency::update_balance(asset_id, &lp_provider, 1_000_000_000_000_000_000_000i128)?; - initial.push(AssetLiquidity{ + initial.push(AssetBalance{ asset_id, amount: initial_liquidity }); - added_liquidity.push(AssetLiquidity{ + added_liquidity.push(AssetBalance{ asset_id, amount: liquidity_added }); @@ -374,8 +374,8 @@ benchmarks! { let initial_liquidity = 1_000_000_000_000_000u128; let liquidity_added = 300_000_000_000_000u128; - let mut initial: Vec> = vec![]; - let mut added_liquidity: Vec> = vec![]; + let mut initial: Vec> = vec![]; + let mut added_liquidity: Vec> = vec![]; let mut asset_ids: Vec = Vec::new() ; for idx in 0..MAX_ASSETS_IN_POOL { @@ -384,11 +384,11 @@ benchmarks! { asset_ids.push(asset_id); T::Currency::update_balance(asset_id, &caller, 1_000_000_000_000_000i128)?; T::Currency::update_balance(asset_id, &lp_provider, 1_000_000_000_000_000_000_000i128)?; - initial.push(AssetLiquidity{ + initial.push(AssetBalance{ asset_id, amount: initial_liquidity }); - added_liquidity.push(AssetLiquidity{ + added_liquidity.push(AssetBalance{ asset_id, amount: liquidity_added }); @@ -419,8 +419,8 @@ benchmarks! { let initial_liquidity = 1_000_000_000_000_000u128; let liquidity_added = 300_000_000_000_000u128; - let mut initial: Vec> = vec![]; - let mut added_liquidity: Vec> = vec![]; + let mut initial: Vec> = vec![]; + let mut added_liquidity: Vec> = vec![]; let mut asset_ids: Vec = Vec::new() ; for idx in 0..MAX_ASSETS_IN_POOL { @@ -429,11 +429,11 @@ benchmarks! { asset_ids.push(asset_id); T::Currency::update_balance(asset_id, &caller, 1_000_000_000_000_000i128)?; T::Currency::update_balance(asset_id, &lp_provider, 1_000_000_000_000_000_000_000i128)?; - initial.push(AssetLiquidity{ + initial.push(AssetBalance{ asset_id, amount: initial_liquidity }); - added_liquidity.push(AssetLiquidity{ + added_liquidity.push(AssetBalance{ asset_id, amount: liquidity_added }); diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index aba70aabd..1c6aba3db 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -58,7 +58,7 @@ pub mod weights; pub use trade_execution::*; -use crate::types::{AssetLiquidity, Balance, PoolInfo, Tradability}; +use crate::types::{AssetBalance, Balance, PoolInfo, Tradability}; use hydradx_traits::pools::DustRemovalAccountWhitelist; use orml_traits::MultiCurrency; use sp_std::collections::btree_map::BTreeMap; @@ -179,15 +179,14 @@ pub mod pallet { pool_id: T::AssetId, who: T::AccountId, shares: Balance, - assets: Vec>, + assets: Vec>, }, /// Liquidity removed. LiquidityRemoved { pool_id: T::AssetId, who: T::AccountId, shares: Balance, - asset: T::AssetId, - amount: Balance, + amounts: Vec>, fee: Balance, }, /// Sell trade executed. Trade fee paid in asset leaving the pool (already subtracted from amount_out). @@ -305,6 +304,9 @@ pub mod pallet { /// New amplification is equal to the previous value. SameAmplification, + + /// Desired amount not reached. + MinimumAmountNotReached, } #[pallet::call] @@ -483,7 +485,7 @@ pub mod pallet { pub fn add_liquidity( origin: OriginFor, pool_id: T::AssetId, - assets: Vec>, + assets: Vec>, ) -> DispatchResult { let who = ensure_signed(origin)?; @@ -512,16 +514,95 @@ pub mod pallet { /// - `pool_id`: Pool Id /// - `asset_id`: id of asset to receive /// - 'share_amount': amount of shares to withdraw + /// - 'min_amount_out': minimum amount to receive /// /// Emits `LiquidityRemoved` event when successful. #[pallet::call_index(4)] #[pallet::weight(::WeightInfo::remove_liquidity_one_asset())] #[transactional] + pub fn remove_liquidity( + origin: OriginFor, + pool_id: T::AssetId, + share_amount: Balance, + min_amount_out: Vec>, + ) -> DispatchResult { + let who = ensure_signed(origin)?; + + ensure!(share_amount > Balance::zero(), Error::::InvalidAssetAmount); + let current_share_balance = T::Currency::free_balance(pool_id, &who); + ensure!(current_share_balance >= share_amount, Error::::InsufficientShares); + ensure!( + current_share_balance == share_amount + || current_share_balance.saturating_sub(share_amount) >= T::MinPoolLiquidity::get(), + Error::::InsufficientShareBalance + ); + + let pool = Pools::::get(pool_id).ok_or(Error::::PoolNotFound)?; + let pool_account = Self::pool_account(pool_id); + let balances = pool.balances::(&pool_account); + let share_issuance = T::Currency::total_issuance(pool_id); + + ensure!( + share_issuance == share_amount + || share_issuance.saturating_sub(share_amount) >= T::MinPoolLiquidity::get(), + Error::::InsufficientLiquidityRemaining + ); + + let amounts = + hydra_dx_math::stableswap::calculate_withdraw_liquidity(&balances, share_amount, share_issuance) + .ok_or(ArithmeticError::Overflow)?; + + let mut min_amounts = min_amount_out.clone(); + min_amounts.sort_by_key(|v| v.asset_id); + for (idx, asset_id) in pool.assets.into_iter().enumerate() { + ensure!( + Self::is_asset_allowed(pool_id, asset_id, Tradability::REMOVE_LIQUIDITY), + Error::::NotAllowed + ); + let min_amount = min_amounts.get_mut(idx).ok_or(Error::::IncorrectAssets)?; + ensure!(min_amount.asset_id == asset_id, Error::::IncorrectAssets); + ensure!(amounts[idx] >= min_amount.amount, Error::::MinimumAmountNotReached); + T::Currency::transfer(asset_id, &pool_account, &who, amounts[idx])?; + min_amount.amount = amounts[idx]; + } + T::Currency::withdraw(pool_id, &who, share_amount)?; + + Self::deposit_event(Event::LiquidityRemoved { + pool_id, + who, + shares: share_amount, + amounts: min_amounts, + fee: 0, + }); + + Ok(()) + } + + /// Remove liquidity from selected pool. + /// + /// Withdraws liquidity of selected asset from a pool. + /// + /// Share amount is burn and LP receives corresponding amount of chosen asset. + /// + /// Withdraw fee is applied to the asset amount. + /// + /// Parameters: + /// - `origin`: liquidity provider + /// - `pool_id`: Pool Id + /// - `asset_id`: id of asset to receive + /// - 'share_amount': amount of shares to withdraw + /// - 'min_amount_out': minimum amount to receive + /// + /// Emits `LiquidityRemoved` event when successful. + #[pallet::call_index(5)] + #[pallet::weight(::WeightInfo::remove_liquidity_one_asset())] + #[transactional] pub fn remove_liquidity_one_asset( origin: OriginFor, pool_id: T::AssetId, asset_id: T::AssetId, share_amount: Balance, + min_amount_out: Balance, ) -> DispatchResult { let who = ensure_signed(origin)?; @@ -565,6 +646,8 @@ pub mod pallet { ) .ok_or(ArithmeticError::Overflow)?; + ensure!(amount >= min_amount_out, Error::::MinimumAmountNotReached); + T::Currency::withdraw(pool_id, &who, share_amount)?; T::Currency::transfer(asset_id, &pool_account, &who, amount)?; @@ -572,8 +655,10 @@ pub mod pallet { pool_id, who, shares: share_amount, - asset: asset_id, - amount, + amounts: vec![AssetBalance { + asset_id: asset_id, + amount, + }], fee, }); @@ -592,7 +677,7 @@ pub mod pallet { /// /// Emits `SellExecuted` event when successful. /// - #[pallet::call_index(5)] + #[pallet::call_index(6)] #[pallet::weight(::WeightInfo::sell())] #[transactional] pub fn sell( @@ -655,7 +740,7 @@ pub mod pallet { /// /// Emits `BuyExecuted` event when successful. /// - #[pallet::call_index(6)] + #[pallet::call_index(7)] #[pallet::weight(::WeightInfo::buy())] #[transactional] pub fn buy( @@ -706,7 +791,7 @@ pub mod pallet { Ok(()) } - #[pallet::call_index(7)] + #[pallet::call_index(8)] #[pallet::weight(::WeightInfo::set_asset_tradable_state())] #[transactional] pub fn set_asset_tradable_state( @@ -845,7 +930,7 @@ impl Pallet { fn do_add_liquidity( who: &T::AccountId, pool_id: T::AssetId, - assets: &[AssetLiquidity], + assets: &[AssetBalance], ) -> Result { let pool = Pools::::get(pool_id).ok_or(Error::::PoolNotFound)?; ensure!(assets.len() <= pool.assets.len(), Error::::MaxAssetsExceeded); diff --git a/pallets/stableswap/src/tests/add_liquidity.rs b/pallets/stableswap/src/tests/add_liquidity.rs index f8f03b00e..f22bfc585 100644 --- a/pallets/stableswap/src/tests/add_liquidity.rs +++ b/pallets/stableswap/src/tests/add_liquidity.rs @@ -1,5 +1,5 @@ use crate::tests::mock::*; -use crate::types::{AssetLiquidity, PoolInfo}; +use crate::types::{AssetBalance, PoolInfo}; use crate::{assert_balance, Error}; use frame_support::{assert_noop, assert_ok}; use sp_runtime::Permill; @@ -41,11 +41,11 @@ fn add_initial_liquidity_should_work_when_called_first_time() { RuntimeOrigin::signed(BOB), pool_id, vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: initial_liquidity_amount }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: initial_liquidity_amount, } @@ -97,11 +97,11 @@ fn add_initial_liquidity_should_fail_when_lp_has_insufficient_balance() { RuntimeOrigin::signed(BOB), pool_id, vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: initial_liquidity_amount }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: initial_liquidity_amount } @@ -145,11 +145,11 @@ fn add_liquidity_should_work_when_initial_liquidity_has_been_provided() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 100 * ONE, }, @@ -168,11 +168,11 @@ fn add_liquidity_should_work_when_initial_liquidity_has_been_provided() { RuntimeOrigin::signed(BOB), pool_id, vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: amount_added }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: amount_added } @@ -215,11 +215,11 @@ fn add_liquidity_should_work_when_order_is_not_sorted() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 100 * ONE, }, @@ -238,11 +238,11 @@ fn add_liquidity_should_work_when_order_is_not_sorted() { RuntimeOrigin::signed(BOB), pool_id, vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: amount_added }, - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: amount_added } @@ -285,11 +285,11 @@ fn add_liquidity_should_fail_when_providing_insufficient_liquidity() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 100 * ONE, }, @@ -306,11 +306,11 @@ fn add_liquidity_should_fail_when_providing_insufficient_liquidity() { RuntimeOrigin::signed(BOB), pool_id, vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: amount_added }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: amount_added } @@ -354,19 +354,19 @@ fn add_liquidity_should_work_when_providing_one_asset_only() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 200 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_c, amount: 300 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_d, amount: 400 * ONE, }, @@ -381,7 +381,7 @@ fn add_liquidity_should_work_when_providing_one_asset_only() { assert_ok!(Stableswap::add_liquidity( RuntimeOrigin::signed(BOB), pool_id, - vec![AssetLiquidity { + vec![AssetBalance { asset_id: asset_a, amount: amount_added },] @@ -425,19 +425,19 @@ fn add_liquidity_should_fail_when_providing_one_asset_not_in_pool() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 200 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_c, amount: 300 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_d, amount: 400 * ONE, }, @@ -454,11 +454,11 @@ fn add_liquidity_should_fail_when_providing_one_asset_not_in_pool() { RuntimeOrigin::signed(BOB), pool_id, vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: amount_added }, - AssetLiquidity { + AssetBalance { asset_id: asset_e, amount: amount_added }, diff --git a/pallets/stableswap/src/tests/invariants.rs b/pallets/stableswap/src/tests/invariants.rs index 920ea3ae3..ace65d741 100644 --- a/pallets/stableswap/src/tests/invariants.rs +++ b/pallets/stableswap/src/tests/invariants.rs @@ -1,5 +1,5 @@ use crate::tests::mock::*; -use crate::types::{AssetLiquidity, PoolInfo}; +use crate::types::{AssetBalance, PoolInfo}; use frame_support::assert_ok; use sp_runtime::{FixedU128, Permill}; use std::num::NonZeroU16; @@ -82,11 +82,11 @@ proptest! { }, InitialLiquidity{ account: ALICE, assets: vec![ - AssetLiquidity{ + AssetBalance{ asset_id: asset_a, amount: initial_liquidity }, - AssetLiquidity{ + AssetBalance{ asset_id: asset_b, amount: initial_liquidity }]}, @@ -103,11 +103,11 @@ proptest! { assert_ok!(Stableswap::add_liquidity( RuntimeOrigin::signed(BOB), pool_id, - vec![AssetLiquidity{ + vec![AssetBalance{ asset_id: asset_a, amount: added_liquidity }, - AssetLiquidity{ + AssetBalance{ asset_id: asset_b, amount: added_liquidity } @@ -159,11 +159,11 @@ proptest! { }, InitialLiquidity{ account: ALICE, assets: vec![ - AssetLiquidity{ + AssetBalance{ asset_id: asset_a, amount: initial_liquidity }, - AssetLiquidity{ + AssetBalance{ asset_id: asset_b, amount: initial_liquidity } @@ -230,11 +230,11 @@ proptest! { }, InitialLiquidity{ account: ALICE, assets: vec![ - AssetLiquidity{ + AssetBalance{ asset_id: asset_a, amount: initial_liquidity }, - AssetLiquidity{ + AssetBalance{ asset_id: asset_b, amount: initial_liquidity } @@ -302,11 +302,11 @@ proptest! { }, InitialLiquidity{ account: ALICE, assets: vec![ - AssetLiquidity{ + AssetBalance{ asset_id: asset_a, amount: initial_liquidity }, - AssetLiquidity{ + AssetBalance{ asset_id: asset_b, amount: initial_liquidity } @@ -396,11 +396,11 @@ proptest! { }, InitialLiquidity{ account: ALICE, assets: vec![ - AssetLiquidity{ + AssetBalance{ asset_id: asset_a, amount: initial_liquidity }, - AssetLiquidity{ + AssetBalance{ asset_id: asset_b, amount: initial_liquidity } diff --git a/pallets/stableswap/src/tests/mock.rs b/pallets/stableswap/src/tests/mock.rs index 89afabc41..75c4109cc 100644 --- a/pallets/stableswap/src/tests/mock.rs +++ b/pallets/stableswap/src/tests/mock.rs @@ -185,7 +185,7 @@ impl Config for Test { pub struct InitialLiquidity { pub(crate) account: AccountId, - pub(crate) assets: Vec>, + pub(crate) assets: Vec>, } pub struct ExtBuilder { @@ -300,7 +300,7 @@ impl ExtBuilder { } } -use crate::types::{AssetLiquidity, PoolInfo}; +use crate::types::{AssetBalance, PoolInfo}; use hydradx_traits::pools::DustRemovalAccountWhitelist; use hydradx_traits::{AccountIdFor, Registry, ShareTokenRegistry}; use sp_runtime::traits::Zero; diff --git a/pallets/stableswap/src/tests/remove_liquidity.rs b/pallets/stableswap/src/tests/remove_liquidity.rs index 110f70801..c77f86e59 100644 --- a/pallets/stableswap/src/tests/remove_liquidity.rs +++ b/pallets/stableswap/src/tests/remove_liquidity.rs @@ -1,5 +1,5 @@ use crate::tests::mock::*; -use crate::types::{AssetLiquidity, PoolInfo}; +use crate::types::{AssetBalance, PoolInfo}; use crate::{assert_balance, Error}; use frame_support::{assert_noop, assert_ok}; use sp_runtime::Permill; @@ -35,15 +35,15 @@ fn remove_liquidity_should_work_when_withdrawing_all_shares() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 200 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_c, amount: 300 * ONE, }, @@ -61,7 +61,7 @@ fn remove_liquidity_should_work_when_withdrawing_all_shares() { assert_ok!(Stableswap::add_liquidity( RuntimeOrigin::signed(BOB), pool_id, - vec![AssetLiquidity { + vec![AssetBalance { asset_id: asset_a, amount: amount_added },] @@ -74,6 +74,7 @@ fn remove_liquidity_should_work_when_withdrawing_all_shares() { pool_id, asset_c, shares, + 0, )); let amount_received = Tokens::free_balance(asset_c, &BOB); @@ -115,15 +116,15 @@ fn remove_liquidity_should_apply_fee_when_withdrawing_all_shares() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 200 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_c, amount: 300 * ONE, }, @@ -141,7 +142,7 @@ fn remove_liquidity_should_apply_fee_when_withdrawing_all_shares() { assert_ok!(Stableswap::add_liquidity( RuntimeOrigin::signed(BOB), pool_id, - vec![AssetLiquidity { + vec![AssetBalance { asset_id: asset_a, amount: amount_added },] @@ -154,6 +155,7 @@ fn remove_liquidity_should_apply_fee_when_withdrawing_all_shares() { pool_id, asset_c, shares, + 0 )); let amount_received = Tokens::free_balance(asset_c, &BOB); @@ -169,7 +171,7 @@ fn remove_liquidity_should_apply_fee_when_withdrawing_all_shares() { fn remove_liquidity_should_fail_when_shares_is_zero() { ExtBuilder::default().build().execute_with(|| { assert_noop!( - Stableswap::remove_liquidity_one_asset(RuntimeOrigin::signed(ALICE), 0u32, 1u32, 0u128), + Stableswap::remove_liquidity_one_asset(RuntimeOrigin::signed(ALICE), 0u32, 1u32, 0u128, 0), Error::::InvalidAssetAmount ); }); @@ -183,7 +185,7 @@ fn remove_liquidity_should_fail_when_shares_is_insufficient() { .build() .execute_with(|| { assert_noop!( - Stableswap::remove_liquidity_one_asset(RuntimeOrigin::signed(BOB), pool_id, 1u32, 200 * ONE), + Stableswap::remove_liquidity_one_asset(RuntimeOrigin::signed(BOB), pool_id, 1u32, 200 * ONE, 0), Error::::InsufficientShares ); }); @@ -201,7 +203,8 @@ fn remove_liquidity_should_fail_when_remaining_shares_is_below_min_limit() { RuntimeOrigin::signed(BOB), pool_id, 1u32, - 100 * ONE - MinimumLiquidity::get() + 1 + 100 * ONE - MinimumLiquidity::get() + 1, + 0, ), Error::::InsufficientShareBalance ); @@ -216,7 +219,7 @@ fn remove_liquidity_should_fail_when_pool_does_not_exists() { .build() .execute_with(|| { assert_noop!( - Stableswap::remove_liquidity_one_asset(RuntimeOrigin::signed(BOB), pool_id, 1u32, 100 * ONE), + Stableswap::remove_liquidity_one_asset(RuntimeOrigin::signed(BOB), pool_id, 1u32, 100 * ONE, 0), Error::::PoolNotFound ); }); @@ -253,15 +256,15 @@ fn remove_liquidity_should_fail_when_requested_asset_not_in_pool() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 200 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_c, amount: 300 * ONE, }, @@ -277,7 +280,7 @@ fn remove_liquidity_should_fail_when_requested_asset_not_in_pool() { assert_ok!(Stableswap::add_liquidity( RuntimeOrigin::signed(BOB), pool_id, - vec![AssetLiquidity { + vec![AssetBalance { asset_id: asset_a, amount: amount_added },] @@ -286,7 +289,7 @@ fn remove_liquidity_should_fail_when_requested_asset_not_in_pool() { let shares = Tokens::free_balance(pool_id, &BOB); assert_noop!( - Stableswap::remove_liquidity_one_asset(RuntimeOrigin::signed(BOB), pool_id, asset_d, shares,), + Stableswap::remove_liquidity_one_asset(RuntimeOrigin::signed(BOB), pool_id, asset_d, shares, 0), Error::::AssetNotInPool ); }); @@ -322,15 +325,15 @@ fn remove_liquidity_should_fail_when_remaining_shares_below_min_liquidity() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 200 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_c, amount: 300 * ONE, }, @@ -346,7 +349,7 @@ fn remove_liquidity_should_fail_when_remaining_shares_below_min_liquidity() { assert_ok!(Stableswap::add_liquidity( RuntimeOrigin::signed(BOB), pool_id, - vec![AssetLiquidity { + vec![AssetBalance { asset_id: asset_a, amount: amount_added },] @@ -360,6 +363,7 @@ fn remove_liquidity_should_fail_when_remaining_shares_below_min_liquidity() { pool_id, asset_c, shares - MinimumLiquidity::get() + 1, + 0, ), Error::::InsufficientShareBalance ); @@ -399,19 +403,19 @@ fn verify_remove_liquidity_against_research_impl() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 1_000_000 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 1_000_000 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_c, amount: 1_000_000 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_d, amount: 1_000_000 * ONE, }, @@ -429,7 +433,7 @@ fn verify_remove_liquidity_against_research_impl() { assert_ok!(Stableswap::add_liquidity( RuntimeOrigin::signed(BOB), pool_id, - vec![AssetLiquidity { + vec![AssetBalance { asset_id: asset_a, amount: amount_added },] @@ -442,6 +446,7 @@ fn verify_remove_liquidity_against_research_impl() { pool_id, asset_b, shares, + 0 )); let amount_received = Tokens::free_balance(asset_b, &BOB); @@ -453,3 +458,70 @@ fn verify_remove_liquidity_against_research_impl() { assert_balance!(pool_account, asset_b, 900152793953094461); }); } + +#[test] +fn remove_liquidity_fail_when_desired_min_limit_is_not_reached() { + let asset_a: AssetId = 1; + let asset_b: AssetId = 2; + let asset_c: AssetId = 3; + + ExtBuilder::default() + .with_endowed_accounts(vec![ + (BOB, asset_a, 200 * ONE), + (ALICE, asset_a, 100 * ONE), + (ALICE, asset_b, 200 * ONE), + (ALICE, asset_c, 300 * ONE), + ]) + .with_registered_asset("one".as_bytes().to_vec(), asset_a) + .with_registered_asset("two".as_bytes().to_vec(), asset_b) + .with_registered_asset("three".as_bytes().to_vec(), asset_c) + .with_pool( + ALICE, + PoolInfo:: { + assets: vec![asset_a, asset_b, asset_c].try_into().unwrap(), + initial_amplification: NonZeroU16::new(100).unwrap(), + final_amplification: NonZeroU16::new(100).unwrap(), + initial_block: 0, + final_block: 0, + trade_fee: Permill::from_percent(0), + withdraw_fee: Permill::from_percent(0), + }, + InitialLiquidity { + account: ALICE, + assets: vec![ + AssetBalance { + asset_id: asset_a, + amount: 100 * ONE, + }, + AssetBalance { + asset_id: asset_b, + amount: 200 * ONE, + }, + AssetBalance { + asset_id: asset_c, + amount: 300 * ONE, + }, + ], + }, + ) + .build() + .execute_with(|| { + let pool_id = get_pool_id_at(0); + let amount_added = 200 * ONE; + + assert_ok!(Stableswap::add_liquidity( + RuntimeOrigin::signed(BOB), + pool_id, + vec![AssetBalance { + asset_id: asset_a, + amount: amount_added + },] + )); + + let shares = Tokens::free_balance(pool_id, &BOB); + assert_noop!( + Stableswap::remove_liquidity_one_asset(RuntimeOrigin::signed(BOB), pool_id, asset_c, shares, 200 * ONE,), + Error::::MinimumAmountNotReached + ); + }); +} diff --git a/pallets/stableswap/src/tests/trades.rs b/pallets/stableswap/src/tests/trades.rs index f4909cad5..59f2fdae7 100644 --- a/pallets/stableswap/src/tests/trades.rs +++ b/pallets/stableswap/src/tests/trades.rs @@ -1,5 +1,5 @@ use crate::tests::mock::*; -use crate::types::{AssetLiquidity, PoolInfo}; +use crate::types::{AssetBalance, PoolInfo}; use crate::{assert_balance, Error}; use std::num::NonZeroU16; @@ -28,11 +28,11 @@ fn sell_should_work_when_correct_input_provided() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 100 * ONE, }, @@ -85,11 +85,11 @@ fn buy_should_work_when_correct_input_provided() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 100 * ONE, }, @@ -143,11 +143,11 @@ fn sell_with_fee_should_work_when_correct_input_provided() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 100 * ONE, }, @@ -204,11 +204,11 @@ fn sell_should_work_when_fee_is_small() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 100 * ONE, }, @@ -265,11 +265,11 @@ fn buy_should_work_when_fee_is_set() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 100 * ONE, }, @@ -331,11 +331,11 @@ fn sell_should_fail_when_insufficient_amount_is_provided() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 100 * ONE, }, @@ -424,11 +424,11 @@ fn buy_should_fail_when_insufficient_amount_is_provided() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 100 * ONE, }, diff --git a/pallets/stableswap/src/types.rs b/pallets/stableswap/src/types.rs index c56a27848..dd35ef7f1 100644 --- a/pallets/stableswap/src/types.rs +++ b/pallets/stableswap/src/types.rs @@ -65,7 +65,7 @@ where } #[derive(Debug, Clone, Encode, Decode, PartialEq, Eq, TypeInfo)] -pub struct AssetLiquidity { +pub struct AssetBalance { pub asset_id: AssetId, pub amount: Balance, } From ee66cd46493ebb935b016e65044c1197deab273e Mon Sep 17 00:00:00 2001 From: Martin Date: Fri, 7 Jul 2023 08:47:44 +0200 Subject: [PATCH 2/4] feat: add min limit to remove liquidity --- math/src/stableswap/math.rs | 8 ---- pallets/stableswap/src/lib.rs | 83 ++--------------------------------- 2 files changed, 3 insertions(+), 88 deletions(-) diff --git a/math/src/stableswap/math.rs b/math/src/stableswap/math.rs index 5c5aede68..2d66d58ae 100644 --- a/math/src/stableswap/math.rs +++ b/math/src/stableswap/math.rs @@ -110,14 +110,6 @@ pub fn calculate_shares( } } -pub fn calculate_withdraw_liquidity( - reserves: &[Balance], - shares: Balance, - share_asset_issuance: Balance, -) -> Option> { - None -} - /// Given amount of shares and asset reserves, calculate corresponding amount of selected asset to be withdrawn. pub fn calculate_withdraw_one_asset( reserves: &[Balance], diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index 1c6aba3db..346c0138d 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -520,83 +520,6 @@ pub mod pallet { #[pallet::call_index(4)] #[pallet::weight(::WeightInfo::remove_liquidity_one_asset())] #[transactional] - pub fn remove_liquidity( - origin: OriginFor, - pool_id: T::AssetId, - share_amount: Balance, - min_amount_out: Vec>, - ) -> DispatchResult { - let who = ensure_signed(origin)?; - - ensure!(share_amount > Balance::zero(), Error::::InvalidAssetAmount); - let current_share_balance = T::Currency::free_balance(pool_id, &who); - ensure!(current_share_balance >= share_amount, Error::::InsufficientShares); - ensure!( - current_share_balance == share_amount - || current_share_balance.saturating_sub(share_amount) >= T::MinPoolLiquidity::get(), - Error::::InsufficientShareBalance - ); - - let pool = Pools::::get(pool_id).ok_or(Error::::PoolNotFound)?; - let pool_account = Self::pool_account(pool_id); - let balances = pool.balances::(&pool_account); - let share_issuance = T::Currency::total_issuance(pool_id); - - ensure!( - share_issuance == share_amount - || share_issuance.saturating_sub(share_amount) >= T::MinPoolLiquidity::get(), - Error::::InsufficientLiquidityRemaining - ); - - let amounts = - hydra_dx_math::stableswap::calculate_withdraw_liquidity(&balances, share_amount, share_issuance) - .ok_or(ArithmeticError::Overflow)?; - - let mut min_amounts = min_amount_out.clone(); - min_amounts.sort_by_key(|v| v.asset_id); - for (idx, asset_id) in pool.assets.into_iter().enumerate() { - ensure!( - Self::is_asset_allowed(pool_id, asset_id, Tradability::REMOVE_LIQUIDITY), - Error::::NotAllowed - ); - let min_amount = min_amounts.get_mut(idx).ok_or(Error::::IncorrectAssets)?; - ensure!(min_amount.asset_id == asset_id, Error::::IncorrectAssets); - ensure!(amounts[idx] >= min_amount.amount, Error::::MinimumAmountNotReached); - T::Currency::transfer(asset_id, &pool_account, &who, amounts[idx])?; - min_amount.amount = amounts[idx]; - } - T::Currency::withdraw(pool_id, &who, share_amount)?; - - Self::deposit_event(Event::LiquidityRemoved { - pool_id, - who, - shares: share_amount, - amounts: min_amounts, - fee: 0, - }); - - Ok(()) - } - - /// Remove liquidity from selected pool. - /// - /// Withdraws liquidity of selected asset from a pool. - /// - /// Share amount is burn and LP receives corresponding amount of chosen asset. - /// - /// Withdraw fee is applied to the asset amount. - /// - /// Parameters: - /// - `origin`: liquidity provider - /// - `pool_id`: Pool Id - /// - `asset_id`: id of asset to receive - /// - 'share_amount': amount of shares to withdraw - /// - 'min_amount_out': minimum amount to receive - /// - /// Emits `LiquidityRemoved` event when successful. - #[pallet::call_index(5)] - #[pallet::weight(::WeightInfo::remove_liquidity_one_asset())] - #[transactional] pub fn remove_liquidity_one_asset( origin: OriginFor, pool_id: T::AssetId, @@ -677,7 +600,7 @@ pub mod pallet { /// /// Emits `SellExecuted` event when successful. /// - #[pallet::call_index(6)] + #[pallet::call_index(5)] #[pallet::weight(::WeightInfo::sell())] #[transactional] pub fn sell( @@ -740,7 +663,7 @@ pub mod pallet { /// /// Emits `BuyExecuted` event when successful. /// - #[pallet::call_index(7)] + #[pallet::call_index(6)] #[pallet::weight(::WeightInfo::buy())] #[transactional] pub fn buy( @@ -791,7 +714,7 @@ pub mod pallet { Ok(()) } - #[pallet::call_index(8)] + #[pallet::call_index(7)] #[pallet::weight(::WeightInfo::set_asset_tradable_state())] #[transactional] pub fn set_asset_tradable_state( From 6acbd050829b1f31177e5129d439896c154a346c Mon Sep 17 00:00:00 2001 From: Martin Date: Fri, 7 Jul 2023 15:47:52 +0200 Subject: [PATCH 3/4] fix test --- Cargo.lock | 2 +- pallets/stableswap/src/lib.rs | 5 +---- pallets/stableswap/src/tests/add_liquidity.rs | 8 ++++---- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a85b0f38f..0fca27baa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7316,7 +7316,7 @@ dependencies = [ [[package]] name = "pallet-stableswap" -version = "2.0.1" +version = "2.0.2" dependencies = [ "bitflags", "frame-benchmarking", diff --git a/pallets/stableswap/src/lib.rs b/pallets/stableswap/src/lib.rs index d31c219bf..272e4d69f 100644 --- a/pallets/stableswap/src/lib.rs +++ b/pallets/stableswap/src/lib.rs @@ -578,10 +578,7 @@ pub mod pallet { pool_id, who, shares: share_amount, - amounts: vec![AssetBalance { - asset_id: asset_id, - amount, - }], + amounts: vec![AssetBalance { asset_id, amount }], fee, }); diff --git a/pallets/stableswap/src/tests/add_liquidity.rs b/pallets/stableswap/src/tests/add_liquidity.rs index c1f5c64f7..fae2f2134 100644 --- a/pallets/stableswap/src/tests/add_liquidity.rs +++ b/pallets/stableswap/src/tests/add_liquidity.rs @@ -497,11 +497,11 @@ fn add_liquidity_should_fail_when_provided_list_contains_same_assets() { InitialLiquidity { account: ALICE, assets: vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: 100 * ONE, }, - AssetLiquidity { + AssetBalance { asset_id: asset_b, amount: 100 * ONE, }, @@ -517,11 +517,11 @@ fn add_liquidity_should_fail_when_provided_list_contains_same_assets() { RuntimeOrigin::signed(BOB), pool_id, vec![ - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: amount_added }, - AssetLiquidity { + AssetBalance { asset_id: asset_a, amount: amount_added } From 5e7fcf755ae49f6bdb372732192fa8511792a580 Mon Sep 17 00:00:00 2001 From: Martin Date: Fri, 7 Jul 2023 15:56:09 +0200 Subject: [PATCH 4/4] bump version --- Cargo.lock | 2 +- pallets/stableswap/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0fca27baa..edf9adf3c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7316,7 +7316,7 @@ dependencies = [ [[package]] name = "pallet-stableswap" -version = "2.0.2" +version = "2.1.0" dependencies = [ "bitflags", "frame-benchmarking", diff --git a/pallets/stableswap/Cargo.toml b/pallets/stableswap/Cargo.toml index 9da47e459..4209d600c 100644 --- a/pallets/stableswap/Cargo.toml +++ b/pallets/stableswap/Cargo.toml @@ -1,6 +1,6 @@ [package] name = 'pallet-stableswap' -version = '2.0.2' +version = '2.1.0' description = 'AMM for correlated assets' authors = ['GalacticCouncil'] edition = '2021'