Added `QuoteMetadata` to `/orders/uid` request #3222

mstrug · 2025-01-08T13:25:42Z

Description

Exposing Quote data in the orderbook API on /orders/uid endpoint.

Changes

This is a follow up of PR #3124 which added storing in the database of some quote information (like interactions, jit orders and pre interactions) for auctions analysis in the order context. Current PR exposes these information (plus the whole quote data) through API, so any interested party can easily access this data.

Added new type OrderQuote
Added new field quote to the OrderMetadata
Removed OrderWithQuote type as the quote is already inside Order struct (through metadata field)

How to test

Updated two tests which validates if proper quote metadata is returned. Current e2e tests.
Tested locally on playground that order quote is returned through API.

github-actions · 2025-01-16T00:08:20Z

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

crates/model/src/order.rs

crates/orderbook/src/database/orders.rs

squadgazzz · 2025-01-23T10:09:54Z

Why don't we want to use QuoteMetadadaV1 in the API schema instead of raw json? How is it expected to use this data?

mstrug · 2025-01-23T23:03:29Z

Why don't we want to use QuoteMetadadaV1 in the API schema instead of raw json? How is it expected to use this data?

Intended use of the quote metadata in /orders/uid response is for debugging/analysis of the quotes from the auction winning solution by the solvers teams.
Because of that the quote metadata in OrderMetadata struct from the data model is not used internally, except that one case of providing that data to the users through API. So I think that the data model doesn't need to know the exact type of the quote metadata.
QuoteMetadataV1 is used only during order quoting to gather metadata for particular quote and store it in database also as raw json value.

MartinquaXD · 2025-01-24T11:09:52Z

crates/model/src/order.rs

+    /// If the order was crated with a quote, then this field contains the
+    /// quote's metadata.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub quote_metadata: Option<serde_json::Value>,


Given that it's also helpful to expose the actual quoted amounts I think the format of this would better be sth like:

quote: { // .. fields stored in `order_quotes` table }

That would expose all the necessary data at once.

github-actions · 2025-02-01T00:08:49Z

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

coderabbitai · 2025-02-07T23:21:52Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

mstrug · 2025-02-14T23:51:15Z

Updated PR description.

crates/orderbook/src/orderbook.rs

crates/orderbook/src/database/orders.rs

crates/model/src/order.rs

m-lord-renkse · 2025-02-17T11:53:16Z

crates/orderbook/src/database/orders.rs

-        order
-            .map(|order_with_quote| {
+
+        let order = match orders::single_full_order_with_quote(&mut ex, &ByteArray(uid.0)).await? {


We could have an early return to None in order to avoid one indexation level.

I'm not sure where we can do the early return, because for the None match we are checking jit orders.

orders::single_full_order_with_quote(&mut ex, &ByteArray(uid.0)) .await? .or_else(|| async { database::jit_orders::get_by_id(&mut ex, &ByteArray(uid.0)) .await? .map(full_order_into_model_order) }) .transpose()

Or

if let Some(order) = orders::single_full_order_with_quote(&mut ex, &ByteArray(uid.0)).await? { return Ok(Some(order)) } database::jit_orders::get_by_id(&mut ex, &ByteArray(uid.0)) .await? .map(full_order_into_model_order) .transpose() }

Thank you @squadgazzz ! exactly I meant that.

Ok, I get what you meant. Refactored.

Not much changed tbh :)
The main benefit of an early return is reducing indentation, making the code easier to read.

So improved the code readability by moving the Quote match to separate function, but in my opinion I don't see here an issue with the indentation.

crates/orderbook/src/database/orders.rs

crates/model/src/order.rs

crates/orderbook/src/orderbook.rs

crates/database/src/orders.rs

crates/orderbook/src/database/orders.rs

crates/database/src/orders.rs

MartinquaXD · 2025-02-18T10:24:06Z

crates/orderbook/src/database/orders.rs

@@ -1280,10 +1248,16 @@ mod tests {
            },
            ..Default::default()
        };
-        db.insert_order(&order, Some(quote)).await.unwrap();
+        db.insert_order(&order, Some(quote.clone())).await.unwrap();


the db.insert_order() signature is now quite weird to me. The order struct now contains quote metadata but we still pass an optional quote into it. This immediately begs the question what is supposed to happen when order.metadata.quote is different from the passed in quote?
Can this API be updated to not have this conflict anymore?
I suspect the underlying problem is probably that we are using the Order and OrderMetadata struct in too many places so it currently wears a lot of different hats which don't quite work in all places. 🤔

The main issue is that shared::order_quoting::Quote returned by validate_and_construct_order() can be converted to model::order::OrderQuote (order.metadata.quote field) but cannot be converted back (for storing in db) because of lack of some fields and I'm not sure if it is sensible to add all of the missing fields (mainly quote id which will be irrelevant after clearing quotes table, but also most of the QuoteData fields).

Also order.metadata.quote is created from the quote so it always has the same values.

Also order.metadata.quote is created from the quote so it always has the same values.

That doesn't really help to reduce currently created confusion where we operate with 2 quotes related to the same order but in different representations.

Still thinking about how this part can be improved:

services/crates/orderbook/src/database/orders.rs

Lines 240 to 242 in 81c3029

if let Some(quote) = quote {

insert_quote(&order.metadata.uid, &quote, &mut ex).await?;

}

The main issue is that shared::order_quoting::Quote returned by validate_and_construct_order() can be converted to model::order::OrderQuote (order.metadata.quote field) but cannot be converted back

I'd say the main issue is that we pass a ton of data into the DB call that is actually not needed. I believe if we only pass the necessary data (signed order data + a few additional data) we'd not have the issue of passing 2 possibly conflicting Quote structs into the same call.

crates/model/src/order.rs

Co-authored-by: ilya <[email protected]>

Implementation

1ccbf34

github-actions bot added the stale label Jan 16, 2025

mstrug removed the stale label Jan 16, 2025

mstrug added 4 commits January 16, 2025 23:19

Moved quote metadata to order metadata

e6206ca

Merge branch 'main' into api/quote-metadata-in-orders

c7efc2d

Updated tests

a843053

Merge branch 'main' into api/quote-metadata-in-orders

10171e0

mstrug changed the title ~~Added QuoteMetadata to /quotes/uid request~~ Added QuoteMetadata to /orders/uid request Jan 17, 2025

mstrug added 3 commits January 22, 2025 23:40

Merge branch 'main' into api/quote-metadata-in-orders

8808793

Changed QuoteMetadata type to json value

a7d3844

Fixed returning jit orders by uid

6b3840e

mstrug marked this pull request as ready for review January 23, 2025 01:52

mstrug requested a review from a team as a code owner January 23, 2025 01:53

squadgazzz reviewed Jan 23, 2025

View reviewed changes

crates/model/src/order.rs Outdated Show resolved Hide resolved

squadgazzz reviewed Jan 23, 2025

View reviewed changes

crates/orderbook/src/database/orders.rs Outdated Show resolved Hide resolved

Small refactoring

2f7915e

MartinquaXD reviewed Jan 24, 2025

View reviewed changes

github-actions bot added the stale label Feb 1, 2025

mstrug removed the stale label Feb 4, 2025

Merge branch 'main' into api/quote-metadata-in-orders

c028e13

mstrug marked this pull request as draft February 4, 2025 14:47

New implementation

4094f95

mstrug added 3 commits February 8, 2025 00:22

Merge branch 'main' into api/quote-metadata-in-orders

09f8f65

Fixed compilation

7424988

Removed OrderWithQuote

603401a

Merge branch 'main' into api/quote-metadata-in-orders

2c511f4

mstrug marked this pull request as ready for review February 14, 2025 23:51

squadgazzz reviewed Feb 17, 2025

View reviewed changes

crates/orderbook/src/orderbook.rs Outdated Show resolved Hide resolved

squadgazzz reviewed Feb 17, 2025

View reviewed changes

crates/orderbook/src/orderbook.rs Outdated Show resolved Hide resolved

squadgazzz reviewed Feb 17, 2025

View reviewed changes

crates/orderbook/src/database/orders.rs Outdated Show resolved Hide resolved

m-lord-renkse reviewed Feb 17, 2025

View reviewed changes

mstrug added 4 commits February 17, 2025 23:59

Update

0e93b31

Moved function used only in tests

89928c0

Fixed clippy warning

dc548ad

Removed set_order_quote() function

61de451

m-lord-renkse reviewed Feb 18, 2025

View reviewed changes

crates/database/src/orders.rs Outdated Show resolved Hide resolved

crates/orderbook/src/database/orders.rs Show resolved Hide resolved

Renamed function

6930802

squadgazzz reviewed Feb 18, 2025

View reviewed changes

crates/database/src/orders.rs Outdated Show resolved Hide resolved

MartinquaXD reviewed Feb 18, 2025

View reviewed changes

mstrug added 4 commits February 18, 2025 14:00

Replaced function in tests

baeeed7

Small refactoring

77bdeec

Readability improvement

81c3029

Merge branch 'main' into api/quote-metadata-in-orders

12e8c87

squadgazzz reviewed Feb 19, 2025

View reviewed changes

crates/model/src/order.rs Outdated Show resolved Hide resolved

Update crates/model/src/order.rs

126a670

Co-authored-by: ilya <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added `QuoteMetadata` to `/orders/uid` request #3222

Added `QuoteMetadata` to `/orders/uid` request #3222

mstrug commented Jan 8, 2025 •

edited

Loading

github-actions bot commented Jan 16, 2025

squadgazzz commented Jan 23, 2025 •

edited

Loading

mstrug commented Jan 23, 2025

MartinquaXD Jan 24, 2025

github-actions bot commented Feb 1, 2025

coderabbitai bot commented Feb 7, 2025

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

mstrug commented Feb 14, 2025

m-lord-renkse Feb 17, 2025

mstrug Feb 17, 2025

squadgazzz Feb 18, 2025 •

edited

Loading

m-lord-renkse Feb 18, 2025

mstrug Feb 18, 2025

squadgazzz Feb 19, 2025

mstrug Feb 19, 2025

MartinquaXD Feb 18, 2025

mstrug Feb 18, 2025

squadgazzz Feb 19, 2025

squadgazzz Feb 19, 2025

MartinquaXD Feb 20, 2025

	if let Some(quote) = quote {
	insert_quote(&order.metadata.uid, &quote, &mut ex).await?;
	}

Added QuoteMetadata to /orders/uid request #3222

Are you sure you want to change the base?

Added QuoteMetadata to /orders/uid request #3222

Conversation

mstrug commented Jan 8, 2025 • edited Loading

Description

Changes

How to test

github-actions bot commented Jan 16, 2025

squadgazzz commented Jan 23, 2025 • edited Loading

mstrug commented Jan 23, 2025

Choose a reason for hiding this comment

github-actions bot commented Feb 1, 2025

coderabbitai bot commented Feb 7, 2025

Review skipped

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

mstrug commented Feb 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

squadgazzz Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Added `QuoteMetadata` to `/orders/uid` request #3222

Added `QuoteMetadata` to `/orders/uid` request #3222

mstrug commented Jan 8, 2025 •

edited

Loading

squadgazzz commented Jan 23, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

squadgazzz Feb 18, 2025 •

edited

Loading