Merge pull request #1530 from c9s/edwin/okx/fix-trade-id

FIX: [okx] fix trade id

pkg/exchange/okex/convert.go

@@ -150,12 +150,9 @@ func toGlobalTrades(orderDetails []okexapi.OrderDetails) ([]types.Trade, error)
 }
 
 func tradeToGlobal(trade okexapi.Trade) types.Trade {
-	// ** We use the bill id as the trade id, because okx uses billId to perform pagination. **
-	billID := trade.BillId
-
 	side := toGlobalSide(trade.Side)
 	return types.Trade{
-		ID:            uint64(billID),
+		ID:            uint64(trade.TradeId),
 		OrderID:       uint64(trade.OrderId),
 		Exchange:      types.ExchangeOKEx,
 		Price:         trade.FillPrice,

pkg/exchange/okex/convert_test.go

@@ -115,7 +115,7 @@ func Test_tradeToGlobal(t *testing.T) {
 
 	t.Run("succeeds with sell/taker", func(t *testing.T) {
 		assert.Equal(tradeToGlobal(res), types.Trade{
-			ID:            uint64(665951654138736652),
+			ID:            uint64(724072849),
 			OrderID:       uint64(665951654130348158),
 			Exchange:      types.ExchangeOKEx,
 			Price:         fixedpoint.NewFromFloat(46446.4),
@@ -135,7 +135,7 @@ func Test_tradeToGlobal(t *testing.T) {
 		newRes := res
 		newRes.Side = okexapi.SideTypeBuy
 		assert.Equal(tradeToGlobal(newRes), types.Trade{
-			ID:            uint64(665951654138736652),
+			ID:            uint64(724072849),
 			OrderID:       uint64(665951654130348158),
 			Exchange:      types.ExchangeOKEx,
 			Price:         fixedpoint.NewFromFloat(46446.4),
@@ -155,7 +155,7 @@ func Test_tradeToGlobal(t *testing.T) {
 		newRes := res
 		newRes.ExecutionType = okexapi.LiquidityTypeMaker
 		assert.Equal(tradeToGlobal(newRes), types.Trade{
-			ID:            uint64(665951654138736652),
+			ID:            uint64(724072849),
 			OrderID:       uint64(665951654130348158),
 			Exchange:      types.ExchangeOKEx,
 			Price:         fixedpoint.NewFromFloat(46446.4),
@@ -176,7 +176,7 @@ func Test_tradeToGlobal(t *testing.T) {
 		newRes.Side = okexapi.SideTypeBuy
 		newRes.ExecutionType = okexapi.LiquidityTypeMaker
 		assert.Equal(tradeToGlobal(newRes), types.Trade{
-			ID:            uint64(665951654138736652),
+			ID:            uint64(724072849),
 			OrderID:       uint64(665951654130348158),
 			Exchange:      types.ExchangeOKEx,
 			Price:         fixedpoint.NewFromFloat(46446.4),

pkg/exchange/okex/exchange.go

@@ -535,6 +535,7 @@ REMARK: If your start time is 90 days earlier, we will update it to now - 90 day
 ** StartTime, EndTime, FromTradeId can be used together. **
 
 If you want to query all trades within a large time range (e.g. total orders > 100), we recommend using batch.TradeBatchQuery.
+We don't support the last trade id as a filter because okx supports bill ID only.
 */
 func (e *Exchange) QueryTrades(ctx context.Context, symbol string, options *types.TradeQueryOptions) (trades []types.Trade, err error) {
 	if symbol == "" {
@@ -544,11 +545,11 @@ func (e *Exchange) QueryTrades(ctx context.Context, symbol string, options *type
 	req := e.client.NewGetTransactionHistoryRequest().InstrumentID(toLocalSymbol(symbol))
 
 	limit := options.Limit
-	req.Limit(uint64(limit))
 	if limit > defaultQueryLimit || limit <= 0 {
 		log.Infof("limit is exceeded default limit %d or zero, got: %d, use default limit", defaultQueryLimit, limit)
-		req.Limit(defaultQueryLimit)
+		limit = defaultQueryLimit
 	}
+	req.Limit(uint64(limit))
 
 	var newStartTime time.Time
 	if options.StartTime != nil {
@@ -569,19 +570,38 @@ func (e *Exchange) QueryTrades(ctx context.Context, symbol string, options *type
 		}
 		req.EndTime(options.EndTime.UTC())
 	}
-	req.Before(strconv.FormatUint(options.LastTradeID, 10))
 
-	if err := queryTradeLimiter.Wait(ctx); err != nil {
+	if options.LastTradeID != 0 {
-		return nil, fmt.Errorf("query trades rate limiter wait error: %w", err)
+		// we don't support the last trade id as a filter because okx supports bill ID only.
+		// we don't have any more fields (types.Trade) to store it.
+		log.Infof("Last trade id not supported on QueryTrades")
 	}
 
-	response, err := req.Do(ctx)
+	for {
-	if err != nil {
+		if err := queryTradeLimiter.Wait(ctx); err != nil {
-		return nil, fmt.Errorf("failed to query trades, err: %w", err)
+			return nil, fmt.Errorf("query trades rate limiter wait error: %w", err)
-	}
+		}
 
-	for _, trade := range response {
+		response, err := req.Do(ctx)
-		trades = append(trades, tradeToGlobal(trade))
+		if err != nil {
+			return nil, fmt.Errorf("failed to query trades, err: %w", err)
+		}
+
+		for _, trade := range response {
+			trades = append(trades, tradeToGlobal(trade))
+		}
+
+		tradeLen := int64(len(response))
+		// a defensive programming to ensure the length of order response is expected.
+		if tradeLen > limit {
+			return nil, fmt.Errorf("unexpected trade length %d", tradeLen)
+		}
+
+		if tradeLen < limit {
+			break
+		}
+		// use Before filter to get all data.
+		req.Before(response[tradeLen-1].BillId.String())
 	}
 
 	return trades, nil