feat(pkg/sequencers): add queue limit in solo sequencer

[julienrbrt]

Overview

Extract and improve solo sequencer changes from #3306

[coderabbitai]

Warning

Rate limit exceeded

@julienrbrt has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 41 minutes and 6 seconds before requesting another review.

To continue reviewing without waiting, purchase usage credits in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dc40fa7f-c728-4c31-813b-77abc0c68402

📥 Commits

Reviewing files that changed from the base of the PR and between 264314f and 094dc44.

📒 Files selected for processing (9)

• .mockery.yaml
• CHANGELOG.md
• pkg/sequencers/based/forced_inclusion_retriever_mock.go
• pkg/sequencers/based/sequencer_test.go
• pkg/sequencers/common/errors.go
• pkg/sequencers/single/queue.go
• pkg/sequencers/single/sequencer.go
• pkg/sequencers/solo/sequencer.go
• pkg/sequencers/solo/sequencer_test.go

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch solo-seq-impr

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Claude finished @julienrbrt's task in 2m 51s —— View job

---

Code Review

• Gather context and read changed files
• Review code quality & architecture
• Review error handling & resilience
• Review concurrency patterns
• Review testing & reliability
• Post comprehensive feedback

---

Overall this is a clean, well-reasoned change. The centralized errors, functional-options pattern, and all-or-nothing queue semantics are all solid design choices. A few things worth fixing or clarifying.

---

Architecture & Code Quality

Positive: The pkg/sequencers/common/errors.go extraction is the right call — both single and solo now share canonical sentinel values via aliasing (var ErrQueueFull = common.ErrQueueFull), so callers can use errors.Is across package boundaries without issue.

Positive: Relocating the ForcedInclusionRetriever mock to pkg/sequencers/based via .mockery.yaml is correct since it's only consumed there, and having it in common was misleading.

Positive: Pre-allocated singleton response objects (emptyBatch, submitBatchResp, verifyBatchOKResp) eliminate per-call heap allocations on hot paths. Fine here since these types aren't mutable by callers.

Inconsistency — non-nil response on error pkg/sequencers/solo/sequencer.go:122

return submitBatchResp, ErrQueueFull

Returning a non-nil response alongside a non-nil error is atypical Go. The single sequencer correctly returns nil on error:

return nil, fmt.Errorf("batch queue is full, cannot accept more batches: %w", err)

Callers that check err != nil first will ignore the response, but it's still an interface contract inconsistency. Suggest returning nil for the response when returning ErrQueueFull.

Fix this →

---

Concurrency

Soft-cap race window pkg/sequencers/solo/sequencer.go:136–192

GetNextBatch drains the queue and zeroes queueBytes under the lock, then releases it, filters txs, and re-acquires the lock to prepend postponed txs. During the window between s.queueBytes = 0 (line 139) and s.queueBytes += bytes (line 190), a concurrent SubmitBatchTxs sees queueBytes = 0 and may admit a full maxQueueBytes payload even though postponed txs are about to be re-enqueued. Since the doc already calls this a "soft cap" this is acceptable — but it's worth an explicit note in WithMaxQueueBytes's godoc so callers aren't surprised:

// The cap may be transiently exceeded during batch processing if SubmitBatchTxs
// races with the postponed-tx re-enqueue in GetNextBatch.

Correct: The append(postponedTxs, s.queue...) pattern correctly prepends postponed txs before any that arrived during filtering. No ordering bug here.

---

Variable Shadowing

var bytes uint64 shadows bytes package import pkg/sequencers/solo/sequencer.go:187

var bytes uint64   // shadows the "bytes" import used in isValid()
for _, tx := range postponedTxs {
    bytes += uint64(len(tx))
}
s.queueBytes += bytes

golangci-lint with govet -shadow will flag this. Rename to postponedBytes (which also mirrors batchBytes used above in SubmitBatchTxs).

Fix this →

---

Error Handling

FilterTxs error path returns unfiltered txs without re-accounting pkg/sequencers/solo/sequencer.go:160–165

if err != nil {
    // ...
    return &coresequencer.GetNextBatchResponse{
        Batch:     &coresequencer.Batch{Transactions: txs},
        ...
    }, nil
}

When FilterTxs errors, the full txs slice is returned (good — best effort). queueBytes was already zeroed and txs is no longer in the queue. This is functionally correct since the block producer will commit them. It's worth a brief comment that on this path queueBytes stays at 0 intentionally.

---

Testing

Missing test for FilterTxs error path — this is one of the 7 Codecov-flagged missing lines. The path at solo/sequencer.go:161–165 (early return on filter error) has no test. Suggest adding:

func TestSoloSequencer_GetNextBatch_FilterError(t *testing.T) {
    mockExec := mocks.NewMockExecutor(t)
    mockExec.On("GetExecutionInfo", mock.Anything).Return(execution.ExecutionInfo{}, nil).Maybe()
    mockExec.On("FilterTxs", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
        Return(nil, errors.New("filter failed"))

    seq := NewSoloSequencer(zerolog.Nop(), []byte("test"), mockExec)
    _, _ = seq.SubmitBatchTxs(context.Background(), coresequencer.SubmitBatchTxsRequest{
        Id:    []byte("test"),
        Batch: &coresequencer.Batch{Transactions: [][]byte{{1}}},
    })

    resp, err := seq.GetNextBatch(context.Background(), coresequencer.GetNextBatchRequest{Id: []byte("test")})
    require.NoError(t, err)
    // Unfiltered txs are returned as-is
    assert.Len(t, resp.Batch.Transactions, 1)
}

Duplicate test helper — createDefaultMockExecutor is byte-for-byte identical in pkg/sequencers/solo/sequencer_test.go and pkg/sequencers/based/sequencer_test.go. If a shared testutil package is ever introduced this is an easy win, but it's not blocking.

Good coverage overall — TestSoloSequencer_SubmitBatchTxs_QueueFull_AllOrNothing, _DrainReleasesCapacity, and _UnboundedByDefault collectively validate the key invariants of the new queue logic. The TestSoloSequencer_GetNextBatch_PostponedTxsRequeued test verifying queueBytes re-accounting is especially valuable.

---

Summary

Severity	Issue
Low	return submitBatchResp, ErrQueueFull — should return nil response on error
Low	var bytes uint64 shadows bytes package import — rename to postponedBytes
Low	Missing test for FilterTxs error path (7 uncovered Codecov lines)
Info	Soft-cap race window during postponed re-enqueue — acceptable, but worth a doc note
Info	createDefaultMockExecutor duplicated across two test packages

[github-actions]

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

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	⏩ skipped	✅ passed	✅ passed	May 6, 2026, 4:08 PM

[codecov]

Codecov Report

❌ Patch coverage is 82.05128% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.86%. Comparing base (264314f) to head (094dc44).

Files with missing lines	Patch %	Lines
pkg/sequencers/solo/sequencer.go	82.05%	6 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3312      +/-   ##
==========================================
+ Coverage   60.69%   60.86%   +0.17%     
==========================================
  Files         127      127              
  Lines       13735    13762      +27     
==========================================
+ Hits         8336     8376      +40     
+ Misses       4494     4474      -20     
- Partials      905      912       +7     

Flag	Coverage Δ
combined	60.86% <82.05%> (+0.17%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.