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.

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: [okx] query recent trades #1583

Merged
merged 2 commits into from
Mar 15, 2024
Merged

Conversation

bailantaotao
Copy link
Collaborator

The historical trade API has a high rate limiter cost (10/2s),

so I have introduced a new recent trade API to address this. If the start time is less than or equal to 3 days, we will utilize the recent trade API.

@bbgokarma-bot
Copy link

Welcome back! @bailantaotao, This pull request may get 1097 BBG.

@bailantaotao bailantaotao requested a review from c9s March 14, 2024 09:20
@bbgokarma-bot
Copy link

Re-estimated karma: this pull request may get 1102 BBG

@@ -55,6 +55,7 @@ const (
defaultQueryLimit = 100

maxHistoricalDataQueryPeriod = 90 * 24 * time.Hour
threeDaysHistoricalPeriod = 3 * 24 * time.Hour
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use maxTradeHistoryPeriod ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have one that supports 90 days, called maxHistoricalDataQueryPeriod.

And now we have another one that's only 3 days, so it's called threeDaysHistoricalPeriod XD.


var newStartTime time.Time
timeNow := e.timeNowFunc()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be just now ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, by using that timeNowFunc(), we can test the function.

@@ -53,13 +53,13 @@ type Trade struct {
PosSide string `json:"posSide"`
}

//go:generate GetRequest -url "/api/v5/trade/fills-history" -type GetTransactionHistoryRequest -responseDataType []Trade
//go:generate GetRequest -url "/api/v5/trade/fills-history" -type GetTransactionHistoryRequest -responseDataType []Trade -rateLimiter 1+10/2s
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sooo cool!!!

Copy link

codecov bot commented Mar 14, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 22.53%. Comparing base (747b75f) to head (2ae1933).
Report is 7 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1583      +/-   ##
==========================================
+ Coverage   22.44%   22.53%   +0.08%     
==========================================
  Files         615      617       +2     
  Lines       44463    44480      +17     
==========================================
+ Hits         9981    10023      +42     
+ Misses      33770    33742      -28     
- Partials      712      715       +3     
Files Coverage Δ
pkg/exchange/okex/exchange.go 15.49% <100.00%> (+15.49%) ⬆️
...ge/okex/okexapi/get_transaction_history_request.go 0.00% <ø> (ø)
...xapi/get_three_days_transaction_history_request.go 0.00% <0.00%> (ø)

... and 7 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 747b75f...2ae1933. Read the comment docs.

@bailantaotao bailantaotao merged commit 3300b71 into main Mar 15, 2024
5 checks passed
@bailantaotao bailantaotao deleted the edwin/okx/query-recent-trades branch March 15, 2024 01:43
@bbgokarma-bot
Copy link

Hi @bailantaotao,

Well done! 1162 BBG has been sent to your polygon wallet. Please check the following tx:

https://polygonscan.com/tx/0x114ebd9e418077538489a6ad84d0c14b8d9a98a4a474707e59ace94e58ee99d2

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants