docs: Add Bug #91 to COMMON_PITFALLS.md and copilot-instructions.md

Bug #91: Math.floor truncation leaves fractional position remnants
- Root cause: new BN(Math.floor(sizeToClose * 1e9)) truncates position sizes
- Solution: SDK's closePosition() uses exact BN arithmetic (baseAssetAmount.abs())
- Added to Quick Reference Table in COMMON_PITFALLS.md
- Added full documentation entry (~70 lines)
- Updated Top 10 Critical Pitfalls in copilot-instructions.md
- Replaced #75 (Wrong Year) with #91 (more impactful to trading operations)
This commit is contained in:
mindesbunister
2026-01-01 13:58:29 +01:00
parent 4bf5761ec0
commit 26ddd177b3
2 changed files with 82 additions and 1 deletions

View File

@@ -879,7 +879,7 @@ docs/COMMON_PITFALLS.md
1. **Position Manager Never Monitors (#77)** - Logs say "added" but isMonitoring=false = $1,000+ losses
2. **Silent SL Placement Failure (#76)** - placeExitOrders() returns SUCCESS with 2/3 orders, no SL protection
3. **Orphan Cleanup Removes Active Orders (#78)** - cancelAllOrders() affects ALL positions on symbol
4. **Wrong Year in SQL Queries (#75)** - Query 2024 dates when current is 2025 = 12× inflated results
4. **Math.floor Truncation in Position Close (#91)** - Leaves fractional remnants → Use SDK closePosition()
5. **Drift SDK Memory Leak (#1)** - JS heap OOM after 10+ hours → Smart health monitoring
6. **Wrong RPC Provider (#2)** - Alchemy breaks Drift SDK → Use Helius only
7. **P&L Compounding Race Condition (#48, #49, #61)** - Multiple closures → Atomic Map.delete()

View File

@@ -99,6 +99,8 @@ This document is the **comprehensive reference** for all documented pitfalls, bu
| 71 | 🔴 CRITICAL | Revenge System | Dec 3, 2025 | Revenge system missing external closure integration |
| 72 | 🔴 CRITICAL | Telegram | Dec 4, 2025 | Telegram webhook conflicts with polling bot |
| 89 | 🔴 CRITICAL | Drift Protocol | Dec 16, 2025 | Drift fractional position remnants after SL execution |
| 90 | 🔴 CRITICAL | Drift Protocol | Dec 31, 2025 | placePerpOrder vs placeAndTakePerpOrder - MARKET orders not filling |
| 91 | 🔴 CRITICAL | Drift Protocol | Jan 1, 2026 | Math.floor truncation leaves fractional position remnants |
| 90 | 🔴 CRITICAL | Drift Protocol | Dec 31, 2025 | placePerpOrder only places orders, doesn't fill - use placeAndTakePerpOrder |
---
@@ -1749,6 +1751,85 @@ docker logs -f trading-bot-v4 | grep "placeAndTakePerpOrder\|IMMEDIATE FILL"
---
### 91. Math.floor Truncation Leaves Fractional Position Remnants (CRITICAL - Jan 1, 2026)
**Severity:** 🔴 CRITICAL
**Category:** Drift Protocol / Floating Point Math
**Date Discovered:** January 1, 2026
**Financial Impact:** Flip failures causing positions to not close, leading to $1,000+ losses
**Symptom:** Position close transactions confirm but position still exists on Drift with tiny fractional remnant (e.g., 0.00000008 SOL)
**User Report:** "Drift UI 'Close All Positions' fails with 'not enough collateral' but clicking 'Market' order directly works"
**Root Cause:**
```typescript
// BROKEN CODE (lib/drift/orders.ts line ~680):
const sizeToClose = (trade.currentSize / currentPrice) * (percentToClose / 100)
const sizeToCloseNormalized = new BN(Math.floor(sizeToClose * 1e9)) // ❌ TRUNCATES!
// Example:
// Original position: 21.83456789012 SOL
// Math.floor(21.83456789012 * 1e9) = 21834567890 (loses 0.12 precision)
// Actual close: 21.834567890 SOL
// Remnant: 0.00000008 SOL still open on Drift
```
**Discovery Path:**
1. Bug #90 fix deployed (placeAndTakePerpOrder) - flip still failed
2. User tested Drift UI: "Close All Positions" failed, but "Market" order worked
3. Key insight: Market order uses EXACT position size from Drift state
4. Investigation revealed Math.floor truncation causes fractional remnants
**THE FIX (Jan 1, 2026):**
```typescript
// FIXED CODE (lib/drift/orders.ts lines 647-690):
if (percentToClose === 100) {
// Use SDK's closePosition() which handles exact BN internally
// SDK gets exact baseAssetAmount from position state (no truncation)
console.log(`🚀 Using SDK closePosition() for 100% close (exact BN, no truncation)...`)
txSig = await driftClient.closePosition(marketConfig.driftMarketIndex)
} else {
// Partial close: Continue using placeAndTakePerpOrder with calculated size
// Truncation is acceptable for partial closes
const sizeToClose = (trade.currentSize / currentPrice) * (percentToClose / 100)
const sizeToCloseNormalized = new BN(Math.floor(sizeToClose * 1e9))
// ... placeAndTakePerpOrder code
}
```
**Why SDK closePosition() Works:**
- Drift SDK's `closePosition()` internally uses `position.baseAssetAmount.abs()`
- This is the EXACT BN value stored in Drift's state - no floating point conversion
- No truncation, no remnants, complete position closure guaranteed
**Files Changed:**
- lib/drift/orders.ts: Lines 647-690 (conditional closePosition vs placeAndTakePerpOrder)
**Prevention Rules:**
1. NEVER use Math.floor for 100% position closes - use SDK's closePosition()
2. Floating point arithmetic introduces truncation errors in BN conversion
3. For exact values, always let SDK read from on-chain state directly
4. Partial closes can tolerate truncation (tiny remnant acceptable)
**Red Flags Indicating This Bug:**
- Transaction confirms but position still shows in Drift UI
- Position size shows tiny remnant (< 0.001 tokens)
- "Close All Positions" button fails with collateral error
- Manual Market order works but automated close fails
**Git Commit:** 4bf5761 "critical: Bug #91 - Use SDK closePosition() for exact BN (no Math.floor truncation)"
**Deployment:** Jan 1, 2026 13:42 UTC (container trading-bot-v4)
**Status:** ✅ FIXED AND DEPLOYED
**Lesson Learned:** Never use floating point math to calculate position sizes for 100% closes. The Drift SDK provides `closePosition()` specifically for this purpose - it reads the exact BN value directly from on-chain state. Math.floor() truncation is invisible but catastrophic for exact position operations.
---
## Appendix: Pattern Recognition
### Common Root Causes