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.

Sign up for GitHub

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

Jump to bottom

fix: mempool filter ibc spam #8409

Merged
merged 4 commits into from
Jun 18, 2024
Merged

fix: mempool filter ibc spam #8409

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5b35e2e
mempool filter
czarcas7ic Jun 18, 2024
1737c1b
clean
czarcas7ic Jun 18, 2024
179d7e7
add tests
czarcas7ic Jun 18, 2024
8677ec7
lint
czarcas7ic Jun 18, 2024
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
50 changes: 44 additions & 6 deletions x/txfees/keeper/feedecorator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package keeper

import (
"bytes"
"fmt"
"path/filepath"

errorsmod "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

icacontrollertypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/controller/types"
transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"

"github.com/osmosis-labs/osmosis/osmomath"
appparams "github.com/osmosis-labs/osmosis/v25/app/params"
mempool1559 "github.com/osmosis-labs/osmosis/v25/x/txfees/keeper/mempool-1559"
Expand Down Expand Up @@ -58,6 +62,40 @@ func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b
}
}

msgs := tx.GetMsgs()
for _, msg := range msgs {
// If one of the msgs is an IBC Transfer msg, limit it's size due to current spam potential.
// 500KB for entire msg
// 400KB for memo
// 65KB for receiver
if transferMsg, ok := msg.(*transfertypes.MsgTransfer); ok {
if transferMsg.Size() > 500000 { // 500KB
return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "msg size is too large")
}

if len([]byte(transferMsg.Memo)) > 400000 { // 400KB
return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "memo is too large")
}

if len(transferMsg.Receiver) > 65000 { // 65KB
return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "receiver address is too large")
}
}

// If one of the msgs is from ICA, limit it's size due to current spam potential.
// 500KB for packet data
// 65KB for sender
if icaMsg, ok := msg.(*icacontrollertypes.MsgSendTx); ok {
if icaMsg.PacketData.Size() > 500000 { // 500KB
return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "packet data is too large")
}

if len([]byte(icaMsg.Owner)) > 65000 { // 65KB
return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "owner address is too large")
}
}
}
Comment on lines +64 to +96
Copy link
Member

Choose a reason for hiding this comment

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

Consider copying some of the violating message JSONs from mintscan and adding them as unit tests

There is an example with arbitrage filters to help with boilerplate setup for this

Copy link
Member Author

Choose a reason for hiding this comment

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

Tests added here 179d7e7


// If this is genesis height, don't check the fee.
// This is needed so that gentx's can be created without having to pay a fee (chicken/egg problem).
if ctx.BlockHeight() == 0 {
Expand Down Expand Up @@ -121,7 +159,7 @@ func (mfd MempoolFeeDecorator) getMinBaseGasPrice(ctx sdk.Context, baseDenom str
// If we are in CheckTx, a separate function is ran locally to ensure sufficient fees for entering our mempool.
// So we ensure that the provided fees meet a minimum threshold for the validator
if (ctx.IsCheckTx() || ctx.IsReCheckTx()) && !simulate {
minBaseGasPrice = sdk.MaxDec(minBaseGasPrice, mfd.GetMinBaseGasPriceForTx(ctx, baseDenom, feeTx))
minBaseGasPrice = osmomath.MaxDec(minBaseGasPrice, mfd.GetMinBaseGasPriceForTx(ctx, baseDenom, feeTx))
}
// If we are in genesis or are simulating a tx, then we actually override all of the above, to set it to 0.
if ctx.BlockHeight() == 0 || simulate {
Expand Down Expand Up @@ -163,18 +201,18 @@ func (mfd MempoolFeeDecorator) GetMinBaseGasPriceForTx(ctx sdk.Context, baseDeno
cfgMinGasPrice := ctx.MinGasPrices().AmountOf(baseDenom)
// the check below prevents tx gas from getting over HighGasTxThreshold which is default to 1_000_000
if tx.GetGas() >= mfd.Opts.HighGasTxThreshold {
cfgMinGasPrice = sdk.MaxDec(cfgMinGasPrice, mfd.Opts.MinGasPriceForHighGasTx)
cfgMinGasPrice = osmomath.MaxDec(cfgMinGasPrice, mfd.Opts.MinGasPriceForHighGasTx)
}
if txfee_filters.IsArbTxLoose(tx) {
cfgMinGasPrice = sdk.MaxDec(cfgMinGasPrice, mfd.Opts.MinGasPriceForArbitrageTx)
cfgMinGasPrice = osmomath.MaxDec(cfgMinGasPrice, mfd.Opts.MinGasPriceForArbitrageTx)
}
// Initial tx only, no recheck
if is1559enabled && ctx.IsCheckTx() && !ctx.IsReCheckTx() {
cfgMinGasPrice = sdk.MaxDec(cfgMinGasPrice, mempool1559.CurEipState.GetCurBaseFee())
cfgMinGasPrice = osmomath.MaxDec(cfgMinGasPrice, mempool1559.CurEipState.GetCurBaseFee())
}
// RecheckTx only
if is1559enabled && ctx.IsReCheckTx() {
cfgMinGasPrice = sdk.MaxDec(cfgMinGasPrice, mempool1559.CurEipState.GetCurRecheckBaseFee())
cfgMinGasPrice = osmomath.MaxDec(cfgMinGasPrice, mempool1559.CurEipState.GetCurRecheckBaseFee())
}
return cfgMinGasPrice
}
Expand Down Expand Up @@ -228,7 +266,7 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo
if feeGranter != nil {
if dfd.feegrantKeeper == nil {
return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "fee grants is not enabled")
} else if !feeGranter.Equals(feePayer) {
} else if !bytes.Equal(feeGranter, feePayer) {
err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, tx.GetMsgs())
if err != nil {
return ctx, errorsmod.Wrapf(err, "%s not allowed to pay fees from %s", feeGranter, feePayer)
Expand Down