Skip to content

Commit

Permalink
feat: make global min gas price gov modifiable (#3094)
Browse files Browse the repository at this point in the history
## Overview

Makes GlobalMinGasPrice hard-coded constant gov modifiable

Fixes - #3092

---------
  • Loading branch information
ninabarbakadze authored Mar 20, 2024
1 parent cee9cd4 commit 8bacf9b
Show file tree
Hide file tree
Showing 19 changed files with 910 additions and 68 deletions.
12 changes: 11 additions & 1 deletion app/ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/cosmos/cosmos-sdk/x/auth/ante"
"github.com/cosmos/cosmos-sdk/x/auth/signing"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
paramkeeper "github.com/cosmos/cosmos-sdk/x/params/keeper"
ibcante "github.com/cosmos/ibc-go/v6/modules/core/ante"
ibckeeper "github.com/cosmos/ibc-go/v6/modules/core/keeper"
)
Expand All @@ -19,6 +20,7 @@ func NewAnteHandler(
signModeHandler signing.SignModeHandler,
sigGasConsumer ante.SignatureVerificationGasConsumer,
channelKeeper *ibckeeper.Keeper,
paramKeeper paramkeeper.Keeper,
msgVersioningGateKeeper *MsgVersioningGateKeeper,
) sdk.AnteHandler {
return sdk.ChainAnteDecorators(
Expand All @@ -43,7 +45,7 @@ func NewAnteHandler(
ante.NewConsumeGasForTxSizeDecorator(accountKeeper),
// Ensure the feepayer (fee granter or first signer) has enough funds to pay for the tx.
// Side effect: deducts fees from the fee payer. Sets the tx priority in context.
ante.NewDeductFeeDecorator(accountKeeper, bankKeeper, feegrantKeeper, CheckTxFeeWithGlobalMinGasPrices),
ante.NewDeductFeeDecorator(accountKeeper, bankKeeper, feegrantKeeper, CheckTxFeeWithGlobalMinGasPricesWrapper(paramKeeper)),
// Set public keys in the context for fee-payer and all signers.
// Contract: must be called before all signature verification decorators.
ante.NewSetPubKeyDecorator(accountKeeper),
Expand Down Expand Up @@ -75,3 +77,11 @@ func NewAnteHandler(
}

var DefaultSigVerificationGasConsumer = ante.DefaultSigVerificationGasConsumer

// The purpose of this wrapper is to enable the passing of an additional paramKeeper parameter
// whilst still satisfying the ante.TxFeeChecker type.
func CheckTxFeeWithGlobalMinGasPricesWrapper(paramKeeper paramkeeper.Keeper) ante.TxFeeChecker {
return func(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) {
return CheckTxFeeWithMinGasPrices(ctx, tx, paramKeeper)
}
}
64 changes: 47 additions & 17 deletions app/ante/fee_checker.go
Original file line number Diff line number Diff line change
@@ -1,54 +1,84 @@
package ante

import (
"fmt"

errors "cosmossdk.io/errors"
"cosmossdk.io/math"
"github.com/celestiaorg/celestia-app/pkg/appconsts"
v1 "github.com/celestiaorg/celestia-app/pkg/appconsts/v1"
"github.com/celestiaorg/celestia-app/x/minfee"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerror "github.com/cosmos/cosmos-sdk/types/errors"
params "github.com/cosmos/cosmos-sdk/x/params/keeper"
)

const (
// priorityScalingFactor is a scaling factor to convert the gas price to a priority.
priorityScalingFactor = 1_000_000
)

// CheckTxFeeWithGlobalMinGasPrices implements the default fee logic, where the minimum price per
// unit of gas is fixed and set globally, and the tx priority is computed from the gas price.
func CheckTxFeeWithGlobalMinGasPrices(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) {
// CheckTxFeeWithMinGasPrices implements default fee validation logic for transactions.
// It ensures that the provided transaction fee meets a minimum threshold for the validator
// as well as a global minimum threshold and computes the tx priority based on the gas price.
func CheckTxFeeWithMinGasPrices(ctx sdk.Context, tx sdk.Tx, paramKeeper params.Keeper) (sdk.Coins, int64, error) {
feeTx, ok := tx.(sdk.FeeTx)
if !ok {
return nil, 0, errors.Wrap(sdkerror.ErrTxDecode, "Tx must be a FeeTx")
}

fee := feeTx.GetFee().AmountOf(appconsts.BondDenom)
gas := feeTx.GetGas()
appVersion := ctx.BlockHeader().Version.App

// global minimum fee only applies to app versions greater than one
if appVersion > v1.Version {
globalMinGasPrice := appconsts.GlobalMinGasPrice(appVersion)
// Ensure that the provided fee meets a minimum threshold for the validator.
// This is only for local mempool purposes, and thus
// is only ran on check tx.
if ctx.IsCheckTx() {
minGasPrice := ctx.MinGasPrices().AmountOf(appconsts.BondDenom)
if !minGasPrice.IsZero() {
err := verifyMinFee(fee, gas, minGasPrice, "insufficient minimum gas price for this validator")
if err != nil {
return nil, 0, err
}
}
}

// convert the global minimum gas price to a big.Int
globalMinGasPriceInt, err := sdk.NewDecFromStr(fmt.Sprintf("%f", globalMinGasPrice))
if err != nil {
return nil, 0, errors.Wrap(err, "invalid GlobalMinGasPrice")
// Ensure that the provided fee meets a global minimum threshold.
// Global minimum fee only applies to app versions greater than one
if ctx.BlockHeader().Version.App > v1.Version {
subspace, exists := paramKeeper.GetSubspace(minfee.ModuleName)
if !exists {
return nil, 0, errors.Wrap(sdkerror.ErrInvalidRequest, "minfee is not a registered subspace")
}

if !subspace.Has(ctx, minfee.KeyGlobalMinGasPrice) {
return nil, 0, errors.Wrap(sdkerror.ErrKeyNotFound, "GlobalMinGasPrice")
}

gasInt := sdk.NewIntFromUint64(gas)
minFee := globalMinGasPriceInt.MulInt(gasInt).RoundInt()
var globalMinGasPrice sdk.Dec
// Gets the global minimum gas price from the param store
// panics if not configured properly
subspace.Get(ctx, minfee.KeyGlobalMinGasPrice, &globalMinGasPrice)

if !fee.GTE(minFee) {
return nil, 0, errors.Wrapf(sdkerror.ErrInsufficientFee, "insufficient fees; got: %s required: %s", fee, minFee)
err := verifyMinFee(fee, gas, globalMinGasPrice, "insufficient gas price for the network")
if err != nil {
return nil, 0, err
}
}

priority := getTxPriority(feeTx.GetFee(), int64(gas))
return feeTx.GetFee(), priority, nil
}

// verifyMinFee validates that the provided transaction fee is sufficient given the provided minimum gas price.
func verifyMinFee(fee math.Int, gas uint64, minGasPrice sdk.Dec, errMsg string) error {
// Determine the required fee by multiplying required minimum gas
// price by the gas limit, where fee = minGasPrice * gas.
minFee := minGasPrice.MulInt(sdk.NewIntFromUint64(gas)).Ceil()
if fee.LT(minFee.TruncateInt()) {
return errors.Wrapf(sdkerror.ErrInsufficientFee, "%s; got: %s required at least: %s", errMsg, fee, minFee)
}
return nil
}

// getTxPriority returns a naive tx priority based on the amount of the smallest denomination of the gas price
// provided in a transaction.
// NOTE: This implementation should not be used for txs with multiple coins.
Expand Down
88 changes: 77 additions & 11 deletions app/ante/min_fee_test.go
Original file line number Diff line number Diff line change
@@ -1,19 +1,29 @@
package ante_test

import (
"fmt"
"math"
"testing"

"github.com/celestiaorg/celestia-app/app"
"github.com/celestiaorg/celestia-app/app/ante"
"github.com/celestiaorg/celestia-app/app/encoding"
"github.com/celestiaorg/celestia-app/pkg/appconsts"
v2 "github.com/celestiaorg/celestia-app/pkg/appconsts/v2"
"github.com/celestiaorg/celestia-app/test/util/testnode"
"github.com/celestiaorg/celestia-app/x/minfee"
"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/store"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
paramkeeper "github.com/cosmos/cosmos-sdk/x/params/keeper"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
"github.com/stretchr/testify/require"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
version "github.com/tendermint/tendermint/proto/tendermint/version"
tmdb "github.com/tendermint/tm-db"
)

func TestCheckTxFeeWithGlobalMinGasPrices(t *testing.T) {
Expand All @@ -27,69 +37,96 @@ func TestCheckTxFeeWithGlobalMinGasPrices(t *testing.T) {
)
require.NoError(t, err)

feeAmount := int64(1000)
// Set the validator's fee
validatorMinGasPrice := 0.8
validatorMinGasPriceDec, err := sdk.NewDecFromStr(fmt.Sprintf("%f", validatorMinGasPrice))
require.NoError(t, err)
validatorMinGasPriceCoin := sdk.NewDecCoinFromDec(appconsts.BondDenom, validatorMinGasPriceDec)

ctx := sdk.Context{}
feeAmount := int64(1000)

globalMinGasPrice := appconsts.DefaultGlobalMinGasPrice
require.NoError(t, err)
paramsKeeper, stateStore := setUp(t)

testCases := []struct {
name string
fee sdk.Coins
gasLimit uint64
appVersion uint64
isCheckTx bool
expErr bool
}{
{
name: "bad tx; fee below required minimum",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount-1)),
gasLimit: uint64(float64(feeAmount) / globalMinGasPrice),
gasLimit: uint64(float64(feeAmount) / v2.GlobalMinGasPrice),
appVersion: uint64(2),
isCheckTx: false,
expErr: true,
},
{
name: "good tx; fee equal to required minimum",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)),
gasLimit: uint64(float64(feeAmount) / globalMinGasPrice),
gasLimit: uint64(float64(feeAmount) / v2.GlobalMinGasPrice),
appVersion: uint64(2),
isCheckTx: false,
expErr: false,
},
{
name: "good tx; fee above required minimum",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount+1)),
gasLimit: uint64(float64(feeAmount) / globalMinGasPrice),
gasLimit: uint64(float64(feeAmount) / v2.GlobalMinGasPrice),
appVersion: uint64(2),
isCheckTx: false,
expErr: false,
},
{
name: "good tx; with no fee (v1)",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)),
gasLimit: uint64(float64(feeAmount) / globalMinGasPrice),
gasLimit: uint64(float64(feeAmount) / v2.GlobalMinGasPrice),
appVersion: uint64(1),
isCheckTx: false,
expErr: false,
},
{
name: "good tx; gas limit and fee are maximum values",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, math.MaxInt64)),
gasLimit: math.MaxUint64,
appVersion: uint64(2),
isCheckTx: false,
expErr: false,
},
{
name: "bad tx; gas limit and fee are 0",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, 0)),
gasLimit: 0,
appVersion: uint64(2),
isCheckTx: false,
expErr: false,
},
{
name: "good tx; minFee = 0.8, rounds up to 1",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount)),
gasLimit: 400,
appVersion: uint64(2),
isCheckTx: false,
expErr: false,
},
{
name: "good tx; fee above node's required minimum",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount+1)),
gasLimit: uint64(float64(feeAmount) / validatorMinGasPrice),
appVersion: uint64(1),
isCheckTx: true,
expErr: false,
},
{
name: "bad tx; fee below node's required minimum",
fee: sdk.NewCoins(sdk.NewInt64Coin(appconsts.BondDenom, feeAmount-1)),
gasLimit: uint64(float64(feeAmount) / validatorMinGasPrice),
appVersion: uint64(1),
isCheckTx: true,
expErr: true,
},
}

for _, tc := range testCases {
Expand All @@ -98,12 +135,22 @@ func TestCheckTxFeeWithGlobalMinGasPrices(t *testing.T) {
builder.SetFeeAmount(tc.fee)
tx := builder.GetTx()

ctx = ctx.WithBlockHeader(tmproto.Header{
ctx := sdk.NewContext(stateStore, tmproto.Header{
Version: version.Consensus{
App: tc.appVersion,
},
})
_, _, err := ante.CheckTxFeeWithGlobalMinGasPrices(ctx, tx)
}, tc.isCheckTx, nil)

ctx = ctx.WithMinGasPrices(sdk.DecCoins{validatorMinGasPriceCoin})

globalminGasPriceDec, err := sdk.NewDecFromStr(fmt.Sprintf("%f", v2.GlobalMinGasPrice))
require.NoError(t, err)

subspace, _ := paramsKeeper.GetSubspace(minfee.ModuleName)
minfee.RegisterMinFeeParamTable(subspace)
subspace.Set(ctx, minfee.KeyGlobalMinGasPrice, globalminGasPriceDec)

_, _, err = ante.CheckTxFeeWithMinGasPrices(ctx, tx, paramsKeeper)
if tc.expErr {
require.Error(t, err)
} else {
Expand All @@ -112,3 +159,22 @@ func TestCheckTxFeeWithGlobalMinGasPrices(t *testing.T) {
})
}
}

func setUp(t *testing.T) (paramkeeper.Keeper, storetypes.CommitMultiStore) {
storeKey := sdk.NewKVStoreKey(paramtypes.StoreKey)
tStoreKey := storetypes.NewTransientStoreKey(paramtypes.TStoreKey)

// Create the state store
db := tmdb.NewMemDB()
stateStore := store.NewCommitMultiStore(db)
stateStore.MountStoreWithDB(storeKey, storetypes.StoreTypeIAVL, db)
stateStore.MountStoreWithDB(tStoreKey, storetypes.StoreTypeTransient, nil)
require.NoError(t, stateStore.LoadLatestVersion())

registry := codectypes.NewInterfaceRegistry()

// Create a params keeper and set the global min gas price
paramsKeeper := paramkeeper.NewKeeper(codec.NewProtoCodec(registry), codec.NewLegacyAmino(), storeKey, tStoreKey)
paramsKeeper.Subspace(minfee.ModuleName)
return paramsKeeper, stateStore
}
13 changes: 13 additions & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/celestiaorg/celestia-app/app/module"
"github.com/celestiaorg/celestia-app/app/posthandler"
"github.com/celestiaorg/celestia-app/x/minfee"
"github.com/celestiaorg/celestia-app/x/mint"
mintkeeper "github.com/celestiaorg/celestia-app/x/mint/keeper"
minttypes "github.com/celestiaorg/celestia-app/x/mint/types"
Expand Down Expand Up @@ -124,6 +125,7 @@ var (
blob.AppModuleBasic{},
blobstream.AppModuleBasic{},
upgrade.AppModuleBasic{},
minfee.AppModuleBasic{},
)

// ModuleEncodingRegisters keeps track of all the module methods needed to
Expand Down Expand Up @@ -456,6 +458,10 @@ func New(
Module: upgrade.NewAppModule(app.UpgradeKeeper),
FromVersion: v2, ToVersion: v2,
},
{
Module: minfee.NewAppModule(app.ParamsKeeper),
FromVersion: v2, ToVersion: v2,
},
})
if err != nil {
panic(err)
Expand Down Expand Up @@ -486,6 +492,7 @@ func New(
authz.ModuleName,
vestingtypes.ModuleName,
upgradetypes.ModuleName,
minfee.ModuleName,
)

app.mm.SetOrderEndBlockers(
Expand All @@ -509,13 +516,16 @@ func New(
authz.ModuleName,
vestingtypes.ModuleName,
upgradetypes.ModuleName,
minfee.ModuleName,
)

// NOTE: The genutils module must occur after staking so that pools are
// properly initialized with tokens from genesis accounts.
// NOTE: Capability module must occur first so that it can initialize any capabilities
// so that other modules that want to create or claim capabilities afterwards in InitChain
// can do so safely.
// NOTE: The minfee module must occur before genutil so DeliverTx can
// successfully pass the fee checking logic
app.mm.SetOrderInitGenesis(
capabilitytypes.ModuleName,
authtypes.ModuleName,
Expand All @@ -527,6 +537,7 @@ func New(
minttypes.ModuleName,
crisistypes.ModuleName,
ibchost.ModuleName,
minfee.ModuleName,
genutiltypes.ModuleName,
evidencetypes.ModuleName,
ibctransfertypes.ModuleName,
Expand Down Expand Up @@ -569,6 +580,7 @@ func New(
encodingConfig.TxConfig.SignModeHandler(),
ante.DefaultSigVerificationGasConsumer,
app.IBCKeeper,
app.ParamsKeeper,
app.MsgGateKeeper,
))
app.SetPostHandler(posthandler.New())
Expand Down Expand Up @@ -801,6 +813,7 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
paramsKeeper.Subspace(ibchost.ModuleName)
paramsKeeper.Subspace(blobtypes.ModuleName)
paramsKeeper.Subspace(blobstreamtypes.ModuleName)
paramsKeeper.Subspace(minfee.ModuleName)

return paramsKeeper
}
Expand Down
Loading

0 comments on commit 8bacf9b

Please sign in to comment.