From b3fd840f796a7c29230aa43cca560110156cf45d Mon Sep 17 00:00:00 2001 From: Ruaridh Date: Wed, 6 May 2020 00:53:04 +0100 Subject: [PATCH] Update keys add cmd patch (#477) * update patch * add tests for patch --- cli_test/cli_test.go | 12 ++++++++++++ cmd/kvcli/cmd_keys.go | 40 ++++++++++++++++++++++++++-------------- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/cli_test/cli_test.go b/cli_test/cli_test.go index 40e9acc2..e60723ab 100644 --- a/cli_test/cli_test.go +++ b/cli_test/cli_test.go @@ -92,6 +92,18 @@ func TestKavaCLIKeysAddRecoverHDPath(t *testing.T) { f.KeysAddRecoverHDPath("test-recoverH5", "dentist task convince chimney quality leave banana trade firm crawl eternal easily", 2, 17, "--legacy-hd-path") require.Equal(t, "kava1v9plmhvyhgxk3th9ydacm7j4z357s3nhhmy0tv", f.KeyAddress("test-recoverH5").String()) + exitSuccess, _, _ := f.KeysAddRecover("test-recover-fail", "dentist task convince chimney quality leave banana trade firm crawl eternal easily", "--legacy-hd-path --hd-path 44'/459'/0'/0/0") + require.False(t, exitSuccess) + + // test -hd-path flag + exitSuccess, _, _ = f.KeysAddRecover("test-recoverH6", "dentist task convince chimney quality leave banana trade firm crawl eternal easily", "--hd-path 44'/459'/0'/0/0") + require.True(t, exitSuccess) + require.Equal(t, "kava1rsjxn2e4dfl3a2qzuzzjvvgjmmate383g9q4cz", f.KeyAddress("test-recoverH6").String()) + + exitSuccess, _, _ = f.KeysAddRecover("test-recoverH7", "dentist task convince chimney quality leave banana trade firm crawl eternal easily", "--hd-path 44'/459'/2'/0/17") + require.True(t, exitSuccess) + require.Equal(t, "kava1xvsfnksmhr887skcfrm4pe3va54tkmrtw7wyer", f.KeyAddress("test-recoverH7").String()) + // Cleanup testing directories f.Cleanup() } diff --git a/cmd/kvcli/cmd_keys.go b/cmd/kvcli/cmd_keys.go index c17d8927..add085b3 100644 --- a/cmd/kvcli/cmd_keys.go +++ b/cmd/kvcli/cmd_keys.go @@ -17,18 +17,28 @@ import ( NOTE TO FUTURE IMPLEMENTERS This monkey patches the sdk `keys` command, therefore needs to be reviewed on any sdk updates. -Where a bip44 coin type is used (cosmos-sdk 18de630d): -- adding local keys - - global variable `sdk.Config.CoinType` is used to derive the key from a mnemonic (supplied by user or generated), but only the private key is stored -- adding ledger keys - - global variable `sdk.Config.CoinType` is used to reference a key on a ledger device, bip44 path (not private key) is stored locally -- signing txs with local keys +The patch adds support for using kava's legacy bip44 coin type to the cli. +Coin types are used to create a bip44 derivation path, which is used as a mapping from mnemonic to private key. + +In cosmos-sdk v0.38.3, all private keys are stored without reference to the mnemonic or bip44 derivation path, except ledger keys. +Ledger keys are just references to a private key on a ledger device. They contain the bip44 derivation path. +To patch the cli, we only need to modify: +- when new ledger references are created +- anything to do with converting a mnemonic to a private key. + +These only happen in `kvcli keys add` cmd. +For private key generation, use a --legacy-hd-path flag to enable old coin type. +The current cosmos ledger app (v1.5.3) only supports the legacy coin type. So we only need to ensure ledger reference creation doesn't use the new coin type. + +Signing txs: +- with local keys - the stored the priv key is used to sign, mnemonics or bip44 paths not involved -- signing txs with ledger +- with ledger - the stored bip44 path is used to instruct the ledger which key to sign with */ const flagLegacyHDPath = "legacy-hd-path" +const flagHDPath = "hd-path" // this is copied from keys add cmd because it's not exported // getModifiedKeysCmd returns the standard cosmos-sdk/client/keys cmd but modified to support new and old bip44 coin types supported by kava. func getModifiedKeysCmd() *cobra.Command { @@ -70,18 +80,20 @@ func monkeyPatchCmdKeysAdd(keysAddCmd *cobra.Command) { // replace the run function with a wrapped version that sets the old coin type in the global config oldRun := keysAddCmd.RunE keysAddCmd.RunE = func(cmd *cobra.Command, args []string) error { - preExistingCoinType := sdk.GetConfig().GetCoinType() + if !viper.GetBool(flagLegacyHDPath) && viper.GetBool(flags.FlagUseLedger) { + return fmt.Errorf("cosmos ledger app only supports legacy bip44 coin type, must use --%s flag when adding ledger key", flagLegacyHDPath) + } + if viper.GetBool(flagLegacyHDPath) && viper.IsSet(flagHDPath) { + return fmt.Errorf("cannot use a custom hd path (--%s) and legacy bip44 coin type (--%s) at the same time", flagHDPath, flagLegacyHDPath) + } if viper.GetBool(flagLegacyHDPath) { + preExistingCoinType := sdk.GetConfig().GetCoinType() sdk.GetConfig().SetCoinType(sdk.CoinType) // set old coin type err := oldRun(cmd, args) - sdk.GetConfig().SetCoinType(preExistingCoinType) // revert to preexisting coin type + sdk.GetConfig().SetCoinType(preExistingCoinType) // revert to pre-existing coin type return err - } else { - if viper.GetBool(flags.FlagUseLedger) { - return fmt.Errorf("cosmos ledger app only supports legacy bip44 coin type, must use --%s flag when adding ledger key", flagLegacyHDPath) - } - return oldRun(cmd, args) } + return oldRun(cmd, args) } }