[Driver] Add SettlementStarted solver notification#4327
[Driver] Add SettlementStarted solver notification#4327kirsanium wants to merge 1 commit intocowprotocol:mainfrom
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Code Review
This pull request introduces a SettlementStarted notification to inform solvers when a settlement process begins. The changes include updating the notification domain models, DTOs, and the competition logic to trigger the event. A critical issue was identified regarding the placement of the notification: it is currently triggered before a submission slot is acquired, which could lead to inconsistent states if the acquisition fails. It is recommended to move the notification call until after the submission slot is successfully secured.
| notify::settlement_started( | ||
| &self.solver, | ||
| settlement.auction_id, | ||
| settlement.solution(), | ||
| ); |
There was a problem hiding this comment.
The SettlementStarted notification is triggered before a submission slot is acquired. If acquire() blocks due to high load or fails (e.g., during driver shutdown or request cancellation), the solver receives a notification for a settlement that hasn't actually started or may never proceed. This can lead to premature or incorrect hedging by PMMs and leaves the solver in an inconsistent state, as no subsequent executed notification will be sent if acquire() fails. Move this call to after the submission slot is successfully acquired.
| notify::settlement_started( | |
| &self.solver, | |
| settlement.auction_id, | |
| settlement.solution(), | |
| ); | |
| let guard = self.submitter_pool.acquire().await.ok_or(Error::SubmissionError)?; | |
| notify::settlement_started(&self.solver, settlement.auction_id, settlement.solution()); |
There was a problem hiding this comment.
We want the notification to fire as early as possible, the risk here is ok. That's also how the PMMs notification system works.
Description
Context
Some PMMs benefit from knowing that their quotes will be used in settlement as early as possible because they can hedge earlier, manage their risks better, and as a result provide better prices. The example would be #3452 feature request from Liquorice PMM - they solved their problem in PR #3451 by implementing a notification system that can notify PMM directly from driver. However, this approach requires PMMs to be involved in pushing a PR to cowprotocol, which is not always possible/feasible/desirable.
Solution
This PR introduces a new solver notification called SettlementStarted, that fires off whenever a solver has won and the settlement of its solution is about to start.
Changes
crates/solvers-dtonotification::Kind::SettlementStartedwas added.crates/driverinfra/notify/notification.rs- domain notification kindKind::SettlementStartedwas added.infra/solver/dto/notification.rs- a mapping fromdriverdomain tosolvers-dtonotification kind was added.infra/notify/mod.rs- functionsettlement_startedwas added, that fires-and-forgets the named notification.domain/competition/mod.rs- anotify::settlement_started()call was added right before trying to settle the solution.How to test
I found no existing notification testing system, so I decided I am not the one to include that. However, it's possible to do in e2e tests by setting up a driver, a solver, calling driver's
settlewith the solver's name on it and look for the notification to be received on the solver's side.Related Issues
Implements #4326