besu
besu copied to clipboard
Updates to DNS discovery
Signed-off-by: Diego López León [email protected]
PR description
This PR adds a new implementation for EIP-14159 replacing DNSDaemon from Tuweni with a new one, vert.x based. This new implementation provides the following benefits:
- Random tree traversal
- Batched discovery
- Periodic traversal continues from last state
- Non blocking implementation, based on vert.x DNS client
Fixed Issue(s)
Fixes #1707
Notes
This is sent as a draft to gather feedback on this implementation. How often should we check the DNS for new peers? How many leaves should be attempted to connect per branch? Should this parameters need to be parameterized through the CLI? Based on the willingness to merge this proposal I would finish the specification by adding support to linked trees
Documentation
- [x] I thought about documentation and added the
doc-change-required
label to this PR if updates are required.
Changelog
- [x] I thought about the changelog and included a changelog update if required.
@diega is something wrong with the DNSDaemon from Tuweni? I didn't see feedback or anything on Tuweni, any issues worth reporting?
Can we engage on this @diega ? Have you done any work to test the implementation and outcomes?
Thanks @atoulme for taking the time of looking at it.
There is nothing wrong with Tuweni's implementation, but it currently doesn't have any dependency on vert.x and I wanted to take the chance of this requirement to make use of vert.x DNS client to better integrate its execution within the existing event loop (instead of using a java.util.Timer
) . Having that said, I coded it in a pretty abstract way so if you think this would be suitable to be part of Tuweni I would be happy to port it to Kotlin and send it as a PR.
@matt-nelson-csi absolutely.
Beside the unit testing part of this PR, I ran it using --bootnodes= --discovery-dns-url=enrtree://AKA3AM6LPBYEUDMVNU3BSVQJ5AD45Y7YPOHJLEF6W26QOE4VTUDPE@all.mainnet.ethdisco.net
, which allowed me to tweak some constants like how many records per round try to connect (I chose 100, but we can change that based on some other experience). It would be difficult to establish a comparison between both implementations regarding peers. Eventually both would attempt to connect to the same set of boot nodes but this implementation does it random and incrementally. A next step would be to only request new nodes from the DNS tree only when we are bellow certain number of peers instead of doing it every 60 seconds.
@diega sounds good. FWIW a PR to Tuweni would be awesome, but if this PR here and now does the job, no problem. Interesting on the use of Vert.x, that makes sense. I think dnsjava has been less mature in ways, I never did try the DNSClient offered by Vert.x.
Hi @diega any reasons we ended up closing this PR instead of moving forward to make it to main?
@gfukushima I don't really remember, but if you find this useful we can reopen it and try to complete the spec