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

README.md

@@ -29,7 +29,6 @@ Please note, we are not currently providing hands-on support for new integration
 
 **🌐 The Block SDK is a toolkit for building customized blocks**. The Block SDK is a set of Cosmos SDK and ABCI++ primitives that allow chains to fully customize blocks to specific use cases. It turns your chain's blocks into a **`highway`** consisting of individual **`lanes`** with their own special functionality.
 
-
 Skip has built out a number of plug-and-play `lanes` on the SDK that your protocol can use, including in-protocol MEV recapture and Oracles! Additionally, the Block SDK can be extended to add **your own custom `lanes`** to configure your blocks to exactly fit your application needs.
 
 ### 📚 Block SDK Documentation

abci/checktx/check_tx_test.go

@@ -1,23 +1,20 @@
 package checktx_test
 
 import (
+	"github.com/cosmos/cosmos-sdk/store"
 	"testing"
 
 	"cosmossdk.io/math"
-
 	db "github.com/cometbft/cometbft-db"
 	cometabci "github.com/cometbft/cometbft/abci/types"
+	"github.com/cometbft/cometbft/libs/log"
 	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
+	storetypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/stretchr/testify/suite"
 
 	"github.com/skip-mev/block-sdk/abci/checktx"
 	"github.com/skip-mev/block-sdk/block"
-
-	"github.com/cometbft/cometbft/libs/log"
-	"github.com/cosmos/cosmos-sdk/store"
-	storetypes "github.com/cosmos/cosmos-sdk/store/types"
-
 	"github.com/skip-mev/block-sdk/block/utils"
 	mevlanetestutils "github.com/skip-mev/block-sdk/lanes/mev/testutils"
 	"github.com/skip-mev/block-sdk/testutils"
@@ -44,6 +41,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,
@@ -57,10 +66,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)
 
@@ -70,6 +80,7 @@ func (s *CheckTxTestSuite) TestCheckTxMempoolParity() {
 	ba := &baseApp{
 		s.Ctx,
 	}
+
 	mevLaneHandler := checktx.NewMEVCheckTxHandler(
 		ba,
 		cacheDecoder.TxDecoder(),
@@ -84,6 +95,7 @@ func (s *CheckTxTestSuite) TestCheckTxMempoolParity() {
 		mempool,
 		cacheDecoder.TxDecoder(),
 		mevLaneHandler,
+		ba,
 	).CheckTx()
 
 	// test that a bid can be successfully inserted to mev-lane on CheckTx
@@ -100,6 +112,35 @@ 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 := handler(cometabci.RequestCheckTx{Tx: txBz, Type: cometabci.CheckTxType_New})
+
+		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 := 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 +169,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 +188,7 @@ func (s *CheckTxTestSuite) TestRemovalOnRecheckTx() {
 			// always fail
 			return cometabci.ResponseCheckTx{Code: 1}
 		},
+		ba,
 	).CheckTx()
 
 	s.Run("tx is removed on check-tx failure when re-check", func() {
@@ -169,11 +215,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 := handler.CheckTx()(cometabci.RequestCheckTx{Tx: []byte("invalid-tx")})
@@ -185,7 +236,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 +273,7 @@ func (s *CheckTxTestSuite) TestMEVCheckTxHandler() {
 		mempool,
 		cacheDecoder.TxDecoder(),
 		mevLaneHandler,
+		ba,
 	).CheckTx()
 
 	// test that a normal tx can be successfully inserted to the mempool
@@ -286,7 +338,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)
@@ -363,5 +415,13 @@ func (ba *baseApp) LastBlockHeight() int64 {
 
 // GetConsensusParams is utilized to retrieve the consensus params.
 func (baseApp) GetConsensusParams(ctx sdk.Context) *cmtproto.ConsensusParams {
-	return ctx.ConsensusParams()
+	return &cmtproto.ConsensusParams{
+		Block: &cmtproto.BlockParams{
+			MaxBytes: 10000,
+			MaxGas:   10000,
+		},
+		Evidence:  nil,
+		Validator: nil,
+		Version:   nil,
+	}
 }

abci/checktx/mempool_parity_check_tx.go

@@ -3,9 +3,9 @@
 import (
 	"fmt"
 
-	"github.com/cometbft/cometbft/libs/log"
-
 	cmtabci "github.com/cometbft/cometbft/abci/types"
+	"github.com/cometbft/cometbft/libs/log"
+	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 
@@ -26,6 +26,10 @@
 
 	// 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 +38,14 @@
 	mempl block.Mempool,
 	txDecoder sdk.TxDecoder,
 	checkTxHandler CheckTx,
+	baseApp BaseApp,
 ) MempoolParityCheckTx {
 	return MempoolParityCheckTx{
 		logger:         logger,
 		mempl:          mempl,
 		txDecoder:      txDecoder,
 		checkTxHandler: checkTxHandler,
+		baseApp:        baseApp,
 	}
 }
 
@@ -79,14 +85,10 @@
 			)
 		}
 
-		// run the checkTxHandler
-		res := 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) && 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(
@@ -95,12 +97,106 @@
 					)
 				}
 			}
+		}()
+
+		// run the checkTxHandler
+		res := 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) && 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,
+			)
+		}
+
+		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,
+			)
 		}
 
 		return res
 	}
 }
 
+// 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) bool {
 	return resp.Code != 0
 }
+
+// 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(),
+	}
+	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

@@ -16,7 +16,7 @@ import (
 	"github.com/skip-mev/block-sdk/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.
@@ -68,7 +68,7 @@ type BaseApp interface {
 	GetConsensusParams(ctx sdk.Context) *cmtproto.ConsensusParams
 }
 
-// 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,
@@ -88,7 +88,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