crypto/keyring: deprecate old keybase (#5889)

Remove the Update method from the Keybase interface.

Remove redundant lazy keybase implementation altogether.

Created LegacyKeybase interface to restrict capabilities to
only those required by keys commands that deal with
legacy keybase such as update and migrate.

Rename keyring.New() -> keyring.NewLegacy().

Rename client/keys.NewKeyBaseFromDir -> NewLegacyKeyBaseFromDir.

crypto/keyiring.NewInMemory() now returns a in-memory keyring.
BackendMemory is added yet not exposed via command line
--keyring-backend flag. keys add uses it when --dry-run flag
is on.

CHANGELOG.md

@@ -51,6 +51,7 @@ that parse log messages.
 * (client) [\#5799](https://github.com/cosmos/cosmos-sdk/pull/5799) The `tx encode/decode` commands, due to change on encoding break compatibility with
 older clients.
 * (x/auth) [\#5844](https://github.com/cosmos/cosmos-sdk/pull/5844) `tx sign` command now returns an error when signing is attempted with offline/multisig keys.
+* (client/keys) [\#5889](https://github.com/cosmos/cosmos-sdk/pull/5889) Remove `keys update` command.
 
 ### API Breaking Changes
 
@@ -77,6 +78,11 @@ 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) Deprecate old keybase implementation:
+  - Remove `Update` from the `Keybase` interface.
+  - `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()

client/keys/root.go

@@ -45,7 +45,6 @@ The pass backend requires GnuPG: https://gnupg.org/
 		ShowKeysCmd(),
 		flags.LineBreak,
 		DeleteKeyCommand(),
-		UpdateKeyCommand(),
 		ParseKeyStringCommand(),
 		MigrateCommand(),
 	)

client/keys/root_test.go

@@ -16,7 +16,7 @@ func TestCommands(t *testing.T) {
 	assert.NotNil(t, rootCommands)
 
 	// Commands are registered
-	assert.Equal(t, 11, len(rootCommands.Commands()))
+	assert.Equal(t, 10, len(rootCommands.Commands()))
 }
 
 func TestMain(m *testing.M) {