diff --git a/backtester/engine/setup.go b/backtester/engine/setup.go index 65967682aa7..e7b382fabc2 100644 --- a/backtester/engine/setup.go +++ b/backtester/engine/setup.go @@ -31,7 +31,6 @@ import ( "github.com/thrasher-corp/gocryptotrader/backtester/funding/trackingcurrencies" "github.com/thrasher-corp/gocryptotrader/backtester/report" gctcommon "github.com/thrasher-corp/gocryptotrader/common" - "github.com/thrasher-corp/gocryptotrader/common/convert" "github.com/thrasher-corp/gocryptotrader/common/key" gctconfig "github.com/thrasher-corp/gocryptotrader/config" "github.com/thrasher-corp/gocryptotrader/currency" @@ -211,7 +210,7 @@ func (bt *BackTest) SetupFromConfig(cfg *config.Config, templatePath, output str bt.orderManager, err = engine.SetupOrderManager(bt.exchangeManager, &engine.CommunicationManager{}, &sync.WaitGroup{}, &gctconfig.OrderManager{ Verbose: verbose, ActivelyTrackFuturesPositions: trackFuturesPositions, - RespectOrderHistoryLimits: convert.BoolPtr(true), + RespectOrderHistoryLimits: true, }) if err != nil { return err diff --git a/config/config.go b/config/config.go index 5e14f884553..949756bc5e4 100644 --- a/config/config.go +++ b/config/config.go @@ -1338,24 +1338,6 @@ func (c *Config) CheckCurrencyStateManager() { } } -// CheckOrderManagerConfig ensures the order manager is setup correctly -func (c *Config) CheckOrderManagerConfig() { - m.Lock() - defer m.Unlock() - if c.OrderManager.Enabled == nil { - c.OrderManager.Enabled = convert.BoolPtr(true) - c.OrderManager.ActivelyTrackFuturesPositions = true - } - if c.OrderManager.RespectOrderHistoryLimits == nil { - c.OrderManager.RespectOrderHistoryLimits = convert.BoolPtr(true) - } - if c.OrderManager.ActivelyTrackFuturesPositions && c.OrderManager.FuturesTrackingSeekDuration >= 0 { - // one isn't likely to have a perpetual futures order open - // for longer than a year - c.OrderManager.FuturesTrackingSeekDuration = -time.Hour * 24 * 365 - } -} - // CheckConnectionMonitorConfig checks and if zero value assigns default values func (c *Config) CheckConnectionMonitorConfig() { m.Lock() @@ -1670,7 +1652,6 @@ func (c *Config) CheckConfig() error { c.CheckConnectionMonitorConfig() c.CheckDataHistoryMonitorConfig() c.CheckCurrencyStateManager() - c.CheckOrderManagerConfig() c.CheckCommunicationsConfig() c.CheckClientBankAccounts() c.CheckBankAccountConfig() diff --git a/config/config_types.go b/config/config_types.go index f1f8cd8403a..cbfeee8fe84 100644 --- a/config/config_types.go +++ b/config/config_types.go @@ -129,11 +129,11 @@ type EncryptionKeyProvider func(confirmKey bool) ([]byte, error) // OrderManager holds settings used for the order manager type OrderManager struct { - Enabled *bool `json:"enabled"` + Enabled bool `json:"enabled"` Verbose bool `json:"verbose"` ActivelyTrackFuturesPositions bool `json:"activelyTrackFuturesPositions"` FuturesTrackingSeekDuration time.Duration `json:"futuresTrackingSeekDuration"` - RespectOrderHistoryLimits *bool `json:"respectOrderHistoryLimits"` + RespectOrderHistoryLimits bool `json:"respectOrderHistoryLimits"` CancelOrdersOnShutdown bool `json:"cancelOrdersOnShutdown"` } diff --git a/config/versions/v4.go b/config/versions/v4.go new file mode 100644 index 00000000000..ff52dad9a1a --- /dev/null +++ b/config/versions/v4.go @@ -0,0 +1,73 @@ +package versions + +import ( + "context" + "errors" + "strconv" + "time" + + "github.com/buger/jsonparser" +) + +// Version4 implements ConfigVersion +type Version4 struct { +} + +func init() { + Manager.registerVersion(4, &Version4{}) +} + +// defaultConfig contains the stateless V4 representation of orderbookManager +// Note: Do not be tempted to use a constant for Duration. Whilst defaults are still written to config, we need to manage default upgrades discretely. +var defaultFuturesTrackingSeekDuration = strconv.Itoa(int(time.Hour) * 24 * 365) + +var defaultConfig = []byte(`{ + "enabled": true, + "verbose": false, + "activelyTrackFuturesPositions": true, + "futuresTrackingSeekDuration": ` + defaultFuturesTrackingSeekDuration + `, + "cancelOrdersOnShutdown": false, + "respectOrderHistoryLimits": true + }`) + +// UpgradeConfig handles upgrading config for OrderManager: +// * Sets OrderManager config to defaults if it doesn't exist or enabled is null +// * Sets respectOrderHistoryLimits to true if it doesn't exist or is null +// * Sets futuresTrackingSeekDuration to positive if it's negative +func (v *Version4) UpgradeConfig(_ context.Context, e []byte) ([]byte, error) { + _, valueType, _, err := jsonparser.Get(e, "orderManager", "enabled") + switch { + case errors.Is(err, jsonparser.KeyPathNotFoundError), valueType == jsonparser.Null: + return jsonparser.Set(e, defaultConfig, "orderManager") + case err != nil: + return e, err + } + + _, valueType, _, err = jsonparser.Get(e, "orderManager", "respectOrderHistoryLimits") + if errors.Is(err, jsonparser.KeyPathNotFoundError) || valueType == jsonparser.Null { + if e, err = jsonparser.Set(e, []byte(`true`), "orderManager", "respectOrderHistoryLimits"); err != nil { + return e, err + } + } + + if i, err := jsonparser.GetInt(e, "orderManager", "futuresTrackingSeekDuration"); err != nil { + if e, err = jsonparser.Set(e, []byte(defaultFuturesTrackingSeekDuration), "orderManager", "futuresTrackingSeekDuration"); err != nil { + return e, err + } + } else if i < 0 { + if e, err = jsonparser.Set(e, []byte(strconv.Itoa(0-int(i))), "orderManager", "futuresTrackingSeekDuration"); err != nil { + return e, err + } + } + return e, nil +} + +// DowngradeConfig just reverses the futuresTrackingSeekDuration to negative, and leaves everything else alone +func (v *Version4) DowngradeConfig(_ context.Context, e []byte) ([]byte, error) { + if i, err := jsonparser.GetInt(e, "orderManager", "futuresTrackingSeekDuration"); err == nil && i > 0 { + if e, err = jsonparser.Set(e, []byte(strconv.Itoa(0-int(i))), "orderManager", "futuresTrackingSeekDuration"); err != nil { + return e, err + } + } + return e, nil +} diff --git a/config/versions/v4_test.go b/config/versions/v4_test.go new file mode 100644 index 00000000000..795ad282046 --- /dev/null +++ b/config/versions/v4_test.go @@ -0,0 +1,67 @@ +package versions + +import ( + "bytes" + "context" + "encoding/json" + "strings" + "testing" + + "github.com/buger/jsonparser" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var ( + rOHLT = `"respectOrderHistoryLimits":true,` + rOHLN = `"respectOrderHistoryLimits":null,` + expDef = `{"orderManager":{"enabled":true,"verbose":false,"activelyTrackFuturesPositions":true,"futuresTrackingSeekDuration":31536000000000000,"cancelOrdersOnShutdown":false,"respectOrderHistoryLimits":true}}` + expUser1 = `{"orderManager":{"enabled":false,"verbose":true,"activelyTrackFuturesPositions":false,"futuresTrackingSeekDuration":47000,"cancelOrdersOnShutdown":true,"respectOrderHistoryLimits":true}}` + expUser2 = strings.Replace(expUser1, `mits":true`, `mits":false`, 1) +) + +var tests = []struct { + name string + in string + out string + err error +}{ + {name: "Bad input should error", err: jsonparser.KeyPathNotFoundError}, + {name: "Missing orderManager should use the defaults", in: "{}", out: expDef}, + {name: "Enabled null should use defaults", in: strings.Replace(expDef, "true", "null", 1), out: expDef}, + {name: "RespectOrderHistoryLimits should be added if missing", in: strings.Replace(expUser1, `,"respectOrderHistoryLimits":true`, "", 1), out: expUser1}, + {name: "RespectOrderHistoryLimits null should default true", in: strings.Replace(expUser1, `mits":true`, `mits":null`, 1), out: expUser1}, + {name: "FutureTracking should be reversed", in: strings.Replace(expUser1, "47", "-47", 1), out: expUser1}, + {name: "Configured orderManager should be left alone", in: expUser2, out: expUser2}, +} + +func TestVersion4Upgrade(t *testing.T) { + t.Parallel() + for _, tt := range tests { + _ = t.Run(tt.name, func(t *testing.T) { + t.Parallel() + out, err := new(Version4).UpgradeConfig(context.Background(), []byte(tt.in)) + if tt.err != nil { + require.ErrorIs(t, err, tt.err) + return + } + require.NoError(t, err) + b := new(bytes.Buffer) + require.NoError(t, json.Compact(b, out), "json.Compact must not error") + require.Equal(t, tt.out, b.String()) + }) + } +} + +func TestVersion4Downgrade(t *testing.T) { + t.Parallel() + + expReversed := strings.Replace(expUser1, "47", "-47", 1) + out, err := new(Version4).DowngradeConfig(context.Background(), []byte(expUser1)) + require.NoError(t, err) + assert.Equal(t, expReversed, string(out), "Downgrade should just reverse the futuresTrackingSeekDuration") + + out, err = new(Version4).DowngradeConfig(context.Background(), []byte(expReversed)) + require.NoError(t, err) + assert.Equal(t, expReversed, string(out), "Downgrade should leave an already negative futuresTrackingSeekDuration alone") +} diff --git a/engine/engine.go b/engine/engine.go index bd9dba18aec..625ffe0de3d 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -165,7 +165,7 @@ func validateSettings(b *Engine, s *Settings, flagSet FlagSet) { b.Settings = *s flagSet.WithBool("coinmarketcap", &b.Settings.EnableCoinmarketcapAnalysis, b.Config.Currency.CryptocurrencyProvider.Enabled) - flagSet.WithBool("ordermanager", &b.Settings.EnableOrderManager, b.Config.OrderManager.Enabled != nil && *b.Config.OrderManager.Enabled) + flagSet.WithBool("ordermanager", &b.Settings.EnableOrderManager, b.Config.OrderManager.Enabled) flagSet.WithBool("currencyconverter", &b.Settings.EnableCurrencyConverter, b.Config.Currency.ForexProviders.IsEnabled("currencyconverter")) diff --git a/engine/order_manager.go b/engine/order_manager.go index 908a7ea9432..36326709594 100644 --- a/engine/order_manager.go +++ b/engine/order_manager.go @@ -39,15 +39,15 @@ func SetupOrderManager(exchangeManager iExchangeManager, communicationsManager i if cfg == nil { return nil, fmt.Errorf("%w OrderManager", errNilConfig) } - - var respectOrderHistoryLimits bool - if cfg.RespectOrderHistoryLimits != nil { - respectOrderHistoryLimits = *cfg.RespectOrderHistoryLimits + if cfg.ActivelyTrackFuturesPositions && cfg.FuturesTrackingSeekDuration <= 0 { + return nil, errInvalidFuturesTrackingSeekDuration } + om := &OrderManager{ shutdown: make(chan struct{}), activelyTrackFuturesPositions: cfg.ActivelyTrackFuturesPositions, - respectOrderHistoryLimits: respectOrderHistoryLimits, + futuresPositionSeekDuration: cfg.FuturesTrackingSeekDuration, + respectOrderHistoryLimits: cfg.RespectOrderHistoryLimits, orderStore: store{ Orders: make(map[string][]*order.Detail), exchangeManager: exchangeManager, @@ -60,15 +60,6 @@ func SetupOrderManager(exchangeManager iExchangeManager, communicationsManager i CancelOrdersOnShutdown: cfg.CancelOrdersOnShutdown, }, } - if cfg.ActivelyTrackFuturesPositions { - if cfg.FuturesTrackingSeekDuration > 0 { - cfg.FuturesTrackingSeekDuration *= -1 - } - if cfg.FuturesTrackingSeekDuration == 0 { - cfg.FuturesTrackingSeekDuration = defaultOrderSeekTime - } - om.futuresPositionSeekDuration = cfg.FuturesTrackingSeekDuration - } return om, nil } @@ -728,7 +719,7 @@ func (m *OrderManager) processOrders() { return } if sd.IsZero() { - sd = time.Now().Add(m.futuresPositionSeekDuration) + sd = time.Now().Add(0 - m.futuresPositionSeekDuration) } positions, err = exchanges[x].GetFuturesPositionOrders(context.TODO(), &futures.PositionsRequest{ Asset: enabledAssets[y], diff --git a/engine/order_manager_test.go b/engine/order_manager_test.go index 2131955d915..4144ac1cac4 100644 --- a/engine/order_manager_test.go +++ b/engine/order_manager_test.go @@ -13,7 +13,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/thrasher-corp/gocryptotrader/common" - "github.com/thrasher-corp/gocryptotrader/common/convert" "github.com/thrasher-corp/gocryptotrader/config" "github.com/thrasher-corp/gocryptotrader/currency" exchange "github.com/thrasher-corp/gocryptotrader/exchanges" @@ -1543,7 +1542,7 @@ func TestGetOpenFuturesPosition(t *testing.T) { t.Fatalf("received: '%v' but expected: '%v'", err, nil) } o, err = SetupOrderManager(em, &CommunicationManager{}, wg, &config.OrderManager{ - Enabled: convert.BoolPtr(true), + Enabled: true, FuturesTrackingSeekDuration: time.Hour, Verbose: true, ActivelyTrackFuturesPositions: true, diff --git a/engine/order_manager_types.go b/engine/order_manager_types.go index 40fe0162731..91ae646175c 100644 --- a/engine/order_manager_types.go +++ b/engine/order_manager_types.go @@ -13,20 +13,20 @@ import ( // OrderManagerName is an exported subsystem name const OrderManagerName = "orders" -// vars for the fund manager package +// Public Errors var ( - // ErrOrdersAlreadyExists occurs when the order already exists in the manager - ErrOrdersAlreadyExists = errors.New("order already exists") - // ErrOrderIDCannotBeEmpty occurs when an order does not have an ID + ErrOrdersAlreadyExists = errors.New("order already exists") ErrOrderIDCannotBeEmpty = errors.New("orderID cannot be empty") - // ErrOrderNotFound occurs when an order is not found in the orderstore - ErrOrderNotFound = errors.New("order does not exist") + ErrOrderNotFound = errors.New("order does not exist") +) +var ( errNilCommunicationsManager = errors.New("cannot start with nil communications manager") errNilOrder = errors.New("nil order received") errFuturesTrackingDisabled = errors.New("tracking futures positions disabled. enable it via config under orderManager activelyTrackFuturesPositions") orderManagerInterval = time.Second * 10 - defaultOrderSeekTime = -time.Hour * 24 * 365 + + errInvalidFuturesTrackingSeekDuration = errors.New("invalid config value for futuresTrackingSeekDuration") ) type orderManagerConfig struct {