Persist streamed quotes by fleupold · Pull Request #4579 · cowprotocol/services

fleupold · 2026-06-30T11:42:22Z

Description

Streamed quotes currently don't carry a quote ID making it impossible to reuse them during order placement. This is problematic for a couple of reasons

It increases the amount of quote requests we are sending to solvers by a non trivial amount.
Re-quoting when the order is placed may mess with intended slippage, market order classification and other things that are needed for a reliable user and integrator experience.
Quotes without an ID can never receive a quote reward. If solvers find a way to segregate the flow, they would be better off selectively not responding to streamed quote requests, which are expected to become the standard way for users to make the decision whether they want to trade (therefore they need to be extra competitive)

Changes

Persist every quote candidate before it gets sent out in the stream
Update the documentation and test cases around this.

How to test

Existing tests

Alternatives considered

https://app.notion.com/p/cownation/RFC-Quote-Persistence-for-Streaming-Quotes-38f8da5f04ca81a98f0fc016a3a37e81 contains two alternative AI generated options:

Mint the id at completion, send it as a final event.
Reserve one id up front, store the best under it (recommended).

The first option leads to having no quote id unless we wait for the full timeout. This defeats the purpose of being able to quickly place orders as soon as we have a quote that is "good enough". Especially with the high deadlines suggested in cowprotocol/docs#638 I think we would not end up getting the final event in a good amount of cases.

In the second option, if the client terminates the connection prematurely there may be subjectivity around which quotes have been received (and therefore what the client saw as "best").

The main downside of the approach in this PR is that it adds a small amount of latency on the critical path (for the storage write), which in the grand scheme of current quote latency is likely irrelevant. It will also create more pressure on the DB, however since unused quotes are very short lived I don't expect this to be a concern in practice. It strikes me as the simplest approach therefore.

github-actions · 2026-06-30T11:42:35Z

Reminder: Please consider backward compatibility when modifying the API specification.
If breaking changes are unavoidable, ensure:

You explicitly pointed out breaking changes.
You communicate the changes to affected teams (at least Frontend team and SAFE team).
You provide proper versioning and migration mechanisms.

Caused by:

crates/orderbook/openapi.yml

claude · 2026-06-30T11:42:47Z

Claude finished @fleupold's task in 1m 52s —— View job

PR Review: Persist streamed quotes

Reviewed the diff against origin/main. The change is small and consistent with the one-shot endpoint (persist quote.data before the volume-fee adjustment, then expose the resulting id). Docs and tests are updated to match. 👍

I left one inline comment on the new persistence block in order_quoting.rs regarding failure semantics when storage is unavailable:

A save failure now drops an otherwise-valid quote (previously it was always emitted, just id-less).
The yielded error is conflated with a per-solver failure one layer up, so a full DB outage surfaces to the client as NoLiquidity rather than an internal error — diverging from the one-shot path.

Both are worth a conscious decision, but not blocking. The latency / DB-pressure tradeoffs are already acknowledged in the PR description, so I won't restate them.

Nothing else stood out — the mock setup (times(0..)) and assertion updates look correct.

gemini-code-assist

Code Review

This pull request updates the streaming quote endpoint to persist generated quotes and return their corresponding IDs, aligning its behavior with the one-shot quote endpoint. The changes span the OpenAPI documentation, quote handler, streaming logic, and tests. Feedback was provided to terminate the stream immediately upon a database save failure rather than continuing, as a database error is likely a persistent infrastructure issue.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

squadgazzz

I have the following concerns:

The stored quotes set now depends on the requesting client's timeout. A stream that closes early persists only the fast solvers' quotes and drops slower solvers, some of which would have returned a better price. Those partial results then sit in the shared pool, and unrelated orders gonna reuse them through find. Wouldn't one client's timeout choice leak into another user's quote quality? The stored quote becomes the order's reference quote.
quotes::find might get slower for orders placed without a quote id. On prod, the planner runs it as a full seq scan, so the cost tracks the total row count. WETH/USDC alone is already ~500 of ~5000 non-expired quotes. Storing one row per solver would inflate that significantly.

squadgazzz · 2026-06-30T12:26:18Z

        assert!(
-            response.id.is_none(),
-            "streaming quotes should have id == null, got {:?}",
+            response.id.is_some(),


Does anything prove that /order with that id hits the stored quote instead of re-quoting?

Good catch, added that to quote_stream_smoke.

squadgazzz · 2026-06-30T12:27:08Z

+                match storage.save(quote.data.clone()).await {
+                    Ok(id) => quote.id = Some(id),
+                    Err(err) => {
+                        yield Err(CalculateQuoteError::Other(err));
+                        continue;
+                    }
+                }


This happens per quote on the hot path, so a slow write delays the event, and a failed write turns a good quote into an error event for that solver. Maybe it makes sense to still stream the quote with an id: null instead of returning an error?

Regarding the slow write, this is the case with the regular order path today and I don't think our DB roundtrip time is material compared to the overhead of sending an auction to all connected solvers.

Regarding the failure case, I think your suggestion makes sense.

fleupold

Wouldn't one client's timeout choice leak into another user's quote quality? The stored quote becomes the order's reference quote.

find_quote is only invoked when an order is placed without a quote id (in which case you are right we are trying to find an existing quote with the exact same amounts). I don't see how this is an issue in practice though. A good integration should only place orders with quote ids.
Also note, that already today someone requesting an optimal quote with a very short timeout will create an entry that another order may hit when being placed without a quote id.

quotes::find might get slower for orders placed without a quote id. On prod, the planner runs it as a full seq scan, so the cost tracks the total row count.

It's true that right now it's a SeqScan, but an index exists so the reason for this is likely that the current table size is too small to justify using the index today. Once it grows (max 20x), I'm pretty confident the index will protect us from getting too slow. In any case we have existing metrics around db latency, which we can monitor.

fleupold · 2026-06-30T19:05:47Z

+                match storage.save(quote.data.clone()).await {
+                    Ok(id) => quote.id = Some(id),
+                    Err(err) => {
+                        yield Err(CalculateQuoteError::Other(err));
+                        continue;
+                    }
+                }


Regarding the slow write, this is the case with the regular order path today and I don't think our DB roundtrip time is material compared to the overhead of sending an auction to all connected solvers.

Regarding the failure case, I think your suggestion makes sense.

fleupold · 2026-06-30T19:18:08Z

        assert!(
-            response.id.is_none(),
-            "streaming quotes should have id == null, got {:?}",
+            response.id.is_some(),


Good catch, added that to quote_stream_smoke.

squadgazzz · 2026-07-01T07:28:58Z

Also note, that already today someone requesting an optimal quote with a very short timeout will create an entry that another order may hit when being placed without a quote id.

Oh, that's right, I completely forgot about it.

squadgazzz · 2026-07-01T08:36:33Z

find_quote is only invoked when an order is placed without a quote id (in which case you are right we are trying to find an existing quote with the exact same amounts). I don't see how this is an issue in practice though.

And currently, this find_quote mechanism doesn't search for the best quote among many:

ORDER BY gas_amount * gas_price * sell_token_price ASC
LIMIT 1

I used this query to get some stats for orders that were created w/ and w/o quote id: https://victorialogs.dev.cow.fi/goto/bfqs2t7z0k4jkf?orgId=1

last 7d on mainnet:
found by the provided id: 359360
couldn't find by the provided id: 1162
an order request was sent w/o id: 67803

So the number of orders with no id is quite high and we probably need to adjust the find_quote logic to find the best quote. Does it make sense?

squadgazzz · 2026-07-01T07:31:34Z

+                // error event.
+                match storage.save(quote.data.clone()).await {
+                    Ok(id) => quote.id = Some(id),
+                    Err(err) => tracing::warn!(?err, "failed to persist streamed quote"),


Maybe an error would help more here to catch DB issues.

Suggested change

Err(err) => tracing::warn!(?err, "failed to persist streamed quote"),

Err(err) => tracing::error!(?err, "failed to persist streamed quote"),

fleupold added 2 commits June 30, 2026 13:29

Persist streamed quotes

a9bccef

reduce code comments

61ec65f

fleupold requested a review from a team as a code owner June 30, 2026 11:42

gemini-code-assist Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread crates/shared/src/order_quoting.rs

claude Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread crates/shared/src/order_quoting.rs

squadgazzz reviewed Jun 30, 2026

View reviewed changes

fleupold commented Jun 30, 2026

View reviewed changes

comments

a650c69

squadgazzz reviewed Jul 1, 2026

View reviewed changes

	Err(err) => tracing::warn!(?err, "failed to persist streamed quote"),
	Err(err) => tracing::error!(?err, "failed to persist streamed quote"),

Uh oh!

Conversation

fleupold commented Jun 30, 2026

Description

Changes

How to test

Alternatives considered

Uh oh!

github-actions Bot commented Jun 30, 2026

Uh oh!

claude Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Persist streamed quotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

squadgazzz left a comment

Choose a reason for hiding this comment

Uh oh!

squadgazzz Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

fleupold Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

squadgazzz Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

fleupold Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

fleupold left a comment

Choose a reason for hiding this comment

Uh oh!

fleupold Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

fleupold Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

squadgazzz commented Jul 1, 2026

Uh oh!

squadgazzz commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

squadgazzz Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

claude Bot commented Jun 30, 2026 •

edited

Loading

squadgazzz commented Jul 1, 2026 •

edited

Loading