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,core,blockwatch: Improve Order Event subscription stability #566

Merged
merged 65 commits into from Dec 18, 2019

Conversation

@fabioberger
Copy link
Contributor

fabioberger commented Nov 29, 2019

This PR ensures that Mesh's OrderWatcher is in complete lock-step about what the latest block it knows about is, and therefore also the block at which validations/re-validations are to be conducted. While these eth_call's are in progress, no further block events are processed, so that we don't introduce any race conditions. This eliminates several bugs:

  • Incoming orders being validated at an outdated block height and us missing block events processed while the validation was ongoing. (#590)
  • Blocks being discovered and stored to the DB but not processed before Mesh gets shut down. Upon re-booting it assumes all stored blocks have been processed. We now only store blocks once they've been fully processed. (#588)

Additionally, it:

  • Batch emits ADDED order events (#559)
  • Properly passes down the top-level context to every order validation eth_call request.
  • Ensures that at least one block was processed before processing incoming orders (RPC/P2P), so we have an updated block height at which to process them.
@fabioberger fabioberger changed the title Refactor/more performant order watching orderwatch,core,blockwatch: Improve Order Event subscription performance Nov 29, 2019
@fabioberger fabioberger force-pushed the refactor/morePerformantOrderWatching branch from 8f68748 to 28305bc Nov 29, 2019
@fabioberger fabioberger marked this pull request as ready for review Dec 2, 2019
@fabioberger fabioberger requested a review from albrow Dec 2, 2019
@fabioberger fabioberger force-pushed the refactor/morePerformantOrderWatching branch 3 times, most recently from ea2b1df to 11df17f Dec 2, 2019
constants/constants.go Outdated Show resolved Hide resolved
core/core.go Outdated Show resolved Hide resolved
encoding/encoding.go Outdated Show resolved Hide resolved
integration-tests/integration_test.go Outdated Show resolved Hide resolved
meshdb/meshdb.go Outdated Show resolved Hide resolved
zeroex/orderwatch/order_watcher_test.go Outdated Show resolved Hide resolved
zeroex/orderwatch/order_watcher.go Outdated Show resolved Hide resolved
core/message_handler.go Show resolved Hide resolved
ethereum/blockwatch/block_watcher.go Outdated Show resolved Hide resolved
ethereum/blockwatch/block_watcher.go Outdated Show resolved Hide resolved
@fabioberger fabioberger force-pushed the refactor/morePerformantOrderWatching branch 2 times, most recently from b770a83 to e429ce3 Dec 3, 2019
@fabioberger fabioberger force-pushed the refactor/morePerformantOrderWatching branch from 05c67ad to 1c4f24b Dec 12, 2019
@fabioberger fabioberger changed the base branch from refactor/blockWatcher to development Dec 12, 2019
@fabioberger fabioberger force-pushed the refactor/morePerformantOrderWatching branch from d1c2756 to 931e05f Dec 12, 2019
@fabioberger fabioberger changed the title orderwatch,core,blockwatch: Improve Order Event subscription performance orderwatch,core,blockwatch: Improve Order Event subscription stability Dec 13, 2019
Copy link
Member

albrow left a comment

Mostly looks pretty good. Much fewer suggestions this time and most of them are minor. I also messaged you separately about test coverage for the Cleanup method.

zeroex/order.go Outdated 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
zeroex/orderwatch/order_watcher.go Outdated Show resolved Hide resolved
@fabioberger fabioberger force-pushed the refactor/morePerformantOrderWatching branch from 958f45b to 123617b Dec 14, 2019
zeroex/orderwatch/order_watcher_test.go Outdated Show resolved Hide resolved
zeroex/orderwatch/order_watcher_test.go Show resolved Hide resolved
@albrow

This comment has been minimized.

Copy link
Member

albrow commented Dec 16, 2019

Also need to address merge conflicts now that #580 is merged.

@fabioberger fabioberger force-pushed the refactor/morePerformantOrderWatching branch from 1aa5e7a to 7141dc9 Dec 17, 2019
@fabioberger fabioberger force-pushed the refactor/morePerformantOrderWatching branch from 7141dc9 to 391dd91 Dec 18, 2019
@albrow albrow self-requested a review Dec 18, 2019
@albrow
albrow approved these changes Dec 18, 2019
@fabioberger fabioberger merged commit 0b1ca79 into development Dec 18, 2019
2 checks passed
2 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
continuous-integration/drone/push Build is passing
Details
@fabioberger fabioberger deleted the refactor/morePerformantOrderWatching branch Dec 18, 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.