fix(mempool parity): Enforce comet / app-side mempool parity in CheckTx + integ. tests [BLO-584] [BLO-635] (#306)

* account setup for network tests

* Add Test case for app-mempool / cmt mempool parity

* add fix

* move check-tx handlers to wrap each other

* linting

* migrate to chaintestutils

* linting

* additional test-case

* lint

* remove paralell tests

* remove MEVLaneI

* fix(check_tx): Check error of GetAuctionBid in ValidateBidTx [BLO-461] (#312)

* add err check in ValidateBidTx

* add test-case for ValidateBidTx

* remove -race flag for integ
This commit is contained in:
Nikhil Vasan
2023-12-18 18:29:06 -08:00
committed by GitHub
parent 4b6e481c3f
commit b5fe2a772c
16 changed files with 1196 additions and 406 deletions
+11
View File
@@ -0,0 +1,11 @@
package checktx
import (
cometabci "github.com/cometbft/cometbft/abci/types"
)
type (
// CheckTx is baseapp's CheckTx method that checks the validity of a
// transaction.
CheckTx func(req *cometabci.RequestCheckTx) (*cometabci.ResponseCheckTx, error)
)
+328
View File
@@ -0,0 +1,328 @@
package checktx_test
import (
"fmt"
"testing"
"cosmossdk.io/log"
"cosmossdk.io/math"
"cosmossdk.io/store"
storetypes "cosmossdk.io/store/types"
cometabci "github.com/cometbft/cometbft/abci/types"
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
db "github.com/cosmos/cosmos-db"
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/skip-mev/block-sdk/lanes/mev"
mevlanetestutils "github.com/skip-mev/block-sdk/lanes/mev/testutils"
"github.com/skip-mev/block-sdk/testutils"
auctiontypes "github.com/skip-mev/block-sdk/x/auction/types"
blocksdktypes "github.com/skip-mev/block-sdk/x/blocksdk/types"
)
type CheckTxTestSuite struct {
mevlanetestutils.MEVLaneTestSuiteBase
}
func TestCheckTxTestSuite(t *testing.T) {
suite.Run(t, new(CheckTxTestSuite))
}
func (s *CheckTxTestSuite) TestCheckTxMempoolParity() {
bidTx, _, err := testutils.CreateAuctionTx(
s.EncCfg.TxConfig,
s.Accounts[0],
sdk.NewCoin(s.GasTokenDenom, math.NewInt(100)),
0,
0,
nil,
100,
)
s.Require().NoError(err)
// create a tx that should not be inserted in the mev-lane
bidTx2, _, err := testutils.CreateAuctionTx(
s.EncCfg.TxConfig,
s.Accounts[0],
sdk.NewCoin(s.GasTokenDenom, math.NewInt(100)),
1,
0,
nil,
100,
)
s.Require().NoError(err)
txs := map[sdk.Tx]bool{
bidTx: true,
}
mevLane := s.InitLane(math.LegacyOneDec(), txs)
mempool, err := block.NewLanedMempool(s.Ctx.Logger(), []block.Lane{mevLane}, moduleLaneFetcher{
mevLane,
})
s.Require().NoError(err)
ba := &baseApp{
s.Ctx,
}
mevLaneHandler := checktx.NewMEVCheckTxHandler(
ba,
s.EncCfg.TxConfig.TxDecoder(),
mevLane,
s.SetUpAnteHandler(txs),
ba.CheckTx,
).CheckTx()
handler := checktx.NewMempoolParityCheckTx(
s.Ctx.Logger(),
mempool,
s.EncCfg.TxConfig.TxDecoder(),
mevLaneHandler,
).CheckTx()
// 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
s.Require().False(mevLane.Contains(bidTx2))
// check tx
txBz, err := s.EncCfg.TxConfig.TxEncoder()(bidTx2)
s.Require().NoError(err)
res, err := handler(&cometabci.RequestCheckTx{Tx: txBz, Type: cometabci.CheckTxType_Recheck})
s.Require().NoError(err)
s.Require().Equal(uint32(1), res.Code)
})
}
func (s *CheckTxTestSuite) TestMempoolParityCheckTx() {
s.Run("tx fails tx-decoding", func() {
handler := checktx.NewMempoolParityCheckTx(
s.Ctx.Logger(),
nil,
s.EncCfg.TxConfig.TxDecoder(),
nil,
)
res, err := handler.CheckTx()(&cometabci.RequestCheckTx{Tx: []byte("invalid-tx")})
s.Require().NoError(err)
s.Require().Equal(uint32(1), res.Code)
})
}
func (s *CheckTxTestSuite) TestMEVCheckTxHandler() {
txs := map[sdk.Tx]bool{}
mevLane := s.InitLane(math.LegacyOneDec(), txs)
mempool, err := block.NewLanedMempool(s.Ctx.Logger(), []block.Lane{mevLane}, moduleLaneFetcher{
mevLane,
})
s.Require().NoError(err)
ba := &baseApp{
s.Ctx,
}
acc := s.Accounts[0]
// create a tx that should not be inserted in the mev-lane
normalTx, err := testutils.CreateRandomTxBz(s.EncCfg.TxConfig, acc, 0, 1, 0, 0)
s.Require().NoError(err)
var gotTx []byte
mevLaneHandler := checktx.NewMEVCheckTxHandler(
ba,
s.EncCfg.TxConfig.TxDecoder(),
mevLane,
s.SetUpAnteHandler(txs),
func(req *cometabci.RequestCheckTx) (*cometabci.ResponseCheckTx, error) {
// expect the above free tx to be sent here
gotTx = req.Tx
return &cometabci.ResponseCheckTx{
Code: uint32(0),
}, nil
},
).CheckTx()
handler := checktx.NewMempoolParityCheckTx(
s.Ctx.Logger(),
mempool,
s.EncCfg.TxConfig.TxDecoder(),
mevLaneHandler,
).CheckTx()
// test that a normal tx can be successfully inserted to the mempool
s.Run("test non-mev tx insertion on CheckTx", func() {
res, err := handler(&cometabci.RequestCheckTx{Tx: normalTx, Type: cometabci.CheckTxType_New})
s.Require().NoError(err)
s.Require().Equal(uint32(0), res.Code)
s.Require().Equal(normalTx, gotTx)
})
}
func (s *CheckTxTestSuite) TestValidateBidTx() {
validBidTx, bundled, err := testutils.CreateAuctionTx(
s.EncCfg.TxConfig,
s.Accounts[0],
sdk.NewCoin(s.GasTokenDenom, math.NewInt(100)),
0,
0,
[]testutils.Account{s.Accounts[0]},
100,
)
s.Require().NoError(err)
txBz, err := s.EncCfg.TxConfig.TxEncoder()(validBidTx)
s.Require().NoError(err)
// create an invalid bid-tx (nested)
bidMsg := auctiontypes.NewMsgAuctionBid(s.Accounts[0].Address, sdk.NewCoin(s.GasTokenDenom, math.NewInt(100)), [][]byte{
txBz,
})
nestedBidTx, err := testutils.CreateTx(
s.EncCfg.TxConfig,
s.Accounts[0],
0,
0,
[]sdk.Msg{bidMsg},
)
s.Require().NoError(err)
// create an invalid bid-tx (signer invalid)
invalidBidMsg := auctiontypes.MsgAuctionBid{
Bidder: "",
Bid: sdk.NewCoin(s.GasTokenDenom, math.NewInt(100)),
Transactions: nil,
}
invalidBidTx, err := testutils.CreateTx(
s.EncCfg.TxConfig,
s.Accounts[0],
0,
0,
[]sdk.Msg{&invalidBidMsg},
)
s.Require().NoError(err)
// create a tx that should not be inserted in the mev-lane
s.Require().NoError(err)
txs := map[sdk.Tx]bool{
validBidTx: true,
bundled[0]: true,
nestedBidTx: true,
invalidBidTx: true,
}
mevLane := s.InitLane(math.LegacyOneDec(), txs)
ba := &baseApp{
s.Ctx,
}
mevLaneHandler := checktx.NewMEVCheckTxHandler(
ba,
s.EncCfg.TxConfig.TxDecoder(),
mevLane,
s.SetUpAnteHandler(txs),
ba.CheckTx,
)
s.Run("expected bid-tx", func() {
bundledTx, err := s.EncCfg.TxConfig.TxEncoder()(bundled[0])
s.Require().NoError(err)
_, err = mevLaneHandler.ValidateBidTx(s.Ctx, validBidTx, &auctiontypes.BidInfo{
Transactions: [][]byte{bundledTx},
})
s.Require().NoError(err)
})
s.Run("nested bid-tx", func() {
nestedBidTxBz, err := s.EncCfg.TxConfig.TxEncoder()(nestedBidTx)
s.Require().NoError(err)
_, err = mevLaneHandler.ValidateBidTx(s.Ctx, nestedBidTx, &auctiontypes.BidInfo{
Transactions: [][]byte{nestedBidTxBz},
})
s.Require().Error(err)
s.Require().Contains(err.Error(), "bundled tx cannot be a bid tx")
})
s.Run("invalid bid-tx", func() {
invalidBidTxBz, err := s.EncCfg.TxConfig.TxEncoder()(invalidBidTx)
s.Require().NoError(err)
_, err = mevLaneHandler.ValidateBidTx(s.Ctx, invalidBidTx, &auctiontypes.BidInfo{
Transactions: [][]byte{invalidBidTxBz},
})
s.Require().Error(err)
s.Require().Contains(err.Error(), "failed to get bid info")
})
}
type baseApp struct {
ctx sdk.Context
}
// CommitMultiStore is utilized to retrieve the latest committed state.
func (ba *baseApp) CommitMultiStore() storetypes.CommitMultiStore {
db := db.NewMemDB()
return store.NewCommitMultiStore(db, ba.ctx.Logger(), nil)
}
// CheckTx is baseapp's CheckTx method that checks the validity of a
// transaction.
func (baseApp) CheckTx(_ *cometabci.RequestCheckTx) (*cometabci.ResponseCheckTx, error) {
return nil, fmt.Errorf("not implemented")
}
// Logger is utilized to log errors.
func (ba *baseApp) Logger() log.Logger {
return ba.ctx.Logger()
}
// LastBlockHeight is utilized to retrieve the latest block height.
func (ba *baseApp) LastBlockHeight() int64 {
return ba.ctx.BlockHeight()
}
// GetConsensusParams is utilized to retrieve the consensus params.
func (baseApp) GetConsensusParams(ctx sdk.Context) cmtproto.ConsensusParams {
return ctx.ConsensusParams()
}
// ChainID is utilized to retrieve the chain ID.
func (ba *baseApp) ChainID() string {
return ba.ctx.ChainID()
}
type moduleLaneFetcher struct {
lane *mev.MEVLane
}
func (mlf moduleLaneFetcher) GetLane(sdk.Context, string) (lane blocksdktypes.Lane, err error) {
return blocksdktypes.Lane{}, nil
}
func (mlf moduleLaneFetcher) GetLanes(sdk.Context) []blocksdktypes.Lane {
return nil
}
+77
View File
@@ -0,0 +1,77 @@
package checktx
import (
"fmt"
"cosmossdk.io/log"
cmtabci "github.com/cometbft/cometbft/abci/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/skip-mev/block-sdk/block"
)
// MempoolParityCheckTx is a CheckTx function that evicts txs that are not in the app-side mempool
// on ReCheckTx. This handler is used to enforce parity in the app-side / comet mempools.
type MempoolParityCheckTx struct {
// logger
logger log.Logger
// app side mempool interface
mempl block.Mempool
// tx-decoder
txDecoder sdk.TxDecoder
// checkTxHandler to wrap
checkTxHandler CheckTx
}
// NewMempoolParityCheckTx returns a new MempoolParityCheckTx handler.
func NewMempoolParityCheckTx(logger log.Logger, mempl block.Mempool, txDecoder sdk.TxDecoder, checkTxHandler CheckTx) MempoolParityCheckTx {
return MempoolParityCheckTx{
logger: logger,
mempl: mempl,
txDecoder: txDecoder,
checkTxHandler: checkTxHandler,
}
}
// CheckTx returns a CheckTx handler that wraps a given CheckTx handler and evicts txs that are not
// in the app-side mempool on ReCheckTx.
func (m MempoolParityCheckTx) CheckTx() CheckTx {
return func(req *cmtabci.RequestCheckTx) (*cmtabci.ResponseCheckTx, error) {
// decode tx
tx, err := m.txDecoder(req.Tx)
if err != nil {
return sdkerrors.ResponseCheckTxWithEvents(
fmt.Errorf("failed to decode tx: %w", err),
0,
0,
nil,
false,
), nil
}
// if the mode is ReCheck and the app's mempool does not contain the given tx, we fail
// immediately, to purge the tx from the comet mempool.
if req.Type == cmtabci.CheckTxType_Recheck && !m.mempl.Contains(tx) {
m.logger.Debug(
"tx from comet mempool not found in app-side mempool",
"tx", tx,
)
return sdkerrors.ResponseCheckTxWithEvents(
fmt.Errorf("tx from comet mempool not found in app-side mempool"),
0,
0,
nil,
false,
), nil
}
// run the checkTxHandler
return m.checkTxHandler(req)
}
}
+301
View File
@@ -0,0 +1,301 @@
package checktx
import (
"context"
"fmt"
cometabci "github.com/cometbft/cometbft/abci/types"
cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"cosmossdk.io/log"
storetypes "cosmossdk.io/store/types"
"github.com/skip-mev/block-sdk/block"
mevlane "github.com/skip-mev/block-sdk/lanes/mev"
"github.com/skip-mev/block-sdk/x/auction/types"
)
// 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.
type MEVCheckTxHandler struct {
// baseApp is utilized to retrieve the latest committed state and to call
// baseapp's CheckTx method.
baseApp BaseApp
// txDecoder is utilized to decode transactions to determine if they are
// bid transactions.
txDecoder sdk.TxDecoder
// MEVLane is utilized to retrieve the bid info of a transaction and to
// insert a bid transaction into the application-side mempool.
mevLane MEVLaneI
// anteHandler is utilized to verify the bid transaction against the latest
// committed state.
anteHandler sdk.AnteHandler
// checkTxHandler is the wrapped CheckTx handler that is used to execute all non-bid txs
checkTxHandler CheckTx
}
// MEVLaneI defines the interface for the mev auction lane. This interface
// is utilized by both the x/auction module and the checkTx handler.
type MEVLaneI interface {
block.Lane
mevlane.Factory
GetTopAuctionTx(ctx context.Context) sdk.Tx
}
// BaseApp is an interface that allows us to call baseapp's CheckTx method
// as well as retrieve the latest committed state.
type BaseApp interface {
// CommitMultiStore is utilized to retrieve the latest committed state.
CommitMultiStore() storetypes.CommitMultiStore
// Logger is utilized to log errors.
Logger() log.Logger
// LastBlockHeight is utilized to retrieve the latest block height.
LastBlockHeight() int64
// GetConsensusParams is utilized to retrieve the consensus params.
GetConsensusParams(ctx sdk.Context) cmtproto.ConsensusParams
// ChainID is utilized to retrieve the chain ID.
ChainID() string
}
// NewCheckTxHandler 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,
txDecoder sdk.TxDecoder,
mevLane MEVLaneI,
anteHandler sdk.AnteHandler,
checkTxHandler CheckTx,
) *MEVCheckTxHandler {
return &MEVCheckTxHandler{
baseApp: baseApp,
txDecoder: txDecoder,
mevLane: mevLane,
anteHandler: anteHandler,
checkTxHandler: checkTxHandler,
}
}
// CheckTxHandler 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
// otherwise the auction can be griefed. No state changes are applied to the state
// during this process.
func (handler *MEVCheckTxHandler) CheckTx() CheckTx {
return func(req *cometabci.RequestCheckTx) (resp *cometabci.ResponseCheckTx, err error) {
defer func() {
if rec := recover(); rec != nil {
handler.baseApp.Logger().Error(
"panic in check tx handler",
"err", rec,
)
err = fmt.Errorf("panic in check tx handler: %s", rec)
resp = sdkerrors.ResponseCheckTxWithEvents(
err,
0,
0,
nil,
false,
)
}
}()
tx, err := handler.txDecoder(req.Tx)
if err != nil {
handler.baseApp.Logger().Info(
"failed to decode tx",
"err", err,
)
return sdkerrors.ResponseCheckTxWithEvents(
fmt.Errorf("failed to decode tx: %w", err),
0,
0,
nil,
false,
), nil
}
// Attempt to get the bid info of the transaction.
bidInfo, err := handler.mevLane.GetAuctionBidInfo(tx)
if err != nil {
handler.baseApp.Logger().Info(
"failed to get auction bid info",
"err", err,
)
return sdkerrors.ResponseCheckTxWithEvents(
fmt.Errorf("failed to get auction bid info: %w", err),
0,
0,
nil,
false,
), nil
}
// If this is not a bid transaction, we just execute it normally.
if bidInfo == nil {
resp, err := handler.checkTxHandler(req)
if err != nil {
handler.baseApp.Logger().Info(
"failed to execute check tx",
"err", err,
)
}
return resp, err
}
// We attempt to get the latest committed state in order to verify transactions
// as if they were to be executed at the top of the block. After verification, this
// context will be discarded and will not apply any state changes.
ctx := handler.GetContextForBidTx(req)
// Verify the bid transaction.
gasInfo, err := handler.ValidateBidTx(ctx, tx, bidInfo)
if err != nil {
handler.baseApp.Logger().Info(
"invalid bid tx",
"err", err,
"height", ctx.BlockHeight(),
"bid_height", bidInfo.Timeout,
"bidder", bidInfo.Bidder,
"bid", bidInfo.Bid,
"is_recheck_tx", ctx.IsReCheckTx(),
)
// attempt to remove the bid from the MEVLane (if it exists)
if handler.mevLane.Contains(tx) {
if err := handler.mevLane.Remove(tx); err != nil {
handler.baseApp.Logger().Error(
"failed to remove bid transaction from mev-lane",
"err", err,
)
}
}
return sdkerrors.ResponseCheckTxWithEvents(
fmt.Errorf("invalid bid tx: %w", err),
gasInfo.GasWanted,
gasInfo.GasUsed,
nil,
false,
), nil
}
handler.baseApp.Logger().Info(
"valid bid tx",
"height", ctx.BlockHeight(),
"bid_height", bidInfo.Timeout,
"bidder", bidInfo.Bidder,
"bid", bidInfo.Bid,
"inserting tx into mempool", true,
)
// If the bid transaction is valid, we know we can insert it into the mempool for consideration in the next block.
if err := handler.mevLane.Insert(ctx, tx); err != nil {
handler.baseApp.Logger().Info(
"invalid bid tx; failed to insert bid transaction into mempool",
"err", err,
)
return sdkerrors.ResponseCheckTxWithEvents(
fmt.Errorf("invalid bid tx; failed to insert bid transaction into mempool: %w", err),
gasInfo.GasWanted,
gasInfo.GasUsed,
nil,
false,
), nil
}
return &cometabci.ResponseCheckTx{
Code: cometabci.CodeTypeOK,
GasWanted: int64(gasInfo.GasWanted),
GasUsed: int64(gasInfo.GasUsed),
}, nil
}
}
// ValidateBidTx is utilized to verify the bid transaction against the latest committed state.
func (handler *MEVCheckTxHandler) ValidateBidTx(ctx sdk.Context, bidTx sdk.Tx, bidInfo *types.BidInfo) (sdk.GasInfo, error) {
// Verify the bid transaction.
ctx, err := handler.anteHandler(ctx, bidTx, false)
if err != nil {
return sdk.GasInfo{}, fmt.Errorf("invalid bid tx; failed to execute ante handler: %w", err)
}
// Store the gas info and priority of the bid transaction before applying changes with other transactions.
gasInfo := sdk.GasInfo{
GasWanted: ctx.GasMeter().Limit(),
GasUsed: ctx.GasMeter().GasConsumed(),
}
// Verify all of the bundled transactions.
for _, tx := range bidInfo.Transactions {
bundledTx, err := handler.mevLane.WrapBundleTransaction(tx)
if err != nil {
return gasInfo, fmt.Errorf("invalid bid tx; failed to decode bundled tx: %w", err)
}
// bid txs cannot be included in bundled txs
bidInfo, err := handler.mevLane.GetAuctionBidInfo(bundledTx)
if err != nil {
return gasInfo, fmt.Errorf("invalid bid tx; failed to get bid info: %w", err)
}
if bidInfo != nil {
return gasInfo, fmt.Errorf("invalid bid tx; bundled tx cannot be a bid tx")
}
if ctx, err = handler.anteHandler(ctx, bundledTx, false); err != nil {
return gasInfo, fmt.Errorf("invalid bid tx; failed to execute bundled transaction: %w", err)
}
}
return gasInfo, nil
}
// GetContextForBidTx is returns the latest committed state and sets the context given
// the checkTx request.
func (handler *MEVCheckTxHandler) GetContextForBidTx(req *cometabci.RequestCheckTx) sdk.Context {
// Retrieve the commit multi-store which is used to retrieve the latest committed state.
ms := handler.baseApp.CommitMultiStore().CacheMultiStore()
// Create a new context based off of the latest committed state.
header := cmtproto.Header{
Height: handler.baseApp.LastBlockHeight(),
ChainID: handler.baseApp.ChainID(),
}
ctx, _ := sdk.NewContext(ms, header, true, handler.baseApp.Logger()).CacheContext()
// Set the context to the correct checking mode.
switch req.Type {
case cometabci.CheckTxType_New:
ctx = ctx.WithIsCheckTx(true)
case cometabci.CheckTxType_Recheck:
ctx = ctx.WithIsReCheckTx(true)
default:
panic("unknown check tx type")
}
// Set the remaining important context values.
ctx = ctx.
WithTxBytes(req.Tx).
WithEventManager(sdk.NewEventManager()).
WithConsensusParams(handler.baseApp.GetConsensusParams(ctx))
return ctx
}