Skip to content

fixes to mempool handling#3281

Merged
pompon0 merged 3 commits intomainfrom
gprusak-fix
Apr 23, 2026
Merged

fixes to mempool handling#3281
pompon0 merged 3 commits intomainfrom
gprusak-fix

Conversation

@pompon0
Copy link
Copy Markdown
Contributor

@pompon0 pompon0 commented Apr 21, 2026

  • made handlePeerUpdates in mempool reactor run even if Broadcast = false, so that CheckTx failures are accounted properly.
  • make autobahn producer wait on a AtomicRecv instead of TxsAvailable channel so that block production is not blocked by execution.

@pompon0 pompon0 requested review from sei-will and wen-coding April 21, 2026 12:24
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 21, 2026

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedApr 23, 2026, 8:49 AM

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 76.40449% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.37%. Comparing base (63f25b1) to head (64c226c).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
sei-tendermint/internal/mempool/tx.go 72.90% 41 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3281      +/-   ##
==========================================
+ Coverage   58.31%   59.37%   +1.06%     
==========================================
  Files        2085     2072      -13     
  Lines      209061   170097   -38964     
==========================================
- Hits       121913   100996   -20917     
+ Misses      78355    60333   -18022     
+ Partials     8793     8768      -25     
Flag Coverage Δ
sei-chain-pr 74.62% <76.40%> (?)
sei-db 69.36% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
sei-tendermint/internal/autobahn/producer/state.go 79.16% <100.00%> (+0.29%) ⬆️
sei-tendermint/internal/mempool/mempool.go 72.82% <100.00%> (-0.11%) ⬇️
sei-tendermint/internal/mempool/reactor/reactor.go 79.62% <100.00%> (ø)
sei-tendermint/internal/mempool/tx.go 80.00% <72.90%> (-3.34%) ⬇️

... and 1795 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread sei-tendermint/internal/mempool/reactor/reactor.go
Comment thread sei-tendermint/internal/autobahn/producer/state.go
Comment thread sei-tendermint/internal/mempool/reactor/reactor.go Outdated
Comment thread sei-tendermint/internal/mempool/reactor/reactor.go
if errors.Is(err, mempool.ErrTxInCache) {
// If the tx is in the cache, then we've been gossiped a tx
// that we've already got. Gossip should be smarter, but it's
// not a problem.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: do we need to add any counter at least? Just in case some validator goes Haywire and send us the same transaction tons of times and waste all bandwidth for nothing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are more serious holes in the tendermint p2p protocol than that. I don't thing we should spend much time on fixing them. If you feel the need to add more metrics for better visibility feel free to do so.

// The expire tx handler unreserves the pending nonce
removeHandler := func(removeFromCache bool) {
if removeFromCache {
txmp.cache.Remove(txHash)
Copy link
Copy Markdown
Contributor

@wen-coding wen-coding Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what happens if some spammer keeps sending us an invalid tx which will fail at app CheckTx? (let's say a bad signature.)

The tx will be added to the cache, fail CheckTx, and then removed from the cache here? Should we also add a separate counter for certain app-level failures which should not happen to prevent against spamming?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would really love to avoid dealing with this code and just jump to rewriting it for autobahn. Your concern is valid though, of course.

@pompon0 pompon0 enabled auto-merge April 23, 2026 08:49
@pompon0 pompon0 added this pull request to the merge queue Apr 23, 2026
Merged via the queue into main with commit 7ea696a Apr 23, 2026
39 checks passed
@pompon0 pompon0 deleted the gprusak-fix branch April 23, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants