Skip to content

strongly typed clist#3249

Merged
pompon0 merged 63 commits into
mainfrom
gprusak-mempool6
Apr 22, 2026
Merged

strongly typed clist#3249
pompon0 merged 63 commits into
mainfrom
gprusak-mempool6

Conversation

@pompon0

@pompon0 pompon0 commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

Made clist generic. Also

  • removed unused maxLength
  • made blocking methods of clist respect ctx
  • simplified CheckTx by removing the callback

@github-actions

github-actions Bot commented Apr 15, 2026

Copy link
Copy Markdown

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

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedApr 22, 2026, 4:05 PM

@pompon0 pompon0 requested review from sei-will and wen-coding April 15, 2026 08:13
@codecov

codecov Bot commented Apr 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.14570% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.39%. Comparing base (8a0da0e) to head (afc44f2).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
sei-tendermint/internal/rpc/core/mempool.go 66.66% 15 Missing and 2 partials ⚠️
sei-tendermint/internal/mempool/mempool.go 57.89% 7 Missing and 1 partial ⚠️
sei-tendermint/internal/evidence/reactor.go 66.66% 6 Missing and 1 partial ⚠️
sei-tendermint/internal/libs/clist/clist.go 97.82% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3249      +/-   ##
==========================================
+ Coverage   58.56%   59.39%   +0.82%     
==========================================
  Files        2072     2072              
  Lines      208020   171562   -36458     
==========================================
- Hits       121833   101895   -19938     
+ Misses      77413    60925   -16488     
+ Partials     8774     8742      -32     
Flag Coverage Δ
sei-chain-pr 70.89% <78.14%> (?)
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/evidence/pool.go 69.75% <100.00%> (ø)
sei-tendermint/internal/mempool/reactor/reactor.go 79.62% <100.00%> (+2.35%) ⬆️
sei-tendermint/internal/mempool/tx.go 83.33% <ø> (ø)
sei-tendermint/internal/libs/clist/clist.go 90.96% <97.82%> (+6.72%) ⬆️
sei-tendermint/internal/evidence/reactor.go 73.75% <66.66%> (-2.64%) ⬇️
sei-tendermint/internal/mempool/mempool.go 72.93% <57.89%> (-0.22%) ⬇️
sei-tendermint/internal/rpc/core/mempool.go 63.00% <66.66%> (-1.76%) ⬇️

... and 1754 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.

next, err := r.evpool.WaitEvidenceFront(ctx)
if err != nil {
panic(fmt.Errorf("failed to convert evidence: %w", err))
return

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: Should we return error some day?

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.

not necessarily, since the error will be always context.Canceled/DeadlineExceeded here. TBD

Comment thread sei-tendermint/internal/evidence/reactor.go
Comment thread sei-tendermint/internal/libs/clist/clist.go Outdated
Comment thread sei-tendermint/internal/libs/clist/clist.go Outdated
// Deprecated and should be removed in 0.37
func (env *Environment) BroadcastTxAsync(ctx context.Context, req *coretypes.RequestBroadcastTx) (*coretypes.ResultBroadcastTx, error) {
go func() { _ = env.Mempool.CheckTx(ctx, req.Tx, nil, mempool.TxInfo{}) }()
go func() { _, _ = env.Mempool.CheckTx(ctx, req.Tx, mempool.TxInfo{}) }()

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.

This ctx is the request context right? Will it be cancelled once the request is done? Then we run CheckTX in a canclled context?

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.

CheckTx in sei-chain/app/abci.go is not blocking, so it ignores the fact that context is cancelled. Context is used for tracing though.

peerMempoolID := r.ids.GetForPeer(peerID)
// TODO: this function does not call any external code, so panics should not be expected.
defer func() {
if e := recover(); e != nil {

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.

Do we not need this recover() any more?

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.

afaict this code is safe now. There is no interface calls which would be allowed to panic.

@pompon0 pompon0 enabled auto-merge April 22, 2026 16:05
@pompon0 pompon0 added this pull request to the merge queue Apr 22, 2026
Merged via the queue into main with commit cf0fb2d Apr 22, 2026
39 checks passed
@pompon0 pompon0 deleted the gprusak-mempool6 branch April 22, 2026 16:32
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