diff --git a/libs/traits/src/swaps.rs b/libs/traits/src/swaps.rs index 32aa2c7b19..ad5367f390 100644 --- a/libs/traits/src/swaps.rs +++ b/libs/traits/src/swaps.rs @@ -150,6 +150,21 @@ pub trait Swaps { swap: Swap, ) -> Result, DispatchError>; + /// Cancel a swap partially or completely. The amount should be expressed in + /// the same currency as the the currency_out of the pending amount. + /// - If there was no previous swap, it errors outs. + /// - If there was a swap with other currency out, it errors outs. + /// - If there was a swap with same currency out: + /// - If the amount is smaller, it decrements it. + /// - If the amount is the same, it removes the inverse swap. + /// - If the amount is greater, it errors out + fn cancel_swap( + who: &AccountId, + swap_id: Self::SwapId, + amount: Self::Amount, + currency_id: Self::CurrencyId, + ) -> DispatchResult; + /// Returns the pending amount for a pending swap. The direction of the swap /// is determined by the `from_currency` parameter. The amount returned is /// denominated in the same currency as the given `from_currency`. @@ -158,13 +173,4 @@ pub trait Swaps { swap_id: Self::SwapId, from_currency: Self::CurrencyId, ) -> Result; - - /// Makes a conversion between 2 currencies using the market ratio between - /// them - // TODO: Should be removed after #1723 - fn convert_by_market( - currency_in: Self::CurrencyId, - currency_out: Self::CurrencyId, - amount_out: Self::Amount, - ) -> Result; } diff --git a/pallets/foreign-investments/src/entities.rs b/pallets/foreign-investments/src/entities.rs index eed4c104ea..796a4fce0a 100644 --- a/pallets/foreign-investments/src/entities.rs +++ b/pallets/foreign-investments/src/entities.rs @@ -164,7 +164,7 @@ impl InvestmentInfo { who: &T::AccountId, investment_id: T::InvestmentId, foreign_amount: T::ForeignBalance, - ) -> Result, DispatchError> { + ) -> Result<(T::ForeignBalance, SwapOf), DispatchError> { let pool_currency = pool_currency_of::(investment_id)?; // We do not want to decrease the whole `foreign_amount` from the investment @@ -184,19 +184,16 @@ impl InvestmentInfo { self.decrease_investment(who, investment_id, pool_investment_decrement)?; - // It's ok to use the market ratio because this amount will be - // cancelled in this instant. - let increasing_pool_amount = T::Swaps::convert_by_market( - pool_currency, - self.foreign_currency, - min(foreign_amount, increasing_foreign_amount).into(), - )?; + let cancelation_amount = min(foreign_amount, increasing_foreign_amount); - Ok(Swap { - currency_in: self.foreign_currency, - currency_out: pool_currency, - amount_out: increasing_pool_amount.ensure_add(pool_investment_decrement.into())?, - }) + Ok(( + cancelation_amount, + Swap { + currency_in: self.foreign_currency, + currency_out: pool_currency, + amount_out: pool_investment_decrement.into(), + }, + )) } /// This method is performed after resolve the swap diff --git a/pallets/foreign-investments/src/impls.rs b/pallets/foreign-investments/src/impls.rs index af92f86b0e..0f5005395d 100644 --- a/pallets/foreign-investments/src/impls.rs +++ b/pallets/foreign-investments/src/impls.rs @@ -7,7 +7,7 @@ use cfg_traits::{ }; use cfg_types::investments::CollectedAmount; use frame_support::pallet_prelude::*; -use sp_runtime::traits::{EnsureAdd, EnsureSub, Zero}; +use sp_runtime::traits::{EnsureAdd, EnsureAddAssign, EnsureSub, Zero}; use sp_std::marker::PhantomData; use crate::{ @@ -80,9 +80,13 @@ impl ForeignInvestment for Pallet { let info = entry.as_mut().ok_or(Error::::InfoNotFound)?; info.ensure_same_foreign(foreign_currency)?; - let swap = info.pre_decrease_swap(who, investment_id, foreign_amount)?; + let (cancelation, swap) = info.pre_decrease_swap(who, investment_id, foreign_amount)?; + let swap_id = (investment_id, Action::Investment); - let status = T::Swaps::apply_swap(who, swap_id, swap.clone())?; + T::Swaps::cancel_swap(who, swap_id, cancelation.into(), foreign_currency)?; + let mut status = T::Swaps::apply_swap(who, swap_id, swap.clone())?; + + status.swapped.ensure_add_assign(cancelation.into())?; let mut msg = None; if !status.swapped.is_zero() { diff --git a/pallets/foreign-investments/src/tests.rs b/pallets/foreign-investments/src/tests.rs index 71c568fd04..673240f41c 100644 --- a/pallets/foreign-investments/src/tests.rs +++ b/pallets/foreign-investments/src/tests.rs @@ -1067,6 +1067,119 @@ mod investment { assert_eq!(MockInvestment::investment(&USER, INVESTMENT_ID), Ok(0)); }); } + + #[test] + fn decrease_with_asymmetric_ratios_higher_decrease() { + const MULTIPLIER: Balance = 1000; + + new_test_ext().execute_with(|| { + util::base_configuration(); + + // We override the market with asymmetric ratios + MockTokenSwaps::mock_convert_by_market(|to, from, amount_from| { + Ok(match (from, to) { + (POOL_CURR, FOREIGN_CURR) => pool_to_foreign(amount_from) * MULTIPLIER, + (FOREIGN_CURR, POOL_CURR) => foreign_to_pool(amount_from), + _ => amount_from, + }) + }); + + assert_ok!(ForeignInvestment::increase_foreign_investment( + &USER, + INVESTMENT_ID, + AMOUNT, + FOREIGN_CURR + )); + + util::fulfill_last_swap(Action::Investment, 3 * AMOUNT / 4); + + assert_ok!(ForeignInvestment::decrease_foreign_investment( + &USER, + INVESTMENT_ID, + AMOUNT, + FOREIGN_CURR + )); + + MockDecreaseInvestHook::mock_notify_status_change(|_, msg| { + assert_eq!( + msg, + ExecutedForeignDecreaseInvest { + amount_decreased: (3 * AMOUNT / 4) * MULTIPLIER + AMOUNT / 4, + foreign_currency: FOREIGN_CURR, + amount_remaining: 0, + } + ); + Ok(()) + }); + + util::fulfill_last_swap(Action::Investment, foreign_to_pool(3 * AMOUNT / 4)); + + assert_eq!( + ForeignInvestmentInfo::::get(&USER, INVESTMENT_ID), + None, + ); + assert_eq!(ForeignInvestment::investment(&USER, INVESTMENT_ID), Ok(0)); + assert_eq!(MockInvestment::investment(&USER, INVESTMENT_ID), Ok(0)); + }); + } + + #[test] + fn decrease_with_asymmetric_ratios_lower_increase() { + const MULTIPLIER: Balance = 1000; + + new_test_ext().execute_with(|| { + util::base_configuration(); + + // We override the market with asymmetric ratios + MockTokenSwaps::mock_convert_by_market(|to, from, amount_from| { + Ok(match (from, to) { + (POOL_CURR, FOREIGN_CURR) => pool_to_foreign(amount_from), + (FOREIGN_CURR, POOL_CURR) => foreign_to_pool(amount_from) * MULTIPLIER, + _ => amount_from, + }) + }); + + assert_ok!(ForeignInvestment::increase_foreign_investment( + &USER, + INVESTMENT_ID, + AMOUNT, + FOREIGN_CURR + )); + + util::fulfill_last_swap(Action::Investment, 3 * AMOUNT / 4); + + assert_ok!(ForeignInvestment::decrease_foreign_investment( + &USER, + INVESTMENT_ID, + AMOUNT, + FOREIGN_CURR + )); + + MockDecreaseInvestHook::mock_notify_status_change(|_, msg| { + assert_eq!( + msg, + ExecutedForeignDecreaseInvest { + amount_decreased: (3 * AMOUNT / 4) * MULTIPLIER + AMOUNT / 4, + foreign_currency: FOREIGN_CURR, + amount_remaining: 0, + } + ); + Ok(()) + }); + + util::fulfill_last_swap( + Action::Investment, + foreign_to_pool(3 * AMOUNT / 4) * MULTIPLIER, + ); + + assert_eq!( + ForeignInvestmentInfo::::get(&USER, INVESTMENT_ID), + None, + ); + assert_eq!(ForeignInvestment::investment(&USER, INVESTMENT_ID), Ok(0)); + assert_eq!(MockInvestment::investment(&USER, INVESTMENT_ID), Ok(0)); + }); + } } } diff --git a/pallets/swaps/src/lib.rs b/pallets/swaps/src/lib.rs index d8002151e7..0afb1a8d58 100644 --- a/pallets/swaps/src/lib.rs +++ b/pallets/swaps/src/lib.rs @@ -93,6 +93,9 @@ pub mod pallet { /// Failed to retrieve the swap. SwapNotFound, + + /// Emitted when the cancelled amount is greater than the pending amount + CancelMoreThanPending, } impl Pallet { @@ -130,12 +133,12 @@ pub mod pallet { } #[allow(clippy::type_complexity)] - fn apply_over_swap( + fn apply_over( who: &T::AccountId, new_swap: Swap, - over_swap_id: Option, + over_order_id: Option, ) -> Result<(SwapStatus, Option), DispatchError> { - match over_swap_id { + match over_order_id { None => { let order_id = T::OrderBook::place_order( who.clone(), @@ -241,6 +244,39 @@ pub mod pallet { } } } + + fn cancel_over( + cancel_amount: T::Balance, + currency_id: T::CurrencyId, + over_order_id: T::OrderId, + ) -> Result, DispatchError> { + let swap = T::OrderBook::get_order_details(over_order_id) + .ok_or(Error::::OrderNotFound)? + .swap; + + if swap.currency_out == currency_id { + match swap.amount_out.cmp(&cancel_amount) { + Ordering::Greater => { + let amount_to_swap = swap.amount_out.ensure_sub(cancel_amount)?; + T::OrderBook::update_order( + over_order_id, + amount_to_swap, + OrderRatio::Market, + )?; + + Ok(Some(over_order_id)) + } + Ordering::Equal => { + T::OrderBook::cancel_order(over_order_id)?; + + Ok(None) + } + Ordering::Less => Err(Error::::CancelMoreThanPending)?, + } + } else { + Ok(Some(over_order_id)) //Noop + } + } } /// Trait to perform swaps without handling directly an order book @@ -264,13 +300,28 @@ pub mod pallet { let previous_order_id = SwapIdToOrderId::::get((who, swap_id)); - let (status, new_order_id) = Self::apply_over_swap(who, swap, previous_order_id)?; + let (status, new_order_id) = Self::apply_over(who, swap, previous_order_id)?; Self::update_id(who, swap_id, new_order_id)?; Ok(status) } + fn cancel_swap( + who: &T::AccountId, + swap_id: Self::SwapId, + balance: T::Balance, + currency_id: T::CurrencyId, + ) -> DispatchResult { + match SwapIdToOrderId::::get((who, swap_id)) { + Some(previous_order_id) => { + let order_id = Self::cancel_over(balance, currency_id, previous_order_id)?; + Self::update_id(who, swap_id, order_id) + } + None => Ok(()), // Noop + } + } + fn pending_amount( who: &T::AccountId, swap_id: Self::SwapId, @@ -282,14 +333,6 @@ pub mod pallet { .map(|order_info| order_info.swap.amount_out) .unwrap_or_default()) } - - fn convert_by_market( - currency_in: Self::CurrencyId, - currency_out: Self::CurrencyId, - amount_out: Self::Amount, - ) -> Result { - T::OrderBook::convert_by_market(currency_in, currency_out, amount_out) - } } impl StatusNotificationHook for Pallet { diff --git a/pallets/swaps/src/tests.rs b/pallets/swaps/src/tests.rs index 047cd5e23e..9a49ef5206 100644 --- a/pallets/swaps/src/tests.rs +++ b/pallets/swaps/src/tests.rs @@ -2,7 +2,7 @@ use cfg_traits::{ swaps::{OrderInfo, OrderRatio, Swap, SwapState, SwapStatus, Swaps as TSwaps}, StatusNotificationHook, }; -use frame_support::assert_ok; +use frame_support::{assert_err, assert_ok}; use crate::{mock::*, *}; @@ -24,6 +24,17 @@ pub const fn b_to_a(amount_b: Balance) -> Balance { amount_b / RATIO } +fn assert_swap_id_registered(order_id: OrderId) { + assert_eq!( + OrderIdToSwapId::::get(order_id), + Some((USER, SWAP_ID)) + ); + assert_eq!( + SwapIdToOrderId::::get((USER, SWAP_ID)), + Some(order_id) + ); +} + mod util { use super::*; @@ -36,20 +47,9 @@ mod util { } } -mod swaps { +mod apply { use super::*; - fn assert_swap_id_registered(order_id: OrderId) { - assert_eq!( - OrderIdToSwapId::::get(order_id), - Some((USER, SWAP_ID)) - ); - assert_eq!( - SwapIdToOrderId::::get((USER, SWAP_ID)), - Some(order_id) - ); - } - #[test] fn swap_over_no_swap() { new_test_ext().execute_with(|| { @@ -289,6 +289,155 @@ mod swaps { } } +mod cancel { + use super::*; + + #[test] + fn swap_over_no_swap() { + new_test_ext().execute_with(|| { + assert_ok!(>::cancel_swap( + &USER, SWAP_ID, AMOUNT, CURRENCY_A + )); + }); + } + + #[test] + fn swap_over_other_currency() { + new_test_ext().execute_with(|| { + MockTokenSwaps::mock_get_order_details(move |swap_id| { + assert_eq!(swap_id, ORDER_ID); + + Some(OrderInfo { + swap: Swap { + currency_in: CURRENCY_B, + currency_out: CURRENCY_A, + amount_out: AMOUNT, + }, + ratio: OrderRatio::Market, + }) + }); + + Swaps::update_id(&USER, SWAP_ID, Some(ORDER_ID)).unwrap(); + + assert_ok!(>::cancel_swap( + &USER, + SWAP_ID, + a_to_b(AMOUNT), + CURRENCY_B + )); + + assert_swap_id_registered(ORDER_ID); + }); + } + + #[test] + fn swap_over_greater_swap() { + const PREVIOUS_AMOUNT: Balance = AMOUNT + b_to_a(50); + + new_test_ext().execute_with(|| { + MockTokenSwaps::mock_get_order_details(|swap_id| { + assert_eq!(swap_id, ORDER_ID); + + // Inverse swap + Some(OrderInfo { + swap: Swap { + currency_in: CURRENCY_A, + currency_out: CURRENCY_B, + amount_out: a_to_b(PREVIOUS_AMOUNT), + }, + ratio: OrderRatio::Market, + }) + }); + MockTokenSwaps::mock_update_order(|swap_id, amount, ratio| { + assert_eq!(swap_id, ORDER_ID); + assert_eq!(amount, a_to_b(PREVIOUS_AMOUNT - AMOUNT)); + assert_eq!(ratio, OrderRatio::Market); + + Ok(()) + }); + + Swaps::update_id(&USER, SWAP_ID, Some(ORDER_ID)).unwrap(); + + assert_ok!(>::cancel_swap( + &USER, + SWAP_ID, + a_to_b(AMOUNT), + CURRENCY_B + )); + + assert_swap_id_registered(ORDER_ID); + }); + } + + #[test] + fn swap_over_same_swap() { + new_test_ext().execute_with(|| { + MockTokenSwaps::mock_get_order_details(|swap_id| { + assert_eq!(swap_id, ORDER_ID); + + // Inverse swap + Some(OrderInfo { + swap: Swap { + currency_in: CURRENCY_A, + currency_out: CURRENCY_B, + amount_out: a_to_b(AMOUNT), + }, + ratio: OrderRatio::Market, + }) + }); + MockTokenSwaps::mock_cancel_order(|swap_id| { + assert_eq!(swap_id, ORDER_ID); + Ok(()) + }); + + Swaps::update_id(&USER, SWAP_ID, Some(ORDER_ID)).unwrap(); + + assert_ok!(>::cancel_swap( + &USER, + SWAP_ID, + a_to_b(AMOUNT), + CURRENCY_B, + ),); + + assert_eq!(OrderIdToSwapId::::get(ORDER_ID), None); + assert_eq!(SwapIdToOrderId::::get((USER, SWAP_ID)), None); + }); + } + + #[test] + fn swap_over_smaller_swap() { + const PREVIOUS_AMOUNT: Balance = AMOUNT - b_to_a(50); + + new_test_ext().execute_with(|| { + MockTokenSwaps::mock_get_order_details(|swap_id| { + assert_eq!(swap_id, ORDER_ID); + + // Inverse swap + Some(OrderInfo { + swap: Swap { + currency_in: CURRENCY_A, + currency_out: CURRENCY_B, + amount_out: a_to_b(PREVIOUS_AMOUNT), + }, + ratio: OrderRatio::Market, + }) + }); + + Swaps::update_id(&USER, SWAP_ID, Some(ORDER_ID)).unwrap(); + + assert_err!( + >::cancel_swap( + &USER, + SWAP_ID, + a_to_b(AMOUNT), + CURRENCY_B + ), + Error::::CancelMoreThanPending + ); + }); + } +} + mod fulfill { use super::*;