-
Notifications
You must be signed in to change notification settings - Fork 96
fix(mempool): checkTxHandler to handle invalid sequence tx #591
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
base: main
Are you sure you want to change the base?
Conversation
mempool/check_tx.go
Outdated
@@ -18,7 +18,7 @@ func NewCheckTxHandler(mempool *ExperimentalEVMMempool) types.CheckTxHandler { | |||
gInfo, result, anteEvents, err := runTx(request.Tx, nil) | |||
if err != nil { | |||
// detect if there is a nonce gap error (only returned for EVM transactions) | |||
if errors.Is(err, ErrNonceGap) { | |||
if errors.Is(err, ErrNonceGap) || errors.Is(err, sdkerrors.ErrInvalidSequence) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add a test that confirms that transactions that have already been included in a block will not be included in the mempool?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I'll add test cases tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test case added! (839d35c)
Description
Fix
CheckTxHandler
to allow tx with "invalid sequence" error.The purpose of this pr is aligning appside mempool with geth mempool.
Because current anteHandler rejects valid tx whose nonce is lower than tx in mempool,
CheckTxHandler
should handle this error and insert that tx into mempool.The reason why does not modify anteHandler but
CheckTxHandler
is, if appside mempool is not enabled, validation of anteHandler is correct. So, we should modifyCheckTxHandler
that is used for appside mempool.Closes: #590
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
main
branch