(fast4) inject final order_uid in proposed_trade_executions by MartinquaXD · Pull Request #4859 · cowprotocol/services

MartinquaXD · 2026-09-03T12:51:11Z

Description

So far we only have dummy order_uids in the proposed_trade_executions table. This PR checks if an order was placed against a fast path quote and then updates the respective proposed_trade_executions rows with the now known order_uid.

Changes

One important implementation detail is that fast path quotes should be associated with order 1:1 while the current logic theoretically allows placing multiple orders against the same reference quote.
While this was initially introduced as an optimization IIRC it does not make a ton of sense anymore so this PR deletes a quotes row when it gets promoted to a persistent order_quotes row.

How to test

updated existing unit tests

claude · 2026-09-03T12:51:51Z

Claude encountered an error after 4m 57s —— View job

PR Review: inject final `order_uid` in fast path `proposed_trade_executions`

Reviewed the promotion flow that rewrites placeholder order_uids once an order is placed against a fast-path quote. The transaction boundaries (insert_order now takes a PgTransaction), the field cleanup (dropping the .to_f64().unwrap() conversions now that order_quotes is sourced directly from the quotes row), and the data-modifying-CTE in finalize_quote_competition (Postgres runs unreferenced WITH UPDATEs to completion, so it's correct) all look sound. All trait callers were updated consistently.

I left 3 inline comments:

orderbook/.../orders.rs (main) — order_quotes is now sourced from delete_and_return_row(quote_id) instead of in-memory order.metadata.quote. If the transient quotes row is already gone (concurrent identical placement resolving to the same row via find_quote, or an expiry race), the order silently gets no order_quotes record — a regression vs. the old in-memory path. Worth confirming intent or decoupling the record insert from the fast-path patch.
orderbook/.../orders.rs (nit) — comment typos ("a auction" → "an auction", "not that we know" → "now that we know").
autopilot/.../onchain_order_events/mod.rs — the promotion loop patches competition_auctions for any quoted order with an auction_id even when the onchain placement failed/was out of market. Consider gating on placement success if ethflow + fast-path can co-occur.

No blocking issues beyond the first point, which is more of a "confirm this is intended" than a definite bug.
· branch fast-path-4

claude · 2026-09-03T12:56:30Z

+    // delete the transient `quotes` row so every order is tied to exactly one
+    // quote — the data is then moved directly into permanent `order_quotes`
+    // table.
+    let quote = match quote_id {
+        Some(id) => database::quotes::delete_and_return_row(ex, id)
+            .await
+            .map_err(InsertionError::DbError)?,
+        None => None,
+    };
+
+    if let Some(quote) = quote {
        let db_quote = database::orders::Quote {
            order_uid,
-            // safe to unwrap as these values were converted from f64 previously
-            gas_amount: quote.gas_amount.to_f64().unwrap(),
-            gas_price: quote.gas_price.to_f64().unwrap(),
-            sell_token_price: quote.sell_token_price.to_f64().unwrap(),
-            sell_amount: u256_to_big_decimal(&quote.sell_amount),
-            buy_amount: u256_to_big_decimal(&quote.buy_amount),
-            solver: ByteArray(quote.solver.0.0),
+            gas_amount: quote.gas_amount,
+            gas_price: quote.gas_price,
+            sell_token_price: quote.sell_token_price,
+            sell_amount: quote.sell_amount,
+            buy_amount: quote.buy_amount,
+            solver: quote.solver,
            verified: quote.verified,
-            metadata: quote.metadata.clone(),
-            auction_id: None,
+            metadata: quote.metadata,
+            auction_id: quote.auction_id,
        };


Behavior change: the order_quotes record can now be silently dropped.

Previously the order_quotes row was always inserted from the in-memory order.metadata.quote whenever a quote existed. Now it is sourced from delete_and_return_row(quote_id), so if the transient quotes row is gone by the time insert_order runs, delete_and_return_row returns None and no order_quotes row is written at all for the order.

That row can legitimately be missing before this call:

Two orders that resolve to the same quote (e.g. find_quote matches by parameters, not just id — identical orders placed concurrently both find quote row X). The first insert_order deletes X and commits; the second gets None → loses its quote record.

remove_expired_quotes racing an order that references a just-expired quote.

The old in-memory path was immune to both. Losing order_quotes affects downstream fee-policy / reward attribution for that order.

Since model::order::OrderQuote doesn't carry auction_id, the cleanest fix is probably to keep inserting order_quotes from order.metadata.quote as before, and use delete_and_return_row only to (a) source auction_id and (b) drive the fast-path competition patch + cleanup — so a missing transient row degrades to "no fast-path patch" rather than "no quote record". At minimum, worth confirming this drop is intentional.

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

+            // the quote is associated with a auction competition indicating
+            // that this is going to be used for a fast path execution.
+            // not that we know the final order uid we can patch up the


Comment typos:

Suggested change

// the quote is associated with a auction competition indicating

// that this is going to be used for a fast path execution.

// not that we know the final order uid we can patch up the

// the quote is associated with an auction competition indicating

// that this is going to be used for a fast path execution.

// now that we know the final order uid we can patch up the

claude · 2026-09-03T12:56:47Z

+        for (quote_id, quote, order) in izip!(&quote_ids, &quotes, &orders) {
+            let Some(quote) = quote else {
+                continue;
+            };
+            let Some(auction_id) = quote.auction_id else {
+                continue;
+            };
+            // The order_quotes row already carries the auction_id (populated
+            // inline above by `insert_quotes`), so all that's left is to drop
+            // the transient `quotes` row and patch competition tables.
+            database::quotes::delete_and_return_row(transaction, *quote_id)
+                .await
+                .context("failed to delete promoted onchain quote")?;
+            database::fast_path::finalize_quote_competition(transaction, auction_id, order.uid)
+                .await
+                .context("failed to patch competition rows for onchain order")?;
+        }


This loop patches competition_auctions.order_uids for every order that has a Some quote with an auction_id, regardless of whether the onchain placement actually succeeded. convert_onchain_order_placement returns quote: Ok(..) even for orders it flags as outside_market_price (it only increments a metric), so a failed/invalid fast-path placement would still overwrite the competition row with ARRAY[order_uid]. If ethflow + fast-path can co-occur, consider gating this on placement success (as the insert_quotes comment above at L346-348 implies quotes are only meaningful for orders that make it into an auction).

github-actions · 2026-09-03T12:57:10Z

⚠️ Claude PR review failed to run (is_error=true, no review posted — usually an Anthropic API/auth/credit error). See the workflow run for details, or check Claude status.

Adds a `database::fast_path` module with `finalize_quote_competition`, which patches the placeholder `order_uid = 0x00…00` rows written at quote time to the real uid once the order is placed. Threads `quote_id: Option<QuoteId>` through `OrderStoring::insert_order` and `replace_order`. When set, the orderbook drops the transient `quotes` row, promotes it into `order_quotes` (carrying `auction_id` forward), and calls `finalize_quote_competition`. The autopilot's onchain-order parser mirrors the same promotion path so ethflow orders backed by a fast-path quote finalise their competition too.

jmg-duarte

Notes, not blocking

jmg-duarte · 2026-09-03T16:42:45Z


        let data_tuple = onchain_order_data.into_iter().map(
-            |(event_index, quote, onchain_order_placement, order, tx_hash)| {
+            |(event_index, quote_id, quote, onchain_order_placement, order, tx_hash)| {


with this amount of parameters, I think it's time we make this a function

jmg-duarte · 2026-09-03T16:43:20Z

        Vec<W>,
+        Vec<i64>,
        Vec<Option<database::orders::Quote>>,
        Vec<(database::events::EventIndex, OnchainOrderPlacement)>,
        Vec<Order>,
        Vec<TxHash>,


IMO we're at a point this should be a struct

fleupold

Looks good, just unsure about what this new "a quote cannot be reused" logic might break.

fleupold · 2026-09-03T16:30:01Z

        order_placement_events: Vec<(ContractEvent, Log)>,
    ) -> Result<(
        Vec<W>,
+        Vec<i64>,


This return type looks pretty complicated. Can we use a QuoteId alias instead of i64 here or refactor it to return a named struct?

fleupold · 2026-09-04T07:51:44Z

+    // delete the transient `quotes` row so every order is tied to exactly one
+    // quote — the data is then moved directly into permanent `order_quotes`
+    // table.
+    let quote = match quote_id {


Can you add an e2e test what happens now when someone tries to use the same quote id twice? I feel like there are cases where we might end up writing an order to the DB without quote information (which may break a bunch of assumption down the line especially around the way fees are charged). I think this can happen if an quote is "found" via search (e.g. an order that doesn't use a quote id) and later or around the same time the order which generated the original quote gets placed specifying the quote id.

I feel like delete_and_return_row should not use fetch_optional but hard fail if the specified quote_id wasn't found on disk.

This way we reject orders that use a quote id which was already used.

I think the other race could be if the quote stream is still open, it may override a finalized auction if a new quote arrives after the order has been placed.

fleupold · 2026-09-04T09:47:55Z

            .await
            .context("insert_orders failed")?;

+        // Promote fast-path quotes for onchain orders (mirrors the trait-based


Aren't we missing applying the validFrom here as well? Probably a sign that the min_fast_path_exclusivity should live only in the autopilot and apply to orders that don't explicitly set a validFrom. Maybe we can "gate" the feature differently in the API?

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

claude Bot reviewed Sep 3, 2026

View reviewed changes

MartinquaXD changed the title ~~inject final order_uid in fast path proposed_trade_executions~~ (fast4) inject final order_uid in proposed_trade_executions Sep 3, 2026

MartinquaXD force-pushed the fast-path-4 branch from a117f40 to 9026308 Compare September 3, 2026 14:12

jmg-duarte approved these changes Sep 3, 2026

View reviewed changes

fleupold reviewed Sep 4, 2026

View reviewed changes

Navigation Menu

Uh oh!

(fast4) inject final order_uid in proposed_trade_executions - #4859

(fast4) inject final order_uid in proposed_trade_executions#4859
MartinquaXD wants to merge 1 commit into
fast-path-3from
fast-path-4

MartinquaXD commented Sep 3, 2026

Uh oh!

claude Bot commented Sep 3, 2026 •

edited

Loading

Uh oh!

claude Bot Sep 3, 2026

Uh oh!

claude Bot Sep 3, 2026

Uh oh!

claude Bot Sep 3, 2026

Uh oh!

github-actions Bot commented Sep 3, 2026

Uh oh!

jmg-duarte left a comment

Uh oh!

jmg-duarte Sep 3, 2026

Uh oh!

jmg-duarte Sep 3, 2026

Uh oh!

fleupold left a comment

Uh oh!

fleupold Sep 3, 2026

Uh oh!

fleupold Sep 4, 2026

Uh oh!

fleupold Sep 4, 2026

Uh oh!

fleupold Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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: inject final order_uid in fast path proposed_trade_executions

Uh oh!

claude Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 3, 2026

Uh oh!

jmg-duarte left a comment

Choose a reason for hiding this comment

Uh oh!

jmg-duarte Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

jmg-duarte Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

fleupold left a comment

Choose a reason for hiding this comment

Uh oh!

fleupold Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

fleupold Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

fleupold Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

fleupold Sep 4, 2026

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: inject final `order_uid` in fast path `proposed_trade_executions`