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

crypto/keyring: deprecate lazy keybase #5889

Merged
merged 29 commits into from
Mar 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
9 changes: 4 additions & 5 deletions client/keys/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ const (
flagInteractive = "interactive"
flagRecover = "recover"
flagNoBackup = "no-backup"
flagDryRun = "dry-run"
flagAccount = "account"
flagIndex = "index"
flagMultisig = "multisig"
Expand Down Expand Up @@ -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")
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions client/keys/add_ledger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
)

func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {
runningUnattended := isRunningUnattended()
config := sdk.GetConfig()

bech32PrefixAccAddr := "terra"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
43 changes: 19 additions & 24 deletions client/keys/add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
)

func Test_runAddCmdBasic(t *testing.T) {
runningUnattended := isRunningUnattended()
cmd := AddKeyCommand()
assert.NotNil(t, cmd)
mockIn, _, _ := tests.ApplyMockIO(cmd)
Expand All @@ -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"}))
}
44 changes: 8 additions & 36 deletions client/keys/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
)

func Test_runDeleteCmd(t *testing.T) {
runningUnattended := isRunningUnattended()
deleteKeyCommand := DeleteKeyCommand()
mockIn, _, _ := tests.ApplyMockIO(deleteKeyCommand)

Expand All @@ -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)
Expand Down
20 changes: 5 additions & 15 deletions client/keys/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
)

func Test_runExportCmd(t *testing.T) {
runningUnattended := isRunningUnattended()
exportKeyCommand := ExportKeyCommand()
mockIn, _, _ := tests.ApplyMockIO(exportKeyCommand)

Expand All @@ -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"}))
}
19 changes: 6 additions & 13 deletions client/keys/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
)

func Test_runImportCmd(t *testing.T) {
runningUnattended := isRunningUnattended()
importKeyCommand := ImportKeyCommand()
mockIn, _, _ := tests.ApplyMockIO(importKeyCommand)

Expand All @@ -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-----
Expand All @@ -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}))
}
10 changes: 0 additions & 10 deletions client/keys/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
)

func Test_runListCmd(t *testing.T) {
runningUnattended := isRunningUnattended()
type args struct {
cmd *cobra.Command
args []string
Expand All @@ -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)
Expand All @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion client/keys/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
1 change: 0 additions & 1 deletion client/keys/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ The pass backend requires GnuPG: https://gnupg.org/
ShowKeysCmd(),
flags.LineBreak,
DeleteKeyCommand(),
UpdateKeyCommand(),
ParseKeyStringCommand(),
MigrateCommand(),
)
Expand Down
2 changes: 1 addition & 1 deletion client/keys/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading