Refactor validator accounts delete to remove cli context dependency (#…

…10686)

* add functional options accounts delete

* bazel run //:gazelle -- fix

Co-authored-by: james-prysm <90280386+james-prysm@users.noreply.github.com>
Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>

cmd/validator/accounts/BUILD.bazel

@@ -1,10 +1,12 @@
-load("@prysm//tools/go:def.bzl", "go_library")
+load("@prysm//tools/go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
     srcs = [
         "accounts.go",
+        "delete.go",
         "list.go",
+        "wallet_utils.go",
     ],
     importpath = "github.com/prysmaticlabs/prysm/cmd/validator/accounts",
     visibility = ["//visibility:public"],
@@ -15,6 +17,7 @@ go_library(
         "//runtime/tos:go_default_library",
         "//validator/accounts:go_default_library",
         "//validator/accounts/iface:go_default_library",
+        "//validator/accounts/userprompt:go_default_library",
         "//validator/accounts/wallet:go_default_library",
         "//validator/client:go_default_library",
         "//validator/keymanager:go_default_library",
@@ -23,3 +26,25 @@ go_library(
         "@com_github_urfave_cli_v2//:go_default_library",
     ],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["delete_test.go"],
+    embed = [":go_default_library"],
+    deps = [
+        "//cmd/validator/flags:go_default_library",
+        "//config/params:go_default_library",
+        "//crypto/bls:go_default_library",
+        "//encoding/bytesutil:go_default_library",
+        "//testing/assert:go_default_library",
+        "//testing/require:go_default_library",
+        "//time:go_default_library",
+        "//validator/accounts:go_default_library",
+        "//validator/accounts/wallet:go_default_library",
+        "//validator/keymanager:go_default_library",
+        "//validator/keymanager/local:go_default_library",
+        "@com_github_google_uuid//:go_default_library",
+        "@com_github_urfave_cli_v2//:go_default_library",
+        "@com_github_wealdtech_go_eth2_wallet_encryptor_keystorev4//:go_default_library",
+    ],
+)

cmd/validator/accounts/accounts.go

@@ -35,11 +35,14 @@ var Commands = &cli.Command{
 				if err := cmd.LoadFlagsFromConfig(cliCtx, cliCtx.Command.Flags); err != nil {
 					return err
 				}
-				return tos.VerifyTosAcceptedOrPrompt(cliCtx)
+				if err := tos.VerifyTosAcceptedOrPrompt(cliCtx); err != nil {
+					return err
+				}
+				features.ConfigureValidator(cliCtx)
+				return nil
 			},
 			Action: func(cliCtx *cli.Context) error {
-				features.ConfigureValidator(cliCtx)
-				if err := accounts.DeleteAccountCli(cliCtx); err != nil {
+				if err := accountsDelete(cliCtx); err != nil {
 					log.Fatalf("Could not delete account: %v", err)
 				}
 				return nil

cmd/validator/accounts/delete.go

@@ -0,0 +1,63 @@
+package accounts
+
+import (
+	"strings"
+
+	"github.com/pkg/errors"
+	"github.com/prysmaticlabs/prysm/cmd"
+	"github.com/prysmaticlabs/prysm/cmd/validator/flags"
+	"github.com/prysmaticlabs/prysm/validator/accounts"
+	"github.com/prysmaticlabs/prysm/validator/accounts/userprompt"
+	"github.com/prysmaticlabs/prysm/validator/client"
+	"github.com/urfave/cli/v2"
+)
+
+func accountsDelete(c *cli.Context) error {
+	w, km, err := walletWithKeymanager(c)
+	if err != nil {
+		return err
+	}
+	dialOpts := client.ConstructDialOptions(
+		c.Int(cmd.GrpcMaxCallRecvMsgSizeFlag.Name),
+		c.String(flags.CertFlag.Name),
+		c.Uint(flags.GrpcRetriesFlag.Name),
+		c.Duration(flags.GrpcRetryDelayFlag.Name),
+	)
+	grpcHeaders := strings.Split(c.String(flags.GrpcHeadersFlag.Name), ",")
+
+	opts := []accounts.Option{
+		accounts.WithWallet(w),
+		accounts.WithKeymanager(km),
+		accounts.WithGRPCDialOpts(dialOpts),
+		accounts.WithBeaconRPCProvider(c.String(flags.BeaconRPCProviderFlag.Name)),
+		accounts.WithGRPCHeaders(grpcHeaders),
+	}
+
+	// Get full set of public keys from the keymanager.
+	validatingPublicKeys, err := km.FetchValidatingPublicKeys(c.Context)
+	if err != nil {
+		return err
+	}
+	if len(validatingPublicKeys) == 0 {
+		return errors.New("wallet is empty, no accounts to delete")
+	}
+	// Filter keys either from CLI flag or from interactive session.
+	filteredPubKeys, err := accounts.FilterPublicKeysFromUserInput(
+		c,
+		flags.DeletePublicKeysFlag,
+		validatingPublicKeys,
+		userprompt.SelectAccountsDeletePromptText,
+	)
+	if err != nil {
+		return errors.Wrap(err, "could not filter public keys for deletion")
+	}
+	opts = append(opts, accounts.WithFilteredPubKeys(filteredPubKeys))
+	opts = append(opts, accounts.WithWalletKeyCount(len(validatingPublicKeys)))
+	opts = append(opts, accounts.WithDeletePublicKeys(c.IsSet(flags.DeletePublicKeysFlag.Name)))
+
+	acc, err := accounts.NewCLIManager(opts...)
+	if err != nil {
+		return err
+	}
+	return acc.Delete(c.Context)
+}

cmd/validator/accounts/delete_test.go

@@ -0,0 +1,191 @@
+package accounts
+
+import (
+	"encoding/hex"
+	"encoding/json"
+	"flag"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/google/uuid"
+	"github.com/prysmaticlabs/prysm/cmd/validator/flags"
+	"github.com/prysmaticlabs/prysm/config/params"
+	"github.com/prysmaticlabs/prysm/crypto/bls"
+	"github.com/prysmaticlabs/prysm/encoding/bytesutil"
+	"github.com/prysmaticlabs/prysm/testing/assert"
+	"github.com/prysmaticlabs/prysm/testing/require"
+	prysmTime "github.com/prysmaticlabs/prysm/time"
+	"github.com/prysmaticlabs/prysm/validator/accounts"
+	"github.com/prysmaticlabs/prysm/validator/accounts/wallet"
+	"github.com/prysmaticlabs/prysm/validator/keymanager"
+	"github.com/prysmaticlabs/prysm/validator/keymanager/local"
+	"github.com/urfave/cli/v2"
+	keystorev4 "github.com/wealdtech/go-eth2-wallet-encryptor-keystorev4"
+)
+
+const (
+	passwordFileName = "password.txt"
+	password         = "OhWOWthisisatest42!$"
+)
+
+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")
+	require.NoError(t, os.MkdirAll(passwordFileDir, params.BeaconIoConfig().ReadWriteExecutePermissions))
+	passwordFilePath := filepath.Join(passwordFileDir, passwordFileName)
+	require.NoError(t, os.WriteFile(passwordFilePath, []byte(password), os.ModePerm))
+	return walletDir, passwordsDir, passwordFilePath
+}
+
+// Returns the fullPath to the newly created keystore file.
+func createKeystore(t *testing.T, path string) (*keymanager.Keystore, string) {
+	validatingKey, err := bls.RandKey()
+	require.NoError(t, err)
+	encryptor := keystorev4.New()
+	cryptoFields, err := encryptor.Encrypt(validatingKey.Marshal(), password)
+	require.NoError(t, err)
+	id, err := uuid.NewRandom()
+	require.NoError(t, err)
+	keystoreFile := &keymanager.Keystore{
+		Crypto:  cryptoFields,
+		ID:      id.String(),
+		Pubkey:  fmt.Sprintf("%x", validatingKey.PublicKey().Marshal()),
+		Version: encryptor.Version(),
+		Name:    encryptor.Name(),
+	}
+	encoded, err := json.MarshalIndent(keystoreFile, "", "\t")
+	require.NoError(t, err)
+	// Write the encoded keystore to disk with the timestamp appended
+	createdAt := prysmTime.Now().Unix()
+	fullPath := filepath.Join(path, fmt.Sprintf(local.KeystoreFileNameFormat, createdAt))
+	require.NoError(t, os.WriteFile(fullPath, encoded, os.ModePerm))
+	return keystoreFile, fullPath
+}
+
+type testWalletConfig struct {
+	exitAll                 bool
+	skipDepositConfirm      bool
+	keymanagerKind          keymanager.Kind
+	numAccounts             int64
+	grpcHeaders             string
+	privateKeyFile          string
+	accountPasswordFile     string
+	walletPasswordFile      string
+	backupPasswordFile      string
+	backupPublicKeys        string
+	voluntaryExitPublicKeys string
+	deletePublicKeys        string
+	keysDir                 string
+	backupDir               string
+	walletDir               string
+}
+
+func setupWalletCtx(
+	tb testing.TB,
+	cfg *testWalletConfig,
+) *cli.Context {
+	app := cli.App{}
+	set := flag.NewFlagSet("test", 0)
+	set.String(flags.WalletDirFlag.Name, cfg.walletDir, "")
+	set.String(flags.KeysDirFlag.Name, cfg.keysDir, "")
+	set.String(flags.KeymanagerKindFlag.Name, cfg.keymanagerKind.String(), "")
+	set.String(flags.DeletePublicKeysFlag.Name, cfg.deletePublicKeys, "")
+	set.String(flags.VoluntaryExitPublicKeysFlag.Name, cfg.voluntaryExitPublicKeys, "")
+	set.String(flags.BackupDirFlag.Name, cfg.backupDir, "")
+	set.String(flags.BackupPasswordFile.Name, cfg.backupPasswordFile, "")
+	set.String(flags.BackupPublicKeysFlag.Name, cfg.backupPublicKeys, "")
+	set.String(flags.WalletPasswordFileFlag.Name, cfg.walletPasswordFile, "")
+	set.String(flags.AccountPasswordFileFlag.Name, cfg.accountPasswordFile, "")
+	set.Int64(flags.NumAccountsFlag.Name, cfg.numAccounts, "")
+	set.Bool(flags.SkipDepositConfirmationFlag.Name, cfg.skipDepositConfirm, "")
+	set.Bool(flags.SkipMnemonic25thWordCheckFlag.Name, true, "")
+	set.Bool(flags.ExitAllFlag.Name, cfg.exitAll, "")
+	set.String(flags.GrpcHeadersFlag.Name, cfg.grpcHeaders, "")
+
+	if cfg.privateKeyFile != "" {
+		set.String(flags.ImportPrivateKeyFileFlag.Name, cfg.privateKeyFile, "")
+		assert.NoError(tb, set.Set(flags.ImportPrivateKeyFileFlag.Name, cfg.privateKeyFile))
+	}
+	assert.NoError(tb, set.Set(flags.WalletDirFlag.Name, cfg.walletDir))
+	assert.NoError(tb, set.Set(flags.SkipMnemonic25thWordCheckFlag.Name, "true"))
+	assert.NoError(tb, set.Set(flags.KeysDirFlag.Name, cfg.keysDir))
+	assert.NoError(tb, set.Set(flags.KeymanagerKindFlag.Name, cfg.keymanagerKind.String()))
+	assert.NoError(tb, set.Set(flags.DeletePublicKeysFlag.Name, cfg.deletePublicKeys))
+	assert.NoError(tb, set.Set(flags.VoluntaryExitPublicKeysFlag.Name, cfg.voluntaryExitPublicKeys))
+	assert.NoError(tb, set.Set(flags.BackupDirFlag.Name, cfg.backupDir))
+	assert.NoError(tb, set.Set(flags.BackupPublicKeysFlag.Name, cfg.backupPublicKeys))
+	assert.NoError(tb, set.Set(flags.BackupPasswordFile.Name, cfg.backupPasswordFile))
+	assert.NoError(tb, set.Set(flags.WalletPasswordFileFlag.Name, cfg.walletPasswordFile))
+	assert.NoError(tb, set.Set(flags.AccountPasswordFileFlag.Name, cfg.accountPasswordFile))
+	assert.NoError(tb, set.Set(flags.NumAccountsFlag.Name, strconv.Itoa(int(cfg.numAccounts))))
+	assert.NoError(tb, set.Set(flags.SkipDepositConfirmationFlag.Name, strconv.FormatBool(cfg.skipDepositConfirm)))
+	assert.NoError(tb, set.Set(flags.ExitAllFlag.Name, strconv.FormatBool(cfg.exitAll)))
+	assert.NoError(tb, set.Set(flags.GrpcHeadersFlag.Name, cfg.grpcHeaders))
+	return cli.NewContext(&app, set, nil)
+}
+
+func TestDeleteAccounts_Noninteractive(t *testing.T) {
+	walletDir, _, passwordFilePath := setupWalletAndPasswordsDir(t)
+	// Write a directory where we will import keys from.
+	keysDir := filepath.Join(t.TempDir(), "keysDir")
+	require.NoError(t, os.MkdirAll(keysDir, os.ModePerm))
+
+	// Create 3 keystore files in the keys directory we can then
+	// import from in our wallet.
+	k1, _ := createKeystore(t, keysDir)
+	time.Sleep(time.Second)
+	k2, _ := createKeystore(t, keysDir)
+	time.Sleep(time.Second)
+	k3, _ := createKeystore(t, keysDir)
+	generatedPubKeys := []string{k1.Pubkey, k2.Pubkey, k3.Pubkey}
+	// Only delete keys 0 and 1.
+	deletePublicKeys := strings.Join(generatedPubKeys[0:2], ",")
+
+	// We initialize a wallet with a local keymanager.
+	cliCtx := setupWalletCtx(t, &testWalletConfig{
+		// Wallet configuration flags.
+		walletDir:           walletDir,
+		keymanagerKind:      keymanager.Local,
+		walletPasswordFile:  passwordFilePath,
+		accountPasswordFile: passwordFilePath,
+		// Flags required for ImportAccounts to work.
+		keysDir: keysDir,
+		// Flags required for DeleteAccounts to work.
+		deletePublicKeys: deletePublicKeys,
+	})
+	w, err := accounts.CreateWalletWithKeymanager(cliCtx.Context, &accounts.CreateWalletConfig{
+		WalletCfg: &wallet.Config{
+			WalletDir:      walletDir,
+			KeymanagerKind: keymanager.Local,
+			WalletPassword: password,
+		},
+	})
+	require.NoError(t, err)
+
+	// We attempt to import accounts.
+	require.NoError(t, accounts.ImportAccountsCli(cliCtx))
+
+	// We attempt to delete the accounts specified.
+	require.NoError(t, accountsDelete(cliCtx))
+
+	keymanager, err := local.NewKeymanager(
+		cliCtx.Context,
+		&local.SetupConfig{
+			Wallet:           w,
+			ListenForChanges: false,
+		},
+	)
+	require.NoError(t, err)
+	remainingAccounts, err := keymanager.FetchValidatingPublicKeys(cliCtx.Context)
+	require.NoError(t, err)
+	require.Equal(t, len(remainingAccounts), 1)
+	remainingPublicKey, err := hex.DecodeString(k3.Pubkey)
+	require.NoError(t, err)
+	assert.DeepEqual(t, remainingAccounts[0], bytesutil.ToBytes48(remainingPublicKey))
+}

cmd/validator/accounts/list.go

@@ -3,14 +3,10 @@ package accounts
 import (
 	"strings"
 
-	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/prysm/cmd"
 	"github.com/prysmaticlabs/prysm/cmd/validator/flags"
 	"github.com/prysmaticlabs/prysm/validator/accounts"
-	"github.com/prysmaticlabs/prysm/validator/accounts/iface"
-	"github.com/prysmaticlabs/prysm/validator/accounts/wallet"
 	"github.com/prysmaticlabs/prysm/validator/client"
-	"github.com/prysmaticlabs/prysm/validator/keymanager"
 	"github.com/urfave/cli/v2"
 )
 
@@ -51,22 +47,3 @@ func accountsList(c *cli.Context) error {
 		c.Context,
 	)
 }
-
-func walletWithKeymanager(c *cli.Context) (*wallet.Wallet, keymanager.IKeymanager, error) {
-	w, err := wallet.OpenWalletOrElseCli(c, func(cliCtx *cli.Context) (*wallet.Wallet, error) {
-		return nil, wallet.ErrNoWalletFound
-	})
-	if err != nil {
-		return nil, nil, errors.Wrap(err, "could not open wallet")
-	}
-	// TODO(#9883) - Remove this when we have a better way to handle this. this is fine.
-	// genesis root is not set here which is used for sign function, but fetch keys should be fine.
-	km, err := w.InitializeKeymanager(c.Context, iface.InitKeymanagerConfig{ListenForChanges: false})
-	if err != nil && strings.Contains(err.Error(), keymanager.IncorrectPasswordErrMsg) {
-		return nil, nil, errors.New("wrong wallet password entered")
-	}
-	if err != nil {
-		return nil, nil, errors.Wrap(err, accounts.ErrCouldNotInitializeKeymanager)
-	}
-	return w, km, nil
-}

cmd/validator/accounts/wallet_utils.go

@@ -0,0 +1,31 @@
+package accounts
+
+import (
+	"strings"
+
+	"github.com/pkg/errors"
+	"github.com/prysmaticlabs/prysm/validator/accounts"
+	"github.com/prysmaticlabs/prysm/validator/accounts/iface"
+	"github.com/prysmaticlabs/prysm/validator/accounts/wallet"
+	"github.com/prysmaticlabs/prysm/validator/keymanager"
+	"github.com/urfave/cli/v2"
+)
+
+func walletWithKeymanager(c *cli.Context) (*wallet.Wallet, keymanager.IKeymanager, error) {
+	w, err := wallet.OpenWalletOrElseCli(c, func(cliCtx *cli.Context) (*wallet.Wallet, error) {
+		return nil, wallet.ErrNoWalletFound
+	})
+	if err != nil {
+		return nil, nil, errors.Wrap(err, "could not open wallet")
+	}
+	// TODO(#9883) - Remove this when we have a better way to handle this. this is fine.
+	// genesis root is not set here which is used for sign function, but fetch keys should be fine.
+	km, err := w.InitializeKeymanager(c.Context, iface.InitKeymanagerConfig{ListenForChanges: false})
+	if err != nil && strings.Contains(err.Error(), keymanager.IncorrectPasswordErrMsg) {
+		return nil, nil, errors.New("wrong wallet password entered")
+	}
+	if err != nil {
+		return nil, nil, errors.Wrap(err, accounts.ErrCouldNotInitializeKeymanager)
+	}
+	return w, km, nil
+}

validator/accounts/BUILD.bazel

@@ -66,7 +66,6 @@ go_test(
     name = "go_default_test",
     srcs = [
         "accounts_backup_test.go",
-        "accounts_delete_test.go",
         "accounts_exit_test.go",
         "accounts_import_test.go",
         "accounts_list_test.go",