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

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This commit is contained in:
levisyin 2023-12-13 15:48:30 +08:00 committed by GitHub
parent fafb5eec04
commit f876b14287
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 106 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

@ -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 `<appd> keys add` and `<appd> 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.

View File

@ -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
}

View File

@ -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"}`

View File

@ -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

View File

@ -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),