R4R: Change --gas=0 semantic and introduce --gas=simulate

PENDING.md

@@ -61,8 +61,9 @@ FEATURES
   * [gov][cli] #2062 added `--proposal` flag to `submit-proposal` that allows a JSON file containing a proposal to be passed in
   * [\#2040](https://github.com/cosmos/cosmos-sdk/issues/2040) Add `--bech` to `gaiacli keys show` and respective REST endpoint to
   provide desired Bech32 prefix encoding
-  * [cli] [\#2047](https://github.com/cosmos/cosmos-sdk/issues/2047) Setting the --gas flag value to 0 triggers a simulation of the tx before the actual execution. The gas estimate obtained via the simulation will be used as gas limit in the actual execution.
-  * [cli] [\#2047](https://github.com/cosmos/cosmos-sdk/issues/2047) The --gas-adjustment flag can be used to adjust the estimate obtained via the simulation triggered by --gas=0.
+  * [cli] [\#2047](https://github.com/cosmos/cosmos-sdk/issues/2047) [\#2306](https://github.com/cosmos/cosmos-sdk/pull/2306) Passing --gas=simulate triggers a simulation of the tx before the actual execution.
+  The gas estimate obtained via the simulation will be used as gas limit in the actual execution.
+  * [cli] [\#2047](https://github.com/cosmos/cosmos-sdk/issues/2047) The --gas-adjustment flag can be used to adjust the estimate obtained via the simulation triggered by --gas=simulate.
   * [cli] [\#2110](https://github.com/cosmos/cosmos-sdk/issues/2110) Add --dry-run flag to perform a simulation of a transaction without broadcasting it. The --gas flag is ignored as gas would be automatically estimated.
   * [cli] [\#2204](https://github.com/cosmos/cosmos-sdk/issues/2204) Support generating and broadcasting messages with multiple signatures via command line:
     * [\#966](https://github.com/cosmos/cosmos-sdk/issues/966) Add --generate-only flag to build an unsigned transaction and write it to STDOUT.

client/context/context.go

@@ -27,8 +27,6 @@ type CLIContext struct {
 	Client          rpcclient.Client
 	Logger          io.Writer
 	Height          int64
-	Gas             int64
-	GasAdjustment   float64
 	NodeURI         string
 	FromAddressName string
 	AccountStore    string
@@ -58,8 +56,6 @@ func NewCLIContext() CLIContext {
 		AccountStore:    ctxAccStoreName,
 		FromAddressName: viper.GetString(client.FlagFrom),
 		Height:          viper.GetInt64(client.FlagHeight),
-		Gas:             viper.GetInt64(client.FlagGas),
-		GasAdjustment:   viper.GetFloat64(client.FlagGasAdjustment),
 		TrustNode:       viper.GetBool(client.FlagTrustNode),
 		UseLedger:       viper.GetBool(client.FlagUseLedger),
 		Async:           viper.GetBool(client.FlagAsync),
@@ -164,9 +160,3 @@ func (ctx CLIContext) WithCertifier(certifier tmlite.Certifier) CLIContext {
 	ctx.Certifier = certifier
 	return ctx
 }
-
-// WithGasAdjustment returns a copy of the context with an updated GasAdjustment flag.
-func (ctx CLIContext) WithGasAdjustment(adjustment float64) CLIContext {
-	ctx.GasAdjustment = adjustment
-	return ctx
-}

client/flags.go

@@ -1,6 +1,11 @@
 package client
 
-import "github.com/spf13/cobra"
+import (
+	"fmt"
+	"strconv"
+
+	"github.com/spf13/cobra"
+)
 
 // nolint
 const (
@@ -9,6 +14,7 @@ const (
 	// occur between the tx simulation and the actual run.
 	DefaultGasAdjustment = 1.0
 	DefaultGasLimit      = 200000
+	GasFlagSimulate      = "simulate"
 
 	FlagUseLedger     = "ledger"
 	FlagChainID       = "chain-id"
@@ -32,7 +38,10 @@ const (
 
 // LineBreak can be included in a command list to provide a blank line
 // to help with readability
-var LineBreak = &cobra.Command{Run: func(*cobra.Command, []string) {}}
+var (
+	LineBreak  = &cobra.Command{Run: func(*cobra.Command, []string) {}}
+	GasFlagVar = GasSetting{Gas: DefaultGasLimit}
+)
 
 // GetCommands adds common flags to query commands
 func GetCommands(cmds ...*cobra.Command) []*cobra.Command {
@@ -58,14 +67,57 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command {
 		c.Flags().String(FlagChainID, "", "Chain ID of tendermint node")
 		c.Flags().String(FlagNode, "tcp://localhost:26657", "<host>:<port> to tendermint rpc interface for this chain")
 		c.Flags().Bool(FlagUseLedger, false, "Use a connected Ledger device")
-		c.Flags().Int64(FlagGas, DefaultGasLimit, "gas limit to set per-transaction; set to 0 to calculate required gas automatically")
 		c.Flags().Float64(FlagGasAdjustment, DefaultGasAdjustment, "adjustment factor to be multiplied against the estimate returned by the tx simulation; if the gas limit is set manually this flag is ignored ")
 		c.Flags().Bool(FlagAsync, false, "broadcast transactions asynchronously")
 		c.Flags().Bool(FlagJson, false, "return output in json format")
 		c.Flags().Bool(FlagPrintResponse, true, "return tx response (only works with async = false)")
 		c.Flags().Bool(FlagTrustNode, true, "Don't verify proofs for query responses")
 		c.Flags().Bool(FlagDryRun, false, "ignore the --gas flag and perform a simulation of a transaction, but don't broadcast it")
 		c.Flags().Bool(FlagGenerateOnly, false, "build an unsigned transaction and write it to STDOUT")
+		// --gas can accept integers and "simulate"
+		c.Flags().Var(&GasFlagVar, "gas", fmt.Sprintf(
+			"gas limit to set per-transaction; set to %q to calculate required gas automatically (default %d)", GasFlagSimulate, DefaultGasLimit))
 	}
 	return cmds
 }
+
+// Gas flag parsing functions
+
+// GasSetting encapsulates the possible values passed through the --gas flag.
+type GasSetting struct {
+	Simulate bool
+	Gas      int64
+}
+
+// Type returns the flag's value type.
+func (v *GasSetting) Type() string { return "string" }
+
+// Set parses and sets the value of the --gas flag.
+func (v *GasSetting) Set(s string) (err error) {
+	v.Simulate, v.Gas, err = ReadGasFlag(s)
+	return
+}
+
+func (v *GasSetting) String() string {
+	if v.Simulate {
+		return GasFlagSimulate
+	}
+	return strconv.FormatInt(v.Gas, 10)
+}
+
+// ParseGasFlag parses the value of the --gas flag.
+func ReadGasFlag(s string) (simulate bool, gas int64, err error) {
+	switch s {
+	case "":
+		gas = DefaultGasLimit
+	case GasFlagSimulate:
+		simulate = true
+	default:
+		gas, err = strconv.ParseInt(s, 10, 64)
+		if err != nil {
+			err = fmt.Errorf("gas must be either integer or %q", GasFlagSimulate)
+			return
+		}
+	}
+	return
+}

client/lcd/lcd_test.go

@@ -267,21 +267,29 @@ func TestCoinSend(t *testing.T) {
 	require.Equal(t, int64(1), mycoins.Amount.Int64())
 
 	// test failure with too little gas
-	res, body, _ = doSendWithGas(t, port, seed, name, password, addr, 100, 0, "")
+	res, body, _ = doSendWithGas(t, port, seed, name, password, addr, "100", 0, "")
+	require.Equal(t, http.StatusInternalServerError, res.StatusCode, body)
+
+	// test failure with negative gas
+	res, body, _ = doSendWithGas(t, port, seed, name, password, addr, "-200", 0, "")
+	require.Equal(t, http.StatusInternalServerError, res.StatusCode, body)
+
+	// test failure with 0 gas
+	res, body, _ = doSendWithGas(t, port, seed, name, password, addr, "0", 0, "")
 	require.Equal(t, http.StatusInternalServerError, res.StatusCode, body)
 
 	// test failure with wrong adjustment
-	res, body, _ = doSendWithGas(t, port, seed, name, password, addr, 0, 0.1, "")
+	res, body, _ = doSendWithGas(t, port, seed, name, password, addr, "simulate", 0.1, "")
 	require.Equal(t, http.StatusInternalServerError, res.StatusCode, body)
 
 	// run simulation and test success with estimated gas
-	res, body, _ = doSendWithGas(t, port, seed, name, password, addr, 0, 0, "?simulate=true")
+	res, body, _ = doSendWithGas(t, port, seed, name, password, addr, "", 0, "?simulate=true")
 	require.Equal(t, http.StatusOK, res.StatusCode, body)
 	var responseBody struct {
 		GasEstimate int64 `json:"gas_estimate"`
 	}
 	require.Nil(t, json.Unmarshal([]byte(body), &responseBody))
-	res, body, _ = doSendWithGas(t, port, seed, name, password, addr, responseBody.GasEstimate, 0, "")
+	res, body, _ = doSendWithGas(t, port, seed, name, password, addr, fmt.Sprintf("%v", responseBody.GasEstimate), 0, "")
 	require.Equal(t, http.StatusOK, res.StatusCode, body)
 }
 
@@ -322,7 +330,7 @@ func TestCoinSendGenerateSignAndBroadcast(t *testing.T) {
 	acc := getAccount(t, port, addr)
 
 	// generate TX
-	res, body, _ := doSendWithGas(t, port, seed, name, password, addr, 0, 0, "?generate_only=true")
+	res, body, _ := doSendWithGas(t, port, seed, name, password, addr, "simulate", 0, "?generate_only=true")
 	require.Equal(t, http.StatusOK, res.StatusCode, body)
 	var msg auth.StdTx
 	require.Nil(t, cdc.UnmarshalJSON([]byte(body), &msg))
@@ -792,7 +800,7 @@ func getAccount(t *testing.T, port string, addr sdk.AccAddress) auth.Account {
 	return acc
 }
 
-func doSendWithGas(t *testing.T, port, seed, name, password string, addr sdk.AccAddress, gas int64, gasAdjustment float64, queryStr string) (res *http.Response, body string, receiveAddr sdk.AccAddress) {
+func doSendWithGas(t *testing.T, port, seed, name, password string, addr sdk.AccAddress, gas string, gasAdjustment float64, queryStr string) (res *http.Response, body string, receiveAddr sdk.AccAddress) {
 
 	// create receive address
 	kb := client.MockKeyBase()
@@ -811,14 +819,14 @@ func doSendWithGas(t *testing.T, port, seed, name, password string, addr sdk.Acc
 	}
 
 	gasStr := ""
-	if gas > 0 {
+	if len(gas) != 0 {
 		gasStr = fmt.Sprintf(`
-		"gas":"%v",
+		"gas":%q,
 		`, gas)
 	}
 	gasAdjustmentStr := ""
 	if gasAdjustment > 0 {
-		gasStr = fmt.Sprintf(`
+		gasAdjustmentStr = fmt.Sprintf(`
 		"gas_adjustment":"%v",
 		`, gasAdjustment)
 	}
@@ -837,7 +845,7 @@ func doSendWithGas(t *testing.T, port, seed, name, password string, addr sdk.Acc
 }
 
 func doSend(t *testing.T, port, seed, name, password string, addr sdk.AccAddress) (receiveAddr sdk.AccAddress, resultTx ctypes.ResultBroadcastTxCommit) {
-	res, body, receiveAddr := doSendWithGas(t, port, seed, name, password, addr, 0, 0, "")
+	res, body, receiveAddr := doSendWithGas(t, port, seed, name, password, addr, "", 0, "")
 	require.Equal(t, http.StatusOK, res.StatusCode, body)
 
 	err := cdc.UnmarshalJSON([]byte(body), &resultTx)

client/utils/utils.go

@@ -24,8 +24,8 @@ func SendTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, msgs []sdk.Msg)
 	if err != nil {
 		return err
 	}
-	autogas := cliCtx.DryRun || (cliCtx.Gas == 0)
-	if autogas {
+
+	if txBldr.SimulateGas || cliCtx.DryRun {
 		txBldr, err = EnrichCtxWithGas(txBldr, cliCtx, cliCtx.FromAddressName, msgs)
 		if err != nil {
 			return err
@@ -50,20 +50,10 @@ func SendTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, msgs []sdk.Msg)
 	return cliCtx.EnsureBroadcastTx(txBytes)
 }
 
-// SimulateMsgs simulates the transaction and returns the gas estimate and the adjusted value.
-func SimulateMsgs(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name string, msgs []sdk.Msg, gas int64) (estimated, adjusted int64, err error) {
-	txBytes, err := txBldr.WithGas(gas).BuildWithPubKey(name, msgs)
-	if err != nil {
-		return
-	}
-	estimated, adjusted, err = CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, cliCtx.GasAdjustment)
-	return
-}
-
 // EnrichCtxWithGas calculates the gas estimate that would be consumed by the
 // transaction and set the transaction's respective value accordingly.
 func EnrichCtxWithGas(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name string, msgs []sdk.Msg) (authtxb.TxBuilder, error) {
-	_, adjusted, err := SimulateMsgs(txBldr, cliCtx, name, msgs, 0)
+	_, adjusted, err := simulateMsgs(txBldr, cliCtx, name, msgs)
 	if err != nil {
 		return txBldr, err
 	}
@@ -143,6 +133,16 @@ func SignStdTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name string,
 	return txBldr.SignStdTx(name, passphrase, stdTx, appendSig)
 }
 
+// SimulateMsgs simulates the transaction and returns the gas estimate and the adjusted value.
+func simulateMsgs(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, name string, msgs []sdk.Msg) (estimated, adjusted int64, err error) {
+	txBytes, err := txBldr.BuildWithPubKey(name, msgs)
+	if err != nil {
+		return
+	}
+	estimated, adjusted, err = CalculateGas(cliCtx.Query, cliCtx.Codec, txBytes, txBldr.GasAdjustment)
+	return
+}
+
 func adjustGasEstimate(estimate int64, adjustment float64) int64 {
 	return int64(adjustment * float64(estimate))
 }
@@ -194,7 +194,7 @@ func buildUnsignedStdTx(txBldr authtxb.TxBuilder, cliCtx context.CLIContext, msg
 	if err != nil {
 		return
 	}
-	if txBldr.Gas == 0 {
+	if txBldr.SimulateGas {
 		txBldr, err = EnrichCtxWithGas(txBldr, cliCtx, cliCtx.FromAddressName, msgs)
 		if err != nil {
 			return

cmd/gaia/cli_test/cli_test.go

@@ -112,8 +112,16 @@ func TestGaiaCLIGasAuto(t *testing.T) {
 	fooAcc = executeGetAccount(t, fmt.Sprintf("gaiacli account %s %v", fooAddr, flags))
 	require.Equal(t, int64(50), fooAcc.GetCoins().AmountOf("steak").Int64())
 
+	// Test failure with negative gas
+	success = executeWrite(t, fmt.Sprintf("gaiacli send %v --gas=-100 --amount=10steak --to=%s --from=foo", flags, barAddr), app.DefaultKeyPass)
+	require.False(t, success)
+
+	// Test failure with 0 gas
+	success = executeWrite(t, fmt.Sprintf("gaiacli send %v --gas=0 --amount=10steak --to=%s --from=foo", flags, barAddr), app.DefaultKeyPass)
+	require.False(t, success)
+
 	// Enable auto gas
-	success, stdout, _ := executeWriteRetStdStreams(t, fmt.Sprintf("gaiacli send %v --json --gas=0 --amount=10steak --to=%s --from=foo", flags, barAddr), app.DefaultKeyPass)
+	success, stdout, _ := executeWriteRetStdStreams(t, fmt.Sprintf("gaiacli send %v --json --gas=simulate --amount=10steak --to=%s --from=foo", flags, barAddr), app.DefaultKeyPass)
 	require.True(t, success)
 	// check that gas wanted == gas used
 	cdc := app.MakeCodec()
@@ -381,7 +389,7 @@ func TestGaiaCLISendGenerateSignAndBroadcast(t *testing.T) {
 
 	// Test generate sendTx, estimate gas
 	success, stdout, stderr = executeWriteRetStdStreams(t, fmt.Sprintf(
-		"gaiacli send %v --amount=10steak --to=%s --from=foo --gas=0 --generate-only",
+		"gaiacli send %v --amount=10steak --to=%s --from=foo --gas=simulate --generate-only",
 		flags, barAddr), []string{}...)
 	require.True(t, success)
 	require.NotEmpty(t, stderr)

docs/light/api.md

@@ -763,7 +763,7 @@ The GovernanceAPI exposes all functionality needed for casting votes on plain te
     	"chain_id": "string",
         "account_number": 0,
     	"sequence": 0,
-    	"gas": 0
+    	"gas": "simulate"
   },
   "depositer": "string",
   "amount": 0,
@@ -866,7 +866,7 @@ The GovernanceAPI exposes all functionality needed for casting votes on plain te
     	"chain_id": "string",
     	"account_number": 0,
     	"sequence": 0,
-    	"gas": 0
+    	"gas": "simulate"
   	},
     // A cosmos address
   	"voter": "string",

docs/sdk/clients.md

@@ -111,7 +111,7 @@ The `--amount` flag accepts the format `--amount=<value|coin_name>`.
 
 ::: tip Note
 You may want to cap the maximum gas that can be consumed by the transaction via the `--gas` flag.
-If set to 0, the gas limit will be automatically estimated.
+If you pass `--gas=simulate`, the gas limit will be automatically estimated.
 Gas estimate might be inaccurate as state changes could occur in between the end of the simulation and the actual execution of a transaction, thus an adjustment is applied on top of the original estimate in order to ensure the transaction is broadcasted successfully. The adjustment can be controlled via the `--gas-adjustment` flag, whose default value is 1.0.
 :::
 

x/auth/client/txbuilder/txbuilder.go

@@ -17,6 +17,8 @@ type TxBuilder struct {
 	AccountNumber int64
 	Sequence      int64
 	Gas           int64
+	GasAdjustment float64
+	SimulateGas   bool
 	ChainID       string
 	Memo          string
 	Fee           string
@@ -36,9 +38,11 @@ func NewTxBuilderFromCLI() TxBuilder {
 
 	return TxBuilder{
 		ChainID:       chainID,
-		Gas:           viper.GetInt64(client.FlagGas),
 		AccountNumber: viper.GetInt64(client.FlagAccountNumber),
+		Gas:           client.GasFlagVar.Gas,
+		GasAdjustment: viper.GetFloat64(client.FlagGasAdjustment),
 		Sequence:      viper.GetInt64(client.FlagSequence),
+		SimulateGas:   client.GasFlagVar.Simulate,
 		Fee:           viper.GetString(client.FlagFee),
 		Memo:          viper.GetString(client.FlagMemo),
 	}