Remove Peer json RPC added

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

@@ -23,6 +23,7 @@
 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 @@ blockchainQueries, new TransactionTracer(blockReplay), parameter),
       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 AdminPeerModification {
+
   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 String getName() {
   }
 
   @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/AdminPeerModification.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 AdminPeerModification implements JsonRpcMethod {
+
+  protected final JsonRpcParameter parameters;
+  protected final P2PNetwork peerNetwork;
+  private static final Logger LOG = LogManager.getLogger();
+
+  public AdminPeerModification(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 AdminPeerModification {
+
+  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 boolean addMaintainConnectionPeer(final Peer peer) {
       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 boolean addMaintainConnectionPeer(final Peer peer) {
     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 boolean addMaintainConnectionPeer(final Peer peer) {
     }
   }
 
+  @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.util.bytes.BytesValue;
 
 import java.util.Collection;
+import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
@@ -66,7 +67,11 @@ public int size() {
   }
 
   public boolean isAlreadyConnected(final BytesValue nodeId) {
-    return connections.containsKey(nodeId);
+    return getConnectionForPeer(nodeId).isPresent();
+  }
+
+  public Optional<PeerConnection> getConnectionForPeer(final BytesValue nodeID) {
+    return Optional.ofNullable(connections.get(nodeID));
   }
 
   @Override

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

@@ -753,6 +753,51 @@ public void onBlockAddedAndPeerNotPermittedShouldDisconnect() {
     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);
   }