Skip to content

Conversation

@the-dev-z
Copy link
Collaborator

Problem

When adjusting stop-loss or take-profit levels, CancelStopOrders() deleted BOTH stop-loss AND take-profit orders simultaneously, causing:

  • Adjusting stop-loss → Take-profit order deleted → Position has no exit plan ❌
  • Adjusting take-profit → Stop-loss order deleted → Position unprotected ❌

Root cause:

CancelStopOrders(symbol) {
  // Cancelled ALL orders with type STOP_MARKET or TAKE_PROFIT_MARKET
  // No distinction between stop-loss and take-profit
}

Solution

1. Added new interface methods (trader/interface.go)

CancelStopLossOrders(symbol string) error      // Only cancel stop-loss orders
CancelTakeProfitOrders(symbol string) error    // Only cancel take-profit orders
CancelStopOrders(symbol string) error          // Deprecated (cancels both)

2. Implemented for all 3 exchanges

Binance (trader/binance_futures.go):

  • CancelStopLossOrders: Filters OrderTypeStopMarket | OrderTypeStop
  • CancelTakeProfitOrders: Filters OrderTypeTakeProfitMarket | OrderTypeTakeProfit
  • Full order type differentiation ✅

Hyperliquid (trader/hyperliquid_trader.go):

  • ⚠️ Limitation: SDK's OpenOrder struct doesn't expose trigger field
  • Both methods call CancelStopOrders (cancels all pending orders)
  • Trade-off: Safe but less precise

Aster (trader/aster_trader.go):

  • CancelStopLossOrders: Filters STOP_MARKET | STOP
  • CancelTakeProfitOrders: Filters TAKE_PROFIT_MARKET | TAKE_PROFIT
  • Full order type differentiation ✅

3. Usage in auto_trader.go

When update_stop_loss or update_take_profit actions are implemented, they will use:

// update_stop_loss:
at.trader.CancelStopLossOrders(symbol)  // Only cancel SL, keep TP
at.trader.SetStopLoss(...)

// update_take_profit:
at.trader.CancelTakeProfitOrders(symbol)  // Only cancel TP, keep SL
at.trader.SetTakeProfit(...)

Impact

  • ✅ Adjusting stop-loss no longer deletes take-profit
  • ✅ Adjusting take-profit no longer deletes stop-loss
  • ✅ Backward compatible: CancelStopOrders still exists (deprecated)
  • ⚠️ Hyperliquid limitation: still cancels all orders (SDK constraint)

Testing

  • ✅ Compiles successfully across all 3 exchanges
  • ⚠️ Requires live testing:
    • Binance: Adjust SL → verify TP remains
    • Binance: Adjust TP → verify SL remains
    • Hyperliquid: Verify behavior with limitation
    • Aster: Verify order filtering works correctly

Code Changes

trader/interface.go: +9 lines (new interface methods)
trader/binance_futures.go: +133 lines (3 new functions)
trader/hyperliquid_trader.go: +56 lines (3 new functions)
trader/aster_trader.go: +157 lines (3 new functions)
Total: +355 lines

📝 Description | 描述

English:
Fixed critical bug where adjusting stop-loss or take-profit levels would delete BOTH orders simultaneously, leaving positions unprotected or without exit plans.

中文:
修復關鍵 bug:調整止損或止盈價格時會同時刪除兩種訂單,導致持倉失去保護或沒有退出計劃。


🎯 Type of Change | 變更類型

  • 🐛 Bug fix | 修復 Bug(不影響現有功能的修復)
  • ✨ New feature | 新功能
  • 💥 Breaking change | 破壞性變更

🔗 Related Issues | 相關 Issue

  • Fixes accidental deletion of take-profit when adjusting stop-loss
  • Fixes accidental deletion of stop-loss when adjusting take-profit
  • Related to update_stop_loss and update_take_profit actions

📋 Changes Made | 具體變更

English:

1. Added new interface methods (trader/interface.go)

CancelStopLossOrders(symbol string) error      // Only cancel stop-loss orders
CancelTakeProfitOrders(symbol string) error    // Only cancel take-profit orders
CancelStopOrders(symbol string) error          // Deprecated (cancels both)

2. Implemented for all 3 exchanges

Binance (trader/binance_futures.go):

  • CancelStopLossOrders: Filters OrderTypeStopMarket | OrderTypeStop
  • CancelTakeProfitOrders: Filters OrderTypeTakeProfitMarket | OrderTypeTakeProfit
  • Full order type differentiation ✅

Hyperliquid (trader/hyperliquid_trader.go):

  • ⚠️ Limitation: SDK's OpenOrder struct doesn't expose trigger field
  • Both methods call CancelStopOrders (cancels all pending orders)
  • Trade-off: Safe but less precise

Aster (trader/aster_trader.go):

  • CancelStopLossOrders: Filters STOP_MARKET | STOP
  • CancelTakeProfitOrders: Filters TAKE_PROFIT_MARKET | TAKE_PROFIT
  • Full order type differentiation ✅

3. Usage in auto_trader.go

When update_stop_loss or update_take_profit actions are implemented:

// update_stop_loss:
at.trader.CancelStopLossOrders(symbol)  // Only cancel SL, keep TP ✅
at.trader.SetStopLoss(...)

// update_take_profit:
at.trader.CancelTakeProfitOrders(symbol)  // Only cancel TP, keep SL ✅
at.trader.SetTakeProfit(...)

中文:

1. 新增接口方法 (trader/interface.go)

CancelStopLossOrders(symbol string) error      // 僅取消止損單
CancelTakeProfitOrders(symbol string) error    // 僅取消止盈單
CancelStopOrders(symbol string) error          // 已廢棄(同時取消兩者)

2. 為所有 3 個交易所實現

Binance (trader/binance_futures.go):

  • CancelStopLossOrders: 過濾 OrderTypeStopMarket | OrderTypeStop
  • CancelTakeProfitOrders: 過濾 OrderTypeTakeProfitMarket | OrderTypeTakeProfit
  • 完整的訂單類型區分 ✅

Hyperliquid (trader/hyperliquid_trader.go):

  • ⚠️ 限制: SDK 的 OpenOrder 結構不公開 trigger 字段
  • 兩個方法都調用 CancelStopOrders(取消所有掛單)
  • 權衡:安全但精度較低

Aster (trader/aster_trader.go):

  • CancelStopLossOrders: 過濾 STOP_MARKET | STOP
  • CancelTakeProfitOrders: 過濾 TAKE_PROFIT_MARKET | TAKE_PROFIT
  • 完整的訂單類型區分 ✅

3. 在 auto_trader.go 中使用

當實現 update_stop_lossupdate_take_profit 動作時:

// update_stop_loss:
at.trader.CancelStopLossOrders(symbol)  // 只取消止損,保留止盈 ✅
at.trader.SetStopLoss(...)

// update_take_profit:
at.trader.CancelTakeProfitOrders(symbol)  // 只取消止盈,保留止損 ✅
at.trader.SetTakeProfit(...)

🧪 Testing | 測試

Test Scenarios | 測試場景

Before Fix:

1. Position has SL @ $95 and TP @ $105
2. Adjust SL to $97 → CancelStopOrders()
3. Result: Both SL and TP deleted ❌
4. Position now has NO exit plan ❌

After Fix:

1. Position has SL @ $95 and TP @ $105
2. Adjust SL to $97 → CancelStopLossOrders()
3. Result: Only SL deleted, TP @ $105 remains ✅
4. Set new SL @ $97
5. Position protected with both SL and TP ✅

Test Environment | 測試環境

  • OS | 操作系統: macOS
  • Go Version | Go 版本: 1.21+
  • Exchange | 交易所: Binance, Hyperliquid, Aster

Test Results | 測試結果

✅ go build ./trader/... - Compilation successful (all 3 exchanges)
⚠️  Requires live testing:
  - [ ] Binance: Adjust SL → verify TP remains
  - [ ] Binance: Adjust TP → verify SL remains
  - [ ] Hyperliquid: Verify behavior with SDK limitation
  - [ ] Aster: Verify order filtering works correctly

✅ Checklist | 檢查清單

Code Quality | 代碼質量

  • My code follows the project's code style | 遵循項目代碼風格
  • I have performed a self-review of my code | 已進行代碼自查
  • I have commented my code | 已添加代碼注釋
  • My changes generate no new warnings or errors | 沒有新警告或錯誤
  • Code compiles successfully | 代碼編譯成功

Testing | 測試

  • I have added tests | 已添加測試
  • New and existing unit tests pass locally | 單元測試通過
  • I have tested on testnet | 已在測試網測試(待驗證)

Git

  • My commits follow the conventional commits format | 遵循 Conventional Commits (fix(trader):)
  • I have rebased my branch on the latest dev branch | 已 rebase 到 dev
  • There are no merge conflicts | 沒有合併衝突

📋 PR Size Estimate | PR 大小估計

  • 🟢 Small (< 100 lines)
  • 🟡 Medium (100-500 lines)
  • 🔴 Large (> 500 lines) | 大(+346 lines across 4 files)

🎯 Review Focus Areas | 審查重點

Please pay special attention to:
請特別注意:

  • Logic changes | 邏輯變更
  • Security implications | 安全影響
  • API changes | API 變更

Specific areas:

  • Interface method signatures (trader/interface.go:43-47)
  • Order type filtering in Binance implementation (trader/binance_futures.go:478, 520)
  • Hyperliquid SDK limitation handling (trader/hyperliquid_trader.go:515-527)
  • Aster order filtering (trader/aster_trader.go:1052, 1102)

⚠️ Known Limitations | 已知限制

Hyperliquid:

  • SDK does not expose order trigger type
  • Both CancelStopLossOrders and CancelTakeProfitOrders cancel ALL pending orders
  • This is a safe fallback but less precise than Binance/Aster
  • Future improvement: Enhance SDK or use alternative order identification

By submitting this PR, I confirm that:

  • I have read the Contributing Guidelines
  • My contribution is licensed under the AGPL-3.0 License
  • I understand this is a voluntary contribution
  • I have the right to submit this code

… prevent accidental deletions

## Problem
When adjusting stop-loss or take-profit levels, `CancelStopOrders()` deleted BOTH stop-loss AND take-profit orders simultaneously, causing:
- **Adjusting stop-loss** → Take-profit order deleted → Position has no exit plan ❌
- **Adjusting take-profit** → Stop-loss order deleted → Position unprotected ❌

**Root cause:**
```go
CancelStopOrders(symbol) {
  // Cancelled ALL orders with type STOP_MARKET or TAKE_PROFIT_MARKET
  // No distinction between stop-loss and take-profit
}
```

## Solution

### 1. Added new interface methods (trader/interface.go)
```go
CancelStopLossOrders(symbol string) error      // Only cancel stop-loss orders
CancelTakeProfitOrders(symbol string) error    // Only cancel take-profit orders
CancelStopOrders(symbol string) error          // Deprecated (cancels both)
```

### 2. Implemented for all 3 exchanges

**Binance (trader/binance_futures.go)**:
- `CancelStopLossOrders`: Filters `OrderTypeStopMarket | OrderTypeStop`
- `CancelTakeProfitOrders`: Filters `OrderTypeTakeProfitMarket | OrderTypeTakeProfit`
- Full order type differentiation ✅

**Hyperliquid (trader/hyperliquid_trader.go)**:
- ⚠️ Limitation: SDK's OpenOrder struct doesn't expose trigger field
- Both methods call `CancelStopOrders` (cancels all pending orders)
- Trade-off: Safe but less precise

**Aster (trader/aster_trader.go)**:
- `CancelStopLossOrders`: Filters `STOP_MARKET | STOP`
- `CancelTakeProfitOrders`: Filters `TAKE_PROFIT_MARKET | TAKE_PROFIT`
- Full order type differentiation ✅

### 3. Usage in auto_trader.go
When `update_stop_loss` or `update_take_profit` actions are implemented, they will use:
```go
// update_stop_loss:
at.trader.CancelStopLossOrders(symbol)  // Only cancel SL, keep TP
at.trader.SetStopLoss(...)

// update_take_profit:
at.trader.CancelTakeProfitOrders(symbol)  // Only cancel TP, keep SL
at.trader.SetTakeProfit(...)
```

## Impact
- ✅ Adjusting stop-loss no longer deletes take-profit
- ✅ Adjusting take-profit no longer deletes stop-loss
- ✅ Backward compatible: `CancelStopOrders` still exists (deprecated)
- ⚠️ Hyperliquid limitation: still cancels all orders (SDK constraint)

## Testing
- ✅ Compiles successfully across all 3 exchanges
- ⚠️ Requires live testing:
  - [ ] Binance: Adjust SL → verify TP remains
  - [ ] Binance: Adjust TP → verify SL remains
  - [ ] Hyperliquid: Verify behavior with limitation
  - [ ] Aster: Verify order filtering works correctly

## Code Changes
```
trader/interface.go: +9 lines (new interface methods)
trader/binance_futures.go: +133 lines (3 new functions)
trader/hyperliquid_trader.go: +56 lines (3 new functions)
trader/aster_trader.go: +157 lines (3 new functions)
Total: +355 lines
```
@github-actions
Copy link

github-actions bot commented Nov 4, 2025

🤖 Advisory Check Results

These are advisory checks to help improve code quality. They won't block your PR from being merged.

📋 PR Information

Title Format: ✅ Good - Follows Conventional Commits
PR Size: 🟡 Medium (346 lines: +346 -0)

🔧 Backend Checks

Go Formatting: ⚠️ Needs formatting

Files needing formatting
api/server.go
config/database.go
main.go
manager/trader_manager.go

Go Vet: ✅ Good
Tests: ✅ Passed

Fix locally:

go fmt ./...      # Format code
go vet ./...      # Check for issues
go test ./...     # Run tests

⚛️ Frontend Checks

Build & Type Check: ✅ Success

Fix locally:

cd web
npm run build  # Test build (includes type checking)

📖 Resources

Questions? Feel free to ask in the comments! 🙏


These checks are advisory and won't block your PR from being merged. This comment is automatically generated from pr-checks-run.yml.

@Icyoung Icyoung merged commit 70fa073 into NoFxAiOS:dev Nov 5, 2025
8 of 10 checks passed
ForeverInLaw pushed a commit to ForeverInLaw/nofx that referenced this pull request Nov 8, 2025
…rofit-separation

fix(trader): separate stop-loss and take-profit order cancellation to prevent accidental deletions
@the-dev-z the-dev-z deleted the fix/stop-loss-take-profit-separation branch November 12, 2025 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants