[TRA-105] Add API for parent subaccount perpetual positions #1282
Conversation
Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange>
Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange>
WalkthroughThe updates involve enhancing functionality and data representation within an indexer system, focusing on perpetual positions and subaccount management. Key changes include adjusting the representation of entry and oracle prices, introducing the Changes
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
indexer/services/comlink/public/swagger.jsonis excluded by!**/*.json
Files selected for processing (8)
- indexer/packages/postgres/tests/helpers/constants.ts (2 hunks)
- indexer/services/comlink/tests/controllers/api/v4/addresses-controller.test.ts (3 hunks)
- indexer/services/comlink/tests/controllers/api/v4/perpetual-positions-controller.test.ts (5 hunks)
- indexer/services/comlink/public/api-documentation.md (18 hunks)
- indexer/services/comlink/src/controllers/api/v4/addresses-controller.ts (1 hunks)
- indexer/services/comlink/src/controllers/api/v4/perpetual-positions-controller.ts (4 hunks)
- indexer/services/comlink/src/request-helpers/request-transformer.ts (2 hunks)
- indexer/services/comlink/src/types.ts (2 hunks)
Additional Context Used
Path-based Instructions (8)
indexer/services/comlink/src/types.ts (1)
**/**:
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/src/controllers/api/v4/perpetual-positions-controller.ts (1)
**/**:
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/src/controllers/api/v4/addresses-controller.ts (1)
**/**:
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/src/request-helpers/request-transformer.ts (1)
**/**:
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/__tests__/controllers/api/v4/perpetual-positions-controller.test.ts (1)
**/**:
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/__tests__/controllers/api/v4/addresses-controller.test.ts (1)
**/**:
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/__tests__/helpers/constants.ts (1)
**/**:
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/api-documentation.md (1)
**/**:
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 (29)
indexer/services/comlink/src/types.ts (2)
103-103: The addition of thesubaccountNumberfield to thePerpetualPositionResponseObjectinterface aligns with the PR's objective to enhance subaccount management. Ensure that all usages of this interface are updated to handle this new field appropriately.
375-378: The introduction of theParentSubaccountPerpetualPositionRequestinterface, extendingParentSubaccountRequestandLimitAndCreatedBeforeRequestwith an additionalstatusfield, is a logical extension for handling parent subaccount perpetual position requests. This design maintains consistency with existing request structures and supports the new functionality well.indexer/services/comlink/src/controllers/api/v4/perpetual-positions-controller.ts (5)
39-39: The import ofgetChildSubaccountNumsis essential for the new functionality allowing the retrieval of child subaccount numbers for a given parent subaccount. This utility will likely play a critical role in thelistPositionsForParentSubaccountmethod.
43-43: The import ofCheckParentSubaccountSchemais necessary for validating requests to the newlistPositionsForParentSubaccountendpoint. It's good practice to ensure that incoming requests meet expected schema requirements.
160-273: ThelistPositionsForParentSubaccountmethod is a significant addition, enabling the retrieval of perpetual positions for parent subaccounts. This method correctly utilizesgetChildSubaccountNumsto fetch child subaccount numbers and then queries for positions associated with these subaccounts. It's well-structured and includes error handling for cases where subaccounts or the latest block are not found. Ensure that the method's logic aligns with the expected behavior and that all possible error conditions are considered.
279-295: TheadjustPerpetualPositionsWithUpdatedFundingfunction is a crucial addition for updating perpetual positions with the latest funding information. This function seems to be correctly implemented, leveraginggetFundingIndexMapsandgetPerpetualPositionsWithUpdatedFundingto adjust positions. It's important to ensure that this function is called appropriately and that its results are handled correctly in the context of parent subaccount management.
150-156: The update to thelistPositionsmethod to include thesubaccountNumberparameter is a necessary change to support the new functionality. This update allows for the retrieval of positions specific to a subaccount, enhancing the granularity of position management. Ensure that all calls to this method are updated to provide thesubaccountNumberparameter.indexer/services/comlink/src/controllers/api/v4/addresses-controller.ts (1)
512-512: The addition of thesubaccount.subaccountNumberparameter to thegetSubaccountResponsefunction call is a necessary update to include thesubaccountNumberin the response objects. This change aligns with the PR's objectives and enhances the detail provided in API responses related to subaccounts. Ensure that thegetSubaccountResponsefunction is updated accordingly to handle this new parameter.indexer/services/comlink/src/request-helpers/request-transformer.ts (3)
76-76: The addition ofsubaccountNumberas a parameter to theperpetualPositionToResponseObjectfunction is a significant change. It ensures that the response object now includes thesubaccountNumber, providing more detailed information about the position's ownership. This change aligns with the PR's objective to enhance subaccount management capabilities.
108-108: IncludingsubaccountNumberdirectly in the return object ofperpetualPositionToResponseObjectwithout any checks or transformations is straightforward and effective. This approach assumes that thesubaccountNumberis always valid and provided, which should be the case given the context of its usage.
108-108: Ensure that thesubaccountNumberis always a valid, non-negative integer and is appropriately validated upstream before being passed to this function. This is crucial for maintaining data integrity and avoiding potential issues with invalid subaccount numbers.indexer/services/comlink/__tests__/controllers/api/v4/perpetual-positions-controller.test.ts (6)
92-92: AddingsubaccountNumberto the expected response object in the test for getting long positions is consistent with the changes made to the actual implementation. This ensures that the tests accurately reflect the new functionality.
144-144: IncludingsubaccountNumberin the expected response for short positions is appropriate and aligns with the implementation changes. It's good to see that tests are updated to reflect the new structure of response objects.
194-194: The inclusion ofsubaccountNumberin the test for retrieving CLOSED positions without adjusting funding is correct and ensures that the tests are comprehensive and cover the new functionality thoroughly.
305-305: The addition ofsubaccountNumberin the test for getting long/short positions across subaccounts is crucial for validating the new feature's functionality. This ensures that the feature works as expected and that the response structure is correctly implemented.
336-336: IncludingsubaccountNumberin the expected response for the isolated position in the test for getting positions across subaccounts is necessary for completeness and accuracy of the test. It verifies that the feature handles various scenarios correctly.
396-396: The presence ofsubaccountNumberin the expected response for CLOSED positions in the test for getting positions across subaccounts is appropriate. It ensures that the tests are aligned with the implementation and that the feature's behavior is correctly verified.indexer/services/comlink/__tests__/controllers/api/v4/addresses-controller.test.ts (3)
109-109: IncludingsubaccountNumberin the expected response object for the test that retrieves subaccount information is consistent with the changes made to the actual implementation. This ensures that the tests accurately reflect the new functionality.
270-270: The addition ofsubaccountNumberin the expected response for the test that retrieves all subaccounts is appropriate and aligns with the implementation changes. It's good to see that tests are updated to reflect the new structure of response objects.
458-458: IncludingsubaccountNumberin the expected response for the test that retrieves all subaccounts for a provided parent is crucial for validating the new feature's functionality. This ensures that the feature works as expected and that the response structure is correctly implemented.indexer/packages/postgres/__tests__/helpers/constants.ts (2)
465-465: The change inentryPricefrom'20000'to'1.5'forisolatedPerpetualPositionseems to align with the PR's goal of adjusting constants to more realistic values. Ensure that this change does not negatively impact other tests that rely on this constant.
640-640: The adjustment oforaclePricefrom'0.000000075'to'1.00'forisolatedMarketappears to be in line with the objective of using more realistic values for testing. It's important to verify that this change is consistent with the intended testing scenarios and does not inadvertently affect other tests.indexer/services/comlink/public/api-documentation.md (7)
88-89: The addition of thesubaccountNumberfield to theopenPerpetualPositionsobject in the API response is consistent with the PR objectives. This change enhances the granularity of the data provided, allowing for better management and tracking of perpetual positions across subaccounts.
106-107: The consistent addition of thesubaccountNumberfield across different properties within theopenPerpetualPositionsobject ensures uniformity in the API response structure. This is a positive change that aligns with the goal of improving subaccount management.
216-217: Including thesubaccountNumberin theopenPerpetualPositionsobject for theGET /addresses/{address}/subaccountNumber/{subaccountNumber}endpoint is a valuable addition, providing clarity on the subaccount associated with each position.
234-235: The addition of thesubaccountNumberfield here is consistent with the changes made in other parts of the documentation, ensuring that the API responses provide detailed information about subaccounts for better management.
347-348: Adding thesubaccountNumberto theopenPerpetualPositionsobject in theGET /addresses/{address}/parentSubaccountNumber/{parentSubaccountNumber}endpoint response is a thoughtful change that enhances the API's functionality by providing more detailed information about subaccount positions.
365-366: The inclusion of thesubaccountNumberfield in this part of the API documentation is consistent with the overall goal of enhancing subaccount management. This change will help users better understand the distribution of positions across subaccounts.
1847-1849: The addition of thesubaccountNumberfield to the perpetual positions API response is a significant enhancement. It aligns with the PR's objectives to improve the management and visibility of perpetual positions across parent and subaccounts. This change will likely be appreciated by users who need to manage multiple subaccounts.
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
indexer/services/comlink/public/swagger.jsonis excluded by!**/*.json
Files selected for processing (3)
- indexer/packages/postgres/tests/helpers/constants.ts (2 hunks)
- indexer/services/comlink/public/api-documentation.md (1 hunks)
- indexer/services/comlink/src/types.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- indexer/packages/postgres/tests/helpers/constants.ts
- indexer/services/comlink/src/types.ts
Additional Context Used
Path-based Instructions (1)
indexer/services/comlink/public/api-documentation.md (1)
**/**:
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/services/comlink/public/api-documentation.md (1)
1866-1965: The addition of thesubaccountNumberfield to theListPositionsForParentSubaccountendpoint and its example response is consistent with the PR objectives. This change enhances the platform's ability to manage perpetual positions across parent and subaccounts, providing more granular control and visibility. The documentation is clear and accurately reflects the new functionality introduced.
indexer/services/comlink/src/controllers/api/v4/perpetual-positions-controller.ts
Show resolved
Hide resolved
indexer/services/comlink/src/controllers/api/v4/perpetual-positions-controller.ts
Show resolved
Hide resolved
| if (subaccounts.length === 0 || latestBlock === undefined) { | ||
| throw new NotFoundError( | ||
| `Found OPEN perpetual positions but no subaccounts with address ${address} ` + | ||
| `and parent Subaccount number ${parentSubaccountNumber}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
I don't think this is possible, because we have a foreign key on the positions table for the subaccount to exist
There was a problem hiding this comment.
I agree it shouldn't be possible based on that constraint. Also brought over from the original API. Should I remove it from there too?
* Add subaccountNumber to PerpetualPositionResponseObject (#1274) Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange> * [CT-712] send updates for both normal order matches and liquidation (#1280) * Remove volatile market (#1263) * [SKI-21] Bump slinky version to v0.3.1 (#1275) * Bump slinky version * Add cp number method * clean up socks logging (#1285) * [CT-681] fix liquidated side and offsetting side for indexer delevera… (#1284) * [CT-681] fix liquidated side and offsetting side for indexer deleveraging events * fix test * fix test * fix lint * [TRA-105] Add API for parent subaccount perpetual positions (#1282) Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange> * Use sample rate with stream destroyed stats. (#1294) * Revert "[CT-708] Indexer track e2e latency (#1237)" (#1292) This reverts commit 60b94df. * Fix swagger generation makefile command / regen swagger docs (#1299) * pull dydx fork to generate swagger properly * remove the print * remove vault constants (#1293) * Remove custom ping message from socks (#1301) * Add subaccountNumber to the OrderResponseObject (#1296) Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange> * sample more metrics (#1304) * [OTE-256] Add upgrade handler to initialize OI during upgrade handler (#1302) * Add upgrade handler to initialize OI during upgrade handler * nits * Fix lib.ErrorLogWithError: properly pass in args (#1306) * fix broken tests (#1312) * Explicitly close websockets on errors (#1290) * Increase the number of allowed connections to 8000 (#1317) * [TRA-104] Add parentSubaccountNumber API for orders (#1313) Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange> * Improve Slinky logs to prevent unnecessary logs (#1289) * [SKI-26]: Prevent funding index update with no oracle prices from (#1321) halting indexer * Skip equity tier limit check in PlaceShortTermOrder (#1318) * Skip equity tier limit check in PlaceShortTermOrder * remove tests * Add comment * fix lint (#1323) --------- Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange> Co-authored-by: shrenujb <98204323+shrenujb@users.noreply.github.com> Co-authored-by: jayy04 <103467857+jayy04@users.noreply.github.com> Co-authored-by: Eric Warehime <eric.warehime@gmail.com> Co-authored-by: vincentwschau <99756290+vincentwschau@users.noreply.github.com> Co-authored-by: Jonathan Fung <121899091+jonfung-dydx@users.noreply.github.com> Co-authored-by: Tian <tian@dydx.exchange> Co-authored-by: Teddy Ding <teddy@dydx.exchange> Co-authored-by: roy-dydx <133032749+roy-dydx@users.noreply.github.com> Co-authored-by: Christopher-Li <Christopher-Li@users.noreply.github.com>
Changelist
Add API to get perpetual positions by parent subaccount number
Test Plan
Added tests
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.