Wallet edit CLI Manager migration

cmd/validator/wallet/BUILD.bazel

@@ -1,8 +1,11 @@
-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 = ["wallet.go"],
+    srcs = [
+        "edit.go",
+        "wallet.go",
+    ],
     importpath = "github.com/prysmaticlabs/prysm/cmd/validator/wallet",
     visibility = ["//visibility:public"],
     deps = [
@@ -11,7 +14,29 @@ go_library(
         "//config/features:go_default_library",
         "//runtime/tos:go_default_library",
         "//validator/accounts:go_default_library",
+        "//validator/accounts/userprompt:go_default_library",
+        "//validator/accounts/wallet:go_default_library",
+        "//validator/keymanager:go_default_library",
+        "//validator/keymanager/remote:go_default_library",
+        "@com_github_pkg_errors//:go_default_library",
         "@com_github_sirupsen_logrus//:go_default_library",
         "@com_github_urfave_cli_v2//:go_default_library",
     ],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["edit_test.go"],
+    embed = [":go_default_library"],
+    deps = [
+        "//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/wallet:go_default_library",
+        "//validator/keymanager:go_default_library",
+        "//validator/keymanager/remote:go_default_library",
+        "@com_github_urfave_cli_v2//:go_default_library",
+    ],
+)

cmd/validator/wallet/edit.go

@@ -0,0 +1,54 @@
+package wallet
+
+import (
+	"fmt"
+
+	"github.com/pkg/errors"
+	"github.com/prysmaticlabs/prysm/validator/accounts"
+	"github.com/prysmaticlabs/prysm/validator/accounts/userprompt"
+	"github.com/prysmaticlabs/prysm/validator/accounts/wallet"
+	"github.com/prysmaticlabs/prysm/validator/keymanager"
+	"github.com/prysmaticlabs/prysm/validator/keymanager/remote"
+	"github.com/urfave/cli/v2"
+)
+
+func remoteWalletEdit(c *cli.Context) error {
+	w, err := wallet.OpenWalletOrElseCli(c, func(cliCtx *cli.Context) (*wallet.Wallet, error) {
+		return nil, wallet.ErrNoWalletFound
+	})
+	if err != nil {
+		return errors.Wrap(err, "could not open wallet")
+	}
+	if w.KeymanagerKind() != keymanager.Remote {
+		return errors.New(
+			fmt.Sprintf("Keymanager type: %s doesn't support configuration editing",
+				w.KeymanagerKind().String()))
+	}
+
+	enc, err := w.ReadKeymanagerConfigFromDisk(c.Context)
+	if err != nil {
+		return errors.Wrap(err, "could not read config")
+	}
+	fileOpts, err := remote.UnmarshalOptionsFile(enc)
+	if err != nil {
+		return errors.Wrap(err, "could not unmarshal config")
+	}
+	log.Info("Current configuration")
+	// Prints the current configuration to stdout.
+	fmt.Println(fileOpts)
+	newCfg, err := userprompt.InputRemoteKeymanagerConfig(c)
+	if err != nil {
+		return errors.Wrap(err, "could not get keymanager config")
+	}
+
+	opts := []accounts.Option{
+		accounts.WithWallet(w),
+		accounts.WithKeymanagerOpts(newCfg),
+	}
+
+	acc, err := accounts.NewCLIManager(opts...)
+	if err != nil {
+		return err
+	}
+	return acc.WalletEdit(c.Context)
+}

cmd/validator/wallet/edit_test.go

@@ -0,0 +1,162 @@
+package wallet
+
+import (
+	"flag"
+	"os"
+	"path/filepath"
+	"strconv"
+	"testing"
+
+	"github.com/prysmaticlabs/prysm/cmd/validator/flags"
+	"github.com/prysmaticlabs/prysm/config/params"
+	"github.com/prysmaticlabs/prysm/testing/assert"
+	"github.com/prysmaticlabs/prysm/testing/require"
+	"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/remote"
+	"github.com/urfave/cli/v2"
+)
+
+const (
+	passwordFileName = "password.txt"
+	password         = "OhWOWthisisatest42!$"
+)
+
+// TODO(mikeneuder): Figure out how to shared these functions with
+// `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) {
+	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
+}
+
+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 TestEditWalletConfiguration(t *testing.T) {
+	walletDir, _, passwordFile := setupWalletAndPasswordsDir(t)
+	cliCtx := setupWalletCtx(t, &testWalletConfig{
+		walletDir:      walletDir,
+		keymanagerKind: keymanager.Remote,
+	})
+	w, err := accounts.CreateWalletWithKeymanager(cliCtx.Context, &accounts.CreateWalletConfig{
+		WalletCfg: &wallet.Config{
+			WalletDir:      walletDir,
+			KeymanagerKind: keymanager.Remote,
+			WalletPassword: "Passwordz0320$",
+		},
+	})
+	require.NoError(t, err)
+
+	originalCfg := &remote.KeymanagerOpts{
+		RemoteCertificate: &remote.CertificateConfig{
+			RequireTls:     true,
+			ClientCertPath: "/tmp/a.crt",
+			ClientKeyPath:  "/tmp/b.key",
+			CACertPath:     "/tmp/c.crt",
+		},
+		RemoteAddr: "my.server.com:4000",
+	}
+	encodedCfg, err := remote.MarshalOptionsFile(cliCtx.Context, originalCfg)
+	assert.NoError(t, err)
+	assert.NoError(t, w.WriteKeymanagerConfigToDisk(cliCtx.Context, encodedCfg))
+
+	wantCfg := &remote.KeymanagerOpts{
+		RemoteCertificate: &remote.CertificateConfig{
+			RequireTls:     true,
+			ClientCertPath: "/tmp/client.crt",
+			ClientKeyPath:  "/tmp/client.key",
+			CACertPath:     "/tmp/ca.crt",
+		},
+		RemoteAddr: "host.example.com:4000",
+	}
+	app := cli.App{}
+	set := flag.NewFlagSet("test", 0)
+	set.String(flags.WalletDirFlag.Name, walletDir, "")
+	set.String(flags.WalletPasswordFileFlag.Name, passwordFile, "")
+	set.String(flags.GrpcRemoteAddressFlag.Name, wantCfg.RemoteAddr, "")
+	set.String(flags.RemoteSignerCertPathFlag.Name, wantCfg.RemoteCertificate.ClientCertPath, "")
+	set.String(flags.RemoteSignerKeyPathFlag.Name, wantCfg.RemoteCertificate.ClientKeyPath, "")
+	set.String(flags.RemoteSignerCACertPathFlag.Name, wantCfg.RemoteCertificate.CACertPath, "")
+	assert.NoError(t, set.Set(flags.WalletDirFlag.Name, walletDir))
+	assert.NoError(t, set.Set(flags.WalletPasswordFileFlag.Name, passwordFile))
+	assert.NoError(t, set.Set(flags.GrpcRemoteAddressFlag.Name, wantCfg.RemoteAddr))
+	assert.NoError(t, set.Set(flags.RemoteSignerCertPathFlag.Name, wantCfg.RemoteCertificate.ClientCertPath))
+	assert.NoError(t, set.Set(flags.RemoteSignerKeyPathFlag.Name, wantCfg.RemoteCertificate.ClientKeyPath))
+	assert.NoError(t, set.Set(flags.RemoteSignerCACertPathFlag.Name, wantCfg.RemoteCertificate.CACertPath))
+	cliCtx = cli.NewContext(&app, set, nil)
+
+	err = remoteWalletEdit(cliCtx)
+	require.NoError(t, err)
+	encoded, err := w.ReadKeymanagerConfigFromDisk(cliCtx.Context)
+	require.NoError(t, err)
+
+	cfg, err := remote.UnmarshalOptionsFile(encoded)
+	assert.NoError(t, err)
+	assert.DeepEqual(t, wantCfg, cfg)
+}

cmd/validator/wallet/wallet.go

@@ -76,13 +76,13 @@ var Commands = &cli.Command{
 				if err := cmd.LoadFlagsFromConfig(cliCtx, cliCtx.Command.Flags); err != nil {
 					return err
 				}
-				return tos.VerifyTosAcceptedOrPrompt(cliCtx)
-			},
-			Action: func(cliCtx *cli.Context) error {
-				if err := features.ConfigureValidator(cliCtx); err != nil {
+				if err := tos.VerifyTosAcceptedOrPrompt(cliCtx); err != nil {
 					return err
 				}
-				if err := accounts.EditWalletConfigurationCli(cliCtx); err != nil {
+				return features.ConfigureValidator(cliCtx)
+			},
+			Action: func(cliCtx *cli.Context) error {
+				if err := remoteWalletEdit(cliCtx); err != nil {
 					log.Fatalf("Could not edit wallet configuration: %v", err)
 				}
 				return nil

validator/accounts/BUILD.bazel

@@ -68,7 +68,6 @@ go_test(
         "accounts_import_test.go",
         "accounts_list_test.go",
         "wallet_create_test.go",
-        "wallet_edit_test.go",
         "wallet_recover_fuzz_test.go",
         "wallet_recover_test.go",
     ],

validator/accounts/cli_manager.go

@@ -9,6 +9,7 @@ import (
 	ethpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1"
 	"github.com/prysmaticlabs/prysm/validator/accounts/wallet"
 	"github.com/prysmaticlabs/prysm/validator/keymanager"
+	"github.com/prysmaticlabs/prysm/validator/keymanager/remote"
 	"google.golang.org/grpc"
 )
 
@@ -28,6 +29,7 @@ func NewCLIManager(opts ...Option) (*AccountsCLIManager, error) {
 type AccountsCLIManager struct {
 	wallet               *wallet.Wallet
 	keymanager           keymanager.IKeymanager
+	keymanagerOpts       *remote.KeymanagerOpts
 	showDepositData      bool
 	showPrivateKeys      bool
 	listValidatorIndices bool

validator/accounts/cli_options.go

@@ -4,6 +4,7 @@ import (
 	"github.com/prysmaticlabs/prysm/crypto/bls"
 	"github.com/prysmaticlabs/prysm/validator/accounts/wallet"
 	"github.com/prysmaticlabs/prysm/validator/keymanager"
+	"github.com/prysmaticlabs/prysm/validator/keymanager/remote"
 	"google.golang.org/grpc"
 )
 
@@ -26,6 +27,14 @@ func WithKeymanager(km keymanager.IKeymanager) Option {
 	}
 }
 
+// WithKeymanagerOpts provides a keymanager configuration to the accounts cli manager.
+func WithKeymanagerOpts(kmo *remote.KeymanagerOpts) Option {
+	return func(acc *AccountsCLIManager) error {
+		acc.keymanagerOpts = kmo
+		return nil
+	}
+}
+
 // WithShowDepositData enables displaying deposit data in the accounts cli manager.
 func WithShowDepositData() Option {
 	return func(acc *AccountsCLIManager) error {

validator/accounts/wallet_edit.go

@@ -1,55 +1,20 @@
 package accounts
 
 import (
-	"fmt"
+	"context"
 
 	"github.com/pkg/errors"
-	"github.com/prysmaticlabs/prysm/validator/accounts/userprompt"
-	"github.com/prysmaticlabs/prysm/validator/accounts/wallet"
-	"github.com/prysmaticlabs/prysm/validator/keymanager"
 	"github.com/prysmaticlabs/prysm/validator/keymanager/remote"
-	"github.com/urfave/cli/v2"
 )
 
-// EditWalletConfigurationCli for a user's on-disk wallet, being able to change
-// things such as remote gRPC credentials for remote signing, derivation paths
-// for HD wallets, and more.
-func EditWalletConfigurationCli(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")
-	}
-	if w.KeymanagerKind() != keymanager.Remote {
-		return errors.New(
-			fmt.Sprintf("Keymanager type: %s doesn't support configuration editing",
-				w.KeymanagerKind().String()))
-	}
-	return editRemoteWallet(cliCtx, w)
-}
-
-func editRemoteWallet(cliCtx *cli.Context, w *wallet.Wallet) error {
-	enc, err := w.ReadKeymanagerConfigFromDisk(cliCtx.Context)
-	if err != nil {
-		return errors.Wrap(err, "could not read config")
-	}
-	opts, err := remote.UnmarshalOptionsFile(enc)
-	if err != nil {
-		return errors.Wrap(err, "could not unmarshal config")
-	}
-	log.Info("Current configuration")
-	// Prints the current configuration to stdout.
-	fmt.Println(opts)
-	newCfg, err := userprompt.InputRemoteKeymanagerConfig(cliCtx)
-	if err != nil {
-		return errors.Wrap(err, "could not get keymanager config")
-	}
-	encodedCfg, err := remote.MarshalOptionsFile(cliCtx.Context, newCfg)
+// WalletEdit changes a user's on-disk wallet configuration: remote gRPC
+// credentials for remote signing, derivation paths for HD wallets, etc.
+func (acm *AccountsCLIManager) WalletEdit(ctx context.Context) error {
+	encodedCfg, err := remote.MarshalOptionsFile(ctx, acm.keymanagerOpts)
 	if err != nil {
 		return errors.Wrap(err, "could not marshal config file")
 	}
-	if err := w.WriteKeymanagerConfigToDisk(cliCtx.Context, encodedCfg); err != nil {
+	if err := acm.wallet.WriteKeymanagerConfigToDisk(ctx, encodedCfg); err != nil {
 		return errors.Wrap(err, "could not write config to disk")
 	}
 	return nil