mirror of
https://github.com/laoxong/nofx.git
synced 2026-06-04 01:48:22 +08:00
fix(agent): address critical issues from PR #1485 review
1. Prevent double-close panic on Stop() by using sync.Once in Scheduler, Brain, and Sentinel; remove duplicate Stop() call in main.go 2. Add trade quantity (100k) and leverage (125x) sanity caps to prevent LLM hallucinations or input errors from reaching the exchange 3. Mask secrets in onboarding setup state — only store "****" markers in SystemConfig instead of plaintext API keys/secrets Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
+2
-1
@@ -17,6 +17,7 @@ type Brain struct {
|
|||||||
logger *slog.Logger
|
logger *slog.Logger
|
||||||
http *http.Client
|
http *http.Client
|
||||||
stopCh chan struct{}
|
stopCh chan struct{}
|
||||||
|
stopOnce sync.Once
|
||||||
recentSignals sync.Map // debounce
|
recentSignals sync.Map // debounce
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -29,7 +30,7 @@ func NewBrain(agent *Agent, logger *slog.Logger) *Brain {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func (b *Brain) Stop() { close(b.stopCh) }
|
func (b *Brain) Stop() { b.stopOnce.Do(func() { close(b.stopCh) }) }
|
||||||
|
|
||||||
// cleanStaleSignals removes debounce entries older than 30 minutes.
|
// cleanStaleSignals removes debounce entries older than 30 minutes.
|
||||||
func (b *Brain) cleanStaleSignals() {
|
func (b *Brain) cleanStaleSignals() {
|
||||||
|
|||||||
+15
-4
@@ -64,13 +64,24 @@ func (a *Agent) saveSetupState(userID int64, s *SetupState) {
|
|||||||
a.store.SetSystemConfig(fmt.Sprintf("setup_step_%d", userID), s.Step)
|
a.store.SetSystemConfig(fmt.Sprintf("setup_step_%d", userID), s.Step)
|
||||||
setConfig(a.store, userID, "exchange", s.Exchange)
|
setConfig(a.store, userID, "exchange", s.Exchange)
|
||||||
setConfig(a.store, userID, "exchange_id", s.ExchangeID)
|
setConfig(a.store, userID, "exchange_id", s.ExchangeID)
|
||||||
setConfig(a.store, userID, "api_key", s.APIKey)
|
// Store only a masked marker for secrets — full values stay in memory only.
|
||||||
setConfig(a.store, userID, "api_secret", s.APISecret)
|
// This prevents plaintext credentials from lingering in the config store
|
||||||
setConfig(a.store, userID, "passphrase", s.Passphrase)
|
// if the setup flow is interrupted before clearSetupState runs.
|
||||||
|
if s.APIKey != "" {
|
||||||
|
setConfig(a.store, userID, "api_key", "****")
|
||||||
|
}
|
||||||
|
if s.APISecret != "" {
|
||||||
|
setConfig(a.store, userID, "api_secret", "****")
|
||||||
|
}
|
||||||
|
if s.Passphrase != "" {
|
||||||
|
setConfig(a.store, userID, "passphrase", "****")
|
||||||
|
}
|
||||||
setConfig(a.store, userID, "ai_provider", s.AIProvider)
|
setConfig(a.store, userID, "ai_provider", s.AIProvider)
|
||||||
setConfig(a.store, userID, "ai_model", s.AIModel)
|
setConfig(a.store, userID, "ai_model", s.AIModel)
|
||||||
setConfig(a.store, userID, "ai_model_id", s.AIModelID)
|
setConfig(a.store, userID, "ai_model_id", s.AIModelID)
|
||||||
setConfig(a.store, userID, "ai_key", s.AIKey)
|
if s.AIKey != "" {
|
||||||
|
setConfig(a.store, userID, "ai_key", "****")
|
||||||
|
}
|
||||||
setConfig(a.store, userID, "ai_base_url", s.AIBaseURL)
|
setConfig(a.store, userID, "ai_base_url", s.AIBaseURL)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+6
-4
@@ -6,13 +6,15 @@ import (
|
|||||||
"log/slog"
|
"log/slog"
|
||||||
"nofx/safe"
|
"nofx/safe"
|
||||||
"strings"
|
"strings"
|
||||||
|
"sync"
|
||||||
"time"
|
"time"
|
||||||
)
|
)
|
||||||
|
|
||||||
type Scheduler struct {
|
type Scheduler struct {
|
||||||
agent *Agent
|
agent *Agent
|
||||||
logger *slog.Logger
|
logger *slog.Logger
|
||||||
stopCh chan struct{}
|
stopCh chan struct{}
|
||||||
|
stopOnce sync.Once
|
||||||
}
|
}
|
||||||
|
|
||||||
func NewScheduler(a *Agent, l *slog.Logger) *Scheduler {
|
func NewScheduler(a *Agent, l *slog.Logger) *Scheduler {
|
||||||
@@ -51,7 +53,7 @@ func (s *Scheduler) Start(ctx context.Context) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Scheduler) Stop() { close(s.stopCh) }
|
func (s *Scheduler) Stop() { s.stopOnce.Do(func() { close(s.stopCh) }) }
|
||||||
|
|
||||||
func (s *Scheduler) dailyReport() {
|
func (s *Scheduler) dailyReport() {
|
||||||
if s.agent.traderManager == nil { return }
|
if s.agent.traderManager == nil { return }
|
||||||
|
|||||||
+2
-1
@@ -41,6 +41,7 @@ type Sentinel struct {
|
|||||||
http *http.Client
|
http *http.Client
|
||||||
logger *slog.Logger
|
logger *slog.Logger
|
||||||
stopCh chan struct{}
|
stopCh chan struct{}
|
||||||
|
stopOnce sync.Once
|
||||||
}
|
}
|
||||||
|
|
||||||
type pricePt struct {
|
type pricePt struct {
|
||||||
@@ -76,7 +77,7 @@ func (s *Sentinel) Start() {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Sentinel) Stop() { close(s.stopCh) }
|
func (s *Sentinel) Stop() { s.stopOnce.Do(func() { close(s.stopCh) }) }
|
||||||
func (s *Sentinel) SymbolCount() int { s.mu.RLock(); defer s.mu.RUnlock(); return len(s.symbols) }
|
func (s *Sentinel) SymbolCount() int { s.mu.RLock(); defer s.mu.RUnlock(); return len(s.symbols) }
|
||||||
func (s *Sentinel) AddSymbol(sym string) { s.mu.Lock(); defer s.mu.Unlock(); for _, x := range s.symbols { if x == sym { return } }; s.symbols = append(s.symbols, sym) }
|
func (s *Sentinel) AddSymbol(sym string) { s.mu.Lock(); defer s.mu.Unlock(); for _, x := range s.symbols { if x == sym { return } }; s.symbols = append(s.symbols, sym) }
|
||||||
func (s *Sentinel) RemoveSymbol(sym string) { s.mu.Lock(); defer s.mu.Unlock(); for i, x := range s.symbols { if x == sym { s.symbols = append(s.symbols[:i], s.symbols[i+1:]...); return } } }
|
func (s *Sentinel) RemoveSymbol(sym string) { s.mu.Lock(); defer s.mu.Unlock(); for i, x := range s.symbols { if x == sym { s.symbols = append(s.symbols[:i], s.symbols[i+1:]...); return } } }
|
||||||
|
|||||||
@@ -194,17 +194,31 @@ func (a *Agent) executeTrade(ctx context.Context, trade *TradeAction) error {
|
|||||||
return fmt.Errorf("no running trader supports trade execution")
|
return fmt.Errorf("no running trader supports trade execution")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Sanity caps to prevent LLM hallucinations or input errors from causing damage.
|
||||||
|
const maxQuantity = 100000.0
|
||||||
|
const maxLeverage = 125
|
||||||
|
|
||||||
|
if trade.Leverage > maxLeverage {
|
||||||
|
return fmt.Errorf("leverage %dx exceeds maximum allowed (%dx)", trade.Leverage, maxLeverage)
|
||||||
|
}
|
||||||
|
|
||||||
switch trade.Action {
|
switch trade.Action {
|
||||||
case "open_long":
|
case "open_long":
|
||||||
if trade.Quantity <= 0 {
|
if trade.Quantity <= 0 {
|
||||||
return fmt.Errorf("quantity must be > 0")
|
return fmt.Errorf("quantity must be > 0")
|
||||||
}
|
}
|
||||||
|
if trade.Quantity > maxQuantity {
|
||||||
|
return fmt.Errorf("quantity %.4f exceeds maximum allowed (%.0f)", trade.Quantity, maxQuantity)
|
||||||
|
}
|
||||||
_, err := underlyingTrader.OpenLong(trade.Symbol, trade.Quantity, trade.Leverage)
|
_, err := underlyingTrader.OpenLong(trade.Symbol, trade.Quantity, trade.Leverage)
|
||||||
return err
|
return err
|
||||||
case "open_short":
|
case "open_short":
|
||||||
if trade.Quantity <= 0 {
|
if trade.Quantity <= 0 {
|
||||||
return fmt.Errorf("quantity must be > 0")
|
return fmt.Errorf("quantity must be > 0")
|
||||||
}
|
}
|
||||||
|
if trade.Quantity > maxQuantity {
|
||||||
|
return fmt.Errorf("quantity %.4f exceeds maximum allowed (%.0f)", trade.Quantity, maxQuantity)
|
||||||
|
}
|
||||||
_, err := underlyingTrader.OpenShort(trade.Symbol, trade.Quantity, trade.Leverage)
|
_, err := underlyingTrader.OpenShort(trade.Symbol, trade.Quantity, trade.Leverage)
|
||||||
return err
|
return err
|
||||||
case "close_long":
|
case "close_long":
|
||||||
|
|||||||
@@ -169,8 +169,7 @@ func main() {
|
|||||||
}
|
}
|
||||||
logger.Info("✅ HTTP server stopped")
|
logger.Info("✅ HTTP server stopped")
|
||||||
|
|
||||||
nofxiAgent.Stop()
|
// nofxiAgent.Stop() is handled by defer above
|
||||||
logger.Info("✅ NOFXi agent stopped")
|
|
||||||
|
|
||||||
// Stop all traders
|
// Stop all traders
|
||||||
traderManager.StopAll()
|
traderManager.StopAll()
|
||||||
|
|||||||
Reference in New Issue
Block a user