fix: bump cometbft and TxReplacementDecorator into ante handler to unblock stuck mempool sequences#661
fix: bump cometbft and TxReplacementDecorator into ante handler to unblock stuck mempool sequences#661hoank101 wants to merge 15 commits into
Conversation
* - fix ibc handling in wasm (#77) - remove fork code to use upstream - remove unused code * Problem: cometbft not up to date * add v14_2 * bump version * bump version * bump version * fix legacy query staking (#79) * fix legacy query staking * bet * fix legacy params for validator delegation fallback * Bet * bet --------- Co-authored-by: Khanh Hoa <hoa@notional.ventures> Co-authored-by: Tuan Tran <tropicaldog17@gmail.com> * consistent alias --------- Co-authored-by: StrathCole <7449529+StrathCole@users.noreply.github.com> Co-authored-by: Kien Trinh <51135161+kien6034@users.noreply.github.com> Co-authored-by: Tuan Tran <tropicaldog17@gmail.com>
fix legacy staking query timeout
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| for k := range info.newTxBytesSet { | ||
| newSet[k] = struct{}{} | ||
| } |
| for k := range info.newTxBytesSet { | ||
| if k != key { | ||
| newSet[k] = struct{}{} | ||
| } | ||
| } |
| for k := range info.newTxBytesSet { | ||
| newSet[k] = struct{}{} | ||
| } |
| for k := range info.newTxBytesSet { | ||
| if k != key { | ||
| newSet[k] = struct{}{} | ||
| } | ||
| } |
|
Hi @StrathCole @fragwuerdig can you guys review this PR? |
|
Wouldn't it be rather necessary to fix the root cause (a tx that doesn't make it into a block should not pass checkTx)? |
The fee.go changes address the most likely root cause (burn tax calculation inconsistency between CheckTx and RecheckTx). However, fixing it completely is not possible within Terra alone - CometBFT's CheckTx is by design a soft filter, and any chain state change (governance tax/gas-price update) between submission and inclusion creates a new discrepancy. The TxReplacementDecorator is defense-in-depth: it provides a recovery path when any future discrepancy causes the same deadlock pattern, which is especially critical for validator operations where getting stuck means being jailed indefinitely. |
|
@fragwuerdig can you review this pr? |
There was a problem hiding this comment.
tracker.Set is called in handleNewTx, but the only cleanups are recheck-success (RemoveTxBytes) and new-tx ante failure. When the replacement is reaped directly into the next block, it is never rechecked, so its entry is never removed. Clear is defined but never called, and there is no PostHandler/Commit sweep — so the common success path leaks.
Keyed by sender bech32, this is an unbounded map growth vector across distinct senders, plantable by any account (queue a few txs, submit one replacement at the committed sequence, let it commit, repeat). It's fee-metered and memory-only — not a correctness/consensus bug — but a slow memory-DoS on long-running validators.
Suggested fix: clear the sender's entry once its committed sequence advances past FromSequence (PostHandler, or a height-based sweep in BeginBlock/PreBlocker), drop the entry on recheck failure too (CometBFT evicts the tx anyway), and add a bounded max-entries/TTL backstop.
| } | ||
|
|
||
| func (mp *FifoMempool) Insert(_ context.Context, tx sdk.Tx) error { | ||
| mp.mtx.Lock() |
There was a problem hiding this comment.
Insert/Remove moved from RLock to Lock and now call getTxBytes (a full proto marshal) while holding the exclusive lock, on a path hit for every CheckTx/RecheckTx. Under load this serializes admission behind a marshal-at-a-time critical section. Suggest marshaling before taking the lock (and/or threading the bytes CheckTx already has).
6 participants
Fix #648
Summary of changes
Problem
A validator operator wallet became permanently stuck and the validator was jailed with no way to unjail, because a CheckTx-accepted transaction at the committed sequence never made it into a block:
identical tx is rejected by the cache (code 19).
The result is a deadlock: every later tx from the wallet is stranded behind a sequence-432 tx that can neither be included nor replaced. For a validator this is fatal — the MsgUnjail / MsgEditValidator txs needed
to recover are themselves queued behind the stuck sequence.
Report of required housekeeping
(FOR ADMIN) Before merging