more progress on refactor

cmd/validator/wallet/keymanager.go

@@ -0,0 +1 @@
+package wallet

validator/keymanager/imported/delete_test.go

@@ -0,0 +1,66 @@
+package imported
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/prysmaticlabs/prysm/encoding/bytesutil"
+	"github.com/prysmaticlabs/prysm/testing/require"
+	mock "github.com/prysmaticlabs/prysm/validator/accounts/testing"
+	"github.com/prysmaticlabs/prysm/validator/keymanager"
+	logTest "github.com/sirupsen/logrus/hooks/test"
+	keystorev4 "github.com/wealdtech/go-eth2-wallet-encryptor-keystorev4"
+)
+
+func TestImportedKeymanager_DeleteKeystores(t *testing.T) {
+	hook := logTest.NewGlobal()
+	wallet := &mock.Wallet{
+		Files:          make(map[string]map[string][]byte),
+		WalletPassword: password,
+	}
+	dr := &Keymanager{
+		wallet:        wallet,
+		accountsStore: &accountStore{},
+	}
+	numAccounts := 5
+	ctx := context.Background()
+	keystores := make([]*keymanager.Keystore, numAccounts)
+	for i := 0; i < numAccounts; i++ {
+		keystores[i] = createRandomKeystore(t, password)
+	}
+	require.NoError(t, dr.ImportKeystores(ctx, keystores, password))
+	accounts, err := dr.FetchValidatingPublicKeys(ctx)
+	require.NoError(t, err)
+	require.Equal(t, numAccounts, len(accounts))
+
+	accountToRemove := uint64(2)
+	accountPubKey := accounts[accountToRemove]
+	// Remove an account from the keystore.
+	require.NoError(t, dr.DeleteAccounts(ctx, [][]byte{accountPubKey[:]}))
+	// Ensure the keystore file was written to the wallet
+	// and ensure we can decrypt it using the EIP-2335 standard.
+	var encodedKeystore []byte
+	for k, v := range wallet.Files[AccountsPath] {
+		if strings.Contains(k, "keystore") {
+			encodedKeystore = v
+		}
+	}
+	require.NotNil(t, encodedKeystore, "could not find keystore file")
+	keystoreFile := &keymanager.Keystore{}
+	require.NoError(t, json.Unmarshal(encodedKeystore, keystoreFile))
+
+	// We extract the accounts from the keystore.
+	decryptor := keystorev4.New()
+	encodedAccounts, err := decryptor.Decrypt(keystoreFile.Crypto, password)
+	require.NoError(t, err, "Could not decrypt validator accounts")
+	store := &accountStore{}
+	require.NoError(t, json.Unmarshal(encodedAccounts, store))
+
+	require.Equal(t, numAccounts-1, len(store.PublicKeys))
+	require.Equal(t, numAccounts-1, len(store.PrivateKeys))
+	require.LogsContain(t, hook, fmt.Sprintf("%#x", bytesutil.Trunc(accountPubKey[:])))
+	require.LogsContain(t, hook, "Successfully deleted validator account")
+}

validator/keymanager/imported/fetch_keys_test.go

@@ -1,7 +1,13 @@
 package imported
 
 import (
+	"context"
+	"testing"
+
 	"github.com/prysmaticlabs/prysm/crypto/bls"
+	"github.com/prysmaticlabs/prysm/encoding/bytesutil"
+	"github.com/prysmaticlabs/prysm/testing/assert"
+	"github.com/prysmaticlabs/prysm/testing/require"
 	mock "github.com/prysmaticlabs/prysm/validator/accounts/testing"
 )
 

validator/keymanager/imported/keymanager.go

@@ -88,6 +88,12 @@ func NewKeymanager(ctx context.Context, cfg *SetupConfig) (*Keymanager, error) {
 		accountsStore:       &accountStore{},
 		accountsChangedFeed: new(event.Feed),
 	}
+	if strings.Contains(cfg.Wallet.AccountsDir(), "imported") ||
+		strings.Contains(cfg.Wallet.AccountsDir(), "derived") {
+		return nil, errors.New(
+			"keymanager kind found in wallet is not compatible as a local keymanager",
+		)
+	}
 
 	if err := k.initializeAccountKeystore(ctx); err != nil {
 		return nil, errors.Wrap(err, "failed to initialize account store")
@@ -101,8 +107,8 @@ func NewKeymanager(ctx context.Context, cfg *SetupConfig) (*Keymanager, error) {
 	return k, nil
 }
 
-// NewInteropKeymanager instantiates a new imported keymanager with the deterministically generated interop keys.
-func NewInteropKeymanager(_ context.Context, offset, numValidatorKeys uint64) (*Keymanager, error) {
+// NewDeterministicKeymanager instantiates a new imported keymanager with the deterministically generated interop keys.
+func NewDeterministicKeymanager(_ context.Context, offset, numValidatorKeys uint64) (*Keymanager, error) {
 	k := &Keymanager{
 		accountsChangedFeed: new(event.Feed),
 	}
@@ -136,6 +142,67 @@ func (km *Keymanager) ValidatingAccountNames() ([]string, error) {
 	return names, nil
 }
 
+// CreateAccountsKeystore creates a new keystore holding the provided keys.
+func (km *Keymanager) CreateAccountsKeystore(
+	_ context.Context,
+	privateKeys, publicKeys [][]byte,
+) (*AccountsKeystoreRepresentation, error) {
+	encryptor := keystorev4.New()
+	id, err := uuid.NewRandom()
+	if err != nil {
+		return nil, err
+	}
+	if len(privateKeys) != len(publicKeys) {
+		return nil, fmt.Errorf(
+			"number of private keys and public keys is not equal: %d != %d", len(privateKeys), len(publicKeys),
+		)
+	}
+	if km.accountsStore == nil {
+		km.accountsStore = &accountStore{
+			PrivateKeys: privateKeys,
+			PublicKeys:  publicKeys,
+		}
+	} else {
+		existingPubKeys := make(map[string]bool)
+		existingPrivKeys := make(map[string]bool)
+		for i := 0; i < len(km.accountsStore.PrivateKeys); i++ {
+			existingPrivKeys[string(km.accountsStore.PrivateKeys[i])] = true
+			existingPubKeys[string(km.accountsStore.PublicKeys[i])] = true
+		}
+		// We append to the accounts store keys only
+		// if the private/secret key do not already exist, to prevent duplicates.
+		for i := 0; i < len(privateKeys); i++ {
+			sk := privateKeys[i]
+			pk := publicKeys[i]
+			_, privKeyExists := existingPrivKeys[string(sk)]
+			_, pubKeyExists := existingPubKeys[string(pk)]
+			if privKeyExists || pubKeyExists {
+				continue
+			}
+			km.accountsStore.PublicKeys = append(km.accountsStore.PublicKeys, pk)
+			km.accountsStore.PrivateKeys = append(km.accountsStore.PrivateKeys, sk)
+		}
+	}
+	err = km.initializeKeysCachesFromKeystore()
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to initialize keys caches")
+	}
+	encodedStore, err := json.MarshalIndent(km.accountsStore, "", "\t")
+	if err != nil {
+		return nil, err
+	}
+	cryptoFields, err := encryptor.Encrypt(encodedStore, km.wallet.Password())
+	if err != nil {
+		return nil, errors.Wrap(err, "could not encrypt accounts")
+	}
+	return &AccountsKeystoreRepresentation{
+		Crypto:  cryptoFields,
+		ID:      id.String(),
+		Version: encryptor.Version(),
+		Name:    encryptor.Name(),
+	}, nil
+}
+
 // Initialize public and secret key caches that are used to speed up the functions
 // FetchValidatingPublicKeys and Sign
 func (km *Keymanager) initializeKeysCachesFromKeystore() error {
@@ -197,64 +264,3 @@ func (km *Keymanager) initializeAccountKeystore(ctx context.Context) error {
 	}
 	return err
 }
-
-// CreateAccountsKeystore creates a new keystore holding the provided keys.
-func (km *Keymanager) CreateAccountsKeystore(
-	_ context.Context,
-	privateKeys, publicKeys [][]byte,
-) (*AccountsKeystoreRepresentation, error) {
-	encryptor := keystorev4.New()
-	id, err := uuid.NewRandom()
-	if err != nil {
-		return nil, err
-	}
-	if len(privateKeys) != len(publicKeys) {
-		return nil, fmt.Errorf(
-			"number of private keys and public keys is not equal: %d != %d", len(privateKeys), len(publicKeys),
-		)
-	}
-	if km.accountsStore == nil {
-		km.accountsStore = &accountStore{
-			PrivateKeys: privateKeys,
-			PublicKeys:  publicKeys,
-		}
-	} else {
-		existingPubKeys := make(map[string]bool)
-		existingPrivKeys := make(map[string]bool)
-		for i := 0; i < len(km.accountsStore.PrivateKeys); i++ {
-			existingPrivKeys[string(km.accountsStore.PrivateKeys[i])] = true
-			existingPubKeys[string(km.accountsStore.PublicKeys[i])] = true
-		}
-		// We append to the accounts store keys only
-		// if the private/secret key do not already exist, to prevent duplicates.
-		for i := 0; i < len(privateKeys); i++ {
-			sk := privateKeys[i]
-			pk := publicKeys[i]
-			_, privKeyExists := existingPrivKeys[string(sk)]
-			_, pubKeyExists := existingPubKeys[string(pk)]
-			if privKeyExists || pubKeyExists {
-				continue
-			}
-			km.accountsStore.PublicKeys = append(km.accountsStore.PublicKeys, pk)
-			km.accountsStore.PrivateKeys = append(km.accountsStore.PrivateKeys, sk)
-		}
-	}
-	err = km.initializeKeysCachesFromKeystore()
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to initialize keys caches")
-	}
-	encodedStore, err := json.MarshalIndent(km.accountsStore, "", "\t")
-	if err != nil {
-		return nil, err
-	}
-	cryptoFields, err := encryptor.Encrypt(encodedStore, km.wallet.Password())
-	if err != nil {
-		return nil, errors.Wrap(err, "could not encrypt accounts")
-	}
-	return &AccountsKeystoreRepresentation{
-		Crypto:  cryptoFields,
-		ID:      id.String(),
-		Version: encryptor.Version(),
-		Name:    encryptor.Name(),
-	}, nil
-}

validator/keymanager/imported/keymanager_test.go

@@ -1,66 +1 @@
 package imported
-
-import (
-	"context"
-	"encoding/json"
-	"fmt"
-	"strings"
-	"testing"
-
-	"github.com/prysmaticlabs/prysm/encoding/bytesutil"
-	"github.com/prysmaticlabs/prysm/testing/require"
-	mock "github.com/prysmaticlabs/prysm/validator/accounts/testing"
-	"github.com/prysmaticlabs/prysm/validator/keymanager"
-	logTest "github.com/sirupsen/logrus/hooks/test"
-	keystorev4 "github.com/wealdtech/go-eth2-wallet-encryptor-keystorev4"
-)
-
-func TestImportedKeymanager_RemoveAccounts(t *testing.T) {
-	hook := logTest.NewGlobal()
-	wallet := &mock.Wallet{
-		Files:          make(map[string]map[string][]byte),
-		WalletPassword: password,
-	}
-	dr := &Keymanager{
-		wallet:        wallet,
-		accountsStore: &accountStore{},
-	}
-	numAccounts := 5
-	ctx := context.Background()
-	keystores := make([]*keymanager.Keystore, numAccounts)
-	for i := 0; i < numAccounts; i++ {
-		keystores[i] = createRandomKeystore(t, password)
-	}
-	require.NoError(t, dr.ImportKeystores(ctx, keystores, password))
-	accounts, err := dr.FetchValidatingPublicKeys(ctx)
-	require.NoError(t, err)
-	require.Equal(t, numAccounts, len(accounts))
-
-	accountToRemove := uint64(2)
-	accountPubKey := accounts[accountToRemove]
-	// Remove an account from the keystore.
-	require.NoError(t, dr.DeleteAccounts(ctx, [][]byte{accountPubKey[:]}))
-	// Ensure the keystore file was written to the wallet
-	// and ensure we can decrypt it using the EIP-2335 standard.
-	var encodedKeystore []byte
-	for k, v := range wallet.Files[AccountsPath] {
-		if strings.Contains(k, "keystore") {
-			encodedKeystore = v
-		}
-	}
-	require.NotNil(t, encodedKeystore, "could not find keystore file")
-	keystoreFile := &keymanager.Keystore{}
-	require.NoError(t, json.Unmarshal(encodedKeystore, keystoreFile))
-
-	// We extract the accounts from the keystore.
-	decryptor := keystorev4.New()
-	encodedAccounts, err := decryptor.Decrypt(keystoreFile.Crypto, password)
-	require.NoError(t, err, "Could not decrypt validator accounts")
-	store := &accountStore{}
-	require.NoError(t, json.Unmarshal(encodedAccounts, store))
-
-	require.Equal(t, numAccounts-1, len(store.PublicKeys))
-	require.Equal(t, numAccounts-1, len(store.PrivateKeys))
-	require.LogsContain(t, hook, fmt.Sprintf("%#x", bytesutil.Trunc(accountPubKey[:])))
-	require.LogsContain(t, hook, "Successfully deleted validator account")
-}

validator/keymanager/imported/recover.go

@@ -8,10 +8,10 @@ import (
 	util "github.com/wealdtech/go-eth2-util"
 )
 
-// RecoverAccountsFromMnemonic given a mnemonic phrase, is able to regenerate N accounts
+// RecoverKeystoresFromMnemonic given a mnemonic phrase, is able to regenerate N accounts
 // from a derived seed, encrypt them according to the EIP-2334 JSON standard, and write them
 // to disk. Then, the mnemonic is never stored nor used by the validator.
-func (km *Keymanager) RecoverAccountsFromMnemonic(
+func (km *Keymanager) RecoverKeystoresFromMnemonic(
 	ctx context.Context, mnemonic, mnemonicPassphrase string, numAccounts int,
 ) error {
 	seed, err := seedFromMnemonic(mnemonic, mnemonicPassphrase)

validator/keymanager/imported/recover_test.go

@@ -1,8 +1,12 @@
 package imported
 
 import (
+	"context"
+	"testing"
+
 	"github.com/prysmaticlabs/prysm/crypto/rand"
 	"github.com/prysmaticlabs/prysm/testing/assert"
+	"github.com/prysmaticlabs/prysm/testing/require"
 	mock "github.com/prysmaticlabs/prysm/validator/accounts/testing"
 	constant "github.com/prysmaticlabs/prysm/validator/testing"
 	"github.com/tyler-smith/go-bip39"
@@ -10,7 +14,7 @@ import (
 
 // We test that using a '25th word' mnemonic passphrase leads to different
 // public keys derived than not specifying the passphrase.
-func TestDerivedKeymanager_MnemnonicPassphrase_DifferentResults(t *testing.T) {
+func TestImportedKeymanager_Recover_25Words(t *testing.T) {
 	ctx := context.Background()
 	wallet := &mock.Wallet{
 		Files:            make(map[string]map[string][]byte),
@@ -23,7 +27,7 @@ func TestDerivedKeymanager_MnemnonicPassphrase_DifferentResults(t *testing.T) {
 	})
 	require.NoError(t, err)
 	numAccounts := 5
-	err = km.RecoverAccountsFromMnemonic(ctx, constant.TestMnemonic, "mnemonicpass", numAccounts)
+	err = km.RecoverKeystoresFromMnemonic(ctx, constant.TestMnemonic, "mnemonicpass", numAccounts)
 	require.NoError(t, err)
 	without25thWord, err := km.FetchValidatingPublicKeys(ctx)
 	require.NoError(t, err)
@@ -38,7 +42,7 @@ func TestDerivedKeymanager_MnemnonicPassphrase_DifferentResults(t *testing.T) {
 	})
 	require.NoError(t, err)
 	// No mnemonic passphrase this time.
-	err = km.RecoverAccountsFromMnemonic(ctx, constant.TestMnemonic, "", numAccounts)
+	err = km.RecoverKeystoresFromMnemonic(ctx, constant.TestMnemonic, "", numAccounts)
 	require.NoError(t, err)
 	with25thWord, err := km.FetchValidatingPublicKeys(ctx)
 	require.NoError(t, err)
@@ -48,7 +52,7 @@ func TestDerivedKeymanager_MnemnonicPassphrase_DifferentResults(t *testing.T) {
 	}
 }
 
-func TestDerivedKeymanager_RecoverSeedRoundTrip(t *testing.T) {
+func TestImportedKeymanager_Recover_RoundTrip(t *testing.T) {
 	mnemonicEntropy := make([]byte, 32)
 	n, err := rand.NewGenerator().Read(mnemonicEntropy)
 	require.NoError(t, err)

validator/keymanager/types.go

@@ -2,19 +2,26 @@ package keymanager
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/prysmaticlabs/prysm/async/event"
 	"github.com/prysmaticlabs/prysm/crypto/bls"
 	validatorpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1/validator-client"
 )
 
+const (
+	Local   = "imported"
+	Derived = "derived"
+	Remote  = "remote"
+)
+
 // IKeymanager defines a struct which can be used to manage keys with important
 // actions such as listing public keys, signing data, and subscribing to key changes.
+// It is defined by the simple composition of a few base features. We can assert whether
+// or not a keymanager has "extra" functionality by casting it to other, useful key
+// management interfaces defined in this package.
 type IKeymanager interface {
-	KeysFetcher
+	PublicKeysFetcher
 	Signer
-	Importer
 	KeyChangeSubscriber
 }
 
@@ -44,6 +51,13 @@ type KeyChangeSubscriber interface {
 	SubscribeAccountChanges(pubKeysChan chan [][48]byte) event.Subscription
 }
 
+// KeyRecoverer allows recovering a keystore from a recovery phrase.
+type KeyRecoverer interface {
+	RecoverKeystoresFromMnemonic(
+		ctx context.Context, mnemonic, mnemonicPassphrase string, numKeys int,
+	) error
+}
+
 // Keystore json file representation as a Go struct.
 type Keystore struct {
 	Crypto  map[string]interface{} `json:"crypto"`
@@ -53,47 +67,6 @@ type Keystore struct {
 	Name    string                 `json:"name"`
 }
 
-// Kind defines an enum for either imported, derived, or remote-signing
-// keystores for Prysm wallets.
-type Kind int
-
-const (
-	// Imported keymanager defines an on-disk, encrypted keystore-capable store.
-	Imported Kind = iota
-	// Derived keymanager using a hierarchical-deterministic algorithm.
-	Derived
-	// Remote keymanager capable of remote-signing data.
-	Remote
-)
-
 // IncorrectPasswordErrMsg defines a common error string representing an EIP-2335
 // keystore password was incorrect.
 const IncorrectPasswordErrMsg = "invalid checksum"
-
-// String marshals a keymanager kind to a string value.
-func (k Kind) String() string {
-	switch k {
-	case Derived:
-		return "derived"
-	case Imported:
-		return "direct"
-	case Remote:
-		return "remote"
-	default:
-		return fmt.Sprintf("%d", int(k))
-	}
-}
-
-// ParseKind from a raw string, returning a keymanager kind.
-func ParseKind(k string) (Kind, error) {
-	switch k {
-	case "derived":
-		return Derived, nil
-	case "direct":
-		return Imported, nil
-	case "remote":
-		return Remote, nil
-	default:
-		return 0, fmt.Errorf("%s is not an allowed keymanager", k)
-	}
-}

validator/keymanager/types_test.go

@@ -2,13 +2,10 @@ package keymanager_test
 
 import (
 	"github.com/prysmaticlabs/prysm/validator/keymanager"
-	"github.com/prysmaticlabs/prysm/validator/keymanager/derived"
 	"github.com/prysmaticlabs/prysm/validator/keymanager/imported"
-	"github.com/prysmaticlabs/prysm/validator/keymanager/remote"
 )
 
 var (
 	_ = keymanager.IKeymanager(&imported.Keymanager{})
-	_ = keymanager.IKeymanager(&derived.Keymanager{})
-	_ = keymanager.IKeymanager(&remote.Keymanager{})
+	//_ = keymanager.IKeymanager(&remote.Keymanager{})
 )

validator/node/node.go

@@ -177,7 +177,7 @@ func (c *ValidatorClient) initializeFromCLI(cliCtx *cli.Context) error {
 	if cliCtx.IsSet(flags.InteropNumValidators.Name) {
 		numValidatorKeys := cliCtx.Uint64(flags.InteropNumValidators.Name)
 		offset := cliCtx.Uint64(flags.InteropStartIndex.Name)
-		keyManager, err = imported.NewInteropKeymanager(cliCtx.Context, offset, numValidatorKeys)
+		keyManager, err = imported.NewDeterministicKeymanager(cliCtx.Context, offset, numValidatorKeys)
 		if err != nil {
 			return errors.Wrap(err, "could not generate interop keys")
 		}