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

Sending DiscV5 WHOAREYOU challenge only once #30581

Open
GrapeBaBa opened this issue Oct 12, 2024 · 5 comments
Open

Sending DiscV5 WHOAREYOU challenge only once #30581

GrapeBaBa opened this issue Oct 12, 2024 · 5 comments

Comments

@GrapeBaBa
Copy link

Other DiscV5 implementation sending only once

Nim: https://github.com/status-im/nim-eth/blob/470baf82bdbf05dd399881123ae9020b8f117871/eth/p2p/discoveryv5/protocol.nim#L404

Rust: https://github.com/sigp/discv5/blob/master/src/handler/mod.rs#L1047

This may cause Invalid nonce error when communicating with other clients from go-ethereum.

We have a change in shisui PR, does it make sense to merge
into go-ethereum?

@karalabe
Copy link
Member

CC @fjl

@fjl
Copy link
Contributor

fjl commented Oct 16, 2024

I think the issue fixed by the PR may be a valid one, but the fix you linked is confusing.
It would be better to perform the check for duplicate WHOAREYOU in the place where it is sent,
instead of dropping it in Encode.

@GrapeBaBa
Copy link
Author

@fjl I only see this function getHandshake in SessionCache could judge duplicate WHOAREYOU, how can I make this code out of Encode?

@fjl
Copy link
Contributor

fjl commented Oct 18, 2024

Hmm. We have an explicit test that verifies the current Geth behavior, the node is supposed to respond to every handshake attempt with a new WHOAREYOU challenge.

This is also explicitly mentioned by the spec, in the Handshake Implementation Considerations section (sentence beginning with "Another important issue is the processing...")

I'm curious if this happens often. Possibly we could change the spec.

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

No branches or pull requests

4 participants
@fjl @karalabe @GrapeBaBa and others