From 538fe2b048f63dab964a61fdc215935f22e74c1f Mon Sep 17 00:00:00 2001 From: vincentwschau <99756290+vincentwschau@users.noreply.github.com> Date: Fri, 22 Dec 2023 14:42:34 -0600 Subject: [PATCH] [IND-546] Prevent IOC/FOK orders from changing order book levels / sending updates. (#898) * [IND-546] Prevent IOC/FOK orders from changing order book levels / sending updates. * Address comments. --- .../redis/__tests__/helpers/constants.ts | 18 +++- .../v4-proto-parser/src/order-helpers.ts | 9 +- .../handlers/order-place-handler.test.ts | 98 +++++++++++++++++-- .../handlers/order-remove-handler.test.ts | 45 +++++++-- .../handlers/order-update-handler.test.ts | 59 +++++++++++ .../src/handlers/order-place-handler.ts | 10 +- .../src/handlers/order-remove-handler.ts | 20 +++- .../src/handlers/order-update-handler.ts | 48 +++++---- 8 files changed, 264 insertions(+), 43 deletions(-) diff --git a/indexer/packages/redis/__tests__/helpers/constants.ts b/indexer/packages/redis/__tests__/helpers/constants.ts index faf0f1a389..71912a20a4 100644 --- a/indexer/packages/redis/__tests__/helpers/constants.ts +++ b/indexer/packages/redis/__tests__/helpers/constants.ts @@ -73,7 +73,7 @@ export const defaultOrder: IndexerOrder = { subticks: Long.fromValue(2_000_000, true), goodTilBlock: 1150, goodTilBlockTime: undefined, - timeInForce: IndexerOrder_TimeInForce.TIME_IN_FORCE_FILL_OR_KILL, + timeInForce: IndexerOrder_TimeInForce.TIME_IN_FORCE_POST_ONLY, reduceOnly: false, clientMetadata: 0, conditionType: IndexerOrder_ConditionType.CONDITION_TYPE_UNSPECIFIED, @@ -91,6 +91,14 @@ export const defaultConditionalOrder: IndexerOrder = { conditionType: IndexerOrder_ConditionType.CONDITION_TYPE_STOP_LOSS, conditionalOrderTriggerSubticks: Long.fromValue(190_000_000, true), }; +export const defaultOrderFok: IndexerOrder = { + ...defaultOrder, + timeInForce: IndexerOrder_TimeInForce.TIME_IN_FORCE_FILL_OR_KILL, +}; +export const defaultOrderIoc: IndexerOrder = { + ...defaultOrder, + timeInForce: IndexerOrder_TimeInForce.TIME_IN_FORCE_IOC, +}; export const defaultOrderUuid: string = OrderTable.orderIdToUuid(defaultOrderId); export const defaultOrderUuidGoodTilBlockTime: string = OrderTable.orderIdToUuid( @@ -126,6 +134,14 @@ export const defaultRedisOrderConditional: RedisOrder = { id: defaultOrderUuidConditional, order: defaultConditionalOrder, }; +export const defaultRedisOrderFok: RedisOrder = { + ...defaultRedisOrder, + order: defaultOrderFok, +}; +export const defaultRedisOrderIoc: RedisOrder = { + ...defaultRedisOrder, + order: defaultOrderIoc, +}; export const orderPlace: OffChainUpdateOrderPlaceUpdateMessage = { orderUpdate: undefined, diff --git a/indexer/packages/v4-proto-parser/src/order-helpers.ts b/indexer/packages/v4-proto-parser/src/order-helpers.ts index 35461b1240..beeb567ac1 100644 --- a/indexer/packages/v4-proto-parser/src/order-helpers.ts +++ b/indexer/packages/v4-proto-parser/src/order-helpers.ts @@ -1,6 +1,6 @@ import { createHash } from 'crypto'; -import { IndexerOrderId } from '@dydxprotocol-indexer/v4-protos'; +import { IndexerOrderId, IndexerOrder_TimeInForce } from '@dydxprotocol-indexer/v4-protos'; import { ORDER_FLAG_CONDITIONAL, ORDER_FLAG_LONG_TERM } from './constants'; @@ -23,3 +23,10 @@ export function isStatefulOrder(orderFlag: number | String): boolean { } return numberOrderFlag === ORDER_FLAG_CONDITIONAL || numberOrderFlag === ORDER_FLAG_LONG_TERM; } + +export function requiresImmediateExecution(tif: IndexerOrder_TimeInForce): boolean { + return ( + tif === IndexerOrder_TimeInForce.TIME_IN_FORCE_FILL_OR_KILL || + tif === IndexerOrder_TimeInForce.TIME_IN_FORCE_IOC + ); +} diff --git a/indexer/services/vulcan/__tests__/handlers/order-place-handler.test.ts b/indexer/services/vulcan/__tests__/handlers/order-place-handler.test.ts index d05feb2abd..f2406f1861 100644 --- a/indexer/services/vulcan/__tests__/handlers/order-place-handler.test.ts +++ b/indexer/services/vulcan/__tests__/handlers/order-place-handler.test.ts @@ -43,7 +43,6 @@ import { StatefulOrderUpdatesCache, CanceledOrderStatus, } from '@dydxprotocol-indexer/redis'; - import { OffChainUpdateV1, IndexerOrder, @@ -104,6 +103,20 @@ describe('order-place-handler', () => { goodTilBlock: undefined, goodTilBlockTime: 1_300_000_000, }; + const replacementOrderFok: IndexerOrder = { + ...redisTestConstants.defaultOrderFok, + goodTilBlock: 1160, + goodTilBlockTime: undefined, + quantums: Long.fromValue(500_000, true), + subticks: Long.fromValue(1_000_000, true), + }; + const replacementOrderIoc: IndexerOrder = { + ...redisTestConstants.defaultOrderIoc, + goodTilBlock: 1160, + goodTilBlockTime: undefined, + quantums: Long.fromValue(500_000, true), + subticks: Long.fromValue(1_000_000, true), + }; const replacedOrder: RedisOrder = convertToRedisOrder( replacementOrder, testConstants.defaultPerpetualMarket, @@ -116,6 +129,14 @@ describe('order-place-handler', () => { replacementOrderConditional, testConstants.defaultPerpetualMarket, ); + const replacedOrderFok: RedisOrder = convertToRedisOrder( + replacementOrderFok, + testConstants.defaultPerpetualMarket, + ); + const replacedOrderIoc: RedisOrder = convertToRedisOrder( + replacementOrderIoc, + testConstants.defaultPerpetualMarket, + ); const replacementUpdate: OffChainUpdateV1 = { orderPlace: { order: replacementOrder, @@ -137,6 +158,20 @@ describe('order-place-handler', () => { OrderPlaceV1_OrderPlacementStatus.ORDER_PLACEMENT_STATUS_BEST_EFFORT_OPENED, }, }; + const replacementUpdateFok: OffChainUpdateV1 = { + orderPlace: { + order: replacementOrderFok, + placementStatus: + OrderPlaceV1_OrderPlacementStatus.ORDER_PLACEMENT_STATUS_BEST_EFFORT_OPENED, + }, + }; + const replacementUpdateIoc: OffChainUpdateV1 = { + orderPlace: { + order: replacementOrderIoc, + placementStatus: + OrderPlaceV1_OrderPlacementStatus.ORDER_PLACEMENT_STATUS_BEST_EFFORT_OPENED, + }, + }; const replacementMessage: KafkaMessage = createKafkaMessage( Buffer.from(Uint8Array.from(OffChainUpdateV1.encode(replacementUpdate).finish())), ); @@ -150,6 +185,12 @@ describe('order-place-handler', () => { OffChainUpdateV1.encode(replacementUpdateConditional).finish(), )), ); + const replacementMessageFok: KafkaMessage = createKafkaMessage( + Buffer.from(Uint8Array.from(OffChainUpdateV1.encode(replacementUpdateFok).finish())), + ); + const replacementMessageIoc: KafkaMessage = createKafkaMessage( + Buffer.from(Uint8Array.from(OffChainUpdateV1.encode(replacementUpdateIoc).finish())), + ); const dbDefaultOrder: OrderFromDatabase = { ...testConstants.defaultOrder, id: testConstants.defaultOrderId, @@ -164,6 +205,16 @@ describe('order-place-handler', () => { id: testConstants.defaultConditionalOrderId, createdAtHeight: '3', }; + const dbDefaultOrderFok: OrderFromDatabase = { + ...testConstants.defaultOrder, + id: testConstants.defaultOrderId, + timeInForce: TimeInForce.FOK, + }; + const dbDefaultOrderIoc: OrderFromDatabase = { + ...testConstants.defaultOrder, + id: testConstants.defaultOrderId, + timeInForce: TimeInForce.IOC, + }; beforeAll(async () => { await dbHelpers.migrate(); @@ -410,6 +461,7 @@ describe('order-place-handler', () => { redisTestConstants.defaultOrderUuid, replacedOrder, true, + true, ], [ 'goodTilBlockTime', @@ -420,6 +472,7 @@ describe('order-place-handler', () => { redisTestConstants.defaultOrderUuidGoodTilBlockTime, replacedOrderGoodTilBlockTime, false, + true, ], [ 'conditional', @@ -430,6 +483,29 @@ describe('order-place-handler', () => { redisTestConstants.defaultOrderUuidConditional, replacedOrderConditional, false, + true, + ], + [ + 'Fill-or-Kill', + redisTestConstants.defaultOrderFok, + replacementMessageFok, + redisTestConstants.defaultRedisOrderFok, + dbDefaultOrderFok, + redisTestConstants.defaultOrderUuid, + replacedOrderFok, + true, + false, + ], + [ + 'Immediate-or-Cancel', + redisTestConstants.defaultOrderIoc, + replacementMessageIoc, + redisTestConstants.defaultRedisOrderIoc, + dbDefaultOrderIoc, + redisTestConstants.defaultOrderUuid, + replacedOrderIoc, + true, + false, ], ])('handles order place for replacing order (with %s), resting on book', async ( _name: string, @@ -440,6 +516,7 @@ describe('order-place-handler', () => { expectedOrderUuid: string, expectedReplacedOrder: RedisOrder, expectSubaccountMessage: boolean, + expectOrderBookUpdate: boolean, ) => { const oldOrderTotalFilled: number = 10; const oldPriceLevelInitialQuantums: number = Number(initialOrderToPlace.quantums) * 2; @@ -521,9 +598,11 @@ describe('order-place-handler', () => { ); // Check the order book levels were updated - expect(orderbook.bids).toHaveLength(1); - expect(orderbook.asks).toHaveLength(0); - expect(orderbook.bids).toContainEqual(expectedPriceLevel); + if (expectOrderBookUpdate) { + expect(orderbook.bids).toHaveLength(1); + expect(orderbook.asks).toHaveLength(0); + expect(orderbook.bids).toContainEqual(expectedPriceLevel); + } // Check the order was removed from the open orders cache await expectOpenOrderIds(testConstants.defaultPerpetualMarket.clobPairId, []); @@ -542,11 +621,12 @@ describe('order-place-handler', () => { testConstants.defaultPerpetualMarket, APIOrderStatusEnum.BEST_EFFORT_OPENED, expectSubaccountMessage, - OrderbookMessage.fromPartial({ - contents: JSON.stringify(orderbookContents), - clobPairId: testConstants.defaultPerpetualMarket.clobPairId, - version: ORDERBOOKS_WEBSOCKET_MESSAGE_VERSION, - }), + expectOrderBookUpdate + ? OrderbookMessage.fromPartial({ + contents: JSON.stringify(orderbookContents), + clobPairId: testConstants.defaultPerpetualMarket.clobPairId, + version: ORDERBOOKS_WEBSOCKET_MESSAGE_VERSION, + }) : undefined, ); expectStats(true); }); diff --git a/indexer/services/vulcan/__tests__/handlers/order-remove-handler.test.ts b/indexer/services/vulcan/__tests__/handlers/order-remove-handler.test.ts index 7d62beac72..a7a88b2c06 100644 --- a/indexer/services/vulcan/__tests__/handlers/order-remove-handler.test.ts +++ b/indexer/services/vulcan/__tests__/handlers/order-remove-handler.test.ts @@ -27,6 +27,7 @@ import { testConstants, testMocks, apiTranslations, + TimeInForce, } from '@dydxprotocol-indexer/postgres'; import { OpenOrdersCache, @@ -123,6 +124,15 @@ describe('OrderRemoveHandler', () => { testConstants.defaultPerpetualMarket.atomicResolution, ); + const dbOrderFok: OrderCreateObject = { + ...testConstants.defaultOrder, + timeInForce: TimeInForce.FOK, + }; + const dbOrderIoc: OrderCreateObject = { + ...testConstants.defaultOrder, + timeInForce: TimeInForce.IOC, + }; + it.each([ [ { @@ -222,6 +232,7 @@ describe('OrderRemoveHandler', () => { testConstants.defaultOrder, redisTestConstants.defaultRedisOrder, redisTestConstants.defaultOrderUuid, + true, undefined, ], [ @@ -230,6 +241,7 @@ describe('OrderRemoveHandler', () => { testConstants.defaultOrderGoodTilBlockTime, redisTestConstants.defaultRedisOrderGoodTilBlockTime, redisTestConstants.defaultOrderUuidGoodTilBlockTime, + true, undefined, ], [ @@ -238,14 +250,34 @@ describe('OrderRemoveHandler', () => { testConstants.defaultConditionalOrder, redisTestConstants.defaultRedisOrderConditional, redisTestConstants.defaultOrderUuidConditional, + true, testConstants.defaultConditionalOrder.triggerPrice, ], + [ + 'Fill-or-Kill', + redisTestConstants.defaultOrderId, + dbOrderFok, + redisTestConstants.defaultRedisOrderFok, + redisTestConstants.defaultOrderUuid, + false, + undefined, + ], + [ + 'Immediate-or-Cancel', + redisTestConstants.defaultOrderId, + dbOrderIoc, + redisTestConstants.defaultRedisOrderIoc, + redisTestConstants.defaultOrderUuid, + false, + undefined, + ], ])('successfully removes order (with %s)', async ( _name: string, removedOrderId: IndexerOrderId, removedOrder: OrderCreateObject, removedRedisOrder: RedisOrder, expectedOrderUuid: string, + expectOrderbookUpdate: boolean, triggerPrice?: string, ) => { const offChainUpdate: OffChainUpdateV1 = orderRemoveToOffChainUpdate({ @@ -302,7 +334,7 @@ describe('OrderRemoveHandler', () => { removedRedisOrder.ticker, OrderSide.BUY, defaultPrice, - remainingOrderbookLevel, + expectOrderbookUpdate ? remainingOrderbookLevel : orderbookLevel, ), expectOrdersCacheEmpty(expectedOrderUuid), expectOrdersDataCacheEmpty(removedOrderId), @@ -365,11 +397,12 @@ describe('OrderRemoveHandler', () => { subaccountId: redisTestConstants.defaultSubaccountId, version: SUBACCOUNTS_WEBSOCKET_MESSAGE_VERSION, }), - OrderbookMessage.fromPartial({ - contents: JSON.stringify(orderbookContents), - clobPairId: testConstants.defaultPerpetualMarket.clobPairId, - version: ORDERBOOKS_WEBSOCKET_MESSAGE_VERSION, - }), + expectOrderbookUpdate + ? OrderbookMessage.fromPartial({ + contents: JSON.stringify(orderbookContents), + clobPairId: testConstants.defaultPerpetualMarket.clobPairId, + version: ORDERBOOKS_WEBSOCKET_MESSAGE_VERSION, + }) : undefined, ); expect(logger.error).not.toHaveBeenCalled(); expectTimingStats(true, true); diff --git a/indexer/services/vulcan/__tests__/handlers/order-update-handler.test.ts b/indexer/services/vulcan/__tests__/handlers/order-update-handler.test.ts index 15a9737f3c..c323a2a145 100644 --- a/indexer/services/vulcan/__tests__/handlers/order-update-handler.test.ts +++ b/indexer/services/vulcan/__tests__/handlers/order-update-handler.test.ts @@ -231,6 +231,65 @@ describe('OrderUpdateHandler', () => { }, ); + it.each([ + [ + 'Fill-or-Kill', + redisTestConstants.defaultOrderFok, + redisTestConstants.defaultOrderId, + redisTestConstants.defaultRedisOrderFok, + ], + [ + 'Immediate-or-Cancel', + redisTestConstants.defaultOrderIoc, + redisTestConstants.defaultOrderId, + redisTestConstants.defaultRedisOrderIoc, + ], + ])( + 'updates new order (with %s) (not resting on book) total filled but does not update order ' + + 'book for IOC order', + async ( + _name: string, + initialOrderToPlace: IndexerOrder, + updatedOrderId: IndexerOrderId, + updatedRedisOrder: RedisOrder, + ) => { + synchronizeWrapBackgroundTask(wrapBackgroundTask); + const producerSendSpy: jest.SpyInstance = jest.spyOn(producer, 'send').mockReturnThis(); + const orderUpdate: redisTestConstants.OffChainUpdateOrderUpdateUpdateMessage = { + ...redisTestConstants.orderUpdate, + orderUpdate: { + ...redisTestConstants.orderUpdate.orderUpdate, + orderId: updatedOrderId, + }, + }; + + // Create a new order by handling an order place message + await handleInitialOrderPlace({ + ...redisTestConstants.orderPlace, + orderPlace: { + order: initialOrderToPlace, + placementStatus: + OrderPlaceV1_OrderPlacementStatus.ORDER_PLACEMENT_STATUS_BEST_EFFORT_OPENED, + }, + }); + jest.runOnlyPendingTimers(); + jest.clearAllMocks(); + await handleOrderUpdate(orderUpdate); + + // No order book update websocket messages should be sent + expectWebsocketMessagesNotSent(producerSendSpy); + await expectOrdersDataCache(orderUpdate); + // Price-level should be 0 as placing the order and updating the order should not have + // changed the order book + await expectOrderbookLevelCache( + testConstants.defaultPerpetualMarket.ticker, + protocolTranslations.protocolOrderSideToOrderSide(initialOrderToPlace.side), + updatedRedisOrder.price, + '0', + ); + }, + ); + it( 'logs error, and caps price level update if new total filled quantums exceed order size', async () => { diff --git a/indexer/services/vulcan/src/handlers/order-place-handler.ts b/indexer/services/vulcan/src/handlers/order-place-handler.ts index 699bf933fb..43f0c0fe9c 100644 --- a/indexer/services/vulcan/src/handlers/order-place-handler.ts +++ b/indexer/services/vulcan/src/handlers/order-place-handler.ts @@ -26,7 +26,9 @@ import { CanceledOrdersCache, StatefulOrderUpdatesCache, } from '@dydxprotocol-indexer/redis'; -import { ORDER_FLAG_SHORT_TERM, getOrderIdHash, isStatefulOrder } from '@dydxprotocol-indexer/v4-proto-parser'; +import { + ORDER_FLAG_SHORT_TERM, getOrderIdHash, isStatefulOrder, requiresImmediateExecution, +} from '@dydxprotocol-indexer/v4-proto-parser'; import { OffChainUpdateV1, IndexerOrder, @@ -186,7 +188,11 @@ export class OrderPlaceHandler extends Handler { ): Promise { // TODO(DEC-1339): Update price levels based on if the order is reduce-only and if the replaced // order is reduce-only. - if (result.replaced !== true || result.restingOnBook !== true) { + if ( + result.replaced !== true || + result.restingOnBook !== true || + requiresImmediateExecution(result.oldOrder!.order!.timeInForce) + ) { return undefined; } diff --git a/indexer/services/vulcan/src/handlers/order-remove-handler.ts b/indexer/services/vulcan/src/handlers/order-remove-handler.ts index 0672b865e5..50c567adfd 100644 --- a/indexer/services/vulcan/src/handlers/order-remove-handler.ts +++ b/indexer/services/vulcan/src/handlers/order-remove-handler.ts @@ -23,7 +23,11 @@ import { removeOrder, CanceledOrdersCache, } from '@dydxprotocol-indexer/redis'; -import { ORDER_FLAG_SHORT_TERM, isStatefulOrder } from '@dydxprotocol-indexer/v4-proto-parser'; +import { + ORDER_FLAG_SHORT_TERM, + isStatefulOrder, + requiresImmediateExecution, +} from '@dydxprotocol-indexer/v4-proto-parser'; import { OffChainUpdateV1, IndexerOrder, @@ -231,7 +235,12 @@ export class OrderRemoveHandler extends Handler { // If an order was removed from the Orders cache and was resting on the book, update the // orderbook levels cache - if (removeOrderResult.removed && removeOrderResult.restingOnBook === true) { + // Orders that require immediate execution do not rest on the book, and also should not lead + // to an update to the orderbook levels cache + if ( + removeOrderResult.removed && + removeOrderResult.restingOnBook === true && + !requiresImmediateExecution(removeOrderResult.removedOrder!.order!.timeInForce)) { await this.updateOrderbook(removeOrderResult, perpetualMarket); } @@ -308,8 +317,11 @@ export class OrderRemoveHandler extends Handler { removeOrderResult.removedOrder!, )); // Do not update orderbook if order being cancelled has no remaining quantums or is - // resting on book - if (!remainingQuantums.eq('0') && removeOrderResult.restingOnBook !== false) { + // resting on book, or requires immediate execution and will not rest on the book + if ( + !remainingQuantums.eq('0') && + removeOrderResult.restingOnBook !== false && + !requiresImmediateExecution(removeOrderResult.removedOrder!.order!.timeInForce)) { await this.updateOrderbook(removeOrderResult, perpetualMarket); } // TODO: consolidate remove handler logic into a single lua script. diff --git a/indexer/services/vulcan/src/handlers/order-update-handler.ts b/indexer/services/vulcan/src/handlers/order-update-handler.ts index f36eed7123..950b7ee73f 100644 --- a/indexer/services/vulcan/src/handlers/order-update-handler.ts +++ b/indexer/services/vulcan/src/handlers/order-update-handler.ts @@ -17,7 +17,7 @@ import { OpenOrdersCache, StatefulOrderUpdatesCache, } from '@dydxprotocol-indexer/redis'; -import { isStatefulOrder } from '@dydxprotocol-indexer/v4-proto-parser'; +import { isStatefulOrder, requiresImmediateExecution } from '@dydxprotocol-indexer/v4-proto-parser'; import { OffChainUpdateV1, OrderUpdateV1, @@ -145,27 +145,35 @@ export class OrderUpdateHandler extends Handler { stats.increment(`${config.SERVICE_NAME}.order_update_with_zero_delta.count`, 1); return; } - const updatedQuantums: number = await this.updatePriceLevel(updateResult, sizeDeltaInQuantums); - const perpetualMarket: PerpetualMarketFromDatabase | undefined = perpetualMarketRefresher - .getPerpetualMarketFromTicker(updateResult.order!.ticker); - if (perpetualMarket === undefined) { - logger.error({ - at: 'OrderUpdateHandler#handle', - message: `Received order update for order with unknown perpetual market, ticker ${ - updateResult.order!.ticker}`, - }); - return; - } + // Orders that require immediate execution do not rest on the order book and will not lead to + // a change in the order book level for the order's price + if (!requiresImmediateExecution(updateResult.order!.order!.timeInForce)) { + const updatedQuantums: number = await this.updatePriceLevel( + updateResult, + sizeDeltaInQuantums, + ); + + const perpetualMarket: PerpetualMarketFromDatabase | undefined = perpetualMarketRefresher + .getPerpetualMarketFromTicker(updateResult.order!.ticker); + if (perpetualMarket === undefined) { + logger.error({ + at: 'OrderUpdateHandler#handle', + message: `Received order update for order with unknown perpetual market, ticker ${ + updateResult.order!.ticker}`, + }); + return; + } - const orderbookMessage: Message = { - value: this.createOrderbookWebsocketMessage( - updateResult.order!, - perpetualMarket, - updatedQuantums, - ), - }; - sendMessageWrapper(orderbookMessage, KafkaTopics.TO_WEBSOCKETS_ORDERBOOKS); + const orderbookMessage: Message = { + value: this.createOrderbookWebsocketMessage( + updateResult.order!, + perpetualMarket, + updatedQuantums, + ), + }; + sendMessageWrapper(orderbookMessage, KafkaTopics.TO_WEBSOCKETS_ORDERBOOKS); + } } /**