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

Author: raynaudoe

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.

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)
 	}

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
 	}
 

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.

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)
 }