From 565685476edc22a5f253f82fdc040f5bec615184 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Thu, 7 Oct 2021 16:59:08 +0530 Subject: [PATCH 01/14] migrate key names correctly --- client/keys/show.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/client/keys/show.go b/client/keys/show.go index d97b4a67d842..ab22987dad26 100644 --- a/client/keys/show.go +++ b/client/keys/show.go @@ -3,6 +3,7 @@ package keys import ( "errors" "fmt" + "strings" "github.com/spf13/cobra" "github.com/tendermint/tendermint/libs/cli" @@ -163,6 +164,9 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) { } func fetchKey(kb keyring.Keyring, keyref string) (*keyring.Record, error) { + if !strings.HasSuffix(keyref, ".info") { + keyref = keyref + ".info" + } // firstly check if the keyref is a key name of a key registered in a keyring. k, err := kb.Key(keyref) // if the key is not there or if we have a problem with a keyring itself then we move to a From 7e9400e4d618db0a04b89b47a4d0ee4e15e66e51 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 8 Oct 2021 15:01:39 +0530 Subject: [PATCH 02/14] fix keynames migration --- client/keys/show.go | 4 ---- crypto/keyring/keyring.go | 15 ++++++++++++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/client/keys/show.go b/client/keys/show.go index ab22987dad26..d97b4a67d842 100644 --- a/client/keys/show.go +++ b/client/keys/show.go @@ -3,7 +3,6 @@ package keys import ( "errors" "fmt" - "strings" "github.com/spf13/cobra" "github.com/tendermint/tendermint/libs/cli" @@ -164,9 +163,6 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) { } func fetchKey(kb keyring.Keyring, keyref string) (*keyring.Record, error) { - if !strings.HasSuffix(keyref, ".info") { - keyref = keyref + ".info" - } // firstly check if the keyref is a key name of a key registered in a keyring. k, err := kb.Key(keyref) // if the key is not there or if we have a problem with a keyring itself then we move to a diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index 9939ac4380ac..00b34417dfd6 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -12,7 +12,6 @@ import ( "strings" "github.com/99designs/keyring" - "github.com/cosmos/go-bip39" "github.com/pkg/errors" "github.com/tendermint/crypto/bcrypt" tmcrypto "github.com/tendermint/tendermint/crypto" @@ -25,6 +24,7 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/go-bip39" ) // Backend options for Keyring @@ -878,6 +878,19 @@ func (ks keystore) MigrateAll() (bool, error) { // migrate converts keyring.Item from amino to proto serialization format. func (ks keystore) migrate(key string) (*Record, bool, error) { + keys, err := ks.db.Keys() + if err != nil { + return nil, false, err + } + for i := 0; i < len(keys); i++ { + if strings.Contains(keys[i], key) { + if !(strings.HasSuffix(key, ".info")) { + key = key + ".info" + break + } + } + } + item, err := ks.db.Get(key) if err != nil { return nil, false, wrapKeyNotFound(err, key) From 603d3a3d50297e6cefde6912c184ecf052d887c7 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 8 Oct 2021 15:30:34 +0530 Subject: [PATCH 03/14] fix something --- crypto/keyring/keyring.go | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index 00b34417dfd6..5700bf322381 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -878,22 +878,12 @@ func (ks keystore) MigrateAll() (bool, error) { // migrate converts keyring.Item from amino to proto serialization format. func (ks keystore) migrate(key string) (*Record, bool, error) { - keys, err := ks.db.Keys() - if err != nil { - return nil, false, err - } - for i := 0; i < len(keys); i++ { - if strings.Contains(keys[i], key) { - if !(strings.HasSuffix(key, ".info")) { - key = key + ".info" - break - } - } - } - item, err := ks.db.Get(key) if err != nil { - return nil, false, wrapKeyNotFound(err, key) + item, err = ks.db.Get(key + ".info") + if err != nil { + return nil, false, wrapKeyNotFound(err, key) + } } if len(item.Data) == 0 { From 0ff4ffacc3efba2c36084cbd9ac4512fd122f1b7 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Fri, 8 Oct 2021 17:13:46 +0530 Subject: [PATCH 04/14] small fix --- crypto/keyring/keyring.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index 5700bf322381..612b13e8889e 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -880,7 +880,7 @@ func (ks keystore) MigrateAll() (bool, error) { func (ks keystore) migrate(key string) (*Record, bool, error) { item, err := ks.db.Get(key) if err != nil { - item, err = ks.db.Get(key + ".info") + item, err = ks.db.Get(fmt.Sprint(key, ".info")) if err != nil { return nil, false, wrapKeyNotFound(err, key) } From 3c3cc747b5a17a469e60b263ddbd705906484799 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Tue, 12 Oct 2021 13:08:10 +0530 Subject: [PATCH 05/14] migrate key names correctly --- crypto/keyring/keyring.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index 612b13e8889e..cdcd2183e4fd 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -787,7 +787,7 @@ func (ks keystore) writeRecord(k *Record) error { } item := keyring.Item{ - Key: key, + Key: fmt.Sprint(key, ".info"), Data: serializedRecord, } @@ -878,12 +878,12 @@ func (ks keystore) MigrateAll() (bool, error) { // migrate converts keyring.Item from amino to proto serialization format. func (ks keystore) migrate(key string) (*Record, bool, error) { + if !(strings.HasPrefix(key, "cosmos")) && !(strings.HasSuffix(key, ".info")) { + key = fmt.Sprint(key, ".info") + } item, err := ks.db.Get(key) if err != nil { - item, err = ks.db.Get(fmt.Sprint(key, ".info")) - if err != nil { - return nil, false, wrapKeyNotFound(err, key) - } + return nil, false, wrapKeyNotFound(err, key) } if len(item.Data) == 0 { @@ -913,7 +913,7 @@ func (ks keystore) migrate(key string) (*Record, bool, error) { } item = keyring.Item{ - Key: key, + Key: fmt.Sprint(key, ".info"), Data: serializedRecord, Description: "SDK kerying version", } From 9b10a094e3c3491ce5e306ff2404a96e5ccd483b Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 13 Oct 2021 12:46:57 +0530 Subject: [PATCH 06/14] address review comments --- client/keys/add_test.go | 15 +++++++-------- client/keys/delete.go | 8 ++++++-- client/keys/delete_test.go | 10 +++++----- client/keys/export.go | 5 +++++ client/keys/import.go | 6 ++++++ client/keys/rename.go | 7 ++++++- client/keys/rename_test.go | 8 ++++---- client/keys/show.go | 7 +++++++ client/keys/show_test.go | 4 ++-- crypto/keyring/keyring.go | 11 ++++------- crypto/keyring/legacy_info.go | 2 ++ crypto/keyring/types.go | 1 + 12 files changed, 55 insertions(+), 29 deletions(-) diff --git a/client/keys/add_test.go b/client/keys/add_test.go index 1db737991114..92c85c4e8376 100644 --- a/client/keys/add_test.go +++ b/client/keys/add_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/stretchr/testify/require" - "github.com/tendermint/tendermint/libs/cli" "github.com/cosmos/cosmos-sdk/client" @@ -65,8 +64,8 @@ func Test_runAddCmdBasic(t *testing.T) { require.NoError(t, cmd.ExecuteContext(ctx)) require.Error(t, cmd.ExecuteContext(ctx)) - mockIn.Reset("y\n") - require.NoError(t, cmd.ExecuteContext(ctx)) + mockIn.Reset("N\n") + require.Error(t, cmd.ExecuteContext(ctx)) cmd.SetArgs([]string{ "keyname4", @@ -152,7 +151,7 @@ func Test_runAddCmdDryRun(t *testing.T) { args: []string{ "testkey", fmt.Sprintf("--%s=%s", flags.FlagDryRun, "false"), - fmt.Sprintf("--%s=%s", flagMultisig, "subkey"), + fmt.Sprintf("--%s=%s", flagMultisig, string(keyring.InfoKey("subkey"))), }, added: true, }, @@ -217,16 +216,16 @@ func Test_runAddCmdDryRun(t *testing.T) { require.NoError(t, cmd.ExecuteContext(ctx)) if tt.added { - _, err := kb.Key("testkey") + _, err := kb.Key(string(keyring.InfoKey("testkey"))) require.NoError(t, err) out, err := ioutil.ReadAll(b) require.NoError(t, err) require.Contains(t, string(out), "name: testkey") } else { - _, err = kb.Key("testkey") + _, err = kb.Key(string(keyring.InfoKey("testkey"))) require.Error(t, err) - require.Equal(t, "testkey: key not found", err.Error()) + require.Equal(t, "testkey.info: key not found", err.Error()) } }) } @@ -272,7 +271,7 @@ func TestAddRecoverFileBackend(t *testing.T) { }) mockIn.Reset(fmt.Sprintf("%s\n%s\n", keyringPassword, keyringPassword)) - k, err := kb.Key("keyname1") + k, err := kb.Key(string(keyring.InfoKey("keyname1"))) require.NoError(t, err) require.Equal(t, "keyname1", k.Name) } diff --git a/client/keys/delete.go b/client/keys/delete.go index 939ede062303..95a2a5ec344f 100644 --- a/client/keys/delete.go +++ b/client/keys/delete.go @@ -2,12 +2,13 @@ package keys import ( "bufio" + "strings" + + "github.com/spf13/cobra" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/input" "github.com/cosmos/cosmos-sdk/crypto/keyring" - - "github.com/spf13/cobra" ) const ( @@ -35,6 +36,9 @@ private keys stored in a ledger device cannot be deleted with the CLI. } for _, name := range args { + if !strings.HasSuffix(name, keyring.InfoSuffix) { + name = string(keyring.InfoKey(name)) + } k, err := clientCtx.Keyring.Key(name) if err != nil { return err diff --git a/client/keys/delete_test.go b/client/keys/delete_test.go index 2250eac5ce9c..b313d8e21d8a 100644 --- a/client/keys/delete_test.go +++ b/client/keys/delete_test.go @@ -53,7 +53,7 @@ func Test_runDeleteCmd(t *testing.T) { err = cmd.ExecuteContext(ctx) require.Error(t, err) - require.EqualError(t, err, "blah: key not found") + require.EqualError(t, err, "blah.info: key not found") // User confirmation missing cmd.SetArgs([]string{ @@ -65,7 +65,7 @@ func Test_runDeleteCmd(t *testing.T) { require.Error(t, err) require.Equal(t, "EOF", err.Error()) - _, err = kb.Key(fakeKeyName1) + _, err = kb.Key(string(keyring.InfoKey(fakeKeyName1))) require.NoError(t, err) // Now there is a confirmation @@ -77,10 +77,10 @@ func Test_runDeleteCmd(t *testing.T) { }) require.NoError(t, cmd.Execute()) - _, err = kb.Key(fakeKeyName1) + _, err = kb.Key(string(keyring.InfoKey(fakeKeyName1))) require.Error(t, err) // Key1 is gone - _, err = kb.Key(fakeKeyName2) + _, err = kb.Key(string(keyring.InfoKey(fakeKeyName2))) require.NoError(t, err) cmd.SetArgs([]string{ @@ -91,6 +91,6 @@ func Test_runDeleteCmd(t *testing.T) { }) require.NoError(t, cmd.Execute()) - _, err = kb.Key(fakeKeyName2) + _, err = kb.Key(string(keyring.InfoKey(fakeKeyName2))) require.Error(t, err) // Key2 is gone } diff --git a/client/keys/export.go b/client/keys/export.go index 13491b5e839a..b0094e553d09 100644 --- a/client/keys/export.go +++ b/client/keys/export.go @@ -3,6 +3,7 @@ package keys import ( "bufio" "fmt" + "strings" "github.com/spf13/cobra" @@ -39,6 +40,10 @@ and export your keys in ASCII-armored encrypted format.`, unarmored, _ := cmd.Flags().GetBool(flagUnarmoredHex) unsafe, _ := cmd.Flags().GetBool(flagUnsafe) + if !strings.HasSuffix(args[0], keyring.InfoSuffix) { + args[0] = string(keyring.InfoKey(args[0])) + } + if unarmored && unsafe { return exportUnsafeUnarmored(cmd, args[0], buf, clientCtx.Keyring) } else if unarmored || unsafe { diff --git a/client/keys/import.go b/client/keys/import.go index 36662a8dd2e6..f164a9bf0fe9 100644 --- a/client/keys/import.go +++ b/client/keys/import.go @@ -3,11 +3,13 @@ package keys import ( "bufio" "io/ioutil" + "strings" "github.com/spf13/cobra" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/input" + "github.com/cosmos/cosmos-sdk/crypto/keyring" ) // ImportKeyCommand imports private keys from a keyfile. @@ -34,6 +36,10 @@ func ImportKeyCommand() *cobra.Command { return err } + if !strings.HasSuffix(args[0], keyring.InfoSuffix) { + args[0] = string(keyring.InfoKey(args[0])) + } + return clientCtx.Keyring.ImportPrivKey(args[0], string(bz), passphrase) }, } diff --git a/client/keys/rename.go b/client/keys/rename.go index 18326c8d356a..3ccb038a7540 100644 --- a/client/keys/rename.go +++ b/client/keys/rename.go @@ -3,11 +3,13 @@ package keys import ( "bufio" "fmt" + "strings" + + "github.com/spf13/cobra" "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. @@ -30,6 +32,9 @@ private keys stored in a ledger device cannot be renamed with the CLI. } oldName, newName := args[0], args[1] + if !strings.HasSuffix(oldName, keyring.InfoSuffix) { + oldName = string(keyring.InfoKey(oldName)) + } k, err := clientCtx.Keyring.Key(oldName) if err != nil { diff --git a/client/keys/rename_test.go b/client/keys/rename_test.go index cf2e859648db..559f3b9d39e4 100644 --- a/client/keys/rename_test.go +++ b/client/keys/rename_test.go @@ -49,7 +49,7 @@ func Test_runRenameCmd(t *testing.T) { 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: key not found") + require.EqualError(t, err, "blah.info: key not found") // User confirmation missing cmd.SetArgs([]string{ @@ -62,7 +62,7 @@ func Test_runRenameCmd(t *testing.T) { require.Error(t, err) require.Equal(t, "EOF", err.Error()) - oldKey, err := kb.Key(fakeKeyName1) + oldKey, err := kb.Key(string(keyring.InfoKey(fakeKeyName1))) require.NoError(t, err) // add a confirmation @@ -76,11 +76,11 @@ func Test_runRenameCmd(t *testing.T) { require.NoError(t, cmd.Execute()) // key1 is gone - _, err = kb.Key(fakeKeyName1) + _, err = kb.Key(string(keyring.InfoKey(fakeKeyName1))) require.Error(t, err) // key2 exists now - renamedKey, err := kb.Key(fakeKeyName2) + renamedKey, err := kb.Key(string(keyring.InfoKey(fakeKeyName2))) require.NoError(t, err) oldPk, err := oldKey.GetPubKey() require.NoError(t, err) diff --git a/client/keys/show.go b/client/keys/show.go index d97b4a67d842..645ebcb0061e 100644 --- a/client/keys/show.go +++ b/client/keys/show.go @@ -3,6 +3,7 @@ package keys import ( "errors" "fmt" + "strings" "github.com/spf13/cobra" "github.com/tendermint/tendermint/libs/cli" @@ -163,6 +164,12 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) { } func fetchKey(kb keyring.Keyring, keyref string) (*keyring.Record, error) { + var key []byte + if !strings.HasPrefix(keyref, "cosmos") && !strings.HasSuffix(keyref, keyring.InfoSuffix) { + key = keyring.InfoKey(keyref) + keyref = string(key) + } + // firstly check if the keyref is a key name of a key registered in a keyring. k, err := kb.Key(keyref) // if the key is not there or if we have a problem with a keyring itself then we move to a diff --git a/client/keys/show_test.go b/client/keys/show_test.go index af3284221cba..84ff73772ae1 100644 --- a/client/keys/show_test.go +++ b/client/keys/show_test.go @@ -62,7 +62,7 @@ func Test_runShowCmd(t *testing.T) { ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx) cmd.SetArgs([]string{"invalid"}) - require.EqualError(t, cmd.ExecuteContext(ctx), "invalid is not a valid name or address: decoding bech32 failed: invalid bech32 string length 7") + require.EqualError(t, cmd.ExecuteContext(ctx), "invalid is not a valid name or address: decoding bech32 failed: invalid separator index -1") cmd.SetArgs([]string{"invalid1", "invalid2"}) require.EqualError(t, cmd.ExecuteContext(ctx), "invalid1 is not a valid name or address: decoding bech32 failed: invalid separator index 7") @@ -103,7 +103,7 @@ func Test_runShowCmd(t *testing.T) { require.NoError(t, cmd.ExecuteContext(ctx)) // try fetch by addr - k, err := kb.Key(fakeKeyName1) + k, err := kb.Key(string(keyring.InfoKey(fakeKeyName1))) require.NoError(t, err) addr, err := k.GetAddress() require.NoError(t, err) diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index cdcd2183e4fd..30d2604d8574 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -771,9 +771,9 @@ func (ks keystore) writeRecord(k *Record) error { return err } - key := k.Name + key := InfoKey(k.Name) - exists, err := ks.existsInDb(addr, key) + exists, err := ks.existsInDb(addr, string(key)) if err != nil { return err } @@ -787,7 +787,7 @@ func (ks keystore) writeRecord(k *Record) error { } item := keyring.Item{ - Key: fmt.Sprint(key, ".info"), + Key: string(key), Data: serializedRecord, } @@ -878,9 +878,6 @@ func (ks keystore) MigrateAll() (bool, error) { // migrate converts keyring.Item from amino to proto serialization format. func (ks keystore) migrate(key string) (*Record, bool, error) { - if !(strings.HasPrefix(key, "cosmos")) && !(strings.HasSuffix(key, ".info")) { - key = fmt.Sprint(key, ".info") - } item, err := ks.db.Get(key) if err != nil { return nil, false, wrapKeyNotFound(err, key) @@ -913,7 +910,7 @@ func (ks keystore) migrate(key string) (*Record, bool, error) { } item = keyring.Item{ - Key: fmt.Sprint(key, ".info"), + Key: key, Data: serializedRecord, Description: "SDK kerying version", } diff --git a/crypto/keyring/legacy_info.go b/crypto/keyring/legacy_info.go index 0480fa5eb262..e178aabe2991 100644 --- a/crypto/keyring/legacy_info.go +++ b/crypto/keyring/legacy_info.go @@ -43,6 +43,8 @@ type legacyLocalInfo struct { Algo hd.PubKeyType `json:"algo"` } +func InfoKey(name string) []byte { return []byte(fmt.Sprintf("%s.%s", name, InfoSuffix)) } + // GetType implements Info interface func (i legacyLocalInfo) GetType() KeyType { return TypeLocal diff --git a/crypto/keyring/types.go b/crypto/keyring/types.go index 307eb9b3f097..447d8d750844 100644 --- a/crypto/keyring/types.go +++ b/crypto/keyring/types.go @@ -37,6 +37,7 @@ const ( // bits of entropy to draw when creating a mnemonic defaultEntropySize = 256 addressSuffix = "address" + InfoSuffix = "info" ) // KeyType reflects a human-readable type for key listing. From 59364646f35d025d98d5fe4813c06a113fbcea0a Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 13 Oct 2021 17:10:32 +0530 Subject: [PATCH 07/14] fix failing tests --- crypto/keyring/keyring.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index 3d8b42893282..a99d6e1f03d3 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -877,6 +877,9 @@ func (ks keystore) MigrateAll() (bool, error) { // migrate converts keyring.Item from amino to proto serialization format. func (ks keystore) migrate(key string) (*Record, bool, error) { + if !(strings.HasSuffix(key, "info")) && !(strings.HasPrefix(key, "cosmos")) { + key = string(InfoKey(key)) + } item, err := ks.db.Get(key) if err != nil { return nil, false, wrapKeyNotFound(err, key) From 73ba70bde2be409d0ba4729ad762ebd672f06f6e Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 13 Oct 2021 17:43:03 +0530 Subject: [PATCH 08/14] fix failing tests --- crypto/keyring/keyring.go | 2 +- crypto/keyring/keyring_test.go | 42 +++++++++++++++++----------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index a99d6e1f03d3..5f1bdd2427bd 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -410,7 +410,7 @@ func (ks keystore) DeleteByAddress(address sdk.Address) error { return err } - err = ks.Delete(k.Name) + err = ks.Delete(string(InfoKey(k.Name))) if err != nil { return err } diff --git a/crypto/keyring/keyring_test.go b/crypto/keyring/keyring_test.go index 6fc968240b21..ed1860deed76 100644 --- a/crypto/keyring/keyring_test.go +++ b/crypto/keyring/keyring_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/99designs/keyring" - bip39 "github.com/cosmos/go-bip39" "github.com/stretchr/testify/require" "github.com/cosmos/cosmos-sdk/codec" @@ -20,6 +19,7 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" + bip39 "github.com/cosmos/go-bip39" ) const ( @@ -119,7 +119,7 @@ func TestKeyManagementKeyRing(t *testing.T) { // deleting a key removes it err = kb.Delete("bad name") require.NotNil(t, err) - err = kb.Delete(n1) + err = kb.Delete(string(InfoKey(n1))) require.NoError(t, err) keyS, err = kb.List() require.NoError(t, err) @@ -145,14 +145,14 @@ func TestKeyManagementKeyRing(t *testing.T) { require.Equal(t, 2, len(keyS)) // delete the offline key - err = kb.Delete(o1) + err = kb.Delete(string(InfoKey(o1))) require.NoError(t, err) keyS, err = kb.List() require.NoError(t, err) require.Equal(t, 1, len(keyS)) // addr cache gets nuked - and test skip flag - require.NoError(t, kb.Delete(n2)) + require.NoError(t, kb.Delete(string(InfoKey(n2)))) } func TestSignVerifyKeyRing(t *testing.T) { @@ -230,7 +230,7 @@ func TestSignVerifyKeyRing(t *testing.T) { // Import a public key armor, err := kb.ExportPubKeyArmor(n2) require.NoError(t, err) - require.NoError(t, kb.Delete(n2)) + require.NoError(t, kb.Delete(string(InfoKey(n2)))) require.NoError(t, kb.ImportPubKey(n3, armor)) i3, err := kb.Key(n3) @@ -261,7 +261,7 @@ func TestExportImportKeyRing(t *testing.T) { armor, err := kb.ExportPrivKeyArmor("john", "apassphrase") require.NoError(t, err) - err = kb.Delete("john") + err = kb.Delete(string(InfoKey("john"))) require.NoError(t, err) err = kb.ImportPrivKey("john2", armor, "apassphrase") @@ -312,7 +312,7 @@ func TestExportImportPubKeyKeyRing(t *testing.T) { // Export the public key only armor, err := kb.ExportPubKeyArmor("john") require.NoError(t, err) - err = kb.Delete("john") + err = kb.Delete(string(InfoKey("john"))) require.NoError(t, err) // Import it under a different name @@ -357,7 +357,7 @@ func TestAdvancedKeyManagementKeyRing(t *testing.T) { require.NotNil(t, err) exported, err := kb.ExportPubKeyArmor(n1) require.Nil(t, err, "%+v", err) - err = kb.Delete(n1) + err = kb.Delete(string(InfoKey(n1))) require.NoError(t, err) // import succeeds @@ -388,7 +388,7 @@ func TestSeedPhraseKeyRing(t *testing.T) { require.NoError(t, err) // now, let us delete this key - err = kb.Delete(n1) + err = kb.Delete(string(InfoKey(n1))) require.Nil(t, err, "%+v", err) _, err = kb.Key(n1) require.NotNil(t, err) @@ -416,7 +416,7 @@ func TestKeyringKeybaseExportImportPrivKey(t *testing.T) { keystr, err := kb.ExportPrivKeyArmor("john", "somepassword") require.NoError(t, err) require.NotEmpty(t, keystr) - err = kb.Delete("john") + err = kb.Delete(string(InfoKey("john"))) require.NoError(t, err) // try import the key - wrong password @@ -432,7 +432,7 @@ func TestKeyringKeybaseExportImportPrivKey(t *testing.T) { // try export non existing key _, err = kb.ExportPrivKeyArmor("john3", "wrongpassword") - require.EqualError(t, err, "john3: key not found") + require.EqualError(t, err, "john3.info: key not found") } func TestInMemoryLanguage(t *testing.T) { @@ -524,7 +524,7 @@ func TestInMemoryKeyManagement(t *testing.T) { require.True(t, key1.Equals(key2)) // deleting a key removes it - err = cstore.Delete("bad name") + err = cstore.Delete(string(InfoKey("bad name"))) require.NotNil(t, err) err = cstore.Delete(n1) require.NoError(t, err) @@ -702,7 +702,7 @@ func TestInMemoryExportImportPrivKey(t *testing.T) { require.NoError(t, err) // delete exported key - require.NoError(t, kb.Delete("john")) + require.NoError(t, kb.Delete(string(InfoKey("john")))) _, err = kb.Key("john") require.Error(t, err) @@ -813,7 +813,7 @@ func TestInMemorySeedPhrase(t *testing.T) { require.NotEmpty(t, mnemonic) // now, let us delete this key - err = cstore.Delete(n1) + err = cstore.Delete(string(InfoKey(n1))) require.Nil(t, err, "%+v", err) _, err = cstore.Key(n1) require.NotNil(t, err) @@ -840,7 +840,7 @@ func TestKeyChain_ShouldFailWhenAddingSameGeneratedAccount(t *testing.T) { _, seed, err := kr.NewMnemonic("test", English, "", DefaultBIP39Passphrase, hd.Secp256k1) require.NoError(t, err) - require.NoError(t, kr.Delete("test")) + require.NoError(t, kr.Delete(string(InfoKey("test")))) path := hd.CreateHDPath(118, 0, 0).String() _, err = kr.NewAccount("test1", seed, "", path, hd.Secp256k1) @@ -1008,7 +1008,7 @@ func TestAltKeyring_Delete(t *testing.T) { require.NoError(t, err) require.Len(t, list, 1) - err = kr.Delete(uid) + err = kr.Delete(string(InfoKey(uid))) require.NoError(t, err) list, err = kr.List() @@ -1147,7 +1147,7 @@ func TestAltKeyring_ImportExportPrivKey(t *testing.T) { passphrase := "somePass" armor, err := kr.ExportPrivKeyArmor(uid, passphrase) require.NoError(t, err) - err = kr.Delete(uid) + err = kr.Delete(string(InfoKey(uid))) require.NoError(t, err) newUID := otherID // Should fail importing with wrong password @@ -1176,7 +1176,7 @@ func TestAltKeyring_ImportExportPrivKey_ByAddress(t *testing.T) { require.NoError(t, err) armor, err := kr.ExportPrivKeyArmorByAddress(addr, passphrase) require.NoError(t, err) - err = kr.Delete(uid) + err = kr.Delete(string(InfoKey(uid))) require.NoError(t, err) newUID := otherID @@ -1203,7 +1203,7 @@ func TestAltKeyring_ImportExportPubKey(t *testing.T) { armor, err := kr.ExportPubKeyArmor(uid) require.NoError(t, err) - err = kr.Delete(uid) + err = kr.Delete(string(InfoKey(uid))) require.NoError(t, err) newUID := otherID @@ -1228,7 +1228,7 @@ func TestAltKeyring_ImportExportPubKey_ByAddress(t *testing.T) { require.NoError(t, err) armor, err := kr.ExportPubKeyArmorByAddress(addr) require.NoError(t, err) - err = kr.Delete(uid) + err = kr.Delete(string(InfoKey(uid))) require.NoError(t, err) newUID := otherID @@ -1312,7 +1312,7 @@ func TestRenameKey(t *testing.T) { run: func(kr Keyring) { oldKeyUID, newKeyUID := "old", "new" oldKeyRecord := newKeyRecord(t, kr, oldKeyUID) - err := kr.Rename(oldKeyUID, newKeyUID) // rename from "old" to "new" + err := kr.Rename(string(InfoKey(oldKeyUID)), newKeyUID) // rename from "old" to "new" require.NoError(t, err) newRecord, err := kr.Key(newKeyUID) // new key should be in keyring require.NoError(t, err) From 7ccdf66a69081ac2daec9d0b300acaa17a588612 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 13 Oct 2021 22:17:36 +0530 Subject: [PATCH 09/14] try fix failing tests --- client/keys/add_ledger_test.go | 9 +++++---- client/tx/tx_test.go | 2 +- crypto/keyring/keyring_test.go | 6 +++--- server/init.go | 7 +++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/client/keys/add_ledger_test.go b/client/keys/add_ledger_test.go index 1901a3e3261f..f41eb3647385 100644 --- a/client/keys/add_ledger_test.go +++ b/client/keys/add_ledger_test.go @@ -1,4 +1,5 @@ -//+build ledger test_ledger_mock +//go:build ledger || test_ledger_mock +// +build ledger test_ledger_mock package keys @@ -185,16 +186,16 @@ func Test_runAddCmdLedgerDryRun(t *testing.T) { require.NoError(t, cmd.ExecuteContext(ctx)) if tt.added { - _, err = kb.Key("testkey") + _, err = kb.Key(string(keyring.InfoKey("testkey"))) require.NoError(t, err) out, err := io.ReadAll(b) require.NoError(t, err) require.Contains(t, string(out), "name: testkey") } else { - _, err = kb.Key("testkey") + _, err = kb.Key(string(keyring.InfoKey("testkey"))) require.Error(t, err) - require.Equal(t, "testkey: key not found", err.Error()) + require.Equal(t, "testkey.info: key not found", err.Error()) } }) } diff --git a/client/tx/tx_test.go b/client/tx/tx_test.go index 8c3ed0ddf07a..c51b98d3dc7f 100644 --- a/client/tx/tx_test.go +++ b/client/tx/tx_test.go @@ -144,7 +144,7 @@ func TestSign(t *testing.T) { // create a new key using a mnemonic generator and test if we can reuse seed to recreate that account _, seed, err := kb.NewMnemonic(from1, keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1) requireT.NoError(err) - requireT.NoError(kb.Delete(from1)) + requireT.NoError(kb.Delete(string(keyring.InfoKey(from1)))) k1, _, err := kb.NewMnemonic(from1, keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1) requireT.NoError(err) diff --git a/crypto/keyring/keyring_test.go b/crypto/keyring/keyring_test.go index ed1860deed76..62009acf17ef 100644 --- a/crypto/keyring/keyring_test.go +++ b/crypto/keyring/keyring_test.go @@ -526,7 +526,7 @@ func TestInMemoryKeyManagement(t *testing.T) { // deleting a key removes it err = cstore.Delete(string(InfoKey("bad name"))) require.NotNil(t, err) - err = cstore.Delete(n1) + err = cstore.Delete(string(InfoKey(n1))) require.NoError(t, err) keyS, err = cstore.List() require.NoError(t, err) @@ -552,14 +552,14 @@ func TestInMemoryKeyManagement(t *testing.T) { require.Equal(t, 2, len(keyS)) // delete the offline key - err = cstore.Delete(o1) + err = cstore.Delete(string(InfoKey(o1))) require.NoError(t, err) keyS, err = cstore.List() require.NoError(t, err) require.Equal(t, 1, len(keyS)) // addr cache gets nuked - and test skip flag - err = cstore.Delete(n2) + err = cstore.Delete(string(InfoKey(n2))) require.NoError(t, err) } diff --git a/server/init.go b/server/init.go index e00ca1d35a70..e0ff18690eb6 100644 --- a/server/init.go +++ b/server/init.go @@ -3,9 +3,8 @@ package server import ( "fmt" - "github.com/cosmos/cosmos-sdk/crypto/keyring" - "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/crypto/keyring" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -28,7 +27,7 @@ func GenerateCoinKey(algo keyring.SignatureAlgo, cdc codec.Codec) (sdk.AccAddres // phrase to recover the private key. func GenerateSaveCoinKey(keybase keyring.Keyring, keyName string, overwrite bool, algo keyring.SignatureAlgo) (sdk.AccAddress, string, error) { exists := false - _, err := keybase.Key(keyName) + _, err := keybase.Key(string(keyring.InfoKey(keyName))) if err == nil { exists = true } @@ -41,7 +40,7 @@ func GenerateSaveCoinKey(keybase keyring.Keyring, keyName string, overwrite bool // generate a private key, with recovery phrase if exists { - err = keybase.Delete(keyName) + err = keybase.Delete(string(keyring.InfoKey(keyName))) if err != nil { return sdk.AccAddress([]byte{}), "", fmt.Errorf( "failed to overwrite key") From 7207fee42270301471b05f8a7a844b6934fd93d6 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 20 Oct 2021 12:32:09 +0530 Subject: [PATCH 10/14] address review comments --- client/keys/add_ledger_test.go | 6 ++--- client/keys/add_test.go | 12 +++++----- client/keys/delete.go | 4 ---- client/keys/delete_test.go | 8 +++---- client/keys/export.go | 5 ---- client/keys/import.go | 6 ----- client/keys/rename.go | 4 ---- client/keys/rename_test.go | 6 ++--- client/keys/show.go | 7 ------ client/keys/show_test.go | 4 ++-- client/tx/tx_test.go | 2 +- crypto/keyring/keyring.go | 13 ++++++---- crypto/keyring/keyring_test.go | 44 +++++++++++++++++----------------- crypto/keyring/legacy_info.go | 2 +- crypto/keyring/types.go | 2 +- server/init.go | 4 ++-- 16 files changed, 53 insertions(+), 76 deletions(-) diff --git a/client/keys/add_ledger_test.go b/client/keys/add_ledger_test.go index 063c28cdd6fb..73b1969a2cd4 100644 --- a/client/keys/add_ledger_test.go +++ b/client/keys/add_ledger_test.go @@ -186,16 +186,16 @@ func Test_runAddCmdLedgerDryRun(t *testing.T) { require.NoError(t, cmd.ExecuteContext(ctx)) if tt.added { - _, err = kb.Key(string(keyring.InfoKey("testkey"))) + _, err = kb.Key("testkey") require.NoError(t, err) out, err := io.ReadAll(b) require.NoError(t, err) require.Contains(t, string(out), "name: testkey") } else { - _, err = kb.Key(string(keyring.InfoKey("testkey"))) + _, err = kb.Key("testkey") require.Error(t, err) - require.Equal(t, "testkey.info: key not found", err.Error()) + require.Equal(t, "testkey: key not found", err.Error()) } }) } diff --git a/client/keys/add_test.go b/client/keys/add_test.go index 8fc1b9cd0d54..edc749122259 100644 --- a/client/keys/add_test.go +++ b/client/keys/add_test.go @@ -64,8 +64,8 @@ func Test_runAddCmdBasic(t *testing.T) { require.NoError(t, cmd.ExecuteContext(ctx)) require.Error(t, cmd.ExecuteContext(ctx)) - mockIn.Reset("N\n") - require.Error(t, cmd.ExecuteContext(ctx)) + mockIn.Reset("y\n") + require.NoError(t, cmd.ExecuteContext(ctx)) cmd.SetArgs([]string{ "keyname4", @@ -151,7 +151,7 @@ func Test_runAddCmdDryRun(t *testing.T) { args: []string{ "testkey", fmt.Sprintf("--%s=%s", flags.FlagDryRun, "false"), - fmt.Sprintf("--%s=%s", flagMultisig, string(keyring.InfoKey("subkey"))), + fmt.Sprintf("--%s=%s", flagMultisig, "subkey"), }, added: true, }, @@ -216,14 +216,14 @@ func Test_runAddCmdDryRun(t *testing.T) { require.NoError(t, cmd.ExecuteContext(ctx)) if tt.added { - _, err := kb.Key(string(keyring.InfoKey("testkey"))) + _, err := kb.Key("testkey") require.NoError(t, err) out, err := io.ReadAll(b) require.NoError(t, err) require.Contains(t, string(out), "name: testkey") } else { - _, err = kb.Key(string(keyring.InfoKey("testkey"))) + _, err = kb.Key("testkey") require.Error(t, err) require.Equal(t, "testkey.info: key not found", err.Error()) } @@ -271,7 +271,7 @@ func TestAddRecoverFileBackend(t *testing.T) { }) mockIn.Reset(fmt.Sprintf("%s\n%s\n", keyringPassword, keyringPassword)) - k, err := kb.Key(string(keyring.InfoKey("keyname1"))) + k, err := kb.Key("keyname1") require.NoError(t, err) require.Equal(t, "keyname1", k.Name) } diff --git a/client/keys/delete.go b/client/keys/delete.go index 95a2a5ec344f..46a269091dab 100644 --- a/client/keys/delete.go +++ b/client/keys/delete.go @@ -2,7 +2,6 @@ package keys import ( "bufio" - "strings" "github.com/spf13/cobra" @@ -36,9 +35,6 @@ private keys stored in a ledger device cannot be deleted with the CLI. } for _, name := range args { - if !strings.HasSuffix(name, keyring.InfoSuffix) { - name = string(keyring.InfoKey(name)) - } k, err := clientCtx.Keyring.Key(name) if err != nil { return err diff --git a/client/keys/delete_test.go b/client/keys/delete_test.go index b313d8e21d8a..f0be3c82e4e1 100644 --- a/client/keys/delete_test.go +++ b/client/keys/delete_test.go @@ -65,7 +65,7 @@ func Test_runDeleteCmd(t *testing.T) { require.Error(t, err) require.Equal(t, "EOF", err.Error()) - _, err = kb.Key(string(keyring.InfoKey(fakeKeyName1))) + _, err = kb.Key(fakeKeyName1) require.NoError(t, err) // Now there is a confirmation @@ -77,10 +77,10 @@ func Test_runDeleteCmd(t *testing.T) { }) require.NoError(t, cmd.Execute()) - _, err = kb.Key(string(keyring.InfoKey(fakeKeyName1))) + _, err = kb.Key(fakeKeyName1) require.Error(t, err) // Key1 is gone - _, err = kb.Key(string(keyring.InfoKey(fakeKeyName2))) + _, err = kb.Key(string(fakeKeyName2)) require.NoError(t, err) cmd.SetArgs([]string{ @@ -91,6 +91,6 @@ func Test_runDeleteCmd(t *testing.T) { }) require.NoError(t, cmd.Execute()) - _, err = kb.Key(string(keyring.InfoKey(fakeKeyName2))) + _, err = kb.Key(fakeKeyName2) require.Error(t, err) // Key2 is gone } diff --git a/client/keys/export.go b/client/keys/export.go index b0094e553d09..13491b5e839a 100644 --- a/client/keys/export.go +++ b/client/keys/export.go @@ -3,7 +3,6 @@ package keys import ( "bufio" "fmt" - "strings" "github.com/spf13/cobra" @@ -40,10 +39,6 @@ and export your keys in ASCII-armored encrypted format.`, unarmored, _ := cmd.Flags().GetBool(flagUnarmoredHex) unsafe, _ := cmd.Flags().GetBool(flagUnsafe) - if !strings.HasSuffix(args[0], keyring.InfoSuffix) { - args[0] = string(keyring.InfoKey(args[0])) - } - if unarmored && unsafe { return exportUnsafeUnarmored(cmd, args[0], buf, clientCtx.Keyring) } else if unarmored || unsafe { diff --git a/client/keys/import.go b/client/keys/import.go index ef1c109d583b..a9d5c185acb7 100644 --- a/client/keys/import.go +++ b/client/keys/import.go @@ -3,13 +3,11 @@ package keys import ( "bufio" "os" - "strings" "github.com/spf13/cobra" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/input" - "github.com/cosmos/cosmos-sdk/crypto/keyring" ) // ImportKeyCommand imports private keys from a keyfile. @@ -36,10 +34,6 @@ func ImportKeyCommand() *cobra.Command { return err } - if !strings.HasSuffix(args[0], keyring.InfoSuffix) { - args[0] = string(keyring.InfoKey(args[0])) - } - return clientCtx.Keyring.ImportPrivKey(args[0], string(bz), passphrase) }, } diff --git a/client/keys/rename.go b/client/keys/rename.go index 3ccb038a7540..f703c60f20eb 100644 --- a/client/keys/rename.go +++ b/client/keys/rename.go @@ -3,7 +3,6 @@ package keys import ( "bufio" "fmt" - "strings" "github.com/spf13/cobra" @@ -32,9 +31,6 @@ private keys stored in a ledger device cannot be renamed with the CLI. } oldName, newName := args[0], args[1] - if !strings.HasSuffix(oldName, keyring.InfoSuffix) { - oldName = string(keyring.InfoKey(oldName)) - } k, err := clientCtx.Keyring.Key(oldName) if err != nil { diff --git a/client/keys/rename_test.go b/client/keys/rename_test.go index 559f3b9d39e4..b32e08c096f9 100644 --- a/client/keys/rename_test.go +++ b/client/keys/rename_test.go @@ -62,7 +62,7 @@ func Test_runRenameCmd(t *testing.T) { require.Error(t, err) require.Equal(t, "EOF", err.Error()) - oldKey, err := kb.Key(string(keyring.InfoKey(fakeKeyName1))) + oldKey, err := kb.Key(fakeKeyName1) require.NoError(t, err) // add a confirmation @@ -76,11 +76,11 @@ func Test_runRenameCmd(t *testing.T) { require.NoError(t, cmd.Execute()) // key1 is gone - _, err = kb.Key(string(keyring.InfoKey(fakeKeyName1))) + _, err = kb.Key(fakeKeyName1) require.Error(t, err) // key2 exists now - renamedKey, err := kb.Key(string(keyring.InfoKey(fakeKeyName2))) + renamedKey, err := kb.Key(fakeKeyName2) require.NoError(t, err) oldPk, err := oldKey.GetPubKey() require.NoError(t, err) diff --git a/client/keys/show.go b/client/keys/show.go index 645ebcb0061e..d97b4a67d842 100644 --- a/client/keys/show.go +++ b/client/keys/show.go @@ -3,7 +3,6 @@ package keys import ( "errors" "fmt" - "strings" "github.com/spf13/cobra" "github.com/tendermint/tendermint/libs/cli" @@ -164,12 +163,6 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) { } func fetchKey(kb keyring.Keyring, keyref string) (*keyring.Record, error) { - var key []byte - if !strings.HasPrefix(keyref, "cosmos") && !strings.HasSuffix(keyref, keyring.InfoSuffix) { - key = keyring.InfoKey(keyref) - keyref = string(key) - } - // firstly check if the keyref is a key name of a key registered in a keyring. k, err := kb.Key(keyref) // if the key is not there or if we have a problem with a keyring itself then we move to a diff --git a/client/keys/show_test.go b/client/keys/show_test.go index 84ff73772ae1..af3284221cba 100644 --- a/client/keys/show_test.go +++ b/client/keys/show_test.go @@ -62,7 +62,7 @@ func Test_runShowCmd(t *testing.T) { ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx) cmd.SetArgs([]string{"invalid"}) - require.EqualError(t, cmd.ExecuteContext(ctx), "invalid is not a valid name or address: decoding bech32 failed: invalid separator index -1") + require.EqualError(t, cmd.ExecuteContext(ctx), "invalid is not a valid name or address: decoding bech32 failed: invalid bech32 string length 7") cmd.SetArgs([]string{"invalid1", "invalid2"}) require.EqualError(t, cmd.ExecuteContext(ctx), "invalid1 is not a valid name or address: decoding bech32 failed: invalid separator index 7") @@ -103,7 +103,7 @@ func Test_runShowCmd(t *testing.T) { require.NoError(t, cmd.ExecuteContext(ctx)) // try fetch by addr - k, err := kb.Key(string(keyring.InfoKey(fakeKeyName1))) + k, err := kb.Key(fakeKeyName1) require.NoError(t, err) addr, err := k.GetAddress() require.NoError(t, err) diff --git a/client/tx/tx_test.go b/client/tx/tx_test.go index c51b98d3dc7f..8c3ed0ddf07a 100644 --- a/client/tx/tx_test.go +++ b/client/tx/tx_test.go @@ -144,7 +144,7 @@ func TestSign(t *testing.T) { // create a new key using a mnemonic generator and test if we can reuse seed to recreate that account _, seed, err := kb.NewMnemonic(from1, keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1) requireT.NoError(err) - requireT.NoError(kb.Delete(string(keyring.InfoKey(from1)))) + requireT.NoError(kb.Delete(from1)) k1, _, err := kb.NewMnemonic(from1, keyring.English, path, keyring.DefaultBIP39Passphrase, hd.Secp256k1) requireT.NoError(err) diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index 5f1bdd2427bd..08892b255987 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -410,7 +410,7 @@ func (ks keystore) DeleteByAddress(address sdk.Address) error { return err } - err = ks.Delete(string(InfoKey(k.Name))) + err = ks.Delete(k.Name) if err != nil { return err } @@ -456,6 +456,9 @@ func (ks keystore) Delete(uid string) error { return err } + if !(strings.HasSuffix(uid, infoSuffix)) { + uid = string(infoKey(uid)) + } err = ks.db.Remove(uid) if err != nil { return err @@ -770,9 +773,9 @@ func (ks keystore) writeRecord(k *Record) error { return err } - key := InfoKey(k.Name) + key := infoKey(k.Name) - exists, err := ks.existsInDb(addr, string(key)) + exists, err := ks.existsInDb(addr, k.Name) if err != nil { return err } @@ -877,8 +880,8 @@ func (ks keystore) MigrateAll() (bool, error) { // migrate converts keyring.Item from amino to proto serialization format. func (ks keystore) migrate(key string) (*Record, bool, error) { - if !(strings.HasSuffix(key, "info")) && !(strings.HasPrefix(key, "cosmos")) { - key = string(InfoKey(key)) + if !(strings.HasSuffix(key, infoSuffix)) && !(strings.HasPrefix(key, sdk.Bech32PrefixAccAddr)) { + key = string(infoKey(key)) } item, err := ks.db.Get(key) if err != nil { diff --git a/crypto/keyring/keyring_test.go b/crypto/keyring/keyring_test.go index 62009acf17ef..539be786608e 100644 --- a/crypto/keyring/keyring_test.go +++ b/crypto/keyring/keyring_test.go @@ -119,7 +119,7 @@ func TestKeyManagementKeyRing(t *testing.T) { // deleting a key removes it err = kb.Delete("bad name") require.NotNil(t, err) - err = kb.Delete(string(InfoKey(n1))) + err = kb.Delete(n1) require.NoError(t, err) keyS, err = kb.List() require.NoError(t, err) @@ -145,14 +145,14 @@ func TestKeyManagementKeyRing(t *testing.T) { require.Equal(t, 2, len(keyS)) // delete the offline key - err = kb.Delete(string(InfoKey(o1))) + err = kb.Delete(o1) require.NoError(t, err) keyS, err = kb.List() require.NoError(t, err) require.Equal(t, 1, len(keyS)) // addr cache gets nuked - and test skip flag - require.NoError(t, kb.Delete(string(InfoKey(n2)))) + require.NoError(t, kb.Delete(n2)) } func TestSignVerifyKeyRing(t *testing.T) { @@ -230,7 +230,7 @@ func TestSignVerifyKeyRing(t *testing.T) { // Import a public key armor, err := kb.ExportPubKeyArmor(n2) require.NoError(t, err) - require.NoError(t, kb.Delete(string(InfoKey(n2)))) + require.NoError(t, kb.Delete(n2)) require.NoError(t, kb.ImportPubKey(n3, armor)) i3, err := kb.Key(n3) @@ -261,7 +261,7 @@ func TestExportImportKeyRing(t *testing.T) { armor, err := kb.ExportPrivKeyArmor("john", "apassphrase") require.NoError(t, err) - err = kb.Delete(string(InfoKey("john"))) + err = kb.Delete("john") require.NoError(t, err) err = kb.ImportPrivKey("john2", armor, "apassphrase") @@ -312,7 +312,7 @@ func TestExportImportPubKeyKeyRing(t *testing.T) { // Export the public key only armor, err := kb.ExportPubKeyArmor("john") require.NoError(t, err) - err = kb.Delete(string(InfoKey("john"))) + err = kb.Delete("john") require.NoError(t, err) // Import it under a different name @@ -357,7 +357,7 @@ func TestAdvancedKeyManagementKeyRing(t *testing.T) { require.NotNil(t, err) exported, err := kb.ExportPubKeyArmor(n1) require.Nil(t, err, "%+v", err) - err = kb.Delete(string(InfoKey(n1))) + err = kb.Delete(n1) require.NoError(t, err) // import succeeds @@ -388,7 +388,7 @@ func TestSeedPhraseKeyRing(t *testing.T) { require.NoError(t, err) // now, let us delete this key - err = kb.Delete(string(InfoKey(n1))) + err = kb.Delete(n1) require.Nil(t, err, "%+v", err) _, err = kb.Key(n1) require.NotNil(t, err) @@ -416,7 +416,7 @@ func TestKeyringKeybaseExportImportPrivKey(t *testing.T) { keystr, err := kb.ExportPrivKeyArmor("john", "somepassword") require.NoError(t, err) require.NotEmpty(t, keystr) - err = kb.Delete(string(InfoKey("john"))) + err = kb.Delete("john") require.NoError(t, err) // try import the key - wrong password @@ -524,9 +524,9 @@ func TestInMemoryKeyManagement(t *testing.T) { require.True(t, key1.Equals(key2)) // deleting a key removes it - err = cstore.Delete(string(InfoKey("bad name"))) + err = cstore.Delete("bad name") require.NotNil(t, err) - err = cstore.Delete(string(InfoKey(n1))) + err = cstore.Delete(n1) require.NoError(t, err) keyS, err = cstore.List() require.NoError(t, err) @@ -552,14 +552,14 @@ func TestInMemoryKeyManagement(t *testing.T) { require.Equal(t, 2, len(keyS)) // delete the offline key - err = cstore.Delete(string(InfoKey(o1))) + err = cstore.Delete(o1) require.NoError(t, err) keyS, err = cstore.List() require.NoError(t, err) require.Equal(t, 1, len(keyS)) // addr cache gets nuked - and test skip flag - err = cstore.Delete(string(InfoKey(n2))) + err = cstore.Delete(n2) require.NoError(t, err) } @@ -702,7 +702,7 @@ func TestInMemoryExportImportPrivKey(t *testing.T) { require.NoError(t, err) // delete exported key - require.NoError(t, kb.Delete(string(InfoKey("john")))) + require.NoError(t, kb.Delete("john")) _, err = kb.Key("john") require.Error(t, err) @@ -813,7 +813,7 @@ func TestInMemorySeedPhrase(t *testing.T) { require.NotEmpty(t, mnemonic) // now, let us delete this key - err = cstore.Delete(string(InfoKey(n1))) + err = cstore.Delete(n1) require.Nil(t, err, "%+v", err) _, err = cstore.Key(n1) require.NotNil(t, err) @@ -840,7 +840,7 @@ func TestKeyChain_ShouldFailWhenAddingSameGeneratedAccount(t *testing.T) { _, seed, err := kr.NewMnemonic("test", English, "", DefaultBIP39Passphrase, hd.Secp256k1) require.NoError(t, err) - require.NoError(t, kr.Delete(string(InfoKey("test")))) + require.NoError(t, kr.Delete("test")) path := hd.CreateHDPath(118, 0, 0).String() _, err = kr.NewAccount("test1", seed, "", path, hd.Secp256k1) @@ -1008,7 +1008,7 @@ func TestAltKeyring_Delete(t *testing.T) { require.NoError(t, err) require.Len(t, list, 1) - err = kr.Delete(string(InfoKey(uid))) + err = kr.Delete(uid) require.NoError(t, err) list, err = kr.List() @@ -1147,7 +1147,7 @@ func TestAltKeyring_ImportExportPrivKey(t *testing.T) { passphrase := "somePass" armor, err := kr.ExportPrivKeyArmor(uid, passphrase) require.NoError(t, err) - err = kr.Delete(string(InfoKey(uid))) + err = kr.Delete(uid) require.NoError(t, err) newUID := otherID // Should fail importing with wrong password @@ -1176,7 +1176,7 @@ func TestAltKeyring_ImportExportPrivKey_ByAddress(t *testing.T) { require.NoError(t, err) armor, err := kr.ExportPrivKeyArmorByAddress(addr, passphrase) require.NoError(t, err) - err = kr.Delete(string(InfoKey(uid))) + err = kr.Delete(uid) require.NoError(t, err) newUID := otherID @@ -1203,7 +1203,7 @@ func TestAltKeyring_ImportExportPubKey(t *testing.T) { armor, err := kr.ExportPubKeyArmor(uid) require.NoError(t, err) - err = kr.Delete(string(InfoKey(uid))) + err = kr.Delete(uid) require.NoError(t, err) newUID := otherID @@ -1228,7 +1228,7 @@ func TestAltKeyring_ImportExportPubKey_ByAddress(t *testing.T) { require.NoError(t, err) armor, err := kr.ExportPubKeyArmorByAddress(addr) require.NoError(t, err) - err = kr.Delete(string(InfoKey(uid))) + err = kr.Delete(uid) require.NoError(t, err) newUID := otherID @@ -1312,7 +1312,7 @@ func TestRenameKey(t *testing.T) { run: func(kr Keyring) { oldKeyUID, newKeyUID := "old", "new" oldKeyRecord := newKeyRecord(t, kr, oldKeyUID) - err := kr.Rename(string(InfoKey(oldKeyUID)), newKeyUID) // rename from "old" to "new" + err := kr.Rename(oldKeyUID, newKeyUID) // rename from "old" to "new" require.NoError(t, err) newRecord, err := kr.Key(newKeyUID) // new key should be in keyring require.NoError(t, err) diff --git a/crypto/keyring/legacy_info.go b/crypto/keyring/legacy_info.go index e178aabe2991..d2d88d8daedc 100644 --- a/crypto/keyring/legacy_info.go +++ b/crypto/keyring/legacy_info.go @@ -43,7 +43,7 @@ type legacyLocalInfo struct { Algo hd.PubKeyType `json:"algo"` } -func InfoKey(name string) []byte { return []byte(fmt.Sprintf("%s.%s", name, InfoSuffix)) } +func infoKey(name string) []byte { return []byte(fmt.Sprintf("%s.%s", name, infoSuffix)) } // GetType implements Info interface func (i legacyLocalInfo) GetType() KeyType { diff --git a/crypto/keyring/types.go b/crypto/keyring/types.go index 447d8d750844..0b893ea4cccc 100644 --- a/crypto/keyring/types.go +++ b/crypto/keyring/types.go @@ -37,7 +37,7 @@ const ( // bits of entropy to draw when creating a mnemonic defaultEntropySize = 256 addressSuffix = "address" - InfoSuffix = "info" + infoSuffix = "info" ) // KeyType reflects a human-readable type for key listing. diff --git a/server/init.go b/server/init.go index e0ff18690eb6..1b6e0cdc51d7 100644 --- a/server/init.go +++ b/server/init.go @@ -27,7 +27,7 @@ func GenerateCoinKey(algo keyring.SignatureAlgo, cdc codec.Codec) (sdk.AccAddres // phrase to recover the private key. func GenerateSaveCoinKey(keybase keyring.Keyring, keyName string, overwrite bool, algo keyring.SignatureAlgo) (sdk.AccAddress, string, error) { exists := false - _, err := keybase.Key(string(keyring.InfoKey(keyName))) + _, err := keybase.Key(keyName) if err == nil { exists = true } @@ -40,7 +40,7 @@ func GenerateSaveCoinKey(keybase keyring.Keyring, keyName string, overwrite bool // generate a private key, with recovery phrase if exists { - err = keybase.Delete(string(keyring.InfoKey(keyName))) + err = keybase.Delete(keyName) if err != nil { return sdk.AccAddress([]byte{}), "", fmt.Errorf( "failed to overwrite key") From de79be4ebd0249ddc9a83c4c453b0ebb6db86da7 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 20 Oct 2021 12:41:34 +0530 Subject: [PATCH 11/14] fix failing tests --- client/keys/add_ledger_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/keys/add_ledger_test.go b/client/keys/add_ledger_test.go index 73b1969a2cd4..e46b7e26fcc4 100644 --- a/client/keys/add_ledger_test.go +++ b/client/keys/add_ledger_test.go @@ -195,7 +195,7 @@ func Test_runAddCmdLedgerDryRun(t *testing.T) { } else { _, err = kb.Key("testkey") require.Error(t, err) - require.Equal(t, "testkey: key not found", err.Error()) + require.Equal(t, "testkey.info: key not found", err.Error()) } }) } From 1552a1333184542b5bfc1e985a0fd5a426f49dce Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 20 Oct 2021 13:01:42 +0530 Subject: [PATCH 12/14] small fix --- client/keys/delete_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/keys/delete_test.go b/client/keys/delete_test.go index f0be3c82e4e1..b87a98da5a80 100644 --- a/client/keys/delete_test.go +++ b/client/keys/delete_test.go @@ -80,7 +80,7 @@ func Test_runDeleteCmd(t *testing.T) { _, err = kb.Key(fakeKeyName1) require.Error(t, err) // Key1 is gone - _, err = kb.Key(string(fakeKeyName2)) + _, err = kb.Key(fakeKeyName2) require.NoError(t, err) cmd.SetArgs([]string{ From 7247840b0917aa4d3af682766c41600ff7d9ddfe Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Wed, 20 Oct 2021 16:21:04 +0530 Subject: [PATCH 13/14] small fix --- crypto/keyring/keyring.go | 6 +++--- crypto/keyring/keyring_test.go | 2 +- crypto/keyring/legacy_info.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index 08892b255987..a46dac60c459 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -457,7 +457,7 @@ func (ks keystore) Delete(uid string) error { } if !(strings.HasSuffix(uid, infoSuffix)) { - uid = string(infoKey(uid)) + uid = infoKey(uid) } err = ks.db.Remove(uid) if err != nil { @@ -775,7 +775,7 @@ func (ks keystore) writeRecord(k *Record) error { key := infoKey(k.Name) - exists, err := ks.existsInDb(addr, k.Name) + exists, err := ks.existsInDb(addr, key) if err != nil { return err } @@ -881,7 +881,7 @@ func (ks keystore) MigrateAll() (bool, error) { // migrate converts keyring.Item from amino to proto serialization format. func (ks keystore) migrate(key string) (*Record, bool, error) { if !(strings.HasSuffix(key, infoSuffix)) && !(strings.HasPrefix(key, sdk.Bech32PrefixAccAddr)) { - key = string(infoKey(key)) + key = infoKey(key) } item, err := ks.db.Get(key) if err != nil { diff --git a/crypto/keyring/keyring_test.go b/crypto/keyring/keyring_test.go index 539be786608e..3afd35676fb7 100644 --- a/crypto/keyring/keyring_test.go +++ b/crypto/keyring/keyring_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/99designs/keyring" + "github.com/cosmos/go-bip39" "github.com/stretchr/testify/require" "github.com/cosmos/cosmos-sdk/codec" @@ -19,7 +20,6 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" - bip39 "github.com/cosmos/go-bip39" ) const ( diff --git a/crypto/keyring/legacy_info.go b/crypto/keyring/legacy_info.go index d2d88d8daedc..8b05b6a0312c 100644 --- a/crypto/keyring/legacy_info.go +++ b/crypto/keyring/legacy_info.go @@ -43,7 +43,7 @@ type legacyLocalInfo struct { Algo hd.PubKeyType `json:"algo"` } -func infoKey(name string) []byte { return []byte(fmt.Sprintf("%s.%s", name, infoSuffix)) } +func infoKey(name string) string { return fmt.Sprintf("%s.%s", name, infoSuffix) } // GetType implements Info interface func (i legacyLocalInfo) GetType() KeyType { From a221b7f58b3acc72e510c0676011fac0ae3033a2 Mon Sep 17 00:00:00 2001 From: likhita-809 Date: Tue, 26 Oct 2021 15:50:10 +0530 Subject: [PATCH 14/14] fix nits --- crypto/keyring/keyring.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index a46dac60c459..7686d18cdd6e 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -440,6 +440,8 @@ func (ks keystore) Rename(oldName, newName string) error { return nil } +// Delete deletes a key in the keyring. `uid` represents the key name, without +// the `.info` suffix. func (ks keystore) Delete(uid string) error { k, err := ks.Key(uid) if err != nil { @@ -456,10 +458,7 @@ func (ks keystore) Delete(uid string) error { return err } - if !(strings.HasSuffix(uid, infoSuffix)) { - uid = infoKey(uid) - } - err = ks.db.Remove(uid) + err = ks.db.Remove(infoKey(uid)) if err != nil { return err } @@ -789,7 +788,7 @@ func (ks keystore) writeRecord(k *Record) error { } item := keyring.Item{ - Key: string(key), + Key: key, Data: serializedRecord, }