Skip to content

Commit

Permalink
fix: remove txs from mempool when antehandler fails in recheck (cosmo…
Browse files Browse the repository at this point in the history
…s#20144)

Co-authored-by: Facundo <[email protected]>
  • Loading branch information
tac0turtle and facundomedica authored May 2, 2024
1 parent 8f10576 commit 599ae55
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 0 deletions.
101 changes: 101 additions & 0 deletions baseapp/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2398,3 +2398,104 @@ func TestOptimisticExecution(t *testing.T) {

require.Equal(t, int64(50), suite.baseApp.LastBlockHeight())
}

func TestABCI_Proposal_FailReCheckTx(t *testing.T) {
pool := mempool.NewPriorityMempool[int64](mempool.PriorityNonceMempoolConfig[int64]{
TxPriority: mempool.NewDefaultTxPriority(),
MaxTx: 0,
SignerExtractor: mempool.NewDefaultSignerExtractionAdapter(),
})

anteOpt := func(bapp *baseapp.BaseApp) {
bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (sdk.Context, error) {
// always fail on recheck, just to test the recheck logic
if ctx.IsReCheckTx() {
return ctx, errors.New("recheck failed in ante handler")
}

return ctx, nil
})
}

suite := NewBaseAppSuite(t, anteOpt, baseapp.SetMempool(pool))
baseapptestutil.RegisterKeyValueServer(suite.baseApp.MsgServiceRouter(), MsgKeyValueImpl{})
baseapptestutil.RegisterCounterServer(suite.baseApp.MsgServiceRouter(), NoopCounterServerImpl{})

_, err := suite.baseApp.InitChain(&abci.RequestInitChain{
ConsensusParams: &cmtproto.ConsensusParams{},
})
require.NoError(t, err)

tx := newTxCounter(t, suite.txConfig, 0, 1)
txBytes, err := suite.txConfig.TxEncoder()(tx)
require.NoError(t, err)

reqCheckTx := abci.RequestCheckTx{
Tx: txBytes,
Type: abci.CheckTxType_New,
}
_, err = suite.baseApp.CheckTx(&reqCheckTx)
require.NoError(t, err)

tx2 := newTxCounter(t, suite.txConfig, 1, 1)

tx2Bytes, err := suite.txConfig.TxEncoder()(tx2)
require.NoError(t, err)

err = pool.Insert(sdk.Context{}, tx2)
require.NoError(t, err)

require.Equal(t, 2, pool.CountTx())

// call prepareProposal before calling recheck tx, just as a sanity check
reqPrepareProposal := abci.RequestPrepareProposal{
MaxTxBytes: 1000,
Height: 1,
}
resPrepareProposal, err := suite.baseApp.PrepareProposal(&reqPrepareProposal)
require.NoError(t, err)
require.Equal(t, 2, len(resPrepareProposal.Txs))

// call recheck on the first tx, it MUST return an error
reqReCheckTx := abci.RequestCheckTx{
Tx: txBytes,
Type: abci.CheckTxType_Recheck,
}
resp, err := suite.baseApp.CheckTx(&reqReCheckTx)
require.NoError(t, err)
require.True(t, resp.IsErr())
require.Equal(t, "recheck failed in ante handler", resp.Log)

// call prepareProposal again, should return only the second tx
resPrepareProposal, err = suite.baseApp.PrepareProposal(&reqPrepareProposal)
require.NoError(t, err)
require.Equal(t, 1, len(resPrepareProposal.Txs))
require.Equal(t, tx2Bytes, resPrepareProposal.Txs[0])

// check the mempool, it should have only the second tx
require.Equal(t, 1, pool.CountTx())

reqProposalTxBytes := [][]byte{
tx2Bytes,
}
reqProcessProposal := abci.RequestProcessProposal{
Txs: reqProposalTxBytes,
Height: reqPrepareProposal.Height,
}

resProcessProposal, err := suite.baseApp.ProcessProposal(&reqProcessProposal)
require.NoError(t, err)
require.Equal(t, abci.ResponseProcessProposal_ACCEPT, resProcessProposal.Status)

// the same txs as in PrepareProposal
res, err := suite.baseApp.FinalizeBlock(&abci.RequestFinalizeBlock{
Height: suite.baseApp.LastBlockHeight() + 1,
Txs: reqProposalTxBytes,
})
require.NoError(t, err)

require.Equal(t, 0, pool.CountTx())

require.NotEmpty(t, res.TxResults[0].Events)
require.True(t, res.TxResults[0].IsOK(), fmt.Sprintf("%v", res))
}
6 changes: 6 additions & 0 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,12 @@ func (app *BaseApp) runTx(mode execMode, txBytes []byte) (gInfo sdk.GasInfo, res
gasWanted = ctx.GasMeter().Limit()

if err != nil {
if mode == execModeReCheck {
// if the ante handler fails on recheck, we want to remove the tx from the mempool
if mempoolErr := app.mempool.Remove(tx); mempoolErr != nil {
return gInfo, nil, anteEvents, errors.Join(err, mempoolErr)
}
}
return gInfo, nil, nil, err
}

Expand Down

0 comments on commit 599ae55

Please sign in to comment.