Remove inconsistent gaiacli keys new/add dualism (#2904)

* Remove inconsistent gaiacli keys new/add dualism
cosmos · Dec 4, 2018 · cfeb1b8 · cfeb1b8
1 parent 5e94650
commit cfeb1b8
Show file tree

Hide file tree

Showing 6 changed files with 176 additions and 239 deletions.
diff --git a/PENDING.md b/PENDING.md
@@ -11,8 +11,7 @@ BREAKING CHANGES
   * [cli] [\#2786](https://github.com/cosmos/cosmos-sdk/pull/2786) Fix redelegation command flow
   * [cli] [\#2829](https://github.com/cosmos/cosmos-sdk/pull/2829) add-genesis-account command now validates state when adding accounts
   * [cli] [\#2804](https://github.com/cosmos/cosmos-sdk/issues/2804) Check whether key exists before passing it on to `tx create-validator`.
-  * [cli] [\#2874](https://github.com/cosmos/cosmos-sdk/pull/2874) `gaiacli tx sign` takes an optional `--output-document` flag to support output redirection.
-  * [cli] [\#2875](https://github.com/cosmos/cosmos-sdk/pull/2875) Refactor `gaiad gentx` and avoid redirection to `gaiacli tx sign` for tx signing.
+  * [cli] [\#2595](https://github.com/cosmos/cosmos-sdk/issues/2595) Remove `keys new` in favor of `keys add` incorporating existing functionality with addition of key recovery functionality.
 
 * Gaia
   * [mint] [\#2825] minting now occurs every block, inflation parameter updates still hourly
@@ -67,6 +66,8 @@ IMPROVEMENTS
 * Gaia CLI  (`gaiacli`)
   * [\#2749](https://github.com/cosmos/cosmos-sdk/pull/2749) Add --chain-id flag to gaiad testnet
   * [\#2819](https://github.com/cosmos/cosmos-sdk/pull/2819) Tx search now supports multiple tags as query parameters
+  * [\#2874](https://github.com/cosmos/cosmos-sdk/pull/2874) `gaiacli tx sign` takes an optional `--output-document` flag to support output redirection.
+  * [\#2875](https://github.com/cosmos/cosmos-sdk/pull/2875) Refactor `gaiad gentx` and avoid redirection to `gaiacli tx sign` for tx signing.
 
 * Gaia
   - #2772 Update BaseApp to not persist state when the ante handler fails on DeliverTx.

diff --git a/client/keys/add.go b/client/keys/add.go
@@ -5,38 +5,50 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"os"
 
-	"github.com/cosmos/cosmos-sdk/client"
+	"github.com/cosmos/go-bip39"
 	"github.com/gorilla/mux"
+	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
+	"github.com/tendermint/tendermint/libs/cli"
 
+	"github.com/cosmos/cosmos-sdk/client"
 	ccrypto "github.com/cosmos/cosmos-sdk/crypto"
 	"github.com/cosmos/cosmos-sdk/crypto/keys"
-
-	"github.com/tendermint/tendermint/libs/cli"
+	"github.com/cosmos/cosmos-sdk/crypto/keys/hd"
 )
 
 const (
-	flagType     = "type"
-	flagRecover  = "recover"
-	flagNoBackup = "no-backup"
-	flagDryRun   = "dry-run"
-	flagAccount  = "account"
-	flagIndex    = "index"
+	flagInteractive = "interactive"
+	flagBIP44Path   = "bip44-path"
+	flagRecover     = "recover"
+	flagNoBackup    = "no-backup"
+	flagDryRun      = "dry-run"
+	flagAccount     = "account"
+	flagIndex       = "index"
 )
 
 func addKeyCommand() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "add <name>",
-		Short: "Create a new key, or import from seed",
-		Long: `Add a public/private key pair to the key store.
-If you select --seed/-s you can recover a key from the seed
-phrase, otherwise, a new key will be generated.`,
+		Short: "Add an encrypted private key (either newly generated or recovered), encrypt it, and save to disk",
+		Long: `Derive a new private key and encrypt to disk.
+Optionally specify a BIP39 mnemonic, a BIP39 passphrase to further secure the mnemonic,
+and a bip32 HD path to derive a specific account. The key will be stored under the given name 
+and encrypted with the given password. The only input that is required is the encryption password.
+
+If run with -i, it will prompt the user for BIP44 path, BIP39 mnemonic, and passphrase.
+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.
+`,
+		Args: cobra.ExactArgs(1),
 		RunE: runAddCmd,
 	}
-	cmd.Flags().StringP(flagType, "t", "secp256k1", "Type of private key (secp256k1|ed25519)")
+	cmd.Flags().BoolP(flagInteractive, "i", false, "Interactively prompt user for BIP39 passphrase and mnemonic")
 	cmd.Flags().Bool(client.FlagUseLedger, false, "Store a local reference to a private key on a Ledger device")
+	cmd.Flags().String(flagBIP44Path, "44'/118'/0'/0/0", "BIP44 path from which to derive a private key")
 	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")
@@ -45,24 +57,28 @@ phrase, otherwise, a new key will be generated.`,
 	return cmd
 }
 
-// TODO remove the above when addressing #1446
+/*
+input
+	- bip39 mnemonic
+	- bip39 passphrase
+	- bip44 path
+	- local encryption password
+output
+	- armor encrypted private key (saved to file)
+*/
 func runAddCmd(cmd *cobra.Command, args []string) error {
 	var kb keys.Keybase
 	var err error
-	var name, pass string
+	var pass string
 
 	buf := client.BufferStdin()
+	name := args[0]
 	if viper.GetBool(flagDryRun) {
 		// we throw this away, so don't enforce args,
 		// we want to get a new random seed phrase quickly
 		kb = client.MockKeyBase()
 		pass = "throwing-this-key-away"
-		name = "inmemorykey"
 	} else {
-		if len(args) != 1 || len(args[0]) == 0 {
-			return errMissingName()
-		}
-		name = args[0]
 		kb, err = GetKeyBaseWithWritePerm()
 		if err != nil {
 			return err
@@ -80,25 +96,40 @@ func runAddCmd(cmd *cobra.Command, args []string) error {
 		// ask for a password when generating a local key
 		if !viper.GetBool(client.FlagUseLedger) {
 			pass, err = client.GetCheckPassword(
-				"Enter a passphrase for your key:",
-				"Repeat the passphrase:", buf)
+				"> Enter a passphrase for your key:",
+				"> Repeat the passphrase:", buf)
 			if err != nil {
 				return err
 			}
 		}
 	}
 
+	interactive := viper.GetBool(flagInteractive)
+	flags := cmd.Flags()
+	bipFlag := flags.Lookup(flagBIP44Path)
+
+	bip44Params, err := getBIP44ParamsAndPath(bipFlag.Value.String(), bipFlag.Changed || !interactive)
+	if err != nil {
+		return err
+	}
+
+	// If we're using ledger, only thing we need is the path. So generate key and
+	// we're done.
 	if viper.GetBool(client.FlagUseLedger) {
 		account := uint32(viper.GetInt(flagAccount))
 		index := uint32(viper.GetInt(flagIndex))
 		path := ccrypto.DerivationPath{44, 118, account, 0, index}
-		algo := keys.SigningAlgo(viper.GetString(flagType))
-		info, err := kb.CreateLedger(name, path, algo)
+		info, err := kb.CreateLedger(name, path, keys.Secp256k1)
 		if err != nil {
 			return err
 		}
+
 		printCreate(info, "")
-	} else if viper.GetBool(flagRecover) {
+		return nil
+	}
+
+	// Recover key from seed passphrase
+	if viper.GetBool(flagRecover) {
 		seed, err := client.GetSeed(
 			"Enter your recovery seed phrase:", buf)
 		if err != nil {
@@ -111,29 +142,108 @@ func runAddCmd(cmd *cobra.Command, args []string) error {
 		// print out results without the seed phrase
 		viper.Set(flagNoBackup, true)
 		printCreate(info, "")
-	} else {
-		algo := keys.SigningAlgo(viper.GetString(flagType))
-		info, seed, err := kb.CreateMnemonic(name, keys.English, pass, algo)
+		return nil
+	}
+
+	var mnemonic string
+	if interactive {
+		mnemonic, err = client.GetString("Enter your bip39 mnemonic, or hit enter to generate one.", buf)
+		if err != nil {
+			return err
+		}
+	}
+
+	if len(mnemonic) == 0 {
+		// read entropy seed straight from crypto.Rand and convert to mnemonic
+		entropySeed, err := bip39.NewEntropy(mnemonicEntropySize)
+		if err != nil {
+			return err
+		}
+
+		mnemonic, err = bip39.NewMnemonic(entropySeed[:])
+		if err != nil {
+			return err
+		}
+	}
+
+	// get bip39 passphrase
+	var bip39Passphrase string
+	if interactive {
+		bip39Passphrase, err = client.GetString(
+			"Enter your bip39 passphrase. This is combined with the mnemonic to derive the seed. "+
+				"Most users should just hit enter to use the default, \"\"", buf)
 		if err != nil {
 			return err
 		}
-		printCreate(info, seed)
+
+		// if they use one, make them re-enter it
+		if len(bip39Passphrase) != 0 {
+			p2, err := client.GetString("Repeat the passphrase:", buf)
+			if err != nil {
+				return err
+			}
+
+			if bip39Passphrase != p2 {
+				return errors.New("passphrases don't match")
+			}
+		}
+	}
+
+	// get the encryption password
+	encryptPassword, err := client.GetCheckPassword(
+		"> Enter a passphrase to encrypt your key to disk:",
+		"> Repeat the passphrase:", buf)
+	if err != nil {
+		return err
 	}
+
+	info, err := kb.Derive(name, mnemonic, bip39Passphrase, encryptPassword, *bip44Params)
+	if err != nil {
+		return err
+	}
+	printCreate(info, mnemonic)
 	return nil
 }
 
+func getBIP44ParamsAndPath(path string, flagSet bool) (*hd.BIP44Params, error) {
+	buf := client.BufferStdin()
+	bip44Path := path
+
+	// if it wasn't set in the flag, give it a chance to overide interactively
+	if !flagSet {
+		var err error
+
+		bip44Path, err = client.GetString(fmt.Sprintf("Enter your bip44 path. Default is %s\n", path), buf)
+		if err != nil {
+			return nil, err
+		}
+
+		if len(bip44Path) == 0 {
+			bip44Path = path
+		}
+	}
+
+	bip44params, err := hd.NewParamsFromPath(bip44Path)
+	if err != nil {
+		return nil, err
+	}
+
+	return bip44params, nil
+}
+
 func printCreate(info keys.Info, seed string) {
 	output := viper.Get(cli.OutputFlag)
 	switch output {
 	case "text":
+		fmt.Fprintln(os.Stderr, "")
 		printKeyInfo(info, Bech32KeyOutput)
 
 		// print seed unless requested not to.
 		if !viper.GetBool(client.FlagUseLedger) && !viper.GetBool(flagNoBackup) {
-			fmt.Println("**Important** write this seed phrase in a safe place.")
-			fmt.Println("It is the only way to recover your account if you ever forget your password.")
-			fmt.Println()
-			fmt.Println(seed)
+			fmt.Fprintln(os.Stderr, "\n**Important** write this seed phrase in a safe place.")
+			fmt.Fprintln(os.Stderr, "It is the only way to recover your account if you ever forget your password.")
+			fmt.Fprintln(os.Stderr)
+			fmt.Fprintln(os.Stderr, seed)
 		}
 	case "json":
 		out, err := Bech32KeyOutput(info)
@@ -152,12 +262,29 @@ func printCreate(info keys.Info, seed string) {
 		if err != nil {
 			panic(err) // really shouldn't happen...
 		}
-		fmt.Println(string(jsonString))
+		fmt.Fprintln(os.Stderr, string(jsonString))
 	default:
 		panic(fmt.Sprintf("I can't speak: %s", output))
 	}
 }
 
+// function to just a new seed to display in the UI before actually persisting it in the keybase
+func getSeed(algo keys.SigningAlgo) string {
+	kb := client.MockKeyBase()
+	pass := "throwing-this-key-away"
+	name := "inmemorykey"
+	_, seed, _ := kb.CreateMnemonic(name, keys.English, pass, algo)
+	return seed
+}
+
+func printPrefixed(msg string) {
+	fmt.Fprintln(os.Stderr, msg)
+}
+
+func printStep() {
+	printPrefixed("-------------------------------------")
+}
+
 /////////////////////////////
 // REST
 
@@ -242,15 +369,6 @@ func AddNewKeyRequestHandler(indent bool) http.HandlerFunc {
 	}
 }
 
-// function to just a new seed to display in the UI before actually persisting it in the keybase
-func getSeed(algo keys.SigningAlgo) string {
-	kb := client.MockKeyBase()
-	pass := "throwing-this-key-away"
-	name := "inmemorykey"
-	_, seed, _ := kb.CreateMnemonic(name, keys.English, pass, algo)
-	return seed
-}
-
 // Seed REST request handler
 func SeedRequestHandler(w http.ResponseWriter, r *http.Request) {
 	vars := mux.Vars(r)