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

* 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 <[email protected]>

* lint fix

* tidy

* remove

* cleanup

---------

Co-authored-by: David Terpay <[email protected]>
Co-authored-by: David Terpay <[email protected]>
(cherry picked from commit f1cde2a)

# Conflicts:
#	README.md
#	abci/abci.go
#	abci/checktx/check_tx_test.go
#	abci/checktx/mempool_parity_check_tx.go
#	block/mocks/lane.go
#	block/mocks/lane_mempool.go
#	go.mod
#	go.sum
#	tests/app/app.go
#	tests/app/testappd/cmd/testnet.go
#	x/auction/types/mocks/bank_keeper.go
#	x/auction/types/mocks/staking_keeper.go

* bettington

---------

Co-authored-by: Alex Johnson <[email protected]>

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 @@ package checktx
 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 @@ 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 +38,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,14 +85,10 @@ func (m MempoolParityCheckTx) CheckTx() CheckTx {
  )
  }
 
- // 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 @@ func (m MempoolParityCheckTx) CheckTx() CheckTx {
  )
  }
  }
+ }()
+
+ // 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