Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit simulation gas #674

Merged
merged 2 commits into from
Nov 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions app/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,27 @@ import (
channelkeeper "github.com/cosmos/cosmos-sdk/x/ibc/core/04-channel/keeper"
ibcante "github.com/cosmos/cosmos-sdk/x/ibc/core/ante"

wasmTypes "github.com/CosmWasm/wasmd/x/wasm/types"

wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper"
)

// NewAnteHandler returns an AnteHandler that checks and increments sequence
// numbers, checks signatures & account numbers, and deducts fees from the first
// signer.
func NewAnteHandler(
ak ante.AccountKeeper, bankKeeper types.BankKeeper,
ak ante.AccountKeeper,
bankKeeper types.BankKeeper,
sigGasConsumer ante.SignatureVerificationGasConsumer,
signModeHandler signing.SignModeHandler,
txCounterStoreKey sdk.StoreKey,
channelKeeper channelkeeper.Keeper,
wasmConfig wasmTypes.WasmConfig,
) sdk.AnteHandler {
// copied sdk https://github.com/cosmos/cosmos-sdk/blob/v0.42.9/x/auth/ante/ante.go
return sdk.ChainAnteDecorators(
ante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first
ante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first
wasmkeeper.NewLimitSimulationGasDecorator(wasmConfig.SimulationGasLimit), // after setup context to enforce limits early
wasmkeeper.NewCountTXDecorator(txCounterStoreKey),
ante.NewRejectExtensionOptionsDecorator(),
ante.NewMempoolFeeDecorator(),
Expand Down
20 changes: 16 additions & 4 deletions app/app.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package app

import (
"fmt"
"io"
"net/http"
"os"
Expand Down Expand Up @@ -235,9 +236,19 @@ type WasmApp struct {
}

// NewWasmApp returns a reference to an initialized WasmApp.
func NewWasmApp(logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bool,
skipUpgradeHeights map[int64]bool, homePath string, invCheckPeriod uint, enabledProposals []wasm.ProposalType,
appOpts servertypes.AppOptions, wasmOpts []wasm.Option, baseAppOptions ...func(*baseapp.BaseApp)) *WasmApp {
func NewWasmApp(
logger log.Logger,
db dbm.DB,
traceStore io.Writer,
loadLatest bool,
skipUpgradeHeights map[int64]bool,
homePath string,
invCheckPeriod uint,
enabledProposals []wasm.ProposalType,
appOpts servertypes.AppOptions,
wasmOpts []wasm.Option,
baseAppOptions ...func(*baseapp.BaseApp),
) *WasmApp {

encodingConfig := MakeEncodingConfig()
appCodec, legacyAmino := encodingConfig.Marshaler, encodingConfig.Amino
Expand Down Expand Up @@ -346,7 +357,7 @@ func NewWasmApp(logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest b
wasmDir := filepath.Join(homePath, "wasm")
wasmConfig, err := wasm.ReadWasmConfig(appOpts)
if err != nil {
panic("error while reading wasm config: " + err.Error())
panic(fmt.Sprintf("error while reading wasm config: %s", err))
}

// The last arguments can contain custom message handlers, and custom query handlers,
Expand Down Expand Up @@ -485,6 +496,7 @@ func NewWasmApp(logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest b
NewAnteHandler(
app.accountKeeper, app.bankKeeper, ante.DefaultSigVerificationGasConsumer,
encodingConfig.TxConfig.SignModeHandler(), keys[wasm.StoreKey], app.ibcKeeper.ChannelKeeper,
wasmConfig,
),
)
app.SetEndBlocker(app.EndBlocker)
Expand Down
41 changes: 41 additions & 0 deletions x/wasm/keeper/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,44 @@ func encodeHeightCounter(height int64, counter uint32) []byte {
func decodeHeightCounter(bz []byte) (int64, uint32) {
return int64(sdk.BigEndianToUint64(bz[0:8])), binary.BigEndian.Uint32(bz[8:])
}

// LimitSimulationGasDecorator ante decorator to limit gas in simulation calls
type LimitSimulationGasDecorator struct {
gasLimit *sdk.Gas
}

// NewLimitSimulationGasDecorator constructor accepts nil value to fallback to block gas limit.
func NewLimitSimulationGasDecorator(gasLimit *sdk.Gas) *LimitSimulationGasDecorator {
if gasLimit != nil && *gasLimit == 0 {
panic("gas limit must not be zero")
}
return &LimitSimulationGasDecorator{gasLimit: gasLimit}
}

// AnteHandle that limits the maximum gas available in simulations only.
// A custom max value can be configured and will be applied when set. The value should not
// exceed the max block gas limit.
// Different values on nodes are not consensus breaking as they affect only
// simulations but may have effect on client user experience.
//
// When no custom value is set then the max block gas is used as default limit.
func (d LimitSimulationGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
if !simulate {
// Wasm code is not executed in checkTX so that we don't need to limit it further.
// Tendermint rejects the TX afterwards when the tx.gas > max block gas.
// On deliverTX we rely on the tendermint/sdk mechanics that ensure
// tx has gas set and gas < max block gas
return next(ctx, tx, simulate)
}

// apply custom node gas limit
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Maybe this could be "simplified"? But fine as is.

if d.gasLimit != nil {
  ctx = ctx.WithGasMeter(sdk.NewGasMeter(*d.gasLimit))
} else if maxGas := ctx.ConsensusParams().GetBlock().MaxGas; maxGas > 0 { 
  ctx = ctx.WithGasMeter(sdk.NewGasMeter(sdk.Gas(maxGas)))
}
return next(ctx, tx, simulate)

if d.gasLimit != nil {
return next(ctx.WithGasMeter(sdk.NewGasMeter(*d.gasLimit)), tx, simulate)
}

// default to max block gas when set, to be on the safe side
if maxGas := ctx.ConsensusParams().GetBlock().MaxGas; maxGas > 0 {
return next(ctx.WithGasMeter(sdk.NewGasMeter(sdk.Gas(maxGas))), tx, simulate)
}
return next(ctx, tx, simulate)
}
82 changes: 80 additions & 2 deletions x/wasm/keeper/ante_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package keeper
package keeper_test

import (
"testing"
"time"

abci "github.com/tendermint/tendermint/abci/types"

"github.com/CosmWasm/wasmd/x/wasm/keeper"

"github.com/cosmos/cosmos-sdk/store"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -97,7 +101,7 @@ func TestCountTxDecorator(t *testing.T) {
var anyTx sdk.Tx

// when
ante := NewCountTXDecorator(keyWasm)
ante := keeper.NewCountTXDecorator(keyWasm)
_, gotErr := ante.AnteHandle(ctx, anyTx, spec.simulate, spec.nextAssertAnte)
if spec.expErr {
require.Error(t, gotErr)
Expand All @@ -107,3 +111,77 @@ func TestCountTxDecorator(t *testing.T) {
})
}
}
func TestLimitSimulationGasDecorator(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice tests

var (
hundred sdk.Gas = 100
zero sdk.Gas = 0
)
specs := map[string]struct {
customLimit *sdk.Gas
consumeGas sdk.Gas
maxBlockGas int64
simulation bool
expErr interface{}
}{
"custom limit set": {
customLimit: &hundred,
consumeGas: hundred + 1,
maxBlockGas: -1,
simulation: true,
expErr: sdk.ErrorOutOfGas{Descriptor: "testing"},
},
"block limit set": {
maxBlockGas: 100,
consumeGas: hundred + 1,
simulation: true,
expErr: sdk.ErrorOutOfGas{Descriptor: "testing"},
},
"no limits set": {
maxBlockGas: -1,
consumeGas: hundred + 1,
simulation: true,
},
"both limits set, custom applies": {
customLimit: &hundred,
consumeGas: hundred - 1,
maxBlockGas: 10,
simulation: true,
},
"not a simulation": {
customLimit: &hundred,
consumeGas: hundred + 1,
simulation: false,
},
"zero custom limit": {
customLimit: &zero,
simulation: true,
expErr: "gas limit must not be zero",
},
}
for name, spec := range specs {
t.Run(name, func(t *testing.T) {
nextAnte := consumeGasAnteHandler(spec.consumeGas)
ctx := sdk.Context{}.
WithGasMeter(sdk.NewInfiniteGasMeter()).
WithConsensusParams(&abci.ConsensusParams{
Block: &abci.BlockParams{MaxGas: spec.maxBlockGas}})
// when
if spec.expErr != nil {
require.PanicsWithValue(t, spec.expErr, func() {
ante := keeper.NewLimitSimulationGasDecorator(spec.customLimit)
ante.AnteHandle(ctx, nil, spec.simulation, nextAnte)
})
return
}
ante := keeper.NewLimitSimulationGasDecorator(spec.customLimit)
ante.AnteHandle(ctx, nil, spec.simulation, nextAnte)
})
}
}

func consumeGasAnteHandler(gasToConsume sdk.Gas) sdk.AnteHandler {
return func(ctx sdk.Context, tx sdk.Tx, simulate bool) (sdk.Context, error) {
ctx.GasMeter().ConsumeGas(gasToConsume, "testing")
return ctx, nil
}
}
15 changes: 13 additions & 2 deletions x/wasm/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ var (

// Module init related flags
const (
flagWasmMemoryCacheSize = "wasm.memory_cache_size"
flagWasmQueryGasLimit = "wasm.query_gas_limit"
flagWasmMemoryCacheSize = "wasm.memory_cache_size"
flagWasmQueryGasLimit = "wasm.query_gas_limit"
flagWasmSimulationGasLimit = "wasm.simulation_gas_limit"
)

// AppModuleBasic defines the basic application module used by the wasm module.
Expand Down Expand Up @@ -199,6 +200,7 @@ func AddModuleInitFlags(startCmd *cobra.Command) {
defaults := DefaultWasmConfig()
startCmd.Flags().Uint32(flagWasmMemoryCacheSize, defaults.MemoryCacheSize, "Sets the size in MiB (NOT bytes) of an in-memory cache for Wasm modules. Set to 0 to disable.")
startCmd.Flags().Uint64(flagWasmQueryGasLimit, defaults.SmartQueryGasLimit, "Set the max gas that can be spent on executing a query with a Wasm contract")
startCmd.Flags().String(flagWasmSimulationGasLimit, "", "Set the max gas that can be spent when executing a simulation TX")
}

// ReadWasmConfig reads the wasm specifig configuration
Expand All @@ -215,6 +217,15 @@ func ReadWasmConfig(opts servertypes.AppOptions) (types.WasmConfig, error) {
return cfg, err
}
}
if v := opts.Get(flagWasmSimulationGasLimit); v != nil {
if raw, ok := v.(string); ok && raw != "" {
limit, err := cast.ToUint64E(v) // non empty string set
if err != nil {
return cfg, err
}
cfg.SimulationGasLimit = &limit
}
}
// attach contract debugging to global "trace" flag
if v := opts.Get(server.FlagTrace); v != nil {
if cfg.ContractDebugMode, err = cast.ToBoolE(v); err != nil {
Expand Down
12 changes: 8 additions & 4 deletions x/wasm/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ import (
)

const (
defaultMemoryCacheSize uint32 = 100 // in MiB
defaultQueryGasLimit uint64 = 3000000
defaultContractDebugMode = false
defaultMemoryCacheSize uint32 = 100 // in MiB
defaultSmartQueryGasLimit uint64 = 3_000_000
defaultContractDebugMode = false
)

func (m Model) ValidateBasic() error {
Expand Down Expand Up @@ -296,6 +296,10 @@ func NewWasmCoins(cosmosCoins sdk.Coins) (wasmCoins []wasmvmtypes.Coin) {

// WasmConfig is the extra config required for wasm
type WasmConfig struct {
// SimulationGasLimit is the max gas to be used in a tx simulation call.
// When not set the consensus max block gas is used instead
SimulationGasLimit *uint64
// SimulationGasLimit is the max gas to be used in a smart query contract call
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just taking a look at this. You mean, SmartQueryGasLimit.

SmartQueryGasLimit uint64
// MemoryCacheSize in MiB not bytes
MemoryCacheSize uint32
Expand All @@ -306,7 +310,7 @@ type WasmConfig struct {
// DefaultWasmConfig returns the default settings for WasmConfig
func DefaultWasmConfig() WasmConfig {
return WasmConfig{
SmartQueryGasLimit: defaultQueryGasLimit,
SmartQueryGasLimit: defaultSmartQueryGasLimit,
MemoryCacheSize: defaultMemoryCacheSize,
ContractDebugMode: defaultContractDebugMode,
}
Expand Down