diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f7708211..0a08488f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +* [\#9480](https://github.com/cosmos/cosmos-sdk/pull/9480) Fix added keys when using `--dry-run`. * [\#9540](https://github.com/cosmos/cosmos-sdk/pull/9540) Add output flag for query txs command. * [\#9460](https://github.com/cosmos/cosmos-sdk/pull/9460) Fix lint error in `MigratePrefixAddress`. * [\#9231](https://github.com/cosmos/cosmos-sdk/pull/9231) Remove redundant staking errors. diff --git a/client/keys/add.go b/client/keys/add.go index d04363205..2e915925d 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -88,7 +88,7 @@ func runAddCmdPrepare(cmd *cobra.Command, args []string) error { return err } - return RunAddCmd(clientCtx, cmd, args, buf) + return runAddCmd(clientCtx, cmd, args, buf) } /* @@ -100,7 +100,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] @@ -117,7 +117,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 @@ -138,19 +141,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 { @@ -160,12 +163,12 @@ 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) } } @@ -176,8 +179,13 @@ func RunAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf if err != nil { return err } - _, err := kb.SavePubKey(name, pk, algo.Name()) - return err + + info, err := kb.SavePubKey(name, pk, algo.Name()) + if err != nil { + return err + } + + 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 index a5fcc304d..69ccb32a4 100644 --- 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" @@ -124,3 +126,66 @@ func Test_runAddCmdLedger(t *testing.T) { "PubKeySecp256k1{034FEF9CD7C4C63588D3B03FEB5281B9D232CBA34D6F3D71AEE59211FFBFE1FE87}", key1.GetPubKey().String()) } + +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, "testkey.info: key not found", err.Error()) + } + }) + } +} diff --git a/client/keys/add_test.go b/client/keys/add_test.go index aa6f68876..06375257a 100644 --- 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{}. + WithJSONCodec(appCodec). + WithKeyringDir(kbHome). + WithKeyring(kb) + ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx) + + path := sdk.GetConfig().GetFullBIP44Path() + _, 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, "testkey.info: key not found", err.Error()) + } + }) + } +} diff --git a/crypto/keyring/keyring.go b/crypto/keyring/keyring.go index 7ec770ae8..eb5762497 100644 --- a/crypto/keyring/keyring.go +++ b/crypto/keyring/keyring.go @@ -760,7 +760,7 @@ func (ks keystore) writeInfo(info Info) error { return err } if exists { - return errors.New("public key already exist in keybase") + return errors.New("public key already exists in keybase") } err = ks.db.Set(keyring.Item{