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

update subaccountId to address #1947

Merged
merged 4 commits into from
Jul 19, 2024
Merged

update subaccountId to address #1947

merged 4 commits into from
Jul 19, 2024

Conversation

affanv14
Copy link
Contributor

@affanv14 affanv14 commented Jul 18, 2024

Changelist

  • Fixes erroneous column in leaderboard table which should have been address

Test Plan

updated tests

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

  • Bug Fixes

    • Replaced all instances of subaccountId with address in leaderboard data, ensuring correct user identification.
  • Tests

    • Updated test cases to align with the new address property.

This update enhances data consistency and integrity by using a more accurate identifier for leaderboard entries.

Copy link
Contributor

coderabbitai bot commented Jul 18, 2024

Walkthrough

The recent changes focus on replacing subaccountId with address in the LeaderboardPnlModel and related database migrations. Test descriptions and constants were updated accordingly to reflect this semantic shift.

Changes

Files (Grouped) Change Summaries
leaderboard-pnl-model.ts Modified idColumn, jsonSchema, and class properties to replace subaccountId with address.
20240717160024_create_leaderboard_pnl_table.ts Replaced subaccountId column with address and updated primary key in migration file.
constants.ts Updated address properties and introduced a new constant defaultAddress2.
leaderboard-pnl-table.test.ts Updated test descriptions and querying properties to use address instead of subaccountId.

Poem

The code was changed, align the score,
Subaccounts we use no more.
Address now takes center stage,
In the database, a brand new page.
Tests and constants all in sync,
An update swift, quick as a blink.
🎉🐰


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.

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, codebase verification and nitpick comments (4)
indexer/packages/postgres/src/stores/leaderboard-pnl-table.ts (1)

29-29: Parameter change: Ensure consistency with other parts of the system.

The parameter subaccountId has been changed to address in the findAll function. However, there are still instances where subaccountId is used. Please update all occurrences to use address to maintain consistency.

  • Files with subaccountId usage:
    • indexer/services/comlink/src/controllers/api/v4/transfers-controller.ts
    • indexer/services/comlink/src/controllers/api/v4/fills-controller.ts
    • indexer/services/comlink/src/controllers/api/v4/historical-pnl-controller.ts
    • indexer/services/comlink/src/controllers/api/v4/asset-positions-controller.ts
    • indexer/services/comlink/src/controllers/api/v4/addresses-controller.ts
    • indexer/services/comlink/src/controllers/api/v4/perpetual-positions-controller.ts
    • indexer/services/roundtable/src/helpers/pnl-validation-helpers.ts
    • indexer/services/roundtable/src/tasks/update-compliance-data.ts
    • indexer/services/roundtable/__tests__/helpers/pnl-ticks-helper.test.ts
    • indexer/services/roundtable/__tests__/tasks/update-compliance-data.test.ts
    • indexer/packages/postgres/src/stores/transfer-table.ts
    • indexer/packages/postgres/src/stores/pnl-ticks-table.ts
    • indexer/packages/postgres/src/stores/order-table.ts
    • indexer/packages/postgres/src/stores/perpetual-position-table.ts
    • e2e-testing/__tests__/transfers.test.ts
    • e2e-testing/__tests__/orders.test.ts
Analysis chain

Parameter change: Ensure consistency with other parts of the system.

The parameter subaccountId has been changed to address. Verify that all function calls to findAll use the updated parameter name.


Parameter change: Ensure consistency with other parts of the system.

The parameter subaccountId has been changed to address. Verify that all function calls to findAll use the updated parameter name.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `findAll` use the updated parameter name `address`.

# Test: Search for the function usage. Expect: Only occurrences of the new parameter name `address`.
rg --type typescript -A 5 $'findAll'

Length of output: 70


Script:

#!/bin/bash
# Description: Verify all function calls to `findAll` use the updated parameter name `address`.

# Test: Search for the function usage. Expect: Only occurrences of the new parameter name `address`.
rg --type ts -A 5 $'findAll'

Length of output: 201777

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

320-320: Field update: Ensure consistency with other parts of the system.

The field SUBACCOUNT_ID has not been fully replaced by ADDRESS across the codebase. The following files still contain references to SUBACCOUNT_ID:

  • indexer/services/comlink/src/controllers/api/v4/addresses-controller.ts
  • indexer/services/comlink/src/controllers/api/v4/asset-positions-controller.ts
  • indexer/packages/postgres/src/types/query-types.ts

Please update all instances of SUBACCOUNT_ID to ADDRESS in these files to ensure consistency.

Analysis chain

Field update: Ensure consistency with other parts of the system.

The field SUBACCOUNT_ID has been changed to ADDRESS. Verify that all references to this interface use the updated field name.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all references to the `LeaderboardPnlQueryConfig` interface use the updated field name `ADDRESS`.

# Test: Search for the interface usage. Expect: Only occurrences of the new field name `ADDRESS`.
rg --type typescript -A 5 $'LeaderboardPnlQueryConfig'

Length of output: 88


Script:

#!/bin/bash
# Description: Verify that all references to the `LeaderboardPnlQueryConfig` interface use the updated field name `ADDRESS`.

# Test: Search for the interface usage. Expect: Only occurrences of the new field name `ADDRESS`.
rg --type ts -A 5 $'LeaderboardPnlQueryConfig'

Length of output: 1513


Script:

#!/bin/bash
# Description: Verify that all references to the `LeaderboardPnlQueryConfig` interface use the updated field name `ADDRESS` instead of `SUBACCOUNT_ID`.

# Test: Search for occurrences of `SUBACCOUNT_ID` and `ADDRESS` in `leaderboard-pnl-table.ts`.
rg 'SUBACCOUNT_ID|ADDRESS' indexer/packages/postgres/src/stores/leaderboard-pnl-table.ts

Length of output: 90


Script:

#!/bin/bash
# Description: Verify that the field name `SUBACCOUNT_ID` has been replaced by `ADDRESS` across all TypeScript files.

# Test: Search for occurrences of `SUBACCOUNT_ID` in all TypeScript files.
rg 'SUBACCOUNT_ID' --type ts

Length of output: 1907

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

888-888: Field update required: Ensure consistency with other parts of the system.

The address field in the defaultLeaderboardPnlOneDayToUpsert constant still uses defaultAddress. Update this field to defaultAddress2 to maintain consistency across the system.

  • File: indexer/packages/postgres/__tests__/helpers/constants.ts
  • Line: 888
Analysis chain

Field update: Ensure consistency with other parts of the system.

The field address in defaultLeaderboardPnlOneDayToUpsert has been updated to use defaultAddress2. Verify that all references to this constant use the updated field name.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all references to the `defaultLeaderboardPnlOneDayToUpsert` constant use the updated field name `address`.

# Test: Search for the constant usage. Expect: Only occurrences of the new field name `address`.
rg --type typescript -A 5 $'defaultLeaderboardPnlOneDayToUpsert'

Length of output: 98


Script:

#!/bin/bash
# Description: Verify that all references to the `defaultLeaderboardPnlOneDayToUpsert` constant use the updated field name `address`.

# Test: Search for the constant usage. Expect: Only occurrences of the new field name `address`.
rg --glob '*.ts' -A 5 'defaultLeaderboardPnlOneDayToUpsert'

Length of output: 2548


880-880: Field update: Ensure consistency with other parts of the system.

The field address in defaultLeaderboardPnl1AllTime has not been updated to use defaultAddress2 as suggested. Please update the address field in defaultLeaderboardPnl1AllTime to defaultAddress2 and ensure consistency across the codebase.

  • indexer/packages/postgres/__tests__/helpers/constants.ts:880
Analysis chain

Field update: Ensure consistency with other parts of the system.

The field address in defaultLeaderboardPnl1AllTime has been updated to use defaultAddress2. Verify that all references to this constant use the updated field name.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all references to the `defaultLeaderboardPnl1AllTime` constant use the updated field name `address`.

# Test: Search for the constant usage. Expect: Only occurrences of the new field name `address`.
rg --type typescript -A 5 $'defaultLeaderboardPnl1AllTime'

Length of output: 92


Script:

#!/bin/bash
# Description: Verify that all references to the `defaultLeaderboardPnl1AllTime` constant use the updated field name `address`.

# Test: Search for the constant usage. Expect: Only occurrences of the new field name `address`.
rg --type ts -A 5 $'defaultLeaderboardPnl1AllTime'

Length of output: 2509


Script:

#!/bin/bash
# Description: Verify that all references to `defaultAddress` are updated to `defaultAddress2`.

# Test: Search for the usage of `defaultAddress`. Expect: No occurrences where `defaultAddress2` should be used.
rg --type ts $'defaultAddress'

Length of output: 35624

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between cdb8a78 and 466903c.

Files selected for processing (8)
  • indexer/packages/postgres/tests/helpers/constants.ts (2 hunks)
  • indexer/packages/postgres/tests/stores/leaderboard-pnl-table.test.ts (2 hunks)
  • indexer/packages/postgres/src/db/migrations/migration_files/20240717160024_create_leaderboard_pnl_table.ts (2 hunks)
  • indexer/packages/postgres/src/models/leaderboard-pnl-model.ts (2 hunks)
  • indexer/packages/postgres/src/stores/leaderboard-pnl-table.ts (5 hunks)
  • indexer/packages/postgres/src/types/db-model-types.ts (1 hunks)
  • indexer/packages/postgres/src/types/leaderboard-pnl-types.ts (1 hunks)
  • indexer/packages/postgres/src/types/query-types.ts (1 hunks)
Additional comments not posted (19)
indexer/packages/postgres/src/types/leaderboard-pnl-types.ts (2)

4-4: Change from subaccountId to address is appropriate.

This change aligns with the PR objective of using address instead of subaccountId.


12-12: Change from subaccountId to address in the enum is appropriate.

This change aligns with the PR objective of using address instead of subaccountId.

indexer/packages/postgres/src/db/migrations/migration_files/20240717160024_create_leaderboard_pnl_table.ts (2)

5-5: Change from subaccountId to address in column definition is appropriate.

This change aligns with the PR objective and ensures the schema uses address instead of subaccountId.


19-19: Change in primary key definition to use address is appropriate.

This change aligns with the PR objective and ensures the schema uses address instead of subaccountId.

indexer/packages/postgres/src/models/leaderboard-pnl-model.ts (5)

16-16: Change in idColumn method to use address is appropriate.

This change aligns with the PR objective and ensures the model uses address instead of subaccountId.


19-19: Verify the removal of relation mappings.

The relationMappings property is emptied. Ensure this change is intentional and does not affect functionality.


25-25: Change in jsonSchema method to require address is appropriate.

This change aligns with the PR objective and ensures the model uses address instead of subaccountId.


32-32: Change in properties definition to use address is appropriate.

This change aligns with the PR objective and ensures the model uses address instead of subaccountId.


41-41: Change in class property to use address is appropriate.

This change aligns with the PR objective and ensures the model uses address instead of subaccountId.

indexer/packages/postgres/__tests__/stores/leaderboard-pnl-table.test.ts (2)

48-48: Change in test description to use address is appropriate.

This change aligns with the PR objective and ensures the test description uses address instead of subaccountId.


57-57: Change in test query to use address is appropriate.

This change aligns with the PR objective and ensures the test query uses address instead of subaccountId.

indexer/packages/postgres/src/stores/leaderboard-pnl-table.ts (4)

39-39: Verify all required fields.

Ensure that all required fields, including the updated address field, are correctly verified.


138-138: Unique identifiers update: Ensure correctness.

The unique identifiers have been updated to use address instead of subaccountId. Verify that the logic for unique identifiers remains consistent and correct.


52-53: Query condition update: Ensure correctness.

The query condition has been updated to use address instead of subaccountId. Verify that the column name LeaderboardPnlColumns.address is correct and exists in the database schema.

Verification successful

Column name verification successful.

The column LeaderboardPnlColumns.address is correctly defined and exists in the database schema.

  • indexer/packages/postgres/src/types/leaderboard-pnl-types.ts:
    export enum LeaderboardPnlColumns {
      address = 'address',
      timeSpan = 'timeSpan',
      pnl = 'pnl',
      currentEquity = 'currentEquity',
      rank = 'rank',
    }
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the column `LeaderboardPnlColumns.address` exists in the database schema.

# Test: Search for the column definition. Expect: Column `address` should be defined in the schema.
rg --type typescript -A 5 $'LeaderboardPnlColumns.address'

Length of output: 92


Script:

#!/bin/bash
# Description: Verify that the column `LeaderboardPnlColumns.address` exists in the database schema.

# Test: Search for the definition of `LeaderboardPnlColumns` and check if it contains the `address` property.
rg 'LeaderboardPnlColumns' --type ts -A 10

Length of output: 6601


127-127: Column update: Ensure correctness.

The column subaccountId has been replaced with address. Verify that the column name LeaderboardPnlColumns.address is correct and exists in the database schema.

Verification successful

Column update: Verification successful.

The column LeaderboardPnlColumns.address is correctly defined in the codebase as part of the LeaderboardPnlColumns enum in indexer/packages/postgres/src/types/leaderboard-pnl-types.ts.

  • Definition found:
    • indexer/packages/postgres/src/types/leaderboard-pnl-types.ts: export enum LeaderboardPnlColumns { address = 'address', ... }
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the column `LeaderboardPnlColumns.address` exists in the database schema.

# Test: Search for the column definition. Expect: Column `address` should be defined in the schema.
rg --type typescript -A 5 $'LeaderboardPnlColumns.address'

Length of output: 92


Script:

#!/bin/bash
# Description: Verify that the column `LeaderboardPnlColumns.address` exists in the database schema.

# Test: Search for the column definition. Expect: Column `address` should be defined in the schema.
rg --type ts -A 5 $'LeaderboardPnlColumns.address'

Length of output: 1767


Script:

#!/bin/bash
# Description: Verify that the column `LeaderboardPnlColumns.address` is defined in the codebase.

# Test: Search for the definition of `LeaderboardPnlColumns.address`. Expect: The column should be defined in the codebase.
rg --type ts 'address' | grep 'LeaderboardPnlColumns'

Length of output: 437


Script:

#!/bin/bash
# Description: Verify that the column `LeaderboardPnlColumns.address` is defined in the codebase.

# Test: Search for the definition of `LeaderboardPnlColumns` to locate the `address` column.
rg --type ts 'LeaderboardPnlColumns' -A 10

Length of output: 6601

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

264-264: Field update: Ensure consistency with other parts of the system.

The field subaccountId has been changed to address. Verify that all references to this interface use the updated field name.

Verification successful

Field update verified: Consistency maintained with the updated field name.

The field subaccountId has been successfully updated to address in the LeaderboardPnlFromDatabase interface, and all references in the codebase reflect this change.

  • indexer/packages/postgres/__tests__/stores/leaderboard-pnl-table.test.ts
  • indexer/packages/postgres/src/stores/leaderboard-pnl-table.ts
  • indexer/packages/postgres/src/types/db-model-types.ts
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all references to the `LeaderboardPnlFromDatabase` interface use the updated field name `address`.

# Test: Search for the interface usage. Expect: Only occurrences of the new field name `address`.
rg --type typescript -A 5 $'LeaderboardPnlFromDatabase'

Length of output: 89


Script:

#!/bin/bash
# Description: Verify that all references to the `LeaderboardPnlFromDatabase` interface use the updated field name `address`.

# Test: Search for the interface usage. Expect: Only occurrences of the new field name `address`.
rg --type ts -A 5 $'LeaderboardPnlFromDatabase'

Length of output: 5887

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

68-68: New constant: Ensure usage consistency.

A new constant defaultAddress2 has been added. Verify that this constant is used consistently and correctly throughout the codebase.


872-872: Field update: Ensure consistency with other parts of the system.

The field address in defaultLeaderboardPnl2OneDay has been updated to use defaultAddress2. Verify that all references to this constant use the updated field name.

Verification successful

Field update verified: Consistency maintained.

The address field in the defaultLeaderboardPnl2OneDay constant has been updated to use defaultAddress2. This update is consistent across the codebase as the address field is only defined within the constant and not explicitly referenced elsewhere.

  • indexer/packages/postgres/__tests__/helpers/constants.ts: The address field is correctly updated.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all references to the `defaultLeaderboardPnl2OneDay` constant use the updated field name `address`.

# Test: Search for the constant usage. Expect: Only occurrences of the new field name `address`.
rg --type typescript -A 5 $'defaultLeaderboardPnl2OneDay'

Length of output: 91


Script:

#!/bin/bash
# Description: Verify that all references to the `defaultLeaderboardPnl2OneDay` constant use the updated field name `address`.

# Test: Search for the constant usage. Expect: Only occurrences of the new field name `address`.
rg --glob '*.ts' --glob '*.tsx' -A 5 'defaultLeaderboardPnl2OneDay'

Length of output: 3565


Script:

#!/bin/bash
# Description: Verify that the `address` field in the `defaultLeaderboardPnl2OneDay` constant is consistently updated across all references.

# Test: Search for the usage of the `address` field within the `defaultLeaderboardPnl2OneDay` references.
rg --glob '*.ts' --glob '*.tsx' -A 5 'defaultLeaderboardPnl2OneDay' | rg 'address'

Length of output: 165


864-864: Field update: Ensure consistency with other parts of the system.

The field address in defaultLeaderboardPnlOneDay has been updated to use defaultAddress2. Verify that all references to this constant use the updated field name.

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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 466903c and 8f76dea.

Files selected for processing (1)
  • indexer/packages/postgres/src/models/leaderboard-pnl-model.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
  • indexer/packages/postgres/src/models/leaderboard-pnl-model.ts

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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8f76dea and 76333e7.

Files selected for processing (2)
  • indexer/packages/postgres/src/db/migrations/migration_files/20240717160024_create_leaderboard_pnl_table.ts (2 hunks)
  • indexer/packages/postgres/src/models/leaderboard-pnl-model.ts (3 hunks)
Files skipped from review as they are similar to previous changes (2)
  • indexer/packages/postgres/src/db/migrations/migration_files/20240717160024_create_leaderboard_pnl_table.ts
  • indexer/packages/postgres/src/models/leaderboard-pnl-model.ts

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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 76333e7 and 81f56e7.

Files selected for processing (2)
  • indexer/packages/postgres/tests/helpers/constants.ts (3 hunks)
  • indexer/packages/postgres/tests/stores/leaderboard-pnl-table.test.ts (3 hunks)
Files skipped from review as they are similar to previous changes (2)
  • indexer/packages/postgres/tests/helpers/constants.ts
  • indexer/packages/postgres/tests/stores/leaderboard-pnl-table.test.ts

@affanv14 affanv14 force-pushed the affan/leaderboard-fix branch from b3aa602 to 81f56e7 Compare July 19, 2024 02:27
@affanv14 affanv14 merged commit d725976 into main Jul 19, 2024
13 checks passed
@affanv14 affanv14 deleted the affan/leaderboard-fix branch July 19, 2024 02:39
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