Skip to content

Commit

Permalink
Binance: Fix UpdateOrderExecutionLimits margin test
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gbjk committed Feb 12, 2024
1 parent 58caf2c commit 2e5736a
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 138 deletions.
103 changes: 50 additions & 53 deletions exchanges/binance/binance.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ import (
"fmt"
"net/http"
"net/url"
"slices"
"sort"
"strconv"
"strings"
"time"

"github.com/thrasher-corp/gocryptotrader/common"
Expand Down Expand Up @@ -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
}
Expand Down
118 changes: 42 additions & 76 deletions exchanges/binance/binance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}

Expand Down Expand Up @@ -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)
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions exchanges/binance/binance_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
8 changes: 2 additions & 6 deletions exchanges/binance/binance_wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion exchanges/btse/btse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 2e5736a

Please sign in to comment.