generate and send new auction_id with fast path quotes - #4713
generate and send new auction_id with fast path quotes#4713MartinquaXD wants to merge 2 commits into
Conversation
|
Claude finished @MartinquaXD's task in 3m 39s —— View job PR ReviewNicely scoped wiring change. The wire is backward-compatible in both directions ( Two inline comments (both minor / discussion-level):
One thing to fix outside the diff — the commit message is out of sync with the code:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
576e622 to
5aab4ce
Compare
Threads an `auction_id: Option<i64>` through `price_estimation::Query` and both sides of the driver `/quote` wire (orderbook-side request DTO and driver-side receiver DTO). Field is omitted from the wire when None, so old drivers keep parsing new requests; `#[serde(default)]` on the driver side lets an old orderbook keep talking to a new driver. For this step every writer still sets `None` — no behavior change. Step 4 will mint a real id in `QuoteParameters::to_price_query`. Also adds a serde smoke test covering both branches of the wire's `skip_serializing_if`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
851e5a7 to
23d1393
Compare
|
Reminder: Please consider backward compatibility when modifying the API specification.
Caused by: |
MartinquaXD
changed the title
| database::auction::get_next_auction_id(&mut ex) | ||
| .await | ||
| .context("failed to fetch next auction_id") |
There was a problem hiding this comment.
It looks like the fast path id never collides with the full auction id, but we have other readers, probably like reward payouts, analytics, circuit breaker. I just wanted to ensure this won't cause any issues, since they assume this id is always a full auction.
There was a problem hiding this comment.
Yeah, the sequence would ensure that there are no duplicated auction ids. Will double check with them that this will not cause any issues. 👌
| auction that will be associated with this quote competition. Only | ||
| populated for fast path orders | ||
| schema: | ||
| type: number |
There was a problem hiding this comment.
Should it be a type: integer with format: int64? IIRC, number allows decimals.
squadgazzz
left a comment
squadgazzz
left a comment
There was a problem hiding this comment.
LGTM, assuming the openapi will be adjusted.
AryanGodara
left a comment
AryanGodara
left a comment
There was a problem hiding this comment.
just some clarifying questons fo rme. Otherwise looks good 👌🏼
| /// auction associated with the quote competition | ||
| /// for fast path quotes | ||
| #[serde(default)] | ||
| auction_id: Option<i64>, |
There was a problem hiding this comment.
Why didn't we have this as a u32 (i mean, rn it makes sense, just asking for when we first set auction_id, since it's a natural number always) 👀
| pub enable_fast_path: bool, | ||
| /// auction associated with the given quote for faciliating | ||
| /// fast path execution. | ||
| pub auction_id: Option<i64>, |
There was a problem hiding this comment.
I've added this in #4678 already. That's ready to merge, so this'll probably need to be rebased after
| .transpose() | ||
| } | ||
|
|
||
| async fn get_next_auction_id(&self) -> Result<i64> { |
There was a problem hiding this comment.
is there any risk of infinite recursion here? If the inside func gets renamed? (ie, the one in crates/orderbook/src/database/quotes.rs)
It works rn ofc, just a rust doubt 😅
3 participants
Description
To reuse the existing driver /settle logic we need to send an auction id with fast path quotes. That way the driver can cache solutions and later settle solutions like it normally would when we send
/settle?auctionId=1234&solutionId=1.Changes
/quoterequest