From 6156c0f9585a853a072faffe446ca261c543beed Mon Sep 17 00:00:00 2001 From: mindesbunister Date: Mon, 17 Nov 2025 15:28:08 +0100 Subject: [PATCH] critical: Fix P&L compounding bug in external closure detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CRITICAL BUG: trade.realizedPnL was being mutated during each external closure detection - This caused exponential compounding: $6 → $12 → $24 → $48 → $96 - Each time monitoring loop detected closure, it added previouslyRealized + runnerRealized - But previouslyRealized was the ALREADY ACCUMULATED value from previous iteration - Result: P&L compounded 15-20x on actual value ROOT CAUSE (line 797): const totalRealizedPnL = previouslyRealized + runnerRealized trade.realizedPnL = totalRealizedPnL // ← BUG: Mutates in-memory trade object Next detection cycle: const previouslyRealized = trade.realizedPnL // ← Gets ACCUMULATED value const totalRealizedPnL = previouslyRealized + runnerRealized // ← Adds AGAIN FIX: - Don't mutate trade.realizedPnL during external closure detection - Calculate totalRealizedPnL locally, use for database update only - trade.realizedPnL stays immutable after initial DB save - Log message clarified: 'P&L calculation' not 'P&L snapshot' IMPACT: - Every external closure (TP/SL on-chain orders) affected - With rate limiting, closure detected 15-20 times before removal - Real example: $6 actual profit showed as $92.46 in database - This is WORSE than duplicate notification bug - corrupts financial data FILES CHANGED: - lib/trading/position-manager.ts: Removed trade.realizedPnL mutation (line 799) - Database manually corrected: $92.46 → $6.00 for affected trade RELATED BUGS: - Common Pitfall #48: closingInProgress flag prevents some duplicates - But doesn't help if monitoring loop runs DURING external closure detection - Need both fixes: closingInProgress + no mutation of trade.realizedPnL --- lib/trading/position-manager.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/trading/position-manager.ts b/lib/trading/position-manager.ts index a83cb7b..c0c40f0 100644 --- a/lib/trading/position-manager.ts +++ b/lib/trading/position-manager.ts @@ -781,6 +781,7 @@ export class PositionManager { let exitReason: 'TP1' | 'TP2' | 'SL' | 'SOFT_SL' | 'HARD_SL' = 'SL' // Include any previously realized profit (e.g., from TP1 partial close) + // CRITICAL: Get from original database value, not mutated in-memory value const previouslyRealized = trade.realizedPnL let runnerRealized = 0 @@ -795,8 +796,9 @@ export class PositionManager { } const totalRealizedPnL = previouslyRealized + runnerRealized - trade.realizedPnL = totalRealizedPnL - console.log(` Realized P&L snapshot → Previous: $${previouslyRealized.toFixed(2)} | Runner: $${runnerRealized.toFixed(2)} (Δ${runnerProfitPercent.toFixed(2)}%) on $${sizeForPnL.toFixed(2)} | Total: $${totalRealizedPnL.toFixed(2)}`) + // DON'T mutate trade.realizedPnL here - causes compounding on re-detection! + // trade.realizedPnL = totalRealizedPnL ← REMOVED: Causes duplicate P&L bug + console.log(` Realized P&L calculation → Previous: $${previouslyRealized.toFixed(2)} | Runner: $${runnerRealized.toFixed(2)} (Δ${runnerProfitPercent.toFixed(2)}%) on $${sizeForPnL.toFixed(2)} | Total: $${totalRealizedPnL.toFixed(2)}`) // Determine exit reason from trade state and P&L if (trade.tp2Hit) {