fix: added key when dry-run is true (cosmos#9480)

RiccardoM · RiccardoM · commit 42c87671b67d · 2021-07-27T14:11:19.000+02:00
(cherry picked from commit 7679820)
diff --git a/client/keys/add.go b/client/keys/add.go
@@ -89,7 +89,7 @@ func runAddCmdPrepare(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	return RunAddCmd(clientCtx, cmd, args, buf)
+	return runAddCmd(clientCtx, cmd, args, buf)
 }
 
 /*
@@ -101,7 +101,7 @@ input
 output
 	- armor encrypted private key (saved to file)
 */
-func RunAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *bufio.Reader) error {
+func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *bufio.Reader) error {
 	var err error
 
 	name := args[0]
@@ -118,7 +118,10 @@ func RunAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf
 		return err
 	}
 
-	if dryRun, _ := cmd.Flags().GetBool(flags.FlagDryRun); !dryRun {
+	if dryRun, _ := cmd.Flags().GetBool(flags.FlagDryRun); dryRun {
+		// use in memory keybase
+		kb = keyring.NewInMemory()
+	} else {
 		_, err = kb.Key(name)
 		if err == nil {
 			// account exists, ask for user confirmation
@@ -139,20 +142,19 @@ func RunAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf
 
 		multisigKeys, _ := cmd.Flags().GetStringSlice(flagMultisig)
 		if len(multisigKeys) != 0 {
-			var pks []cryptotypes.PubKey
-
+			pks := make([]cryptotypes.PubKey, len(multisigKeys))
 			multisigThreshold, _ := cmd.Flags().GetInt(flagMultiSigThreshold)
 			if err := validateMultisigThreshold(multisigThreshold, len(multisigKeys)); err != nil {
 				return err
 			}
 
-			for _, keyname := range multisigKeys {
+			for i, keyname := range multisigKeys {
 				k, err := kb.Key(keyname)
 				if err != nil {
 					return err
 				}
 
-				pks = append(pks, k.GetPubKey())
+				pks[i] = k.GetPubKey()
 			}
 
 			if noSort, _ := cmd.Flags().GetBool(flagNoSort); !noSort {
@@ -162,27 +164,29 @@ func RunAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf
 			}
 
 			pk := multisig.NewLegacyAminoPubKey(multisigThreshold, pks)
-			if _, err := kb.SaveMultisig(name, pk); err != nil {
+			info, err := kb.SaveMultisig(name, pk)
+			if err != nil {
 				return err
 			}
 
-			cmd.PrintErrf("Key %q saved to disk.\n", name)
-			return nil
+			return printCreate(cmd, info, false, "", outputFormat)
 		}
 	}
 
 	pubKey, _ := cmd.Flags().GetString(FlagPublicKey)
 	if pubKey != "" {
-		pk, err := sdk.GetPubKeyFromBech32(sdk.Bech32PubKeyTypeAccPub, pubKey)
+		var pk cryptotypes.PubKey
+		err = ctx.JSONMarshaler.UnmarshalInterfaceJSON([]byte(pubKey), &pk)
 		if err != nil {
 			return err
 		}
 
-		if _, err := kb.SavePubKey(name, pk, algo.Name()); err != nil {
+		info, err := kb.SavePubKey(name, pk, algo.Name())
+		if err != nil {
 			return err
 		}
 
-		return nil
+		return printCreate(cmd, info, false, "", outputFormat)
 	}
 
 	coinType, _ := cmd.Flags().GetUint32(flagCoinType)
diff --git a/client/keys/add_ledger_test.go b/client/keys/add_ledger_test.go
@@ -3,8 +3,10 @@
 package keys
 
 import (
+	"bytes"
 	"context"
 	"fmt"
+	"io/ioutil"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -125,3 +127,66 @@ func Test_runAddCmdLedger(t *testing.T) {
 		"cosmospub1addwnpepqd87l8xhcnrrtzxnkql7k55ph8fr9jarf4hn6udwukfprlalu8lgw0urza0",
 		sdk.MustBech32ifyPubKey(sdk.Bech32PubKeyTypeAccPub, key1.GetPubKey()))
 }
+
+func Test_runAddCmdLedgerDryRun(t *testing.T) {
+	testData := []struct {
+		name  string
+		args  []string
+		added bool
+	}{
+		{
+			name: "ledger account is added",
+			args: []string{
+				"testkey",
+				fmt.Sprintf("--%s=%s", flags.FlagDryRun, "false"),
+				fmt.Sprintf("--%s=%s", flags.FlagUseLedger, "true"),
+			},
+			added: true,
+		},
+		{
+			name: "ledger account is not added with dry run",
+			args: []string{
+				"testkey",
+				fmt.Sprintf("--%s=%s", flags.FlagDryRun, "true"),
+				fmt.Sprintf("--%s=%s", flags.FlagUseLedger, "true"),
+			},
+			added: false,
+		},
+	}
+	for _, tt := range testData {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			cmd := AddKeyCommand()
+			cmd.Flags().AddFlagSet(Commands("home").PersistentFlags())
+
+			kbHome := t.TempDir()
+			mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
+			kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn)
+			require.NoError(t, err)
+
+			clientCtx := client.Context{}.
+				WithKeyringDir(kbHome).
+				WithKeyring(kb)
+			ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)
+
+			b := bytes.NewBufferString("")
+			cmd.SetOut(b)
+
+			cmd.SetArgs(tt.args)
+			require.NoError(t, cmd.ExecuteContext(ctx))
+
+			if tt.added {
+				_, err = kb.Key("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")
+				require.Error(t, err)
+				require.Equal(t, "The specified item could not be found in the keyring", err.Error())
+			}
+		})
+	}
+}
diff --git a/client/keys/add_test.go b/client/keys/add_test.go
@@ -1,8 +1,10 @@
 package keys
 
 import (
+	"bytes"
 	"context"
 	"fmt"
+	"io/ioutil"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -13,6 +15,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"
 )
@@ -114,3 +117,113 @@ func Test_runAddCmdBasic(t *testing.T) {
 	mockIn.Reset("\n" + password + "\n" + "fail" + "\n")
 	require.Error(t, cmd.ExecuteContext(ctx))
 }
+
+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"}`
+
+	testData := []struct {
+		name  string
+		args  []string
+		added bool
+	}{
+		{
+			name: "account is added",
+			args: []string{
+				"testkey",
+				fmt.Sprintf("--%s=%s", flags.FlagDryRun, "false"),
+			},
+			added: true,
+		},
+		{
+			name: "account is not added with dry run",
+			args: []string{
+				"testkey",
+				fmt.Sprintf("--%s=%s", flags.FlagDryRun, "true"),
+			},
+			added: false,
+		},
+		{
+			name: "multisig account is added",
+			args: []string{
+				"testkey",
+				fmt.Sprintf("--%s=%s", flags.FlagDryRun, "false"),
+				fmt.Sprintf("--%s=%s", flagMultisig, "subkey"),
+			},
+			added: true,
+		},
+		{
+			name: "multisig account is not added with dry run",
+			args: []string{
+				"testkey",
+				fmt.Sprintf("--%s=%s", flags.FlagDryRun, "true"),
+				fmt.Sprintf("--%s=%s", flagMultisig, "subkey"),
+			},
+			added: false,
+		},
+		{
+			name: "pubkey account is added",
+			args: []string{
+				"testkey",
+				fmt.Sprintf("--%s=%s", flags.FlagDryRun, "false"),
+				fmt.Sprintf("--%s=%s", FlagPublicKey, pubkey1),
+			},
+			added: true,
+		},
+		{
+			name: "pubkey account is not added with dry run",
+			args: []string{
+				"testkey",
+				fmt.Sprintf("--%s=%s", flags.FlagDryRun, "true"),
+				fmt.Sprintf("--%s=%s", FlagPublicKey, pubkey2),
+			},
+			added: false,
+		},
+	}
+	for _, tt := range testData {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			cmd := AddKeyCommand()
+			cmd.Flags().AddFlagSet(Commands("home").PersistentFlags())
+
+			kbHome := t.TempDir()
+			mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
+			kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn)
+			require.NoError(t, err)
+
+			appCodec := simapp.MakeTestEncodingConfig().Marshaler
+			clientCtx := client.Context{}.
+				WithJSONMarshaler(appCodec).
+				WithKeyringDir(kbHome).
+				WithKeyring(kb)
+			ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)
+
+			path := sdk.GetConfig().GetFullFundraiserPath()
+			_, err = kb.NewAccount("subkey", testutil.TestMnemonic, "", path, hd.Secp256k1)
+			require.NoError(t, err)
+
+			t.Cleanup(func() {
+				_ = kb.Delete("subkey")
+			})
+
+			b := bytes.NewBufferString("")
+			cmd.SetOut(b)
+
+			cmd.SetArgs(tt.args)
+			require.NoError(t, cmd.ExecuteContext(ctx))
+
+			if tt.added {
+				_, err = kb.Key("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")
+				require.Error(t, err)
+				require.Equal(t, "The specified item could not be found in the keyring", err.Error())
+			}
+		})
+	}
+}
diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go
@@ -744,7 +744,7 @@ func (ks keystore) writeInfo(info Info) error {
 
 	exists, err := ks.existsInDb(info)
 	if exists {
-		return errors.New("public key already exist in keybase")
+		return errors.New("public key already exists in keybase")
 	}
 
 	if err != nil {
diff --git a/crypto/keyring/legacy_test.go b/crypto/keyring/legacy_test.go
@@ -53,5 +53,5 @@ func TestLegacyKeybase(t *testing.T) {
 
 	err = importer.Import("test", armoredInfo)
 	require.Error(t, err)
-	require.Equal(t, `public key already exist in keybase`, err.Error())
+	require.Equal(t, `public key already exists in keybase`, err.Error())
 }

Original file line number	Diff line number	Diff line change
`@@ -744,7 +744,7 @@ func (ks keystore) writeInfo(info Info) error {`
`744`	`744`
`745`	`745`	`exists, err := ks.existsInDb(info)`
`746`	`746`	`if exists {`
`747`		`- return errors.New("public key already exist in keybase")`
	`747`	`+ return errors.New("public key already exists in keybase")`
`748`	`748`	`}`
`749`	`749`
`750`	`750`	`if err != nil {`
Original file line number	Diff line number	Diff line change
`@@ -53,5 +53,5 @@ func TestLegacyKeybase(t *testing.T) {`
`53`	`53`
`54`	`54`	`err = importer.Import("test", armoredInfo)`
`55`	`55`	`require.Error(t, err)`
`56`		- require.Equal(t, `public key already exist in keybase`, err.Error())
	`56`	+ require.Equal(t, `public key already exists in keybase`, err.Error())
`57`	`57`	`}`