hyperledger / besu

An enterprise-grade Java-based, Apache 2.0 licensed Ethereum client https://wiki.hyperledger.org/display/besu
https://www.hyperledger.org/projects/besu
Apache License 2.0
1.44k stars 765 forks source link

feat: Node discovery via DNS #7129

Closed usmansaleem closed 1 month ago

usmansaleem commented 2 months ago

PR description

Fold Tuweni dns discovery module into Besu p2p discovery module. The Tuweni dns-discovery was Kotlin code using kotlin co-routines. It has been translated (and adapted) in Java. Utilized vertx worker verticle to launch dns daemon in a separate thread outside event loop. Rest of the method protocol is exactly same at the moment.

The screenshot shows mainnet peer connection for four nodes which were configured with SNAP and CHECKPOINT.

Screenshot 2024-06-06 at 10 08 47 AM

Fixed Issue(s)

Thanks for sending a pull request! Have you done the following?

Locally, you can run these tests to catch failures early:

shemnon commented 2 months ago

I'm curious as to why this was ported to a separate Consensys lib and not folded directly into Besu. Are there other projects that use this library? Is it because it's written on Kotlin? It's only 5 production files and 2 test files if wiring in a kotlin compiler step isn't desired (surely less effort than maintaining an ongoing deployment). I't also makes it more difficult to alter the classes if features such as streaming the nodes instead of querying them all in one go is desired.

usmansaleem commented 2 months ago

@shemnon Hi Danno, the initial thought was just to port it as it is to make further changes and include it as a library, however, you've raised valid points, I concur that folding these classes (the dns-discovery) into besu should not be too difficult. I am not aware of any other projects using the Tuweni dns-discovery module.

@macfarla any further thoughts?

macfarla commented 2 months ago

@shemnon Hi Danno, the initial thought was just to port it as it is to make further changes and include it as a library, however, you've raised valid points, I concur that folding these classes (the dns-discovery) into besu should not be too difficult. I am not aware of any other projects using the Tuweni dns-discovery module.

@macfarla any further thoughts?

yeah fair point @shemnon if it's easier long term I'm not opposed to folding it into Besu. Initial thinking was we could still revert to tuweni libs again if they got updated but that also may not eventuate.

shemnon commented 1 month ago

As a bit more context, I'm looking at folding the tuweni-bytes libraries back into besu (undoing #215 in effect). My main reason is a tighter optmiization loop when speeding up the EVM as well as being able to purpose-build some of the bytes implementation for speed. Coordinating these changes across repos creates significant friction.