HTTP Validator API: /eth/v1/keystores (#13113)

* WIP

* fixing tests

* fixing bazel

* fixing api client

* fixing tests

* fixing more tests and bazel

* fixing trace and more bazel issues

* fixing router path function definitions

* fixing more tests and deep source issues

* adding delete test

* if a route is provided, reregister before the catch all on the middleware.

* fixing linting

* fixing deepsource complaint

* gaz

* more deepsource issues

* fixing missed err check

* changing how routes are registered

* radek reviews

* Update validator/rpc/handlers_keymanager.go

Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>

* Update validator/rpc/handlers_keymanager.go

Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>

* fixing unit test after sammy's review

* adding radek's comments

---------

Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>

validator/accounts/BUILD.bazel

@@ -33,7 +33,6 @@ go_library(
         "//encoding/bytesutil:go_default_library",
         "//io/file:go_default_library",
         "//io/prompt:go_default_library",
-        "//proto/eth/service:go_default_library",
         "//proto/prysm/v1alpha1:go_default_library",
         "//validator/accounts/iface:go_default_library",
         "//validator/accounts/petnames:go_default_library",
@@ -64,6 +63,7 @@ go_library(
 go_test(
     name = "go_default_test",
     srcs = [
+        "accounts_delete_test.go",
         "accounts_exit_test.go",
         "accounts_import_test.go",
         "accounts_list_test.go",
@@ -82,7 +82,6 @@ go_test(
         "//crypto/bls:go_default_library",
         "//encoding/bytesutil:go_default_library",
         "//io/file:go_default_library",
-        "//proto/eth/service:go_default_library",
         "//proto/prysm/v1alpha1:go_default_library",
         "//testing/assert:go_default_library",
         "//testing/require:go_default_library",
@@ -92,6 +91,7 @@ go_test(
         "//validator/keymanager/derived:go_default_library",
         "//validator/keymanager/local:go_default_library",
         "//validator/testing:go_default_library",
+        "@com_github_ethereum_go_ethereum//common/hexutil:go_default_library",
         "@com_github_golang_mock//gomock:go_default_library",
         "@com_github_google_uuid//:go_default_library",
         "@com_github_pkg_errors//:go_default_library",

validator/accounts/accounts_delete.go

@@ -3,13 +3,12 @@ package accounts
 import (
 	"context"
 	"fmt"
-	"os"
 	"strings"
 
 	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/prysm/v4/encoding/bytesutil"
 	"github.com/prysmaticlabs/prysm/v4/io/prompt"
-	ethpbservice "github.com/prysmaticlabs/prysm/v4/proto/eth/service"
+	"github.com/prysmaticlabs/prysm/v4/validator/keymanager"
 )
 
 // Delete the accounts that the user requests to be deleted from the wallet.
@@ -26,7 +25,7 @@ func (acm *CLIManager) Delete(ctx context.Context) error {
 		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,
+				acm.inputReader, fmt.Sprintf(promptText, au.BrightGreen(formattedPubKeys[0])), prompt.ValidateYesOrNo,
 			)
 			if err != nil {
 				return err
@@ -41,7 +40,7 @@ func (acm *CLIManager) Delete(ctx context.Context) error {
 			} else {
 				promptText = fmt.Sprintf(promptText, len(acm.filteredPubKeys), au.BrightGreen(allAccountStr))
 			}
-			resp, err := prompt.ValidatePrompt(os.Stdin, promptText, prompt.ValidateYesOrNo)
+			resp, err := prompt.ValidatePrompt(acm.inputReader, promptText, prompt.ValidateYesOrNo)
 			if err != nil {
 				return err
 			}
@@ -76,11 +75,11 @@ func DeleteAccount(ctx context.Context, cfg *DeleteConfig) error {
 	}
 	for i, status := range statuses {
 		switch status.Status {
-		case ethpbservice.DeletedKeystoreStatus_ERROR:
+		case keymanager.StatusError:
 			log.Errorf("Error deleting key %#x: %s", bytesutil.Trunc(cfg.DeletePublicKeys[i]), status.Message)
-		case ethpbservice.DeletedKeystoreStatus_NOT_ACTIVE:
+		case keymanager.StatusNotActive:
 			log.Warnf("Duplicate key %#x found in delete request", bytesutil.Trunc(cfg.DeletePublicKeys[i]))
-		case ethpbservice.DeletedKeystoreStatus_NOT_FOUND:
+		case keymanager.StatusNotFound:
 			log.Warnf("Could not find keystore for %#x", bytesutil.Trunc(cfg.DeletePublicKeys[i]))
 		}
 	}

validator/accounts/accounts_delete_test.go

@@ -0,0 +1,78 @@
+package accounts
+
+import (
+	"bytes"
+	"context"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common/hexutil"
+	"github.com/prysmaticlabs/prysm/v4/crypto/bls"
+	"github.com/prysmaticlabs/prysm/v4/testing/assert"
+	"github.com/prysmaticlabs/prysm/v4/testing/require"
+	"github.com/prysmaticlabs/prysm/v4/validator/keymanager"
+	"github.com/prysmaticlabs/prysm/v4/validator/keymanager/local"
+	logTest "github.com/sirupsen/logrus/hooks/test"
+)
+
+func TestDelete(t *testing.T) {
+	hook := logTest.NewGlobal()
+	ctx := context.Background()
+	// import keys
+	numAccounts := 5
+	keystores := make([]*keymanager.Keystore, numAccounts)
+	passwords := make([]string, numAccounts)
+	for i := 0; i < numAccounts; i++ {
+		keystores[i] = createRandomKeystore(t, password)
+		passwords[i] = password
+	}
+	pubkey1, err := hexutil.Decode("0x" + keystores[0].Pubkey)
+	require.NoError(t, err)
+	p1, err := bls.PublicKeyFromBytes(pubkey1)
+	require.NoError(t, err)
+	pubkey2, err := hexutil.Decode("0x" + keystores[1].Pubkey)
+	require.NoError(t, err)
+	p2, err := bls.PublicKeyFromBytes(pubkey2)
+	require.NoError(t, err)
+
+	walletDir, passwordsDir, walletPasswordFile := setupWalletAndPasswordsDir(t)
+	cliCtx := setupWalletCtx(t, &testWalletConfig{
+		walletDir:          walletDir,
+		passwordsDir:       passwordsDir,
+		keymanagerKind:     keymanager.Local,
+		walletPasswordFile: walletPasswordFile,
+	})
+
+	var stdin bytes.Buffer
+	stdin.Write([]byte("Y"))
+	opts := []Option{
+		WithWalletDir(walletDir),
+		WithKeymanagerType(keymanager.Local),
+		WithWalletPassword("Passwordz0320$"),
+		WithCustomReader(&stdin),
+		WithFilteredPubKeys([]bls.PublicKey{p1, p2}),
+	}
+	acc, err := NewCLIManager(opts...)
+	require.NoError(t, err)
+	w, err := acc.WalletCreate(cliCtx.Context)
+	require.NoError(t, err)
+	km, err := local.NewKeymanager(
+		cliCtx.Context,
+		&local.SetupConfig{
+			Wallet:           w,
+			ListenForChanges: false,
+		},
+	)
+	require.NoError(t, err)
+	acc.keymanager = km
+
+	_, err = km.ImportKeystores(cliCtx.Context, keystores, passwords)
+	require.NoError(t, err)
+
+	// test delete
+	err = acc.Delete(ctx)
+	require.NoError(t, err)
+	keys, err := km.FetchValidatingPublicKeys(cliCtx.Context)
+	require.NoError(t, err)
+	require.Equal(t, len(keys), 3)
+	assert.LogsContain(t, hook, "Attempted to delete accounts.")
+}

validator/accounts/accounts_import.go

@@ -18,7 +18,6 @@ import (
 	"github.com/prysmaticlabs/prysm/v4/encoding/bytesutil"
 	"github.com/prysmaticlabs/prysm/v4/io/file"
 	"github.com/prysmaticlabs/prysm/v4/io/prompt"
-	ethpbservice "github.com/prysmaticlabs/prysm/v4/proto/eth/service"
 	"github.com/prysmaticlabs/prysm/v4/validator/accounts/wallet"
 	"github.com/prysmaticlabs/prysm/v4/validator/keymanager"
 	keystorev4 "github.com/wealdtech/go-eth2-wallet-encryptor-keystorev4"
@@ -157,11 +156,11 @@ func (acm *CLIManager) Import(ctx context.Context) error {
 	var successfullyImportedAccounts []string
 	for i, status := range statuses {
 		switch status.Status {
-		case ethpbservice.ImportedKeystoreStatus_IMPORTED:
+		case keymanager.StatusImported:
 			successfullyImportedAccounts = append(successfullyImportedAccounts, keystoresImported[i].Pubkey)
-		case ethpbservice.ImportedKeystoreStatus_DUPLICATE:
+		case keymanager.StatusDuplicate:
 			log.Warnf("Duplicate key %s found in import request, skipped", keystoresImported[i].Pubkey)
-		case ethpbservice.ImportedKeystoreStatus_ERROR:
+		case keymanager.StatusError:
 			log.Warnf("Could not import keystore for %s: %s", keystoresImported[i].Pubkey, status.Message)
 		}
 	}
@@ -179,12 +178,12 @@ func (acm *CLIManager) Import(ctx context.Context) error {
 
 // 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) ([]*ethpbservice.ImportedKeystoreStatus, error) {
+func ImportAccounts(ctx context.Context, cfg *ImportAccountsConfig) ([]*keymanager.KeyStatus, error) {
 	if cfg.AccountPassword == "" {
-		statuses := make([]*ethpbservice.ImportedKeystoreStatus, len(cfg.Keystores))
+		statuses := make([]*keymanager.KeyStatus, len(cfg.Keystores))
 		for i, keystore := range cfg.Keystores {
-			statuses[i] = &ethpbservice.ImportedKeystoreStatus{
-				Status: ethpbservice.ImportedKeystoreStatus_ERROR,
+			statuses[i] = &keymanager.KeyStatus{
+				Status: keymanager.StatusError,
 				Message: fmt.Sprintf(
 					"account password is required to import keystore %s",
 					keystore.Pubkey,
@@ -249,15 +248,15 @@ func importPrivateKeyAsAccount(ctx context.Context, wallet *wallet.Wallet, impor
 	}
 	for _, status := range statuses {
 		switch status.Status {
-		case ethpbservice.ImportedKeystoreStatus_IMPORTED:
+		case keymanager.StatusImported:
 			fmt.Printf(
 				"Imported account with public key %#x, view all accounts by running `accounts list`\n",
 				au.BrightMagenta(bytesutil.Trunc(privKey.PublicKey().Marshal())),
 			)
 			return nil
-		case ethpbservice.ImportedKeystoreStatus_ERROR:
+		case keymanager.StatusError:
 			return fmt.Errorf("could not import keystore for %s: %s", keystore.Pubkey, status.Message)
-		case ethpbservice.ImportedKeystoreStatus_DUPLICATE:
+		case keymanager.StatusDuplicate:
 			return fmt.Errorf("duplicate key %s skipped", keystore.Pubkey)
 		}
 	}

validator/accounts/accounts_import_test.go

@@ -12,7 +12,6 @@ import (
 	"github.com/prysmaticlabs/prysm/v4/config/params"
 	"github.com/prysmaticlabs/prysm/v4/crypto/bls"
 	"github.com/prysmaticlabs/prysm/v4/encoding/bytesutil"
-	ethpbservice "github.com/prysmaticlabs/prysm/v4/proto/eth/service"
 	"github.com/prysmaticlabs/prysm/v4/testing/assert"
 	"github.com/prysmaticlabs/prysm/v4/testing/require"
 	"github.com/prysmaticlabs/prysm/v4/validator/accounts/iface"
@@ -54,7 +53,7 @@ func TestImportAccounts_NoPassword(t *testing.T) {
 	})
 	require.NoError(t, err)
 	require.Equal(t, 1, len(resp))
-	require.Equal(t, resp[0].Status, ethpbservice.ImportedKeystoreStatus_ERROR)
+	require.Equal(t, resp[0].Status, keymanager.StatusError)
 }
 
 func TestImport_SortByDerivationPath(t *testing.T) {

validator/accounts/cli_manager.go

@@ -2,6 +2,8 @@ package accounts
 
 import (
 	"context"
+	"io"
+	"os"
 	"time"
 
 	"github.com/pkg/errors"
@@ -21,6 +23,7 @@ import (
 func NewCLIManager(opts ...Option) (*CLIManager, error) {
 	acc := &CLIManager{
 		mnemonicLanguage: derived.DefaultMnemonicLanguage,
+		inputReader:      os.Stdin,
 	}
 	for _, opt := range opts {
 		if err := opt(acc); err != nil {
@@ -64,6 +67,7 @@ type CLIManager struct {
 	mnemonic25thWord     string
 	beaconApiEndpoint    string
 	beaconApiTimeout     time.Duration
+	inputReader          io.Reader
 }
 
 func (acm *CLIManager) prepareBeaconClients(ctx context.Context) (*iface.ValidatorClient, *iface.NodeClient, error) {

validator/accounts/cli_options.go

@@ -1,6 +1,7 @@
 package accounts
 
 import (
+	"io"
 	"time"
 
 	"github.com/prysmaticlabs/prysm/v4/crypto/bls"
@@ -251,3 +252,11 @@ func WithNumAccounts(numAccounts int) Option {
 		return nil
 	}
 }
+
+// WithCustomReader changes the default reader
+func WithCustomReader(reader io.Reader) Option {
+	return func(acc *CLIManager) error {
+		acc.inputReader = reader
+		return nil
+	}
+}