Skip to content

Also consider settle queue before rejecting solutions#4337

Closed
MartinquaXD wants to merge 4 commits intomainfrom
consider-settle-queue-before-rejecing-requests
Closed

Also consider settle queue before rejecting solutions#4337
MartinquaXD wants to merge 4 commits intomainfrom
consider-settle-queue-before-rejecing-requests

Conversation

@MartinquaXD
Copy link
Copy Markdown
Contributor

@MartinquaXD MartinquaXD commented Apr 15, 2026

Description

The reference driver rejects new solutions when there is already a backlog of solutions that still need to be submitted because they will most likely not be mined in time. This is intended to protect very competitive solvers from penalties when they win too much but can't submit fast enough.
#4167 introduced a bug where the check whether to reject the /solve request only looks at the available tx submission slots but not the settle queue.
This has the consequence that a solver with only a single submission EOA that won an auction will reject /solve requests until the previous solution was submitted.

Changes

Only reject new solutions if we don't have a submission slot AND the settle queue is full.

@MartinquaXD MartinquaXD requested a review from a team as a code owner April 15, 2026 16:11
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the ensure_settle_queue_capacity method to include a check on the settlement queue's capacity. A logic error was identified where the capacity check is unreliable because requests are dequeued immediately, potentially leading to the driver accepting more auctions than it can handle concurrently. It is suggested to limit the number of concurrent spawned tasks to make the queue capacity check meaningful.

Comment on lines +774 to +775
if !self.submitter_pool.has_capacity() && self.settle_queue.capacity() == 0 {
tracing::warn!(
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.

medium

The check self.settle_queue.capacity() == 0 is unreliable for determining if the settlement backlog is full. The process_settle_requests loop dequeues requests and spawns tasks immediately, which means the settle_queue channel is almost always empty, even when many settlements are pending (waiting for a submission slot). While DoS concerns are not applicable for these trusted requests per repository rules, this logic error allows the driver to accept more auctions than it can handle concurrently, potentially leading to penalties for the solver due to settlements not being mined in time. To fix this, the process_settle_requests loop should be modified to limit the number of concurrent spawned tasks to total_slots. This would cause the settle_queue channel to actually buffer the backlog and make the capacity check meaningful.

@MartinquaXD MartinquaXD added the hotfix Labels PRs that should be applied into production right away label Apr 15, 2026
@MartinquaXD MartinquaXD changed the title Consider settle queue before rejecting requests Also consider settle queue before rejecting solutions Apr 15, 2026
Comment thread crates/driver/src/domain/competition/mod.rs
@jmg-duarte
Copy link
Copy Markdown
Contributor

jmg-duarte commented Apr 15, 2026

when they win too much but can't submit fast enough.

tenor

Comment on lines +798 to +806
let permit = match self.submitter_pool.acquire().await {
Some(guard) => guard,
None => {
if let Err(err) = request.response_sender.send(Err(Error::SubmissionError)) {
tracing::warn!(?err, "failed to report submission error");
}
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.

Suggested change
let permit = match self.submitter_pool.acquire().await {
Some(guard) => guard,
None => {
if let Err(err) = request.response_sender.send(Err(Error::SubmissionError)) {
tracing::warn!(?err, "failed to report submission error");
}
return;
}
};
let Some(permit) = match self.submitter_pool.acquire().await else {
if let Err(err) = request.response_sender.send(Err(Error::SubmissionError)) {
tracing::warn!(?err, "failed to report submission error");
}
return;
};

@MartinquaXD MartinquaXD marked this pull request as draft April 15, 2026 17:10
@MartinquaXD
Copy link
Copy Markdown
Contributor Author

Closing in favor of #4338

@github-actions github-actions bot locked and limited conversation to collaborators Apr 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

hotfix Labels PRs that should be applied into production right away

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants