Cosmos-sdk: Change gas simulation logic to a separate flag/parameter

[fedekunze]

Summary

Change gas simulation logic to a separate flag/parameter to improve the UX

Problem Definition

As discussed with @faboweb, it's not clear from a UX perspective that you would expect to run a gas simulation by setting --gas=0. Proposed designs follow:

Proposals

Proposal A: add an extra flag

• Add a --estimate-gas=<true/false> flag (on CLI) and parameter (on gaia-lite) to change the logic from https://github.com/cosmos/cosmos-sdk/pull/2047, which currently has to set --gas=0 to run the gas simulation
• Require that the --gas flag and parameter can only take positive values
• If both flags are given: --estimate-gas=true and --gas =$AMOUNT > 0, it runs the simulation

Proposal B: do not add an extra flag

• Make --gas flag accept a conventional string value, e.g. simulate/auto/estimate that would trigger tx simulation and set the gas according to the gas estimate returned by the simulation.
• Requires --gas flag values to be checked against a string conventional value/parsed into int64.

Rationale:

• Avoid adding an extra parameter to control gas for the sake of interface's simplicity. Gas setting should be controlled via only one dedicated parameter: --gas
• What if the user inputs both --gas=$AMOUNT and --estimate-gas? Such a command line would be misleading and ambiguous, i.e. what should the expected behaviour be when the user sets a gas allocation and switches gas simulation on? Which flag should take precedence?

Proposal C: add an extra flag and exit if conflicting flags are passed

• Add a --estimate-gas=<true/false> flag (on CLI) and parameter (on gaia-lite) (as above)
• Exit with an error if both --estimate-gas=true and --gas=$AMOUNT are supplied.

---

For Admin Use

• [x] Not duplicate issue
• [x] Appropriate labels applied
• [x] Appropriate contributors tagged
• [x] Contributor assigned/self-assigned

[ValarDragon]

Once we make simulation default, I don't see why this is an issue? I'd expect setting gas to not have it be simulated (or at least only warning me if I gave an insufficient amount), and to have it be simulated by default.

Currently its not default due to it depending on the private key, which causes weird ledger UX + has sidechannel leakage concerns.

[alexanderbez]

iff we do decide to have two flags, the command/endpoint should halt immediately and gracefully with a warning stating that both cannot be provided.

Simulation will be default now @ValarDragon ?

[alessio]

@alexanderbez I've put your suggestion up among the other proposals

[ValarDragon]

With the newly updated proposals, Proposal B looks great to me!

[alessio]

Let's vote:

Proposal A 👍
Proposal B 😄
Proposal C 🎉

[alessio]

/cc @jackzampolin @jaekwon @cwgoes

[jackzampolin]

We already have a ton of flags so adding another one will confuse matters. I voted for B because we could default to something sane (i.e. --gas=estimate) and advanced users can put in their own value for gas if they need to (i.e. --gas=100000). Seems user-friendly and understandable.

[alexanderbez]

Proposal B looks sound to me. Parsing and validating can easily be done with string constants within a switch case and handling integers as the default.