-
-
Notifications
You must be signed in to change notification settings - Fork 345
FIX: [binance] apply new ws endpoint and use ed25519 authentication for spot trading #2009
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
37509fd
to
37b7f95
Compare
4b6946e
to
b2ab6a2
Compare
30ed652
to
3329be6
Compare
849fca6
to
65b2398
Compare
65b2398
to
858ec93
Compare
if c.IsUsingEd25519Auth() { | ||
// Ed25519 authentication | ||
if len(c.PrivateKey) == 0 { | ||
return nil, errEmptyPrivateKey |
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.
It should be impossible to happen while you have passed IsUsingEd25519Auth
.
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.
But I think it is a good practice to apply defensive programming this way.
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.
I just updated the validations
const FuturesWebSocketURL = "wss://fstream.binance.com" | ||
const FuturesWebSocketTestURL = "wss://stream.binancefuture.com" | ||
const TestNetFuturesWebSocketURL = "wss://stream.binancefuture.com" |
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.
This one does not look like testnet url.
Is it on purpose or a typo?
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.
It's just a rename from the above line<
pkg/exchange/binance/exchange.go
Outdated
usingEd25519 bool | ||
} | ||
|
||
func (e *ed25519authentication) getEd25519PrivateKey() ed25519.PrivateKey { |
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.
Looks like we are not using this method.
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.
yeah we can remove it
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.
removed
pkg/exchange/binance/exchange.go
Outdated
return e.privateKey | ||
} | ||
|
||
func (e *ed25519authentication) isUsingEd25519Auth() bool { |
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.
ditto.
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.
removed
…am. Backward compatible to HMAC authentication
…n RESTful API requests
- simplify exchange factory keys - add OptionKeyAPIPrivateKey - improve binance private key loading
38e987d
to
c0df746
Compare
listenKey
is deprecated:https://developers.binance.com/docs/binance-spot-api-docs/CHANGELOG#user-data-streams