(fast7) Kick off fast path handling in runloop by MartinquaXD · Pull Request #4862 · cowprotocol/services

MartinquaXD · 2026-09-03T13:25:54Z

Description

Implements the heart of the fast path handling managed by the autopilot.

Changes

DB query to gather all the data needed for the fast path execution
determine which fees apply to the order
create fee_policies DB entries
update all bids by applying the volume fees to them
use extracted SettleCallCoordinator
a few metrics

How to test

few DB tests but main test will be e2e test in the next PR

claude · 2026-09-03T13:26:24Z

Claude encountered an error after 6m 23s —— View job

PR Review: Kick off fast path handling in runloop

Reviewed the diff against origin/fast-path-6. Overall this is clean and well-documented — the transactional NOTIFY design (order insert + finalize_quote_competition commit together, so the notification only fires once the promoted proposed_trade_executions rows are visible) is nicely done, and apply_volume_fee's direction matches the driver's fee_from_volume (fee in buy token for sell orders, sell token for buy orders). The volume-fee math is internally consistent with the pinned limit prices.

I left 4 inline comments; the two worth attention before merge:

within_limit check vs. signed limit (driver/.../solution/mod.rs:586) — the todo: double check if this makes sense guards the core price invariant. The check uses the fee-adjusted limit_prices rather than the user's signed order limit; nothing prevents limit_prices from crossing the signed limit, which would make the pinned settlement revert on-chain instead of failing fast.
Fees persisted before settle (fast_path.rs:167) — bid rewrites + fee_policies inserts happen before the /settle attempt, so they're orphaned on settle failure.

Plus two lower-priority notes: unbounded per-order background task running a 6-table join for every order (indexing / concurrency / feature gating), and settle failures logged only at debug.

Nothing blocking given the DB/e2e tests are landing in the follow-up PRs.
· fast-path-7

claude · 2026-09-03T13:32:24Z

+        // todo: double check if this makes sense...
        let within_limit = match order.side {
-            order::Side::Sell => user.buy_amount(&clearing)?.0 >= limit_prices.buy,
-            order::Side::Buy => user.sell_amount(&clearing)?.0 <= limit_prices.sell,
+            order::Side::Sell => user_trade.buy_amount(&clearing)?.0 >= limit_prices.buy,
+            order::Side::Buy => user_trade.sell_amount(&clearing)?.0 <= limit_prices.sell,
        };
        if !within_limit {
+            // todo: descriptive error message evaluating how much the price was
+            // off roughly
            return Err(error::Error::FastPathLimitNotMet);


The todo: double check if this makes sense is worth resolving before this ships, since it guards the core price invariant.

The check compares the solver's own clearing output against limit_prices (the fee-adjusted amounts computed by the autopilot), not the user's signed order limit (order.buy.amount / order.sell.amount). A few things to confirm:

Nothing here guarantees limit_prices.buy >= order.buy.amount (sell order) or limit_prices.sell <= order.sell.amount (buy order). If the volume fee pushes limit_prices past the signed limit, the settlement is pinned (a few lines below) to a price the settlement contract will reject on-chain — the fast path would then always revert rather than fail fast here.

Because the fill is immediately pinned to limit_prices, this within_limit check only validates that the solver's solution was good enough, not that the user's signed limit is honored.

Consider also asserting the signed limit here so a mispriced fast path is rejected cheaply instead of on-chain.

claude · 2026-09-03T13:32:29Z

+        self.persistence
+            .record_fast_path_fees(
+                fast_path_data.auction_id,
+                order_uid,
+                fast_path_data.model_order.data.kind,
+                &volume_factors,
+                &policies,
+            )
+            .await?;


record_fast_path_fees mutates persistent state (rewrites every bid's executed_sell/executed_buy on proposed_trade_executions and inserts fee_policies) before the /settle call is attempted. If the subsequent settle fails, these writes are left behind for a settlement that never happened, so the quote's synthetic auction now carries fee-adjusted bids and fee policies for an order that will instead go through a normal auction.

Is that acceptable for downstream accounting/analytics, or should the fee application be deferred until the settlement is confirmed (or rolled back on failure)? Worth a note either way.

claude · 2026-09-03T13:32:34Z

+                tokio::spawn(
+                    async move {
+                        match persistence.fast_path_order(order_uid).await {
+                            // not a fast path order -> do nothing
+                            Ok(None) => {}
+                            Err(err) => tracing::error!(?err, "failed to look up fast path order"),
+                            Ok(Some(order)) => fast_path.handle(order).await,
+                        };
+                    }
+                    .instrument(tracing::info_span!("fast_path", ?order_uid)),
+                );


Every order insert now spawns an unbounded background task that runs single_fast_path_order — a 6-table join (orders ⨝ order_quotes ⨝ proposed_solutions ⨝ proposed_trade_executions ⨝ competition_auctions + two correlated interactions subqueries + app_data). This runs for every order, including when fast path is effectively disabled (the query just returns None).

Two things worth considering:

Confirm the join columns are covered by indexes (order_quotes.order_uid, proposed_solutions(auction_id, is_winner), proposed_trade_executions(auction_id, solution_uid, order_uid)) so this stays cheap under normal order volume.

The spawn is unbounded — a burst of order inserts spawns an unbounded number of concurrent DB-querying tasks. A small concurrency limit / semaphore would bound the DB load.

If fast path can be globally toggled, gating this lookup on that flag would avoid the per-order cost entirely when it's off.

Sounds reasonable. In general having a quick check if an order is not fast path (and only run the joins if it is) probably makes sense.

claude · 2026-09-03T13:32:36Z

+        Metrics::fast_path_finished(&winner.name, res.is_ok());
+        match res {
+            Ok(tx) => tracing::info!(?tx, "settled order"),
+            Err(err) => tracing::debug!(?err, "failed to settle order"),


A failed fast-path settlement is only surfaced via the fast_path_executions{result="failure"} counter and a debug-level log. Given this is the terminal outcome of the whole fast-path attempt (and the order silently falls back to the regular auction), warn would make failures diagnosable in production without needing debug logging enabled.

github-actions · 2026-09-03T13:33:05Z

⚠️ 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 `run_loop::fast_path::FastPathHandler`: on each new order the autopilot picks up, it recovers the placeholder trade execution via `persistence::fast_path_order` (backed by a new `database::fast_path::single_fast_path_order` join query that carries the winning `FastPathOrder` row), computes the fee policies the order would receive in a regular auction (`ProtocolFees::apply`), applies the Volume-type factors to the recorded bid amounts via a new `shared::fee::apply_volume_fees` helper, and fires the `/settle` request through the shared `SettleCallCoordinator`. Persists the outcome atomically via `persistence::record_fast_path_fees`, which fetches every solver's bid on the order, rewrites each bid's own `executed_sell`/`executed_buy` with the same volume factors (so losing bids stay directly comparable to the winner), and inserts the fee-policy rows. The DB layer gains `database::fast_path::{FastPathBid, fast_path_bids, apply_fees_to_fast_path_bids}` — the last one does the bulk update in a single query via a `VALUES` join. `SolvableOrdersCache` now takes `Arc<ProtocolFees>` and `Arc<Vec<Address>>` for the jit owners so the same references are shared with the fast-path handler. Also extends the driver's `/settle` DTO with a `fast_path: Option<FastPath>` variant that carries the placed order + limit prices + native prices. When set, the driver skips the auction lookup and re-encodes the settlement from the request. Fulfillment gains a `with_order` helper that rebuilds a fulfillment against the placed order while keeping the recorded fill and haircut fee.

fleupold

This PR is the big one. Lots of comments, not sure what the best way to make progress is (maybe a sync meeting)

fleupold · 2026-09-04T08:59:46Z

@@ -0,0 +1,220 @@
+//! Handles a fast-path order the moment it lands: computes the applicable


is the fast path really part of the run loop module? Shouldn't it be its own top level module?

fleupold · 2026-09-04T09:05:14Z

                .clearing_price(order.buy.token)
                .ok_or(error::Error::FastPathOrderMismatch)?,
        };
+        // todo: double check if this makes sense...


I think those orders should be rejected to begin with.

fleupold · 2026-09-04T09:09:28Z

+    /// Applies fees to every solver's bid on a fast-path order and stamps
+    /// the applicable fee policies. Runs after the autopilot picks up the
+    /// placed order and computes the policies via `ProtocolFees::apply`.
+    ///


This sounds like a pretty logic heavy operation and not a simple persistence step. Can we move the actual computation into the fast path component and keep persistence as a simple convert and write to disk?

fleupold · 2026-09-04T09:11:43Z

+}
+
+/// The data the autopilot needs to settle a fast-path order out of competition.
+pub struct FastPathOrder {


Normally, these live in persistence/dto/...

In general, this 1k + line class looks pretty horrible.

fleupold · 2026-09-04T09:15:08Z

+/// fast-path order in a single query. Each row is matched by its own
+/// `solution_uid`, so different bids can be updated to different values.
+#[instrument(skip_all)]
+pub async fn apply_fees_to_fast_path_bids(


Overall, this is pretty hacky. It feels like we are writing to this table many times from different places, which creates a risk of data inconsistency. Is there not a way how we can incorporate the fees at order creation time already?

Also, if we update trade execution, we probably also need to update proposed_solutions.prices as otherwise our data become inconsistent.

fleupold · 2026-09-04T09:18:54Z

+/// - Buy orders: the sell amount is increased by `sell * factor`.
+///
+/// Uses high-precision scaling so sub-BPS factors don't round to zero.
+pub fn apply_volume_fee(sell: U256, buy: U256, kind: OrderKind, factor: FeeFactor) -> (U256, U256) {


This logic must already exist in many places in our code. If we make it available in the shared crate now, can we make the other call sites use it as well please?

fleupold · 2026-09-04T09:20:08Z

+                tokio::spawn(
+                    async move {
+                        match persistence.fast_path_order(order_uid).await {
+                            // not a fast path order -> do nothing
+                            Ok(None) => {}
+                            Err(err) => tracing::error!(?err, "failed to look up fast path order"),
+                            Ok(Some(order)) => fast_path.handle(order).await,
+                        };
+                    }
+                    .instrument(tracing::info_span!("fast_path", ?order_uid)),
+                );


Sounds reasonable. In general having a quick check if an order is not fast path (and only run the joins if it is) probably makes sense.

fleupold · 2026-09-04T09:20:50Z

+                // immediately spawn separate task to never delay processing
+                // fast path orders


Right now, both the leader and secondary autopilot would process fast orders, which will cause a mess I believe. Somehow this also needs the "leader" check.

fleupold · 2026-09-04T09:22:48Z

+        // todo: currently uses the regular auction's submission deadline but
+        // some smarter logic that takes block intervals, the progress
+        // of the current auction, and the order's valid_from into
+        // account should be implemented in the future
+        let deadline = self.eth.current_block().borrow().number + self.submission_deadline;


This should take the order's validFrom into account shouldn't it? Otherwise we may see an order in the main auction while a solver still thinks it's exclusive which will cause trouble.

Also, do we need to update block and deadline in the database at some point? Keeping a block of 0 might cause subtle issues with the settlement observer.

fleupold · 2026-09-04T09:28:50Z

+        )
+        .await
+        .context("apply_fees_to_fast_path_bids")?;
+        database::fee_policies::insert_batch(tx.deref_mut(), policy_rows)


nit: but should we drop any non volume fee policies to avoid confusion?

jmg-duarte

Felix already did quite a through job 😅

jmg-duarte · 2026-09-04T13:13:00Z

+    async fn insert_bid(
+        db: &mut PgConnection,
+        auction_id: AuctionId,
+        order_uid: OrderUid,
+        solution_uid: i64,
+        executed_sell: i64,
+        executed_buy: i64,
+    ) {


Is there no other function that does this in the database crate? there doesn't seem to be anything special here to have it in tests and i'd expect we have it somewhere else

jmg-duarte · 2026-09-04T13:14:56Z

+    const QUERY: &str = const_format::concatcp!(
+        "SELECT ",
+        "o.uid, o.owner, o.creation_timestamp, o.sell_token, o.buy_token, ",
+        "o.sell_amount, o.buy_amount, o.valid_to, o.app_data, o.kind, ",
+        "o.partially_fillable, o.signature, o.receiver, o.signing_scheme, ",
+        "o.sell_token_balance, o.buy_token_balance, o.class, ",
+        "array(SELECT (p.target, p.value, p.data) FROM interactions p",
+        " WHERE p.order_uid = o.uid AND p.execution = 'pre' ORDER BY p.index) AS pre_interactions, ",
+        "array(SELECT (p.target, p.value, p.data) FROM interactions p",
+        " WHERE p.order_uid = o.uid AND p.execution = 'post' ORDER BY p.index) AS post_interactions, ",
+        "ad.full_app_data AS full_app_data, ",
+        "oq.auction_id AS auction_id, ps.id AS solution_id, ps.uid AS solution_uid, ps.solver AS solver, ",
+        "pte.executed_sell AS executed_sell, pte.executed_buy AS executed_buy, ",
+        "ca.price_tokens AS price_tokens, ca.price_values AS price_values",
+        " FROM orders o",
+        " JOIN order_quotes oq ON oq.order_uid = o.uid",
+        " JOIN proposed_solutions ps ON ps.auction_id = oq.auction_id AND ps.is_winner",
+        " JOIN proposed_trade_executions pte",
+        " ON pte.auction_id = ps.auction_id AND pte.solution_uid = ps.uid AND pte.order_uid = o.uid",
+        " JOIN competition_auctions ca ON ca.id = oq.auction_id",
+        " LEFT JOIN app_data ad ON ad.contract_app_data = o.app_data",
+        " WHERE o.uid = $1",
+        " LIMIT 1",


instead of this mess of " and ", can't we use a proper string, even if the indentation shows slightly weird on RDS? if need be I can write a macro to fix the string inside 😅

this is just hard to read

jmg-duarte · 2026-09-04T13:24:39Z

+            while let Some(order_uid) = receiver.next().await {
                self.wake_notify.notify_one();


With fast_path coming (and more orders through the pipe I expect (?)), I wonder if we should instead collect batches here with recv_many, it would allow us to batch the DB queries instead of doing them 1 by 1 (of course it depends on whether, at any moment in time, more than one element is waiting in queue)

This would've worked before to maybe reduce ms's of latency but now we're issuing DB queries 🤔

jmg-duarte · 2026-09-04T13:26:23Z

+                            // not a fast path order -> do nothing
+                            Ok(None) => {}
+                            Err(err) => tracing::error!(?err, "failed to look up fast path order"),
+                            Ok(Some(order)) => fast_path.handle(order).await,


This makes me wonder if this and the settlement coordinator should have "queue interfaces" instead of actual functions we call, but at the same time, its just a different style (?)

jmg-duarte · 2026-09-04T14:01:27Z

+    /// comparison that doesn't exist here — but Surplus / PriceImprovement
+    /// policies are still recorded so downstream accounting reflects reality
+    /// if they ever become applicable.
+    async fn compute_and_persist_fees(


i thought fast path was supposed to give you exclusive execution upon quote victory, so it would have a quote to compare to by default?

MartinquaXD requested a review from a team as a code owner September 3, 2026 13:25

MartinquaXD changed the title ~~Kick off fast path handling in runloop~~ (fast7) Kick off fast path handling in runloop Sep 3, 2026

claude Bot reviewed Sep 3, 2026

View reviewed changes

MartinquaXD force-pushed the fast-path-7 branch from fa88d3a to 70b917f Compare September 3, 2026 14:22

fleupold reviewed Sep 4, 2026

View reviewed changes

jmg-duarte reviewed Sep 4, 2026

View reviewed changes

		@@ -0,0 +1,220 @@
		//! Handles a fast-path order the moment it lands: computes the applicable

		// immediately spawn separate task to never delay processing
		// fast path orders

		while let Some(order_uid) = receiver.next().await {
		self.wake_notify.notify_one();

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: Kick off fast path handling in runloop

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!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

jmg-duarte 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

claude Bot commented Sep 3, 2026 •

edited

Loading