From 2e5736a63160d86fe5fb0c2bbe53bec57ee0a8c2 Mon Sep 17 00:00:00 2001 From: Gareth Kirwan Date: Thu, 1 Feb 2024 11:42:12 +0700 Subject: [PATCH] Binance: Fix UpdateOrderExecutionLimits margin test 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 | 103 ++++++++++++----------- 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, 97 insertions(+), 138 deletions(-) diff --git a/exchanges/binance/binance.go b/exchanges/binance/binance.go index fa498f5eb74..ef077a336f3 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 } - for z := range assets { - l := order.MinMaxLevel{ - Pair: cp, - Asset: assets[z], - } + l := order.MinMaxLevel{ + Pair: cp, + Asset: a, + } - 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 - } + 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 } - - 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 df8b2458fea..01f988d290f 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 de28e29652f..15172903b6e 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 d981fa4b2f3..9f04982d082 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 8f136b9a295..8a566d31f32 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) {