[PAN-2560] Consolidate p2p node info methods

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

@@ -138,7 +138,6 @@ public TestNode(
             .metricsSystem(new NoOpMetricsSystem())
             .build();
     network = networkRunner.getNetwork();
-    this.port = network.getLocalPeerInfo().getPort();
     network.subscribeDisconnect(
         (connection, reason, initiatedByPeer) -> disconnections.put(connection, reason));
 
@@ -157,7 +156,7 @@ public TestNode(
             metricsSystem,
             syncState);
     networkRunner.start();
-
+    this.port = network.getLocalEnode().get().getListeningPort();
     selfPeer = new DefaultPeer(id(), endpoint());
   }
 

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

@@ -455,7 +455,7 @@ private JsonRpcResponse process(final JsonObject requestJson, final Optional<Use
         LOG.debug(e);
         return errorResponse(id, JsonRpcError.INVALID_PARAMS);
       } catch (final RuntimeException e) {
-        LOG.debug(e);
+        LOG.error(e);
         return errorResponse(id, JsonRpcError.INTERNAL_ERROR);
       }
     } else {

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

@@ -21,14 +21,10 @@
 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;
@@ -49,9 +45,6 @@ public JsonRpcResponse response(final JsonRpcRequest req) {
       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;
     }
   }
 

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

@@ -20,22 +20,18 @@
 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.api.P2PNetwork;
-import tech.pegasys.pantheon.ethereum.p2p.wire.PeerInfo;
 import tech.pegasys.pantheon.util.bytes.BytesValue;
+import tech.pegasys.pantheon.util.enode.EnodeURL;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
 
 import com.google.common.collect.ImmutableMap;
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
 
 public class AdminNodeInfo implements JsonRpcMethod {
 
-  private static final Logger LOG = LogManager.getLogger();
-
   private final String clientVersion;
   private final int networkId;
   private final GenesisConfigOptions genesisConfigOptions;
@@ -62,53 +58,46 @@ public String getName() {
 
   @Override
   public JsonRpcResponse response(final JsonRpcRequest req) {
+    final Map<String, Object> response = new HashMap<>();
+    final Map<String, Integer> ports = new HashMap<>();
 
-    try {
-      final Map<String, Object> response = new HashMap<>();
-      final Map<String, Integer> ports = new HashMap<>();
+    if (!peerNetwork.isP2pEnabled()) {
+      return new JsonRpcErrorResponse(req.getId(), JsonRpcError.P2P_DISABLED);
+    }
+    final Optional<EnodeURL> maybeEnode = peerNetwork.getLocalEnode();
+    if (!maybeEnode.isPresent()) {
+      return new JsonRpcErrorResponse(req.getId(), JsonRpcError.P2P_NETWORK_NOT_RUNNING);
+    }
+    final EnodeURL enode = maybeEnode.get();
 
-      final PeerInfo peerInfo = peerNetwork.getLocalPeerInfo();
-      final BytesValue nodeId = peerInfo.getNodeId();
-      peerNetwork
-          .getAdvertisedPeer()
-          .ifPresent(
-              advertisedPeer -> {
-                response.put("enode", advertisedPeer.getEnodeURLString());
-                ports.put("discovery", advertisedPeer.getEndpoint().getUdpPort());
-                response.put("ip", advertisedPeer.getEndpoint().getHost());
-                response.put(
-                    "listenAddr",
-                    advertisedPeer.getEndpoint().getHost() + ":" + peerInfo.getPort());
-              });
-      response.put("id", nodeId.toString().substring(2));
-      response.put("name", clientVersion);
+    final BytesValue nodeId = enode.getNodeId();
+    response.put("enode", enode.toString());
+    ports.put("discovery", enode.getEffectiveDiscoveryPort());
+    response.put("ip", enode.getIp());
+    response.put("listenAddr", enode.getIp() + ":" + enode.getListeningPort());
+    response.put("id", nodeId.toString().substring(2));
+    response.put("name", clientVersion);
 
-      ports.put("listener", peerInfo.getPort());
-      response.put("ports", ports);
+    ports.put("listener", enode.getListeningPort());
+    response.put("ports", ports);
 
-      final ChainHead chainHead = blockchainQueries.getBlockchain().getChainHead();
-      response.put(
-          "protocols",
-          ImmutableMap.of(
-              "eth",
-              ImmutableMap.of(
-                  "config",
-                  genesisConfigOptions.asMap(),
-                  "difficulty",
-                  chainHead.getTotalDifficulty().toLong(),
-                  "genesis",
-                  blockchainQueries.getBlockHashByNumber(0).get().toString(),
-                  "head",
-                  chainHead.getHash().toString(),
-                  "network",
-                  networkId)));
+    final ChainHead chainHead = blockchainQueries.getBlockchain().getChainHead();
+    response.put(
+        "protocols",
+        ImmutableMap.of(
+            "eth",
+            ImmutableMap.of(
+                "config",
+                genesisConfigOptions.asMap(),
+                "difficulty",
+                chainHead.getTotalDifficulty().toLong(),
+                "genesis",
+                blockchainQueries.getBlockHashByNumber(0).get().toString(),
+                "head",
+                chainHead.getHash().toString(),
+                "network",
+                networkId)));
 
-      return new JsonRpcSuccessResponse(req.getId(), response);
-    } catch (final P2pDisabledException e) {
-      return new JsonRpcErrorResponse(req.getId(), JsonRpcError.P2P_DISABLED);
-    } catch (final Exception e) {
-      LOG.error("Error processing request: " + req, e);
-      throw e;
-    }
+    return new JsonRpcSuccessResponse(req.getId(), response);
   }
 }

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

@@ -24,11 +24,7 @@
 import java.util.List;
 import java.util.stream.Collectors;
 
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-
 public class AdminPeers implements JsonRpcMethod {
-  private static final Logger LOG = LogManager.getLogger();
   private final P2PNetwork peerDiscoveryAgent;
 
   public AdminPeers(final P2PNetwork peerDiscoveryAgent) {
@@ -50,9 +46,6 @@ public JsonRpcResponse response(final JsonRpcRequest req) {
       return result;
     } catch (P2pDisabledException e) {
       return new JsonRpcErrorResponse(req.getId(), JsonRpcError.P2P_DISABLED);
-    } catch (final Exception e) {
-      LOG.error("Error processing request: " + req, e);
-      throw e;
     }
   }
 }

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

@@ -41,7 +41,7 @@ public JsonRpcResponse response(final JsonRpcRequest req) {
       return p2pDisabledResponse(req);
     }
 
-    final Optional<EnodeURL> enodeURL = p2pNetwork.getSelfEnodeURL();
+    final Optional<EnodeURL> enodeURL = p2pNetwork.getLocalEnode();
     if (!enodeURL.isPresent()) {
       return enodeUrlNotAvailable(req);
     }
@@ -54,6 +54,6 @@ private JsonRpcErrorResponse p2pDisabledResponse(final JsonRpcRequest req) {
   }
 
   private JsonRpcErrorResponse enodeUrlNotAvailable(final JsonRpcRequest req) {
-    return new JsonRpcErrorResponse(req.getId(), JsonRpcError.ENODE_NOT_AVAILABLE);
+    return new JsonRpcErrorResponse(req.getId(), JsonRpcError.P2P_NETWORK_NOT_RUNNING);
   }
 }

ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/response/JsonRpcError.java

@@ -28,7 +28,7 @@ public enum JsonRpcError {
 
   // P2P related errors
   P2P_DISABLED(-32000, "P2P has been disabled. This functionality is not available"),
-  ENODE_NOT_AVAILABLE(-32000, "Enode URL not available"),
+  P2P_NETWORK_NOT_RUNNING(-32000, "P2P network is not running"),
 
   // Filter & Subscription Errors
   FILTER_NOT_FOUND(-32000, "Filter not found"),

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

@@ -14,7 +14,6 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.anyLong;
-import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.when;
 
 import tech.pegasys.pantheon.config.GenesisConfigOptions;
@@ -24,14 +23,15 @@
 import tech.pegasys.pantheon.ethereum.core.Hash;
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.JsonRpcRequest;
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.queries.BlockchainQueries;
+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.wire.PeerInfo;
 import tech.pegasys.pantheon.util.bytes.BytesValue;
 import tech.pegasys.pantheon.util.uint.UInt256;
 
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
@@ -55,16 +55,13 @@ public class AdminNodeInfoTest {
   private final BytesValue nodeId =
       BytesValue.fromHexString(
           "0x0f1b319e32017c3fcb221841f0f978701b4e9513fe6a567a2db43d43381a9c7e3dfe7cae13cbc2f56943400bacaf9082576ab087cd51983b17d729ae796f6807");
-  private final PeerInfo localPeer = new PeerInfo(5, "0x0", Collections.emptyList(), 30303, nodeId);
   private final ChainHead testChainHead = new ChainHead(Hash.EMPTY, UInt256.ONE);
   private final GenesisConfigOptions genesisConfigOptions =
       new StubGenesisConfigOptions().chainId(2019);
   private final DefaultPeer defaultPeer = new DefaultPeer(nodeId, "1.2.3.4", 7890, 30303);
 
   @Before
   public void setup() {
-    when(p2pNetwork.getLocalPeerInfo()).thenReturn(localPeer);
-    doReturn(Optional.of(this.defaultPeer)).when(p2pNetwork).getAdvertisedPeer();
     when(blockchainQueries.getBlockchain()).thenReturn(blockchain);
     when(blockchainQueries.getBlockHashByNumber(anyLong())).thenReturn(Optional.of(Hash.EMPTY));
     when(blockchain.getChainHead()).thenReturn(testChainHead);
@@ -76,9 +73,10 @@ public void setup() {
 
   @Test
   public void shouldReturnCorrectResult() {
+    when(p2pNetwork.isP2pEnabled()).thenReturn(true);
+    when(p2pNetwork.getLocalEnode()).thenReturn(Optional.of(defaultPeer.getEnodeURL()));
     final JsonRpcRequest request = adminNodeInfo();
 
-    final JsonRpcSuccessResponse actual = (JsonRpcSuccessResponse) method.response(request);
     final Map<String, Object> expected = new HashMap<>();
     expected.put(
         "enode",
@@ -106,9 +104,39 @@ public void shouldReturnCorrectResult() {
                 "network",
                 2018)));
 
+    final JsonRpcResponse response = method.response(request);
+    assertThat(response).isInstanceOf(JsonRpcSuccessResponse.class);
+    final JsonRpcSuccessResponse actual = (JsonRpcSuccessResponse) response;
     assertThat(actual.getResult()).isEqualTo(expected);
   }
 
+  @Test
+  public void returnsErrorWhenP2PDisabled() {
+    when(p2pNetwork.isP2pEnabled()).thenReturn(false);
+    final JsonRpcRequest request = adminNodeInfo();
+
+    final JsonRpcResponse expectedResponse =
+        new JsonRpcErrorResponse(request.getId(), JsonRpcError.P2P_DISABLED);
+
+    final JsonRpcResponse response = method.response(request);
+    assertThat(response).isInstanceOf(JsonRpcErrorResponse.class);
+    assertThat(response).isEqualToComparingFieldByField(expectedResponse);
+  }
+
+  @Test
+  public void returnsErrorWhenP2PNotReady() {
+    when(p2pNetwork.isP2pEnabled()).thenReturn(true);
+    when(p2pNetwork.getLocalEnode()).thenReturn(Optional.empty());
+    final JsonRpcRequest request = adminNodeInfo();
+
+    final JsonRpcResponse expectedResponse =
+        new JsonRpcErrorResponse(request.getId(), JsonRpcError.P2P_NETWORK_NOT_RUNNING);
+
+    final JsonRpcResponse response = method.response(request);
+    assertThat(response).isInstanceOf(JsonRpcErrorResponse.class);
+    assertThat(response).isEqualToComparingFieldByField(expectedResponse);
+  }
+
   private JsonRpcRequest adminNodeInfo() {
     return new JsonRpcRequest("2.0", "admin_nodeInfo", new Object[] {});
   }

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

@@ -61,7 +61,7 @@ public void shouldReturnExpectedMethodName() {
   @Test
   public void shouldReturnEnode() {
     when(p2PNetwork.isP2pEnabled()).thenReturn(true);
-    doReturn(enodeURL).when(p2PNetwork).getSelfEnodeURL();
+    doReturn(enodeURL).when(p2PNetwork).getLocalEnode();
 
     final JsonRpcRequest request = netEnodeRequest();
     final JsonRpcResponse expectedResponse =
@@ -84,11 +84,11 @@ public void shouldReturnErrorWhenP2pDisabled() {
   @Test
   public void shouldReturnErrorWhenP2PEnabledButNoEnodeFound() {
     when(p2PNetwork.isP2pEnabled()).thenReturn(true);
-    doReturn(Optional.empty()).when(p2PNetwork).getSelfEnodeURL();
+    doReturn(Optional.empty()).when(p2PNetwork).getLocalEnode();
 
     final JsonRpcRequest request = netEnodeRequest();
     final JsonRpcResponse expectedResponse =
-        new JsonRpcErrorResponse(request.getId(), JsonRpcError.ENODE_NOT_AVAILABLE);
+        new JsonRpcErrorResponse(request.getId(), JsonRpcError.P2P_NETWORK_NOT_RUNNING);
 
     assertThat(method.response(request)).isEqualToComparingFieldByField(expectedResponse);
   }

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

@@ -17,7 +17,6 @@
 import tech.pegasys.pantheon.ethereum.p2p.api.MessageData;
 import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork;
 import tech.pegasys.pantheon.ethereum.p2p.api.PeerConnection;
-import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer;
 import tech.pegasys.pantheon.ethereum.p2p.peers.Peer;
 import tech.pegasys.pantheon.ethereum.p2p.wire.Capability;
 import tech.pegasys.pantheon.ethereum.p2p.wire.DefaultMessage;
@@ -100,7 +99,7 @@ private void disconnect(
     }
   }
 
-  private final class MockP2PNetwork implements P2PNetwork {
+  private static final class MockP2PNetwork implements P2PNetwork {
 
     private final MockNetwork network;
 
@@ -178,35 +177,29 @@ public void stop() {}
     @Override
     public void awaitStop() {}
 
-    @Override
-    public Optional<Peer> getAdvertisedPeer() {
-      return Optional.of(new DefaultPeer(self.getId(), "127.0.0.1", 0, 0));
-    }
-
     @Override
     public void start() {}
 
     @Override
     public void close() {}
 
     @Override
-    public PeerInfo getLocalPeerInfo() {
-      return new PeerInfo(
-          5, self.getId().toString(), new ArrayList<>(capabilities), 0, self.getId());
+    public boolean isListening() {
+      return true;
     }
 
     @Override
-    public boolean isListening() {
+    public boolean isP2pEnabled() {
       return true;
     }
 
     @Override
-    public boolean isP2pEnabled() {
+    public boolean isDiscoveryEnabled() {
       return true;
     }
 
     @Override
-    public Optional<EnodeURL> getSelfEnodeURL() {
+    public Optional<EnodeURL> getLocalEnode() {
       return Optional.empty();
     }
   }