fix: mempool lane size check on CheckTx (#561)

* push

* init

* fix setup

* format

* fix test

* use lane

* ok

* finalize

* fix everything

* lint fix:

* Update abci/checktx/mempool_parity_check_tx.go

Co-authored-by: David Terpay <35130517+davidterpay@users.noreply.github.com>

* lint fix

* tidy

* remove

* cleanup

---------

Co-authored-by: David Terpay <david.terpay@gmail.com>
Co-authored-by: David Terpay <35130517+davidterpay@users.noreply.github.com>

Author: Alex Johnson

README.md

@@ -25,7 +25,7 @@
 
 > **Note**: The BlockSDK is open source software that Skip maintains. We strive to be responsive to questions and issues within 1-2 weeks - please ask in our [#block-sdk-support discord](https://discord.com/invite/hFeHVAE26P) channel, or open a GitHub issue!
 
-**Please note the status of BlockSDK maintainenace:**
+**Please note the status of BlockSDK maintenance:**
 
 1. We are not currently providing hands-on support for new integrations.
 2. We have not yet completed our entire testing process for the FreeLane. We recommend integrators who want to have the best experience utilize the MEV Lane and the Default Lane only at this time.

abci/abci.go

@@ -8,6 +8,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/cosmos/cosmos-sdk/baseapp"
+
 	"github.com/skip-mev/block-sdk/v2/block"
 	"github.com/skip-mev/block-sdk/v2/block/proposals"
 	"github.com/skip-mev/block-sdk/v2/block/utils"

abci/checktx/check_tx_test.go

@@ -43,6 +43,18 @@ func (s *CheckTxTestSuite) TestCheckTxMempoolParity() {
 	)
 	s.Require().NoError(err)
 
+	hugeBidTx, _, err := testutils.CreateNAuctionTx(
+		s.EncCfg.TxConfig,
+		s.Accounts[0],
+		sdk.NewCoin(s.GasTokenDenom, math.NewInt(100)),
+		0,
+		0,
+		[]testutils.Account{s.Accounts[0]},
+		100,
+		100000,
+	)
+	s.Require().NoError(err)
+
 	// create a tx that should not be inserted in the mev-lane
 	bidTx2, _, err := testutils.CreateAuctionTx(
 		s.EncCfg.TxConfig,
@@ -56,10 +68,11 @@ func (s *CheckTxTestSuite) TestCheckTxMempoolParity() {
 	s.Require().NoError(err)
 
 	txs := map[sdk.Tx]bool{
-		bidTx: true,
+		bidTx:     true,
+		hugeBidTx: true,
 	}
 
-	mevLane := s.InitLane(math.LegacyOneDec(), txs)
+	mevLane := s.InitLane(math.LegacyOneDec(), txs, true)
 	mempool, err := block.NewLanedMempool(s.Ctx.Logger(), []block.Lane{mevLane})
 	s.Require().NoError(err)
 
@@ -69,6 +82,7 @@ func (s *CheckTxTestSuite) TestCheckTxMempoolParity() {
 	ba := &baseApp{
 		s.Ctx,
 	}
+
 	mevLaneHandler := checktx.NewMEVCheckTxHandler(
 		ba,
 		cacheDecoder.TxDecoder(),
@@ -82,6 +96,7 @@ func (s *CheckTxTestSuite) TestCheckTxMempoolParity() {
 		mempool,
 		cacheDecoder.TxDecoder(),
 		mevLaneHandler,
+		ba,
 	).CheckTx()
 
 	// test that a bid can be successfully inserted to mev-lane on CheckTx
@@ -99,6 +114,36 @@ func (s *CheckTxTestSuite) TestCheckTxMempoolParity() {
 		s.Require().True(mevLane.Contains(bidTx))
 	})
 
+	// test that a bid will fail to be inserted as it is too large
+	s.Run("test bid insertion failure on CheckTx - too large", func() {
+		txBz, err := s.EncCfg.TxConfig.TxEncoder()(hugeBidTx)
+		s.Require().NoError(err)
+
+		// check tx
+		res, err := handler(&cometabci.RequestCheckTx{Tx: txBz, Type: cometabci.CheckTxType_New})
+		s.Require().NoError(err)
+
+		s.Require().Equal(uint32(1), res.Code)
+
+		// check that the mev-lane does not contain the bid
+		s.Require().False(mevLane.Contains(bidTx))
+	})
+
+	// test that a bid can be successfully inserted to mev-lane on CheckTx
+	s.Run("test bid insertion on CheckTx", func() {
+		txBz, err := s.EncCfg.TxConfig.TxEncoder()(bidTx)
+		s.Require().NoError(err)
+
+		// check tx
+		res, err := handler(&cometabci.RequestCheckTx{Tx: txBz, Type: cometabci.CheckTxType_New})
+		s.Require().NoError(err)
+
+		s.Require().Equal(uint32(0), res.Code)
+
+		// check that the mev-lane contains the bid
+		s.Require().True(mevLane.Contains(bidTx))
+	})
+
 	// test that a bid-tx (not in mev-lane) can be removed from the mempool on ReCheck
 	s.Run("test bid removal on ReCheckTx", func() {
 		// assert that the mev-lane does not contain the bidTx2
@@ -128,13 +173,17 @@ func (s *CheckTxTestSuite) TestRemovalOnRecheckTx() {
 	)
 	s.Require().NoError(err)
 
-	mevLane := s.InitLane(math.LegacyOneDec(), nil)
+	mevLane := s.InitLane(math.LegacyOneDec(), nil, true)
 	mempool, err := block.NewLanedMempool(s.Ctx.Logger(), []block.Lane{mevLane})
 	s.Require().NoError(err)
 
 	cacheDecoder, err := utils.NewDefaultCacheTxDecoder(s.EncCfg.TxConfig.TxDecoder())
 	s.Require().NoError(err)
 
+	ba := &baseApp{
+		s.Ctx,
+	}
+
 	handler := checktx.NewMempoolParityCheckTx(
 		s.Ctx.Logger(),
 		mempool,
@@ -143,6 +192,7 @@ func (s *CheckTxTestSuite) TestRemovalOnRecheckTx() {
 			// always fail
 			return &cometabci.ResponseCheckTx{Code: 1}, nil
 		},
+		ba,
 	).CheckTx()
 
 	s.Run("tx is removed on check-tx failure when re-check", func() {
@@ -169,11 +219,16 @@ func (s *CheckTxTestSuite) TestMempoolParityCheckTx() {
 		cacheDecoder, err := utils.NewDefaultCacheTxDecoder(s.EncCfg.TxConfig.TxDecoder())
 		s.Require().NoError(err)
 
+		ba := &baseApp{
+			s.Ctx,
+		}
+
 		handler := checktx.NewMempoolParityCheckTx(
 			s.Ctx.Logger(),
 			nil,
 			cacheDecoder.TxDecoder(),
 			nil,
+			ba,
 		)
 
 		res, err := handler.CheckTx()(&cometabci.RequestCheckTx{Tx: []byte("invalid-tx")})
@@ -186,7 +241,7 @@ func (s *CheckTxTestSuite) TestMempoolParityCheckTx() {
 func (s *CheckTxTestSuite) TestMEVCheckTxHandler() {
 	txs := map[sdk.Tx]bool{}
 
-	mevLane := s.InitLane(math.LegacyOneDec(), txs)
+	mevLane := s.InitLane(math.LegacyOneDec(), txs, true)
 	mempool, err := block.NewLanedMempool(s.Ctx.Logger(), []block.Lane{mevLane})
 	s.Require().NoError(err)
 
@@ -222,6 +277,7 @@ func (s *CheckTxTestSuite) TestMEVCheckTxHandler() {
 		mempool,
 		cacheDecoder.TxDecoder(),
 		mevLaneHandler,
+		ba,
 	).CheckTx()
 
 	// test that a normal tx can be successfully inserted to the mempool
@@ -287,7 +343,7 @@ func (s *CheckTxTestSuite) TestValidateBidTx() {
 		invalidBidTx: true,
 	}
 
-	mevLane := s.InitLane(math.LegacyOneDec(), txs)
+	mevLane := s.InitLane(math.LegacyOneDec(), txs, true)
 
 	cacheDecoder, err := utils.NewDefaultCacheTxDecoder(s.EncCfg.TxConfig.TxDecoder())
 	s.Require().NoError(err)
@@ -347,7 +403,7 @@ func (ba *baseApp) CommitMultiStore() storetypes.CommitMultiStore {
 
 // CheckTx is baseapp's CheckTx method that checks the validity of a
 // transaction.
-func (baseApp) CheckTx(_ *cometabci.RequestCheckTx) (*cometabci.ResponseCheckTx, error) {
+func (ba *baseApp) CheckTx(_ *cometabci.RequestCheckTx) (*cometabci.ResponseCheckTx, error) {
 	return nil, fmt.Errorf("not implemented")
 }
 
@@ -362,8 +418,17 @@ func (ba *baseApp) LastBlockHeight() int64 {
 }
 
 // GetConsensusParams is utilized to retrieve the consensus params.
-func (baseApp) GetConsensusParams(ctx sdk.Context) cmtproto.ConsensusParams {
-	return ctx.ConsensusParams()
+func (ba *baseApp) GetConsensusParams(_ sdk.Context) cmtproto.ConsensusParams {
+	return cmtproto.ConsensusParams{
+		Block: &cmtproto.BlockParams{
+			MaxBytes: 10000,
+			MaxGas:   10000,
+		},
+		Evidence:  nil,
+		Validator: nil,
+		Version:   nil,
+		Abci:      nil,
+	}
 }
 
 // ChainID is utilized to retrieve the chain ID.

abci/checktx/mempool_parity_check_tx.go

@@ -3,6 +3,8 @@ package checktx
 import (
 	"fmt"
 
+	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
+
 	"cosmossdk.io/log"
 
 	cmtabci "github.com/cometbft/cometbft/abci/types"
@@ -26,6 +28,10 @@ type MempoolParityCheckTx struct {
 
 	// checkTxHandler to wrap
 	checkTxHandler CheckTx
+
+	// baseApp is utilized to retrieve the latest committed state and to call
+	// baseapp's CheckTx method.
+	baseApp BaseApp
 }
 
 // NewMempoolParityCheckTx returns a new MempoolParityCheckTx handler.
@@ -34,12 +40,14 @@ func NewMempoolParityCheckTx(
 	mempl block.Mempool,
 	txDecoder sdk.TxDecoder,
 	checkTxHandler CheckTx,
+	baseApp BaseApp,
 ) MempoolParityCheckTx {
 	return MempoolParityCheckTx{
 		logger:         logger,
 		mempl:          mempl,
 		txDecoder:      txDecoder,
 		checkTxHandler: checkTxHandler,
+		baseApp:        baseApp,
 	}
 }
 
@@ -79,29 +87,119 @@ func (m MempoolParityCheckTx) CheckTx() CheckTx {
 			), nil
 		}
 
-		// run the checkTxHandler
-		res, checkTxError := m.checkTxHandler(req)
-
-		// if re-check fails for a transaction, we'll need to explicitly purge the tx from
-		// the app-side mempool
-		if isInvalidCheckTxExecution(res, checkTxError) && isReCheck {
-			// check if the tx exists first
-			if txInMempool {
+		// prepare cleanup closure to remove tx if marked
+		removeTx := false
+		defer func() {
+			if removeTx {
 				// remove the tx
 				if err := m.mempl.Remove(tx); err != nil {
 					m.logger.Debug(
 						"failed to remove tx from app-side mempool when purging for re-check failure",
 						"removal-err", err,
-						"check-tx-err", checkTxError,
 					)
 				}
 			}
+		}()
+
+		// run the checkTxHandler
+		res, checkTxError := m.checkTxHandler(req)
+		// if re-check fails for a transaction, we'll need to explicitly purge the tx from
+		// the app-side mempool
+		if isInvalidCheckTxExecution(res, checkTxError) && isReCheck && txInMempool {
+			removeTx = true
+		}
+
+		sdkCtx := m.GetContextForTx(req)
+		lane, err := m.matchLane(sdkCtx, tx)
+		if err != nil {
+			if isReCheck && txInMempool {
+				removeTx = true
+			}
+
+			m.logger.Debug("failed to match lane", "lane", lane, "err", err)
+			return sdkerrors.ResponseCheckTxWithEvents(
+				err,
+				0,
+				0,
+				nil,
+				false,
+			), nil
+		}
+
+		consensusParams := sdkCtx.ConsensusParams()
+		laneSize := lane.GetMaxBlockSpace().MulInt64(consensusParams.GetBlock().GetMaxBytes()).TruncateInt64()
+
+		txSize := int64(len(req.Tx))
+		if txSize > laneSize {
+			if isReCheck && txInMempool {
+				removeTx = true
+			}
+
+			m.logger.Debug(
+				"tx size exceeds max block bytes",
+				"tx", tx,
+				"tx size", txSize,
+				"max bytes", laneSize,
+			)
+
+			return sdkerrors.ResponseCheckTxWithEvents(
+				fmt.Errorf("tx size exceeds max bytes for lane %s", lane.Name()),
+				0,
+				0,
+				nil,
+				false,
+			), nil
 		}
 
 		return res, checkTxError
 	}
 }
 
+// matchLane returns a Lane if the given tx matches the Lane.
+func (m MempoolParityCheckTx) matchLane(ctx sdk.Context, tx sdk.Tx) (block.Lane, error) {
+	var lane block.Lane
+	// find corresponding lane for this tx
+	for _, l := range m.mempl.Registry() {
+		if l.Match(ctx, tx) {
+			lane = l
+			break
+		}
+	}
+
+	if lane == nil {
+		m.logger.Debug(
+			"failed match tx to lane",
+			"tx", tx,
+		)
+
+		return nil, fmt.Errorf("failed match tx to lane")
+	}
+
+	return lane, nil
+}
+
 func isInvalidCheckTxExecution(resp *cmtabci.ResponseCheckTx, checkTxErr error) bool {
 	return resp == nil || resp.Code != 0 || checkTxErr != nil
 }
+
+// GetContextForTx is returns the latest committed state and sets the context given
+// the checkTx request.
+func (m MempoolParityCheckTx) GetContextForTx(req *cmtabci.RequestCheckTx) sdk.Context {
+	// Retrieve the commit multi-store which is used to retrieve the latest committed state.
+	ms := m.baseApp.CommitMultiStore().CacheMultiStore()
+
+	// Create a new context based off of the latest committed state.
+	header := cmtproto.Header{
+		Height:  m.baseApp.LastBlockHeight(),
+		ChainID: m.baseApp.ChainID(),
+	}
+	ctx, _ := sdk.NewContext(ms, header, true, m.baseApp.Logger()).CacheContext()
+
+	// Set the remaining important context values.
+	ctx = ctx.
+		WithTxBytes(req.Tx).
+		WithEventManager(sdk.NewEventManager()).
+		WithConsensusParams(m.baseApp.GetConsensusParams(ctx))
+
+	return ctx
+}

abci/checktx/mev_check_tx.go

@@ -17,7 +17,7 @@ import (
 	"github.com/skip-mev/block-sdk/v2/x/auction/types"
 )
 
-// MevCheckTxHandler is a wrapper around baseapp's CheckTx method that allows us to
+// MEVCheckTxHandler is a wrapper around baseapp's CheckTx method that allows us to
 // verify bid transactions against the latest committed state. All other transactions
 // are executed normally using base app's CheckTx. This defines all of the
 // dependencies that are required to verify a bid transaction.
@@ -69,7 +69,7 @@ type BaseApp interface {
 	ChainID() string
 }
 
-// NewCheckTxHandler constructs a new CheckTxHandler instance. This method fails if the given LanedMempool does not have a lane
+// NewMEVCheckTxHandler constructs a new CheckTxHandler instance. This method fails if the given LanedMempool does not have a lane
 // adhering to the MevLaneI interface
 func NewMEVCheckTxHandler(
 	baseApp BaseApp,
@@ -87,7 +87,7 @@ func NewMEVCheckTxHandler(
 	}
 }
 
-// CheckTxHandler is a wrapper around baseapp's CheckTx method that allows us to
+// CheckTx is a wrapper around baseapp's CheckTx method that allows us to
 // verify bid transactions against the latest committed state. All other transactions
 // are executed normally. We must verify each bid tx and all of its bundled transactions
 // before we can insert it into the mempool against the latest commit state because