Config: Eliminate shared config version package vars (#1867)

* Config: Replace consts for Version2 from/to

Could have moved them to v2/consts.go but doesn't seem enough RoI

* Config: Use generic error in Version4

* Config: Fix Version5 pkg vars

* Config: Fix Version6 test func names and shared test var

config/versions/v2.go

@@ -13,26 +13,21 @@ func init() {
 	Manager.registerVersion(2, &Version2{})
 }
 
-const (
-	from = "GDAX"
-	to   = "CoinbasePro"
-)
-
 // Exchanges returns just GDAX and CoinbasePro
-func (v *Version2) Exchanges() []string { return []string{from, to} }
+func (v *Version2) Exchanges() []string { return []string{"GDAX", "CoinbasePro"} }
 
 // UpgradeExchange will change the exchange name from GDAX to CoinbasePro
 func (v *Version2) UpgradeExchange(_ context.Context, e []byte) ([]byte, error) {
-	if n, err := jsonparser.GetString(e, "name"); err == nil && n == from {
-		return jsonparser.Set(e, []byte(`"`+to+`"`), "name")
+	if n, err := jsonparser.GetString(e, "name"); err == nil && n == "GDAX" {
+		return jsonparser.Set(e, []byte(`"CoinbasePro"`), "name")
 	}
 	return e, nil
 }
 
 // DowngradeExchange will change the exchange name from CoinbasePro to GDAX
 func (v *Version2) DowngradeExchange(_ context.Context, e []byte) ([]byte, error) {
-	if n, err := jsonparser.GetString(e, "name"); err == nil && n == to {
-		return jsonparser.Set(e, []byte(`"`+from+`"`), "name")
+	if n, err := jsonparser.GetString(e, "name"); err == nil && n == "CoinbasePro" {
+		return jsonparser.Set(e, []byte(`"GDAX"`), "name")
 	}
 	return e, nil
 }

config/versions/v4.go

@@ -10,11 +10,6 @@ import (
 	"github.com/buger/jsonparser"
 )
 
-var (
-	errUpgradingAssetTypes    = errors.New("error upgrading assetTypes")
-	errUpgradingCurrencyPairs = errors.New("error upgrading currencyPairs.pairs")
-)
-
 // Version4 is an Exchange upgrade to move currencyPairs.assetTypes to currencyPairs.pairs.*.assetEnabled
 type Version4 struct{}
 
@@ -36,7 +31,7 @@ func (v *Version4) UpgradeExchange(_ context.Context, e []byte) ([]byte, error)
 	}
 	_, err := jsonparser.ArrayEach(e, assetTypesFn, "currencyPairs", "assetTypes")
 	if err != nil && !errors.Is(err, jsonparser.KeyPathNotFoundError) {
-		return e, fmt.Errorf("%w: %w", errUpgradingAssetTypes, err)
+		return e, fmt.Errorf("%w assetTypes: %w", errUpgrading, err)
 	}
 
 	assetEnabledFn := func(assetBytes, v []byte, _ jsonparser.ValueType, _ int) (err error) {
@@ -59,7 +54,7 @@ func (v *Version4) UpgradeExchange(_ context.Context, e []byte) ([]byte, error)
 		return err
 	}
 	if err = jsonparser.ObjectEach(bytes.Clone(e), assetEnabledFn, "currencyPairs", "pairs"); err != nil {
-		return e, fmt.Errorf("%w: %w", errUpgradingCurrencyPairs, err)
+		return e, fmt.Errorf("%w currencyPairs.pairs: %w", errUpgrading, err)
 	}
 	e = jsonparser.Delete(e, "currencyPairs", "assetTypes")
 	return e, err

config/versions/v4_test.go

@@ -24,10 +24,12 @@ func TestVersion4Upgrade(t *testing.T) {
 	t.Parallel()
 
 	_, err := new(Version4).UpgradeExchange(context.Background(), []byte{})
-	require.ErrorIs(t, err, errUpgradingAssetTypes)
+	require.ErrorIs(t, err, errUpgrading)
+	require.ErrorContains(t, err, `assetTypes`)
 
 	_, err = new(Version4).UpgradeExchange(context.Background(), []byte(`{}`))
-	require.ErrorIs(t, err, errUpgradingCurrencyPairs)
+	require.ErrorIs(t, err, errUpgrading)
+	require.ErrorContains(t, err, `currencyPairs.pairs`)
 
 	in := []byte(`{"name":"Cracken","currencyPairs":{"assetTypes":["spot"],"pairs":{"spot":{"enabled":"BTC-AUD","available":"BTC-AUD"},"futures":{"assetEnabled":true},"options":{},"margin":{"assetEnabled":null}}}}`)
 	out, err := new(Version4).UpgradeExchange(context.Background(), in)

config/versions/v5.go

@@ -4,9 +4,9 @@ import (
 	"context"
 	"errors"
 	"strconv"
-	"time"
 
 	"github.com/buger/jsonparser"
+	v5 "github.com/thrasher-corp/gocryptotrader/config/versions/v5"
 )
 
 // Version5 implements ConfigVersion
@@ -16,19 +16,6 @@ func init() {
 	Manager.registerVersion(5, &Version5{})
 }
 
-// defaultConfig contains the stateless V5 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.FormatInt(int64(time.Hour)*24*365, 10)
-
-var defaultConfigV5 = []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
@@ -37,7 +24,7 @@ func (v *Version5) 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, defaultConfigV5, "orderManager")
+		return jsonparser.Set(e, v5.DefaultOrderbookConfig, "orderManager")
 	case err != nil:
 		return e, err
 	}
@@ -50,7 +37,7 @@ func (v *Version5) UpgradeConfig(_ context.Context, e []byte) ([]byte, error) {
 	}
 
 	if i, err := jsonparser.GetInt(e, "orderManager", "futuresTrackingSeekDuration"); err != nil {
-		if e, err = jsonparser.Set(e, []byte(defaultFuturesTrackingSeekDuration), "orderManager", "futuresTrackingSeekDuration"); err != nil {
+		if e, err = jsonparser.Set(e, []byte(v5.DefaultFuturesTrackingSeekDuration), "orderManager", "futuresTrackingSeekDuration"); err != nil {
 			return e, err
 		}
 	} else if i < 0 {

config/versions/v5/defaults.go

@@ -0,0 +1,20 @@
+package v5
+
+import (
+	"strconv"
+	"time"
+)
+
+// DefaultFuturesTrackingSeekDuration contains the default futures tracking seek duration
+// Note: Do not be tempted to use an external package constant for Duration; This is the value at v5 only
+var DefaultFuturesTrackingSeekDuration = strconv.FormatInt(int64(time.Hour)*24*365, 10)
+
+// DefaultOrderbookConfig contains the stateless V5 representation of orderbookManager
+var DefaultOrderbookConfig = []byte(`{
+  "enabled": true,
+  "verbose": false,
+  "activelyTrackFuturesPositions": true,
+  "futuresTrackingSeekDuration": ` + DefaultFuturesTrackingSeekDuration + `,
+  "cancelOrdersOnShutdown": false,
+  "respectOrderHistoryLimits": true
+ }`)

config/versions/v5_test.go

@@ -12,29 +12,28 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-var (
-	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 TestVersion5Upgrade(t *testing.T) {
 	t.Parallel()
+
+	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)
+
+	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},
+	}
+
 	for _, tt := range tests {
 		_ = t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
@@ -54,12 +53,13 @@ func TestVersion5Upgrade(t *testing.T) {
 func TestVersion5Downgrade(t *testing.T) {
 	t.Parallel()
 
-	expReversed := strings.Replace(expUser1, "47", "-47", 1)
-	out, err := new(Version5).DowngradeConfig(context.Background(), []byte(expUser1))
+	in := `{"orderManager":{"enabled":false,"verbose":true,"activelyTrackFuturesPositions":false,"futuresTrackingSeekDuration":-47000,"cancelOrdersOnShutdown":true,"respectOrderHistoryLimits":true}}`
+	exp := `{"orderManager":{"enabled":false,"verbose":true,"activelyTrackFuturesPositions":false,"futuresTrackingSeekDuration":-47000,"cancelOrdersOnShutdown":true,"respectOrderHistoryLimits":true}}`
+	out, err := new(Version5).DowngradeConfig(context.Background(), []byte(in))
 	require.NoError(t, err)
-	assert.Equal(t, expReversed, string(out), "DowngradeConfig should just reverse the futuresTrackingSeekDuration")
+	assert.Equal(t, exp, string(out), "DowngradeConfig should just reverse the futuresTrackingSeekDuration")
 
-	out, err = new(Version5).DowngradeConfig(context.Background(), []byte(expReversed))
+	out, err = new(Version5).DowngradeConfig(context.Background(), []byte(exp))
 	require.NoError(t, err)
-	assert.Equal(t, expReversed, string(out), "DowngradeConfig should leave an already negative futuresTrackingSeekDuration alone")
+	assert.Equal(t, exp, string(out), "DowngradeConfig should leave an already negative futuresTrackingSeekDuration alone")
 }

config/versions/v6_test.go

@@ -11,25 +11,14 @@ import (
 	v6 "github.com/thrasher-corp/gocryptotrader/config/versions/v6"
 )
 
-var testData = []byte(`{
-	"portfolioAddresses": {
-	  "addresses": [
-	   {
-		"Address": "1JCe8z4jJVNXSjohjM4i9Hh813dLCNx2Sy",
-		"CoinType": "BTC",
-		"Balance": 0.00108832,
-		"Description": "",
-		"WhiteListed": false,
-		"ColdStorage": false,
-		"SupportedExchanges": ""
-	   }
-	  ]
-	}
-}`)
-
-func TestUpgrade(t *testing.T) {
+func TestVersion6Upgrade(t *testing.T) {
 	t.Parallel()
-	r, err := new(Version6).UpgradeConfig(context.Background(), testData)
+
+	in := []byte(`
+{"portfolioAddresses":{"addresses":[{"Address":"1JCe8z4jJVNXSjohjM4i9Hh813dLCNx2Sy","CoinType":"BTC","Balance":0.00108832,"Description":"","WhiteListed":false,"ColdStorage":false,"SupportedExchanges":""}]}}
+	`)
+
+	r, err := new(Version6).UpgradeConfig(context.Background(), in)
 	require.NoError(t, err, "UpgradeConfig must not error")
 	require.True(t, bytes.Contains(r, v6.DefaultConfig))
 
@@ -38,14 +27,14 @@ func TestUpgrade(t *testing.T) {
 	assert.Equal(t, r, r2, "UpgradeConfig should not affect an already upgraded config")
 }
 
-func TestDowngrade(t *testing.T) {
+func TestVersion6Downgrade(t *testing.T) {
 	t.Parallel()
 
-	r, err := new(Version6).UpgradeConfig(context.Background(), testData)
-	require.NoError(t, err, "UpgradeConfig must not error")
-	require.True(t, bytes.Contains(r, v6.DefaultConfig))
+	in := []byte(`
+{"portfolioAddresses":{"addresses":[{"Address":"1JCe8z4jJVNXSjohjM4i9Hh813dLCNx2Sy","CoinType":"BTC","Balance":0.00108832,"Description":"","WhiteListed":false,"ColdStorage":false,"SupportedExchanges":""}],"providers":[{"name":"Ethplorer","enabled":true},{"name":"XRPScan","enabled":true},{"name":"CryptoID","enabled":false,"apiKey":"Key"}]}}
+`)
 
-	r, err = new(Version6).DowngradeConfig(context.Background(), r)
+	r, err := new(Version6).DowngradeConfig(context.Background(), in)
 	require.NoError(t, err, "DowngradeConfig must not error")
 	_, _, _, err = jsonparser.Get(r, "portfolioAddresses", "providers") //nolint:dogsled // Return values not needed
 	assert.ErrorIs(t, err, jsonparser.KeyPathNotFoundError, "providers should be removed")

config/versions/versions.go

@@ -40,6 +40,7 @@ var (
 	errConfigVersionUnavail  = errors.New("version is higher than the latest available version")
 	errConfigVersionNegative = errors.New("version is negative")
 	errConfigVersionMax      = errors.New("version is above max versions")
+	errUpgrading             = errors.New("error upgrading")
 )
 
 // ConfigVersion is a version that affects the general configuration