Skip to content

Conversation

dboyliao
Copy link
Collaborator

@dboyliao dboyliao requested review from c9s and gx578007 April 28, 2025 06:19
@dboyliao dboyliao force-pushed the dboy/binance-remove-listenKey branch from 37509fd to 37b7f95 Compare April 28, 2025 07:57
@dboyliao dboyliao force-pushed the dboy/binance-remove-listenKey branch 2 times, most recently from 4b6946e to b2ab6a2 Compare April 28, 2025 08:46
@dboyliao dboyliao changed the title FIX: [binance] Remove listen key handling (deprecated) from Stream. Use WebSocket auth request instead. FIX: [binance] Ed25519 private key support for Stream. Apr 28, 2025
@dboyliao dboyliao force-pushed the dboy/binance-remove-listenKey branch 5 times, most recently from 30ed652 to 3329be6 Compare April 28, 2025 09:16
@dboyliao dboyliao force-pushed the dboy/binance-remove-listenKey branch 2 times, most recently from 849fca6 to 65b2398 Compare April 28, 2025 14:07
@dboyliao dboyliao requested a review from c9s April 28, 2025 14:07
@dboyliao dboyliao force-pushed the dboy/binance-remove-listenKey branch from 65b2398 to 858ec93 Compare April 28, 2025 14:54
@c9s c9s requested review from bailantaotao and zenixls2 May 6, 2025 09:31
if c.IsUsingEd25519Auth() {
// Ed25519 authentication
if len(c.PrivateKey) == 0 {
return nil, errEmptyPrivateKey
Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

Copy link
Owner

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"
Copy link
Collaborator Author

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?

Copy link
Owner

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<

usingEd25519 bool
}

func (e *ed25519authentication) getEd25519PrivateKey() ed25519.PrivateKey {
Copy link
Collaborator Author

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.

Copy link
Owner

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

Copy link
Owner

Choose a reason for hiding this comment

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

removed

return e.privateKey
}

func (e *ed25519authentication) isUsingEd25519Auth() bool {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ditto.

Copy link
Owner

Choose a reason for hiding this comment

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

removed

@c9s c9s force-pushed the dboy/binance-remove-listenKey branch from 38e987d to c0df746 Compare May 6, 2025 15:07
@c9s c9s merged commit a89cbf6 into main May 7, 2025
3 checks passed
@c9s c9s deleted the dboy/binance-remove-listenKey branch May 7, 2025 08:02
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