Use quote id just for debugging #3054
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 68165ea6c428ec5a1646c8b5c5232cdd996576b5.
This reverts commit cf7551b76f60a8547016b09b65a463f91d9e0921.
MartinquaXD
force-pushed
the
quote-id-just-for-debugging
branch
from
4e14ca5
to
c2ad5f3
Compare
squadgazzz
reviewed
Oct 17, 2024
squadgazzz
reviewed
Oct 17, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Passing the quote id during the order creation request has currently weird and not well documented behavior.
Essentially it's used to streamline fetching quotes from the DB and allows us to better link an order creation request with a quote request.
If you provide no quote id the backend computes a fresh quote in the order creation requests so this usually never fails. However, if you provide a quote id we are currently throwing errors if the referenced quote is expired or the parameters don't match.
That means passing the optional quote id (that the openapi scheme documents just as a debugging tool) can only be detrimental for the user compared to passing no id at all.
Changes
This PR makes it such that this ID is actually only used for debugging and can not introduce new error variants. That means if we can find a quote based on the order's search parameters we just use that but if we can't find it we always try to compute a fresh one. This eliminates 2 error cases (quote expired, or parameter mismatch) and makes it easier to reason about the code.
How to test
the (unit) test coverage on that logic was already pretty good so I mostly had to adjust some tests (to please the compiler) and remove a few test cases for the 2 removed error variants.
1 e2e test needed to be updated. The test relied on using the quote id to make sure the backend looks up an outdated quote while creating the order.
So previously we could:
But now we have to make sure the order would get classified as market / limit at the time of creation. Before we could use the liquidity sources as a synchronization mechanism but now that's no longer possible. As an easy (and more explicit) way around that I added a function to conveniently enable or disable a solver account to make sure no solutions get settled until we are ready.