lukechilds / hyperdex-bugtracker

0 stars 1 forks source link

Kickstart stuck swap #1

Closed siulynot closed 6 years ago

siulynot commented 6 years ago

A way to kickstart stuck swaps is needed... maybe it would be nice to have a button on each swap to unstuck it.

Also an automatic kickstart would be nice to have at app restart so that any pending stuck swaps get credited without any action from the user.

lukechilds commented 6 years ago

Also an automatic kickstart would be nice to have at app restart so that any pending stuck swaps get credited without any action from the user.

Do you know if marketmaker will do this automatically on restart?

lukechilds commented 6 years ago

I spoke to @jl777 and he seemed to imply it would by reading the filesystem DB, but I don't think I've seen that happen in practice.

siulynot commented 6 years ago

marketmaker needs to have the .finished file deleted so it can trigger a swapstatus.

jl777 commented 6 years ago

once a .finished file is created, it is skipped by marketmaker. usually this isnt a problem, but if you want to reprocess a specific swap the way to do it is to delete the .finished file and call swapstatus for the requestid/quoteid

lukechilds commented 6 years ago

@jl777 Is there an api call to do this?

Like kickstart(uuid) or similar? We don't want to rely on editing the on disk DB, all communication should happen over JSON-RPC.

jl777 commented 6 years ago

can you pass in requestid/quoteid ?

jl777 commented 6 years ago

pushed kickstart(requestid,quoteid) api to dev branch

sindresorhus commented 6 years ago

I think we should stick to always using the UUID, so what @lukechilds suggested, kickstart(uuid), would be better.

jl777 commented 6 years ago

internally, that would require marketmaker to scan all files which are stored by requestid/quoteid, so when you say it would be better it seems you are ignoring some critical implementation issues.

sindresorhus commented 6 years ago

I don't know how marketmaker works internally. My comment is purely from a user perspective.

sindresorhus commented 6 years ago

that would require marketmaker to scan all files which are stored by requestid/quoteid

Would it be possible to change it to store by UUID?

jl777 commented 6 years ago

I dont have any internal structure mapping uuid to requestid/quoteid. My understanding is the GUI already had this. what is so hard about sending in the requestid/quoteid?

jl777 commented 6 years ago

the user can specify based on uuid, gui maps from uuid to requestid/quoteid (if any)

lukechilds commented 6 years ago

We don't have requestid/quoteid exposed on our swap objects because we aren't using it for anything. We do have it stored in the DB though for debugging purposes, if implementing kickstart by uuid is going to cause complications on the marketmaker end then don't worry about it, we can refactor to use kickstart(requestid, quoteid).

We can aim to move all swap related API methods over to uuid for v2.

jl777 commented 6 years ago

thanks. there just isnt any internal infrastructure for uuid, which was a very late addition

siulynot commented 6 years ago

But this issue cant wait to be solved until mmV2 is ready. If the app is going public, it really needs a way to kickstart a stuck swap. If it doesnt, there will be a torrent of support tickets related to this.

By deleting the .finished file or rescanind the DB/SWAPS directory to look for swaps without .finished files is enough to get it going.

jl777 commented 6 years ago

that is already implemented with the kickstart(requestid,quoteid) api I added today

lukechilds commented 6 years ago

@jl777 what is the expected functionality of kickstart(requestid, quoteid)?

I started a trade, stopped mm after dexfee. Then restarted it and ran kickstart(requestid, quoteid). It returned the swap JSON but the swap never progresses.

Output of swapstatus(requestid, quoteid):

{
  "uuid": "34b8a4e43e5d53c344f069db25e122859219846eb2190853f3b5eafb1e28b892",
  "expiration": 1526562504,
  "tradeid": 2638541493,
  "requestid": 2891704030,
  "quoteid": 3434410450,
  "iambob": 0,
  "Bgui": "",
  "Agui": "hyperdex",
  "gui": "hyperdex",
  "bob": "CHIPS",
  "srcamount": 0.83012373,
  "bobtxfee": 0.0001,
  "alice": "KMD",
  "destamount": 0.10009,
  "alicetxfee": 0.00001,
  "aliceid": "14211404868928798721",
  "sentflags": [
    "myfee"
  ],
  "values": [
    0,
    0,
    0,
    0,
    0,
    0,
    0.00012881,
    0,
    0,
    0,
    0
  ],
  "result": "success",
  "status": "pending",
  "bobdeposit": "0000000000000000000000000000000000000000000000000000000000000000",
  "alicepayment": "0000000000000000000000000000000000000000000000000000000000000000",
  "bobpayment": "0000000000000000000000000000000000000000000000000000000000000000",
  "paymentspent": "0000000000000000000000000000000000000000000000000000000000000000",
  "Apaymentspent": "0000000000000000000000000000000000000000000000000000000000000000",
  "depositspent": "0000000000000000000000000000000000000000000000000000000000000000"
}
lukechilds commented 6 years ago

Also tried with a swap that was further on (after I sent alicepayment), but kickstarting still doesn't seem to do anything.

swapstatus output after kickstarting:

{
  "uuid": "daf5309d3b577a83af2925eb1f66dd37326a33af03f19bfbc668262513c7de79",
  "expiration": 1526564585,
  "tradeid": 799379735,
  "requestid": 4181843802,
  "quoteid": 690354882,
  "iambob": 0,
  "Bgui": "",
  "Agui": "hyperdex",
  "gui": "hyperdex",
  "bob": "CHIPS",
  "srcamount": 0.74955267,
  "bobtxfee": 0.0001,
  "alice": "KMD",
  "destamount": 0.10009,
  "alicetxfee": 0.00001,
  "aliceid": "14211589809510612993",
  "sentflags": [
    "alicepayment",
    "bobdeposit",
    "myfee"
  ],
  "values": [
    0,
    0,
    0,
    0.10011,
    0.84344675,
    0,
    0.00012881,
    0,
    0,
    0,
    0
  ],
  "result": "success",
  "status": "pending",
  "bobdeposit": "da9b1c1c1e90a09b92fcf92ab8b737311252659712936286aa0081bb5411d5a9",
  "alicepayment": "d1496eff630fa145e571f2369484a26168051630b4fb0788e56be726a69706a3",
  "bobpayment": "0000000000000000000000000000000000000000000000000000000000000000",
  "paymentspent": "0000000000000000000000000000000000000000000000000000000000000000",
  "Apaymentspent": "0000000000000000000000000000000000000000000000000000000000000000",
  "depositspent": "0000000000000000000000000000000000000000000000000000000000000000"
}
jl777 commented 6 years ago

kickstart can only make progress if progress is possible. likely you need to wait the 4hrs and unless the .finished file was created in error, kickstart wont be needed

lukechilds commented 6 years ago

I see, so it's not expected that the trade should just pickup where it left off?

So the issue is how do we handle users who close the app half way through the trade? When they reopen the app we will display the trade as partially complete because we don't have any more information. I was hoping kickstarting would resume the trade, but if that's not possible, is there any way we can more info from the response of the kickstart command?

So we know if the swap will resume or will be rewound or has errored or if the funds are lost?

jl777 commented 6 years ago

swapstatus shows what the status is. there are many possible states it can be in... mm should not be killed in the middle of a swap. if you do, it is likely you end up not being able to complete the swap. however, as long as the DB/SWAP files are there, it should unwind any trade that is started so you get either the alicereclaim or the aliceclaim to recover the alicepayment or bobdeposit

as long as it doesnt have a status:finished, it is not finished even if it does have a status:finished (i think it is possible in some cases of getting SPV errors to get it prematurely) it can be kickstarted. which just deletes the .finished file and does a swapstatus

from the alice side, if alicepayment is not sent, then nothing is really at risk. the dexfee is paid regardless. if alicepayment is set, then after 4 hours it can be reclaimed or better yet the bobdeposit will be claimed if it is still there

lukechilds commented 6 years ago

mm should not be killed in the middle of a swap. if you do, it is likely you end up not being able to complete the swap.

Yeah, I understand it shouldn't be intentionally killed, I was just doing it to simulate network loss/power cut/system crash and see how we can recover.

however, as long as the DB/SWAP files are there, it should unwind any trade that is started so you get either the alicereclaim or the aliceclaim to recover the alicepayment or bobdeposit

This is good, but we need a way to signal to the user if the funds are lost or if the trade is going to be rewound after a certain amount of time or is currently being rewound or has successfully been rewound. Currently we just show them the trade is 3/5 completed and have no way to show them anything else. Is there any way we con find out any of this data? Will we get any socket updates when a rewind tx happens?

if alicepayment is set, then after 4 hours it can be reclaimed or better yet the bobdeposit will be claimed if it is still there

And we don't need to do anything manually to enable this? mm will handle it on it's own?

What if mm was down for 24 hours or more. Will it still automatically reclaim the funds or are they lost?

jl777 commented 6 years ago

mm will process all unfinished swaps and advance it to the next step, if possible this happens automatically when mm is running

all mm knows is the swapstatus, so whatever status to display to the user would be based on the swapstatus

lukechilds commented 6 years ago

Ok, will we get any socket updates when a rewind tx happens?

Also, is there any danger in doing a kickstart during or after a successful swap?

Just wondering if we add a kickstart button if we need to signal to the users it should only be used in emergencies and if there could be any negative side affects.

jl777 commented 6 years ago

all it does is remove the .finished file and do a swapstatus call if the swap was already finished, then calling it again wont do any harm

doing it in the middle of a swap should just change the timing of when it is called and should be harmless

lukechilds commented 6 years ago

Thanks.

And re the socket updates, will we get any for rewind txs so we can update the UI on a successful rewind?

jl777 commented 6 years ago

what do you mean by "rewind tx"? what is happening is the .finished file is deleted and swapstatus is called, which in turn will generate whatever events the swapstatus will trigger

lukechilds commented 6 years ago

what do you mean by "rewind tx"?

Currently we listen on the socket for updates with the swap uuid so we can update the status in the UI. We get a socket message for each transatction e.g myfee, bobdeposit, alicepayment, bobpayment, alicespend.

as long as the DB/SWAP files are there, it should unwind any trade that is started so you get either the alicereclaim or the aliceclaim to recover the alicepayment or bobdeposit

I'm asking if the swap fails and it unwinds, will we receive socket updates for the recovery transactions so we can dispaly in the UI that the swap has successfully been reverted.

jl777 commented 6 years ago

alicereclaim and aliceclaim are using the same code that all the other transactions are and should generate the corresponding event

sindresorhus commented 6 years ago

Moved to https://github.com/lukechilds/hyperdex/issues/283

Let's continue the discussion there.