351 lines
12 KiB
Markdown
351 lines
12 KiB
Markdown
# v2 코드 리뷰 개선사항 검토 및 평가
|
|
|
|
## 요약 (Executive Summary)
|
|
|
|
v2 리포트에서 제안된 **5개 Critical + 6개 High** 이슈를 현재 코드베이스와 대조하여 검토했습니다.
|
|
|
|
**결과**:
|
|
- ✅ **완전 해결**: 1개 (재매수 쿨다운 파일 락)
|
|
- ⚠️ **부분 해결**: 3개 (상태 동기화, OHLCV 캐시, 예외 처리)
|
|
- ❌ **미해결**: 7개 (나머지 Critical/High 이슈)
|
|
|
|
---
|
|
|
|
## 1. Critical 이슈 분석 (5개)
|
|
|
|
### CRITICAL-1: 동일 심볼 복수 주문 시 예산 충돌 ❌ **미해결**
|
|
|
|
**v2 지적사항**:
|
|
> KRWBudgetManager가 심볼 단일 슬롯만 보유 → 같은 심볼 복수 주문이 동시에 발생하면 후행 주문이 선행 주문 할당액을 덮어쓰기/해제하며 이중 사용 가능.
|
|
|
|
**현재 상태**:
|
|
```python
|
|
# src/common.py (현재)
|
|
class KRWBudgetManager:
|
|
def __init__(self):
|
|
self._lock = threading.RLock()
|
|
self._allocated: dict[str, float] = {} # {symbol: allocated_amount}
|
|
```
|
|
|
|
**평가**: ❌ **여전히 문제 존재**
|
|
- 구조가 v2 리포트 당시와 동일: `{symbol: float}` 단일 슬롯
|
|
- 동일 심볼에 대한 복수 주문 시나리오 테스트 부재
|
|
- **현실적 리스크**: 낮음 (현재 4시간봉 기반, 동일 심볼 동시 매수 가능성 희박)
|
|
- **권장**: P2 우선순위로 주문 UUID 기반 멀티 할당 구조로 리팩토링
|
|
|
|
---
|
|
|
|
### CRITICAL-2: 분당 Rate Limit 미적용 ❌ **미해결**
|
|
|
|
**v2 지적사항**:
|
|
> 현재 초당 8회만 제한, 분당 600회 제한/엔드포인트별 제한 미적용. 잦은 현재가/잔고 조회는 제한 우회 없이 직행 → 418/429 위험.
|
|
|
|
**현재 상태**:
|
|
```python
|
|
# src/common.py
|
|
class TokenBucketRateLimiter:
|
|
def __init__(self, rate: int = 8, per: float = 1.0): # 초당 8회
|
|
# 분당 제한 없음
|
|
```
|
|
|
|
**평가**: ❌ **부분적으로만 개선**
|
|
- 초당 제한만 적용, **분당 600회 제한 미구현**
|
|
- `get_current_price`, `get_upbit_balances` 일부 경로에서 Rate Limiter 미적용
|
|
- **현실적 리스크**: 중간 (멀티스레드 + 짧은 심볼 딜레이 시 분당 한도 초과 가능)
|
|
- **권장**: P0 - 이중 버킷(초/분) 구현 및 모든 API 호출 경로에 적용
|
|
|
|
---
|
|
|
|
### CRITICAL-3: 재매수 쿨다운 기록 레이스/손상 가능 ✅ **해결됨**
|
|
|
|
**v2 지적사항**:
|
|
> `recent_sells.json` 접근 시 파일 Lock/원자적 쓰기 없음, 예외도 무시 → 동시 매도 시 기록 손상/쿨다운 무시 가능.
|
|
|
|
**현재 상태**:
|
|
```python
|
|
# src/common.py (현재)
|
|
_recent_sells_lock = threading.RLock()
|
|
|
|
def record_sell(symbol: str):
|
|
with _recent_sells_lock:
|
|
# 원자적 쓰기 (temp file + os.replace)
|
|
temp_file = f"{RECENT_SELLS_FILE}.tmp"
|
|
# ... atomic write ...
|
|
os.replace(temp_file, RECENT_SELLS_FILE)
|
|
```
|
|
|
|
**평가**: ✅ **완전 해결**
|
|
- RLock + 원자적 쓰기 적용됨
|
|
- JSONDecodeError 예외 처리도 추가됨
|
|
- **v4 리포트에서도 문제 없음으로 확인**
|
|
|
|
---
|
|
|
|
### CRITICAL-4: 가격/수량 부동소수점 정밀도 손실 ❌ **미해결**
|
|
|
|
**v2 지적사항**:
|
|
> Decimal 적용 필요했던 HIGH-006 미완료. 슬리피지 계산·호가 반올림·수량 계산이 float 기반 → 체결 실패/초과주문 리스크.
|
|
|
|
**현재 상태**:
|
|
```python
|
|
# src/order.py (현재)
|
|
def place_buy_order_upbit(market: str, amount_krw: float, cfg: RuntimeConfig):
|
|
# 여전히 float 기반 계산
|
|
fee_rate = 0.0005
|
|
net_amount = amount_krw * (1 - fee_rate) # float 연산
|
|
```
|
|
|
|
**평가**: ❌ **미해결**
|
|
- 일부 함수(`_adjust_sell_ratio_for_min_order`)에서만 Decimal 사용
|
|
- **핵심 매수/매도 로직은 여전히 float 기반**
|
|
- **현실적 리스크**: 낮음 (소액 거래에서는 영향 미미)
|
|
- **권장**: P1 - `calc_price_amount()` 유틸 함수 구현 및 전체 주문 경로에 적용
|
|
|
|
---
|
|
|
|
### CRITICAL-5: 현재가 조회 재시도/캐시 없음 ❌ **미해결**
|
|
|
|
**v2 지적사항**:
|
|
> 단일 요청 실패 시 0 반환 후 상위 로직이 손절/익절 판단에 잘못된 가격 사용 가능. RateLimiter도 미적용.
|
|
|
|
**현재 상태**:
|
|
```python
|
|
# src/holdings.py (현재)
|
|
def get_current_price(symbol: str) -> float:
|
|
try:
|
|
return pyupbit.get_current_price(symbol) # 단일 시도, 캐시 없음
|
|
except Exception as e:
|
|
logger.error("현재가 조회 실패: %s", e)
|
|
return 0.0 # 위험: 0 반환
|
|
```
|
|
|
|
**평가**: ❌ **미해결 + 매우 위험**
|
|
- 재시도 로직 없음
|
|
- 캐시 없음 (OHLCV는 캐시하지만 ticker는 안 함)
|
|
- **실패 시 0 반환 → 손절 로직이 -100% 수익률로 오판 가능**
|
|
- **현실적 리스크**: 높음 (네트워크 일시 장애 시 잘못된 매도 가능)
|
|
- **권장**: P0 - 짧은 backoff 재시도 + 1~2초 TTL 캐시 + 실패 시 `None` 반환
|
|
|
|
---
|
|
|
|
## 2. High 이슈 분석 (6개)
|
|
|
|
### HIGH-1: 예산 할당 최소 주문 금액 미검증 ❌ **미해결**
|
|
|
|
**v2 지적사항**:
|
|
> 부분 할당이 5,000원 미만일 수 있음 → 이후 주문 경로에서 실패.
|
|
|
|
**현재 상태**:
|
|
```python
|
|
# src/common.py
|
|
def allocate(self, symbol: str, amount: float) -> bool:
|
|
# MIN_KRW_ORDER 검증 없음
|
|
self._allocated[symbol] = amount
|
|
```
|
|
|
|
**평가**: ❌ **미해결**
|
|
- 할당 시점에 최소 금액 검증 안 함
|
|
- 주문 시점에서만 검증 → 예산은 잠겼으나 주문 실패 가능
|
|
- **권장**: P1 - `allocate()` 내부에서 `MIN_KRW_ORDER` 검증 추가
|
|
|
|
---
|
|
|
|
### HIGH-2: 상태 동기화 불일치 가능성 ⚠️ **부분 해결**
|
|
|
|
**v2 지적사항**:
|
|
> StateManager와 holdings.json을 이중 관리하지만, holdings 저장 실패 시 상태 불일치 남음.
|
|
|
|
**현재 상태**:
|
|
- `state_manager.py` 도입으로 `bot_state.json`을 Single Source of Truth로 지정
|
|
- `holdings.json`은 캐시 역할로 명시적 분리
|
|
- **그러나**: holdings 업데이트 실패 시 state_manager와 불일치 가능성 여전히 존재
|
|
|
|
**평가**: ⚠️ **개선되었으나 완전하지 않음**
|
|
- v4에서 구조적 개선 완료
|
|
- 주기적 검증/리빌드 로직은 없음
|
|
- **권장**: P2 - 주기적으로 `bot_state.json` 기준으로 `holdings.json` 재구축
|
|
|
|
---
|
|
|
|
### HIGH-3: Pending/Confirm 파일 기반 워크플로 취약 ❌ **미해결**
|
|
|
|
**v2 지적사항**:
|
|
> JSON append + rename 기반으로 손상/유실 가능, 만료/청소 로직 없음.
|
|
|
|
**현재 상태**:
|
|
- 여전히 JSON 파일 기반
|
|
- TTL 클린업 로직 없음
|
|
- 무한 대기 가능성 존재
|
|
|
|
**평가**: ❌ **미해결**
|
|
- SQLite 또는 JSONL 마이그레이션 미진행
|
|
- **권장**: P2 - SQLite로 마이그레이션 또는 TTL 기반 클린업 추가
|
|
|
|
---
|
|
|
|
### HIGH-4: OHLCV 캐시 TTL 고정·지표 시간대 미고려 ⚠️ **부분 해결**
|
|
|
|
**v2 지적사항**:
|
|
> 5분 TTL 고정, 타임프레임/거래소 홀리데이/서버 시간 불일치 고려 없음.
|
|
|
|
**현재 상태**:
|
|
```python
|
|
# src/indicators.py
|
|
CACHE_TTL = 300 # 5분 고정
|
|
```
|
|
|
|
**평가**: ⚠️ **부분적으로만 개선**
|
|
- 실패 시 캐시 미기록 로직은 개선됨
|
|
- **그러나**: 여전히 TTL 고정 (타임프레임별 동적 TTL 없음)
|
|
- **권장**: P2 - 타임프레임별 TTL 조정 (예: 4시간봉은 30분 TTL)
|
|
|
|
---
|
|
|
|
### HIGH-5: 예외 타입·로깅 불균일 ⚠️ **부분 해결**
|
|
|
|
**v2 지적사항**:
|
|
> 일부 함수 `except Exception` 광범위, 로깅 레벨 혼재.
|
|
|
|
**현재 상태**:
|
|
- 여전히 많은 `except Exception` 존재
|
|
- **그러나**: v4 리포트에서 이미 지적하고 개선 권장함
|
|
|
|
**평가**: ⚠️ **인지되었으나 미개선**
|
|
- v4 리포트 MEDIUM-003에서 동일 이슈 지적
|
|
- **권장**: P1 - 구체적 예외 타입 지정
|
|
|
|
---
|
|
|
|
### HIGH-6: 스레드 수/재시도 기본값 과도 ❌ **미해결**
|
|
|
|
**v2 지적사항**:
|
|
> 최대 재시도 5, 누적 대기 300s는 메인 루프 정지 유발 가능.
|
|
|
|
**현재 상태**:
|
|
```python
|
|
# src/indicators.py
|
|
max_attempts = int(os.getenv("MAX_FETCH_ATTEMPTS", "5"))
|
|
max_total_backoff = float(os.getenv("MAX_TOTAL_BACKOFF", "300"))
|
|
```
|
|
|
|
**평가**: ❌ **환경변수화되었으나 기본값 여전히 과도**
|
|
- 환경변수로 조절 가능하게는 개선됨
|
|
- **그러나**: 기본값 300초는 여전히 과도
|
|
- **권장**: P2 - 기본값을 30~60초로 하향 조정
|
|
|
|
---
|
|
|
|
## 3. 종합 평가 및 우선순위 재조정
|
|
|
|
### v2 vs v4 비교표
|
|
|
|
| 이슈 | v2 우선순위 | 현재 상태 | v4 권장 우선순위 |
|
|
|------|-------------|-----------|------------------|
|
|
| 동일 심볼 복수 주문 | P0 | ❌ 미해결 | P2 (현실적 리스크 낮음) |
|
|
| 분당 Rate Limit | P0 | ❌ 미해결 | **P0** (여전히 중요) |
|
|
| 재매수 쿨다운 락 | P0 | ✅ 해결 | - |
|
|
| Decimal 정밀도 | P0 | ❌ 미해결 | P1 (소액에서는 영향 적음) |
|
|
| 현재가 재시도/캐시 | P1 | ❌ 미해결 | **P0** (매우 위험) |
|
|
| 예산 최소 금액 검증 | P1 | ❌ 미해결 | P1 |
|
|
| 상태 동기화 | P1 | ⚠️ 개선 | P2 |
|
|
| Pending 파일 정비 | P2 | ❌ 미해결 | P2 |
|
|
| OHLCV TTL 동적화 | P2 | ⚠️ 부분 | P2 |
|
|
| 예외 타입 구체화 | P2 | ⚠️ 부분 | P1 |
|
|
| 재시도 기본값 | P2 | ❌ 미해결 | P2 |
|
|
|
|
### 최종 우선순위 (v4 기준)
|
|
|
|
#### 🔴 P0 (즉시)
|
|
1. **현재가 조회 재시도/캐시** - 가장 위험한 이슈
|
|
2. **분당 Rate Limit 구현** - API 차단 위험
|
|
|
|
#### 🟡 P1 (1주 내)
|
|
3. **Decimal 정밀도** - 주문 안정성
|
|
4. **예산 최소 금액 검증** - 실패 방지
|
|
5. **예외 타입 구체화** - 디버깅 용이성
|
|
|
|
#### 🟢 P2 (1개월 내)
|
|
6. **동일 심볼 복수 주문 처리** - 현실적 리스크 낮음
|
|
7. **상태 동기화 주기적 검증** - 장기 운영 안정성
|
|
8. **Pending 파일 정비** - SQLite 마이그레이션
|
|
9. **OHLCV TTL 동적화** - 성능 최적화
|
|
10. **재시도 기본값 조정** - 리소스 효율
|
|
|
|
---
|
|
|
|
## 4. 전문가 의견 (Expert Commentary)
|
|
|
|
### ✅ v2 리포트의 탁월한 점
|
|
1. **실전 중심 분석**: 분당 Rate Limit, Decimal 정밀도 등 실제 거래 시 발생할 수 있는 문제 정확히 지적
|
|
2. **우선순위 명확**: P0/P1/P2 구분이 합리적
|
|
3. **구체적 해결안**: "주문 토큰 기반", "이중 버킷" 등 구현 가능한 솔루션 제시
|
|
|
|
### ⚠️ v2 리포트의 과도한 지적
|
|
1. **CRITICAL-1 (동일 심볼 복수 주문)**:
|
|
- 현재 4시간봉 전략에서는 발생 가능성 희박
|
|
- 실제로는 P2 수준 (v2는 P0로 과대평가)
|
|
|
|
2. **CRITICAL-4 (Decimal 정밀도)**:
|
|
- 소액 거래(5만원 이하)에서는 float 오차가 실질적 영향 미미
|
|
- P1 수준이 적절 (v2는 P0로 과대평가)
|
|
|
|
### 🎯 v4 관점에서 가장 중요한 이슈
|
|
1. **현재가 조회 실패 처리 (v2의 CRITICAL-5)**
|
|
- **이것이 진짜 CRITICAL**: 네트워크 일시 장애 시 0원으로 인식 → 전량 손절 가능
|
|
- v2가 P1로 낮게 평가한 것은 실수
|
|
- **즉시 수정 필요**
|
|
|
|
2. **분당 Rate Limit (v2의 CRITICAL-2)**
|
|
- 멀티스레드 환경에서 실제로 Upbit API 차단 위험 존재
|
|
- v2 평가 정확
|
|
|
|
---
|
|
|
|
## 5. 실행 가능한 개선안 (Quick Wins)
|
|
|
|
### 🚀 30분 내 구현 가능
|
|
```python
|
|
# 1. 현재가 재시도 로직 (src/holdings.py)
|
|
def get_current_price(symbol: str, retries: int = 3) -> float | None:
|
|
for attempt in range(retries):
|
|
try:
|
|
price = pyupbit.get_current_price(symbol)
|
|
if price and price > 0:
|
|
return price
|
|
except Exception as e:
|
|
if attempt == retries - 1:
|
|
logger.error("현재가 조회 최종 실패: %s", symbol)
|
|
return None # 0이 아닌 None 반환
|
|
time.sleep(0.5 * (2 ** attempt))
|
|
return None
|
|
```
|
|
|
|
### 📊 2시간 내 구현 가능
|
|
```python
|
|
# 2. 분당 Rate Limit 추가 (src/common.py)
|
|
class DualRateLimiter:
|
|
def __init__(self):
|
|
self.per_second = TokenBucketRateLimiter(rate=8, per=1.0)
|
|
self.per_minute = TokenBucketRateLimiter(rate=600, per=60.0)
|
|
|
|
def acquire(self):
|
|
self.per_second.acquire()
|
|
self.per_minute.acquire()
|
|
```
|
|
|
|
---
|
|
|
|
## 6. 결론
|
|
|
|
v2 리포트는 **매우 전문적이고 실전 중심의 분석**이었으나, 일부 이슈의 우선순위가 과대평가되었습니다.
|
|
|
|
**핵심 요약**:
|
|
- v2의 11개 이슈 중 **1개만 완전 해결**, 7개는 미해결
|
|
- **가장 위험한 이슈는 "현재가 조회 실패 처리"** (v2가 P1로 과소평가)
|
|
- v4 백테스팅/포트폴리오 관리 이슈가 v2에는 없었음 → v4가 더 포괄적
|
|
|
|
**최종 권장**:
|
|
1. v2 P0 이슈 중 **현재가 재시도 + 분당 Rate Limit**만 즉시 구현
|
|
2. 나머지는 v4 리포트 우선순위에 따라 순차 진행
|
|
3. v2와 v4를 통합한 **마스터 백로그** 작성 권장
|