Skip to content

Commit

Permalink
fix: mempool filter ibc spam (#8409)
Browse files Browse the repository at this point in the history 
* mempool filter

* clean

* add tests

* lint
  • Loading branch information
@czarcas7ic
czarcas7ic authored Jun 18, 2024
1 parent ff6e8b5 commit 3f220fd
Show file tree
Hide file tree
Showing 2 changed files with 213 additions and 0 deletions.
37 changes: 37 additions & 0 deletions x/txfees/keeper/feedecorator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import (
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 +61,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")
}
}
}

// 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
176 changes: 176 additions & 0 deletions x/txfees/keeper/feedecorator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,26 @@ package keeper_test
import (
"fmt"

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

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

clienttx "github.com/cosmos/cosmos-sdk/client/tx"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
"github.com/cosmos/cosmos-sdk/x/bank/testutil"
clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"

authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"

"github.com/osmosis-labs/osmosis/osmomath"
appparams "github.com/osmosis-labs/osmosis/v25/app/params"
"github.com/osmosis-labs/osmosis/v25/x/txfees/keeper"
"github.com/osmosis-labs/osmosis/v25/x/txfees/types"
)

Expand Down Expand Up @@ -169,3 +184,164 @@ func (s *KeeperTestSuite) TestFeeDecorator() {
})
}
}

func (s *KeeperTestSuite) TestMempoolFeeDecorator_AnteHandle_MsgTransfer() {
s.SetupTest(false)
mfd := keeper.NewMempoolFeeDecorator(*s.App.TxFeesKeeper, types.NewDefaultMempoolFeeOptions())

// Test cases
testCases := []struct {
name string
msg sdk.Msg
expectedErr error
}{
{
name: "MsgTransfer with valid size",
msg: &transfertypes.MsgTransfer{
SourcePort: "transfer",
SourceChannel: "channel-0",
Token: sdk.NewCoin("uosmo", sdk.NewInt(1000)),
Sender: "osmo1sender",
Receiver: "osmo1receiver",
TimeoutHeight: clienttypes.Height{},
TimeoutTimestamp: 0,
Memo: "valid memo",
},
},
{
name: "MsgTransfer in total too large",
msg: &transfertypes.MsgTransfer{
SourcePort: "transfer",
SourceChannel: "channel-0",
Token: sdk.NewCoin("uosmo", sdk.NewInt(1000)),
Sender: string(make([]byte, 35001)),
Receiver: string(make([]byte, 65000)),
TimeoutHeight: clienttypes.Height{},
TimeoutTimestamp: 0,
Memo: string(make([]byte, 400000)),
},
expectedErr: errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "msg size is too large"),
},
{
name: "MsgTransfer with memo too large",
msg: &transfertypes.MsgTransfer{
SourcePort: "transfer",
SourceChannel: "channel-0",
Token: sdk.NewCoin("uosmo", sdk.NewInt(1000)),
Sender: "osmo1sender",
Receiver: "osmo1receiver",
TimeoutHeight: clienttypes.Height{},
TimeoutTimestamp: 0,
Memo: string(make([]byte, 400001)), // 400KB + 1
},
expectedErr: errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "memo is too large"),
},
{
name: "MsgTransfer with receiver too large",
msg: &transfertypes.MsgTransfer{
SourcePort: "transfer",
SourceChannel: "channel-0",
Token: sdk.NewCoin("uosmo", sdk.NewInt(1000)),
Sender: "osmo1sender",
Receiver: string(make([]byte, 65001)), // 65KB + 1
TimeoutHeight: clienttypes.Height{},
TimeoutTimestamp: 0,
Memo: "valid memo",
},
expectedErr: errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "receiver address is too large"),
},
{
name: "MsgSendTx with valid packet data size",
msg: &icacontrollertypes.MsgSendTx{
Owner: "osmo1owner",
ConnectionId: "connection-0",
PacketData: icatypes.InterchainAccountPacketData{
Type: icatypes.EXECUTE_TX,
Data: make([]byte, 400000),
Memo: "valid memo",
},
},
},
{
name: "MsgSendTx with packet data size too large",
msg: &icacontrollertypes.MsgSendTx{
Owner: "osmo1owner",
ConnectionId: "connection-0",
PacketData: icatypes.InterchainAccountPacketData{
Type: icatypes.EXECUTE_TX,
Data: make([]byte, 400000), // 400KB
Memo: string(make([]byte, 100000)), // 100KB
},
},
expectedErr: errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "packet data is too large"),
},
{
name: "MsgSendTx with owner address too large",
msg: &icacontrollertypes.MsgSendTx{
Owner: string(make([]byte, 65001)), // 65KB + 1,
ConnectionId: "connection-0",
PacketData: icatypes.InterchainAccountPacketData{
Type: icatypes.EXECUTE_TX,
Data: make([]byte, 400000),
Memo: "valid memo",
},
},
expectedErr: errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "owner address is too large"),
},
}

for _, tc := range testCases {
s.Run(tc.name, func() {
baseDenom, _ := s.App.TxFeesKeeper.GetBaseDenom(s.Ctx)
txFee := sdk.NewCoins(sdk.NewCoin(baseDenom, sdk.NewInt(250000)))
tx, err := s.prepareTx(tc.msg, txFee)
s.Require().NoError(err)

_, err = mfd.AnteHandle(s.Ctx, tx, false, nextAnteHandler)

if tc.expectedErr != nil {
s.Require().Error(err)
s.Require().Equal(tc.expectedErr.Error(), err.Error())
} else {
s.Require().NoError(err)
}
})
}
}

func (s *KeeperTestSuite) prepareTx(msg sdk.Msg, txFee sdk.Coins) (sdk.Tx, error) {
txBuilder := s.clientCtx.TxConfig.NewTxBuilder()
priv0, _, addr0 := testdata.KeyTestPubAddr()
acc1 := s.App.AccountKeeper.NewAccountWithAddress(s.Ctx, addr0)
s.App.AccountKeeper.SetAccount(s.Ctx, acc1)
privs, accNums, accSeqs := []cryptotypes.PrivKey{priv0}, []uint64{0}, []uint64{0}
signerData := authsigning.SignerData{
ChainID: s.Ctx.ChainID(),
AccountNumber: accNums[0],
Sequence: accSeqs[0],
}

sigV2, err := clienttx.SignWithPrivKey(
1,
signerData,
txBuilder,
privs[0],
s.clientCtx.TxConfig,
accSeqs[0],
)
if err != nil {
return nil, err
}

err = testutil.FundAccount(s.App.BankKeeper, s.Ctx, addr0, txFee)
if err != nil {
return nil, err
}

tx := s.BuildTx(txBuilder, []sdk.Msg{msg}, sigV2, "", txFee, 100000000)
return tx, nil
}

func nextAnteHandler(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) {
return ctx, nil
}

0 comments on commit 3f220fd

Please sign in to comment.