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>

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",

validator/accounts/accounts_backup.go

@@ -69,7 +69,7 @@ func BackupAccountsCli(cliCtx *cli.Context) error {
 
 	// Allow the user to interactively select the accounts to backup or optionally
 	// provide them via cli flags as a string of comma-separated, hex strings.
-	filteredPubKeys, err := filterPublicKeysFromUserInput(
+	filteredPubKeys, err := FilterPublicKeysFromUserInput(
 		cliCtx,
 		flags.BackupPublicKeysFlag,
 		pubKeys,

validator/accounts/accounts_delete.go

@@ -7,64 +7,23 @@ import (
 	"strings"
 
 	"github.com/pkg/errors"
-	"github.com/prysmaticlabs/prysm/cmd/validator/flags"
 	"github.com/prysmaticlabs/prysm/encoding/bytesutil"
 	"github.com/prysmaticlabs/prysm/io/prompt"
 	ethpbservice "github.com/prysmaticlabs/prysm/proto/eth/service"
-	"github.com/prysmaticlabs/prysm/validator/accounts/iface"
-	"github.com/prysmaticlabs/prysm/validator/accounts/userprompt"
-	"github.com/prysmaticlabs/prysm/validator/accounts/wallet"
-	"github.com/prysmaticlabs/prysm/validator/keymanager"
-	"github.com/urfave/cli/v2"
 )
 
-// DeleteAccountCli deletes the accounts that the user requests to be deleted from the wallet.
-// This function uses the CLI to extract necessary values.
-func DeleteAccountCli(cliCtx *cli.Context) error {
-	w, err := wallet.OpenWalletOrElseCli(cliCtx, func(cliCtx *cli.Context) (*wallet.Wallet, error) {
-		return nil, wallet.ErrNoWalletFound
-	})
-	if err != nil {
-		return errors.Wrap(err, "could not open wallet")
-	}
-	// TODO(#9883) - Remove this when we have a better way to handle this.
-	if w.KeymanagerKind() == keymanager.Remote || w.KeymanagerKind() == keymanager.Web3Signer {
-		return errors.New(
-			"remote and web3signer wallets cannot delete accounts locally. please delete the account on the remote signer node",
-		)
-	}
-	kManager, err := w.InitializeKeymanager(cliCtx.Context, iface.InitKeymanagerConfig{ListenForChanges: false})
-	if err != nil {
-		return errors.Wrap(err, ErrCouldNotInitializeKeymanager)
-	}
-	validatingPublicKeys, err := kManager.FetchValidatingPublicKeys(cliCtx.Context)
-	if err != nil {
-		return err
-	}
-	if len(validatingPublicKeys) == 0 {
-		return errors.New("wallet is empty, no accounts to delete")
-	}
-	// Allow the user to interactively select the accounts to delete or optionally
-	// provide them via cli flags as a string of comma-separated, hex strings.
-	filteredPubKeys, err := filterPublicKeysFromUserInput(
-		cliCtx,
-		flags.DeletePublicKeysFlag,
-		validatingPublicKeys,
-		userprompt.SelectAccountsDeletePromptText,
-	)
-	if err != nil {
-		return errors.Wrap(err, "could not filter public keys for deletion")
-	}
-	rawPublicKeys := make([][]byte, len(filteredPubKeys))
-	formattedPubKeys := make([]string, len(filteredPubKeys))
-	for i, pk := range filteredPubKeys {
+// Deletes the accounts that the user requests to be deleted from the wallet.
+func (acm *AccountsCLIManager) Delete(ctx context.Context) error {
+	rawPublicKeys := make([][]byte, len(acm.filteredPubKeys))
+	formattedPubKeys := make([]string, len(acm.filteredPubKeys))
+	for i, pk := range acm.filteredPubKeys {
 		pubKeyBytes := pk.Marshal()
 		rawPublicKeys[i] = pubKeyBytes
 		formattedPubKeys[i] = fmt.Sprintf("%#x", bytesutil.Trunc(pubKeyBytes))
 	}
 	allAccountStr := strings.Join(formattedPubKeys, ", ")
-	if !cliCtx.IsSet(flags.DeletePublicKeysFlag.Name) {
-		if len(filteredPubKeys) == 1 {
+	if !acm.deletePublicKeys {
+		if len(acm.filteredPubKeys) == 1 {
 			promptText := "Are you sure you want to delete 1 account? (%s) Y/N"
 			resp, err := prompt.ValidatePrompt(
 				os.Stdin, fmt.Sprintf(promptText, au.BrightGreen(formattedPubKeys[0])), prompt.ValidateYesOrNo,
@@ -77,10 +36,10 @@ func DeleteAccountCli(cliCtx *cli.Context) error {
 			}
 		} else {
 			promptText := "Are you sure you want to delete %d accounts? (%s) Y/N"
-			if len(filteredPubKeys) == len(validatingPublicKeys) {
+			if len(acm.filteredPubKeys) == acm.walletKeyCount {
 				promptText = fmt.Sprintf("Are you sure you want to delete all accounts? Y/N (%s)", au.BrightGreen(allAccountStr))
 			} else {
-				promptText = fmt.Sprintf(promptText, len(filteredPubKeys), au.BrightGreen(allAccountStr))
+				promptText = fmt.Sprintf(promptText, len(acm.filteredPubKeys), au.BrightGreen(allAccountStr))
 			}
 			resp, err := prompt.ValidatePrompt(os.Stdin, promptText, prompt.ValidateYesOrNo)
 			if err != nil {
@@ -91,8 +50,8 @@ func DeleteAccountCli(cliCtx *cli.Context) error {
 			}
 		}
 	}
-	if err := DeleteAccount(cliCtx.Context, &DeleteConfig{
-		Keymanager:       kManager,
+	if err := DeleteAccount(ctx, &DeleteConfig{
+		Keymanager:       acm.keymanager,
 		DeletePublicKeys: rawPublicKeys,
 	}); err != nil {
 		return err
@@ -104,7 +63,7 @@ func DeleteAccountCli(cliCtx *cli.Context) error {
 	return nil
 }
 
-// DeleteAccount deletes the accounts that the user requests to be deleted from the wallet.
+// DeleteAccount permforms the deletion on the Keymanager.
 func DeleteAccount(ctx context.Context, cfg *DeleteConfig) error {
 	if len(cfg.DeletePublicKeys) == 1 {
 		log.Info("Deleting account...")

validator/accounts/accounts_delete_test.go

@@ -1,77 +0,0 @@
-package accounts
-
-import (
-	"encoding/hex"
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-	"time"
-
-	"github.com/prysmaticlabs/prysm/encoding/bytesutil"
-	"github.com/prysmaticlabs/prysm/testing/assert"
-	"github.com/prysmaticlabs/prysm/testing/require"
-	"github.com/prysmaticlabs/prysm/validator/accounts/wallet"
-	"github.com/prysmaticlabs/prysm/validator/keymanager"
-	"github.com/prysmaticlabs/prysm/validator/keymanager/local"
-)
-
-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 := CreateWalletWithKeymanager(cliCtx.Context, &CreateWalletConfig{
-		WalletCfg: &wallet.Config{
-			WalletDir:      walletDir,
-			KeymanagerKind: keymanager.Local,
-			WalletPassword: password,
-		},
-	})
-	require.NoError(t, err)
-
-	// We attempt to import accounts.
-	require.NoError(t, ImportAccountsCli(cliCtx))
-
-	// We attempt to delete the accounts specified.
-	require.NoError(t, DeleteAccountCli(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))
-}

validator/accounts/accounts_exit.go

@@ -164,7 +164,7 @@ func interact(
 	if !cliCtx.IsSet(flags.ExitAllFlag.Name) {
 		// Allow the user to interactively select the accounts to exit or optionally
 		// provide them via cli flags as a string of comma-separated, hex strings.
-		filteredPubKeys, err := filterPublicKeysFromUserInput(
+		filteredPubKeys, err := FilterPublicKeysFromUserInput(
 			cliCtx,
 			flags.VoluntaryExitPublicKeysFlag,
 			validatingPublicKeys,

validator/accounts/accounts_helper.go

@@ -11,13 +11,14 @@ import (
 	"github.com/urfave/cli/v2"
 )
 
-func filterPublicKeysFromUserInput(
+// FilterPublicKeysFromUserInput collects the set of public keys from the
+// command line or an interactive session.
+func FilterPublicKeysFromUserInput(
 	cliCtx *cli.Context,
 	publicKeysFlag *cli.StringFlag,
 	validatingPublicKeys [][fieldparams.BLSPubkeyLength]byte,
 	selectionPrompt string,
 ) ([]bls.PublicKey, error) {
-	var filteredPubKeys []bls.PublicKey
 	if cliCtx.IsSet(publicKeysFlag.Name) {
 		pubKeyStrings := strings.Split(cliCtx.String(publicKeysFlag.Name), ",")
 		if len(pubKeyStrings) == 0 {
@@ -26,22 +27,27 @@ func filterPublicKeysFromUserInput(
 				publicKeysFlag.Name,
 			)
 		}
-		for _, str := range pubKeyStrings {
-			pkString := str
-			if strings.Contains(pkString, "0x") {
-				pkString = pkString[2:]
-			}
-			pubKeyBytes, err := hex.DecodeString(pkString)
-			if err != nil {
-				return nil, errors.Wrapf(err, "could not decode string %s as hex", pkString)
-			}
-			blsPublicKey, err := bls.PublicKeyFromBytes(pubKeyBytes)
-			if err != nil {
-				return nil, errors.Wrapf(err, "%#x is not a valid BLS public key", pubKeyBytes)
-			}
-			filteredPubKeys = append(filteredPubKeys, blsPublicKey)
-		}
-		return filteredPubKeys, nil
+		return filterPublicKeys(pubKeyStrings)
 	}
 	return selectAccounts(selectionPrompt, validatingPublicKeys)
 }
+
+func filterPublicKeys(pubKeyStrings []string) ([]bls.PublicKey, error) {
+	var filteredPubKeys []bls.PublicKey
+	for _, str := range pubKeyStrings {
+		pkString := str
+		if strings.Contains(pkString, "0x") {
+			pkString = pkString[2:]
+		}
+		pubKeyBytes, err := hex.DecodeString(pkString)
+		if err != nil {
+			return nil, errors.Wrapf(err, "could not decode string %s as hex", pkString)
+		}
+		blsPublicKey, err := bls.PublicKeyFromBytes(pubKeyBytes)
+		if err != nil {
+			return nil, errors.Wrapf(err, "%#x is not a valid BLS public key", pubKeyBytes)
+		}
+		filteredPubKeys = append(filteredPubKeys, blsPublicKey)
+	}
+	return filteredPubKeys, nil
+}

validator/accounts/cli_manager.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/pkg/errors"
 	grpcutil "github.com/prysmaticlabs/prysm/api/grpc"
+	"github.com/prysmaticlabs/prysm/crypto/bls"
 	ethpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1"
 	"github.com/prysmaticlabs/prysm/validator/accounts/wallet"
 	"github.com/prysmaticlabs/prysm/validator/keymanager"
@@ -30,9 +31,12 @@ type AccountsCLIManager struct {
 	showDepositData      bool
 	showPrivateKeys      bool
 	listValidatorIndices bool
+	deletePublicKeys     bool
 	dialOpts             []grpc.DialOption
 	grpcHeaders          []string
 	beaconRPCProvider    string
+	filteredPubKeys      []bls.PublicKey
+	walletKeyCount       int
 }
 
 func (acm *AccountsCLIManager) prepareBeaconClients(ctx context.Context) (*ethpb.BeaconNodeValidatorClient, *ethpb.NodeClient, error) {

validator/accounts/cli_options.go

@@ -1,6 +1,7 @@
 package accounts
 
 import (
+	"github.com/prysmaticlabs/prysm/crypto/bls"
 	"github.com/prysmaticlabs/prysm/validator/accounts/wallet"
 	"github.com/prysmaticlabs/prysm/validator/keymanager"
 	"google.golang.org/grpc"
@@ -72,3 +73,26 @@ func WithBeaconRPCProvider(provider string) Option {
 		return nil
 	}
 }
+
+// WithFilteredPubKeys adds public key strings parsed from CLI.
+func WithFilteredPubKeys(filteredPubKeys []bls.PublicKey) Option {
+	return func(acc *AccountsCLIManager) error {
+		acc.filteredPubKeys = filteredPubKeys
+		return nil
+	}
+}
+
+// WithWalletKeyCount tracks the number of keys in a wallet.
+func WithWalletKeyCount(walletKeyCount int) Option {
+	return func(acc *AccountsCLIManager) error {
+		acc.walletKeyCount = walletKeyCount
+		return nil
+	}
+}
+
+func WithDeletePublicKeys(deletePublicKeys bool) Option {
+	return func(acc *AccountsCLIManager) error {
+		acc.deletePublicKeys = deletePublicKeys
+		return nil
+	}
+}

validator/accounts/petnames/BUILD.bazel

@@ -4,7 +4,10 @@ go_library(
     name = "go_default_library",
     srcs = ["names.go"],
     importpath = "github.com/prysmaticlabs/prysm/validator/accounts/petnames",
-    visibility = ["//validator:__subpackages__"],
+    visibility = [
+        "//cmd/validator:__subpackages__",
+        "//validator:__subpackages__",
+    ],
     deps = [
         "//crypto/hash:go_default_library",
         "//crypto/rand:go_default_library",