-
Notifications
You must be signed in to change notification settings - Fork 878
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Node discovery via DNS #7129
Merged
usmansaleem
merged 42 commits into
hyperledger:main
from
usmansaleem:consensys_dns_discovery
Jun 5, 2024
Merged
Changes from 26 commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
ea7545f
feat: Use besu-dns-discovery
usmansaleem 5bbdeb6
changelog: Update changelog
usmansaleem e75b5de
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem ea62a8a
typo
macfarla 9a780d8
test: Fix unit test mocking
usmansaleem f0684c8
Merge remote-tracking branch 'origin/consensys_dns_discovery' into co…
usmansaleem ce46275
test: spotless fix
usmansaleem 094c976
feat: Add dns discovery classes
usmansaleem a778c20
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem 1704974
test: Add unit tests
usmansaleem 486c61c
feat: Use a separate thread to convert vertx dns-client call to sync
usmansaleem 31a121b
codefix: spotless apply
usmansaleem 45d8faa
fix unit test and refactor periodic timer task
usmansaleem 00c519d
changelog: Update changelog
usmansaleem 2193921
refactor and cleaning up
usmansaleem 355efee
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem 734c786
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem 107bf02
add final keyword
usmansaleem 2ab08bf
changelog: Update changelog
usmansaleem 0ad16c1
codefix: spotless apply
usmansaleem b521d42
revert gradle verification metadata
usmansaleem e54cc02
throw illegalstateexception if dnsdaemon is not started as a vertx ve…
usmansaleem ab81575
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem e2b8339
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem 9e593bc
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem d95011e
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem e77fa1e
codefix: remove unnecessary this prefix
usmansaleem 400f560
codefix: decode public key in constructor
usmansaleem dd2f741
codefix: Remove unnecessary files
usmansaleem 9b2aed9
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem 16507f6
codefix: Fix LOG class reference
usmansaleem 9750fff
codefix: Reduce initial delay and recurring delay in dnsdaemontest
usmansaleem 8cf02d7
codefix: Read dns txt entries from a json file in mock dns server
usmansaleem c40a643
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem f081e64
Convert Mock Dns Server as Vertx Verticle for easier management
usmansaleem 574ec28
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem 636aedb
increase second lookup time in test
usmansaleem b5dfc73
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem f91143b
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem 806bf5b
Update NOTICE
usmansaleem 8abc6cc
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem 4ed7736
Merge remote-tracking branch 'upstream/main' into consensys_dns_disco…
usmansaleem File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
107 changes: 107 additions & 0 deletions
107
ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/dns/DNSDaemon.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
/* | ||
* Copyright contributors to Hyperledger Besu. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with | ||
* the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on | ||
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations under the License. | ||
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
package org.hyperledger.besu.ethereum.p2p.discovery.dns; | ||
|
||
import java.util.List; | ||
import java.util.Optional; | ||
|
||
import io.vertx.core.AbstractVerticle; | ||
import org.apache.tuweni.devp2p.EthereumNodeRecord; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
// Adapted from https://github.com/tmio/tuweni and licensed under Apache 2.0 | ||
/** | ||
* Resolves DNS records over time, refreshing records. This is written as a Vertx Verticle which | ||
* allows to run outside the Vertx event loop | ||
*/ | ||
public class DNSDaemon extends AbstractVerticle { | ||
private static final Logger LOG = LoggerFactory.getLogger(DNSDaemon.class); | ||
private final String enrLink; | ||
private final long seq; | ||
private final long initialDelay; | ||
private final long delay; | ||
private final Optional<DNSDaemonListener> listener; | ||
private final Optional<String> dnsServer; | ||
private Optional<Long> periodicTaskId = Optional.empty(); | ||
private DNSResolver dnsResolver; | ||
|
||
/** | ||
* Creates a new DNSDaemon. | ||
* | ||
* @param enrLink the ENR link to start with, of the form enrtree://PUBKEY@domain | ||
* @param listener Listener notified when records are read and whenever they are updated. | ||
* @param seq the sequence number of the root record. If the root record seq is higher, proceed | ||
* with visit. | ||
* @param initialDelay the delay in milliseconds before the first poll of DNS records. | ||
* @param delay the delay in milliseconds at which to poll DNS records. If negative or zero, it | ||
* runs only once. | ||
* @param dnsServer the DNS server to use for DNS query. If null, the default DNS server will be | ||
* used. | ||
*/ | ||
public DNSDaemon( | ||
final String enrLink, | ||
final DNSDaemonListener listener, | ||
final long seq, | ||
final long initialDelay, | ||
final long delay, | ||
final String dnsServer) { | ||
this.enrLink = enrLink; | ||
this.listener = Optional.ofNullable(listener); | ||
this.seq = seq; | ||
this.initialDelay = initialDelay; | ||
this.delay = delay; | ||
this.dnsServer = Optional.ofNullable(dnsServer); | ||
} | ||
|
||
/** Starts the DNSDaemon. */ | ||
@Override | ||
public void start() { | ||
if (vertx == null) { | ||
throw new IllegalStateException("DNSDaemon must be deployed as a vertx verticle."); | ||
} | ||
|
||
LOG.info("Starting DNSDaemon for {}, using {} DNS host.", enrLink, dnsServer.orElse("default")); | ||
this.dnsResolver = new DNSResolver(vertx, enrLink, seq, dnsServer); | ||
usmansaleem marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (delay > 0) { | ||
periodicTaskId = Optional.of(vertx.setPeriodic(initialDelay, delay, this::refreshENRRecords)); | ||
} else { | ||
// do one-shot resolution | ||
refreshENRRecords(0L); | ||
} | ||
} | ||
|
||
/** Stops the DNSDaemon. */ | ||
@Override | ||
public void stop() { | ||
LOG.info("Stopping DNSDaemon for {}", enrLink); | ||
periodicTaskId.ifPresent(vertx::cancelTimer); | ||
dnsResolver.close(); | ||
} | ||
|
||
/** | ||
* Refresh enr records by calling dnsResolver and updating the listeners. | ||
* | ||
* @param taskId the task id of the periodic task | ||
*/ | ||
void refreshENRRecords(final Long taskId) { | ||
LOG.debug("Refreshing DNS records"); | ||
final long startTime = System.nanoTime(); | ||
final List<EthereumNodeRecord> ethereumNodeRecords = dnsResolver.collectAll(); | ||
final long endTime = System.nanoTime(); | ||
LOG.debug("Time taken to DNSResolver.collectAll: {} ms", (endTime - startTime) / 1_000_000); | ||
listener.ifPresent(it -> it.newRecords(dnsResolver.sequence(), ethereumNodeRecords)); | ||
} | ||
} |
32 changes: 32 additions & 0 deletions
32
.../p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/dns/DNSDaemonListener.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
/* | ||
* Copyright contributors to Hyperledger Besu. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with | ||
* the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on | ||
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations under the License. | ||
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
package org.hyperledger.besu.ethereum.p2p.discovery.dns; | ||
|
||
import java.util.List; | ||
|
||
import org.apache.tuweni.devp2p.EthereumNodeRecord; | ||
|
||
// Adapted from https://github.com/tmio/tuweni and licensed under Apache 2.0 | ||
/** Callback listening to updates of the DNS records. */ | ||
@FunctionalInterface | ||
public interface DNSDaemonListener { | ||
/** | ||
* Callback called when the seq is updated on the DNS server | ||
* | ||
* @param seq the update identifier of the records | ||
* @param records the records stored on the server | ||
*/ | ||
void newRecords(long seq, List<EthereumNodeRecord> records); | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this sufficient attribution as far as the apache license is concerned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is. Although we may have to update our NOTICE file with Tuweni's NOTICE file. Our NOTICE file is empty though. Also, we didn't do a verbatim copy of the code, we adapted and converted it into Java language from Kotlin language.
https://opensource.stackexchange.com/questions/8717/how-to-attribute-code-from-a-project-under-apache-v2-in-a-new-project
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/tmio/tuweni/blob/main/NOTICE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we add all contributors info to the notice going forward?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if we are "reproducing" or "including" code from an Apache license project that contains a NOTICE file, it ought to be included in our NOTICE file as well. I am not sure whether we had to include NOTICE attributions from Vertx and other libraries that are included in binary form. @non-fungible-nelson FYI for your input as well.