Skip to content

Commit

Permalink
Fix dns discovery issue (hyperledger#3033)
Browse files Browse the repository at this point in the history
We are creating but not starting the dns discovery daemon. After this modification the node find peers instantly

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

Co-authored-by: garyschulte <garyschulte@gmail.com>
  • Loading branch information
2 people authored and eum602 committed Nov 3, 2023
1 parent ec6d2fd commit 8132f58
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 6 deletions.
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

0 comments on commit 8132f58

Please sign in to comment.