diff --git a/acceptance-tests/build.gradle b/acceptance-tests/build.gradle index 3fe6f33d83..b2439b57f7 100644 --- a/acceptance-tests/build.gradle +++ b/acceptance-tests/build.gradle @@ -26,6 +26,7 @@ dependencies { testImplementation project(':ethereum:blockcreation') testImplementation project(':ethereum:jsonrpc') testImplementation project(':ethereum:permissioning') + testImplementation project(':ethereum:graphqlrpc') testImplementation project(':ethereum:rlp') testImplementation project(':metrics:core') testImplementation project(':pantheon') diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java index a5d45ba5ea..4a99510f3f 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java @@ -23,9 +23,11 @@ import tech.pegasys.pantheon.controller.PantheonControllerBuilder; import tech.pegasys.pantheon.ethereum.eth.EthereumWireProtocolConfiguration; import tech.pegasys.pantheon.ethereum.eth.sync.SynchronizerConfiguration; import tech.pegasys.pantheon.ethereum.eth.transactions.PendingTransactions; +import tech.pegasys.pantheon.ethereum.graphqlrpc.GraphQLRpcConfiguration; import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; import tech.pegasys.pantheon.services.kvstore.RocksDbConfiguration; +import tech.pegasys.pantheon.util.enode.EnodeURL; import java.io.IOException; import java.nio.file.Path; @@ -33,10 +35,12 @@ import java.time.Clock; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import com.google.common.io.Files; import io.vertx.core.Vertx; @@ -56,9 +60,13 @@ public class ThreadPantheonNodeRunner implements PantheonNodeRunner { } final MetricsSystem noOpMetricsSystem = new NoOpMetricsSystem(); + final List bootnodes = + node.getConfiguration().bootnodes().stream() + .map(EnodeURL::fromURI) + .collect(Collectors.toList()); final EthNetworkConfig.Builder networkConfigBuilder = new EthNetworkConfig.Builder(EthNetworkConfig.getNetworkConfig(DEV)) - .setBootNodes(node.getConfiguration().bootnodes()); + .setBootNodes(bootnodes); node.getConfiguration().getGenesisConfig().ifPresent(networkConfigBuilder::setGenesisConfig); final EthNetworkConfig ethNetworkConfig = networkConfigBuilder.build(); final PantheonControllerBuilder builder = @@ -104,6 +112,7 @@ public class ThreadPantheonNodeRunner implements PantheonNodeRunner { .metricsSystem(noOpMetricsSystem) .metricsConfiguration(node.metricsConfiguration()) .p2pEnabled(node.isP2pEnabled()) + .graphQLRpcConfiguration(GraphQLRpcConfiguration.createDefault()) .build(); runner.start(); diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/MockPeerConnection.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/MockPeerConnection.java index 9b352c4186..886165b00b 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/MockPeerConnection.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/MockPeerConnection.java @@ -42,7 +42,11 @@ public class MockPeerConnection implements PeerConnection { this.nodeId = Peer.randomId(); this.peer = DefaultPeer.fromEnodeURL( - EnodeURL.builder().ipAddress("127.0.0.1").nodeId(nodeId).listeningPort(30303).build()); + EnodeURL.builder() + .ipAddress("127.0.0.1") + .nodeId(nodeId) + .discoveryAndListeningPorts(30303) + .build()); this.peerInfo = new PeerInfo(5, "Mock", new ArrayList<>(caps), 30303, nodeId); } diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNode.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNode.java index b4949c63ba..b04fa57225 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNode.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNode.java @@ -200,7 +200,7 @@ public class TestNode implements Closeable { + "@" + selfPeer.getEnodeURL().getIpAsString() + ':' - + selfPeer.getEnodeURL().getListeningPort(); + + selfPeer.getEnodeURL().getListeningPortOrZero(); } public void receiveRemoteTransaction(final Transaction transaction) { diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminNodeInfo.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminNodeInfo.java index 12fd7d476c..924115675d 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminNodeInfo.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminNodeInfo.java @@ -73,13 +73,19 @@ public class AdminNodeInfo implements JsonRpcMethod { final BytesValue nodeId = enode.getNodeId(); response.put("enode", enode.toString()); - ports.put("discovery", enode.getEffectiveDiscoveryPort()); response.put("ip", enode.getIpAsString()); - response.put("listenAddr", enode.getIpAsString() + ":" + enode.getListeningPort()); + if (enode.isListening()) { + response.put("listenAddr", enode.getIpAsString() + ":" + enode.getListeningPort().getAsInt()); + } response.put("id", nodeId.toUnprefixedString()); response.put("name", clientVersion); - ports.put("listener", enode.getListeningPort()); + if (enode.isRunningDiscovery()) { + ports.put("discovery", enode.getDiscoveryPortOrZero()); + } + if (enode.isListening()) { + ports.put("listener", enode.getListeningPort().getAsInt()); + } response.put("ports", ports); final ChainHead chainHead = blockchainQueries.getBlockchain().getChainHead(); diff --git a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/NetServices.java b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/NetServices.java index 6f0dd277ba..d62064fefe 100644 --- a/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/NetServices.java +++ b/ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/NetServices.java @@ -65,11 +65,13 @@ public class NetServices implements JsonRpcMethod { if (p2pNetwork.isP2pEnabled()) { p2pNetwork .getLocalEnode() + .filter(e -> e.isListening()) .ifPresent( enode -> servicesMapBuilder.put( "p2p", - createServiceDetailsMap(enode.getIpAsString(), enode.getListeningPort()))); + createServiceDetailsMap( + enode.getIpAsString(), enode.getListeningPort().getAsInt()))); } if (metricsConfiguration.isEnabled()) { servicesMapBuilder.put( diff --git a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminNodeInfoTest.java b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminNodeInfoTest.java index 47883024de..cb980c32e8 100644 --- a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminNodeInfoTest.java +++ b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminNodeInfoTest.java @@ -34,6 +34,7 @@ import tech.pegasys.pantheon.util.enode.EnodeURL; import tech.pegasys.pantheon.util.uint.UInt256; import java.math.BigInteger; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Optional; @@ -119,6 +120,144 @@ public class AdminNodeInfoTest { assertThat(actual.getResult()).isEqualTo(expected); } + @Test + public void handlesLocalEnodeWithListeningAndDiscoveryDisabled() { + final EnodeURL localEnode = + EnodeURL.builder() + .nodeId(nodeId) + .ipAddress("1.2.3.4") + .discoveryAndListeningPorts(0) + .build(); + + when(p2pNetwork.isP2pEnabled()).thenReturn(true); + when(p2pNetwork.getLocalEnode()).thenReturn(Optional.of(localEnode)); + final JsonRpcRequest request = adminNodeInfo(); + + final Map expected = new HashMap<>(); + expected.put( + "enode", + "enode://0f1b319e32017c3fcb221841f0f978701b4e9513fe6a567a2db43d43381a9c7e3dfe7cae13cbc2f56943400bacaf9082576ab087cd51983b17d729ae796f6807@1.2.3.4:0"); + expected.put( + "id", + "0f1b319e32017c3fcb221841f0f978701b4e9513fe6a567a2db43d43381a9c7e3dfe7cae13cbc2f56943400bacaf9082576ab087cd51983b17d729ae796f6807"); + expected.put("ip", "1.2.3.4"); + expected.put("name", "testnet/1.0/this/that"); + expected.put("ports", Collections.emptyMap()); + expected.put( + "protocols", + ImmutableMap.of( + "eth", + ImmutableMap.of( + "config", + genesisConfigOptions.asMap(), + "difficulty", + 1L, + "genesis", + Hash.EMPTY.toString(), + "head", + Hash.EMPTY.toString(), + "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 handlesLocalEnodeWithListeningDisabled() { + final EnodeURL localEnode = + EnodeURL.builder() + .nodeId(nodeId) + .ipAddress("1.2.3.4") + .discoveryAndListeningPorts(0) + .discoveryPort(7890) + .build(); + + when(p2pNetwork.isP2pEnabled()).thenReturn(true); + when(p2pNetwork.getLocalEnode()).thenReturn(Optional.of(localEnode)); + final JsonRpcRequest request = adminNodeInfo(); + + final Map expected = new HashMap<>(); + expected.put( + "enode", + "enode://0f1b319e32017c3fcb221841f0f978701b4e9513fe6a567a2db43d43381a9c7e3dfe7cae13cbc2f56943400bacaf9082576ab087cd51983b17d729ae796f6807@1.2.3.4:0?discport=7890"); + expected.put( + "id", + "0f1b319e32017c3fcb221841f0f978701b4e9513fe6a567a2db43d43381a9c7e3dfe7cae13cbc2f56943400bacaf9082576ab087cd51983b17d729ae796f6807"); + expected.put("ip", "1.2.3.4"); + expected.put("name", "testnet/1.0/this/that"); + expected.put("ports", ImmutableMap.of("discovery", 7890)); + expected.put( + "protocols", + ImmutableMap.of( + "eth", + ImmutableMap.of( + "config", + genesisConfigOptions.asMap(), + "difficulty", + 1L, + "genesis", + Hash.EMPTY.toString(), + "head", + Hash.EMPTY.toString(), + "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 handlesLocalEnodeWithDiscoveryDisabled() { + final EnodeURL localEnode = + EnodeURL.builder() + .nodeId(nodeId) + .ipAddress("1.2.3.4") + .discoveryAndListeningPorts(0) + .listeningPort(7890) + .build(); + + when(p2pNetwork.isP2pEnabled()).thenReturn(true); + when(p2pNetwork.getLocalEnode()).thenReturn(Optional.of(localEnode)); + final JsonRpcRequest request = adminNodeInfo(); + + final Map expected = new HashMap<>(); + expected.put( + "enode", + "enode://0f1b319e32017c3fcb221841f0f978701b4e9513fe6a567a2db43d43381a9c7e3dfe7cae13cbc2f56943400bacaf9082576ab087cd51983b17d729ae796f6807@1.2.3.4:7890?discport=0"); + expected.put( + "id", + "0f1b319e32017c3fcb221841f0f978701b4e9513fe6a567a2db43d43381a9c7e3dfe7cae13cbc2f56943400bacaf9082576ab087cd51983b17d729ae796f6807"); + expected.put("ip", "1.2.3.4"); + expected.put("listenAddr", "1.2.3.4:7890"); + expected.put("name", "testnet/1.0/this/that"); + expected.put("ports", ImmutableMap.of("listener", 7890)); + expected.put( + "protocols", + ImmutableMap.of( + "eth", + ImmutableMap.of( + "config", + genesisConfigOptions.asMap(), + "difficulty", + 1L, + "genesis", + Hash.EMPTY.toString(), + "head", + Hash.EMPTY.toString(), + "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); 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 a74e913b94..01257092a2 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 @@ -267,7 +267,11 @@ public final class MockNetwork { @Override public PeerInfo getPeerInfo() { return new PeerInfo( - 5, "mock-network-client", capabilities, to.getEnodeURL().getListeningPort(), to.getId()); + 5, + "mock-network-client", + capabilities, + to.getEnodeURL().getListeningPortOrZero(), + to.getId()); } @Override diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/InsufficientPeersPermissioningProvider.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/InsufficientPeersPermissioningProvider.java index 85d736bfcd..d6ea15ff2c 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/InsufficientPeersPermissioningProvider.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/InsufficientPeersPermissioningProvider.java @@ -49,7 +49,9 @@ public class InsufficientPeersPermissioningProvider implements ContextualNodePer private boolean isNotABootnode(final PeerConnection peerConnection) { return bootnodeEnodes.stream() - .noneMatch((bootNode) -> peerConnection.getRemoteEnode().sameEndpoint(bootNode)); + .noneMatch( + (bootNode) -> + EnodeURL.sameListeningEndpoint(peerConnection.getRemoteEnode(), bootNode)); } private long countP2PNetworkNonBootnodeConnections() { @@ -74,8 +76,9 @@ public class InsufficientPeersPermissioningProvider implements ContextualNodePer } private boolean checkEnode(final EnodeURL localEnode, final EnodeURL enode) { - return (enode.sameEndpoint(localEnode) - || bootnodeEnodes.stream().anyMatch(enode::sameEndpoint)); + return (EnodeURL.sameListeningEndpoint(localEnode, enode) + || bootnodeEnodes.stream() + .anyMatch(bootNode -> EnodeURL.sameListeningEndpoint(bootNode, enode))); } private void handleConnect(final PeerConnection peerConnection) { diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/config/DiscoveryConfiguration.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/config/DiscoveryConfiguration.java index 1d96708945..cec4adc9b4 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/config/DiscoveryConfiguration.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/config/DiscoveryConfiguration.java @@ -14,19 +14,17 @@ package tech.pegasys.pantheon.ethereum.p2p.config; import static java.util.stream.Collectors.toList; -import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeer; -import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; +import tech.pegasys.pantheon.util.enode.EnodeURL; -import java.net.URI; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.stream.Collectors; import java.util.stream.Stream; public class DiscoveryConfiguration { - public static List MAINNET_BOOTSTRAP_NODES = + public static List MAINNET_BOOTSTRAP_NODES = Collections.unmodifiableList( Stream.of( "enode://a979fb575495b8d6db44f750317d0f4622bf4c2aa3365d6af7c284339968eef29b69ad0dce72a4d8db5ebb4968de0e3bec910127f134779fbcb0cb6d3331163c@52.16.188.185:30303", @@ -35,27 +33,27 @@ public class DiscoveryConfiguration { "enode://158f8aab45f6d19c6cbf4a089c2670541a8da11978a2f90dbf6a502a4a3bab80d288afdbeb7ec0ef6d92de563767f3b1ea9e8e334ca711e9f8e2df5a0385e8e6@13.75.154.138:30303", "enode://1118980bf48b0a3640bdba04e0fe78b1add18e1cd99bf22d53daac1fd9972ad650df52176e7c7d89d1114cfef2bc23a2959aa54998a46afcf7d91809f0855082@52.74.57.123:30303", "enode://979b7fa28feeb35a4741660a16076f1943202cb72b6af70d327f053e248bab9ba81760f39d0701ef1d8f89cc1fbd2cacba0710a12cd5314d5e0c9021aa3637f9@5.1.83.226:30303") - .map(URI::create) + .map(EnodeURL::fromString) .collect(toList())); - public static List RINKEBY_BOOTSTRAP_NODES = + public static List RINKEBY_BOOTSTRAP_NODES = Collections.unmodifiableList( Stream.of( "enode://a24ac7c5484ef4ed0c5eb2d36620ba4e4aa13b8c84684e1b4aab0cebea2ae45cb4d375b77eab56516d34bfbd3c1a833fc51296ff084b770b94fb9028c4d25ccf@52.169.42.101:30303", "enode://343149e4feefa15d882d9fe4ac7d88f885bd05ebb735e547f12e12080a9fa07c8014ca6fd7f373123488102fe5e34111f8509cf0b7de3f5b44339c9f25e87cb8@52.3.158.184:30303", "enode://b6b28890b006743680c52e64e0d16db57f28124885595fa03a562be1d2bf0f3a1da297d56b13da25fb992888fd556d4c1a27b1f39d531bde7de1921c90061cc6@159.89.28.211:30303") - .map(URI::create) + .map(EnodeURL::fromString) .collect(toList())); - public static List ROPSTEN_BOOTSTRAP_NODES = + public static List ROPSTEN_BOOTSTRAP_NODES = Collections.unmodifiableList( Stream.of( "enode://6332792c4a00e3e4ee0926ed89e0d27ef985424d97b6a45bf0f23e51f0dcb5e66b875777506458aea7af6f9e4ffb69f43f3778ee73c81ed9d34c51c4b16b0b0f@52.232.243.152:30303", "enode://94c15d1b9e2fe7ce56e458b9a3b672ef11894ddedd0c6f247e0f1d3487f52b66208fb4aeb8179fce6e3a749ea93ed147c37976d67af557508d199d9594c35f09@192.81.208.223:30303", "enode://30b7ab30a01c124a6cceca36863ece12c4f5fa68e3ba9b0b51407ccc002eeed3b3102d20a88f1c1d3c3154e2449317b8ef95090e77b312d5cc39354f86d5d606@52.176.7.10:30303", "enode://865a63255b3bb68023b6bffd5095118fcc13e79dcf014fe4e47e065c350c7cc72af2e53eff895f11ba1bbb6a2b33271c1116ee870f266618eadfc2e78aa7349c@52.176.100.77:30303") - .map(URI::create) + .map(EnodeURL::fromString) .collect(toList())); - public static List GOERLI_BOOTSTRAP_NODES = + public static List GOERLI_BOOTSTRAP_NODES = Collections.unmodifiableList( Stream.of( "enode://011f758e6552d105183b1761c5e2dea0111bc20fd5f6422bc7f91e0fabbec9a6595caf6239b37feb773dddd3f87240d99d859431891e4a642cf2a0a9e6cbb98a@51.141.78.53:30303", @@ -63,7 +61,7 @@ public class DiscoveryConfiguration { "enode://46add44b9f13965f7b9875ac6b85f016f341012d84f975377573800a863526f4da19ae2c620ec73d11591fa9510e992ecc03ad0751f53cc02f7c7ed6d55c7291@94.237.54.114:30313", "enode://c1f8b7c2ac4453271fa07d8e9ecf9a2e8285aa0bd0c07df0131f47153306b0736fd3db8924e7a9bf0bed6b1d8d4f87362a71b033dc7c64547728d953e43e59b2@52.64.155.147:30303", "enode://f4a9c6ee28586009fb5a96c8af13a58ed6d8315a9eee4772212c1d4d9cebe5a8b8a78ea4434f318726317d04a3f531a1ef0420cf9752605a562cfe858c46e263@213.186.16.82:30303") - .map(URI::create) + .map(EnodeURL::fromString) .collect(toList())); private boolean active = true; @@ -71,12 +69,25 @@ public class DiscoveryConfiguration { private int bindPort = 30303; private String advertisedHost = "127.0.0.1"; private int bucketSize = 16; - private List bootstrapPeers = new ArrayList<>(); + private List bootnodes = new ArrayList<>(); public static DiscoveryConfiguration create() { return new DiscoveryConfiguration(); } + public static void assertValidBootnodes(final List bootnodes) { + final List invalidEnodes = + bootnodes.stream().filter(e -> !e.isRunningDiscovery()).collect(Collectors.toList()); + + if (invalidEnodes.size() > 0) { + String invalidBootnodes = + invalidEnodes.stream().map(EnodeURL::toString).collect(Collectors.joining(",")); + String errorMsg = + "Bootnodes must have discovery enabled. Invalid bootnodes: " + invalidBootnodes + "."; + throw new IllegalArgumentException(errorMsg); + } + } + public String getBindHost() { return bindHost; } @@ -104,12 +115,13 @@ public class DiscoveryConfiguration { return this; } - public List getBootstrapPeers() { - return bootstrapPeers; + public List getBootnodes() { + return bootnodes; } - public DiscoveryConfiguration setBootstrapPeers(final Collection bootstrapPeers) { - this.bootstrapPeers = bootstrapPeers.stream().map(DefaultPeer::fromURI).collect(toList()); + public DiscoveryConfiguration setBootnodes(final List bootnodes) { + assertValidBootnodes(bootnodes); + this.bootnodes = bootnodes; return this; } @@ -145,12 +157,12 @@ public class DiscoveryConfiguration { && bucketSize == that.bucketSize && Objects.equals(bindHost, that.bindHost) && Objects.equals(advertisedHost, that.advertisedHost) - && Objects.equals(bootstrapPeers, that.bootstrapPeers); + && Objects.equals(bootnodes, that.bootnodes); } @Override public int hashCode() { - return Objects.hash(active, bindHost, bindPort, advertisedHost, bucketSize, bootstrapPeers); + return Objects.hash(active, bindHost, bindPort, advertisedHost, bucketSize, bootnodes); } @Override @@ -168,8 +180,8 @@ public class DiscoveryConfiguration { + '\'' + ", bucketSize=" + bucketSize - + ", bootstrapPeers=" - + bootstrapPeers + + ", bootnodes=" + + bootnodes + '}'; } } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/Endpoint.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/Endpoint.java index 09a512e168..ff8e7389ff 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/Endpoint.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/Endpoint.java @@ -52,11 +52,12 @@ public class Endpoint { } public static Endpoint fromEnode(final EnodeURL enode) { - final OptionalInt tcpPort = - enode.getDiscoveryPort().isPresent() - ? OptionalInt.of(enode.getListeningPort()) - : OptionalInt.empty(); - return new Endpoint(enode.getIp().getHostAddress(), enode.getEffectiveDiscoveryPort(), tcpPort); + checkArgument( + enode.getDiscoveryPort().isPresent(), + "Attempt to create a discovery endpoint for an enode with discovery disabled."); + final int discoveryPort = enode.getDiscoveryPort().getAsInt(); + final OptionalInt listeningPort = enode.getListeningPort(); + return new Endpoint(enode.getIp().getHostAddress(), discoveryPort, listeningPort); } public EnodeURL toEnode(final BytesValue nodeId) { diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java index 9f1fd72083..72e4bfd725 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgent.java @@ -31,7 +31,6 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PeerTable; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PingPacketData; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.TimerUtil; import tech.pegasys.pantheon.ethereum.p2p.peers.DefaultPeerId; -import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage; import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningController; @@ -107,10 +106,7 @@ public abstract class PeerDiscoveryAgent implements DisconnectCallback { this.peerBlacklist = peerBlacklist; this.nodePermissioningController = nodePermissioningController; this.bootstrapPeers = - config.getBootstrapPeers().stream() - .map(Peer::getEnodeURL) - .map(DiscoveryPeer::fromEnode) - .collect(Collectors.toList()); + config.getBootnodes().stream().map(DiscoveryPeer::fromEnode).collect(Collectors.toList()); this.config = config; this.keyPair = keyPair; @@ -320,7 +316,7 @@ public abstract class PeerDiscoveryAgent implements DisconnectCallback { checkArgument( config.getBindPort() == 0 || NetworkUtility.isValidPort(config.getBindPort()), "valid port number required"); - checkArgument(config.getBootstrapPeers() != null, "bootstrapPeers cannot be null"); + checkArgument(config.getBootnodes() != null, "bootstrapPeers cannot be null"); checkArgument(config.getBucketSize() > 0, "bucket size cannot be negative nor zero"); } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/VertxPeerDiscoveryAgent.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/VertxPeerDiscoveryAgent.java index e2910e209d..268a2929ee 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/VertxPeerDiscoveryAgent.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/VertxPeerDiscoveryAgent.java @@ -146,7 +146,7 @@ public class VertxPeerDiscoveryAgent extends PeerDiscoveryAgent { CompletableFuture result = new CompletableFuture<>(); socket.send( packet.encode(), - peer.getEnodeURL().getEffectiveDiscoveryPort(), + peer.getEndpoint().getUdpPort(), peer.getEnodeURL().getIpAsString(), ar -> { if (ar.failed()) { diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryController.java index 968294f990..81b4ffb7b5 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryController.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryController.java @@ -401,10 +401,10 @@ public class PeerDiscoveryController { private void refreshTableIfRequired() { final long now = System.currentTimeMillis(); if (lastRefreshTime + tableRefreshIntervalMs <= now) { - LOG.info("Peer table refresh triggered by timer expiry"); + LOG.debug("Peer table refresh triggered by timer expiry"); refreshTable(); } else if (!peerRequirement.hasSufficientPeers()) { - LOG.info("Peer table refresh triggered by insufficient peers"); + LOG.debug("Peer table refresh triggered by insufficient peers"); refreshTable(); } } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetwork.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetwork.java index 1b87a3fa91..d3a2e819a2 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetwork.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetwork.java @@ -12,6 +12,7 @@ */ package tech.pegasys.pantheon.ethereum.p2p.network; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; @@ -361,6 +362,9 @@ public class DefaultP2PNetwork implements P2PNetwork { @Override public boolean addMaintainConnectionPeer(final Peer peer) { + checkArgument( + peer.getEnodeURL().isListening(), + "Invalid enode url. Enode url must contain a non-zero listening port."); final boolean added = peerMaintainConnectionList.add(peer); if (isPeerAllowed(peer) && !isConnectingOrConnected(peer)) { // Connect immediately if appropriate @@ -440,18 +444,25 @@ public class DefaultP2PNetwork implements P2PNetwork { } LOG.trace("Initiating connection to peer: {}", peer.getId()); - final EnodeURL enode = peer.getEnodeURL(); final CompletableFuture existingPendingConnection = pendingConnections.putIfAbsent(peer, connectionFuture); if (existingPendingConnection != null) { LOG.debug("Attempted to connect to peer with pending connection: {}", peer.getId()); return existingPendingConnection; } + final EnodeURL enode = peer.getEnodeURL(); + if (!enode.isListening()) { + final String errorMsg = + "Attempt to connect to peer with no listening port: " + enode.toString(); + LOG.warn(errorMsg); + connectionFuture.completeExceptionally(new IllegalStateException(errorMsg)); + return connectionFuture; + } new Bootstrap() .group(workers) .channel(NioSocketChannel.class) - .remoteAddress(new InetSocketAddress(enode.getIp(), enode.getListeningPort())) + .remoteAddress(new InetSocketAddress(enode.getIp(), enode.getListeningPort().getAsInt())) .option(ChannelOption.TCP_NODELAY, true) .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, TIMEOUT_SECONDS * 1000) .handler( @@ -710,8 +721,7 @@ public class DefaultP2PNetwork implements P2PNetwork { peerDiscoveryAgent .getAdvertisedPeer() .map(Peer::getEnodeURL) - .map(EnodeURL::getEffectiveDiscoveryPort) - .map(OptionalInt::of) + .map(EnodeURL::getDiscoveryPort) .orElse(OptionalInt.empty()); final EnodeURL localEnode = diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramer.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramer.java index a505268de2..bbe5c9c766 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramer.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramer.java @@ -170,18 +170,13 @@ final class DeFramer extends ByteToMessageDecoder { private Peer createPeer(final PeerInfo peerInfo, final ChannelHandlerContext ctx) { final InetSocketAddress remoteAddress = ((InetSocketAddress) ctx.channel().remoteAddress()); int port = peerInfo.getPort(); - if (port == 0) { - // Most peers advertise a port of "0", just set a default best guess in this case. - // We can't simply use the remote address port because peers initiating inbound connections - // are free to choose any port they want for their side of the connection. The remote port - // does not actually correspond to the peer's listening port. - port = EnodeURL.DEFAULT_LISTENING_PORT; - } return DefaultPeer.fromEnodeURL( EnodeURL.builder() .nodeId(peerInfo.getNodeId()) .ipAddress(remoteAddress.getAddress()) .listeningPort(port) + // Discovery information is unknown, so disable it + .disableDiscovery() .build()); } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/StaticNodesParser.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/StaticNodesParser.java index a948b8cf06..e2aaf98f96 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/StaticNodesParser.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/peers/StaticNodesParser.java @@ -12,6 +12,7 @@ */ package tech.pegasys.pantheon.ethereum.p2p.peers; +import static com.google.common.base.Preconditions.checkArgument; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Collections.emptySet; @@ -68,9 +69,12 @@ public class StaticNodesParser { private static EnodeURL decodeString(final String input) { try { - return EnodeURL.fromString(input); + final EnodeURL enode = EnodeURL.fromString(input); + checkArgument( + enode.isListening(), "Static node must be configured with a valid listening port."); + return enode; } catch (IllegalArgumentException ex) { - LOG.info("Illegally constructed enode supplied ({})", input); + LOG.info("Illegal static enode supplied ({}). {}", input, ex.getMessage()); throw ex; } } diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/PeerInfo.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/PeerInfo.java index 0630d444ec..c0a2dd664e 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/PeerInfo.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/PeerInfo.java @@ -78,10 +78,11 @@ public class PeerInfo { } /** - * This value is meant to represent the port at which a peer is listening for connections. - * However, most peers actually advertise a port of "0" so this value is not reliable. + * This value is meant to represent the port at which a peer is listening for connections. A value + * of zero means the peer is not listening for incoming connections. * - * @return (Unreliable) The tcp port on which the peer is listening for connections + * @return The tcp port on which the peer is listening for connections, 0 indicates the peer is + * not listening for connections. */ public int getPort() { return port; diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/config/DiscoveryConfigurationTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/config/DiscoveryConfigurationTest.java new file mode 100644 index 0000000000..6d10ed0ef1 --- /dev/null +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/config/DiscoveryConfigurationTest.java @@ -0,0 +1,60 @@ +/* + * 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.p2p.config; + +import static org.assertj.core.api.Java6Assertions.assertThatThrownBy; + +import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; +import tech.pegasys.pantheon.util.enode.EnodeURL; + +import java.util.Collections; + +import org.junit.Test; + +public class DiscoveryConfigurationTest { + + @Test + public void setBootnodes_withDiscoveryDisabled() { + final EnodeURL invalidBootnode = + EnodeURL.builder() + .nodeId(Peer.randomId()) + .ipAddress("127.0.0.1") + .listeningPort(30303) + .disableDiscovery() + .build(); + + DiscoveryConfiguration config = DiscoveryConfiguration.create(); + + assertThatThrownBy(() -> config.setBootnodes(Collections.singletonList(invalidBootnode))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid bootnodes") + .hasMessageContaining("Bootnodes must have discovery enabled"); + } + + @Test + public void setBootnodes_withListeningDisabled() { + final EnodeURL invalidBootnode = + EnodeURL.builder() + .nodeId(Peer.randomId()) + .ipAddress("127.0.0.1") + .discoveryAndListeningPorts(0) + .build(); + + DiscoveryConfiguration config = DiscoveryConfiguration.create(); + + assertThatThrownBy(() -> config.setBootnodes(Collections.singletonList(invalidBootnode))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid bootnodes") + .hasMessageContaining("Bootnodes must have discovery enabled"); + } +} diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java index b8e5356da4..55d26a23c8 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java @@ -14,6 +14,7 @@ package tech.pegasys.pantheon.ethereum.p2p.discovery; import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -25,10 +26,12 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.MockPeerDiscoveryAg import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.NeighborsPacketData; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.Packet; import tech.pegasys.pantheon.ethereum.p2p.discovery.internal.PacketType; +import tech.pegasys.pantheon.ethereum.p2p.peers.Peer; import tech.pegasys.pantheon.ethereum.p2p.peers.PeerBlacklist; import tech.pegasys.pantheon.ethereum.p2p.wire.PeerInfo; import tech.pegasys.pantheon.ethereum.p2p.wire.messages.DisconnectMessage.DisconnectReason; import tech.pegasys.pantheon.util.bytes.BytesValue; +import tech.pegasys.pantheon.util.enode.EnodeURL; import java.util.Collections; import java.util.List; @@ -42,6 +45,23 @@ public class PeerDiscoveryAgentTest { private static final int BROADCAST_TCP_PORT = 30303; private final PeerDiscoveryTestHelper helper = new PeerDiscoveryTestHelper(); + @Test + public void createAgentWithInvalidBootnodes() { + final EnodeURL invalidBootnode = + EnodeURL.builder() + .nodeId(Peer.randomId()) + .ipAddress("127.0.0.1") + .listeningPort(30303) + .disableDiscovery() + .build(); + + assertThatThrownBy( + () -> helper.createDiscoveryAgent(helper.agentBuilder().bootnodes(invalidBootnode))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid bootnodes") + .hasMessageContaining("Bootnodes must have discovery enabled"); + } + @Test public void neighborsPacketFromUnbondedPeerIsDropped() { // Start an agent with no bootstrap peers. diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java index 8a55aae3b0..ad9264061c 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/PeerDiscoveryTestHelper.java @@ -27,7 +27,7 @@ import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningContro import tech.pegasys.pantheon.util.bytes.BytesValue; import tech.pegasys.pantheon.util.enode.EnodeURL; -import java.net.URI; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -69,7 +69,11 @@ public class PeerDiscoveryTestHelper { final BytesValue peerId = keyPair.getPublicKey().getEncodedBytes(); final int port = nextAvailablePort.incrementAndGet(); return DiscoveryPeer.fromEnode( - EnodeURL.builder().nodeId(peerId).ipAddress(LOOPBACK_IP_ADDR).listeningPort(port).build()); + EnodeURL.builder() + .nodeId(peerId) + .ipAddress(LOOPBACK_IP_ADDR) + .discoveryAndListeningPorts(port) + .build()); } public Packet createPingPacket( @@ -176,10 +180,10 @@ public class PeerDiscoveryTestHelper { private PeerBlacklist blacklist = new PeerBlacklist(); private Optional nodePermissioningController = Optional.empty(); - private List bootstrapPeers = Collections.emptyList(); + private List bootnodes = Collections.emptyList(); private boolean active = true; - public AgentBuilder( + private AgentBuilder( final Map agents, final AtomicInteger nextAvailablePort) { this.agents = agents; @@ -187,7 +191,7 @@ public class PeerDiscoveryTestHelper { } public AgentBuilder bootstrapPeers(final List peers) { - this.bootstrapPeers = asEnodes(peers); + this.bootnodes = asEnodes(peers); return this; } @@ -195,11 +199,13 @@ public class PeerDiscoveryTestHelper { return bootstrapPeers(asList(peers)); } - private List asEnodes(final List peers) { - return peers.stream() - .map(Peer::getEnodeURLString) - .map(URI::create) - .collect(Collectors.toList()); + public AgentBuilder bootnodes(final EnodeURL... bootnodes) { + this.bootnodes = Arrays.asList(bootnodes); + return this; + } + + private List asEnodes(final List peers) { + return peers.stream().map(Peer::getEnodeURL).collect(Collectors.toList()); } public AgentBuilder nodePermissioningController(final NodePermissioningController controller) { @@ -219,7 +225,7 @@ public class PeerDiscoveryTestHelper { public MockPeerDiscoveryAgent build() { final DiscoveryConfiguration config = new DiscoveryConfiguration(); - config.setBootstrapPeers(bootstrapPeers); + config.setBootnodes(bootnodes); config.setBindPort(nextAvailablePort.incrementAndGet()); config.setActive(active); diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java index c401156a5d..714fece115 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerDiscoveryControllerTest.java @@ -1122,7 +1122,7 @@ public class PeerDiscoveryControllerTest { EnodeURL.builder() .nodeId(id) .ipAddress("127.0.0.1") - .listeningPort(100 + counter.incrementAndGet()) + .discoveryAndListeningPorts(100 + counter.incrementAndGet()) .build())); doReturn(keccak).when(peer).keccak256(); newPeers.add(peer); diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerTableTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerTableTest.java index 9587770e88..28045eaaf9 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerTableTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/discovery/internal/PeerTableTest.java @@ -49,7 +49,7 @@ public class PeerTableTest { EnodeURL.builder() .nodeId(Peer.randomId()) .ipAddress("127.0.0.1") - .listeningPort(12345) + .discoveryAndListeningPorts(12345) .build()); final PeerTable table = new PeerTable(localPeer.getId(), 16); final PeerTable.AddResult result = table.tryAdd(localPeer); diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetworkTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetworkTest.java index c6e0a2a371..edf0c360cd 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetworkTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/DefaultP2PNetworkTest.java @@ -69,7 +69,6 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; -import org.mockito.ArgumentMatcher; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @@ -124,6 +123,26 @@ public final class DefaultP2PNetworkTest { verify(network, never()).connect(peer); } + @Test + public void addMaintainConnectionPeer_withNonListeningEnode() { + final DefaultP2PNetwork network = mockNetwork(); + network.start(); + final Peer peer = + DefaultPeer.fromEnodeURL( + EnodeURL.builder() + .nodeId(Peer.randomId()) + .ipAddress("127.0.0.1") + .useDefaultPorts() + .disableListening() + .build()); + + assertThatThrownBy(() -> network.addMaintainConnectionPeer(peer)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Enode url must contain a non-zero listening port"); + + verify(network, never()).connect(peer); + } + @Test public void addingRepeatMaintainedPeersReturnsFalse() { final P2PNetwork network = network(); @@ -502,6 +521,27 @@ public final class DefaultP2PNetworkTest { .hasMessageContaining("Attempt to connect to peer before network is ready"); } + @Test + public void connect_toNonListeningPeer() { + final DefaultP2PNetwork network = network(); + network.start(); + final Peer peer = + DefaultPeer.fromEnodeURL( + EnodeURL.builder() + .ipAddress("127.0.0.1") + .nodeId(Peer.randomId()) + .disableListening() + .discoveryPort(30303) + .build()); + + final CompletableFuture connectionResult = network.connect(peer); + assertThat(connectionResult).isCompletedExceptionally(); + assertThatThrownBy(connectionResult::get) + .hasCauseInstanceOf(IllegalStateException.class) + .hasMessageContaining( + "Attempt to connect to peer with no listening port: " + peer.getEnodeURLString()); + } + private DiscoveryPeer createDiscoveryPeer() { return createDiscoveryPeer(Peer.randomId(), 999); } @@ -529,7 +569,7 @@ public final class DefaultP2PNetworkTest { 5, "test", Arrays.asList(Capability.create("eth", 63)), - remoteEnode.getListeningPort(), + remoteEnode.getListeningPortOrZero(), remoteEnode.getNodeId()); final PeerConnection peerConnection = mock(PeerConnection.class); @@ -612,32 +652,12 @@ public final class DefaultP2PNetworkTest { return EnodeURL.builder() .ipAddress(InetAddress.getLoopbackAddress().getHostAddress()) .nodeId(nodeId) - .listeningPort(listenPort) + .discoveryAndListeningPorts(listenPort) .build(); } - public static class EnodeURLMatcher implements ArgumentMatcher { - - private final EnodeURL enodeURL; - - EnodeURLMatcher(final EnodeURL enodeURL) { - this.enodeURL = enodeURL; - } - - @Override - public boolean matches(final EnodeURL argument) { - if (argument == null) { - return false; - } else { - return enodeURL.getNodeId().equals(argument.getNodeId()) - && enodeURL.getIp().equals(argument.getIp()) - && enodeURL.getListeningPort() == argument.getListeningPort(); - } - } - } - private EnodeURL enodeEq(final EnodeURL enodeURL) { - return argThat(new EnodeURLMatcher(enodeURL)); + return argThat((EnodeURL e) -> EnodeURL.sameListeningEndpoint(e, enodeURL)); } private static SubProtocol subProtocol() { diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/NetworkingServiceLifecycleTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/NetworkingServiceLifecycleTest.java index 3ee08b803b..0ac734a46f 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/NetworkingServiceLifecycleTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/NetworkingServiceLifecycleTest.java @@ -53,8 +53,8 @@ public class NetworkingServiceLifecycleTest { try (final P2PNetwork service = builder().build()) { service.start(); final EnodeURL enode = service.getLocalEnode().get(); - final int udpPort = enode.getEffectiveDiscoveryPort(); - final int tcpPort = enode.getListeningPort(); + final int udpPort = enode.getDiscoveryPortOrZero(); + final int tcpPort = enode.getListeningPortOrZero(); assertEquals(config.getDiscovery().getAdvertisedHost(), enode.getIpAsString()); assertThat(udpPort).isNotZero(); @@ -124,7 +124,9 @@ public class NetworkingServiceLifecycleTest { try (final P2PNetwork service1 = builder().config(config).build()) { service1.start(); final NetworkingConfiguration config = configWithRandomPorts(); - config.getDiscovery().setBindPort(service1.getLocalEnode().get().getEffectiveDiscoveryPort()); + final int usedPort = service1.getLocalEnode().get().getDiscoveryPortOrZero(); + assertThat(usedPort).isNotZero(); + config.getDiscovery().setBindPort(usedPort); try (final P2PNetwork service2 = builder().config(config).build()) { try { service2.start(); diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/P2PNetworkTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/P2PNetworkTest.java index e8c30008ed..b768dc2192 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/P2PNetworkTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/P2PNetworkTest.java @@ -54,7 +54,6 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; -import org.mockito.ArgumentMatcher; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @@ -97,7 +96,7 @@ public class P2PNetworkTest { connector.start(); final EnodeURL listenerEnode = listener.getLocalEnode().get(); final BytesValue listenId = listenerEnode.getNodeId(); - final int listenPort = listenerEnode.getListeningPort(); + final int listenPort = listenerEnode.getListeningPort().getAsInt(); assertThat( connector @@ -119,7 +118,7 @@ public class P2PNetworkTest { connector.start(); final EnodeURL listenerEnode = listener.getLocalEnode().get(); final BytesValue listenId = listenerEnode.getNodeId(); - final int listenPort = listenerEnode.getListeningPort(); + final int listenPort = listenerEnode.getListeningPort().getAsInt(); assertThat( connector @@ -159,7 +158,7 @@ public class P2PNetworkTest { connector1.start(); final EnodeURL listenerEnode = listener.getLocalEnode().get(); final BytesValue listenId = listenerEnode.getNodeId(); - final int listenPort = listenerEnode.getListeningPort(); + final int listenPort = listenerEnode.getListeningPort().getAsInt(); final Peer listeningPeer = createPeer(listenId, listenPort); assertThat( @@ -209,7 +208,7 @@ public class P2PNetworkTest { connector.start(); final EnodeURL listenerEnode = listener.getLocalEnode().get(); final BytesValue listenId = listenerEnode.getNodeId(); - final int listenPort = listenerEnode.getListeningPort(); + final int listenPort = listenerEnode.getListeningPort().getAsInt(); final Peer listenerPeer = createPeer(listenId, listenPort); final CompletableFuture connectFuture = connector.connect(listenerPeer); @@ -230,11 +229,11 @@ public class P2PNetworkTest { final EnodeURL localEnode = localNetwork.getLocalEnode().get(); final BytesValue localId = localEnode.getNodeId(); - final int localPort = localEnode.getListeningPort(); + final int localPort = localEnode.getListeningPort().getAsInt(); final EnodeURL remoteEnode = remoteNetwork.getLocalEnode().get(); final BytesValue remoteId = remoteEnode.getNodeId(); - final int remotePort = remoteEnode.getListeningPort(); + final int remotePort = remoteEnode.getListeningPort().getAsInt(); final Peer localPeer = createPeer(localId, localPort); final Peer remotePeer = createPeer(remoteId, remotePort); @@ -278,7 +277,11 @@ public class P2PNetworkTest { config, Collections.emptyList(), selfEnode.getNodeId()); // turn on whitelisting by adding a different node NOT remote node localWhitelistController.addNode( - EnodeURL.builder().ipAddress("127.0.0.1").nodeId(Peer.randomId()).build()); + EnodeURL.builder() + .ipAddress("127.0.0.1") + .useDefaultPorts() + .nodeId(Peer.randomId()) + .build()); final NodePermissioningController nodePermissioningController = new NodePermissioningController( Optional.empty(), Collections.singletonList(localWhitelistController)); @@ -295,7 +298,7 @@ public class P2PNetworkTest { final EnodeURL localEnode = localNetwork.getLocalEnode().get(); final BytesValue localId = localEnode.getNodeId(); - final int localPort = localEnode.getListeningPort(); + final int localPort = localEnode.getListeningPort().getAsInt(); final Peer localPeer = createPeer(localId, localPort); @@ -325,7 +328,7 @@ public class P2PNetworkTest { EnodeURL.builder() .ipAddress(InetAddress.getLoopbackAddress().getHostAddress()) .nodeId(nodeId) - .listeningPort(listenPort) + .discoveryAndListeningPorts(listenPort) .build()); } @@ -357,26 +360,6 @@ public class P2PNetworkTest { }; } - public static class EnodeURLMatcher implements ArgumentMatcher { - - private final EnodeURL enodeURL; - - EnodeURLMatcher(final EnodeURL enodeURL) { - this.enodeURL = enodeURL; - } - - @Override - public boolean matches(final EnodeURL argument) { - if (argument == null) { - return false; - } else { - return enodeURL.getNodeId().equals(argument.getNodeId()) - && enodeURL.getIp().equals(argument.getIp()) - && enodeURL.getListeningPort() == argument.getListeningPort(); - } - } - } - private DefaultP2PNetwork.Builder builder() { return DefaultP2PNetwork.builder() .vertx(vertx) diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramerTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramerTest.java index e588b0248e..f1f781d57f 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramerTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/DeFramerTest.java @@ -156,6 +156,14 @@ public class DeFramerTest { assertThat(connectFuture).isNotCompletedExceptionally(); PeerConnection peerConnection = connectFuture.get(); assertThat(peerConnection.getPeerInfo()).isEqualTo(remotePeerInfo); + + EnodeURL expectedEnode = + EnodeURL.builder() + .configureFromEnode(peer.getEnodeURL()) + // Discovery information is not available from peer info + .disableDiscovery() + .build(); + assertThat(peerConnection.getPeer().getEnodeURL()).isEqualTo(expectedEnode); assertThat(out).isEmpty(); // Next phase of pipeline should be setup @@ -206,8 +214,10 @@ public class DeFramerTest { EnodeURL.builder() .ipAddress(remoteAddress.getAddress()) .nodeId(peer.getId()) - // Listening port should be replaced with default port - .listeningPort(EnodeURL.DEFAULT_LISTENING_PORT) + // Listening port should be disabled + .disableListening() + // Discovery port is unknown + .disableDiscovery() .build(); assertThat(peerConnection.getPeer().getEnodeURL()).isEqualTo(expectedEnode); @@ -237,7 +247,7 @@ public class DeFramerTest { peerInfo.getVersion(), peerInfo.getClientId(), peerInfo.getCapabilities(), - peer.getEnodeURL().getListeningPort(), + peer.getEnodeURL().getListeningPortOrZero(), mismatchedId); final DeFramer deFramer = createDeFramer(peer); @@ -340,7 +350,7 @@ public class DeFramerTest { return DefaultPeer.fromEnodeURL( EnodeURL.builder() .ipAddress(remoteAddress.getAddress()) - .listeningPort(remotePort) + .discoveryAndListeningPorts(remotePort) .nodeId(Peer.randomId()) .build()); } @@ -350,7 +360,7 @@ public class DeFramerTest { peerInfo.getVersion(), peerInfo.getClientId(), peerInfo.getCapabilities(), - forPeer.getEnodeURL().getListeningPort(), + forPeer.getEnodeURL().getListeningPortOrZero(), forPeer.getId()); } diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/NettyPeerConnectionTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/NettyPeerConnectionTest.java index cf6c30b9d6..d061b59c0c 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/NettyPeerConnectionTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/network/netty/NettyPeerConnectionTest.java @@ -55,7 +55,7 @@ public class NettyPeerConnectionTest { DefaultPeer.fromEnodeURL( EnodeURL.builder() .ipAddress("127.0.0.1") - .listeningPort(12345) + .discoveryAndListeningPorts(12345) .nodeId(Peer.randomId()) .build()); connection = diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerTest.java index 2e78838b13..a2f09f34f5 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/PeerTest.java @@ -22,8 +22,6 @@ import tech.pegasys.pantheon.ethereum.p2p.discovery.PeerDiscoveryStatus; import tech.pegasys.pantheon.util.bytes.BytesValue; import tech.pegasys.pantheon.util.enode.EnodeURL; -import java.util.OptionalInt; - import com.google.common.net.InetAddresses; import org.junit.Test; @@ -36,10 +34,18 @@ public class PeerTest { "c7849b663d12a2b5bf05b1ebf5810364f4870d5f1053fbd7500d38bc54c705b453d7511ca8a4a86003d34d4c8ee0bbfcd387aa724f5b240b3ab4bbb994a1e09b"); final Peer peer = DiscoveryPeer.fromEnode( - EnodeURL.builder().nodeId(id).ipAddress("127.0.0.1").listeningPort(5000).build()); + EnodeURL.builder() + .nodeId(id) + .ipAddress("127.0.0.1") + .discoveryAndListeningPorts(5000) + .build()); final Peer peer2 = DiscoveryPeer.fromEnode( - EnodeURL.builder().nodeId(id).ipAddress("127.0.0.1").listeningPort(5001).build()); + EnodeURL.builder() + .nodeId(id) + .ipAddress("127.0.0.1") + .discoveryAndListeningPorts(5001) + .build()); assertNotEquals(peer, peer2); } @@ -50,10 +56,18 @@ public class PeerTest { "c7849b663d12a2b5bf05b1ebf5810364f4870d5f1053fbd7500d38bc54c705b453d7511ca8a4a86003d34d4c8ee0bbfcd387aa724f5b240b3ab4bbb994a1e09b"); final Peer peer = DiscoveryPeer.fromEnode( - EnodeURL.builder().nodeId(id).ipAddress("127.0.0.1").listeningPort(5000).build()); + EnodeURL.builder() + .nodeId(id) + .ipAddress("127.0.0.1") + .discoveryAndListeningPorts(5000) + .build()); final Peer peer2 = DiscoveryPeer.fromEnode( - EnodeURL.builder().nodeId(id).ipAddress("127.0.0.1").listeningPort(5001).build()); + EnodeURL.builder() + .nodeId(id) + .ipAddress("127.0.0.1") + .discoveryAndListeningPorts(5001) + .build()); assertNotEquals(peer.hashCode(), peer2.hashCode()); } @@ -64,7 +78,7 @@ public class PeerTest { EnodeURL.builder() .nodeId(Peer.randomId()) .ipAddress("127.0.0.1") - .listeningPort(5000) + .discoveryAndListeningPorts(5000) .build()); assertEquals(PeerDiscoveryStatus.KNOWN, peer.getStatus()); } @@ -79,8 +93,8 @@ public class PeerTest { "c7849b663d12a2b5bf05b1ebf5810364f4870d5f1053fbd7500d38bc54c705b453d7511ca8a4a86003d34d4c8ee0bbfcd387aa724f5b240b3ab4bbb994a1e09b"), peer.getId()); assertEquals("172.20.0.4", peer.getEnodeURL().getIpAsString()); - assertEquals(30403, peer.getEnodeURL().getListeningPort()); - assertEquals(OptionalInt.empty(), peer.getEnodeURL().getDiscoveryPort()); + assertEquals(30403, peer.getEnodeURL().getListeningPortOrZero()); + assertEquals(30403, peer.getEnodeURL().getDiscoveryPortOrZero()); } @Test @@ -94,8 +108,8 @@ public class PeerTest { peer.getId()); assertEquals( InetAddresses.forString("2001:db8:85a3::8a2e:370:7334"), peer.getEnodeURL().getIp()); - assertEquals(30403, peer.getEnodeURL().getListeningPort()); - assertEquals(OptionalInt.empty(), peer.getEnodeURL().getDiscoveryPort()); + assertEquals(30403, peer.getEnodeURL().getListeningPortOrZero()); + assertEquals(30403, peer.getEnodeURL().getDiscoveryPortOrZero()); } @Test(expected = IllegalArgumentException.class) @@ -124,8 +138,8 @@ public class PeerTest { "c7849b663d12a2b5bf05b1ebf5810364f4870d5f1053fbd7500d38bc54c705b453d7511ca8a4a86003d34d4c8ee0bbfcd387aa724f5b240b3ab4bbb994a1e09b"), peer.getId()); assertEquals("172.20.0.4", peer.getEnodeURL().getIpAsString()); - assertEquals(22222, peer.getEnodeURL().getEffectiveDiscoveryPort()); - assertEquals(12345, peer.getEnodeURL().getListeningPort()); + assertEquals(22222, peer.getEnodeURL().getDiscoveryPortOrZero()); + assertEquals(12345, peer.getEnodeURL().getListeningPortOrZero()); } @Test diff --git a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/StaticNodesParserTest.java b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/StaticNodesParserTest.java index 271991c1cf..c433a0f4fb 100644 --- a/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/StaticNodesParserTest.java +++ b/ethereum/p2p/src/test/java/tech/pegasys/pantheon/ethereum/p2p/peers/StaticNodesParserTest.java @@ -46,24 +46,25 @@ public class StaticNodesParserTest { .nodeId( "50203c6bfca6874370e71aecc8958529fd723feb05013dc1abca8fc1fff845c5259faba05852e9dfe5ce172a7d6e7c2a3a5eaa8b541c8af15ea5518bbff5f2fa") .ipAddress("127.0.0.1") + .useDefaultPorts() .build(), EnodeURL.builder() .nodeId( "02beb46bc17227616be44234071dfa18516684e45eed88049190b6cb56b0bae218f045fd0450f123b8f55c60b96b78c45e8e478004293a8de6818aa4e02eff97") .ipAddress("127.0.0.1") - .listeningPort(30304) + .discoveryAndListeningPorts(30304) .build(), EnodeURL.builder() .nodeId( "819e5cbd81f123516b10f04bf620daa2b385efef06d77253148b814bf1bb6197ff58ebd1fd7bf5dc765b49a4440c733bf941e479c800173f2bfeb887e4fbcbc2") .ipAddress("127.0.0.1") - .listeningPort(30305) + .discoveryAndListeningPorts(30305) .build(), EnodeURL.builder() .nodeId( "6cf53e25d2a98a22e7e205a86bda7077e3c8a7bc99e5ff88ddfd2037a550969ab566f069ffa455df0cfae0c21f7aec3447e414eccc473a3e8b20984b90f164ac") .ipAddress("127.0.0.1") - .listeningPort(30306) + .discoveryAndListeningPorts(30306) .build()); @Rule public TemporaryFolder testFolder = new TemporaryFolder(); @@ -87,6 +88,17 @@ public class StaticNodesParserTest { .isInstanceOf(IllegalArgumentException.class); } + @Test + public void fromPath_withNonListeningNodesThrowsException() { + final URL resource = + StaticNodesParserTest.class.getResource("invalid_static_nodes_no_listening_port.json"); + final File invalidFile = new File(resource.getFile()); + + assertThatThrownBy(() -> StaticNodesParser.fromPath(invalidFile.toPath())) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Static node must be configured with a valid listening port"); + } + @Test public void nonJsonFileThrowsAnException() throws IOException { final File tempFile = testFolder.newFile("file.txt"); diff --git a/ethereum/p2p/src/test/resources/tech/pegasys/pantheon/ethereum/p2p/peers/invalid_static_nodes_no_listening_port.json b/ethereum/p2p/src/test/resources/tech/pegasys/pantheon/ethereum/p2p/peers/invalid_static_nodes_no_listening_port.json new file mode 100644 index 0000000000..f450402138 --- /dev/null +++ b/ethereum/p2p/src/test/resources/tech/pegasys/pantheon/ethereum/p2p/peers/invalid_static_nodes_no_listening_port.json @@ -0,0 +1,4 @@ +["enode://50203c6bfca6874370e71aecc8958529fd723feb05013dc1abca8fc1fff845c5259faba05852e9dfe5ce172a7d6e7c2a3a5eaa8b541c8af15ea5518bbff5f2fa@127.0.0.1:30303", + "enode://02beb46bc17227616be44234071dfa18516684e45eed88049190b6cb56b0bae218f045fd0450f123b8f55c60b96b78c45e8e478004293a8de6818aa4e02eff97@127.0.0.1:0", + "enode://819e5cbd81f123516b10f04bf620daa2b385efef06d77253148b814bf1bb6197ff58ebd1fd7bf5dc765b49a4440c733bf941e479c800173f2bfeb887e4fbcbc2@127.0.0.1:30305", + "enode://6cf53e25d2a98a22e7e205a86bda7077e3c8a7bc99e5ff88ddfd2037a550969ab566f069ffa455df0cfae0c21f7aec3447e414eccc473a3e8b20984b90f164ac@127.0.0.1:30306"] \ No newline at end of file diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningController.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningController.java index 3832664bde..607acf5b1d 100644 --- a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningController.java +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningController.java @@ -199,19 +199,6 @@ public class NodeLocalConfigPermissioningController implements NodePermissioning return peers.parallelStream().map(EnodeURL::toString).collect(Collectors.toList()); } - private boolean compareEnodes(final EnodeURL nodeA, final EnodeURL nodeB) { - boolean idsMatch = nodeA.getNodeId().equals(nodeB.getNodeId()); - boolean hostsMatch = nodeA.getIp().equals(nodeB.getIp()); - boolean listeningPortsMatch = nodeA.getListeningPort() == nodeB.getListeningPort(); - boolean discoveryPortsMatch = true; - if (nodeA.getDiscoveryPort().isPresent() && nodeB.getDiscoveryPort().isPresent()) { - discoveryPortsMatch = - nodeA.getDiscoveryPort().getAsInt() == nodeB.getDiscoveryPort().getAsInt(); - } - - return idsMatch && hostsMatch && listeningPortsMatch && discoveryPortsMatch; - } - public boolean isPermitted(final String enodeURL) { return isPermitted(EnodeURL.fromString(enodeURL)); } @@ -220,7 +207,7 @@ public class NodeLocalConfigPermissioningController implements NodePermissioning if (Objects.equals(localNodeId, node.getNodeId())) { return true; } - return nodesWhitelist.stream().anyMatch(p -> compareEnodes(p, node)); + return nodesWhitelist.stream().anyMatch(p -> EnodeURL.sameListeningEndpoint(p, node)); } public List getNodesWhitelist() { diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeSmartContractPermissioningController.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeSmartContractPermissioningController.java index 81fb9bac02..78e34fa7de 100644 --- a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeSmartContractPermissioningController.java +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeSmartContractPermissioningController.java @@ -142,7 +142,7 @@ public class NodeSmartContractPermissioningController implements NodePermissioni private static BytesValue encodeEnodeUrl(final EnodeURL enode) { return BytesValues.concatenate( - enode.getNodeId(), encodeIp(enode.getIp()), encodePort(enode.getListeningPort())); + enode.getNodeId(), encodeIp(enode.getIp()), encodePort(enode.getListeningPortOrZero())); } // As a function parameter an ip needs to be the appropriate number of bytes, big endian, and diff --git a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningControllerTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningControllerTest.java index f628687780..fbb70c4258 100644 --- a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningControllerTest.java +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeLocalConfigPermissioningControllerTest.java @@ -194,7 +194,8 @@ public class NodeLocalConfigPermissioningControllerTest { } @Test - public void whenCheckingIfNodeIsPermittedDiscoveryPortShouldNotBeConsideredIfAbsent() { + public void + whenCheckingIfNodeIsPermittedDiscoveryPortShouldNotBeConsidered_implicitDiscPortMismatch() { String peerWithDiscoveryPortSet = "enode://aaaa80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@127.0.0.1:30303?discport=10001"; String peerWithoutDiscoveryPortSet = @@ -206,7 +207,8 @@ public class NodeLocalConfigPermissioningControllerTest { } @Test - public void whenCheckingIfNodeIsPermittedDiscoveryPortShouldBeConsideredIfPresent() { + public void + whenCheckingIfNodeIsPermittedDiscoveryPortShouldBeNotConsidered_explicitDiscPortMismatch() { String peerWithDiscoveryPortSet = "enode://aaaa80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@127.0.0.1:30303?discport=10001"; String peerWithDifferentDiscoveryPortSet = @@ -214,7 +216,46 @@ public class NodeLocalConfigPermissioningControllerTest { controller.addNodes(Arrays.asList(peerWithDifferentDiscoveryPortSet)); - assertThat(controller.isPermitted(peerWithDiscoveryPortSet)).isFalse(); + assertThat(controller.isPermitted(peerWithDiscoveryPortSet)).isTrue(); + } + + @Test + public void + whenCheckingIfNodeIsPermittedDiscoveryPortShouldNotBeConsidered_nodeToCheckHasDiscDisabled() { + String peerWithDiscoveryPortSet = + "enode://aaaa80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@127.0.0.1:30303?discport=10001"; + String peerWithoutDiscoveryPortSet = + "enode://aaaa80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@127.0.0.1:30303?discport=0"; + + controller.addNodes(Arrays.asList(peerWithDiscoveryPortSet)); + + assertThat(controller.isPermitted(peerWithoutDiscoveryPortSet)).isTrue(); + } + + @Test + public void + whenCheckingIfNodeIsPermittedDiscoveryPortShouldNotBeConsidered_whitelistedNodeHasDiscDisabled() { + String peerWithDiscoveryPortSet = + "enode://aaaa80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@127.0.0.1:30303?discport=0"; + String peerWithoutDiscoveryPortSet = + "enode://aaaa80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@127.0.0.1:30303?discport=123"; + + controller.addNodes(Arrays.asList(peerWithDiscoveryPortSet)); + + assertThat(controller.isPermitted(peerWithoutDiscoveryPortSet)).isTrue(); + } + + @Test + public void + whenCheckingIfNodeIsPermittedDiscoveryPortShouldNotBeConsidered_whitelistAndNodeHaveDiscDisabled() { + String peerWithDiscoveryPortSet = + "enode://aaaa80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@127.0.0.1:30303?discport=0"; + String peerWithoutDiscoveryPortSet = + "enode://aaaa80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@127.0.0.1:30303?discport=0"; + + controller.addNodes(Arrays.asList(peerWithDiscoveryPortSet)); + + assertThat(controller.isPermitted(peerWithoutDiscoveryPortSet)).isTrue(); } @Test diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/Runner.java b/pantheon/src/main/java/tech/pegasys/pantheon/Runner.java index 3d759d8aea..17a2f31f34 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/Runner.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/Runner.java @@ -17,7 +17,6 @@ import tech.pegasys.pantheon.ethereum.graphqlrpc.GraphQLRpcHttpService; import tech.pegasys.pantheon.ethereum.jsonrpc.JsonRpcHttpService; import tech.pegasys.pantheon.ethereum.jsonrpc.websocket.WebSocketService; import tech.pegasys.pantheon.ethereum.p2p.NetworkRunner; -import tech.pegasys.pantheon.ethereum.p2p.api.P2PNetwork; import tech.pegasys.pantheon.metrics.prometheus.MetricsService; import tech.pegasys.pantheon.util.enode.EnodeURL; @@ -161,18 +160,20 @@ public class Runner implements AutoCloseable { private void writePantheonPortsToFile() { final Properties properties = new Properties(); - final P2PNetwork network = networkRunner.getNetwork(); if (networkRunner.getNetwork().isP2pEnabled()) { networkRunner .getNetwork() .getLocalEnode() .ifPresent( enode -> { - if (network.isDiscoveryEnabled()) { + if (enode.getDiscoveryPort().isPresent()) { properties.setProperty( - "discovery", String.valueOf(enode.getEffectiveDiscoveryPort())); + "discovery", String.valueOf(enode.getDiscoveryPort().getAsInt())); + } + if (enode.getListeningPort().isPresent()) { + properties.setProperty( + "p2p", String.valueOf(enode.getListeningPort().getAsInt())); } - properties.setProperty("p2p", String.valueOf(enode.getListeningPort())); }); } diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java b/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java index 765a54b619..c4c12899e1 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java @@ -76,7 +76,6 @@ import tech.pegasys.pantheon.util.bytes.BytesValue; import tech.pegasys.pantheon.util.enode.EnodeURL; import java.io.IOException; -import java.net.URI; import java.nio.file.Path; import java.util.ArrayList; import java.util.Collection; @@ -205,7 +204,7 @@ public class RunnerBuilder { final DiscoveryConfiguration discoveryConfiguration; if (discovery) { - final Collection bootstrap; + final List bootstrap; if (ethNetworkConfig.getBootNodes() == null) { bootstrap = DiscoveryConfiguration.MAINNET_BOOTSTRAP_NODES; } else { @@ -215,7 +214,7 @@ public class RunnerBuilder { DiscoveryConfiguration.create() .setBindPort(p2pListenPort) .setAdvertisedHost(p2pAdvertisedHost) - .setBootstrapPeers(bootstrap); + .setBootnodes(bootstrap); } else { discoveryConfiguration = DiscoveryConfiguration.create().setActive(false); } @@ -246,10 +245,7 @@ public class RunnerBuilder { new PeerBlacklist( bannedNodeIds.stream().map(BytesValue::fromHexString).collect(Collectors.toSet())); - final List bootnodesAsEnodeURLs = - discoveryConfiguration.getBootstrapPeers().stream() - .map(p -> EnodeURL.fromString(p.getEnodeURLString())) - .collect(Collectors.toList()); + final List bootnodes = discoveryConfiguration.getBootnodes(); final Optional localPermissioningConfiguration = permissioningConfiguration.flatMap(PermissioningConfiguration::getLocalConfig); @@ -263,7 +259,7 @@ public class RunnerBuilder { final BytesValue localNodeId = keyPair.getPublicKey().getEncodedBytes(); final Optional nodePermissioningController = buildNodePermissioningController( - bootnodesAsEnodeURLs, synchronizer, transactionSimulator, localNodeId); + bootnodes, synchronizer, transactionSimulator, localNodeId); NetworkBuilder inactiveNetwork = (caps) -> new NoopP2PNetwork(); NetworkBuilder activeNetwork = @@ -291,7 +287,7 @@ public class RunnerBuilder { nodePermissioningController.ifPresent( n -> n.setInsufficientPeersPermissioningProvider( - new InsufficientPeersPermissioningProvider(network, bootnodesAsEnodeURLs))); + new InsufficientPeersPermissioningProvider(network, bootnodes))); final TransactionPool transactionPool = pantheonController.getTransactionPool(); final MiningCoordinator miningCoordinator = pantheonController.getMiningCoordinator(); diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/EthNetworkConfig.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/EthNetworkConfig.java index 41f4a55fb2..41d68953ca 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/EthNetworkConfig.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/EthNetworkConfig.java @@ -18,11 +18,13 @@ import static tech.pegasys.pantheon.ethereum.p2p.config.DiscoveryConfiguration.M import static tech.pegasys.pantheon.ethereum.p2p.config.DiscoveryConfiguration.RINKEBY_BOOTSTRAP_NODES; import static tech.pegasys.pantheon.ethereum.p2p.config.DiscoveryConfiguration.ROPSTEN_BOOTSTRAP_NODES; +import tech.pegasys.pantheon.util.enode.EnodeURL; + import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; import java.util.ArrayList; -import java.util.Collection; +import java.util.List; import java.util.Objects; import com.google.common.base.Preconditions; @@ -42,10 +44,10 @@ public class EthNetworkConfig { private static final String DEV_GENESIS = "/dev.json"; private final String genesisConfig; private final int networkId; - private final Collection bootNodes; + private final List bootNodes; public EthNetworkConfig( - final String genesisConfig, final int networkId, final Collection bootNodes) { + final String genesisConfig, final int networkId, final List bootNodes) { Preconditions.checkNotNull(genesisConfig); Preconditions.checkNotNull(bootNodes); this.genesisConfig = genesisConfig; @@ -61,7 +63,7 @@ public class EthNetworkConfig { return networkId; } - public Collection getBootNodes() { + public List getBootNodes() { return bootNodes; } @@ -146,7 +148,7 @@ public class EthNetworkConfig { private String genesisConfig; private int networkId; - private Collection bootNodes; + private List bootNodes; public Builder(final EthNetworkConfig ethNetworkConfig) { this.genesisConfig = ethNetworkConfig.genesisConfig; @@ -164,7 +166,7 @@ public class EthNetworkConfig { return this; } - public Builder setBootNodes(final Collection bootNodes) { + public Builder setBootNodes(final List bootNodes) { this.bootNodes = bootNodes; return this; } diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index 13fc70a754..1962b5effc 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -54,6 +54,7 @@ import tech.pegasys.pantheon.ethereum.jsonrpc.JsonRpcConfiguration; import tech.pegasys.pantheon.ethereum.jsonrpc.RpcApi; import tech.pegasys.pantheon.ethereum.jsonrpc.RpcApis; import tech.pegasys.pantheon.ethereum.jsonrpc.websocket.WebSocketConfiguration; +import tech.pegasys.pantheon.ethereum.p2p.config.DiscoveryConfiguration; import tech.pegasys.pantheon.ethereum.p2p.peers.StaticNodesParser; import tech.pegasys.pantheon.ethereum.permissioning.LocalPermissioningConfiguration; import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration; @@ -190,14 +191,15 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { bootNodes = values.stream() .filter(value -> !value.isEmpty()) - .map(value -> EnodeURL.fromString(value).toURI()) + .map(EnodeURL::fromString) .collect(Collectors.toList()); + DiscoveryConfiguration.assertValidBootnodes(bootNodes); } catch (final IllegalArgumentException e) { throw new ParameterException(commandLine, e.getMessage()); } } - private Collection bootNodes = null; + private List bootNodes = null; @Option( names = {"--max-peers"}, @@ -689,9 +691,13 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { logger.info("Connecting to {} static nodes.", staticNodes.size()); logger.trace("Static Nodes = {}", staticNodes); + List enodeURIs = + ethNetworkConfig.getBootNodes().stream() + .map(EnodeURL::toURI) + .collect(Collectors.toList()); permissioningConfiguration .flatMap(PermissioningConfiguration::getLocalConfig) - .ifPresent(p -> ensureAllNodesAreInWhitelist(ethNetworkConfig.getBootNodes(), p)); + .ifPresent(p -> ensureAllNodesAreInWhitelist(enodeURIs, p)); permissioningConfiguration .flatMap(PermissioningConfiguration::getLocalConfig) diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/RunnerTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/RunnerTest.java index 94bd76582c..b4cb3d0b6b 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/RunnerTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/RunnerTest.java @@ -219,9 +219,7 @@ public final class RunnerTest { final EnodeURL enode = runnerAhead.getLocalEnode().get(); final EthNetworkConfig behindEthNetworkConfiguration = new EthNetworkConfig( - EthNetworkConfig.jsonConfig(DEV), - DEV_NETWORK_ID, - Collections.singletonList(enode.toURI())); + EthNetworkConfig.jsonConfig(DEV), DEV_NETWORK_ID, Collections.singletonList(enode)); runnerBehind = runnerBuilder .pantheonController(controllerBehind) diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index 7c4c21f7df..0dd5caed7b 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -315,11 +315,11 @@ public class PantheonCommandTest extends CommandTestAbstract { verify(mockRunnerBuilder).metricsConfiguration(eq(metricsConfiguration)); verify(mockRunnerBuilder).build(); - final Collection nodes = + final List nodes = asList( - URI.create("enode://" + VALID_NODE_ID + "@192.168.0.1:4567"), - URI.create("enode://" + VALID_NODE_ID + "@192.168.0.1:4567"), - URI.create("enode://" + VALID_NODE_ID + "@192.168.0.1:4567")); + EnodeURL.fromString("enode://" + VALID_NODE_ID + "@192.168.0.1:4567"), + EnodeURL.fromString("enode://" + VALID_NODE_ID + "@192.168.0.1:4567"), + EnodeURL.fromString("enode://" + VALID_NODE_ID + "@192.168.0.1:4567")); assertThat(ethNetworkConfigArgumentCaptor.getValue().getBootNodes()).isEqualTo(nodes); final EthNetworkConfig networkConfig = @@ -957,6 +957,20 @@ public class PantheonCommandTest extends CommandTestAbstract { assertThat(commandErrorOutput.toString()).startsWith(expectedErrorOutputStart); } + @Test + public void callingWithBootnodeThatHasDiscoveryDisabledMustDisplayErrorAndUsage() { + final String validBootnode = + "enode://d2567893371ea5a6fa6371d483891ed0d129e79a8fc74d6df95a00a6545444cd4a6960bbffe0b4e2edcf35135271de57ee559c0909236bbc2074346ef2b5b47c@127.0.0.1:30304"; + final String invalidBootnode = + "enode://02567893371ea5a6fa6371d483891ed0d129e79a8fc74d6df95a00a6545444cd4a6960bbffe0b4e2edcf35135271de57ee559c0909236bbc2074346ef2b5b47c@127.0.0.1:30303?discport=0"; + final String bootnodesValue = validBootnode + "," + invalidBootnode; + parseCommand("--bootnodes", bootnodesValue); + assertThat(commandOutput.toString()).isEmpty(); + final String expectedErrorOutputStart = + "Bootnodes must have discovery enabled. Invalid bootnodes: " + invalidBootnode + "."; + assertThat(commandErrorOutput.toString()).startsWith(expectedErrorOutputStart); + } + // This test ensures non regression on https://pegasys1.atlassian.net/browse/PAN-2387 @Test public void callingWithInvalidBootnodeAndEqualSignMustDisplayErrorAndUsage() { @@ -985,7 +999,8 @@ public class PantheonCommandTest extends CommandTestAbstract { verify(mockRunnerBuilder).build(); assertThat(ethNetworkConfigArgumentCaptor.getValue().getBootNodes()) - .isEqualTo(Stream.of(validENodeStrings).map(URI::create).collect(Collectors.toList())); + .isEqualTo( + Stream.of(validENodeStrings).map(EnodeURL::fromString).collect(Collectors.toList())); assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()).isEmpty(); @@ -2204,7 +2219,8 @@ public class PantheonCommandTest extends CommandTestAbstract { verify(mockControllerBuilder).build(); assertThat(networkArg.getValue().getBootNodes()) - .isEqualTo(Stream.of(validENodeStrings).map(URI::create).collect(Collectors.toList())); + .isEqualTo( + Stream.of(validENodeStrings).map(EnodeURL::fromString).collect(Collectors.toList())); assertThat(networkArg.getValue().getNetworkId()).isEqualTo(1234567); assertThat(commandOutput.toString()).isEmpty(); @@ -2426,12 +2442,14 @@ public class PantheonCommandTest extends CommandTestAbstract { .nodeId( "50203c6bfca6874370e71aecc8958529fd723feb05013dc1abca8fc1fff845c5259faba05852e9dfe5ce172a7d6e7c2a3a5eaa8b541c8af15ea5518bbff5f2fa") .ipAddress("127.0.0.1") + .useDefaultPorts() .build(); final EnodeURL whiteListedNode = EnodeURL.builder() .nodeId( "50203c6bfca6874370e71aecc8958529fd723feb05013dc1abca8fc1fff845c5259faba05852e9dfe5ce172a7d6e7c2a3a5eaa8b541c8af15ea5518bbff5f2fa") + .useDefaultPorts() .ipAddress("127.0.0.1") .listeningPort(30304) .build(); diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/util/LocalPermissioningConfigurationValidatorTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/util/LocalPermissioningConfigurationValidatorTest.java index 92168f126e..90d183364e 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/util/LocalPermissioningConfigurationValidatorTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/util/LocalPermissioningConfigurationValidatorTest.java @@ -19,10 +19,14 @@ import tech.pegasys.pantheon.cli.EthNetworkConfig; import tech.pegasys.pantheon.cli.NetworkName; import tech.pegasys.pantheon.ethereum.permissioning.LocalPermissioningConfiguration; import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfigurationBuilder; +import tech.pegasys.pantheon.util.enode.EnodeURL; +import java.net.URI; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; +import java.util.stream.Collectors; import com.google.common.io.Resources; import org.junit.Test; @@ -47,8 +51,10 @@ public class LocalPermissioningConfigurationValidatorTest { PermissioningConfigurationBuilder.permissioningConfiguration( true, toml.toAbsolutePath().toString(), true, toml.toAbsolutePath().toString()); + final List enodeURIs = + ethNetworkConfig.getBootNodes().stream().map(EnodeURL::toURI).collect(Collectors.toList()); PermissioningConfigurationValidator.areAllNodesAreInWhitelist( - ethNetworkConfig.getBootNodes(), permissioningConfiguration); + enodeURIs, permissioningConfiguration); } @Test @@ -66,8 +72,12 @@ public class LocalPermissioningConfigurationValidatorTest { true, toml.toAbsolutePath().toString(), true, toml.toAbsolutePath().toString()); try { + final List enodeURIs = + ethNetworkConfig.getBootNodes().stream() + .map(EnodeURL::toURI) + .collect(Collectors.toList()); PermissioningConfigurationValidator.areAllNodesAreInWhitelist( - ethNetworkConfig.getBootNodes(), permissioningConfiguration); + enodeURIs, permissioningConfiguration); fail("expected exception because ropsten bootnodes are not in node-whitelist"); } catch (Exception e) { assertThat(e.getMessage()).startsWith("Specified node(s) not in nodes-whitelist"); diff --git a/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java b/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java index 0b8eb60400..d0b4f2bd6c 100644 --- a/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java +++ b/util/src/main/java/tech/pegasys/pantheon/util/enode/EnodeURL.java @@ -13,6 +13,7 @@ package tech.pegasys.pantheon.util.enode; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; import tech.pegasys.pantheon.util.NetworkUtility; import tech.pegasys.pantheon.util.bytes.BytesValue; @@ -37,9 +38,7 @@ public class EnodeURL { private final BytesValue nodeId; private final InetAddress ip; - private final Integer listeningPort; - // DiscoveryPort will only be present if it differs from listening port, otherwise - // the discovery port is assumed to match the listening port + private final OptionalInt listeningPort; private final OptionalInt discoveryPort; // Error messages @@ -49,12 +48,12 @@ public class EnodeURL { private EnodeURL( final BytesValue nodeId, final InetAddress address, - final Integer listeningPort, + final OptionalInt listeningPort, final OptionalInt discoveryPort) { checkArgument( nodeId.size() == NODE_ID_SIZE, "Invalid node id. Expected id of length: 64 bytes."); checkArgument( - NetworkUtility.isValidPort(listeningPort), + !listeningPort.isPresent() || NetworkUtility.isValidPort(listeningPort.getAsInt()), "Invalid listening port. Port should be between 1 - 65535."); checkArgument( !discoveryPort.isPresent() || NetworkUtility.isValidPort(discoveryPort.getAsInt()), @@ -63,12 +62,7 @@ public class EnodeURL { this.nodeId = nodeId; this.ip = address; this.listeningPort = listeningPort; - // Only explicitly define a discovery port if it differs from the listening port - if (discoveryPort.isPresent() && discoveryPort.getAsInt() != listeningPort) { - this.discoveryPort = discoveryPort; - } else { - this.discoveryPort = OptionalInt.empty(); - } + this.discoveryPort = discoveryPort; } public static Builder builder() { @@ -99,6 +93,10 @@ public class EnodeURL { uri.getScheme().equalsIgnoreCase("enode"), "Invalid URI scheme (must equal \"enode\")."); checkArgument(NODE_ID_PATTERN.matcher(uri.getUserInfo()).matches(), INVALID_NODE_ID_LENGTH); + final BytesValue id = BytesValue.fromHexString(uri.getUserInfo()); + String host = uri.getHost(); + int tcpPort = uri.getPort(); + // Parse discport if it exists OptionalInt discoveryPort = OptionalInt.empty(); String query = uri.getQuery(); @@ -109,12 +107,10 @@ public class EnodeURL { discoveryPort = discPort == null ? discoveryPort : OptionalInt.of(discPort); } checkArgument(discoveryPort.isPresent(), "Invalid discovery port: '" + query + "'."); + } else { + discoveryPort = OptionalInt.of(tcpPort); } - final BytesValue id = BytesValue.fromHexString(uri.getUserInfo()); - String host = uri.getHost(); - int tcpPort = uri.getPort(); - return builder() .ipAddress(host) .nodeId(id) @@ -127,18 +123,43 @@ public class EnodeURL { checkArgument(argument != null && !argument.trim().isEmpty(), message); } + public static boolean sameListeningEndpoint(final EnodeURL enodeA, final EnodeURL enodeB) { + if (enodeA == null || enodeB == null) { + return false; + } + + return Objects.equal(enodeA.nodeId, enodeB.nodeId) + && Objects.equal(enodeA.ip, enodeB.ip) + && Objects.equal(enodeA.listeningPort, enodeB.listeningPort); + } + public URI toURI() { final String uri = String.format( "enode://%s@%s:%d", - nodeId.toUnprefixedString(), InetAddresses.toUriString(ip), listeningPort); - if (discoveryPort.isPresent()) { - return URI.create(uri + String.format("?discport=%d", discoveryPort.getAsInt())); + nodeId.toUnprefixedString(), InetAddresses.toUriString(ip), getListeningPortOrZero()); + final OptionalInt discPort = getDiscPortQueryParam(); + if (discPort.isPresent()) { + return URI.create(uri + String.format("?discport=%d", discPort.getAsInt())); } else { return URI.create(uri); } } + /** + * Returns the discovery port only if it differs from the listening port + * + * @return + */ + private OptionalInt getDiscPortQueryParam() { + final int listeningPort = getListeningPortOrZero(); + final int discoveryPort = getDiscoveryPortOrZero(); + if (listeningPort == discoveryPort) { + return OptionalInt.empty(); + } + return OptionalInt.of(discoveryPort); + } + public static URI asURI(final String url) { return fromString(url).toURI(); } @@ -155,26 +176,28 @@ public class EnodeURL { return ip; } - public int getListeningPort() { + public boolean isListening() { + return listeningPort.isPresent(); + } + + public boolean isRunningDiscovery() { + return discoveryPort.isPresent(); + } + + public OptionalInt getListeningPort() { return listeningPort; } - public OptionalInt getDiscoveryPort() { - return discoveryPort; + public int getListeningPortOrZero() { + return listeningPort.orElse(0); } - /** - * @return Returns the discovery port if explicitly specified, otherwise returns the listening - * port. - */ - public int getEffectiveDiscoveryPort() { - return discoveryPort.orElse(listeningPort); + public OptionalInt getDiscoveryPort() { + return discoveryPort; } - public boolean sameEndpoint(final EnodeURL enode) { - return Objects.equal(nodeId, enode.nodeId) - && Objects.equal(ip, enode.ip) - && Objects.equal(listeningPort, enode.listeningPort); + public int getDiscoveryPortOrZero() { + return discoveryPort.orElse(0); } @Override @@ -205,16 +228,31 @@ public class EnodeURL { public static class Builder { private BytesValue nodeId; - private Integer listeningPort = 30303; - private OptionalInt discoveryPort = OptionalInt.empty(); + private OptionalInt listeningPort; + private OptionalInt discoveryPort; private InetAddress ip; private Builder() {}; public EnodeURL build() { + validate(); return new EnodeURL(nodeId, ip, listeningPort, discoveryPort); } + private void validate() { + checkState(nodeId != null, "Node id must be configured."); + checkState(listeningPort != null, "Listening port must be configured."); + checkState(discoveryPort != null, "Discovery port must be configured."); + checkState(ip != null, "Ip address must be configured."); + } + + public Builder configureFromEnode(final EnodeURL enode) { + return this.nodeId(enode.getNodeId()) + .listeningPort(enode.getListeningPort()) + .discoveryPort(enode.getDiscoveryPort()) + .ipAddress(enode.getIp()); + } + public Builder nodeId(final BytesValue nodeId) { this.nodeId = nodeId; return this; @@ -246,18 +284,92 @@ public class EnodeURL { return this; } - public Builder listeningPort(final Integer listeningPort) { - this.listeningPort = listeningPort; + public Builder discoveryAndListeningPorts(final int listeningAndDiscoveryPorts) { + listeningPort(listeningAndDiscoveryPorts); + discoveryPort(listeningAndDiscoveryPorts); + return this; + } + + public Builder disableListening() { + this.listeningPort = OptionalInt.empty(); + return this; + } + + public Builder disableDiscovery() { + this.discoveryPort = OptionalInt.empty(); + return this; + } + + public Builder useDefaultPorts() { + discoveryAndListeningPorts(EnodeURL.DEFAULT_LISTENING_PORT); + return this; + } + + /** + * An optional listening port value. If the value is empty of equal to 0, the listening port + * will be empty - indicating the corresponding node is not listening. + * + * @param listeningPort If non-empty represents the port to listen on, if empty means the node + * is not listening + * @return The modified builder + */ + public Builder listeningPort(final OptionalInt listeningPort) { + if (listeningPort.isPresent() && listeningPort.getAsInt() == 0) { + this.listeningPort = OptionalInt.empty(); + } else { + this.listeningPort = listeningPort; + } + return this; + } + + /** + * An listening port value. A value of 0 means the node is not listening. + * + * @param listeningPort If non-zero, represents the port on which to listen for connections. A + * value of 0 means the node is not listening for connections. + * @return The modified builder + */ + public Builder listeningPort(final int listeningPort) { + if (listeningPort == 0) { + this.listeningPort = OptionalInt.empty(); + } else { + this.listeningPort = OptionalInt.of(listeningPort); + } return this; } + /** + * The port on which to listen for discovery messages. A value that is empty or equal to 0, + * indicates that the node is not listening for discovery messages. + * + * @param discoveryPort If non-empty and non-zero, represents the port on which to listen for + * discovery messages. Otherwise, indicates that the node is not running discovery. + * @return The modified builder + */ public Builder discoveryPort(final OptionalInt discoveryPort) { - this.discoveryPort = discoveryPort; + if (discoveryPort.isPresent() && discoveryPort.getAsInt() == 0) { + this.discoveryPort = OptionalInt.empty(); + } else { + this.discoveryPort = discoveryPort; + } + return this; } + /** + * The port on which to listen for discovery messages. A value that is equal to 0, indicates + * that the node is not listening for discovery messages. + * + * @param discoveryPort If non-zero, represents the port on which to listen for discovery + * messages. Otherwise, indicates that the node is not running discovery. + * @return The modified builder + */ public Builder discoveryPort(final int discoveryPort) { - this.discoveryPort = OptionalInt.of(discoveryPort); + if (discoveryPort == 0) { + this.discoveryPort = OptionalInt.empty(); + } else { + this.discoveryPort = OptionalInt.of(discoveryPort); + } return this; } } diff --git a/util/src/test/java/tech/pegasys/pantheon/util/enode/EnodeURLTest.java b/util/src/test/java/tech/pegasys/pantheon/util/enode/EnodeURLTest.java index 323f890b9f..e021a5bbf5 100644 --- a/util/src/test/java/tech/pegasys/pantheon/util/enode/EnodeURLTest.java +++ b/util/src/test/java/tech/pegasys/pantheon/util/enode/EnodeURLTest.java @@ -26,14 +26,14 @@ public class EnodeURLTest { private final String VALID_NODE_ID = "6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0"; private final String IPV4_ADDRESS = "192.168.0.1"; - private final String IPV6_FULL_ADDRESS = "[2001:db8:85a3:0:0:8a2e:0370:7334]"; - private final String IPV6_COMPACT_ADDRESS = "[2001:db8:85a3::8a2e:0370:7334]"; + private final String IPV6_FULL_ADDRESS = "[2001:db8:85a3:0:0:8a2e:370:7334]"; + private final String IPV6_COMPACT_ADDRESS = "[2001:db8:85a3::8a2e:370:7334]"; private final int P2P_PORT = 30303; private final int DISCOVERY_PORT = 30301; private final String DISCOVERY_QUERY = "discport=" + DISCOVERY_PORT; @Test - public void new_withMatchingDiscoveryAndListeningPorts() { + public void build_withMatchingDiscoveryAndListeningPorts() { final EnodeURL enode = EnodeURL.builder() .nodeId(VALID_NODE_ID) @@ -41,13 +41,12 @@ public class EnodeURLTest { .listeningPort(P2P_PORT) .discoveryPort(OptionalInt.of(P2P_PORT)) .build(); - assertThat(enode.getListeningPort()).isEqualTo(P2P_PORT); - // A discovery port matching the listening port should not be explicitly specified - assertThat(enode.getDiscoveryPort()).isEmpty(); + assertThat(enode.getListeningPortOrZero()).isEqualTo(P2P_PORT); + assertThat(enode.getDiscoveryPortOrZero()).isEqualTo(P2P_PORT); } @Test - public void new_withNonMatchingDiscoveryAndListeningPorts() { + public void build_withNonMatchingDiscoveryAndListeningPorts() { final EnodeURL enode = EnodeURL.builder() .nodeId(VALID_NODE_ID) @@ -55,9 +54,8 @@ public class EnodeURLTest { .listeningPort(P2P_PORT) .discoveryPort(OptionalInt.of(DISCOVERY_PORT)) .build(); - assertThat(enode.getListeningPort()).isEqualTo(P2P_PORT); - // A discovery port matching the listening port should not be explicitly specified - assertThat(enode.getDiscoveryPort()).isEqualTo(OptionalInt.of(DISCOVERY_PORT)); + assertThat(enode.getListeningPortOrZero()).isEqualTo(P2P_PORT); + assertThat(enode.getDiscoveryPortOrZero()).isEqualTo(DISCOVERY_PORT); } @Test @@ -75,6 +73,7 @@ public class EnodeURLTest { final EnodeURL enodeURL = EnodeURL.fromString(enodeURLString); assertThat(enodeURL).isEqualTo(expectedEnodeURL); + assertThat(enodeURL.toString()).isEqualTo(enodeURLString); } @Test @@ -83,13 +82,14 @@ public class EnodeURLTest { EnodeURL.builder() .nodeId(VALID_NODE_ID) .ipAddress(IPV4_ADDRESS) - .listeningPort(P2P_PORT) + .discoveryAndListeningPorts(P2P_PORT) .build(); final String enodeURLString = "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":" + P2P_PORT; final EnodeURL enodeURL = EnodeURL.fromString(enodeURLString); assertThat(enodeURL).isEqualTo(expectedEnodeURL); + assertThat(enodeURL.toString()).isEqualTo(enodeURLString); } @Test @@ -117,7 +117,7 @@ public class EnodeURLTest { } @Test - public void fromString_ithIPV6InCompactFormShouldBuildExpectedEnodeURLObject() { + public void fromString_withIPV6InCompactFormShouldBuildExpectedEnodeURLObject() { final EnodeURL expectedEnodeURL = EnodeURL.builder() .nodeId(VALID_NODE_ID) @@ -138,6 +138,54 @@ public class EnodeURLTest { final EnodeURL enodeURL = EnodeURL.fromString(enodeURLString); assertThat(enodeURL).isEqualTo(expectedEnodeURL); + assertThat(enodeURL.toString()).isEqualTo(enodeURLString); + } + + @Test + public void fromString_with0ValuedDiscoveryPort() { + final String query = "discport=0"; + final String enodeURLString = + "enode://" + VALID_NODE_ID + "@" + IPV6_COMPACT_ADDRESS + ":" + P2P_PORT + "?" + query; + + EnodeURL enodeURL = EnodeURL.fromString(enodeURLString); + assertThat(enodeURL.getNodeId().toUnprefixedString()).isEqualTo(VALID_NODE_ID); + assertThat("[" + enodeURL.getIpAsString() + "]").isEqualTo(IPV6_FULL_ADDRESS); + assertThat(enodeURL.getListeningPort()).isEqualTo(OptionalInt.of(P2P_PORT)); + assertThat(enodeURL.getDiscoveryPortOrZero()).isEqualTo(0); + assertThat(enodeURL.isListening()).isTrue(); + assertThat(enodeURL.isRunningDiscovery()).isFalse(); + + assertThat(enodeURL.toString()).isEqualTo(enodeURLString); + } + + @Test + public void fromString_with0ValuedListeningPort() { + final String enodeURLString = "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":" + 0; + + EnodeURL enodeURL = EnodeURL.fromString(enodeURLString); + assertThat(enodeURL.getNodeId().toUnprefixedString()).isEqualTo(VALID_NODE_ID); + assertThat(enodeURL.getIpAsString()).isEqualTo(IPV4_ADDRESS); + assertThat(enodeURL.getListeningPortOrZero()).isEqualTo(0); + assertThat(enodeURL.getDiscoveryPortOrZero()).isEqualTo(0); + assertThat(enodeURL.isListening()).isFalse(); + assertThat(enodeURL.isRunningDiscovery()).isFalse(); + + assertThat(enodeURL.toString()).isEqualTo(enodeURLString); + } + + @Test + public void fromString_with0ValuedListeningPortAndExplicit0ValuedDiscPort() { + final String query = "discport=0"; + final String enodeURLString = + "enode://" + VALID_NODE_ID + "@" + IPV4_ADDRESS + ":" + 0 + "?" + query; + + EnodeURL enodeURL = EnodeURL.fromString(enodeURLString); + assertThat(enodeURL.getNodeId().toUnprefixedString()).isEqualTo(VALID_NODE_ID); + assertThat(enodeURL.getIpAsString()).isEqualTo(IPV4_ADDRESS); + assertThat(enodeURL.getListeningPortOrZero()).isEqualTo(0); + assertThat(enodeURL.getDiscoveryPortOrZero()).isEqualTo(0); + assertThat(enodeURL.isListening()).isFalse(); + assertThat(enodeURL.isRunningDiscovery()).isFalse(); } @Test @@ -318,6 +366,15 @@ public class EnodeURLTest { .hasMessageContaining("Invalid empty value."); } + @Test + public void fromStringInvalidNodeIdLengthHasDescriptiveMessage() { + String invalidEnodeURL = + String.format("enode://%s@%s:%d", VALID_NODE_ID.substring(1), IPV4_ADDRESS, P2P_PORT); + assertThatThrownBy(() -> EnodeURL.fromString(invalidEnodeURL)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("node ID must have exactly 128 hexadecimal"); + } + @Test public void toURI_WithDiscoveryPortCreateExpectedURI() { final String enodeURLString = @@ -338,41 +395,333 @@ public class EnodeURLTest { } @Test - public void getEffectiveDiscoveryPort_withMatchingDiscoveryAndListeningPorts() { - final EnodeURL enode = + public void builder_setEachPortExplicitly() { + final EnodeURL enodeURL = EnodeURL.builder() .nodeId(VALID_NODE_ID) .ipAddress(IPV4_ADDRESS) .listeningPort(P2P_PORT) - .discoveryPort(OptionalInt.of(P2P_PORT)) + .discoveryPort(DISCOVERY_PORT) .build(); - assertThat(enode.getListeningPort()).isEqualTo(P2P_PORT); - // A discovery port matching the listening port should not be explicitly specified - assertThat(enode.getDiscoveryPort()).isEmpty(); - assertThat(enode.getEffectiveDiscoveryPort()).isEqualTo(P2P_PORT); + + assertThat(enodeURL.getNodeId().toUnprefixedString()).isEqualTo(VALID_NODE_ID); + assertThat(enodeURL.getIpAsString()).isEqualTo(IPV4_ADDRESS); + assertThat(enodeURL.getListeningPortOrZero()).isEqualTo(P2P_PORT); + assertThat(enodeURL.getDiscoveryPortOrZero()).isEqualTo(DISCOVERY_PORT); + assertThat(enodeURL.isListening()).isTrue(); + assertThat(enodeURL.isRunningDiscovery()).isTrue(); } @Test - public void getEffectiveDiscoveryPort_withDistinctDiscoveryAndListeningPorts() { - final EnodeURL enode = + public void builder_setPortsTogether() { + final EnodeURL enodeURL = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .discoveryAndListeningPorts(P2P_PORT) + .build(); + + assertThat(enodeURL.getNodeId().toUnprefixedString()).isEqualTo(VALID_NODE_ID); + assertThat(enodeURL.getIpAsString()).isEqualTo(IPV4_ADDRESS); + assertThat(enodeURL.getListeningPortOrZero()).isEqualTo(P2P_PORT); + assertThat(enodeURL.getDiscoveryPortOrZero()).isEqualTo(P2P_PORT); + assertThat(enodeURL.isListening()).isTrue(); + assertThat(enodeURL.isRunningDiscovery()).isTrue(); + } + + @Test + public void builder_setDefaultPorts() { + final EnodeURL enodeURL = + EnodeURL.builder().nodeId(VALID_NODE_ID).ipAddress(IPV4_ADDRESS).useDefaultPorts().build(); + + assertThat(enodeURL.getNodeId().toUnprefixedString()).isEqualTo(VALID_NODE_ID); + assertThat(enodeURL.getIpAsString()).isEqualTo(IPV4_ADDRESS); + assertThat(enodeURL.getListeningPortOrZero()).isEqualTo(EnodeURL.DEFAULT_LISTENING_PORT); + assertThat(enodeURL.getDiscoveryPortOrZero()).isEqualTo(EnodeURL.DEFAULT_LISTENING_PORT); + assertThat(enodeURL.isListening()).isTrue(); + assertThat(enodeURL.isRunningDiscovery()).isTrue(); + } + + @Test + public void builder_discoveryDisabled() { + final EnodeURL enodeURL = EnodeURL.builder() .nodeId(VALID_NODE_ID) .ipAddress(IPV4_ADDRESS) .listeningPort(P2P_PORT) - .discoveryPort(OptionalInt.of(DISCOVERY_PORT)) + .disableDiscovery() .build(); - assertThat(enode.getListeningPort()).isEqualTo(P2P_PORT); - // A discovery port matching the listening port should not be explicitly specified - assertThat(enode.getDiscoveryPort()).isEqualTo(OptionalInt.of(DISCOVERY_PORT)); - assertThat(enode.getEffectiveDiscoveryPort()).isEqualTo(DISCOVERY_PORT); + + assertThat(enodeURL.getNodeId().toUnprefixedString()).isEqualTo(VALID_NODE_ID); + assertThat(enodeURL.getIpAsString()).isEqualTo(IPV4_ADDRESS); + assertThat(enodeURL.getListeningPortOrZero()).isEqualTo(P2P_PORT); + assertThat(enodeURL.getDiscoveryPortOrZero()).isEqualTo(0); + assertThat(enodeURL.isListening()).isTrue(); + assertThat(enodeURL.isRunningDiscovery()).isFalse(); } @Test - public void fromStringInvalidNodeIdLengthHasDescriptiveMessage() { - String invalidEnodeURL = - String.format("enode://%s@%s:%d", VALID_NODE_ID.substring(1), IPV4_ADDRESS, P2P_PORT); - assertThatThrownBy(() -> EnodeURL.fromString(invalidEnodeURL)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("node ID must have exactly 128 hexadecimal"); + public void builder_listeningDisabled() { + final EnodeURL enodeURL = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .discoveryPort(P2P_PORT) + .disableListening() + .build(); + + assertThat(enodeURL.getNodeId().toUnprefixedString()).isEqualTo(VALID_NODE_ID); + assertThat(enodeURL.getIpAsString()).isEqualTo(IPV4_ADDRESS); + assertThat(enodeURL.getListeningPortOrZero()).isEqualTo(0); + assertThat(enodeURL.getDiscoveryPortOrZero()).isEqualTo(P2P_PORT); + assertThat(enodeURL.isListening()).isFalse(); + assertThat(enodeURL.isRunningDiscovery()).isTrue(); + } + + @Test + public void builder_listeningAndDiscoveryDisabled() { + final EnodeURL enodeURL = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .disableDiscovery() + .disableListening() + .build(); + + assertThat(enodeURL.getNodeId().toUnprefixedString()).isEqualTo(VALID_NODE_ID); + assertThat(enodeURL.getIpAsString()).isEqualTo(IPV4_ADDRESS); + assertThat(enodeURL.getListeningPortOrZero()).isEqualTo(0); + assertThat(enodeURL.getDiscoveryPortOrZero()).isEqualTo(0); + assertThat(enodeURL.isListening()).isFalse(); + assertThat(enodeURL.isRunningDiscovery()).isFalse(); + } + + @Test + public void builder_setPortsTo0() { + final EnodeURL enodeURL = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .discoveryAndListeningPorts(0) + .build(); + + assertThat(enodeURL.getNodeId().toUnprefixedString()).isEqualTo(VALID_NODE_ID); + assertThat(enodeURL.getIpAsString()).isEqualTo(IPV4_ADDRESS); + assertThat(enodeURL.getListeningPortOrZero()).isEqualTo(0); + assertThat(enodeURL.getDiscoveryPortOrZero()).isEqualTo(0); + assertThat(enodeURL.isListening()).isFalse(); + assertThat(enodeURL.isRunningDiscovery()).isFalse(); + } + + @Test + public void builder_setDiscoveryPortTo0() { + final EnodeURL enodeURL = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .discoveryPort(0) + .listeningPort(P2P_PORT) + .build(); + + assertThat(enodeURL.getNodeId().toUnprefixedString()).isEqualTo(VALID_NODE_ID); + assertThat(enodeURL.getIpAsString()).isEqualTo(IPV4_ADDRESS); + assertThat(enodeURL.getListeningPortOrZero()).isEqualTo(P2P_PORT); + assertThat(enodeURL.getDiscoveryPortOrZero()).isEqualTo(0); + assertThat(enodeURL.isListening()).isTrue(); + assertThat(enodeURL.isRunningDiscovery()).isFalse(); + } + + @Test + public void builder_setListeningPortTo0() { + final EnodeURL enodeURL = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .discoveryPort(P2P_PORT) + .listeningPort(0) + .build(); + + assertThat(enodeURL.getNodeId().toUnprefixedString()).isEqualTo(VALID_NODE_ID); + assertThat(enodeURL.getIpAsString()).isEqualTo(IPV4_ADDRESS); + assertThat(enodeURL.getListeningPortOrZero()).isEqualTo(0); + assertThat(enodeURL.getDiscoveryPortOrZero()).isEqualTo(P2P_PORT); + assertThat(enodeURL.isListening()).isFalse(); + assertThat(enodeURL.isRunningDiscovery()).isTrue(); + } + + @Test + public void builder_setDiscoveryPortToEmptyValue() { + final EnodeURL enodeURL = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .discoveryPort(OptionalInt.empty()) + .listeningPort(P2P_PORT) + .build(); + + assertThat(enodeURL.getNodeId().toUnprefixedString()).isEqualTo(VALID_NODE_ID); + assertThat(enodeURL.getIpAsString()).isEqualTo(IPV4_ADDRESS); + assertThat(enodeURL.getListeningPortOrZero()).isEqualTo(P2P_PORT); + assertThat(enodeURL.getDiscoveryPortOrZero()).isEqualTo(0); + assertThat(enodeURL.isListening()).isTrue(); + assertThat(enodeURL.isRunningDiscovery()).isFalse(); + } + + @Test + public void builder_setListeningPortToEmptyValue() { + final EnodeURL enodeURL = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .discoveryPort(P2P_PORT) + .listeningPort(OptionalInt.empty()) + .build(); + + assertThat(enodeURL.getNodeId().toUnprefixedString()).isEqualTo(VALID_NODE_ID); + assertThat(enodeURL.getIpAsString()).isEqualTo(IPV4_ADDRESS); + assertThat(enodeURL.getListeningPortOrZero()).isEqualTo(0); + assertThat(enodeURL.getDiscoveryPortOrZero()).isEqualTo(P2P_PORT); + assertThat(enodeURL.isListening()).isFalse(); + assertThat(enodeURL.isRunningDiscovery()).isTrue(); + } + + @Test + public void builder_discoveryNotSpecified() { + final EnodeURL.Builder builder = + EnodeURL.builder().nodeId(VALID_NODE_ID).ipAddress(IPV4_ADDRESS).listeningPort(P2P_PORT); + + assertThatThrownBy(builder::build) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Discovery port must be configured"); + } + + @Test + public void builder_listeningPortNotSpecified() { + final EnodeURL.Builder builder = + EnodeURL.builder().nodeId(VALID_NODE_ID).ipAddress(IPV4_ADDRESS).discoveryPort(P2P_PORT); + + assertThatThrownBy(builder::build) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Listening port must be configured"); + } + + @Test + public void builder_nodeIdNotSpecified() { + final EnodeURL.Builder builder = + EnodeURL.builder().ipAddress(IPV4_ADDRESS).discoveryAndListeningPorts(P2P_PORT); + + assertThatThrownBy(builder::build) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Node id must be configured"); + } + + @Test + public void builder_ipNotSpecified() { + final EnodeURL.Builder builder = + EnodeURL.builder().nodeId(VALID_NODE_ID).discoveryAndListeningPorts(P2P_PORT); + + assertThatThrownBy(builder::build) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Ip address must be configured"); + } + + @Test + public void sameListeningEndpoint_forMatchingEnodes() { + final EnodeURL enodeA = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .listeningPort(P2P_PORT) + .discoveryPort(DISCOVERY_PORT) + .build(); + final EnodeURL enodeB = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .listeningPort(P2P_PORT) + .discoveryPort(DISCOVERY_PORT + 1) + .build(); + + assertThat(EnodeURL.sameListeningEndpoint(enodeA, enodeB)).isTrue(); + } + + @Test + public void sameListeningEndpoint_differentListeningPorts() { + final EnodeURL enodeA = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .listeningPort(P2P_PORT) + .discoveryPort(DISCOVERY_PORT) + .build(); + final EnodeURL enodeB = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .listeningPort(P2P_PORT + 1) + .discoveryPort(DISCOVERY_PORT) + .build(); + + assertThat(EnodeURL.sameListeningEndpoint(enodeA, enodeB)).isFalse(); + } + + @Test + public void sameListeningEndpoint_differentIps() { + final EnodeURL enodeA = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV6_COMPACT_ADDRESS) + .listeningPort(P2P_PORT) + .discoveryPort(DISCOVERY_PORT) + .build(); + final EnodeURL enodeB = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .listeningPort(P2P_PORT) + .discoveryPort(DISCOVERY_PORT) + .build(); + + assertThat(EnodeURL.sameListeningEndpoint(enodeA, enodeB)).isFalse(); + } + + @Test + public void sameListeningEndpoint_listeningDisabledForOne() { + final EnodeURL enodeA = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .disableListening() + .discoveryPort(DISCOVERY_PORT) + .build(); + final EnodeURL enodeB = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .listeningPort(P2P_PORT) + .discoveryPort(DISCOVERY_PORT) + .build(); + + assertThat(EnodeURL.sameListeningEndpoint(enodeA, enodeB)).isFalse(); + } + + @Test + public void sameListeningEndpoint_listeningDisabledForBoth() { + final EnodeURL enodeA = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .disableListening() + .discoveryPort(DISCOVERY_PORT) + .build(); + final EnodeURL enodeB = + EnodeURL.builder() + .nodeId(VALID_NODE_ID) + .ipAddress(IPV4_ADDRESS) + .disableListening() + .discoveryPort(DISCOVERY_PORT) + .build(); + + assertThat(EnodeURL.sameListeningEndpoint(enodeA, enodeB)).isTrue(); } }