From 33ef157a246430baabfedfe7fc1d6994796902d5 Mon Sep 17 00:00:00 2001 From: mindesbunister Date: Fri, 21 Nov 2025 16:45:43 +0100 Subject: [PATCH] docs: Update Common Pitfall #56 with order filtering bug fix Added Nov 21 false positive investigation and resolution: - Bot showed '32 open orders' when Drift UI showed 0 - Root cause: orderId > 0 check didn't verify baseAssetAmount - Drift's 32-slot array contained historical metadata - Fix: Enhanced filter with baseAssetAmount.eq(new BN(0)) check - Result: Accurate order count reporting Updated with correct filtering code example and dual commit references. --- .github/copilot-instructions.md | 36 +++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d00efeb..d4654df 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -2970,15 +2970,22 @@ trade.realizedPnL += actualRealizedPnL // NOT: result.realizedPnL from SDK - **Impact:** Multi-timeframe data collection now operational for Phase 1 analysis (50+ signals per timeframe target) - **Lesson:** Background jobs should use Drift oracle prices (always available) not Pyth cache (real-time only). Always initialize external services before calling their methods. Verify background jobs are actually working by checking database state, not just logs. -56. **Ghost orders after external closures (CRITICAL - Fixed Nov 20, 2025):** +56. **Ghost orders after external closures (CRITICAL - Fixed Nov 20, 2025) + False order count bug (Fixed Nov 21, 2025):** - **Symptom:** Position closed externally (on-chain SL/TP order filled), but TP/SL orders remain active on Drift - **Root Cause:** Position Manager's external closure handler didn't call `cancelAllOrders()` before completing trade - **Real incident (Nov 20, 13:30 CET):** * SHORT position stopped out at $142.48 * Position closed successfully on Drift - * TP1 order at $140.66 still active (32 total ghost orders found!) - * Manual cleanup via `/api/trading/cancel-orders` cancelled 32 orders + * TP1 order at $140.66 still active + * Manual cleanup via `/api/trading/cancel-orders` cancelled orders * Risk: If price dropped to $140.66 later, ghost order would fill → unintended LONG position + - **FALSE POSITIVE BUG (Nov 21, 2025):** + * Bot logs showed "32 open orders to cancel" on every restart + * Drift UI showed 0 orders (correct) + * Root cause: Filter checked `orderId > 0` but didn't verify `baseAssetAmount` + * Drift's 32-slot order array contained historical metadata with non-zero orderIds but zero baseAssetAmount + * Fix: Added `baseAssetAmount.eq(new BN(0))` check to filter truly active orders + * Result: Bot now correctly reports 0 orders when none exist - **Impact:** Every external closure (SL/TP fills) leaves ghost orders on exchange - **Why dangerous:** * Ghost orders can trigger unintended positions if price moves to those levels @@ -3023,11 +3030,24 @@ trade.realizedPnL += actualRealizedPnL // NOT: result.realizedPnL from SDK * Update database with exit details * Stop monitoring if no more trades * 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:** 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. + - **Why 32 orders:** Drift SDK's `userAccount.orders` array has 32 order slots (fixed size), old filter counted slots with non-zero orderIds even when baseAssetAmount was zero + - **Correct filtering (Nov 21, 2025):** + ```typescript + const ordersToCancel = userAccount.orders.filter( + (order: any) => { + if (order.marketIndex !== marketConfig.driftMarketIndex) return false + if (!order.orderId || order.orderId === 0) return false + // CRITICAL: Check baseAssetAmount - empty slots have zero amount + if (!order.baseAssetAmount || order.baseAssetAmount.eq(new BN(0))) return false + return true + } + ) + console.log(`📋 Found ${ordersToCancel.length} open orders (checked ${userAccount.orders.length} total slots)`) + ``` + - **Files changed:** `lib/drift/orders.ts` (cancelAllOrders function), `lib/trading/position-manager.ts` (external closure handler) + - **Commits:** a3a6222 "critical: Cancel ghost orders after external closures" (Nov 20), 29fce01 "fix: Correct order filtering" (Nov 21) + - **Deployed:** Nov 20, 2025 15:25 CET (ghost cleanup), Nov 21, 2025 (accurate filtering) + - **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. Always verify SDK filter logic matches reality - check transaction results, not just logs. 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)