From 4882f933b1a108d578343573595fb06d7fd319c2 Mon Sep 17 00:00:00 2001 From: Ezequiel Raynaudo Date: Fri, 9 Sep 2022 08:38:10 -0300 Subject: [PATCH] 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 --- CHANGELOG.md | 1 + client/keys/list.go | 5 +++ client/keys/migrate.go | 2 +- crypto/keyring/keyring.go | 54 +++++++------------------------- crypto/keyring/migration_test.go | 4 +-- 5 files changed, 20 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e6017add61b..90c39d1075ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,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 * (x/authz) [#12648](https://github.com/cosmos/cosmos-sdk/pull/12648) Add an allow list, an optional list of addresses allowed to receive bank assets via authz MsgSend grant. * (sdk.Coins) [#12627](https://github.com/cosmos/cosmos-sdk/pull/12627) Make a Denoms method on sdk.Coins. * (testutil) [#12973](https://github.com/cosmos/cosmos-sdk/pull/12973) Add generic `testutil.RandSliceElem` function which selects a random element from the list. 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 fc3f004139a1..b15ae1de50e7 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. @@ -517,43 +517,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) { @@ -895,30 +859,34 @@ 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) }