-
Notifications
You must be signed in to change notification settings - Fork 723
support /fapi/v3/account #698
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
Conversation
Signed-off-by: vlw <[email protected]>
d200e4a
to
60b08b5
Compare
@vlw I don't think this is needed anymore since the upgrade was done in place right? |
What do you mean you don't need it? The current /fapi/v2/account is already deprecated and uses 10 API rate limit, the new /fapi/v3/account uses 5 API rate limit (see https://www.binance.com/en/support/announcement/detail/19d4e3cd0758426584dd9686eb56ec64 ). But the V3 response contains less useful data, e.g. no entry price in position information, but isolated wallet information (which was not present in v2). Also V3 request is processed faster. |
As long as binance provides the ability to use API v2 and v3, we should also provide the same to users |
@vlw sorry my bad, I was assuming you were already replacing the v2/account here: https://github.com/ccxt/go-binance/pull/699/files it is indeed the same endpoint but a different topic |
@vlw My question is, do we really need to keep supporting v2? can't we make this replacement in place? |
I think we should leave both versions as we did in positionRisk. In /fapi/v3/balance it was possible to do that, because there the content of the response didn't change at all. In account/v3 the structure of the response has changed, so it should be a separate interface so you don't get unexpected problems in user programs after the library update. |
d200e4a
to
60b08b5
Compare
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.
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
v2/futures/client.go:532
- [nitpick] Consider clarifying this comment to explicitly mention that the service targets the '/fapi/v3/account' endpoint for better context.
// NewGetAccountV3Service init getting account service
v2/futures/account_service.go:129
- [nitpick] Consider updating the comment to indicate that GetAccountV3Service is specifically for retrieving account information from the '/fapi/v3/account' endpoint.
// GetAccountV3Service get account info
No description provided.