From abbd2d4dd38f241666d57e972b51ea5986e65ad2 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Thu, 20 Dec 2018 19:21:07 +0000 Subject: [PATCH] Change --gas=simulate to --gas=auto when sending txs (#3170) --- PENDING.md | 2 ++ client/flags.go | 11 +++++------ client/lcd/lcd_test.go | 15 ++++++++++++++- cmd/gaia/cli_test/cli_test.go | 4 ++-- docs/gaia/gaiacli.md | 2 +- 5 files changed, 24 insertions(+), 10 deletions(-) diff --git a/PENDING.md b/PENDING.md index eb8a9dda68..e57abc87e9 100644 --- a/PENDING.md +++ b/PENDING.md @@ -14,6 +14,8 @@ BREAKING CHANGES * Gaia * https://github.com/cosmos/cosmos-sdk/issues/2838 - Move store keys to constants + * [\#3162](https://github.com/cosmos/cosmos-sdk/issues/3162) The `--gas` flag now takes `auto` instead of `simulate` + in order to trigger a simulation of the tx before the actual execution. * SDK * [\#3064](https://github.com/cosmos/cosmos-sdk/issues/3064) Sanitize `sdk.Coin` denom. Coins denoms are now case insensitive, i.e. 100fooToken equals to 100FOOTOKEN. diff --git a/client/flags.go b/client/flags.go index 7b37ec7154..4589d7fe0b 100644 --- a/client/flags.go +++ b/client/flags.go @@ -1,7 +1,6 @@ package client import ( - "errors" "fmt" "strconv" @@ -16,7 +15,7 @@ const ( // occur between the tx simulation and the actual run. DefaultGasAdjustment = 1.0 DefaultGasLimit = 200000 - GasFlagSimulate = "simulate" + GasFlagAuto = "auto" FlagUseLedger = "ledger" FlagChainID = "chain-id" @@ -92,7 +91,7 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command { 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)) + "gas limit to set per-transaction; set to %q to calculate required gas automatically (default %d)", GasFlagAuto, DefaultGasLimit)) viper.BindPFlag(FlagTrustNode, c.Flags().Lookup(FlagTrustNode)) viper.BindPFlag(FlagUseLedger, c.Flags().Lookup(FlagUseLedger)) viper.BindPFlag(FlagChainID, c.Flags().Lookup(FlagChainID)) @@ -142,7 +141,7 @@ func (v *GasSetting) Set(s string) (err error) { func (v *GasSetting) String() string { if v.Simulate { - return GasFlagSimulate + return GasFlagAuto } return strconv.FormatUint(v.Gas, 10) } @@ -152,12 +151,12 @@ func ParseGas(gasStr string) (simulateAndExecute bool, gas uint64, err error) { switch gasStr { case "": gas = DefaultGasLimit - case GasFlagSimulate: + case GasFlagAuto: simulateAndExecute = true default: gas, err = strconv.ParseUint(gasStr, 10, 64) if err != nil { - err = errors.New("gas must be a positive integer") + err = fmt.Errorf("gas must be either integer or %q", GasFlagAuto) return } } diff --git a/client/lcd/lcd_test.go b/client/lcd/lcd_test.go index b6b7a77175..5c6d7de183 100644 --- a/client/lcd/lcd_test.go +++ b/client/lcd/lcd_test.go @@ -190,10 +190,23 @@ func TestCoinSend(t *testing.T) { require.Equal(t, http.StatusInternalServerError, res.StatusCode, body) require.Nil(t, err) + // test failure with negative gas + res, body, _ = doTransferWithGas(t, port, seed, name1, memo, pw, addr, "-200", 0, false, false, fees) + require.Equal(t, http.StatusBadRequest, res.StatusCode, body) + // test failure with negative adjustment res, body, _ = doTransferWithGas(t, port, seed, name1, memo, pw, addr, "10000", -0.1, true, false, fees) require.Equal(t, http.StatusBadRequest, res.StatusCode, body) + // test failure with 0 gas + res, body, _ = doTransferWithGas(t, port, seed, name1, memo, pw, addr, "0", 0, false, false, fees) + require.Equal(t, http.StatusInternalServerError, res.StatusCode, body) + + // test failure with wrong adjustment + res, body, _ = doTransferWithGas(t, port, seed, name1, memo, pw, addr, client.GasFlagAuto, 0.1, false, false, fees) + + require.Equal(t, http.StatusInternalServerError, res.StatusCode, body) + // run simulation and test success with estimated gas res, body, _ = doTransferWithGas(t, port, seed, name1, memo, pw, addr, "10000", 1.0, true, false, fees) require.Equal(t, http.StatusOK, res.StatusCode, body) @@ -228,7 +241,7 @@ func TestCoinSendGenerateSignAndBroadcast(t *testing.T) { acc := getAccount(t, port, addr) // generate TX - res, body, _ := doTransferWithGas(t, port, seed, name1, memo, "", addr, "200000", 1, false, true, fees) + res, body, _ := doTransferWithGas(t, port, seed, name1, memo, "", addr, client.GasFlagAuto, 1, false, true, fees) require.Equal(t, http.StatusOK, res.StatusCode, body) var msg auth.StdTx require.Nil(t, cdc.UnmarshalJSON([]byte(body), &msg)) diff --git a/cmd/gaia/cli_test/cli_test.go b/cmd/gaia/cli_test/cli_test.go index ba92f838d8..bd001f6490 100644 --- a/cmd/gaia/cli_test/cli_test.go +++ b/cmd/gaia/cli_test/cli_test.go @@ -188,7 +188,7 @@ func TestGaiaCLIGasAuto(t *testing.T) { require.False(t, success) // Enable auto gas - success, stdout, _ := executeWriteRetStdStreams(t, fmt.Sprintf("gaiacli tx send %v --json --gas=simulate --amount=10%s --to=%s --from=foo", flags, stakeTypes.DefaultBondDenom, barAddr), app.DefaultKeyPass) + success, stdout, _ := executeWriteRetStdStreams(t, fmt.Sprintf("gaiacli tx send %v --json --gas=auto --amount=10%s --to=%s --from=foo", flags, stakeTypes.DefaultBondDenom, barAddr), app.DefaultKeyPass) require.True(t, success) // check that gas wanted == gas used cdc := app.MakeCodec() @@ -563,7 +563,7 @@ func TestGaiaCLISendGenerateSignAndBroadcast(t *testing.T) { // Test generate sendTx, estimate gas success, stdout, stderr = executeWriteRetStdStreams(t, fmt.Sprintf( - "gaiacli tx send %v --amount=10%s --to=%s --from=foo --gas=simulate --generate-only", + "gaiacli tx send %v --amount=10%s --to=%s --from=foo --gas=auto --generate-only", flags, stakeTypes.DefaultBondDenom, barAddr), []string{}...) require.True(t, success) require.NotEmpty(t, stderr) diff --git a/docs/gaia/gaiacli.md b/docs/gaia/gaiacli.md index ba2e8c8c01..102de5c892 100644 --- a/docs/gaia/gaiacli.md +++ b/docs/gaia/gaiacli.md @@ -134,7 +134,7 @@ The `--amount` flag accepts the format `--amount=`. ::: tip Note You may want to cap the maximum gas that can be consumed by the transaction via the `--gas` flag. -If you pass `--gas=simulate`, the gas limit will be automatically estimated. +If you pass `--gas=auto`, the gas supply will be automatically estimated before executing the transaction. 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. :::