Fix Gas Flag Usage + CLI Flag APIs (#6685)

* Use new APIs

* fix usage

* fix usage of gas flag

* tests: TestParseGasSetting
This commit is contained in:
Alexander Bezobchuk
2020-07-11 08:13:46 +00:00
committed by GitHub
parent e1476c1f9d
commit e7554bb3b0
44 changed files with 408 additions and 180 deletions
+18 -54
View File
@@ -21,9 +21,7 @@ const (
// DefaultKeyringBackend
DefaultKeyringBackend = keyring.BackendOS
)
const (
// BroadcastBlock defines a tx broadcasting mode where the client waits for
// the tx to be committed in a block.
BroadcastBlock = "block"
@@ -50,6 +48,7 @@ const (
FlagSequence = "sequence"
FlagMemo = "memo"
FlagFees = "fees"
FlagGas = "gas"
FlagGasPrices = "gas-prices"
FlagBroadcastMode = "broadcast-mode"
FlagDryRun = "dry-run"
@@ -66,10 +65,7 @@ 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) {}}
GasFlagVar = GasSetting{Gas: DefaultGasLimit}
)
var LineBreak = &cobra.Command{Run: func(*cobra.Command, []string) {}}
// AddQueryFlagsToCmd adds common flags to a module query command.
func AddQueryFlagsToCmd(cmd *cobra.Command) {
@@ -112,16 +108,8 @@ func AddTxFlagsToCmd(cmd *cobra.Command) {
cmd.Flags().String(FlagKeyringBackend, DefaultKeyringBackend, "Select keyring's backend (os|file|kwallet|pass|test)")
cmd.Flags().String(FlagSignMode, "", "Choose sign mode (direct|amino-json), this is an advanced feature")
// --gas can accept integers and "simulate"
//
// TODO: Remove usage of var in favor of string as this is technical creating
// a singleton usage pattern and can cause issues in parallel tests.
//
// REF: https://github.com/cosmos/cosmos-sdk/issues/6545
cmd.Flags().Var(&GasFlagVar, "gas", fmt.Sprintf(
"gas limit to set per-transaction; set to %q to calculate required gas automatically (default %d)",
GasFlagAuto, DefaultGasLimit,
))
// --gas can accept integers and "auto"
cmd.Flags().String(FlagGas, "", fmt.Sprintf("gas limit to set per-transaction; set to %q to calculate sufficient gas automatically (default %d)", GasFlagAuto, DefaultGasLimit))
cmd.MarkFlagRequired(FlagChainID)
@@ -135,62 +123,38 @@ func AddTxFlagsToCmd(cmd *cobra.Command) {
viper.BindPFlag(FlagKeyringBackend, cmd.Flags().Lookup(FlagKeyringBackend))
}
// GetCommands adds common flags to query commands.
//
// TODO: REMOVE.
func GetCommands(cmds ...*cobra.Command) []*cobra.Command {
for _, c := range cmds {
AddQueryFlagsToCmd(c)
}
return cmds
}
// PostCommands adds common flags for commands to post tx
//
// TODO: REMOVE.
func PostCommands(cmds ...*cobra.Command) []*cobra.Command {
for _, c := range cmds {
AddTxFlagsToCmd(c)
}
return cmds
}
// GasSetting encapsulates the possible values passed through the --gas flag.
type GasSetting struct {
Simulate bool
Gas uint64
}
// 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 = ParseGas(s)
return
}
func (v *GasSetting) String() string {
if v.Simulate {
return GasFlagAuto
}
return strconv.FormatUint(v.Gas, 10)
}
// ParseGas parses the value of the gas option.
func ParseGas(gasStr string) (simulateAndExecute bool, gas uint64, err error) {
// ParseGasSetting parses a string gas value. The value may either be 'auto',
// which indicates a transaction should be executed in simulate mode to
// automatically find a sufficient gas value, or a string integer. It returns an
// error if a string integer is provided which cannot be parsed.
func ParseGasSetting(gasStr string) (GasSetting, error) {
switch gasStr {
case "":
gas = DefaultGasLimit
return GasSetting{false, DefaultGasLimit}, nil
case GasFlagAuto:
simulateAndExecute = true
return GasSetting{true, 0}, nil
default:
gas, err = strconv.ParseUint(gasStr, 10, 64)
gas, err := strconv.ParseUint(gasStr, 10, 64)
if err != nil {
err = fmt.Errorf("gas must be either integer or %q", GasFlagAuto)
return
return GasSetting{}, fmt.Errorf("gas must be either integer or %s", GasFlagAuto)
}
return GasSetting{false, gas}, nil
}
return
}
+38
View File
@@ -0,0 +1,38 @@
package flags_test
import (
"testing"
"github.com/stretchr/testify/require"
"github.com/cosmos/cosmos-sdk/client/flags"
)
func TestParseGasSetting(t *testing.T) {
testCases := []struct {
name string
input string
expected flags.GasSetting
expectErr bool
}{
{"empty input", "", flags.GasSetting{false, flags.DefaultGasLimit}, false},
{"auto", flags.GasFlagAuto, flags.GasSetting{true, 0}, false},
{"valid custom gas", "73800", flags.GasSetting{false, 73800}, false},
{"invalid custom gas", "-73800", flags.GasSetting{false, 0}, true},
}
for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
gs, err := flags.ParseGasSetting(tc.input)
if tc.expectErr {
require.Error(t, err)
} else {
require.NoError(t, err)
require.Equal(t, tc.expected, gs)
}
})
}
}
+9 -4
View File
@@ -52,13 +52,16 @@ func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) Factory {
gasAdj, _ := flagSet.GetFloat64(flags.FlagGasAdjustment)
memo, _ := flagSet.GetString(flags.FlagMemo)
gasStr, _ := flagSet.GetString(flags.FlagGas)
gasSetting, _ := flags.ParseGasSetting(gasStr)
f := Factory{
txGenerator: clientCtx.TxGenerator,
accountRetriever: clientCtx.AccountRetriever,
keybase: clientCtx.Keyring,
chainID: clientCtx.ChainID,
gas: flags.GasFlagVar.Gas,
simulateAndExecute: flags.GasFlagVar.Simulate,
gas: gasSetting.Gas,
simulateAndExecute: gasSetting.Simulate,
accountNumber: accNum,
sequence: accSeq,
gasAdjustment: gasAdj,
@@ -96,14 +99,16 @@ func NewFactoryFromDeprecated(input io.Reader) Factory {
signMode = signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON
}
gasSetting, _ := flags.ParseGasSetting(viper.GetString(flags.FlagGas))
f := Factory{
keybase: kb,
chainID: viper.GetString(flags.FlagChainID),
accountNumber: viper.GetUint64(flags.FlagAccountNumber),
sequence: viper.GetUint64(flags.FlagSequence),
gas: flags.GasFlagVar.Gas,
gas: gasSetting.Gas,
simulateAndExecute: gasSetting.Simulate,
gasAdjustment: viper.GetFloat64(flags.FlagGasAdjustment),
simulateAndExecute: flags.GasFlagVar.Simulate,
memo: viper.GetString(flags.FlagMemo),
signMode: signMode,
}
+3 -3
View File
@@ -149,7 +149,7 @@ func WriteGeneratedTxResponse(
return
}
simAndExec, gas, err := flags.ParseGas(br.Gas)
gasSetting, err := flags.ParseGasSetting(br.Gas)
if rest.CheckBadRequestError(w, err) {
return
}
@@ -157,14 +157,14 @@ func WriteGeneratedTxResponse(
txf := Factory{fees: br.Fees, gasPrices: br.GasPrices}.
WithAccountNumber(br.AccountNumber).
WithSequence(br.Sequence).
WithGas(gas).
WithGas(gasSetting.Gas).
WithGasAdjustment(gasAdj).
WithMemo(br.Memo).
WithChainID(br.ChainID).
WithSimulateAndExecute(br.Simulate).
WithTxGenerator(ctx.TxGenerator)
if br.Simulate || simAndExec {
if br.Simulate || gasSetting.Simulate {
if gasAdj < 0 {
rest.WriteErrorResponse(w, http.StatusBadRequest, sdkerrors.ErrorInvalidGasAdjustment.Error())
return