Skip to content

Commit

Permalink
refactor!: Keyring migration (#9695)
Browse files Browse the repository at this point in the history 
<!--
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

The draft PR #9222 
Closes: #7108

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

- implement proto definition for `Record` 
- rename `Info.go` to `legacyInfo.go` within `keyring` package
- implement CLI `migrate` command that migrates all keys from  legacyInfo to proto according to @robert-zaremba migration [algorithm](#9222)
- remove legacy keybase entirely.
- add `Migrate` and `MigrateAll` functions  in `keyring.go` for single key and all keys migration
- add tests
- fix tests

---

### 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...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] 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)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [x] reviewed "Files changed" and left comments if necessary
- [ ] 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...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
  • Loading branch information
@cyberbono3
cyberbono3 committed Sep 20, 2021
1 parent a0ecfe5 commit 6cbbd6d
Show file tree
Hide file tree
Showing 80 changed files with 4,100 additions and 1,302 deletions.
18 changes: 18 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,23 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* [\#9695](https://github.com/cosmos/cosmos-sdk/pull/9695) Migrate keys from `Info` -> `Record`
* Add new `codec.Codec` argument in:
* `keyring.NewInMemory`
* `keyring.New`
* Rename:
* `SavePubKey` to `SaveOfflineKey`.
* `NewMultiInfo`, `NewLedgerInfo` to `NewLegacyMultiInfo`, `newLegacyLedgerInfo` respectively. Move them into `legacy_info.go`.
* `NewOfflineInfo` to `newLegacyOfflineInfo` and move it to `migration_test.go`.
* Return:
*`keyring.Record, error` in `SaveOfflineKey`, `SaveLedgerKey`, `SaveMultiSig`, `Key` and `KeyByAddress`.
*`keyring.Record` instead of `Info` in `NewMnemonic` and `List`.
* Remove `algo` argument from :
* `SaveOfflineKey`
* Take `keyring.Record` instead of `Info` as first argument in:
* `MkConsKeyOutput`
* `MkValKeyOutput`
* `MkAccKeyOutput`
* [\#10077](https://github.com/cosmos/cosmos-sdk/pull/10077) Remove telemetry on `GasKV` and `CacheKV` store Get/Set operations, significantly improving their performance.
* [\#10022](https://github.com/cosmos/cosmos-sdk/pull/10022) `AuthKeeper` interface in `x/auth` now includes a function `HasAccount`.
* [\#9759](https://github.com/cosmos/cosmos-sdk/pull/9759) `NewAccountKeeeper` in `x/auth` now takes an additional `bech32Prefix` argument that represents `sdk.Bech32MainPrefix`.
Expand Down Expand Up @@ -83,6 +100,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### CLI Breaking Changes

* [\#9695](https://github.com/cosmos/cosmos-sdk/pull/9695) `<app> keys migrate` CLI command now takes no arguments
* [\#9246](https://github.com/cosmos/cosmos-sdk/pull/9246) Removed the CLI flag `--setup-config-only` from the `testnet` command and added the subcommand `init-files`.

### Improvements
Expand Down
17 changes: 11 additions & 6 deletions client/context.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,27 +323,32 @@ func GetFromFields(kr keyring.Keyring, from string, genOnly bool) (sdk.AccAddres
return nil, "", 0, nil
}

var info keyring.Info
var k *keyring.Record
if addr, err := sdk.AccAddressFromBech32(from); err == nil {
info, err = kr.KeyByAddress(addr)
k, err = kr.KeyByAddress(addr)
if err != nil {
return nil, "", 0, err
}
} else {
info, err = kr.Key(from)
k, err = kr.Key(from)
if err != nil {
return nil, "", 0, err
}
}

return info.GetAddress(), info.GetName(), info.GetType(), nil
addr, err := k.GetAddress()
if err != nil {
return nil, "", 0, err
}

return addr, k.Name, k.GetType(), nil
}

// NewKeyringFromBackend gets a Keyring object from a backend
func NewKeyringFromBackend(ctx Context, backend string) (keyring.Keyring, error) {
if ctx.GenerateOnly || ctx.Simulate {
return keyring.New(sdk.KeyringServiceName(), keyring.BackendMemory, ctx.KeyringDir, ctx.Input, ctx.KeyringOptions...)
backend = keyring.BackendMemory
}

return keyring.New(sdk.KeyringServiceName(), backend, ctx.KeyringDir, ctx.Input, ctx.KeyringOptions...)
return keyring.New(sdk.KeyringServiceName(), backend, ctx.KeyringDir, ctx.Input, ctx.Codec, ctx.KeyringOptions...)
}
36 changes: 19 additions & 17 deletions client/keys/add.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"fmt"
"sort"

bip39 "github.com/cosmos/go-bip39"
"github.com/cosmos/go-bip39"
"github.com/spf13/cobra"

"github.com/cosmos/cosmos-sdk/client"
Expand Down Expand Up @@ -119,7 +119,7 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf

if dryRun, _ := cmd.Flags().GetBool(flags.FlagDryRun); dryRun {
// use in memory keybase
kb = keyring.NewInMemory()
kb = keyring.NewInMemory(ctx.Codec)
} else {
_, err = kb.Key(name)
if err == nil {
Expand Down Expand Up @@ -153,7 +153,11 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf
return err
}

pks[i] = k.GetPubKey()
key, err := k.GetPubKey()
if err != nil {
return err
}
pks[i] = key
}

if noSort, _ := cmd.Flags().GetBool(flagNoSort); !noSort {
Expand All @@ -163,29 +167,28 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf
}

pk := multisig.NewLegacyAminoPubKey(multisigThreshold, pks)
info, err := kb.SaveMultisig(name, pk)
k, err := kb.SaveMultisig(name, pk)
if err != nil {
return err
}

return printCreate(cmd, info, false, "", outputFormat)
return printCreate(cmd, k, false, "", outputFormat)
}
}

pubKey, _ := cmd.Flags().GetString(FlagPublicKey)
if pubKey != "" {
var pk cryptotypes.PubKey
err = ctx.Codec.UnmarshalInterfaceJSON([]byte(pubKey), &pk)
if err != nil {
if err = ctx.Codec.UnmarshalInterfaceJSON([]byte(pubKey), &pk); err != nil {
return err
}

info, err := kb.SavePubKey(name, pk, algo.Name())
k, err := kb.SaveOfflineKey(name, pk)
if err != nil {
return err
}

return printCreate(cmd, info, false, "", outputFormat)
return printCreate(cmd, k, false, "", outputFormat)
}

coinType, _ := cmd.Flags().GetUint32(flagCoinType)
Expand All @@ -203,13 +206,12 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf
// If we're using ledger, only thing we need is the path and the bech32 prefix.
if useLedger {
bech32PrefixAccAddr := sdk.GetConfig().GetBech32AccountAddrPrefix()

info, err := kb.SaveLedgerKey(name, algo, bech32PrefixAccAddr, coinType, account, index)
k, err := kb.SaveLedgerKey(name, hd.Secp256k1, bech32PrefixAccAddr, coinType, account, index)
if err != nil {
return err
}

return printCreate(cmd, info, false, "", outputFormat)
return printCreate(cmd, k, false, "", outputFormat)
}

// Get bip39 mnemonic
Expand Down Expand Up @@ -271,7 +273,7 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf
}
}

info, err := kb.NewAccount(name, mnemonic, bip39Passphrase, hdPath, algo)
k, err := kb.NewAccount(name, mnemonic, bip39Passphrase, hdPath, algo)
if err != nil {
return err
}
Expand All @@ -283,14 +285,14 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf
mnemonic = ""
}

return printCreate(cmd, info, showMnemonic, mnemonic, outputFormat)
return printCreate(cmd, k, showMnemonic, mnemonic, outputFormat)
}

func printCreate(cmd *cobra.Command, info keyring.Info, showMnemonic bool, mnemonic string, outputFormat string) error {
func printCreate(cmd *cobra.Command, k *keyring.Record, showMnemonic bool, mnemonic, outputFormat string) error {
switch outputFormat {
case OutputFormatText:
cmd.PrintErrln()
printKeyInfo(cmd.OutOrStdout(), info, keyring.MkAccKeyOutput, outputFormat)
printKeyringRecord(cmd.OutOrStdout(), k, keyring.MkAccKeyOutput, outputFormat)

// print mnemonic unless requested not to.
if showMnemonic {
Expand All @@ -300,7 +302,7 @@ func printCreate(cmd *cobra.Command, info keyring.Info, showMnemonic bool, mnemo
fmt.Fprintln(cmd.ErrOrStderr(), mnemonic)
}
case OutputFormatJSON:
out, err := keyring.MkAccKeyOutput(info)
out, err := keyring.MkAccKeyOutput(k)
if err != nil {
return err
}
Expand Down
36 changes: 23 additions & 13 deletions client/keys/add_ledger_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"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/simapp"
"github.com/cosmos/cosmos-sdk/testutil"
sdk "github.com/cosmos/cosmos-sdk/types"
)
Expand All @@ -42,7 +43,8 @@ func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {
// Prepare a keybase
kbHome := t.TempDir()

clientCtx := client.Context{}.WithKeyringDir(kbHome)
cdc := simapp.MakeTestEncodingConfig().Codec
clientCtx := client.Context{}.WithKeyringDir(kbHome).WithCodec(cdc)
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)

cmd.SetArgs([]string{
Expand All @@ -61,7 +63,7 @@ func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {
require.NoError(t, cmd.ExecuteContext(ctx))

// Now check that it has been stored properly
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn)
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn, cdc)
require.NoError(t, err)
require.NotNil(t, kb)
t.Cleanup(func() {
Expand All @@ -72,11 +74,13 @@ func Test_runAddCmdLedgerWithCustomCoinType(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, key1)

require.Equal(t, "keyname1", key1.GetName())
require.Equal(t, "keyname1", key1.Name)
require.Equal(t, keyring.TypeLedger, key1.GetType())
pub, err := key1.GetPubKey()
require.NoError(t, err)
require.Equal(t,
"PubKeySecp256k1{03028F0D5A9FD41600191CDEFDEA05E77A68DFBCE286241C0190805B9346667D07}",
key1.GetPubKey().String())
pub.String())

config.SetPurpose(44)
config.SetCoinType(118)
Expand All @@ -91,8 +95,9 @@ func Test_runAddCmdLedger(t *testing.T) {

mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
kbHome := t.TempDir()
encCfg := simapp.MakeTestEncodingConfig()

clientCtx := client.Context{}.WithKeyringDir(kbHome)
clientCtx := client.Context{}.WithKeyringDir(kbHome).WithCodec(encCfg.Codec)
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)

cmd.SetArgs([]string{
Expand All @@ -108,8 +113,10 @@ func Test_runAddCmdLedger(t *testing.T) {
require.NoError(t, cmd.ExecuteContext(ctx))

// Now check that it has been stored properly
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn)
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn, encCfg.Codec)
require.NoError(t, err)

// Now check that it has been stored properly
require.NotNil(t, kb)
t.Cleanup(func() {
_ = kb.Delete("keyname1")
Expand All @@ -120,14 +127,16 @@ func Test_runAddCmdLedger(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, key1)

require.Equal(t, "keyname1", key1.GetName())
require.Equal(t, keyring.TypeLedger, key1.GetType())
require.Equal(t, "keyname1", key1.Name)
pub, err := key1.GetPubKey()
require.NoError(t, err)
require.Equal(t,
"PubKeySecp256k1{034FEF9CD7C4C63588D3B03FEB5281B9D232CBA34D6F3D71AEE59211FFBFE1FE87}",
key1.GetPubKey().String())
pub.String())
}

func Test_runAddCmdLedgerDryRun(t *testing.T) {
cdc := simapp.MakeTestEncodingConfig().Codec
testData := []struct {
name string
args []string
Expand All @@ -152,6 +161,7 @@ func Test_runAddCmdLedgerDryRun(t *testing.T) {
added: false,
},
}

for _, tt := range testData {
tt := tt
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -160,14 +170,14 @@ func Test_runAddCmdLedgerDryRun(t *testing.T) {

kbHome := t.TempDir()
mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn)
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn, cdc)
require.NoError(t, err)

clientCtx := client.Context{}.
WithKeyringDir(kbHome).
WithKeyring(kb)
WithKeyring(kb).
WithCodec(cdc)
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)

b := bytes.NewBufferString("")
cmd.SetOut(b)

Expand All @@ -184,7 +194,7 @@ func Test_runAddCmdLedgerDryRun(t *testing.T) {
} else {
_, 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())
}
})
}
Expand Down
26 changes: 15 additions & 11 deletions client/keys/add_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@ func Test_runAddCmdBasic(t *testing.T) {
mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
kbHome := t.TempDir()

kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn)
cdc := simapp.MakeTestEncodingConfig().Codec

kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn, cdc)
require.NoError(t, err)

clientCtx := client.Context{}.WithKeyringDir(kbHome).WithInput(mockIn)
clientCtx := client.Context{}.WithKeyringDir(kbHome).WithInput(mockIn).WithCodec(cdc)
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)

t.Cleanup(func() {
Expand Down Expand Up @@ -122,6 +124,7 @@ func Test_runAddCmdBasic(t *testing.T) {
func Test_runAddCmdDryRun(t *testing.T) {
pubkey1 := `{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"AtObiFVE4s+9+RX5SP8TN9r2mxpoaT4eGj9CJfK7VRzN"}`
pubkey2 := `{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"A/se1vkqgdQ7VJQCM4mxN+L+ciGhnnJ4XYsQCRBMrdRi"}`
cdc := simapp.MakeTestEncodingConfig().Codec

testData := []struct {
name string
Expand Down Expand Up @@ -189,12 +192,12 @@ func Test_runAddCmdDryRun(t *testing.T) {

kbHome := t.TempDir()
mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn)

kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn, cdc)
require.NoError(t, err)

appCodec := simapp.MakeTestEncodingConfig().Codec
clientCtx := client.Context{}.
WithCodec(appCodec).
WithCodec(cdc).
WithKeyringDir(kbHome).
WithKeyring(kb)
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)
Expand All @@ -214,7 +217,7 @@ func Test_runAddCmdDryRun(t *testing.T) {
require.NoError(t, cmd.ExecuteContext(ctx))

if tt.added {
_, err = kb.Key("testkey")
_, err := kb.Key("testkey")
require.NoError(t, err)

out, err := ioutil.ReadAll(b)
Expand All @@ -223,7 +226,7 @@ func Test_runAddCmdDryRun(t *testing.T) {
} else {
_, 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())
}
})
}
Expand All @@ -232,11 +235,12 @@ func Test_runAddCmdDryRun(t *testing.T) {
func TestAddRecoverFileBackend(t *testing.T) {
cmd := AddKeyCommand()
cmd.Flags().AddFlagSet(Commands("home").PersistentFlags())
cdc := simapp.MakeTestEncodingConfig().Codec

mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
kbHome := t.TempDir()

clientCtx := client.Context{}.WithKeyringDir(kbHome).WithInput(mockIn)
clientCtx := client.Context{}.WithKeyringDir(kbHome).WithInput(mockIn).WithCodec(cdc)
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)

cmd.SetArgs([]string{
Expand All @@ -259,7 +263,7 @@ func TestAddRecoverFileBackend(t *testing.T) {
mockIn.Reset(fmt.Sprintf("%s\n%s\n%s\n", mnemonic, keyringPassword, keyringPassword))
require.NoError(t, cmd.ExecuteContext(ctx))

kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendFile, kbHome, mockIn)
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendFile, kbHome, mockIn, cdc)
require.NoError(t, err)

t.Cleanup(func() {
Expand All @@ -268,7 +272,7 @@ func TestAddRecoverFileBackend(t *testing.T) {
})

mockIn.Reset(fmt.Sprintf("%s\n%s\n", keyringPassword, keyringPassword))
info, err := kb.Key("keyname1")
k, err := kb.Key("keyname1")
require.NoError(t, err)
require.Equal(t, "keyname1", info.GetName())
require.Equal(t, "keyname1", k.Name)
}
Loading

0 comments on commit 6cbbd6d

Please sign in to comment.