Merge remote-tracking branch 'origin/main' into fix/storage-version-m…

…ismatch

libs/traits/src/swaps.rs

@@ -150,6 +150,21 @@ pub trait Swaps<AccountId> {
 		swap: Swap<Self::Amount, Self::CurrencyId>,
 	) -> Result<SwapStatus<Self::Amount>, 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<AccountId> {
 		swap_id: Self::SwapId,
 		from_currency: Self::CurrencyId,
 	) -> Result<Self::Amount, DispatchError>;
-
-	/// 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<Self::Amount, DispatchError>;
 }

pallets/foreign-investments/src/entities.rs

@@ -164,7 +164,7 @@ impl<T: Config> InvestmentInfo<T> {
 		who: &T::AccountId,
 		investment_id: T::InvestmentId,
 		foreign_amount: T::ForeignBalance,
-	) -> Result<SwapOf<T>, DispatchError> {
+	) -> Result<(T::ForeignBalance, SwapOf<T>), DispatchError> {
 		let pool_currency = pool_currency_of::<T>(investment_id)?;
 
 		// We do not want to decrease the whole `foreign_amount` from the investment
@@ -184,19 +184,16 @@ impl<T: Config> InvestmentInfo<T> {
 
 		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

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<T: Config> ForeignInvestment<T::AccountId> for Pallet<T> {
 			let info = entry.as_mut().ok_or(Error::<T>::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() {

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::<Runtime>::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::<Runtime>::get(&USER, INVESTMENT_ID),
+					None,
+				);
+				assert_eq!(ForeignInvestment::investment(&USER, INVESTMENT_ID), Ok(0));
+				assert_eq!(MockInvestment::investment(&USER, INVESTMENT_ID), Ok(0));
+			});
+		}
 	}
 }
 

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<T: Config> Pallet<T> {
@@ -130,12 +133,12 @@ pub mod pallet {
 		}
 
 		#[allow(clippy::type_complexity)]
-		fn apply_over_swap(
+		fn apply_over(
 			who: &T::AccountId,
 			new_swap: Swap<T::Balance, T::CurrencyId>,
-			over_swap_id: Option<T::OrderId>,
+			over_order_id: Option<T::OrderId>,
 		) -> Result<(SwapStatus<T::Balance>, Option<T::OrderId>), 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<Option<T::OrderId>, DispatchError> {
+			let swap = T::OrderBook::get_order_details(over_order_id)
+				.ok_or(Error::<T>::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::<T>::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::<T>::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::<T>::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<Self::Amount, DispatchError> {
-			T::OrderBook::convert_by_market(currency_in, currency_out, amount_out)
-		}
 	}
 
 	impl<T: Config> StatusNotificationHook for Pallet<T> {