[IND-402] Have vulcan re-queue cached order updates instead of ender.

[vincentwschau]

Changelist

Fixes issue found in testing #683 , where ender does not find the cached update. There's a race condition where ender processes a block before vulcan caches the order update. Moved re-queuing logic to vulcan to fix the issue as the order update will either be cached beforethe order place is received, or the order update will be processed after the order place is received.

Other updates:

• remove re-queuing of order updates from ender
• update vulcan to send both keys and values in Kafka messages

Test Plan

Unit tested, and tested on dev.

Author/Reviewer Checklist

• If this PR has changes that result in a different app state given the same prior state and transaction list, manually add the state-breaking label.
• If this PR isn't state-breaking but has changes that modify behavior in PrepareProposal or ProcessProposal, manually add the label proposal-breaking.
• If this PR is one of many that implement a specific feature, manually label them all feature:[feature-name].
• If you wish to for mergify-bot to automatically create a PR to backport your change to a release branch, manually add the label backport/[branch-name].
• Manually add any of the following labels: refactor, chore, bug.

[linear]

IND-402 Fix Stateful order placement with no fill

context

Stateful orders that are not filled do not get added to the order book.

Issue:
1. Orders aren't set to be "resting on the book" until an OrderUpdate message is sent for the message. After receiving an OrderPlace message the order does not have it's size added to the order book.
2. OrderUpdate messages aren't processed if an OrderPlace message was not previously processed for the order.
3. For stateful orders
- the OrderPlace message is sent after ender processes the order
- the OrderUpdate message is sent by the node after the stateful order is placed
4. The OrderUpdate message is received before the OrderPlace, due to it being sent from a different service.

Potential Solution:

• keep track of all OrderUpdate messages for stateful orders that don't update an order in a separate cache similar to the CanceledOrdersCache

• when a stateful order placement is ingested by ender, check if there's a corresponding OrderUpdate message, and if so, send it to vulcan

• the difference in time between the OrderPlace and OrderUpdate should be a most a few blocks, so keep only the last 1 minute of OrderUpdate messages in the cache

[coderabbitai]

Walkthrough

The changes primarily focus on transitioning from a Redis-based caching system to a Kafka-based messaging system for handling stateful order updates. The code has been refactored to remove Redis dependencies and introduce Kafka message handling. The tests have been updated to reflect these changes.

Changes

File(s)	Summary
.../stateful-order-placement-handler.test.ts .../stateful-order-placement-handler.ts	Removed Redis dependencies and simplified the logic for handling stateful order updates.
.../order-place-handler.test.ts .../order-place-handler.ts	Introduced handling of cached order updates and sending off-chain update messages.
.../websocket-helpers.ts	Added a new function to handle off-chain update messages.
.../send-message-helper.test.ts .../config.ts .../order-remove-handler.ts .../order-update-handler.ts .../index.ts .../send-message-helper.ts	Transitioned from a Redis-based system to a Kafka-based system for message handling. Renamed and refactored functions to reflect this change. Updated configuration and error handling.

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• If you reply to a review comment from CodeRabbit, the bot will automatically respond.
• To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
• Note: Review comments are made on code diffs or files, not on the PR overview.
• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.json

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between cba238d and 29f63d6.

Files selected for processing (11)

• indexer/services/ender/tests/handlers/stateful-order/stateful-order-placement-handler.test.ts (5 hunks)
• indexer/services/ender/src/handlers/stateful-order/stateful-order-placement-handler.ts (2 hunks)
• indexer/services/vulcan/tests/handlers/order-place-handler.test.ts (8 hunks)
• indexer/services/vulcan/tests/helpers/websocket-helpers.ts (2 hunks)
• indexer/services/vulcan/tests/lib/send-message-helper.test.ts (6 hunks)
• indexer/services/vulcan/src/config.ts (1 hunks)
• indexer/services/vulcan/src/handlers/order-place-handler.ts (5 hunks)
• indexer/services/vulcan/src/handlers/order-remove-handler.ts (4 hunks)
• indexer/services/vulcan/src/handlers/order-update-handler.ts (2 hunks)
• indexer/services/vulcan/src/index.ts (1 hunks)
• indexer/services/vulcan/src/lib/send-message-helper.ts (6 hunks)

Files skipped from review due to trivial changes (6)

• indexer/services/ender/tests/handlers/stateful-order/stateful-order-placement-handler.test.ts
• indexer/services/ender/src/handlers/stateful-order/stateful-order-placement-handler.ts
• indexer/services/vulcan/tests/lib/send-message-helper.test.ts
• indexer/services/vulcan/src/config.ts
• indexer/services/vulcan/src/handlers/order-update-handler.ts
• indexer/services/vulcan/src/index.ts

Additional comments: 29

indexer/services/vulcan/__tests__/helpers/websocket-helpers.ts (2)

• 2-2: The import of OffChainUpdateV1 is new and seems to be used in the new function expectOffchainUpdateMessage. Ensure that this import is used elsewhere in the codebase and that the new function is being called correctly.

• 33-48: The new function expectOffchainUpdateMessage is added to handle off-chain update messages. It checks if the topic of the producer record is equal to KafkaTopics.TO_VULCAN, decodes the message, and checks if the decoded message and key are equal to the expected ones. This function seems to be correctly implemented.

indexer/services/vulcan/src/handlers/order-remove-handler.ts (4)

• 36-40: The import statement for sendWebsocketWrapper has been replaced with sendMessageWrapper. Ensure that the new function sendMessageWrapper is compatible with the existing code and performs the same functionality as the old sendWebsocketWrapper function.

• 222-229: The subaccountMessage object has been updated to use the Message type from kafkajs instead of Buffer. The sendWebsocketWrapper function has been replaced with sendMessageWrapper. Ensure that the Message type is compatible with the sendMessageWrapper function and that the message is correctly sent.

• 279-289: The subaccountMessage object has been updated to use the Message type from kafkajs instead of Buffer. The sendWebsocketWrapper function has been replaced with sendMessageWrapper. Ensure that the Message type is compatible with the sendMessageWrapper function and that the message is correctly sent.

• 323-330: The orderbookMessage object has been updated to use the Message type from kafkajs instead of Buffer. The sendWebsocketWrapper function has been replaced with sendMessageWrapper. Ensure that the Message type is compatible with the sendMessageWrapper function and that the message is correctly sent.

indexer/services/vulcan/src/lib/send-message-helper.ts (6)

• 9-9: The type of queuedMessages has been changed from Buffer[] to Message[]. Ensure that all usages of queuedMessages throughout the codebase have been updated to handle Message objects instead of Buffer objects.

• 34-34: The function sendWebsocketWrapper has been renamed to sendMessageWrapper and its parameter type has been changed from Buffer to Message. Ensure that all calls to this function throughout the codebase have been updated to match the new function signature.

• 51-51: The configuration property FLUSH_WEBSOCKET_MESSAGES_INTERVAL_MS has been renamed to FLUSH_KAFKA_MESSAGES_INTERVAL_MS. Ensure that the configuration files and environment variables have been updated to reflect this change.

• 78-78: The type of messages has been changed from Buffer[] to Message[]. Ensure that all usages of messages throughout the codebase have been updated to handle Message objects instead of Buffer objects.

• 92-92: The messages array is now directly passed to producer.send(), whereas previously each Buffer in the array was wrapped in an object with a value property. Ensure that the producer.send() function can handle an array of Message objects.

• 105-105: The function sendWebsocketWrapper has been replaced with sendMessageWrapper. Ensure that the sendMessageWrapper function can handle Message objects.

indexer/services/vulcan/src/handlers/order-place-handler.ts (6)

• 26-28: The new import StatefulOrderUpdatesCache and isStatefulOrder from @dydxprotocol-indexer/redis and @dydxprotocol-indexer/v4-proto-parser respectively, are added to handle stateful orders. Ensure that these modules are correctly implemented and tested.

• 53-54: The comment indicates that if the order is a stateful order, it will attempt to remove any cached order update from the StatefulOrderUpdatesCache, and then queue the order update to be re-sent and re-processed. This is a new behavior introduced in the handle method.

• 131-134: The condition to check if an order is stateful has been updated. Previously, it was checking if goodTilBlockTime was defined. Now it uses the isStatefulOrder function. Ensure that this change is intended and correctly implemented.

144:
The new method sendCachedOrderUpdate is called for stateful orders. This method is responsible for sending any cached order updates.

• 146-153: The subaccountMessage is now an object of type Message instead of Buffer. The value field of the Message object is assigned the result of createSubaccountWebsocketMessage. This change is part of the update to use Message objects instead of Buffer objects for queued messages.

154:
The sendMessageWrapper function is used instead of sendWebsocketWrapper. This change is part of the update to make the function more generic and usable across multiple handlers.

• 160-167: Similar to the subaccountMessage, the orderbookMessage is now an object of type Message instead of Buffer. The value field of the Message object is assigned the result of createOrderbookWebsocketMessage. The sendMessageWrapper function is used to send the message.

• 363-384: The new method sendCachedOrderUpdate is added. This method removes and sends the cached order update for the given order id if it exists. It uses the removeStatefulOrderUpdate method from StatefulOrderUpdatesCache to remove the cached order update. If a cached order update is found, it creates a Message object and sends it using the sendMessageWrapper function.

indexer/services/vulcan/__tests__/handlers/order-place-handler.test.ts (11)

• 43-43: The import of StatefulOrderUpdatesCache from @dydxprotocol-indexer/redis indicates that the test suite now includes tests for stateful order updates. This is in line with the PR summary which mentions the addition of a new method sendCachedOrderUpdate to handle cached order updates for stateful orders.

• 54-55: The import of OrderUpdateV1 from @dydxprotocol-indexer/v4-protos suggests that the test suite now includes tests for order updates. This aligns with the PR summary which mentions the addition of a new method sendCachedOrderUpdate to handle cached order updates for stateful orders.

• 62-62: The import of expectOffchainUpdateMessage from ../helpers/websocket-helpers suggests that the test suite now includes tests for off-chain update messages. This aligns with the PR summary which mentions the addition of a new method sendCachedOrderUpdate to handle cached order updates for stateful orders.

• 71-74: The OffchainUpdateRecord interface has been added to represent off-chain update records. This aligns with the PR summary which mentions the addition of a new method sendCachedOrderUpdate to handle cached order updates for stateful orders.

• 651-687: The test case 'handles order place with OPEN placement status, does not exist initially' has been updated to include scenarios with and without cached order updates. This aligns with the PR summary which mentions the addition of a new method sendCachedOrderUpdate to handle cached order updates for stateful orders.

• 699-713: The test case now includes a setup for cached order updates. If a cached order update is provided, it is added to the StatefulOrderUpdatesCache and an expectedOffchainUpdate is created. This aligns with the PR summary which mentions the addition of a new method sendCachedOrderUpdate to handle cached order updates for stateful orders.

• 732-733: The expectWebsocketMessagesSent function is now called with expectedOffchainUpdate as an argument. This suggests that the function has been updated to check for off-chain update messages.

• 1019-1019: The expectWebsocketMessagesSent function now accepts an expectedOffchainUpdate parameter. This suggests that the function has been updated to check for off-chain update messages.

• 1030-1044: The test case now includes checks for off-chain update messages. If an expectedOffchainUpdate is provided, the test case expects an off-chain update message to be sent and checks the message using the expectOffchainUpdateMessage function.

• 1088-1090: The test case now increments the callIndex after checking the subaccount message. This suggests that the order of messages being sent has been updated.

• 1092-1092: The test case now checks the orderbook message after checking the subaccount message and off-chain update message. This suggests that the order of messages being sent has been updated.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 29f63d6 and bfd4222.

Files selected for processing (1)

• indexer/services/vulcan/tests/handlers/order-place-handler.test.ts (8 hunks)

Additional comments: 9

indexer/services/vulcan/__tests__/handlers/order-place-handler.test.ts (9)

• 43-43: The import of StatefulOrderUpdatesCache from @dydxprotocol-indexer/redis indicates that the test suite now includes tests for the handling of cached order updates. This aligns with the PR summary that mentions the addition of cached order update handling in the OrderPlaceHandler class.

• 54-55: The import of OrderUpdateV1 from @dydxprotocol-indexer/v4-protos is new and is likely used for testing the handling of cached order updates.

• 62-62: The addition of expectOffchainUpdateMessage to the import from ../helpers/websocket-helpers indicates that the test suite now includes tests for sending off-chain update messages. This aligns with the PR summary that mentions the replacement of the sendWebsocketWrapper function with sendMessageWrapper to send Kafka messages.

• 71-74: The OffchainUpdateRecord interface is new and is likely used for testing the handling of cached order updates.

• 651-687: The test case 'handles order place with OPEN placement status, does not exist initially' has been updated to include scenarios with and without cached order updates. This aligns with the PR summary that mentions the addition of cached order update handling in the OrderPlaceHandler class.

• 699-713: The addition of code to add a cached order update to the StatefulOrderUpdatesCache and set expectedOffchainUpdate indicates that the test case now includes a scenario where a cached order update exists. This aligns with the PR summary that mentions the addition of cached order update handling in the OrderPlaceHandler class.

• 1019-1019: The addition of expectedOffchainUpdate to the parameters of the expectWebsocketMessagesSent function indicates that the function now checks for the sending of off-chain update messages. This aligns with the PR summary that mentions the replacement of the sendWebsocketWrapper function with sendMessageWrapper to send Kafka messages.

• 1030-1044: The addition of code to check for the sending of an off-chain update message indicates that the expectWebsocketMessagesSent function now includes a scenario where an off-chain update message is sent. This aligns with the PR summary that mentions the replacement of the sendWebsocketWrapper function with sendMessageWrapper to send Kafka messages.

• 1088-1089: The update to increment callIndex after checking for a subaccount message indicates that the expectWebsocketMessagesSent function now accounts for the possibility of an off-chain update message being sent before a subaccount message. This aligns with the PR summary that mentions the replacement of the sendWebsocketWrapper function with sendMessageWrapper to send Kafka messages.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between bfd4222 and b4079d1.

Files selected for processing (1)

• indexer/services/vulcan/src/handlers/order-place-handler.ts (5 hunks)

Additional comments: 6

indexer/services/vulcan/src/handlers/order-place-handler.ts (6)

• 26-28: The StatefulOrderUpdatesCache and getOrderIdHash and isStatefulOrder are newly imported. Ensure that these are used correctly in the code.

• 53-54: The comment indicates that for stateful orders, the code will attempt to remove any cached order update and then queue the order update to be re-sent and re-processed. This is a new behavior and should be tested thoroughly.

• 131-134: The condition for checking if an order is stateful has been changed. Previously, it was based on the goodTilBlockTime property of the order. Now, it uses the isStatefulOrder function with the orderFlags property of the order. Ensure that this change is intentional and correctly reflects the definition of a stateful order.

144:
The sendCachedOrderUpdate method is called for stateful orders. This is a new behavior and should be tested thoroughly.

• 146-154: The sendWebsocketWrapper function has been replaced with sendMessageWrapper and now handles Message objects instead of Buffer objects. Ensure that this change is compatible with the rest of the codebase and that the Message objects are correctly formed.

• 160-167: Similar to the previous comment, the sendWebsocketWrapper function has been replaced with sendMessageWrapper and now handles Message objects instead of Buffer objects. Ensure that this change is compatible with the rest of the codebase and that the Message objects are correctly formed.

• 363-384: The sendCachedOrderUpdate method is a new addition. It removes and sends the cached order update for a given order id if it exists. This method should be tested thoroughly to ensure it behaves as expected.

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between b4079d1 and 46fa4da.

Files selected for processing (1)

• indexer/services/ender/tests/handlers/stateful-order/stateful-order-placement-handler.test.ts (5 hunks)

Files skipped from review due to trivial changes (1)

• indexer/services/ender/tests/handlers/stateful-order/stateful-order-placement-handler.test.ts