cmd/geth: remove unlock commandline flag

accounts/keystore/account_cache.go

@@ -44,8 +44,7 @@ func byURL(a, b accounts.Account) int {
 	return a.URL.Cmp(b.URL)
 }
 
-// AmbiguousAddrError is returned when attempting to unlock
-// an address for which more than one file exists.
+// AmbiguousAddrError is returned when an address matches multiple files.
 type AmbiguousAddrError struct {
 	Addr    common.Address
 	Matches []accounts.Account

cmd/geth/accountcmd.go

@@ -17,14 +17,16 @@
 package main
 
 import (
+	"errors"
 	"fmt"
 	"os"
+	"strings"
 
 	"github.com/ethereum/go-ethereum/accounts"
 	"github.com/ethereum/go-ethereum/accounts/keystore"
 	"github.com/ethereum/go-ethereum/cmd/utils"
+	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
-	"github.com/ethereum/go-ethereum/log"
 	"github.com/urfave/cli/v2"
 )
 
@@ -219,60 +221,22 @@ func accountList(ctx *cli.Context) error {
 	return nil
 }
 
-// tries unlocking the specified account a few times.
-func unlockAccount(ks *keystore.KeyStore, address string, i int, passwords []string) (accounts.Account, string) {
-	account, err := utils.MakeAddress(ks, address)
-	if err != nil {
-		utils.Fatalf("Could not list accounts: %v", err)
-	}
-	for trials := 0; trials < 3; trials++ {
-		prompt := fmt.Sprintf("Unlocking account %s | Attempt %d/%d", address, trials+1, 3)
-		password := utils.GetPassPhraseWithList(prompt, false, i, passwords)
-		err = ks.Unlock(account, password)
-		if err == nil {
-			log.Info("Unlocked account", "address", account.Address.Hex())
-			return account, password
-		}
-		if err, ok := err.(*keystore.AmbiguousAddrError); ok {
-			log.Info("Unlocked account", "address", account.Address.Hex())
-			return ambiguousAddrRecovery(ks, err, password), password
-		}
-		if err != keystore.ErrDecrypt {
-			// No need to prompt again if the error is not decryption-related.
-			break
-		}
-	}
-	// All trials expended to unlock account, bail out
-	utils.Fatalf("Failed to unlock account %s (%v)", address, err)
-
-	return accounts.Account{}, ""
-}
-
-func ambiguousAddrRecovery(ks *keystore.KeyStore, err *keystore.AmbiguousAddrError, auth string) accounts.Account {
-	fmt.Printf("Multiple key files exist for address %x:\n", err.Addr)
-	for _, a := range err.Matches {
-		fmt.Println("  ", a.URL)
-	}
-	fmt.Println("Testing your password against all of them...")
-	var match *accounts.Account
-	for i, a := range err.Matches {
-		if e := ks.Unlock(a, auth); e == nil {
-			match = &err.Matches[i]
-			break
-		}
+// readPasswordFromFile reads the first line of the given file, trims line endings,
+// and returns the password and whether the reading was successful.
+func readPasswordFromFile(path string) (string, bool) {
+	if path == "" {
+		return "", false
 	}
-	if match == nil {
-		utils.Fatalf("None of the listed files could be unlocked.")
-		return accounts.Account{}
+	text, err := os.ReadFile(path)
+	if err != nil {
+		utils.Fatalf("Failed to read password file: %v", err)
 	}
-	fmt.Printf("Your password unlocked %s\n", match.URL)
-	fmt.Println("In order to avoid this warning, you need to remove the following duplicate key files:")
-	for _, a := range err.Matches {
-		if a != *match {
-			fmt.Println("  ", a.URL)
-		}
+	lines := strings.Split(string(text), "\n")
+	if len(lines) == 0 {
+		return "", false
 	}
-	return *match
+	// Sanitise DOS line endings.
+	return strings.TrimRight(lines[0], "\r"), true
 }
 
 // accountCreate creates a new account into the keystore defined by the CLI flags.
@@ -292,8 +256,10 @@ func accountCreate(ctx *cli.Context) error {
 		scryptP = keystore.LightScryptP
 	}
 
-	password := utils.GetPassPhraseWithList("Your new account is locked with a password. Please give a password. Do not forget this password.", true, 0, utils.MakePasswordList(ctx))
-
+	password, ok := readPasswordFromFile(ctx.Path(utils.PasswordFileFlag.Name))
+	if !ok {
+		password = utils.GetPassPhrase("Your new account is locked with a password. Please give a password. Do not forget this password.", true)
+	}
 	account, err := keystore.StoreKey(keydir, password, scryptN, scryptP)
 
 	if err != nil {
@@ -323,10 +289,23 @@ func accountUpdate(ctx *cli.Context) error {
 	ks := backends[0].(*keystore.KeyStore)
 
 	for _, addr := range ctx.Args().Slice() {
-		account, oldPassword := unlockAccount(ks, addr, 0, nil)
-		newPassword := utils.GetPassPhraseWithList("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)
+		if !common.IsHexAddress(addr) {
+			return errors.New("address must be specified in hexadecimal form")
+		}
+		account := accounts.Account{Address: common.HexToAddress(addr)}
+		newPassword := utils.GetPassPhrase("Please give a NEW password. Do not forget this password.", true)
+		updateFn := func(attempt int) error {
+			prompt := fmt.Sprintf("Please provide the OLD password for account %s | Attempt %d/%d", addr, attempt+1, 3)
+			password := utils.GetPassPhrase(prompt, false)
+			return ks.Update(account, password, newPassword)
+		}
+		// let user attempt unlock thrice.
+		err := updateFn(0)
+		for attempts := 1; attempts < 3 && errors.Is(err, keystore.ErrDecrypt); attempts++ {
+			err = updateFn(attempts)
+		}
+		if err != nil {
+			return fmt.Errorf("could not update account: %w", err)
 		}
 	}
 	return nil
@@ -347,10 +326,12 @@ func importWallet(ctx *cli.Context) error {
 	if len(backends) == 0 {
 		utils.Fatalf("Keystore is not available")
 	}
+	password, ok := readPasswordFromFile(ctx.Path(utils.PasswordFileFlag.Name))
+	if !ok {
+		password = utils.GetPassPhrase("", false)
+	}
 	ks := backends[0].(*keystore.KeyStore)
-	passphrase := utils.GetPassPhraseWithList("", false, 0, utils.MakePasswordList(ctx))
-
-	acct, err := ks.ImportPreSaleKey(keyJSON, passphrase)
+	acct, err := ks.ImportPreSaleKey(keyJSON, password)
 	if err != nil {
 		utils.Fatalf("%v", err)
 	}
@@ -373,9 +354,11 @@ func accountImport(ctx *cli.Context) error {
 		utils.Fatalf("Keystore is not available")
 	}
 	ks := backends[0].(*keystore.KeyStore)
-	passphrase := utils.GetPassPhraseWithList("Your new account is locked with a password. Please give a password. Do not forget this password.", true, 0, utils.MakePasswordList(ctx))
-
-	acct, err := ks.ImportECDSA(key, passphrase)
+	password, ok := readPasswordFromFile(ctx.Path(utils.PasswordFileFlag.Name))
+	if !ok {
+		password = utils.GetPassPhrase("Your new account is locked with a password. Please give a password. Do not forget this password.", true)
+	}
+	acct, err := ks.ImportECDSA(key, password)
 	if err != nil {
 		utils.Fatalf("Could not create the account: %v", err)
 	}

cmd/geth/accountcmd_test.go

@@ -20,7 +20,6 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
-	"strings"
 	"testing"
 
 	"github.com/cespare/cp"
@@ -171,12 +170,12 @@ func TestAccountUpdate(t *testing.T) {
 		"f466859ead1932d743d622cb74fc058882e8648a")
 	defer geth.ExpectExit()
 	geth.Expect(`
-Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3
+Please give a NEW password. Do not forget this password.
 !! Unsupported terminal, password will be echoed.
-Password: {{.InputLine "foobar"}}
-Please give a new password. Do not forget this password.
 Password: {{.InputLine "foobar2"}}
 Repeat password: {{.InputLine "foobar2"}}
+Please provide the OLD password for account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3
+Password: {{.InputLine "foobar"}}
 `)
 }
 
@@ -206,172 +205,3 @@ Password: {{.InputLine "wrong"}}
 Fatal: could not decrypt key with given password
 `)
 }
-
-func TestUnlockFlag(t *testing.T) {
-	t.Parallel()
-	geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t),
-		"--unlock", "f466859ead1932d743d622cb74fc058882e8648a", "console", "--exec", "loadScript('testdata/empty.js')")
-	geth.Expect(`
-Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3
-!! Unsupported terminal, password will be echoed.
-Password: {{.InputLine "foobar"}}
-undefined
-`)
-	geth.ExpectExit()
-
-	wantMessages := []string{
-		"Unlocked account",
-		"=0xf466859eAD1932D743d622CB74FC058882E8648A",
-	}
-	for _, m := range wantMessages {
-		if !strings.Contains(geth.StderrText(), m) {
-			t.Errorf("stderr text does not contain %q", m)
-		}
-	}
-}
-
-func TestUnlockFlagWrongPassword(t *testing.T) {
-	t.Parallel()
-	geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t),
-		"--unlock", "f466859ead1932d743d622cb74fc058882e8648a", "console", "--exec", "loadScript('testdata/empty.js')")
-
-	defer geth.ExpectExit()
-	geth.Expect(`
-Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3
-!! Unsupported terminal, password will be echoed.
-Password: {{.InputLine "wrong1"}}
-Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 2/3
-Password: {{.InputLine "wrong2"}}
-Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 3/3
-Password: {{.InputLine "wrong3"}}
-Fatal: Failed to unlock account f466859ead1932d743d622cb74fc058882e8648a (could not decrypt key with given password)
-`)
-}
-
-// https://github.com/ethereum/go-ethereum/issues/1785
-func TestUnlockFlagMultiIndex(t *testing.T) {
-	t.Parallel()
-	geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t),
-		"--unlock", "f466859ead1932d743d622cb74fc058882e8648a", "--unlock", "0,2", "console", "--exec", "loadScript('testdata/empty.js')")
-
-	geth.Expect(`
-Unlocking account 0 | Attempt 1/3
-!! Unsupported terminal, password will be echoed.
-Password: {{.InputLine "foobar"}}
-Unlocking account 2 | Attempt 1/3
-Password: {{.InputLine "foobar"}}
-undefined
-`)
-	geth.ExpectExit()
-
-	wantMessages := []string{
-		"Unlocked account",
-		"=0x7EF5A6135f1FD6a02593eEdC869c6D41D934aef8",
-		"=0x289d485D9771714CCe91D3393D764E1311907ACc",
-	}
-	for _, m := range wantMessages {
-		if !strings.Contains(geth.StderrText(), m) {
-			t.Errorf("stderr text does not contain %q", m)
-		}
-	}
-}
-
-func TestUnlockFlagPasswordFile(t *testing.T) {
-	t.Parallel()
-	geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t),
-		"--unlock", "f466859ead1932d743d622cb74fc058882e8648a", "--password", "testdata/passwords.txt", "--unlock", "0,2", "console", "--exec", "loadScript('testdata/empty.js')")
-
-	geth.Expect(`
-undefined
-`)
-	geth.ExpectExit()
-
-	wantMessages := []string{
-		"Unlocked account",
-		"=0x7EF5A6135f1FD6a02593eEdC869c6D41D934aef8",
-		"=0x289d485D9771714CCe91D3393D764E1311907ACc",
-	}
-	for _, m := range wantMessages {
-		if !strings.Contains(geth.StderrText(), m) {
-			t.Errorf("stderr text does not contain %q", m)
-		}
-	}
-}
-
-func TestUnlockFlagPasswordFileWrongPassword(t *testing.T) {
-	t.Parallel()
-	geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t),
-		"--unlock", "f466859ead1932d743d622cb74fc058882e8648a", "--password",
-		"testdata/wrong-passwords.txt", "--unlock", "0,2")
-	defer geth.ExpectExit()
-	geth.Expect(`
-Fatal: Failed to unlock account 0 (could not decrypt key with given password)
-`)
-}
-
-func TestUnlockFlagAmbiguous(t *testing.T) {
-	t.Parallel()
-	store := filepath.Join("..", "..", "accounts", "keystore", "testdata", "dupes")
-	geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t),
-		"--unlock", "f466859ead1932d743d622cb74fc058882e8648a", "--keystore",
-		store, "--unlock", "f466859ead1932d743d622cb74fc058882e8648a",
-		"console", "--exec", "loadScript('testdata/empty.js')")
-	defer geth.ExpectExit()
-
-	// Helper for the expect template, returns absolute keystore path.
-	geth.SetTemplateFunc("keypath", func(file string) string {
-		abs, _ := filepath.Abs(filepath.Join(store, file))
-		return abs
-	})
-	geth.Expect(`
-Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3
-!! Unsupported terminal, password will be echoed.
-Password: {{.InputLine "foobar"}}
-Multiple key files exist for address f466859ead1932d743d622cb74fc058882e8648a:
-   keystore://{{keypath "1"}}
-   keystore://{{keypath "2"}}
-Testing your password against all of them...
-Your password unlocked keystore://{{keypath "1"}}
-In order to avoid this warning, you need to remove the following duplicate key files:
-   keystore://{{keypath "2"}}
-undefined
-`)
-	geth.ExpectExit()
-
-	wantMessages := []string{
-		"Unlocked account",
-		"=0xf466859eAD1932D743d622CB74FC058882E8648A",
-	}
-	for _, m := range wantMessages {
-		if !strings.Contains(geth.StderrText(), m) {
-			t.Errorf("stderr text does not contain %q", m)
-		}
-	}
-}
-
-func TestUnlockFlagAmbiguousWrongPassword(t *testing.T) {
-	t.Parallel()
-	store := filepath.Join("..", "..", "accounts", "keystore", "testdata", "dupes")
-	geth := runMinimalGeth(t, "--port", "0", "--ipcdisable", "--datadir", tmpDatadirWithKeystore(t),
-		"--unlock", "f466859ead1932d743d622cb74fc058882e8648a", "--keystore",
-		store, "--unlock", "f466859ead1932d743d622cb74fc058882e8648a")
-
-	defer geth.ExpectExit()
-
-	// Helper for the expect template, returns absolute keystore path.
-	geth.SetTemplateFunc("keypath", func(file string) string {
-		abs, _ := filepath.Abs(filepath.Join(store, file))
-		return abs
-	})
-	geth.Expect(`
-Unlocking account f466859ead1932d743d622cb74fc058882e8648a | Attempt 1/3
-!! Unsupported terminal, password will be echoed.
-Password: {{.InputLine "wrong"}}
-Multiple key files exist for address f466859ead1932d743d622cb74fc058882e8648a:
-   keystore://{{keypath "1"}}
-   keystore://{{keypath "2"}}
-Testing your password against all of them...
-Fatal: None of the listed files could be unlocked.
-`)
-	geth.ExpectExit()
-}