Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CT-839] Add blockHeight to subaccount websocket message #1585

Merged
merged 6 commits into from
May 28, 2024
Merged

Conversation

dydxwill
Copy link
Contributor

@dydxwill dydxwill commented May 24, 2024

Changelist

Add blockHeight to subaccount websocket message

Test Plan

Unit tested

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 the PR has breaking postgres changes to the indexer add the indexer-postgres-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.

Summary by CodeRabbit

  • New Features

    • Implemented caching and updating of the latest block height from the database for better performance and accuracy.
    • Included block height information in various functionalities to enhance data integrity.
  • Enhancements

    • Improved order handling by incorporating the latest block height data in the order removal process.

Copy link
Contributor

coderabbitai bot commented May 24, 2024

Walkthrough

The updates introduce a caching mechanism for the latest block height in the block-height-refresher.ts file. This mechanism is utilized across different services (ender and vulcan) to include block height information in their operations. Key functions for starting the refresh loop, updating, retrieving, and clearing the block height are added. These changes ensure that the latest block height is consistently available and used in message contents.

Changes

Files Change Summary
indexer/packages/postgres/src/loops/block-height-refresher.ts Introduced functions for caching and updating the latest block height in-memory.
indexer/services/ender/src/helpers/kafka-helper.ts Added blockHeight parameter to functions for including block height in SubaccountMessageContents.
indexer/services/vulcan/src/handlers/order-remove-handler.ts Integrated blockHeightRefresher to retrieve and use the latest block height in OrderRemoveHandler.

Sequence Diagram(s) (Beta)

sequenceDiagram
    participant Postgres as BlockHeightRefresher
    participant Ender as KafkaHelper
    participant Vulcan as OrderRemoveHandler
    participant DB as Database

    Note over Postgres: Initialization
    Postgres->>DB: Fetch latest block height
    DB-->>Postgres: Return latest block height
    Postgres->>Postgres: Cache block height

    Note over Ender, Vulcan: Usage
    Ender->>Postgres: getLatestBlockHeight()
    Postgres-->>Ender: Return cached block height
    Ender->>Ender: Use block height in message contents
    
    Vulcan->>Postgres: getLatestBlockHeight()
    Postgres-->>Vulcan: Return cached block height
    Vulcan->>Vulcan: Use block height in message contents
Loading

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

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 an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to full the review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • 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/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Outside diff range and nitpick comments (6)
indexer/packages/kafka/src/websocket-helper.ts (1)

[!TIP]
Codebase Verification

The blockHeight parameter is not passed when calling the generateSubaccountMessageContents function in stateful-order-placement-handler.ts. This needs to be corrected to ensure proper integration.

  • File: indexer/services/ender/src/handlers/stateful-order/stateful-order-placement-handler.ts
  • Issue: blockHeight parameter is missing in the function call to generateSubaccountMessageContents.
Analysis chain

Line range hint 49-93: Verify the correct integration of blockHeight in the generateSubaccountMessageContents function.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the correct integration of `blockHeight` in the `generateSubaccountMessageContents` function.

# Test: Search for the function usage. Expect: Correct implementation.
rg --type typescript $'generateSubaccountMessageContents'

Length of output: 91



Script:

#!/bin/bash
# Description: Verify the correct integration of `blockHeight` in the `generateSubaccountMessageContents` function.

# Test: Search for the function definition and its usage. Expect: Correct implementation.
rg --type ts 'function generateSubaccountMessageContents'
rg --type ts 'generateSubaccountMessageContents'

Length of output: 781



Script:

#!/bin/bash
# Description: Extract the relevant code for `generateSubaccountMessageContents` function definition and its usage.

# Extract the function definition from `indexer/packages/kafka/src/websocket-helper.ts`
rg --type ts --context 10 'function generateSubaccountMessageContents' indexer/packages/kafka/src/websocket-helper.ts

# Extract the usage from `indexer/services/ender/src/handlers/stateful-order/stateful-order-placement-handler.ts`
rg --type ts --context 10 'generateSubaccountMessageContents' indexer/services/ender/src/handlers/stateful-order/stateful-order-placement-handler.ts

Length of output: 2333

indexer/packages/postgres/src/types/websocket-message-types.ts (1)

40-40: Add a comment describing the purpose of blockHeight.

Consider adding a comment above the blockHeight field to explain its purpose and usage, enhancing code readability and maintainability.

indexer/services/socks/src/lib/subscription.ts (2)

Line range hint 125-126: Prefer template literals over string concatenation for constructing error messages.

- `Too many subscribe attempts for channel ${channel}-${subscriptionId}. Please ` + ' reconnect and try again.'
+ `Too many subscribe attempts for channel ${channel}-${subscriptionId}. Please reconnect and try again.`

Line range hint 255-257: Consider using for...of instead of forEach for better performance and readability.

- subscriptionList.forEach((subscription) => {
+ for (const subscription of subscriptionList) {
indexer/services/vulcan/__tests__/handlers/order-update-handler.test.ts (1)

Line range hint 512-512: Specify a more appropriate type instead of any for bidContents to ensure type safety.

- function orderbookMessageFromBidContents(bidContents: any): OrderbookMessage {
+ function orderbookMessageFromBidContents(bidContents: OrderbookMessageContents): OrderbookMessage {
indexer/services/vulcan/__tests__/handlers/order-remove-handler.test.ts (1)

Line range hint 490-490: Remove non-null assertions and handle potential null values properly.

- const order: OrderFromDatabase | undefined = await OrderTable.findById(orderId);
- expect(order!).toBeDefined();
+ const order: OrderFromDatabase | undefined = await OrderTable.findById(orderId);
+ expect(order).toBeDefined();
+ if (order === undefined) throw new Error('Order not found');

Also applies to: 494-494, 497-497, 499-499, 500-500, 500-500, 501-501, 503-503, 509-509, 632-632, 636-636, 639-639, 641-641, 642-642, 642-642, 643-643, 645-645, 651-651, 774-774

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 76c548c and 85d634b.
Files selected for processing (15)
  • indexer/packages/kafka/src/websocket-helper.ts (3 hunks)
  • indexer/packages/postgres/tests/loops/block-height-refresher.test.ts (1 hunks)
  • indexer/packages/postgres/src/config.ts (1 hunks)
  • indexer/packages/postgres/src/index.ts (1 hunks)
  • indexer/packages/postgres/src/loops/block-height-refresher.ts (1 hunks)
  • indexer/packages/postgres/src/types/websocket-message-types.ts (1 hunks)
  • indexer/services/comlink/public/websocket-documentation.md (3 hunks)
  • indexer/services/socks/src/index.ts (2 hunks)
  • indexer/services/socks/src/lib/subscription.ts (3 hunks)
  • indexer/services/vulcan/tests/handlers/order-place-handler.test.ts (4 hunks)
  • indexer/services/vulcan/tests/handlers/order-remove-handler.test.ts (13 hunks)
  • indexer/services/vulcan/tests/handlers/order-update-handler.test.ts (2 hunks)
  • indexer/services/vulcan/src/handlers/order-place-handler.ts (2 hunks)
  • indexer/services/vulcan/src/handlers/order-remove-handler.ts (10 hunks)
  • indexer/services/vulcan/src/index.ts (2 hunks)
Files skipped from review due to trivial changes (1)
  • indexer/packages/postgres/src/index.ts
Additional Context Used
LanguageTool (2)
indexer/services/comlink/public/websocket-documentation.md (2)

Near line 11: Possible missing comma found.
Context: ...all ### Connect Upon connecting to v4 Websockets you will receive an initial connection ...
Rule ID: AI_HYDRA_LEO_MISSING_COMMA


Near line 31: Possible missing comma found.
Context: ...e Utilize the same message to subscribe but replace the type with unsubscribe. Fo...
Rule ID: AI_HYDRA_LEO_MISSING_COMMA

Markdownlint (43)
indexer/services/comlink/public/websocket-documentation.md (43)

100: Column: 1
Hard tabs


101: Column: 1
Hard tabs


102: Column: 1
Hard tabs


103: Column: 1
Hard tabs


104: Column: 1
Hard tabs


105: Column: 1
Hard tabs


106: Column: 1
Hard tabs


107: Column: 1
Hard tabs


111: Column: 1
Hard tabs


113: Column: 1
Hard tabs


115: Column: 1
Hard tabs


117: Column: 1
Hard tabs


119: Column: 1
Hard tabs


446: Column: 5
Hard tabs


447: Column: 5
Hard tabs


468: Column: 5
Hard tabs


469: Column: 5
Hard tabs


478: Column: 1
Hard tabs


479: Column: 1
Hard tabs


480: Column: 1
Hard tabs


481: Column: 1
Hard tabs


482: Column: 1
Hard tabs


696: Column: 5
Hard tabs


705: Column: 1
Hard tabs


706: Column: 1
Hard tabs


707: Column: 1
Hard tabs


708: Column: 1
Hard tabs


709: Column: 1
Hard tabs


710: Column: 1
Hard tabs


718: Column: 1
Hard tabs


859: Column: 1
Hard tabs


860: Column: 1
Hard tabs


861: Column: 1
Hard tabs


862: Column: 1
Hard tabs


93: Expected: 1; Actual: 2
Multiple consecutive blank lines


372: Expected: 1; Actual: 2
Multiple consecutive blank lines


473: Expected: 1; Actual: 2
Multiple consecutive blank lines


515: Expected: 1; Actual: 2
Multiple consecutive blank lines


700: Expected: 1; Actual: 2
Multiple consecutive blank lines


777: Expected: 1; Actual: 2
Multiple consecutive blank lines


854: Expected: 1; Actual: 2
Multiple consecutive blank lines


62: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines


63: null
Fenced code blocks should be surrounded by blank lines

Biome (134)
indexer/packages/kafka/src/websocket-helper.ts (20)

52-52: Forbidden non-null assertion.


65-65: Forbidden non-null assertion.


65-65: Forbidden non-null assertion.


67-67: Forbidden non-null assertion.


67-67: Forbidden non-null assertion.


67-67: Forbidden non-null assertion.


69-69: Forbidden non-null assertion.


69-69: Forbidden non-null assertion.


71-71: Forbidden non-null assertion.


76-76: Forbidden non-null assertion.


80-80: Forbidden non-null assertion.


81-81: Forbidden non-null assertion.


81-81: Forbidden non-null assertion.


82-82: Forbidden non-null assertion.


84-84: Forbidden non-null assertion.


89-89: Forbidden non-null assertion.


90-90: Forbidden non-null assertion.


115-115: Forbidden non-null assertion.


115-115: Forbidden non-null assertion.


115-115: Forbidden non-null assertion.

indexer/packages/postgres/src/loops/block-height-refresher.ts (2)

12-12: This type annotation is trivially inferred from its initialization.


8-9: All these imports are only used as types.

indexer/packages/postgres/src/types/websocket-message-types.ts (9)

1-1: All these imports are only used as types.


1-2: All these imports are only used as types.


2-7: Some named imports are only used as types.


7-8: All these imports are only used as types.


8-9: All these imports are only used as types.


9-10: All these imports are only used as types.


10-11: All these imports are only used as types.


11-12: All these imports are only used as types.


12-13: All these imports are only used as types.

indexer/services/socks/src/lib/subscription.ts (12)

125-126: Template literals are preferred over string concatenation.


162-162: This type annotation is trivially inferred from its initialization.


255-257: Prefer for...of instead of forEach.


256-256: Forbidden non-null assertion.


313-313: This type annotation is trivially inferred from its initialization.


317-317: Change to an optional chain.


327-327: Change to an optional chain.


357-377: Prefer for...of instead of forEach.


510-510: Forbidden non-null assertion.


5-15: Some named imports are only used as types.


15-16: All these imports are only used as types.


20-27: Some named imports are only used as types.

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

204-208: Prefer for...of instead of forEach.


582-582: Forbidden non-null assertion.


723-723: Forbidden non-null assertion.


809-809: Forbidden non-null assertion.


926-926: Forbidden non-null assertion.


926-926: Forbidden non-null assertion.


1018-1018: Unexpected any. Specify a different type.


1146-1146: This type annotation is trivially inferred from its initialization.


1175-1175: This type annotation is trivially inferred from its initialization.


1187-1187: This type annotation is trivially inferred from its initialization.


1200-1200: Forbidden non-null assertion.


1202-1202: Forbidden non-null assertion.


1202-1202: Forbidden non-null assertion.


1207-1207: Forbidden non-null assertion.


1207-1207: Forbidden non-null assertion.


1210-1210: Forbidden non-null assertion.


1210-1210: Forbidden non-null assertion.


1210-1210: Forbidden non-null assertion.


1212-1212: Forbidden non-null assertion.


1212-1212: Forbidden non-null assertion.

indexer/services/vulcan/__tests__/handlers/order-remove-handler.test.ts (20)

192-192: Unexpected any. Specify a different type.


490-490: Forbidden non-null assertion.


494-494: Forbidden non-null assertion.


497-497: Forbidden non-null assertion.


499-499: Forbidden non-null assertion.


500-500: Forbidden non-null assertion.


500-500: Forbidden non-null assertion.


501-501: Forbidden non-null assertion.


503-503: Forbidden non-null assertion.


509-509: Forbidden non-null assertion.


632-632: Forbidden non-null assertion.


636-636: Forbidden non-null assertion.


639-639: Forbidden non-null assertion.


641-641: Forbidden non-null assertion.


642-642: Forbidden non-null assertion.


642-642: Forbidden non-null assertion.


643-643: Forbidden non-null assertion.


645-645: Forbidden non-null assertion.


651-651: Forbidden non-null assertion.


774-774: Forbidden non-null assertion.

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

512-512: Unexpected any. Specify a different type.


547-547: Forbidden non-null assertion.


583-583: Forbidden non-null assertion.


594-594: Forbidden non-null assertion.


598-598: Forbidden non-null assertion.


601-601: Forbidden non-null assertion.


618-618: Unexpected any. Specify a different type.


1-8: Some named imports are only used as types.


15-24: Some named imports are only used as types.


31-39: Some named imports are only used as types.


44-45: All these imports are only used as types.

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

63-63: Forbidden non-null assertion.


64-64: Forbidden non-null assertion.


65-65: Forbidden non-null assertion.


69-69: Forbidden non-null assertion.


91-91: Forbidden non-null assertion.


113-113: Forbidden non-null assertion.


136-136: Forbidden non-null assertion.


136-136: Forbidden non-null assertion.


137-137: Forbidden non-null assertion.


137-137: Forbidden non-null assertion.


166-166: Forbidden non-null assertion.


195-195: Forbidden non-null assertion.


195-195: Forbidden non-null assertion.


228-228: Forbidden non-null assertion.


228-228: Forbidden non-null assertion.


229-229: Forbidden non-null assertion.


246-246: Forbidden non-null assertion.


246-246: Forbidden non-null assertion.


248-248: Forbidden non-null assertion.


289-289: Forbidden non-null assertion.

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

82-82: Forbidden non-null assertion.


104-104: Forbidden non-null assertion.


117-117: Forbidden non-null assertion.


119-119: Forbidden non-null assertion.


147-150: The function should not return a value because its return type is void.


154-157: The function should not return a value because its return type is void.


161-164: The function should not return a value because its return type is void.


169-172: The function should not return a value because its return type is void.


176-179: The function should not return a value because its return type is void.


204-204: Forbidden non-null assertion.


250-250: Forbidden non-null assertion.


250-250: Forbidden non-null assertion.


272-272: Forbidden non-null assertion.


274-274: Forbidden non-null assertion.


291-291: Forbidden non-null assertion.


315-315: Forbidden non-null assertion.


328-328: Forbidden non-null assertion.


350-350: Forbidden non-null assertion.


357-357: Forbidden non-null assertion.


357-357: Forbidden non-null assertion.

Path-based Instructions (14)
indexer/packages/postgres/__tests__/loops/block-height-refresher.test.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/packages/postgres/src/config.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/packages/postgres/src/loops/block-height-refresher.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/vulcan/src/index.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/socks/src/index.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/packages/kafka/src/websocket-helper.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/packages/postgres/src/types/websocket-message-types.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

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

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/comlink/public/websocket-documentation.md (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/socks/src/lib/subscription.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/vulcan/__tests__/handlers/order-update-handler.test.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

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

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

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

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/vulcan/__tests__/handlers/order-remove-handler.test.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

Additional comments not posted (30)
indexer/packages/postgres/__tests__/loops/block-height-refresher.test.ts (3)

9-11: Ensure updateBlockHeight is called with the correct initial conditions.

Verification successful

The initial conditions for calling updateBlockHeight appear to be correctly set up in the beforeAll block, as it follows the migrate and seedData calls. Additionally, updateBlockHeight is called within a test case to verify its functionality.

  • beforeAll block ensures the database is migrated and seeded before calling updateBlockHeight.
  • The test case verifies that updateBlockHeight correctly updates the block height.
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that `updateBlockHeight` is called with the correct initial conditions in the `beforeAll` block.

# Test: Search for the function usage. Expect: Proper initial conditions.
rg --type typescript $'await updateBlockHeight()'

Length of output: 85



Script:

#!/bin/bash
# Description: Verify that `updateBlockHeight` is called with the correct initial conditions in the `beforeAll` block.

# Test: Search for the function usage. Expect: Proper initial conditions.
rg --type ts $'await updateBlockHeight()'

Length of output: 252



Script:

#!/bin/bash
# Description: Extract surrounding lines of `await updateBlockHeight()` to verify initial conditions.

# Extract 5 lines before and after each instance of `await updateBlockHeight()`.
rg --type ts --context 5 'await updateBlockHeight()' indexer/packages/postgres/__tests__/loops/block-height-refresher.test.ts

Length of output: 621


21-22: Check the correctness of the getLatestBlockHeight function.

Verification successful

The getLatestBlockHeight function appears to be correctly implemented. It checks if latestBlockHeight is set, logs an error and throws an exception if it is not, and returns the latestBlockHeight otherwise. This ensures that the function either provides a valid block height or fails with a clear error message.

  • The function is defined in indexer/packages/postgres/src/loops/block-height-refresher.ts.
  • It is used in multiple files, including tests, which indicates it is well-integrated into the codebase.

The implementation is as follows:

export function getLatestBlockHeight(): string {
  if (!latestBlockHeight) {
    const message: string = 'Unable to find latest block height';
    logger.error({
      at: 'block-height-refresher#getLatestBlockHeight',
      message,
    });
    throw new Error(message);
  }
  return latestBlockHeight;
}

This implementation ensures that the function behaves as expected, either returning the latest block height or throwing an error if it cannot find it.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the correctness of the `getLatestBlockHeight` function.

# Test: Search for the function usage. Expect: Correct implementation.
rg --type typescript $'getLatestBlockHeight'

Length of output: 78



Script:

#!/bin/bash
# Description: Verify the correctness of the `getLatestBlockHeight` function.

# Test: Search for the function definition and usage in TypeScript files.
rg 'function getLatestBlockHeight' --type ts
rg 'getLatestBlockHeight' --type ts

Length of output: 3248



Script:

#!/bin/bash
# Description: Extract the implementation of the `getLatestBlockHeight` function.

# Extract the function implementation from the specified file.
rg --multiline --multiline-dotall --type ts 'export function getLatestBlockHeight\(\): string \{[^}]*\}' indexer/packages/postgres/src/loops/block-height-refresher.ts

Length of output: 410



Script:

#!/bin/bash
# Description: Extract the complete implementation of the `getLatestBlockHeight` function.

# Extract the complete function implementation from the specified file.
rg --multiline --multiline-dotall --type ts 'export function getLatestBlockHeight\(\): string \{[^}]*\}' indexer/packages/postgres/src/loops/block-height-refresher.ts -A 10

Length of output: 621


32-36: Ensure environment-specific logic is robust and well-tested.

Verification successful

The clear function in block-height-refresher.ts contains the necessary environment-specific logic to throw an error when called in a non-test environment. The test in block-height-refresher.test.ts correctly verifies this behavior.

  • The clear function implementation:

    export function clear(): void {
      if (config.NODE_ENV !== NodeEnv.TEST) {
        throw new Error('clear cannot be used in non-test env');
      }
      latestBlockHeight = '';
    }
  • The test case:

    it('throws an error when clear is called in non-test environment', () => {
      const originalEnv = config.NODE_ENV;
      config.NODE_ENV = 'production';
      expect(() => clear()).toThrowError('clear cannot be used in non-test env');
      config.NODE_ENV = originalEnv;
    });

Both the implementation and the test case are robust and correctly handle the environment-specific logic.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the environment-specific logic in the `clear` function.

# Test: Search for the function usage. Expect: Robust implementation.
rg --type typescript $'clear()'

Length of output: 67



Script:

#!/bin/bash
# Description: Verify the environment-specific logic in the `clear` function.

# Correct the file type and search for the `clear` function definition and its usage.
rg --type ts 'function clear' 
rg --type ts 'clear()'

Length of output: 38805



Script:

#!/bin/bash
# Description: Extract and review the implementation of the `clear` function in `block-height-refresher.ts`.

# Extract the implementation of the `clear` function.
ast-grep --lang typescript --pattern $'export function clear() {
  $$$
}' indexer/packages/postgres/src/loops/block-height-refresher.ts

Length of output: 136



Script:

#!/bin/bash
# Description: Extract and review the implementation of the `clear` function in `block-height-refresher.ts`.

# Adjust the pattern to match the `clear` function definition.
ast-grep --lang typescript --pattern 'export function clear() {
  $$$
}' indexer/packages/postgres/src/loops/block-height-refresher.ts

Length of output: 136



Script:

#!/bin/bash
# Description: List all function definitions in `block-height-refresher.ts` to ensure we capture the `clear` function.

# List all function definitions in the file.
ast-grep --lang typescript --pattern 'function $_($$$) {
  $$$
}' indexer/packages/postgres/src/loops/block-height-refresher.ts

Length of output: 129



Script:

#!/bin/bash
# Description: List all function definitions in `block-height-refresher.ts` to ensure we capture the `clear` function.

# List all function definitions in the file.
rg 'function clear' indexer/packages/postgres/src/loops/block-height-refresher.ts

Length of output: 115



Script:

#!/bin/bash
# Description: Extract the full implementation of the `clear` function in `block-height-refresher.ts`.

# Extract the full implementation of the `clear` function.
sed -n '/export function clear()/,/^}/p' indexer/packages/postgres/src/loops/block-height-refresher.ts

Length of output: 272

indexer/packages/postgres/src/config.ts (1)

38-38: Ensure the new configuration setting is correctly integrated and documented.

Verification successful

The new configuration setting BLOCK_HEIGHT_REFRESHER_INTERVAL_MS is correctly integrated into the startUpdateLoop function within block-height-refresher.ts. This confirms that the setting is being used as intended.

  • indexer/packages/postgres/src/config.ts
  • indexer/packages/postgres/src/loops/block-height-refresher.ts
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the integration and documentation of the new configuration setting.

# Test: Search for the usage of the new configuration setting. Expect: Correct integration and documentation.
rg --type typescript $'BLOCK_HEIGHT_REFRESHER_INTERVAL_MS'

Length of output: 92



Script:

#!/bin/bash
# Description: Verify the integration and documentation of the new configuration setting.

# Test: Search for the usage of the new configuration setting. Expect: Correct integration and documentation.
rg --type ts 'BLOCK_HEIGHT_REFRESHER_INTERVAL_MS'

Length of output: 282



Script:

#!/bin/bash
# Description: Extract the relevant lines from `block-height-refresher.ts` to verify the usage of the new configuration setting.

# Extract lines around the usage of the new configuration setting.
rg --type ts -A 5 -B 5 'config.BLOCK_HEIGHT_REFRESHER_INTERVAL_MS' indexer/packages/postgres/src/loops/block-height-refresher.ts

Length of output: 389

indexer/packages/postgres/src/loops/block-height-refresher.ts (3)

53-59: Confirm the environment-specific logic in the clear function is secure and tested.

Verification successful

The environment-specific logic in the clear function is secure and tested. The function is used in multiple test files, including block-height-refresher.test.ts, which contains a test to ensure it throws an error when called in a non-test environment.

  • indexer/packages/postgres/__tests__/loops/block-height-refresher.test.ts: Contains a test that verifies the clear function throws an error in non-test environments.
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the security and testing of the environment-specific logic in the `clear` function.

# Test: Search for the function usage. Expect: Secure and tested implementation.
rg --type typescript $'clear()'

Length of output: 67



Script:

#!/bin/bash
# Description: Verify the security and testing of the environment-specific logic in the `clear` function.

# Test: Search for the function usage. Expect: Secure and tested implementation.
rg --type ts 'clear()'

Length of output: 37881


28-36: Check the performance and error handling of the updateBlockHeight function.

Verification successful

The updateBlockHeight function is used within a Promise.all block, which is a performance optimization for concurrent execution. Additionally, the function is wrapped in wrapBackgroundTask, which includes error handling by catching and logging errors.

  • Performance: The use of Promise.all ensures that updateBlockHeight runs concurrently with other tasks, optimizing performance.
  • Error Handling: The wrapBackgroundTask function catches any errors from the promise and logs them, ensuring robust error handling.

Locations:

  • indexer/services/socks/src/index.ts
  • indexer/services/vulcan/src/index.ts
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the performance and error handling of the `updateBlockHeight` function.

# Test: Search for the function usage. Expect: Optimal performance and robust error handling.
rg --type typescript $'updateBlockHeight'

Length of output: 75



Script:

#!/bin/bash
# Description: Verify the performance and error handling of the `updateBlockHeight` function.

# Test: Search for the function usage. Expect: Optimal performance and robust error handling.
rg --type ts 'updateBlockHeight'

Length of output: 1381



Script:

#!/bin/bash
# Description: Extract the context of `updateBlockHeight` usage for performance and error handling analysis.

# Extract lines around the usage of `updateBlockHeight` in the identified files.
rg --type ts -A 5 -B 5 'updateBlockHeight' indexer/services/vulcan/src/index.ts indexer/services/socks/src/index.ts

Length of output: 1669



Script:

#!/bin/bash
# Description: Extract the implementation of `wrapBackgroundTask` to verify error handling.

# Search for the implementation of `wrapBackgroundTask` in the codebase.
rg --type ts 'function wrapBackgroundTask' -A 10 -B 2

Length of output: 928


17-22: Verify the correct configuration and error handling in the start function.

indexer/services/vulcan/src/index.ts (1)

22-26: Verify the concurrent update of block height and perpetual markets at service start.

Verification successful

The concurrent update logic for blockHeightRefresher and perpetualMarketRefresher is correctly implemented in index.ts. The Promise.all ensures that both updateBlockHeight and updatePerpetualMarkets are executed concurrently.

  • indexer/services/vulcan/src/index.ts:
    • Lines 22-26: Concurrent update logic using Promise.all.
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the concurrent update of block height and perpetual markets at service start.

# Test: Search for the concurrent update logic. Expect: Correct implementation.
rg --type typescript $'updateBlockHeight'

Length of output: 75



Script:

#!/bin/bash
# Description: Verify the concurrent update of block height and perpetual markets at service start.

# Test: Search for the concurrent update logic. Expect: Correct implementation.
rg --type ts 'updateBlockHeight'

Length of output: 1381



Script:

#!/bin/bash
# Description: Extract relevant lines from index.ts to verify concurrent update logic.

# Extract lines around the usage of updateBlockHeight in index.ts
rg --type ts -A 5 -B 5 'updateBlockHeight' indexer/services/vulcan/src/index.ts

Length of output: 445

indexer/services/socks/src/index.ts (1)

43-47: Ensure the correct initialization and concurrent update of block height and perpetual markets.

Verification successful

The search results indicate that the functions updateBlockHeight and updatePerpetualMarkets are indeed being used in the indexer/services/socks/src/index.ts file, as well as in other parts of the codebase. This confirms that the initialization and concurrent update of block height and perpetual markets are correctly implemented.

  • updateBlockHeight is found in indexer/services/socks/src/index.ts and other files.
  • updatePerpetualMarkets is also found in indexer/services/socks/src/index.ts and other files.

This suggests that the functions are properly integrated and utilized within the socks service.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the correct initialization and concurrent update of block height and perpetual markets.

# Test: Search for the initialization logic. Expect: Correct implementation.
rg --type typescript $'updateBlockHeight'

Length of output: 75



Script:

#!/bin/bash
# Description: Verify the correct initialization and concurrent update of block height and perpetual markets.

# Test: Search for the initialization logic. Expect: Correct implementation.
rg --type ts 'updateBlockHeight'

# Test: Search for the initialization logic. Expect: Correct implementation.
rg --type ts 'updatePerpetualMarkets'

Length of output: 10478

indexer/packages/kafka/src/websocket-helper.ts (1)

103-110: Ensure the createSubaccountWebsocketMessage function correctly handles the blockHeight parameter.

Verification successful

The createSubaccountWebsocketMessage function correctly handles the blockHeight parameter by including it in the contents object only if it is defined. This is evident from the following snippet in indexer/packages/kafka/src/websocket-helper.ts:

...(blockHeight && { blockHeight }),

Additionally, the function is used in indexer/services/vulcan/src/handlers/order-place-handler.ts where blockHeight is passed as an argument:

value: createSubaccountWebsocketMessage(
  redisOrder,
  dbOrder,
  perpetualMarket,
  placementStatus,
  blockHeightRefresher.getLatestBlockHeight(),
),

This confirms that the blockHeight parameter is correctly handled in the function.

  • indexer/packages/kafka/src/websocket-helper.ts: Correctly includes blockHeight if defined.
  • indexer/services/vulcan/src/handlers/order-place-handler.ts: Passes blockHeight to the function.
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Ensure the `createSubaccountWebsocketMessage` function correctly handles the `blockHeight` parameter.

# Test: Search for the function usage. Expect: Correct handling of the `blockHeight` parameter.
rg --type typescript $'createSubaccountWebsocketMessage'

Length of output: 90



Script:

#!/bin/bash
# Description: Ensure the `createSubaccountWebsocketMessage` function correctly handles the `blockHeight` parameter.

# Test: Search for the function usage. Expect: Correct handling of the `blockHeight` parameter.
rg --type ts $'createSubaccountWebsocketMessage'

Length of output: 1320



Script:

#!/bin/bash
# Description: Extract relevant code snippets to verify the handling of the `blockHeight` parameter in the `createSubaccountWebsocketMessage` function.

# Extract the function definition and its usage in the relevant files.
rg --type ts --context 10 'createSubaccountWebsocketMessage' indexer/packages/kafka/src/websocket-helper.ts
rg --type ts --context 10 'createSubaccountWebsocketMessage' indexer/services/vulcan/src/handlers/order-place-handler.ts

Length of output: 2608

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

154-154: Ensure blockHeightRefresher is properly initialized and updated.

Verification successful

blockHeightRefresher is properly initialized and updated in the codebase.

  • indexer/services/vulcan/src/index.ts: Contains calls to blockHeightRefresher.updateBlockHeight() and blockHeightRefresher.start().
  • Test files also include calls to blockHeightRefresher.updateBlockHeight(), indicating it is being tested.
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the initialization and update mechanism of `blockHeightRefresher`.

# Test: Search for the initialization and update calls for `blockHeightRefresher`.
rg --type typescript $'blockHeightRefresher.*start|blockHeightRefresher.*updateBlockHeight'

Length of output: 127



Script:

#!/bin/bash
# Description: Verify the initialization and update mechanism of `blockHeightRefresher`.

# Test: Search for the initialization and update calls for `blockHeightRefresher` in TypeScript files.
rg 'blockHeightRefresher.*start|blockHeightRefresher.*updateBlockHeight' --glob '*.ts'

Length of output: 980

indexer/services/comlink/public/websocket-documentation.md (1)

88-88: Documentation accurately reflects the inclusion of blockHeight.

The updated documentation correctly includes the blockHeight field in the websocket message examples, aligning with the changes made in the codebase.

indexer/services/socks/src/lib/subscription.ts (2)

Line range hint 538-569: The addition of blockHeight to the initial response is implemented correctly.


9-9: Ensure blockHeightRefresher is utilized appropriately in the subscription logic.

Verification successful

The blockHeightRefresher is utilized in the subscription.ts file, specifically in the line blockHeightRefresher.getLatestBlockHeight(). This indicates that it is being used appropriately in the subscription logic.

  • blockHeightRefresher is imported and used in the method getLatestBlockHeight().
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that `blockHeightRefresher` is used correctly in the subscription logic.

# Test: Search for the usage of `blockHeightRefresher`. Expect: Proper usage in subscription logic.
rg --type typescript $'blockHeightRefresher' indexer/services/socks/src/lib/subscription.ts

Length of output: 125



Script:

#!/bin/bash
# Description: Verify that `blockHeightRefresher` is used correctly in the subscription logic.

# Test: Search for the usage of `blockHeightRefresher`. Expect: Proper usage in subscription logic.
rg --type ts 'blockHeightRefresher' indexer/services/socks/src/lib/subscription.ts

Length of output: 159

indexer/services/vulcan/__tests__/handlers/order-update-handler.test.ts (2)

64-67: The addition of blockHeightRefresher.updateBlockHeight() in the test setup is implemented correctly.


17-17: Ensure blockHeightRefresher is utilized appropriately in the test logic.

Verification successful

Ensure blockHeightRefresher is utilized appropriately in the test logic.

  • blockHeightRefresher is used in the test file, specifically in the line blockHeightRefresher.updateBlockHeight().
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that `blockHeightRefresher` is used correctly in the test logic.

# Test: Search for the usage of `blockHeightRefresher`. Expect: Proper usage in test logic.
rg --type typescript $'blockHeightRefresher' indexer/services/vulcan/__tests__/handlers/order-update-handler.test.ts

Length of output: 150



Script:

#!/bin/bash
# Description: Verify that `blockHeightRefresher` is used correctly in the test logic.

# Test: Search for the usage of `blockHeightRefresher`. Expect: Proper usage in test logic.
rg --type ts 'blockHeightRefresher' indexer/services/vulcan/__tests__/handlers/order-update-handler.test.ts

Length of output: 181

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

4-4: Ensure proper handling of blockHeightRefresher import.

This import is crucial for the new functionality to fetch the latest block height. Ensure that the blockHeightRefresher is correctly implemented and tested in its respective module.


237-237: Integrate blockHeightRefresher.getLatestBlockHeight() to fetch block height for websocket messages.

This change correctly fetches the latest block height to be included in the websocket message, ensuring that the message data is up-to-date with the latest blockchain state.


299-299: Use blockHeightRefresher.getLatestBlockHeight() in handling order removals.

Properly fetching the latest block height for the websocket message when an order is not found in Redis. This ensures that even in cases of missing orders, the message sent is accurate with the current block height.


339-339: Include latest block height in websocket messages for removed orders.

This integration ensures that the websocket message for order removals includes the most current block height, which is critical for clients to have consistent and up-to-date information.


577-577: Ensure blockHeight is included in websocket messages when orders are canceled.

The inclusion of blockHeight in the websocket message when an order is canceled ensures that the client-side can accurately track changes against the blockchain state.


624-624: Correctly append blockHeight to the websocket message contents.

Appending the blockHeight to the websocket message contents ensures that all relevant data regarding the order and its context are transmitted accurately.


641-641: Include block height in websocket messages for order removal results.

This ensures that the websocket message contains the most recent block height, providing a complete and accurate context for the order removal.


686-686: Ensure blockHeight is appended to the websocket message for order removals.

Appending the blockHeight to the websocket message ensures that the message is comprehensive and reflects the current state of the blockchain.


702-702: Include block height in websocket messages from Postgres orders.

This change ensures that the websocket message includes the latest block height when sending updates from Postgres orders, maintaining consistency and accuracy in the data provided to clients.


734-734: Correctly append blockHeight to the websocket message for comprehensive data transmission.

This final inclusion of blockHeight in the websocket message ensures that all messages sent are up-to-date with the latest blockchain information, which is crucial for maintaining data integrity and relevance.

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

19-20: Ensure proper initialization and usage of blockHeightRefresher.

The addition of blockHeightRefresher in the imports section is consistent with the PR's objective to integrate block height into the system's operations. This change is crucial for fetching the latest block height in subsequent operations within this test file.


81-83: Correctly integrate block height updates in the test setup.

The addition of blockHeightRefresher.updateBlockHeight() in the beforeAll setup ensures that the block height is updated before running the tests. This is essential for tests that depend on the current block height, aligning with the PR's objectives.


1236-1236: Ensure the latest block height is fetched correctly.

The integration of blockHeightRefresher.getLatestBlockHeight() to fetch the latest block height for the websocket message is correctly implemented. This ensures that the websocket messages contain the most recent block height information, which is crucial for clients tracking block-specific data.

indexer/services/vulcan/__tests__/handlers/order-remove-handler.test.ts (1)

259-259: Ensure that blockHeightRefresher.getLatestBlockHeight() is called correctly and handle potential errors or undefined values.

Also applies to: 317-317, 513-513, 655-655, 796-796, 939-939, 1250-1250, 1368-1368, 1494-1494, 1636-1636, 1774-1774

Comment on lines +240 to +245
await dbHelpers.clearData();
await testMocks.seedData();
await perpetualMarketRefresher.updatePerpetualMarkets();
await Promise.all([
perpetualMarketRefresher.updatePerpetualMarkets(),
blockHeightRefresher.updateBlockHeight(),
]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor to use for...of for better readability and performance.

- await Promise.all([
-   perpetualMarketRefresher.updatePerpetualMarkets(),
-   blockHeightRefresher.updateBlockHeight(),
- ]);
+ for (const refresher of [perpetualMarketRefresher, blockHeightRefresher]) {
+   await refresher.update();
+ }

This change enhances readability and potentially improves error handling by processing updates sequentially rather than in parallel.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
await dbHelpers.clearData();
await testMocks.seedData();
await perpetualMarketRefresher.updatePerpetualMarkets();
await Promise.all([
perpetualMarketRefresher.updatePerpetualMarkets(),
blockHeightRefresher.updateBlockHeight(),
]);
await dbHelpers.clearData();
await testMocks.seedData();
for (const refresher of [perpetualMarketRefresher, blockHeightRefresher]) {
await refresher.update();
}

@@ -30,7 +30,7 @@ import {
testConstants,
testMocks,
apiTranslations,
TimeInForce,
TimeInForce, blockHeightRefresher,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specify explicit types instead of using any.

- orderRemoveJson: any
+ orderRemoveJson: OrderRemoveV1

Committable suggestion was skipped due low confidence.

Comment on lines +92 to +95
await Promise.all([
perpetualMarketRefresher.updatePerpetualMarkets(),
blockHeightRefresher.updateBlockHeight(),
]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

The search results have provided the implementation of blockHeightRefresher.updateBlockHeight() in indexer/packages/postgres/src/loops/block-height-refresher.ts. Here is the relevant snippet:

/**
 * Updates in-memory latest block height.
 */
export async function updateBlockHeight(options?: Options): Promise<void> {
  const startTime: number = Date.now();
  const latestBlock: BlockFromDatabase = await BlockTable.getLatest(
    options || { readReplica: true },
  );

  latestBlockHeight = latestBlock.blockHeight;
  stats.timing(`${config.SERVICE_NAME}.loops.update_block_height`, Date.now() - startTime);
}

This function does not include any error handling mechanisms. To ensure robustness, it should handle potential errors that might occur during the database query or other operations.

  • Location to address:
    • indexer/packages/postgres/src/loops/block-height-refresher.ts: Add error handling in the updateBlockHeight function.
Analysis chain

Ensure that blockHeightRefresher.updateBlockHeight() is properly tested for failure scenarios.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that blockHeightRefresher.updateBlockHeight() handles errors correctly.

# Test: Search for error handling in blockHeightRefresher.updateBlockHeight(). Expect: Proper error handling.
rg --type typescript $'blockHeightRefresher.updateBlockHeight' --context 5

Length of output: 108



Script:

#!/bin/bash
# Description: Verify that blockHeightRefresher.updateBlockHeight() handles errors correctly.

# Test: Search for error handling in blockHeightRefresher.updateBlockHeight(). Expect: Proper error handling.
rg --type ts 'blockHeightRefresher.updateBlockHeight' --context 5

Length of output: 6164



Script:

#!/bin/bash
# Description: Locate the implementation of blockHeightRefresher.updateBlockHeight() and check for error handling.

# Find the definition of updateBlockHeight in the codebase.
rg --type ts 'updateBlockHeight' -g '*.ts' --context 10

Length of output: 17216

import { BlockFromDatabase, Options } from '../types';
import { startUpdateLoop } from './loopHelper';

let latestBlockHeight: string = '';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unnecessary type annotation for latestBlockHeight.

- let latestBlockHeight: string = '';
+ let latestBlockHeight = '';

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
let latestBlockHeight: string = '';
let latestBlockHeight = '';

Comment on lines +41 to +50
export function getLatestBlockHeight(): string {
if (!latestBlockHeight) {
const message: string = 'Unable to find latest block height';
logger.error({
at: 'block-height-refresher#getLatestBlockHeight',
message,
});
throw new Error(message);
}
return latestBlockHeight;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

The extracted context from the handler files shows that getLatestBlockHeight is used directly without any apparent error handling mechanisms around it. This indicates that if getLatestBlockHeight throws an error, it might not be caught and handled properly in these contexts.

  • Files to review for error handling:
    • indexer/services/vulcan/src/handlers/order-remove-handler.ts
    • indexer/services/vulcan/src/handlers/order-place-handler.ts

Ensure that calls to getLatestBlockHeight are wrapped in try-catch blocks or have appropriate error handling mechanisms to prevent unhandled exceptions.

Analysis chain

Ensure robust error handling in the getLatestBlockHeight function.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify robust error handling in the `getLatestBlockHeight` function.

# Test: Search for the function usage. Expect: Robust error handling.
rg --type typescript $'getLatestBlockHeight'

Length of output: 78



Script:

#!/bin/bash
# Description: Verify robust error handling in the `getLatestBlockHeight` function.

# Test: Search for the function usage. Expect: Robust error handling.
rg --type ts 'getLatestBlockHeight'

Length of output: 3090



Script:

#!/bin/bash
# Description: Extract and inspect the usage of `getLatestBlockHeight` in handler files to verify robust error handling.

# Extract lines around the usage of `getLatestBlockHeight` in handler files.
rg --type ts --context 5 'getLatestBlockHeight' indexer/services/vulcan/src/handlers/order-remove-handler.ts indexer/services/vulcan/src/handlers/order-place-handler.ts

Length of output: 4264

@dydxwill dydxwill changed the title Add blockHeight to subaccount websocket message [CT-839] Add blockHeight to subaccount websocket message May 24, 2024
Copy link

linear bot commented May 24, 2024

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range and nitpick comments (9)
indexer/services/ender/src/helpers/kafka-helper.ts (2)

Line range hint 53-67: Addition of blockHeight to addPositionsToContents function aligns with PR objectives.

However, consider handling potential null values gracefully instead of using non-null assertions.

-        address: transfer.senderWalletAddress ?? senderSubaccountId!.owner,
+        address: transfer.senderWalletAddress ?? senderSubaccountId?.owner,
-        subaccountNumber: transfer.senderWalletAddress ? undefined : senderSubaccountId!.number,
+        subaccountNumber: transfer.senderWalletAddress ? undefined : senderSubaccountId?.number,
-        address: transfer.recipientWalletAddress ?? recipientSubaccountId!.owner,
+        address: transfer.recipientWalletAddress ?? recipientSubaccountId?.owner,
-        subaccountNumber: transfer.recipientWalletAddress ? undefined : recipientSubaccountId!.number,
+        subaccountNumber: transfer.recipientWalletAddress ? undefined : recipientSubaccountId?.number,

Line range hint 236-260: Addition of optional blockHeight to generateTransferContents function is correctly implemented.

However, consider removing non-null assertions to prevent potential runtime errors.

-        address: transfer.senderWalletAddress ?? senderSubaccountId!.owner,
+        address: transfer.senderWalletAddress ?? senderSubaccountId?.owner,
-        subaccountNumber: transfer.senderWalletAddress ? undefined : senderSubaccountId!.number,
+        subaccountNumber: transfer.senderWalletAddress ? undefined : senderSubaccountId?.number,
-        address: transfer.recipientWalletAddress ?? recipientSubaccountId!.owner,
+        address: transfer.recipientWalletAddress ?? recipientSubaccountId?.owner,
-        subaccountNumber: transfer.recipientWalletAddress ? undefined : recipientSubaccountId!.number,
+        subaccountNumber: transfer.recipientWalletAddress ? undefined : recipientSubaccountId?.number,
indexer/services/ender/__tests__/helpers/kafka-helper.test.ts (4)

Line range hint 107-137: Refactor to avoid non-null assertions.

- expect(contents.perpetualPositions!.length).toEqual(1);
- expect(contents.assetPositions!.length).toEqual(1);
+ expect(contents.perpetualPositions?.length).toEqual(1);
+ expect(contents.assetPositions?.length).toEqual(1);

Line range hint 153-168: Refactor to avoid non-null assertions.

- expect(contents.assetPositions!.length).toEqual(1);
+ expect(contents.assetPositions?.length).toEqual(1);

Line range hint 184-205: Refactor to avoid non-null assertions.

- expect(contents.perpetualPositions!.length).toEqual(1);
+ expect(contents.perpetualPositions?.length).toEqual(1);

Line range hint 235-291: Refactor to avoid non-null assertions.

- expect(contents.perpetualPositions!.length).toEqual(2);
- expect(contents.assetPositions!.length).toEqual(2);
+ expect(contents.perpetualPositions?.length).toEqual(2);
+ expect(contents.assetPositions?.length).toEqual(2);
indexer/services/ender/__tests__/handlers/transfer-handler.test.ts (1)

Line range hint 654-665: Refactor to avoid non-null assertions.

- senderContents = generateTransferContents(
-   transfer,
-   asset,
-   event.sender!.subaccountId!,
-   event.sender!.subaccountId!,
-   event.recipient!.subaccountId,
-   blockHeight,
- );
- recipientContents = generateTransferContents(
-   transfer,
-   asset,
-   event.recipient!.subaccountId!,
-   event.sender!.subaccountId,
-   event.recipient!.subaccountId!,
-   blockHeight,
- );
+ senderContents = generateTransferContents(
+   transfer,
+   asset,
+   event.sender?.subaccountId,
+   event.sender?.subaccountId,
+   event.recipient?.subaccountId,
+   blockHeight,
+ );
+ recipientContents = generateTransferContents(
+   transfer,
+   asset,
+   event.recipient?.subaccountId,
+   event.sender?.subaccountId,
+   event.recipient?.subaccountId,
+   blockHeight,
+ );
indexer/services/ender/__tests__/handlers/subaccount-update-handler.test.ts (2)

Line range hint 137-137: Avoid using non-null assertions as they can lead to runtime errors if assumptions about non-nullability prove incorrect.

Consider adding appropriate null checks or handling potential null values gracefully to ensure robustness.

Also applies to: 138-138, 232-232, 234-234, 249-249, 303-303, 312-312, 351-351, 387-387, 393-393, 401-401, 440-440, 449-449, 482-482, 517-517


Line range hint 196-196: Replace any type with more specific types to improve type safety and maintainability.

Using any bypasses TypeScript's type checking, which can lead to bugs that are hard to detect. Define a more specific type or interface for these cases.

Also applies to: 274-274, 331-331, 411-411, 462-462

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 85d634b and 17744c4.
Files selected for processing (15)
  • indexer/services/comlink/public/websocket-documentation.md (3 hunks)
  • indexer/services/ender/tests/handlers/subaccount-update-handler.test.ts (1 hunks)
  • indexer/services/ender/tests/handlers/transfer-handler.test.ts (2 hunks)
  • indexer/services/ender/tests/helpers/indexer-proto-helpers.ts (3 hunks)
  • indexer/services/ender/tests/helpers/kafka-helper.test.ts (12 hunks)
  • indexer/services/ender/src/handlers/order-fills/abstract-order-fill-handler.ts (1 hunks)
  • indexer/services/ender/src/handlers/stateful-order/conditional-order-placement-handler.ts (1 hunks)
  • indexer/services/ender/src/handlers/stateful-order/stateful-order-placement-handler.ts (2 hunks)
  • indexer/services/ender/src/handlers/subaccount-update-handler.ts (1 hunks)
  • indexer/services/ender/src/handlers/trading-rewards-handler.ts (1 hunks)
  • indexer/services/ender/src/handlers/transfer-handler.ts (2 hunks)
  • indexer/services/ender/src/helpers/kafka-helper.ts (4 hunks)
  • indexer/services/socks/tests/lib/message-forwarder.test.ts (2 hunks)
  • indexer/services/socks/tests/lib/subscriptions.test.ts (2 hunks)
  • indexer/services/socks/src/lib/subscription.ts (5 hunks)
Additional Context Used
LanguageTool (2)
indexer/services/comlink/public/websocket-documentation.md (2)

Near line 11: Possible missing comma found.
Context: ...all ### Connect Upon connecting to v4 Websockets you will receive an initial connection ...
Rule ID: AI_HYDRA_LEO_MISSING_COMMA


Near line 31: Possible missing comma found.
Context: ...e Utilize the same message to subscribe but replace the type with unsubscribe. Fo...
Rule ID: AI_HYDRA_LEO_MISSING_COMMA

Markdownlint (44)
indexer/services/comlink/public/websocket-documentation.md (44)

100: Column: 1
Hard tabs


101: Column: 1
Hard tabs


102: Column: 1
Hard tabs


103: Column: 1
Hard tabs


104: Column: 1
Hard tabs


105: Column: 1
Hard tabs


106: Column: 1
Hard tabs


107: Column: 1
Hard tabs


111: Column: 1
Hard tabs


113: Column: 1
Hard tabs


115: Column: 1
Hard tabs


117: Column: 1
Hard tabs


119: Column: 1
Hard tabs


121: Column: 1
Hard tabs


446: Column: 5
Hard tabs


447: Column: 5
Hard tabs


468: Column: 5
Hard tabs


469: Column: 5
Hard tabs


478: Column: 1
Hard tabs


479: Column: 1
Hard tabs


480: Column: 1
Hard tabs


481: Column: 1
Hard tabs


482: Column: 1
Hard tabs


696: Column: 5
Hard tabs


705: Column: 1
Hard tabs


706: Column: 1
Hard tabs


707: Column: 1
Hard tabs


708: Column: 1
Hard tabs


709: Column: 1
Hard tabs


710: Column: 1
Hard tabs


718: Column: 1
Hard tabs


859: Column: 1
Hard tabs


860: Column: 1
Hard tabs


861: Column: 1
Hard tabs


862: Column: 1
Hard tabs


93: Expected: 1; Actual: 2
Multiple consecutive blank lines


372: Expected: 1; Actual: 2
Multiple consecutive blank lines


473: Expected: 1; Actual: 2
Multiple consecutive blank lines


515: Expected: 1; Actual: 2
Multiple consecutive blank lines


700: Expected: 1; Actual: 2
Multiple consecutive blank lines


777: Expected: 1; Actual: 2
Multiple consecutive blank lines


854: Expected: 1; Actual: 2
Multiple consecutive blank lines


62: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines


63: null
Fenced code blocks should be surrounded by blank lines

Biome (174)
indexer/services/ender/__tests__/handlers/subaccount-update-handler.test.ts (20)

137-137: Forbidden non-null assertion.


138-138: Forbidden non-null assertion.


196-196: Unexpected any. Specify a different type.


232-232: Forbidden non-null assertion.


234-234: Forbidden non-null assertion.


249-249: Forbidden non-null assertion.


274-274: Unexpected any. Specify a different type.


303-303: Forbidden non-null assertion.


312-312: Forbidden non-null assertion.


331-331: Unexpected any. Specify a different type.


351-351: Forbidden non-null assertion.


387-387: Forbidden non-null assertion.


393-393: Forbidden non-null assertion.


401-401: Forbidden non-null assertion.


411-411: Unexpected any. Specify a different type.


440-440: Forbidden non-null assertion.


449-449: Forbidden non-null assertion.


462-462: Unexpected any. Specify a different type.


482-482: Forbidden non-null assertion.


517-517: Forbidden non-null assertion.

indexer/services/ender/__tests__/handlers/transfer-handler.test.ts (20)

81-81: Forbidden non-null assertion.


81-81: Forbidden non-null assertion.


85-85: Forbidden non-null assertion.


85-85: Forbidden non-null assertion.


89-89: Forbidden non-null assertion.


89-89: Forbidden non-null assertion.


90-90: Forbidden non-null assertion.


90-90: Forbidden non-null assertion.


96-96: Forbidden non-null assertion.


96-96: Forbidden non-null assertion.


97-97: Forbidden non-null assertion.


97-97: Forbidden non-null assertion.


485-485: This type annotation is trivially inferred from its initialization.


517-517: Forbidden non-null assertion.


517-517: Forbidden non-null assertion.


517-517: Forbidden non-null assertion.


518-518: Forbidden non-null assertion.


518-518: Forbidden non-null assertion.


518-518: Forbidden non-null assertion.


525-525: Forbidden non-null assertion.

indexer/services/ender/__tests__/helpers/indexer-proto-helpers.ts (20)

1-1: A Node.js builtin module should be imported with the node: protocol.


91-91: This type annotation is trivially inferred from its initialization.


181-181: Unexpected any. Specify a different type.


196-196: Unexpected any. Specify a different type.


246-246: Unexpected any. Specify a different type.


257-257: Unexpected any. Specify a different type.


301-301: Unexpected any. Specify a different type.


314-314: Unexpected any. Specify a different type.


419-419: Unexpected any. Specify a different type.


661-661: This type annotation is trivially inferred from its initialization.


662-662: This type annotation is trivially inferred from its initialization.


663-663: This type annotation is trivially inferred from its initialization.


681-681: Forbidden non-null assertion.


685-685: Forbidden non-null assertion.


709-709: This type annotation is trivially inferred from its initialization.


710-710: This type annotation is trivially inferred from its initialization.


711-711: This type annotation is trivially inferred from its initialization.


717-717: Forbidden non-null assertion.


722-722: Forbidden non-null assertion.


723-723: Forbidden non-null assertion.

indexer/services/ender/__tests__/helpers/kafka-helper.test.ts (15)

110-110: Forbidden non-null assertion.


111-111: Forbidden non-null assertion.


127-127: Forbidden non-null assertion.


128-128: Forbidden non-null assertion.


158-158: Forbidden non-null assertion.


159-159: Forbidden non-null assertion.


187-187: Forbidden non-null assertion.


188-188: Forbidden non-null assertion.


239-239: Forbidden non-null assertion.


240-240: Forbidden non-null assertion.


255-255: Forbidden non-null assertion.


272-272: Forbidden non-null assertion.


273-273: Forbidden non-null assertion.


282-282: Forbidden non-null assertion.


1-23: Some named imports are only used as types.

indexer/services/ender/src/handlers/order-fills/abstract-order-fill-handler.ts (4)

1-13: Some named imports are only used as types.


14-20: Some named imports are only used as types.


20-21: All these imports are only used as types.


27-31: All these imports are only used as types.

indexer/services/ender/src/handlers/stateful-order/conditional-order-placement-handler.ts (12)

19-19: This type annotation is trivially inferred from its initialization.


23-23: Forbidden non-null assertion.


23-23: Forbidden non-null assertion.


23-23: Forbidden non-null assertion.


35-35: Forbidden non-null assertion.


35-35: Forbidden non-null assertion.


35-35: Forbidden non-null assertion.


35-35: Forbidden non-null assertion.


1-6: Some named imports are only used as types.


6-10: All these imports are only used as types.


10-11: All these imports are only used as types.


13-14: All these imports are only used as types.

indexer/services/ender/src/handlers/stateful-order/stateful-order-placement-handler.ts (20)

30-30: This type annotation is trivially inferred from its initialization.


36-36: Forbidden non-null assertion.


36-36: Forbidden non-null assertion.


36-36: Forbidden non-null assertion.


38-38: Forbidden non-null assertion.


38-38: Forbidden non-null assertion.


38-38: Forbidden non-null assertion.


47-47: Forbidden non-null assertion.


47-47: Forbidden non-null assertion.


47-47: Forbidden non-null assertion.


47-47: Forbidden non-null assertion.


49-49: Forbidden non-null assertion.


49-49: Forbidden non-null assertion.


49-49: Forbidden non-null assertion.


49-49: Forbidden non-null assertion.


64-64: Forbidden non-null assertion.


64-64: Forbidden non-null assertion.


66-66: Forbidden non-null assertion.


66-66: Forbidden non-null assertion.


84-84: Forbidden non-null assertion.

indexer/services/ender/src/handlers/subaccount-update-handler.ts (9)

27-27: This type annotation is trivially inferred from its initialization.


32-32: Forbidden non-null assertion.


35-35: Forbidden non-null assertion.


89-89: Forbidden non-null assertion.


99-99: Forbidden non-null assertion.


1-16: Some named imports are only used as types.


17-18: All these imports are only used as types.


21-22: All these imports are only used as types.


22-23: All these imports are only used as types.

indexer/services/ender/src/handlers/trading-rewards-handler.ts (5)

15-15: This type annotation is trivially inferred from its initialization.


1-6: Some named imports are only used as types.


6-7: All these imports are only used as types.


8-9: All these imports are only used as types.


10-11: All these imports are only used as types.

indexer/services/ender/src/handlers/transfer-handler.ts (19)

16-16: This type annotation is trivially inferred from its initialization.


53-53: Forbidden non-null assertion.


57-57: Forbidden non-null assertion.


57-57: Forbidden non-null assertion.


58-58: Forbidden non-null assertion.


59-59: Forbidden non-null assertion.


66-66: Forbidden non-null assertion.


66-66: Forbidden non-null assertion.


71-71: Forbidden non-null assertion.


75-75: Forbidden non-null assertion.


75-75: Forbidden non-null assertion.


76-76: Forbidden non-null assertion.


77-77: Forbidden non-null assertion.


84-84: Forbidden non-null assertion.


84-84: Forbidden non-null assertion.


1-7: Some named imports are only used as types.


7-8: All these imports are only used as types.


8-9: All these imports are only used as types.


11-12: All these imports are only used as types.

indexer/services/ender/src/helpers/kafka-helper.ts (8)

241-241: Forbidden non-null assertion.


243-243: Forbidden non-null assertion.


246-246: Forbidden non-null assertion.


248-248: Forbidden non-null assertion.


285-285: Forbidden non-null assertion.


286-286: Forbidden non-null assertion.


1-30: Some named imports are only used as types.


30-31: All these imports are only used as types.

indexer/services/socks/__tests__/lib/message-forwarder.test.ts (9)

297-297: Unexpected any. Specify a different type.


591-591: Unexpected any. Specify a different type.


18-28: Some named imports are only used as types.


28-29: All these imports are only used as types.


175-175: Don't use 'Object' as a type.


176-176: Don't use 'Object' as a type.


554-554: Don't use 'Object' as a type.


580-580: Don't use 'Object' as a type.


601-601: Don't use 'Object' as a type.

indexer/services/socks/__tests__/lib/subscriptions.test.ts (1)

70-70: Don't use 'Object' as a type.

indexer/services/socks/src/lib/subscription.ts (12)

125-126: Template literals are preferred over string concatenation.


162-162: This type annotation is trivially inferred from its initialization.


255-257: Prefer for...of instead of forEach.


256-256: Forbidden non-null assertion.


313-313: This type annotation is trivially inferred from its initialization.


317-317: Change to an optional chain.


327-327: Change to an optional chain.


357-377: Prefer for...of instead of forEach.


510-510: Forbidden non-null assertion.


5-15: Some named imports are only used as types.


15-16: All these imports are only used as types.


20-27: Some named imports are only used as types.

Path-based Instructions (15)
indexer/services/ender/src/handlers/trading-rewards-handler.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/ender/src/handlers/stateful-order/conditional-order-placement-handler.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/ender/src/handlers/transfer-handler.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/ender/src/handlers/subaccount-update-handler.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/ender/src/handlers/stateful-order/stateful-order-placement-handler.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/ender/src/handlers/order-fills/abstract-order-fill-handler.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/ender/src/helpers/kafka-helper.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/socks/__tests__/lib/subscriptions.test.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/socks/__tests__/lib/message-forwarder.test.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/ender/__tests__/helpers/kafka-helper.test.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/ender/__tests__/handlers/transfer-handler.test.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/comlink/public/websocket-documentation.md (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/socks/src/lib/subscription.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/ender/__tests__/helpers/indexer-proto-helpers.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/ender/__tests__/handlers/subaccount-update-handler.test.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

Additional comments not posted (16)
indexer/services/ender/src/handlers/trading-rewards-handler.ts (1)

54-54: Ensure that this.block.height is always defined before use to avoid runtime errors.

indexer/services/ender/src/handlers/stateful-order/conditional-order-placement-handler.ts (1)

50-50: Ensure that this.block.height is always defined before use to avoid runtime errors.

indexer/services/ender/src/handlers/transfer-handler.ts (1)

60-60: Ensure that this.block.height is always defined before use to avoid runtime errors.

Also applies to: 78-78

indexer/services/ender/src/handlers/subaccount-update-handler.ts (1)

94-94: Ensure that this.block.height is always defined before use to avoid runtime errors.

indexer/services/ender/src/handlers/stateful-order/stateful-order-placement-handler.ts (1)

102-102: Ensure that this.block.height is always defined before use to avoid runtime errors.

indexer/services/ender/src/handlers/order-fills/abstract-order-fill-handler.ts (1)

75-75: Ensure that this.block.height is always defined before use to avoid runtime errors.

Verification successful

The IndexerTendermintBlock interface includes a height property, confirming that this.block.height is defined and initialized before use.

  • indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/indexer_manager/event.ts:
    export interface IndexerTendermintBlock {
      height: number;
      time?: Date;
      events: IndexerTendermintEvent[];
      txHashes: string[];
    }
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that `this.block.height` is always initialized before use.

# Test: Search for the initialization of `this.block.height`. Expect: Initialization before use.
rg --type python $'this.block.height ='

Length of output: 71



Script:

#!/bin/bash
# Description: Verify that `this.block.height` is always initialized before use.

# Test: Search for the initialization of `this.block.height`. Expect: Initialization before use.
rg --type typescript 'this.block.height ='

Length of output: 79



Script:

#!/bin/bash
# Description: Verify that `this.block.height` is always initialized before use.

# Test: Search for the initialization of `this.block.height`. Expect: Initialization before use.
rg 'this.block.height ='

Length of output: 111



Script:

#!/bin/bash
# Description: Verify that `this.block` is initialized before `this.block.height` is used.

# Test: Search for the initialization of `this.block`. Expect: Initialization before use.
rg 'this.block ='

Length of output: 780



Script:

#!/bin/bash
# Description: Verify that `this.block` is initialized with a `height` property before use.

# Test: Search for the definition and structure of `block` in the relevant files.
rg 'block' indexer/services/ender/src/validators/validator.ts indexer/services/ender/src/handlers/handler.ts indexer/services/ender/src/lib/block-processor.ts

Length of output: 4780



Script:

#!/bin/bash
# Description: Verify that `IndexerTendermintBlock` includes a `height` property.

# Test: Search for the definition of `IndexerTendermintBlock` to confirm it has a `height` property.
rg 'IndexerTendermintBlock' 

Length of output: 38760



Script:

#!/bin/bash
# Description: Verify that `IndexerTendermintBlock` includes a `height` property.

# Test: Extract the definition of `IndexerTendermintBlock` from `event.pb.go` and `event.ts`.
rg --multiline --multiline-dotall 'type IndexerTendermintBlock struct \{[^}]*\}' indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/indexer_manager/event.pb.go
rg --multiline --multiline-dotall 'interface IndexerTendermintBlock \{[^}]*\}' indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/indexer_manager/event.ts

Length of output: 599

indexer/services/socks/__tests__/lib/subscriptions.test.ts (1)

76-79: Inclusion of blockHeightRefresher in test setup aligns with PR objectives and is correctly implemented.

indexer/services/socks/__tests__/lib/message-forwarder.test.ts (1)

185-188: Inclusion of blockHeightRefresher in test setup aligns with PR objectives and is correctly implemented.

indexer/services/ender/__tests__/helpers/kafka-helper.test.ts (1)

86-91: Ensure consistent handling of optional blockHeight in SubaccountMessageContents.

indexer/services/comlink/public/websocket-documentation.md (2)

Line range hint 60-88: Update the WebSocket documentation to include the blockHeight field.

The addition of the blockHeight field in the WebSocket message for subaccounts is clearly documented. This change aligns with the PR's objective to add block height information to the subaccount messages, enhancing the data provided to clients.


121-122: Ensure the optional blockHeight field is consistently documented.

The optional nature of the blockHeight field in the SubaccountMessageContents interface is correctly documented. This allows for flexibility in message content, accommodating scenarios where the block height might not be available.

indexer/services/socks/src/lib/subscription.ts (3)

9-9: Integration of blockHeightRefresher in imports.

This change is necessary for fetching the latest block height as part of the websocket message enhancements.


Line range hint 538-569: Updated getInitialResponseForSubaccountSubscription to include blockHeight.

The inclusion of the latest block height in the websocket response for subaccounts is consistent with the PR's objectives to enhance websocket messages with block height information.


Line range hint 607-638: Updated getInitialResponseForParentSubaccountSubscription to include blockHeight.

This change aligns with the PR's goal of enhancing websocket messages with block height information, ensuring that parent subaccount subscriptions also receive the latest block height.

indexer/services/ender/__tests__/helpers/indexer-proto-helpers.ts (2)

Line range hint 688-743: Updated Kafka message generation to include blockHeight.

The inclusion of blockHeight in the Kafka messages for subaccount updates aligns with the PR's objectives to enhance messages with block height information.


803-803: Updated Kafka message generation to include blockHeight.

This change ensures that the Kafka messages for order fills and positions also include the latest block height, consistent with the enhancements described in the PR.

@@ -570,6 +574,7 @@ export class OrderRemoveHandler extends Handler {
canceledOrder: OrderFromDatabase | undefined,
orderRemove: OrderRemoveV1,
ticker: string,
blockHeight: string | undefined,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in what case would it be undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it shouldn't be undefined. removed.

@@ -150,6 +151,7 @@ export class OrderPlaceHandler extends Handler {
dbOrder,
perpetualMarket,
placementStatus,
blockHeightRefresher.getLatestBlockHeight(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this expected to be the block that this order belonged to? Can there be a situation where the latest block height is increased by ender before this is called such that this is a later block than the one corresponding to this order?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is the latest block processed by Ender. Yes, that situation is possible.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Outside diff range and nitpick comments (4)
indexer/services/ender/src/helpers/kafka-helper.ts (2)

Line range hint 53-67: Ensure that the blockHeight parameter is properly documented in the function's docstring.

Consider adding a description for the blockHeight parameter in the function's documentation to maintain clarity and completeness.


Line range hint 237-261: Avoid using non-null assertions. Consider checking for null or undefined before using the values.

- sender: {
-   address: transfer.senderWalletAddress ?? senderSubaccountId!.owner,
-   subaccountNumber: transfer.senderWalletAddress ? undefined : senderSubaccountId!.number,
- },
- recipient: {
-   address: transfer.recipientWalletAddress ?? recipientSubaccountId!.owner,
-   subaccountNumber: transfer.recipientWalletAddress ? undefined : recipientSubaccountId!.number,
- },
+ let senderAddress = transfer.senderWalletAddress ?? senderSubaccountId?.owner;
+ let senderSubaccountNumber = transfer.senderWalletAddress ? undefined : senderSubaccountId?.number;
+ let recipientAddress = transfer.recipientWalletAddress ?? recipientSubaccountId?.owner;
+ let recipientSubaccountNumber = transfer.recipientWalletAddress ? undefined : recipientSubaccountId?.number;
+ if (!senderAddress || !recipientAddress) {
+   throw new Error('Sender or recipient information is missing');
+ }
+ sender: { address: senderAddress, subaccountNumber: senderSubaccountNumber },
+ recipient: { address: recipientAddress, subaccountNumber: recipientSubaccountNumber },
indexer/services/vulcan/src/handlers/order-remove-handler.ts (2)

Line range hint 237-299: Ensure that the blockHeight parameter is properly handled and documented in the function's docstring.

Consider adding a description for the blockHeight parameter in the function's documentation to maintain clarity and completeness.


Line range hint 204-204: Avoid using non-null assertions. Consider checking for null or undefined before using the values.

- const orderRemove: OrderRemoveV1 = update.orderRemove!;
- const clobPairId: string = orderRemove.removedOrderId!.clobPairId.toString();
- const order: OrderFromDatabase | undefined = await runFuncWithTimingStat(
-   OrderTable.findById(OrderTable.orderIdToUuid(orderRemove.removedOrderId!)),
- );
- const perpetualMarket: PerpetualMarketFromDatabase | undefined = perpetualMarketRefresher
-   .getPerpetualMarketFromClobPairId(orderRemove.removedOrderId!.clobPairId.toString());
+ const orderRemove: OrderRemoveV1 = update.orderRemove;
+ if (!orderRemove || !orderRemove.removedOrderId) {
+   logger.error('OrderRemove or removedOrderId is undefined');
+   return;
+ }
+ const clobPairId: string = orderRemove.removedOrderId.clobPairId.toString();
+ const order: OrderFromDatabase | undefined = await runFuncWithTimingStat(
+   OrderTable.findById(OrderTable.orderIdToUuid(orderRemove.removedOrderId)),
+ );
+ const perpetualMarket: PerpetualMarketFromDatabase | undefined = perpetualMarketRefresher
+   .getPerpetualMarketFromClobPairId(orderRemove.removedOrderId.clobPairId.toString());

Also applies to: 250-250, 272-272, 274-274, 291-291, 315-315, 328-328, 350-350, 357-357

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 17744c4 and 424b75a.
Files selected for processing (3)
  • indexer/packages/postgres/src/loops/block-height-refresher.ts (1 hunks)
  • indexer/services/ender/src/helpers/kafka-helper.ts (4 hunks)
  • indexer/services/vulcan/src/handlers/order-remove-handler.ts (10 hunks)
Additional Context Used
Biome (30)
indexer/packages/postgres/src/loops/block-height-refresher.ts (2)

12-12: This type annotation is trivially inferred from its initialization.


8-9: All these imports are only used as types.

indexer/services/ender/src/helpers/kafka-helper.ts (8)

242-242: Forbidden non-null assertion.


244-244: Forbidden non-null assertion.


247-247: Forbidden non-null assertion.


249-249: Forbidden non-null assertion.


286-286: Forbidden non-null assertion.


287-287: Forbidden non-null assertion.


1-30: Some named imports are only used as types.


30-31: All these imports are only used as types.

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

82-82: Forbidden non-null assertion.


104-104: Forbidden non-null assertion.


117-117: Forbidden non-null assertion.


119-119: Forbidden non-null assertion.


147-150: The function should not return a value because its return type is void.


154-157: The function should not return a value because its return type is void.


161-164: The function should not return a value because its return type is void.


169-172: The function should not return a value because its return type is void.


176-179: The function should not return a value because its return type is void.


204-204: Forbidden non-null assertion.


250-250: Forbidden non-null assertion.


250-250: Forbidden non-null assertion.


272-272: Forbidden non-null assertion.


274-274: Forbidden non-null assertion.


291-291: Forbidden non-null assertion.


315-315: Forbidden non-null assertion.


328-328: Forbidden non-null assertion.


350-350: Forbidden non-null assertion.


357-357: Forbidden non-null assertion.


357-357: Forbidden non-null assertion.

Path-based Instructions (3)
indexer/packages/postgres/src/loops/block-height-refresher.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

indexer/services/ender/src/helpers/kafka-helper.ts (1)

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

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

Pattern **/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.

Additional comments not posted (1)
indexer/packages/postgres/src/loops/block-height-refresher.ts (1)

12-12: Remove unnecessary type annotation for latestBlockHeight.

- let latestBlockHeight: string = '';
+ let latestBlockHeight = '';

Likely invalid or redundant comment.

Comment on lines +8 to +9
import * as BlockTable from '../stores/block-table';
import { BlockFromDatabase, Options } from '../types';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using type-only imports for BlockTable and BlockFromDatabase as they are only used as types.

import type { BlockFromDatabase, Options } from '../types';
import type * as BlockTable from '../stores/block-table';

Comment on lines +56 to +58
if (config.NODE_ENV !== NodeEnv.TEST) {
throw new Error('clear cannot be used in non-test env');
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throwing errors in non-test environments could be risky. Consider using a safer approach to handle this condition.

- throw new Error('clear cannot be used in non-test env');
+ logger.error('clear cannot be used in non-test env');
+ return;

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if (config.NODE_ENV !== NodeEnv.TEST) {
throw new Error('clear cannot be used in non-test env');
}
if (config.NODE_ENV !== NodeEnv.TEST) {
logger.error('clear cannot be used in non-test env');
return;
}

@dydxwill dydxwill requested a review from chenyaoy May 28, 2024 16:06
@dydxwill dydxwill merged commit 50919ec into main May 28, 2024
11 checks passed
@dydxwill dydxwill deleted the wl/bh_socks branch May 28, 2024 16:29
@dydxwill
Copy link
Contributor Author

https://github.com/Mergifyio backport release/indexer/v5.x

Copy link
Contributor

mergify bot commented May 28, 2024

backport release/indexer/v5.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request May 28, 2024
dydxwill added a commit that referenced this pull request May 28, 2024
) (#1591)

Co-authored-by: dydxwill <119354122+dydxwill@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants