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

Adam/add candles hloc #2047

Merged
merged 3 commits into from
Aug 7, 2024
Merged

Adam/add candles hloc #2047

merged 3 commits into from
Aug 7, 2024

Conversation

adamfraser
Copy link
Contributor

@adamfraser adamfraser commented Aug 6, 2024

Changelist

Re-add orderbookMidPriceOpen and Close to candles table

Test Plan

Tested manually in Dev environments

Summary by CodeRabbit

  • New Features

    • Introduced new branch trigger for GitHub Actions, enhancing workflow responsiveness.
    • Added orderbookMidPriceOpen and orderbookMidPriceClose properties to various models and API documentation, improving market data representation.
  • Bug Fixes

    • Enhanced candle generation process by integrating order book mid prices, ensuring accurate financial data management.
  • Tests

    • Added comprehensive test suite for getOrderBookMidPrice, validating edge cases and mid-price calculations.
  • Documentation

    • Updated API documentation and Swagger schema to include new mid-price fields for better client insights.

Copy link
Contributor

coderabbitai bot commented Aug 6, 2024

Walkthrough

The recent changes enhance the system's ability to manage order book data within the context of financial candle generation. Key additions include new properties for order book mid prices in various data models, updates to workflow triggers for specific branches, and the introduction of new functions and tests to handle these changes effectively. Overall, these modifications improve accuracy and responsiveness to market conditions.

Changes

Files Change Summary
.github/workflows/indexer-build-and-push-{dev,mainnet,testnet}.yml Added branch pattern 'adam/add-candles-hloc' to trigger workflows for builds and pushes.
indexer/packages/postgres/__tests__/helpers/constants.ts Added orderbookMidPriceOpen and orderbookMidPriceClose properties to defaultCandle.
indexer/packages/postgres/__tests__/stores/candle-table.test.ts Modified CandleUpdateObject to include new order book mid price properties in updates.
indexer/packages/postgres/src/db/migrations/migration_files/20240627152937_candles_add_mid_book_price_open_and_close.ts Introduced migration to add orderbookMidPriceOpen and orderbookMidPriceClose columns to the candles table.
indexer/packages/postgres/src/models/candle-model.ts Updated CandleModel to include new order book mid price properties.
indexer/packages/postgres/src/types/candle-types.ts Added optional properties orderbookMidPriceOpen and orderbookMidPriceClose to CandleCreateObject and CandleUpdateObject.
indexer/packages/postgres/src/types/db-model-types.ts Extended CandleFromDatabase interface with orderbookMidPriceOpen and orderbookMidPriceClose.
indexer/packages/redis/__tests__/caches/orderbook-levels-cache.test.ts Introduced a test suite for the new getOrderBookMidPrice function.
indexer/packages/redis/src/caches/orderbook-levels-cache.ts Added getOrderBookMidPrice function to calculate mid prices from order book levels.
indexer/services/comlink/public/api-documentation.md Updated API documentation to include new fields for order book mid prices.
indexer/services/comlink/public/swagger.json Added orderbookMidPriceOpen and orderbookMidPriceClose to the API schema.
indexer/services/ender/__tests__/helpers/redis-helpers.ts Added updatePriceLevel function for managing price levels in order books.
indexer/services/ender/src/lib/candles-generator.ts Enhanced CandlesGenerator to incorporate order book mid prices in candle generation logic.
indexer/services/roundtable/src/lib/athena-ddl-tables/candles.ts Updated table definitions to include new order book mid price columns.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant API
    participant Redis
    participant DB

    Client->>API: Request for candle data
    API->>Redis: Get order book prices
    Redis-->>API: Return mid prices
    API->>DB: Fetch candle data
    DB-->>API: Return candle data
    API-->>Client: Send combined response
Loading

🐰 In fields of green where candles sway,
New prices dance, come out to play!
With open highs and closing lows,
The market’s tale in numbers flows.
So hop along, let joy be known,
For every trade, our skills have grown! 🐇✨


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 do a full 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 Configuration 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.

@adamfraser adamfraser force-pushed the adam/add-candles-hloc branch from c17b38d to b38174e Compare August 6, 2024 20:35
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: 6

Outside diff range, codebase verification and nitpick comments (6)
indexer/packages/postgres/src/models/candle-model.ts (1)

83-85: Verify the usage of new properties.

The new properties orderbookMidPriceOpen and orderbookMidPriceClose are not handled in the instances where CandleModel is used. Please ensure that these properties are appropriately managed wherever CandleModel is utilized in the codebase.

  • Files to update:
    • indexer/packages/postgres/src/stores/candle-table.ts
Analysis chain

Verify the usage of new properties.

Ensure that all instances where CandleModel is used are updated to handle the new properties orderbookMidPriceOpen and orderbookMidPriceClose.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances where `CandleModel` is used to ensure the new properties are handled.

# Test: Search for the usage of `CandleModel`. Expect: Instances where the new properties are handled.
rg --type typescript -A 5 $'CandleModel'

Length of output: 74


Script:

#!/bin/bash
# Description: Verify all instances where `CandleModel` is used to ensure the new properties are handled.

# Test: Search for the usage of `CandleModel`. Expect: Instances where the new properties are handled.
rg -A 5 'CandleModel'

Length of output: 3632


Script:

#!/bin/bash
# Description: Verify the usage of new properties `orderbookMidPriceOpen` and `orderbookMidPriceClose` in the codebase.

# Test: Search for the usage of `orderbookMidPriceOpen` and `orderbookMidPriceClose` in relevant files.
rg 'orderbookMidPriceOpen|orderbookMidPriceClose' indexer/packages/postgres/src/stores/candle-table.ts

Length of output: 104


Script:

#!/bin/bash
# Description: Verify the usage of new properties `orderbookMidPriceOpen` and `orderbookMidPriceClose` in the codebase.

# Test: Search for the usage of `orderbookMidPriceOpen` and `orderbookMidPriceClose` in the entire codebase.
rg 'orderbookMidPriceOpen|orderbookMidPriceClose'

Length of output: 10317

indexer/packages/postgres/src/types/db-model-types.ts (1)

201-202: Add JSDoc comments for new properties.

Consider adding JSDoc comments to the new properties orderbookMidPriceOpen and orderbookMidPriceClose to maintain consistency and improve code readability.

/**
 * Mid price of the order book at the opening of the candle.
 */
orderbookMidPriceOpen?: string | null;

/**
 * Mid price of the order book at the closing of the candle.
 */
orderbookMidPriceClose?: string | null;
indexer/services/ender/src/lib/candles-generator.ts (4)

46-47: Add JSDoc comment for OrderbookMidPrice type.

Consider adding a JSDoc comment to the OrderbookMidPrice type to clarify its purpose and usage.

/**
 * Represents the mid price of the order book, which can be a string or undefined.
 */
type OrderbookMidPrice = string | undefined;

209-220: Clarify the comment for createUpdateOrPassPostgresCandle.

The comment explaining the different cases for createUpdateOrPassPostgresCandle is helpful but could be made clearer by using bullet points or a table format.

/**
 * Creates, updates, or does nothing for a ticker and resolution depending on the following cases:
 * 
 * Cases:
 * - Candle doesn't exist & there is no block update: do nothing
 * - Candle doesn't exist & there is a block update: create candle
 * - Candle exists & !sameStartTime & there is a block update: create candle,
 *   update previous candle orderbookMidPriceClose
 * - Candle exists & !sameStartTime & there is no block update: create empty candle,
 *   update previous candle orderbookMidPriceClose
 * - Candle exists & sameStartTime & no block update: do nothing
 * - Candle exists & sameStartTime & block update: update candle
 * 
 * The orderbookMidPriceClose/Open are updated for each candle at the start and end of
 * each resolution period. Whenever we create a new candle we set the orderbookMidPriceClose/Open.
 * If there is a previous candle & we're creating a new one (this occurs at the
 * beginning of a resolution period) set the previous candle's orderbookMidPriceClose.
 */

Line range hint 373-388:
Ensure proper error handling in createCandleInPostgres.

The function createCandleInPostgres does not handle potential errors from CandleTable.create. Consider adding error handling to improve robustness.

return CandleTable.create(candle, this.writeOptions);

Line range hint 405-420:
Ensure proper error handling in createEmptyCandleInPostgres.

The function createEmptyCandleInPostgres does not handle potential errors from CandleTable.create. Consider adding error handling to improve robustness.

return CandleTable.create(candle, this.writeOptions);
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1b0f052 and 132629f.

Files selected for processing (17)
  • .github/workflows/indexer-build-and-push-dev-staging.yml (1 hunks)
  • .github/workflows/indexer-build-and-push-mainnet.yml (1 hunks)
  • .github/workflows/indexer-build-and-push-testnet.yml (1 hunks)
  • indexer/packages/postgres/tests/helpers/constants.ts (1 hunks)
  • indexer/packages/postgres/tests/stores/candle-table.test.ts (1 hunks)
  • indexer/packages/postgres/src/db/migrations/migration_files/20240627152937_candles_add_mid_book_price_open_and_close.ts (1 hunks)
  • indexer/packages/postgres/src/models/candle-model.ts (2 hunks)
  • indexer/packages/postgres/src/types/candle-types.ts (2 hunks)
  • indexer/packages/postgres/src/types/db-model-types.ts (1 hunks)
  • indexer/packages/redis/tests/caches/orderbook-levels-cache.test.ts (2 hunks)
  • indexer/packages/redis/src/caches/orderbook-levels-cache.ts (1 hunks)
  • indexer/services/comlink/public/api-documentation.md (4 hunks)
  • indexer/services/comlink/public/swagger.json (1 hunks)
  • indexer/services/ender/tests/helpers/redis-helpers.ts (2 hunks)
  • indexer/services/ender/tests/lib/candles-generator.test.ts (22 hunks)
  • indexer/services/ender/src/lib/candles-generator.ts (14 hunks)
  • indexer/services/roundtable/src/lib/athena-ddl-tables/candles.ts (2 hunks)
Additional comments not posted (36)
indexer/packages/postgres/src/db/migrations/migration_files/20240627152937_candles_add_mid_book_price_open_and_close.ts (2)

3-10: LGTM! The migration to add new columns is correctly structured.

The up function correctly alters the candles table to add orderbookMidPriceOpen and orderbookMidPriceClose columns as nullable decimals.


12-19: LGTM! The rollback migration is correctly structured.

The down function correctly alters the candles table to drop orderbookMidPriceOpen and orderbookMidPriceClose columns, ensuring reversibility.

.github/workflows/indexer-build-and-push-mainnet.yml (1)

9-9: LGTM! The new branch pattern is correctly added.

The on section now includes the new branch pattern 'adam/add-candles-hloc', which will trigger the workflow for changes in this branch.

.github/workflows/indexer-build-and-push-testnet.yml (1)

9-9: LGTM! The new branch pattern is correctly added.

The on section now includes the new branch pattern 'adam/add-candles-hloc', which will trigger the workflow for changes in this branch.

indexer/packages/postgres/src/types/candle-types.ts (2)

15-16: LGTM! The new properties are correctly defined.

The additions of orderbookMidPriceOpen and orderbookMidPriceClose as optional properties in CandleCreateObject are appropriate and align with the intended enhancements.


29-30: LGTM! The new properties are correctly defined.

The additions of orderbookMidPriceOpen and orderbookMidPriceClose as optional properties in CandleUpdateObject are appropriate and align with the intended enhancements.

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

35-49: LGTM! The new function is correctly defined.

The addition of updatePriceLevel enhances the module's functionality related to order book management. The function is well-defined and integrates correctly with the OrderbookLevelsCache.

indexer/services/roundtable/src/lib/athena-ddl-tables/candles.ts (2)

22-23: LGTM! The new columns are correctly defined.

The additions of orderbookMidPriceOpen and orderbookMidPriceClose to RAW_TABLE_COLUMNS are appropriate and align with the intended enhancements.


38-39: LGTM! The new columns are correctly defined.

The additions of orderbookMidPriceOpen and orderbookMidPriceClose to TABLE_COLUMNS are appropriate and align with the intended enhancements.

.github/workflows/indexer-build-and-push-dev-staging.yml (1)

9-9: LGTM!

The addition of the new branch pattern 'adam/add-candles-hloc' to the list of branches that trigger the workflow is appropriate and aligns with the PR objectives.

indexer/packages/postgres/src/models/candle-model.ts (1)

53-54: LGTM!

The addition of orderbookMidPriceOpen and orderbookMidPriceClose properties to the CandleModel class is well-implemented and aligns with the PR objectives. The properties are correctly defined with appropriate types and validation patterns.

indexer/packages/postgres/__tests__/stores/candle-table.test.ts (1)

68-69: LGTM!

The addition of orderbookMidPriceClose and orderbookMidPriceOpen properties to the CandleUpdateObject in the CandleTable test suite is well-implemented and aligns with the PR objectives. The changes enhance the test suite to handle the new properties effectively.

Also applies to: 72-72

indexer/services/ender/src/lib/candles-generator.ts (1)

446-447: Ensure orderbookMidPriceOpen and orderbookMidPriceClose are not undefined.

Before using orderbookMidPriceOpen and orderbookMidPriceClose, ensure they are not undefined to avoid potential issues.

orderbookMidPriceOpen: existingCandle.orderbookMidPriceOpen ?? undefined,
orderbookMidPriceClose: existingCandle.orderbookMidPriceClose ?? undefined,

Likely invalid or redundant comment.

indexer/packages/redis/__tests__/caches/orderbook-levels-cache.test.ts (5)

700-748: LGTM! The test case comprehensively covers the mid price calculation.

The test case sets up price levels and verifies the mid price calculation for various scenarios.


751-771: LGTM! The test case comprehensively covers the mid price calculation for very small numbers.

The test case sets up very small price levels and verifies the mid price calculation.


773-793: LGTM! The test case comprehensively covers the mid price calculation with varying decimal precision.

The test case sets up price levels with different decimal precision and verifies the mid price calculation.


795-806: LGTM! The test case comprehensively covers the scenario with no bids or asks.

The test case sets up a scenario with no bids or asks and verifies the mid price calculation.


808-820: LGTM! The test case comprehensively covers the scenario with NaN as humanPrice.

The test case sets up a scenario with NaN as the humanPrice and verifies the mid price calculation.

indexer/packages/postgres/__tests__/helpers/constants.ts (1)

728-729: LGTM! The addition of orderbookMidPriceOpen and orderbookMidPriceClose enhances the defaultCandle constant.

These new properties improve the representation of market data.

indexer/services/ender/__tests__/lib/candles-generator.test.ts (12)

Line range hint 21-37:
LGTM! The new import statements are consistent with the changes in the test cases.

The imports include OrderSide and getOrderbookMidPriceMap.


Line range hint 116-137:
LGTM! The test case comprehensively covers the creation of candles with no open positions.

The test case sets up order book levels and verifies the creation of candles with orderbookMidPriceOpen and orderbookMidPriceClose.


Line range hint 158-183:
LGTM! The test case comprehensively covers the creation of candles with open interest.

The test case sets up order book levels and verifies the creation of candles with open interest.


Line range hint 201-260:
LGTM! The test case comprehensively covers the update of existing candles.

The test case sets up existing candles and verifies their update with new order book levels.


Line range hint 275-310:
LGTM! The test case comprehensively covers the creation of an empty candle.

The test case sets up an empty candle and verifies its creation with order book levels.


Line range hint 326-341:
LGTM! The test case comprehensively covers the creation of a new candle if the existing candle is from a past normalized start time.

The test case sets up an existing candle from a past normalized start time and verifies the creation of a new candle.


Line range hint 357-372:
LGTM! The test case comprehensively covers the update of an empty candle.

The test case sets up an empty candle and verifies its update with order book levels.


376-381: LGTM! The test case comprehensively covers the scenario with no trades and no existing candle.

The test case sets up a scenario with no trades and no existing candle and verifies that no new candle is created.


Line range hint 397-419:
LGTM! The test case comprehensively covers the scenario with no trades and an existing candle.

The test case sets up a scenario with no trades and an existing candle and verifies that the existing candle is not updated.


468-565: LGTM! The test case comprehensively covers the update of orderbookMidPriceClose if the start time is past the candle resolution.

The test case sets up existing candles and verifies that the orderbookMidPriceClose is updated if the start time is past the candle resolution.


567-659: LGTM! The test case comprehensively covers the creation of an empty candle and the update of orderbookMidPriceClose if the start time is past the candle resolution.

The test case sets up existing candles and verifies the creation of an empty candle and the update of orderbookMidPriceClose if the start time is past the candle resolution.


662-680: LGTM! The test case comprehensively covers the creation of an order book price map for each market.

The test case sets up order book levels for different markets and verifies the creation of an order book price map.

indexer/services/comlink/public/swagger.json (2)

306-309: Addition of orderbookMidPriceOpen property looks good.

The property is correctly defined as a nullable string, consistent with the existing schema.


310-313: Addition of orderbookMidPriceClose property looks good.

The property is correctly defined as a nullable string, consistent with the existing schema.

indexer/services/comlink/public/api-documentation.md (3)

695-695: Verify the addition of orderbookMidPriceOpen field.

Ensure that the orderbookMidPriceOpen field is correctly documented and integrated into the response schema.


696-696: Verify the addition of orderbookMidPriceClose field.

Ensure that the orderbookMidPriceClose field is correctly documented and integrated into the response schema.


3708-3709: Verify the addition of orderbookMidPriceOpen and orderbookMidPriceClose fields.

Ensure that the orderbookMidPriceOpen and orderbookMidPriceClose fields are correctly documented and integrated into the response schema.

Comment on lines +533 to +555
export async function getOrderBookMidPrice(
ticker: string,
client: RedisClient,
): Promise<string | undefined> {
const levels = await getOrderBookLevels(ticker, client, {
removeZeros: true,
sortSides: true,
uncrossBook: true,
limitPerSide: 1,
});

if (levels.bids.length === 0 || levels.asks.length === 0) {
return undefined;
}

const bestAsk = Big(levels.asks[0].humanPrice);
const bestBid = Big(levels.bids[0].humanPrice);

if (bestAsk === undefined || bestBid === undefined) {
return undefined;
}
return bestBid.plus(bestAsk).div(2).toFixed();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure proper error handling in getOrderBookMidPrice.

The function getOrderBookMidPrice does not handle potential errors from getOrderBookLevels. Consider adding error handling to improve robustness.

export async function getOrderBookMidPrice(
  ticker: string,
  client: RedisClient,
): Promise<string | undefined> {
  try {
    const levels = await getOrderBookLevels(ticker, client, {
      removeZeros: true,
      sortSides: true,
      uncrossBook: true,
      limitPerSide: 1,
    });

    if (levels.bids.length === 0 || levels.asks.length === 0) {
      return undefined;
    }

    const bestAsk = Big(levels.asks[0].humanPrice);
    const bestBid = Big(levels.bids[0].humanPrice);

    if (bestAsk === undefined || bestBid === undefined) {
      return undefined;
    }
    return bestBid.plus(bestAsk).div(2).toFixed();
  } catch (error) {
    logger.error({ at: 'getOrderBookMidPrice', ticker, error });
    return undefined;
  }
}
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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function getOrderBookMidPrice(
ticker: string,
client: RedisClient,
): Promise<string | undefined> {
const levels = await getOrderBookLevels(ticker, client, {
removeZeros: true,
sortSides: true,
uncrossBook: true,
limitPerSide: 1,
});
if (levels.bids.length === 0 || levels.asks.length === 0) {
return undefined;
}
const bestAsk = Big(levels.asks[0].humanPrice);
const bestBid = Big(levels.bids[0].humanPrice);
if (bestAsk === undefined || bestBid === undefined) {
return undefined;
}
return bestBid.plus(bestAsk).div(2).toFixed();
}
export async function getOrderBookMidPrice(
ticker: string,
client: RedisClient,
): Promise<string | undefined> {
try {
const levels = await getOrderBookLevels(ticker, client, {
removeZeros: true,
sortSides: true,
uncrossBook: true,
limitPerSide: 1,
});
if (levels.bids.length === 0 || levels.asks.length === 0) {
return undefined;
}
const bestAsk = Big(levels.asks[0].humanPrice);
const bestBid = Big(levels.bids[0].humanPrice);
if (bestAsk === undefined || bestBid === undefined) {
return undefined;
}
return bestBid.plus(bestAsk).div(2).toFixed();
} catch (error) {
logger.error({ at: 'getOrderBookMidPrice', ticker, error });
return undefined;
}
}

Comment on lines +227 to +228
orderbookMidPrice: OrderbookMidPrice,
): Promise<CandleFromDatabase | undefined>[] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure orderbookMidPrice is not undefined.

Before using orderbookMidPrice, ensure it is not undefined to avoid potential issues.

orderbookMidPrice: OrderbookMidPrice,

Committable suggestion was skipped due to low confidence.

Comment on lines +469 to +470
orderbookMidPriceClose: existingCandle.orderbookMidPriceClose ?? undefined,
orderbookMidPriceOpen: existingCandle.orderbookMidPriceOpen ?? undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure orderbookMidPriceOpen and orderbookMidPriceClose are not undefined.

Before using orderbookMidPriceOpen and orderbookMidPriceClose, ensure they are not undefined to avoid potential issues.

orderbookMidPriceClose: existingCandle.orderbookMidPriceClose ?? undefined,
orderbookMidPriceOpen: existingCandle.orderbookMidPriceOpen ?? undefined,

Committable suggestion was skipped due to low confidence.

@@ -167,6 +171,7 @@
const promises: Promise<CandleFromDatabase | undefined>[] = [];

const openInterestMap: OpenInterestMap = await this.getOpenInterestMap();
const orderbookMidPriceMap = await getOrderbookMidPriceMap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure proper error handling for getOrderbookMidPriceMap.

The function getOrderbookMidPriceMap does not handle potential errors from OrderbookLevelsCache.getOrderBookMidPrice. Consider adding error handling to improve robustness.

const orderbookMidPriceMap = await getOrderbookMidPriceMap();

Committable suggestion was skipped due to low confidence.

Comment on lines +476 to +490
private async updateCandleWithOrderbookMidPriceInPostgres(
existingCandle: CandleFromDatabase,
orderbookMidPrice: OrderbookMidPrice,
): Promise<CandleFromDatabase> {

const candle: CandleUpdateObject = {
id: existingCandle.id,
low: existingCandle.low,
high: existingCandle.high,
close: existingCandle.close,
baseTokenVolume: existingCandle.baseTokenVolume,
usdVolume: existingCandle.usdVolume,
trades: existingCandle.trades,
orderbookMidPriceOpen: existingCandle.orderbookMidPriceOpen ?? undefined,
orderbookMidPriceClose: orderbookMidPrice,
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure proper error handling in updateCandleWithOrderbookMidPriceInPostgres.

The function updateCandleWithOrderbookMidPriceInPostgres does not handle potential errors from CandleTable.update. Consider adding error handling to improve robustness.

return CandleTable.update(candle, this.writeOptions) as Promise<CandleFromDatabase>;

Committable suggestion was skipped due to low confidence.

Comment on lines +533 to +554
/**
* Get the cached orderbook mid price for a given ticker
*/
export async function getOrderbookMidPriceMap(): Promise<{ [ticker: string]: OrderbookMidPrice; }> {
const perpetualMarkets = Object.values(perpetualMarketRefresher.getPerpetualMarketsMap());

const promises = perpetualMarkets.map(async (perpetualMarket: PerpetualMarketFromDatabase) => {
const price = await OrderbookLevelsCache.getOrderBookMidPrice(
perpetualMarket.ticker,
redisClient,
);
return { [perpetualMarket.ticker]: price === undefined ? undefined : price };
});

const pricesArray = await Promise.all(promises);
const priceMap: { [ticker: string]: OrderbookMidPrice; } = {};
pricesArray.forEach((price) => {
Object.assign(priceMap, price);
});

return priceMap;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure proper error handling in getOrderbookMidPriceMap.

The function getOrderbookMidPriceMap does not handle potential errors from OrderbookLevelsCache.getOrderBookMidPrice. Consider adding error handling to improve robustness.

export async function getOrderbookMidPriceMap(): Promise<{ [ticker: string]: OrderbookMidPrice; }> {
  try {
    const perpetualMarkets = Object.values(perpetualMarketRefresher.getPerpetualMarketsMap());

    const promises = perpetualMarkets.map(async (perpetualMarket: PerpetualMarketFromDatabase) => {
      const price = await OrderbookLevelsCache.getOrderBookMidPrice(
        perpetualMarket.ticker,
        redisClient,
      );
      return { [perpetualMarket.ticker]: price === undefined ? undefined : price };
    });

    const pricesArray = await Promise.all(promises);
    const priceMap: { [ticker: string]: OrderbookMidPrice; } = {};
    pricesArray.forEach((price) => {
      Object.assign(priceMap, price);
    });

    return priceMap;
  } catch (error) {
    logger.error({ at: 'getOrderbookMidPriceMap', error });
    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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Get the cached orderbook mid price for a given ticker
*/
export async function getOrderbookMidPriceMap(): Promise<{ [ticker: string]: OrderbookMidPrice; }> {
const perpetualMarkets = Object.values(perpetualMarketRefresher.getPerpetualMarketsMap());
const promises = perpetualMarkets.map(async (perpetualMarket: PerpetualMarketFromDatabase) => {
const price = await OrderbookLevelsCache.getOrderBookMidPrice(
perpetualMarket.ticker,
redisClient,
);
return { [perpetualMarket.ticker]: price === undefined ? undefined : price };
});
const pricesArray = await Promise.all(promises);
const priceMap: { [ticker: string]: OrderbookMidPrice; } = {};
pricesArray.forEach((price) => {
Object.assign(priceMap, price);
});
return priceMap;
}
export async function getOrderbookMidPriceMap(): Promise<{ [ticker: string]: OrderbookMidPrice; }> {
try {
const perpetualMarkets = Object.values(perpetualMarketRefresher.getPerpetualMarketsMap());
const promises = perpetualMarkets.map(async (perpetualMarket: PerpetualMarketFromDatabase) => {
const price = await OrderbookLevelsCache.getOrderBookMidPrice(
perpetualMarket.ticker,
redisClient,
);
return { [perpetualMarket.ticker]: price === undefined ? undefined : price };
});
const pricesArray = await Promise.all(promises);
const priceMap: { [ticker: string]: OrderbookMidPrice; } = {};
pricesArray.forEach((price) => {
Object.assign(priceMap, price);
});
return priceMap;
} catch (error) {
logger.error({ at: 'getOrderbookMidPriceMap', error });
return {};
}
}

@adamfraser adamfraser merged commit 73b04dc into main Aug 7, 2024
11 checks passed
@adamfraser adamfraser deleted the adam/add-candles-hloc branch August 7, 2024 18:04
@adamfraser
Copy link
Contributor Author

@Mergifyio backport release/indexer/v5.x

Copy link
Contributor

mergify bot commented Aug 7, 2024

backport release/indexer/v5.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Aug 7, 2024
(cherry picked from commit 73b04dc)
adamfraser added a commit that referenced this pull request Aug 7, 2024
@adamfraser
Copy link
Contributor Author

@Mergifyio backport release/indexer/v6.x

Copy link
Contributor

mergify bot commented Aug 21, 2024

backport release/indexer/v6.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Aug 21, 2024
(cherry picked from commit 73b04dc)

# Conflicts:
#	indexer/packages/postgres/src/db/migrations/migration_files/20240627152937_candles_add_mid_book_price_open_and_close.ts
#	indexer/packages/redis/__tests__/caches/orderbook-levels-cache.test.ts
#	indexer/packages/redis/src/caches/orderbook-levels-cache.ts
#	indexer/services/ender/src/lib/candles-generator.ts
adamfraser added a commit that referenced this pull request Aug 21, 2024
@coderabbitai coderabbitai bot mentioned this pull request Dec 12, 2024
6 tasks
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