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

orderwatch: Check for stale `removed` orders more frequently #471

Merged
merged 8 commits into from Oct 25, 2019

Conversation

@fabioberger
Copy link
Contributor

fabioberger commented Oct 24, 2019

Fixes: #459

This PR:

  • Adds a process to OrderWatcher that checks for orders that have been flagged for removal and should now be permanently deleted every 5 minutes.
  • Stops updating an order DB entry's lastUpdatedTime when the entry is in-fact not updated (i.e., when we revalidate the order but no change to fillability was found). This could cause an order from a particularly active maker to stick around indefinitely despite being expired/cancelled/fullyFilled, because the changes in the maker's balance would cause this order to constantly be re-validated and it's lastUpdatedTime to constantly get bumped up.
  • Slightly increases permanentlyDeleteAfter to 25 block depth to account for us flagging an order for removal due to UTC expiry, but it still potentially getting filled/cancelled a few blocks after. This adjustment is out of extreme caution.
@fabioberger fabioberger requested a review from albrow Oct 24, 2019
zeroex/orderwatch/order_watcher.go Show resolved Hide resolved
zeroex/orderwatch/order_watcher.go Outdated Show resolved Hide resolved
zeroex/orderwatch/order_watcher.go Outdated Show resolved Hide resolved
zeroex/orderwatch/order_watcher.go Show resolved Hide resolved
@albrow albrow self-requested a review Oct 24, 2019
@albrow
albrow approved these changes Oct 24, 2019
Copy link
Member

albrow left a comment

Just one small comment and other than that looks good to me!

fabioberger added 7 commits Oct 24, 2019
…marked for removal and should be permanently deleted
…te-change found when re-validating. Otherwise, a `removed` order can stick around forever if it's maker keeps transferring their excess balance for example
…rs to account for cases where order is flagged for removal due to UTC expiration and might still get filled/cancelled slightly after
@fabioberger fabioberger force-pushed the feature/pruneRemovedMoreFrequently branch from cf5ab8f to ac2ed19 Oct 25, 2019
@fabioberger fabioberger merged commit 760c1cb into development Oct 25, 2019
1 check passed
1 check passed
ci/circleci: build Your tests passed on CircleCI!
Details
@fabioberger fabioberger deleted the feature/pruneRemovedMoreFrequently branch Oct 25, 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.