Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: [okx] fix trade id #1530

Merged
merged 1 commit into from
Feb 15, 2024
Merged

FIX: [okx] fix trade id #1530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions pkg/exchange/okex/convert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions pkg/exchange/okex/convert_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand Down
42 changes: 31 additions & 11 deletions pkg/exchange/okex/exchange.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "" {
Expand All @@ -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 {
Expand All @@ -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 {
return nil, fmt.Errorf("query trades rate limiter wait error: %w", err)
if options.LastTradeID != 0 {
// 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)
if err != nil {
return nil, fmt.Errorf("failed to query trades, err: %w", err)
}
for {
if err := queryTradeLimiter.Wait(ctx); err != nil {
return nil, fmt.Errorf("query trades rate limiter wait error: %w", err)
}

for _, trade := range response {
trades = append(trades, tradeToGlobal(trade))
response, err := req.Do(ctx)
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
Expand Down
Loading