root/trading_bot_v4
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

critical: Fix P&L compounding bug in external closure detection

Browse Source 
- 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
This commit is contained in:
mindesbunister
2025-11-17 15:28:08 +01:00
parent 3aeb00f998
commit 6156c0f958
1 changed files with 4 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
lib/trading/position-manager.ts
View File

@@ -781,6 +781,7 @@ export class PositionManager {
let exitReason: 'TP1' | 'TP2' | 'SL' | 'SOFT_SL' | 'HARD_SL' = 'SL' let exitReason: 'TP1' | 'TP2' | 'SL' | 'SOFT_SL' | 'HARD_SL' = 'SL'
// Include any previously realized profit (e.g., from TP1 partial close) // 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 const previouslyRealized = trade.realizedPnL
let runnerRealized = 0 let runnerRealized = 0
@@ -795,8 +796,9 @@ export class PositionManager {
} }
const totalRealizedPnL = previouslyRealized + runnerRealized const totalRealizedPnL = previouslyRealized + runnerRealized
trade.realizedPnL = totalRealizedPnL // DON'T mutate trade.realizedPnL here - causes compounding on re-detection!
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)}`) // 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 // Determine exit reason from trade state and P&L
if (trade.tp2Hit) { if (trade.tp2Hit) {