Remove Peer json RPC added (#1129)

When RemovePeer is invoked, the specified peer is removed from the "Maintained" connections in NettyP2p, and the connection to said peer is disconnected.

The peer is not added to the blacklist, on the premise that this behaviour will typically be used when peer-discovery is disabled, thus no future connection to the peer in question will be attempted (except via an AddPeer JSON RPC call).

Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>

ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcMethodsFactory.java

@@ -23,6 +23,7 @@ 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.AdminNodeInfo;
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.AdminPeers;
+import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.AdminRemovePeer;
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.DebugMetrics;
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.DebugStorageRangeAt;
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.methods.DebugTraceTransaction;
@@ -273,6 +274,7 @@ public class JsonRpcMethodsFactory {
       addMethods(
           enabledMethods,
           new AdminAddPeer(p2pNetwork, parameter),
+          new AdminRemovePeer(p2pNetwork, parameter),
           new AdminNodeInfo(
               clientVersion, networkId, genesisConfigOptions, p2pNetwork, blockchainQueries),
           new AdminPeers(p2pNetwork));

ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeer.java

@@ -12,30 +12,26 @@
  */
 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.P2pDisabledException;
 import tech.pegasys.pantheon.ethereum.p2p.PeerNotPermittedException;
 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 tech.pegasys.pantheon.util.enode.EnodeURL;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
-public class AdminAddPeer implements JsonRpcMethod {
+public class AdminAddPeer extends AdminModifyPeer {
+
   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;
+    super(peerNetwork, parameters);
   }
 
   @Override
@@ -44,27 +40,16 @@ public class AdminAddPeer implements JsonRpcMethod {
   }
 
   @Override
-  public JsonRpcResponse response(final JsonRpcRequest req) {
-    if (req.getParamLength() != 1) {
-      return new JsonRpcErrorResponse(req.getId(), JsonRpcError.INVALID_PARAMS);
-    }
+  protected JsonRpcResponse performOperation(final Object id, final String enode) {
     try {
-      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 P2pDisabledException e) {
-      return new JsonRpcErrorResponse(req.getId(), JsonRpcError.P2P_DISABLED);
+      LOG.debug("Adding ({}) to peers", enode);
+      final EnodeURL enodeURL = new EnodeURL(enode);
+      final Peer peer = DefaultPeer.fromEnodeURL(enodeURL);
+      boolean addedToNetwork = peerNetwork.addMaintainConnectionPeer(peer);
+      return new JsonRpcSuccessResponse(id, addedToNetwork);
     } catch (final PeerNotPermittedException e) {
       return new JsonRpcErrorResponse(
-          req.getId(), JsonRpcError.NON_PERMITTED_NODE_CANNOT_BE_ADDED_AS_A_PEER);
-    } catch (final Exception e) {
-      LOG.error("Error processing request: " + req, e);
-      throw e;
+          id, JsonRpcError.NON_PERMITTED_NODE_CANNOT_BE_ADDED_AS_A_PEER);
     }
   }
 }

ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminModifyPeer.java

@@ -0,0 +1,59 @@
+/*
+ * 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 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.p2p.P2pDisabledException;
+import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+public abstract class AdminModifyPeer implements JsonRpcMethod {
+
+  protected final JsonRpcParameter parameters;
+  protected final P2PNetwork peerNetwork;
+  private static final Logger LOG = LogManager.getLogger();
+
+  public AdminModifyPeer(final P2PNetwork peerNetwork, final JsonRpcParameter parameters) {
+    this.peerNetwork = peerNetwork;
+    this.parameters = parameters;
+  }
+
+  @Override
+  public JsonRpcResponse response(final JsonRpcRequest req) {
+    if (req.getParamLength() != 1) {
+      return new JsonRpcErrorResponse(req.getId(), JsonRpcError.INVALID_PARAMS);
+    }
+    try {
+      final String enodeString = parameters.required(req.getParams(), 0, String.class);
+      return performOperation(req.getId(), enodeString);
+    } 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 P2pDisabledException e) {
+      return new JsonRpcErrorResponse(req.getId(), JsonRpcError.P2P_DISABLED);
+    } catch (final Exception e) {
+      LOG.error(getName() + " - Error processing request: " + req, e);
+      throw e;
+    }
+  }
+
+  protected abstract JsonRpcResponse performOperation(final Object id, final String enode);
+}

ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminRemovePeer.java

@@ -0,0 +1,46 @@
+/*
+ * 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 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.p2p.api.P2PNetwork;
+import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer;
+import tech.pegasys.pantheon.util.enode.EnodeURL;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+public class AdminRemovePeer extends AdminModifyPeer {
+
+  private static final Logger LOG = LogManager.getLogger();
+
+  public AdminRemovePeer(final P2PNetwork peerNetwork, final JsonRpcParameter parameters) {
+    super(peerNetwork, parameters);
+  }
+
+  @Override
+  public String getName() {
+    return "admin_removePeer";
+  }
+
+  @Override
+  protected JsonRpcResponse performOperation(final Object id, final String enode) {
+    LOG.debug("Remove ({}) to peer cache", enode);
+    final EnodeURL enodeURL = new EnodeURL(enode);
+    final boolean result =
+        peerNetwork.removeMaintainedConnectionPeer(DefaultPeer.fromEnodeURL(enodeURL));
+    return new JsonRpcSuccessResponse(id, result);
+  }
+}

ethereum/mock-p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/testing/MockNetwork.java

@@ -167,6 +167,11 @@ public final class MockNetwork {
       return true;
     }
 
+    @Override
+    public boolean removeMaintainedConnectionPeer(final Peer peer) {
+      return true;
+    }
+
     @Override
     public void checkMaintainedConnectionPeers() {}
 

ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/NoopP2PNetwork.java

@@ -52,6 +52,11 @@ public class NoopP2PNetwork implements P2PNetwork {
     throw new P2pDisabledException("P2P networking disabled.  Unable to connect to add peer.");
   }
 
+  @Override
+  public boolean removeMaintainedConnectionPeer(final Peer peer) {
+    throw new P2pDisabledException("P2P networking disabled.  Unable to remove a connected peer.");
+  }
+
   @Override
   public void checkMaintainedConnectionPeers() {}
 

ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/api/P2PNetwork.java

@@ -76,6 +76,16 @@ public interface P2PNetwork extends Closeable {
    */
   boolean addMaintainConnectionPeer(final Peer peer);
 
+  /**
+   * Removes a {@link Peer} from a list indicating any existing efforts to connect to a given peer
+   * should be removed, and if connected, the peer should be disconnected
+   *
+   * @param peer The peer to which connections are not longer required
+   * @return boolean representing whether or not the peer has been disconnected, or if it was not
+   *     currently connected.
+   */
+  boolean removeMaintainedConnectionPeer(final Peer peer);
+
   /**
    * Trigger that an external clock can use to make the network attempt connections to maintained
    * peers

ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/NettyP2PNetwork.java

@@ -378,6 +378,21 @@ public class NettyP2PNetwork implements P2PNetwork {
     }
   }
 
+  @Override
+  public boolean removeMaintainedConnectionPeer(final Peer peer) {
+    final boolean removed = peerMaintainConnectionList.remove(peer);
+
+    final CompletableFuture<PeerConnection> connectionFuture = pendingConnections.get(peer);
+    if (connectionFuture != null) {
+      connectionFuture.thenAccept(connection -> connection.disconnect(DisconnectReason.REQUESTED));
+    }
+
+    final Optional<PeerConnection> peerConnection = connections.getConnectionForPeer(peer.getId());
+    peerConnection.ifPresent(pc -> pc.disconnect(DisconnectReason.REQUESTED));
+
+    return removed;
+  }
+
   @Override
   public void checkMaintainedConnectionPeers() {
     for (final Peer peer : peerMaintainConnectionList) {

ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/netty/PeerConnectionRegistry.java

@@ -24,6 +24,7 @@ import tech.pegasys.pantheon.metrics.MetricsSystem;
 import tech.pegasys.pantheon.util.bytes.BytesValue;
 
 import java.util.Collection;
+import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
@@ -69,6 +70,10 @@ public class PeerConnectionRegistry implements DisconnectCallback {
     return connections.containsKey(nodeId);
   }
 
+  public Optional<PeerConnection> getConnectionForPeer(final BytesValue nodeID) {
+    return Optional.ofNullable(connections.get(nodeID));
+  }
+
   @Override
   public void onDisconnect(
       final PeerConnection connection,

ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/NettyP2PNetworkTest.java

@@ -753,6 +753,51 @@ public final class NettyP2PNetworkTest {
     verify(permittedPeerConnection, never()).disconnect(any());
   }
 
+  @Test
+  public void removePeerReturnsTrueIfNodeWasInMaintaineConnectionsAndDisconnectsIfInPending() {
+    final NettyP2PNetwork nettyP2PNetwork = mockNettyP2PNetwork();
+    nettyP2PNetwork.start();
+
+    final Peer localPeer = mockPeer("127.0.0.1", 30301);
+    final Peer remotePeer = mockPeer("127.0.0.2", 30302);
+    final PeerConnection peerConnection = mockPeerConnection(localPeer, remotePeer);
+
+    nettyP2PNetwork.addMaintainConnectionPeer(remotePeer);
+    assertThat(nettyP2PNetwork.peerMaintainConnectionList.contains(remotePeer)).isTrue();
+    assertThat(nettyP2PNetwork.pendingConnections.containsKey(remotePeer)).isTrue();
+    assertThat(nettyP2PNetwork.removeMaintainedConnectionPeer(remotePeer)).isTrue();
+    assertThat(nettyP2PNetwork.peerMaintainConnectionList.contains(remotePeer)).isFalse();
+
+    // Note: The pendingConnection future is not removed.
+    assertThat(nettyP2PNetwork.pendingConnections.containsKey(remotePeer)).isTrue();
+
+    // Complete the connection, and ensure "disconnect is automatically called.
+    nettyP2PNetwork.pendingConnections.get(remotePeer).complete(peerConnection);
+    verify(peerConnection).disconnect(DisconnectReason.REQUESTED);
+  }
+
+  @Test
+  public void removePeerReturnsFalseIfNotInMaintainedListButDisconnectsPeer() {
+    final NettyP2PNetwork nettyP2PNetwork = mockNettyP2PNetwork();
+    nettyP2PNetwork.start();
+
+    final Peer localPeer = mockPeer("127.0.0.1", 30301);
+    final Peer remotePeer = mockPeer("127.0.0.2", 30302);
+    final PeerConnection peerConnection = mockPeerConnection(localPeer, remotePeer);
+
+    CompletableFuture<PeerConnection> future = nettyP2PNetwork.connect(remotePeer);
+
+    assertThat(nettyP2PNetwork.peerMaintainConnectionList.contains(remotePeer)).isFalse();
+    assertThat(nettyP2PNetwork.pendingConnections.containsKey(remotePeer)).isTrue();
+    future.complete(peerConnection);
+    assertThat(nettyP2PNetwork.pendingConnections.containsKey(remotePeer)).isFalse();
+
+    assertThat(nettyP2PNetwork.removeMaintainedConnectionPeer(remotePeer)).isFalse();
+    assertThat(nettyP2PNetwork.peerMaintainConnectionList.contains(remotePeer)).isFalse();
+
+    verify(peerConnection).disconnect(DisconnectReason.REQUESTED);
+  }
+
   private BlockAddedEvent blockAddedEvent() {
     return mock(BlockAddedEvent.class);
   }