d0zingcat/gocryptotrader
1
0
Fork 0
mirror of https://github.com/d0zingcat/gocryptotrader.git synced 2026-05-24 23:16:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Binance: Fix GetHistoricTrades failing live (#1442)

Browse Source 
This test was failing with `-tags mock_test_off` because the API imposes
a default limit of 500 now, so we'd just get 500 trades and no batching.

If the user doesn't provide a limit, that means we need to assume
batching
This commit is contained in:
Gareth Kirwan
2024-01-11 03:21:04 +01:00
committed by GitHub
parent fb6d12ac69
commit 614042110a
2 changed files with 18 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
exchanges/binance/binance.go
View File

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

31
exchanges/binance/binance_test.go
View File

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