Config: Tighten config version handling as uint16 (#1825)

* Config: Tighten config version handling as uint16

This constrains the versions to uint16 and improves error handling.
LatestVersion becomes literally that.
Fixes handling for negative or overflowing versions in config

* Config: Rename LatestVersion to UseLatestVersion

cmd/config/config.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"flag"
 	"fmt"
+	"math"
 	"os"
 	"slices"
 	"strings"
@@ -23,15 +24,15 @@ func main() {
 
 	var in, out, keyStr string
 	var inplace bool
-	var version int
+	var version uint
 
 	fs := flag.NewFlagSet("config", flag.ExitOnError)
 	fs.Usage = func() { usage(fs) }
 	fs.StringVar(&in, "in", defaultCfgFile, "The config input file to process")
 	fs.StringVar(&out, "out", "[in].out", "The config output file")
 	fs.BoolVar(&inplace, "edit", false, "Edit; Save result to the original file")
 	fs.StringVar(&keyStr, "key", "", "The key to use for AES encryption")
-	fs.IntVar(&version, "version", 0, "The version to downgrade to")
+	fs.UintVar(&version, "version", 0, "The version to downgrade to")
 
 	cmd, args := parseCommand(os.Args[1:])
 	if cmd == "" {
@@ -85,13 +86,13 @@ func main() {
 				usage(fs)
 				os.Exit(3)
 			}
-			version = versions.LatestVersion
-		} else if version < 0 {
-			fmt.Fprintln(os.Stderr, "Error: version must be positive")
+			version = versions.UseLatestVersion
+		} else if version >= math.MaxUint16 {
+			fmt.Fprintln(os.Stderr, "Error: version must be less than 65535")
 			usage(fs)
 			os.Exit(3)
 		}
-		if data, err = versions.Manager.Deploy(context.Background(), data, version); err != nil {
+		if data, err = versions.Manager.Deploy(context.Background(), data, uint16(version)); err != nil {
 			fatal("Unable to " + cmd + " config; Error: " + err.Error())
 		}
 		if !isEncrypted {

config/config.go

@@ -1504,7 +1504,7 @@ func (c *Config) readConfig(d io.Reader) error {
 		}
 	}
 
-	if j, err = versions.Manager.Deploy(context.Background(), j, versions.LatestVersion); err != nil {
+	if j, err = versions.Manager.Deploy(context.Background(), j, versions.UseLatestVersion); err != nil {
 		return err
 	}
 

config/versions/versions.go

@@ -17,6 +17,7 @@ import (
 	"errors"
 	"fmt"
 	"log"
+	"math"
 	"os"
 	"slices"
 	"strconv"
@@ -26,17 +27,20 @@ import (
 	"github.com/thrasher-corp/gocryptotrader/common"
 )
 
-// LatestVersion used as version param to Deploy to automatically use the latest version
-const LatestVersion = -1
+// UseLatestVersion used as version param to Deploy to automatically use the latest version
+const UseLatestVersion = math.MaxUint16
 
 var (
-	errMissingVersion      = errors.New("missing version")
-	errVersionIncompatible = errors.New("version does not implement ConfigVersion or ExchangeVersion")
-	errModifyingExchange   = errors.New("error modifying exchange config")
-	errNoVersions          = errors.New("error retrieving latest config version: No config versions are registered")
-	errApplyingVersion     = errors.New("error applying version")
-	errConfigVersion       = errors.New("version in config file is higher than the latest available version")
-	errTargetVersion       = errors.New("target downgrade version is higher than the latest available version")
+	errMissingVersion        = errors.New("missing version")
+	errVersionIncompatible   = errors.New("version does not implement ConfigVersion or ExchangeVersion")
+	errModifyingExchange     = errors.New("error modifying exchange config")
+	errNoVersions            = errors.New("error retrieving latest config version: No config versions are registered")
+	errApplyingVersion       = errors.New("error applying version")
+	errTargetVersion         = errors.New("target downgrade version is higher than the latest available version")
+	errConfigVersion         = errors.New("invalid version in config")
+	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")
 )
 
 // ConfigVersion is a version that affects the general configuration
@@ -62,9 +66,9 @@ type manager struct {
 var Manager = &manager{}
 
 // Deploy upgrades or downgrades the config between versions
-// Pass LatestVersion for version to use the latest version automatically
+// Pass UseLatestVersion for version to use the latest version automatically
 // Prints an error an exits if the config file version or version param is not registered
-func (m *manager) Deploy(ctx context.Context, j []byte, version int) ([]byte, error) {
+func (m *manager) Deploy(ctx context.Context, j []byte, version uint16) ([]byte, error) {
 	if err := m.checkVersions(); err != nil {
 		return j, err
 	}
@@ -75,28 +79,33 @@ func (m *manager) Deploy(ctx context.Context, j []byte, version int) ([]byte, er
 	}
 
 	target := latest
-	if version != LatestVersion {
+	if version != UseLatestVersion {
 		target = version
 	}
 
 	m.m.RLock()
 	defer m.m.RUnlock()
 
 	current64, err := jsonparser.GetInt(j, "version")
-	current := int(current64)
 	switch {
 	case errors.Is(err, jsonparser.KeyPathNotFoundError):
-		current = -1
+		// With no version first upgrade is to Version1; current64 is already 0
 	case err != nil:
-		return j, fmt.Errorf("%w `version`: %w", common.ErrGettingField, err)
+		return j, fmt.Errorf("%w: %w `version`: %w", errConfigVersion, common.ErrGettingField, err)
+	case current64 < 0:
+		return j, fmt.Errorf("%w: %w `version`: `%d`", errConfigVersion, errConfigVersionNegative, current64)
+	case current64 >= UseLatestVersion:
+		return j, fmt.Errorf("%w: %w `version`: `%d`", errConfigVersion, errConfigVersionMax, current64)
 	}
+	current := uint16(current64)
 
 	switch {
 	case target == current:
 		return j, nil
 	case latest < current:
-		warnVersionNotRegistered(current, latest, errConfigVersion)
-		return j, errConfigVersion
+		err := fmt.Errorf("%w: %w", errConfigVersion, errConfigVersionUnavail)
+		warnVersionNotRegistered(current, latest, err)
+		return j, err
 	case target > latest:
 		warnVersionNotRegistered(target, latest, errTargetVersion)
 		return j, errTargetVersion
@@ -136,7 +145,7 @@ func (m *manager) Deploy(ctx context.Context, j []byte, version int) ([]byte, er
 			current = patchVersion - 1
 		}
 
-		if j, err = jsonparser.Set(j, []byte(strconv.Itoa(current)), "version"); err != nil {
+		if j, err = jsonparser.Set(j, []byte(strconv.FormatUint(uint64(current), 10)), "version"); err != nil {
 			return j, fmt.Errorf("%w `version` during %s %v: %w", common.ErrSettingField, action, patchVersion, err)
 		}
 	}
@@ -202,13 +211,13 @@ func (m *manager) registerVersion(ver int, v any) {
 }
 
 // latest returns the highest version number
-func (m *manager) latest() (int, error) {
+func (m *manager) latest() (uint16, error) {
 	m.m.RLock()
 	defer m.m.RUnlock()
 	if len(m.versions) == 0 {
 		return 0, errNoVersions
 	}
-	return len(m.versions) - 1, nil
+	return uint16(len(m.versions)) - 1, nil
 }
 
 // checkVersions ensures that registered versions are consistent
@@ -228,7 +237,7 @@ func (m *manager) checkVersions() error {
 	return nil
 }
 
-func warnVersionNotRegistered(current, latest int, msg error) {
+func warnVersionNotRegistered(current, latest uint16, msg error) {
 	fmt.Fprintf(os.Stderr, `
 %s ('%d' > '%d')
 Switch back to the version of GoCryptoTrader containing config version '%d' and run:

config/versions/versions_test.go

@@ -13,64 +13,67 @@ import (
 func TestDeploy(t *testing.T) {
 	t.Parallel()
 	m := manager{}
-	_, err := m.Deploy(context.Background(), []byte(``), LatestVersion)
+	_, err := m.Deploy(context.Background(), []byte(``), UseLatestVersion)
 	assert.ErrorIs(t, err, errNoVersions)
 
 	m.registerVersion(1, &TestVersion1{})
-	_, err = m.Deploy(context.Background(), []byte(``), LatestVersion)
+	_, err = m.Deploy(context.Background(), []byte(``), UseLatestVersion)
 	require.ErrorIs(t, err, errVersionIncompatible)
 
 	m = manager{}
 
 	m.registerVersion(0, &Version0{})
-	_, err = m.Deploy(context.Background(), []byte(`not an object`), LatestVersion)
+	m.registerVersion(1, &Version1{})
+	_, err = m.Deploy(context.Background(), []byte(`not an object`), UseLatestVersion)
 	require.ErrorIs(t, err, jsonparser.KeyPathNotFoundError, "Must throw the correct error trying to add version to bad json")
 	require.ErrorIs(t, err, common.ErrSettingField, "Must throw the correct error trying to add version to bad json")
 	require.ErrorContains(t, err, "version", "Must throw the correct error trying to add version to bad json")
 
-	_, err = m.Deploy(context.Background(), []byte(`{"version":"not an int"}`), LatestVersion)
+	_, err = m.Deploy(context.Background(), []byte(`{"version":"not an int"}`), UseLatestVersion)
 	require.ErrorIs(t, err, common.ErrGettingField, "Must throw the correct error trying to get version from bad json")
 
-	in := []byte(`{"version":0,"exchanges":[{"name":"Juan"}]}`)
-	j, err := m.Deploy(context.Background(), in, LatestVersion)
-	require.NoError(t, err)
-	assert.Equal(t, string(in), string(j))
+	_, err = m.Deploy(context.Background(), []byte(`{"version":65535}`), UseLatestVersion)
+	require.ErrorIs(t, err, errConfigVersionMax, "Must throw the correct error when version is too high")
 
-	m.registerVersion(1, &Version1{})
-	j, err = m.Deploy(context.Background(), in, LatestVersion)
+	_, err = m.Deploy(context.Background(), []byte(`{"version":-1}`), UseLatestVersion)
+	require.ErrorIs(t, err, errConfigVersionNegative, "Must throw the correct error when version is negative")
+
+	in := []byte(`{"version":0,"exchanges":[{"name":"Juan"}]}`)
+	j, err := m.Deploy(context.Background(), in, UseLatestVersion)
 	require.NoError(t, err)
 	assert.Contains(t, string(j), `"version": 1`)
 
+	j2, err := m.Deploy(context.Background(), j, UseLatestVersion)
+	require.NoError(t, err, "Deploy the same version again must not error")
+	require.Equal(t, string(j2), string(j), "Deploy the same version again must not change config")
+
 	_, err = m.Deploy(context.Background(), j, 2)
 	assert.ErrorIs(t, err, errTargetVersion, "Downgrade to a unregistered version should not be allowed")
 
 	m.versions = append(m.versions, &TestVersion2{ConfigErr: true, ExchErr: false})
-	_, err = m.Deploy(context.Background(), j, LatestVersion)
+	_, err = m.Deploy(context.Background(), j, UseLatestVersion)
 	require.ErrorIs(t, err, errUpgrade)
 
 	m.versions[len(m.versions)-1] = &TestVersion2{ConfigErr: false, ExchErr: true}
-	_, err = m.Deploy(context.Background(), in, LatestVersion)
+	_, err = m.Deploy(context.Background(), in, UseLatestVersion)
 	require.Implements(t, (*ExchangeVersion)(nil), m.versions[1])
 	require.ErrorIs(t, err, errUpgrade)
 
-	j2, err := m.Deploy(context.Background(), j, 0)
+	j2, err = m.Deploy(context.Background(), j, 0)
 	require.NoError(t, err)
 	assert.Contains(t, string(j2), `"version": 0`, "Explicit downgrade should work correctly")
 
 	m.versions = m.versions[:1]
-	_, err = m.Deploy(context.Background(), j, LatestVersion)
-	assert.ErrorIs(t, err, errConfigVersion, "Config version ahead of latest version should error")
-
-	_, err = m.Deploy(context.Background(), j, 0)
-	assert.ErrorIs(t, err, errConfigVersion, "Config version ahead of latest version should error")
+	_, err = m.Deploy(context.Background(), j, UseLatestVersion)
+	assert.ErrorIs(t, err, errConfigVersionUnavail, "Config version ahead of latest version should error")
 }
 
 // TestExchangeDeploy exercises exchangeDeploy
 // There are a number of error paths we can't currently cover without exposing unacceptable risks to the hot-paths as well
 func TestExchangeDeploy(t *testing.T) {
 	t.Parallel()
 	m := manager{}
-	_, err := m.Deploy(context.Background(), []byte(``), LatestVersion)
+	_, err := m.Deploy(context.Background(), []byte(``), UseLatestVersion)
 	assert.ErrorIs(t, err, errNoVersions)
 
 	v := &TestVersion2{}
@@ -113,10 +116,10 @@ func TestLatest(t *testing.T) {
 	m.registerVersion(1, &Version1{})
 	v, err := m.latest()
 	require.NoError(t, err)
-	assert.Equal(t, 1, v)
+	assert.Equal(t, uint16(1), v)
 
 	m.registerVersion(2, &Version2{})
 	v, err = m.latest()
 	require.NoError(t, err)
-	assert.Equal(t, 2, v)
+	assert.Equal(t, uint16(2), v)
 }