feat: unified trade events #910
Conversation
|
what if we call the new pallet something more generic ... eg. amm-support. it might be useful for other things, not only one specific trade event. |
pallets/trade-event/src/lib.rs
Outdated
| who: T::AccountId, | ||
| pool: PoolType<AssetId>, |
There was a problem hiding this comment.
after discussion with @jak-pan , we suggest:
| who: T::AccountId, | |
| pool: PoolType<AssetId>, | |
| swapper: T::AccountId, | |
| filler: T::AccountId, // ICE acc, Omnipool acc, stablepool acc, xyk account etc.. | |
| type: FillerType, |
enum FillerType {
Omnipool,
Stableswap,
XYK,
LBP,
OTC,
ICE,
}There was a problem hiding this comment.
Who is filler in OTC?
There was a problem hiding this comment.
type is a keyword, cannot be used
There was a problem hiding this comment.
Who is
fillerin OTC?
the one filling order
There was a problem hiding this comment.
isn't it the one who owns the order?
|
Crate versions that have not been updated:
New crates:
Crate versions that have been updated:
Runtime version has not been increased. |
…ant to block any operations when wrongly implemented
integration-tests/src/dca.rs
Outdated
| RuntimeEvent::AmmSupport(pallet_amm_support::Event::Swapped { | ||
| swapper: ALICE.into(), | ||
| filler: Omnipool::protocol_account(), | ||
| filler_type: pallet_amm_support::types::Filler::Omnipool, |
There was a problem hiding this comment.
Would be nice if this had more complex route since it's integration test but I guess we can do that separately.
| operation: pallet_amm_support::types::TradeOperation::ExactOut, | ||
| inputs: vec![Asset::new(HDX, 140421094367051)], | ||
| outputs: vec![Asset::new(LRNA, 70210545436437)], | ||
| fees: vec![Fee::new(LRNA, 35105272718, Omnipool::protocol_account()),], |
There was a problem hiding this comment.
What happens with referrals, should we see it here?
There was a problem hiding this comment.
(Just OT question, doesn't fit context)
| fees: vec![Fee::new(LRNA::get(), 24590163934, Omnipool::protocol_account()),], | ||
| operation_id: vec![ | ||
| ExecutionType::Xcm(topic_id, 0), | ||
| ExecutionType::XcmExchange(1), |
There was a problem hiding this comment.
the API of AssetExchange does the full xcm instructions, so can't have the topic ID easily
_origin: Option<&Location>,
give: AssetsInHolding,
want: &Assets,
maximal: bool,
What we could maybe do is to read the topic from our stack storage, as at this point, the Xcm([u8; 32], IncrementalIdType) should be there. Not sure if it is always there, otherwise we could do use Optional type. WDYT?
| last_two_swapped_events, | ||
| vec![ | ||
| RuntimeEvent::AmmSupport(pallet_amm_support::Event::Swapped { | ||
| swapper: TempAccountForXcmAssetExchange::get(), |
There was a problem hiding this comment.
Can we get original acc or just this temp acc
There was a problem hiding this comment.
unfortunately only the temp acc as the actual trade happens by that account (on behalf of user), that's how we could implement asset exchange back then.
integration-tests/src/referrals.rs
Outdated
| operation: TradeOperation::ExactIn, | ||
| inputs: vec![Asset::new(LRNA, 1205165959)], | ||
| outputs: vec![Asset::new(DAI, 26663424573622008)], | ||
| fees: vec![Fee::new(DAI, 70524156750724, Omnipool::protocol_account())], |
There was a problem hiding this comment.
Looks like we're missing referral fees here
integration-tests/src/router.rs
Outdated
| pallet_amm_support::Event::Swapped { | ||
| swapper: BOB.into(), | ||
| filler: LBP::get_pair_id(pallet_lbp::types::AssetPair::new(DAI, HDX)), | ||
| filler_type: pallet_amm_support::types::Filler::LBP, |
There was a problem hiding this comment.
Shouldn't we have shareToken here?
| @@ -129,7 +129,7 @@ pub fn calculate_shares<const D: u8>( | |||
| amplification: Balance, | |||
| share_issuance: Balance, | |||
| fee: Permill, | |||
| ) -> Option<Balance> { | |||
| ) -> Option<(Balance, Vec<Balance>)> { | |||
There was a problem hiding this comment.
This looks like breaking change.
There was a problem hiding this comment.
UI should be notified updated and ready for this.
| } | ||
| .into(), | ||
| pallet_amm_support::Event::Swapped { | ||
| swapper: OtcSettlements::account_id(), |
There was a problem hiding this comment.
This is flipped. You are filling existing swap (order)
| amount_in: order.amount_in, | ||
| amount_out: order.amount_out, | ||
| fee, | ||
| }); | ||
|
|
||
| pallet_amm_support::Pallet::<T>::deposit_trade_event( | ||
| who, |
Fixes #755
Additional todos:
TODO: