Skip to content

Marketmaker cancel bug workaround#485

Merged
sindresorhus merged 1 commit into
masterfrom
mm-cancel-bug-workaround
Aug 30, 2018
Merged

Marketmaker cancel bug workaround#485
sindresorhus merged 1 commit into
masterfrom
mm-cancel-bug-workaround

Conversation

@lukechilds
Copy link
Copy Markdown
Member

Extends #484. Just targeting the #484 branch for now so you can easily see what's changed in the diff/commits. If we decide to merge this it should be merged into master after #484. Even if #484 isn't merged, this will still likely be useful for #481.

This ignores cancel events sent over the socket by mm if the swap is in progress. An in-progress swap can't be cancelled, they are sent in error by mm. Relevent bug (jl777/SuperNET#956).

I have done some testing and this appears to be working for me. I made the following swap which caused mm to incorrectly send failed events:

https://gist.github.com/lukechilds/b348cd594fab918c2bdd13b7e002ac5e

The changes I've made to the swap object formatting logic ignored the cancel events in this specific scenario and the UI displayed correctly throughout the entire swap.

This is a bit of a dirty hack but it should be ok until the bug is resolved in mm. I've only done some light testing so would advise testing this a bit more extensively before merging.

@lukechilds lukechilds requested a review from a team August 22, 2018 17:18
@sindresorhus sindresorhus changed the base branch from open-orders to master August 27, 2018 10:58
@sindresorhus sindresorhus force-pushed the mm-cancel-bug-workaround branch from 6d5d9d4 to eb21456 Compare August 27, 2018 11:00
@sindresorhus sindresorhus force-pushed the mm-cancel-bug-workaround branch from eb21456 to cb15fbf Compare August 27, 2018 11:00
@sindresorhus sindresorhus merged commit 29ca860 into master Aug 30, 2018
@sindresorhus sindresorhus deleted the mm-cancel-bug-workaround branch August 30, 2018 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants