From bc5826cd80fc4da8c03a3c918f2644bef421e66b Mon Sep 17 00:00:00 2001 From: Ruaridh Date: Thu, 13 Feb 2020 00:49:32 +0000 Subject: [PATCH] Add cli support for new bip44 coin type (#364) * first attempt * refactor, add tests * update comments --- app/app.go | 7 ++++ cli_test/cli_test.go | 19 +++++++--- cmd/kvcli/cmd_keys.go | 86 +++++++++++++++++++++++++++++++++++++++++++ cmd/kvcli/main.go | 8 ++-- cmd/kvd/main.go | 3 +- 5 files changed, 113 insertions(+), 10 deletions(-) create mode 100644 cmd/kvcli/cmd_keys.go diff --git a/app/app.go b/app/app.go index be60f667..5d74f728 100644 --- a/app/app.go +++ b/app/app.go @@ -34,6 +34,7 @@ import ( const ( appName = "kava" Bech32MainPrefix = "kava" + Bip44CoinType = 459 // see https://github.com/satoshilabs/slips/blob/master/slip-0044.md ) var ( @@ -302,12 +303,18 @@ func MakeCodec() *codec.Codec { return cdc.Seal() } +// SetBech32AddressPrefixes sets the global prefix to be used when serializing addresses to bech32 strings. func SetBech32AddressPrefixes(config *sdk.Config) { config.SetBech32PrefixForAccount(Bech32MainPrefix, Bech32MainPrefix+sdk.PrefixPublic) config.SetBech32PrefixForValidator(Bech32MainPrefix+sdk.PrefixValidator+sdk.PrefixOperator, Bech32MainPrefix+sdk.PrefixValidator+sdk.PrefixOperator+sdk.PrefixPublic) config.SetBech32PrefixForConsensusNode(Bech32MainPrefix+sdk.PrefixValidator+sdk.PrefixConsensus, Bech32MainPrefix+sdk.PrefixValidator+sdk.PrefixConsensus+sdk.PrefixPublic) } +// SetBip44CoinType sets the global coin type to be used in hierarchical deterministic wallets. +func SetBip44CoinType(config *sdk.Config) { + config.SetCoinType(Bip44CoinType) +} + // application updates every end block func (app *App) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock { return app.mm.BeginBlock(ctx, req) diff --git a/cli_test/cli_test.go b/cli_test/cli_test.go index fe8079d8..3c5995b9 100644 --- a/cli_test/cli_test.go +++ b/cli_test/cli_test.go @@ -61,7 +61,12 @@ func TestGaiaCLIKeysAddRecover(t *testing.T) { exitSuccess, _, _ = f.KeysAddRecover("test-recover", "dentist task convince chimney quality leave banana trade firm crawl eternal easily") require.True(t, exitSuccess) - require.Equal(t, "kava1qcfdf69js922qrdr4yaww3ax7gjml6pd39p8lj", f.KeyAddress("test-recover").String()) + require.Equal(t, "kava1rsjxn2e4dfl3a2qzuzzjvvgjmmate383g9q4cz", f.KeyAddress("test-recover").String()) + + // test old bip44 coin type + exitSuccess, _, _ = f.KeysAddRecover("test-recover-legacy", "dentist task convince chimney quality leave banana trade firm crawl eternal easily", "--legacy-hd-path") + require.True(t, exitSuccess) + require.Equal(t, "kava1qcfdf69js922qrdr4yaww3ax7gjml6pd39p8lj", f.KeyAddress("test-recover-legacy").String()) // Cleanup testing directories f.Cleanup() @@ -72,16 +77,20 @@ func TestGaiaCLIKeysAddRecoverHDPath(t *testing.T) { f := InitFixtures(t) f.KeysAddRecoverHDPath("test-recoverHD1", "dentist task convince chimney quality leave banana trade firm crawl eternal easily", 0, 0) - require.Equal(t, "kava1qcfdf69js922qrdr4yaww3ax7gjml6pd39p8lj", f.KeyAddress("test-recoverHD1").String()) + require.Equal(t, "kava1rsjxn2e4dfl3a2qzuzzjvvgjmmate383g9q4cz", f.KeyAddress("test-recoverHD1").String()) f.KeysAddRecoverHDPath("test-recoverH2", "dentist task convince chimney quality leave banana trade firm crawl eternal easily", 1, 5) - require.Equal(t, "kava1pdfav2cjhry9k79nu6r8kgknnjtq6a7rcr0qlr", f.KeyAddress("test-recoverH2").String()) + require.Equal(t, "kava1qpj6nstqn0n5gzcsaezspuhulje6msjq5t8cq5", f.KeyAddress("test-recoverH2").String()) f.KeysAddRecoverHDPath("test-recoverH3", "dentist task convince chimney quality leave banana trade firm crawl eternal easily", 1, 17) - require.Equal(t, "kava1909k354n6wl8ujzu6kmh49w4d02ax7qvrrcgx5", f.KeyAddress("test-recoverH3").String()) + require.Equal(t, "kava1vayfpstgapt7dmv7074kc3ll8xpf0rlzvh4k08", f.KeyAddress("test-recoverH3").String()) f.KeysAddRecoverHDPath("test-recoverH4", "dentist task convince chimney quality leave banana trade firm crawl eternal easily", 2, 17) - require.Equal(t, "kava1v9plmhvyhgxk3th9ydacm7j4z357s3nhhmy0tv", f.KeyAddress("test-recoverH4").String()) + require.Equal(t, "kava1xvsfnksmhr887skcfrm4pe3va54tkmrtw7wyer", f.KeyAddress("test-recoverH4").String()) + + // test old bip44 coin type + 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()) // Cleanup testing directories f.Cleanup() diff --git a/cmd/kvcli/cmd_keys.go b/cmd/kvcli/cmd_keys.go new file mode 100644 index 00000000..d1bc35cd --- /dev/null +++ b/cmd/kvcli/cmd_keys.go @@ -0,0 +1,86 @@ +package main + +import ( + "fmt" + + "github.com/cosmos/cosmos-sdk/client/flags" + "github.com/cosmos/cosmos-sdk/client/keys" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/spf13/cobra" + "github.com/spf13/viper" + + "github.com/kava-labs/kava/app" +) + +/* +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 stored the priv key is used to sign, mnemonics or bip44 paths not involved +- signing txs with ledger + - the stored bip44 path is used to instruct the ledger which key to sign with +*/ + +const flagLegacyHDPath = "legacy-hd-path" + +// 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 { + keysCmd := keys.Commands() + for _, c := range keysCmd.Commands() { + if c.Name() == "add" { + monkeyPatchCmdKeysAdd(c) + break + } + } + return keysCmd +} + +// monkeyPatchCmdKeysAdd modifies the `keys add` command to use the old bip44 coin type when a flag is passed. +func monkeyPatchCmdKeysAdd(keysAddCmd *cobra.Command) { + // add flag + keysAddCmd.Flags().Bool(flagLegacyHDPath, false, fmt.Sprintf("Use the old bip44 coin type (%d) to derive addresses from mnemonics.", sdk.CoinType)) + + // replace description + keysAddCmd.Long = fmt.Sprintf(`Derive a new private key and encrypt to disk. + Optionally specify a BIP39 mnemonic, a BIP39 passphrase to further secure the mnemonic, + and BIP44 account/index numbers to derive a specific key. The key will be stored under the given name + and encrypted with the given password. + + NOTE: This cli defaults to Kava's BIP44 coin type %d. Use the --%s flag to use the old one (%d). + + The flag --recover allows one to recover a key from a seed passphrase. + If run with --dry-run, a key would be generated (or recovered) but not stored to the + local keystore. + Use the --pubkey flag to add arbitrary public keys to the keystore for constructing + multisig transactions. + + You can add a multisig key by passing the list of key names you want the public + key to be composed of to the --multisig flag and the minimum number of signatures + required through --multisig-threshold. The keys are sorted by address, unless + the flag --nosort is set. + `, app.Bip44CoinType, flagLegacyHDPath, sdk.CoinType) + + // 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) { + sdk.GetConfig().SetCoinType(sdk.CoinType) // set old coin type + err := oldRun(cmd, args) + sdk.GetConfig().SetCoinType(preExistingCoinType) // revert to preexisting 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) + } + } +} diff --git a/cmd/kvcli/main.go b/cmd/kvcli/main.go index 2fac4819..9873421f 100644 --- a/cmd/kvcli/main.go +++ b/cmd/kvcli/main.go @@ -12,7 +12,6 @@ import ( "github.com/tendermint/tendermint/libs/cli" "github.com/cosmos/cosmos-sdk/client" - "github.com/cosmos/cosmos-sdk/client/keys" "github.com/cosmos/cosmos-sdk/client/lcd" "github.com/cosmos/cosmos-sdk/client/rpc" sdk "github.com/cosmos/cosmos-sdk/types" @@ -33,10 +32,11 @@ func main() { // Instantiate the codec for the command line application cdc := app.MakeCodec() - // Read in the configuration file for the sdk + // Set the global config + // config is not sealed as the cli supports two coin types for legacy reasons. config := sdk.GetConfig() app.SetBech32AddressPrefixes(config) - config.Seal() + app.SetBip44CoinType(config) // TODO: setup keybase, viper object, etc. to be passed into // the below functions and eliminate global vars, like we do @@ -62,7 +62,7 @@ func main() { client.LineBreak, lcd.ServeCommand(cdc, registerRoutes), client.LineBreak, - keys.Commands(), + getModifiedKeysCmd(), client.LineBreak, version.Cmd, client.NewCompletionCmd(rootCmd, true), diff --git a/cmd/kvd/main.go b/cmd/kvd/main.go index 587b9543..bef94402 100644 --- a/cmd/kvd/main.go +++ b/cmd/kvd/main.go @@ -18,9 +18,9 @@ import ( "github.com/cosmos/cosmos-sdk/server" "github.com/cosmos/cosmos-sdk/store" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth" genutilcli "github.com/cosmos/cosmos-sdk/x/genutil/client/cli" "github.com/cosmos/cosmos-sdk/x/staking" - "github.com/cosmos/cosmos-sdk/x/auth" "github.com/kava-labs/kava/app" ) @@ -35,6 +35,7 @@ func main() { config := sdk.GetConfig() app.SetBech32AddressPrefixes(config) + app.SetBip44CoinType(config) config.Seal() ctx := server.NewDefaultContext()