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
This commit is contained in:
103
.github/copilot-instructions.md
vendored
103
.github/copilot-instructions.md
vendored
@@ -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):
|
||||
|
||||
Reference in New Issue
Block a user