From f8fec4c066a7205cf704e4701ed4fb1f94c3ee28 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Thu, 17 Jan 2019 14:27:36 +1000 Subject: [PATCH 1/8] [NC-1970] empty add peer call and indexing of pending connections --- .../internal/methods/AdminAddPeer.java | 56 +++++++++++++++++++ .../ethereum/p2p/netty/NettyP2PNetwork.java | 21 ++++--- 2 files changed, 69 insertions(+), 8 deletions(-) create mode 100644 ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java new file mode 100644 index 0000000000..fe8063adfd --- /dev/null +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java @@ -0,0 +1,56 @@ +/* + * Copyright 2018 ConsenSys AG. + * + * 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. + */ +package tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods; + +import java.util.List; +import java.util.stream.Collectors; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.JsonRpcRequest; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.parameters.JsonRpcParameter; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcResponse; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.results.PeerResult; +import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork; +import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; +import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; + +public class AdminAddPeer implements JsonRpcMethod { + private static final Logger LOG = LogManager.getLogger(); + private final P2PNetwork peerNetwork; + private final JsonRpcParameter parameters; + + public AdminAddPeer(final P2PNetwork peerNetwork, final JsonRpcParameter parameters) { + this.peerNetwork = peerNetwork; + this.parameters = parameters; + } + + @Override + public String getName() { + return "admin_addPeer"; + } + + @Override + public JsonRpcResponse response(final JsonRpcRequest req) { + + final String enodeString = parameters.required(req.getParams(), 0, String.class); + final Peer peer = DefaultPeer.fromURI(enodeString); + + try { + return null; + } catch (final Exception e) { + LOG.error("Error processing request: " + req, e); + throw e; + } + } +} diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java index 95f5f36c35..7170a34678 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java @@ -83,7 +83,7 @@ * distance from us is equal to the index of the bucket. The value x in the distance function * corresponds to our node ID (public key). * - *

Upper layers in the stack subscribe to events from the peer discocvery layer and initiate/drop + *

Upper layers in the stack subscribe to events from the peer discovery layer and initiate/drop * connections accordingly. * *

RLPx Wire Protocol

@@ -127,7 +127,7 @@ public final class NettyP2PNetwork implements P2PNetwork { private final PeerConnectionRegistry connections; - private final AtomicInteger pendingConnections = new AtomicInteger(0); + private final Map> pendingConnections = new ConcurrentHashMap<>(); private final EventLoopGroup boss = new NioEventLoopGroup(1); @@ -275,8 +275,8 @@ protected void initChannel(final SocketChannel ch) { return; } - boolean isPeerBlacklisted = peerBlacklist.contains(connection); - boolean isPeerNotWhitelisted = !isPeerWhitelisted(connection, ch); + final boolean isPeerBlacklisted = peerBlacklist.contains(connection); + final boolean isPeerNotWhitelisted = !isPeerWhitelisted(connection, ch); if (isPeerBlacklisted || isPeerNotWhitelisted) { connection.disconnect(DisconnectReason.UNKNOWN); @@ -302,7 +302,7 @@ private boolean isPeerWhitelisted(final PeerConnection connection, final SocketC } private int connectionCount() { - return pendingConnections.get() + connections.size(); + return pendingConnections.size() + connections.size(); } @Override @@ -315,7 +315,11 @@ public CompletableFuture connect(final Peer peer) { LOG.trace("Initiating connection to peer: {}", peer.getId()); final CompletableFuture connectionFuture = new CompletableFuture<>(); final Endpoint endpoint = peer.getEndpoint(); - pendingConnections.incrementAndGet(); + final CompletableFuture existingPendingConnection = pendingConnections.putIfAbsent(peer, connectionFuture); + if (existingPendingConnection != null) { + LOG.debug("Attempted to connect to peer with pending connection: {}", peer.getId()); + return existingPendingConnection; + } new Bootstrap() .group(workers) @@ -358,7 +362,7 @@ protected void initChannel(final SocketChannel ch) { connectionFuture.whenComplete( (connection, t) -> { - pendingConnections.decrementAndGet(); + pendingConnections.remove(peer); if (t == null) { onConnectionEstablished(connection); LOG.debug("Connection established to peer: {}", peer.getId()); @@ -373,7 +377,7 @@ protected void initChannel(final SocketChannel ch) { private void logConnections() { LOG.debug( "Connections: {} pending, {} active connections.", - pendingConnections.get(), + pendingConnections.size(), connections.size()); } @@ -406,6 +410,7 @@ public void run() { } }); peerBondedObserverId = OptionalLong.of(observerId); + } catch (final Exception ex) { throw new IllegalStateException(ex); } From 30991c24df28c715ee69149ace7b94650742dd72 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Fri, 18 Jan 2019 18:04:37 +1000 Subject: [PATCH 2/8] [NC-1970] bunch of spotless --- .../internal/methods/AdminAddPeer.java | 9 ++-- .../ethereum/p2p/testing/MockNetwork.java | 8 ++++ .../pantheon/ethereum/p2p/NetworkRunner.java | 5 ++ .../pantheon/ethereum/p2p/api/P2PNetwork.java | 4 ++ .../ethereum/p2p/netty/NettyP2PNetwork.java | 48 ++++++++++++++++--- 5 files changed, 61 insertions(+), 13 deletions(-) diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java index fe8063adfd..35e96a99e1 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java @@ -12,19 +12,16 @@ */ package tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods; -import java.util.List; -import java.util.stream.Collectors; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.JsonRpcRequest; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.parameters.JsonRpcParameter; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcResponse; -import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse; -import tech.pegasys.pantheon.ethereum.jsonrpc.internal.results.PeerResult; import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork; import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + public class AdminAddPeer implements JsonRpcMethod { private static final Logger LOG = LogManager.getLogger(); private final P2PNetwork peerNetwork; diff --git a/ethereum/mock-p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/testing/MockNetwork.java b/ethereum/mock-p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/testing/MockNetwork.java index 2842195816..bf8d7afaa8 100644 --- a/ethereum/mock-p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/testing/MockNetwork.java +++ b/ethereum/mock-p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/testing/MockNetwork.java @@ -162,6 +162,14 @@ public void subscribeDisconnect(final DisconnectCallback callback) { disconnectCallbacks.subscribe(callback); } + @Override + public boolean addMaintainConnectionPeer(final Peer peer) { + return true; + } + + @Override + public void checkMaintainedConnectionPeers() {} + @Override public void stop() {} diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/NetworkRunner.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/NetworkRunner.java index 8d3a40fc2c..b87d729436 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/NetworkRunner.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/NetworkRunner.java @@ -31,6 +31,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; @@ -50,6 +51,8 @@ public class NetworkRunner implements AutoCloseable { private final ExecutorService networkExecutor = Executors.newFixedThreadPool( 1, new ThreadFactoryBuilder().setNameFormat(this.getClass().getSimpleName()).build()); + private final ScheduledExecutorService networkCheckExecutor = + Executors.newSingleThreadScheduledExecutor(); private final P2PNetwork network; private final Map subProtocols; @@ -87,6 +90,7 @@ public void start() { LOG.info("Starting Network."); setupHandlers(); networkExecutor.submit(network); + networkCheckExecutor.schedule(network::checkMaintainedConnectionPeers, 60, TimeUnit.SECONDS); } else { LOG.error("Attempted to start already running network."); } @@ -100,6 +104,7 @@ public void stop() { protocolManager.stop(); } networkExecutor.shutdown(); + networkCheckExecutor.shutdown(); shutdown.countDown(); } else { LOG.error("Attempted to stop already stopped network."); diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java index 066c22b60f..a6e29db941 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java @@ -65,6 +65,10 @@ public interface P2PNetwork extends Closeable, Runnable { */ void subscribeDisconnect(DisconnectCallback consumer); + boolean addMaintainConnectionPeer(final Peer peer); + + void checkMaintainedConnectionPeers(); + /** Stops the P2P network layer. */ void stop(); diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java index 7170a34678..53fc1deaff 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java @@ -41,6 +41,7 @@ import java.net.InetSocketAddress; import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.OptionalLong; @@ -49,7 +50,6 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; import com.google.common.annotations.VisibleForTesting; @@ -125,9 +125,12 @@ public final class NettyP2PNetwork implements P2PNetwork { private final PeerBlacklist peerBlacklist; private OptionalLong peerBondedObserverId = OptionalLong.empty(); + private final Collection peerMaintainConnectionList; + private final PeerConnectionRegistry connections; - private final Map> pendingConnections = new ConcurrentHashMap<>(); + private final Map> pendingConnections = + new ConcurrentHashMap<>(); private final EventLoopGroup boss = new NioEventLoopGroup(1); @@ -176,6 +179,7 @@ public NettyP2PNetwork( connections = new PeerConnectionRegistry(metricsSystem); this.peerBlacklist = peerBlacklist; this.nodeWhitelistController = nodeWhitelistController; + this.peerMaintainConnectionList = new HashSet<>(); peerDiscoveryAgent = new VertxPeerDiscoveryAgent( vertx, @@ -301,6 +305,26 @@ private boolean isPeerWhitelisted(final PeerConnection connection, final SocketC connection.getPeer().getPort())); } + @Override + public boolean addMaintainConnectionPeer(final Peer peer) { + final boolean added = peerMaintainConnectionList.add(peer); + if (added) { + connect(peer); + return true; + } else { + return false; + } + } + + @Override + public void checkMaintainedConnectionPeers() { + for (final Peer peer : peerMaintainConnectionList) { + if (!(isConnecting(peer) || isConnected(peer))) { + connect(peer); + } + } + } + private int connectionCount() { return pendingConnections.size() + connections.size(); } @@ -315,7 +339,8 @@ public CompletableFuture connect(final Peer peer) { LOG.trace("Initiating connection to peer: {}", peer.getId()); final CompletableFuture connectionFuture = new CompletableFuture<>(); final Endpoint endpoint = peer.getEndpoint(); - final CompletableFuture existingPendingConnection = pendingConnections.putIfAbsent(peer, connectionFuture); + final CompletableFuture existingPendingConnection = + pendingConnections.putIfAbsent(peer, connectionFuture); if (existingPendingConnection != null) { LOG.debug("Attempted to connect to peer with pending connection: {}", peer.getId()); return existingPendingConnection; @@ -403,19 +428,28 @@ public void run() { final long observerId = peerDiscoveryAgent.observePeerBondedEvents( peerBondedEvent -> { + final Peer peer = peerBondedEvent.getPeer(); if (connectionCount() < maxPeers - && peerBondedEvent.getPeer().getEndpoint().getTcpPort().isPresent() - && !connections.isAlreadyConnected(peerBondedEvent.getPeer().getId())) { - connect(peerBondedEvent.getPeer()); + && peer.getEndpoint().getTcpPort().isPresent() + && !isConnecting(peer) + && !isConnected(peer)) { + connect(peer); } }); peerBondedObserverId = OptionalLong.of(observerId); - } catch (final Exception ex) { throw new IllegalStateException(ex); } } + private boolean isConnecting(final Peer peer) { + return pendingConnections.containsKey(peer); + } + + private boolean isConnected(final Peer peer) { + return connections.isAlreadyConnected(peer.getId()); + } + @Override public void stop() { sendClientQuittingToPeers(); From 967d7af26128b137c437f8875e3a2297674002c1 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Mon, 21 Jan 2019 09:05:30 +1000 Subject: [PATCH 3/8] [NC-1970] jsonrpc endpoint does something --- .../jsonrpc/internal/methods/AdminAddPeer.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java index 35e96a99e1..75e43de590 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java @@ -13,8 +13,12 @@ package tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.JsonRpcRequest; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.exception.InvalidJsonRpcParameters; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.parameters.JsonRpcParameter; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcError; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcErrorResponse; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcResponse; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse; import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork; import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; @@ -39,12 +43,15 @@ public String getName() { @Override public JsonRpcResponse response(final JsonRpcRequest req) { - - final String enodeString = parameters.required(req.getParams(), 0, String.class); - final Peer peer = DefaultPeer.fromURI(enodeString); - try { - return null; + final String enodeString = parameters.required(req.getParams(), 0, String.class); + final Peer peer = DefaultPeer.fromURI(enodeString); + final boolean added = peerNetwork.addMaintainConnectionPeer(peer); + return new JsonRpcSuccessResponse(req.getId(), added); + } catch (final InvalidJsonRpcParameters e) { + return new JsonRpcErrorResponse(req.getId(), JsonRpcError.INVALID_PARAMS); + } catch (final IllegalArgumentException e) { + return new JsonRpcErrorResponse(req.getId(), JsonRpcError.PARSE_ERROR); } catch (final Exception e) { LOG.error("Error processing request: " + req, e); throw e; From a0b6a0f4d2662677d76455eef6c4c6cc38a124b5 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Mon, 21 Jan 2019 20:09:25 +1000 Subject: [PATCH 4/8] [NC-1970] tests for netty p2p network changes --- .../ethereum/p2p/netty/NettyP2PNetwork.java | 9 +- .../ethereum/p2p/NettyP2PNetworkTest.java | 114 ++++++++++++++++-- .../org.mockito.plugins.MockMaker | 1 + 3 files changed, 114 insertions(+), 10 deletions(-) create mode 100644 ethereum/p2p/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java index 53fc1deaff..1823eaa2db 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java @@ -125,11 +125,14 @@ public final class NettyP2PNetwork implements P2PNetwork { private final PeerBlacklist peerBlacklist; private OptionalLong peerBondedObserverId = OptionalLong.empty(); - private final Collection peerMaintainConnectionList; + @VisibleForTesting + public final Collection peerMaintainConnectionList; - private final PeerConnectionRegistry connections; + @VisibleForTesting + public final PeerConnectionRegistry connections; - private final Map> pendingConnections = + @VisibleForTesting + public final Map> pendingConnections = new ConcurrentHashMap<>(); private final EventLoopGroup boss = new NioEventLoopGroup(1); diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java index 21f91a6e67..3540798ee3 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java @@ -17,9 +17,15 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.AbstractMap.SimpleEntry; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork; import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; @@ -52,6 +58,7 @@ import org.junit.Test; /** Tests for {@link NettyP2PNetwork}. */ +@RunWith(MockitoJUnitRunner.StrictStubs.class) public final class NettyP2PNetworkTest { private final Vertx vertx = Vertx.vertx(); @@ -401,11 +408,10 @@ public void rejectIncomingConnectionFromNonWhitelistedPeer() throws Exception { final SECP256K1.KeyPair localKp = SECP256K1.KeyPair.generate(); final SECP256K1.KeyPair remoteKp = SECP256K1.KeyPair.generate(); final BytesValue localId = localKp.getPublicKey().getEncodedBytes(); - final BytesValue remoteId = remoteKp.getPublicKey().getEncodedBytes(); final PeerBlacklist localBlacklist = new PeerBlacklist(); final PeerBlacklist remoteBlacklist = new PeerBlacklist(); final PermissioningConfiguration config = PermissioningConfiguration.createDefault(); - NodeWhitelistController localWhitelist = new NodeWhitelistController(config); + final NodeWhitelistController localWhitelist = new NodeWhitelistController(config); // turn on whitelisting by adding a different node NOT remote node localWhitelist.addNode(mockPeer()); @@ -469,6 +475,88 @@ public void rejectIncomingConnectionFromNonWhitelistedPeer() throws Exception { } } + @Test + public void addingMaintainedNetworkPeerStartsConnection() { + final NettyP2PNetwork network = spy(mockNettyP2PNetwork()); + final Peer peer = mockPeer(); + + assertThat(network.addMaintainConnectionPeer(peer)).isTrue(); + + assertThat(network.peerMaintainConnectionList).contains(peer); + verify(network, times(1)).connect(peer); + } + + @Test + public void addingRepeatMaintainedPeersReturnsFalse() { + final NettyP2PNetwork network = mockNettyP2PNetwork(); + final Peer peer = mockPeer(); + assertThat(network.addMaintainConnectionPeer(peer)).isTrue(); + assertThat(network.addMaintainConnectionPeer(peer)).isFalse(); + } + + @Test + public void checkMaintainedConnectionPeersTriesToConnect() { + final NettyP2PNetwork network = mockNettyP2PNetwork(); + final Peer peer = mockPeer(); + assertThat(network.addMaintainConnectionPeer(peer)).isTrue(); + + final CompletableFuture connection; + connection = network.pendingConnections.remove(peer); + assertThat(connection).isNotNull(); + assertThat(connection.cancel(true)).isTrue(); + + network.checkMaintainedConnectionPeers(); + assertThat(network.pendingConnections).containsKey(peer); + } + + @Test + public void checkMaintainedConnectionPeersDoesntReconnectPendingPeers() { + final NettyP2PNetwork network = mockNettyP2PNetwork(); + final Peer peer = mockPeer(); + assertThat(network.addMaintainConnectionPeer(peer)).isTrue(); + + { + final CompletableFuture connection; + connection = network.pendingConnections.remove(peer); + assertThat(connection).isNotNull(); + assertThat(connection.cancel(true)).isTrue(); + } + + { + network.checkMaintainedConnectionPeers(); + final CompletableFuture connection; + connection = network.pendingConnections.get(peer); + assertThat(connection).isNotNull(); + network.checkMaintainedConnectionPeers(); + assertThat(network.pendingConnections).contains(new SimpleEntry<>(peer, connection)); + } + } + + @Test + public void checkMaintainedConnectionPeersDoesntReconnectConnectedPeers() { + final NettyP2PNetwork network = spy(mockNettyP2PNetwork()); + final Peer peer = mockPeer(); + verify(network, never()).connect(peer); + assertThat(network.addMaintainConnectionPeer(peer)).isTrue(); + verify(network, times(1)).connect(peer); + + { + final CompletableFuture connection; + connection = network.pendingConnections.remove(peer); + assertThat(connection).isNotNull(); + assertThat(connection.cancel(true)).isTrue(); + } + + { + final PeerConnection peerConnection = mockPeerConnection(peer.getId()); + network.connections.registerConnection(peerConnection); + network.checkMaintainedConnectionPeers(); + verify(network, times(1)).connect(peer); + } + + } + + private SubProtocol subProtocol() { return new SubProtocol() { @Override @@ -529,14 +617,27 @@ public void shouldSendClientQuittingWhenNetworkStops() { verify(peerConnection).disconnect(eq(DisconnectReason.CLIENT_QUITTING)); } - private PeerConnection mockPeerConnection() { + @Test + public void shouldntAttemptNewConnectionToPendingPeer() { + final NettyP2PNetwork nettyP2PNetwork = mockNettyP2PNetwork(); + final Peer peer = mockPeer(); + + final CompletableFuture connectingFuture = nettyP2PNetwork.connect(peer); + assertThat(nettyP2PNetwork.connect(peer)).isEqualTo(connectingFuture); + } + + private PeerConnection mockPeerConnection(final BytesValue id) { final PeerInfo peerInfo = mock(PeerInfo.class); - when(peerInfo.getNodeId()).thenReturn(BytesValue.fromHexString("0x00")); + when(peerInfo.getNodeId()).thenReturn(id); final PeerConnection peerConnection = mock(PeerConnection.class); when(peerConnection.getPeer()).thenReturn(peerInfo); return peerConnection; } + private PeerConnection mockPeerConnection() { + return mockPeerConnection(BytesValue.fromHexString("0x00")); + } + private NettyP2PNetwork mockNettyP2PNetwork() { final DiscoveryConfiguration noDiscovery = DiscoveryConfiguration.create().setActive(false); final SECP256K1.KeyPair keyPair = SECP256K1.KeyPair.generate(); @@ -560,10 +661,9 @@ private NettyP2PNetwork mockNettyP2PNetwork() { private Peer mockPeer() { final Peer peer = mock(Peer.class); + final BytesValue id = SECP256K1.KeyPair.generate().getPublicKey().getEncodedBytes(); when(peer.getId()) - .thenReturn( - BytesValue.fromHexString( - "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000")); + .thenReturn(id); when(peer.getEndpoint()).thenReturn(new Endpoint("127.0.0.1", 30303, OptionalInt.of(30303))); return peer; } diff --git a/ethereum/p2p/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker b/ethereum/p2p/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker new file mode 100644 index 0000000000..ca6ee9cea8 --- /dev/null +++ b/ethereum/p2p/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker @@ -0,0 +1 @@ +mock-maker-inline \ No newline at end of file From 0e2022f775f6e144247c64349718a768a3f170a2 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Mon, 21 Jan 2019 20:46:43 +1000 Subject: [PATCH 5/8] [NC-1970] Tests for json rpc call --- .../jsonrpc/JsonRpcMethodsFactory.java | 2 + .../internal/methods/AdminAddPeerTest.java | 116 ++++++++++++++++++ .../ethereum/p2p/netty/NettyP2PNetwork.java | 6 +- .../ethereum/p2p/NettyP2PNetworkTest.java | 11 +- 4 files changed, 124 insertions(+), 11 deletions(-) create mode 100644 ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeerTest.java diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcMethodsFactory.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcMethodsFactory.java index c149a23825..79e068f3e8 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcMethodsFactory.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcMethodsFactory.java @@ -17,6 +17,7 @@ import tech.pegasys.pantheon.ethereum.core.Synchronizer; import tech.pegasys.pantheon.ethereum.core.TransactionPool; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.filter.FilterManager; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.AdminAddPeer; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.AdminPeers; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.DebugMetrics; import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.DebugStorageRangeAt; @@ -238,6 +239,7 @@ blockchainQueries, new TransactionTracer(blockReplay), parameter), } if (rpcApis.contains(RpcApis.ADMIN)) { addMethods(enabledMethods, new AdminPeers(p2pNetwork)); + addMethods(enabledMethods, new AdminAddPeer(p2pNetwork, parameter)); } if (rpcApis.contains(RpcApis.PERM)) { addMethods( diff --git a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeerTest.java b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeerTest.java new file mode 100644 index 0000000000..0b0c654244 --- /dev/null +++ b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeerTest.java @@ -0,0 +1,116 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * 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. + */ +package tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.JsonRpcRequest; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.parameters.JsonRpcParameter; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcError; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcErrorResponse; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcResponse; +import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse; +import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.StrictStubs.class) +public class AdminAddPeerTest { + + @Mock private P2PNetwork p2pNetwork; + private final JsonRpcParameter parameter = new JsonRpcParameter(); + + private AdminAddPeer method; + + @Before + public void setup() { + method = new AdminAddPeer(p2pNetwork, parameter); + } + + @Test + public void requestIsMissingParameter() { + final JsonRpcRequest request = new JsonRpcRequest("2.0", "admin_addPeer", new String[] {}); + final JsonRpcResponse expectedResponse = + new JsonRpcErrorResponse(request.getId(), JsonRpcError.INVALID_PARAMS); + + final JsonRpcResponse actualResponse = method.response(request); + + assertThat(actualResponse).isEqualToComparingFieldByField(expectedResponse); + } + + @Test + public void requestHasInvalidEnode() { + final JsonRpcRequest request = + new JsonRpcRequest("2.0", "admin_addPeer", new String[] {"asdf"}); + final JsonRpcResponse expectedResponse = + new JsonRpcErrorResponse(request.getId(), JsonRpcError.PARSE_ERROR); + + final JsonRpcResponse actualResponse = method.response(request); + + assertThat(actualResponse).isEqualToComparingFieldByField(expectedResponse); + } + + @Test + public void requestAddsValidEnode() { + when(p2pNetwork.addMaintainConnectionPeer(any())).thenReturn(true); + + final JsonRpcRequest request = + new JsonRpcRequest( + "2.0", + "admin_addPeer", + new String[] { + "enode://" + + "00000000000000000000000000000000" + + "00000000000000000000000000000000" + + "00000000000000000000000000000000" + + "00000000000000000000000000000000" + + "@127.0.0.1:30303" + }); + + final JsonRpcResponse expectedResponse = new JsonRpcSuccessResponse(request.getId(), true); + + final JsonRpcResponse actualResponse = method.response(request); + + assertThat(actualResponse).isEqualToComparingFieldByField(expectedResponse); + } + + @Test + public void requestReturnsFalseIfAddFails() { + when(p2pNetwork.addMaintainConnectionPeer(any())).thenReturn(false); + + final JsonRpcRequest request = + new JsonRpcRequest( + "2.0", + "admin_addPeer", + new String[] { + "enode://" + + "00000000000000000000000000000000" + + "00000000000000000000000000000000" + + "00000000000000000000000000000000" + + "00000000000000000000000000000000" + + "@127.0.0.1:30303" + }); + + final JsonRpcResponse expectedResponse = new JsonRpcSuccessResponse(request.getId(), false); + + final JsonRpcResponse actualResponse = method.response(request); + + assertThat(actualResponse).isEqualToComparingFieldByField(expectedResponse); + } +} diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java index 1823eaa2db..5f2caa2166 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java @@ -125,11 +125,9 @@ public final class NettyP2PNetwork implements P2PNetwork { private final PeerBlacklist peerBlacklist; private OptionalLong peerBondedObserverId = OptionalLong.empty(); - @VisibleForTesting - public final Collection peerMaintainConnectionList; + @VisibleForTesting public final Collection peerMaintainConnectionList; - @VisibleForTesting - public final PeerConnectionRegistry connections; + @VisibleForTesting public final PeerConnectionRegistry connections; @VisibleForTesting public final Map> pendingConnections = diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java index 3540798ee3..e8f5508a3d 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java @@ -23,9 +23,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import java.util.AbstractMap.SimpleEntry; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork; import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection; @@ -48,6 +45,7 @@ import tech.pegasys.pantheon.util.bytes.BytesValue; import java.net.InetAddress; +import java.util.AbstractMap.SimpleEntry; import java.util.List; import java.util.OptionalInt; import java.util.concurrent.CompletableFuture; @@ -56,6 +54,8 @@ import io.vertx.core.Vertx; import org.junit.After; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; /** Tests for {@link NettyP2PNetwork}. */ @RunWith(MockitoJUnitRunner.StrictStubs.class) @@ -553,10 +553,8 @@ public void checkMaintainedConnectionPeersDoesntReconnectConnectedPeers() { network.checkMaintainedConnectionPeers(); verify(network, times(1)).connect(peer); } - } - private SubProtocol subProtocol() { return new SubProtocol() { @Override @@ -662,8 +660,7 @@ private NettyP2PNetwork mockNettyP2PNetwork() { private Peer mockPeer() { final Peer peer = mock(Peer.class); final BytesValue id = SECP256K1.KeyPair.generate().getPublicKey().getEncodedBytes(); - when(peer.getId()) - .thenReturn(id); + when(peer.getId()).thenReturn(id); when(peer.getEndpoint()).thenReturn(new Endpoint("127.0.0.1", 30303, OptionalInt.of(30303))); return peer; } From f48ca3a446ee83f9226672c9d65a7c69466bfb80 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Mon, 21 Jan 2019 22:29:45 +1000 Subject: [PATCH 6/8] [NC-1970] repaired the tests and turned off final class mocking --- .../ethereum/p2p/netty/NettyP2PNetwork.java | 2 +- .../ethereum/p2p/NettyP2PNetworkTest.java | 32 ++++--------------- .../org.mockito.plugins.MockMaker | 1 - 3 files changed, 8 insertions(+), 27 deletions(-) delete mode 100644 ethereum/p2p/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java index 5f2caa2166..4233a3131c 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java @@ -107,7 +107,7 @@ * Selection * @see devp2p RLPx */ -public final class NettyP2PNetwork implements P2PNetwork { +public class NettyP2PNetwork implements P2PNetwork { private static final Logger LOG = LogManager.getLogger(); private static final int TIMEOUT_SECONDS = 30; diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java index e8f5508a3d..cc7b680027 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java @@ -45,7 +45,6 @@ import tech.pegasys.pantheon.util.bytes.BytesValue; import java.net.InetAddress; -import java.util.AbstractMap.SimpleEntry; import java.util.List; import java.util.OptionalInt; import java.util.concurrent.CompletableFuture; @@ -496,40 +495,23 @@ public void addingRepeatMaintainedPeersReturnsFalse() { @Test public void checkMaintainedConnectionPeersTriesToConnect() { - final NettyP2PNetwork network = mockNettyP2PNetwork(); + final NettyP2PNetwork network = spy(mockNettyP2PNetwork()); final Peer peer = mockPeer(); - assertThat(network.addMaintainConnectionPeer(peer)).isTrue(); - - final CompletableFuture connection; - connection = network.pendingConnections.remove(peer); - assertThat(connection).isNotNull(); - assertThat(connection.cancel(true)).isTrue(); + network.peerMaintainConnectionList.add(peer); network.checkMaintainedConnectionPeers(); - assertThat(network.pendingConnections).containsKey(peer); + verify(network, times(1)).connect(peer); } @Test public void checkMaintainedConnectionPeersDoesntReconnectPendingPeers() { - final NettyP2PNetwork network = mockNettyP2PNetwork(); + final NettyP2PNetwork network = spy(mockNettyP2PNetwork()); final Peer peer = mockPeer(); - assertThat(network.addMaintainConnectionPeer(peer)).isTrue(); - { - final CompletableFuture connection; - connection = network.pendingConnections.remove(peer); - assertThat(connection).isNotNull(); - assertThat(connection.cancel(true)).isTrue(); - } + network.pendingConnections.put(peer, new CompletableFuture<>()); - { - network.checkMaintainedConnectionPeers(); - final CompletableFuture connection; - connection = network.pendingConnections.get(peer); - assertThat(connection).isNotNull(); - network.checkMaintainedConnectionPeers(); - assertThat(network.pendingConnections).contains(new SimpleEntry<>(peer, connection)); - } + network.checkMaintainedConnectionPeers(); + verify(network, times(0)).connect(peer); } @Test diff --git a/ethereum/p2p/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker b/ethereum/p2p/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker deleted file mode 100644 index ca6ee9cea8..0000000000 --- a/ethereum/p2p/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker +++ /dev/null @@ -1 +0,0 @@ -mock-maker-inline \ No newline at end of file From 6915b81c58ba06b49a41eb43c7d941a346617a14 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Tue, 22 Jan 2019 08:36:11 +1000 Subject: [PATCH 7/8] [NC-1970] Adding missing docs --- .../pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java index a6e29db941..6c47575c28 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java @@ -65,8 +65,19 @@ public interface P2PNetwork extends Closeable, Runnable { */ void subscribeDisconnect(DisconnectCallback consumer); + /** + * Adds a {@link Peer} to a list indicating efforts should be made to always stay connected to it + * + * @param peer + * @return boolean representing whether or not the peer has been added to the list or was already + * on it + */ boolean addMaintainConnectionPeer(final Peer peer); + /** + * Trigger that an external clock can use to make the network attempt connections to maintained + * peers + */ void checkMaintainedConnectionPeers(); /** Stops the P2P network layer. */ From 8a46b1c3907c4c55adc9f91e4e27bd34b7ad37d6 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Tue, 22 Jan 2019 08:58:04 +1000 Subject: [PATCH 8/8] [NC-1970] javadoc error --- .../java/tech/pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java index 6c47575c28..6100a1a087 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java @@ -68,7 +68,7 @@ public interface P2PNetwork extends Closeable, Runnable { /** * Adds a {@link Peer} to a list indicating efforts should be made to always stay connected to it * - * @param peer + * @param peer The peer that should be connected to * @return boolean representing whether or not the peer has been added to the list or was already * on it */