feat(orderbook): expirable maker orders #2516
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1753,6 +1753,7 @@ pub struct MakerOrder { | |
| pub swap_version: SwapVersion, | ||
| #[cfg(feature = "ibc-routing-for-swaps")] | ||
| order_metadata: OrderMetadata, | ||
| timeout_in_minutes: Option<u16>, | ||
| } | ||
|
|
||
| pub struct MakerOrderBuilder<'a> { | ||
|
|
@@ -1768,6 +1769,7 @@ pub struct MakerOrderBuilder<'a> { | |
| swap_version: u8, | ||
| #[cfg(feature = "ibc-routing-for-swaps")] | ||
| order_metadata: OrderMetadata, | ||
| timeout_in_minutes: Option<u16>, | ||
| } | ||
|
|
||
| /// Contains extra and/or optional metadata (e.g., protocol-specific information) that can | ||
|
|
@@ -1930,6 +1932,7 @@ impl<'a> MakerOrderBuilder<'a> { | |
| swap_version: SWAP_VERSION_DEFAULT, | ||
| #[cfg(feature = "ibc-routing-for-swaps")] | ||
| order_metadata: OrderMetadata::default(), | ||
| timeout_in_minutes: None, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1968,6 +1971,8 @@ impl<'a> MakerOrderBuilder<'a> { | |
| self | ||
| } | ||
|
|
||
| pub fn set_timeout(&mut self, timeout_in_minutes: u16) { self.timeout_in_minutes = Some(timeout_in_minutes); } | ||
|
|
||
| /// When a new [MakerOrderBuilder::new] is created, it sets [SWAP_VERSION_DEFAULT]. | ||
| /// However, if user has not specified in the config to use TPU V2, | ||
| /// the MakerOrderBuilder's swap_version is changed to legacy. | ||
|
|
@@ -2033,6 +2038,7 @@ impl<'a> MakerOrderBuilder<'a> { | |
| swap_version: SwapVersion::from(self.swap_version), | ||
| #[cfg(feature = "ibc-routing-for-swaps")] | ||
| order_metadata: self.order_metadata, | ||
| timeout_in_minutes: self.timeout_in_minutes, | ||
| }) | ||
| } | ||
|
|
||
|
|
@@ -2060,6 +2066,7 @@ impl<'a> MakerOrderBuilder<'a> { | |
| swap_version: SwapVersion::from(self.swap_version), | ||
| #[cfg(feature = "ibc-routing-for-swaps")] | ||
| order_metadata: self.order_metadata, | ||
| timeout_in_minutes: self.timeout_in_minutes, | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -2194,6 +2201,7 @@ impl From<TakerOrder> for MakerOrder { | |
| // TODO: Add test coverage for this once we have an integration test for this feature. | ||
| #[cfg(feature = "ibc-routing-for-swaps")] | ||
| order_metadata: taker_order.request.order_metadata, | ||
| timeout_in_minutes: None, | ||
| }, | ||
| // The "buy" taker order is recreated with reversed pair as Maker order is always considered as "sell" | ||
| TakerAction::Buy => { | ||
|
|
@@ -2220,6 +2228,7 @@ impl From<TakerOrder> for MakerOrder { | |
| // TODO: Add test coverage for this once we have an integration test for this feature. | ||
| #[cfg(feature = "ibc-routing-for-swaps")] | ||
| order_metadata: taker_order.request.order_metadata, | ||
| timeout_in_minutes: None, | ||
| } | ||
| }, | ||
| } | ||
|
|
@@ -2962,7 +2971,7 @@ impl OrdermatchContext { | |
| } | ||
|
|
||
| pub struct MakerOrdersContext { | ||
| orders: TimedMap<Uuid, Arc<AsyncMutex<MakerOrder>>>, | ||
| order_tickers: HashMap<Uuid, String>, | ||
| count_by_tickers: HashMap<String, usize>, | ||
| /// The `check_balance_update_loop` future abort handles associated stored by corresponding tickers. | ||
|
|
@@ -2976,7 +2985,7 @@ impl MakerOrdersContext { | |
| let balance_loops = ctx.abortable_system.create_subsystem()?; | ||
|
|
||
| Ok(MakerOrdersContext { | ||
| orders: TimedMap::new_with_map_kind(timed_map::MapKind::FxHashMap), | ||
| order_tickers: HashMap::new(), | ||
| count_by_tickers: HashMap::new(), | ||
| balance_loops, | ||
|
|
@@ -2988,7 +2997,21 @@ impl MakerOrdersContext { | |
|
|
||
| self.order_tickers.insert(order.uuid, order.base.clone()); | ||
| *self.count_by_tickers.entry(order.base.clone()).or_insert(0) += 1; | ||
|
|
||
| if let Some(t) = order.timeout_in_minutes { | ||
| // Use unchecked write to skip automatic cleanup as we need to handle | ||
| // expired orders manually. | ||
| self.orders.insert_expirable_unchecked( | ||
| order.uuid, | ||
| Arc::new(AsyncMutex::new(order)), | ||
| Duration::from_secs(t as u64 * 60), | ||
| ); | ||
| } else { | ||
| // Use unchecked write to skip automatic cleanup as we need to handle | ||
| // expired orders manually. | ||
| self.orders | ||
| .insert_constant_unchecked(order.uuid, Arc::new(AsyncMutex::new(order))); | ||
| } | ||
| } | ||
|
|
||
| fn get_order(&self, uuid: &Uuid) -> Option<&Arc<AsyncMutex<MakerOrder>>> { self.orders.get(uuid) } | ||
|
|
@@ -3562,6 +3585,19 @@ pub async fn lp_ordermatch_loop(ctx: MmArc) { | |
| handle_timed_out_maker_matches(ctx.clone(), &ordermatch_ctx).await; | ||
| check_balance_for_maker_orders(ctx.clone(), &ordermatch_ctx).await; | ||
|
|
||
| let expired_orders = ordermatch_ctx.maker_orders_ctx.lock().orders.drop_expired_entries(); | ||
|
|
||
| for (uuid, order_mutex) in expired_orders { | ||
| log::info!("Order '{uuid}' is expired, cancelling"); | ||
|
|
||
| let order = order_mutex.lock().await; | ||
| maker_order_cancelled_p2p_notify(&ctx, &order); | ||
| delete_my_maker_order(ctx.clone(), order.clone(), MakerOrderCancellationReason::Expired) | ||
| .compat() | ||
| .await | ||
| .ok(); | ||
| } | ||
|
|
||
| { | ||
| // remove "timed out" pubkeys states with their orders from orderbook | ||
| let mut orderbook = ordermatch_ctx.orderbook.lock(); | ||
|
|
@@ -3592,9 +3628,9 @@ pub async fn lp_ordermatch_loop(ctx: MmArc) { | |
| let mut to_cancel = Vec::new(); | ||
| { | ||
| let orderbook = ordermatch_ctx.orderbook.lock(); | ||
| for uuid in ordermatch_ctx.maker_orders_ctx.lock().orders.keys() { | ||
| if !orderbook.order_set.contains_key(&uuid) { | ||
| missing_uuids.push(uuid); | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -4795,6 +4831,7 @@ pub struct SetPriceReq { | |
| rel_nota: Option<bool>, | ||
| #[serde(default = "get_true")] | ||
| save_in_history: bool, | ||
| timeout_in_minutes: Option<u16>, | ||
|
Author
There was a problem hiding this comment. I think using seconds would be unnecessary and can be bad for the network stability (imagine people sending orders with ~5-10 seconds of expiration time).
Collaborator
There was a problem hiding this comment. Makes sense, will wait for @cipig input since he requested this
Collaborator
There was a problem hiding this comment.
On the other hand, if or when we can support this, it won't be bad as it will mirror the CEX experience a lot.
Collaborator
There was a problem hiding this comment. And they can cancel it in whatever seconds they like manually or using scripts
Author
There was a problem hiding this comment. It wouldn't be harmful on CEX platforms because spamming very short-lived orders doesn't affect other nodes. But on a P2P network I think orders should live for at least 30 seconds or so. For example orders that last only 5 seconds just put pressure on the network, especially since it can take up to 5 seconds for peers to fully sync. Instead doing manual validations on the expiration time, I decided to make it Also, when taking so many users into account, with seconds, this feature alone can put quite a lot of pressure on the network, with minutes, it should be way less compare to seconds.
Collaborator
There was a problem hiding this comment. I understand and agree. I was just arguing the opposite case, I plan to leave it at minutes unless @cipig objects and wants something like 30 seconds.
Collaborator
There was a problem hiding this comment. Cancelling in seconds doesn't have to have an immediate p2p message propagated but if it's matched before it's cancelled on remote nodes, the maker node will not proceed with the order when it's not available locally. And takers choose between the best response from multiple makers anyway so it won't cause problems. But that's for another PR or issue if needed. There was a problem hiding this comment.
Minutes is perfectly fine. I added it in my bot with a 20 minute timeout (cipig/mmtools@5ee5dbe). It needs anyway 5-15 minutes to update all orders, depending on number of coins. If the bot dies for whatever reason, orders are cancelled like this: That's exactly what i wanted, as a safety measure for my own bad programming :-) |
||
| } | ||
|
|
||
| #[derive(Deserialize)] | ||
|
|
@@ -5059,6 +5096,11 @@ pub async fn create_maker_order(ctx: &MmArc, req: SetPriceReq) -> Result<MakerOr | |
| .with_save_in_history(req.save_in_history) | ||
| .with_base_orderbook_ticker(ordermatch_ctx.orderbook_ticker(base_coin.ticker())) | ||
| .with_rel_orderbook_ticker(ordermatch_ctx.orderbook_ticker(rel_coin.ticker())); | ||
|
|
||
| if let Some(t) = req.timeout_in_minutes { | ||
| builder.set_timeout(t); | ||
| } | ||
|
|
||
| if !ctx.use_trading_proto_v2() { | ||
| builder.set_legacy_swap_v(); | ||
| } | ||
|
|
@@ -5349,6 +5391,7 @@ pub enum MakerOrderCancellationReason { | |
| Fulfilled, | ||
| InsufficientBalance, | ||
| Cancelled, | ||
| Expired, | ||
| } | ||
|
|
||
| #[derive(Display)] | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.