Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Bank multi-send not working as expected #16948

Closed
charymalloju opened this issue Jul 12, 2023 · 5 comments · Fixed by #16950
Closed

[Bug]: Bank multi-send not working as expected #16948

charymalloju opened this issue Jul 12, 2023 · 5 comments · Fixed by #16950
Labels

Comments

@charymalloju
Copy link
Contributor

Summary of Bug

When I am trying to execute multi-send transactions with comma-separated addresses it is not working but It's working with the space-separated addresses.

Version

v0.46.7

Steps to Reproduce

./simd tx bank multi-send comos1.... cosmos2....,cosmos3.... 10stake 10stake  --from mykey1 --chain-id testnet 
Error: decoding bech32 failed: invalid checksum (expected skjz8l got nucmdn)
Usage:
  simd tx bank multi-send [from_key_or_address] [to_address_1, to_address_2, ...] [amount] [flags]

but if we change the command to space separate it is working as expected.

**success**
./simd tx bank multi-send cosmos1.... cosmos2.... cosmos3.... 10stake  --from mykey1 --chain-id testnet 
auth_info:
  fee:
    amount: []
    gas_limit: "200000"
    granter: ""
    payer: ""
  signer_infos: []
  tip: null
body:
  extension_options: []
  memo: ""
  messages:
  - '@type': /cosmos.bank.v1beta1.MsgMultiSend
    inputs:
    - address: cosmos1zpspl7tqfjhcymamshhwycmcj7lmjhxyxtj8u4
      coins:
      - amount: "20"
        denom: stake
    outputs:
    - address: cosmos1gph99ltt659zh9w96n8p442ve350j9vzku9uz2
      coins:
      - amount: "10"
        denom: stake
    - address: cosmos1475w3r0lfapwtmhn5tl5pf38yka5g0w8nucmdn
      coins:
      - amount: "10"
        denom: stake
  non_critical_extension_options: []
  timeout_height: "0"
signatures: []
confirm transaction before signing and broadcasting [y/N]: 
@charymalloju
Copy link
Contributor Author

I'm open to work on this if anyone acknowledges this. Thanks!

@facundomedica
Copy link
Member

I believe this is an issue with the example, looking at the code it's clear we expect spaces. I think we need to remove the commas in the example

var output []types.Output
for _, arg := range args[1 : len(args)-1] {
toAddr, err := sdk.AccAddressFromBech32(arg)
if err != nil {
return err
}
output = append(output, types.NewOutput(toAddr, sendCoins))
}

@julienrbrt
Copy link
Member

This has never worked with comma separated before, maybe the Use of the example could be more precise.

@julienrbrt julienrbrt added T: UX and removed T:Bug labels Jul 12, 2023
@charymalloju
Copy link
Contributor Author

But, I feel UX wise comma separated would be more helpful, isn't it? what do you think if we could make the command to work as per the description??

@julienrbrt
Copy link
Member

julienrbrt commented Jul 12, 2023

But, I feel UX wise comma separated would be more helpful, isn't it? what do you think if we could make the command to work as per the description??

Not using comma separated is way more readable and takes advantage of cobra variable arguments.
There is only a typo in the example. IMHO we should just fix the typo and keep the behavior the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants