Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Update EnodeURL to support enodes with listening disabled #1403

Merged
merged 19 commits into from
May 13, 2019
Merged

Update EnodeURL to support enodes with listening disabled #1403

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
5e11699
Update EnodeURL to support enodes with listening disabled
mbaxter May 6, 2019
3c47032
Simplify method
mbaxter May 7, 2019
7ccc0eb
Return early if P2PNetwork is asked to connect to an invalid enode
mbaxter May 8, 2019
513ab3c
Add tests for parsing 0-valued ports
mbaxter May 8, 2019
977f55e
Add tests for EnodeURL.Builder
mbaxter May 8, 2019
042ef7b
Run code formatter
mbaxter May 8, 2019
5ebf7a3
Add tests for EnodeURL.sameListeningEndpoint()
mbaxter May 8, 2019
ca31fbf
Throw exception if PeerDiscoveryAgent receives invalid bootnodes
mbaxter May 9, 2019
3b41c03
Keep bootnodes as EnodeURLs within DiscoveryConfiguration
mbaxter May 10, 2019
bfa7c3f
Move bootnode validation to DiscoveryConfiguration
mbaxter May 10, 2019
2ef811b
Add some more tests, run code formatter
mbaxter May 10, 2019
dfd3ea0
Add missing graphql dependency to RunnerBuilder
mbaxter May 10, 2019
46af8ad
Fix CLI tests
mbaxter May 10, 2019
72ff9fa
Quieter logs
mbaxter May 10, 2019
b40ab18
Require static-nodes to have a non-zero listening port
mbaxter May 10, 2019
762aa06
Require maintained peers to have a non-zero listening port
mbaxter May 10, 2019
224e64a
Update localConfig node permissioning ignore discovery port
mbaxter May 10, 2019
069777c
Address review comments
mbaxter May 13, 2019
51d46ba
Merge branch 'master' into PAN-2616/make-enode-ports-optional
mbaxter May 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions acceptance-tests/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
11 changes: 10 additions & 1 deletion ...c/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,24 @@
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;
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;
Expand All @@ -56,9 +60,13 @@ public void startNode(final PantheonNode node) {
}

final MetricsSystem noOpMetricsSystem = new NoOpMetricsSystem();
final List<EnodeURL> 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 =
Expand Down Expand Up @@ -104,6 +112,7 @@ public void startNode(final PantheonNode node) {
.metricsSystem(noOpMetricsSystem)
.metricsConfiguration(node.metricsConfiguration())
.p2pEnabled(node.isP2pEnabled())
.graphQLRpcConfiguration(GraphQLRpcConfiguration.createDefault())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this was failing on my branch and not master - was getting a null pointer exception because this dependency wasn't set ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any ideas @shemnon ??

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this was missed.

The big issue I see is that the threaded acceptance runner is only ever run by developers, the process runner is run on the jenkins boxes instead and would have broke there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, so maybe it was just a flaky test and when I ran it locally I got the null pointer?

.build();

runner.start();
Expand Down
2 changes: 1 addition & 1 deletion ...reum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/manager/MockPeerConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public MockPeerConnection(final Set<Capability> caps, final PeerSendHandler onSe
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).ports(30303).build());
this.peerInfo = new PeerInfo(5, "Mock", new ArrayList<>(caps), 30303, nodeId);
}

Expand Down
2 changes: 1 addition & 1 deletion ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TestNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ public String toString() {
+ "@"
+ selfPeer.getEnodeURL().getIpAsString()
+ ':'
+ selfPeer.getEnodeURL().getListeningPort();
+ selfPeer.getEnodeURL().getListeningPortOrZero();
}

public void receiveRemoteTransaction(final Transaction transaction) {
Expand Down
12 changes: 9 additions & 3 deletions .../src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminNodeInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,19 @@ public JsonRpcResponse response(final JsonRpcRequest req) {

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();
Expand Down
4 changes: 3 additions & 1 deletion ...pc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/NetServices.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,13 @@ public JsonRpcResponse response(final JsonRpcRequest req) {
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(
Expand Down
125 changes: 125 additions & 0 deletions .../test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/AdminNodeInfoTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
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;
Expand Down Expand Up @@ -119,6 +120,130 @@ public void shouldReturnCorrectResult() {
assertThat(actual.getResult()).isEqualTo(expected);
}

@Test
public void handlesLocalEnodeWithListeningAndDiscoveryDisabled() {
final EnodeURL localEnode =
EnodeURL.builder().nodeId(nodeId).ipAddress("1.2.3.4").ports(0).build();

when(p2pNetwork.isP2pEnabled()).thenReturn(true);
when(p2pNetwork.getLocalEnode()).thenReturn(Optional.of(localEnode));
final JsonRpcRequest request = adminNodeInfo();

final Map<String, Object> 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").ports(0).discoveryPort(7890).build();

when(p2pNetwork.isP2pEnabled()).thenReturn(true);
when(p2pNetwork.getLocalEnode()).thenReturn(Optional.of(localEnode));
final JsonRpcRequest request = adminNodeInfo();

final Map<String, Object> 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").ports(0).listeningPort(7890).build();

when(p2pNetwork.isP2pEnabled()).thenReturn(true);
when(p2pNetwork.getLocalEnode()).thenReturn(Optional.of(localEnode));
final JsonRpcRequest request = adminNodeInfo();

final Map<String, Object> 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);
Expand Down
6 changes: 5 additions & 1 deletion ethereum/mock-p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/testing/MockNetwork.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,11 @@ public Peer getPeer() {
@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
Expand Down
9 changes: 6 additions & 3 deletions .../main/java/tech/pegasys/pantheon/ethereum/p2p/InsufficientPeersPermissioningProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ public InsufficientPeersPermissioningProvider(

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() {
Expand All @@ -74,8 +76,9 @@ && checkEnode(maybeSelfEnode.get(), destinationEnode)) {
}

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) {
Expand Down
Loading