Client Discovery Improvements

[holgerd77]

This PR improves DiscV4 discovery in various ways.

First it adds an explicit logging message for discovery on startup. This is helpful since it remains often somewhat unclear if and what form of discovery is running.

Second it expands the --bootnodes option for the client to also accept a bootnode.txt file (e.g. from EthPandaOps devnet-10 config.

Third it introduces a new confirmed-peers mechanism for devp2p DPT. This allows to set a higher quality bar for sendNeighbour requests by only send to peers which have been manually confirmed. Otherwise discovery is hammering out requests like crazy to - mainly - mainnet peers not finding any needle in this haystack which rendered the feature more or less useless for all networks except mainnet (in fact we had deactivated in client for everything except mainnet).

Now only peers who are on the correct network and where a network connection was/is possible get the findNeighbour requests. This makes discV4 discovery requests acceptable again.

I was able to re-activate peer discovery by default for small networks - here devnet-10 - and have a slowly growing peer base (here the yellow numbers)! 🎉

Still need to do some testing - also write some tests - but I am confident that this approach works adequately and we can likely merge in.

[codecov]

Codecov Report

Merging #3120 (0196597) into master (ffd9ede) will decrease coverage by 10.70%.
The diff coverage is n/a.

❗ Current head 0196597 differs from pull request most recent head 69f7351. Consider uploading reports for the commit 69f7351 to get more accurate results

Additional details and impacted files

Flag	Coverage Δ
block	?
blockchain	?
client	?
common	?
ethash	?
evm	71.87% <ø> (ø)
statemanager	?
trie	?
tx	?
util	87.57% <ø> (ø)
vm	75.98% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[holgerd77]

Ok, I did some additional improvements and tested under different network conditions (small devnet, testnet, mainnet), following results:

• testnets (like Holesky): previously discV4 deactivated (too much noise), now with onlyConfirmed: -> helps finding peers (tested with DNS deactivated)
• small testnets (like devnet-10): previously no discovery, now with onlyConfirmed: most useful case, helps finding new peers at all
• mainnet: reduces peer discovery speed (since most peers are mainnet anyhow): -> have deactivated

I have also added a descent new test suite with mockups, see dpt.spec.ts.

Open for review. 🙂

[jochem-brouwer]

One comment, and am still tracking down where the filter for the network is 🤔 (see discord, have to track down a long event queue and likely take a wrong path somewhere there)

[jochem-brouwer]

Peers are never removed from _confirmedPeers, so at some point we cannot confirm new peers since the set is full? Should the peer id be removed on removePeer?

[holgerd77]

Yeah, that was the lazy version assuming a very slow growth. But removing on removePeer definitely make sense, will add. 👍

[jochem-brouwer]

Also, the CI fails on the new tests 🤔

[acolytec3]

LGTM