fix: ensure app and comet mempool parity - #689
Merged
Merged
Conversation
8 tasks
Collaborator
Author
|
I was able to patch my node and inject Moonshot's unjail transaction directly into my node. My node did not AppHash and Moonshot validator is now unjailed. |
…mm post handler only when msgs execute Fixes #648: a MsgEditValidator whose gas limit covered the ante handler but not the dyncomm post handler passed CheckTx, ran out of gas in PrepareProposal and was dropped from the app-side mempool only. CometBFT kept it forever and every later tx of the sender stayed blocked behind it. - dyncomm post handler: run only in FinalizeBlock and simulate, the modes that execute msgs. CheckTx, PrepareProposal and ProcessProposal now charge the same gas, so a tx that enters the mempool can be proposed; with too little gas it fails in FinalizeBlock and consumes its sequence like any other out-of-gas tx. Running in simulate (previously skipped) makes --gas auto include the ~7k gas of SetTargetCommissionRate. - Mempool parity CheckTx handler: on ReCheckTx, evict txs from the CometBFT mempool that are no longer in the app-side mempool, and remove txs that fail CheckTx after BaseApp inserted them. CheckTx errors honour --trace. CONSENSUS BREAKING: ProcessProposal no longer charges the dyncomm post handler gas, so nodes with and without this change disagree on proposals that contain such txs. Requires a coordinated upgrade. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
fragwuerdig
force-pushed
the
fix/mp-parity
branch
from
October 1, 2026 20:19
ae1a569 to
032453d
Compare
Empty handler to coordinate the chain-wide binary switch for the dyncomm post handler change, which alters what ProcessProposal accepts. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
StrathCole
approved these changes
Oct 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #648: a tx could pass
CheckTx, get dropped by PrepareProposal, and then stay in the CometBFT mempool forever, which blocked its sender.CheckTx/ReCheckTx. It already ran inPrepareProposal/ProcessProposal, so a tx with only enough gas for the ante handler got into the mempool and could never be included. NowCheckTxrejects it up front.CheckTxhandler keeps the CometBFT mempool in line with the app-side mempool:ReCheckTxevict txs that are no longer in the app-side mempool (e.g. dropped byPrepareProposal).runTx()error, it removes the tx from the app-side mempool too.BaseAppinserts the tx before the post handler runs, so a post-handler failure would otherwise leave it behind.FifoMempool.Contains(). It always returns true when the mempool is disabled.Tests
Containsand the parity handler (eviction, removal after a failed ante/post handler, other senders' txs untouched).x/dyncomm/ante:Consensus
Not consensus breaking. The changes only affect
CheckTx/ReCheckTxand local mempool state.PrepareProposal,ProcessProposalandFinalizeBlockbehave identically, so patched and unpatched validators can run side by side. Unpatched nodes can still hold stuck txs (#648) until they upgrade.