Refactor Gas/Fee Model (#3258)

PENDING.md

@@ -95,6 +95,11 @@ IMPROVEMENTS
   * [\#3158](https://github.com/cosmos/cosmos-sdk/pull/3158) Validate slashing genesis
   * [\#3172](https://github.com/cosmos/cosmos-sdk/pull/3172) Support minimum fees in a local testnet.
   * [\#3250](https://github.com/cosmos/cosmos-sdk/pull/3250) Refactor integration tests and increase coverage
+  * [\#3248](https://github.com/cosmos/cosmos-sdk/issues/3248) Refactor tx fee
+  model:
+    * Validators specify minimum gas prices instead of minimum fees
+    * Clients may provide either fees or gas prices directly
+    * The gas prices of a tx must meet a validator's minimum
   * [\#2859](https://github.com/cosmos/cosmos-sdk/issues/2859) Rename `TallyResult` in gov proposals to `FinalTallyResult`
   * [\#3286](https://github.com/cosmos/cosmos-sdk/pull/3286) Fix `gaiad gentx` printout of account's addresses, i.e. user bech32 instead of hex.
 

baseapp/baseapp.go

@@ -74,8 +74,9 @@ type BaseApp struct {
 	// TODO move this in the future to baseapp param store on main store.
 	consensusParams *abci.ConsensusParams
 
-	// spam prevention
-	minimumFees sdk.Coins
+	// The minimum gas prices a validator is willing to accept for processing a
+	// transaction. This is mainly used for DoS and spam prevention.
+	minGasPrices sdk.DecCoins
 
 	// flag for sealing
 	sealed bool
@@ -213,13 +214,17 @@ func (app *BaseApp) initFromMainStore(mainKey *sdk.KVStoreKey) error {
 	return nil
 }
 
-func (app *BaseApp) setMinimumFees(fees sdk.Coins) { app.minimumFees = fees }
+func (app *BaseApp) setMinGasPrices(gasPrices sdk.DecCoins) {
+	app.minGasPrices = gasPrices
+}
 
 // NewContext returns a new Context with the correct store, the given header, and nil txBytes.
 func (app *BaseApp) NewContext(isCheckTx bool, header abci.Header) sdk.Context {
 	if isCheckTx {
-		return sdk.NewContext(app.checkState.ms, header, true, app.Logger).WithMinimumFees(app.minimumFees)
+		return sdk.NewContext(app.checkState.ms, header, true, app.Logger).
+			WithMinGasPrices(app.minGasPrices)
 	}
+
 	return sdk.NewContext(app.deliverState.ms, header, false, app.Logger)
 }
 
@@ -240,7 +245,7 @@ func (app *BaseApp) setCheckState(header abci.Header) {
 	ms := app.cms.CacheMultiStore()
 	app.checkState = &state{
 		ms:  ms,
-		ctx: sdk.NewContext(ms, header, true, app.Logger).WithMinimumFees(app.minimumFees),
+		ctx: sdk.NewContext(ms, header, true, app.Logger).WithMinGasPrices(app.minGasPrices),
 	}
 }
 
@@ -455,8 +460,9 @@ func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res
 	}
 
 	// Cache wrap the commit-multistore for safety.
-	ctx := sdk.NewContext(app.cms.CacheMultiStore(), app.checkState.ctx.BlockHeader(), true, app.Logger).
-		WithMinimumFees(app.minimumFees)
+	ctx := sdk.NewContext(
+		app.cms.CacheMultiStore(), app.checkState.ctx.BlockHeader(), true, app.Logger,
+	).WithMinGasPrices(app.minGasPrices)
 
 	// Passes the rest of the path as an argument to the querier.
 	// For example, in the path "custom/gov/proposal/test", the gov querier gets []string{"proposal", "test"} as the path

baseapp/options.go

@@ -18,13 +18,14 @@ func SetPruning(opts sdk.PruningOptions) func(*BaseApp) {
 	return func(bap *BaseApp) { bap.cms.SetPruning(opts) }
 }
 
-// SetMinimumFees returns an option that sets the minimum fees on the app.
-func SetMinimumFees(minFees string) func(*BaseApp) {
-	fees, err := sdk.ParseCoins(minFees)
+// SetMinimumGasPrices returns an option that sets the minimum gas prices on the app.
+func SetMinGasPrices(gasPricesStr string) func(*BaseApp) {
+	gasPrices, err := sdk.ParseDecCoins(gasPricesStr)
 	if err != nil {
-		panic(fmt.Sprintf("invalid minimum fees: %v", err))
+		panic(fmt.Sprintf("invalid minimum gas prices: %v", err))
 	}
-	return func(bap *BaseApp) { bap.setMinimumFees(fees) }
+
+	return func(bap *BaseApp) { bap.setMinGasPrices(gasPrices) }
 }
 
 func (app *BaseApp) SetName(name string) {

client/flags.go

@@ -30,6 +30,7 @@ const (
 	FlagSequence           = "sequence"
 	FlagMemo               = "memo"
 	FlagFees               = "fees"
+	FlagGasPrices          = "gas-prices"
 	FlagAsync              = "async"
 	FlagJson               = "json"
 	FlagPrintResponse      = "print-response"
@@ -79,6 +80,7 @@ func PostCommands(cmds ...*cobra.Command) []*cobra.Command {
 		c.Flags().Uint64(FlagSequence, 0, "Sequence number to sign the tx")
 		c.Flags().String(FlagMemo, "", "Memo to send along with transaction")
 		c.Flags().String(FlagFees, "", "Fees to pay along with transaction; eg: 10stake,1atom")
+		c.Flags().String(FlagGasPrices, "", "Gas prices to determine the transaction fee (e.g. 0.00001stake)")
 		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().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 ")

client/lcd/lcd_test.go

@@ -262,7 +262,8 @@ func TestCoinSendGenerateSignAndBroadcast(t *testing.T) {
 	payload := authrest.SignBody{
 		Tx: msg,
 		BaseReq: utils.NewBaseReq(
-			name1, pw, "", viper.GetString(client.FlagChainID), "", "", accnum, sequence, nil, false, false,
+			name1, pw, "", viper.GetString(client.FlagChainID), "", "",
+			accnum, sequence, nil, nil, false, false,
 		),
 	}
 	json, err := cdc.MarshalJSON(payload)

client/lcd/swagger-ui/swagger.yaml

@@ -2238,6 +2238,10 @@ definitions:
         type: array
         items:
           $ref: "#/definitions/Coin"
+      gas_prices:
+        type: array
+        items:
+          $ref: "#/definitions/DecCoin"
       generate_only:
         type: boolean
         example: false

client/lcd/test_helpers.go

@@ -647,7 +647,7 @@ func doSign(t *testing.T, port, name, password, chainID string, accnum, sequence
 	payload := authrest.SignBody{
 		Tx: msg,
 		BaseReq: utils.NewBaseReq(
-			name, password, "", chainID, "", "", accnum, sequence, nil, false, false,
+			name, password, "", chainID, "", "", accnum, sequence, nil, nil, false, false,
 		),
 	}
 	json, err := cdc.MarshalJSON(payload)
@@ -703,8 +703,9 @@ func doTransferWithGas(t *testing.T, port, seed, name, memo, password string, ad
 	sequence := acc.GetSequence()
 	chainID := viper.GetString(client.FlagChainID)
 
-	baseReq := utils.NewBaseReq(name, password, memo, chainID, gas,
-		fmt.Sprintf("%f", gasAdjustment), accnum, sequence, fees,
+	baseReq := utils.NewBaseReq(
+		name, password, memo, chainID, gas,
+		fmt.Sprintf("%f", gasAdjustment), accnum, sequence, fees, nil,
 		generateOnly, simulate,
 	)
 
@@ -736,7 +737,7 @@ func doDelegate(t *testing.T, port, name, password string,
 	accnum := acc.GetAccountNumber()
 	sequence := acc.GetSequence()
 	chainID := viper.GetString(client.FlagChainID)
-	baseReq := utils.NewBaseReq(name, password, "", chainID, "", "", accnum, sequence, fees, false, false)
+	baseReq := utils.NewBaseReq(name, password, "", chainID, "", "", accnum, sequence, fees, nil, false, false)
 	msg := msgDelegationsInput{
 		BaseReq:       baseReq,
 		DelegatorAddr: delAddr,
@@ -770,7 +771,7 @@ func doUndelegate(t *testing.T, port, name, password string,
 	accnum := acc.GetAccountNumber()
 	sequence := acc.GetSequence()
 	chainID := viper.GetString(client.FlagChainID)
-	baseReq := utils.NewBaseReq(name, password, "", chainID, "", "", accnum, sequence, fees, false, false)
+	baseReq := utils.NewBaseReq(name, password, "", chainID, "", "", accnum, sequence, fees, nil, false, false)
 	msg := msgUndelegateInput{
 		BaseReq:       baseReq,
 		DelegatorAddr: delAddr,
@@ -806,7 +807,7 @@ func doBeginRedelegation(t *testing.T, port, name, password string,
 	sequence := acc.GetSequence()
 
 	chainID := viper.GetString(client.FlagChainID)
-	baseReq := utils.NewBaseReq(name, password, "", chainID, "", "", accnum, sequence, fees, false, false)
+	baseReq := utils.NewBaseReq(name, password, "", chainID, "", "", accnum, sequence, fees, nil, false, false)
 
 	msg := msgBeginRedelegateInput{
 		BaseReq:          baseReq,
@@ -1037,7 +1038,7 @@ func doSubmitProposal(t *testing.T, port, seed, name, password string, proposerA
 	accnum := acc.GetAccountNumber()
 	sequence := acc.GetSequence()
 	chainID := viper.GetString(client.FlagChainID)
-	baseReq := utils.NewBaseReq(name, password, "", chainID, "", "", accnum, sequence, fees, false, false)
+	baseReq := utils.NewBaseReq(name, password, "", chainID, "", "", accnum, sequence, fees, nil, false, false)
 
 	pr := postProposalReq{
 		Title:          "Test",
@@ -1133,7 +1134,7 @@ func doDeposit(t *testing.T, port, seed, name, password string, proposerAddr sdk
 	accnum := acc.GetAccountNumber()
 	sequence := acc.GetSequence()
 	chainID := viper.GetString(client.FlagChainID)
-	baseReq := utils.NewBaseReq(name, password, "", chainID, "", "", accnum, sequence, fees, false, false)
+	baseReq := utils.NewBaseReq(name, password, "", chainID, "", "", accnum, sequence, fees, nil, false, false)
 
 	dr := depositReq{
 		Depositor: proposerAddr,
@@ -1187,7 +1188,7 @@ func doVote(t *testing.T, port, seed, name, password string, proposerAddr sdk.Ac
 	accnum := acc.GetAccountNumber()
 	sequence := acc.GetSequence()
 	chainID := viper.GetString(client.FlagChainID)
-	baseReq := utils.NewBaseReq(name, password, "", chainID, "", "", accnum, sequence, fees, false, false)
+	baseReq := utils.NewBaseReq(name, password, "", chainID, "", "", accnum, sequence, fees, nil, false, false)
 
 	vr := voteReq{
 		Voter:   proposerAddr,
@@ -1319,7 +1320,7 @@ func getSigningInfo(t *testing.T, port string, validatorPubKey string) slashing.
 func doUnjail(t *testing.T, port, seed, name, password string,
 	valAddr sdk.ValAddress, fees sdk.Coins) (resultTx ctypes.ResultBroadcastTxCommit) {
 	chainID := viper.GetString(client.FlagChainID)
-	baseReq := utils.NewBaseReq(name, password, "", chainID, "", "", 1, 1, fees, false, false)
+	baseReq := utils.NewBaseReq(name, password, "", chainID, "", "", 1, 1, fees, nil, false, false)
 
 	ur := unjailReq{
 		BaseReq: baseReq,

client/utils/rest.go

@@ -101,30 +101,33 @@ func WriteGenerateStdTxResponse(w http.ResponseWriter, cdc *codec.Codec, txBldr
 // BaseReq defines a structure that can be embedded in other request structures
 // that all share common "base" fields.
 type BaseReq struct {
-	Name          string    `json:"name"`
-	Password      string    `json:"password"`
-	Memo          string    `json:"memo"`
-	ChainID       string    `json:"chain_id"`
-	AccountNumber uint64    `json:"account_number"`
-	Sequence      uint64    `json:"sequence"`
-	Fees          sdk.Coins `json:"fees"`
-	Gas           string    `json:"gas"`
-	GasAdjustment string    `json:"gas_adjustment"`
-	GenerateOnly  bool      `json:"generate_only"`
-	Simulate      bool      `json:"simulate"`
+	Name          string       `json:"name"`
+	Password      string       `json:"password"`
+	Memo          string       `json:"memo"`
+	ChainID       string       `json:"chain_id"`
+	AccountNumber uint64       `json:"account_number"`
+	Sequence      uint64       `json:"sequence"`
+	Fees          sdk.Coins    `json:"fees"`
+	GasPrices     sdk.DecCoins `json:"gas_prices"`
+	Gas           string       `json:"gas"`
+	GasAdjustment string       `json:"gas_adjustment"`
+	GenerateOnly  bool         `json:"generate_only"`
+	Simulate      bool         `json:"simulate"`
 }
 
 // NewBaseReq creates a new basic request instance and sanitizes its values
 func NewBaseReq(
 	name, password, memo, chainID string, gas, gasAdjustment string,
-	accNumber, seq uint64, fees sdk.Coins, genOnly, simulate bool) BaseReq {
+	accNumber, seq uint64, fees sdk.Coins, gasPrices sdk.DecCoins, genOnly, simulate bool,
+) BaseReq {
 
 	return BaseReq{
 		Name:          strings.TrimSpace(name),
 		Password:      password,
 		Memo:          strings.TrimSpace(memo),
 		ChainID:       strings.TrimSpace(chainID),
 		Fees:          fees,
+		GasPrices:     gasPrices,
 		Gas:           strings.TrimSpace(gas),
 		GasAdjustment: strings.TrimSpace(gasAdjustment),
 		AccountNumber: accNumber,
@@ -136,11 +139,10 @@ func NewBaseReq(
 
 // Sanitize performs basic sanitization on a BaseReq object.
 func (br BaseReq) Sanitize() BaseReq {
-	newBr := NewBaseReq(
+	return NewBaseReq(
 		br.Name, br.Password, br.Memo, br.ChainID, br.Gas, br.GasAdjustment,
-		br.AccountNumber, br.Sequence, br.Fees, br.GenerateOnly, br.Simulate,
+		br.AccountNumber, br.Sequence, br.Fees, br.GasPrices, br.GenerateOnly, br.Simulate,
 	)
-	return newBr
 }
 
 // ValidateBasic performs basic validation of a BaseReq. If custom validation
@@ -152,18 +154,28 @@ func (br BaseReq) ValidateBasic(w http.ResponseWriter) bool {
 		case len(br.Password) == 0:
 			WriteErrorResponse(w, http.StatusUnauthorized, "password required but not specified")
 			return false
+
 		case len(br.ChainID) == 0:
 			WriteErrorResponse(w, http.StatusUnauthorized, "chain-id required but not specified")
 			return false
-		case !br.Fees.IsValid():
-			WriteErrorResponse(w, http.StatusPaymentRequired, "invalid or insufficient fees")
+
+		case !br.Fees.IsZero() && !br.GasPrices.IsZero():
+			// both fees and gas prices were provided
+			WriteErrorResponse(w, http.StatusBadRequest, "cannot provide both fees and gas prices")
+			return false
+
+		case !br.Fees.IsValid() && !br.GasPrices.IsValid():
+			// neither fees or gas prices were provided
+			WriteErrorResponse(w, http.StatusPaymentRequired, "invalid fees or gas prices provided")
 			return false
 		}
 	}
+
 	if len(br.Name) == 0 {
 		WriteErrorResponse(w, http.StatusUnauthorized, "name required but not specified")
 		return false
 	}
+
 	return true
 }
 
@@ -203,8 +215,12 @@ func ReadRESTReq(w http.ResponseWriter, r *http.Request, cdc *codec.Codec, req i
 // supplied messages. Finally, it broadcasts the signed transaction to a node.
 //
 // NOTE: Also see CompleteAndBroadcastTxCli.
-// NOTE: Also see x/staking/client/rest/tx.go delegationsRequestHandlerFn.
-func CompleteAndBroadcastTxREST(w http.ResponseWriter, r *http.Request, cliCtx context.CLIContext, baseReq BaseReq, msgs []sdk.Msg, cdc *codec.Codec) {
+// NOTE: Also see x/stake/client/rest/tx.go delegationsRequestHandlerFn.
+func CompleteAndBroadcastTxREST(
+	w http.ResponseWriter, r *http.Request, cliCtx context.CLIContext,
+	baseReq BaseReq, msgs []sdk.Msg, cdc *codec.Codec,
+) {
+
 	gasAdjustment, ok := ParseFloat64OrReturnBadRequest(w, baseReq.GasAdjustment, client.DefaultGasAdjustment)
 	if !ok {
 		return
@@ -216,9 +232,11 @@ func CompleteAndBroadcastTxREST(w http.ResponseWriter, r *http.Request, cliCtx c
 		return
 	}
 
-	txBldr := authtxb.NewTxBuilder(GetTxEncoder(cdc), baseReq.AccountNumber,
+	txBldr := authtxb.NewTxBuilder(
+		GetTxEncoder(cdc), baseReq.AccountNumber,
 		baseReq.Sequence, gas, gasAdjustment, baseReq.Simulate,
-		baseReq.ChainID, baseReq.Memo, baseReq.Fees)
+		baseReq.ChainID, baseReq.Memo, baseReq.Fees, baseReq.GasPrices,
+	)
 
 	if baseReq.Simulate || simulateAndExecute {
 		if gasAdjustment < 0 {