search

autonity / aut

A command-line RPC client for Autonity
MIT License
11 stars 10 forks source link

Add `--keystore` option to `aut account new` and `aut account import-private-key`? #66

Closed rmsams closed 1 year ago

rmsams commented 1 year ago

The two aut account commands for creating a new keyfile from an existing private key (import-private-key) or a newly generated private key (new) both currently mandate that user defines the keyfile name with the --key-file option. To me, this seems like it should be a non-default, special case where user for some reason wants to specify the keyfile filename. The default case should be like it is with clef newaccount, where user only specifies the keystore directory (optionally, if keystore is defined in user's .autrc config) and the keyfile name is autogenerated using a timestamp and the ethereum account (all lowercase, not checksum) of the key saved in the file, e.g., 

UTC--2022-02-07T17-19-56.517538000Z--ca57f3b40b42fcce3c37b8d18adbca5260ca72ec

This is a safer practice, always having the account in the filename. When user comes to use one of these keyfiles, custom names introduce an abstraction that can easily lead to error unless user cat's the keyfile to confirm that her assumption about the account being used is correct.

It's also better to keep the keyfile creation practice consistent with geth, as user may be using both clef and autcli. Consistency across tools writing to the same directory should be the default unless there's a good reason to deviate.

Finally, this avoids user having to deal with one of the hardest problems in computer science! ;)

There are only two hard things in Computer Science: cache invalidation and naming things.
— Phil Karlton

Proposal: in both of these commands, add an option --keystore which accepts a path to user's keystore. When this option is used, keyfile filename is autogenerated per above convention. When --key-file is used, behaviour is as currently implemented (if --keystore is also present, it's ignored and command echo's a warning message saying so). If neither option is used, assume the first case and search .autrc for the keystore value and throw an error if not found.

dtebbs commented 1 year ago

Added to v0.0.6

dtebbs commented 1 year ago

Now available in develop:

$ pipx install --force git+ssh://git@github.com/autonity/autcli.git@develop

(note the change from --key-file to --keyfile for consistency) Let me know if there is further feedback.