Skip to content

Commit

Permalink
Config: Move OrderManager upgrade to Version management
Browse files Browse the repository at this point in the history
  • Loading branch information
gbjk committed Dec 3, 2024
1 parent 026f88f commit 30017c4
Show file tree
Hide file tree
Showing 9 changed files with 159 additions and 48 deletions.
3 changes: 1 addition & 2 deletions backtester/engine/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
19 changes: 0 additions & 19 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -1670,7 +1652,6 @@ func (c *Config) CheckConfig() error {
c.CheckConnectionMonitorConfig()
c.CheckDataHistoryMonitorConfig()
c.CheckCurrencyStateManager()
c.CheckOrderManagerConfig()
c.CheckCommunicationsConfig()
c.CheckClientBankAccounts()
c.CheckBankAccountConfig()
Expand Down
4 changes: 2 additions & 2 deletions config/config_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

Expand Down
73 changes: 73 additions & 0 deletions config/versions/v4.go
Original file line number Diff line number Diff line change
@@ -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)

Check failure on line 22 in config/versions/v4.go

View workflow job for this annotation

GitHub Actions / GoCryptoTrader back-end (ubuntu-latest, 386, true, true)

constant 3600000000000 overflows int

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
}
68 changes: 68 additions & 0 deletions config/versions/v4_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
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) {

Check failure on line 38 in config/versions/v4_test.go

View workflow job for this annotation

GitHub Actions / lint

TestVersion4Upgrade's subtests should call t.Parallel (tparallel)
t.Parallel()

b := new(bytes.Buffer)
for _, tt := range tests {
_ = t.Run(tt.name, func(t *testing.T) {
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.Reset()
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(expUser1))

Check failure on line 65 in config/versions/v4_test.go

View workflow job for this annotation

GitHub Actions / lint

ineffectual assignment to out (ineffassign)
require.NoError(t, err)
assert.Equal(t, expReversed, expReversed, "Downgrade should leave an already negative futuresTrackingSeekDuration alone")

Check failure on line 67 in config/versions/v4_test.go

View workflow job for this annotation

GitHub Actions / lint

useless-assert: asserting of the same variable (testifylint)
}
2 changes: 1 addition & 1 deletion engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))

Expand Down
21 changes: 6 additions & 15 deletions engine/order_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
}

Expand Down Expand Up @@ -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],
Expand Down
3 changes: 1 addition & 2 deletions engine/order_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 7 additions & 7 deletions engine/order_manager_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 30017c4

Please sign in to comment.