From 814929525351b6a63bea6d190845b32a1ca20a56 Mon Sep 17 00:00:00 2001 From: shemnon Date: Fri, 30 Nov 2018 09:10:23 -0700 Subject: [PATCH 1/2] [MINOR] no fixed ports in tests Some of our tests rely on fixed network ports being available to run their test. Add code to choose OS assigned ephemiral ports in those cases. --- .../ethereum/p2p/discovery/PeerDiscoveryAgentTest.java | 10 +++++++--- .../p2p/discovery/PeerDiscoveryTestHelper.java | 8 ++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java index d1335bbd54..d6caca156c 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java @@ -32,6 +32,7 @@ import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage.DisconnectReason; import tech.pegasys.pantheon.util.bytes.BytesValue; +import java.io.IOException; import java.net.SocketAddress; import java.util.Arrays; import java.util.Collections; @@ -128,24 +129,26 @@ public void neighborsPacketLimited() { } @Test - public void shouldEvictPeerOnDisconnect() { + public void shouldEvictPeerOnDisconnect() throws IOException { final Vertx vertx = Vertx.vertx(); final SECP256K1.KeyPair keyPair = SECP256K1.KeyPair.generate(); final BytesValue id = fromHexString( "c7849b663d12a2b5bf05b1ebf5810364f4870d5f1053fbd7500d38bc54c705b453d7511ca8a4a86003d34d4c8ee0bbfcd387aa724f5b240b3ab4bbb994a1e09b"); - final DefaultPeer peer = new DiscoveryPeer(id, "127.0.0.1", 30303); + final int port = PeerDiscoveryTestHelper.generateEphemiralPort(); + final DefaultPeer peer = new DiscoveryPeer(id, "127.0.0.1", port); final PeerDiscoveryAgent peerDiscoveryAgent = new PeerDiscoveryAgent( vertx, keyPair, DiscoveryConfiguration.create() .setBindHost("127.0.0.1") + .setBindPort(port) .setBootstrapPeers(Lists.newArrayList(peer)), () -> true, new PeerBlacklist()); - peerDiscoveryAgent.start(30303).join(); + peerDiscoveryAgent.start(port).join(); assertThat(peerDiscoveryAgent.getPeers().size()).isEqualTo(1); @@ -289,6 +292,7 @@ public void blacklistIncompatiblePeerWhoIssuesDisconnect() throws Exception { public void shouldBeActiveWhenConfigIsTrue() { final DiscoveryConfiguration config = new DiscoveryConfiguration(); config.setActive(true); + config.setBindPort(0); final PeerDiscoveryAgent agent = startDiscoveryAgent(config, new PeerBlacklist()); diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java index 0f1ab660a5..12a3903d7c 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java @@ -15,6 +15,8 @@ import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.ethereum.p2p.peers.Endpoint; +import java.io.IOException; +import java.net.ServerSocket; import java.util.OptionalInt; import java.util.stream.Stream; @@ -36,4 +38,10 @@ public static DiscoveryPeer[] generatePeers(final SECP256K1.KeyPair... keypairs) public static DiscoveryPeer[] generateDiscoveryPeers(final SECP256K1.KeyPair... keypairs) { return Stream.of(generatePeers(keypairs)).map(DiscoveryPeer::new).toArray(DiscoveryPeer[]::new); } + + public static int generateEphemiralPort() throws IOException { + try (final ServerSocket serverSocket = new ServerSocket(0)) { + return serverSocket.getLocalPort(); + } + } } From 40112c27f6dea49f7cd785684df9fbc1feaec639 Mon Sep 17 00:00:00 2001 From: shemnon Date: Fri, 30 Nov 2018 20:52:27 -0700 Subject: [PATCH 2/2] [MINOR] no fixed ports in tests Approach #2, First enable "0" port in PeerDiscoveryAgent so it binds to an open port. Then change the test to use two PeerDiscoveryAgents The change to zero is accomplished by populating the endpoint with the actual bound socket instead of the value passed in. If it is zero the value will change otherwise it will be the value passed in. --- .../p2p/discovery/PeerDiscoveryAgent.java | 2 +- .../p2p/discovery/PeerDiscoveryAgentTest.java | 41 ++++++++++--------- .../discovery/PeerDiscoveryTestHelper.java | 8 ---- 3 files changed, 23 insertions(+), 28 deletions(-) diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java index cc5786d3c1..df01fca322 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java @@ -171,7 +171,7 @@ public CompletableFuture start(final int tcpPort) { completion.completeExceptionally(cause); return; } - initialize(res.result(), tcpPort); + initialize(res.result(), res.result().localAddress().port()); this.isActive = true; completion.complete(null); }); diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java index d6caca156c..12d78a4ca9 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java @@ -14,7 +14,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; -import static tech.pegasys.pantheon.util.bytes.BytesValue.fromHexString; import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; @@ -32,7 +31,6 @@ import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage.DisconnectReason; import tech.pegasys.pantheon.util.bytes.BytesValue; -import java.io.IOException; import java.net.SocketAddress; import java.util.Arrays; import java.util.Collections; @@ -129,33 +127,39 @@ public void neighborsPacketLimited() { } @Test - public void shouldEvictPeerOnDisconnect() throws IOException { + public void shouldEvictPeerOnDisconnect() { final Vertx vertx = Vertx.vertx(); - final SECP256K1.KeyPair keyPair = SECP256K1.KeyPair.generate(); - final BytesValue id = - fromHexString( - "c7849b663d12a2b5bf05b1ebf5810364f4870d5f1053fbd7500d38bc54c705b453d7511ca8a4a86003d34d4c8ee0bbfcd387aa724f5b240b3ab4bbb994a1e09b"); - - final int port = PeerDiscoveryTestHelper.generateEphemiralPort(); - final DefaultPeer peer = new DiscoveryPeer(id, "127.0.0.1", port); - final PeerDiscoveryAgent peerDiscoveryAgent = + + final SECP256K1.KeyPair keyPair1 = SECP256K1.KeyPair.generate(); + final PeerDiscoveryAgent peerDiscoveryAgent1 = + new PeerDiscoveryAgent( + vertx, + keyPair1, + DiscoveryConfiguration.create().setBindHost("127.0.0.1").setBindPort(0), + () -> true, + new PeerBlacklist()); + peerDiscoveryAgent1.start(0).join(); + final DefaultPeer peer = peerDiscoveryAgent1.getAdvertisedPeer(); + + final SECP256K1.KeyPair keyPair2 = SECP256K1.KeyPair.generate(); + final PeerDiscoveryAgent peerDiscoveryAgent2 = new PeerDiscoveryAgent( vertx, - keyPair, + keyPair2, DiscoveryConfiguration.create() .setBindHost("127.0.0.1") - .setBindPort(port) + .setBindPort(0) .setBootstrapPeers(Lists.newArrayList(peer)), () -> true, new PeerBlacklist()); - peerDiscoveryAgent.start(port).join(); + peerDiscoveryAgent2.start(0).join(); - assertThat(peerDiscoveryAgent.getPeers().size()).isEqualTo(1); + assertThat(peerDiscoveryAgent2.getPeers().size()).isEqualTo(1); - final PeerConnection peerConnection = createAnonymousPeerConnection(id); - peerDiscoveryAgent.onDisconnect(peerConnection, DisconnectReason.REQUESTED, true); + final PeerConnection peerConnection = createAnonymousPeerConnection(peer.getId()); + peerDiscoveryAgent2.onDisconnect(peerConnection, DisconnectReason.REQUESTED, true); - assertThat(peerDiscoveryAgent.getPeers().size()).isEqualTo(0); + assertThat(peerDiscoveryAgent2.getPeers().size()).isEqualTo(0); } @Test @@ -292,7 +296,6 @@ public void blacklistIncompatiblePeerWhoIssuesDisconnect() throws Exception { public void shouldBeActiveWhenConfigIsTrue() { final DiscoveryConfiguration config = new DiscoveryConfiguration(); config.setActive(true); - config.setBindPort(0); final PeerDiscoveryAgent agent = startDiscoveryAgent(config, new PeerBlacklist()); diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java index 12a3903d7c..0f1ab660a5 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java @@ -15,8 +15,6 @@ import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.ethereum.p2p.peers.Endpoint; -import java.io.IOException; -import java.net.ServerSocket; import java.util.OptionalInt; import java.util.stream.Stream; @@ -38,10 +36,4 @@ public static DiscoveryPeer[] generatePeers(final SECP256K1.KeyPair... keypairs) public static DiscoveryPeer[] generateDiscoveryPeers(final SECP256K1.KeyPair... keypairs) { return Stream.of(generatePeers(keypairs)).map(DiscoveryPeer::new).toArray(DiscoveryPeer[]::new); } - - public static int generateEphemiralPort() throws IOException { - try (final ServerSocket serverSocket = new ServerSocket(0)) { - return serverSocket.getLocalPort(); - } - } }