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

critical: Fix P&L compounding in external closure detection

Browse Source 
Root cause: trade.realizedPnL was reading from in-memory ActiveTrade object
which could have stale/mutated values from previous detection cycles.

Bug sequence:
1. External closure detected, calculates P&L including previouslyRealized
2. Updates database with totalRealizedPnL
3. Same closure detected again (due to race condition or rate limits)
4. Reads previouslyRealized from same in-memory object (now has accumulated value)
5. Adds MORE P&L to it, compounds 2x, 5x, 10x

Real impact: 9 expected P&L became 81 (10x inflation)

Fix: Remove previouslyRealized from calculation entirely for external closures.
External closures calculate ONLY the current position P&L, not cumulative.
Database will have correct cumulative value if TP1 was processed separately.

Lines changed: lib/trading/position-manager.ts:785-803
- Removed: const previouslyRealized = trade.realizedPnL
- Removed: previouslyRealized + runnerRealized
- Now: totalRealizedPnL = runnerRealized (ONLY this closure's P&L)

Tested: Build completed successfully, container deployed and monitoring positions
This commit is contained in:
mindesbunister
2025-11-19 08:41:10 +01:00
parent a69d2b51a5
commit 7833686b7b
1 changed files with 11 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -782,10 +782,10 @@ export class PositionManager {
// CRITICAL: Use trade state flags, not current price (on-chain orders filled in the past!)
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
// CRITICAL: Calculate P&L for THIS close only, do NOT add previouslyRealized
// Previous bug: Added trade.realizedPnL which could be from prior detection cycles
// This caused 10x P&L inflation when same trade detected multiple times
// FIX: Calculate ONLY the runner P&L for this specific closure
let runnerRealized = 0
let runnerProfitPercent = 0
if (!wasPhantom) {
@@ -797,10 +797,13 @@ export class PositionManager {
runnerRealized = (sizeForPnL * runnerProfitPercent) / 100
}
const totalRealizedPnL = previouslyRealized + runnerRealized
// 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)}`)
// For external closures, we DON'T know if TP1 already hit, so calculate full position P&L
// Database will have correct previouslyRealized if TP1 was hit
// For external closures, we DON'T know if TP1 already hit, so calculate full position P&L
// Database will have correct previouslyRealized if TP1 was hit
const totalRealizedPnL = runnerRealized
console.log(` External closure P&L → ${runnerProfitPercent.toFixed(2)}% on $${sizeForPnL.toFixed(2)} = $${totalRealizedPnL.toFixed(2)}`)
// Determine exit reason from trade state and P&L
if (trade.tp2Hit) {