Cleanup of Keymanager Deleter interface (#10415)

* Migrating Keymanager account list functionality into each keymanager type

* Addressing review comments

* Adding newline at end of BUILD.bazel

* bazel run //:gazelle -- fix

* account deleter cleanup

* bazel run //:gazelle -- fix

* remove stale logging statement

* adding deleter interface to mock functions

* fixing deepsource findings

* go.sum

* bazel run //:gazelle -- fix

* go mod t-dy -compat=1.17

Co-authored-by: james-prysm <90280386+james-prysm@users.noreply.github.com>
Co-authored-by: prestonvanloon <preston@prysmaticlabs.com>
This commit is contained in:
Mike Neuder 2022-03-30 17:52:54 -04:00 committed by GitHub
parent c68894b77d
commit ade7d705ec
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 45 additions and 29 deletions

View File

@ -1,7 +1,6 @@
package accounts
import (
"github.com/prysmaticlabs/prysm/validator/accounts/wallet"
"github.com/prysmaticlabs/prysm/validator/keymanager"
)
@ -11,9 +10,8 @@ var (
ErrCouldNotInitializeKeymanager = "could not initialize keymanager"
)
// Config specifies parameters for accounts commands.
type Config struct {
Wallet *wallet.Wallet
// DeleteConfig specifies parameters for the accounts delete command.
type DeleteConfig struct {
Keymanager keymanager.IKeymanager
DeletePublicKeys [][]byte
}

View File

@ -91,8 +91,7 @@ func DeleteAccountCli(cliCtx *cli.Context) error {
}
}
}
if err := DeleteAccount(cliCtx.Context, &Config{
Wallet: w,
if err := DeleteAccount(cliCtx.Context, &DeleteConfig{
Keymanager: kManager,
DeletePublicKeys: rawPublicKeys,
}); err != nil {
@ -106,17 +105,13 @@ func DeleteAccountCli(cliCtx *cli.Context) error {
}
// DeleteAccount deletes the accounts that the user requests to be deleted from the wallet.
func DeleteAccount(ctx context.Context, cfg *Config) error {
deleter, ok := cfg.Keymanager.(keymanager.Deleter)
if !ok {
return errors.New("keymanager does not implement Deleter interface")
}
func DeleteAccount(ctx context.Context, cfg *DeleteConfig) error {
if len(cfg.DeletePublicKeys) == 1 {
log.Info("Deleting account...")
} else {
log.Info("Deleting accounts...")
}
statuses, err := deleter.DeleteKeystores(ctx, cfg.DeletePublicKeys)
statuses, err := cfg.Keymanager.DeleteKeystores(ctx, cfg.DeletePublicKeys)
if err != nil {
return errors.Wrap(err, "could not delete accounts")
}

View File

@ -17,6 +17,7 @@ import (
fieldparams "github.com/prysmaticlabs/prysm/config/fieldparams"
"github.com/prysmaticlabs/prysm/crypto/bls"
"github.com/prysmaticlabs/prysm/encoding/bytesutil"
ethpbservice "github.com/prysmaticlabs/prysm/proto/eth/service"
ethpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1"
validatorpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1/validator-client"
"github.com/prysmaticlabs/prysm/testing/assert"
@ -59,6 +60,10 @@ func (km *mockRemoteKeymanager) ListKeymanagerAccounts(ctx context.Context, cfg
return remote.ListKeymanagerAccountsImpl(ctx, cfg, km, km.opts)
}
func (*mockRemoteKeymanager) DeleteKeystores(context.Context, [][]byte) ([]*ethpbservice.DeletedKeystoreStatus, error) {
return nil, nil
}
func createRandomKeystore(t testing.TB, password string) *keymanager.Keystore {
encryptor := keystorev4.New()
id, err := uuid.NewRandom()

View File

@ -116,6 +116,7 @@ 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",
"//proto/prysm/v1alpha1/block:go_default_library",
"//proto/prysm/v1alpha1/validator-client:go_default_library",

View File

@ -21,6 +21,7 @@ import (
validator_service_config "github.com/prysmaticlabs/prysm/config/validator/service"
"github.com/prysmaticlabs/prysm/crypto/bls"
"github.com/prysmaticlabs/prysm/encoding/bytesutil"
ethpbservice "github.com/prysmaticlabs/prysm/proto/eth/service"
ethpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1"
validatorpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1/validator-client"
"github.com/prysmaticlabs/prysm/testing/assert"
@ -115,6 +116,11 @@ func (*mockKeymanager) ListKeymanagerAccounts(
return nil
}
func (*mockKeymanager) DeleteKeystores(context.Context, [][]byte,
) ([]*ethpbservice.DeletedKeystoreStatus, error) {
return nil, nil
}
func generateMockStatusResponse(pubkeys [][]byte) *ethpb.ValidatorActivationResponse {
multipleStatus := make([]*ethpb.ValidatorActivationResponse_Status, len(pubkeys))
for i, key := range pubkeys {

View File

@ -16,6 +16,7 @@ go_library(
"//config/fieldparams:go_default_library",
"//crypto/bls:go_default_library",
"//encoding/bytesutil:go_default_library",
"//proto/eth/service:go_default_library",
"//proto/prysm/v1alpha1/validator-client:go_default_library",
"//validator/keymanager:go_default_library",
"//validator/keymanager/remote-utils:go_default_library",

View File

@ -14,6 +14,7 @@ import (
fieldparams "github.com/prysmaticlabs/prysm/config/fieldparams"
"github.com/prysmaticlabs/prysm/crypto/bls"
"github.com/prysmaticlabs/prysm/encoding/bytesutil"
ethpbservice "github.com/prysmaticlabs/prysm/proto/eth/service"
validatorpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1/validator-client"
"github.com/prysmaticlabs/prysm/validator/keymanager"
remote_utils "github.com/prysmaticlabs/prysm/validator/keymanager/remote-utils"
@ -239,13 +240,18 @@ func (*Keymanager) SubscribeAccountChanges(_ chan [][48]byte) event.Subscription
})
}
// ExtractKeystores is not supported for the remote keymanager type.
// ExtractKeystores is not supported for the remote-web3signer keymanager type.
func (*Keymanager) ExtractKeystores(
ctx context.Context, publicKeys []bls.PublicKey, password string,
) ([]*keymanager.Keystore, error) {
return nil, errors.New("extracting keys is not supported for a web3signer keymanager")
}
// DeleteKeystores is not supported for the remote-web3signer keymanager type.
func (km *Keymanager) DeleteKeystores(context.Context, [][]byte) ([]*ethpbservice.DeletedKeystoreStatus, error) {
return nil, errors.New("Wrong wallet type: web3-signer. Only Imported or Derived wallets can delete accounts")
}
func (km *Keymanager) ListKeymanagerAccounts(ctx context.Context, cfg keymanager.ListKeymanagerAccountConfig) error {
au := aurora.NewAurora(true)
fmt.Printf("(keymanager kind) %s\n", au.BrightGreen("web3signer").Bold())

View File

@ -17,6 +17,7 @@ go_library(
"//config/fieldparams:go_default_library",
"//crypto/bls:go_default_library",
"//encoding/bytesutil:go_default_library",
"//proto/eth/service:go_default_library",
"//proto/prysm/v1alpha1/validator-client:go_default_library",
"//validator/keymanager:go_default_library",
"//validator/keymanager/remote-utils:go_default_library",

View File

@ -20,6 +20,7 @@ import (
fieldparams "github.com/prysmaticlabs/prysm/config/fieldparams"
"github.com/prysmaticlabs/prysm/crypto/bls"
"github.com/prysmaticlabs/prysm/encoding/bytesutil"
ethpbservice "github.com/prysmaticlabs/prysm/proto/eth/service"
validatorpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1/validator-client"
"github.com/prysmaticlabs/prysm/validator/keymanager"
remote_utils "github.com/prysmaticlabs/prysm/validator/keymanager/remote-utils"
@ -277,6 +278,11 @@ func (*Keymanager) ExtractKeystores(
return nil, errors.New("extracting keys not supported for a remote keymanager")
}
// DeleteKeystores is not supported for the remote keymanager type.
func (*Keymanager) DeleteKeystores(context.Context, [][]byte) ([]*ethpbservice.DeletedKeystoreStatus, error) {
return nil, errors.New("Wrong wallet type: web3-signer. Only Imported or Derived wallets can delete accounts")
}
func (km *Keymanager) ListKeymanagerAccounts(ctx context.Context, cfg keymanager.ListKeymanagerAccountConfig) error {
return ListKeymanagerAccountsImpl(ctx, cfg, km, km.KeymanagerOpts())
}

View File

@ -10,6 +10,7 @@ go_library(
"//async/event:go_default_library",
"//config/fieldparams:go_default_library",
"//crypto/bls:go_default_library",
"//proto/eth/service:go_default_library",
"//proto/prysm/v1alpha1/validator-client:go_default_library",
"//validator/keymanager:go_default_library",
],

View File

@ -7,6 +7,7 @@ import (
"github.com/prysmaticlabs/prysm/async/event"
fieldparams "github.com/prysmaticlabs/prysm/config/fieldparams"
"github.com/prysmaticlabs/prysm/crypto/bls"
ethpbservice "github.com/prysmaticlabs/prysm/proto/eth/service"
validatorpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1/validator-client"
"github.com/prysmaticlabs/prysm/validator/keymanager"
)
@ -60,3 +61,8 @@ func (*MockKeymanager) ListKeymanagerAccounts(
context.Context, keymanager.ListKeymanagerAccountConfig) error {
return nil
}
func (*MockKeymanager) DeleteKeystores(context.Context, [][]byte,
) ([]*ethpbservice.DeletedKeystoreStatus, error) {
return nil, nil
}

View File

@ -19,6 +19,7 @@ type IKeymanager interface {
KeyChangeSubscriber
KeyStoreExtractor
AccountLister
Deleter
}
// KeysFetcher for validating private and public keys.

View File

@ -173,11 +173,7 @@ func (s *Server) DeleteAccounts(
if err != nil {
return nil, err
}
if s.wallet.KeymanagerKind() != keymanager.Local && s.wallet.KeymanagerKind() != keymanager.Derived {
return nil, status.Error(codes.FailedPrecondition, "Only Imported or Derived wallets can delete accounts")
}
if err := accounts.DeleteAccount(ctx, &accounts.Config{
Wallet: s.wallet,
if err := accounts.DeleteAccount(ctx, &accounts.DeleteConfig{
Keymanager: km,
DeletePublicKeys: req.PublicKeysToDelete,
}); err != nil {

View File

@ -152,15 +152,10 @@ func (s *Server) DeleteKeystores(
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not get keymanager: %v", err)
}
deleter, ok := km.(keymanager.Deleter)
if !ok {
statuses := groupExportErrors(req, "Keymanager kind cannot delete keys")
return &ethpbservice.DeleteKeystoresResponse{Data: statuses}, nil
}
if len(req.Pubkeys) == 0 {
return &ethpbservice.DeleteKeystoresResponse{Data: make([]*ethpbservice.DeletedKeystoreStatus, 0)}, nil
}
statuses, err := deleter.DeleteKeystores(ctx, req.Pubkeys)
statuses, err := km.DeleteKeystores(ctx, req.Pubkeys)
if err != nil {
return nil, status.Errorf(codes.Internal, "Could not delete keys: %v", err)
}

View File

@ -472,11 +472,9 @@ func TestServer_DeleteKeystores_WrongKeymanagerKind(t *testing.T) {
wallet: w,
validatorService: vs,
}
response, err := s.DeleteKeystores(ctx, &ethpbservice.DeleteKeystoresRequest{Pubkeys: [][]byte{[]byte("a")}})
require.NoError(t, err)
require.Equal(t, 1, len(response.Data))
require.Equal(t, ethpbservice.DeletedKeystoreStatus_ERROR, response.Data[0].Status)
require.Equal(t, "Keymanager kind cannot delete keys", response.Data[0].Message)
_, err = s.DeleteKeystores(ctx, &ethpbservice.DeleteKeystoresRequest{Pubkeys: [][]byte{[]byte("a")}})
require.ErrorContains(t, "Wrong wallet type", err)
require.ErrorContains(t, "Only Imported or Derived wallets can delete accounts", err)
}
func setupServerWithWallet(t testing.TB) *Server {