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: Avoid updating orders multiple times in same txn (part 2) #623

Merged
merged 5 commits into from Jan 9, 2020

Conversation

@fabioberger
Copy link
Contributor

fabioberger commented Jan 6, 2020

Fixes: #608

Currently it is possible that we attempt to update orders that expire or unexpire twice within the same txn. This causes the second update to noop since our DB txns do not support multiple updates within a single txn. This PR stops this from happening.

@fabioberger fabioberger requested review from albrow and jalextowle Jan 6, 2020
@fabioberger fabioberger self-assigned this Jan 6, 2020
Copy link
Member

albrow left a comment

Just a few small suggestions, but could you also add a regression test?

CHANGELOG.md Outdated Show resolved Hide resolved
zeroex/orderwatch/order_watcher.go Show resolved Hide resolved
Copy link
Contributor

jalextowle left a comment

LGTM

zeroex/orderwatch/order_watcher.go Show resolved Hide resolved
@fabioberger fabioberger force-pushed the fix/conflictingTxnOperations branch from d95156e to 1a2af6c Jan 9, 2020
@albrow albrow self-requested a review Jan 9, 2020
@albrow
albrow approved these changes Jan 9, 2020
@fabioberger fabioberger merged commit 75b53ac into development Jan 9, 2020
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 fix/conflictingTxnOperations branch Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.