Skip to content

(fast1) Thread solution_id through price-estimation #4855

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MartinquaXD wants to merge 1 commit into from
+82 −1
Open

(fast1) Thread solution_id through price-estimation #4855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions crates/price-estimation/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,9 @@ pub struct Estimate {
/// Whether the quoting solver supports fast-path (out-of-competition)
/// 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`.

@jmg-duarte jmg-duarte Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment about the threading applies here too

pub solution_id: Option<u64>,
/// Data associated with this estimation.
#[debug(ignore)]
pub execution: QuoteExecution,
Expand Down
1 change: 1 addition & 0 deletions crates/price-estimation/src/native/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ mod tests {
solver: Address::repeat_byte(1),
verified: false,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
})
}
Expand Down
18 changes: 18 additions & 0 deletions crates/price-estimation/src/sanitized.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ impl SanitizedPriceEstimator {
solver: Default::default(),
verified: true,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
};
tracing::debug!(?query, ?estimation, "generate trivial price estimation");
Expand All @@ -141,6 +142,7 @@ impl SanitizedPriceEstimator {
solver: Default::default(),
verified: true,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
};
tracing::debug!(?query, ?estimation, "generate trivial unwrap estimation");
Expand All @@ -156,6 +158,7 @@ impl SanitizedPriceEstimator {
solver: Default::default(),
verified: true,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
};
tracing::debug!(?query, ?estimation, "generate trivial wrap estimation");
Expand Down Expand Up @@ -224,6 +227,7 @@ mod tests {
solver: Default::default(),
verified: false,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
}),
),
Expand All @@ -250,6 +254,7 @@ mod tests {
solver: Default::default(),
verified: false,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
}),
),
Expand Down Expand Up @@ -293,6 +298,7 @@ mod tests {
solver: Default::default(),
verified: false,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
}),
),
Expand All @@ -316,6 +322,7 @@ mod tests {
solver: Default::default(),
verified: true,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
}),
),
Expand All @@ -338,6 +345,7 @@ mod tests {
solver: Default::default(),
verified: true,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
}),
),
Expand All @@ -361,6 +369,7 @@ mod tests {
solver: Default::default(),
verified: true,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
}),
),
Expand All @@ -384,6 +393,7 @@ mod tests {
solver: Default::default(),
verified: true,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
}),
),
Expand Down Expand Up @@ -457,6 +467,7 @@ mod tests {
solver: Default::default(),
verified: false,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
})
}
Expand All @@ -474,6 +485,7 @@ mod tests {
solver: Default::default(),
verified: false,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
})
}
Expand All @@ -491,6 +503,7 @@ mod tests {
solver: Default::default(),
verified: false,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
})
}
Expand All @@ -508,6 +521,7 @@ mod tests {
solver: Default::default(),
verified: false,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
})
}
Expand Down Expand Up @@ -555,6 +569,7 @@ mod tests {
solver: Default::default(),
verified: true,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
}),
),
Expand All @@ -576,6 +591,7 @@ mod tests {
solver: Default::default(),
verified: true,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
}),
),
Expand All @@ -602,6 +618,7 @@ mod tests {
solver: Default::default(),
verified: true,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
})
}
Expand All @@ -619,6 +636,7 @@ mod tests {
solver: Default::default(),
verified: true,
supports_fast_path: false,
solution_id: None,
execution: Default::default(),
})
}
Expand Down
20 changes: 19 additions & 1 deletion crates/price-estimation/src/trade_finding/external.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ impl From<dto::LegacyQuote> for LegacyTrade {
solver: quote.solver,
tx_origin: quote.tx_origin,
supports_fast_path: quote.supports_fast_path,
solution_id: quote.solution_id,
}
}
}
Expand Down Expand Up @@ -215,6 +216,7 @@ impl From<dto::Quote> for Trade {
tx_origin: quote.tx_origin,
jit_orders: quote.jit_orders,
supports_fast_path: quote.supports_fast_path,
solution_id: quote.solution_id,
}
}
}
Expand Down Expand Up @@ -363,6 +365,7 @@ impl TradeFinding for ExternalTradeFinder {
gas_estimate,
solver: trade.solver(),
supports_fast_path: trade.supports_fast_path(),
solution_id: trade.solution_id(),
execution: QuoteExecution {
interactions: map_interactions_data(trade.interactions()),
pre_interactions: map_interactions_data(trade.pre_interactions()),
Expand Down Expand Up @@ -402,7 +405,12 @@ pub mod dto {
pub amount: U256,
pub kind: OrderKind,
pub deadline: chrono::DateTime<chrono::Utc>,
#[serde(default, skip_serializing_if = "std::ops::Not::not")]
// Renamed to match the driver's `enableFastPath` field name.
#[serde(
rename = "enableFastPath",
Comment on lines +408 to +410

@jmg-duarte jmg-duarte Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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",

default,
skip_serializing_if = "std::ops::Not::not"
)]
pub fast_path: bool,
#[serde(skip_serializing_if = "Option::is_none")]
pub auction_id: Option<i64>,
Expand All @@ -429,6 +437,11 @@ pub mod dto {
pub tx_origin: Option<Address>,
#[serde(default)]
pub supports_fast_path: bool,
/// 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>,

@fleupold fleupold Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@MartinquaXD MartinquaXD Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@fleupold fleupold Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@fleupold fleupold Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@fleupold fleupold Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

}

#[serde_as]
Expand All @@ -448,6 +461,11 @@ pub mod dto {
pub jit_orders: Vec<JitOrder>,
#[serde(default)]
pub supports_fast_path: bool,
/// 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>,
Comment on lines +464 to +468

@jmg-duarte jmg-duarte Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

#[serde_as]
Expand Down
16 changes: 16 additions & 0 deletions crates/price-estimation/src/trade_finding/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ pub struct Quote {
pub solver: Address,
/// Whether the quoting solver supports fast-path execution.
pub supports_fast_path: bool,
/// Solver-assigned solution id, when the underlying quote response
/// carries one.
pub solution_id: Option<u64>,
#[debug(ignore)]
pub execution: QuoteExecution,
}
Expand Down Expand Up @@ -90,6 +93,13 @@ impl TradeKind {
}
}

pub fn solution_id(&self) -> Option<u64> {
match self {
TradeKind::Legacy(trade) => trade.solution_id,
TradeKind::Regular(trade) => trade.solution_id,
}
}

pub fn simulation_solver_address(&self) -> Address {
self.tx_origin().unwrap_or(self.solver())
}
Expand Down Expand Up @@ -148,6 +158,9 @@ pub struct LegacyTrade {
pub tx_origin: Option<Address>,
/// Whether the quoting solver supports fast-path execution.
pub supports_fast_path: bool,
/// Solver-assigned solution id for this trade, if the response carried
/// one.
Comment on lines +161 to +162

@jmg-duarte jmg-duarte Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

pub solution_id: Option<u64>,
}

/// A trade with JIT orders.
Expand All @@ -169,6 +182,9 @@ pub struct Trade {
pub jit_orders: Vec<dto::JitOrder>,
/// Whether the quoting solver supports fast-path execution.
pub supports_fast_path: bool,
/// Solver-assigned solution id for this trade, if the response carried
/// one.
pub solution_id: Option<u64>,
}

impl Trade {
Expand Down
1 change: 1 addition & 0 deletions crates/price-estimation/src/trade_finding/trade_estimator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ impl Inner {
solver: quote.solver,
verified: false,
supports_fast_path: quote.supports_fast_path,
solution_id: quote.solution_id,
execution: quote.execution,
})
}
Expand Down
2 changes: 2 additions & 0 deletions crates/price-estimation/src/trade_verifier.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ impl TradeVerifier {
solver: trade.solver(),
verified: false,
supports_fast_path: trade.supports_fast_path(),
solution_id: trade.solution_id(),
execution: QuoteExecution {
interactions: map_interactions_data(trade.interactions()),
pre_interactions: map_interactions_data(trade.pre_interactions()),
Expand Down Expand Up @@ -837,6 +838,7 @@ fn ensure_quote_accuracy(
solver: trade.solver(),
verified: true,
supports_fast_path: trade.supports_fast_path(),
solution_id: trade.solution_id(),
execution: QuoteExecution {
interactions: map_interactions_data(trade.interactions()),
pre_interactions: map_interactions_data(trade.pre_interactions()),
Expand Down
Loading
Loading