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

Create new command for MsgMultiSend to send different amounts to multiple addresses. #14617

Closed
danbryan opened this issue Jan 13, 2023 · 12 comments

Comments

@danbryan
Copy link

Summary

Create new command for MsgMultiSend to send different amounts to multiple addresses.

Problem Definition

Currently multi-send will only send the same amount to multiple addresses. This has it's purposes, but there is a need to be able to send different amount. For example the rewards for Game of Chains requires distributing ~97 different amounts of atom to 97 different validators from a multi sig wallet. Doing 97 transactions would be a huge effort, and currently the cosmos-sdk does not support this.

Proposal

Create a new command multi-send-csv that allows you to define an input address,token and and multiple outputaddress,token in 2 CSVs. For example.

input.csv

cosmos1e6xlqh6szdgx05txgshk0kdll3eh0h2f5404k8,100000uatom 

output.csv

smos16wdara3hkjuqy3gygwtr8mds9wttg7xt2asp7k,50000uatom
cosmos1yjlrvsxuhuvug2ypkl0yg9m93fgqfw4u9qqyp3,40000uatom
cosmos1gkg70dlfzcymwu5mykg528g0elp87kg2ysxf9e,30000uatom
cosmos1k72mrtjcvchqd7x5pmw2fuu77gz402ndm9qta3,20000uatom
cosmos189zaqnzul8pdj5qej9gsx0x94k80h36pg20jxe,10000uatom

Then use the command.

simd tx bank multi-send-csv input.csv output.csv --node http://rpc.water.theta-testnet.polypore.xyz:26657/ --from cosmos1e6xlqh6szdgx05txgshk0kdll3eh0h2f5404k8 --chain-id theta-testnet-001 --gas-prices 0.0025uatom --gas-adjustment 1.3

Which would create the following output.

auth_info:
  fee:
    amount:
    - amount: "331"
      denom: uatom
    gas_limit: "132143"
    granter: ""
    payer: ""
  signer_infos: []
  tip: null
body:
  extension_options: []
  memo: ""
  messages:
  - '@type': /cosmos.bank.v1beta1.MsgMultiSend
    inputs:
    - address: cosmos1e6xlqh6szdgx05txgshk0kdll3eh0h2f5404k8
      coins:
      - amount: "150000"
        denom: uatom
    outputs:
    - address: cosmos16wdara3hkjuqy3gygwtr8mds9wttg7xt2asp7k
      coins:
      - amount: "50000"
        denom: uatom
    - address: cosmos1yjlrvsxuhuvug2ypkl0yg9m93fgqfw4u9qqyp3
      coins:
      - amount: "40000"
        denom: uatom
    - address: cosmos1gkg70dlfzcymwu5mykg528g0elp87kg2ysxf9e
      coins:
      - amount: "30000"
        denom: uatom
    - address: cosmos1k72mrtjcvchqd7x5pmw2fuu77gz402ndm9qta3
      coins:
      - amount: "20000"
        denom: uatom
    - address: cosmos189zaqnzul8pdj5qej9gsx0x94k80h36pg20jxe
      coins:
      - amount: "10000"
        denom: uatom
  non_critical_extension_options: []
  timeout_height: "0"
signatures: []
confirm transaction before signing and broadcasting [y/N]:
@KyleMoser KyleMoser self-assigned this Jan 13, 2023
@julienrbrt
Copy link
Member

julienrbrt commented Jan 14, 2023

Why not allowing the existing command to support this?

We could add a flag that reads from an CSV as well so it works for both cases (same amount to multiple addresses or different amounts to multiple addresses) 🤔

@danbryan
Copy link
Author

curious for thoughts on using a CSV vs flags. Counter example could be something like.

simd tx bank multi-send \
--source-tokens cosmos1e6xlqh6szdgx05txgshk0kdll3eh0h2f5404k8,100000uatom \
--dest-tokens cosmos16wdara3hkjuqy3gygwtr8mds9wttg7xt2asp7k,50000uatom \
--dest-tokens cosmos1yjlrvsxuhuvug2ypkl0yg9m93fgqfw4u9qqyp3,40000uatom \
--dest-tokens cosmos1gkg70dlfzcymwu5mykg528g0elp87kg2ysxf9e,30000uatom \
--dest-tokens cosmos1k72mrtjcvchqd7x5pmw2fuu77gz402ndm9qta3,20000uatom \
--dest-tokens cosmos189zaqnzul8pdj5qej9gsx0x94k80h36pg20jxe,10000uatom \
--node http://rpc.water.theta-testnet.polypore.xyz:26657/ \
--from cosmos1e6xlqh6szdgx05txgshk0kdll3eh0h2f5404k8 \
--chain-id theta-testnet-001 --gas-prices 0.0025uatom --gas-adjustment 1.3

@KyleMoser
Copy link
Contributor

KyleMoser commented Jan 14, 2023

Why not allowing the existing command to support this?

We could add a flag that reads from an CSV as well so it works for both cases (same amount to multiple addresses or different amounts to multiple addresses) thinking

@julienrbrt That is actually how I originally developed it. What I found was that the documentation became unwieldy - there were tons of flags and it wasn't clear to the user that there were two separate ways to use the command (one with the CSVs specified and one without). I can go change it back to that again, and work on the docs a bit more.

IMO, if a CSV is specified it should send the exact amounts that are in the CSV (split flag should not be allowed in that case). Otherwise there are more edge cases to worry about, and the CSV format would be different.

@julienrbrt
Copy link
Member

Why not allowing the existing command to support this?

We could add a flag that reads from an CSV as well so it works for both cases (same amount to multiple addresses or different amounts to multiple addresses) thinking

@julienrbrt That is actually how I originally developed it. What I found was that the documentation became unwieldy - there were tons of flags and it wasn't clear to the user that there were two separate ways to use the command (one with the CSVs specified and one without). I can go change it back to that again, and work on the docs a bit more.

IMO, if a CSV is specified it should send the exact amounts that are in the CSV (split flag should not be allowed in that case). Otherwise there are more edge cases to worry about, and the CSV format would be different.

I agree. You can make the csv and split flag mutually exclusive and add an error msg when used together.

Having good docs is for sure a win.

@alexanderbez
Copy link
Contributor

I would personally prefer not to read from CSV. Instead just augment the current multi-send command to accept the following as args:

<addr>,<coins>;<addr>,<coins>;...

Then we split on ; to get all the tuples and then further split each tuple on , to get the address and coins.

E.g.

cosmos16wdara3hkjuqy3gygwtr8mds9wttg7xt2asp7k,50000uatom;cosmos189zaqnzul8pdj5qej9gsx0x94k80h36pg20jxe,10000uatom;...

Note, these should NOT be flags. This input is integral to the command, i.e. not optional and thus should be an arg.

@tac0turtle
Copy link
Member

are you able to use the multi msg command, this seems like a fine alternative to multisend

@atheeshp
Copy link
Contributor

atheeshp commented Mar 8, 2023

sign-batch with --append flag may be helps here? ref: #13066
edit: most of the current discussion on this issue is happened here (#13066) and later finalized to introduce the --append flag to sign-batch (not exactly sure though cc: @alexanderbez, @anilcse )

@anilcse
Copy link
Collaborator

anilcse commented Mar 8, 2023

I am actually in favour of adding a --multi flag for all the transactions. Which will take either csv or comma(,) separated values to generate multiple messages for each type. example;

tx bank send <from>, <to>, <amount>, --multi <from2>,<to2>,<amount2>,<from3>,<to3>,<amount3>,...
tx staking delegate <validator>,<amount>, --multi <val2>,<amount2>,<val3>,<amount3>,...

@alexanderbez
Copy link
Contributor

What does --multi do for messages other than the native MsgMultiSend? Like what does --multi do for MsgDelegate?

@anilcse
Copy link
Collaborator

anilcse commented Mar 8, 2023

What does --multi do for messages other than the native MsgMultiSend? Like what does --multi do for MsgDelegate?

Delegates to multiple validators in a single transaction (we can avoid generating multiple messages for it)

@alexanderbez
Copy link
Contributor

I'm not sure how I feel about that UX. But it also conflates the convo we're having here about MsgMultiSend

@julienrbrt
Copy link
Member

Because no other users since this issue has requested this feature, and the possible UX confusion of adding a --multi flag we'll close this.

@julienrbrt julienrbrt reopened this Oct 1, 2024
@julienrbrt julienrbrt closed this as not planned Won't fix, can't repro, duplicate, stale Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants