Merge pull request #14388 from bas-vk/cli-account-mngt

cmd/geth: reorganise account/wallet command/flags

3 files changed, 95 insertions, 70 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",
+		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 import /path/to/my/presale.wallet
+		    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.
+		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: "<keyFile>",
-				Description: `
-TODO: Please write this
-`,
-			},
-		},
+		`,
 	}
 	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 <DATADIR>/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 <passwordfile> 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: "<address>",
+				Flags: []cli.Flag{
+					utils.DataDirFlag,
+					utils.KeyStoreDirFlag,
+					utils.LightKDFFlag,
+				},
 				Description: `
     geth account update <address>
 
@@ -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 <passwordfile> account update <address>
+    geth account update [options] <address>
 
 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: "<keyFile>",
 				Description: `
     geth account import <keyfile>
@@ -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 <passwordfile> account import <keyfile>
+    geth account import [options] <keyfile>
 
 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)
+	}
+}