From 1719980f73f660ac7909e22401eb082922cce4b6 Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Tue, 28 May 2024 14:26:19 -0400 Subject: [PATCH 01/14] Send correct indexer messages for short term order replacement --- protocol/x/clob/memclob/memclob.go | 32 +++++++++++++++++------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/protocol/x/clob/memclob/memclob.go b/protocol/x/clob/memclob/memclob.go index 1f7b30c417..cb4a9e99e0 100644 --- a/protocol/x/clob/memclob/memclob.go +++ b/protocol/x/clob/memclob/memclob.go @@ -492,23 +492,27 @@ func (m *MemClobPriceTimePriority) PlaceOrder( } if m.generateOffchainUpdates { - // If this is a replacement order, then ensure we send the appropriate replacement message. + // If existing order ID is found and the price of the existing order is different from the new order, create an order removal message first + // because Indexer order place handler supports replacement, but only with the same price. orderId := order.OrderId - if _, found := m.openOrders.getOrder(ctx, orderId); found { - if message, success := off_chain_updates.CreateOrderReplaceMessage( - ctx, - order, - ); success { - offchainUpdates.AddReplaceMessage(orderId, message) - } - } else { - if message, success := off_chain_updates.CreateOrderPlaceMessage( - ctx, - order, - ); success { - offchainUpdates.AddPlaceMessage(order.OrderId, message) + if existingOrder, found := m.openOrders.getOrder(ctx, orderId); found { + if order.Subticks != existingOrder.Subticks { + if message, success := off_chain_updates.CreateOrderRemoveMessageWithReason( + ctx, + orderId, + indexersharedtypes.OrderRemovalReason_ORDER_REMOVAL_REASON_REPLACED, + ocutypes.OrderRemoveV1_ORDER_REMOVAL_STATUS_BEST_EFFORT_CANCELED, + ); success { + offchainUpdates.AddRemoveMessage(orderId, message) + } } } + if message, success := off_chain_updates.CreateOrderPlaceMessage( + ctx, + order, + ); success { + offchainUpdates.AddPlaceMessage(order.OrderId, message) + } } // Attempt to match the order against the orderbook. From e5233de8321be043fc477bf7d5de604ce1c7ba31 Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Tue, 28 May 2024 14:31:47 -0400 Subject: [PATCH 02/14] update ci --- .github/workflows/indexer-build-and-push-dev-staging.yml | 1 + .github/workflows/protocol-build-and-push.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/indexer-build-and-push-dev-staging.yml b/.github/workflows/indexer-build-and-push-dev-staging.yml index 5a98b72552..f4084b1628 100644 --- a/.github/workflows/indexer-build-and-push-dev-staging.yml +++ b/.github/workflows/indexer-build-and-push-dev-staging.yml @@ -6,6 +6,7 @@ on: # yamllint disable-line rule:truthy - main - 'release/indexer/v[0-9]+.[0-9]+.x' # e.g. release/indexer/v0.1.x - 'release/indexer/v[0-9]+.x' # e.g. release/indexer/v1.x + - 'chenyao/short-term-order-replacement-indexer-messages' # TODO(DEC-837): Customize github build and push to ECR by service with paths jobs: diff --git a/.github/workflows/protocol-build-and-push.yml b/.github/workflows/protocol-build-and-push.yml index fc26dd6e69..8c9509b731 100644 --- a/.github/workflows/protocol-build-and-push.yml +++ b/.github/workflows/protocol-build-and-push.yml @@ -6,6 +6,7 @@ on: # yamllint disable-line rule:truthy - main - 'release/protocol/v[0-9]+.[0-9]+.x' # e.g. release/protocol/v0.1.x - 'release/protocol/v[0-9]+.x' # e.g. release/protocol/v1.x + - 'chenyao/short-term-order-replacement-indexer-messages' jobs: build-and-push-dev: From bbd916ed3768e726b0a8e205433106cf8a8ccd6e Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Tue, 28 May 2024 15:59:03 -0400 Subject: [PATCH 03/14] update replacement logic --- protocol/x/clob/memclob/memclob.go | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/protocol/x/clob/memclob/memclob.go b/protocol/x/clob/memclob/memclob.go index cb4a9e99e0..dc5ca6ad29 100644 --- a/protocol/x/clob/memclob/memclob.go +++ b/protocol/x/clob/memclob/memclob.go @@ -492,8 +492,11 @@ func (m *MemClobPriceTimePriority) PlaceOrder( } if m.generateOffchainUpdates { - // If existing order ID is found and the price of the existing order is different from the new order, create an order removal message first - // because Indexer order place handler supports replacement, but only with the same price. + // If existing order ID is found and the price of the existing order is different from the new order, + // create an order removal message and then an order place message. + // If the price of the existing order is the same as the new order, don't send any messages here because + // the size of the order will get updated by the Order Update message after the order is matched. + // Otherwise, this is a new order and so send an order place message. orderId := order.OrderId if existingOrder, found := m.openOrders.getOrder(ctx, orderId); found { if order.Subticks != existingOrder.Subticks { @@ -505,13 +508,21 @@ func (m *MemClobPriceTimePriority) PlaceOrder( ); success { offchainUpdates.AddRemoveMessage(orderId, message) } + + if message, success := off_chain_updates.CreateOrderPlaceMessage( + ctx, + order, + ); success { + offchainUpdates.AddPlaceMessage(order.OrderId, message) + } + } + } else { + if message, success := off_chain_updates.CreateOrderPlaceMessage( + ctx, + order, + ); success { + offchainUpdates.AddPlaceMessage(order.OrderId, message) } - } - if message, success := off_chain_updates.CreateOrderPlaceMessage( - ctx, - order, - ); success { - offchainUpdates.AddPlaceMessage(order.OrderId, message) } } From 9c68b0ab8e2437e2ccbaa51b384e0f37bfc9c349 Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Tue, 28 May 2024 17:03:33 -0400 Subject: [PATCH 04/14] revert ci change --- .github/workflows/indexer-build-and-push-dev-staging.yml | 1 - .github/workflows/protocol-build-and-push.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/workflows/indexer-build-and-push-dev-staging.yml b/.github/workflows/indexer-build-and-push-dev-staging.yml index f4084b1628..5a98b72552 100644 --- a/.github/workflows/indexer-build-and-push-dev-staging.yml +++ b/.github/workflows/indexer-build-and-push-dev-staging.yml @@ -6,7 +6,6 @@ on: # yamllint disable-line rule:truthy - main - 'release/indexer/v[0-9]+.[0-9]+.x' # e.g. release/indexer/v0.1.x - 'release/indexer/v[0-9]+.x' # e.g. release/indexer/v1.x - - 'chenyao/short-term-order-replacement-indexer-messages' # TODO(DEC-837): Customize github build and push to ECR by service with paths jobs: diff --git a/.github/workflows/protocol-build-and-push.yml b/.github/workflows/protocol-build-and-push.yml index 8c9509b731..fc26dd6e69 100644 --- a/.github/workflows/protocol-build-and-push.yml +++ b/.github/workflows/protocol-build-and-push.yml @@ -6,7 +6,6 @@ on: # yamllint disable-line rule:truthy - main - 'release/protocol/v[0-9]+.[0-9]+.x' # e.g. release/protocol/v0.1.x - 'release/protocol/v[0-9]+.x' # e.g. release/protocol/v1.x - - 'chenyao/short-term-order-replacement-indexer-messages' jobs: build-and-push-dev: From 9eff31737019d69042f1dcaaf3f71785029e44d0 Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Tue, 28 May 2024 20:13:31 -0400 Subject: [PATCH 05/14] fix tests --- .../memclob_place_order_reduce_only_test.go | 2 ++ .../clob/memclob/memclob_place_order_test.go | 12 +++++-- protocol/x/clob/memclob/memclob_test_util.go | 36 +++++++++++++------ 3 files changed, 37 insertions(+), 13 deletions(-) diff --git a/protocol/x/clob/memclob/memclob_place_order_reduce_only_test.go b/protocol/x/clob/memclob/memclob_place_order_reduce_only_test.go index 3a2f4f184e..56872d5f85 100644 --- a/protocol/x/clob/memclob/memclob_place_order_reduce_only_test.go +++ b/protocol/x/clob/memclob/memclob_place_order_reduce_only_test.go @@ -1385,6 +1385,7 @@ func TestPlaceOrder_ReduceOnly(t *testing.T) { tc.expectedCancelledReduceOnlyOrders, // TODO(IND-261): Add tests for replaced reduce-only orders. false, + false, ) }) } @@ -1668,6 +1669,7 @@ func TestPlaceOrder_LongTermReduceOnlyRemovals(t *testing.T) { tc.expectedNewMatches, tc.expectedCancelledReduceOnlyOrders, false, + false, ) }) } diff --git a/protocol/x/clob/memclob/memclob_place_order_test.go b/protocol/x/clob/memclob/memclob_place_order_test.go index 89cd408d7f..b2597bde8b 100644 --- a/protocol/x/clob/memclob/memclob_place_order_test.go +++ b/protocol/x/clob/memclob/memclob_place_order_test.go @@ -348,6 +348,7 @@ func TestPlaceOrder_AddOrderToOrderbook(t *testing.T) { ordersOnBook = append(ordersOnBook, &order) } + expectedReplacementOrderPriceChanged := false for _, matchableOrder := range ordersOnBook { // Note we assume these are regular orders since liquidation orders cannot rest on // the book. @@ -356,6 +357,9 @@ func TestPlaceOrder_AddOrderToOrderbook(t *testing.T) { // is no longer on the book. matchableOrderOrder := matchableOrder.MustGetOrder() if matchableOrderOrder.OrderId == tc.order.OrderId && tc.order.MustCmpReplacementOrder(&matchableOrderOrder) > 0 { + if matchableOrderOrder.Subticks != tc.order.Subticks { + expectedReplacementOrderPriceChanged = true + } continue } @@ -407,6 +411,7 @@ func TestPlaceOrder_AddOrderToOrderbook(t *testing.T) { []expectedMatch{}, []types.OrderId{}, tc.expectedToReplaceOrder, + expectedReplacementOrderPriceChanged, ) }) } @@ -2244,8 +2249,7 @@ func TestPlaceOrder_MatchOrders_PreexistingMatches(t *testing.T) { expectedOrderStatus: types.InternalError, expectedToReplaceOrder: true, - - expectedErr: types.ErrFokOrderCouldNotBeFullyFilled, + expectedErr: types.ErrFokOrderCouldNotBeFullyFilled, }, } @@ -2304,6 +2308,7 @@ func TestPlaceOrder_MatchOrders_PreexistingMatches(t *testing.T) { tc.expectedNewMatches, []types.OrderId{}, tc.expectedToReplaceOrder, + false, ) }) } @@ -3300,6 +3305,7 @@ func TestPlaceOrder_PostOnly(t *testing.T) { []expectedMatch{}, []types.OrderId{}, false, + false, ) }) } @@ -3443,6 +3449,7 @@ func TestPlaceOrder_ImmediateOrCancel(t *testing.T) { tc.expectedCollatCheck, []types.OrderId{}, false, + false, ) }) } @@ -4117,6 +4124,7 @@ func TestPlaceOrder_FillOrKill(t *testing.T) { expectedMatches, []types.OrderId{}, false, + false, ) }) } diff --git a/protocol/x/clob/memclob/memclob_test_util.go b/protocol/x/clob/memclob/memclob_test_util.go index 43ad915f39..8be30247ee 100644 --- a/protocol/x/clob/memclob/memclob_test_util.go +++ b/protocol/x/clob/memclob/memclob_test_util.go @@ -1315,6 +1315,7 @@ func assertPlaceOrderOffchainMessages( expectedNewMatches []expectedMatch, expectedCancelledReduceOnlyOrders []types.OrderId, expectedToReplaceOrder bool, + expectedReplacementOrderPriceChanged bool, ) { actualOffchainMessages := offchainUpdates.GetMessages() expectedOffchainMessages := []msgsender.Message{} @@ -1322,23 +1323,36 @@ func assertPlaceOrderOffchainMessages( // If there are no errors expected, an order place message should be sent. if expectedErr == nil || doesErrorProduceOffchainMessages(expectedErr) { - var updateMessage msgsender.Message if expectedToReplaceOrder { - updateMessage = off_chain_updates.MustCreateOrderReplaceMessage( - ctx, - order, - ) + if expectedReplacementOrderPriceChanged { + removeMessage := off_chain_updates.MustCreateOrderRemoveMessageWithReason( + ctx, + order.OrderId, + indexershared.OrderRemovalReason_ORDER_REMOVAL_REASON_REPLACED, + ocutypes.OrderRemoveV1_ORDER_REMOVAL_STATUS_BEST_EFFORT_CANCELED, + ) + placeMessage := off_chain_updates.MustCreateOrderPlaceMessage( + ctx, + order, + ) + expectedOffchainMessages = append( + expectedOffchainMessages, + removeMessage, + placeMessage, + ) + require.Equal(t, expectedOffchainMessages, actualOffchainMessages[:len(expectedOffchainMessages)]) + } } else { - updateMessage = off_chain_updates.MustCreateOrderPlaceMessage( + placeMessage := off_chain_updates.MustCreateOrderPlaceMessage( ctx, order, ) + expectedOffchainMessages = append( + expectedOffchainMessages, + placeMessage, + ) + require.Equal(t, expectedOffchainMessages, actualOffchainMessages[:len(expectedOffchainMessages)]) } - expectedOffchainMessages = append( - expectedOffchainMessages, - updateMessage, - ) - require.Equal(t, expectedOffchainMessages, actualOffchainMessages[:len(expectedOffchainMessages)]) } // Reduce-only order removals are sent before updates if the maker orders are removed during From 2412d023bb636e262db8c771be1358b7409df3d6 Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Thu, 30 May 2024 11:40:37 -0400 Subject: [PATCH 06/14] remove sending orderbook message in order-place-handler --- .../vulcan/src/handlers/order-place-handler.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/indexer/services/vulcan/src/handlers/order-place-handler.ts b/indexer/services/vulcan/src/handlers/order-place-handler.ts index 9954969964..55d6955171 100644 --- a/indexer/services/vulcan/src/handlers/order-place-handler.ts +++ b/indexer/services/vulcan/src/handlers/order-place-handler.ts @@ -157,20 +157,6 @@ export class OrderPlaceHandler extends Handler { }; sendMessageWrapper(subaccountMessage, KafkaTopics.TO_WEBSOCKETS_SUBACCOUNTS); } - - // TODO(IND-68): Remove once order replacement flow in V4 protocol removes the old order and - // places the updated order. - if (updatedQuantums !== undefined) { - const orderbookMessage: Message = { - value: this.createOrderbookWebsocketMessage( - placeOrderResult.oldOrder!, - perpetualMarket, - updatedQuantums, - ), - headers, - }; - sendMessageWrapper(orderbookMessage, KafkaTopics.TO_WEBSOCKETS_ORDERBOOKS); - } } /** From 45f795077bab4f5512e5248e0d3315043543a741 Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Thu, 30 May 2024 11:45:18 -0400 Subject: [PATCH 07/14] always send order place message --- protocol/x/clob/memclob/memclob.go | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/protocol/x/clob/memclob/memclob.go b/protocol/x/clob/memclob/memclob.go index d79213d105..131b04cc60 100644 --- a/protocol/x/clob/memclob/memclob.go +++ b/protocol/x/clob/memclob/memclob.go @@ -495,11 +495,9 @@ func (m *MemClobPriceTimePriority) PlaceOrder( } if m.generateOffchainUpdates { + // Send an order place message. // If existing order ID is found and the price of the existing order is different from the new order, - // create an order removal message and then an order place message. - // If the price of the existing order is the same as the new order, don't send any messages here because - // the size of the order will get updated by the Order Update message after the order is matched. - // Otherwise, this is a new order and so send an order place message. + // create an order removal message first so we can remove the original price level from the orderbook. orderId := order.OrderId if existingOrder, found := orderbook.getOrder(orderId); found { if order.Subticks != existingOrder.Subticks { @@ -511,22 +509,14 @@ func (m *MemClobPriceTimePriority) PlaceOrder( ); success { offchainUpdates.AddRemoveMessage(orderId, message) } - - if message, success := off_chain_updates.CreateOrderPlaceMessage( - ctx, - order, - ); success { - offchainUpdates.AddPlaceMessage(order.OrderId, message) - } - } - } else { - if message, success := off_chain_updates.CreateOrderPlaceMessage( - ctx, - order, - ); success { - offchainUpdates.AddPlaceMessage(order.OrderId, message) } } + if message, success := off_chain_updates.CreateOrderPlaceMessage( + ctx, + order, + ); success { + offchainUpdates.AddPlaceMessage(order.OrderId, message) + } } // Attempt to match the order against the orderbook. From 1bbaee8c4778c012d4af1a5b2099889ca361880c Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Thu, 30 May 2024 12:39:22 -0400 Subject: [PATCH 08/14] lint --- indexer/services/vulcan/src/handlers/order-place-handler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/indexer/services/vulcan/src/handlers/order-place-handler.ts b/indexer/services/vulcan/src/handlers/order-place-handler.ts index 55d6955171..17eaeafb0a 100644 --- a/indexer/services/vulcan/src/handlers/order-place-handler.ts +++ b/indexer/services/vulcan/src/handlers/order-place-handler.ts @@ -99,7 +99,7 @@ export class OrderPlaceHandler extends Handler { // TODO(IND-68): Remove once order replacement flow in V4 protocol removes the old order and // places the updated order. - const updatedQuantums: number | undefined = await this.updatePriceLevel( + await this.updatePriceLevel( placeOrderResult, perpetualMarket, update, From 096c8b48f62f9a2b6ab579732c13d4259107a575 Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Thu, 30 May 2024 13:02:44 -0400 Subject: [PATCH 09/14] fix vulcan tests --- .../vulcan/__tests__/handlers/order-place-handler.test.ts | 6 ------ 1 file changed, 6 deletions(-) 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 f4c139b7f2..a71617b510 100644 --- a/indexer/services/vulcan/__tests__/handlers/order-place-handler.test.ts +++ b/indexer/services/vulcan/__tests__/handlers/order-place-handler.test.ts @@ -641,12 +641,6 @@ describe('order-place-handler', () => { testConstants.defaultPerpetualMarket, APIOrderStatusEnum.BEST_EFFORT_OPENED, expectSubaccountMessage, - expectOrderBookUpdate - ? OrderbookMessage.fromPartial({ - contents: JSON.stringify(orderbookContents), - clobPairId: testConstants.defaultPerpetualMarket.clobPairId, - version: ORDERBOOKS_WEBSOCKET_MESSAGE_VERSION, - }) : undefined, ); expectStats(true); }); From 0f3185a84ef557a9bdcc6f992d7dbf7208994172 Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Thu, 30 May 2024 13:03:00 -0400 Subject: [PATCH 10/14] fix membclob tests --- protocol/x/clob/memclob/memclob_test_util.go | 25 +++++++------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/protocol/x/clob/memclob/memclob_test_util.go b/protocol/x/clob/memclob/memclob_test_util.go index d7f4489784..ddac1d590d 100644 --- a/protocol/x/clob/memclob/memclob_test_util.go +++ b/protocol/x/clob/memclob/memclob_test_util.go @@ -1308,28 +1308,21 @@ func assertPlaceOrderOffchainMessages( indexershared.OrderRemovalReason_ORDER_REMOVAL_REASON_REPLACED, ocutypes.OrderRemoveV1_ORDER_REMOVAL_STATUS_BEST_EFFORT_CANCELED, ) - placeMessage := off_chain_updates.MustCreateOrderPlaceMessage( - ctx, - order, - ) expectedOffchainMessages = append( expectedOffchainMessages, removeMessage, - placeMessage, ) - require.Equal(t, expectedOffchainMessages, actualOffchainMessages[:len(expectedOffchainMessages)]) } - } else { - placeMessage := off_chain_updates.MustCreateOrderPlaceMessage( - ctx, - order, - ) - expectedOffchainMessages = append( - expectedOffchainMessages, - placeMessage, - ) - require.Equal(t, expectedOffchainMessages, actualOffchainMessages[:len(expectedOffchainMessages)]) } + placeMessage := off_chain_updates.MustCreateOrderPlaceMessage( + ctx, + order, + ) + expectedOffchainMessages = append( + expectedOffchainMessages, + placeMessage, + ) + require.Equal(t, expectedOffchainMessages, actualOffchainMessages[:len(expectedOffchainMessages)]) } // Reduce-only order removals are sent before updates if the maker orders are removed during From e6aef53c367868d64d54f646bc7b1c89748f64cc Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Thu, 30 May 2024 14:13:18 -0400 Subject: [PATCH 11/14] lint --- .../__tests__/handlers/order-place-handler.test.ts | 13 ------------- 1 file changed, 13 deletions(-) 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 a71617b510..a367631530 100644 --- a/indexer/services/vulcan/__tests__/handlers/order-place-handler.test.ts +++ b/indexer/services/vulcan/__tests__/handlers/order-place-handler.test.ts @@ -7,7 +7,6 @@ import { import { synchronizeWrapBackgroundTask } from '@dydxprotocol-indexer/dev'; import { createKafkaMessage, - ORDERBOOKS_WEBSOCKET_MESSAGE_VERSION, producer, SUBACCOUNTS_WEBSOCKET_MESSAGE_VERSION, getTriggerPrice, @@ -19,7 +18,6 @@ import { blockHeightRefresher, BlockTable, dbHelpers, - OrderbookMessageContents, OrderFromDatabase, OrderTable, PerpetualMarketFromDatabase, @@ -62,7 +60,6 @@ import { redisClient, redisClient as client } from '../../src/helpers/redis/redi import { onMessage } from '../../src/lib/on-message'; import { expectCanceledOrderStatus, expectOpenOrderIds, handleInitialOrderPlace } from '../helpers/helpers'; import { expectOffchainUpdateMessage, expectWebsocketOrderbookMessage, expectWebsocketSubaccountMessage } from '../helpers/websocket-helpers'; -import { OrderbookSide } from '../../src/lib/types'; import { getOrderIdHash, isLongTermOrder, isStatefulOrder } from '@dydxprotocol-indexer/v4-proto-parser'; import { defaultKafkaHeaders } from '../helpers/constants'; import config from '../../src/config'; @@ -545,10 +542,6 @@ describe('order-place-handler', () => { const expectedPriceLevelQuantums: number = ( oldPriceLevelInitialQuantums - (Number(initialOrderToPlace.quantums) - oldOrderTotalFilled) ); - const expectedPriceLevelSize: string = protocolTranslations.quantumsToHumanFixedString( - expectedPriceLevelQuantums.toString(), - testConstants.defaultPerpetualMarket.atomicResolution, - ); const expectedPriceLevel: PriceLevel = { humanPrice: expectedRedisOrder.price, quantums: expectedPriceLevelQuantums.toString(), @@ -628,12 +621,6 @@ describe('order-place-handler', () => { await expectOpenOrderIds(testConstants.defaultPerpetualMarket.clobPairId, []); expect(logger.error).not.toHaveBeenCalled(); - const orderbookContents: OrderbookMessageContents = { - [OrderbookSide.BIDS]: [[ - redisTestConstants.defaultPrice, - expectedPriceLevelSize, - ]], - }; expectWebsocketMessagesSent( producerSendSpy, expectedReplacedOrder, From 89834965211d855371f4cfb8667a47ec6b47c0dd Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Fri, 31 May 2024 13:48:36 -0400 Subject: [PATCH 12/14] Add TODO --- protocol/x/clob/memclob/memclob.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/protocol/x/clob/memclob/memclob.go b/protocol/x/clob/memclob/memclob.go index b1f88d66d6..f19d1b10eb 100644 --- a/protocol/x/clob/memclob/memclob.go +++ b/protocol/x/clob/memclob/memclob.go @@ -496,8 +496,9 @@ func (m *MemClobPriceTimePriority) PlaceOrder( if m.generateOffchainUpdates { // Send an order place message. - // If existing order ID is found and the price of the existing order is different from the new order, + // For replacement orders, if the price of the existing order is different from the new order, // create an order removal message first so we can remove the original price level from the orderbook. + // TODO (CT-884): send OrderReplaceV1 message for replacement orders and add order-replace-handler to Vulcan orderId := order.OrderId if existingOrder, found := orderbook.getOrder(orderId); found { if order.Subticks != existingOrder.Subticks { From b8df6230861a2001017fede578e03755d1a7d642 Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Fri, 31 May 2024 16:44:13 -0400 Subject: [PATCH 13/14] don't update OrderbookLevelsCache in order-place-handler --- .../__tests__/handlers/order-place-handler.test.ts | 13 ++----------- .../vulcan/src/handlers/order-place-handler.ts | 8 -------- 2 files changed, 2 insertions(+), 19 deletions(-) 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 7a7d43b775..4fff013d98 100644 --- a/indexer/services/vulcan/__tests__/handlers/order-place-handler.test.ts +++ b/indexer/services/vulcan/__tests__/handlers/order-place-handler.test.ts @@ -536,14 +536,9 @@ describe('order-place-handler', () => { ) => { const oldOrderTotalFilled: number = 10; const oldPriceLevelInitialQuantums: number = Number(initialOrderToPlace.quantums) * 2; - // After replacing the order the quantums at the price level of the old order should be: - // initial quantums - (old order quantums - old order total filled) - const expectedPriceLevelQuantums: number = ( - oldPriceLevelInitialQuantums - (Number(initialOrderToPlace.quantums) - oldOrderTotalFilled) - ); const expectedPriceLevel: PriceLevel = { humanPrice: expectedRedisOrder.price, - quantums: expectedPriceLevelQuantums.toString(), + quantums: oldPriceLevelInitialQuantums.toString(), lastUpdated: expect.stringMatching(/^[0-9]{10}$/), }; @@ -575,6 +570,7 @@ describe('order-place-handler', () => { newTotalFilledQuantums: oldOrderTotalFilled, client, }); + // Update the price level in the order book to a value larger than the quantums of the order await OrderbookLevelsCache.updatePriceLevel({ ticker: testConstants.defaultPerpetualMarket.ticker, @@ -1086,11 +1082,6 @@ describe('order-place-handler', () => { APIOrderStatusEnum.BEST_EFFORT_OPENED, true, ); - - expect(logger.info).toHaveBeenCalledWith(expect.objectContaining({ - at: 'OrderPlaceHandler#handle', - message: 'Total filled of order in Redis exceeds order quantums.', - })); }); }); }); diff --git a/indexer/services/vulcan/src/handlers/order-place-handler.ts b/indexer/services/vulcan/src/handlers/order-place-handler.ts index d22a706d93..2916093cf2 100644 --- a/indexer/services/vulcan/src/handlers/order-place-handler.ts +++ b/indexer/services/vulcan/src/handlers/order-place-handler.ts @@ -96,14 +96,6 @@ export class OrderPlaceHandler extends Handler { placeOrderResult, }); - // TODO(IND-68): Remove once order replacement flow in V4 protocol removes the old order and - // places the updated order. - await this.updatePriceLevel( - placeOrderResult, - perpetualMarket, - update, - ); - if (placeOrderResult.replaced) { stats.increment(`${config.SERVICE_NAME}.place_order_handler.replaced_order`, 1); } From 0d900eb40babe65716e7b78a401a90fcc7195458 Mon Sep 17 00:00:00 2001 From: Chenyao Yu <4844716+chenyaoy@users.noreply.github.com> Date: Mon, 3 Jun 2024 15:01:13 -0400 Subject: [PATCH 14/14] remove unused code --- .../src/handlers/order-place-handler.ts | 81 ------------------- 1 file changed, 81 deletions(-) diff --git a/indexer/services/vulcan/src/handlers/order-place-handler.ts b/indexer/services/vulcan/src/handlers/order-place-handler.ts index 2916093cf2..d32021c43b 100644 --- a/indexer/services/vulcan/src/handlers/order-place-handler.ts +++ b/indexer/services/vulcan/src/handlers/order-place-handler.ts @@ -6,11 +6,9 @@ import { OrderTable, PerpetualMarketFromDatabase, perpetualMarketRefresher, - protocolTranslations, } from '@dydxprotocol-indexer/postgres'; import { CanceledOrdersCache, - OrderbookLevelsCache, placeOrder, PlaceOrderResult, StatefulOrderUpdatesCache, @@ -21,7 +19,6 @@ import { isLongTermOrder, isStatefulOrder, ORDER_FLAG_SHORT_TERM, - requiresImmediateExecution, } from '@dydxprotocol-indexer/v4-proto-parser'; import { IndexerOrder, @@ -31,7 +28,6 @@ import { OrderUpdateV1, RedisOrder, } from '@dydxprotocol-indexer/v4-protos'; -import Big from 'big.js'; import { IHeaders, Message } from 'kafkajs'; import config from '../config'; @@ -139,83 +135,6 @@ export class OrderPlaceHandler extends Handler { } } - /** - * Updates the price level given the result of calling `placeOrder`. - * @param result `PlaceOrderResult` from calling `placeOrder` - * @param perpetualMarket Perpetual market object corresponding to the perpetual market of the - * order - * @param update Off-chain update - * @returns - */ - // eslint-disable-next-line @typescript-eslint/require-await - protected async updatePriceLevel( - result: PlaceOrderResult, - perpetualMarket: PerpetualMarketFromDatabase, - update: OffChainUpdateV1, - ): 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 || - requiresImmediateExecution(result.oldOrder!.order!.timeInForce) - ) { - return undefined; - } - - const remainingSizeDeltaInQuantums: Big = this.getRemainingSizeDeltaInQuantums(result); - - if (remainingSizeDeltaInQuantums.eq(0)) { - // No update to the price level if remaining quantums = 0 - // An order could have remaining quantums = 0 intra-block, as an order is only considered - // filled once the fills are committed in a block - return undefined; - } - - if (remainingSizeDeltaInQuantums.lt(0)) { - // Log error and skip updating orderbook levels if old order had negative remaining - // quantums - logger.info({ - at: 'OrderPlaceHandler#handle', - message: 'Total filled of order in Redis exceeds order quantums.', - placeOrderResult: result, - update, - }); - stats.increment(`${config.SERVICE_NAME}.order_place_total_filled_exceeds_size`, 1); - return undefined; - } - - // If the remaining size is not equal or less than 0, it must be greater than 0. - // Remove the remaining size of the replaced order from the orderbook, by decrementing - // the total size of orders at the price of the replaced order - return runFuncWithTimingStat( - OrderbookLevelsCache.updatePriceLevel({ - ticker: perpetualMarket.ticker, - side: protocolTranslations.protocolOrderSideToOrderSide(result.oldOrder!.order!.side), - humanPrice: result.oldOrder!.price, - // Delta should be -1 * remaining size of order in quantums and an integer - sizeDeltaInQuantums: remainingSizeDeltaInQuantums.mul(-1).toFixed(0), - client: redisClient, - }), - this.generateTimingStatsOptions('update_price_level'), - ); - } - - /** - * Gets the remaining size of the old order if the order was replaced. - * @param result Result of placing an order, should be for a replaced order so both `oldOrder` and - * `oldTotalFilledQuantums` properties should exist on the place order result. - * @returns Remaining size of the old order that was replaced. - */ - protected getRemainingSizeDeltaInQuantums(result: PlaceOrderResult): Big { - const sizeDeltaInQuantums: Big = Big( - result.oldOrder!.order!.quantums.toString(), - ).minus( - result.oldTotalFilledQuantums!, - ); - return sizeDeltaInQuantums; - } - protected validateOrderPlace(orderPlace: OrderPlaceV1): void { if (orderPlace.order === undefined) { this.logAndThrowParseMessageError('Invalid OrderPlace, order is undefined');