From af6a1f3d01447b90d2e9aef23e5ab3c67c276277 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Tue, 12 Mar 2024 14:20:52 +0000 Subject: [PATCH] swaps: use longer final_cltv_delta for client-normal-swap This gives more time for the client to come back online. see https://github.com/spesmilo/electrum/issues/8940 - re note on submarine_swaps.py#L53: lnpeer.Peer.maybe_fulfill_htlc only checks against MIN_FINAL_CLTV_DELTA_ACCEPTED(=144), so this increased cltv_delta is not enforced when receiving the htlc on ln. It is put in the invoice, so the sender is supposed to honour it ofc. It would be nice to enforce it (make the check in maybe_fulfill_htlc dependent on what was in the invoice). --- electrum/lnpeer.py | 1 + electrum/lnworker.py | 7 +++++-- electrum/submarine_swaps.py | 12 ++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py index bd94eedfa6d5..3f8c35242d92 100644 --- a/electrum/lnpeer.py +++ b/electrum/lnpeer.py @@ -2119,6 +2119,7 @@ def log_fail_reason(reason: str): else: raise Exception(f"unexpected {mpp_resolution=}") + # TODO check against actual min_final_cltv_expiry_delta from invoice (and give 2-3 blocks of leeway?) if local_height + MIN_FINAL_CLTV_DELTA_ACCEPTED > htlc.cltv_abs: if not already_forwarded: log_fail_reason(f"htlc.cltv_abs is unreasonably close") diff --git a/electrum/lnworker.py b/electrum/lnworker.py index f79560043dd7..1d799de38432 100644 --- a/electrum/lnworker.py +++ b/electrum/lnworker.py @@ -2146,9 +2146,10 @@ def get_bolt11_invoice( payment_hash: bytes, amount_msat: Optional[int], message: str, - expiry: int, + expiry: int, # expiration of invoice (in seconds, relative) fallback_address: Optional[str], channels: Optional[Sequence[Channel]] = None, + min_final_cltv_expiry_delta: Optional[int] = None, ) -> Tuple[LnAddr, str]: assert isinstance(payment_hash, bytes), f"expected bytes, but got {type(payment_hash)}" @@ -2169,12 +2170,14 @@ def get_bolt11_invoice( amount_btc = amount_msat/Decimal(COIN*1000) if amount_msat else None if expiry == 0: expiry = LN_EXPIRY_NEVER + if min_final_cltv_expiry_delta is None: + min_final_cltv_expiry_delta = MIN_FINAL_CLTV_DELTA_FOR_INVOICE lnaddr = LnAddr( paymenthash=payment_hash, amount=amount_btc, tags=[ ('d', message), - ('c', MIN_FINAL_CLTV_DELTA_FOR_INVOICE), + ('c', min_final_cltv_expiry_delta), ('x', expiry), ('9', invoice_features), ('f', fallback_address), diff --git a/electrum/submarine_swaps.py b/electrum/submarine_swaps.py index 84ecd7cf867c..6b5fb5480f3d 100644 --- a/electrum/submarine_swaps.py +++ b/electrum/submarine_swaps.py @@ -50,9 +50,11 @@ MIN_LOCKTIME_DELTA = 60 LOCKTIME_DELTA_REFUND = 70 MAX_LOCKTIME_DELTA = 100 +MIN_FINAL_CLTV_DELTA_FOR_CLIENT = 3 * 144 # note: put in invoice, but is not enforced by receiver in lnpeer.py assert MIN_LOCKTIME_DELTA <= LOCKTIME_DELTA_REFUND <= MAX_LOCKTIME_DELTA assert MAX_LOCKTIME_DELTA < lnutil.MIN_FINAL_CLTV_DELTA_ACCEPTED assert MAX_LOCKTIME_DELTA < lnutil.MIN_FINAL_CLTV_DELTA_FOR_INVOICE +assert MAX_LOCKTIME_DELTA < MIN_FINAL_CLTV_DELTA_FOR_CLIENT # The script of the reverse swaps has one extra check in it to verify @@ -443,6 +445,7 @@ def add_normal_swap( our_privkey: bytes, prepay: bool, channels: Optional[Sequence['Channel']] = None, + min_final_cltv_expiry_delta: Optional[int] = None, ) -> Tuple[SwapData, str, str]: """creates a hold invoice""" if prepay: @@ -458,6 +461,7 @@ def add_normal_swap( expiry=300, fallback_address=None, channels=channels, + min_final_cltv_expiry_delta=min_final_cltv_expiry_delta, ) # add payment info to lnworker self.lnworker.add_payment_info_for_hold_invoice(payment_hash, invoice_amount_sat) @@ -471,6 +475,7 @@ def add_normal_swap( expiry=300, fallback_address=None, channels=channels, + min_final_cltv_expiry_delta=min_final_cltv_expiry_delta, ) self.lnworker.bundle_payments([payment_hash, prepay_hash]) self.prepayments[prepay_hash] = payment_hash @@ -666,6 +671,13 @@ async def request_normal_swap( our_privkey=refund_privkey, prepay=False, channels=channels, + # When the client is doing a normal swap, we create a ln-invoice with larger than usual final_cltv_delta. + # If the user goes offline after broadcasting the funding tx (but before it is mined and + # the server claims it), they need to come back online before the held ln-htlc expires (see #8940). + # If the held ln-htlc expires, and the funding tx got confirmed, the server will have claimed the onchain + # funds, and the ln-htlc will be timed out onchain (and channel force-closed). i.e. the user loses the swap + # amount. Increasing the final_cltv_delta the user puts in the invoice extends this critical window. + min_final_cltv_expiry_delta=MIN_FINAL_CLTV_DELTA_FOR_CLIENT, ) return swap, invoice