Add function to calculate liquidation price offchain [PFT-2474]#99
Add function to calculate liquidation price offchain [PFT-2474]#99
Conversation
WalkthroughThe pull request introduces a new method Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Perps
participant Offchain
Client->>Perps: calculateLiquidationPriceOffchain(params)
Perps->>Offchain: calculateLiquidationPriceOffchain(params)
Offchain-->>Perps: return calculated liquidationPrice
Perps-->>Client: return liquidationPrice
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
🪧 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: 0
🧹 Nitpick comments (1)
src/perps/offchain.ts (1)
82-84: Consider using strict equality comparison.The code currently uses
==for comparison, but it's generally a best practice in TypeScript/JavaScript to use===for strict equality comparison to avoid unexpected type coercions.- if (formattedPositionSize == 0) return formattedMarketPrice; - if (availableMarginInUsd == 0) return 0; + if (formattedPositionSize === 0) return formattedMarketPrice; + if (availableMarginInUsd === 0) return 0;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/perps/index.ts(2 hunks)src/perps/offchain.ts(1 hunks)test/perps/offchain.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
src/perps/index.ts (1)
src/perps/offchain.ts (1)
calculateLiquidationPriceOffchain(71-96)
test/perps/offchain.test.ts (1)
src/perps/offchain.ts (1)
calculateLiquidationPriceOffchain(71-96)
🔇 Additional comments (8)
src/perps/index.ts (2)
11-11: LGTM: Import added correctly.The import for
calculateLiquidationPriceOffchainhas been correctly added to the imports from './offchain'.
82-100: LGTM: Well-structured method implementation.The implementation follows the established pattern in the class, serving as a clean wrapper around the imported function. The method has a proper comment and clearly passes all parameters to the underlying function.
test/perps/offchain.test.ts (4)
5-13: LGTM: Edge case well tested.This test correctly verifies that when position size is 0, the function returns the market price (2000), which is the expected behavior for this edge case.
15-23: LGTM: Correct calculation for long position.The test correctly verifies the liquidation price calculation for a long position:
- lossPerToken = Math.abs((20 - 100) / 1) = 80
- liquidationPrice = 2000 - 80 = 1920
Using toBeCloseTo() is appropriate for floating-point comparisons.
35-43: LGTM: Edge case well tested.This test correctly verifies that when available margin is 0, the function returns 0, which is the expected behavior for this edge case.
25-33:Details
❓ Verification inconclusive
Verify the calculation for short positions.
The test logic seems correct mathematically:
- lossPerToken = Math.abs((20 - 1000) / -1) = 980
- liquidationPrice = 2000 + 980 = 2980
However, please verify if the business logic is correct. Is it expected that when marginBufferInUsd (20) is much less than availableMarginInUsd (1000), the liquidation price would be significantly higher than the market price for a short position?
🏁 Script executed:
#!/bin/bash # Search for other instances of liquidation price calculations for short positions rg -A 5 -B 5 "liquidation.*price.*short" --type tsLength of output: 744
Subject: Verify Consistency of Short Position Liquidation Price Calculation
The current test snippet calculates the liquidation price for a short position as 2980—with the math showing a loss per token of 980 added to the market price (2000 + 980). However, another context within the same file expects a liquidation price of 1920. Please verify whether the business logic should yield 2980 or 1920 for short positions and update the tests accordingly.
- Confirm the intended formula for calculating the liquidation price for short positions.
- Ensure that all tests in
test/perps/offchain.test.tsconsistently reflect the correct expected value.src/perps/offchain.ts (2)
63-70: LGTM: Well-documented function.The JSDoc comments are clear and descriptive, providing good documentation for the function parameters and return value.
86-93:Details
❓ Verification inconclusive
Verify the liquidation price calculation logic.
The calculation for
lossPerTokenuses the formula:
Math.abs((marginBufferInUsd - availableMarginInUsd) / formattedPositionSize)For a short position where marginBufferInUsd is less than availableMarginInUsd (as in the test), this will result in a large positive number, making the liquidation price significantly higher than the market price.
Please verify this aligns with the expected business logic for liquidation prices for short positions.
🏁 Script executed:
#!/bin/bash # Search for other implementations or documentation of liquidation price calculations rg "liquidation.*price" --type ts -A 5 -B 5Length of output: 2827
Liquidation Price Calculation Logic – Please Confirm Business Logic Alignment
The current implementation calculates the loss per token as follows:
const lossPerToken = Math.abs((marginBufferInUsd - availableMarginInUsd) / formattedPositionSize); let liquidationPrice; if (formattedPositionSize > 0) { liquidationPrice = formattedMarketPrice - lossPerToken; } else { liquidationPrice = formattedMarketPrice + lossPerToken; }Our investigation found that the tests in
test/perps/offchain.test.tsverify the logic for long positions—for example, with inputs(availableMarginInUsd: 100, marginBufferInUsd: 20, formattedPositionSize: 1, formattedMarketPrice: 2000), the liquidation price comes out to be approximately 1920. For short positions, the same formula (when applied with, say,(availableMarginInUsd: 1000, marginBufferInUsd: 20, formattedPositionSize: -1, formattedMarketPrice: 2000)) produces a liquidation price significantly higher than the market price (i.e. 2000 + 980 = 2980).Please confirm that this behavior—subtracting the loss per token from the market price for long positions and adding it for short positions—aligns with the intended business logic, especially given that a short position with a smaller margin buffer than available margin results in a much higher liquidation threshold.
2 participants
Summary by CodeRabbit