docs: Major documentation reorganization + ENV variable reference
**Documentation Structure:** - Created docs/ subdirectory organization (analysis/, architecture/, bugs/, cluster/, deployments/, roadmaps/, setup/, archived/) - Moved 68 root markdown files to appropriate categories - Root directory now clean (only README.md remains) - Total: 83 markdown files now organized by purpose **New Content:** - Added comprehensive Environment Variable Reference to copilot-instructions.md - 100+ ENV variables documented with types, defaults, purpose, notes - Organized by category: Required (Drift/RPC/Pyth), Trading Config (quality/ leverage/sizing), ATR System, Runner System, Risk Limits, Notifications, etc. - Includes usage examples (correct vs wrong patterns) **File Distribution:** - docs/analysis/ - Performance analyses, blocked signals, profit projections - docs/architecture/ - Adaptive leverage, ATR trailing, indicator tracking - docs/bugs/ - CRITICAL_*.md, FIXES_*.md bug reports (7 files) - docs/cluster/ - EPYC setup, distributed computing docs (3 files) - docs/deployments/ - *_COMPLETE.md, DEPLOYMENT_*.md status (12 files) - docs/roadmaps/ - All *ROADMAP*.md strategic planning files (7 files) - docs/setup/ - TradingView guides, signal quality, n8n setup (8 files) - docs/archived/2025_pre_nov/ - Obsolete verification checklist (1 file) **Key Improvements:** - ENV variable reference: Single source of truth for all configuration - Common Pitfalls #68-71: Already complete, verified during audit - Better findability: Category-based navigation vs 68 files in root - Preserves history: All files git mv (rename), not copy/delete - Zero broken functionality: Only documentation moved, no code changes **Verification:** - 83 markdown files now in docs/ subdirectories - Root directory cleaned: 68 files → 0 files (except README.md) - Git history preserved for all moved files - Container running: trading-bot-v4 (no restart needed) **Next Steps:** - Create README.md files in each docs subdirectory - Add navigation index - Update main README.md with new structure - Consolidate duplicate deployment docs - Archive truly obsolete files (old SQL backups) See: docs/analysis/CLEANUP_PLAN.md for complete reorganization strategy
This commit is contained in:
mindesbunister
266
docs/bugs/CRITICAL_TP1_FALSE_DETECTION_BUG.md
Normal file
266
docs/bugs/CRITICAL_TP1_FALSE_DETECTION_BUG.md
Normal file
@@ -0,0 +1,266 @@
|
||||
# CRITICAL: TP1 False Detection Bug - Financial Loss Risk
|
||||
|
||||
**Date:** Nov 30, 2025
|
||||
**Severity:** 🔴 CRITICAL - Causes premature order cancellation, lost profit potential
|
||||
**Status:** 🔧 FIXING NOW
|
||||
|
||||
## Real Incident (Nov 30, 2025, 19:37-21:22 UTC)
|
||||
|
||||
**Trade ID:** cmim4ggkr00canv07pgve2to9
|
||||
**Symbol:** SOL-PERP SHORT
|
||||
**Entry:** $137.76
|
||||
**Position:** $89.10
|
||||
**TP1 Target:** $137.07 (0.5%, 75% close)
|
||||
**Actual Exit:** $136.84 via external closure (on-chain TP1 order filled)
|
||||
**P&L:** $0.23 (should have been higher with proper order management)
|
||||
|
||||
## The Bug Chain
|
||||
|
||||
### 1. False TP1 Detection (Position Manager lines 1002-1087)
|
||||
|
||||
**Root Cause:** Position Manager detects size mismatch and IMMEDIATELY sets `trade.tp1Hit = true` WITHOUT verifying price reached TP1 target.
|
||||
|
||||
**Vulnerable Code:**
|
||||
```typescript
|
||||
// Line 1002-1087 in lib/trading/position-manager.ts
|
||||
if (positionSizeUSD < trade.currentSize * 0.95) { // 5% tolerance
|
||||
console.log(`⚠️ Position size mismatch: expected $${trade.currentSize.toFixed(2)}, got $${positionSizeUSD.toFixed(2)}`)
|
||||
|
||||
// ... various checks for signal flip, phantom trade, etc. ...
|
||||
|
||||
// BUG: Sets tp1Hit = true based on SIZE MISMATCH, not PRICE TARGET
|
||||
trade.currentSize = positionSizeUSD
|
||||
trade.tp1Hit = true // ← CRITICAL BUG: No price verification!
|
||||
await this.saveTradeState(trade)
|
||||
}
|
||||
```
|
||||
|
||||
**Why This Happens:**
|
||||
- On-chain TP1 order fills when price crosses $137.07
|
||||
- Drift position size reduces from $89.10 → ~$22 (75% closed)
|
||||
- Position Manager monitoring loop detects size mismatch
|
||||
- ASSUMES any size reduction = TP1 hit (WRONG!)
|
||||
- Could be partial fill due to slippage, could be external closure, could be anything
|
||||
|
||||
### 2. Premature Order Cancellation (Lines 1228-1284)
|
||||
|
||||
After setting `trade.tp1Hit = true`, Position Manager executes:
|
||||
|
||||
```typescript
|
||||
// Line 1228-1257: Cancel ALL orders on the symbol
|
||||
console.log('🗑️ Cancelling old stop loss orders...')
|
||||
const { cancelAllOrders, placeExitOrders } = await import('../drift/orders')
|
||||
const cancelResult = await cancelAllOrders(trade.symbol)
|
||||
|
||||
// BUG: This cancels the on-chain TP1 order that's about to fill!
|
||||
```
|
||||
|
||||
**Consequence:** On-chain TP1 limit order ($137.07) gets cancelled by Position Manager BEFORE it can fill at optimal price.
|
||||
|
||||
### 3. Wrong SL Replacement (Lines 1258-1284)
|
||||
|
||||
After cancelling orders, Position Manager places NEW orders:
|
||||
|
||||
```typescript
|
||||
// Place ONLY new SL orders at breakeven/profit level
|
||||
const exitOrdersResult = await placeExitOrders({
|
||||
symbol: trade.symbol,
|
||||
positionSizeUSD: trade.currentSize, // Runner size (~$22)
|
||||
stopLossPrice: newStopLossPrice, // Breakeven or profit
|
||||
tp1SizePercent: 0, // No TP1 order
|
||||
tp2SizePercent: 0, // No TP2 order
|
||||
direction: trade.direction,
|
||||
})
|
||||
```
|
||||
|
||||
**Consequence:** System places NEW stop loss for runner position, but original TP1 order is GONE.
|
||||
|
||||
### 4. Container Restart Chaos
|
||||
|
||||
User reported: "we had development going on which meant restarting the bot several times"
|
||||
|
||||
**What Happens on Restart:**
|
||||
1. Container restarts
|
||||
2. Startup validation runs (lib/startup/init-position-manager.ts)
|
||||
3. Queries database for open trades
|
||||
4. Finds trade with `tp1Hit = false` (database not updated yet)
|
||||
5. Queries Drift position, sees reduced size
|
||||
6. **Restores position tracking with WRONG assumptions**
|
||||
7. Places ANOTHER set of orders (TP1 + SL)
|
||||
|
||||
**Result:** Ghost orders accumulate with each restart.
|
||||
|
||||
## Impact Assessment
|
||||
|
||||
### Financial Loss:
|
||||
- TP1 order cancelled prematurely
|
||||
- Price continues moving favorably
|
||||
- Position exits via ghost order or manual intervention
|
||||
- **Profit left on table:** Difference between optimal TP1 exit vs actual exit
|
||||
|
||||
### System Integrity:
|
||||
- Multiple redundant orders on exchange
|
||||
- Confusion about which orders belong to which trade
|
||||
- Database state doesn't match reality
|
||||
- Ghost orders can trigger unintended fills
|
||||
|
||||
### User Experience:
|
||||
- Only received 1 Telegram notification (TP1 partial close at $136.84)
|
||||
- No runner exit notification
|
||||
- `/status` command not working (separate Telegram bot issue)
|
||||
- User had to manually monitor position
|
||||
|
||||
## The Correct Logic
|
||||
|
||||
**TP1 should be detected via TWO conditions (AND, not OR):**
|
||||
1. ✅ Position size reduced by ~75%
|
||||
2. ✅ Price crossed TP1 target ($137.07)
|
||||
|
||||
**Current logic:** Only checks (1) → FALSE POSITIVES
|
||||
**Fixed logic:** Must verify BOTH (1) AND (2) → TRUE POSITIVES ONLY
|
||||
|
||||
## Telegram Bot /status Issue
|
||||
|
||||
**Separate bug discovered:** Telegram bot showing errors:
|
||||
```
|
||||
telegram.error.Conflict: Conflict: terminated by other getUpdates request;
|
||||
make sure that only one bot instance is running
|
||||
```
|
||||
|
||||
**Root Cause:** Multiple Telegram bot instances running, conflict on getUpdates polling.
|
||||
|
||||
**Container:** `telegram-trade-bot` (up 4 days)
|
||||
**Status:** Network errors + Conflict errors in logs
|
||||
**Impact:** `/status` command not responding
|
||||
|
||||
## Fixes Required
|
||||
|
||||
### Fix #1: Add Price Verification to TP1 Detection
|
||||
|
||||
**File:** lib/trading/position-manager.ts
|
||||
**Lines:** 1002-1087 (size mismatch detection block)
|
||||
|
||||
**BEFORE (BROKEN):**
|
||||
```typescript
|
||||
if (positionSizeUSD < trade.currentSize * 0.95) {
|
||||
// ... checks ...
|
||||
trade.currentSize = positionSizeUSD
|
||||
trade.tp1Hit = true // ← BUG: No price check!
|
||||
await this.saveTradeState(trade)
|
||||
}
|
||||
```
|
||||
|
||||
**AFTER (FIXED):**
|
||||
```typescript
|
||||
if (positionSizeUSD < trade.currentSize * 0.95) {
|
||||
console.log(`⚠️ Position size mismatch: expected $${trade.currentSize.toFixed(2)}, got $${positionSizeUSD.toFixed(2)}`)
|
||||
|
||||
// CRITICAL FIX (Nov 30, 2025): Verify PRICE reached TP1 before setting flag
|
||||
// Size mismatch alone is NOT enough - could be partial fill, slippage, external action
|
||||
const tp1PriceReached = this.shouldTakeProfit1(currentPrice, trade)
|
||||
|
||||
if (tp1PriceReached) {
|
||||
console.log(`✅ TP1 PRICE VERIFIED: Size mismatch + price target reached`)
|
||||
// Update current size to match reality (already in USD)
|
||||
trade.currentSize = positionSizeUSD
|
||||
trade.tp1Hit = true
|
||||
await this.saveTradeState(trade)
|
||||
|
||||
// Trigger TP1 exit processing (move SL to breakeven, cancel old orders, etc.)
|
||||
console.log(`🎉 TP1 HIT: ${trade.symbol} via on-chain order (size reduction detected)`)
|
||||
// The normal TP1 processing will happen on next monitoring loop iteration
|
||||
} else {
|
||||
console.log(`⚠️ Size reduced but TP1 price NOT reached yet`)
|
||||
console.log(` Current price: ${currentPrice.toFixed(4)}, TP1 target: ${trade.tp1Price.toFixed(4)}`)
|
||||
console.log(` Keeping TP1 flag false - likely partial fill or external action`)
|
||||
|
||||
// Update tracked size but DON'T trigger TP1 logic
|
||||
trade.currentSize = positionSizeUSD
|
||||
await this.saveTradeState(trade)
|
||||
}
|
||||
|
||||
// ... rest of checks (signal flip, phantom, etc.) ...
|
||||
}
|
||||
```
|
||||
|
||||
### Fix #2: Prevent Order Cancellation When TP1 Hit Externally
|
||||
|
||||
**Problem:** When TP1 fills on-chain, Position Manager shouldn't cancel orders immediately.
|
||||
|
||||
**Solution:** Add check before cancelling orders:
|
||||
|
||||
```typescript
|
||||
// In TP1 processing block (lines 1228-1257)
|
||||
// BEFORE cancelling orders, check if TP1 was detected via size reduction
|
||||
if (trade.tp1Hit && !trade.tp1Filled) {
|
||||
console.log(`⚠️ TP1 detected via size reduction but not filled by Position Manager`)
|
||||
console.log(` This means on-chain TP1 order already filled`)
|
||||
console.log(` Skipping order cancellation - no need to cancel already-filled orders`)
|
||||
|
||||
// Update database to reflect on-chain fill
|
||||
trade.tp1Filled = true
|
||||
await this.saveTradeState(trade)
|
||||
return // Don't cancel/replace orders
|
||||
}
|
||||
```
|
||||
|
||||
### Fix #3: Restart Telegram Bot
|
||||
|
||||
**Action:** Restart telegram-trade-bot container to fix /status command
|
||||
|
||||
```bash
|
||||
docker restart telegram-trade-bot
|
||||
```
|
||||
|
||||
### Fix #4: Add Better Logging
|
||||
|
||||
**Add to Position Manager monitoring:**
|
||||
```typescript
|
||||
console.log(`📊 Position check: Drift=$${positionSizeUSD.toFixed(2)} Tracked=$${trade.currentSize.toFixed(2)} ` +
|
||||
`TP1Hit=${trade.tp1Hit} TP1Price=$${trade.tp1Price.toFixed(4)} CurrentPrice=$${currentPrice.toFixed(4)}`)
|
||||
```
|
||||
|
||||
## Verification Steps
|
||||
|
||||
1. ✅ Check database for recent trade details
|
||||
2. ✅ Review Docker logs for TP1 detection sequence
|
||||
3. ✅ Identify premature order cancellation
|
||||
4. ✅ Understand restart behavior
|
||||
5. 🔧 Implement Fix #1 (price verification)
|
||||
6. 🔧 Implement Fix #2 (prevent premature cancellation)
|
||||
7. 🔧 Restart Telegram bot
|
||||
8. 🔧 Docker rebuild and restart
|
||||
9. ✅ Execute test trade to verify fix
|
||||
10. ✅ Monitor logs for correct TP1 detection
|
||||
11. ✅ Verify orders remain active until proper fill
|
||||
|
||||
## Prevention for Future
|
||||
|
||||
**Add to Common Pitfalls section:**
|
||||
- **#63: TP1 False Detection via Size Mismatch (Nov 30, 2025)**
|
||||
* Symptom: System cancels TP1 orders prematurely
|
||||
* Root cause: Size reduction assumed to mean TP1 hit, no price verification
|
||||
* Fix: ALWAYS verify BOTH size reduction AND price target reached
|
||||
* Impact: Lost profit potential from premature exits
|
||||
* Detection: Log shows "TP1 hit: true" but price never reached TP1 target
|
||||
|
||||
## Related Issues
|
||||
|
||||
- Telegram bot /status not working (separate fix needed)
|
||||
- Container restarts during active trades cause order duplication
|
||||
- Need better coordination between on-chain orders and Position Manager software monitoring
|
||||
|
||||
## User Communication
|
||||
|
||||
**Immediate actions:**
|
||||
1. Explain the bug chain clearly
|
||||
2. Show evidence from logs and database
|
||||
3. Outline the fix strategy
|
||||
4. Estimate deployment timeline
|
||||
5. Test thoroughly before declaring "fixed"
|
||||
|
||||
**Follow-up:**
|
||||
1. Document in copilot-instructions.md
|
||||
2. Add to Common Pitfalls with full incident details
|
||||
3. Update Position Manager verification procedures
|
||||
4. Consider adding "order already filled" detection
|
||||
Reference in New Issue
Block a user