Cache fast-path quote solutions in the driver - #4678
Conversation
|
Reminder: Please consider backward compatibility when modifying the API specification.
Caused by: |
|
Claude finished @AryanGodara's task in 2m 7s —— View job PR Review: Cache fast-path quote solutions in the driver
Overall this is a clean, well-scoped change and the code reads well. Caching the solution (not a One point worth revisiting — posted inline:
Nothing else stood out — the fast-path/ |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
MartinquaXD
left a comment
MartinquaXD
left a comment
There was a problem hiding this comment.
Did you already check how those quotes could be turned into submittable transactions? That could inform us which additional API changes are needed.
| /// Upper bound on cached fast-path quote solutions, each fast-path quote pushes | ||
| /// exactly one entry, at a cadence unrelated to auctions. | ||
| const MAX_CACHED_QUOTE_SOLUTIONS: usize = 100; |
There was a problem hiding this comment.
I think if we want to give solvers optimal information for risk/inventory management we should probably tell them an expiration for the quote. This can then be used for the cache eviction.
Not necessary in the very first step, I think.
| /// throwaway auction it was solved against. | ||
| #[derive(Debug)] | ||
| pub struct CachedQuoteSolution { | ||
| pub auction: Auction, |
There was a problem hiding this comment.
Why are we storing the full auction here? Since we don't compute any new data we only need to have the solution be associated with the auction_id, no?
| Ok(scored.into_iter().map(|(solved, _)| solved).collect()) | ||
| } | ||
|
|
||
| /// Caches a fast-path quote's solution, keyed by the auction id (allocated |
There was a problem hiding this comment.
nit: the solution is not really keyed by anything since we are not storing it in a map to uphold uniqueness.
That being said using a map makes more sense here since those solutions will on average not be used in order.
| // For fast-path quotes the orderbook allocates a real auction id from the | ||
| // shared `auctions` sequence and forwards it here, so the solution can be | ||
| // encoded into a settlement and cached for a later `/settle`. |
There was a problem hiding this comment.
The driver should not make any assumptions on how the id gets generated. The only important invariant is that there will be a unique auction_id for each fast path quote.
| /// For fast-path quotes: the id of the cached solution and the auction id | ||
| /// it was cached under, so the caller can later settle it via `/settle`. | ||
| /// `None` for regular quotes. | ||
| pub solution_id: Option<u64>, | ||
| pub auction_id: Option<i64>, |
There was a problem hiding this comment.
Feels like this should actually not be part of the quote but rather be in the key the quote gets stored in a collection with.
| if self.enable_fast_path && !solver.fast_path_enabled() { | ||
| return Err(Error::QuotingFailed(QuotingFailed::FastPathNotSupported)); | ||
| } | ||
| let fast_path = self.enable_fast_path && solver.fast_path_enabled(); |
There was a problem hiding this comment.
At this point we should stop looking at solver.fast_path_enabled(). We already checked above that fast path support is enabled.
| .map(auction::Id); | ||
| let auction = self | ||
| .fake_auction(eth, tokens, solver.quote_using_limit_orders()) | ||
| .fake_auction(eth, tokens, solver.quote_using_limit_orders(), auction_id) |
There was a problem hiding this comment.
why do we have to pass auction_id if self already contains it?
| quote.solution_id = Some(solution_id); | ||
| quote.auction_id = Some(auction_id.0); |
There was a problem hiding this comment.
Seems wrong to add this data to the quote itself. Unless I overlook something I'd go with HashMap<(auction_id, solution_id), Quote> or something like that.
2 participants
Description
Fast-path (out-of-competition) execution reuses a quote's solution to settle the matching order during an exclusivity window instead of running a full auction. For that the driver must keep the solution it computes at quote time (rn
/quotediscards it). Now we cache the fast-path quote solution in the driver, keyed by(auction_id, solution_id), and usesthose ids on the quote response so a later settle can find it.Why cache the solution and not a ready
Settlement: a quote runs against an unsigned order, so a submittable settlement can't be encoded/simulated until the real signed order exists at settle time. (tried this, goes beyond scope of the PR 🤔 )Changes
quote_solutionscache toCompetition(CachedQuoteSolution { auction, solution }) +cache_quote_solution(): no simulation, bounded like thesettlementsdeque./quotecaches the solution when the quote is fast-path and the request carries anauctionId, and echoessolutionId+auctionIdon the response./quoterequest acceptsauctionId; the handler now passes the wholeCompetitiontoOrder::quote(was just&risk_detector) so it can reach the cache — the risk-detector filtering is unchanged.fastPath→enableFastPathto match the code.Out of scope / follow-ups:
auctionIdfrom the sharedauctionssequence and sending it in the/quoterequest (BE-56) — not included; the driver accepts the id but nothing supplies one yetRelated Issues
BE-58
Follow-ups: BE-56, BE-57, BE-61.