From f14c5213026e5cf0c15b4652ff303e5ebc08a5f9 Mon Sep 17 00:00:00 2001 From: 21in7 Date: Thu, 19 Mar 2026 23:55:14 +0900 Subject: [PATCH] =?UTF-8?q?fix:=20critical=20bugs=20=E2=80=94=20double=20f?= =?UTF-8?q?ee,=20SL/TP=20atomicity,=20PnL=20race,=20graceful=20shutdown?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C5: Remove duplicate entry_fee deduction in backtester (balance and net_pnl) C1: Add SL/TP retry (3x) with emergency market close on final failure C3: Add _close_lock to prevent PnL double recording between callback and monitor C8: Add SIGTERM/SIGINT handler with per-symbol order cancellation before exit Co-Authored-By: Claude Opus 4.6 (1M context) --- CLAUDE.md | 1 + docs/plans/2026-03-19-critical-bugfixes.md | 53 +++++ main.py | 50 +++- src/backtester.py | 3 +- src/bot.py | 260 +++++++++++++-------- 5 files changed, 265 insertions(+), 102 deletions(-) create mode 100644 docs/plans/2026-03-19-critical-bugfixes.md diff --git a/CLAUDE.md b/CLAUDE.md index bb5cb9c..a34b682 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -141,3 +141,4 @@ All design documents and implementation plans are stored in `docs/plans/` with t | 2026-03-06 | `strategy-parameter-sweep` (plan) | Completed | | 2026-03-07 | `weekly-report` (plan) | Completed | | 2026-03-07 | `code-review-improvements` | Partial (#1,#2,#4,#5,#6,#8 완료) | +| 2026-03-19 | `critical-bugfixes` (C5,C1,C3,C8) | Completed | diff --git a/docs/plans/2026-03-19-critical-bugfixes.md b/docs/plans/2026-03-19-critical-bugfixes.md new file mode 100644 index 0000000..07df3c1 --- /dev/null +++ b/docs/plans/2026-03-19-critical-bugfixes.md @@ -0,0 +1,53 @@ +# Critical Bugfixes Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix 4 critical bugs identified in code review (C5, C1, C3, C8) + +**Architecture:** Direct fixes to backtester.py, bot.py, main.py — no new files needed + +**Tech Stack:** Python asyncio, signal handling + +--- + +## Task 1: C5 — Backtester double fee deduction + atr≤0 fee leak + +**Files:** +- Modify: `src/backtester.py:494-501` + +- [x] Remove `self.balance -= entry_fee` at L496. The fee is already deducted in `_close_position` via `net_pnl = gross_pnl - entry_fee - exit_fee`. +- [x] This also fixes the atr≤0 early return bug — since balance is no longer modified before ATR check, early return doesn't leak fees. + +## Task 2: C1 — SL/TP atomicity with retry and emergency close + +**Files:** +- Modify: `src/bot.py:461-475` + +- [x] Wrap SL/TP placement in `_place_sl_tp_with_retry()` with 3 retries and 1s backoff +- [x] Track `sl_placed` and `tp_placed` independently to avoid re-placing successful orders +- [x] On final failure, call `_emergency_close()` which market-closes the position and notifies via Discord +- [x] `_emergency_close` also handles its own failure with critical log + Discord alert + +## Task 3: C3 — PnL double recording race condition + +**Files:** +- Modify: `src/bot.py` (init, _on_position_closed, _position_monitor) + +- [x] Add `self._close_lock = asyncio.Lock()` to `__init__` +- [x] Wrap `_on_position_closed` body with `async with self._close_lock` +- [x] Wrap SYNC path in `_position_monitor` with `async with self._close_lock` +- [x] Add double-check after lock acquisition in monitor (callback may have already processed) + +## Task 4: C8 — Graceful shutdown with signal handler + +**Files:** +- Modify: `main.py` + +- [x] Add `signal.SIGTERM` and `signal.SIGINT` handlers via `loop.add_signal_handler()` +- [x] Use `asyncio.Event` + `asyncio.wait(FIRST_COMPLETED)` pattern +- [x] `_graceful_shutdown()`: cancel all open orders per bot (with 5s timeout), then cancel tasks +- [x] Log shutdown progress for each symbol + +## Verification + +- [x] All 138 existing tests pass (0 failures) diff --git a/main.py b/main.py index 33e5c11..387df6d 100644 --- a/main.py +++ b/main.py @@ -1,4 +1,5 @@ import asyncio +import signal from datetime import datetime, timedelta, timezone from dotenv import load_dotenv from loguru import logger @@ -21,6 +22,25 @@ async def _daily_reset_loop(risk: RiskManager): risk.reset_daily() +async def _graceful_shutdown(bots: list[TradingBot], tasks: list[asyncio.Task]): + """모든 봇의 오픈 주문 취소 후 태스크를 정리한다.""" + logger.info("Graceful shutdown 시작 — 오픈 주문 취소 중...") + for bot in bots: + try: + await asyncio.wait_for(bot.exchange.cancel_all_orders(), timeout=5) + logger.info(f"[{bot.symbol}] 오픈 주문 취소 완료") + except Exception as e: + logger.warning(f"[{bot.symbol}] 오픈 주문 취소 실패 (무시): {e}") + + for task in tasks: + task.cancel() + results = await asyncio.gather(*tasks, return_exceptions=True) + for r in results: + if isinstance(r, Exception) and not isinstance(r, asyncio.CancelledError): + logger.warning(f"태스크 종료 중 예외: {r}") + logger.info("Graceful shutdown 완료") + + async def main(): setup_logger(log_level="INFO") config = Config() @@ -39,11 +59,35 @@ async def main(): bots.append(bot) logger.info(f"멀티심볼 봇 시작: {config.symbols} ({len(bots)}개 인스턴스)") - await asyncio.gather( - *[bot.run() for bot in bots], - _daily_reset_loop(risk), + + # 시그널 핸들러 등록 + loop = asyncio.get_running_loop() + shutdown_event = asyncio.Event() + + def _signal_handler(): + logger.warning("종료 시그널 수신 (SIGTERM/SIGINT)") + shutdown_event.set() + + for sig in (signal.SIGTERM, signal.SIGINT): + loop.add_signal_handler(sig, _signal_handler) + + tasks = [ + asyncio.create_task(bot.run(), name=f"bot-{bot.symbol}") + for bot in bots + ] + tasks.append(asyncio.create_task(_daily_reset_loop(risk), name="daily-reset")) + + # 종료 시그널 대기 vs 태스크 완료 (먼저 발생하는 쪽) + shutdown_task = asyncio.create_task(shutdown_event.wait(), name="shutdown-wait") + done, pending = await asyncio.wait( + tasks + [shutdown_task], + return_when=asyncio.FIRST_COMPLETED, ) + # 시그널이든 태스크 종료든 graceful shutdown 수행 + shutdown_task.cancel() + await _graceful_shutdown(bots, tasks) + if __name__ == "__main__": asyncio.run(main()) diff --git a/src/backtester.py b/src/backtester.py index a7e30ca..c157bf0 100644 --- a/src/backtester.py +++ b/src/backtester.py @@ -491,9 +491,8 @@ class Backtester: buy_side = "BUY" if signal == "LONG" else "SELL" entry_price = _apply_slippage(price, buy_side, self.cfg.slippage_pct) - # 수수료 + # 수수료 (청산 시 net_pnl에서 차감하므로 여기서 balance 차감하지 않음) entry_fee = _calc_fee(entry_price, quantity, self.cfg.fee_pct) - self.balance -= entry_fee # SL/TP 계산 atr = float(row.get("atr", 0)) diff --git a/src/bot.py b/src/bot.py index ced999a..a88716a 100644 --- a/src/bot.py +++ b/src/bot.py @@ -77,6 +77,7 @@ class TradingBot: self._entry_quantity: float | None = None self._is_reentering: bool = False # _close_and_reenter 중 콜백 상태 초기화 방지 self._close_event = asyncio.Event() # 콜백 청산 완료 대기용 + self._close_lock = asyncio.Lock() # 청산 처리 원자성 보장 (C3 fix) self._prev_oi: float | None = None # OI 변화율 계산용 이전 값 self._oi_history: deque = deque(maxlen=96) # z-score 윈도우(96=1일분 15분봉) self._funding_history: deque = deque(maxlen=96) @@ -459,20 +460,80 @@ class TradingBot: ) sl_side = "SELL" if signal == "LONG" else "BUY" - await self.exchange.place_order( - side=sl_side, - quantity=quantity, - order_type="STOP_MARKET", - stop_price=self.exchange._round_price(stop_loss), - reduce_only=True, - ) - await self.exchange.place_order( - side=sl_side, - quantity=quantity, - order_type="TAKE_PROFIT_MARKET", - stop_price=self.exchange._round_price(take_profit), - reduce_only=True, - ) + try: + await self._place_sl_tp_with_retry( + sl_side, quantity, stop_loss, take_profit + ) + except Exception as e: + logger.error( + f"[{self.symbol}] SL/TP 배치 최종 실패 — 긴급 청산: {e}" + ) + await self._emergency_close(side, quantity) + + _SL_TP_MAX_RETRIES = 3 + + async def _place_sl_tp_with_retry( + self, sl_side: str, quantity: float, stop_loss: float, take_profit: float + ) -> None: + """SL/TP 주문을 재시도 로직과 함께 배치한다. 최종 실패 시 예외를 raise.""" + sl_placed = False + tp_placed = False + last_error = None + + for attempt in range(1, self._SL_TP_MAX_RETRIES + 1): + try: + if not sl_placed: + await self.exchange.place_order( + side=sl_side, + quantity=quantity, + order_type="STOP_MARKET", + stop_price=self.exchange._round_price(stop_loss), + reduce_only=True, + ) + sl_placed = True + if not tp_placed: + await self.exchange.place_order( + side=sl_side, + quantity=quantity, + order_type="TAKE_PROFIT_MARKET", + stop_price=self.exchange._round_price(take_profit), + reduce_only=True, + ) + tp_placed = True + return # 둘 다 성공 + except Exception as e: + last_error = e + logger.warning( + f"[{self.symbol}] SL/TP 배치 실패 (시도 {attempt}/{self._SL_TP_MAX_RETRIES}): {e}" + ) + if attempt < self._SL_TP_MAX_RETRIES: + await asyncio.sleep(1) + + raise last_error # 모든 재시도 실패 + + async def _emergency_close(self, entry_side: str, quantity: float) -> None: + """SL/TP 배치 실패 시 포지션을 긴급 시장가 청산한다.""" + try: + close_side = "SELL" if entry_side == "BUY" else "BUY" + await self.exchange.cancel_all_orders() + await self.exchange.place_order( + side=close_side, quantity=quantity, reduce_only=True + ) + await self.risk.close_position(self.symbol, 0.0) + self.current_trade_side = None + self._entry_price = None + self._entry_quantity = None + self.notifier.notify_info( + f"🚨 [{self.symbol}] SL/TP 배치 실패 → 긴급 청산 완료" + ) + logger.warning(f"[{self.symbol}] 긴급 청산 완료") + except Exception as e: + logger.critical( + f"[{self.symbol}] 긴급 청산마저 실패! 수동 개입 필요: {e}" + ) + self.notifier.notify_info( + f"🔴 [{self.symbol}] 긴급 청산 실패! 수동 청산 필요: {e}" + ) def _calc_estimated_pnl(self, exit_price: float) -> float: """진입가·수량 기반 예상 PnL 계산 (수수료 미반영).""" @@ -489,47 +550,48 @@ class TradingBot: exit_price: float, ) -> None: """User Data Stream에서 청산 감지 시 호출되는 콜백.""" - # 이미 Flat 상태면 중복 처리 방지 (SYNC 또는 process_candle에서 먼저 처리됨) - if self.current_trade_side is None and not self._is_reentering: - logger.debug(f"[{self.symbol}] 이미 Flat 상태 — 콜백 건너뜀") + async with self._close_lock: + # 이미 Flat 상태면 중복 처리 방지 (SYNC 또는 process_candle에서 먼저 처리됨) + if self.current_trade_side is None and not self._is_reentering: + logger.debug(f"[{self.symbol}] 이미 Flat 상태 — 콜백 건너뜀") + self._close_event.set() + return + + estimated_pnl = self._calc_estimated_pnl(exit_price) + diff = net_pnl - estimated_pnl + + await self.risk.close_position(self.symbol, net_pnl) + + self.notifier.notify_close( + symbol=self.symbol, + side=self.current_trade_side or "UNKNOWN", + close_reason=close_reason, + exit_price=exit_price, + estimated_pnl=estimated_pnl, + net_pnl=net_pnl, + diff=diff, + ) + + logger.success( + f"[{self.symbol}] 포지션 청산({close_reason}): 예상={estimated_pnl:+.4f}, " + f"순수익={net_pnl:+.4f}, 차이={diff:+.4f} USDT" + ) + + # 거래 기록 저장 + 킬스위치 검사 (청산 후 항상 수행) + self._append_trade(net_pnl, close_reason) + self._check_kill_switch() + + # _close_and_reenter 대기 해제 self._close_event.set() - return - estimated_pnl = self._calc_estimated_pnl(exit_price) - diff = net_pnl - estimated_pnl + # _close_and_reenter 중이면 신규 포지션 상태를 덮어쓰지 않는다 + if self._is_reentering: + return - await self.risk.close_position(self.symbol, net_pnl) - - self.notifier.notify_close( - symbol=self.symbol, - side=self.current_trade_side or "UNKNOWN", - close_reason=close_reason, - exit_price=exit_price, - estimated_pnl=estimated_pnl, - net_pnl=net_pnl, - diff=diff, - ) - - logger.success( - f"[{self.symbol}] 포지션 청산({close_reason}): 예상={estimated_pnl:+.4f}, " - f"순수익={net_pnl:+.4f}, 차이={diff:+.4f} USDT" - ) - - # 거래 기록 저장 + 킬스위치 검사 (청산 후 항상 수행) - self._append_trade(net_pnl, close_reason) - self._check_kill_switch() - - # _close_and_reenter 대기 해제 - self._close_event.set() - - # _close_and_reenter 중이면 신규 포지션 상태를 덮어쓰지 않는다 - if self._is_reentering: - return - - # Flat 상태로 초기화 - self.current_trade_side = None - self._entry_price = None - self._entry_quantity = None + # Flat 상태로 초기화 + self.current_trade_side = None + self._entry_price = None + self._entry_quantity = None _MONITOR_INTERVAL = 300 # 5분 @@ -544,52 +606,56 @@ class TradingBot: try: actual_pos = await self.exchange.get_position() if actual_pos is None: - logger.warning( - f"[{self.symbol}] 포지션 불일치 감지: " - f"봇={self.current_trade_side}, 바이낸스=포지션 없음 — 상태 동기화" - ) - # Binance income API에서 실제 PnL 조회 - realized_pnl = 0.0 - commission = 0.0 - exit_price = 0.0 - try: - pnl_rows, comm_rows = await self.exchange.get_recent_income(limit=10) - if pnl_rows: - realized_pnl = sum(float(r.get("income", "0")) for r in pnl_rows) - if comm_rows: - commission = sum(abs(float(r.get("income", "0"))) for r in comm_rows) - except Exception: - pass - net_pnl = realized_pnl - commission - # exit_price 추정: 진입가 + PnL/수량 - if self._entry_quantity and self._entry_quantity > 0 and self._entry_price: - if self.current_trade_side == "LONG": - exit_price = self._entry_price + realized_pnl / self._entry_quantity - else: - exit_price = self._entry_price - realized_pnl / self._entry_quantity + async with self._close_lock: + # Lock 획득 후 재확인 (콜백이 먼저 처리했을 수 있음) + if self.current_trade_side is None: + continue + logger.warning( + f"[{self.symbol}] 포지션 불일치 감지: " + f"봇={self.current_trade_side}, 바이낸스=포지션 없음 — 상태 동기화" + ) + # Binance income API에서 실제 PnL 조회 + realized_pnl = 0.0 + commission = 0.0 + exit_price = 0.0 + try: + pnl_rows, comm_rows = await self.exchange.get_recent_income(limit=10) + if pnl_rows: + realized_pnl = sum(float(r.get("income", "0")) for r in pnl_rows) + if comm_rows: + commission = sum(abs(float(r.get("income", "0"))) for r in comm_rows) + except Exception: + pass + net_pnl = realized_pnl - commission + # exit_price 추정: 진입가 + PnL/수량 + if self._entry_quantity and self._entry_quantity > 0 and self._entry_price: + if self.current_trade_side == "LONG": + exit_price = self._entry_price + realized_pnl / self._entry_quantity + else: + exit_price = self._entry_price - realized_pnl / self._entry_quantity - await self.risk.close_position(self.symbol, net_pnl) - self.notifier.notify_close( - symbol=self.symbol, - side=self.current_trade_side, - close_reason="SYNC", - exit_price=exit_price, - estimated_pnl=realized_pnl, - net_pnl=net_pnl, - diff=net_pnl - realized_pnl, - ) - logger.info( - f"[{self.symbol}] 청산 감지(SYNC): exit={exit_price:.4f}, " - f"rp={realized_pnl:+.4f}, commission={commission:.4f}, " - f"net_pnl={net_pnl:+.4f}" - ) - self._append_trade(net_pnl, "SYNC") - self._check_kill_switch() - self.current_trade_side = None - self._entry_price = None - self._entry_quantity = None - self._close_event.set() - continue + await self.risk.close_position(self.symbol, net_pnl) + self.notifier.notify_close( + symbol=self.symbol, + side=self.current_trade_side, + close_reason="SYNC", + exit_price=exit_price, + estimated_pnl=realized_pnl, + net_pnl=net_pnl, + diff=net_pnl - realized_pnl, + ) + logger.info( + f"[{self.symbol}] 청산 감지(SYNC): exit={exit_price:.4f}, " + f"rp={realized_pnl:+.4f}, commission={commission:.4f}, " + f"net_pnl={net_pnl:+.4f}" + ) + self._append_trade(net_pnl, "SYNC") + self._check_kill_switch() + self.current_trade_side = None + self._entry_price = None + self._entry_quantity = None + self._close_event.set() + continue except Exception as e: logger.debug(f"[{self.symbol}] 포지션 동기화 확인 실패 (무시): {e}")