Add cross margin liquidation notification helper function#102
Add cross margin liquidation notification helper function#102
Conversation
WalkthroughThis pull request updates the package version from "2.2.13" to "2.2.15" and modifies the Subgraph class by replacing the method Changes
Suggested reviewers
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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: 1
🧹 Nitpick comments (3)
src/subgraph/positions/subgraphQueries.ts (1)
392-404: New query for fetching cross-margin liquidated positions.The query is well-structured for its specific purpose: fetching liquidated positions for a cross-margin account. However, it only retrieves minimal data (accountId, totalAmountLiquidated, and lastRefresh) compared to other position queries in this file that fetch comprehensive position details.
Consider enhancing the query to fetch more comprehensive position data if needed for notifications, such as liquidationTxHash, positionSize, etc., similar to other position queries in this file.
src/subgraph/index.ts (1)
234-241: New method for retrieving cross-margin liquidated positions.This method extends the Subgraph class with the ability to retrieve cross-margin positions that have been liquidated after a specified timestamp.
The comment "Get open positions by user address" is inaccurate. It should be updated to reflect the actual purpose of retrieving liquidated cross-margin positions. Suggest changing to:
- // Get open positions by user address + // Get liquidated cross-margin positions by SNX account ID and last refresh timestampsrc/subgraph/positions/index.ts (1)
195-216: New function to fetch cross-margin liquidated positions.The function follows the established pattern in the codebase for fetching positions, including proper error handling and response mapping.
There's significant code duplication between this function and
getUserPositionsBySnxAccount(lines 177-193). Consider extracting the common logic into a reusable helper function to improve maintainability:+ // Helper function to format SNX account ID + const formatSnxAccountId = (snxAccountId: string): string => { + return !snxAccountId.includes('PERP') ? 'PERP-'.concat(snxAccountId) : snxAccountId; + }; + + // Generic function to fetch SNX account data with error handling + const fetchSnxAccountData = async ( + subgraphEndpoint: string, + query: string, + ): Promise<SnxAccount | undefined> => { + try { + const subgraphResponse: any = await request(subgraphEndpoint, query); + return mapResponseToSnxAccount(subgraphResponse?.snxAccount); + } catch (error) { + console.error('Error fetching positions:', error); + return undefined; + } + }; // Get positions by SNX Account id export const getUserPositionsBySnxAccount = async ( subgraphEndpoint: string, snxAccountId: string, ): Promise<SnxAccount | undefined> => { - let formattedSnxAccountId = snxAccountId; - try { - if (!snxAccountId.includes('PERP')) { - formattedSnxAccountId = 'PERP-'.concat(snxAccountId); - } - const subgraphResponse: any = await request(subgraphEndpoint, fetchPositionsBySnxAccount(formattedSnxAccountId)); - const snxAccount = mapResponseToSnxAccount(subgraphResponse?.snxAccount); - return snxAccount; - } catch (error) { - console.error('Error fetching positions:', error); - return undefined; - } + const formattedSnxAccountId = formatSnxAccountId(snxAccountId); + return fetchSnxAccountData(subgraphEndpoint, fetchPositionsBySnxAccount(formattedSnxAccountId)); }; // Get liquidated cross-margin positions by SNX Account id export const getCrossMarginPositionsBySnxAccount = async ( subgraphEndpoint: string, snxAccountId: string, lastRefresh: number, ): Promise<SnxAccount | undefined> => { - let formattedSnxAccountId = snxAccountId; - try { - if (!snxAccountId.includes('PERP')) { - formattedSnxAccountId = 'PERP-'.concat(snxAccountId); - } - const subgraphResponse: any = await request( - subgraphEndpoint, - fetchCrossMarginPositionsBySnxAccount(formattedSnxAccountId, lastRefresh), - ); - const snxAccount = mapResponseToSnxAccount(subgraphResponse?.snxAccount); - return snxAccount; - } catch (error) { - console.error('Error fetching positions:', error); - return undefined; - } + const formattedSnxAccountId = formatSnxAccountId(snxAccountId); + return fetchSnxAccountData( + subgraphEndpoint, + fetchCrossMarginPositionsBySnxAccount(formattedSnxAccountId, lastRefresh) + ); };Update the comment to be more specific about the function's purpose:
- // Get positions by SNX Account id + // Get liquidated cross-margin positions by SNX Account id and lastRefresh timestamp
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.json(1 hunks)src/subgraph/index.ts(2 hunks)src/subgraph/positions/index.ts(2 hunks)src/subgraph/positions/subgraphQueries.ts(1 hunks)test/subgraph-tests/position.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/subgraph/positions/index.ts (4)
src/subgraph/index.ts (1)
getCrossMarginPositionsBySnxAccount(235-241)src/interfaces/sdkTypes.ts (1)
SnxAccount(17-37)src/subgraph/positions/subgraphQueries.ts (1)
fetchCrossMarginPositionsBySnxAccount(392-404)src/common/subgraphMapper.ts (1)
mapResponseToSnxAccount(37-68)
test/subgraph-tests/position.test.ts (1)
test/index.ts (1)
getParifiSdkInstanceForTesting(5-33)
src/subgraph/index.ts (2)
src/interfaces/sdkTypes.ts (1)
SnxAccount(17-37)src/subgraph/positions/index.ts (1)
getCrossMarginPositionsBySnxAccount(196-216)
🪛 Biome (1.9.4)
test/subgraph-tests/position.test.ts
[error] 56-56: Don't focus the test.
The 'only' method is often used for debugging or during implementation. It should be removed before deploying to production.
Consider removing 'only' to ensure all tests are executed.
Unsafe fix: Remove focus from test.
(lint/suspicious/noFocusedTests)
🔇 Additional comments (4)
package.json (1)
3-3: Version bump for new functionality.The package version has been incremented from 2.2.13 to 2.2.14, which correctly follows semantic versioning for the addition of the cross-margin liquidation notification helper function.
src/subgraph/index.ts (1)
27-27: Import for the new cross-margin positions function.The import is correctly added to support the new functionality.
src/subgraph/positions/index.ts (1)
5-5: Import for new GraphQL query function.Correctly added the import for the new GraphQL query function.
test/subgraph-tests/position.test.ts (1)
47-47: The exclusive test marker has been properly removed.Good change. Removing the
.onlyensures this test will run along with other tests in the suite, which is necessary for comprehensive test coverage.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test/subgraph-tests/position.test.ts (1)
37-37:⚠️ Potential issueRemove the
.onlytest marker before merging to production.The
.onlymethod is used for focusing on a specific test during development, but it should be removed before merging to ensure all tests in the suite are executed.-it.only('should return positions for a snx account id', async () => { +it('should return positions for a snx account id', async () => {🧰 Tools
🪛 Biome (1.9.4)
[error] 37-37: Don't focus the test.
The 'only' method is often used for debugging or during implementation. It should be removed before deploying to production.
Consider removing 'only' to ensure all tests are executed.
Unsafe fix: Remove focus from test.(lint/suspicious/noFocusedTests)
🧹 Nitpick comments (3)
src/subgraph/index.ts (2)
35-35: Remove import from test directoryThe import statement from a test directory into source code is unconventional. The imported
statusis not used in this file.-import { status } from '../../test/common/constants';
228-235: Update function comment to match its purposeThe comment above this function incorrectly states "Get open positions by user address" but the function actually retrieves liquidated positions by SNX account.
- // Get open positions by user address + // Get liquidated positions by SNX account public async getUserLiquidatedPositionsBySnxAccount( snxAccountId: string, lastRefresh: number, ): Promise<SnxAccount | undefined> {test/subgraph-tests/position.test.ts (1)
40-47: Test structure could be improvedThe test wraps the actual test code in an unnecessary code block. Consider simplifying the structure:
it('should return positions for a snx account id', async () => { const parifiSdk = await getParifiSdkInstanceForTesting(); - { - // It should return position data just with the snx account id const snxAccountId = '12868688093503149326'; const lastRefresh = 1741189060; const positionData = await parifiSdk.subgraph.getUserLiquidatedPositionsBySnxAccount(snxAccountId, lastRefresh); // Add assertions to verify liquidated positions data expect(positionData).toBeDefined(); expect(positionData?.id).toEqual(`PERP-${snxAccountId}`); expect(positionData?.positions?.length).toEqual(1); - } });🧰 Tools
🪛 GitHub Actions: Test contracts sdk
[error] 44-44: GraphQL Error (Code: 403): Error fetching positions: ClientError: GraphQL Error (Code: 403)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.json(1 hunks)src/subgraph/index.ts(2 hunks)src/subgraph/positions/index.ts(2 hunks)src/subgraph/positions/subgraphQueries.ts(3 hunks)test/subgraph-tests/position.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- src/subgraph/positions/subgraphQueries.ts
- src/subgraph/positions/index.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/subgraph/index.ts (2)
src/interfaces/sdkTypes.ts (1)
SnxAccount(17-37)src/subgraph/positions/index.ts (1)
getUserLiquidatedPositionsBySnxAccount(176-196)
test/subgraph-tests/position.test.ts (1)
test/index.ts (1)
getParifiSdkInstanceForTesting(5-33)
🪛 Biome (1.9.4)
test/subgraph-tests/position.test.ts
[error] 37-37: Don't focus the test.
The 'only' method is often used for debugging or during implementation. It should be removed before deploying to production.
Consider removing 'only' to ensure all tests are executed.
Unsafe fix: Remove focus from test.
(lint/suspicious/noFocusedTests)
🪛 GitHub Actions: Test contracts sdk
test/subgraph-tests/position.test.ts
[error] 44-44: GraphQL Error (Code: 403): Error fetching positions: ClientError: GraphQL Error (Code: 403)
🔇 Additional comments (2)
src/subgraph/index.ts (1)
229-234: Function implementation looks goodThe implementation correctly retrieves liquidated positions for a specific SNX account ID with the given
lastRefreshparameter, matching the interface defined in the positions module.test/subgraph-tests/position.test.ts (1)
28-28: Good: Removed test focus from closed positions testRemoving
.onlyfrom this test is the right approach to ensure all tests run in the suite.
2 participants
Summary by CodeRabbit
Chores
New Features
Tests