-
Notifications
You must be signed in to change notification settings - Fork 20.5k
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
p2p, p2p/discover, p2p/discv5: implement UDP port sharing #15200
Conversation
cc296eb
to
b3fba77
Compare
b3fba77
to
313384f
Compare
p2p/discv5/udp.go
Outdated
@@ -376,6 +368,7 @@ func encodePacket(priv *ecdsa.PrivateKey, ptype byte, req interface{}) (p, hash | |||
// add the hash to the front. Note: this doesn't protect the | |||
// packet in any way. | |||
hash = crypto.Keccak256(packet[macSize:]) | |||
hash[0]++ // guarantee incompatibility between v4 and temporary v5 packet formats |
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 makes the format incompatible with v4, but also breaks compatibility with the previous v5 format. The DHT will be split. I'm not sure that's OK. We're about to break the format anyway though, so maybe it is OK after all. I held off merging this PR because of this issue.
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.
Port sharing breaks compatibility anyway, not because of the format but because of the changed port address (the +1/-1 offset was hardcoded). I think if we do not break compatibility with existing v5 format that would cause hell. I want to get rid of the dual port chaos and have a fresh start. Right now the server advertising of the latest release is broken anyway (there was a bug that broke registration when registering multiple topics) so we have nothing to lose. I hope if we release this and people update then we will at least return to pre-LES/2 usability which will be fine until we can release the new and reworked discovery.
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.
OK
…5200) This commit affects p2p/discv5 "topic discovery" by running it on the same UDP port where the old discovery works. This is realized by giving an "unhandled" packet channel to the old v4 discovery packet handler where all invalid packets are sent. These packets are then processed by v5. v5 packets are always invalid when interpreted by v4 and vice versa. This is ensured by adding one to the first byte of the packet hash in v5 packets. DiscoveryV5Bootnodes is also changed to point to new bootnodes that are implementing the changed packet format with modified hash. Existing and new v5 bootnodes are both running on different ports ATM.
…5200) This commit affects p2p/discv5 "topic discovery" by running it on the same UDP port where the old discovery works. This is realized by giving an "unhandled" packet channel to the old v4 discovery packet handler where all invalid packets are sent. These packets are then processed by v5. v5 packets are always invalid when interpreted by v4 and vice versa. This is ensured by adding one to the first byte of the packet hash in v5 packets. DiscoveryV5Bootnodes is also changed to point to new bootnodes that are implementing the changed packet format with modified hash. Existing and new v5 bootnodes are both running on different ports ATM.
This PR breaks v5 DHT connectivity. les clients running this PR won't find servers running an earlier version.
This PR affects p2p/discv5 "topic discovery" by running it on the same UDP port where the old discovery works. This is realized by giving an "unhandled" packet channel to the old v4 discovery packet handler where all invalid packets are sent. These packets are then processed by v5. v5 packets are always invalid when interpreted by v4 and vice versa. This is ensured by adding one to the first byte of the packet hash in v5 packets.
DiscoveryV5Bootnodes is also changed to point to new bootnodes that are implementing the changed packet format with modified hash. Existing and new v5 bootnodes are both running on different ports ATM.