From 7deda4eaa6707513844b5afd8635e9da9e6c9760 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Wed, 23 Jan 2019 13:29:25 +1000 Subject: [PATCH] NC 2004 no discovery still talk to bootnodes (#624) * added test and DSL for node with --no-discovery * reversed boolean for discovery * changed to Boolean and arity = 1 Signed-off-by: Adrian Sutton --- .../ClusterNoDiscoveryAcceptanceTest.java | 50 +++++++++++++++++++ .../acceptance/dsl/node/PantheonNode.java | 10 +++- .../dsl/node/ProcessPantheonNodeRunner.java | 5 ++ .../dsl/node/ThreadPantheonNodeRunner.java | 2 +- .../factory/PantheonFactoryConfiguration.java | 9 +++- .../PantheonFactoryConfigurationBuilder.java | 9 +++- .../dsl/node/factory/PantheonNodeFactory.java | 8 ++- .../ethereum/p2p/wire/Capability.java | 3 +- .../pegasys/pantheon/cli/PantheonCommand.java | 3 +- .../pantheon/cli/PantheonCommandTest.java | 16 ++++-- 10 files changed, 103 insertions(+), 12 deletions(-) create mode 100644 acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/ClusterNoDiscoveryAcceptanceTest.java diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/ClusterNoDiscoveryAcceptanceTest.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/ClusterNoDiscoveryAcceptanceTest.java new file mode 100644 index 0000000000..11fbf391df --- /dev/null +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/ClusterNoDiscoveryAcceptanceTest.java @@ -0,0 +1,50 @@ +/* + * Copyright 2018 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.tests.acceptance; + +import tech.pegasys.pantheon.tests.acceptance.dsl.AcceptanceTestBase; +import tech.pegasys.pantheon.tests.acceptance.dsl.node.Node; +import tech.pegasys.pantheon.tests.acceptance.dsl.node.cluster.Cluster; +import tech.pegasys.pantheon.tests.acceptance.dsl.node.cluster.ClusterConfiguration; +import tech.pegasys.pantheon.tests.acceptance.dsl.node.cluster.ClusterConfigurationBuilder; + +import org.junit.Before; +import org.junit.Test; + +public class ClusterNoDiscoveryAcceptanceTest extends AcceptanceTestBase { + + private Node fullNode; + private Node noDiscoveryNode; + private Cluster noDiscoveryCluster; + + @Before + public void setUp() throws Exception { + final ClusterConfiguration clusterConfiguration = + new ClusterConfigurationBuilder().setAwaitPeerDiscovery(false).build(); + noDiscoveryCluster = new Cluster(clusterConfiguration, net); + noDiscoveryNode = pantheon.createNodeWithNoDiscovery("noDiscovery"); + fullNode = pantheon.createArchiveNode("node2"); + noDiscoveryCluster.start(noDiscoveryNode, fullNode); + } + + @Test + public void shouldNotConnectToOtherPeer() { + fullNode.verify(net.awaitPeerCount(0)); + } + + @Override + public void tearDownAcceptanceTestBase() { + noDiscoveryCluster.stop(); + super.tearDownAcceptanceTestBase(); + } +} diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/PantheonNode.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/PantheonNode.java index c1a53eb75f..7091374bd4 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/PantheonNode.java +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/PantheonNode.java @@ -75,6 +75,7 @@ public class PantheonNode implements Node, NodeConfiguration, RunnableNode, Auto private final PermissioningConfiguration permissioningConfiguration; private final GenesisConfigProvider genesisConfigProvider; private final boolean devMode; + private final boolean discoveryEnabled; private List bootnodes = new ArrayList<>(); private PantheonWeb3j pantheonWeb3j; @@ -90,7 +91,8 @@ public class PantheonNode implements Node, NodeConfiguration, RunnableNode, Auto final boolean devMode, final GenesisConfigProvider genesisConfigProvider, final int p2pPort, - final Boolean p2pEnabled) + final Boolean p2pEnabled, + final boolean discovery) throws IOException { this.homeDirectory = Files.createTempDirectory("acctest"); this.keyPair = KeyPairUtil.loadKeyPair(homeDirectory); @@ -104,6 +106,7 @@ public class PantheonNode implements Node, NodeConfiguration, RunnableNode, Auto this.genesisConfigProvider = genesisConfigProvider; this.devMode = devMode; this.p2pEnabled = p2pEnabled; + this.discoveryEnabled = discovery; LOG.info("Created PantheonNode {}", this.toString()); } @@ -333,6 +336,10 @@ public class PantheonNode implements Node, NodeConfiguration, RunnableNode, Auto return devMode; } + public boolean isDiscoveryEnabled() { + return discoveryEnabled; + } + PermissioningConfiguration getPermissioningConfiguration() { return permissioningConfiguration; } @@ -345,6 +352,7 @@ public class PantheonNode implements Node, NodeConfiguration, RunnableNode, Auto .add("homeDirectory", homeDirectory) .add("keyPair", keyPair) .add("p2pEnabled", p2pEnabled) + .add("discoveryEnabled", discoveryEnabled) .toString(); } diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ProcessPantheonNodeRunner.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ProcessPantheonNodeRunner.java index eb04b3d7db..b0ec5e4444 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ProcessPantheonNodeRunner.java +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ProcessPantheonNodeRunner.java @@ -58,6 +58,11 @@ public class ProcessPantheonNodeRunner implements PantheonNodeRunner { params.add("--dev-mode"); } + if (!node.isDiscoveryEnabled()) { + params.add("--no-discovery"); + params.add("true"); + } + params.add("--p2p-listen"); params.add(node.p2pListenAddress()); 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 3519bf4be8..15079ebe66 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 @@ -77,7 +77,7 @@ public class ThreadPantheonNodeRunner implements PantheonNodeRunner { new RunnerBuilder() .vertx(Vertx.vertx()) .pantheonController(pantheonController) - .discovery(true) + .discovery(node.isDiscoveryEnabled()) .bootstrapPeers(node.bootnodes()) .discoveryHost(node.hostName()) .discoveryPort(node.p2pPort()) diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonFactoryConfiguration.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonFactoryConfiguration.java index 6c7f6e6f22..876508f4a7 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonFactoryConfiguration.java +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonFactoryConfiguration.java @@ -28,6 +28,7 @@ class PantheonFactoryConfiguration { private final MetricsConfiguration metricsConfiguration; private final PermissioningConfiguration permissioningConfiguration; private final boolean devMode; + private final Boolean discoveryEnabled; private final GenesisConfigProvider genesisConfigProvider; private final Boolean p2pEnabled; @@ -40,7 +41,8 @@ class PantheonFactoryConfiguration { final PermissioningConfiguration permissioningConfiguration, final boolean devMode, final GenesisConfigProvider genesisConfigProvider, - final Boolean p2pEnabled) { + final Boolean p2pEnabled, + final Boolean discoveryEnabled) { this.name = name; this.miningParameters = miningParameters; this.jsonRpcConfiguration = jsonRpcConfiguration; @@ -50,6 +52,7 @@ class PantheonFactoryConfiguration { this.devMode = devMode; this.genesisConfigProvider = genesisConfigProvider; this.p2pEnabled = p2pEnabled; + this.discoveryEnabled = discoveryEnabled; } public String getName() { @@ -80,6 +83,10 @@ class PantheonFactoryConfiguration { return devMode; } + public Boolean isDiscoveryEnabled() { + return discoveryEnabled; + } + public GenesisConfigProvider getGenesisConfigProvider() { return genesisConfigProvider; } diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonFactoryConfigurationBuilder.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonFactoryConfigurationBuilder.java index 388a4c43a4..d9dcd3d79d 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonFactoryConfigurationBuilder.java +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonFactoryConfigurationBuilder.java @@ -35,6 +35,7 @@ public class PantheonFactoryConfigurationBuilder { private PermissioningConfiguration permissioningConfiguration = PermissioningConfiguration.createDefault(); private boolean devMode = true; + private Boolean discoveryEnabled = true; private GenesisConfigProvider genesisConfigProvider = ignore -> Optional.empty(); private Boolean p2pEnabled = true; @@ -108,6 +109,11 @@ public class PantheonFactoryConfigurationBuilder { return this; } + public PantheonFactoryConfigurationBuilder setDiscoveryEnabled(final Boolean discoveryEnabled) { + this.discoveryEnabled = discoveryEnabled; + return this; + } + public PantheonFactoryConfigurationBuilder setP2pEnabled(final Boolean p2pEnabled) { this.p2pEnabled = p2pEnabled; return this; @@ -123,6 +129,7 @@ public class PantheonFactoryConfigurationBuilder { permissioningConfiguration, devMode, genesisConfigProvider, - p2pEnabled); + p2pEnabled, + discoveryEnabled); } } diff --git a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonNodeFactory.java b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonNodeFactory.java index 3e66ea3640..18054fab0a 100644 --- a/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonNodeFactory.java +++ b/acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/factory/PantheonNodeFactory.java @@ -58,7 +58,8 @@ public class PantheonNodeFactory { config.isDevMode(), config.getGenesisConfigProvider(), serverSocket.getLocalPort(), - config.getP2pEnabled()); + config.getP2pEnabled(), + config.isDiscoveryEnabled()); serverSocket.close(); return node; @@ -190,6 +191,11 @@ public class PantheonNodeFactory { .build()); } + public PantheonNode createNodeWithNoDiscovery(final String name) throws IOException { + return create( + new PantheonFactoryConfigurationBuilder().setName(name).setDiscoveryEnabled(false).build()); + } + public PantheonNode createCliqueNode(final String name) throws IOException { return create( new PantheonFactoryConfigurationBuilder() diff --git a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/Capability.java b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/Capability.java index 3e67caba35..baa627b6db 100644 --- a/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/Capability.java +++ b/ethereum/p2p/src/main/java/tech/pegasys/pantheon/ethereum/p2p/wire/Capability.java @@ -24,8 +24,7 @@ import java.util.Objects; /** * Represents a client capability. * - * @see Capability wire + * @see Capability wire * format */ public class Capability { 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 9d13e94268..6db8cf6cf7 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -165,7 +165,8 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { // meaning that it's probably the right way to handle disabling options. @Option( names = {"--no-discovery"}, - description = "Disable p2p peer discovery (default: ${DEFAULT-VALUE})" + description = "Disable p2p peer discovery (default: ${DEFAULT-VALUE})", + arity = "1" ) private final Boolean noPeerDiscovery = false; 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 c4dcb63a2e..8553727cdf 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -523,17 +523,25 @@ public class PantheonCommandTest extends CommandTestAbstract { } @Test - public void discoveryOptionMustBeUsed() { - parseCommand("--no-discovery"); + public void discoveryOptionValueTrueMustBeUsed() { + parseCommand("--no-discovery", "true"); // Discovery stored in runner is the negative of the option passed to CLI - // So as passing the option means noDiscovery will be true, then discovery is false in runner - verify(mockRunnerBuilder.discovery(eq(false))).build(); assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()).isEmpty(); } + @Test + public void discoveryOptionValueFalseMustBeUsed() { + parseCommand("--no-discovery", "false"); + // Discovery stored in runner is the negative of the option passed to CLI + verify(mockRunnerBuilder.discovery(eq(true))).build(); + + assertThat(commandOutput.toString()).isEmpty(); + assertThat(commandErrorOutput.toString()).isEmpty(); + } + @Ignore("NC-2015 - Temporarily enabling zero-arg --bootnodes to permit 'bootnode' configuration") @Test public void callingWithBootnodesOptionButNoValueMustDisplayErrorAndUsage() {