-
Notifications
You must be signed in to change notification settings - Fork 183
generate and send new auction_id with fast path quotes #4713
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.
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
base: link-quotes-to-auction
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,14 @@ paths: | |
| schema: | ||
| type: boolean | ||
| required: false | ||
| - in: query | ||
| name: auctionId | ||
| description: | | ||
| auction that will be associated with this quote competition. Only | ||
| populated for fast path orders | ||
| schema: | ||
| type: number | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should it be a |
||
| required: false | ||
| responses: | ||
| "200": | ||
| description: Quote successfully created. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,6 +119,9 @@ pub struct Order { | |
| pub side: order::Side, | ||
| pub deadline: chrono::DateTime<chrono::Utc>, | ||
| pub enable_fast_path: bool, | ||
| /// auction associated with the given quote for faciliating | ||
| /// fast path execution. | ||
| pub auction_id: Option<i64>, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added this in #4678 already. That's ready to merge, so this'll probably need to be rebased after |
||
| } | ||
|
|
||
| impl Order { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ impl Order { | |
| }, | ||
| deadline: self.deadline, | ||
| enable_fast_path: self.enable_fast_path, | ||
| auction_id: self.auction_id, | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -32,6 +33,10 @@ pub struct Order { | |
| deadline: chrono::DateTime<chrono::Utc>, | ||
| #[serde(default)] | ||
| enable_fast_path: bool, | ||
| /// auction associated with the quote competition | ||
| /// for fast path quotes | ||
| #[serde(default)] | ||
| auction_id: Option<i64>, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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) 👀 |
||
| } | ||
|
|
||
| #[derive(Debug, Deserialize)] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,4 +53,15 @@ impl QuoteStoring for Postgres { | |
| .map(|quote| Ok((quote.id, quote.try_into()?))) | ||
| .transpose() | ||
| } | ||
|
|
||
| async fn get_next_auction_id(&self) -> Result<i64> { | ||
| let _timer = super::Metrics::get() | ||
| .database_queries | ||
| .with_label_values(&["get_next_auction_id"]) | ||
| .start_timer(); | ||
| let mut ex = self.pool.acquire().await?; | ||
| database::auction::get_next_auction_id(&mut ex) | ||
| .await | ||
| .context("failed to fetch next auction_id") | ||
|
Comment on lines
+63
to
+65
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, the sequence would ensure that there are no duplicated auction ids. Will double check with them that this will not cause any issues. 👌 |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 😅