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

@@ -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
 }

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)
 		}
 	}
 }

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"`

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)
 	}

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) {