Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: key rename cli command #9601

Merged
merged 29 commits into from
Jul 19, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
238dbe2
added key renaming
technicallyty Jun 28, 2021
46762a6
add cli cmds
technicallyty Jun 28, 2021
950752a
changelog
technicallyty Jun 28, 2021
4368e14
gofmt
technicallyty Jun 28, 2021
8283b67
move changelog
technicallyty Jun 28, 2021
9acfb00
fix root_test
technicallyty Jun 29, 2021
16b6632
Apply suggestions from code review
technicallyty Jul 5, 2021
ec01a52
Merge branch 'master' into ty/9407-rename_keys_cli
technicallyty Jul 5, 2021
26ec3a5
add equality function
technicallyty Jul 5, 2021
9a2ba38
Update crypto/keyring/keyring.go
technicallyty Jul 6, 2021
2c7395b
move passphrase to const defs
technicallyty Jul 6, 2021
588c082
refactor test
technicallyty Jul 6, 2021
ed51363
allow duplicate keys as long as names are different
technicallyty Jul 6, 2021
8d464aa
Merge branch 'master' into ty/9407-rename_keys_cli
technicallyty Jul 6, 2021
fc19867
lint
technicallyty Jul 6, 2021
c6aed98
Update CHANGELOG.md
technicallyty Jul 8, 2021
3c1e9b9
Update crypto/keyring/keyring.go
technicallyty Jul 8, 2021
4044320
Merge branch 'master' into ty/9407-rename_keys_cli
technicallyty Jul 9, 2021
e4d2a3f
Merge branch 'master' into ty/9407-rename_keys_cli
mergify[bot] Jul 10, 2021
2d563b2
Merge branch 'master' into ty/9407-rename_keys_cli
mergify[bot] Jul 12, 2021
1a9347c
Merge branch 'master' into ty/9407-rename_keys_cli
mergify[bot] Jul 12, 2021
102169b
Merge branch 'master' into ty/9407-rename_keys_cli
mergify[bot] Jul 12, 2021
e3cca96
Merge branch 'master' into ty/9407-rename_keys_cli
mergify[bot] Jul 12, 2021
3865a9e
Merge branch 'master' into ty/9407-rename_keys_cli
mergify[bot] Jul 13, 2021
aae59f9
Merge branch 'master' into ty/9407-rename_keys_cli
mergify[bot] Jul 13, 2021
e6f8508
Merge branch 'master' into ty/9407-rename_keys_cli
mergify[bot] Jul 13, 2021
e5e35f6
Merge branch 'master' into ty/9407-rename_keys_cli
mergify[bot] Jul 17, 2021
4edd229
Merge branch 'master' into ty/9407-rename_keys_cli
technicallyty Jul 18, 2021
c21bd5f
Merge branch 'master' into ty/9407-rename_keys_cli
amaury1093 Jul 19, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add cli cmds
  • Loading branch information
technicallyty committed Jun 28, 2021
commit 46762a6eba3a4f414a4226548d211677c78091b1
66 changes: 66 additions & 0 deletions client/keys/rename.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
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 <from> <to>",
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
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
}

fromName, toName := args[0], args[1]
technicallyty marked this conversation as resolved.
Show resolved Hide resolved

info, err := clientCtx.Keyring.Key(fromName)
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
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(fromName, toName); err != nil {
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if err := clientCtx.Keyring.Rename(fromName, toName); err != 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", fromName, toName))
technicallyty marked this conversation as resolved.
Show resolved Hide resolved

return nil
},
}

cmd.Flags().BoolP(flagYes, "y", false, "Skip confirmation prompt when renaming offline or ledger key references")

return cmd
}
98 changes: 98 additions & 0 deletions client/keys/rename_test.go
Original file line number Diff line number Diff line change
@@ -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 := "runDeleteCmd_Key1"
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
fakeKeyName2 := "runDeleteCmd_Key2"
technicallyty marked this conversation as resolved.
Show resolved Hide resolved

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")

// no confirmation
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
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())

ogKey, err := kb.Key(fakeKeyName1)
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
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, ogKey.GetPubKey(), renamedKey.GetPubKey())
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
require.Equal(t, ogKey.GetType(), renamedKey.GetType())
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
require.Equal(t, ogKey.GetAddress(), renamedKey.GetAddress())
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
require.Equal(t, ogKey.GetAlgo(), renamedKey.GetAlgo())
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
require.Equal(t, ogKey.GetAlgo(), renamedKey.GetAlgo())
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())
}
2 changes: 1 addition & 1 deletion crypto/keyring/keyring.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ func (ks keystore) DeleteByAddress(address sdk.Address) error {
return nil
}

func (ks keystore) Rename(from string, to string) error {
func (ks keystore) Rename(from, to string) error {
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
_, err := ks.Key(to)
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
if err == nil {
return errors.New(fmt.Sprintf("rename failed: %s already exists in keyring", to))
technicallyty marked this conversation as resolved.
Show resolved Hide resolved
Expand Down