swaps: add explicit check that (onchain_locktime < LN_locktime)

This was already implicitly checked. This diff makes the check explicit, and serves as an additional sanity-check.
- for client-forward-swaps, we have
  - "cltv safety requirement: (onchain_locktime < LN_locktime),   otherwise client is vulnerable"
  - server chooses onchain locktime delta = 70
    71255c1e73/electrum/submarine_swaps.py (L701)
  - client checks that onchain locktime delta is <100
    71255c1e73/electrum/submarine_swaps.py (L887)
  - client chooses LN locktime delta = 432
    71255c1e73/electrum/submarine_swaps.py (L907)
- for client-reverse-swaps, we have
  - "cltv safety requirement: (onchain_locktime < LN_locktime),   otherwise server is vulnerable"
  - server chooses onchain locktime delta = 70
    71255c1e73/electrum/submarine_swaps.py (L598)
  - server chooses LN locktime delta: unset, i.e. our default of 147
    71255c1e73/electrum/submarine_swaps.py (L612)
    71255c1e73/electrum/lnworker.py (L2273)

electrum/lnpeer.py

@@ -2545,6 +2545,8 @@ class Peer(Logger, EventListener):
             return None, None
 
         # TODO check against actual min_final_cltv_expiry_delta from invoice (and give 2-3 blocks of leeway?)
+        # note: payment_bundles might get split here, e.g. one payment is "already forwarded" and the other is not.
+        #       In practice, for the swap prepayment use case, this does not matter.
         if local_height + MIN_FINAL_CLTV_DELTA_ACCEPTED > htlc.cltv_abs and not already_forwarded:
             log_fail_reason(f"htlc.cltv_abs is unreasonably close")
             raise exc_incorrect_or_unknown_pd
@@ -2580,7 +2582,9 @@ class Peer(Logger, EventListener):
                 return None, (payment_key, callback)
 
         # TODO don't accept payments twice for same invoice
-        # TODO check invoice expiry
+        # note: we don't check invoice expiry (bolt11 'x' field) on the receiver-side.
+        #       - semantics are weird: would make sense for simple-payment-receives, but not
+        #         if htlc is expected to be pending for a while, e.g. for a hold-invoice.
         info = self.lnworker.get_payment_info(payment_hash)
         if info is None:
             log_fail_reason(f"no payment_info found for RHASH {htlc.payment_hash.hex()}")

electrum/submarine_swaps.py

@@ -190,7 +190,7 @@ class SwapOffer:
 @attr.s
 class SwapData(StoredObject):
     is_reverse = attr.ib(type=bool)  # for whoever is running code (PoV of client or server)
-    locktime = attr.ib(type=int)
+    locktime = attr.ib(type=int)  # onchain, abs
     onchain_amount = attr.ib(type=int)  # in sats
     lightning_amount = attr.ib(type=int)  # in sats
     redeem_script = attr.ib(type=bytes, converter=hex_to_bytes)
@@ -624,7 +624,7 @@ class SwapManager(Logger):
     def add_normal_swap(
             self, *,
             redeem_script: bytes,
-            locktime: int,  # onchain
+            locktime: int,  # onchain, abs
             onchain_amount_sat: int,
             lightning_amount_sat: int,
             payment_hash: bytes,
@@ -644,7 +644,7 @@ class SwapManager(Logger):
         else:
             invoice_amount_sat = lightning_amount_sat
 
-        _, invoice = self.lnworker.get_bolt11_invoice(
+        lnaddr1, invoice = self.lnworker.get_bolt11_invoice(
             payment_hash=payment_hash,
             amount_msat=invoice_amount_sat * 1000,
             message='Submarine swap',
@@ -653,12 +653,17 @@ class SwapManager(Logger):
             channels=channels,
             min_final_cltv_expiry_delta=min_final_cltv_expiry_delta,
         )
+        margin_to_get_refund_tx_mined = MIN_LOCKTIME_DELTA
+        if not (locktime + margin_to_get_refund_tx_mined < self.network.get_local_height() + lnaddr1.get_min_final_cltv_delta()):
+            raise Exception(
+                f"onchain locktime ({locktime}+{margin_to_get_refund_tx_mined}) "
+                f"too close to LN-htlc-expiry ({self.network.get_local_height()+lnaddr1.get_min_final_cltv_delta()})")
         # add payment info to lnworker
         self.lnworker.add_payment_info_for_hold_invoice(payment_hash, invoice_amount_sat)
 
         if prepay:
             prepay_hash = self.lnworker.create_payment_info(amount_msat=prepay_amount_sat*1000)
-            _, prepay_invoice = self.lnworker.get_bolt11_invoice(
+            lnaddr2, prepay_invoice = self.lnworker.get_bolt11_invoice(
                 payment_hash=prepay_hash,
                 amount_msat=prepay_amount_sat * 1000,
                 message='Submarine swap prepayment',
@@ -669,6 +674,7 @@ class SwapManager(Logger):
             )
             self.lnworker.bundle_payments([payment_hash, prepay_hash])
             self._prepayments[prepay_hash] = payment_hash
+            assert lnaddr1.get_min_final_cltv_delta() == lnaddr2.get_min_final_cltv_delta()
         else:
             prepay_invoice = None
             prepay_hash = None
@@ -1015,6 +1021,8 @@ class SwapManager(Logger):
         - User spends on-chain output, revealing preimage.
         - Server fulfills HTLC using preimage.
 
+        cltv safety requirement: (onchain_locktime < LN_locktime),   otherwise server is vulnerable
+
         Note: expected_onchain_amount_sat is BEFORE deducting the on-chain claim tx fee.
         Note: prepayment_sat is passed as argument instead of accessing self.mining_fee to ensure
         the mining fees the user sees in the GUI are also the values used for the checks performed here.