From f6c0f76cc56e98ac564dd523569b03468a387ef7 Mon Sep 17 00:00:00 2001 From: Bas van Kervel Date: Fri, 28 Apr 2017 12:38:49 +0200 Subject: [PATCH] cmd/geth: reorganise account/wallet command/flags --- cmd/geth/accountcmd.go | 127 ++++++++++++++++++------------------ cmd/geth/accountcmd_test.go | 16 ++--- cmd/utils/flags.go | 26 +++++++- 3 files changed, 97 insertions(+), 72 deletions(-) diff --git a/cmd/geth/accountcmd.go b/cmd/geth/accountcmd.go index 90f79a47e..1a3c63da9 100644 --- a/cmd/geth/accountcmd.go +++ b/cmd/geth/accountcmd.go @@ -31,41 +31,33 @@ import ( var ( walletCommand = cli.Command{ - Name: "wallet", - Usage: "Manage Ethereum presale wallets", - ArgsUsage: "", - Category: "ACCOUNT COMMANDS", - Description: ` - geth wallet import /path/to/my/presale.wallet - -will prompt for your password and imports your ether presale account. -It can be used non-interactively with the --password option taking a -passwordfile as argument containing the wallet password in plaintext. - -`, - Subcommands: []cli.Command{ - { - Action: importWallet, - Name: "import", - Usage: "Import Ethereum presale wallet", - ArgsUsage: "", - Description: ` -TODO: Please write this -`, - }, + Name: "wallet", + Usage: "Import Ethereum presale wallets", + Action: utils.MigrateFlags(importWallet), + Category: "ACCOUNT COMMANDS", + Flags: []cli.Flag{ + utils.DataDirFlag, + utils.KeyStoreDirFlag, + utils.PasswordFileFlag, + utils.LightKDFFlag, }, + Description: ` + geth wallet [options] /path/to/my/presale.wallet + + will prompt for your password and imports your ether presale account. + It can be used non-interactively with the --password option taking a + passwordfile as argument containing the wallet password in plaintext. + + `, } accountCommand = cli.Command{ - Action: accountList, - Name: "account", - Usage: "Manage accounts", - ArgsUsage: "", - Category: "ACCOUNT COMMANDS", + Name: "account", + Usage: "Manage accounts", + Category: "ACCOUNT COMMANDS", Description: ` -Manage accounts lets you create new accounts, list all existing accounts, -import a private key into a new account. -' help' shows a list of subcommands or help for one subcommand. +Manage accounts, list all existing accounts, import a private key into a new +account, create a new account or update an existing account. It supports interactive mode, when you are prompted for password as well as non-interactive mode where passwords are supplied via a given password file. @@ -80,36 +72,34 @@ Note that exporting your key in unencrypted format is NOT supported. Keys are stored under /keystore. It is safe to transfer the entire directory or the individual keys therein between ethereum nodes by simply copying. -Make sure you backup your keys regularly. -In order to use your account to send transactions, you need to unlock them using -the '--unlock' option. The argument is a space separated list of addresses or -indexes. If used non-interactively with a passwordfile, the file should contain -the respective passwords one per line. If you unlock n accounts and the password -file contains less than n entries, then the last password is meant to apply to -all remaining accounts. - -And finally. DO NOT FORGET YOUR PASSWORD. -`, +Make sure you backup your keys regularly.`, Subcommands: []cli.Command{ { - Action: accountList, - Name: "list", - Usage: "Print account addresses", - ArgsUsage: " ", + Name: "list", + Usage: "Print summary of existing accounts", + Action: utils.MigrateFlags(accountList), + Flags: []cli.Flag{ + utils.DataDirFlag, + utils.KeyStoreDirFlag, + }, Description: ` -TODO: Please write this -`, +Print a short summary of all accounts`, }, { - Action: accountCreate, - Name: "new", - Usage: "Create a new account", - ArgsUsage: " ", + Name: "new", + Usage: "Create a new account", + Action: utils.MigrateFlags(accountCreate), + Flags: []cli.Flag{ + utils.DataDirFlag, + utils.KeyStoreDirFlag, + utils.PasswordFileFlag, + utils.LightKDFFlag, + }, Description: ` geth account new -Creates a new account. Prints the address. +Creates a new account and prints the address. The account is saved in encrypted format, you are prompted for a passphrase. @@ -117,17 +107,20 @@ You must remember this passphrase to unlock your account in the future. For non-interactive use the passphrase can be specified with the --password flag: - geth --password account new - Note, this is meant to be used for testing only, it is a bad idea to save your password to file or expose in any other way. `, }, { - Action: accountUpdate, Name: "update", Usage: "Update an existing account", + Action: utils.MigrateFlags(accountUpdate), ArgsUsage: "
", + Flags: []cli.Flag{ + utils.DataDirFlag, + utils.KeyStoreDirFlag, + utils.LightKDFFlag, + }, Description: ` geth account update
@@ -141,16 +134,22 @@ format to the newest format or change the password for an account. For non-interactive use the passphrase can be specified with the --password flag: - geth --password account update
+ geth account update [options]
Since only one password can be given, only format update can be performed, changing your password is only possible interactively. `, }, { - Action: accountImport, - Name: "import", - Usage: "Import a private key into a new account", + Name: "import", + Usage: "Import a private key into a new account", + Action: utils.MigrateFlags(accountImport), + Flags: []cli.Flag{ + utils.DataDirFlag, + utils.KeyStoreDirFlag, + utils.PasswordFileFlag, + utils.LightKDFFlag, + }, ArgsUsage: "", Description: ` geth account import @@ -166,7 +165,7 @@ You must remember this passphrase to unlock your account in the future. For non-interactive use the passphrase can be specified with the -password flag: - geth --password account import + geth account import [options] Note: As you can directly copy your encrypted accounts to another ethereum instance, @@ -298,10 +297,12 @@ func accountUpdate(ctx *cli.Context) error { stack, _ := makeConfigNode(ctx) ks := stack.AccountManager().Backends(keystore.KeyStoreType)[0].(*keystore.KeyStore) - account, oldPassword := unlockAccount(ctx, ks, ctx.Args().First(), 0, nil) - newPassword := getPassPhrase("Please give a new password. Do not forget this password.", true, 0, nil) - if err := ks.Update(account, oldPassword, newPassword); err != nil { - utils.Fatalf("Could not update the account: %v", err) + for _, addr := range ctx.Args() { + account, oldPassword := unlockAccount(ctx, ks, addr, 0, nil) + newPassword := getPassPhrase("Please give a new password. Do not forget this password.", true, 0, nil) + if err := ks.Update(account, oldPassword, newPassword); err != nil { + utils.Fatalf("Could not update the account: %v", err) + } } return nil } diff --git a/cmd/geth/accountcmd_test.go b/cmd/geth/accountcmd_test.go index adcb72454..5f9f67677 100644 --- a/cmd/geth/accountcmd_test.go +++ b/cmd/geth/accountcmd_test.go @@ -43,13 +43,13 @@ func tmpDatadirWithKeystore(t *testing.T) string { } func TestAccountListEmpty(t *testing.T) { - geth := runGeth(t, "account") + geth := runGeth(t, "account", "list") geth.expectExit() } func TestAccountList(t *testing.T) { datadir := tmpDatadirWithKeystore(t) - geth := runGeth(t, "--datadir", datadir, "account") + geth := runGeth(t, "account", "list", "--datadir", datadir) defer geth.expectExit() if runtime.GOOS == "windows" { geth.expect(` @@ -67,7 +67,7 @@ Account #2: {289d485d9771714cce91d3393d764e1311907acc} keystore://{{.Datadir}}/k } func TestAccountNew(t *testing.T) { - geth := runGeth(t, "--lightkdf", "account", "new") + geth := runGeth(t, "account", "new", "--lightkdf") defer geth.expectExit() geth.expect(` Your new account is locked with a password. Please give a password. Do not forget this password. @@ -79,7 +79,7 @@ Repeat passphrase: {{.InputLine "foobar"}} } func TestAccountNewBadRepeat(t *testing.T) { - geth := runGeth(t, "--lightkdf", "account", "new") + geth := runGeth(t, "account", "new", "--lightkdf") defer geth.expectExit() geth.expect(` Your new account is locked with a password. Please give a password. Do not forget this password. @@ -92,9 +92,9 @@ Fatal: Passphrases do not match func TestAccountUpdate(t *testing.T) { datadir := tmpDatadirWithKeystore(t) - geth := runGeth(t, + geth := runGeth(t, "account", "update", "--datadir", datadir, "--lightkdf", - "account", "update", "f466859ead1932d743d622cb74fc058882e8648a") + "f466859ead1932d743d622cb74fc058882e8648a") defer geth.expectExit() geth.expect(` Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3 @@ -107,7 +107,7 @@ Repeat passphrase: {{.InputLine "foobar2"}} } func TestWalletImport(t *testing.T) { - geth := runGeth(t, "--lightkdf", "wallet", "import", "testdata/guswallet.json") + geth := runGeth(t, "wallet", "import", "--lightkdf", "testdata/guswallet.json") defer geth.expectExit() geth.expect(` !! Unsupported terminal, password will be echoed. @@ -122,7 +122,7 @@ Address: {d4584b5f6229b7be90727b0fc8c6b91bb427821f} } func TestWalletImportBadPassword(t *testing.T) { - geth := runGeth(t, "--lightkdf", "wallet", "import", "testdata/guswallet.json") + geth := runGeth(t, "wallet", "import", "--lightkdf", "testdata/guswallet.json") defer geth.expectExit() geth.expect(` !! Unsupported terminal, password will be echoed. diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index e9b0d86a4..9e80bba60 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -647,7 +647,7 @@ func setEtherbase(ctx *cli.Context, ks *keystore.KeyStore, cfg *eth.Config) { } } -// MakePasswordList reads password lines from the file specified by --password. +// MakePasswordList reads password lines from the file specified by the global --password flag. func MakePasswordList(ctx *cli.Context) []string { path := ctx.GlobalString(PasswordFileFlag.Name) if path == "" { @@ -979,3 +979,27 @@ func MakeConsolePreloads(ctx *cli.Context) []string { } return preloads } + +// MigrateFlags sets the global flag from a local flag when it's set. +// This is a temporary function used for migrating old command/flags to the +// new format. +// +// e.g. geth account new --keystore /tmp/mykeystore --lightkdf +// +// is equivalent after calling this method with: +// +// geth --keystore /tmp/mykeystore --lightkdf account new +// +// This allows the use of the existing configuration functionality. +// When all flags are migrated this function can be removed and the existing +// configuration functionality must be changed that is uses local flags +func MigrateFlags(action func(ctx *cli.Context) error) func(*cli.Context) error { + return func(ctx *cli.Context) error { + for _, name := range ctx.FlagNames() { + if ctx.IsSet(name) { + ctx.GlobalSet(name, ctx.String(name)) + } + } + return action(ctx) + } +}