swaps: add explicit check that (onchain_locktime < LN_locktime)
What changed, and why it matters
This commit adds an extra safety check in Electrum's submarine-swap feature to make sure the on-chain refund deadline is earlier than the Lightning Network payment deadline. The commit message says this was already implicitly guaranteed by other checks, and the code change just makes it explicit. It also adds clarifying comments about Lightning invoice expiry handling and payment bundles. There is no direct evidence that a real vulnerability was exploitable before this change.
Treat as a defensive hardening patch. Reviewers should verify that the new explicit check correctly captures the intended safety invariant and that MIN_LOCKTIME_DELTA provides adequate mining margin. No urgent response appears necessary based on the supplied materials.
Security signals we found
Explicit CLTV safety check added for submarine swaps
Commit message describes the check as already implicitly enforced
Clarifying comments added around invoice expiry and payment bundle behavior
No CVE, advisory, or researcher attribution present in commit materials
Evidence from the diff
In electrum/submarine_swaps.py, add_normal_swap() now explicitly verifies that locktime + MIN_LOCKTIME_DELTA is less than current_height + the invoice’s min_final_cltv_delta before creating a submarine swap. The commit message explains that for client-forward-swaps the client already enforced onchain locktime delta < 100 while choosing LN locktime delta = 432, and for client-reverse-swaps the server already used onchain locktime delta = 70 with default LN locktime delta = 147, so the onchain_locktime < LN_locktime relationship was already implicitly satisfied. The change also captures the lnaddr objects and asserts both the main invoice and prepay invoice share the same min_final_cltv_delta. Two comment-only changes in lnpeer.py clarify receiver-side invoice expiry and payment bundle splitting behavior.
Changed components
electrum/submarine_swaps.pyelectrum/lnpeer.pyInspect captured patch +17 / −5
diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py
index 90ad535..bad460b 100644
--- a/electrum/lnpeer.py
+++ b/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()}")
diff --git a/electrum/submarine_swaps.py b/electrum/submarine_swaps.py
index f0e84c5..bf006ac 100644
--- a/electrum/submarine_swaps.py
+++ b/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.
Why this scored 29/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.