ElementsProject / peerswap

MIT License
109 stars 40 forks source link

fees get paid even opening tx for swap fails #324

Open warioishere opened 1 week ago

warioishere commented 1 week ago

I just created a swap with strike and got an error message from their side:

2024/10/28 20:25:21 [INFO] [Swap:8b981107d39e60899a93ee06b99ad32a327a38e557949a892bc15860c8d95bae] Start new swap-out: peer: 03c8e5f583585cac1de2b7503a6ccd3c12ba477cfd139cd4905be504c2f48e86bd chanId: 836614:>
2024/10/28 20:25:24 [INFO] [Swap:8b981107d39e60899a93ee06b99ad32a327a38e557949a892bc15860c8d95bae] Paid Feeinvoice of 300 sats
2024/10/28 20:25:24 [INFO] [Swap:8b981107d39e60899a93ee06b99ad32a327a38e557949a892bc15860c8d95bae] Swap canceled. Reason: -13:Error: Please enter the wallet passphrase with walletpassphrase first.
2024/10/28 20:25:24 [INFO] [Swap:8b981107d39e60899a93ee06b99ad32a327a38e557949a892bc15860c8d95bae] Warning: Paid swap-out prepayment, but swap canceled before receiving opening transaction

this already happend in the past but I didnt realize it that time, now I see theres defintiv a bug, or a missbevavior as ps shouldnt pay fees if the swap might fail.

PS probably needs a pre-check to validate that the swap would work.

maybe @Impa10r and @zapomatic can explain it better, because they also experience this bug already.

zapomatic commented 1 week ago

confirmed, same error from Zap-O-Matic initiating a LN -> LBTC swap with Strike. It looks like their LBTC wallet is locked, but we only know about it after sending the 300 sats. Would expect either the pre-payment is conducted after an initial check with the node to ensure their wallet is ready, or that the fee is bounced back after this particular kind of failure in which the fee is not spent.

YusukeShimizu commented 1 week ago

Thank you for reporting the issue.
Based on your suggestion, I think it is appropriate to check in advance when receiving a swap out request.

A GetOnchainBalance check is performed in the State_SwapOutReceiver_CreateSwap state.
https://github.com/ElementsProject/peerswap/blob/b7070cac0bec85a308df8da46b0aead57c7d95d4/swap/actions.go#L382

However, since this check alone may not be sufficient in some cases, I am considering updating the process as follows:

  1. Opening tx Construction and Signing When a swap-out request is received, receiver should first construct and sign an on-chain transaction.
    If the wallet is locked during this step, signing will fail, allowing us to detect issues at that point. This will help prevent problems caused by wallet lock states like those you reported.
    Reference link: signmessage RPC

  2. testmempoolaccept Additionally, to verify whether the transaction can be broadcasted successfully, we will use testmempoolaccept. If any issues arise during this step, an error will be returned at that point, preventing inaccurate swap processing.

I thik these improvements will reduce the risk.

Points to Note

However, even with this approach, it does not completely prevent cases where only prepayment is received and then a swap cancellation occurs afterward. For such abuse cases, there is already a mechanism within the system that records as "suspicious peer."