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

zeroex: Treat partially fillable orders as invalid #333

Merged

Conversation

@fabioberger
Copy link
Contributor

fabioberger commented Aug 2, 2019

0x has this quirk where an order can be partially fillable. What this means is that the order maker has a sufficient balance/allowance to fill some portion of the outstanding order but not the total remaining amount. We used to consider any order that can be filled for a non-zero amount as valid. This PR introduces a breaking change to instead consider any order where the fillableAmount != remainingAmount to be considered invalid.

After doing an empirical analysis of existing 0x orderbooks, the only orders that we've found in this state were mistakes. Additionally, it makes market fill methods less effective, since the 0x smart contracts do not validate balance/allowance amounts before fills but rely on thetoken transfer reverting to detect insufficient funds/allowance. This can happed for partially filled orders and so we've decided to simply reject these orders from the onset. This change also reduces the cognitive load on devs building on 0x, since they can now rely on the filled mapping on the smart contract to know how much of an order is still fillable.

@fabioberger fabioberger requested a review from albrow Aug 2, 2019
@albrow
albrow approved these changes Aug 2, 2019
Copy link
Member

albrow left a comment

Code changes look good to me. Just an open question about whether we should bump the version number for rendezvous and pubsub.

core/core.go Outdated Show resolved Hide resolved
@fabioberger fabioberger force-pushed the refactor/treatPartiallyFillableOrdersAsInvalid branch from 9b4269a to 560ec9d Aug 2, 2019
@fabioberger fabioberger force-pushed the refactor/treatPartiallyFillableOrdersAsInvalid branch from 560ec9d to f8543e3 Aug 2, 2019
@fabioberger fabioberger merged commit 57cd0e9 into development Aug 2, 2019
1 check passed
1 check passed
ci/circleci: build Your tests passed on CircleCI!
Details
@fabioberger fabioberger deleted the refactor/treatPartiallyFillableOrdersAsInvalid branch Aug 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.