CLI: fixing account import ux bugs (#13328)

* fixing account import checking wallet twice, and adding sub folder search with a depth of 2

* removing uneeded check

* fixing unit test

* adding reset cache to fix potential flake

* improving test based on feedback

cmd/validator/accounts/import.go

@@ -6,13 +6,11 @@ import (
 	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/prysm/v4/cmd"
 	"github.com/prysmaticlabs/prysm/v4/cmd/validator/flags"
-	"github.com/prysmaticlabs/prysm/v4/io/prompt"
 	"github.com/prysmaticlabs/prysm/v4/validator/accounts"
 	"github.com/prysmaticlabs/prysm/v4/validator/accounts/iface"
 	"github.com/prysmaticlabs/prysm/v4/validator/accounts/userprompt"
 	"github.com/prysmaticlabs/prysm/v4/validator/accounts/wallet"
 	"github.com/prysmaticlabs/prysm/v4/validator/client"
-	"github.com/prysmaticlabs/prysm/v4/validator/keymanager"
 	"github.com/urfave/cli/v2"
 )
 
@@ -62,84 +60,5 @@ func accountsImport(c *cli.Context) error {
 }
 
 func walletImport(c *cli.Context) (*wallet.Wallet, error) {
-	return wallet.OpenWalletOrElseCli(c, func(cliCtx *cli.Context) (*wallet.Wallet, error) {
-		walletDir, err := userprompt.InputDirectory(cliCtx, userprompt.WalletDirPromptText, flags.WalletDirFlag)
-		if err != nil {
-			return nil, err
-		}
-		exists, err := wallet.Exists(walletDir)
-		if err != nil {
-			return nil, errors.Wrap(err, wallet.CheckExistsErrMsg)
-		}
-		if exists {
-			isValid, err := wallet.IsValid(walletDir)
-			if err != nil {
-				return nil, errors.Wrap(err, wallet.CheckValidityErrMsg)
-			}
-			if !isValid {
-				return nil, errors.New(wallet.InvalidWalletErrMsg)
-			}
-			walletPassword, err := wallet.InputPassword(
-				cliCtx,
-				flags.WalletPasswordFileFlag,
-				wallet.PasswordPromptText,
-				false, /* Do not confirm password */
-				wallet.ValidateExistingPass,
-			)
-			if err != nil {
-				return nil, err
-			}
-			return wallet.OpenWallet(cliCtx.Context, &wallet.Config{
-				WalletDir:      walletDir,
-				WalletPassword: walletPassword,
-			})
-		}
-
-		wCfg, err := ExtractWalletDirPassword(cliCtx)
-		if err != nil {
-			return nil, err
-		}
-		w := wallet.New(&wallet.Config{
-			KeymanagerKind: keymanager.Local,
-			WalletDir:      wCfg.Dir,
-			WalletPassword: wCfg.Password,
-		})
-		if err = accounts.CreateLocalKeymanagerWallet(cliCtx.Context, w); err != nil {
-			return nil, errors.Wrap(err, "could not create keymanager")
-		}
-		log.WithField("wallet-path", wCfg.Dir).Info(
-			"Successfully created new wallet",
-		)
-		return w, nil
-	})
-}
-
-// WalletDirPassword holds the directory and password of a wallet.
-type WalletDirPassword struct {
-	Dir      string
-	Password string
-}
-
-// ExtractWalletDirPassword prompts the user for wallet directory and password.
-func ExtractWalletDirPassword(cliCtx *cli.Context) (WalletDirPassword, error) {
-	// Get wallet dir and check that no wallet exists at the location.
-	walletDir, err := userprompt.InputDirectory(cliCtx, userprompt.WalletDirPromptText, flags.WalletDirFlag)
-	if err != nil {
-		return WalletDirPassword{}, err
-	}
-	walletPassword, err := prompt.InputPassword(
-		cliCtx,
-		flags.WalletPasswordFileFlag,
-		wallet.NewWalletPasswordPromptText,
-		wallet.ConfirmPasswordPromptText,
-		true, /* Should confirm password */
-		prompt.ValidatePasswordInput,
-	)
-	if err != nil {
-		return WalletDirPassword{}, err
-	}
-	return WalletDirPassword{
-		Dir:      walletDir,
-		Password: walletPassword,
-	}, nil
+	return wallet.OpenWalletOrElseCli(c, wallet.OpenOrCreateNewWallet)
 }

cmd/validator/accounts/wallet_utils_test.go

@@ -70,9 +70,10 @@ func TestWalletWithKeymanager(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, len(keys), 2)
 	require.Equal(t, w.KeymanagerKind(), keymanager.Local)
-	hexKeys := []string{hexutil.Encode(keys[0][:])[2:], hexutil.Encode(keys[1][:])[2:]} // imported keystores don't include the 0x in name
 
-	assert.LogsContain(t, logHook, fmt.Sprintf("Imported accounts %v,", hexKeys))
+	assert.LogsContain(t, logHook, fmt.Sprintf("Imported accounts"))
+	assert.LogsContain(t, logHook, hexutil.Encode(keys[0][:])[2:])
+	assert.LogsContain(t, logHook, hexutil.Encode(keys[1][:])[2:])
 }
 
 func TestWalletWithKeymanager_web3signer(t *testing.T) {

cmd/validator/wallet/BUILD.bazel

@@ -37,18 +37,15 @@ go_test(
     ],
     embed = [":go_default_library"],
     deps = [
-        "//cmd/validator/accounts:go_default_library",
         "//cmd/validator/flags:go_default_library",
         "//config/params:go_default_library",
         "//testing/assert:go_default_library",
         "//testing/require:go_default_library",
-        "//validator/accounts:go_default_library",
         "//validator/accounts/iface:go_default_library",
         "//validator/accounts/wallet:go_default_library",
         "//validator/keymanager:go_default_library",
         "//validator/keymanager/derived:go_default_library",
         "//validator/keymanager/local:go_default_library",
-        "@com_github_pkg_errors//:go_default_library",
         "@com_github_sirupsen_logrus//:go_default_library",
         "@com_github_sirupsen_logrus//hooks/test:go_default_library",
         "@com_github_urfave_cli_v2//:go_default_library",

cmd/validator/wallet/create_test.go

@@ -8,13 +8,10 @@ import (
 	"strconv"
 	"testing"
 
-	"github.com/pkg/errors"
-	cmdacc "github.com/prysmaticlabs/prysm/v4/cmd/validator/accounts"
 	"github.com/prysmaticlabs/prysm/v4/cmd/validator/flags"
 	"github.com/prysmaticlabs/prysm/v4/config/params"
 	"github.com/prysmaticlabs/prysm/v4/testing/assert"
 	"github.com/prysmaticlabs/prysm/v4/testing/require"
-	"github.com/prysmaticlabs/prysm/v4/validator/accounts"
 	"github.com/prysmaticlabs/prysm/v4/validator/accounts/wallet"
 	"github.com/prysmaticlabs/prysm/v4/validator/keymanager"
 	"github.com/prysmaticlabs/prysm/v4/validator/keymanager/local"
@@ -30,7 +27,7 @@ const (
 
 // `cmd/validator/accounts/delete_test.go`. https://pastebin.com/2n2VB7Ez is
 // the error I couldn't get around.
-func setupWalletAndPasswordsDir(t testing.TB) (string, string, string) {
+func SetupWalletAndPasswordsDir(t testing.TB) (string, string, string) {
 	walletDir := filepath.Join(t.TempDir(), "wallet")
 	passwordsDir := filepath.Join(t.TempDir(), "passwords")
 	passwordFileDir := filepath.Join(t.TempDir(), "passwordFile")
@@ -40,7 +37,7 @@ func setupWalletAndPasswordsDir(t testing.TB) (string, string, string) {
 	return walletDir, passwordsDir, passwordFilePath
 }
 
-type testWalletConfig struct {
+type TestWalletConfig struct {
 	exitAll                 bool
 	skipDepositConfirm      bool
 	keymanagerKind          keymanager.Kind
@@ -59,9 +56,9 @@ type testWalletConfig struct {
 	passwordsDir            string
 }
 
-func setupWalletCtx(
+func SetupWalletCtx(
 	tb testing.TB,
-	cfg *testWalletConfig,
+	cfg *TestWalletConfig,
 ) *cli.Context {
 	app := cli.App{}
 	set := flag.NewFlagSet("test", 0)
@@ -110,44 +107,27 @@ func init() {
 
 func TestCreateOrOpenWallet(t *testing.T) {
 	hook := logTest.NewGlobal()
-	walletDir, passwordsDir, walletPasswordFile := setupWalletAndPasswordsDir(t)
-	cliCtx := setupWalletCtx(t, &testWalletConfig{
+	walletDir, passwordsDir, walletPasswordFile := SetupWalletAndPasswordsDir(t)
+	cliCtx := SetupWalletCtx(t, &TestWalletConfig{
 		walletDir:          walletDir,
 		passwordsDir:       passwordsDir,
 		keymanagerKind:     keymanager.Local,
 		walletPasswordFile: walletPasswordFile,
 	})
-	createLocalWallet := func(cliCtx *cli.Context) (*wallet.Wallet, error) {
-		cfg, err := cmdacc.ExtractWalletDirPassword(cliCtx)
-		if err != nil {
-			return nil, err
-		}
-		w := wallet.New(&wallet.Config{
-			KeymanagerKind: keymanager.Local,
-			WalletDir:      cfg.Dir,
-			WalletPassword: cfg.Password,
-		})
-		if err = accounts.CreateLocalKeymanagerWallet(cliCtx.Context, w); err != nil {
-			return nil, errors.Wrap(err, "could not create keymanager")
-		}
-		log.WithField("wallet-path", cfg.Dir).Info(
-			"Successfully created new wallet",
-		)
-		return w, nil
-	}
-	createdWallet, err := wallet.OpenWalletOrElseCli(cliCtx, createLocalWallet)
+
+	createdWallet, err := wallet.OpenWalletOrElseCli(cliCtx, wallet.OpenOrCreateNewWallet)
 	require.NoError(t, err)
 	require.LogsContain(t, hook, "Successfully created new wallet")
 
-	openedWallet, err := wallet.OpenWalletOrElseCli(cliCtx, createLocalWallet)
+	openedWallet, err := wallet.OpenWalletOrElseCli(cliCtx, wallet.OpenOrCreateNewWallet)
 	require.NoError(t, err)
 	assert.Equal(t, createdWallet.KeymanagerKind(), openedWallet.KeymanagerKind())
 	assert.Equal(t, createdWallet.AccountsDir(), openedWallet.AccountsDir())
 }
 
 func TestCreateWallet_Local(t *testing.T) {
-	walletDir, passwordsDir, walletPasswordFile := setupWalletAndPasswordsDir(t)
-	cliCtx := setupWalletCtx(t, &testWalletConfig{
+	walletDir, passwordsDir, walletPasswordFile := SetupWalletAndPasswordsDir(t)
+	cliCtx := SetupWalletCtx(t, &TestWalletConfig{
 		walletDir:          walletDir,
 		passwordsDir:       passwordsDir,
 		keymanagerKind:     keymanager.Local,
@@ -168,8 +148,8 @@ func TestCreateWallet_Local(t *testing.T) {
 }
 
 func TestCreateWallet_Derived(t *testing.T) {
-	walletDir, passwordsDir, passwordFile := setupWalletAndPasswordsDir(t)
-	cliCtx := setupWalletCtx(t, &testWalletConfig{
+	walletDir, passwordsDir, passwordFile := SetupWalletAndPasswordsDir(t)
+	cliCtx := SetupWalletCtx(t, &TestWalletConfig{
 		walletDir:          walletDir,
 		passwordsDir:       passwordsDir,
 		walletPasswordFile: passwordFile,
@@ -190,8 +170,8 @@ func TestCreateWallet_Derived(t *testing.T) {
 
 // TestCreateWallet_WalletAlreadyExists checks for expected error if trying to create a wallet when there is one already.
 func TestCreateWallet_WalletAlreadyExists(t *testing.T) {
-	walletDir, passwordsDir, passwordFile := setupWalletAndPasswordsDir(t)
-	cliCtx := setupWalletCtx(t, &testWalletConfig{
+	walletDir, passwordsDir, passwordFile := SetupWalletAndPasswordsDir(t)
+	cliCtx := SetupWalletCtx(t, &TestWalletConfig{
 		walletDir:          walletDir,
 		passwordsDir:       passwordsDir,
 		walletPasswordFile: passwordFile,
@@ -207,7 +187,7 @@ func TestCreateWallet_WalletAlreadyExists(t *testing.T) {
 	_, err = CreateAndSaveWalletCli(cliCtx)
 	require.ErrorContains(t, "already exists", err)
 
-	cliCtx = setupWalletCtx(t, &testWalletConfig{
+	cliCtx = SetupWalletCtx(t, &TestWalletConfig{
 		walletDir:          walletDir,
 		passwordsDir:       passwordsDir,
 		walletPasswordFile: passwordFile,

validator/accounts/BUILD.bazel

@@ -34,7 +34,6 @@ go_library(
         "//io/file:go_default_library",
         "//io/prompt:go_default_library",
         "//proto/prysm/v1alpha1:go_default_library",
-        "//validator/accounts/iface:go_default_library",
         "//validator/accounts/petnames:go_default_library",
         "//validator/accounts/userprompt:go_default_library",
         "//validator/accounts/wallet:go_default_library",

validator/accounts/accounts_import.go

@@ -8,7 +8,6 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
-	"sort"
 	"strconv"
 	"strings"
 
@@ -81,52 +80,16 @@ func (acm *CLIManager) Import(ctx context.Context) error {
 	if !ok {
 		return errors.New("keymanager cannot import keystores")
 	}
-
+	log.Info("importing validator keystores...")
 	// Check if the user wishes to import a one-off, private key directly
 	// as an account into the Prysm validator.
 	if acm.importPrivateKeys {
 		return importPrivateKeyAsAccount(ctx, acm.wallet, k, acm.privateKeyFile)
 	}
 
-	// Consider that the keysDir might be a path to a specific file and handle accordingly.
-	isDir, err := file.HasDir(acm.keysDir)
+	keystoresImported, err := processDirectory(ctx, acm.keysDir, 0)
 	if err != nil {
-		return errors.Wrap(err, "could not determine if path is a directory")
-	}
-	keystoresImported := make([]*keymanager.Keystore, 0)
-	if isDir {
-		files, err := os.ReadDir(acm.keysDir)
-		if err != nil {
-			return errors.Wrap(err, "could not read dir")
-		}
-		if len(files) == 0 {
-			return fmt.Errorf("directory %s has no files, cannot import from it", acm.keysDir)
-		}
-		filesInDir := make([]string, 0)
-		for i := 0; i < len(files); i++ {
-			if files[i].IsDir() {
-				continue
-			}
-			filesInDir = append(filesInDir, files[i].Name())
-		}
-		// Sort the imported keystores by derivation path if they
-		// specify this value in their filename.
-		sort.Sort(byDerivationPath(filesInDir))
-		for _, name := range filesInDir {
-			keystore, err := readKeystoreFile(ctx, filepath.Join(acm.keysDir, name))
-			if err != nil && strings.Contains(err.Error(), "could not decode keystore json") {
-				continue
-			} else if err != nil {
-				return errors.Wrapf(err, "could not import keystore at path: %s", name)
-			}
-			keystoresImported = append(keystoresImported, keystore)
-		}
-	} else {
-		keystore, err := readKeystoreFile(ctx, acm.keysDir)
-		if err != nil {
-			return errors.Wrap(err, "could not import keystore")
-		}
-		keystoresImported = append(keystoresImported, keystore)
+		return errors.Wrap(err, "unable to process directory and import keys")
 	}
 
 	var accountsPassword string
@@ -176,6 +139,59 @@ func (acm *CLIManager) Import(ctx context.Context) error {
 	return nil
 }
 
+// Recursive function to process directories and files.
+func processDirectory(ctx context.Context, dir string, depth int) ([]*keymanager.Keystore, error) {
+	maxdepth := 2
+	if depth > maxdepth {
+		log.Infof("stopped checking folders for keystores after max depth of %d was reached", maxdepth)
+		return nil, nil // Stop recursion after two levels.
+	}
+	log.Infof("checking directory for keystores: %s", dir)
+	isDir, err := file.HasDir(dir)
+	if err != nil {
+		return nil, errors.Wrap(err, "could not determine if path is a directory")
+	}
+
+	keystoresImported := make([]*keymanager.Keystore, 0)
+
+	if isDir {
+		files, err := os.ReadDir(dir)
+		if err != nil {
+			return nil, errors.Wrap(err, "could not read dir")
+		}
+		if len(files) == 0 {
+			return nil, fmt.Errorf("directory %s has no files, cannot import from it", dir)
+		}
+		for _, f := range files {
+			fullPath := filepath.Join(dir, f.Name())
+			if f.IsDir() {
+				subKeystores, err := processDirectory(ctx, fullPath, depth+1)
+				if err != nil {
+					return nil, err
+				}
+				keystoresImported = append(keystoresImported, subKeystores...)
+			} else {
+				keystore, err := readKeystoreFile(ctx, fullPath)
+				if err != nil {
+					if strings.Contains(err.Error(), "could not decode keystore json") {
+						continue
+					}
+					return nil, errors.Wrapf(err, "could not import keystore at path: %s", fullPath)
+				}
+				keystoresImported = append(keystoresImported, keystore)
+			}
+		}
+	} else {
+		keystore, err := readKeystoreFile(ctx, dir)
+		if err != nil {
+			return nil, errors.Wrap(err, "could not import keystore")
+		}
+		keystoresImported = append(keystoresImported, keystore)
+	}
+
+	return keystoresImported, nil
+}
+
 // ImportAccounts can import external, EIP-2335 compliant keystore.json files as
 // new accounts into the Prysm validator wallet.
 func ImportAccounts(ctx context.Context, cfg *ImportAccountsConfig) ([]*keymanager.KeyStatus, error) {

validator/accounts/wallet/BUILD.bazel

@@ -34,12 +34,15 @@ go_test(
     srcs = ["wallet_test.go"],
     deps = [
         ":go_default_library",
+        "//cmd/validator/flags:go_default_library",
         "//config/params:go_default_library",
         "//testing/assert:go_default_library",
         "//testing/require:go_default_library",
         "//validator/accounts/iface:go_default_library",
+        "//validator/keymanager:go_default_library",
         "//validator/keymanager/remote-web3signer:go_default_library",
         "@com_github_ethereum_go_ethereum//common/hexutil:go_default_library",
         "@com_github_sirupsen_logrus//:go_default_library",
+        "@com_github_urfave_cli_v2//:go_default_library",
     ],
 )

validator/accounts/wallet/wallet.go

@@ -164,6 +164,7 @@ func OpenWalletOrElseCli(cliCtx *cli.Context, otherwise func(cliCtx *cli.Context
 	}
 	isValid, err := IsValid(cliCtx.String(flags.WalletDirFlag.Name))
 	if errors.Is(err, ErrNoWalletFound) {
+		// reprompts the user for a valid dir
 		return otherwise(cliCtx)
 	}
 	if err != nil {
@@ -193,6 +194,65 @@ func OpenWalletOrElseCli(cliCtx *cli.Context, otherwise func(cliCtx *cli.Context
 	})
 }
 
+// OpenOrCreateNewWallet takes a cli and returns a wallet either opening an existing valid wallet or creating a new one.
+func OpenOrCreateNewWallet(cliCtx *cli.Context) (*Wallet, error) {
+	walletDir, err := accountsprompt.InputDirectory(cliCtx, accountsprompt.WalletDirPromptText, flags.WalletDirFlag)
+	if err != nil {
+		return nil, err
+	}
+	exists, err := Exists(walletDir)
+	if err != nil {
+		return nil, errors.Wrap(err, CheckExistsErrMsg)
+	}
+	if exists {
+		isValid, err := IsValid(walletDir)
+		if err != nil {
+			return nil, errors.Wrap(err, CheckValidityErrMsg)
+		}
+		if !isValid {
+			return nil, errors.New(InvalidWalletErrMsg)
+		}
+		walletPassword, err := InputPassword(
+			cliCtx,
+			flags.WalletPasswordFileFlag,
+			PasswordPromptText,
+			false, /* Do not confirm password */
+			ValidateExistingPass,
+		)
+		if err != nil {
+			return nil, err
+		}
+		return OpenWallet(cliCtx.Context, &Config{
+			WalletDir:      walletDir,
+			WalletPassword: walletPassword,
+		})
+	}
+	// create a new wallet in the dir
+	walletPassword, err := prompt.InputPassword(
+		cliCtx,
+		flags.WalletPasswordFileFlag,
+		NewWalletPasswordPromptText,
+		ConfirmPasswordPromptText,
+		true, /* Should confirm password */
+		prompt.ValidatePasswordInput,
+	)
+	if err != nil {
+		return nil, err
+	}
+	w := New(&Config{
+		KeymanagerKind: keymanager.Local,
+		WalletDir:      walletDir,
+		WalletPassword: walletPassword,
+	})
+	if err := w.SaveWallet(); err != nil {
+		return nil, errors.Wrap(err, "could not save wallet to disk")
+	}
+	log.WithField("wallet-path", walletDir).Info(
+		"Successfully created new wallet",
+	)
+	return w, nil
+}
+
 // NewWalletForWeb3Signer returns a new wallet for web3 signer which is temporary and not stored locally.
 func NewWalletForWeb3Signer() *Wallet {
 	// wallet is just a temporary wallet for web3 signer used to call initialize keymanager.

validator/accounts/wallet/wallet_test.go

@@ -2,19 +2,24 @@ package wallet_test
 
 import (
 	"context"
+	"flag"
 	"io"
 	"os"
 	"path/filepath"
+	"reflect"
 	"testing"
 
 	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/prysmaticlabs/prysm/v4/cmd/validator/flags"
 	"github.com/prysmaticlabs/prysm/v4/config/params"
 	"github.com/prysmaticlabs/prysm/v4/testing/assert"
 	"github.com/prysmaticlabs/prysm/v4/testing/require"
 	"github.com/prysmaticlabs/prysm/v4/validator/accounts/iface"
 	"github.com/prysmaticlabs/prysm/v4/validator/accounts/wallet"
+	"github.com/prysmaticlabs/prysm/v4/validator/keymanager"
 	remoteweb3signer "github.com/prysmaticlabs/prysm/v4/validator/keymanager/remote-web3signer"
 	"github.com/sirupsen/logrus"
+	"github.com/urfave/cli/v2"
 )
 
 func init() {
@@ -84,3 +89,85 @@ func TestWallet_InitializeKeymanager_web3Signer_nilConfig(t *testing.T) {
 	assert.NotNil(t, err)
 	assert.Equal(t, nil, km)
 }
+
+func TestOpenOrCreateNewWallet(t *testing.T) {
+	walletDir := filepath.Join(t.TempDir(), "wallet")
+	newDir := filepath.Join(t.TempDir(), "new")
+	passwordFileDir := filepath.Join(t.TempDir(), "passwordFile")
+	require.NoError(t, os.MkdirAll(passwordFileDir, params.BeaconIoConfig().ReadWriteExecutePermissions))
+	passwordFilePath1 := filepath.Join(passwordFileDir, "password1.txt")
+	passwordFilePath2 := filepath.Join(passwordFileDir, "password2.txt")
+
+	type args struct {
+		cliCtx *cli.Context
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    *wallet.Wallet
+		wantErr bool
+	}{
+		{
+			name: "New Wallet",
+			args: args{
+				cliCtx: func() *cli.Context {
+					app := cli.App{}
+					set := flag.NewFlagSet("test", 0)
+					require.NoError(t, os.MkdirAll(newDir, 0700))
+					require.NoError(t, os.WriteFile(passwordFilePath1, []byte("newnewnew"), os.ModePerm))
+					set.String(flags.WalletDirFlag.Name, newDir, "") // don't set it
+					set.String(flags.KeymanagerKindFlag.Name, keymanager.Local.String(), "")
+					set.String(flags.WalletPasswordFileFlag.Name, passwordFilePath1, "")
+					assert.NoError(t, set.Set(flags.KeymanagerKindFlag.Name, keymanager.Local.String()))
+					assert.NoError(t, set.Set(flags.WalletPasswordFileFlag.Name, passwordFilePath1))
+					return cli.NewContext(&app, set, nil)
+				}(),
+			},
+			want: wallet.New(&wallet.Config{
+				WalletDir:      newDir,
+				KeymanagerKind: keymanager.Local,
+				WalletPassword: "newnewnew",
+			}),
+		},
+		{
+			name: "Existing Wallet",
+			args: args{
+				cliCtx: func() *cli.Context {
+					app := cli.App{}
+					set := flag.NewFlagSet("test", 0)
+					set.String(flags.WalletDirFlag.Name, walletDir, "")
+					set.String(flags.KeymanagerKindFlag.Name, keymanager.Local.String(), "")
+					set.String(flags.WalletPasswordFileFlag.Name, passwordFilePath2, "")
+					require.NoError(t, os.WriteFile(passwordFilePath2, []byte("existing"), os.ModePerm))
+					w := wallet.New(&wallet.Config{
+						WalletDir:      walletDir,
+						KeymanagerKind: keymanager.Local,
+						WalletPassword: "existing",
+					})
+					require.NoError(t, w.SaveWallet())
+					assert.NoError(t, set.Set(flags.WalletDirFlag.Name, walletDir))
+					assert.NoError(t, set.Set(flags.KeymanagerKindFlag.Name, keymanager.Local.String()))
+					assert.NoError(t, set.Set(flags.WalletPasswordFileFlag.Name, passwordFilePath2))
+					return cli.NewContext(&app, set, nil)
+				}(),
+			},
+			want: wallet.New(&wallet.Config{
+				WalletDir:      walletDir,
+				KeymanagerKind: keymanager.Local,
+				WalletPassword: "existing",
+			}),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := wallet.OpenOrCreateNewWallet(tt.args.cliCtx)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("OpenOrCreateNewWallet() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("OpenOrCreateNewWallet() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

validator/accounts/wallet_create.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 
 	"github.com/pkg/errors"
-	"github.com/prysmaticlabs/prysm/v4/validator/accounts/iface"
 	"github.com/prysmaticlabs/prysm/v4/validator/accounts/wallet"
 	"github.com/prysmaticlabs/prysm/v4/validator/keymanager"
 	"github.com/prysmaticlabs/prysm/v4/validator/keymanager/derived"
@@ -22,19 +21,10 @@ func (acm *CLIManager) WalletCreate(ctx context.Context) (*wallet.Wallet, error)
 	var err error
 	switch w.KeymanagerKind() {
 	case keymanager.Local:
-		if err = CreateLocalKeymanagerWallet(ctx, w); err != nil {
-			return nil, errors.Wrap(err, "could not initialize wallet")
+		if err := w.SaveWallet(); err != nil {
+			return nil, errors.Wrap(err, "could not initialize wallet: could not save wallet to disk")
 		}
-		// TODO(#9883) - Remove this when we have a better way to handle this. should be safe to use for now.
-		km, err := w.InitializeKeymanager(ctx, iface.InitKeymanagerConfig{ListenForChanges: false})
-		if err != nil {
-			return nil, errors.Wrap(err, ErrCouldNotInitializeKeymanager)
-		}
-		localKm, ok := km.(*local.Keymanager)
-		if !ok {
-			return nil, errors.Wrap(err, ErrCouldNotInitializeKeymanager)
-		}
-		accountsKeystore, err := localKm.CreateAccountsKeystore(ctx, make([][]byte, 0), make([][]byte, 0))
+		accountsKeystore, err := local.CreateEmptyKeyStoreRepresentationForNewWallet(ctx, w.Password())
 		if err != nil {
 			return nil, err
 		}
@@ -45,7 +35,6 @@ func (acm *CLIManager) WalletCreate(ctx context.Context) (*wallet.Wallet, error)
 		if err = w.WriteFileAtPath(ctx, local.AccountsPath, local.AccountsKeystoreFileName, encodedAccounts); err != nil {
 			return nil, err
 		}
-
 		log.WithField("--wallet-dir", acm.walletDir).Info(
 			"Successfully created wallet with ability to import keystores",
 		)
@@ -71,16 +60,6 @@ func (acm *CLIManager) WalletCreate(ctx context.Context) (*wallet.Wallet, error)
 	return w, nil
 }
 
-func CreateLocalKeymanagerWallet(_ context.Context, wallet *wallet.Wallet) error {
-	if wallet == nil {
-		return errors.New("nil wallet")
-	}
-	if err := wallet.SaveWallet(); err != nil {
-		return errors.Wrap(err, "could not save wallet to disk")
-	}
-	return nil
-}
-
 func createDerivedKeymanagerWallet(
 	ctx context.Context,
 	wallet *wallet.Wallet,

validator/keymanager/local/keymanager.go

@@ -323,6 +323,13 @@ func CreateAccountsKeystoreRepresentation(
 	}, nil
 }
 
+// CreateEmptyKeyStoreRepresentationForNewWallet creates a placeholder accounts keystore for a new Prysm Local Wallet.
+func CreateEmptyKeyStoreRepresentationForNewWallet(ctx context.Context, walletPassword string) (*AccountsKeystoreRepresentation, error) {
+	// make sure everything is clean when creating this.
+	ResetCaches()
+	return CreateAccountsKeystoreRepresentation(ctx, &accountStore{}, walletPassword)
+}
+
 // CreateOrUpdateInMemoryAccountsStore will set or update the local accounts store and update the local cache.
 // This function DOES NOT save the accounts store to disk.
 func (km *Keymanager) CreateOrUpdateInMemoryAccountsStore(_ context.Context, privateKeys, publicKeys [][]byte) error {
@@ -425,13 +432,10 @@ func (km *Keymanager) ListKeymanagerAccounts(ctx context.Context, cfg keymanager
 func CreatePrintoutOfKeys(keys [][]byte) string {
 	var keysStr string
 	for i, k := range keys {
-		if i == 0 {
-			keysStr += fmt.Sprintf("%#x", bytesutil.Trunc(k))
-		} else if i == len(keys)-1 {
-			keysStr += fmt.Sprintf("%#x", bytesutil.Trunc(k))
-		} else {
-			keysStr += fmt.Sprintf(",%#x", bytesutil.Trunc(k))
+		if i != 0 {
+			keysStr += "," // Add a comma before each key except the first one
 		}
+		keysStr += fmt.Sprintf("%#x", bytesutil.Trunc(k))
 	}
 	return keysStr
 }

validator/node/node.go

@@ -216,6 +216,7 @@ func (c *ValidatorClient) initializeFromCLI(cliCtx *cli.Context, router *mux.Rou
 		if cliCtx.IsSet(flags.Web3SignerURLFlag.Name) {
 			c.wallet = wallet.NewWalletForWeb3Signer()
 		} else {
+			fmt.Println("initializeFromCLI asking for wallet")
 			w, err := wallet.OpenWalletOrElseCli(cliCtx, func(cliCtx *cli.Context) (*wallet.Wallet, error) {
 				return nil, wallet.ErrNoWalletFound
 			})