jl777 / SuperNET

57 stars 222 forks source link

Using `recentswaps` gives unreliable data #626

Open lukechilds opened 6 years ago

lukechilds commented 6 years ago

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: https://github.com/jl777/SuperNET/issues/600#issuecomment-367613936

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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