Skip to content
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

dex: exact spill price handling in FillRoute #2535

Merged
merged 3 commits into from
May 11, 2023
Merged

Conversation

hdevalence
Copy link
Member

This PR changes the FillRoute implementation to fill each successive frontier transactionally, letting us inspect the exact, actual price and compare it with the spill price. This avoids the situation in the current code where the spill price is only checked after committing to exceeding it.

The current behavior is retained in one special case, however: if we haven't consumed any positions, by default we don't consider the spill price. The motivation is that the spill price is an estimate, and we haven't ruled out the possibility that there could be two very similar routes, each with an estimated price below their actual price, which could cause the spill/fill cycle to alternate between them, always avoiding doing any filling because the fill would be more than the (underestimated) price of the alternative. By default, fill_route will now ignore the spill price until executing at least once, to ensure that routing makes progress. Callers can opt out of this behavior with fill_route_exact.

…plying fills

This commit adds a FrontierTx that we can use to transactionally apply a single
fill to the frontier state, allowing us to check the actual spill price prior
to filling a trade.
@hdevalence hdevalence requested a review from erwanor May 11, 2023 01:35
@hdevalence hdevalence temporarily deployed to smoke-test May 11, 2023 01:35 — with GitHub Actions Inactive
@hdevalence hdevalence merged commit ea793b8 into main May 11, 2023
@hdevalence hdevalence deleted the fillroute-spill branch May 11, 2023 02:39
@cratelyn cratelyn added the A-dex Area: Relates to the dex label Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dex Area: Relates to the dex
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants