From 95948f6692bbc463dce04fbb989b60892a0e491e Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 23 Sep 2022 14:26:27 -0500 Subject: [PATCH] perf: reduce user's password prompts when calling keyring List function (backport #13207) (#13368) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf: reduce user's password prompts when calling keyring List function (#13207) * Reduce user password prompts by taking advantage of the already existing MigrateAll function * Print message when no records were found on the keyring * Update changelog * Fix migration test * Add keys sort (cherry picked from commit 4882f933b1a108d578343573595fb06d7fd319c2) # Conflicts: # CHANGELOG.md * fix conflicts * suggestions Co-authored-by: Aleksandr Bezobchuk Co-authored-by: Ezequiel Raynaudo Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Julien Robert Co-authored-by: Aleksandr Bezobchuk --- CHANGELOG.md | 1 + client/keys/list.go | 5 +++ client/keys/migrate.go | 2 +- crypto/keyring/keyring.go | 55 +++++++------------------------- crypto/keyring/migration_test.go | 4 +-- 5 files changed, 21 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b90d49eeb347..c224e19799cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features +* (cli) [#13207](https://github.com/cosmos/cosmos-sdk/pull/13207) Reduce user's password prompts when calling keyring `List()` function. * (cli) [#13353](https://github.com/cosmos/cosmos-sdk/pull/13353) Add `tx group draft-proposal` command for generating group proposal JSONs (skeleton). * (cli) [#13304](https://github.com/cosmos/cosmos-sdk/pull/13304) Add `tx gov draft-proposal` command for generating proposal JSONs (skeleton). * (x/authz) [#13047](https://github.com/cosmos/cosmos-sdk/pull/13047) Add a GetAuthorization function to the keeper. diff --git a/client/keys/list.go b/client/keys/list.go index 73c2e91c22af..7d47c8896b26 100644 --- a/client/keys/list.go +++ b/client/keys/list.go @@ -33,6 +33,11 @@ func runListCmd(cmd *cobra.Command, _ []string) error { return err } + if len(records) == 0 { + cmd.Println("No records were found in keyring") + return nil + } + if ok, _ := cmd.Flags().GetBool(flagListNames); !ok { return printKeyringRecords(cmd.OutOrStdout(), records, clientCtx.OutputFormat) } diff --git a/client/keys/migrate.go b/client/keys/migrate.go index f76206f6ed9f..0a9fc78e2143 100644 --- a/client/keys/migrate.go +++ b/client/keys/migrate.go @@ -34,7 +34,7 @@ func runMigrateCmd(cmd *cobra.Command, _ []string) error { return err } - if err = clientCtx.Keyring.MigrateAll(); err != nil { + if _, err = clientCtx.Keyring.MigrateAll(); err != nil { return err } diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index 370e0111cec4..d34bfe3c0369 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -120,7 +120,7 @@ type Importer interface { // Migrator is implemented by key stores and enables migration of keys from amino to proto type Migrator interface { - MigrateAll() error + MigrateAll() ([]*Record, error) } // Exporter is implemented by key stores that support export of public and private keys. @@ -492,43 +492,7 @@ func wrapKeyNotFound(err error, msg string) error { } func (ks keystore) List() ([]*Record, error) { - if err := ks.MigrateAll(); err != nil { - return nil, err - } - - keys, err := ks.db.Keys() - if err != nil { - return nil, err - } - - var res []*Record //nolint:prealloc - sort.Strings(keys) - for _, key := range keys { - // Recall that each key is twice in the keyring: - // - once with the `.info` suffix, which holds the key info - // - another time with the `.address` suffix, which only holds a reference to its associated `.info` key - if !strings.HasSuffix(key, infoSuffix) { - continue - } - - item, err := ks.db.Get(key) - if err != nil { - return nil, err - } - - if len(item.Data) == 0 { - return nil, sdkerrors.ErrKeyNotFound.Wrap(key) - } - - k, err := ks.protoUnmarshalRecord(item.Data) - if err != nil { - return nil, err - } - - res = append(res, k) - } - - return res, nil + return ks.MigrateAll() } func (ks keystore) NewMnemonic(uid string, language Language, hdPath, bip39Passphrase string, algo SignatureAlgo) (*Record, string, error) { @@ -870,30 +834,35 @@ func (ks keystore) writeMultisigKey(name string, pk types.PubKey) (*Record, erro return k, ks.writeRecord(k) } -func (ks keystore) MigrateAll() error { +func (ks keystore) MigrateAll() ([]*Record, error) { keys, err := ks.db.Keys() if err != nil { - return err + return nil, err } if len(keys) == 0 { - return nil + return nil, nil } + sort.Strings(keys) + + var recs []*Record for _, key := range keys { // The keyring items only with `.info` consists the key info. if !strings.HasSuffix(key, infoSuffix) { continue } - _, err := ks.migrate(key) + rec, err := ks.migrate(key) if err != nil { fmt.Printf("migrate err for key %s: %q\n", key, err) continue } + + recs = append(recs, rec) } - return nil + return recs, nil } // migrate converts keyring.Item from amino to proto serialization format. diff --git a/crypto/keyring/migration_test.go b/crypto/keyring/migration_test.go index 2435671e4e47..066cd9ceb227 100644 --- a/crypto/keyring/migration_test.go +++ b/crypto/keyring/migration_test.go @@ -191,12 +191,12 @@ func (s *MigrationTestSuite) TestMigrateAllLegacyMultiOffline() { s.Require().NoError(s.ks.SetItem(item)) - err = s.kb.MigrateAll() + _, err = s.kb.MigrateAll() s.Require().NoError(err) } func (s *MigrationTestSuite) TestMigrateAllNoItem() { - err := s.kb.MigrateAll() + _, err := s.kb.MigrateAll() s.Require().NoError(err) }