crypto/keyring: deprecate lazy keybase

CHANGELOG.md

@@ -77,6 +77,10 @@ to now accept a `codec.JSONMarshaler` for modular serialization of genesis state
 * (crypto/keyring) [\#5866](https://github.com/cosmos/cosmos-sdk/pull/5866) Move `Keyring` and `Keybase` implementations and their associated types from `crypto/keys/` to `crypto/keyring/`.
 * (crypto) [\#5880](https://github.com/cosmos/cosmos-sdk/pull/5880) Merge `crypto/keys/mintkey` into `crypto`.
 * (crypto/keyring) [\#5858](https://github.com/cosmos/cosmos-sdk/pull/5858) Make Keyring store keys by name and address's hexbytes representation.
+* (crypto/keyring) [\#5889](https://github.com/cosmos/cosmos-sdk/pull/5889) Depreacate old keybase implementation:
+  - `NewKeyring()` now accepts a new backend: `MemoryBackend`.
+  - `New()` has been renamed to`NewLegacy()`, which now returns a `LegacyKeybase` type that only allows migration of keys from the legacy keybase to a new keyring.
+* (client/keys) [\#5889](https://github.com/cosmos/cosmos-sdk/pull/5889) Rename `NewKeyBaseFromDir()` -> `NewLegacyKeyBaseFromDir()`.
 
 ### Features
 

client/keys/add.go

@@ -27,7 +27,6 @@ const (
 	flagInteractive = "interactive"
 	flagRecover     = "recover"
 	flagNoBackup    = "no-backup"
-	flagDryRun      = "dry-run"
 	flagAccount     = "account"
 	flagIndex       = "index"
 	flagMultisig    = "multisig"
@@ -72,7 +71,7 @@ the flag --nosort is set.
 	cmd.Flags().Bool(flags.FlagUseLedger, false, "Store a local reference to a private key on a Ledger device")
 	cmd.Flags().Bool(flagRecover, false, "Provide seed phrase to recover existing key instead of creating")
 	cmd.Flags().Bool(flagNoBackup, false, "Don't print out seed phrase (if others are watching the terminal)")
-	cmd.Flags().Bool(flagDryRun, false, "Perform action, but don't add key to local keystore")
+	cmd.Flags().Bool(flags.FlagDryRun, false, "Perform action, but don't add key to local keystore")
 	cmd.Flags().String(flagHDPath, "", "Manual HD Path derivation (overrides BIP44 config)")
 	cmd.Flags().Uint32(flagAccount, 0, "Account number for HD derivation")
 	cmd.Flags().Uint32(flagIndex, 0, "Address index number for HD derivation")
@@ -83,15 +82,15 @@ the flag --nosort is set.
 
 func getKeybase(transient bool, buf io.Reader) (keyring.Keybase, error) {
 	if transient {
-		return keyring.NewInMemory(), nil
+		return keyring.NewKeyring(sdk.KeyringServiceName(), keyring.BackendMemory, viper.GetString(flags.FlagHome), buf)
 	}
 
 	return keyring.NewKeyring(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), viper.GetString(flags.FlagHome), buf)
 }
 
 func runAddCmd(cmd *cobra.Command, args []string) error {
 	inBuf := bufio.NewReader(cmd.InOrStdin())
-	kb, err := getKeybase(viper.GetBool(flagDryRun), inBuf)
+	kb, err := getKeybase(viper.GetBool(flags.FlagDryRun), inBuf)
 	if err != nil {
 		return err
 	}
@@ -124,7 +123,7 @@ func RunAddCmd(cmd *cobra.Command, args []string, kb keyring.Keybase, inBuf *buf
 		return keyring.ErrUnsupportedSigningAlgo
 	}
 
-	if !viper.GetBool(flagDryRun) {
+	if !viper.GetBool(flags.FlagDryRun) {
 		_, err = kb.Get(name)
 		if err == nil {
 			// account exists, ask for user confirmation

client/keys/add_ledger_test.go

@@ -17,7 +17,6 @@ import (
 )
 
 func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {
-	runningUnattended := isRunningUnattended()
 	config := sdk.GetConfig()
 
 	bech32PrefixAccAddr := "terra"
@@ -58,9 +57,6 @@ func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {
 		kb.Delete("keyname1", "", false)
 	})
 	mockIn.Reset("test1234\n")
-	if runningUnattended {
-		mockIn.Reset("test1234\ntest1234\n")
-	}
 	key1, err := kb.Get("keyname1")
 	require.NoError(t, err)
 	require.NotNil(t, key1)
@@ -79,7 +75,6 @@ func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {
 }
 
 func Test_runAddCmdLedger(t *testing.T) {
-	runningUnattended := isRunningUnattended()
 	cmd := AddKeyCommand()
 	require.NotNil(t, cmd)
 	mockIn, _, _ := tests.ApplyMockIO(cmd)
@@ -105,9 +100,6 @@ func Test_runAddCmdLedger(t *testing.T) {
 		kb.Delete("keyname1", "", false)
 	})
 	mockIn.Reset("test1234\n")
-	if runningUnattended {
-		mockIn.Reset("test1234\ntest1234\n")
-	}
 	key1, err := kb.Get("keyname1")
 	require.NoError(t, err)
 	require.NotNil(t, key1)

client/keys/add_test.go

@@ -16,7 +16,6 @@ import (
 )
 
 func Test_runAddCmdBasic(t *testing.T) {
-	runningUnattended := isRunningUnattended()
 	cmd := AddKeyCommand()
 	assert.NotNil(t, cmd)
 	mockIn, _, _ := tests.ApplyMockIO(cmd)
@@ -27,31 +26,27 @@ func Test_runAddCmdBasic(t *testing.T) {
 	viper.Set(flags.FlagHome, kbHome)
 	viper.Set(cli.OutputFlag, OutputFormatText)
 
-	if runningUnattended {
-		mockIn.Reset("testpass1\ntestpass1\n")
-	} else {
-		mockIn.Reset("y\n")
-		kb, err := keyring.NewKeyring(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), kbHome, mockIn)
-		require.NoError(t, err)
-		t.Cleanup(func() {
-			kb.Delete("keyname1", "", false) // nolint:errcheck
-			kb.Delete("keyname2", "", false) // nolint:errcheck
-		})
-	}
+	mockIn.Reset("y\n")
+	kb, err := keyring.NewKeyring(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), kbHome, mockIn)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		kb.Delete("keyname1", "", false) // nolint:errcheck
+		kb.Delete("keyname2", "", false) // nolint:errcheck
+	})
 	assert.NoError(t, runAddCmd(cmd, []string{"keyname1"}))
 
-	if runningUnattended {
-		mockIn.Reset("testpass1\nN\n")
-	} else {
-		mockIn.Reset("N\n")
-	}
+	mockIn.Reset("N\n")
 	assert.Error(t, runAddCmd(cmd, []string{"keyname1"}))
 
-	if runningUnattended {
-		mockIn.Reset("testpass1\nN\n")
-	} else {
-		mockIn.Reset("y\n")
-	}
-	err := runAddCmd(cmd, []string{"keyname2"})
-	assert.NoError(t, err)
+	assert.NoError(t, runAddCmd(cmd, []string{"keyname2"}))
+	assert.Error(t, runAddCmd(cmd, []string{"keyname2"}))
+	mockIn.Reset("y\n")
+	assert.NoError(t, runAddCmd(cmd, []string{"keyname2"}))
+
+	// test --dry-run
+	assert.NoError(t, runAddCmd(cmd, []string{"keyname4"}))
+	assert.Error(t, runAddCmd(cmd, []string{"keyname4"}))
+
+	viper.Set(flags.FlagDryRun, true)
+	assert.NoError(t, runAddCmd(cmd, []string{"keyname4"}))
 }

client/keys/delete_test.go

@@ -15,7 +15,6 @@ import (
 )
 
 func Test_runDeleteCmd(t *testing.T) {
-	runningUnattended := isRunningUnattended()
 	deleteKeyCommand := DeleteKeyCommand()
 	mockIn, _, _ := tests.ApplyMockIO(deleteKeyCommand)
 
@@ -26,78 +25,51 @@ func Test_runDeleteCmd(t *testing.T) {
 
 	fakeKeyName1 := "runDeleteCmd_Key1"
 	fakeKeyName2 := "runDeleteCmd_Key2"
-	if !runningUnattended {
-		kb, err := keyring.NewKeyring(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), viper.GetString(flags.FlagHome), mockIn)
-		require.NoError(t, err)
-		t.Cleanup(func() {
-			kb.Delete("runDeleteCmd_Key1", "", false) // nolint:errcheck
-			kb.Delete("runDeleteCmd_Key2", "", false) // nolint:errcheck
+	kb, err := keyring.NewKeyring(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), viper.GetString(flags.FlagHome), mockIn)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		kb.Delete("runDeleteCmd_Key1", "", false) // nolint:errcheck
+		kb.Delete("runDeleteCmd_Key2", "", false) // nolint:errcheck
 
-		})
-	}
+	})
 	// Now add a temporary keybase
 	kbHome, cleanUp := tests.NewTestCaseDir(t)
 	t.Cleanup(cleanUp)
 	viper.Set(flags.FlagHome, kbHome)
 
 	// Now
-	kb, err := keyring.NewKeyring(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), kbHome, mockIn)
+	kb, err = keyring.NewKeyring(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), kbHome, mockIn)
 	require.NoError(t, err)
-	if runningUnattended {
-		mockIn.Reset("testpass1\ntestpass1\n")
-	}
 	_, err = kb.CreateAccount(fakeKeyName1, tests.TestMnemonic, "", "", "0", keyring.Secp256k1)
 	require.NoError(t, err)
 
-	if runningUnattended {
-		mockIn.Reset("testpass1\ntestpass1\n")
-	}
 	_, err = kb.CreateAccount(fakeKeyName2, tests.TestMnemonic, "", "", "1", keyring.Secp256k1)
 	require.NoError(t, err)
 
-	if runningUnattended {
-		mockIn.Reset("testpass1\ntestpass1\n")
-	}
 	err = runDeleteCmd(deleteKeyCommand, []string{"blah"})
 	require.Error(t, err)
 	require.Equal(t, "The specified item could not be found in the keyring", err.Error())
 
 	// User confirmation missing
 	err = runDeleteCmd(deleteKeyCommand, []string{fakeKeyName1})
 	require.Error(t, err)
-	if runningUnattended {
-		require.Equal(t, "aborted", err.Error())
-	} else {
-		require.Equal(t, "EOF", err.Error())
-	}
+	require.Equal(t, "EOF", err.Error())
 
 	{
-		if runningUnattended {
-			mockIn.Reset("testpass1\n")
-		}
 		_, err = kb.Get(fakeKeyName1)
 		require.NoError(t, err)
 
 		// Now there is a confirmation
 		viper.Set(flagYes, true)
-		if runningUnattended {
-			mockIn.Reset("testpass1\ntestpass1\n")
-		}
 		require.NoError(t, runDeleteCmd(deleteKeyCommand, []string{fakeKeyName1}))
 
 		_, err = kb.Get(fakeKeyName1)
 		require.Error(t, err) // Key1 is gone
 	}
 
 	viper.Set(flagYes, true)
-	if runningUnattended {
-		mockIn.Reset("testpass1\n")
-	}
 	_, err = kb.Get(fakeKeyName2)
 	require.NoError(t, err)
-	if runningUnattended {
-		mockIn.Reset("testpass1\ny\ntestpass1\n")
-	}
 	err = runDeleteCmd(deleteKeyCommand, []string{fakeKeyName2})
 	require.NoError(t, err)
 	_, err = kb.Get(fakeKeyName2)

client/keys/export_test.go

@@ -13,7 +13,6 @@ import (
 )
 
 func Test_runExportCmd(t *testing.T) {
-	runningUnattended := isRunningUnattended()
 	exportKeyCommand := ExportKeyCommand()
 	mockIn, _, _ := tests.ApplyMockIO(exportKeyCommand)
 
@@ -25,23 +24,14 @@ func Test_runExportCmd(t *testing.T) {
 	// create a key
 	kb, err := keyring.NewKeyring(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), viper.GetString(flags.FlagHome), mockIn)
 	require.NoError(t, err)
-	if !runningUnattended {
-		t.Cleanup(func() {
-			kb.Delete("keyname1", "", false) // nolint:errcheck
-		})
-	}
-
-	if runningUnattended {
-		mockIn.Reset("testpass1\ntestpass1\n")
-	}
+	t.Cleanup(func() {
+		kb.Delete("keyname1", "", false) // nolint:errcheck
+	})
+
 	_, err = kb.CreateAccount("keyname1", tests.TestMnemonic, "", "123456789", "", keyring.Secp256k1)
 	require.NoError(t, err)
 
 	// Now enter password
-	if runningUnattended {
-		mockIn.Reset("123456789\n123456789\ntestpass1\n")
-	} else {
-		mockIn.Reset("123456789\n123456789\n")
-	}
+	mockIn.Reset("123456789\n123456789\n")
 	require.NoError(t, runExportCmd(exportKeyCommand, []string{"keyname1"}))
 }

client/keys/import_test.go

@@ -15,7 +15,6 @@ import (
 )
 
 func Test_runImportCmd(t *testing.T) {
-	runningUnattended := isRunningUnattended()
 	importKeyCommand := ImportKeyCommand()
 	mockIn, _, _ := tests.ApplyMockIO(importKeyCommand)
 
@@ -24,13 +23,11 @@ func Test_runImportCmd(t *testing.T) {
 	t.Cleanup(cleanUp)
 	viper.Set(flags.FlagHome, kbHome)
 
-	if !runningUnattended {
-		kb, err := keyring.NewKeyring(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), viper.GetString(flags.FlagHome), mockIn)
-		require.NoError(t, err)
-		t.Cleanup(func() {
-			kb.Delete("keyname1", "", false) // nolint:errcheck
-		})
-	}
+	kb, err := keyring.NewKeyring(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), viper.GetString(flags.FlagHome), mockIn)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		kb.Delete("keyname1", "", false) // nolint:errcheck
+	})
 
 	keyfile := filepath.Join(kbHome, "key.asc")
 	armoredKey := `-----BEGIN TENDERMINT PRIVATE KEY-----
@@ -45,10 +42,6 @@ HbP+c6JmeJy9JXe2rbbF1QtCX1gLqGcDQPBXiCtFvP7/8wTZtVOPj8vREzhZ9ElO
 	require.NoError(t, ioutil.WriteFile(keyfile, []byte(armoredKey), 0644))
 
 	// Now enter password
-	if runningUnattended {
-		mockIn.Reset("123456789\n12345678\n12345678\n")
-	} else {
-		mockIn.Reset("123456789\n")
-	}
+	mockIn.Reset("123456789\n")
 	require.NoError(t, runImportCmd(importKeyCommand, []string{"keyname1", keyfile}))
 }

client/keys/list_test.go

@@ -14,7 +14,6 @@ import (
 )
 
 func Test_runListCmd(t *testing.T) {
-	runningUnattended := isRunningUnattended()
 	type args struct {
 		cmd  *cobra.Command
 		args []string
@@ -34,9 +33,6 @@ func Test_runListCmd(t *testing.T) {
 	mockIn, _, _ := tests.ApplyMockIO(cmdBasic)
 	kb, err := keyring.NewKeyring(sdk.KeyringServiceName(), viper.GetString(flags.FlagKeyringBackend), viper.GetString(flags.FlagHome), mockIn)
 	require.NoError(t, err)
-	if runningUnattended {
-		mockIn.Reset("testpass1\ntestpass1\n")
-	}
 
 	_, err = kb.CreateAccount("something", tests.TestMnemonic, "", "", "", keyring.Secp256k1)
 	require.NoError(t, err)
@@ -57,18 +53,12 @@ func Test_runListCmd(t *testing.T) {
 	for _, tt := range testData {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			if runningUnattended {
-				mockIn.Reset("testpass1\ntestpass1\n")
-			}
 			viper.Set(flagListNames, false)
 			viper.Set(flags.FlagHome, tt.kbDir)
 			if err := runListCmd(tt.args.cmd, tt.args.args); (err != nil) != tt.wantErr {
 				t.Errorf("runListCmd() error = %v, wantErr %v", err, tt.wantErr)
 			}
 
-			if runningUnattended {
-				mockIn.Reset("testpass1\ntestpass1\n")
-			}
 			viper.Set(flagListNames, true)
 			if err := runListCmd(tt.args.cmd, tt.args.args); (err != nil) != tt.wantErr {
 				t.Errorf("runListCmd() error = %v, wantErr %v", err, tt.wantErr)

client/keys/migrate.go

@@ -43,10 +43,12 @@ It is recommended to run in 'dry-run' mode first to verify all key migration mat
 func runMigrateCmd(cmd *cobra.Command, args []string) error {
 	// instantiate legacy keybase
 	rootDir := viper.GetString(flags.FlagHome)
-	legacyKb, err := NewKeyBaseFromDir(rootDir)
+	var legacyKb keyring.LegacyKeybase
+	legacyKb, err := NewLegacyKeyBaseFromDir(rootDir)
 	if err != nil {
 		return err
 	}
+	defer legacyKb.Close()
 
 	// fetch list of keys from legacy keybase
 	oldKeys, err := legacyKb.List()