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 <adrian.sutton@consensys.net>

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();
+  }
+}

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<String> 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();
   }
 

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());
 

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())

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;
   }

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);
   }
 }

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()

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 <a href=
+ * @see <a href= "https://github.com/ethereum/devp2p/blob/master/devp2p.md">Capability wire
- *     "https://github.com/ethereum/wiki/wiki/%C3%90%CE%9EVp2p-Wire-Protocol#p2p">Capability wire
  *     format</a>
  */
 public class Capability {

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;
 

pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java

@@ -523,17 +523,25 @@ public class PantheonCommandTest extends CommandTestAbstract {
   }
 
   @Test
-  public void discoveryOptionMustBeUsed() {
+  public void discoveryOptionValueTrueMustBeUsed() {
-    parseCommand("--no-discovery");
+    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() {