Fix missing base leg USD value calculation in neutral-trade drift util#73
Fix missing base leg USD value calculation in neutral-trade drift util#73
Conversation
Co-authored-by: zknpr <96851588+zknpr@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The adapter at projects/neutral-trade exports TVL: |
Greptile SummaryThis PR implements the previously-skipped TODO in Key changes:
Issues found:
Confidence Score: 2/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["getTvl(api, driftVaultAddresses)"] --> B["Fetch & deserialize user positions"]
B --> C["Batch-fetch PerpMarket accounts → perpAccountMap"]
C --> D["For each perpPosition"]
D --> E{"baseTokenMint exists?"}
E -- "Yes (e.g. SOL-PERP)" --> F["api.add(baseTokenMint, baseBalance)"]
E -- "No (e.g. HYPE-PERP)" --> G["Read perpAccount.data.readBigInt64LE(72)"]
G --> H["usdValue = base_asset_amount × price / 1e9"]
H --> I["api.add(USDC mint, usdValue)"]
F --> J["api.add(USDC mint, quoteBalance)"]
I --> J
J --> K["Compute funding rate PnL"]
K --> L["api.add(USDC mint, fundingRatePnl)"]
style G fill:#ffcccc,stroke:#cc0000
style H fill:#ffcccc,stroke:#cc0000
G -.->|"⚠️ Offset 72 = last_oracle_price_twap_5min\n(live price is at offset 40)"| G
Last reviewed commit: afcf873 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const price = perpAccount.data.readBigInt64LE(72); | ||
| const usdValue = (position.base_asset_amount * price) / BigInt(10 ** 9); |
There was a problem hiding this comment.
Wrong buffer offset reads 5-minute TWAP, not live oracle price
Based on the Drift v2 PerpMarket account layout, the AMM struct begins right after the 8-byte Anchor discriminator, with the following field order:
| Global Offset | Field |
|---|---|
| 8 | AMM.oracle (Pubkey, 32 bytes) |
| 40 | HistoricalOracleData.last_oracle_price ← live oracle price |
| 48 | HistoricalOracleData.last_oracle_conf |
| 56 | HistoricalOracleData.last_oracle_delay |
| 64 | HistoricalOracleData.last_oracle_price_twap (1-hour TWAP) |
| 72 | HistoricalOracleData.last_oracle_price_twap_5min ← what the code reads |
| 80 | HistoricalOracleData.last_oracle_price_twap_ts |
Reading at offset 72 picks up last_oracle_price_twap_5min (the 5-minute time-weighted average price), not the live oracle price described in the PR. During periods of high volatility — exactly the times when HYPE-PERP is most active — this TWAP can diverge significantly from the actual price and will cause the TVL to be measurably incorrect.
The live oracle price is at offset 40. If a TWAP is intentionally preferred for stability, that should be documented with a comment.
| const price = perpAccount.data.readBigInt64LE(72); | |
| const usdValue = (position.base_asset_amount * price) / BigInt(10 ** 9); | |
| const price = perpAccount.data.readBigInt64LE(40); // AMM.historical_oracle_data.last_oracle_price (offset = 8 discriminator + 32 oracle pubkey) | |
| const usdValue = (position.base_asset_amount * price) / BigInt(10 ** 9); |
| const perpAccount = perpAccountMap[position.market_index]; | ||
| if (perpAccount && perpAccount.data) { | ||
| const price = perpAccount.data.readBigInt64LE(72); | ||
| const usdValue = (position.base_asset_amount * price) / BigInt(10 ** 9); | ||
| api.add(getTokenMintFromMarketIndex(0), usdValue); | ||
| } |
There was a problem hiding this comment.
Magic numbers with no derivation comments
Unlike getPerpMarketFundingRates, which documents its offset as:
const CUMULATIVE_FUNDING_OFFSET = 8 + 48 + 32 + 256 + (16 * 15) + 24;the new code uses bare magic numbers 72 and 10 ** 9 without any comment explaining what struct field is at that offset or what precision constant is being applied. This makes it very hard to verify correctness or maintain in the future.
Consider adding an explanatory constant and comment, similar to the existing pattern:
// Drift v2 PerpMarket layout: 8 (discriminator) + 32 (AMM.oracle pubkey) = 40
const ORACLE_PRICE_OFFSET = 40; // AMM.historical_oracle_data.last_oracle_price
// base_asset_amount is in BASE_PRECISION (1e9), price is in PRICE_PRECISION (1e6)
// USD value in USDC raw (6 decimals) = (base/1e9) * (price/1e6) * 1e6 = base * price / 1e9
const price = perpAccount.data.readBigInt64LE(ORACLE_PRICE_OFFSET);
const usdValue = (position.base_asset_amount * price) / BigInt(10 ** 9);2 participants
Implemented the TODO in
projects/neutral-trade/utils/drift.jsto find the USD value for base tokens without an SPL spot mint (likeHYPE-PERP). It now extracts the oracle price from thePerpMarketaccount buffer at offset 72, adjusts for precision, and adds the value as USDC to the API object.PR created automatically by Jules for task 11210373544589687715 started by @zknpr