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

Race condition in using ManageOffer operation to modify an offer #181

Open
ScottHogge opened this issue Sep 27, 2018 · 2 comments
Open

Race condition in using ManageOffer operation to modify an offer #181

ScottHogge opened this issue Sep 27, 2018 · 2 comments
Labels
bug CAP that addresses broken behavior in the current protocol. CAP Represents an issue that requires a CAP. help wanted Open especially for those who want to write a CAP/SEP! needs draft This an issue that has no corresponding draft, and as such has not entered the CAP/SEP process.

Comments

@ScottHogge
Copy link

When using the ManageOffer operation to update an offer, the amount field replaces the currently available (unfilled) amount on the offer, rather than replacing the total liability specified when the offer was made. This leads to the following behavior:

Person A: Submit transaction 1: Sell 100 Marbles @ X1
Network processes transaction 1.
Person B: Submit transaction 2: Buy 90 Marbles @ X1
Person A: Submit transaction 3: Change offer to Sell 100 Marbles @ X2
Person C: Submit transaction 4: Buy 100 Marbles @ X2
Network processes transactions 2,3,4

At the end of this, Person A has sold 190 Marbles because of the race condition, whereas he intended to only sell 100 but update his price.

Pretty much all financial markets allow for atomic changes to orders, but its always very clear in the specs that it updates the total liability, not the outstanding liability, specifically because of the race condition with in-flight messages. For example, Nasdaq's proprietary specification states the following for the quantity field: "Total number of shares liable, inclusive of previous executions and Self Match Prevention decremented shares on this order chain"

@ScottHogge ScottHogge changed the title Race condition in using ManageOffer operation to modify an order Race condition in using ManageOffer operation to modify an offer Sep 27, 2018
@orbitlens
Copy link
Contributor

orbitlens commented Sep 27, 2018

As far as I understand, Person A will sell either 190 or 100 marbles in your example depending on the transactions application order, which is pseudo-random. The offer update is atomic, so the transaction 2 will either update the order if it was not fully executed yet or return an error if tx 4 is applied before the tx 2 (offer "Sell 100 Marbles @ X1" has been already executed and the expected offer_id does not exist anymore).

It makes things even more interesting.

@theaeolianmachine
Copy link
Contributor

@jonjove @MonsieurNicolas — any thoughts on this? Seems gnarly, and I'm not sure if any past changes about liabilities in the protocol have changed this scenario.

@theaeolianmachine theaeolianmachine added help wanted Open especially for those who want to write a CAP/SEP! CAP Represents an issue that requires a CAP. needs draft This an issue that has no corresponding draft, and as such has not entered the CAP/SEP process. bug CAP that addresses broken behavior in the current protocol. labels Mar 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug CAP that addresses broken behavior in the current protocol. CAP Represents an issue that requires a CAP. help wanted Open especially for those who want to write a CAP/SEP! needs draft This an issue that has no corresponding draft, and as such has not entered the CAP/SEP process.
Projects
None yet
Development

No branches or pull requests

4 participants
@theaeolianmachine @orbitlens @ScottHogge and others