diff --git a/x/txfees/keeper/feedecorator.go b/x/txfees/keeper/feedecorator.go index fa34516059a..51273a757d6 100644 --- a/x/txfees/keeper/feedecorator.go +++ b/x/txfees/keeper/feedecorator.go @@ -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" @@ -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 { diff --git a/x/txfees/keeper/feedecorator_test.go b/x/txfees/keeper/feedecorator_test.go index 0c4d5113191..4984649b109 100644 --- a/x/txfees/keeper/feedecorator_test.go +++ b/x/txfees/keeper/feedecorator_test.go @@ -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" ) @@ -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 +}