From 9b280a40160ac5ad3d03a088297fcc217ca45ab8 Mon Sep 17 00:00:00 2001 From: mindesbunister Date: Thu, 20 Nov 2025 15:34:07 +0100 Subject: [PATCH] docs: Add Common Pitfall #57 - P&L calculation inaccuracy fix - Documented Nov 20, 2025 fix for 36% P&L calculation errors - External closure handler was using monitoring loop's stale currentPrice - Real incident: Database -$101.68 vs Drift -$138.35 actual - Fix: Query Drift's userAccount.perpPositions[].settledPnl directly - Fallback to calculation if Drift query fails - Updated #56 commit reference from [pending] to a3a6222 - Both fixes deployed Nov 20, 2025 15:25 CET --- .github/copilot-instructions.md | 103 +++++++++++++++++++++++++++++++- 1 file changed, 102 insertions(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index ed4d07e..5f52045 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -2940,8 +2940,109 @@ trade.realizedPnL += actualRealizedPnL // NOT: result.realizedPnL from SDK * Clean slate - no ghost orders left - **Why 32 orders:** Drift SDK's `userAccount.orders` array has 32 order slots, bot found SOL-PERP orders in many slots from current + previous trades - **Files changed:** `lib/trading/position-manager.ts` - Added order cancellation to external closure handler - - **Commit:** [pending] "critical: Cancel ghost orders after external closures" + - **Commit:** a3a6222 "critical: Cancel ghost orders after external closures" + - **Deployed:** Nov 20, 2025 15:25 CET - **Lesson:** When detecting external closures, always clean up ALL related on-chain state (orders, positions). Ghost orders are financial risks - they can execute when you're not watching. + +57. **P&L calculation inaccuracy for external closures (CRITICAL - Fixed Nov 20, 2025):** + - **Symptom:** Database P&L shows -$101.68 when Drift UI shows -$138.35 actual (36% error) + - **Root Cause:** External closure handler calculates P&L from monitoring loop's `currentPrice`, which lags behind actual fill price + - **Real incident (Nov 20, 13:30 CET):** + * SHORT stopped out at $142.48 + * Database calculated: -$101.68 (from monitoring price) + * Drift actual: -$138.35 (from actual fill price) + * Discrepancy: $36.67 (36% underreported) + * User proved NOT fees (screenshot showed $0.20 total, not $36) + - **Impact:** Every external closure (on-chain SL/TP fills) reports wrong P&L, can be 30-40% off actual + - **Why it happens:** + * Position Manager monitoring loop checks price every 2 seconds + * On-chain orders fill at specific price (instant) + * Monitoring loop detects closure seconds later + * Uses stale `currentPrice` from loop, not actual fill price + * Gap between fill and detection = calculation error + - **Fix (Nov 20, 2025):** + ```typescript + // In lib/trading/position-manager.ts external closure handler (lines 854-900): + + // BEFORE (BROKEN): + let runnerRealized = 0 + let runnerProfitPercent = 0 + if (!wasPhantom) { + runnerProfitPercent = this.calculateProfitPercent( + trade.entryPrice, + currentPrice, // ← BUG: Monitoring loop price, not actual fill + trade.direction + ) + runnerRealized = (sizeForPnL * runnerProfitPercent) / 100 + } + const totalRealizedPnL = runnerRealized + + // AFTER (FIXED): + // CRITICAL FIX (Nov 20, 2025): Query Drift's ACTUAL P&L instead of calculating + let totalRealizedPnL = 0 + let runnerProfitPercent = 0 + + if (!wasPhantom) { + // Query Drift's settled P&L (source of truth) + const driftService = await initializeDriftService() + const marketConfig = getMarketConfig(trade.symbol) + + try { + const userAccount = driftService.getClient().getUserAccount() + if (userAccount) { + // Find perpPosition for this market + const position = userAccount.perpPositions.find((p: any) => + p.marketIndex === marketConfig.driftMarketIndex + ) + + if (position) { + // Use Drift's settled P&L (authoritative) + const settledPnL = Number(position.settledPnl || 0) / 1e6 // Convert to USD + if (Math.abs(settledPnL) > 0.01) { + totalRealizedPnL = settledPnL + runnerProfitPercent = (totalRealizedPnL / sizeForPnL) * 100 + console.log(` ✅ Using Drift's actual P&L: $${totalRealizedPnL.toFixed(2)} (settled)`) + } + } + } + } catch (driftError) { + console.error('⚠️ Failed to query Drift P&L, falling back to calculation:', driftError) + } + + // Fallback: Calculate from price if Drift query fails + if (totalRealizedPnL === 0) { + runnerProfitPercent = this.calculateProfitPercent( + trade.entryPrice, + currentPrice, + trade.direction + ) + totalRealizedPnL = (sizeForPnL * runnerProfitPercent) / 100 + console.log(` ⚠️ Using calculated P&L (fallback): ${runnerProfitPercent.toFixed(2)}% on $${sizeForPnL.toFixed(2)} = $${totalRealizedPnL.toFixed(2)}`) + } + } else { + console.log(` Phantom trade P&L: $0.00`) + } + ``` + - **Import change (line 7):** + ```typescript + // BEFORE: + import { getDriftService } from '../drift/client' + + // AFTER: + import { getDriftService, initializeDriftService } from '../drift/client' + ``` + - **Behavior now:** + * External closure detected → Initialize Drift service + * Query userAccount.perpPositions for matching marketIndex + * Extract settledPnl field (Drift's authoritative P&L) + * Convert micro-units to USD (divide by 1e6) + * If settledPnl > $0.01: Use it, log "✅ Using Drift's actual P&L" + * If unavailable: Calculate from price, log "⚠️ Using calculated P&L (fallback)" + * Database gets accurate P&L matching Drift UI + - **Files changed:** `lib/trading/position-manager.ts` (import + external closure P&L calculation) + - **Commit:** 8e600c8 "critical: Fix P&L calculation to use Drift's actual settledPnl" + - **Deployed:** Nov 20, 2025 15:25 CET + - **Lesson:** When blockchain/exchange has authoritative data (settledPnL), query it directly instead of calculating. Timing matters - monitoring loop price ≠ actual fill price. For financial data, always prefer source of truth over derived calculations. ```typescript // In app/api/settings/route.ts (lines ~150, ~270) // BEFORE (BROKEN):