docs: Add pitfall #94 - P&L using oracle price instead of actual fill price

- Documented 5.7% P&L over-reporting bug (Drift $133.03 vs Database $140.56) - Root cause: closePosition() used oracle price instead of actual fill price - Fix: getActualFillPriceFromTx() helper extracts real fills from transaction logs - WrappedEvent has properties directly on object, NOT nested under .data - References commit c1bff0d (Jan 11, 2026)
2026-01-11 21:10:09 +01:00
parent c1bff0d0f4
commit 2af13eaa88
1 changed files with 76 additions and 1 deletions
--- a/docs/COMMON_PITFALLS.md
+++ b/docs/COMMON_PITFALLS.md
@@ -103,6 +103,7 @@ This document is the **comprehensive reference** for all documented pitfalls, bu
 | 91 | 🔴 CRITICAL | Orders | Jan 1, 2026 | Math.floor truncation leaves fractional position remnants |
 | 92 | 🔴 CRITICAL | Orders | Jan 6, 2026 | Exit order token sizing mismatch - TP/SL different sizes than position |
 | 93 | 🔴 CRITICAL | Data Integrity | Jan 6, 2026 | Autosync garbage entry prices from corrupted quoteAsset/baseAsset division |
+| 94 | 🔴 CRITICAL | P&L Calculation | Jan 11, 2026 | P&L calculated using oracle price instead of actual fill price |

 ---

@@ -116,6 +117,7 @@ This document is the **comprehensive reference** for all documented pitfalls, bu
 - [#54](#pitfall-54-mfemae-storing-dollars-instead-of-percentages-critical---fixed-nov-23-2025) - MFE/MAE storing dollars instead of percentages
 - [#57](#pitfall-57-pl-calculation-inaccuracy-for-external-closures-critical---fixed-nov-20-2025) - P&L calculation inaccuracy for external closures
 - [#61](#pitfall-61-pl-compounding-still-happening-despite-all-guards-critical---under-investigation-nov-24-2025) - P&L compounding STILL happening
+- [#94](#pitfall-94-pl-calculated-using-oracle-price-instead-of-actual-fill-price-critical---fixed-jan-11-2026) - P&L using oracle price instead of actual fill price

 ### 🔴 Race Conditions & Duplicates
 - [#27](#pitfall-27-runner-stop-loss-gap---no-protection-between-tp1-and-tp2-critical---fixed-nov-15-2025) - Runner stop loss gap - no protection between TP1 and TP2
@@ -2162,6 +2164,79 @@ docker exec trading-bot-postgres psql -U postgres -d trading_bot_v4 -c \

 ---

+---
+
+### 89. P&L Calculated Using Oracle Price Instead of Actual Fill Price (🔴 CRITICAL - Jan 11, 2026)
+
+**Symptom:** Database P&L consistently over-reports profits compared to Drift UI
+
+**Real Incident (Jan 11, 2026):**
+- Drift UI showed: $133.03 profit (1.41%)
+- Database showed: $140.56 profit (1.48%)
+- **Over-reported: $7.53 (5.7% error)**
+
+**Root Cause:**
+- `closePosition()` in `lib/drift/orders.ts` calculated P&L using `oraclePrice` at close time
+- Oracle price is the index price at that moment, NOT the actual fill price
+- Actual fills occur at different price due to order book dynamics, slippage, market impact
+- Oracle price was $235.62, actual fill was ~$235.42 (0.08% difference × 10x leverage = 0.8% P&L error)
+
+**THE FIX (Jan 11, 2026 - Commit c1bff0d):**
+```typescript
+// Added helper to extract actual fill from transaction logs
+async function getActualFillPriceFromTx(
+  connection: any,
+  driftClient: any, 
+  txSig: string
+): Promise<{ fillPrice: number; baseAmount: number; quoteAmount: number } | null> {
+  const tx = await connection.getTransaction(txSig, { maxSupportedTransactionVersion: 0 })
+  const logParser = new LogParser(driftClient.program)
+  const events = logParser.parseEventsFromTransaction(tx)
+  
+  for (const event of events) {
+    if (event.eventType === 'OrderActionRecord') {
+      const record = event as any  // WrappedEvent has props directly, not on .data
+      if (record.action?.fill && record.baseAssetAmountFilled && record.quoteAssetAmountFilled) {
+        const baseAmount = record.baseAssetAmountFilled.toNumber() / 1e9
+        const quoteAmount = record.quoteAssetAmountFilled.toNumber() / 1e6
+        return { fillPrice: quoteAmount / baseAmount, baseAmount, quoteAmount }
+      }
+    }
+  }
+  return null
+}
+
+// In closePosition() - use actual fill price for P&L
+const fillData = await getActualFillPriceFromTx(connection, driftClient, txSig)
+let exitPrice = oraclePrice  // fallback
+if (fillData) {
+  exitPrice = fillData.fillPrice
+  console.log(`✅ Using ACTUAL fill price: $${exitPrice.toFixed(4)} (oracle was: $${oraclePrice.toFixed(4)})`)
+}
+```
+
+**Key Discovery - WrappedEvent Type:**
+- Initial code used `event.data.baseAssetAmountFilled` → TypeScript error
+- Drift SDK's `WrappedEvent` has properties directly on event object, NOT nested under `.data`
+- Correct access: `(event as any).baseAssetAmountFilled`
+
+**Files Changed:**
+- `lib/drift/orders.ts` - Added LogParser import, getActualFillPriceFromTx() helper, modified P&L calculation
+
+**Prevention Rules:**
+1. ALWAYS use actual transaction fill data for P&L, not oracle/index prices
+2. Oracle price is reference only - actual fills differ due to slippage, order book
+3. With leverage (10x), small price differences amplify to significant P&L errors
+4. Log both oracle and actual fill for comparison/debugging
+
+**Red Flags Indicating This Bug:**
+- Telegram P&L doesn't match Drift UI
+- Database profits consistently higher than exchange
+- Discrepancy increases with larger positions
+- Error percentage varies (not a fixed offset)
+
+---
+
 ## Cross-Reference Index

 - **See Also:** `.github/copilot-instructions.md` - Main AI agent instructions with Top 10 Critical Pitfalls
@@ -2170,5 +2245,5 @@ docker exec trading-bot-postgres psql -U postgres -d trading_bot_v4 -c \

 ---

-**Last Updated:** January 6, 2026  
+**Last Updated:** January 11, 2026  
 **Maintainer:** AI Agent team following "NOTHING gets lost" principle