[PAN-2595] Consolidate local enode representation (PegaSysEng#1376)

ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNodeList.java

@@ -59,12 +59,6 @@ public TestNode create(
     return node;
   }
 
-  public void startNetworks() {
-    for (final TestNode node : nodes) {
-      node.network.start();
-    }
-  }
-
   public void connectAndAssertAll()
       throws InterruptedException, ExecutionException, TimeoutException {
     for (int i = 0; i < nodes.size(); i++) {

ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPoolPropagationTest.java

@@ -47,7 +47,6 @@ public void tearDown() {
 
   /** Helper to do common setup tasks. */
   private void initTest(final TestNodeList txNodes) throws Exception {
-    txNodes.startNetworks();
     txNodes.connectAndAssertAll();
     txNodes.logPeerConnections();
     txNodes.assertPeerCounts();

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

@@ -13,12 +13,8 @@
 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.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.ConnectingToLocalNodeException;
-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;
@@ -42,17 +38,10 @@ public String getName() {
 
   @Override
   protected JsonRpcResponse performOperation(final Object id, final String enode) {
-    try {
-      LOG.debug("Adding ({}) to peers", enode);
-      final EnodeURL enodeURL = EnodeURL.fromString(enode);
-      final Peer peer = DefaultPeer.fromEnodeURL(enodeURL);
-      boolean addedToNetwork = peerNetwork.addMaintainConnectionPeer(peer);
-      return new JsonRpcSuccessResponse(id, addedToNetwork);
-    } catch (final PeerNotPermittedException e) {
-      return new JsonRpcErrorResponse(
-          id, JsonRpcError.NON_PERMITTED_NODE_CANNOT_BE_ADDED_AS_A_PEER);
-    } catch (final ConnectingToLocalNodeException e) {
-      return new JsonRpcErrorResponse(id, JsonRpcError.CANT_CONNECT_TO_LOCAL_PEER);
-    }
+    LOG.debug("Adding ({}) to peers", enode);
+    final EnodeURL enodeURL = EnodeURL.fromString(enode);
+    final Peer peer = DefaultPeer.fromEnodeURL(enodeURL);
+    boolean addedToNetwork = peerNetwork.addMaintainConnectionPeer(peer);
+    return new JsonRpcSuccessResponse(id, addedToNetwork);
   }
 }

ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminAddPeerTest.java

@@ -22,9 +22,7 @@
 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.ConnectingToLocalNodeException;
 import tech.pegasys.pantheon.ethereum.p2p.P2pDisabledException;
-import tech.pegasys.pantheon.ethereum.p2p.PeerNotPermittedException;
 import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork;
 
 import org.junit.Before;
@@ -151,31 +149,4 @@ public void requestReturnsErrorWhenP2pDisabled() {
 
     assertThat(actualResponse).isEqualToComparingFieldByField(expectedResponse);
   }
-
-  @Test
-  public void requestReturnsErrorWhenPeerNotWhitelisted() {
-    when(p2pNetwork.addMaintainConnectionPeer(any())).thenThrow(new PeerNotPermittedException());
-
-    final JsonRpcResponse expectedResponse =
-        new JsonRpcErrorResponse(
-            validRequest.getId(), JsonRpcError.NON_PERMITTED_NODE_CANNOT_BE_ADDED_AS_A_PEER);
-
-    final JsonRpcResponse actualResponse = method.response(validRequest);
-
-    assertThat(actualResponse).isEqualToComparingFieldByField(expectedResponse);
-  }
-
-  @Test
-  public void
-      p2pNetworkThrowsConnectingToLocalNodeExceptionReturnsCantConnectToLocalNodeJsonError() {
-    when(p2pNetwork.addMaintainConnectionPeer(any()))
-        .thenThrow(new ConnectingToLocalNodeException());
-
-    final JsonRpcResponse expectedResponse =
-        new JsonRpcErrorResponse(validRequest.getId(), JsonRpcError.CANT_CONNECT_TO_LOCAL_PEER);
-
-    final JsonRpcResponse actualResponse = method.response(validRequest);
-
-    assertThat(actualResponse).isEqualToComparingFieldByField(expectedResponse);
-  }
 }

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

@@ -21,13 +21,14 @@
 
 import java.util.Collection;
 import java.util.Optional;
+import java.util.function.Supplier;
 
 /**
  * A permissioning provider that only provides an answer when we have no peers outside of our
  * bootnodes
  */
 public class InsufficientPeersPermissioningProvider implements ContextualNodePermissioningProvider {
-  private final EnodeURL selfEnode;
+  private final Supplier<Optional<EnodeURL>> selfEnode;
   private final P2PNetwork p2pNetwork;
   private final Collection<EnodeURL> bootnodeEnodes;
   private long nonBootnodePeerConnections;
@@ -37,12 +38,13 @@ public class InsufficientPeersPermissioningProvider implements ContextualNodePer
    * Creates the provider observing the provided p2p network
    *
    * @param p2pNetwork the p2p network to observe
-   * @param selfEnode the advertised enode address of this node
+   * @param selfEnode A supplier that provides a representation of the locally running node, if
+   *     available
    * @param bootnodeEnodes the bootnodes that this node is configured to connection to
    */
   public InsufficientPeersPermissioningProvider(
       final P2PNetwork p2pNetwork,
-      final EnodeURL selfEnode,
+      final Supplier<Optional<EnodeURL>> selfEnode,
       final Collection<EnodeURL> bootnodeEnodes) {
     this.selfEnode = selfEnode;
     this.p2pNetwork = p2pNetwork;
@@ -64,17 +66,23 @@ private long countP2PNetworkNonBootnodeConnections() {
   @Override
   public Optional<Boolean> isPermitted(
       final EnodeURL sourceEnode, final EnodeURL destinationEnode) {
+    Optional<EnodeURL> maybeSelfEnode = selfEnode.get();
     if (nonBootnodePeerConnections > 0) {
       return Optional.empty();
-    } else if (checkEnode(sourceEnode) && checkEnode(destinationEnode)) {
+    } else if (!maybeSelfEnode.isPresent()) {
+      // The local node is not yet ready, so we can't validate enodes yet
+      return Optional.empty();
+    } else if (checkEnode(maybeSelfEnode.get(), sourceEnode)
+        && checkEnode(maybeSelfEnode.get(), destinationEnode)) {
       return Optional.of(true);
     } else {
       return Optional.empty();
     }
   }
 
-  private boolean checkEnode(final EnodeURL enode) {
-    return (enode.sameEndpoint(selfEnode) || bootnodeEnodes.stream().anyMatch(enode::sameEndpoint));
+  private boolean checkEnode(final EnodeURL localEnode, final EnodeURL enode) {
+    return (enode.sameEndpoint(localEnode)
+        || bootnodeEnodes.stream().anyMatch(enode::sameEndpoint));
   }
 
   private void handleConnect(final PeerConnection peerConnection) {

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

@@ -77,21 +77,23 @@ public interface P2PNetwork extends Closeable {
   void subscribeDisconnect(DisconnectCallback consumer);
 
   /**
-   * Adds a {@link Peer} to a list indicating efforts should be made to always stay connected to it
+   * Adds a {@link Peer} to a list indicating efforts should be made to always stay connected
+   * regardless of maxPeer limits. Non-permitted peers may be added to this list, but will not
+   * actually be connected to as long as they are prohibited.
    *
    * @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
+   * @return boolean representing whether or not the peer has been added to the list, false is
+   *     returned if the peer was already on the list
    */
   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
+   * Disconnect and remove the given {@link Peer} from the maintained peer list. Peer is
+   * disconnected even if it is not in the maintained peer list. See {@link
+   * #addMaintainConnectionPeer(Peer)} for details on the maintained peer list.
    *
    * @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.
+   * @return boolean representing whether the peer was removed from the maintained peer list
    */
   boolean removeMaintainedConnectionPeer(final Peer peer);