fixes #233 Allow nodekey to be specified separately to --datadir (#234)

fixes #233 Allow nodekey to be specified separately to --datadir

Add a `--node-private-key <FILE>` option on CLI to enable the use of a custom key file.
if not set, uses the default "key" file in the data dir
if set but doesn't exist, creates a new key file
if set and exists, uses the specified key file

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java

@@ -18,6 +18,7 @@
 import tech.pegasys.pantheon.RunnerBuilder;
 import tech.pegasys.pantheon.cli.EthNetworkConfig;
 import tech.pegasys.pantheon.cli.PantheonControllerBuilder;
+import tech.pegasys.pantheon.controller.KeyPairUtil;
 import tech.pegasys.pantheon.controller.PantheonController;
 import tech.pegasys.pantheon.ethereum.eth.sync.SynchronizerConfiguration.Builder;
 
@@ -60,7 +61,8 @@ public void startNode(final PantheonNode node) {
               ethNetworkConfig,
               false,
               node.getMiningParameters(),
-              true);
+              true,
+              KeyPairUtil.getDefaultKeyFile(node.homeDirectory()));
     } catch (final IOException e) {
       throw new RuntimeException("Error building PantheonController", e);
     }

pantheon/src/main/java/tech/pegasys/pantheon/Runner.java

@@ -32,8 +32,6 @@
 
 public class Runner implements AutoCloseable {
 
-  static final String KEY_PATH = "key";
-
   private static final Logger LOG = LogManager.getLogger();
 
   private final Vertx vertx;

pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java

@@ -19,6 +19,7 @@
 import tech.pegasys.pantheon.cli.custom.CorsAllowedOriginsProperty;
 import tech.pegasys.pantheon.consensus.clique.jsonrpc.CliqueRpcApis;
 import tech.pegasys.pantheon.consensus.ibft.jsonrpc.IbftRpcApis;
+import tech.pegasys.pantheon.controller.KeyPairUtil;
 import tech.pegasys.pantheon.controller.PantheonController;
 import tech.pegasys.pantheon.ethereum.core.Address;
 import tech.pegasys.pantheon.ethereum.core.MiningParameters;
@@ -145,6 +146,14 @@ public static class RpcApisConversionException extends Exception {
   )
   private final Path dataDir = getDefaultPantheonDataDir();
 
+  @Option(
+    names = {"--node-private-key"},
+    paramLabel = MANDATORY_PATH_FORMAT_HELP,
+    description =
+        "the path to the node's private key file. (default: a file named \"key\" in the Pantheon data folder.)"
+  )
+  private final File nodePrivateKeyFile = KeyPairUtil.getDefaultKeyFile(dataDir);
+
   // Genesis file path with null default option if the option
   // is not defined on command line as this default is handled by Runner
   // to use mainnet json file from resources
@@ -442,7 +451,8 @@ PantheonController<?> buildController() {
           ethNetworkConfig(),
           syncWithOttoman,
           new MiningParameters(coinbase, minTransactionGasPrice, extraData, isMiningEnabled),
-          isDevMode);
+          isDevMode,
+          nodePrivateKeyFile);
     } catch (final InvalidConfigurationException e) {
       throw new ExecutionException(new CommandLine(this), e.getMessage());
     } catch (final IOException e) {

pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonControllerBuilder.java

@@ -23,6 +23,7 @@
 import tech.pegasys.pantheon.ethereum.development.DevelopmentProtocolSchedule;
 import tech.pegasys.pantheon.ethereum.eth.sync.SynchronizerConfiguration;
 
+import java.io.File;
 import java.io.IOException;
 import java.nio.file.Path;
 
@@ -36,11 +37,12 @@ public PantheonController<?> build(
       final EthNetworkConfig ethNetworkConfig,
       final boolean syncWithOttoman,
       final MiningParameters miningParameters,
-      final boolean isDevMode)
+      final boolean isDevMode,
+      final File nodePrivateKeyFile)
       throws IOException {
     // instantiate a controller with mainnet config if no genesis file is defined
     // otherwise use the indicated genesis file
-    final KeyPair nodeKeys = loadKeyPair(homePath);
+    final KeyPair nodeKeys = loadKeyPair(nodePrivateKeyFile);
     if (isDevMode) {
       final GenesisConfigFile genesisConfig = GenesisConfigFile.development();
       return MainnetPantheonController.init(

pantheon/src/main/java/tech/pegasys/pantheon/controller/KeyPairUtil.java

@@ -24,8 +24,7 @@
 public class KeyPairUtil {
   private static final Logger LOG = LogManager.getLogger();
 
-  public static SECP256K1.KeyPair loadKeyPair(final Path home) throws IOException {
-    final File keyFile = home.resolve("key").toFile();
+  public static SECP256K1.KeyPair loadKeyPair(final File keyFile) throws IOException {
     final SECP256K1.KeyPair key;
     if (keyFile.exists()) {
       key = SECP256K1.KeyPair.load(keyFile);
@@ -40,4 +39,12 @@ public static SECP256K1.KeyPair loadKeyPair(final Path home) throws IOException
     }
     return key;
   }
+
+  public static SECP256K1.KeyPair loadKeyPair(final Path homeDirectory) throws IOException {
+    return loadKeyPair(getDefaultKeyFile(homeDirectory));
+  }
+
+  public static File getDefaultKeyFile(final Path homeDirectory) {
+    return homeDirectory.resolve("key").toFile();
+  }
 }

pantheon/src/test/java/tech/pegasys/pantheon/RunnerTest.java

@@ -137,7 +137,7 @@ private void syncFromGenesis(final SyncMode mode) throws Exception {
 
       // Setup runner with no block data
       final Path dbBehind = temp.newFolder().toPath();
-      final KeyPair behindDbNodeKeys = loadKeyPair(dbBehind);
+      final KeyPair behindDbNodeKeys = loadKeyPair(dbBehind.resolve("key").toFile());
       final PantheonController<Void> controllerBehind =
           MainnetPantheonController.init(
               temp.newFolder().toPath(),

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

@@ -25,6 +25,7 @@
 import tech.pegasys.pantheon.util.BlockImporter;
 
 import java.io.ByteArrayOutputStream;
+import java.io.File;
 import java.io.PrintStream;
 import java.nio.file.Path;
 import java.util.Collection;
@@ -65,6 +66,7 @@ public abstract class CommandTestAbstract {
 
   @Captor ArgumentCaptor<Collection<String>> stringListArgumentCaptor;
   @Captor ArgumentCaptor<Path> pathArgumentCaptor;
+  @Captor ArgumentCaptor<File> fileArgumentCaptor;
   @Captor ArgumentCaptor<String> stringArgumentCaptor;
   @Captor ArgumentCaptor<Integer> intArgumentCaptor;
   @Captor ArgumentCaptor<JsonRpcConfiguration> jsonRpcConfigArgumentCaptor;
@@ -75,7 +77,7 @@ public void initMocks() throws Exception {
     // doReturn used because of generic PantheonController
     Mockito.doReturn(mockController)
         .when(mockControllerBuilder)
-        .build(any(), any(), any(), anyBoolean(), any(), anyBoolean());
+        .build(any(), any(), any(), anyBoolean(), any(), anyBoolean(), any());
 
     when(mockSyncConfBuilder.build()).thenReturn(mockSyncConf);
   }

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

@@ -144,7 +144,14 @@ public void callingPantheonCommandWithoutOptionsMustSyncWithDefaultValues() thro
     final ArgumentCaptor<EthNetworkConfig> networkArg =
         ArgumentCaptor.forClass(EthNetworkConfig.class);
     verify(mockControllerBuilder)
-        .build(any(), isNotNull(), networkArg.capture(), eq(false), miningArg.capture(), eq(false));
+        .build(
+            any(),
+            isNotNull(),
+            networkArg.capture(),
+            eq(false),
+            miningArg.capture(),
+            eq(false),
+            isNotNull());
 
     verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FULL));
 
@@ -283,7 +290,8 @@ public void OverrideDefaultValuesIfKeyIsPresentInConfigFile() throws IOException
             eq(networkConfig),
             eq(false),
             any(),
-            anyBoolean());
+            anyBoolean(),
+            any());
 
     // TODO: Re-enable as per NC-1057/NC-1681
     // verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FAST));
@@ -321,7 +329,7 @@ public void NoOverrideDefaultValuesIfKeyIsNotPresentInConfigFile() throws IOExce
             any(),
             any());
 
-    verify(mockControllerBuilder).build(any(), any(), any(), eq(false), any(), eq(false));
+    verify(mockControllerBuilder).build(any(), any(), any(), eq(false), any(), eq(false), any());
 
     // TODO: Re-enable as per NC-1057/NC-1681
     // verify(mockSyncConfBuilder).syncMode(ArgumentMatchers.eq(SyncMode.FULL));
@@ -332,14 +340,36 @@ public void NoOverrideDefaultValuesIfKeyIsNotPresentInConfigFile() throws IOExce
     assertThat(commandErrorOutput.toString()).isEmpty();
   }
 
+  @Test
+  public void nodekeyOptionMustBeUsed() throws Exception {
+    final File file = new File("./specific/key");
+
+    parseCommand("--node-private-key", file.getPath());
+
+    verify(mockControllerBuilder)
+        .build(
+            any(),
+            isNotNull(),
+            any(),
+            eq(false),
+            any(),
+            anyBoolean(),
+            fileArgumentCaptor.capture());
+
+    assertThat(fileArgumentCaptor.getValue()).isEqualTo(file);
+
+    assertThat(commandOutput.toString()).isEmpty();
+    assertThat(commandErrorOutput.toString()).isEmpty();
+  }
+
   @Test
   public void dataDirOptionMustBeUsed() throws Exception {
     final Path path = Paths.get(".");
 
     parseCommand("--datadir", path.toString());
 
     verify(mockControllerBuilder)
-        .build(any(), pathArgumentCaptor.capture(), any(), eq(false), any(), anyBoolean());
+        .build(any(), pathArgumentCaptor.capture(), any(), eq(false), any(), anyBoolean(), any());
 
     assertThat(pathArgumentCaptor.getValue()).isEqualByComparingTo(path);
 
@@ -356,7 +386,7 @@ public void genesisPathOptionMustBeUsed() throws Exception {
     parseCommand("--genesis", path.toString());
 
     verify(mockControllerBuilder)
-        .build(any(), any(), networkArg.capture(), anyBoolean(), any(), anyBoolean());
+        .build(any(), any(), networkArg.capture(), anyBoolean(), any(), anyBoolean(), any());
 
     assertThat(networkArg.getValue().getGenesisConfig()).isEqualTo(path.toUri());
 
@@ -904,7 +934,7 @@ public void miningIsEnabledWhenSpecified() throws Exception {
         ArgumentCaptor.forClass(MiningParameters.class);
 
     verify(mockControllerBuilder)
-        .build(any(), any(), any(), anyBoolean(), miningArg.capture(), anyBoolean());
+        .build(any(), any(), any(), anyBoolean(), miningArg.capture(), anyBoolean(), any());
     assertThat(commandOutput.toString()).isEmpty();
     assertThat(commandErrorOutput.toString()).isEmpty();
     assertThat(miningArg.getValue().isMiningEnabled()).isTrue();
@@ -926,7 +956,7 @@ public void miningParametersAreCaptured() throws Exception {
         ArgumentCaptor.forClass(MiningParameters.class);
 
     verify(mockControllerBuilder)
-        .build(any(), any(), any(), anyBoolean(), miningArg.capture(), anyBoolean());
+        .build(any(), any(), any(), anyBoolean(), miningArg.capture(), anyBoolean(), any());
     assertThat(commandOutput.toString()).isEmpty();
     assertThat(commandErrorOutput.toString()).isEmpty();
     assertThat(miningArg.getValue().getCoinbase()).isEqualTo(Optional.of(requestedCoinbase));
@@ -938,7 +968,7 @@ public void miningParametersAreCaptured() throws Exception {
   @Test
   public void devModeOptionMustBeUsed() throws Exception {
     parseCommand("--dev-mode");
-    verify(mockControllerBuilder).build(any(), any(), any(), anyBoolean(), any(), eq(true));
+    verify(mockControllerBuilder).build(any(), any(), any(), anyBoolean(), any(), eq(true), any());
     assertThat(commandOutput.toString()).isEmpty();
     assertThat(commandErrorOutput.toString()).isEmpty();
   }
@@ -950,7 +980,7 @@ public void rinkebyValuesAreUsed() throws Exception {
     final ArgumentCaptor<EthNetworkConfig> networkArg =
         ArgumentCaptor.forClass(EthNetworkConfig.class);
     verify(mockControllerBuilder)
-        .build(any(), any(), networkArg.capture(), anyBoolean(), any(), anyBoolean());
+        .build(any(), any(), networkArg.capture(), anyBoolean(), any(), anyBoolean(), any());
     assertThat(commandOutput.toString()).isEmpty();
     assertThat(commandErrorOutput.toString()).isEmpty();
     assertThat(networkArg.getValue()).isEqualTo(EthNetworkConfig.rinkeby());
@@ -972,7 +1002,7 @@ public void rinkebyValuesCanBeOverridden() throws Exception {
     final ArgumentCaptor<EthNetworkConfig> networkArg =
         ArgumentCaptor.forClass(EthNetworkConfig.class);
     verify(mockControllerBuilder)
-        .build(any(), any(), networkArg.capture(), anyBoolean(), any(), anyBoolean());
+        .build(any(), any(), networkArg.capture(), anyBoolean(), any(), anyBoolean(), any());
     assertThat(commandOutput.toString()).isEmpty();
     assertThat(commandErrorOutput.toString()).isEmpty();
     assertThat(networkArg.getValue().getGenesisConfig()).isEqualTo(path.toUri());