diff --git a/proto/eth/service/key_management.pb.go b/proto/eth/service/key_management.pb.go index 90d4355ca4..b8a305b8eb 100755 --- a/proto/eth/service/key_management.pb.go +++ b/proto/eth/service/key_management.pb.go @@ -87,9 +87,10 @@ func (ImportedKeystoreStatus_Status) EnumDescriptor() ([]byte, []int) { type DeletedKeystoreStatus_Status int32 const ( - DeletedKeystoreStatus_DELETED DeletedKeystoreStatus_Status = 0 - DeletedKeystoreStatus_NOT_FOUND DeletedKeystoreStatus_Status = 1 - DeletedKeystoreStatus_ERROR DeletedKeystoreStatus_Status = 2 + DeletedKeystoreStatus_DELETED DeletedKeystoreStatus_Status = 0 + DeletedKeystoreStatus_NOT_FOUND DeletedKeystoreStatus_Status = 1 + DeletedKeystoreStatus_NOT_ACTIVE DeletedKeystoreStatus_Status = 2 + DeletedKeystoreStatus_ERROR DeletedKeystoreStatus_Status = 3 ) // Enum value maps for DeletedKeystoreStatus_Status. @@ -97,12 +98,14 @@ var ( DeletedKeystoreStatus_Status_name = map[int32]string{ 0: "DELETED", 1: "NOT_FOUND", - 2: "ERROR", + 2: "NOT_ACTIVE", + 3: "ERROR", } DeletedKeystoreStatus_Status_value = map[string]int32{ - "DELETED": 0, - "NOT_FOUND": 1, - "ERROR": 2, + "DELETED": 0, + "NOT_FOUND": 1, + "NOT_ACTIVE": 2, + "ERROR": 3, } ) @@ -625,7 +628,7 @@ var file_proto_eth_service_key_management_proto_rawDesc = []byte{ 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x22, 0x30, 0x0a, 0x06, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x0c, 0x0a, 0x08, 0x49, 0x4d, 0x50, 0x4f, 0x52, 0x54, 0x45, 0x44, 0x10, 0x00, 0x12, 0x0d, 0x0a, 0x09, 0x44, 0x55, 0x50, 0x4c, 0x49, 0x43, 0x41, 0x54, 0x45, 0x10, 0x01, 0x12, 0x09, - 0x0a, 0x05, 0x45, 0x52, 0x52, 0x4f, 0x52, 0x10, 0x02, 0x22, 0xae, 0x01, 0x0a, 0x15, 0x44, 0x65, + 0x0a, 0x05, 0x45, 0x52, 0x52, 0x4f, 0x52, 0x10, 0x02, 0x22, 0xbe, 0x01, 0x0a, 0x15, 0x44, 0x65, 0x6c, 0x65, 0x74, 0x65, 0x64, 0x4b, 0x65, 0x79, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x4a, 0x0a, 0x06, 0x73, 0x74, 0x61, 0x74, 0x75, 0x73, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0e, 0x32, 0x32, 0x2e, 0x65, 0x74, 0x68, 0x65, 0x72, 0x65, 0x75, 0x6d, 0x2e, 0x65, @@ -633,10 +636,11 @@ var file_proto_eth_service_key_management_proto_rawDesc = []byte{ 0x65, 0x64, 0x4b, 0x65, 0x79, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x2e, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x52, 0x06, 0x73, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x18, 0x0a, 0x07, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, - 0x52, 0x07, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x22, 0x2f, 0x0a, 0x06, 0x53, 0x74, 0x61, + 0x52, 0x07, 0x6d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x22, 0x3f, 0x0a, 0x06, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x12, 0x0b, 0x0a, 0x07, 0x44, 0x45, 0x4c, 0x45, 0x54, 0x45, 0x44, 0x10, 0x00, 0x12, 0x0d, 0x0a, 0x09, 0x4e, 0x4f, 0x54, 0x5f, 0x46, 0x4f, 0x55, 0x4e, 0x44, 0x10, 0x01, 0x12, - 0x09, 0x0a, 0x05, 0x45, 0x52, 0x52, 0x4f, 0x52, 0x10, 0x02, 0x32, 0xb9, 0x03, 0x0a, 0x0d, 0x4b, + 0x0e, 0x0a, 0x0a, 0x4e, 0x4f, 0x54, 0x5f, 0x41, 0x43, 0x54, 0x49, 0x56, 0x45, 0x10, 0x02, 0x12, + 0x09, 0x0a, 0x05, 0x45, 0x52, 0x52, 0x4f, 0x52, 0x10, 0x03, 0x32, 0xb9, 0x03, 0x0a, 0x0d, 0x4b, 0x65, 0x79, 0x4d, 0x61, 0x6e, 0x61, 0x67, 0x65, 0x6d, 0x65, 0x6e, 0x74, 0x12, 0x78, 0x0a, 0x0d, 0x4c, 0x69, 0x73, 0x74, 0x4b, 0x65, 0x79, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x73, 0x12, 0x16, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, diff --git a/proto/eth/service/key_management.proto b/proto/eth/service/key_management.proto index 52e8790dfe..94da434e70 100644 --- a/proto/eth/service/key_management.proto +++ b/proto/eth/service/key_management.proto @@ -130,7 +130,8 @@ message DeletedKeystoreStatus { enum Status { DELETED = 0; NOT_FOUND = 1; - ERROR = 2; + NOT_ACTIVE = 2; + ERROR = 3; } Status status = 1; string message = 2; diff --git a/validator/accounts/BUILD.bazel b/validator/accounts/BUILD.bazel index 3b0f40c094..4da748bce8 100644 --- a/validator/accounts/BUILD.bazel +++ b/validator/accounts/BUILD.bazel @@ -32,6 +32,7 @@ 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", diff --git a/validator/accounts/accounts_delete.go b/validator/accounts/accounts_delete.go index 1270458892..936f2b6fb1 100644 --- a/validator/accounts/accounts_delete.go +++ b/validator/accounts/accounts_delete.go @@ -10,12 +10,11 @@ import ( "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/prysmaticlabs/prysm/validator/keymanager/derived" - "github.com/prysmaticlabs/prysm/validator/keymanager/imported" "github.com/urfave/cli/v2" ) @@ -102,37 +101,28 @@ 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 { - switch cfg.Wallet.KeymanagerKind() { - case keymanager.Remote: - return errors.New("cannot delete accounts for a remote keymanager") - case keymanager.Imported: - km, ok := cfg.Keymanager.(*imported.Keymanager) - if !ok { - return errors.New("not a imported keymanager") + deleter, ok := cfg.Keymanager.(keymanager.Deleter) + if !ok { + return errors.New("keymanager does not implement Deleter interface") + } + if len(cfg.DeletePublicKeys) == 1 { + log.Info("Deleting account...") + } else { + log.Info("Deleting accounts...") + } + statuses, err := deleter.DeleteKeystores(ctx, cfg.DeletePublicKeys) + if err != nil { + return errors.Wrap(err, "could not delete accounts") + } + for i, status := range statuses { + switch status.Status { + case ethpbservice.DeletedKeystoreStatus_ERROR: + log.Errorf("Error deleting key %#x: %s", bytesutil.Trunc(cfg.DeletePublicKeys[i]), status.Message) + case ethpbservice.DeletedKeystoreStatus_NOT_ACTIVE: + log.Warnf("Duplicate key %#x found in delete request", bytesutil.Trunc(cfg.DeletePublicKeys[i])) + case ethpbservice.DeletedKeystoreStatus_NOT_FOUND: + log.Warnf("Could not find keystore for %#x", bytesutil.Trunc(cfg.DeletePublicKeys[i])) } - if len(cfg.DeletePublicKeys) == 1 { - log.Info("Deleting account...") - } else { - log.Info("Deleting accounts...") - } - if err := km.DeleteAccounts(ctx, cfg.DeletePublicKeys); err != nil { - return errors.Wrap(err, "could not delete accounts") - } - case keymanager.Derived: - km, ok := cfg.Keymanager.(*derived.Keymanager) - if !ok { - return errors.New("not a derived keymanager") - } - if len(cfg.DeletePublicKeys) == 1 { - log.Info("Deleting account...") - } else { - log.Info("Deleting accounts...") - } - if err := km.DeleteAccounts(ctx, cfg.DeletePublicKeys); err != nil { - return errors.Wrap(err, "could not delete accounts") - } - default: - return fmt.Errorf(errKeymanagerNotSupported, cfg.Wallet.KeymanagerKind()) } return nil } diff --git a/validator/keymanager/BUILD.bazel b/validator/keymanager/BUILD.bazel index 3e67d31934..945cfbde07 100644 --- a/validator/keymanager/BUILD.bazel +++ b/validator/keymanager/BUILD.bazel @@ -15,6 +15,7 @@ go_library( deps = [ "//async/event:go_default_library", "//crypto/bls:go_default_library", + "//proto/eth/service:go_default_library", "//proto/prysm/v1alpha1/validator-client:go_default_library", ], ) diff --git a/validator/keymanager/derived/BUILD.bazel b/validator/keymanager/derived/BUILD.bazel index ba6aee0708..47101ee8ed 100644 --- a/validator/keymanager/derived/BUILD.bazel +++ b/validator/keymanager/derived/BUILD.bazel @@ -17,6 +17,7 @@ go_library( "//crypto/bls:go_default_library", "//crypto/rand:go_default_library", "//io/prompt:go_default_library", + "//proto/eth/service:go_default_library", "//proto/prysm/v1alpha1/validator-client:go_default_library", "//validator/accounts/iface:go_default_library", "//validator/keymanager:go_default_library", diff --git a/validator/keymanager/derived/keymanager.go b/validator/keymanager/derived/keymanager.go index a951c240e9..f2d44959fa 100644 --- a/validator/keymanager/derived/keymanager.go +++ b/validator/keymanager/derived/keymanager.go @@ -7,6 +7,7 @@ import ( "github.com/pkg/errors" "github.com/prysmaticlabs/prysm/async/event" "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/accounts/iface" "github.com/prysmaticlabs/prysm/validator/keymanager" @@ -106,9 +107,11 @@ func (km *Keymanager) FetchValidatingPrivateKeys(ctx context.Context) ([][32]byt return km.importedKM.FetchValidatingPrivateKeys(ctx) } -// DeleteAccounts for a derived keymanager. -func (km *Keymanager) DeleteAccounts(ctx context.Context, publicKeys [][]byte) error { - return km.importedKM.DeleteAccounts(ctx, publicKeys) +// DeleteKeystores for a derived keymanager. +func (km *Keymanager) DeleteKeystores( + ctx context.Context, publicKeys [][]byte, +) ([]*ethpbservice.DeletedKeystoreStatus, error) { + return km.importedKM.DeleteKeystores(ctx, publicKeys) } // SubscribeAccountChanges creates an event subscription for a channel diff --git a/validator/keymanager/imported/BUILD.bazel b/validator/keymanager/imported/BUILD.bazel index c184ef7892..abd80be186 100644 --- a/validator/keymanager/imported/BUILD.bazel +++ b/validator/keymanager/imported/BUILD.bazel @@ -4,6 +4,7 @@ go_library( name = "go_default_library", srcs = [ "backup.go", + "delete.go", "doc.go", "import.go", "keymanager.go", @@ -24,6 +25,7 @@ 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/validator-client:go_default_library", "//runtime/interop:go_default_library", "//validator/accounts/iface:go_default_library", @@ -44,6 +46,7 @@ go_test( name = "go_default_test", srcs = [ "backup_test.go", + "delete_test.go", "import_test.go", "keymanager_test.go", "refresh_test.go", @@ -53,6 +56,7 @@ go_test( "//async/event: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", "//testing/assert:go_default_library", "//testing/require:go_default_library", diff --git a/validator/keymanager/imported/delete.go b/validator/keymanager/imported/delete.go new file mode 100644 index 0000000000..83f0a9cefc --- /dev/null +++ b/validator/keymanager/imported/delete.go @@ -0,0 +1,93 @@ +package imported + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + + "github.com/pkg/errors" + "github.com/prysmaticlabs/prysm/encoding/bytesutil" + ethpbservice "github.com/prysmaticlabs/prysm/proto/eth/service" + "github.com/sirupsen/logrus" +) + +// DeleteKeystores takes in public keys and removes the accounts from the wallet. +// This includes their disk keystore and cached keystore, but maintains the slashing +// protection history in the database. +func (km *Keymanager) DeleteKeystores( + ctx context.Context, publicKeys [][]byte, +) ([]*ethpbservice.DeletedKeystoreStatus, error) { + // Check for duplicate keys and filter them out. + trackedPublicKeys := make(map[[48]byte]bool) + statuses := make([]*ethpbservice.DeletedKeystoreStatus, 0, len(publicKeys)) + var store *AccountsKeystoreRepresentation + var err error + deletedKeys := make([][]byte, 0, len(publicKeys)) + for _, publicKey := range publicKeys { + // Check if the key in the request is a duplicate. + if _, ok := trackedPublicKeys[bytesutil.ToBytes48(publicKey)]; ok { + statuses = append(statuses, ðpbservice.DeletedKeystoreStatus{ + Status: ethpbservice.DeletedKeystoreStatus_NOT_ACTIVE, + }) + continue + } + var index int + var found bool + for j, pubKey := range km.accountsStore.PublicKeys { + if bytes.Equal(pubKey, publicKey) { + index = j + found = true + break + } + } + if !found { + statuses = append(statuses, ðpbservice.DeletedKeystoreStatus{ + Status: ethpbservice.DeletedKeystoreStatus_NOT_FOUND, + }) + continue + } + deletedPublicKey := km.accountsStore.PublicKeys[index] + deletedKeys = append(deletedKeys, deletedPublicKey) + km.accountsStore.PrivateKeys = append(km.accountsStore.PrivateKeys[:index], km.accountsStore.PrivateKeys[index+1:]...) + km.accountsStore.PublicKeys = append(km.accountsStore.PublicKeys[:index], km.accountsStore.PublicKeys[index+1:]...) + store, err = km.CreateAccountsKeystore(ctx, km.accountsStore.PrivateKeys, km.accountsStore.PublicKeys) + if err != nil { + return nil, errors.Wrap(err, "could not rewrite accounts keystore") + } + statuses = append(statuses, ðpbservice.DeletedKeystoreStatus{ + Status: ethpbservice.DeletedKeystoreStatus_DELETED, + }) + trackedPublicKeys[bytesutil.ToBytes48(publicKey)] = true + } + if len(deletedKeys) == 0 { + return statuses, nil + } + var deletedKeysStr string + for i, k := range deletedKeys { + if i == 0 { + deletedKeysStr += fmt.Sprintf("%#x", bytesutil.Trunc(k)) + } else if i == len(deletedKeys)-1 { + deletedKeysStr += fmt.Sprintf("%#x", bytesutil.Trunc(k)) + } else { + deletedKeysStr += fmt.Sprintf(",%#x", bytesutil.Trunc(k)) + } + } + log.WithFields(logrus.Fields{ + "publicKeys": deletedKeysStr, + }).Info("Successfully deleted validator key(s)") + + // Write the encoded keystore. + encoded, err := json.MarshalIndent(store, "", "\t") + if err != nil { + return nil, err + } + if err := km.wallet.WriteFileAtPath(ctx, AccountsPath, AccountsKeystoreFileName, encoded); err != nil { + return nil, errors.Wrap(err, "could not write keystore file for accounts") + } + err = km.initializeKeysCachesFromKeystore() + if err != nil { + return nil, errors.Wrap(err, "failed to initialize key caches") + } + return statuses, nil +} diff --git a/validator/keymanager/imported/delete_test.go b/validator/keymanager/imported/delete_test.go new file mode 100644 index 0000000000..534725aee3 --- /dev/null +++ b/validator/keymanager/imported/delete_test.go @@ -0,0 +1,126 @@ +package imported + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "testing" + + "github.com/prysmaticlabs/prysm/encoding/bytesutil" + ethpbservice "github.com/prysmaticlabs/prysm/proto/eth/service" + "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)) + + t.Run("keys not found", func(t *testing.T) { + notFoundPubKey := [48]byte{1, 2, 3} + notFoundPubKey2 := [48]byte{4, 5, 6} + statuses, err := dr.DeleteKeystores(ctx, [][]byte{notFoundPubKey[:], notFoundPubKey2[:]}) + require.NoError(t, err) + require.Equal(t, 2, len(statuses)) + require.Equal(t, ethpbservice.DeletedKeystoreStatus_NOT_FOUND, statuses[0].Status) + require.Equal(t, ethpbservice.DeletedKeystoreStatus_NOT_FOUND, statuses[1].Status) + }) + t.Run("deletes properly", func(t *testing.T) { + accountToRemove := uint64(2) + accountPubKey := accounts[accountToRemove] + statuses, err := dr.DeleteKeystores(ctx, [][]byte{accountPubKey[:]}) + require.NoError(t, err) + + require.Equal(t, 1, len(statuses)) + require.Equal(t, ethpbservice.DeletedKeystoreStatus_DELETED, statuses[0].Status) + + // 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 key(s)") + }) + t.Run("returns NOT_ACTIVE status for duplicate public key in request", func(t *testing.T) { + accountToRemove := uint64(3) + accountPubKey := accounts[accountToRemove] + statuses, err := dr.DeleteKeystores(ctx, [][]byte{ + accountPubKey[:], + accountPubKey[:], // Add in the same key a few more times. + accountPubKey[:], + accountPubKey[:], + }) + require.NoError(t, err) + + require.Equal(t, 4, len(statuses)) + for i, st := range statuses { + if i == 0 { + require.Equal(t, ethpbservice.DeletedKeystoreStatus_DELETED, st.Status) + } else { + require.Equal(t, ethpbservice.DeletedKeystoreStatus_NOT_ACTIVE, st.Status) + } + } + + // 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-2, len(store.PublicKeys)) + require.Equal(t, numAccounts-2, len(store.PrivateKeys)) + require.LogsContain(t, hook, fmt.Sprintf("%#x", bytesutil.Trunc(accountPubKey[:]))) + require.LogsContain(t, hook, "Successfully deleted validator key(s)") + }) +} diff --git a/validator/keymanager/imported/keymanager.go b/validator/keymanager/imported/keymanager.go index 970a5010a1..57fa4ff7ec 100644 --- a/validator/keymanager/imported/keymanager.go +++ b/validator/keymanager/imported/keymanager.go @@ -1,7 +1,6 @@ package imported import ( - "bytes" "context" "encoding/json" "fmt" @@ -18,7 +17,6 @@ import ( "github.com/prysmaticlabs/prysm/validator/accounts/iface" "github.com/prysmaticlabs/prysm/validator/accounts/petnames" "github.com/prysmaticlabs/prysm/validator/keymanager" - "github.com/sirupsen/logrus" keystorev4 "github.com/wealdtech/go-eth2-wallet-encryptor-keystorev4" "go.opencensus.io/trace" ) @@ -158,52 +156,6 @@ func (km *Keymanager) initializeKeysCachesFromKeystore() error { return nil } -// DeleteAccounts takes in public keys and removes the accounts entirely. This includes their disk keystore and cached keystore. -func (km *Keymanager) DeleteAccounts(ctx context.Context, publicKeys [][]byte) error { - for _, publicKey := range publicKeys { - var index int - var found bool - for i, pubKey := range km.accountsStore.PublicKeys { - if bytes.Equal(pubKey, publicKey) { - index = i - found = true - break - } - } - if !found { - return fmt.Errorf("could not find public key %#x", publicKey) - } - deletedPublicKey := km.accountsStore.PublicKeys[index] - accountName := petnames.DeterministicName(deletedPublicKey, "-") - km.accountsStore.PrivateKeys = append(km.accountsStore.PrivateKeys[:index], km.accountsStore.PrivateKeys[index+1:]...) - km.accountsStore.PublicKeys = append(km.accountsStore.PublicKeys[:index], km.accountsStore.PublicKeys[index+1:]...) - - newStore, err := km.CreateAccountsKeystore(ctx, km.accountsStore.PrivateKeys, km.accountsStore.PublicKeys) - if err != nil { - return errors.Wrap(err, "could not rewrite accounts keystore") - } - - // Write the encoded keystore. - encoded, err := json.MarshalIndent(newStore, "", "\t") - if err != nil { - return err - } - if err := km.wallet.WriteFileAtPath(ctx, AccountsPath, AccountsKeystoreFileName, encoded); err != nil { - return errors.Wrap(err, "could not write keystore file for accounts") - } - - log.WithFields(logrus.Fields{ - "name": accountName, - "publicKey": fmt.Sprintf("%#x", bytesutil.Trunc(deletedPublicKey)), - }).Info("Successfully deleted validator account") - err = km.initializeKeysCachesFromKeystore() - if err != nil { - return errors.Wrap(err, "failed to initialize keys caches") - } - } - return nil -} - // FetchValidatingPublicKeys fetches the list of active public keys from the imported account keystores. func (km *Keymanager) FetchValidatingPublicKeys(ctx context.Context) ([][48]byte, error) { ctx, span := trace.StartSpan(ctx, "keymanager.FetchValidatingPublicKeys") diff --git a/validator/keymanager/imported/keymanager_test.go b/validator/keymanager/imported/keymanager_test.go index 5add2eff47..194e11a0e5 100644 --- a/validator/keymanager/imported/keymanager_test.go +++ b/validator/keymanager/imported/keymanager_test.go @@ -3,7 +3,6 @@ package imported import ( "context" "encoding/json" - "fmt" "strings" "testing" @@ -14,60 +13,9 @@ import ( "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") -} - func TestImportedKeymanager_FetchValidatingPublicKeys(t *testing.T) { wallet := &mock.Wallet{ Files: make(map[string]map[string][]byte), diff --git a/validator/keymanager/types.go b/validator/keymanager/types.go index e2bcb047fc..69fe29e392 100644 --- a/validator/keymanager/types.go +++ b/validator/keymanager/types.go @@ -6,16 +6,45 @@ import ( "github.com/prysmaticlabs/prysm/async/event" "github.com/prysmaticlabs/prysm/crypto/bls" + ethpbservice "github.com/prysmaticlabs/prysm/proto/eth/service" validatorpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1/validator-client" ) // IKeymanager defines a general keymanager interface for Prysm wallets. type IKeymanager interface { - // FetchValidatingPublicKeys fetches the list of active public keys that should be used to validate with. + PublicKeysFetcher + Signer + KeyChangeSubscriber +} + +// KeysFetcher for validating private and public keys. +type KeysFetcher interface { + FetchValidatingPrivateKeys(ctx context.Context) ([][32]byte, error) + PublicKeysFetcher +} + +// PublicKeysFetcher for validating public keys. +type PublicKeysFetcher interface { FetchValidatingPublicKeys(ctx context.Context) ([][48]byte, error) - // Sign signs a message using a validator key. +} + +// Signer allows signing messages using a validator private key. +type Signer interface { Sign(context.Context, *validatorpb.SignRequest) (bls.Signature, error) - // SubscribeAccountChanges subscribes to changes made to the underlying keys. +} + +// Importer can import new keystores into the keymanager. +type Importer interface { + ImportKeystores(ctx context.Context, keystores []*Keystore, importsPassword string) error +} + +// Deleter can delete keystores from the keymanager. +type Deleter interface { + DeleteKeystores(ctx context.Context, publicKeys [][]byte) ([]*ethpbservice.DeletedKeystoreStatus, error) +} + +// KeyChangeSubscriber allows subscribing to changes made to the underlying keys. +type KeyChangeSubscriber interface { SubscribeAccountChanges(pubKeysChan chan [][48]byte) event.Subscription } diff --git a/validator/keymanager/types_test.go b/validator/keymanager/types_test.go index 315692086c..71e765875e 100644 --- a/validator/keymanager/types_test.go +++ b/validator/keymanager/types_test.go @@ -11,4 +11,11 @@ var ( _ = keymanager.IKeymanager(&imported.Keymanager{}) _ = keymanager.IKeymanager(&derived.Keymanager{}) _ = keymanager.IKeymanager(&remote.Keymanager{}) + + // More granular assertions. + _ = keymanager.KeysFetcher(&imported.Keymanager{}) + _ = keymanager.KeysFetcher(&derived.Keymanager{}) + _ = keymanager.Importer(&imported.Keymanager{}) + _ = keymanager.Deleter(&imported.Keymanager{}) + _ = keymanager.Deleter(&derived.Keymanager{}) ) diff --git a/validator/rpc/BUILD.bazel b/validator/rpc/BUILD.bazel index 7480a7c684..f6b202a468 100644 --- a/validator/rpc/BUILD.bazel +++ b/validator/rpc/BUILD.bazel @@ -91,6 +91,7 @@ go_test( "//crypto/rand: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/validator-client:go_default_library", "//testing/assert:go_default_library", diff --git a/validator/rpc/standard_api.go b/validator/rpc/standard_api.go index 1b92542a35..a5433eb5c7 100644 --- a/validator/rpc/standard_api.go +++ b/validator/rpc/standard_api.go @@ -2,18 +2,20 @@ package rpc import ( "context" + "encoding/json" "fmt" "github.com/golang/protobuf/ptypes/empty" ethpbservice "github.com/prysmaticlabs/prysm/proto/eth/service" "github.com/prysmaticlabs/prysm/validator/keymanager" "github.com/prysmaticlabs/prysm/validator/keymanager/derived" + slashingprotection "github.com/prysmaticlabs/prysm/validator/slashing-protection-history" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) // ListKeystores implements the standard validator key management API. -func (s Server) ListKeystores( +func (s *Server) ListKeystores( ctx context.Context, _ *empty.Empty, ) (*ethpbservice.ListKeystoresResponse, error) { if !s.walletInitialized { @@ -36,3 +38,41 @@ func (s Server) ListKeystores( Keystores: keystoreResponse, }, nil } + +// DeleteKeystores allows for deleting specified public keys from Prysm. +func (s *Server) DeleteKeystores( + ctx context.Context, req *ethpbservice.DeleteKeystoresRequest, +) (*ethpbservice.DeleteKeystoresResponse, error) { + if !s.walletInitialized { + return nil, status.Error(codes.Internal, "Wallet not ready") + } + deleter, ok := s.keymanager.(keymanager.Deleter) + if !ok { + return nil, status.Error(codes.Internal, "Keymanager kind cannot delete keys") + } + statuses, err := deleter.DeleteKeystores(ctx, req.PublicKeys) + if err != nil { + return nil, status.Errorf(codes.Internal, "Could not delete keys: %v", err) + } + keysToFilter := req.PublicKeys + exportedHistory, err := slashingprotection.ExportStandardProtectionJSON(ctx, s.valDB, keysToFilter...) + if err != nil { + return nil, status.Errorf( + codes.Internal, + "Could not export slashing protection history: %v", + err, + ) + } + jsonHist, err := json.Marshal(exportedHistory) + if err != nil { + return nil, status.Errorf( + codes.Internal, + "Could not export slashing protection history: %v", + err, + ) + } + return ðpbservice.DeleteKeystoresResponse{ + Statuses: statuses, + SlashingProtection: string(jsonHist), + }, nil +} diff --git a/validator/rpc/standard_api_test.go b/validator/rpc/standard_api_test.go index bdbdeb3840..de717e0d5c 100644 --- a/validator/rpc/standard_api_test.go +++ b/validator/rpc/standard_api_test.go @@ -2,17 +2,21 @@ package rpc import ( "context" + "encoding/json" "fmt" "testing" "github.com/golang/protobuf/ptypes/empty" + ethpbservice "github.com/prysmaticlabs/prysm/proto/eth/service" + validatorpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1/validator-client" "github.com/prysmaticlabs/prysm/testing/require" "github.com/prysmaticlabs/prysm/validator/accounts" "github.com/prysmaticlabs/prysm/validator/accounts/iface" "github.com/prysmaticlabs/prysm/validator/accounts/wallet" + "github.com/prysmaticlabs/prysm/validator/db/kv" "github.com/prysmaticlabs/prysm/validator/keymanager" "github.com/prysmaticlabs/prysm/validator/keymanager/derived" - constant "github.com/prysmaticlabs/prysm/validator/testing" + mocks "github.com/prysmaticlabs/prysm/validator/testing" ) func TestServer_ListKeystores(t *testing.T) { @@ -52,7 +56,7 @@ func TestServer_ListKeystores(t *testing.T) { numAccounts := 50 dr, ok := km.(*derived.Keymanager) require.Equal(t, true, ok) - err = dr.RecoverAccountsFromMnemonic(ctx, constant.TestMnemonic, "", numAccounts) + err = dr.RecoverAccountsFromMnemonic(ctx, mocks.TestMnemonic, "", numAccounts) require.NoError(t, err) expectedKeys, err := dr.FetchValidatingPublicKeys(ctx) require.NoError(t, err) @@ -71,3 +75,99 @@ func TestServer_ListKeystores(t *testing.T) { } }) } + +func TestServer_DeleteKeystores(t *testing.T) { + ctx := context.Background() + t.Run("wallet not ready", func(t *testing.T) { + s := Server{} + _, err := s.DeleteKeystores(context.Background(), nil) + require.ErrorContains(t, "Wallet not ready", err) + }) + localWalletDir := setupWalletDir(t) + defaultWalletPath = localWalletDir + w, err := accounts.CreateWalletWithKeymanager(ctx, &accounts.CreateWalletConfig{ + WalletCfg: &wallet.Config{ + WalletDir: defaultWalletPath, + KeymanagerKind: keymanager.Derived, + WalletPassword: strongPass, + }, + SkipMnemonicConfirm: true, + }) + require.NoError(t, err) + km, err := w.InitializeKeymanager(ctx, iface.InitKeymanagerConfig{ListenForChanges: false}) + require.NoError(t, err) + + s := &Server{ + keymanager: km, + walletInitialized: true, + wallet: w, + } + numAccounts := 50 + dr, ok := km.(*derived.Keymanager) + require.Equal(t, true, ok) + err = dr.RecoverAccountsFromMnemonic(ctx, mocks.TestMnemonic, "", numAccounts) + require.NoError(t, err) + + publicKeys, err := km.FetchValidatingPublicKeys(ctx) + require.NoError(t, err) + require.Equal(t, numAccounts, len(publicKeys)) + + // Create a validator database. + validatorDB, err := kv.NewKVStore(ctx, defaultWalletPath, &kv.Config{ + PubKeys: publicKeys, + }) + require.NoError(t, err) + s.valDB = validatorDB + + // Have to close it after import is done otherwise it complains db is not open. + defer func() { + require.NoError(t, validatorDB.Close()) + }() + + // Generate mock slashing history. + attestingHistory := make([][]*kv.AttestationRecord, 0) + proposalHistory := make([]kv.ProposalHistoryForPubkey, len(publicKeys)) + for i := 0; i < len(publicKeys); i++ { + proposalHistory[i].Proposals = make([]kv.Proposal, 0) + } + mockJSON, err := mocks.MockSlashingProtectionJSON(publicKeys, attestingHistory, proposalHistory) + require.NoError(t, err) + + // JSON encode the protection JSON and save it. + encoded, err := json.Marshal(mockJSON) + require.NoError(t, err) + + _, err = s.ImportSlashingProtection(ctx, &validatorpb.ImportSlashingProtectionRequest{ + SlashingProtectionJson: string(encoded), + }) + require.NoError(t, err) + rawPubKeys := make([][]byte, numAccounts) + for i := 0; i < numAccounts; i++ { + rawPubKeys[i] = publicKeys[i][:] + } + + // Deletes properly and returns slashing protection history. + resp, err := s.DeleteKeystores(ctx, ðpbservice.DeleteKeystoresRequest{ + PublicKeys: rawPubKeys, + }) + require.NoError(t, err) + require.Equal(t, numAccounts, len(resp.Statuses)) + for _, status := range resp.Statuses { + require.Equal(t, ethpbservice.DeletedKeystoreStatus_DELETED, status.Status) + } + publicKeys, err = km.FetchValidatingPublicKeys(ctx) + require.NoError(t, err) + require.Equal(t, 0, len(publicKeys)) + require.Equal(t, numAccounts, len(mockJSON.Data)) + + // Returns slashing protection history if already deleted. + resp, err = s.DeleteKeystores(ctx, ðpbservice.DeleteKeystoresRequest{ + PublicKeys: rawPubKeys, + }) + require.NoError(t, err) + require.Equal(t, numAccounts, len(resp.Statuses)) + for _, status := range resp.Statuses { + require.Equal(t, ethpbservice.DeletedKeystoreStatus_NOT_FOUND, status.Status) + } + require.Equal(t, numAccounts, len(mockJSON.Data)) +} diff --git a/validator/slashing-protection-history/export.go b/validator/slashing-protection-history/export.go index 0f37245385..37f6a577b5 100644 --- a/validator/slashing-protection-history/export.go +++ b/validator/slashing-protection-history/export.go @@ -16,7 +16,15 @@ import ( // ExportStandardProtectionJSON extracts all slashing protection data from a validator database // and packages it into an EIP-3076 compliant, standard -func ExportStandardProtectionJSON(ctx context.Context, validatorDB db.Database) (*format.EIPSlashingProtectionFormat, error) { +func ExportStandardProtectionJSON( + ctx context.Context, + validatorDB db.Database, + keysToFilter ...[]byte, +) (*format.EIPSlashingProtectionFormat, error) { + keysFilterMap := make(map[string]bool, len(keysToFilter)) + for _, k := range keysToFilter { + keysFilterMap[string(k)] = true + } interchangeJSON := &format.EIPSlashingProtectionFormat{} genesisValidatorsRoot, err := validatorDB.GenesisValidatorsRoot(ctx) if err != nil { @@ -50,6 +58,9 @@ func ExportStandardProtectionJSON(ctx context.Context, validatorDB db.Database) len(proposedPublicKeys), "Extracting signed blocks by validator public key", ) for _, pubKey := range proposedPublicKeys { + if _, ok := keysFilterMap[string(pubKey[:])]; len(keysToFilter) > 0 && !ok { + continue + } pubKeyHex, err := pubKeyToHexString(pubKey[:]) if err != nil { return nil, errors.Wrap(err, "could not convert public key to hex string") @@ -73,6 +84,9 @@ func ExportStandardProtectionJSON(ctx context.Context, validatorDB db.Database) len(attestedPublicKeys), "Extracting signed attestations by validator public key", ) for _, pubKey := range attestedPublicKeys { + if _, ok := keysFilterMap[string(pubKey[:])]; len(keysToFilter) > 0 && !ok { + continue + } pubKeyHex, err := pubKeyToHexString(pubKey[:]) if err != nil { return nil, errors.Wrap(err, "could not convert public key to hex string") diff --git a/validator/slashing-protection-history/round_trip_test.go b/validator/slashing-protection-history/round_trip_test.go index 3ca5a0aa2f..f782f90a91 100644 --- a/validator/slashing-protection-history/round_trip_test.go +++ b/validator/slashing-protection-history/round_trip_test.go @@ -128,6 +128,43 @@ func TestImportExport_RoundTrip_SkippedAttestationEpochs(t *testing.T) { require.DeepEqual(t, wanted.Data, eipStandard.Data) } +func TestImportExport_FilterKeys(t *testing.T) { + ctx := context.Background() + numValidators := 10 + publicKeys, err := slashtest.CreateRandomPubKeys(numValidators) + require.NoError(t, err) + validatorDB := dbtest.SetupDB(t, publicKeys) + + // First we setup some mock attesting and proposal histories and create a mock + // standard slashing protection format JSON struct. + attestingHistory, proposalHistory := slashtest.MockAttestingAndProposalHistories(publicKeys) + require.NoError(t, err) + wanted, err := slashtest.MockSlashingProtectionJSON(publicKeys, attestingHistory, proposalHistory) + require.NoError(t, err) + + // We encode the standard slashing protection struct into a JSON format. + blob, err := json.Marshal(wanted) + require.NoError(t, err) + buf := bytes.NewBuffer(blob) + + // Next, we attempt to import it into our validator database. + err = history.ImportStandardProtectionJSON(ctx, validatorDB, buf) + require.NoError(t, err) + + // Next up, we export our slashing protection database into the EIP standard file. + // Next, we attempt to import it into our validator database. + rawKeys := make([][]byte, 5) + for i := 0; i < len(rawKeys); i++ { + rawKeys[i] = publicKeys[i][:] + } + eipStandard, err := history.ExportStandardProtectionJSON(ctx, validatorDB, rawKeys...) + require.NoError(t, err) + + // We compare the metadata fields from import to export. + require.Equal(t, wanted.Metadata, eipStandard.Metadata) + require.Equal(t, len(rawKeys), len(eipStandard.Data)) +} + func TestImportInterchangeData_OK(t *testing.T) { ctx := context.Background() numValidators := 10