Skip to content
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

Fix dns discovery issue #3033

Merged
merged 4 commits into from
Nov 10, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -296,8 +296,9 @@ public String getDNSDiscoveryURL() {
return dnsDiscoveryURL;
}

public void setDnsDiscoveryURL(final String dnsDiscoveryURL) {
public DiscoveryConfiguration setDnsDiscoveryURL(final String dnsDiscoveryURL) {
this.dnsDiscoveryURL = dnsDiscoveryURL;
return this;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ public void start() {
dnsPeers.set(peers);
});
}
getDnsDaemon().ifPresent(DNSDaemon::start);

final int listeningPort = rlpxAgent.start().join();
final int discoveryPort =
Expand Down Expand Up @@ -267,9 +268,7 @@ public void stop() {
return;
}

if (dnsDaemon != null) {
dnsDaemon.close();
}
getDnsDaemon().ifPresent(DNSDaemon::close);

peerConnectionScheduler.shutdownNow();
peerDiscoveryAgent.stop().whenComplete((res, err) -> shutdownLatch.countDown());
Expand Down Expand Up @@ -321,6 +320,11 @@ public boolean removeMaintainedConnectionPeer(final Peer peer) {
return wasRemoved;
}

@VisibleForTesting
Optional<DNSDaemon> getDnsDaemon() {
return Optional.ofNullable(dnsDaemon);
}

@VisibleForTesting
void checkMaintainedConnectionPeers() {
if (!localNode.isReady()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -67,7 +68,6 @@
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;

@RunWith(MockitoJUnitRunner.StrictStubs.class)
Expand Down Expand Up @@ -231,7 +231,7 @@ public void start_withNatManager() {
when(upnpNatManager.queryExternalIPAddress())
.thenReturn(CompletableFuture.completedFuture(externalIp));

final NatService natService = Mockito.spy(new NatService(Optional.of(upnpNatManager)));
final NatService natService = spy(new NatService(Optional.of(upnpNatManager)));
final P2PNetwork network = builder().natService(natService).build();

network.start();
Expand Down Expand Up @@ -333,6 +333,36 @@ public void cannotAddNodeWithSameEnodeID() {
assertThat(network.addMaintainConnectionPeer(peer)).isFalse();
}

@Test
public void shouldNotStartDnsDiscoveryWhenDNSURLIsNotConfigured() {
// spy on DefaultP2PNetwork
DefaultP2PNetwork testClass = spy(network());

testClass.start();
// ensure we called getDnsDaemon during start, and that it is NOT present:
verify(testClass, times(1)).getDnsDaemon();
assertThat(testClass.getDnsDaemon()).isNotPresent();
}

@Test
public void shouldStartDnsDiscoveryWhenDNSURLIsNotConfigured() {
// create a discovery config with a dns config
DiscoveryConfiguration disco =
DiscoveryConfiguration.create().setDnsDiscoveryURL("enrtree://mock@localhost");

// spy on config to return dns discovery config:
NetworkingConfiguration dnsConfig =
when(spy(config).getDiscovery()).thenReturn(disco).getMock();

// spy on DefaultP2PNetwork
DefaultP2PNetwork testClass = spy((DefaultP2PNetwork) builder().config(dnsConfig).build());

testClass.start();
// ensure we called getDnsDaemon during start, and that it is present:
verify(testClass, times(1)).getDnsDaemon();
assertThat(testClass.getDnsDaemon()).isPresent();
}

private DefaultP2PNetwork network() {
return (DefaultP2PNetwork) builder().build();
}
Expand Down