From 2af13eaa88af307e830ea976894f5db8d94bd385 Mon Sep 17 00:00:00 2001 From: mindesbunister Date: Sun, 11 Jan 2026 21:10:09 +0100 Subject: [PATCH] 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) --- docs/COMMON_PITFALLS.md | 77 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/docs/COMMON_PITFALLS.md b/docs/COMMON_PITFALLS.md index d0f8a55..7225d6e 100644 --- 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