Fix recoverer and tx broadcasting race

[danielgranhao]

Addresses #647

This PR:

• Changes the order in which we fetch the onchain wallet tx map and the swaps histories in the recoverer implementation. There could be a race condition where a refund tx (for a send swap) was broadcasted in between the 2 steps, causing the swap history to include the refund tx, but for it to be missing in the onchain wallet tx map. This would then cause us to think the refund tx was a claim tx because we assume a claim tx is the one that does not spend to or from our wallet.
• Adds a log for when we cannot get the swap preimage from the claim tx. We were simply ignoring this error, making it hard to understand that the preimage was not being found.

Open questions:

• Should we use the preimage as a final check that a tx is indeed a claim tx? If we fail to get the preimage, we could clear the claim tx id from the swap.

[roeierez]

@danielgranhao while this mitigate some of the cases I can still see races. For example:

1. The recoverer starts and sync wallet and fetches scripts history
2. Then the status stream triggers the swap claim/refund and update the swap
3. The recoverer update again the swap with stale data.

This PR though makes it less likely to happen.
In my mind we need to think of ways to prevent this stale update, one option I was thinking of which would interested to get your opinion is:

1. Recoverer fetch the swaps to recover from the db (already does it)
2. At the end only updates swaps that haven't been updated since the fetch.

The next recoverer process should be good since we already broadcasted the claim/refund transaction.

[roeierez]

• Should we use the preimage as a final check that a tx is indeed a claim tx? If we fail to get the preimage, we could clear the claim tx id from the swap.

Yes, good catch. I think this is needed. the claim tx can be persisted in the db only with the preimage.

[danielgranhao]

In my mind we need to think of ways to prevent this stale update, one option I was thinking of which would interested to get your opinion is:

Recoverer fetch the swaps to recover from the db (already does it)

• At the end only updates swaps that haven't been updated since the fetch.
• The next recoverer process should be good since we already broadcasted the claim/refund transaction.

Yes, I agree this PR doesn't entirely fix the underlying issue. I think an optimistic approach like the one you propose could work well. I'll work on it.

[danielgranhao]

Dropped in favor of #652 and #653