fix(swaps): no remote fail when sending payment

[sangaman]

This prevents the maker from failing a swap due to a swap failed packet received from the taker if the maker has already started sending its payment. This is because the taker may still claim the payment, and the maker needs to continue monitoring its outgoing payment until it is certain that the payment cannot be claimed.

Fixes #1749.

[ghost]

The changes look good @sangaman. As a follow-up what do you think about making the this.failDeal explicit and by default not failing the deal? At the moment it's the opposite.

[sangaman]

The changes look good @sangaman. As a follow-up what do you think about making the this.failDeal explicit and by default not failing the deal? At the moment it's the opposite.

I'll rearrange the logic in a separate PR so as to not hold this one up.

[kilrau]

Created that follow-up issue.

[ghost]

Been restarting the builds for several hours now and they seem to be failing with random errors.

[sangaman]

Been restarting the builds for several hours now and they seem to be failing with random errors.

Same for me... I'm going to dig into this to see if this change somehow breaks the security tests. Getting context deadline exceeded there (sometimes) so it could be hanging waiting for a swap to fail that doesn't fail due to this change perhaps?

[sangaman]

I'm testing a fix for the simulation tests. The tests were expecting the maker to fail due to SwapTimedOut, which is actually not desirable and what this PR is intended to fix.

[sangaman]

I believe I straightened out the simulation tests. However, it appears that I had to revert the security sim test changes from c9be219. The maker should not be canceling his incoming htlc until his outgoing htlc has expired, that's what this PR is fixing.

[kilrau]

I believe I straightened out the simulation tests. However, it appears that I had to revert the security sim test changes from c9be219. The maker should not be canceling his incoming htlc until his outgoing htlc has expired, that's what this PR is fixing.

That sounds quite right. Can you give check the simtest changes once more? @erkarl

[raladev]

There is still no monitoring continuation for maker side after maker's xud restart.

Steps:

1. place 9 sell ETH/BTC orders as a maker, fill them all by 1 order of a taker -> most of swaps failed (29/07/2020 15:33:58.510 in maker's log)
2. monitoring messages in maker logs each 5 mins (29/07/2020 15:47:33.970 in maker's log)
3. restart maker's xud (29/07/2020 16:01:13.444 in maker's log)
4. wait -> No monitor messages
5. shutdown both envs -> swap err messages in the taker's xud log after disconnecting from all peers. (29/07/2020 16:17:41.217 in taker's log)

Screenshot from 2020-07-29 19-36-15
Screenshot from 2020-07-29 19-36-27

monitor_xud_taker.log
monitor_xud_maker.log

maker's xud db - https://gofile.io/d/7GKTmK
taker's xud db - https://gofile.io/d/uqMD6s

[ghost]

Also needs a rebase.

[sangaman]

There is still no monitoring continuation for maker side after maker's xud restart.

This is very helpful but after some investigation I'm pretty sure this is a separate issue from this PR and the issue it indends to fix. It's due to the fact that a connext sendpayment call can "fail" while the payment is still in limbo. It's actually somewhat related to #1794, the solution is to not fail the deal when a sendpayment calls, and to have a mechanism within swaps to monitor swaps that are still in an active status even after they've timed out and even after our attempts to send payment have failed, up until we get confirmation that all HTLCs are canceled/expired. Everything in SwapRecovery will then only be used for when xud crashes or is restarted while there are active swaps.