Automatically fix stuck swaps #416
Conversation
Even if the 4 hour timelock hasn't expired, some of them might still be claimable if we already know `bobpayment`
There are edge cases in marketmaker where the amounts aren't always set. sentflags appears to be more reliable.
The marketmaker supplied values `srcamount` and `destamount` are incorrect if the swap ends with any tx other than alicespend
marketmaker sends duplicate messages for some txs.
This reverts commit f7cacaa.
The reason for the long delay was that the kickstart command is quite expensive and blocks marketmaker. However we only run this on swaps that have been swapping for longer than four hours, so it shouldn't actually match swaps fire the kickstart command very often.
Even if the atomic swap protocol completes successfully, the swap should be classed as failed if the user didin't end up with the funds they wanted.
|
Also, I'm aware the This function should probably be refactored into it's own file with more readable methods at some point. |
Can you add a |
app/renderer/containers/Exchange.js
Outdated
| }); | ||
|
|
||
| const FIFTEEN_MINUTES = 1000 * 60 * 15; | ||
| fireEvery(async () => { |
There was a problem hiding this comment.
fireEvery({minutes: 15}, async () => {
app/renderer/swap-db.js
Outdated
| swap.transactions.push({ | ||
| stage: 'myfee', | ||
| coin: message.alice, | ||
| txid: undefined, // This is currently mising from marketmaker responses |
There was a problem hiding this comment.
Can you link to a marketmaker issue?
There was a problem hiding this comment.
There is no GH issue, but I mentioned it to James and he said he'd fixed it. Will test and create an issue if it isn't resolved.
| // There is a bug in marketmaker where it doesn't always correctly report | ||
| // the values. If aliceclaim is 0 we fallback to bobdeposit as it should | ||
| // be very close (same amount minus our claim txfee) | ||
| // https://github.com/jl777/SuperNET/issues/920 |
There was a problem hiding this comment.
This seems to be fixed now? jl777/SuperNET#920 (comment)
There was a problem hiding this comment.
Yep, should be, yet to check that.
There was a problem hiding this comment.
Doesn't appear to be resolved jl777/SuperNET#920 (comment)
Missing value bug now fixed in marketmaker
This reverts commit 7518fc4.
|
Oh yeah! Feels good 🙌
|
Resolves #163
This PR resolves the issue of users getting swaps that are stuck mid swap (e.g in
swap 3/5state).If a swap fails due to connectivity issues or closing the app the swap would become "stuck". Although the funds were cryptographically safe and reclaimable via the atomic swap protocol,
marketmakerwouldn't actually reclaim the funds automatically without custom intervention via the console or CLI.We are now manually looping over old swaps every 15 mins that are in a stuck state and "kickstarting" them to get
marketmakerto recheck the status.There are quite a lot of changes required to the current code base to get this working. I will try and break down the changes here and explain why they are necessary.
Get updates on stuck swaps 1e7ceee 5eaedd9
We were previously listening for updates over the socket for a swap by adding a new event listener for each swap that listens out for messages containing it's UUID. This was not a great approach and resulted in a few crazy hacks to avoid memory leaks whilst also correctly handling quirky
marketmakerbehaviour where a swap would be reported as failed and the event listener was removed, only to later send us a completion message.Now we only attach a single event listener and check all messages against a list of all known swap UUIDs. This means we can guarantee we will always catch updates to swaps, if they were started in a previous session and even if they are received in a strange order, e.g failed then complete. It's also much simpler code and avoids risks of memory leaks having a large amount of event listeners.
Force kickstart stuck swaps 2e89499 6514951
This is supposed to be handled automatically by
marketmaker, but I've never personally seen it actually work. What we now do is loops over all swaps that are currently swapping and were started over 4 hours ago, then callmarketmaker'skickstartmethod. This forces it to check the current state of the swap and if something has gone wrong or Bob hasn't responded it will attempt to reclaim the funds from the timelocked address. This is why we only check if the swap is over 4 hours old, because the timelocks last 4 hours. We do this instantly on startup and then check again every 15 minutes.On the first startup check, we check all pending swaps, even if they weren't started over 4 hours ago, this is because there are some scenarios where a swap can be reverted sooner.
This means you don't need to keep HyperDEX (and
marketmakerrunning). You can close the app, open it the next day, and it will instantly reclaim your stuck swap if possible.Recalculate the state of a completed swap fc1caab 45b5b49
Due to the fact that HyperDEX (and
marketmaker) might not be running for the entire swap, we can't rely on the WebSocket message history to provide an accurate overview of what happened during the swap. The completed message we get at the end of every normal swap and "kickstarted" stuck swaps contains all the information we need to rebuild the full history of what happened.So we now have two ways of building a formatted swap object. The socket messages are used while a swap is in progress. Once a swap is completed, the final completed socket message is used to reconstruct the entire swap transaction history and state which we know contains all data. The resulting structure of the swap object follows the same pattern in both methods so all existing UI code works no matter how the swap data was calculated.
This means if a swap is completed it should never have missing tx data, it should always list the entire chain of TXs, even if it wasn't online the whole time. It also means we correctly handle weird states like when the swap doesn't go to plan and we reclaim our original payment or claim
bobdepositinstead ofbobpayment. It also abstracts out quite a few quirks inmarketmakerresponses.Showing kickstarted swaps TXs in the swap modal 1a95811 0e55952
Kickstarted swaps don't always have the same transactions exposed as our current code expects which breaks the modal layout. I've modified the transactions array to not allow duplicate transactions (sometimes
marketmakersends multiple messages for the same TX which we were pushing into the transactions array). This means while the swap is in progress we can rely on just looping over the transaction array for the current data are less the 5 items fill them in with the placeholders.Once a swap is completed however, there may not be 5 transactions due to the reversion and we don't want to show the placeholder boxes as those transactions are no longer expected to occur. However the reversion transactions are handled by the completed message and added to the transactions array, so just looping over that means they will show up, we just skip showing the placeholder boxes.
There's also a few other bits handling weird mm responses and adding extra data for stuck swaps, but they're fairly self explanatory.
To show how successfully resumed swaps look in the UI:
A swap that was stuck, but then resumed in time to proceed as normal
By either a reboot, losing network connectivity, accidentally closing the app etc.
This swap appears just like a normal swap that completed without issue:
Notice the final TX is
alicespend, the expected TX for a successful swapA swap that was stuck due to Bob not sending
bobpaymentIf Bob doesn't send
bobpaymentyou need to wait four hours for the timelock onbobdepositto expire. The we can claimbobdepositwith analiceclaimTX.bobdepostis 12.5% more than the amount we were supposed to get. We get to keep this to punish Bob for being a dick. (In a normal trade Bob would reclaim his ownbobdepositafter we spendbobpaymentwithalicespend).You don't actually need to keep the app running for four hours though, you could go to bed and run the app in the morning and it will reclaim everything.
This swap appears just like a normal swap in the swap list. However in the modal it will show only four TXs in the TX chain and you'll notice that it shows a much higher percentage message than normal due to our 12.5% bonus:
Notice the final TX is
aliceclaimand is higher than what we asked forA swap that was stuck due to Bob not sending any payments
If Bob doesn't send any payments (
bobdepositorbobpayment) we can't really do much about it. We can reclaim ouralicepaymentfunds but we lose ourdexfeepayment.Although
marketmakerreports the status as successfully completed (which kind of makes sense in terms of the atomic swap protocol), I'm mapping this to a failed swap in our logic because if the user doesn't get their desired funds, it's failed from their perspective.So it will show in the swap list as reverted with the same styles as a failed/unmatched swap. The received value will be set as 0 and you will see the reclaim TX as the final TX in the chain. There is also a relatively vague message explaining what went wrong. (We really don't know what went wrong, it could have been many things):
Notice the final TX is
alicereclaimwhich just claims back our original fundsAs far as I'm aware, this correctly handles all stuck states and displays them correctly in the UI. You are guaranteed to either:
dexfee)