Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FI: Add swapping events #1764

Merged
merged 9 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions libs/mocks/src/token_swaps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ pub mod pallet {
register_call!(move |(a, b, c)| f(a, b, c));
}

pub fn mock_market_ratio(
f: impl Fn(T::CurrencyId, T::CurrencyId) -> Result<T::Ratio, DispatchError> + 'static,
) {
register_call!(move |(a, b)| f(a, b));
}

pub fn mock_fill_order(
f: impl Fn(T::AccountId, T::OrderId, T::BalanceOut) -> DispatchResult + 'static,
) {
Expand Down Expand Up @@ -120,6 +126,13 @@ pub mod pallet {
execute_call!((a, b, c))
}

fn market_ratio(
a: Self::CurrencyId,
b: Self::CurrencyId,
) -> Result<Self::Ratio, DispatchError> {
execute_call!((a, b))
}

fn fill_order(a: T::AccountId, b: Self::OrderId, c: Self::BalanceOut) -> DispatchResult {
execute_call!((a, b, c))
}
Expand Down
11 changes: 10 additions & 1 deletion libs/traits/src/swaps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,17 @@ pub trait TokenSwaps<Account> {
currency_out: Self::CurrencyId,
amount_out: Self::BalanceOut,
) -> Result<Self::BalanceIn, DispatchError>;

/// Returns the conversion ratio to convert currency out into currency in,
fn market_ratio(
currency_in: Self::CurrencyId,
currency_out: Self::CurrencyId,
) -> Result<Self::Ratio, DispatchError>;
}

/// A representation of a currency swap in process.
#[derive(Clone, PartialEq, Eq, Debug, Encode, Decode, TypeInfo, MaxEncodedLen)]
pub struct SwapState<AmountIn, AmountOut, Currency> {
pub struct SwapInfo<AmountIn, AmountOut, Currency, Ratio> {
/// Swap not yet processed with the pending outcomming amount
pub remaining: Swap<AmountOut, Currency>,

Expand All @@ -112,6 +118,9 @@ pub struct SwapState<AmountIn, AmountOut, Currency> {
/// Amount of incoming currency already swapped denominated in outgoing
/// currency
pub swapped_out: AmountOut,

/// Ratio used to swap `swapped_out` into `swapped_in`
pub ratio: Ratio,
Comment on lines +122 to +123
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OrderInfo returned by TokenSwaps::get_order_details contains a ratio field of type OrderRatio<Ratio>. Maybe that suffices such that we don't need to introduce an additional ratio field?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OrderRatio is the ratio you want to configure: the way the order behaves. The ratio here is the ratio value itself. It differs, i.e., when OrderRatio::Market, we do not know the ratio value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation! Makes sense to keep as is then.

}

/// Used as result of `Pallet::apply_swap()`
Expand Down
69 changes: 59 additions & 10 deletions pallets/foreign-investments/src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use cfg_traits::{
investments::{ForeignInvestment, Investment, InvestmentCollector},
swaps::{SwapState, Swaps},
swaps::{Swap, SwapInfo, SwapStatus, Swaps},
StatusNotificationHook,
};
use cfg_types::investments::CollectedAmount;
Expand All @@ -12,8 +12,8 @@ use sp_std::marker::PhantomData;

use crate::{
entities::{InvestmentInfo, RedemptionInfo},
pallet::{Config, Error, ForeignInvestmentInfo, ForeignRedemptionInfo, Pallet},
pool_currency_of, Action, SwapId,
pallet::{Config, Error, Event, ForeignInvestmentInfo, ForeignRedemptionInfo, Pallet},
pool_currency_of, Action, SwapId, SwapOf,
};

impl<T: Config> ForeignInvestment<T::AccountId> for Pallet<T> {
Expand Down Expand Up @@ -57,6 +57,8 @@ impl<T: Config> ForeignInvestment<T::AccountId> for Pallet<T> {
}
}

Pallet::<T>::deposit_apply_swap_events(who, swap_id, &swap, &status)?;

Ok::<_, DispatchError>(msg)
})?;

Expand Down Expand Up @@ -108,6 +110,8 @@ impl<T: Config> ForeignInvestment<T::AccountId> for Pallet<T> {
}
}

Pallet::<T>::deposit_apply_swap_events(who, swap_id, &swap, &status)?;

if info.is_completed(who, investment_id)? {
*entry = None;
}
Expand Down Expand Up @@ -209,23 +213,66 @@ impl<T: Config> ForeignInvestment<T::AccountId> for Pallet<T> {
}
}

impl<T: Config> Pallet<T> {
fn deposit_apply_swap_events(
who: &T::AccountId,
swap_id: SwapId<T>,
swap: &SwapOf<T>,
status: &SwapStatus<T::SwapBalance>,
) -> DispatchResult {
if !status.swapped.is_zero() {
Pallet::<T>::deposit_event(Event::SwapCancelled {
who: who.clone(),
swap_id,
remaining: Swap {
amount_out: status.pending,
..swap.clone()
},
Comment on lines +227 to +230
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that ..swap.clone() contains the currency_in and currency_out to know the swap direction.

cancelled_in: status.swapped,
opposite_in: T::Swaps::pending_amount(who, swap_id, swap.currency_in)?,
});
}

if !status.pending.is_zero() {
Pallet::<T>::deposit_event(Event::SwapCreated {
who: who.clone(),
swap_id,
swap: Swap {
amount_out: status.pending,
..swap.clone()
},
})
}
Comment on lines +223 to +245
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it be that both status.swapped and status.pending are non zero?

IF YES:

  • How would one interpret the swap_id of the two events?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it could. swap_id will be the same for both. Note that swap_id is just a tuple (T::InvestmentId, Action) not an order_id (which is a number from order-book).

If the above two events are dispatched, it means that when you apply a swap, part of the amount was automatically fulfilled by cancelation with an inverse order (notified by SwapCancelled event), and part of the swap could not be fulfilled with the inverse order: such remaining amount needs to be swapped (notified it by SwapCreated event).

Possible states after applying swaps are:

  • pending_amount == 0 and swapped amount > 0 => it means applying a swap has partially or fully canceled any previous inverse swaps. The required amount is obtained from the inverse amount. No more amount needs to be swapped. OrderBook will Not create a new order.
  • pending amount > 0 and swapped amount > 0 => It means applying a swap has fully canceled any previous inverse swaps, but more amount needs to be swapped. Orderbook will create a new order.
  • pending amount > 0 and swapped amount == 0 => There was no inverse order, so we need to create a new order.

NOTE: Probably, a better name for swap_status.swapped is swap_status.canceled. It all depends on how you interpret it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot, that helped!


Ok(())
}
}

pub struct FulfilledSwapHook<T>(PhantomData<T>);
impl<T: Config> StatusNotificationHook for FulfilledSwapHook<T> {
type Error = DispatchError;
type Id = (T::AccountId, SwapId<T>);
type Status = SwapState<T::SwapBalance, T::SwapBalance, T::CurrencyId>;
type Status = SwapInfo<T::SwapBalance, T::SwapBalance, T::CurrencyId, T::SwapRatio>;

fn notify_status_change(
(who, (investment_id, action)): Self::Id,
swap_state: Self::Status,
swap_info: Self::Status,
) -> DispatchResult {
let pool_currency = pool_currency_of::<T>(investment_id)?;
let swapped_amount_in = swap_state.swapped_in;
let swapped_amount_out = swap_state.swapped_out;
let pending_amount = swap_state.remaining.amount_out;
let swapped_amount_in = swap_info.swapped_in;
let swapped_amount_out = swap_info.swapped_out;
let pending_amount = swap_info.remaining.amount_out;

Pallet::<T>::deposit_event(Event::SwapFullfilled {
who: who.clone(),
swap_id: (investment_id, action),
remaining: swap_info.remaining.clone(),
swapped_in: swap_info.swapped_in,
swapped_out: swap_info.swapped_out,
});

match action {
Action::Investment => match pool_currency == swap_state.remaining.currency_in {
Action::Investment => match pool_currency == swap_info.remaining.currency_in {
true => SwapDone::<T>::for_increase_investment(
&who,
investment_id,
Expand Down Expand Up @@ -308,7 +355,9 @@ impl<T: Config> StatusNotificationHook for CollectedRedemptionHook<T> {

if let Some(swap) = swap {
let swap_id = (investment_id, Action::Redemption);
let status = T::Swaps::apply_swap(&who, swap_id, swap)?;
let status = T::Swaps::apply_swap(&who, swap_id, swap.clone())?;

Pallet::<T>::deposit_apply_swap_events(&who, swap_id, &swap, &status)?;

if !status.swapped.is_zero() {
SwapDone::<T>::for_redemption(
Expand Down
34 changes: 33 additions & 1 deletion pallets/foreign-investments/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ pub mod pallet {
};
use cfg_types::investments::{ExecutedForeignCollect, ExecutedForeignDecreaseInvest};
use frame_support::pallet_prelude::*;
use sp_runtime::traits::AtLeast32BitUnsigned;
use sp_runtime::traits::{AtLeast32BitUnsigned, One};

use super::*;

Expand All @@ -116,6 +116,8 @@ pub mod pallet {
/// depends.
#[pallet::config]
pub trait Config: frame_system::Config {
type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;

/// Represents a foreign amount
type ForeignBalance: Parameter
+ Member
Expand Down Expand Up @@ -149,6 +151,9 @@ pub mod pallet {
/// Any balances used in TokenSwaps
type SwapBalance: Parameter + Member + AtLeast32BitUnsigned + Default + Copy + MaxEncodedLen;

/// Ratio used for swapping amounts
type SwapRatio: Parameter + Member + Copy + MaxEncodedLen + One;

/// The currency type of transferrable tokens
type CurrencyId: Parameter + Member + Copy + MaxEncodedLen;

Expand Down Expand Up @@ -263,4 +268,31 @@ pub mod pallet {
/// The decrease is greater than the current investment/redemption
TooMuchDecrease,
}

#[pallet::event]
#[pallet::generate_deposit(pub(super) fn deposit_event)]
pub enum Event<T: Config> {
// The swap is created and now is wating to be fulfilled
SwapCreated {
who: T::AccountId,
swap_id: SwapId<T>,
swap: SwapOf<T>,
},
// The swap was fulfilled by another participant.
SwapFullfilled {
who: T::AccountId,
swap_id: SwapId<T>,
remaining: SwapOf<T>,
swapped_in: T::SwapBalance,
swapped_out: T::SwapBalance,
},
// The swap was fulfilled by cancelling an opposite swap for the same foreign investment.
SwapCancelled {
who: T::AccountId,
swap_id: SwapId<T>,
remaining: SwapOf<T>,
cancelled_in: T::SwapBalance,
opposite_in: T::SwapBalance,
},
}
Comment on lines +272 to +297
Copy link
Contributor Author

@lemunozm lemunozm Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hieronx I've needed to add a new SwapCancelled. All events are seen from the perspective of who did the swap.

Wouldn’t it possible to add an optional arg to the decrease fulfilled event, with the swap ID that was used to cancel it out?

This can not be possible due that swap ID is the same for both directions. Note that swap id is not the order id, which is unknown from FI perspective. As a workaround, I've added the opposite_in field to SwapCancelled, which has the amount in the opposite direction that has not been canceled. That way, if opposite_in is 0, that means that the cancellation has also canceled the opposite order. That is what you need to know.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Only one nit: in SwapFulfilled, wouldn't it make more sense to have the swapped_out param before the swapped_in param?

Copy link
Contributor Author

@lemunozm lemunozm Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by "before" in this context? Placed in swapped line numbers? Probably, it makes more sense. But right now, we have such conventions in all places in the codebase. I'd prefer to keep it as it is everywhere (first param in, second param out). But we should probably look into it soon 👍🏻

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well we should never change these event definitions again after merging this, at least not without a very strong reason ;) Would be a pain on the subquery side. But sure it is fine to keep it as is then if that is the convention now

}
6 changes: 5 additions & 1 deletion pallets/foreign-investments/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,9 @@ impl pallet_foreign_investments::Config for Runtime {
type InvestmentId = InvestmentId;
type PoolBalance = Balance;
type PoolInspect = MockPools;
type RuntimeEvent = RuntimeEvent;
type SwapBalance = Balance;
type SwapRatio = Ratio;
type Swaps = Swaps;
type TrancheBalance = Balance;
}
Expand All @@ -158,5 +160,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
.build_storage::<Runtime>()
.unwrap();

sp_io::TestExternalities::new(storage)
let mut ext = sp_io::TestExternalities::new(storage);
ext.execute_with(|| frame_system::Pallet::<Runtime>::set_block_number(1));
ext
}
59 changes: 57 additions & 2 deletions pallets/foreign-investments/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use cfg_traits::{
investments::{ForeignInvestment as _, Investment, TrancheCurrency},
swaps::{OrderInfo, OrderRatio, Swap, SwapState, TokenSwaps},
swaps::{OrderInfo, OrderRatio, Swap, SwapInfo, TokenSwaps},
StatusNotificationHook,
};
use cfg_types::investments::{
CollectedAmount, ExecutedForeignCollect, ExecutedForeignDecreaseInvest,
};
use frame_support::{assert_err, assert_ok};
use sp_runtime::traits::One;
use sp_std::sync::{Arc, Mutex};

use crate::{
Expand Down Expand Up @@ -57,6 +58,14 @@ mod util {
}
}

pub fn market_ratio(to: CurrencyId, from: CurrencyId) -> Ratio {
match (from, to) {
(POOL_CURR, FOREIGN_CURR) => Ratio::from_rational(1, STABLE_RATIO),
(FOREIGN_CURR, POOL_CURR) => Ratio::from_rational(STABLE_RATIO, 1),
_ => Ratio::one(),
}
}

pub fn configure_pool() {
MockPools::mock_currency_for(|pool_id| {
assert_eq!(pool_id, INVESTMENT_ID.of_pool());
Expand Down Expand Up @@ -105,6 +114,8 @@ mod util {
MockTokenSwaps::mock_convert_by_market(|to, from, amount_from| {
Ok(convert_currencies(to, from, amount_from))
});

MockTokenSwaps::mock_market_ratio(|to, from| Ok(util::market_ratio(to, from)));
}

// Setup basic investment system
Expand Down Expand Up @@ -146,7 +157,7 @@ mod util {

Swaps::notify_status_change(
order_id,
SwapState {
SwapInfo {
remaining: Swap {
amount_out: order.swap.amount_out - amount_out,
..order.swap
Expand All @@ -158,6 +169,7 @@ mod util {
)
.unwrap(),
swapped_out: amount_out,
ratio: util::market_ratio(order.swap.currency_in, order.swap.currency_out),
},
)
.unwrap();
Expand Down Expand Up @@ -225,6 +237,19 @@ mod investment {
Ok(AMOUNT)
);
assert_eq!(MockInvestment::investment(&USER, INVESTMENT_ID), Ok(0));

System::assert_has_event(
Event::SwapCreated {
who: USER,
swap_id: (INVESTMENT_ID, Action::Investment),
swap: Swap {
amount_out: AMOUNT,
currency_out: FOREIGN_CURR,
currency_in: POOL_CURR,
},
}
.into(),
);
});
}

Expand Down Expand Up @@ -351,6 +376,21 @@ mod investment {
Ok(AMOUNT * 3 / 4)
);
assert_eq!(MockInvestment::investment(&USER, INVESTMENT_ID), Ok(0));

System::assert_has_event(
Event::SwapCancelled {
who: USER,
swap_id: (INVESTMENT_ID, Action::Investment),
remaining: Swap {
amount_out: 0,
currency_out: POOL_CURR,
currency_in: FOREIGN_CURR,
},
cancelled_in: AMOUNT / 4,
opposite_in: 3 * AMOUNT / 4,
}
.into(),
);
});
}

Expand Down Expand Up @@ -409,6 +449,21 @@ mod investment {
MockInvestment::investment(&USER, INVESTMENT_ID),
Ok(foreign_to_pool(AMOUNT / 4))
);

System::assert_has_event(
Event::SwapFullfilled {
who: USER,
swap_id: (INVESTMENT_ID, Action::Investment),
remaining: Swap {
amount_out: 3 * AMOUNT / 4,
currency_out: FOREIGN_CURR,
currency_in: POOL_CURR,
},
swapped_in: foreign_to_pool(AMOUNT / 4),
swapped_out: AMOUNT / 4,
}
.into(),
);
});
}

Expand Down
Loading
Loading