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

Using recentswaps gives unreliable data #626

Open
lukechilds opened this issue Feb 28, 2018 · 6 comments
Open

Using recentswaps gives unreliable data #626

lukechilds opened this issue Feb 28, 2018 · 6 comments

Comments

@lukechilds
Copy link
Contributor

I've refactored our exchange view to use the recentswaps command to poll for the latest pending trades and then query them once they're in progress with swapstatus(requestid, quoteid) as per your suggestion: #600 (comment)

However when using this method, recentswaps doesn't give reliable information. I've noticed three scenarios when polling recentswaps:

Scenario 1

Sometimes we start a swap, it appears in the pending object returned by recentswaps, stays their until timeleft is 0 and is then removed if the order doesn't get filled.

This makes sense and is how I expected the API to work.

Scenario 2

Other times we start a swap, it appears in the pending object returned by recentswaps for a few seconds and then disappears. The order never gets filled but it doesn't stay in the pending object, it disappears after a few seconds even though timeleft is still greater than 0.

When this happens we can't be sure of the state of the swap because it also happens in scenario 3.

Scenario 3

When making a successful swap, it first appears in the pending object for a while, then when it gets matched it seems to disappear completely from recentswaps for a random amount of time to then come back in the swaps object with requestid/quoteid to query for more information.

When the trade is missing there's no way for us to reliably know the state of the swap. We don't know if it has been filled and is going to appear in the swaps object (scenario 3) or if it hasn't been filled and has just stopped showing in the recentswaps output (scenario 2). There's no way for us to differentiate.

We can resolve this by always assuming a swap is pending until it appears in swaps or the timeleft is 0. However this is a messy solution, it means we have separate timers tracking timeleft, one in mm and one in our app, timing is not ms accurate in JavaScript so this isn't reliable. There may be scenarios where a swap has been matched but we still show it as pending because we don't have up to date data. More seriously, it might be possible for a trade to get matched in the last few ms of timeleft in mm but in our app we consider timeleft to be expired, and so we consider the swap to have failed but actually mm is executing it.

I think the correct behaviour would be for pending swaps to always be in the output of swapstatus until they have expired. And for swaps that eventually get filled to always be in either pending or swaps. There shouldn't be a time when they're in limbo.

Alternatively, this could be solved by allowing us to query swapstatus with tradeid which we get instantly from buy. If we can poll swapstatus(tradeid) for the status of the swap mm can return detailed status pending/matched/complete/failed. That way we can be sure we are displaying reliable information.

However I'm not sure if the second option would be easy or performant to implement.

@jl777
Copy link
Owner

jl777 commented Mar 1, 2018

scenario 1 and 2 appear to be normal behavior. the issue is that there is a period where it is in limbo. I will investigate that and try to make it always be in either pending or the recentswaps (if it is on track to being a real swap)

@lukechilds
Copy link
Contributor Author

scenario 1 and 2 appear to be normal behavior.

So it's expected in scenario 2 that a trade can disappear from the pending object when it still has timeleft? Shouldn't it stay there for the duration of timeleft?

Sometimes it does and sometimes it doesn't. Shouldn't it always be consistent?

Or is there something happening inside mm that is determining it'll never be possible to be matched, and therefore removing it?

the issue is that there is a period where it is in limbo. I will investigate that and try to make it always be in either pending or the recentswaps (if it is on track to being a real swap)

That's great, thanks 👌

jl777 added a commit that referenced this issue Mar 1, 2018
@jl777
Copy link
Owner

jl777 commented Mar 1, 2018

pushed a version to jl777 branch that supports "fast":1 flag for swapstatus call with requestid/quoteid, it will limit itself to local file access and should never take more than time for at most 10 HDD file accesses

also, moved the adding to the "list" file before the "pending" object is cleared, so you might see recentswaps return the same swap in its list and in the "pending" object, but this avoids the limbo state

@lukechilds
Copy link
Contributor Author

Unfortunately this bug still doesn't appear to be fixed.

Running Marketmaker 0.1 27770 (193c76a) I got:

timestamp: 1519966954727
recentswaps
{"result":"success","swaps":[],"netamounts":[],"pending":{"expiration":1519967012,"timeleft":58,"tradeid":945257520,"requestid":0,"quoteid":0,"bob":"CHIPS","base":"CHIPS","basevalue":0.82130035,"alice":"KMD","rel":"KMD","relvalue":0.10001,"aliceid":5124924058741572000}}

timestamp: 1519966955731 (one second later)
recentswaps
{"result":"success","swaps":[],"netamounts":[]}

timestamp: 1519967013467 (57 seconds later)
recentswaps
{"result":"success","swaps":[[1099268696,304288325]],"netamounts":[]}

So for 57 seconds the trade was "in limbo".

@lukechilds
Copy link
Contributor Author

Also a separate issue where once the trade is in swaps we can't query it with swapstatus(requestid, quoteid).

I'll create a new issue for tracking that.

@jl777
Copy link
Owner

jl777 commented Mar 2, 2018

i must have missed a codepath for adding to the list file, investigating

DeckerSU pushed a commit to DeckerSU/SuperNET that referenced this issue Nov 5, 2022
…#628)

* Stop swaps when "stop" RPC is invoked WIP

* Wait for swaps to stop before returning from lp_init.

* WIP.

* Implemented FileLock.

* Adding FileLock to run_maker_swap and run_taker_swap WIP.

* WIP.

* Check whether the swap was finished on kick start when lock is acquired.

* Treat empty or broken file lock as expired. Switch to float for time comparisons.
Move swaps file lock related docker tests to separate module.
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

No branches or pull requests

2 participants