From 9ff502bac21399ad1e3dbeffb306da9cfb8b914b Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Mon, 12 Feb 2024 08:03:11 +0100 Subject: [PATCH] Binance: Fix UpdateOrderExecutionLimits margin test (#1464) Calls for limits for margin asset were not doing anything if spot is enabled. This caused intermittent failures when the sequence of tests meant margin was tested before spot, since the limits wouldn't be loaded. But it also highlighted that API users calling Update Limits for margin outside the context of a loop on all assets would not get an update. Also intermittently when the loop of UpdateLimits on assets puts margin first --- exchanges/binance/binance.go | 107 ++++++++++++------------ exchanges/binance/binance_test.go | 118 ++++++++++----------------- exchanges/binance/binance_types.go | 4 +- exchanges/binance/binance_wrapper.go | 8 +- exchanges/btse/btse_test.go | 2 +- 5 files changed, 99 insertions(+), 140 deletions(-) diff --git a/exchanges/binance/binance.go b/exchanges/binance/binance.go index fa498f5e..ef077a33 100644 --- a/exchanges/binance/binance.go +++ b/exchanges/binance/binance.go @@ -7,8 +7,10 @@ import ( "fmt" "net/http" "net/url" + "slices" "sort" "strconv" + "strings" "time" "github.com/thrasher-corp/gocryptotrader/common" @@ -1209,72 +1211,67 @@ func (b *Binance) MaintainWsAuthStreamKey(ctx context.Context) error { }, request.AuthenticatedRequest) } -// FetchSpotExchangeLimits fetches spot order execution limits -func (b *Binance) FetchSpotExchangeLimits(ctx context.Context) ([]order.MinMaxLevel, error) { - var limits []order.MinMaxLevel - spot, err := b.GetExchangeInfo(ctx) +// FetchExchangeLimits fetches order execution limits filtered by asset +func (b *Binance) FetchExchangeLimits(ctx context.Context, a asset.Item) ([]order.MinMaxLevel, error) { + if a != asset.Spot && a != asset.Margin { + return nil, fmt.Errorf("%w %v", asset.ErrNotSupported, a) + } + + resp, err := b.GetExchangeInfo(ctx) if err != nil { return nil, err } - for x := range spot.Symbols { + aUpper := strings.ToUpper(a.String()) + + limits := make([]order.MinMaxLevel, 0, len(resp.Symbols)) + for _, s := range resp.Symbols { var cp currency.Pair - cp, err = currency.NewPairFromStrings(spot.Symbols[x].BaseAsset, - spot.Symbols[x].QuoteAsset) + cp, err = currency.NewPairFromStrings(s.BaseAsset, s.QuoteAsset) if err != nil { return nil, err } - var assets []asset.Item - for y := range spot.Symbols[x].Permissions { - switch spot.Symbols[x].Permissions[y] { - case "SPOT": - assets = append(assets, asset.Spot) - case "MARGIN": - assets = append(assets, asset.Margin) - default: - // "LEVERAGED", "TRD_GRP_003", "TRD_GRP_004", "TRD_GRP_005" etc are unused permissions - // for spot exchange limits + + if !slices.Contains(s.Permissions, aUpper) { + continue + } + + l := order.MinMaxLevel{ + Pair: cp, + Asset: a, + } + + for _, f := range s.Filters { + // TODO: Unhandled filters: + // maxPosition, trailingDelta, percentPriceBySide, maxNumAlgoOrders + switch f.FilterType { + case priceFilter: + l.MinPrice = f.MinPrice + l.MaxPrice = f.MaxPrice + l.PriceStepIncrementSize = f.TickSize + case percentPriceFilter: + l.MultiplierUp = f.MultiplierUp + l.MultiplierDown = f.MultiplierDown + l.AveragePriceMinutes = f.AvgPriceMinutes + case lotSizeFilter: + l.MaximumBaseAmount = f.MaxQty + l.MinimumBaseAmount = f.MinQty + l.AmountStepIncrementSize = f.StepSize + case notionalFilter: + l.MinNotional = f.MinNotional + case icebergPartsFilter: + l.MaxIcebergParts = f.Limit + case marketLotSizeFilter: + l.MarketMinQty = f.MinQty + l.MarketMaxQty = f.MaxQty + l.MarketStepIncrementSize = f.StepSize + case maxNumOrdersFilter: + l.MaxTotalOrders = f.MaxNumOrders + l.MaxAlgoOrders = f.MaxNumAlgoOrders } } - for z := range assets { - l := order.MinMaxLevel{ - Pair: cp, - Asset: assets[z], - } - - for _, f := range spot.Symbols[x].Filters { - // TODO: Unhandled filters: - // maxPosition, trailingDelta, percentPriceBySide, maxNumAlgoOrders - switch f.FilterType { - case priceFilter: - l.MinPrice = f.MinPrice - l.MaxPrice = f.MaxPrice - l.PriceStepIncrementSize = f.TickSize - case percentPriceFilter: - l.MultiplierUp = f.MultiplierUp - l.MultiplierDown = f.MultiplierDown - l.AveragePriceMinutes = f.AvgPriceMinutes - case lotSizeFilter: - l.MaximumBaseAmount = f.MaxQty - l.MinimumBaseAmount = f.MinQty - l.AmountStepIncrementSize = f.StepSize - case notionalFilter: - l.MinNotional = f.MinNotional - case icebergPartsFilter: - l.MaxIcebergParts = f.Limit - case marketLotSizeFilter: - l.MarketMinQty = f.MinQty - l.MarketMaxQty = f.MaxQty - l.MarketStepIncrementSize = f.StepSize - case maxNumOrdersFilter: - l.MaxTotalOrders = f.MaxNumOrders - l.MaxAlgoOrders = f.MaxNumAlgoOrders - } - } - - limits = append(limits, l) - } + limits = append(limits, l) } return limits, nil } diff --git a/exchanges/binance/binance_test.go b/exchanges/binance/binance_test.go index df8b2458..01f988d2 100644 --- a/exchanges/binance/binance_test.go +++ b/exchanges/binance/binance_test.go @@ -1083,14 +1083,12 @@ func TestGetMarkPriceKline(t *testing.T) { func TestGetExchangeInfo(t *testing.T) { t.Parallel() info, err := b.GetExchangeInfo(context.Background()) - if err != nil { - t.Error(err) - } + require.NoError(t, err, "GetExchangeInfo must not error") if mockTests { - serverTime := time.Date(2022, 2, 25, 3, 50, 40, int(601*time.Millisecond), time.UTC) - if !info.ServerTime.Equal(serverTime) { - t.Errorf("Expected %v, got %v", serverTime, info.ServerTime) - } + exp := time.Date(2022, 2, 25, 3, 50, 40, int(601*time.Millisecond), time.UTC) + assert.True(t, info.ServerTime.Equal(exp), "ServerTime should be correct") + } else { + assert.WithinRange(t, info.ServerTime, time.Now().Add(-24*time.Hour), time.Now().Add(24*time.Hour), "ServerTime should be within a day of now") } } @@ -2782,93 +2780,61 @@ func TestFormatUSDTMarginedFuturesPair(t *testing.T) { } } -func TestFetchSpotExchangeLimits(t *testing.T) { +func TestFetchExchangeLimits(t *testing.T) { t.Parallel() - limits, err := b.FetchSpotExchangeLimits(context.Background()) - if !errors.Is(err, nil) { - t.Errorf("received '%v', expected '%v'", err, nil) - } - if len(limits) == 0 { - t.Error("expected a response") - } + limits, err := b.FetchExchangeLimits(context.Background(), asset.Spot) + assert.NoError(t, err, "FetchExchangeLimits should not error") + assert.NotEmpty(t, limits, "Should get some limits back") + + limits, err = b.FetchExchangeLimits(context.Background(), asset.Margin) + assert.NoError(t, err, "FetchExchangeLimits should not error") + assert.NotEmpty(t, limits, "Should get some limits back") + + _, err = b.FetchExchangeLimits(context.Background(), asset.Futures) + assert.ErrorIs(t, err, asset.ErrNotSupported, "FetchExchangeLimits should error on other asset types") } func TestUpdateOrderExecutionLimits(t *testing.T) { t.Parallel() + tests := map[asset.Item]currency.Pair{ asset.Spot: currency.NewPair(currency.BTC, currency.USDT), asset.Margin: currency.NewPair(currency.ETH, currency.BTC), } for _, a := range []asset.Item{asset.CoinMarginedFutures, asset.USDTMarginedFutures} { pairs, err := b.FetchTradablePairs(context.Background(), a) - if err != nil { - t.Errorf("Error fetching dated %s pairs for test: %v", a, err) - } + require.NoErrorf(t, err, "FetchTradablePairs should not error for %s", a) + require.NotEmptyf(t, pairs, "Should get some pairs for %s", a) tests[a] = pairs[0] } for _, a := range b.GetAssetTypes(false) { - if err := b.UpdateOrderExecutionLimits(context.Background(), a); err != nil { - t.Error("Binance UpdateOrderExecutionLimits() error", err) - continue - } + err := b.UpdateOrderExecutionLimits(context.Background(), a) + require.NoError(t, err, "UpdateOrderExecutionLimits should not error") p := tests[a] limits, err := b.GetOrderExecutionLimits(a, p) - if err != nil { - t.Errorf("Binance GetOrderExecutionLimits() error during TestUpdateOrderExecutionLimits; Asset: %s Pair: %s Err: %v", a, p, err) - continue - } - if limits.MinPrice == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty MinPrice; Asset: %s, Pair: %s, Got: %v", a, p, limits.MinPrice) - } - if limits.MaxPrice == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty MaxPrice; Asset: %s, Pair: %s, Got: %v", a, p, limits.MaxPrice) - } - if limits.PriceStepIncrementSize == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty PriceStepIncrementSize; Asset: %s, Pair: %s, Got: %v", a, p, limits.PriceStepIncrementSize) - } - if limits.MinimumBaseAmount == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty MinAmount; Asset: %s, Pair: %s, Got: %v", a, p, limits.MinimumBaseAmount) - } - if limits.MaximumBaseAmount == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty MaxAmount; Asset: %s, Pair: %s, Got: %v", a, p, limits.MaximumBaseAmount) - } - if limits.AmountStepIncrementSize == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty AmountStepIncrementSize; Asset: %s, Pair: %s, Got: %v", a, p, limits.AmountStepIncrementSize) - } - if a == asset.USDTMarginedFutures && limits.MinNotional == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty MinNotional; Asset: %s, Pair: %s, Got: %v", a, p, limits.MinNotional) - } - if limits.MarketMaxQty == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty MarketMaxQty; Asset: %s, Pair: %s, Got: %v", a, p, limits.MarketMaxQty) - } - if limits.MaxTotalOrders == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty MaxTotalOrders; Asset: %s, Pair: %s, Got: %v", a, p, limits.MaxTotalOrders) - } - - if a == asset.Spot || a == asset.Margin { - if limits.MaxIcebergParts == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty MaxIcebergParts; Asset: %s, Pair: %s, Got: %v", a, p, limits.MaxIcebergParts) - } - } - - if a == asset.CoinMarginedFutures || a == asset.USDTMarginedFutures { - if limits.MultiplierUp == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty MultiplierUp; Asset: %s, Pair: %s, Got: %v", a, p, limits.MultiplierUp) - } - if limits.MultiplierDown == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty MultiplierDown; Asset: %s, Pair: %s, Got: %v", a, p, limits.MultiplierDown) - } - if limits.MarketMinQty == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty MarketMinQty; Asset: %s, Pair: %s, Got: %v", a, p, limits.MarketMinQty) - } - if limits.MarketStepIncrementSize == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty MarketStepIncrementSize; Asset: %s, Pair: %s, Got: %v", a, p, limits.MarketStepIncrementSize) - } - if limits.MaxAlgoOrders == 0 { - t.Errorf("Binance UpdateOrderExecutionLimits empty MaxAlgoOrders; Asset: %s, Pair: %s, Got: %v", a, p, limits.MaxAlgoOrders) - } + require.NoErrorf(t, err, "GetOrderExecutionLimits should not error for %s pair %s", a, p) + assert.Positivef(t, limits.MinPrice, "MinPrice must be positive for %s pair %s", a, p) + assert.Positivef(t, limits.MaxPrice, "MaxPrice must be positive for %s pair %s", a, p) + assert.Positivef(t, limits.PriceStepIncrementSize, "PriceStepIncrementSize must be positive for %s pair %s", a, p) + assert.Positivef(t, limits.MinimumBaseAmount, "MinimumBaseAmount must be positive for %s pair %s", a, p) + assert.Positivef(t, limits.MaximumBaseAmount, "MaximumBaseAmount must be positive for %s pair %s", a, p) + assert.Positivef(t, limits.AmountStepIncrementSize, "AmountStepIncrementSize must be positive for %s pair %s", a, p) + assert.Positivef(t, limits.MarketMaxQty, "MarketMaxQty must be positive for %s pair %s", a, p) + assert.Positivef(t, limits.MaxTotalOrders, "MaxTotalOrders must be positive for %s pair %s", a, p) + switch a { + case asset.Spot, asset.Margin: + assert.Positivef(t, limits.MaxIcebergParts, "MaxIcebergParts must be positive for %s pair %s", a, p) + case asset.USDTMarginedFutures: + assert.Positivef(t, limits.MinNotional, "MinNotional must be positive for %s pair %s", a, p) + fallthrough + case asset.CoinMarginedFutures: + assert.Positivef(t, limits.MultiplierUp, "MultiplierUp must be positive for %s pair %s", a, p) + assert.Positivef(t, limits.MultiplierDown, "MultiplierDown must be positive for %s pair %s", a, p) + assert.Positivef(t, limits.MarketMinQty, "MarketMinQty must be positive for %s pair %s", a, p) + assert.Positivef(t, limits.MarketStepIncrementSize, "MarketStepIncrementSize must be positive for %s pair %s", a, p) + assert.Positivef(t, limits.MaxAlgoOrders, "MaxAlgoOrders must be positive for %s pair %s", a, p) } } } diff --git a/exchanges/binance/binance_types.go b/exchanges/binance/binance_types.go index de28e296..15172903 100644 --- a/exchanges/binance/binance_types.go +++ b/exchanges/binance/binance_types.go @@ -44,13 +44,13 @@ type ExchangeInfo struct { Msg string `json:"msg"` Timezone string `json:"timezone"` ServerTime time.Time `json:"serverTime"` - RateLimits []struct { + RateLimits []*struct { RateLimitType string `json:"rateLimitType"` Interval string `json:"interval"` Limit int `json:"limit"` } `json:"rateLimits"` ExchangeFilters interface{} `json:"exchangeFilters"` - Symbols []struct { + Symbols []*struct { Symbol string `json:"symbol"` Status string `json:"status"` BaseAsset string `json:"baseAsset"` diff --git a/exchanges/binance/binance_wrapper.go b/exchanges/binance/binance_wrapper.go index d981fa4b..9f04982d 100644 --- a/exchanges/binance/binance_wrapper.go +++ b/exchanges/binance/binance_wrapper.go @@ -1894,17 +1894,13 @@ func (b *Binance) UpdateOrderExecutionLimits(ctx context.Context, a asset.Item) var err error switch a { case asset.Spot: - limits, err = b.FetchSpotExchangeLimits(ctx) + limits, err = b.FetchExchangeLimits(ctx, asset.Spot) case asset.USDTMarginedFutures: limits, err = b.FetchUSDTMarginExchangeLimits(ctx) case asset.CoinMarginedFutures: limits, err = b.FetchCoinMarginExchangeLimits(ctx) case asset.Margin: - if err = b.CurrencyPairs.IsAssetEnabled(asset.Spot); err != nil { - limits, err = b.FetchSpotExchangeLimits(ctx) - } else { - return nil - } + limits, err = b.FetchExchangeLimits(ctx, asset.Margin) default: err = fmt.Errorf("%w %v", asset.ErrNotSupported, a) } diff --git a/exchanges/btse/btse_test.go b/exchanges/btse/btse_test.go index 8f136b9a..8a566d31 100644 --- a/exchanges/btse/btse_test.go +++ b/exchanges/btse/btse_test.go @@ -204,7 +204,7 @@ func TestWrapperGetServerTime(t *testing.T) { t.Parallel() st, err := b.GetServerTime(context.Background(), asset.Spot) assert.NoError(t, err, "GetServerTime should not error") - assert.WithinRange(t, st, time.Now().Add(-24*time.Hour), time.Now().Add(24*time.Hour), "Time should be within a day of what now") + assert.WithinRange(t, st, time.Now().Add(-24*time.Hour), time.Now().Add(24*time.Hour), "Time should be within a day of now") } func TestGetWalletInformation(t *testing.T) {