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

swapstatus(requestid, quoteid) shows invalid swap data on fresh swaps #631

Open
lukechilds opened this issue Mar 2, 2018 · 3 comments
Open

Comments

@lukechilds
Copy link
Contributor

lukechilds commented Mar 2, 2018

Following on from #626

Once a swap moves from pending to swaps in recentswaps and gives us requestid/quoteid, if we try to query it it shows invalid data:

timestamp: 1519967013467
recentswaps
{"result":"success","swaps":[[1099268696,304288325]],"netamounts":[]}

timestamp: 1519967014152 (0.6 seconds later)
swapstatus(requestid, quoteid)
{"error":"swap never started","status":"finished","bob":"","src":"","alice":"","dest":"","requestid":1099268696,"quoteid":304288325}

timestamp: 1519967015696 (1.5 seconds later)
swapstatus(requestid, quoteid)
{"expiration":1519982552,"tradeid":945257520,"requestid":1099268696,"quoteid":304288325,"iambob":0,"Bgui":"","Agui":"hyperdex","gui":"hyperdex","bob":"CHIPS","srcamount":0.82319963,"bobtxfee":0.0001,"alice":"KMD","destamount":0.1,"alicetxfee":0.00001,"aliceid":"5124924058741571585","sentflags":["myfee"],"values":[0,0,0,0,0,0,0.0001287,0,0,0,0],"result":"success","status":"pending","bobdeposit":"0000000000000000000000000000000000000000000000000000000000000000","alicepayment":"0000000000000000000000000000000000000000000000000000000000000000","bobpayment":"0000000000000000000000000000000000000000000000000000000000000000","paymentspent":"0000000000000000000000000000000000000000000000000000000000000000","Apaymentspent":"0000000000000000000000000000000000000000000000000000000000000000","depositspent":"0000000000000000000000000000000000000000000000000000000000000000"}

As soon as the swap is moved out of pending and we can query it it's in a weird state for 2.1 seconds. There is an error saying it never started but the status is set to finished which is confusing. After 2.1 seconds it starts it's normal pending status.

I think the correct behaviour should be that as soon the trade has moved from pending to swaps in the swapstatus output so we can query it, it should be set as pending.

@jl777
Copy link
Owner

jl777 commented Mar 2, 2018

maybe you can wait 3 seconds before the first query? you are actually querying it and it is returning all the data it has about the swap

what is happening is that the requestid/quoteid is added to the list of all swaps
then as the state changes, files are created and that allows the swapstatus to update the return result.

it seems for the first few seconds there is no new file as I am now adding the requestid/quoteid to the tracking BEFORE it actually starts (to fix the limbo issue). Of course I still need to fully fix the limbo issue, so maybe this is a side effect of that bug.

However, internally what is happening is that there is no info about that swap at all the first couple seconds after it appears on the tracking list. so "swap never started" is technically accurate and I have no way of knowing if it will start in 2.1 seconds or maybe it wont ever start...

this uncertainity is the result of needing the other party to be following the protocol and if they dont, it eventually times out. So we can say that if after 2 minutes it is still in the "swap never started" that it wont ever start.

@lukechilds
Copy link
Contributor Author

Maybe it would be more accurate to keep the swap in the pending object until it's fully started.

It seems like we need more statuses, it's kind of confusing because recentswaps gives us either pending or swap. Insinuating that when it's moved out of pending it's no longer pending.

However when we query swapstatus(requestid, quoteid) (which we can only do when it's no long er pending) it says the status is finished. Then back to pending. Then back to finished again.

So it seems like two different commands have two different ideas of what pending means.

Could I propose some more statuses to give us a better idea of what's happening?

A trade should always have one of the following statuses:

matching
swapping
finished
failed

The only possibilities should be:

  • failed
  • matching -> failed
  • matching -> swapping -> failed
  • matching -> swapping -> finished

matching is as soon as an order is placed (waiting for a match).

swapping is the duration of the swap.

We can use bobpayment,alicepayment,bobdeposit,myfee flags to display more accurate progress in the UI while it's in swapping state.

finished is when the trade has fully completed.

failed is when a swap has failed for any reason.

Preferably also combined with a helpful error message e.g: "Unable to match", "Other party cancelled" etc.

@jl777
Copy link
Owner

jl777 commented Mar 2, 2018

I will try to do this, but cant guarantee there wont be some edge cases, especially if you are finding gaps of one second.

DeckerSU pushed a commit to DeckerSU/SuperNET that referenced this issue Nov 5, 2022
* Exclude mm2-seednodes-optimisation from CI triggers.

* Count the number of sent messages and their total size in seednode loop.

* Add max_msg_size to seednode_loop metrics.

* Log send buffer size when electrum client is connected.

* Log send buffer size for seednode listener and every connected client TcpStream.

* Log nodelay value of TcpListener and every TcpStream.

* Try to set nodelay = true.

* Try to set_send_buffer_size to 200000.

* Try to refactor seednode_loop to async_std.

* Recv messages only and log them.

* Drop connection if EOF reached.

* Enable rebroadcast back.

* Log message send attempts.

* Try to increase channel capacity.

* Use vec of mpsc senders instead of async_std mpmc.

* Remove excessive logging.

* Log the invalid JSON message and it's sender.

* Skip lines that are not valid JSON.

* Log JSON deserialize error only when buffer.len() > 1.

* Try tokio TCP implementation instead of async_std.

* Log the case when buffer can be probably lost.

* Split incoming connections TcpStreams and process them in separate async loops.

* Do not send message back to peer from which it was received.

* Remove excessive logging.

* Enable CI build for this branch back.

* Try to spawn read and write processing loops on tokio 0.2 runtime.

* Use String as P2PMessage content to avoid non-deterministic serialization issues.
The deserialized and then serialized message might produce non-equal string so
same message can be serialized to JSON differently causing double processing on
client nodes connected to several seeds.

* Explicitly disable debug_assertions for release profile.

* Update tokio to 0.2.22.

* Fix after merge.
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