Skip to content

Conversation

@zhouyongyou
Copy link
Collaborator

The GetCurrentKlines function had two critical bugs causing price data to become stale and corrupt:

  1. Incorrect return logic: Returned error even when data fetch succeeded
  2. Race condition: Returned slice reference instead of deep copy, causing concurrent data corruption

💥 Impact

  • BTC price stuck at 106xxx while actual market was 107xxx+ ❌
  • LLM calculated take-profit based on stale prices → orders failed validation ❌
  • Statistics showed incorrect P&L (0.00%) due to corrupted data ❌
  • Alt-coins filtered out due to failed market data fetch ❌

✅ Solution

  1. Fixed return logic: only return error when actual failure occurs
  2. Return deep copy instead of reference to prevent race conditions
  3. Downgrade subscription errors to warnings (non-blocking)

🧪 Test Results

  • ✅ Price updates in real-time
  • ✅ Take-profit orders execute successfully
  • ✅ P&L calculations accurate
  • ✅ Alt-coins now tradeable

📁 Files Changed

  • market/monitor.go - GetCurrentKlines function (20 additions, 7 deletions)

🔬 Technical Details

Before (Buggy Code)

if !exists {
    klines, err := apiClient.GetKlines(symbol, _time, 100)
    m.getKlineDataMap(_time).Store(strings.ToUpper(symbol), klines)
    // ... subscription logic ...
    return klines, fmt.Errorf("symbol不存在") // ❌ Returns error even on success!
}
return value.([]Kline), nil // ❌ Returns shared reference, not copy!

After (Fixed Code)

if !exists {
    klines, err := apiClient.GetKlines(symbol, _time, 100)
    if err != nil {
        return nil, fmt.Errorf("获取%v分钟K线失败: %v", _time, err)
    }
    m.getKlineDataMap(_time).Store(strings.ToUpper(symbol), klines)
    // ... subscription logic (errors downgraded to warnings) ...

    // ✅ Return deep copy
    result := make([]Kline, len(klines))
    copy(result, klines)
    return result, nil
}

// ✅ Return deep copy to prevent race conditions
klines := value.([]Kline)
result := make([]Kline, len(klines))
copy(result, klines)
return result, nil

🔗 Related Issues

Fixes price feed mechanism, concurrent data access

📊 Impact Chain

GetCurrentKlines returns stale data
    ↓
market.Get() reads fixed price (106xxx)
    ↓
Passed to LLM as CurrentPrice
    ↓
1. LLM calculates take-profit based on old price → validation fails ❌
2. Market data fetch fails → alt-coins filtered out ❌
3. Historical records use wrong prices → P&L shows 0.00% ❌

⚠️ Severity: Critical

This bug affects:

  • Trading execution (orders fail validation)
  • Risk management (incorrect stop-loss/take-profit)
  • Performance tracking (corrupted statistics)
  • Asset selection (alt-coins excluded)

Recommend immediate merge and deployment.

@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: 🟢 Small (27 lines: +20 -7)

🔧 Backend Checks

Go Formatting: ✅ Good
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.

## Problem
GetCurrentKlines had two critical bugs causing price data to become stale:
1. Incorrect return logic: returned error even when data fetch succeeded
2. Race condition: returned slice reference instead of deep copy, causing concurrent data corruption

## Impact
- BTC price stuck at 106xxx while actual market price was 107xxx+
- LLM calculated take-profit based on stale prices → orders failed validation
- Statistics showed incorrect P&L (0.00%) due to corrupted historical data
- Alt-coins filtered out due to failed market data fetch

## Solution
1. Fixed return logic: only return error when actual failure occurs
2. Return deep copy instead of reference to prevent race conditions
3. Downgrade subscription errors to warnings (non-blocking)

## Test Results
✅ Price updates in real-time
✅ Take-profit orders execute successfully
✅ P&L calculations accurate
✅ Alt-coins now tradeable

Related: Price feed mechanism, concurrent data access
@zhouyongyou zhouyongyou force-pushed the fix/market-price-staleness branch from d1838a4 to b721002 Compare November 4, 2025 09:21
@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: 🟢 Small (27 lines: +20 -7)

🔧 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 6c78d9d into NoFxAiOS:dev Nov 5, 2025
8 of 10 checks passed
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