Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: mempool lane size check on CheckTx #561

Merged
merged 17 commits into from
Jul 2, 2024
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions abci/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
81 changes: 73 additions & 8 deletions abci/checktx/check_tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)

Expand All @@ -69,6 +82,7 @@ func (s *CheckTxTestSuite) TestCheckTxMempoolParity() {
ba := &baseApp{
s.Ctx,
}

mevLaneHandler := checktx.NewMEVCheckTxHandler(
ba,
cacheDecoder.TxDecoder(),
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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() {
Expand All @@ -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")})
Expand All @@ -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)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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")
}

Expand All @@ -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.
Expand Down
116 changes: 107 additions & 9 deletions abci/checktx/mempool_parity_check_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import (
"fmt"

cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"

"cosmossdk.io/log"

cmtabci "github.com/cometbft/cometbft/abci/types"
Expand All @@ -26,6 +28,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.
Expand All @@ -34,12 +40,14 @@
mempl block.Mempool,
txDecoder sdk.TxDecoder,
checkTxHandler CheckTx,
baseApp BaseApp,
) MempoolParityCheckTx {
return MempoolParityCheckTx{
logger: logger,
mempl: mempl,
txDecoder: txDecoder,
checkTxHandler: checkTxHandler,
baseApp: baseApp,
}
}

Expand Down Expand Up @@ -79,29 +87,119 @@
), 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)
aljo242 marked this conversation as resolved.
Show resolved Hide resolved
lane, err := m.matchLane(sdkCtx, tx)
if err != nil {
if isReCheck && txInMempool {
removeTx = true

Check warning on line 116 in abci/checktx/mempool_parity_check_tx.go

View check run for this annotation

Codecov / codecov/patch

abci/checktx/mempool_parity_check_tx.go#L115-L116

Added lines #L115 - L116 were not covered by tests
}

m.logger.Debug("failed to match lane", "lane", lane, "err", err)
return sdkerrors.ResponseCheckTxWithEvents(
err,
0,
0,
nil,
false,
), nil

Check warning on line 126 in abci/checktx/mempool_parity_check_tx.go

View check run for this annotation

Codecov / codecov/patch

abci/checktx/mempool_parity_check_tx.go#L119-L126

Added lines #L119 - L126 were not covered by tests
}

consensusParams := sdkCtx.ConsensusParams()
aljo242 marked this conversation as resolved.
Show resolved Hide resolved
laneSize := lane.GetMaxBlockSpace().MulInt64(consensusParams.GetBlock().GetMaxBytes()).TruncateInt64()

txSize := int64(len(req.Tx))
if txSize > laneSize {
if isReCheck && txInMempool {
removeTx = true

Check warning on line 135 in abci/checktx/mempool_parity_check_tx.go

View check run for this annotation

Codecov / codecov/patch

abci/checktx/mempool_parity_check_tx.go#L135

Added line #L135 was not covered by tests
}

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,
)

Check warning on line 173 in abci/checktx/mempool_parity_check_tx.go

View check run for this annotation

Codecov / codecov/patch

abci/checktx/mempool_parity_check_tx.go#L170-L173

Added lines #L170 - L173 were not covered by tests

return nil, fmt.Errorf("failed match tx to lane")

Check warning on line 175 in abci/checktx/mempool_parity_check_tx.go

View check run for this annotation

Codecov / codecov/patch

abci/checktx/mempool_parity_check_tx.go#L175

Added line #L175 was not covered by tests
}

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
}
6 changes: 3 additions & 3 deletions abci/checktx/mev_check_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
Loading
Loading