Implement p2p discovery v5

[gsalgado]

The discovery v4 protocol is really bad at finding LES peers, so we had to resort to hard-coding known LES peers in the PeerPool we use with our LightChain. The latest release of geth (1.8.x) implements the discovery v5 protocol, though, which is much better at finding peers that support a specific sub-protocol (e.g. LES), so we should implement that (probably even dropping v4) and change our PeerPool to use p2p.DiscoveryProtocol instead of hard-coded nodes.

[gsalgado]

FWIW, I've started experimenting with it in https://github.com/gsalgado/py-evm/tree/discv5 a while ago

[gsalgado]

I'm planning to resume work on this, and am trying to think of any reasons why we might want to have the option to run a node that supports only the discovery v4 proto. Given that v5 is (AIUI) a backwards-compatible extension of v4 (meaning a v5 node can still talk to nodes that only support v4), the only reason I can think of is that the new code written to support v5 may be more fragile/buggy than the existing v4 code, but then all of trinity is still in early alpha and definitely not stable/reliable, specially the current discv4 implementation, which I wrote in my early ethereum/asyncio days. I think it'd make more sense for us to simply extend our current implementation to match the v5 spec, instead of maintaining two separate implementations. @pipermerriam, @carver, any thoughts on this?

[pipermerriam]

I'm fine with v5 only approach, we can always dig the v4 one back out of history if things don't go as planned.

[carver]

Dropping v4-only sounds good to me :+1:

[gsalgado]

They actually changed things and now support both v4 and v5 simultaneously, with a prefix to distinguish v5 messages. Also, v5 is not enabled by default, so if we would support only v5 (as I'd originally suggested), we wouldn't be able to talk to most of the nodes.

[gsalgado]

We now have a basic implementation of discv5 that can be enabled via a command line flag, but since they plan to rework geth's implementation soon, it doesn't make sense to invest more effort on this until then

[fjl]

Hi guys, I've created a tracking issue for the v5 spec over in the devp2p repo. We can discuss the protocol there :).

[EBGToo]

I should expect current versions of Geth to continue to support v4? Between v4 and v5 the UDP packet header has changed, incompatibly. Currently, when connecting with the v4 UDP header (as packet-header = hash || signature || packet-type) I end up being thrown a errBadPrefix from the v5 code:

        /// v5
    prefix, sig, sigdata := buf[:versionPrefixSize], buf[versionPrefixSize:headSize], buf[headSize:]
    if !bytes.Equal(prefix, versionPrefix) {
        return errBadPrefix
    }

(because versionPrefix is not a hash...). I've not enabled v5 discovery on my Geth node. How is this expected to work?

[carver]

Hi guys, I've created a tracking issue for the v5 spec over in the devp2p repo. We can discuss the protocol there :).

Not sure what the original tracking issue was, but here is the milestone tracking all the v5 things: https://github.com/ethereum/devp2p/milestone/2

[gsalgado]

So, this unfinished v5 implementation was based on an old draft of the spec and now we have a new implementation based on the current spec draft, therefore I'm closing this