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

Add public key address export subcommand #888

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
28 changes: 25 additions & 3 deletions docs/Reference/Pantheon-CLI-Syntax.md
Original file line number Diff line number Diff line change
Expand Up @@ -917,14 +917,36 @@ This command provides node public key related actions.
#### export

```bash tab="Syntax"
$ pantheon public-key export --to=<key-file>
$ pantheon public-key export [--to=<key-file>]
```

```bash tab="Example"
```bash tab="Example (to standard output)"
$ pantheon public-key export
```

```bash tab="Example (to file)"
$ pantheon public-key export --to=/home/me/me_project/not_precious_pub_key
```

Exports node public key to the specified file.
Outputs the node public key to standard output or write it in the specified file if option
`--to=<key-file>` is defined.

#### export-address

```bash tab="Syntax"
$ pantheon public-key export-address [--to=<address-file>]
```

```bash tab="Example (to standard output)"
$ pantheon public-key export-address
```

```bash tab="Example (to file)"
$ pantheon public-key export-address --to=/home/me/me_project/me_node_address
```

Outputs the node public key address to standard output or write it in the specified file if option
`--to=<key-file>` is defined.

### password

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@
import static tech.pegasys.pantheon.cli.DefaultCommandValues.MANDATORY_FILE_FORMAT_HELP;
import static tech.pegasys.pantheon.cli.PublicKeySubCommand.COMMAND_NAME;

import tech.pegasys.pantheon.cli.PublicKeySubCommand.AddressSubCommand;
import tech.pegasys.pantheon.cli.PublicKeySubCommand.ExportSubCommand;
import tech.pegasys.pantheon.controller.PantheonController;
import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair;
import tech.pegasys.pantheon.ethereum.core.Address;
import tech.pegasys.pantheon.ethereum.core.Util;

import java.io.BufferedWriter;
import java.io.File;
Expand All @@ -41,7 +44,7 @@
name = COMMAND_NAME,
description = "This command provides node public key related actions.",
mixinStandardHelpOptions = true,
subcommands = {ExportSubCommand.class})
subcommands = {ExportSubCommand.class, AddressSubCommand.class})
class PublicKeySubCommand implements Runnable {
private static final Logger LOG = LogManager.getLogger();

Expand Down Expand Up @@ -69,23 +72,23 @@ public void run() {
/**
* Public key export sub-command
*
* <p>Export of the public key takes a file as parameter to export directly by writing the key in
* the file. A direct output of the key to standard out is not done because we don't want the key
* value to be polluted by other information like logs that are in KeyPairUtil that is inevitable.
* <p>Export of the public key is writing the key to the standard output by default. An option
* enables to write it in a file. Indeed, a direct output of the value to standard out is not
* always recommended as reading can be made difficult as the value can be mixed with other
* information like logs that are in KeyPairUtil that is inevitable.
*/
@Command(
name = "export",
description = "This command exports the node public key to a file.",
description = "This command outputs the node public key. Default output is standard output.",
mixinStandardHelpOptions = true)
static class ExportSubCommand implements Runnable {

@Option(
names = "--to",
required = true,
paramLabel = MANDATORY_FILE_FORMAT_HELP,
description = "File to write public key to",
description = "File to write public key to instead of standard output",
arity = "1..1")
private final File publicKeyExportFile = null;
private File publicKeyExportFile = null;

@SuppressWarnings("unused")
@ParentCommand
Expand All @@ -99,14 +102,70 @@ public void run() {
final PantheonController<?> controller = parentCommand.parentCommand.buildController();
final KeyPair keyPair = controller.getLocalNodeKeyPair();

// this publicKeyExportFile can never be null because of Picocli arity requirement
//noinspection ConstantConditions
final Path path = publicKeyExportFile.toPath();
// if we have an output file defined, print to it
// otherwise print to standard output.
if (publicKeyExportFile != null) {
final Path path = publicKeyExportFile.toPath();

try (final BufferedWriter fileWriter = Files.newBufferedWriter(path, UTF_8)) {
fileWriter.write(keyPair.getPublicKey().toString());
} catch (final IOException e) {
LOG.error("An error occurred while trying to write the public key", e);
}
} else {
parentCommand.out.println(keyPair.getPublicKey().toString());
}
}
}

try (final BufferedWriter fileWriter = Files.newBufferedWriter(path, UTF_8)) {
fileWriter.write(keyPair.getPublicKey().toString());
} catch (final IOException e) {
LOG.error("An error occurred while trying to write the public key", e);
/**
* Public key address export sub-command
*
* <p>Export of the public key address is writing the address to the standard output by default.
* An option enables to write it in a file. Indeed, a direct output of the value to standard out
* is not always recommended as reading can be made difficult as the value can be mixed with other
* information like logs that are in KeyPairUtil that is inevitable.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: by default, print to stdout. That's pretty standard syntax for something like this. Let the user choose to add the file option if they want. I suspect a valuable use case is scripting (eg startup script) where user would rather grab from stdout (and run through grep if necessary). Dealing with files can be a PITA, depending on installation (permissions, stale file, stale file that doesn't get updated and thus user gets broken value)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes but doesn't work as we output all logs to standard out. Files are way more easy to use just a cat and you got the content.

Copy link
Contributor

Choose a reason for hiding this comment

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

See my comment in --to, if we make it optional we can default to std out. Do we have any CLI guidelines on positional parameters? That would be another option. Another option is to add another flag like --to-console and require one of those two flags.

However I think there should be some way to get it to output to console. The ease of use and default setting I am flexible on.

Copy link
Contributor

Choose a reason for hiding this comment

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

agree on making the --to file optional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--to is now optional. I also made the change on the other one. It's retro-compatible with the previous way to use the commands.

@Command(
name = "export-address",
description =
"This command outputs the node's public key address. "
+ "Default output is standard output.",
mixinStandardHelpOptions = true)
static class AddressSubCommand implements Runnable {

@Option(
names = "--to",
paramLabel = MANDATORY_FILE_FORMAT_HELP,
description = "File to write address to instead of standard output",
arity = "1..1")
private File addressExportFile = null;

@SuppressWarnings("unused")
@ParentCommand
private PublicKeySubCommand parentCommand; // Picocli injects reference to parent command

@Override
public void run() {
checkNotNull(parentCommand);
checkNotNull(parentCommand.parentCommand);

final PantheonController<?> controller = parentCommand.parentCommand.buildController();
final KeyPair keyPair = controller.getLocalNodeKeyPair();
final Address address = Util.publicKeyToAddress(keyPair.getPublicKey());

// if we have an output file defined, print to it
// otherwise print to standard output.
if (addressExportFile != null) {
final Path path = addressExportFile.toPath();

try (final BufferedWriter fileWriter = Files.newBufferedWriter(path, UTF_8)) {
fileWriter.write(address.toString());
} catch (final IOException e) {
LOG.error("An error occurred while trying to write the public key address", e);
}
} else {
parentCommand.out.println(address.toString());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
import java.io.PrintStream;
import java.net.URI;
import java.nio.file.Path;
Expand Down Expand Up @@ -129,9 +130,15 @@ public void initMocks() throws Exception {

// Display outputs for debug purpose
@After
public void displayOutput() {
public void displayOutput() throws IOException {
TEST_LOGGER.info("Standard output {}", commandOutput.toString());
TEST_LOGGER.info("Standard error {}", commandErrorOutput.toString());

outPrintStream.close();
commandOutput.close();

errPrintStream.close();
commandErrorOutput.close();
}

CommandLine.Model.CommandSpec parseCommand(final String... args) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static org.mockito.Mockito.when;

import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair;
import tech.pegasys.pantheon.ethereum.core.Util;

import java.io.File;

Expand All @@ -36,15 +37,35 @@ public class PublicKeySubCommandTest extends CommandTestAbstract {
+ System.lineSeparator()
+ "Commands:"
+ System.lineSeparator()
+ " export This command exports the node public key to a file."
+ " export This command outputs the node public key. Default output is"
+ System.lineSeparator()
+ " standard output."
+ System.lineSeparator()
+ " export-address This command outputs the node's public key address. Default"
+ System.lineSeparator()
+ " output is standard output."
+ System.lineSeparator();

private static final String EXPECTED_PUBLIC_KEY_EXPORT_USAGE =
"Usage: pantheon public-key export [-hV] --to=<FILE>"
"Usage: pantheon public-key export [-hV] [--to=<FILE>]"
+ System.lineSeparator()
+ "This command outputs the node public key. Default output is standard output."
+ System.lineSeparator()
+ " --to=<FILE> File to write public key to instead of standard output"
+ System.lineSeparator()
+ " -h, --help Show this help message and exit."
+ System.lineSeparator()
+ " -V, --version Print version information and exit."
+ System.lineSeparator();

private static final String EXPECTED_PUBLIC_KEY_EXPORT_ADDRESS_USAGE =
"Usage: pantheon public-key export-address [-hV] [--to=<FILE>]"
+ System.lineSeparator()
+ "This command outputs the node's public key address. Default output is standard"
+ System.lineSeparator()
+ "This command exports the node public key to a file."
+ "output."
+ System.lineSeparator()
+ " --to=<FILE> File to write public key to"
+ " --to=<FILE> File to write address to instead of standard output"
+ System.lineSeparator()
+ " -h, --help Show this help message and exit."
+ System.lineSeparator()
Expand All @@ -53,14 +74,16 @@ public class PublicKeySubCommandTest extends CommandTestAbstract {

private static final String PUBLIC_KEY_SUBCOMMAND_NAME = "public-key";
private static final String PUBLIC_KEY_EXPORT_SUBCOMMAND_NAME = "export";
private static final String PUBLIC_KEY_EXPORT_ADDRESS_SUBCOMMAND_NAME = "export-address";

// bublic-key sub-command
// public-key sub-command
@Test
public void publicKeySubCommandExistAnbHaveSubCommands() {
CommandSpec spec = parseCommand();
assertThat(spec.subcommands()).containsKeys(PUBLIC_KEY_SUBCOMMAND_NAME);
assertThat(spec.subcommands().get(PUBLIC_KEY_SUBCOMMAND_NAME).getSubcommands())
.containsKeys(PUBLIC_KEY_EXPORT_SUBCOMMAND_NAME);
.containsKeys(PUBLIC_KEY_EXPORT_SUBCOMMAND_NAME)
.containsKeys(PUBLIC_KEY_EXPORT_ADDRESS_SUBCOMMAND_NAME);
assertThat(commandOutput.toString()).isEmpty();
assertThat(commandErrorOutput.toString()).isEmpty();
}
Expand All @@ -79,12 +102,17 @@ public void callingPublicKeySubCommandHelpMustDisplayUsage() {
assertThat(commandErrorOutput.toString()).isEmpty();
}

// Export sub-sub-command
// Export public key sub-sub-command
@Test
public void callingPublicKeyExportSubCommandWithoutPathMustDisplayErrorAndUsage() {
public void callingPublicKeyExportSubCommandWithoutPathMustWriteKeyToStandardOutput() {
final KeyPair keyPair = KeyPair.generate();
when(mockController.getLocalNodeKeyPair()).thenReturn(keyPair);

parseCommand(PUBLIC_KEY_SUBCOMMAND_NAME, PUBLIC_KEY_EXPORT_SUBCOMMAND_NAME);
final String expectedErrorOutputStart = "Missing required option '--to=<FILE>'";
assertThat(commandErrorOutput.toString()).startsWith(expectedErrorOutputStart);

final String expectedOutputStart = keyPair.getPublicKey().toString();
assertThat(commandOutput.toString()).startsWith(expectedOutputStart);
assertThat(commandErrorOutput.toString()).isEmpty();
}

@Test
Expand Down Expand Up @@ -114,4 +142,48 @@ public void callingPublicKeyExportSubCommandWithFilePathMustWritePublicKeyInThis
assertThat(commandOutput.toString()).isEmpty();
assertThat(commandErrorOutput.toString()).isEmpty();
}

// Export address sub-sub-command
@Test
public void callingPublicKeyExportAddressSubCommandWithoutPathMustWriteAddressToStandardOutput() {
final KeyPair keyPair = KeyPair.generate();
when(mockController.getLocalNodeKeyPair()).thenReturn(keyPair);

parseCommand(PUBLIC_KEY_SUBCOMMAND_NAME, PUBLIC_KEY_EXPORT_ADDRESS_SUBCOMMAND_NAME);

final String expectedOutputStart = Util.publicKeyToAddress(keyPair.getPublicKey()).toString();
assertThat(commandOutput.toString()).startsWith(expectedOutputStart);
assertThat(commandErrorOutput.toString()).isEmpty();
}

@Test
public void callingPublicKeyExportAddressSubCommandHelpMustDisplayUsage() {
parseCommand(PUBLIC_KEY_SUBCOMMAND_NAME, PUBLIC_KEY_EXPORT_ADDRESS_SUBCOMMAND_NAME, "--help");
assertThat(commandOutput.toString()).startsWith(EXPECTED_PUBLIC_KEY_EXPORT_ADDRESS_USAGE);
assertThat(commandErrorOutput.toString()).isEmpty();
}

@Test
public void callingPublicKeyExportAddressSubCommandWithFilePathMustWriteAddressInThisFile()
throws Exception {

final KeyPair keyPair = KeyPair.generate();

when(mockController.getLocalNodeKeyPair()).thenReturn(keyPair);

final File file = File.createTempFile("public", "address");

parseCommand(
PUBLIC_KEY_SUBCOMMAND_NAME,
PUBLIC_KEY_EXPORT_ADDRESS_SUBCOMMAND_NAME,
"--to",
file.getPath());

assertThat(contentOf(file))
.startsWith(Util.publicKeyToAddress(keyPair.getPublicKey()).toString())
.endsWith(Util.publicKeyToAddress(keyPair.getPublicKey()).toString());

assertThat(commandOutput.toString()).isEmpty();
assertThat(commandErrorOutput.toString()).isEmpty();
}
}