crypto/keyring: fix offline keys migration

.github/workflows/test.yml

@@ -201,7 +201,7 @@ jobs:
         if: env.GIT_DIFF
       - name: test & coverage report creation
         run: |
-          cat pkgs.txt.part.${{ matrix.part }} | xargs go test -mod=readonly -json -timeout 30m -race -tags='cgo ledger test_ledger_mock'  > ${{ matrix.part }}-race-output.txt
+          xargs --arg-file=pkgs.txt.part.${{ matrix.part }} go test -mod=readonly -json -timeout 30m -race -tags='cgo ledger test_ledger_mock' | tee ${{ matrix.part }}-race-output.txt
         if: env.GIT_DIFF
       - uses: actions/upload-artifact@v2
         with:

CHANGELOG.md

@@ -78,6 +78,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (x/slashing) [\#8427](https://github.com/cosmos/cosmos-sdk/pull/8427) Fix query signing infos command
 * (server) [\#8399](https://github.com/cosmos/cosmos-sdk/pull/8399) fix gRPC-web flag default value
 * (server) [\#8641](https://github.com/cosmos/cosmos-sdk/pull/8641) Fix Tendermint and application configuration reading from file
+* (client/keys) [\#8639] (https://github.com/cosmos/cosmos-sdk/pull/8639) Fix keys migrate for mulitisig, offline, and ledger keys. The migrate command now takes a positional old_home_dir argument.
 
 ## [v0.41.3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.41.3) - 2021-02-18
 

client/keys/migrate.go

@@ -22,16 +22,18 @@ const migratePassphrase = "NOOP_PASSPHRASE"
 // MigrateCommand migrates key information from legacy keybase to OS secret store.
 func MigrateCommand() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "migrate",
+		Use:   "migrate <old_home_dir>",
 		Short: "Migrate keys from the legacy (db-based) Keybase",
-		Long: `Migrate key information from the legacy (db-based) Keybase to the new keyring-based Keybase.
+		Long: `Migrate key information from the legacy (db-based) Keybase to the new keyring-based Keyring.
+The legacy Keybase used to persist keys in a LevelDB database stored in a 'keys' sub-directory of
+the old client application's home directory, e.g. $HOME/.gaiacli/keys/.
 For each key material entry, the command will prompt if the key should be skipped or not. If the key
 is not to be skipped, the passphrase must be entered. The key will only be migrated if the passphrase
 is correct. Otherwise, the command will exit and migration must be repeated.
 
 It is recommended to run in 'dry-run' mode first to verify all key migration material.
 `,
-		Args: cobra.ExactArgs(0),
+		Args: cobra.ExactArgs(1),
 		RunE: runMigrateCmd,
 	}
 
@@ -44,12 +46,12 @@ func runMigrateCmd(cmd *cobra.Command, args []string) error {
 
 	// instantiate legacy keybase
 	var legacyKb keyring.LegacyKeybase
-	legacyKb, err := NewLegacyKeyBaseFromDir(rootDir)
+	legacyKb, err := NewLegacyKeyBaseFromDir(args[0])
 	if err != nil {
 		return err
 	}
 
-	defer legacyKb.Close()
+	defer func() { _ = legacyKb.Close() }()
 
 	// fetch list of keys from legacy keybase
 	oldKeys, err := legacyKb.List()
@@ -71,7 +73,7 @@ func runMigrateCmd(cmd *cobra.Command, args []string) error {
 			return errors.Wrap(err, "failed to create temporary directory for dryrun migration")
 		}
 
-		defer os.RemoveAll(tmpDir)
+		defer func() { _ = os.RemoveAll(tmpDir) }()
 
 		migrator, err = keyring.New(keyringServiceName, keyring.BackendTest, tmpDir, buf)
 	} else {
@@ -91,11 +93,11 @@ func runMigrateCmd(cmd *cobra.Command, args []string) error {
 		return nil
 	}
 
-	for _, key := range oldKeys {
-		keyName := key.GetName()
-		keyType := key.GetType()
+	for _, oldInfo := range oldKeys {
+		keyName := oldInfo.GetName()
+		keyType := oldInfo.GetType()
 
-		cmd.PrintErrf("Migrating key: '%s (%s)' ...\n", key.GetName(), keyType)
+		cmd.PrintErrf("Migrating key: '%s (%s)' ...\n", keyName, keyType)
 
 		// allow user to skip migrating specific keys
 		ok, err := input.GetConfirmation("Skip key migration?", buf, cmd.ErrOrStderr())
@@ -106,13 +108,15 @@ func runMigrateCmd(cmd *cobra.Command, args []string) error {
 			continue
 		}
 
+		// TypeLocal needs an additional step to ask password.
+		// The other keyring types are handled by ImportInfo.
 		if keyType != keyring.TypeLocal {
-			pubkeyArmor, err := legacyKb.ExportPubKey(keyName)
-			if err != nil {
-				return err
+			infoImporter, ok := migrator.(keyring.InfoImporter)
+			if !ok {
+				return fmt.Errorf("the Keyring implementation does not support import operations of Info types.")
 			}
 
-			if err := migrator.ImportPubKey(keyName, pubkeyArmor); err != nil {
+			if err = infoImporter.ImportInfo(oldInfo); err != nil {
 				return err
 			}
 
@@ -135,8 +139,10 @@ func runMigrateCmd(cmd *cobra.Command, args []string) error {
 		if err := migrator.ImportPrivKey(keyName, armoredPriv, migratePassphrase); err != nil {
 			return err
 		}
+
 	}
-	cmd.Print("Migration Complete")
+
+	cmd.PrintErrln("Migration complete.")
 
 	return err
 }

client/keys/migrate_test.go

@@ -5,44 +5,38 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/cosmos/cosmos-sdk/client"
+
+	"github.com/stretchr/testify/require"
+
 	"github.com/otiai10/copy"
 	"github.com/stretchr/testify/assert"
-	"github.com/tendermint/tendermint/libs/cli"
 
-	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/crypto/keyring"
 	"github.com/cosmos/cosmos-sdk/testutil"
 )
 
 func Test_runMigrateCmd(t *testing.T) {
-	cmd := AddKeyCommand()
-	_ = testutil.ApplyMockIODiscardOutErr(cmd)
-	cmd.Flags().AddFlagSet(Commands("home").PersistentFlags())
-
 	kbHome := t.TempDir()
-
 	clientCtx := client.Context{}.WithKeyringDir(kbHome)
 	ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)
 
-	copy.Copy("testdata", kbHome)
-	cmd.SetArgs([]string{
-		"keyname1",
-		fmt.Sprintf("--%s=%s", cli.OutputFlag, OutputFormatText),
-		fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest),
-	})
-	assert.NoError(t, cmd.ExecuteContext(ctx))
+	require.NoError(t, copy.Copy("testdata", kbHome))
 
-	cmd = MigrateCommand()
+	cmd := MigrateCommand()
 	cmd.Flags().AddFlagSet(Commands("home").PersistentFlags())
-	mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
+	//mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
+	mockIn, mockOut := testutil.ApplyMockIO(cmd)
 
 	cmd.SetArgs([]string{
-		fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome),
+		kbHome,
+		//fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome),
 		fmt.Sprintf("--%s=true", flags.FlagDryRun),
 		fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest),
 	})
 
-	mockIn.Reset("test1234\ntest1234\n")
+	mockIn.Reset("\n12345678\n\n\n\n\n")
+	t.Log(mockOut.String())
 	assert.NoError(t, cmd.ExecuteContext(ctx))
 }

client/keys/testdata/keys/keys.db/CURRENT

@@ -1 +1 @@
-MANIFEST-000005
+MANIFEST-000167

client/keys/testdata/keys/keys.db/CURRENT.bak

@@ -1 +1 @@
-MANIFEST-000003
+MANIFEST-000165