feat(client/keys): check multisig key duplicate (backport #18703) (#24153)

Co-authored-by: levisyin <150114626+levisyin@users.noreply.github.com>
Co-authored-by: aljo242 <alex@interchainlabs.io>
This commit is contained in:
mergify[bot] 2025-03-27 15:43:01 +00:00 committed by GitHub
parent fc725a35d6
commit f4f9ea5a4a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 103 additions and 6 deletions

1
.gitignore vendored
View File

@ -18,6 +18,7 @@ dist
tools-stamp
buf-stamp
artifacts
simapp/simd/simd
# Go
go.work

View File

@ -56,6 +56,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Improvements
* (client/keys) [#18703](https://github.com/cosmos/cosmos-sdk/pull/18703) Improve `<appd> keys add` and `<appd> keys show` by checking whether there are duplicate keys in the multisig case.
* (client/keys) [#18745](https://github.com/cosmos/cosmos-sdk/pull/18745) Improve `<appd> keys export` and `<appd> keys mnemonic` by adding --yes option to skip interactive confirmation.
* (x/bank) [#24106](https://github.com/cosmos/cosmos-sdk/pull/24106) `SendCoins` now checks for `SendRestrictions` before instead of after deducting coins using `subUnlockedCoins`.
* (crypto/ledger) [#24036](https://github.com/cosmos/cosmos-sdk/pull/24036) Improve error message when deriving paths using index > 100

View File

@ -172,8 +172,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
}

View File

@ -119,6 +119,74 @@ 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)
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"}`

View File

@ -70,11 +70,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)
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)
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

View File

@ -127,6 +127,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),