Skip to content

Commit

Permalink
feat: Add gas limits for queries (backport cosmos#454) (cosmos#472)
Browse files Browse the repository at this point in the history
Co-authored-by: Roman <[email protected]>
  • Loading branch information
mergify[bot] and p0mvn authored Jul 2, 2023
1 parent bb88501 commit 9294e56
Show file tree
Hide file tree
Showing 9 changed files with 139 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

* (baseapp) [#16239](https://github.com/cosmos/cosmos-sdk/pull/16239) Add Gas Limits to allow node operators to resource bound queries.

## [v0.45.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.45.0) - 2022-01-18

### State Machine Breaking
Expand Down
26 changes: 25 additions & 1 deletion baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -553,11 +553,29 @@ func (app *BaseApp) ApplySnapshotChunk(req abci.RequestApplySnapshotChunk) abci.
}
}

// This comes from ICA, and is made out of an abundance of caution.
// Should be deleted during the next Osmosis major release, as we don't need this extra safety check.
var stateMachineQueryList = []string{"/ibc.applications.transfer.v1.Query/DenomTrace", "/cosmos.auth.v1beta1.Query/Account", "/cosmos.auth.v1beta1.Query/Params", "/cosmos.bank.v1beta1.Query/Balance", "/cosmos.bank.v1beta1.Query/DenomMetadata", "/cosmos.bank.v1beta1.Query/Params", "/cosmos.bank.v1beta1.Query/SupplyOf", "/cosmos.distribution.v1beta1.Query/Params", "/cosmos.distribution.v1beta1.Query/DelegatorWithdrawAddress", "/cosmos.distribution.v1beta1.Query/ValidatorCommission", "/cosmos.gov.v1beta1.Query/Deposit", "/cosmos.gov.v1beta1.Query/Params", "/cosmos.gov.v1beta1.Query/Vote", "/cosmos.slashing.v1beta1.Query/Params", "/cosmos.slashing.v1beta1.Query/SigningInfo", "/cosmos.staking.v1beta1.Query/Delegation", "/cosmos.staking.v1beta1.Query/Params", "/cosmos.staking.v1beta1.Query/Validator", "/osmosis.epochs.v1beta1.Query/EpochInfos", "/osmosis.epochs.v1beta1.Query/CurrentEpoch", "/osmosis.gamm.v1beta1.Query/NumPools", "/osmosis.gamm.v1beta1.Query/TotalLiquidity", "/osmosis.gamm.v1beta1.Query/Pool", "/osmosis.gamm.v1beta1.Query/PoolParams", "/osmosis.gamm.v1beta1.Query/TotalPoolLiquidity", "/osmosis.gamm.v1beta1.Query/TotalShares", "/osmosis.gamm.v1beta1.Query/CalcJoinPoolShares", "/osmosis.gamm.v1beta1.Query/CalcExitPoolCoinsFromShares", "/osmosis.gamm.v1beta1.Query/CalcJoinPoolNoSwapShares", "/osmosis.gamm.v1beta1.Query/PoolType", "/osmosis.gamm.v2.Query/SpotPrice", "/osmosis.gamm.v1beta1.Query/EstimateSwapExactAmountIn", "/osmosis.gamm.v1beta1.Query/EstimateSwapExactAmountOut", "/osmosis.incentives.Query/ModuleToDistributeCoins", "/osmosis.incentives.Query/LockableDurations", "/osmosis.lockup.Query/ModuleBalance", "/osmosis.lockup.Query/ModuleLockedAmount", "/osmosis.lockup.Query/AccountUnlockableCoins", "/osmosis.lockup.Query/AccountUnlockingCoins", "/osmosis.lockup.Query/LockedDenom", "/osmosis.lockup.Query/LockedByID", "/osmosis.lockup.Query/NextLockID", "/osmosis.mint.v1beta1.Query/EpochProvisions", "/osmosis.mint.v1beta1.Query/Params", "/osmosis.poolincentives.v1beta1.Query/GaugeIds", "/osmosis.superfluid.Query/Params", "/osmosis.superfluid.Query/AssetType", "/osmosis.superfluid.Query/AllAssets", "/osmosis.superfluid.Query/AssetMultiplier", "/osmosis.poolmanager.v1beta1.Query/NumPools", "/osmosis.poolmanager.v1beta1.Query/EstimateSwapExactAmountIn", "/osmosis.poolmanager.v1beta1.Query/EstimateSwapExactAmountOut", "/osmosis.txfees.v1beta1.Query/FeeTokens", "/osmosis.txfees.v1beta1.Query/DenomSpotPrice", "/osmosis.txfees.v1beta1.Query/DenomPoolId", "/osmosis.txfees.v1beta1.Query/BaseDenom", "/osmosis.tokenfactory.v1beta1.Query/Params", "/osmosis.tokenfactory.v1beta1.Query/DenomAuthorityMetadata", "/osmosis.twap.v1beta1.Query/ArithmeticTwap", "/osmosis.twap.v1beta1.Query/ArithmeticTwapToNow", "/osmosis.twap.v1beta1.Query/GeometricTwap", "/osmosis.twap.v1beta1.Query/GeometricTwapToNow", "/osmosis.twap.v1beta1.Query/Params", "/osmosis.downtimedetector.v1beta1.Query/RecoveredSinceDowntimeOfLength"}

func queryListContainsReq(req abci.RequestQuery) bool {
for _, query := range stateMachineQueryList {
if req.Path == query {
return true
}
}
return false
}

func (app *BaseApp) handleQueryGRPC(handler GRPCQueryHandler, req abci.RequestQuery) abci.ResponseQuery {
ctx, err := app.createQueryContext(req.Height, req.Prove)
if err != nil {
return sdkerrors.QueryResultWithDebug(err, app.trace)
}
// use infinite gas metering for potential state machine queries
// delete in next major release
if queryListContainsReq(req) {
ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
}

res, err := handler(ctx, req)
if err != nil {
Expand Down Expand Up @@ -634,10 +652,16 @@ func (app *BaseApp) createQueryContext(height int64, prove bool) (sdk.Context, e
return sdk.Context{}, fmt.Errorf("failed to load cache multi store for height %d: %w", height, err)
}

gasLimit := app.queryGasLimit
// arbitrarily declare that historical queries cause 2x gas load.
if height != lastBlockHeight {
gasLimit /= 2
}

// branch the commit-multistore for safety
ctx := sdk.NewContext(
cacheMS, app.checkState.ctx.BlockHeader(), true, app.logger,
).WithMinGasPrices(app.minGasPrices).WithBlockHeight(height)
).WithMinGasPrices(app.minGasPrices).WithBlockHeight(height).WithGasMeter(sdk.NewGasMeter(gasLimit))

if height != lastBlockHeight {
rms, ok := app.cms.(*rootmulti.Store)
Expand Down
5 changes: 5 additions & 0 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package baseapp

import (
"fmt"
"math"
"reflect"
"strings"

Expand Down Expand Up @@ -89,6 +90,9 @@ type BaseApp struct { // nolint: maligned
// application parameter store.
paramStore ParamStore

// max gas for remote query.
// unbounded if 0.
queryGasLimit uint64
// 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
Expand Down Expand Up @@ -198,6 +202,7 @@ func NewBaseApp(
msgServiceRouter: NewMsgServiceRouter(),
txDecoder: txDecoder,
fauxMerkleMode: false,
queryGasLimit: math.MaxUint64,
}

for _, option := range options {
Expand Down
85 changes: 85 additions & 0 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ import (
"io/ioutil"
"math/rand"
"os"
"reflect"
"strings"
"sync"
"testing"
"time"
"unsafe"

"github.com/gogo/protobuf/jsonpb"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -76,6 +78,20 @@ func registerTestCodec(cdc *codec.LegacyAmino) {
cdc.RegisterConcrete(&msgNoRoute{}, "cosmos-sdk/baseapp/msgNoRoute", nil)
}

func getQueryBaseapp(t *testing.T) *BaseApp {
db := dbm.NewMemDB()
name := t.Name()
app := NewBaseApp(name, defaultLogger(), db, nil)

app.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{Height: 1}})
app.Commit()

app.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{Height: 2}})
app.Commit()

return app
}

// aminoTxEncoder creates a amino TxEncoder for testing purposes.
func aminoTxEncoder() sdk.TxEncoder {
cdc := codec.NewLegacyAmino()
Expand Down Expand Up @@ -623,6 +639,75 @@ func TestInitChainer(t *testing.T) {
require.Equal(t, value, res.Value)
}

type ctxType string

const (
QueryCtx ctxType = "query"
CheckTxCtx ctxType = "checkTx"
DeliverTxCtx ctxType = "deliverTx"
)

var ctxTypes = []ctxType{QueryCtx, CheckTxCtx}

func getCheckStateCtx(app *BaseApp) sdk.Context {
v := reflect.ValueOf(app).Elem()
f := v.FieldByName("checkState")
rf := reflect.NewAt(f.Type(), unsafe.Pointer(f.UnsafeAddr())).Elem()
return rf.MethodByName("Context").Call(nil)[0].Interface().(sdk.Context)
}

func getDeliverStateCtx(app *BaseApp) sdk.Context {
v := reflect.ValueOf(app).Elem()
f := v.FieldByName("deliverState")
rf := reflect.NewAt(f.Type(), unsafe.Pointer(f.UnsafeAddr())).Elem()
return rf.MethodByName("Context").Call(nil)[0].Interface().(sdk.Context)
}

func (c ctxType) GetCtx(t *testing.T, bapp *BaseApp) sdk.Context {
if c == QueryCtx {
ctx, err := bapp.createQueryContext(1, false)
require.NoError(t, err)
return ctx
} else if c == CheckTxCtx {
return getCheckStateCtx(bapp)
}
// TODO: Not supported yet
return getDeliverStateCtx(bapp)
}

func TestQueryGasLimit(t *testing.T) {
testCases := []struct {
queryGasLimit uint64
gasActuallyUsed uint64
shouldQueryErr bool
}{
{queryGasLimit: 100, gasActuallyUsed: 50, shouldQueryErr: false}, // Valid case
{queryGasLimit: 100, gasActuallyUsed: 150, shouldQueryErr: true}, // gasActuallyUsed > queryGasLimit
{queryGasLimit: 0, gasActuallyUsed: 50, shouldQueryErr: false}, // fuzzing with queryGasLimit = 0
{queryGasLimit: 0, gasActuallyUsed: 0, shouldQueryErr: false}, // both queryGasLimit and gasActuallyUsed are 0
{queryGasLimit: 200, gasActuallyUsed: 200, shouldQueryErr: true}, // gasActuallyUsed > queryGasLimit
{queryGasLimit: 100, gasActuallyUsed: 1000, shouldQueryErr: true}, // gasActuallyUsed > queryGasLimit
}

for _, tc := range testCases {
for _, ctxType := range ctxTypes {
t.Run(fmt.Sprintf("%s: %d - %d", ctxType, tc.queryGasLimit, tc.gasActuallyUsed), func(t *testing.T) {
app := getQueryBaseapp(t)
SetQueryGasLimit(tc.queryGasLimit)(app)
ctx := ctxType.GetCtx(t, app)

// query gas limit should have no effect when CtxType != QueryCtx
f := func() { ctx.GasMeter().ConsumeGas(tc.gasActuallyUsed, "test") }
if tc.shouldQueryErr && ctxType == QueryCtx {
require.Panics(t, f)
} else {
require.NotPanics(t, f)
}
})
}
}
}

func TestInitChain_AppVersionSetToZero(t *testing.T) {
const expectedAppVersion = uint64(0)

Expand Down
10 changes: 10 additions & 0 deletions baseapp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package baseapp
import (
"fmt"
"io"
"math"

dbm "github.com/tendermint/tm-db"

Expand Down Expand Up @@ -32,6 +33,15 @@ func SetMinGasPrices(gasPricesStr string) func(*BaseApp) {
return func(bapp *BaseApp) { bapp.setMinGasPrices(gasPrices) }
}

// SetQueryGasLimit returns an option that sets a gas limit for queries.
func SetQueryGasLimit(queryGasLimit uint64) func(*BaseApp) {
if queryGasLimit == 0 {
queryGasLimit = math.MaxUint64
}

return func(bapp *BaseApp) { bapp.queryGasLimit = queryGasLimit }
}

// SetHaltHeight returns a BaseApp option function that sets the halt block height.
func SetHaltHeight(blockHeight uint64) func(*BaseApp) {
return func(bapp *BaseApp) { bapp.setHaltHeight(blockHeight) }
Expand Down
5 changes: 5 additions & 0 deletions server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ type BaseConfig struct {
// specified in this config (e.g. 0.25token1;0.0001token2).
MinGasPrices string `mapstructure:"minimum-gas-prices"`

// The maximum amount of gas a grpc/Rest query may consume.
// If set to 0, it is unbounded.
QueryGasLimit uint64 `mapstructure:"query-gas-limit"`

Pruning string `mapstructure:"pruning"`
PruningKeepRecent string `mapstructure:"pruning-keep-recent"`
PruningInterval string `mapstructure:"pruning-interval"`
Expand Down Expand Up @@ -224,6 +228,7 @@ func DefaultConfig() *Config {
return &Config{
BaseConfig: BaseConfig{
MinGasPrices: defaultMinGasPrices,
QueryGasLimit: 0,
InterBlockCache: true,
Pruning: pruningtypes.PruningOptionDefault,
PruningKeepRecent: "0",
Expand Down
4 changes: 4 additions & 0 deletions server/config/toml.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ const DefaultConfigTemplate = `# This is a TOML config file.
# specified in this config (e.g. 0.25token1;0.0001token2).
minimum-gas-prices = "{{ .BaseConfig.MinGasPrices }}"
# The maximum gas a query coming over rest/grpc may consume.
# If this is set to zero, the query can consume an unbounded amount of gas.
query-gas-limit = "{{ .BaseConfig.QueryGasLimit }}"
# default: only the last 100,000 states(approximately 1 week worth of state) are kept; pruning at 100 block intervals
# nothing: all historic states will be saved, nothing will be deleted (i.e. archiving node)
# everything: 10 latest states will be kept; pruning at 10 block intervals.
Expand Down
2 changes: 2 additions & 0 deletions server/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const (
flagTraceStore = "trace-store"
flagCPUProfile = "cpu-profile"
FlagMinGasPrices = "minimum-gas-prices"
FlagQueryGasLimit = "query-gas-limit"
FlagHaltHeight = "halt-height"
FlagHaltTime = "halt-time"
FlagInterBlockCache = "inter-block-cache"
Expand Down Expand Up @@ -142,6 +143,7 @@ which accepts a path for the resulting pprof file.
cmd.Flags().String(flagTransport, "socket", "Transport protocol: socket, grpc")
cmd.Flags().String(flagTraceStore, "", "Enable KVStore tracing to an output file")
cmd.Flags().String(FlagMinGasPrices, "", "Minimum gas prices to accept for transactions; Any fee in a tx must meet this minimum (e.g. 0.01photino;0.0001stake)")
cmd.Flags().Uint64(FlagQueryGasLimit, 0, "Maximum gas a Rest/Grpc query can consume. Blank and 0 imply unbounded.")
cmd.Flags().IntSlice(FlagUnsafeSkipUpgrades, []int{}, "Skip a set of upgrade heights to continue the old binary")
cmd.Flags().Uint64(FlagHaltHeight, 0, "Block height at which to gracefully halt the chain and shutdown the node")
cmd.Flags().Uint64(FlagHaltTime, 0, "Minimum block time (in Unix seconds) at which to gracefully halt the chain and shutdown the node")
Expand Down
1 change: 1 addition & 0 deletions simapp/simd/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ func (a appCreator) newApp(logger log.Logger, db dbm.DB, traceStore io.Writer, a
appOpts,
baseapp.SetPruning(pruningOpts),
baseapp.SetMinGasPrices(cast.ToString(appOpts.Get(server.FlagMinGasPrices))),
baseapp.SetQueryGasLimit(cast.ToUint64(appOpts.Get(server.FlagQueryGasLimit))),
baseapp.SetHaltHeight(cast.ToUint64(appOpts.Get(server.FlagHaltHeight))),
baseapp.SetHaltTime(cast.ToUint64(appOpts.Get(server.FlagHaltTime))),
baseapp.SetMinRetainBlocks(cast.ToUint64(appOpts.Get(server.FlagMinRetainBlocks))),
Expand Down

0 comments on commit 9294e56

Please sign in to comment.