Auditing the position tracking in the execution engine #3364
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
EVODelavega
approved these changes
Apr 16, 2021
peterbarrow
approved these changes
Apr 16, 2021
It is possible, although I'm not sure we should store that in the MarketPosition which is pretty much a pod type. |
applyFees is called in SubmitOrder, orderCancelReplace and LeaveAuction. While it would be acceptable to unregister the order when in the 2 first ones, it is not when LeavingAuction as this is applied on orders already uncrossed for which the position has been updated previously in handleConfirmation. This the unregister order out of the applyFees function, and call it from the caller.
Not finding a position in this engine should never be possible, this would mean that we did not register a position properly, or that we are trying to unregister one which has maybe already been unregistered. In any case that's a bug, and shall never happen, a panic is appropriate in this case.
For both these calls the following applies, it should be impossible to:
- update a position for a party which never registerer a potential positon
- decrease a potential position for more than the actual potential position
(basically assert trade.Size < potential{buy/sell}
…Order
We know added the following check in cases we AmendOrder:
- Panic if the party did not have a position
- Panic if the Remaining from the original order is > potential{buy/sell}
We know added the following check in cases we UnregisterOrder:
- Panic if the Remaining from the original order is > potential{buy/sell}
jeremyletang
force-pushed
the
bugfix/audit-position-tracking
branch
from
f29bcde
to
f157b30
Compare
- ensure we insert the new expiry date when amending changes it - cancel orders before settings flags for expiry changes, (make \ sure we do not expire twice, or add non-existing orders) - ensure we do not reduce position for order not foundInTheBook - remove PeggedOrder from the list if it's been cancelled by amend
- use unregisterAndReject if error on book - use IsFinished in SubmitOrder - do not submit pegged order to the book if auction is started
peterbarrow
approved these changes
Apr 16, 2021
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.
This will add a couple of things:
I'll try to keep the commit message sensible, please review eventually them one by one.