(fast1) Thread solution_id through price-estimation by MartinquaXD · Pull Request #4855 · cowprotocol/services

MartinquaXD · 2026-09-03T12:31:52Z

Description

Fast path quotes are supposed to be treated as firm commitments. In order to store the solutions properly in the DB and have the autopilot reference a specific solution in the /settle call we need to receive a solution ID from the driver and thread it through the program.

Changes

add solution_id to quote response DTO and thread the data through the program
also uses serde(rename) for the currently misaligned flag indicating whether fast path orders are supported with the quote - this will be cleaned up later

How to test

e2e tests coming up at the end of the PR stack

Adds an `Option<u64> solution_id` field to `Estimate`, `LegacyTrade`, `Trade` and `QuoteResponse`, plumbed through the trade-finding / verifier stack. Comes from the solver's quote response DTO and defaults to `None` for every existing call site. Standalone mechanical change: nothing yet reads `solution_id`. Later patches will store it in `proposed_solutions.id` for fast-path competition data. Signed-off-by: MartinquaXD <martin@cow.fi>

claude · 2026-09-03T12:32:23Z

Claude finished @MartinquaXD's task in 1m 29s —— View job

PR Review: Thread `solution_id` through price-estimation

Read PR diff and understand the change
Verify field-name alignment (driver ↔ price-estimation DTOs)
Check the enableFastPath rename
Look for missed threading / correctness issues

Summary

This is a straightforward, correct mechanical change. solution_id: Option<u64> is threaded end-to-end: driver DTO → dto::Quote/dto::LegacyQuote → Trade/LegacyTrade → TradeKind::solution_id() → Estimate → QuoteResponse. I traced each hop and no path drops it.

Two things I specifically verified:

Field-name alignment for solution_id ✅ — The driver serializes it as solutionId (crates/driver/.../quote/dto/quote.rs:52 under rename_all = "camelCase"), and both dto::Quote and dto::LegacyQuote deserialize with rename_all = "camelCase", so solution_id ↔ solutionId matches. #[serde(default)] also keeps it backwards-compatible with responses that omit it.
The enableFastPath rename ✅ — This fixes a real request-side mismatch: the driver expects enableFastPath (quote/dto/order.rs, openapi.yml:49), but the struct-level rename_all = "camelCase" was previously serializing the flag as fastPath, so it was silently ignored. The field-level rename correctly overrides the struct rename. As noted in the PR description, this is a stopgap to be cleaned up later.

No blocking issues found — no inline comments to add. The bulk of the diff is solution_id: None in struct literals/tests, which is required for compilation and is fine.

One minor forward-looking note (not actionable in this PR): the doc comments say the value ultimately lands in proposed_solutions.id. Worth double-checking the u64 ↔ DB column type (i64?) conversion when the persistence PR lands, since driver-side ids are u64.
· branch fast-path-1

fleupold

Not blocking, but for other reasons, I think it might be better if we chose and communicate a unique quote id with every quote request (rather than letting solvers chose it themselves optionally for fast path orders).

fleupold · 2026-09-03T15:52:20Z

+        /// `QuoteResponse.solution_id` and, ultimately, into
+        /// `proposed_solutions.id` when a fast-path quote is persisted.
+        #[serde(default)]
+        pub solution_id: Option<u64>,


I would prefer if we gave every quote request a unique id (not just the winning one) and send it as part of the request to solvers. This way they could later on see which specific call caused the winning quote and we could properly index every single quote response in our data infra as well.

That fast path quote request id could later serve as a solution id.

Together with the quote request we already send a unique auction id. Later on we send the auction_id together with the solver's solution_id like we do with settle requests today.

Sure, but why don't we simply create a unique quote request ID that we send to each solver (and the winning one becomes the actual quote ID)? This way we don't need them to also come up with a solution id (which may not be unique across solvers) and create this compound primary key (fake auction id + solution id).

This would also generalize for non fast path quotes, which currently are hard to identify in clickhouse and allow solvers to map which of the many quote request we send them ended up being used on the final order.

I'm bringing this up now, because changing it later on will be much more expensive. If we see a benefit in having globally unique quote IDs, I think now is the moment to introduce them (and reuse them for fast path solution identification).

I also don't think the change would be that expensive to implement.

For streamed quotes for instance, we probably need a unique ID for each response, so the client can place an order from any of the responses (and not wait for some final ID to arrive at the end).

jmg-duarte

Nothing blocking, but IMO the "threading" notes should be addressed

jmg-duarte · 2026-09-03T15:37:26Z

+        // Renamed to match the driver's `enableFastPath` field name.
+        #[serde(
+            rename = "enableFastPath",


nano nit:

Suggested change

// Renamed to match the driver's `enableFastPath` field name.

#[serde(

rename = "enableFastPath",

#[serde(

// Renamed to match the driver's `enableFastPath` field name.

rename = "enableFastPath",

jmg-duarte · 2026-09-03T15:38:49Z

+        /// Solver-assigned solution id for this quote. Threaded through to
+        /// `QuoteResponse.solution_id` and, ultimately, into
+        /// `proposed_solutions.id` when a fast-path quote is persisted.
+        #[serde(default)]
+        pub solution_id: Option<u64>,


The "threaded through" can rot and we can always use editor functions to follow, I'd say a description of the systems it needs to go through would be better

jmg-duarte · 2026-09-03T15:42:10Z

+    /// Solver-assigned solution id for this trade, if the response carried
+    /// one.


nano-nit (apply to others if you take this suggestion):

Suggested change

/// Solver-assigned solution id for this trade, if the response carried

/// one.

/// Solver-assigned solution id for this trade,

/// if the response carried one.

jmg-duarte · 2026-09-03T15:42:56Z

    /// execution for this order.
    pub supports_fast_path: bool,
+    /// Solver-assigned solution id when the underlying quote response
+    /// carried one. Threaded end-to-end into `QuoteResponse.solution_id`.


comment about the threading applies here too

jmg-duarte · 2026-09-03T15:43:35Z

+    /// response carries one. Populated into `proposed_solutions.id` when
+    /// the quote is persisted as fast-path competition data.


its not clear what this "proposed_solutions" is about

MartinquaXD requested a review from a team as a code owner September 3, 2026 12:31

MartinquaXD changed the title ~~Thread solution_id through price-estimation~~ (fast1) Thread solution_id through price-estimation Sep 3, 2026

fleupold approved these changes Sep 3, 2026

View reviewed changes

jmg-duarte approved these changes Sep 3, 2026

View reviewed changes

Navigation Menu

Uh oh!

(fast1) Thread solution_id through price-estimation - #4855

(fast1) Thread solution_id through price-estimation#4855
MartinquaXD wants to merge 1 commit into
mainfrom
fast-path-1

MartinquaXD commented Sep 3, 2026

Uh oh!

claude Bot commented Sep 3, 2026 •

edited

Loading

Uh oh!

fleupold left a comment

Uh oh!

fleupold Sep 3, 2026

Uh oh!

MartinquaXD Sep 4, 2026

Uh oh!

fleupold Sep 4, 2026

Uh oh!

fleupold Sep 4, 2026

Uh oh!

fleupold Sep 4, 2026

Uh oh!

jmg-duarte left a comment •

edited

Loading

Uh oh!

jmg-duarte Sep 3, 2026

Uh oh!

jmg-duarte Sep 3, 2026

Uh oh!

jmg-duarte Sep 3, 2026

Uh oh!

jmg-duarte Sep 3, 2026

Uh oh!

jmg-duarte Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		/// Solver-assigned solution id for this trade, if the response carried
		/// one.

		/// response carries one. Populated into `proposed_solutions.id` when
		/// the quote is persisted as fast-path competition data.

Uh oh!

Conversation

MartinquaXD commented Sep 3, 2026

Description

Changes

How to test

Uh oh!

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

Uh oh!

PR Review: Thread solution_id through price-estimation

Summary

Uh oh!

fleupold left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmg-duarte left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

claude Bot commented Sep 3, 2026 •

edited

Loading

PR Review: Thread `solution_id` through price-estimation

jmg-duarte left a comment •

edited

Loading