From 86cbc5f246ba7f25d55b084091107472739c095d Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Tue, 7 Oct 2025 12:04:10 +0800 Subject: [PATCH 1/2] test to reproduce out of order nonce --- .../internal/mempool/mempool_test.go | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/sei-tendermint/internal/mempool/mempool_test.go b/sei-tendermint/internal/mempool/mempool_test.go index e6189fccb1..df8615ead6 100644 --- a/sei-tendermint/internal/mempool/mempool_test.go +++ b/sei-tendermint/internal/mempool/mempool_test.go @@ -1206,3 +1206,107 @@ func TestMempoolExpiration(t *testing.T) { require.Equal(t, 0, txmp.expirationIndex.Size()) require.Equal(t, 0, txmp.txStore.Size()) } + +type nonceCheckApplication struct { + *application + nonces map[string]uint64 + pendingNonces map[string]map[uint64]struct{} + pendingNonceMtx *sync.RWMutex +} + +func (app *nonceCheckApplication) CheckTx(_ context.Context, req *abci.RequestCheckTx) (*abci.ResponseCheckTxV2, error) { + nonce := uint64(req.Tx[0]) + sender := string(req.Tx[1:]) + if nonce < app.nonces[sender] { + return &abci.ResponseCheckTxV2{ + ResponseCheckTx: &abci.ResponseCheckTx{ + Code: code.CodeTypeBadNonce, + }, + }, nil + } + res := &abci.ResponseCheckTxV2{ + ResponseCheckTx: &abci.ResponseCheckTx{ + Code: code.CodeTypeOK, + }, + EVMNonce: nonce, + EVMSenderAddress: sender, + IsEVM: true, + Checker: func() abci.PendingTxCheckerResponse { return abci.Accepted }, + ExpireTxHandler: func() {}, + } + app.pendingNonceMtx.Lock() + defer app.pendingNonceMtx.Unlock() + if _, ok := app.pendingNonces[sender]; !ok { + app.pendingNonces[sender] = map[uint64]struct{}{} + } + app.pendingNonces[sender][nonce] = struct{}{} + if nonce == app.nonces[sender] { + return res, nil + } + res.IsPendingTransaction = true + res.ExpireTxHandler = func() { + app.pendingNonceMtx.Lock() + defer app.pendingNonceMtx.Unlock() + delete(app.pendingNonces[sender], nonce) + } + res.Checker = func() abci.PendingTxCheckerResponse { + app.pendingNonceMtx.RLock() + defer app.pendingNonceMtx.RUnlock() + for nextPending := app.nonces[sender]; ; nextPending++ { + if _, ok := app.pendingNonces[sender][nextPending]; !ok { + if nonce < nextPending { + return abci.Accepted + } + return abci.Pending + } + } + } + return res, nil +} + +func TestTxMempool_PendingNonce(t *testing.T) { + // Setup + ctx := t.Context() + + client := abciclient.NewLocalClient(log.NewNopLogger(), &nonceCheckApplication{ + application: &application{Application: kvstore.NewApplication()}, + nonces: map[string]uint64{"sender": 0}, + pendingNonces: map[string]map[uint64]struct{}{}, + pendingNonceMtx: &sync.RWMutex{}, + }) + if err := client.Start(ctx); err != nil { + t.Fatal(err) + } + t.Cleanup(client.Wait) + txmp := setup(t, client, 500) + txmp.config.TTLNumBlocks = 10 + txmp.height = 1 + + tx1 := append([]byte{0}, []byte("sender")...) + require.NoError(t, txmp.CheckTx(ctx, tx1, nil, TxInfo{SenderID: 0, SenderNodeID: "test"})) + require.Len(t, txmp.pendingTxs.txs, 0) + require.Len(t, txmp.priorityIndex.txs, 1) + + require.NoError(t, txmp.Update(ctx, txmp.height+1, []types.Tx{}, []*abci.ExecTxResult{}, nil, nil, false)) + require.Len(t, txmp.pendingTxs.txs, 0) + require.Len(t, txmp.priorityIndex.txs, 1) + + tx2 := append([]byte{1}, []byte("sender")...) + require.NoError(t, txmp.CheckTx(ctx, tx2, nil, TxInfo{SenderID: 0, SenderNodeID: "test"})) + require.Len(t, txmp.pendingTxs.txs, 1) + require.Len(t, txmp.priorityIndex.txs, 1) + + require.NoError(t, txmp.Update(ctx, txmp.height+1, []types.Tx{}, []*abci.ExecTxResult{}, nil, nil, false)) + require.Len(t, txmp.pendingTxs.txs, 0) // both txs are promoted to priorityIndex + require.Len(t, txmp.priorityIndex.txs, 1) + require.Len(t, txmp.priorityIndex.evmQueue["sender"], 2) // two transactions from the sender + + // a few blocks later, the txs haven't been included in a block yet... + require.NoError(t, txmp.Update(ctx, txmp.height+txmp.config.TTLNumBlocks-1, []types.Tx{}, []*abci.ExecTxResult{}, nil, nil, false)) + require.Len(t, txmp.priorityIndex.txs, 1) + require.Len(t, txmp.priorityIndex.evmQueue["sender"], 1) // two transactions from the sender + + txs := txmp.ReapMaxBytesMaxGas(1000, 1000, 1000) + require.Len(t, txs, 1) + require.Equal(t, tx2, []byte(txs[0])) // tx2 with out of order nonce is included! +} From 110545156a5dbbfa0aa0a001aa9baed716026ccf Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Wed, 8 Oct 2025 15:50:00 +0800 Subject: [PATCH 2/2] Add lightweight nonce check before proposing block --- app/abci.go | 32 +++ sei-cosmos/baseapp/abci.go | 25 ++- sei-tendermint/abci/client/grpc_client.go | 9 + sei-tendermint/abci/client/mocks/client.go | 10 + sei-tendermint/abci/client/socket_client.go | 9 + .../abci/example/kvstore/kvstore.go | 4 + sei-tendermint/abci/types/application.go | 12 +- .../abci/types/mocks/application.go | 10 + sei-tendermint/config/config.go | 5 + sei-tendermint/config/toml.go | 2 + .../internal/consensus/mempool_test.go | 11 +- .../internal/consensus/replay_stubs.go | 8 +- sei-tendermint/internal/mempool/mempool.go | 18 +- .../internal/mempool/mempool_test.go | 48 ++++- .../internal/mempool/mocks/mempool.go | 4 +- sei-tendermint/internal/mempool/tx.go | 1 + sei-tendermint/internal/mempool/types.go | 2 +- sei-tendermint/internal/proxy/client.go | 10 + sei-tendermint/internal/state/execution.go | 2 +- .../internal/state/execution_test.go | 2 +- x/evm/keeper/keeper.go | 26 +++ x/evm/keeper/keeper_test.go | 188 ++++++++++++++++++ 22 files changed, 411 insertions(+), 27 deletions(-) diff --git a/app/abci.go b/app/abci.go index b01e210e16..d8f4123540 100644 --- a/app/abci.go +++ b/app/abci.go @@ -2,10 +2,13 @@ package app import ( "context" + "fmt" "time" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/sei-protocol/sei-chain/utils/metrics" + evmtypes "github.com/sei-protocol/sei-chain/x/evm/types" abci "github.com/tendermint/tendermint/abci/types" "go.opentelemetry.io/otel/attribute" ) @@ -38,6 +41,26 @@ func (app *App) CheckTx(ctx context.Context, req *abci.RequestCheckTx) (*abci.Re return app.BaseApp.CheckTx(ctx, req) } +func (app *App) CheckTxWrapped(ctx context.Context, req *abci.RequestCheckTx) (*abci.ResponseCheckTxV2, any, error) { + _, span := app.GetBaseApp().TracingInfo.Start("CheckTxWrapped") + defer span.End() + tx, err := app.txDecoder(req.Tx) + if err != nil { + res := sdkerrors.ResponseCheckTx(err, 0, 0, false) + return &abci.ResponseCheckTxV2{ResponseCheckTx: &res}, nil, err + } + res, err := app.CheckTxDecoded(tx, req) + if err != nil { + return res, nil, err + } + if tx != nil && len(tx.GetMsgs()) > 0 { + if evmMsg, ok := tx.GetMsgs()[0].(*evmtypes.MsgEVMTransaction); ok { + return res, evmMsg, nil + } + } + return res, nil, nil +} + func (app *App) DeliverTx(ctx sdk.Context, req abci.RequestDeliverTx, tx sdk.Tx, checksum [32]byte) abci.ResponseDeliverTx { defer metrics.MeasureDeliverTxDuration(time.Now()) // ensure we carry the initial context from tracer here @@ -80,3 +103,12 @@ func (app *App) LoadLatest(ctx context.Context, req *abci.RequestLoadLatest) (*a app.mounter() return app.BaseApp.LoadLatest(ctx, req) } + +func (app *App) CheckNonce(ctx context.Context, req any, index int) (bool, error) { + sdkCtx := app.GetCheckCtx() + msg, ok := req.(*evmtypes.MsgEVMTransaction) + if !ok { + return false, fmt.Errorf("invalid request type: %T", req) + } + return app.EvmKeeper.CheckNonce(sdkCtx, msg, index) +} diff --git a/sei-cosmos/baseapp/abci.go b/sei-cosmos/baseapp/abci.go index 0486df9cfd..ea392edb82 100644 --- a/sei-cosmos/baseapp/abci.go +++ b/sei-cosmos/baseapp/abci.go @@ -208,7 +208,15 @@ func (app *BaseApp) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) (res abc // the ResponseCheckTx will contain relevant gas execution context. func (app *BaseApp) CheckTx(ctx context.Context, req *abci.RequestCheckTx) (*abci.ResponseCheckTxV2, error) { defer telemetry.MeasureSince(time.Now(), "abci", "check_tx") + tx, err := app.txDecoder(req.Tx) + if err != nil { + res := sdkerrors.ResponseCheckTx(err, 0, 0, app.trace) + return &abci.ResponseCheckTxV2{ResponseCheckTx: &res}, err + } + return app.CheckTxDecoded(tx, req) +} +func (app *BaseApp) CheckTxDecoded(tx sdk.Tx, req *abci.RequestCheckTx) (*abci.ResponseCheckTxV2, error) { var mode runTxMode switch { @@ -223,11 +231,6 @@ func (app *BaseApp) CheckTx(ctx context.Context, req *abci.RequestCheckTx) (*abc } sdkCtx := app.getContextForTx(mode, req.Tx) - tx, err := app.txDecoder(req.Tx) - if err != nil { - res := sdkerrors.ResponseCheckTx(err, 0, 0, app.trace) - return &abci.ResponseCheckTxV2{ResponseCheckTx: &res}, err - } gInfo, result, _, priority, pendingTxChecker, expireTxHandler, txCtx, err := app.runTx(sdkCtx, mode, tx, sha256.Sum256(req.Tx)) if err != nil { res := sdkerrors.ResponseCheckTx(err, gInfo.GasWanted, gInfo.GasUsed, app.trace) @@ -1258,3 +1261,15 @@ func (app *BaseApp) GetTxPriorityHint(_ context.Context, req *abci.RequestGetTxP Priority: priority, }, nil } + +func (app *BaseApp) CheckTxWrapped(ctx context.Context, req *abci.RequestCheckTx) (*abci.ResponseCheckTxV2, any, error) { + res, err := app.CheckTx(ctx, req) + if err != nil { + return res, nil, err + } + return res, nil, nil +} + +func (app *BaseApp) CheckNonce(ctx context.Context, req any, index int) (bool, error) { + return true, nil +} diff --git a/sei-tendermint/abci/client/grpc_client.go b/sei-tendermint/abci/client/grpc_client.go index 44f1910dcc..bb44dfcbed 100644 --- a/sei-tendermint/abci/client/grpc_client.go +++ b/sei-tendermint/abci/client/grpc_client.go @@ -190,3 +190,12 @@ func (cli *grpcClient) LoadLatest(ctx context.Context, params *types.RequestLoad func (cli *grpcClient) GetTxPriorityHint(ctx context.Context, req *types.RequestGetTxPriorityHint) (*types.ResponseGetTxPriorityHint, error) { return cli.client.GetTxPriorityHint(ctx, types.ToRequestGetTxPriorityHint(req).GetGetTxPriorityHint(), grpc.WaitForReady(true)) } + +func (cli *grpcClient) CheckNonce(ctx context.Context, req any, index int) (bool, error) { + return false, errors.New("not implemented") +} + +func (cli *grpcClient) CheckTxWrapped(ctx context.Context, req *types.RequestCheckTx) (*types.ResponseCheckTxV2, any, error) { + res, err := cli.CheckTx(ctx, req) + return res, nil, err +} diff --git a/sei-tendermint/abci/client/mocks/client.go b/sei-tendermint/abci/client/mocks/client.go index a19313957d..12f0052303 100644 --- a/sei-tendermint/abci/client/mocks/client.go +++ b/sei-tendermint/abci/client/mocks/client.go @@ -601,6 +601,16 @@ func (_m *Client) VerifyVoteExtension(_a0 context.Context, _a1 *types.RequestVer return r0, r1 } +func (_m *Client) CheckNonce(_a0 context.Context, _a1 any, _a2 int) (bool, error) { + ret := _m.Called(_a0, _a1, _a2) + return ret.Get(0).(bool), ret.Error(1) +} + +func (_m *Client) CheckTxWrapped(_a0 context.Context, _a1 *types.RequestCheckTx) (*types.ResponseCheckTxV2, any, error) { + ret := _m.Called(_a0, _a1) + return ret.Get(0).(*types.ResponseCheckTxV2), ret.Get(1).(any), ret.Error(2) +} + // Wait provides a mock function with no fields func (_m *Client) Wait() { _m.Called() diff --git a/sei-tendermint/abci/client/socket_client.go b/sei-tendermint/abci/client/socket_client.go index e2ede03857..126bbb9a1b 100644 --- a/sei-tendermint/abci/client/socket_client.go +++ b/sei-tendermint/abci/client/socket_client.go @@ -377,6 +377,15 @@ func (cli *socketClient) GetTxPriorityHint(ctx context.Context, req *types.Reque return res.GetGetTxPriorityHint(), nil } +func (cli *socketClient) CheckNonce(ctx context.Context, req any, index int) (bool, error) { + return false, errors.New("not implemented") +} + +func (cli *socketClient) CheckTxWrapped(ctx context.Context, req *types.RequestCheckTx) (*types.ResponseCheckTxV2, any, error) { + res, err := cli.CheckTx(ctx, req) + return res, nil, err +} + //---------------------------------------- func resMatchesReq(req *types.Request, res *types.Response) (ok bool) { diff --git a/sei-tendermint/abci/example/kvstore/kvstore.go b/sei-tendermint/abci/example/kvstore/kvstore.go index ac0d122b1c..3c5bbe6cd7 100644 --- a/sei-tendermint/abci/example/kvstore/kvstore.go +++ b/sei-tendermint/abci/example/kvstore/kvstore.go @@ -209,6 +209,10 @@ func (*Application) CheckTx(_ context.Context, req *types.RequestCheckTx) (*type return &types.ResponseCheckTxV2{ResponseCheckTx: &types.ResponseCheckTx{Code: code.CodeTypeOK, GasWanted: 1}}, nil } +func (*Application) CheckTxWrapped(_ context.Context, req *types.RequestCheckTx) (*types.ResponseCheckTxV2, any, error) { + return &types.ResponseCheckTxV2{ResponseCheckTx: &types.ResponseCheckTx{Code: code.CodeTypeOK, GasWanted: 1}}, nil, nil +} + func (app *Application) Commit(_ context.Context) (*types.ResponseCommit, error) { app.mu.Lock() defer app.mu.Unlock() diff --git a/sei-tendermint/abci/types/application.go b/sei-tendermint/abci/types/application.go index 326c109982..54516b3a41 100644 --- a/sei-tendermint/abci/types/application.go +++ b/sei-tendermint/abci/types/application.go @@ -12,7 +12,9 @@ type Application interface { Query(context.Context, *RequestQuery) (*ResponseQuery, error) // Query for state // Mempool Connection - CheckTx(context.Context, *RequestCheckTx) (*ResponseCheckTxV2, error) // Validate a tx for the mempool + CheckTx(context.Context, *RequestCheckTx) (*ResponseCheckTxV2, error) // Validate a tx for the mempool; deprecated + CheckTxWrapped(context.Context, *RequestCheckTx) (*ResponseCheckTxV2, any, error) // Validate a tx for the mempool + CheckNonce(context.Context, any, int) (bool, error) // Consensus Connection InitChain(context.Context, *RequestInitChain) (*ResponseInitChain, error) // Initialize blockchain w validators/other info from TendermintCore @@ -56,6 +58,14 @@ func (BaseApplication) CheckTx(_ context.Context, req *RequestCheckTx) (*Respons return &ResponseCheckTxV2{ResponseCheckTx: &ResponseCheckTx{Code: CodeTypeOK}}, nil } +func (BaseApplication) CheckTxWrapped(_ context.Context, req *RequestCheckTx) (*ResponseCheckTxV2, any, error) { + return &ResponseCheckTxV2{ResponseCheckTx: &ResponseCheckTx{Code: CodeTypeOK}}, nil, nil +} + +func (BaseApplication) CheckNonce(_ context.Context, req any, index int) (bool, error) { + return true, nil +} + func (BaseApplication) Commit(_ context.Context) (*ResponseCommit, error) { return &ResponseCommit{}, nil } diff --git a/sei-tendermint/abci/types/mocks/application.go b/sei-tendermint/abci/types/mocks/application.go index 808db0f446..66f27e9798 100644 --- a/sei-tendermint/abci/types/mocks/application.go +++ b/sei-tendermint/abci/types/mocks/application.go @@ -494,6 +494,16 @@ func (_m *Application) VerifyVoteExtension(_a0 context.Context, _a1 *types.Reque return r0, r1 } +func (_m *Application) CheckNonce(_a0 context.Context, _a1 any, _a2 int) (bool, error) { + ret := _m.Called(_a0, _a1, _a2) + return ret.Get(0).(bool), ret.Error(1) +} + +func (_m *Application) CheckTxWrapped(_a0 context.Context, _a1 *types.RequestCheckTx) (*types.ResponseCheckTxV2, any, error) { + ret := _m.Called(_a0, _a1) + return ret.Get(0).(*types.ResponseCheckTxV2), ret.Get(1).(any), ret.Error(2) +} + // NewApplication creates a new instance of Application. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func NewApplication(t interface { diff --git a/sei-tendermint/config/config.go b/sei-tendermint/config/config.go index ce80847392..e8969d834a 100644 --- a/sei-tendermint/config/config.go +++ b/sei-tendermint/config/config.go @@ -862,6 +862,10 @@ type MempoolConfig struct { // // See DropUtilisationThreshold and DropPriorityThreshold. DropPriorityReservoirSize int `mapstructure:"drop-priority-reservoir-size"` + + // Do a JIT check of nonce before proposing a block to make sure no transaction + // with invalid nonce is included in the block. + CheckNonceBeforePropose bool `mapstructure:"check-nonce-before-propose"` } // DefaultMempoolConfig returns a default configuration for the Tendermint mempool. @@ -888,6 +892,7 @@ func DefaultMempoolConfig() *MempoolConfig { DropPriorityThreshold: 0.1, DropUtilisationThreshold: 1.0, DropPriorityReservoirSize: 10_240, + CheckNonceBeforePropose: false, } } diff --git a/sei-tendermint/config/toml.go b/sei-tendermint/config/toml.go index a693662801..283f53b3f8 100644 --- a/sei-tendermint/config/toml.go +++ b/sei-tendermint/config/toml.go @@ -457,6 +457,8 @@ drop-utilisation-threshold = {{ .Mempool.DropUtilisationThreshold }} # See DropUtilisationThreshold and DropPriorityThreshold. drop-priority-reservoir-size = {{ .Mempool.DropPriorityReservoirSize }} +check-nonce-before-propose = {{ .Mempool.CheckNonceBeforePropose }} + ####################################################### ### State Sync Configuration Options ### ####################################################### diff --git a/sei-tendermint/internal/consensus/mempool_test.go b/sei-tendermint/internal/consensus/mempool_test.go index ca58b0561a..79919cea4d 100644 --- a/sei-tendermint/internal/consensus/mempool_test.go +++ b/sei-tendermint/internal/consensus/mempool_test.go @@ -236,7 +236,7 @@ func TestMempoolRmBadTx(t *testing.T) { // check for the tx for { - txs := assertMempool(t, cs.txNotifier).ReapMaxBytesMaxGas(int64(len(txBytes)), -1, -1) + txs := assertMempool(t, cs.txNotifier).ReapMaxBytesMaxGas(ctx, int64(len(txBytes)), -1, -1) if len(txs) == 0 { emptyMempoolCh <- struct{}{} return @@ -328,6 +328,15 @@ func (app *CounterApplication) CheckTx(_ context.Context, req *abci.RequestCheck return &abci.ResponseCheckTxV2{ResponseCheckTx: &abci.ResponseCheckTx{Code: code.CodeTypeOK}}, nil } +func (app *CounterApplication) CheckTxWrapped(ctx context.Context, req *abci.RequestCheckTx) (*abci.ResponseCheckTxV2, any, error) { + res, err := app.CheckTx(ctx, req) + return res, nil, err +} + +func (app *CounterApplication) CheckNonce(ctx context.Context, req any, index int) (bool, error) { + return true, nil +} + func txAsUint64(tx []byte) uint64 { tx8 := make([]byte, 8) copy(tx8[len(tx8)-len(tx):], tx) diff --git a/sei-tendermint/internal/consensus/replay_stubs.go b/sei-tendermint/internal/consensus/replay_stubs.go index cac3065f21..0d251f9ce7 100644 --- a/sei-tendermint/internal/consensus/replay_stubs.go +++ b/sei-tendermint/internal/consensus/replay_stubs.go @@ -37,9 +37,11 @@ func (emptyMempool) Size() int { return 0 } func (emptyMempool) CheckTx(context.Context, types.Tx, func(*abci.ResponseCheckTx), mempool.TxInfo) error { return nil } -func (emptyMempool) RemoveTxByKey(txKey types.TxKey) error { return nil } -func (emptyMempool) ReapMaxBytesMaxGas(_, _, _ int64) types.Txs { return types.Txs{} } -func (emptyMempool) ReapMaxTxs(n int) types.Txs { return types.Txs{} } +func (emptyMempool) RemoveTxByKey(txKey types.TxKey) error { return nil } +func (emptyMempool) ReapMaxBytesMaxGas(_ context.Context, _, _, _ int64) types.Txs { + return types.Txs{} +} +func (emptyMempool) ReapMaxTxs(n int) types.Txs { return types.Txs{} } func (emptyMempool) Update( _ context.Context, _ int64, diff --git a/sei-tendermint/internal/mempool/mempool.go b/sei-tendermint/internal/mempool/mempool.go index 2cf67fed57..57f1ba0235 100644 --- a/sei-tendermint/internal/mempool/mempool.go +++ b/sei-tendermint/internal/mempool/mempool.go @@ -363,7 +363,7 @@ func (txmp *TxMempool) CheckTx( txmp.duplicateTxsCache.Increment(txHash) } - res, err := txmp.proxyAppConn.CheckTx(ctx, &abci.RequestCheckTx{Tx: tx}) + res, evmMsg, err := txmp.proxyAppConn.CheckTxWrapped(ctx, &abci.RequestCheckTx{Tx: tx}) txmp.totalCheckTxCount.Add(1) if err != nil { txmp.metrics.NumberOfFailedCheckTxs.Add(1) @@ -402,6 +402,7 @@ func (txmp *TxMempool) CheckTx( isEVM: res.IsEVM, removeHandler: removeHandler, estimatedGas: res.GasEstimated, + evmMessage: evmMsg, } if err == nil { @@ -533,7 +534,7 @@ func (txmp *TxMempool) Flush() { // NOTE: // - Transactions returned are not removed from the mempool transaction // store or indexes. -func (txmp *TxMempool) ReapMaxBytesMaxGas(maxBytes, maxGasWanted, maxGasEstimated int64) types.Txs { +func (txmp *TxMempool) ReapMaxBytesMaxGas(ctx context.Context, maxBytes, maxGasWanted, maxGasEstimated int64) types.Txs { txmp.mtx.Lock() defer txmp.mtx.Unlock() @@ -549,6 +550,7 @@ func (txmp *TxMempool) ReapMaxBytesMaxGas(maxBytes, maxGasWanted, maxGasEstimate // do not reap anything if threshold is not met return txs } + checkNonceCount := 0 txmp.priorityIndex.ForEachTx(func(wtx *WrappedTx) bool { size := types.ComputeProtoSizeForTxs([]types.Tx{wtx.tx}) @@ -557,6 +559,18 @@ func (txmp *TxMempool) ReapMaxBytesMaxGas(maxBytes, maxGasWanted, maxGasEstimate return false } + if wtx.evmMessage != nil && txmp.config.CheckNonceBeforePropose { + nonceValid, err := txmp.proxyAppConn.CheckNonce(ctx, wtx.evmMessage, checkNonceCount) + checkNonceCount++ + if err != nil { + txmp.logger.Error("error checking nonce", "error", err) + return false + } + if !nonceValid { + return false + } + } + // if the tx doesn't have a gas estimate, fallback to gas wanted var txGasEstimate int64 if wtx.estimatedGas >= MinGasEVMTx && wtx.estimatedGas <= wtx.gasWanted { diff --git a/sei-tendermint/internal/mempool/mempool_test.go b/sei-tendermint/internal/mempool/mempool_test.go index df8615ead6..91c0b28329 100644 --- a/sei-tendermint/internal/mempool/mempool_test.go +++ b/sei-tendermint/internal/mempool/mempool_test.go @@ -166,6 +166,15 @@ func (app *application) CheckTx(_ context.Context, req *abci.RequestCheckTx) (*a }}, nil } +func (app *application) CheckTxWrapped(ctx context.Context, req *abci.RequestCheckTx) (*abci.ResponseCheckTxV2, any, error) { + res, err := app.CheckTx(ctx, req) + return res, nil, err +} + +func (app *application) CheckNonce(ctx context.Context, req any, index int) (bool, error) { + return true, nil +} + func (app *application) GetTxPriorityHint(context.Context, *abci.RequestGetTxPriorityHint) (*abci.ResponseGetTxPriorityHint, error) { return &abci.ResponseGetTxPriorityHint{ // Return non-zero priority to allow testing the eviction logic effectively. @@ -410,7 +419,7 @@ func TestTxMempool_ReapMaxBytesMaxGas(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - reapedTxs := txmp.ReapMaxBytesMaxGas(-1, 50, -1) + reapedTxs := txmp.ReapMaxBytesMaxGas(ctx, -1, 50, -1) ensurePrioritized(reapedTxs) require.Equal(t, len(tTxs), txmp.Size()) require.Equal(t, int64(5690), txmp.SizeBytes()) @@ -421,7 +430,7 @@ func TestTxMempool_ReapMaxBytesMaxGas(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - reapedTxs := txmp.ReapMaxBytesMaxGas(1000, -1, -1) + reapedTxs := txmp.ReapMaxBytesMaxGas(ctx, 1000, -1, -1) ensurePrioritized(reapedTxs) require.Equal(t, len(tTxs), txmp.Size()) require.Equal(t, int64(5690), txmp.SizeBytes()) @@ -433,7 +442,7 @@ func TestTxMempool_ReapMaxBytesMaxGas(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - reapedTxs := txmp.ReapMaxBytesMaxGas(1500, 30, -1) + reapedTxs := txmp.ReapMaxBytesMaxGas(ctx, 1500, 30, -1) ensurePrioritized(reapedTxs) require.Equal(t, len(tTxs), txmp.Size()) require.Equal(t, int64(5690), txmp.SizeBytes()) @@ -444,7 +453,7 @@ func TestTxMempool_ReapMaxBytesMaxGas(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - reapedTxs := txmp.ReapMaxBytesMaxGas(-1, 2, -1) + reapedTxs := txmp.ReapMaxBytesMaxGas(ctx, -1, 2, -1) ensurePrioritized(reapedTxs) require.Equal(t, len(tTxs), txmp.Size()) require.Len(t, reapedTxs, 2) @@ -454,7 +463,7 @@ func TestTxMempool_ReapMaxBytesMaxGas(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - reapedTxs := txmp.ReapMaxBytesMaxGas(-1, -1, 50) + reapedTxs := txmp.ReapMaxBytesMaxGas(ctx, -1, -1, 50) ensurePrioritized(reapedTxs) require.Equal(t, len(tTxs), txmp.Size()) require.Len(t, reapedTxs, 50) @@ -502,7 +511,7 @@ func TestTxMempool_ReapMaxBytesMaxGas_FallbackToGasWanted(t *testing.T) { wg.Add(1) go func() { defer wg.Done() - reapedTxs := txmp.ReapMaxBytesMaxGas(-1, -1, 50) + reapedTxs := txmp.ReapMaxBytesMaxGas(ctx, -1, -1, 50) ensurePrioritized(reapedTxs) require.Equal(t, len(tTxs), txmp.Size()) require.Len(t, reapedTxs, 50) @@ -606,7 +615,7 @@ func TestTxMempool_ReapMaxBytesMaxGas_MinGasEVMTxThreshold(t *testing.T) { // With MinGasEVMTx=21000, estimatedGas (10000) is ignored and we fallback to gasWanted (50000). // Setting maxGasEstimated below gasWanted should therefore result in 0 reaped txs. - reaped := txmp.ReapMaxBytesMaxGas(-1, -1, 40000) + reaped := txmp.ReapMaxBytesMaxGas(ctx, -1, -1, 40000) require.Len(t, reaped, 0) // Note: If MinGasEVMTx is changed to 0, the same scenario would use estimatedGas (10000) @@ -669,7 +678,7 @@ func TestTxMempool_Reap_SkipGasUnfitAndCollectMinTxs(t *testing.T) { } // Reap with a maxGasEstimated that makes the first tx unfit but allows many small txs - reaped := txmp.ReapMaxBytesMaxGas(-1, -1, 50) + reaped := txmp.ReapMaxBytesMaxGas(ctx, -1, -1, 50) require.Len(t, reaped, MinTxsToPeek) // Ensure all reaped small txs are under gas constraint @@ -710,7 +719,7 @@ func TestTxMempool_Reap_SkipGasUnfitStopsAtMinEvenWithCapacity(t *testing.T) { } // Make the gas limit very small so the first (big) tx is unfit and we only collect MinTxsPerBlock - reaped := txmp.ReapMaxBytesMaxGas(-1, -1, 10) + reaped := txmp.ReapMaxBytesMaxGas(ctx, -1, -1, 10) require.Len(t, reaped, MinTxsToPeek) } @@ -1264,6 +1273,19 @@ func (app *nonceCheckApplication) CheckTx(_ context.Context, req *abci.RequestCh return res, nil } +func (app *nonceCheckApplication) CheckTxWrapped(ctx context.Context, req *abci.RequestCheckTx) (*abci.ResponseCheckTxV2, any, error) { + res, err := app.CheckTx(ctx, req) + return res, uint64(req.Tx[0]), err +} + +func (app *nonceCheckApplication) CheckNonce(ctx context.Context, req any, index int) (bool, error) { + nonce := req.(uint64) + if nonce == 0 { + return true, nil + } + return false, nil +} + func TestTxMempool_PendingNonce(t *testing.T) { // Setup ctx := t.Context() @@ -1281,6 +1303,7 @@ func TestTxMempool_PendingNonce(t *testing.T) { txmp := setup(t, client, 500) txmp.config.TTLNumBlocks = 10 txmp.height = 1 + txmp.config.CheckNonceBeforePropose = true tx1 := append([]byte{0}, []byte("sender")...) require.NoError(t, txmp.CheckTx(ctx, tx1, nil, TxInfo{SenderID: 0, SenderNodeID: "test"})) @@ -1306,7 +1329,12 @@ func TestTxMempool_PendingNonce(t *testing.T) { require.Len(t, txmp.priorityIndex.txs, 1) require.Len(t, txmp.priorityIndex.evmQueue["sender"], 1) // two transactions from the sender - txs := txmp.ReapMaxBytesMaxGas(1000, 1000, 1000) + txs := txmp.ReapMaxBytesMaxGas(ctx, 1000, 1000, 1000) + require.Len(t, txs, 0) // tx2 has invalid nonce so won't be reaped with CheckNonceBeforePropose turned on + + // now with CheckNonceBeforePropose turned off, tx2 should be reaped (which isn't ideal) + txmp.config.CheckNonceBeforePropose = false + txs = txmp.ReapMaxBytesMaxGas(ctx, 1000, 1000, 1000) require.Len(t, txs, 1) require.Equal(t, tx2, []byte(txs[0])) // tx2 with out of order nonce is included! } diff --git a/sei-tendermint/internal/mempool/mocks/mempool.go b/sei-tendermint/internal/mempool/mocks/mempool.go index b656d106e5..c0b072775f 100644 --- a/sei-tendermint/internal/mempool/mocks/mempool.go +++ b/sei-tendermint/internal/mempool/mocks/mempool.go @@ -109,8 +109,8 @@ func (_m *Mempool) Lock() { } // ReapMaxBytesMaxGas provides a mock function with given fields: maxBytes, maxGas, maxGasEstimated -func (_m *Mempool) ReapMaxBytesMaxGas(maxBytes int64, maxGas int64, maxGasEstimated int64) types.Txs { - ret := _m.Called(maxBytes, maxGas, maxGasEstimated) +func (_m *Mempool) ReapMaxBytesMaxGas(ctx context.Context, maxBytes int64, maxGas int64, maxGasEstimated int64) types.Txs { + ret := _m.Called(ctx, maxBytes, maxGas, maxGasEstimated) if len(ret) == 0 { panic("no return value specified for ReapMaxBytesMaxGas") diff --git a/sei-tendermint/internal/mempool/tx.go b/sei-tendermint/internal/mempool/tx.go index c437184f73..150eb7abd7 100644 --- a/sei-tendermint/internal/mempool/tx.go +++ b/sei-tendermint/internal/mempool/tx.go @@ -77,6 +77,7 @@ type WrappedTx struct { evmAddress string evmNonce uint64 isEVM bool + evmMessage any // *evmtypes.MsgEVMTransaction to avoid import cycle } // IsBefore returns true if the WrappedTx is before the given WrappedTx diff --git a/sei-tendermint/internal/mempool/types.go b/sei-tendermint/internal/mempool/types.go index 7115461223..da4127a9c1 100644 --- a/sei-tendermint/internal/mempool/types.go +++ b/sei-tendermint/internal/mempool/types.go @@ -52,7 +52,7 @@ type Mempool interface { // // If all 3 maxes are negative, there is no cap on the size of all returned // transactions (~ all available transactions). - ReapMaxBytesMaxGas(maxBytes, maxGas, maxGasEstimated int64) types.Txs + ReapMaxBytesMaxGas(ctx context.Context, maxBytes, maxGas, maxGasEstimated int64) types.Txs // ReapMaxTxs reaps up to max transactions from the mempool. If max is // negative, there is no cap on the size of all returned transactions diff --git a/sei-tendermint/internal/proxy/client.go b/sei-tendermint/internal/proxy/client.go index 369140842c..2fa24d8a50 100644 --- a/sei-tendermint/internal/proxy/client.go +++ b/sei-tendermint/internal/proxy/client.go @@ -214,6 +214,16 @@ func (app *proxyClient) ApplySnapshotChunk(ctx context.Context, req *types.Reque return app.client.ApplySnapshotChunk(ctx, req) } +func (app *proxyClient) CheckNonce(ctx context.Context, req any, index int) (bool, error) { + defer addTimeSample(app.metrics.MethodTiming.With("method", "check_nonce", "type", "sync"))() + return app.client.CheckNonce(ctx, req, index) +} + +func (app *proxyClient) CheckTxWrapped(ctx context.Context, req *types.RequestCheckTx) (*types.ResponseCheckTxV2, any, error) { + defer addTimeSample(app.metrics.MethodTiming.With("method", "check_tx_wrapped", "type", "sync"))() + return app.client.CheckTxWrapped(ctx, req) +} + // addTimeSample returns a function that, when called, adds an observation to m. // The observation added to m is the number of seconds ellapsed since addTimeSample // was initially called. addTimeSample is meant to be called in a defer to calculate diff --git a/sei-tendermint/internal/state/execution.go b/sei-tendermint/internal/state/execution.go index 6b616c697f..86aa8b69fd 100644 --- a/sei-tendermint/internal/state/execution.go +++ b/sei-tendermint/internal/state/execution.go @@ -110,7 +110,7 @@ func (blockExec *BlockExecutor) CreateProposalBlock( // Fetch a limited amount of valid txs maxDataBytes := types.MaxDataBytes(maxBytes, evSize, state.Validators.Size()) - txs := blockExec.mempool.ReapMaxBytesMaxGas(maxDataBytes, maxGasWanted, maxGas) + txs := blockExec.mempool.ReapMaxBytesMaxGas(ctx, maxDataBytes, maxGasWanted, maxGas) block = state.MakeBlock(height, txs, lastCommit, evidence, proposerAddr) rpp, err := blockExec.appClient.PrepareProposal( ctx, diff --git a/sei-tendermint/internal/state/execution_test.go b/sei-tendermint/internal/state/execution_test.go index cf2f404047..4de9157051 100644 --- a/sei-tendermint/internal/state/execution_test.go +++ b/sei-tendermint/internal/state/execution_test.go @@ -967,7 +967,7 @@ func TestCreateProposalBlockPanicRecovery(t *testing.T) { // Create mock mempool mp := &mpmocks.Mempool{} - mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything, mock.Anything).Return(types.Txs{}) + mp.On("ReapMaxBytesMaxGas", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(types.Txs{}) blockExec := sm.NewBlockExecutor( stateStore, diff --git a/x/evm/keeper/keeper.go b/x/evm/keeper/keeper.go index 7ecf2bf2a8..d825b87d99 100644 --- a/x/evm/keeper/keeper.go +++ b/x/evm/keeper/keeper.go @@ -93,6 +93,8 @@ type Keeper struct { customPrecompiles map[common.Address]putils.VersionedPrecompiles latestCustomPrecompiles map[common.Address]vm.PrecompiledContract latestUpgrade string + + tempNonces map[common.Address]uint64 } type AddressNoncePair struct { @@ -153,6 +155,7 @@ func NewKeeper( cachedFeeCollectorAddressMtx: &sync.RWMutex{}, keyToNonce: make(map[tmtypes.TxKey]*AddressNoncePair), receiptStore: receiptStateStore, + tempNonces: map[common.Address]uint64{}, } return k } @@ -625,6 +628,29 @@ func (k *Keeper) getReplayBlockCtx(ctx sdk.Context) (*vm.BlockContext, error) { }, nil } +func (k *Keeper) CheckNonce(ctx sdk.Context, msg *types.MsgEVMTransaction, index int) (bool, error) { + if index == 0 { + k.tempNonces = map[common.Address]uint64{} + } + if msg.Derived == nil { + return false, fmt.Errorf("derived is nil") + } + from := msg.Derived.SenderEVMAddr + ethtx, _ := msg.AsTransaction() + if nonce, ok := k.tempNonces[from]; ok { + if nonce == ethtx.Nonce() { + k.tempNonces[from]++ + return true, nil + } + return false, nil + } + if k.GetNonce(ctx, from) == ethtx.Nonce() { + k.tempNonces[from]++ + return true, nil + } + return false, nil +} + func uint64Cmp(a, b uint64) int { if a < b { return -1 diff --git a/x/evm/keeper/keeper_test.go b/x/evm/keeper/keeper_test.go index 10d9e557f9..b0158b08bc 100644 --- a/x/evm/keeper/keeper_test.go +++ b/x/evm/keeper/keeper_test.go @@ -11,17 +11,21 @@ import ( "strings" "sync" "testing" + "time" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/ethereum/go-ethereum/common" ethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/params" "github.com/sei-protocol/sei-chain/app" "github.com/sei-protocol/sei-chain/testutil/keeper" testkeeper "github.com/sei-protocol/sei-chain/testutil/keeper" "github.com/sei-protocol/sei-chain/utils" "github.com/sei-protocol/sei-chain/x/evm/config" + "github.com/sei-protocol/sei-chain/x/evm/derived" evmkeeper "github.com/sei-protocol/sei-chain/x/evm/keeper" "github.com/sei-protocol/sei-chain/x/evm/types" "github.com/sei-protocol/sei-chain/x/evm/types/ethtx" @@ -387,3 +391,187 @@ func TestGetBaseFeeBeforeV620(t *testing.T) { baseFeeOther := keeper.GetBaseFee(ctxOtherChain) require.NotNil(t, baseFeeOther, "Base fee should not be nil for non-pacific-1 chains") } + +func TestKeeper_CheckNonce(t *testing.T) { + k, ctx := keeper.MockEVMKeeper() + chainID := k.ChainID(ctx) + chainCfg := types.DefaultChainConfig() + ethCfg := chainCfg.EthereumConfig(chainID) + blockNum := big.NewInt(ctx.BlockHeight()) + privKey := testkeeper.MockPrivateKey() + testPrivHex := hex.EncodeToString(privKey.Bytes()) + key, _ := crypto.HexToECDSA(testPrivHex) + fromAddr := crypto.PubkeyToAddress(key.PublicKey) + + tests := []struct { + name string + setup func(ctx sdk.Context, k *evmkeeper.Keeper) + msg *types.MsgEVMTransaction + index int + expectedResult bool + expectedError string + }{ + { + name: "nil derived message should return error", + msg: &types.MsgEVMTransaction{ + Derived: nil, + }, + index: 0, + expectedResult: false, + expectedError: "derived is nil", + }, + { + name: "first transaction with correct nonce should pass", + setup: func(ctx sdk.Context, k *evmkeeper.Keeper) { + // Set nonce to 0 for the address + k.SetNonce(ctx, fromAddr, 0) + }, + msg: createMockEVMTransaction(t, privKey, ethCfg, blockNum, 0), + index: 0, + expectedResult: true, + expectedError: "", + }, + { + name: "first transaction with incorrect nonce should fail", + setup: func(ctx sdk.Context, k *evmkeeper.Keeper) { + // Set nonce to 5 for the address + k.SetNonce(ctx, fromAddr, 5) + }, + msg: createMockEVMTransaction(t, privKey, ethCfg, blockNum, 0), + index: 0, + expectedResult: false, + expectedError: "", + }, + { + name: "second transaction with correct nonce should pass", + setup: func(ctx sdk.Context, k *evmkeeper.Keeper) { + // Set nonce to 0 for the address + k.SetNonce(ctx, fromAddr, 1) + }, + msg: createMockEVMTransaction(t, privKey, ethCfg, blockNum, 1), + index: 1, + expectedResult: true, + expectedError: "", + }, + { + name: "second transaction with incorrect nonce should fail", + setup: func(ctx sdk.Context, k *evmkeeper.Keeper) { + // Set nonce to 0 for the address + k.SetNonce(ctx, fromAddr, 0) + }, + msg: createMockEVMTransaction(t, privKey, ethCfg, blockNum, 2), + index: 1, + expectedResult: false, + expectedError: "", + }, + { + name: "transaction with nonce already in tempNonces should pass", + setup: func(ctx sdk.Context, k *evmkeeper.Keeper) { + // Set nonce to 0 for the address + k.SetNonce(ctx, fromAddr, 0) + }, + msg: createMockEVMTransaction(t, privKey, ethCfg, blockNum, 0), + index: 1, + expectedResult: true, + expectedError: "", + }, + { + name: "transaction with nonce already in tempNonces but different value should fail", + setup: func(ctx sdk.Context, k *evmkeeper.Keeper) { + // Set nonce to 0 for the address + k.SetNonce(ctx, fromAddr, 0) + }, + msg: createMockEVMTransaction(t, privKey, ethCfg, blockNum, 1), + index: 1, + expectedResult: false, + expectedError: "", + }, + { + name: "multiple transactions in sequence should work", + setup: func(ctx sdk.Context, k *evmkeeper.Keeper) { + // Set nonce to 0 for the address + k.SetNonce(ctx, fromAddr, 0) + }, + msg: createMockEVMTransaction(t, privKey, ethCfg, blockNum, 0), + index: 0, + expectedResult: true, + expectedError: "", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + k, ctx := keeper.MockEVMKeeper() + if test.setup != nil { + test.setup(ctx, k) + } + + result, err := k.CheckNonce(ctx, test.msg, test.index) + + if test.expectedError != "" { + require.Error(t, err) + require.Contains(t, err.Error(), test.expectedError) + } else { + require.NoError(t, err) + } + + require.Equal(t, test.expectedResult, result) + }) + } +} + +func TestKeeper_CheckNonce_SequentialTransactions(t *testing.T) { + k, ctx := keeper.MockEVMKeeper() + chainID := k.ChainID(ctx) + chainCfg := types.DefaultChainConfig() + ethCfg := chainCfg.EthereumConfig(chainID) + blockNum := big.NewInt(ctx.BlockHeight()) + privKey := testkeeper.MockPrivateKey() + testPrivHex := hex.EncodeToString(privKey.Bytes()) + key, _ := crypto.HexToECDSA(testPrivHex) + fromAddr := crypto.PubkeyToAddress(key.PublicKey) + + // Set initial nonce + k.SetNonce(ctx, fromAddr, 0) + + // Test sequential transactions + for i := 0; i < 5; i++ { + msg := createMockEVMTransaction(t, privKey, ethCfg, blockNum, uint64(i)) + result, err := k.CheckNonce(ctx, msg, i) + require.NoError(t, err) + require.True(t, result, "Transaction %d should pass", i) + } +} + +func createMockEVMTransaction(t *testing.T, privKey cryptotypes.PrivKey, ethCfg *params.ChainConfig, blockNum *big.Int, nonce uint64) *types.MsgEVMTransaction { + // Convert cryptotypes.PrivKey to ECDSA key + testPrivHex := hex.EncodeToString(privKey.Bytes()) + key, err := crypto.HexToECDSA(testPrivHex) + require.NoError(t, err) + + to := new(common.Address) + txData := ethtypes.DynamicFeeTx{ + Nonce: nonce, + GasFeeCap: big.NewInt(10000000000000), + Gas: 1000, + To: to, + Value: big.NewInt(1000000000000000), + Data: []byte("test data"), + ChainID: ethCfg.ChainID, + } + + signer := ethtypes.MakeSigner(ethCfg, blockNum, uint64(time.Now().Unix())) + tx, err := ethtypes.SignTx(ethtypes.NewTx(&txData), signer, key) + require.NoError(t, err) + + typedTx, err := ethtx.NewDynamicFeeTx(tx) + require.NoError(t, err) + + msg, err := types.NewMsgEVMTransaction(typedTx) + require.NoError(t, err) + msg.Derived = &derived.Derived{ + SenderEVMAddr: crypto.PubkeyToAddress(key.PublicKey), + } + + return msg +}