Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: replace the keystore command with account cmd #54

Merged
merged 4 commits into from
Jul 12, 2023

Conversation

flywukong
Copy link
Collaborator

@flywukong flywukong commented Jul 5, 2023

Description

  1. remove default password file loading logic for safe
  2. support "account new " , "account export " and "account import" command
  3. replace the keystore command with account cmd
  4. support version command

Rationale

improve usage experiment

Example

qa-test % ./gnfd-cmd account new
Please enter a passphrase now:
create new account: {0x21c1228786b93C656206Ccc6a753E0831B81023f} successfully
qa-test % ./gnfd-cmd account list
Account : { 21C1228786B93C656206CCC6A753E0831B81023F },  keystore /Users/user/.gnfd-cmd/keystore/key.json


qa-test % ./gnfd-cmd version
Greenfield Cmd Version: v0.0.9-alpha.1

Changes

Notable changes:

  • support account cmd

@flywukong flywukong changed the title Refactor: replace the keystore command with account cmd refactor: replace the keystore command with account cmd Jul 5, 2023
Examples:
$ gnfd-cmd keystore generate --privKeyFile key.txt `,
$ gnfd-cmd account import --privKeyFile key.txt ./key.json`,
Copy link

Choose a reason for hiding this comment

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

Keep to the default path? No need to specify the output keystore file.

Copy link

Choose a reason for hiding this comment

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

The private key file is must and is the only parameter, so no need to add this private key flag?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, improved it.

qa-test % ./gnfd-cmd  -k key.json  account import  key.txt3
Please enter a passphrase now:
import account successfully, key address: 0x54a22B39A51E3446b70A27F1d68753267EBa5745, encrypted key file: key.json

if err != nil {
return toCmdErr(err)
// key.txt contains the origin private hex string
$ gnfd-cmd -k key.json account import key.txt `,
Copy link

Choose a reason for hiding this comment

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

move -k key.json behind the import key.txt is better?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a global flag, which does not belong to the account command. All other commands share this flag, so needs to put it in front.

Copy link
Collaborator Author

@flywukong flywukong Jul 12, 2023

Choose a reason for hiding this comment

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

In most cases, users should not need to specify -k

@keefel keefel merged commit cfb2cfe into develop Jul 12, 2023
flywukong added a commit that referenced this pull request Jul 14, 2023
* fix: remove default password file

* refactor: replace keystore command with account cmd

* fix: fix command description and log

* feat: support account export commands and support version cmd

---------

Co-authored-by: leowkong <[email protected]>
flywukong added a commit that referenced this pull request Jul 17, 2023
* fix: remove default password file

* refactor: replace keystore command with account cmd

* fix: fix command description and log

* feat: support account export commands and support version cmd

---------

Co-authored-by: leowkong <[email protected]>
@flywukong flywukong deleted the refactor-account branch October 13, 2023 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants