-
-
Notifications
You must be signed in to change notification settings - Fork 300
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.
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
FEATURE: add QueryClosedOrders() and QueryTrades() for okex #1312
Changes from 5 commits
99a69f4
ad72062
3b63858
6fd86fe
648b82e
cc55d67
b1c6e01
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,6 @@ import ( | |
// Market data limiter means public api, this includes QueryMarkets, QueryTicker, QueryTickers, QueryKLines | ||
var ( | ||
marketDataLimiter = rate.NewLimiter(rate.Every(100*time.Millisecond), 5) | ||
tradeRateLimiter = rate.NewLimiter(rate.Every(100*time.Millisecond), 5) | ||
orderRateLimiter = rate.NewLimiter(rate.Every(300*time.Millisecond), 5) | ||
) | ||
|
||
|
@@ -32,6 +31,11 @@ const ID = "okex" | |
// PlatformToken is the platform currency of OKEx, pre-allocate static string here | ||
const PlatformToken = "OKB" | ||
|
||
const ( | ||
// Constant For query limit | ||
defaultQueryLimit = 100 | ||
) | ||
|
||
var log = logrus.WithFields(logrus.Fields{ | ||
"exchange": ID, | ||
}) | ||
|
@@ -360,7 +364,7 @@ func (e *Exchange) QueryOrder(ctx context.Context, q types.OrderQuery) (*types.O | |
return nil, errors.New("okex.QueryOrder: OrderId or ClientOrderId is required parameter") | ||
} | ||
req := e.client.NewGetOrderDetailsRequest() | ||
req.InstrumentID(q.Symbol). | ||
req.InstrumentID(toLocalSymbol(q.Symbol)). | ||
OrderID(q.OrderID). | ||
ClientOrderID(q.ClientOrderID) | ||
|
||
|
@@ -380,9 +384,9 @@ func (e *Exchange) QueryOrderTrades(ctx context.Context, q types.OrderQuery) ([] | |
log.Warn("!!!OKEX EXCHANGE API NOTICE!!! Okex does not support searching for trades using OrderClientId.") | ||
} | ||
|
||
req := e.client.NewGetTransactionHistoriesRequest() | ||
req := e.client.NewGetTransactionHistoryRequest() | ||
if len(q.Symbol) != 0 { | ||
req.InstrumentID(q.Symbol) | ||
req.InstrumentID(toLocalSymbol(q.Symbol)) | ||
} | ||
|
||
if len(q.OrderID) != 0 { | ||
|
@@ -411,6 +415,126 @@ func (e *Exchange) QueryOrderTrades(ctx context.Context, q types.OrderQuery) ([] | |
if errs != nil { | ||
return nil, errs | ||
} | ||
return trades, nil | ||
} | ||
|
||
/* | ||
QueryClosedOrders can query closed orders in last 3 months, there are no time interval limitations, as long as until >= since. | ||
Please Use lastOrderID as cursor, only return orders later than that order, that order is not included. | ||
If you want to query orders by time range, please just pass since and until. | ||
If you want to query by cursor, please pass lastOrderID. | ||
Because it gets the correct response even when you pass all parameters with the right time interval and invalid lastOrderID, like 0. | ||
Time interval boundary unit is second. | ||
since is inclusive, ex. order created in 1694155903, get response if query since 1694155903, get empty if query since 1694155904 | ||
until is not inclusive, ex. order created in 1694155903, get response if query until 1694155904, get empty if query until 1694155903 | ||
*/ | ||
func (e *Exchange) QueryClosedOrders(ctx context.Context, symbol string, since, until time.Time, lastOrderID uint64) ([]types.Order, error) { | ||
bailantaotao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if symbol == "" { | ||
return nil, ErrSymbolRequired | ||
} | ||
|
||
if err := orderRateLimiter.Wait(ctx); err != nil { | ||
return nil, fmt.Errorf("query closed order rate limiter wait error: %w", err) | ||
} | ||
|
||
var lastOrder string | ||
if lastOrderID <= 0 { | ||
lastOrder = "" | ||
} else { | ||
lastOrder = strconv.FormatUint(lastOrderID, 10) | ||
} | ||
|
||
res, err := e.client.NewGetOrderHistoryRequest(). | ||
InstrumentID(toLocalSymbol(symbol)). | ||
StartTime(since). | ||
EndTime(until). | ||
Limit(defaultQueryLimit). | ||
Before(lastOrder). | ||
c9s marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Do(ctx) | ||
|
||
if err != nil { | ||
return nil, fmt.Errorf("failed to call get order histories error: %w", err) | ||
} | ||
|
||
var orders []types.Order | ||
|
||
for _, order := range res { | ||
o, err2 := toGlobalOrder(&order) | ||
if err2 != nil { | ||
err = multierr.Append(err, err2) | ||
continue | ||
} | ||
|
||
orders = append(orders, *o) | ||
} | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return types.SortOrdersAscending(orders), nil | ||
} | ||
|
||
/* | ||
QueryTrades can query trades in last 3 months, there are no time interval limitations, as long as end_time >= start_time. | ||
OKEX do not provide api to query by tradeID, So use /api/v5/trade/orders-history-archive as its official site do. | ||
bailantaotao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
If you want to query trades by time range, please just pass start_time and end_time. | ||
Because it gets the correct response even when you pass all parameters with the right time interval and invalid LastTradeID, like 0. | ||
*/ | ||
func (e *Exchange) QueryTrades(ctx context.Context, symbol string, options *types.TradeQueryOptions) ([]types.Trade, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps we need to handle query trade with pagination, if you query result greater that 100, the other result will missing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Already handle, like below
|
||
if symbol == "" { | ||
return nil, ErrSymbolRequired | ||
} | ||
|
||
if options.LastTradeID > 0 { | ||
log.Warn("!!!OKEX EXCHANGE API NOTICE!!! Okex does not support searching for trades using TradeId.") | ||
} | ||
|
||
req := e.client.NewGetTransactionHistoryRequest().InstrumentID(toLocalSymbol(symbol)) | ||
|
||
limit := uint64(options.Limit) | ||
if limit > defaultQueryLimit || limit <= 0 { | ||
limit = defaultQueryLimit | ||
log.Debugf("limit is exceeded default limit %d or zero, got: %d, Do not pass limit", defaultQueryLimit, options.Limit) | ||
bailantaotao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} else { | ||
req.Limit(limit) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Always pass the limit when making a request. If the server updates the default query limit, your pagination may not work. How about:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. That make sense ! |
||
|
||
if err := orderRateLimiter.Wait(ctx); err != nil { | ||
return nil, fmt.Errorf("query trades rate limiter wait error: %w", err) | ||
} | ||
|
||
var err error | ||
var response []okexapi.OrderDetails | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand, you're calling QueryOrderHistory in the QueryTrades api? is this expected? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes ! because OKX didn't provide any api query by trade_id, but QueryOrderHistory has trade_id in its response. So I call QueryOrderHistory to get the same effect. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OKX didn't provide any api query like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 你可以測試pagination是否可以動嗎? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you are right, 加了unit test 後發現應該要用 |
||
if options.StartTime == nil && options.EndTime == nil { | ||
return nil, fmt.Errorf("StartTime and EndTime are require parameter!") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. required There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated |
||
} else { // query by time interval | ||
if options.StartTime != nil { | ||
req.StartTime(*options.StartTime) | ||
} | ||
if options.EndTime != nil { | ||
req.EndTime(*options.EndTime) | ||
} | ||
|
||
var billID = "" // billId should be emtpy, can't be 0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A few questions:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All is descending order. |
||
for { // pagenation should use "after" (earlier than) | ||
res, err := req. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle rate limit There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I handle it in line 503 - 505 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rate limit的 token應該是每執行1次request就要消耗1個,但你的做法會變成他消耗1個後,有可能發出大於1的requests 這可以下個PR再改 |
||
After(billID). | ||
Do(ctx) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to call get order histories error: %w", err) | ||
} | ||
response = append(response, res...) | ||
if len(res) == int(limit) { | ||
billID = res[limit-1].BillID | ||
} else { | ||
break | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It can be simpler:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated |
||
} | ||
} | ||
|
||
trades, err := toGlobalTrades(response) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to trans order detail to trades error: %w", err) | ||
} | ||
return trades, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package okexapi | ||
|
||
import ( | ||
"time" | ||
|
||
"github.com/c9s/requestgen" | ||
) | ||
|
||
//go:generate GetRequest -url "/api/v5/trade/orders-history-archive" -type GetOrderHistoryRequest -responseDataType .APIResponse | ||
type GetOrderHistoryRequest struct { | ||
client requestgen.AuthenticatedAPIClient | ||
|
||
instrumentType InstrumentType `param:"instType,query"` | ||
instrumentID *string `param:"instId,query"` | ||
orderType *OrderType `param:"ordType,query"` | ||
// underlying and instrumentFamil Applicable to FUTURES/SWAP/OPTION | ||
underlying *string `param:"uly,query"` | ||
instrumentFamily *string `param:"instFamily,query"` | ||
|
||
state *OrderState `param:"state,query"` | ||
after *string `param:"after,query"` | ||
before *string `param:"before,query"` | ||
startTime *time.Time `param:"begin,query,milliseconds"` | ||
|
||
// endTime for each request, startTime and endTime can be any interval, but should be in last 3 months | ||
endTime *time.Time `param:"end,query,milliseconds"` | ||
|
||
// limit for data size per page. Default: 100 | ||
limit *uint64 `param:"limit,query"` | ||
} | ||
|
||
type OrderList []OrderDetails | ||
|
||
// NewGetOrderHistoriesRequest is descending order by createdTime | ||
func (c *RestClient) NewGetOrderHistoryRequest() *GetOrderHistoryRequest { | ||
return &GetOrderHistoryRequest{ | ||
client: c, | ||
instrumentType: InstrumentTypeSpot, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the bill id be found at okx console?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
console? you mean web page? No, You can't find billId in official site, actually you can't find orderId and tradeId either.