Client: Discovery Improvements

[holgerd77]

trafficstars

When working on the Shandong testnet in #2316 we realized that discovery of the client still needs improvements respectively it would be desired to have this more robust and some communication flows checked and improved.

Here is a list of potential TODOs written down along the analysis:

1. First round bootnode grap-up

As far as I have observed EthereumJS first-round connection behavior, I think we do not get connections from all but rather 1 bootnode in the first round (so: the parallel connection attempts to the bootnodes listed for a network).

I guess this needs some deeper investigation and somewhat grounded connection analysis to get this improved (I once tried on these kind of things but realized that this is really hard to debug, maybe we can even use some modified version of this testnet setup for this kind of stuff (separate chain ID)? :thinking:).

2. FindNeighbours behavioral analysis and adoption

In one discussion - I guess with Martin from Geth - it turned out that our findNeighbours behavior doesn't match the one from Geth and we should adopt. So basically I was puzzled that I saw Geth bootnodes where we didn't get a connection to all of them and wondered how we then could ever find enough peers with findNeighbours, since I thought this would be coupled respectively consecutively run. Turned out that it seems not to be and we should look at the order of calls there respectively what we currently do there and compare what we should do (sorry, can't describe 100% to the point, needs additional validation).

3. DiscV4/DNS Discovery

If I see this right, I think we actually do not have any discovery means activated for custom testnets by default :face_with_raised_eyebrow:, seeing in the config file: https://github.com/ethereumjs/ethereumjs-monorepo/blob/master/packages/client/lib/config.ts

DiscV4 discovery is deactivated by default except for mainnet (with the reasoning that it is basically a needle-in-a-haystack search for everything except mainnet), see https://github.com/ethereumjs/ethereumjs-monorepo/blob/master/packages/client/lib/config.ts#L206. And DNS discovery is only activated for testnets where we know that there are DNS discovery networks being setup (we have this for 'ropsten', 'rinkeby', 'goerli', 'sepolia' activated, see https://github.com/ethereumjs/ethereumjs-monorepo/blob/master/packages/client/lib/config.ts#L444.

Writing this down, I totally wonder how other clients are solving this discovery part for small custom testnets given the DiscV4 assumptions are true (basically useless). Are they doing discovery at all or just skip that and only rely on the bootnodes? :thinking: Maybe @parithosh#0530 can shine some light on this?

Another (side) question: how difficult is it actually to set up DNS discovery, so the serving part? So would this be an option to theoretically do this? I have absolutely no feeling for that.

---

All three TODOs likely need substantial deep dive analysis and partly somewhat extensive prepartory multi-client testnet setup (or at least: e.g. some local EthereumJS <-> Geth communication analysis) to get to somewhat substantial, robust and lasting improvements.

(so plan a day or two for each TODO)

[g11tech]

I think the minimal discovery functionality should be:

If you are connected to one peer, you can discover all peers of it and (then connect to them and discover more), which is basically discv5's findNode protocol. What are your thoughts regarding that @holgerd77

[parithosh]

Afaik, on testnets we provide the bootnode and the nodes use the bootnode to discover other peers. We do not use DNS discovery afaik, I'm not sure how easy/hard it is to setup either - we haven't had any peering issues that required it in the past.

[g11tech]

correct, thats the discv5 functionality CL use, wondering if geth also uses same/similar

[acolytec3]

correct, thats the discv5 functionality CL use, wondering if geth also uses same/similar

Do we want to add discv5 discovery to our EL client? Wouldn't be a far reach from what ultralight already has.