-
Notifications
You must be signed in to change notification settings - Fork 86
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
FI: Add swapping events #1764
Changes from all commits
f40c31a
a4c781d
c34db97
22e94d9
941c19c
44e962e
4ccdf67
c86da3c
6d4215f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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> { | ||
|
@@ -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) | ||
})?; | ||
|
||
|
@@ -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; | ||
} | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that |
||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could it be that both IF YES:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it could. 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 Possible states after applying swaps are:
NOTE: Probably, a better name for There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
|
@@ -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( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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::*; | ||
|
||
|
@@ -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 | ||
|
@@ -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; | ||
|
||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hieronx I've needed to add a new
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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good to me! Only one nit: in There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
OrderInfo
returned byTokenSwaps::get_order_details
contains aratio
field of typeOrderRatio<Ratio>
. Maybe that suffices such that we don't need to introduce an additional ratio field?There was a problem hiding this comment.
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. Theratio
here is the ratio value itself. It differs, i.e., whenOrderRatio::Market
, we do not know the ratio value.There was a problem hiding this comment.
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.