crypto/keyring: fix offline keys migration (#8639)

Fix `keys migrate` command (#8703)

crypto/keyring: reinstate the InfoImporter interface

InfoImporter is implemented by those Keyring implementations
that support import of Info objects.

Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: Jonathan Gimeno <jgimeno@gmail.com>
Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>

.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:

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,9 @@ 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