Move past multiple orders in Filling state
#1775
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
luckysori
force-pushed
the
fix/two-open-orders
branch
2 times, most recently
from
7282d5f
to
8674eeb
Compare
I don't see any benefit in dumbing down this model for the frontend. And it can create confusion when debugging issues.
If no new `Order`s can be submitted until the current `Order` in `Filling` is either moved to `Filled` or `Failed`, we ensure that we cannot have multiple `Order`s in `Filling` at the same time. This is important because this was supposed to be an invariant which we were not upholding.
The previous patch attempted to prevent the situation from happening, but we have already encountered instances where this has happened. To handle them, we choose to: - Take the _oldest_ `Order` in `OrderState::Filling`, because it is the one that was never resolved in the first place. This order is the one most likely to match the unfinished update to the subchannel. - Discard all other `Order`s in `OrderState::Filling`, because the user should not have been able to create them anyway.
The function `order_failed` does some cleanup when an order could not be filled. But if the DB does not know about this order for some reason, there isn't really anything to clean up in terms of orders and the rest of the cleanup should continue.
luckysori
force-pushed
the
fix/two-open-orders
branch
from
8674eeb
to
20ba2b5
Compare
bonomat
approved these changes
Jan 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #1762.
Ultimately, it's still bad that we have no way of linking an order that is in
Fillingto a particular subchannel update. With such information we would be able to more confidently discard irrelevant orders and continue with the only relevant one.I considered other approaches, but I think this is the simplest.