Skip to content

Commit

Permalink
perf: reduce user's password prompts when calling keyring List functi…
Browse files Browse the repository at this point in the history
…on (backport cosmos#13207) (cosmos#13368)

* perf: reduce user's password prompts when calling keyring List function (cosmos#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 4882f93)

# Conflicts:
#	CHANGELOG.md

* fix conflicts

* suggestions

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>

Co-authored-by: Ezequiel Raynaudo <raynaudo.ee@gmail.com>
Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
  • Loading branch information
5 people authored Sep 23, 2022
1 parent c351441 commit 95948f6
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 46 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions client/keys/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion client/keys/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
55 changes: 12 additions & 43 deletions crypto/keyring/keyring.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions crypto/keyring/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down

0 comments on commit 95948f6

Please sign in to comment.