diff --git a/exchanges/binance/binance.go b/exchanges/binance/binance.go index e1423d9e..2cea9467 100644 --- a/exchanges/binance/binance.go +++ b/exchanges/binance/binance.go @@ -296,16 +296,12 @@ func (b *Binance) GetUserMarginInterestHistory(ctx context.Context, assetCurrenc func (b *Binance) GetAggregatedTrades(ctx context.Context, arg *AggregatedTradeRequestParams) ([]AggregatedTrade, error) { params := url.Values{} params.Set("symbol", arg.Symbol.String()) - // if the user request is directly not supported by the exchange, we might be able to fulfill it + // If the user request is directly not supported by the exchange, we might be able to fulfill it // by merging results from multiple API requests - needBatch := false - if arg.Limit > 0 { - if arg.Limit > 1000 { - // remote call doesn't support higher limits - needBatch = true - } else { - params.Set("limit", strconv.Itoa(arg.Limit)) - } + needBatch := true // Need to batch unless user has specified a limit + if arg.Limit > 0 && arg.Limit <= 1000 { + needBatch = false + params.Set("limit", strconv.Itoa(arg.Limit)) } if arg.FromID != 0 { params.Set("fromId", strconv.FormatInt(arg.FromID, 10)) @@ -321,7 +317,7 @@ func (b *Binance) GetAggregatedTrades(ctx context.Context, arg *AggregatedTradeR needBatch = needBatch || (!arg.StartTime.IsZero() && !arg.EndTime.IsZero() && arg.EndTime.Sub(arg.StartTime) > time.Hour) // Fall back to batch requests, if possible and necessary if needBatch { - // fromId xor start time must be set + // fromId or start time must be set canBatch := arg.FromID == 0 != arg.StartTime.IsZero() if canBatch { // Split the request into multiple diff --git a/exchanges/binance/binance_test.go b/exchanges/binance/binance_test.go index e4c53d99..93c37c8a 100644 --- a/exchanges/binance/binance_test.go +++ b/exchanges/binance/binance_test.go @@ -1478,27 +1478,20 @@ func TestNewOrderTest(t *testing.T) { func TestGetHistoricTrades(t *testing.T) { t.Parallel() - currencyPair, err := currency.NewPairFromString("BTCUSDT") - if err != nil { - t.Fatal(err) - } - start, err := time.Parse(time.RFC3339, "2020-01-02T15:04:05Z") - if err != nil { - t.Fatal(err) - } - result, err := b.GetHistoricTrades(context.Background(), - currencyPair, asset.Spot, start, start.Add(15*time.Minute)) - if err != nil { - t.Error(err) - } - var expected int + p := currency.NewPair(currency.BTC, currency.USDT) + start := time.Unix(1577977445, 0) // 2020-01-02 15:04:05 + end := start.Add(15 * time.Minute) // 2020-01-02 15:19:05 + result, err := b.GetHistoricTrades(context.Background(), p, asset.Spot, start, end) + assert.NoError(t, err, "GetHistoricTrades should not error") + expected := 2134 if mockTests { - expected = 5 - } else { - expected = 2134 + expected = 1002 } - if len(result) != expected { - t.Errorf("GetHistoricTrades() expected %v entries, got %v", expected, len(result)) + assert.Equal(t, expected, len(result), "GetHistoricTrades should return correct number of entries") // assert.Len doesn't produce clear messages on result + for _, r := range result { + if !assert.WithinRange(t, r.Timestamp, start, end, "All trades should be within time range") { + break + } } }