Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config: AssetEnabled upgrade #1735

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
+671 −877
Open

Config: AssetEnabled upgrade #1735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
4d32d83
Config: Move assetEnabled upgrade to Version management
gbjk Nov 13, 2024
9253089
Assets: Do not error on asset not enabled, or disabled
gbjk Nov 26, 2024
95d9c4f
CurrencyManager: Set AssetEnabled when StorePairs(enabled)
gbjk Nov 26, 2024
f67c6fe
RPCServer: Fix tests expecting StoreAssetPairFormat to enable the asset
gbjk Nov 26, 2024
7b5b7d4
Bitfinex: Fix tests for MarginFunding subs
gbjk Nov 27, 2024
7caff83
GCTWrapper: Improve TestMain clarity
gbjk Nov 27, 2024
ce57ece
Linter: Fixes for exchange.go
gbjk Nov 27, 2024
cb6f6f0
BTSE: Add futures to testconfig
gbjk Nov 27, 2024
b6105a2
Exchanges: Rename StoreAssetPairStore
gbjk Nov 28, 2024
06558ca
Config: AssetEnabled upgrade should respect assetTypes
gbjk Dec 1, 2024
4a4fb99
fixup! CurrencyManager: Set AssetEnabled when StorePairs(enabled)
gbjk Dec 13, 2024
36e3dff
fixup! Config: Move assetEnabled upgrade to Version management
gbjk Dec 27, 2024
d6c8966
fixup! RPCServer: Fix tests expecting StoreAssetPairFormat to enable …
gbjk Dec 27, 2024
bfa72e2
fixup! Exchanges: Rename StoreAssetPairStore
gbjk Dec 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions backtester/data/kline/api/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"time"

"github.com/thrasher-corp/gocryptotrader/backtester/common"
"github.com/thrasher-corp/gocryptotrader/common/convert"
"github.com/thrasher-corp/gocryptotrader/currency"
"github.com/thrasher-corp/gocryptotrader/engine"
"github.com/thrasher-corp/gocryptotrader/exchanges/asset"
Expand All @@ -30,7 +29,7 @@ func TestLoadCandles(t *testing.T) {
b.CurrencyPairs.Pairs[asset.Spot] = &currency.PairStore{
Available: currency.Pairs{cp},
Enabled: currency.Pairs{cp},
AssetEnabled: convert.BoolPtr(true),
AssetEnabled: true,
ConfigFormat: &currency.PairFormat{Uppercase: true},
RequestFormat: &currency.PairFormat{Uppercase: true}}
tt1 := time.Now().Add(-time.Minute).Round(gctkline.OneMin.Duration())
Expand Down Expand Up @@ -68,7 +67,7 @@ func TestLoadTrades(t *testing.T) {
b.CurrencyPairs.Pairs[asset.Spot] = &currency.PairStore{
Available: currency.Pairs{cp},
Enabled: currency.Pairs{cp},
AssetEnabled: convert.BoolPtr(true),
AssetEnabled: true,
ConfigFormat: &currency.PairFormat{Uppercase: true},
RequestFormat: &currency.PairFormat{Uppercase: true}}
interval := gctkline.OneMin
Expand Down
5 changes: 2 additions & 3 deletions backtester/data/kline/live/live_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"time"

"github.com/thrasher-corp/gocryptotrader/backtester/common"
"github.com/thrasher-corp/gocryptotrader/common/convert"
"github.com/thrasher-corp/gocryptotrader/currency"
"github.com/thrasher-corp/gocryptotrader/engine"
"github.com/thrasher-corp/gocryptotrader/exchanges/asset"
Expand All @@ -33,7 +32,7 @@ func TestLoadCandles(t *testing.T) {
b.CurrencyPairs.Pairs[asset.Spot] = &currency.PairStore{
Available: currency.Pairs{cp},
Enabled: currency.Pairs{cp},
AssetEnabled: convert.BoolPtr(true),
AssetEnabled: true,
RequestFormat: pFormat,
ConfigFormat: pFormat,
}
Expand Down Expand Up @@ -68,7 +67,7 @@ func TestLoadTrades(t *testing.T) {
b.CurrencyPairs.Pairs[asset.Spot] = &currency.PairStore{
Available: currency.Pairs{cp},
Enabled: currency.Pairs{cp},
AssetEnabled: convert.BoolPtr(true),
AssetEnabled: true,
RequestFormat: pFormat,
ConfigFormat: pFormat,
}
Expand Down
9 changes: 4 additions & 5 deletions backtester/engine/backtest_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (
"github.com/thrasher-corp/gocryptotrader/backtester/funding"
"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"
"github.com/thrasher-corp/gocryptotrader/currency"
"github.com/thrasher-corp/gocryptotrader/database"
Expand Down Expand Up @@ -182,7 +181,7 @@ func TestLoadDataAPI(t *testing.T) {
b.CurrencyPairs.Pairs[asset.Spot] = &currency.PairStore{
Available: currency.Pairs{cp},
Enabled: currency.Pairs{cp},
AssetEnabled: convert.BoolPtr(true),
AssetEnabled: true,
ConfigFormat: &currency.PairFormat{Uppercase: true},
RequestFormat: &currency.PairFormat{Uppercase: true}}

Expand Down Expand Up @@ -236,7 +235,7 @@ func TestLoadDataCSV(t *testing.T) {
b.CurrencyPairs.Pairs[asset.Spot] = &currency.PairStore{
Available: currency.Pairs{cp},
Enabled: currency.Pairs{cp},
AssetEnabled: convert.BoolPtr(true),
AssetEnabled: true,
ConfigFormat: &currency.PairFormat{Uppercase: true},
RequestFormat: &currency.PairFormat{Uppercase: true}}
_, err = bt.loadData(cfg, exch, cp, asset.Spot, false)
Expand Down Expand Up @@ -301,7 +300,7 @@ func TestLoadDataDatabase(t *testing.T) {
b.CurrencyPairs.Pairs[asset.Spot] = &currency.PairStore{
Available: currency.Pairs{cp},
Enabled: currency.Pairs{cp},
AssetEnabled: convert.BoolPtr(true),
AssetEnabled: true,
ConfigFormat: &currency.PairFormat{Uppercase: true},
RequestFormat: &currency.PairFormat{Uppercase: true}}
bt.databaseManager, err = engine.SetupDatabaseConnectionManager(&cfg.DataSettings.DatabaseData.Config)
Expand Down Expand Up @@ -384,7 +383,7 @@ func TestLoadDataLive(t *testing.T) {
b.CurrencyPairs.Pairs[asset.Spot] = &currency.PairStore{
Available: currency.Pairs{cp},
Enabled: currency.Pairs{cp},
AssetEnabled: convert.BoolPtr(true),
AssetEnabled: true,
ConfigFormat: &currency.PairFormat{Uppercase: true},
RequestFormat: &currency.PairFormat{Uppercase: true}}
_, err = bt.loadData(cfg, exch, cp, asset.Spot, false)
Expand Down
43 changes: 14 additions & 29 deletions backtester/engine/live_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"testing"
"time"

"github.com/stretchr/testify/require"
"github.com/thrasher-corp/gocryptotrader/backtester/common"
"github.com/thrasher-corp/gocryptotrader/backtester/data"
datakline "github.com/thrasher-corp/gocryptotrader/backtester/data/kline"
Expand All @@ -15,7 +16,6 @@ import (
"github.com/thrasher-corp/gocryptotrader/backtester/funding"
"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/currency"
"github.com/thrasher-corp/gocryptotrader/engine"
"github.com/thrasher-corp/gocryptotrader/exchanges/asset"
Expand Down Expand Up @@ -316,25 +316,18 @@ func TestFetchLatestData(t *testing.T) {
funding: &fakeFunding{},
}
_, err := dataHandler.FetchLatestData()
if !errors.Is(err, engine.ErrSubSystemNotStarted) {
t.Errorf("received '%v' expected '%v'", err, engine.ErrSubSystemNotStarted)
}
require.ErrorIs(t, err, engine.ErrSubSystemNotStarted)

dataHandler.started = 1
_, err = dataHandler.FetchLatestData()
if !errors.Is(err, nil) {
t.Errorf("received '%v' expected '%v'", err, nil)
}
cp := currency.NewPair(currency.BTC, currency.USDT).Format(
currency.PairFormat{
Uppercase: true,
})
require.NoError(t, err)
cp := currency.NewBTCUSDT()
f := &binanceus.Binanceus{}
f.SetDefaults()
fb := f.GetBase()
fbA := fb.CurrencyPairs.Pairs[asset.Spot]
fbA.Enabled = fbA.Enabled.Add(cp)
fbA.Available = fbA.Available.Add(cp)
require.NoError(t, fb.CurrencyPairs.SetAssetEnabled(asset.Spot, true), "SetAssetEnabled must not error")
require.NoError(t, fb.CurrencyPairs.StorePairs(asset.Spot, currency.Pairs{cp}, false), "StorePairs must not error")
require.NoError(t, fb.CurrencyPairs.StorePairs(asset.Spot, currency.Pairs{cp}, true), "StorePairs must not error")
dataHandler.sourcesToCheck = []*liveDataSourceDataHandler{
{
exchange: f,
Expand Down Expand Up @@ -370,15 +363,11 @@ func TestFetchLatestData(t *testing.T) {
}
dataHandler.dataHolder = &fakeDataHolder{}
_, err = dataHandler.FetchLatestData()
if !errors.Is(err, nil) {
t.Errorf("received '%v' expected '%v'", err, nil)
}
require.NoError(t, err)

var dh *dataChecker
_, err = dh.FetchLatestData()
if !errors.Is(err, gctcommon.ErrNilPointer) {
t.Errorf("received '%v' expected '%v'", err, gctcommon.ErrNilPointer)
}
require.ErrorIs(t, err, gctcommon.ErrNilPointer)
}

func TestLoadCandleData(t *testing.T) {
Expand All @@ -402,7 +391,7 @@ func TestLoadCandleData(t *testing.T) {
eba := exch.CurrencyPairs.Pairs[asset.Spot]
eba.Available = eba.Available.Add(cp)
eba.Enabled = eba.Enabled.Add(cp)
eba.AssetEnabled = convert.BoolPtr(true)
eba.AssetEnabled = true
l.exchange = exch
l.dataType = common.DataCandle
l.asset = asset.Spot
Expand Down Expand Up @@ -440,16 +429,12 @@ func TestSetDataForClosingAllPositions(t *testing.T) {
}

dataHandler.started = 1
cp := currency.NewPair(currency.BTC, currency.USDT).Format(
currency.PairFormat{
Uppercase: true,
})
cp := currency.NewBTCUSDT()
f := &binanceus.Binanceus{}
f.SetDefaults()
fb := f.GetBase()
fbA := fb.CurrencyPairs.Pairs[asset.Spot]
fbA.Enabled = fbA.Enabled.Add(cp)
fbA.Available = fbA.Available.Add(cp)
err := fb.CurrencyPairs.StorePairs(asset.Spot, currency.Pairs{cp}, true)
require.NoError(t, err, "StorePairs must not error")
dataHandler.sourcesToCheck = []*liveDataSourceDataHandler{
{
exchange: f,
Expand Down Expand Up @@ -484,7 +469,7 @@ func TestSetDataForClosingAllPositions(t *testing.T) {
},
}
dataHandler.dataHolder = &fakeDataHolder{}
_, err := dataHandler.FetchLatestData()
_, err = dataHandler.FetchLatestData()
if !errors.Is(err, nil) {
t.Errorf("received '%v' expected '%v'", err, nil)
}
Expand Down
2 changes: 1 addition & 1 deletion backtester/engine/setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func (bt *BackTest) SetupFromConfig(cfg *config.Config, templatePath, output str
if !ok {
return fmt.Errorf("%v %v %w", cfg.CurrencySettings[i].ExchangeName, cfg.CurrencySettings[i].Asset, asset.ErrNotSupported)
}
exchangeAsset.AssetEnabled = convert.BoolPtr(true)
exchangeAsset.AssetEnabled = true
cp := currency.NewPair(cfg.CurrencySettings[i].Base, cfg.CurrencySettings[i].Quote).Format(*exchangeAsset.RequestFormat)
exchangeAsset.Available = exchangeAsset.Available.Add(cp)
exchangeAsset.Enabled = exchangeAsset.Enabled.Add(cp)
Expand Down
27 changes: 9 additions & 18 deletions backtester/eventhandlers/exchange/exchange_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"time"

"github.com/shopspring/decimal"
"github.com/stretchr/testify/require"
"github.com/thrasher-corp/gocryptotrader/backtester/common"
"github.com/thrasher-corp/gocryptotrader/backtester/data"
"github.com/thrasher-corp/gocryptotrader/backtester/data/kline"
Expand All @@ -16,7 +17,6 @@ import (
"github.com/thrasher-corp/gocryptotrader/backtester/eventtypes/order"
"github.com/thrasher-corp/gocryptotrader/backtester/funding"
gctcommon "github.com/thrasher-corp/gocryptotrader/common"
"github.com/thrasher-corp/gocryptotrader/common/convert"
gctconfig "github.com/thrasher-corp/gocryptotrader/config"
"github.com/thrasher-corp/gocryptotrader/currency"
"github.com/thrasher-corp/gocryptotrader/engine"
Expand Down Expand Up @@ -245,31 +245,22 @@ func TestExecuteOrder(t *testing.T) {
em := engine.NewExchangeManager()
const testExchange = "binanceus"
exch, err := em.NewExchangeByName(testExchange)
if err != nil {
t.Fatal(err)
}
require.NoError(t, err, "NewExchangeByName must not error")
exch.SetDefaults()
exchB := exch.GetBase()
exchB.States = currencystate.NewCurrencyStates()
err = em.Add(exch)
if !errors.Is(err, nil) {
t.Fatalf("received: '%v' but expected: '%v'", err, nil)
}
require.NoError(t, err, "ExchangeManager.Add exchange must not error")
bot.OrderManager, err = engine.SetupOrderManager(em, &engine.CommunicationManager{}, &bot.ServicesWG, &gctconfig.OrderManager{})
if !errors.Is(err, nil) {
t.Errorf("received: %v, expected: %v", err, nil)
}
require.NoError(t, err, "engine.SetupOrderManager must not error")
err = bot.OrderManager.Start()
if !errors.Is(err, nil) {
t.Errorf("received: %v, expected: %v", err, nil)
}
require.NoError(t, err, "OrderManager.Start must not error")

p := currency.NewPair(currency.BTC, currency.USDT)
a := asset.Spot
_, err = exch.FetchOrderbook(context.Background(), p, a)
if err != nil {
t.Fatal(err)
}
require.NoError(t, exchB.CurrencyPairs.SetAssetEnabled(a, true), "SetAssetEnabled must not error")
_, err = exch.FetchOrderbook(context.Background(), p, asset.Spot)
require.NoError(t, err, "FetchOrderbook must not error")
f := &binanceus.Binanceus{}
f.Name = testExchange
cs := Settings{
Expand Down Expand Up @@ -381,7 +372,7 @@ func TestExecuteOrderBuySellSizeLimit(t *testing.T) {
},
Pairs: map[asset.Item]*currency.PairStore{
asset.Spot: {
AssetEnabled: convert.BoolPtr(true),
AssetEnabled: true,
},
},
}
Expand Down
3 changes: 1 addition & 2 deletions backtester/funding/trackingcurrencies/trackingcurrencies_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"errors"
"testing"

"github.com/thrasher-corp/gocryptotrader/common/convert"
"github.com/thrasher-corp/gocryptotrader/currency"
"github.com/thrasher-corp/gocryptotrader/engine"
"github.com/thrasher-corp/gocryptotrader/exchanges/asset"
Expand Down Expand Up @@ -55,7 +54,7 @@ func TestCreateUSDTrackingPairs(t *testing.T) {
eba := exchB.CurrencyPairs.Pairs[a]
eba.Available = eba.Available.Add(cp, cp2, cp3)
eba.Enabled = eba.Enabled.Add(cp, cp2, cp3)
eba.AssetEnabled = convert.BoolPtr(true)
eba.AssetEnabled = true

err = em.Add(exch)
if !errors.Is(err, nil) {
Expand Down
10 changes: 6 additions & 4 deletions cmd/exchange_template/wrapper_file.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,22 +55,24 @@ func ({{.Variable}} *{{.CapitalName}}) SetDefaults() {
// can use this example below:

fmt1 := currency.PairStore{
AssetEnabled: true,
RequestFormat: &currency.PairFormat{Uppercase: true},
ConfigFormat: &currency.PairFormat{Uppercase: true},
}

fmt2 := currency.PairStore{
AssetEnabled: true,
RequestFormat: &currency.PairFormat{Uppercase: true},
ConfigFormat: &currency.PairFormat{Uppercase: true, Delimiter: ":"},
}

err = {{.Variable}}.StoreAssetPairFormat(asset.Spot, fmt1)
err = {{.Variable}}.StoreAssetPairStore(asset.Spot, fmt1)
if err != nil {
log.Errorln(log.ExchangeSys, err)
log.Errorf(log.ExchangeSys, "%s error storing `%s` default asset formats: %s", {{.Variable}}.Name, asset.Spot, err)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you wrap all these details on the error paths in StoreAssetPairStore? That way we don't need to have this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole direction here was the opposite of this change request.
The ideology I'm following is that functions return the error they encounter, without their own context.
If you call a go standard library function, you'll get the same thing.
It's down to the caller to decide if it wants to give context to that error, and what that context looks like from it's perspective.
From the perspective of the function caller, they know what they called, and so the error returned should be undecorated.

Case in point: StoreAssetPairStore is not default asset formats so these callers all needed to clarify the context they were calling that function in.

This is why you'll often see me decorate errors one level above where GCT has previously been doing it.
Sometimes it can feel slightly weighty, in this example where we copy the same context everywhere.

We could definitely add a common error for errStoringDefaultAssetFormats though, and I think that's warranted.
I'll circle back to this after the others and see if you've replied :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding one more bit of context:
This implies that functions must also add their own context.
e.g. If StoreAssetPairStore encountered some error interacting with a database, like "Record not found", We'd expect that error to come without context.
So StoreAssetPairStore should add the context error searching for glove size in database: %w.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From an implementation perspective, maintaining this standard can introduce toil during implementation and reviews, especially since any failure in SetDefaults would inherently cause the exchange to malfunction. Differentiating between errors in this context seems redundant. Still suggest wrapping asset and exchange for the exchange method.
But I will leave this open for other peoples perspective. @thrasher- @gloriousCode

}
err = {{.Variable}}.StoreAssetPairFormat(asset.Margin, fmt2)
err = {{.Variable}}.StoreAssetPairStore(asset.Margin, fmt2)
if err != nil {
log.Errorln(log.ExchangeSys, err)
log.Errorf(log.ExchangeSys, "%s error storing `%s` default asset formats: %s", {{.Variable}}.Name, asset.Margin, err)
}

// Fill out the capabilities/features that the exchange supports
Expand Down
12 changes: 4 additions & 8 deletions cmd/exchange_wrapper_standards/exchange_wrapper_standards_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"testing"
"time"

"github.com/stretchr/testify/require"
"github.com/thrasher-corp/gocryptotrader/common"
"github.com/thrasher-corp/gocryptotrader/common/key"
"github.com/thrasher-corp/gocryptotrader/config"
Expand Down Expand Up @@ -100,14 +101,9 @@ func setupExchange(ctx context.Context, t *testing.T, name string, cfg *config.C
b := exch.GetBase()

assets := b.CurrencyPairs.GetAssetTypes(false)
if len(assets) == 0 {
t.Fatalf("Cannot setup %v, exchange has no assets", name)
}
for j := range assets {
err = b.CurrencyPairs.SetAssetEnabled(assets[j], true)
if err != nil && !errors.Is(err, currency.ErrAssetAlreadyEnabled) {
t.Fatalf("Cannot setup %v SetAssetEnabled %v", name, err)
}
require.NotEmpty(t, assets, "exchange %s must have assets", name)
for _, a := range assets {
require.NoErrorf(t, b.CurrencyPairs.SetAssetEnabled(a, true), "exchange %s SetAssetEnabled must not error for %s", name, a)
}

// Add +1 to len to verify that exchanges can handle requests with unset pairs and assets
Expand Down
10 changes: 0 additions & 10 deletions config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -904,16 +904,6 @@ func (c *Config) CheckExchangeConfigValues() error {
continue
}

for _, a := range assets {
if err := e.CurrencyPairs.IsAssetEnabled(a); errors.Is(err, currency.ErrAssetIsNil) {
// Checks if we have an old config without the ability to enable disable the entire asset
log.Warnf(log.ConfigMgr, "Exchange %s: upgrading config for asset type %s and setting enabled.\n", e.Name, a)
if err := e.CurrencyPairs.SetAssetEnabled(a, true); err != nil {
return err
}
}
}

if enabled := e.CurrencyPairs.GetAssetTypes(true); len(enabled) == 0 {
// turn on an asset if all disabled
log.Warnf(log.ConfigMgr, "%s assets disabled, turning on asset %s", e.Name, assets[0])
Expand Down
Loading
Loading