feat!: key rename cli command (#9601)

<!--
The default pull request template is for types feat, fix, or refactor.
For other templates, add one of the following parameters to the url:
- template=docs.md
- template=other.md
-->

## Description
this PR adds a new function to the keyring interface, as well as a CLI command to rename a key in the keyring

ref: #9407

<!-- Add a description of the changes that this PR introduces and the files that
are the most critical to review. -->

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] provided a link to the relevant issue or specification
- [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [x] added a changelog entry to `CHANGELOG.md`
- [x] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [x] updated the relevant documentation or specification
- [x] reviewed "Files changed" and left comments if necessary
- [x] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [x] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] confirmed `!` in the type prefix if API or client breaking change
- [x] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [x] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [x] reviewed tests and test coverage
- [x] manually tested (if applicable)

CHANGELOG.md

@@ -49,6 +49,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * [\#9427](https://github.com/cosmos/cosmos-sdk/pull/9427) Move simapp `FundAccount` and `FundModuleAccount` to `x/bank/testutil`
 * (client/tx) [\#9421](https://github.com/cosmos/cosmos-sdk/pull/9421/) `BuildUnsignedTx`, `BuildSimTx`, `PrintUnsignedStdTx` functions are moved to
   the Tx Factory as methods.
+* (client/keys) [\#9407](https://github.com/cosmos/cosmos-sdk/pull/9601) Added `keys rename` CLI command and `Keyring.Rename` interface method to rename a key in the keyring.
 * (x/slashing) [\#9458](https://github.com/cosmos/cosmos-sdk/pull/9458) Coins burned from slashing is now returned from Slash function and included in Slash event.
 * [\#9246](https://github.com/cosmos/cosmos-sdk/pull/9246) The `New` method for the network package now returns an error.
 * (codec) [\#9521](https://github.com/cosmos/cosmos-sdk/pull/9521) Removed deprecated `clientCtx.JSONCodec` from `client.Context`.

client/keys/rename.go

@@ -0,0 +1,67 @@
+package keys
+
+import (
+	"bufio"
+	"fmt"
+
+	"github.com/cosmos/cosmos-sdk/client"
+	"github.com/cosmos/cosmos-sdk/client/input"
+	"github.com/cosmos/cosmos-sdk/crypto/keyring"
+	"github.com/spf13/cobra"
+)
+
+// RenameKeyCommand renames a key from the key store.
+func RenameKeyCommand() *cobra.Command {
+	cmd := &cobra.Command{
+		Use:   "rename <old_name> <new_name>",
+		Short: "Rename an existing key",
+		Long: `Rename a key from the Keybase backend.
+
+Note that renaming offline or ledger keys will rename
+only the public key references stored locally, i.e.
+private keys stored in a ledger device cannot be renamed with the CLI.
+`,
+		Args: cobra.ExactArgs(2),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			buf := bufio.NewReader(cmd.InOrStdin())
+			clientCtx, err := client.GetClientQueryContext(cmd)
+			if err != nil {
+				return err
+			}
+
+			oldName, newName := args[0], args[1]
+
+			info, err := clientCtx.Keyring.Key(oldName)
+			if err != nil {
+				return err
+			}
+
+			// confirm rename, unless -y is passed
+			if skip, _ := cmd.Flags().GetBool(flagYes); !skip {
+				prompt := fmt.Sprintf("Key reference will be renamed from %s to %s. Continue?", args[0], args[1])
+				if yes, err := input.GetConfirmation(prompt, buf, cmd.ErrOrStderr()); err != nil {
+					return err
+				} else if !yes {
+					return nil
+				}
+			}
+
+			if err := clientCtx.Keyring.Rename(oldName, newName); err != nil {
+				return err
+			}
+
+			if info.GetType() == keyring.TypeLedger || info.GetType() == keyring.TypeOffline {
+				cmd.PrintErrln("Public key reference renamed")
+				return nil
+			}
+
+			cmd.PrintErrln(fmt.Sprintf("Key was successfully renamed from %s to %s", oldName, newName))
+
+			return nil
+		},
+	}
+
+	cmd.Flags().BoolP(flagYes, "y", false, "Skip confirmation prompt when renaming offline or ledger key references")
+
+	return cmd
+}

client/keys/rename_test.go

@@ -0,0 +1,98 @@
+package keys
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/cosmos/cosmos-sdk/client"
+	"github.com/cosmos/cosmos-sdk/client/flags"
+	"github.com/cosmos/cosmos-sdk/crypto/hd"
+	"github.com/cosmos/cosmos-sdk/crypto/keyring"
+	"github.com/cosmos/cosmos-sdk/testutil"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+)
+
+func Test_runRenameCmd(t *testing.T) {
+	// temp keybase
+	kbHome := t.TempDir()
+	cmd := RenameKeyCommand()
+	cmd.Flags().AddFlagSet(Commands(kbHome).PersistentFlags())
+	mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
+
+	yesF, _ := cmd.Flags().GetBool(flagYes)
+	require.False(t, yesF)
+
+	fakeKeyName1 := "runRenameCmd_Key1"
+	fakeKeyName2 := "runRenameCmd_Key2"
+
+	path := sdk.GetConfig().GetFullBIP44Path()
+
+	kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn)
+	require.NoError(t, err)
+
+	// put fakeKeyName1 in keyring
+	_, err = kb.NewAccount(fakeKeyName1, testutil.TestMnemonic, "", path, hd.Secp256k1)
+	require.NoError(t, err)
+
+	clientCtx := client.Context{}.
+		WithKeyringDir(kbHome).
+		WithKeyring(kb)
+
+	ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)
+
+	// rename a key 'blah' which doesnt exist
+	cmd.SetArgs([]string{"blah", "blaah", fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome)})
+	err = cmd.ExecuteContext(ctx)
+	require.Error(t, err)
+	require.EqualError(t, err, "blah.info: key not found")
+
+	// User confirmation missing
+	cmd.SetArgs([]string{
+		fakeKeyName1,
+		"nokey",
+		fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome),
+		fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest),
+	})
+	err = cmd.Execute()
+	require.Error(t, err)
+	require.Equal(t, "EOF", err.Error())
+
+	oldKey, err := kb.Key(fakeKeyName1)
+	require.NoError(t, err)
+
+	// add a confirmation
+	cmd.SetArgs([]string{
+		fakeKeyName1,
+		fakeKeyName2,
+		fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome),
+		fmt.Sprintf("--%s=true", flagYes),
+		fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest),
+	})
+	require.NoError(t, cmd.Execute())
+
+	// key1 is gone
+	_, err = kb.Key(fakeKeyName1)
+	require.Error(t, err)
+
+	// key2 exists now
+	renamedKey, err := kb.Key(fakeKeyName2)
+	require.NoError(t, err)
+
+	require.Equal(t, oldKey.GetPubKey(), renamedKey.GetPubKey())
+	require.Equal(t, oldKey.GetType(), renamedKey.GetType())
+	require.Equal(t, oldKey.GetAddress(), renamedKey.GetAddress())
+	require.Equal(t, oldKey.GetAlgo(), renamedKey.GetAlgo())
+
+	// try to rename key1 but it doesnt exist anymore so error
+	cmd.SetArgs([]string{
+		fakeKeyName1,
+		fakeKeyName2,
+		fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome),
+		fmt.Sprintf("--%s=true", flagYes),
+		fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest),
+	})
+	require.Error(t, cmd.Execute())
+}

client/keys/root.go

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

client/keys/root_test.go

@@ -11,5 +11,5 @@ func TestCommands(t *testing.T) {
 	assert.NotNil(t, rootCommands)
 
 	// Commands are registered
-	assert.Equal(t, 9, len(rootCommands.Commands()))
+	assert.Equal(t, 10, len(rootCommands.Commands()))
 }

crypto/keyring/keyring.go

@@ -41,6 +41,9 @@ const (
 	keyringFileDirName = "keyring-file"
 	keyringTestDirName = "keyring-test"
 	passKeyringPrefix  = "keyring-%s"
+
+	// temporary pass phrase for exporting a key during a key rename
+	passPhrase = "temp"
 )
 
 var (
@@ -64,6 +67,9 @@ type Keyring interface {
 	Delete(uid string) error
 	DeleteByAddress(address sdk.Address) error
 
+	// Rename an existing key from the Keyring
+	Rename(from string, to string) error
+
 	// NewMnemonic generates a new mnemonic, derives a hierarchical deterministic key from it, and
 	// persists the key to storage. Returns the generated mnemonic and the key Info.
 	// It returns an error if it fails to generate a key for the given algo type, or if
@@ -288,8 +294,10 @@ func (ks keystore) ExportPrivKeyArmorByAddress(address sdk.Address, encryptPassp
 }
 
 func (ks keystore) ImportPrivKey(uid, armor, passphrase string) error {
-	if _, err := ks.Key(uid); err == nil {
-		return fmt.Errorf("cannot overwrite key: %s", uid)
+	if k, err := ks.Key(uid); err == nil {
+		if uid == k.GetName() {
+			return fmt.Errorf("cannot overwrite key: %s", uid)
+		}
 	}
 
 	privKey, algo, err := crypto.UnarmorDecryptPrivKey(armor, passphrase)
@@ -426,6 +434,30 @@ func (ks keystore) DeleteByAddress(address sdk.Address) error {
 	return nil
 }
 
+func (ks keystore) Rename(oldName, newName string) error {
+	_, err := ks.Key(newName)
+	if err == nil {
+		return fmt.Errorf("rename failed: %s already exists in the keyring", newName)
+	}
+
+	armor, err := ks.ExportPrivKeyArmor(oldName, passPhrase)
+	if err != nil {
+		return err
+	}
+
+	err = ks.ImportPrivKey(newName, armor, passPhrase)
+	if err != nil {
+		return err
+	}
+
+	err = ks.Delete(oldName)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func (ks keystore) Delete(uid string) error {
 	info, err := ks.Key(uid)
 	if err != nil {
@@ -783,16 +815,22 @@ func (ks keystore) writeInfo(info Info) error {
 }
 
 // existsInDb returns true if key is in DB. Error is returned only when we have error
-// different thant ErrKeyNotFound
+// different than ErrKeyNotFound
 func (ks keystore) existsInDb(info Info) (bool, error) {
-	if _, err := ks.db.Get(addrHexKeyAsString(info.GetAddress())); err == nil {
-		return true, nil // address lookup succeeds - info exists
+	if item, err := ks.db.Get(addrHexKeyAsString(info.GetAddress())); err == nil {
+		if item.Key == info.GetName() {
+			return true, nil // address lookup succeeds - info exists
+		}
+		return false, nil
 	} else if err != keyring.ErrKeyNotFound {
 		return false, err // received unexpected error - returns error
 	}
 
-	if _, err := ks.db.Get(infoKey(info.GetName())); err == nil {
-		return true, nil // uid lookup succeeds - info exists
+	if item, err := ks.db.Get(infoKey(info.GetName())); err == nil {
+		if item.Key == info.GetName() {
+			return true, nil // uid lookup succeeds - info exists
+		}
+		return false, nil
 	} else if err != keyring.ErrKeyNotFound {
 		return false, err // received unexpected error - returns
 	}

crypto/keyring/keyring_test.go

@@ -1153,6 +1153,72 @@ func TestBackendConfigConstructors(t *testing.T) {
 	require.Equal(t, "keyring-test", backend.PassPrefix)
 }
 
+func TestRenameKey(t *testing.T) {
+	testCases := []struct {
+		name string
+		run  func(Keyring)
+	}{
+		{
+			name: "rename a key",
+			run: func(kr Keyring) {
+				oldKeyUID, newKeyUID := "old", "new"
+				oldKeyInfo := newKeyInfo(t, kr, oldKeyUID)
+				err := kr.Rename(oldKeyUID, newKeyUID) // rename from "old" to "new"
+				require.NoError(t, err)
+				newInfo, err := kr.Key(newKeyUID) // new key should be in keyring
+				require.NoError(t, err)
+				requireEqualRenamedKey(t, newInfo, oldKeyInfo) // oldinfo and newinfo should be the same
+				oldKeyInfo, err = kr.Key(oldKeyUID)            // old key should be gone from keyring
+				require.Error(t, err)
+			},
+		},
+		{
+			name: "cant rename a key that doesnt exist",
+			run: func(kr Keyring) {
+				err := kr.Rename("bogus", "bogus2")
+				require.Error(t, err)
+			},
+		},
+		{
+			name: "cant rename a key to an already existing key name",
+			run: func(kr Keyring) {
+				key1, key2 := "existingKey", "existingKey2" // create 2 keys
+				newKeyInfo(t, kr, key1)
+				newKeyInfo(t, kr, key2)
+				err := kr.Rename(key2, key1)
+				require.Equal(t, fmt.Errorf("rename failed: %s already exists in the keyring", key1), err)
+				assertKeysExist(t, kr, key1, key2) // keys should still exist after failed rename
+			},
+		},
+		{
+			name: "cant rename key to itself",
+			run: func(kr Keyring) {
+				keyName := "keyName"
+				newKeyInfo(t, kr, keyName)
+				err := kr.Rename(keyName, keyName)
+				require.Equal(t, fmt.Errorf("rename failed: %s already exists in the keyring", keyName), err)
+				assertKeysExist(t, kr, keyName)
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		kr := newKeyring(t, "testKeyring")
+		t.Run(tc.name, func(t *testing.T) {
+			tc.run(kr)
+		})
+	}
+}
+
+func requireEqualRenamedKey(t *testing.T, newKey, oldKey Info) {
+	require.NotEqual(t, newKey.GetName(), oldKey.GetName())
+	require.Equal(t, newKey.GetAddress(), oldKey.GetAddress())
+	require.Equal(t, newKey.GetPubKey(), oldKey.GetPubKey())
+	require.Equal(t, newKey.GetAlgo(), oldKey.GetAlgo())
+	require.Equal(t, newKey.GetType(), oldKey.GetType())
+}
+
 func requireEqualInfo(t *testing.T, key Info, mnemonic Info) {
 	require.Equal(t, key.GetName(), mnemonic.GetName())
 	require.Equal(t, key.GetAddress(), mnemonic.GetAddress())
@@ -1161,4 +1227,23 @@ func requireEqualInfo(t *testing.T, key Info, mnemonic Info) {
 	require.Equal(t, key.GetType(), mnemonic.GetType())
 }
 
+func newKeyring(t *testing.T, name string) Keyring {
+	kr, err := New(name, "test", t.TempDir(), nil)
+	require.NoError(t, err)
+	return kr
+}
+
+func newKeyInfo(t *testing.T, kr Keyring, name string) Info {
+	keyInfo, _, err := kr.NewMnemonic(name, English, sdk.FullFundraiserPath, DefaultBIP39Passphrase, hd.Secp256k1)
+	require.NoError(t, err)
+	return keyInfo
+}
+
+func assertKeysExist(t *testing.T, kr Keyring, names ...string) {
+	for _, n := range names {
+		_, err := kr.Key(n)
+		require.NoError(t, err)
+	}
+}
+
 func accAddr(info Info) sdk.AccAddress { return info.GetAddress() }