feat(cli): add `multi-msg-sign`

[gsk967]

Description

Added --append flags to sign-batch cli cmd, this will combine the messages which are generated from --generate-only and it will generate single transaction

ref: #13066

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.

I have...

• [ ] included the correct type prefix in the PR title
• [ ] added ! to the type prefix if API or client breaking change
• [ ] targeted the correct branch (see PR Targeting)
• [ ] provided a link to the relevant issue or specification
• [ ] followed the guidelines for building modules
• [ ] included the necessary unit and integration tests
• [ ] added a changelog entry to CHANGELOG.md
• [ ] included comments for documenting Go code
• [ ] updated the relevant documentation or specification
• [ ] reviewed "Files changed" and left comments if necessary
• [ ] confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.

I have...

• [ ] confirmed the correct type prefix in the PR title
• [ ] confirmed ! in the type prefix if API or client breaking change
• [ ] confirmed all author checklist items have been addressed
• [ ] reviewed state machine logic
• [ ] reviewed API design and naming
• [ ] reviewed documentation is accurate
• [ ] reviewed tests and test coverage
• [ ] manually tested (if applicable)

[codecov[bot]]

Codecov Report

Merging #13147 (d0780aa) into main (4fe7797) will decrease coverage by 1.50%. The diff coverage is 24.13%.

:exclamation: Current head d0780aa differs from pull request most recent head ea5d655. Consider uploading reports for the commit ea5d655 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13147      +/-   ##
==========================================
- Coverage   55.87%   54.37%   -1.51%     
==========================================
  Files         646      645       -1     
  Lines       54895    54924      +29     
==========================================
- Hits        30675    29863     -812     
- Misses      21762    22663     +901     
+ Partials     2458     2398      -60     

Impacted Files	Coverage Δ
baseapp/grpcrouter.go	90.00% <ø> (ø)
baseapp/grpcrouter_helpers.go	25.00% <ø> (ø)
baseapp/grpcserver.go	1.72% <ø> (ø)
baseapp/msg_service_router.go	85.29% <ø> (+4.41%)	:arrow_up:
baseapp/options.go	67.92% <0.00%> (-0.60%)	:arrow_down:
client/broadcast.go	54.54% <ø> (+2.99%)	:arrow_up:
client/cmd.go	57.73% <ø> (ø)
client/config/toml.go	55.55% <ø> (ø)
client/context.go	54.49% <ø> (-1.79%)	:arrow_down:
client/flags/flags.go	19.35% <ø> (-0.32%)	:arrow_down:
... and 255 more

[atheeshp]

How is this different from sign-batch?

[anilcse]

How is this different from sign-batch?

Oh seems like it is. @gsk967 can we merge this solution with sign-batch to accept multiple tx files. How does that sound @alexanderbez . I like some refactoring in this pr tbh

[alexanderbez]

How is this different from sign-batch?

Oh seems like it is. @gsk967 can we merge this solution with sign-batch to accept multiple tx files. How does that sound @alexanderbez . I like some refactoring in this pr tbh

Yeah, sure!

[sahith-narahari]

How is this different from sign-batch?

Oh seems like it is. @gsk967 can we merge this solution with sign-batch to accept multiple tx files. How does that sound @alexanderbez . I like some refactoring in this pr tbh

How about we add a flag append to common TxFlags which takes file name and adds the tx to a common file. This should limit changes to sign-batch command and is better ux than handling one file per tx imo.

[alexanderbez]

How would that work? Say we have a new flag, e.g. multi-msg-file=<some-file>. Would this require the tx to be not signed and be appended to the file <some-file>? What changes would be made to sign-batch?

[sahith-narahari]

So sign-batch expects a file to have multiple unsigned txs seperated by next line. The command flow would look something like this

$ simd tx bank send <args...|flags...> --generate-only > txs.json
$ simd tx staking delegate <args...|flags...> --generate-only --append txs.json
$ simd auth sign-batch txs.json --from key1

[alexanderbez]

Makes sense. But I would have all txs use the new flag so make the UX easier. If the flag is present and the file is empty, then just populate the empty file 👍

[atheeshp]

So sign-batch expects a file to have multiple unsigned txs seperated by next line. The command flow would look something like this

$ simd tx bank send <args...|flags...> --generate-only > txs.json
$ simd tx staking delegate <args...|flags...> --generate-only --append txs.json
$ simd auth sign-batch txs.json --from key1

I think we don't need any flag to be introduced here, since we have >> (file append operation) by default.

• > replaces text in file
• >> appends to the file with newline delimiter

The following lines will do the same as above mentioned lines by @sahith-narahari .

$ simd tx bank send <args...|flags...> --generate-only >> txs.json
$ simd tx staking delegate <args...|flags...> --generate-only >> txs.json
$ simd tx sign-batch txs.json --from key1

We can just update the documentation.

cc: @anilCSE , @alexanderbez

[sahith-narahari]

Oh right, we can close the issue in this case then

[alexanderbez]

@atheeshp I don't understand how directing output will work though? The >> operator will just append one JSON blob into an existing file, giving you malformed JSON. What we want is a single tx, i.e. a single JSON blob that appends messages.

[alexanderbez]

We should consolidate between this work and sign-batch (ref: https://github.com/cosmos/cosmos-sdk/pull/13200)

[gsk967]

We should consolidate between this work and sign-batch (ref: #13200)

I think we can't merge sign-batch and multi-msg-sign because multi-msg-sign is combining the unsigned messages of transactions from --generated-only flag and will generate single signed transaction but sign-batch will generate the signed tx for each unsigned tx in single file (https://github.com/cosmos/cosmos-sdk/pull/13200#issuecomment-1240849828)

[anilcse]

We should consolidate between this work and sign-batch (ref: #13200)

I think we can't merge sign-batch and multi-msg-sign because multi-msg-sign is combining the unsigned messages of transactions from --generated-only flag and will generate single signed transaction but sign-batch will generate the signed tx for each unsigned tx in single file (#13200 (comment))

I would advise to change the sign-batch behaviour here and generate single file. Am I missing out any usecases where we need multiple signed files and thus forcing us to do multiple broadcast txs? Cc @alexanderbez

[alexanderbez]

Yes, this is exactly what I'm proposing! Modify sign-batch to act like multi-msg-sign. I.e. it works on a single transaction file with multiple messages generated from other single txs commands.

[atheeshp]

@atheeshp I don't understand how directing output will work though? The >> operator will just append one JSON blob into an existing file, giving you malformed JSON.

Yes, that will append all txs into one json and can be broadcasted one by one(as per the present docs).

What we want is a single tx, i.e. a single JSON blob that appends messages.

yeah, this could be very handy.

[alexanderbez]

I'm having trouble following this convo at this point...I don't think we're talking about the same thing. @gsk967 and I both believe sign-batch is flawed in UX and should be revised to what was proposed above.

[atheeshp]

I'm having trouble following this convo at this point...I don't think we're talking about the same thing. @gsk967 and I both believe sign-batch is flawed in UX and should be revised to what was proposed above.

Yes, I agree that current UX for sign-batch is bad. I'm aligning with your proposed solution.

[amaury1093]

What is the status on this? Are we changhing the behavior of sign-batch as described here https://github.com/cosmos/cosmos-sdk/pull/13147#issuecomment-1242007885 ?

[gsk967]

@atheeshp @alexanderbez @amaurym @anilCSE

Added --append flag to the sign-batch cli cmd to combine multiple messages and generate the single signed tx

[alexanderbez]

Amazing! @julienrbrt or @facundomedica would be willing to help test this feature with me please?

[julienrbrt]

Amazing! @julienrbrt or @facundomedica would be willing to help test this feature with me please?

I can check tomorrow morning :)

[julienrbrt]

Tested and works, but if there is documentation to update, we should do it in this PR.

[alexanderbez]

There are definitely CLI docs that we can look to update. Doesn't have to be super in depth, but maybe just a note in a CLI doc?

[julienrbrt]

There are definitely CLI docs that we can look to update. Doesn't have to be super in depth, but maybe just a note in a CLI doc?

Looking at the auth docs, there is actually no documentation for any tx auth commands: https://docs.cosmos.network/main/modules/auth/section_06.html. It probably shouldn't be done in this issue then. I'll create an issue to do this as follow-up.