diff --git a/.gitignore b/.gitignore index 43e8b06a15..0fc9f90295 100644 --- a/.gitignore +++ b/.gitignore @@ -18,6 +18,7 @@ dist tools-stamp buf-stamp artifacts +simapp/simd/simd # Go go.work diff --git a/CHANGELOG.md b/CHANGELOG.md index 9be3419529..0cc91d2382 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* (client/keys) [#18703](https://github.com/cosmos/cosmos-sdk/pull/18703) Improve ` keys add` and ` keys show` by checking whether there are duplicate keys in the multisig case. * (x/gov) [#18707](https://github.com/cosmos/cosmos-sdk/pull/18707) Improve genesis validation. * (x/bank) [#18636](https://github.com/cosmos/cosmos-sdk/pull/18636) `SendCoinsFromModuleToAccount`, `SendCoinsFromModuleToModule`, `SendCoinsFromAccountToModule`, `DelegateCoinsFromAccountToModule`, `UndelegateCoinsFromModuleToAccount`, `MintCoins` and `BurnCoins` methods now returns an error instead of panicking if any module accounts does not exist or unauthorized. * (x/distribution) [#18636](https://github.com/cosmos/cosmos-sdk/pull/18636) `CalculateDelegationRewards` and `DelegationTotalRewards` methods no longer panics on any sanity checks and instead returns appropriate errors. diff --git a/client/keys/add.go b/client/keys/add.go index 527209c918..a30a2d865a 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -164,8 +164,14 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf return err } - for i, keyname := range multisigKeys { - k, err := kb.Key(keyname) + seenKeys := make(map[string]struct{}) + for i, keyName := range multisigKeys { + if _, ok := seenKeys[keyName]; ok { + return fmt.Errorf("duplicate multisig keys: %s", keyName) + } + seenKeys[keyName] = struct{}{} + + k, err := kb.Key(keyName) if err != nil { return err } diff --git a/client/keys/add_test.go b/client/keys/add_test.go index 84b81833bb..53ce3af279 100644 --- a/client/keys/add_test.go +++ b/client/keys/add_test.go @@ -127,6 +127,77 @@ func Test_runAddCmdBasic(t *testing.T) { require.Error(t, cmd.ExecuteContext(ctx)) } +func Test_runAddCmdMultisigDupKeys(t *testing.T) { + cmd := AddKeyCommand() + cmd.Flags().AddFlagSet(Commands().PersistentFlags()) + + mockIn := testutil.ApplyMockIODiscardOutErr(cmd) + kbHome := t.TempDir() + + cdc := moduletestutil.MakeTestEncodingConfig().Codec + kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn, cdc) + require.NoError(t, err) + + clientCtx := client.Context{}. + WithKeyringDir(kbHome). + WithInput(mockIn). + WithCodec(cdc). + WithAddressCodec(addresscodec.NewBech32Codec("cosmos")). + WithValidatorAddressCodec(addresscodec.NewBech32Codec("cosmosvaloper")). + WithConsensusAddressCodec(addresscodec.NewBech32Codec("cosmosvalcons")) + + ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx) + + t.Cleanup(func() { + _ = kb.Delete("keyname1") + _ = kb.Delete("keyname2") + _ = kb.Delete("multisigname") + }) + + cmd.SetArgs([]string{ + "keyname1", + fmt.Sprintf("--%s=%s", flags.FlagKeyringDir, kbHome), + fmt.Sprintf("--%s=%s", flags.FlagOutput, flags.OutputFormatText), + fmt.Sprintf("--%s=%s", flags.FlagKeyType, hd.Secp256k1Type), + fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest), + }) + require.NoError(t, cmd.ExecuteContext(ctx)) + + cmd.SetArgs([]string{ + "keyname2", + fmt.Sprintf("--%s=%s", flags.FlagKeyringDir, kbHome), + fmt.Sprintf("--%s=%s", flags.FlagOutput, flags.OutputFormatText), + fmt.Sprintf("--%s=%s", flags.FlagKeyType, hd.Secp256k1Type), + fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest), + }) + require.NoError(t, cmd.ExecuteContext(ctx)) + + cmd.SetArgs([]string{ + "multisigname", + fmt.Sprintf("--%s=%s", flags.FlagKeyringDir, kbHome), + fmt.Sprintf("--%s=%s", flags.FlagOutput, flags.OutputFormatText), + fmt.Sprintf("--%s=%s", flags.FlagKeyType, hd.Secp256k1Type), + fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest), + fmt.Sprintf("--%s=%s", flagMultisig, "keyname1,keyname2"), + fmt.Sprintf("--%s=%s", flagMultiSigThreshold, "2"), + }) + require.NoError(t, cmd.ExecuteContext(ctx)) + + cmd.SetArgs([]string{ + "multisigname", + fmt.Sprintf("--%s=%s", flags.FlagKeyringDir, kbHome), + fmt.Sprintf("--%s=%s", flags.FlagOutput, flags.OutputFormatText), + fmt.Sprintf("--%s=%s", flags.FlagKeyType, hd.Secp256k1Type), + fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest), + fmt.Sprintf("--%s=%s", flagMultisig, "keyname1,keyname1"), + fmt.Sprintf("--%s=%s", flagMultiSigThreshold, "2"), + }) + mockIn.Reset("y\n") + require.Error(t, cmd.ExecuteContext(ctx)) + mockIn.Reset("y\n") + require.EqualError(t, cmd.ExecuteContext(ctx), "duplicate multisig keys: keyname1") +} + 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"}` diff --git a/client/keys/show.go b/client/keys/show.go index 582e6ec9c1..afda1b5fd6 100644 --- a/client/keys/show.go +++ b/client/keys/show.go @@ -71,11 +71,20 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) { } } else { pks := make([]cryptotypes.PubKey, len(args)) - for i, keyref := range args { - k, err := fetchKey(clientCtx.Keyring, keyref, clientCtx.AddressCodec) - if err != nil { - return fmt.Errorf("%s is not a valid name or address: %w", keyref, err) + seenKeys := make(map[string]struct{}) + for i, keyRef := range args { + if _, ok := seenKeys[keyRef]; ok { + // we just show warning message instead of return error in case someone relies on this behavior. + cmd.PrintErrf("WARNING: duplicate keys found: %s.\n\n", keyRef) + } else { + seenKeys[keyRef] = struct{}{} } + + k, err := fetchKey(clientCtx.Keyring, keyRef, clientCtx.AddressCodec) + if err != nil { + return fmt.Errorf("%s is not a valid name or address: %w", keyRef, err) + } + key, err := k.GetPubKey() if err != nil { return err diff --git a/client/keys/show_test.go b/client/keys/show_test.go index 6763855259..5d7f9c0332 100644 --- a/client/keys/show_test.go +++ b/client/keys/show_test.go @@ -134,6 +134,18 @@ func Test_runShowCmd(t *testing.T) { }) require.EqualError(t, cmd.ExecuteContext(ctx), "threshold must be a positive integer") + // Now try multisig key duplicate + _, mockOut := testutil.ApplyMockIO(cmd) + cmd.SetArgs([]string{ + fakeKeyName1, fakeKeyName1, + fmt.Sprintf("--%s=%s", flags.FlagKeyringDir, kbHome), + fmt.Sprintf("--%s=%s", FlagBechPrefix, sdk.PrefixAccount), + fmt.Sprintf("--%s=2", flagMultiSigThreshold), + fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest), + }) + require.NoError(t, cmd.ExecuteContext(ctx)) + require.Contains(t, mockOut.String(), fmt.Sprintf("WARNING: duplicate keys found: %s", fakeKeyName1)) + cmd.SetArgs([]string{ fakeKeyName1, fakeKeyName2, fmt.Sprintf("--%s=%s", flags.FlagKeyringDir, kbHome),