(fast2) Persist synthetic solver competition at quote time - #4856
(fast2) Persist synthetic solver competition at quote time#4856MartinquaXD wants to merge 1 commit into
Conversation
Adds `shared::quote_storage::save_quote_competition`, called by the
orderbook and autopilot quote-storage paths (`QuoteStoring::save_quote`).
For quotes that carry an `auction_id`, it also writes rows into
`competition_auctions`, `proposed_solutions` and
`proposed_trade_executions` (with the user's trade recorded under a
sentinel `order_uid = 0x00…00` because the real uid isn't known yet).
JIT orders proposed by solvers are intentionally not persisted here —
they live in the `quotes.metadata` JSON blob and the driver re-encodes
them at settle time.
Also adds supporting DB helpers:
* `database::quotes::delete_and_return_row` — used later at order
placement to promote a transient quote into an `order_quotes` row.
* `database::solver_competition_v2::{delete_by_auction_id,
save_from_quote}` — bulk writes and cleanup for the synthetic
competition.
Purely additive: nothing reads these rows yet.
|
Claude finished @MartinquaXD's task in 3m 16s —— View job PR Review: Persist synthetic solver competition at quote time
Overall this looks solid: the delete-then-insert approach correctly makes streamed sub-quotes idempotent (the plain I left 2 inline comments:
Neither blocks the core mechanism; the first is the one worth a decision before merge. I did not run the build/tests (the e2e coverage is noted as landing at the end of the stack). |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| let auction = Auction { | ||
| id: auction_id, | ||
| // Block, deadline, and order_uids are unknown at quote time; real | ||
| // values are populated when the user places the order and a full | ||
| // auction runs. | ||
| block: 0, | ||
| deadline: 0, | ||
| order_uids: Vec::new(), | ||
| price_tokens: native_price_tokens, | ||
| price_values: native_price_values, | ||
| surplus_capturing_jit_order_owners: Vec::new(), | ||
| penalty_caps_native: Some(Vec::new()), | ||
| }; | ||
| database::auction::save(&mut *tx, auction) | ||
| .await | ||
| .context("failed to save competition_auctions row")?; |
There was a problem hiding this comment.
Synthetic quote auctions are written into competition_auctions with block: 0, deadline: 0 and an id drawn from the same auctions sequence as real auctions (get_next_auction_id). This leaks into the "latest" queries that key off that table:
solver_competition_v2::load_latest(publicGET /v2/solver_competition/latest) doesORDER BY id DESCfiltered bydeadline <= current_block. A synthetic quote auction has the highest id anddeadline = 0 <= current_block, so right after any fast-path quote the endpoint returns a never-settled competition withblock = 0, placeholder scores, and a zeroorder_uid.auction::fetch_latest_prices(MAX(id)) andauction::fetch_latest_token_price("most recent auction that priced the token") will likewise start resolving against these 2-token synthetic rows.
Quotes that never become orders leave these block = 0 rows behind permanently. Is polluting the latest-competition / latest-price paths intended here, or should synthetic auctions be excluded from those queries (or given a sentinel deadline)? Unlike order_uid/score, block/deadline aren't called out as a follow-up TODO.
There was a problem hiding this comment.
Seems like a reasonable concern. Maybe differentiating those synthetic auctions more explicitly (even via a fast path column) may make sense.
Why don't we set block and deadline? Isn't there a well defined exclusivity period?
| for (index, quote) in data.quotes().iter().enumerate() { | ||
| let Some(solution_id) = quote.solution_id else { | ||
| continue; | ||
| }; | ||
| let solution_uid = i64::try_from(index).expect("solution index fits in i64"); |
There was a problem hiding this comment.
Non-winner quotes that lack a solution_id are silently skipped, but solution_uid is derived from the original enumerate() index. So if e.g. index 1 is skipped, the persisted uids become 0, 2, 3, …. load_by_id then computes ranking = uid + 1, producing gaps (1, 3, 4) in the ranking presented via the competition API. Consider assigning uid from a running counter over the kept solutions so ranks stay contiguous.
There was a problem hiding this comment.
I guess the translation is done, because solver chosen solution ids might not be unique. I think this is another example of why we should chose the quote id and simply reuse it instead of relying on solvers to generate one. It will also help our internal tracing.
MartinquaXD
changed the title
| for (index, quote) in data.quotes().iter().enumerate() { | ||
| let Some(solution_id) = quote.solution_id else { | ||
| continue; | ||
| }; | ||
| let solution_uid = i64::try_from(index).expect("solution index fits in i64"); |
There was a problem hiding this comment.
I guess the translation is done, because solver chosen solution ids might not be unique. I think this is another example of why we should chose the quote id and simply reuse it instead of relying on solvers to generate one. It will also help our internal tracing.
| @@ -0,0 +1,161 @@ | |||
| //! Persistence helpers for quote competitions. Shared between the orderbook | |||
There was a problem hiding this comment.
Weren't we trying to reduce the amount of code we put in the share crate? Could this module live in the database crate directly?
There was a problem hiding this comment.
Given we introduce a fast path db module later, I feel like the part of the code that is fake auction specific should actually live there. Wdyt?
| let auction = Auction { | ||
| id: auction_id, | ||
| // Block, deadline, and order_uids are unknown at quote time; real | ||
| // values are populated when the user places the order and a full | ||
| // auction runs. | ||
| block: 0, | ||
| deadline: 0, | ||
| order_uids: Vec::new(), | ||
| price_tokens: native_price_tokens, | ||
| price_values: native_price_values, | ||
| surplus_capturing_jit_order_owners: Vec::new(), | ||
| penalty_caps_native: Some(Vec::new()), | ||
| }; | ||
| database::auction::save(&mut *tx, auction) | ||
| .await | ||
| .context("failed to save competition_auctions row")?; |
There was a problem hiding this comment.
Seems like a reasonable concern. Maybe differentiating those synthetic auctions more explicitly (even via a fast path column) may make sense.
Why don't we set block and deadline? Isn't there a well defined exclusivity period?
| WITH | ||
| del_te AS (DELETE FROM proposed_trade_executions WHERE auction_id = $1), | ||
| del_jo AS (DELETE FROM proposed_jit_orders WHERE auction_id = $1), | ||
| del_ps AS (DELETE FROM proposed_solutions WHERE auction_id = $1) | ||
| DELETE FROM competition_auctions WHERE id = $1 |
| ); | ||
| return Ok(()); | ||
| } | ||
|
|
There was a problem hiding this comment.
Using Ok here is misleading, the log already points at it with the error level even
| for (index, quote) in data.quotes().iter().enumerate() { | ||
| let Some(solution_id) = quote.solution_id else { | ||
| continue; | ||
| }; |
There was a problem hiding this comment.
Really asks for a filter_map but then it becomes more verbose than it already is
| // Placeholder for the user's future order — the real uid is written | ||
| // when the order is placed. | ||
| let sell = u256_to_big_decimal("e.quoted_sell_amount); | ||
| let buy = u256_to_big_decimal("e.quoted_buy_amount); | ||
| let orders = vec
Description
In order for all the API queries and bookkeeping to work with fast path orders we need to build "synthetic" competition data based on the quote competition.
This only happens for quote competitions which have an auction_id associated with them which is the indicator that proper auction data is needed.
Changes
populates
competition_auctions,proposed_solutionsandproposed_trade_executionsfor quotes that need it.Because some data is unknown until an order actually gets created we store a dummy
order_uidandscore. Those will be updated in a later PR.Also note that we need to handle streamed quotes in a special way. Because streamed quotes need to be stored after every sub-quote we first delete any already existing competition data before we add insert new data.
How to test
e2e tests at the end of the PR stack