search

ethereum / go-ethereum

Go implementation of the Ethereum protocol
https://geth.ethereum.org
GNU Lesser General Public License v3.0
47.43k stars 20.07k forks source link

Sending DiscV5 WHOAREYOU challenge only once #30581

Open GrapeBaBa opened 1 week ago

GrapeBaBa commented 1 week ago

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 commented 1 week ago

CC @fjl

fjl commented 6 days ago

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 commented 5 days ago

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

fjl commented 4 days ago

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.