lnwatcher: catch exc during ctx output sweeping
What the developer wrote
Authored by f321x
lnwatcher: catch exc during ctx output sweeping
Catch exceptions occuring during the sweep of a single
ctx output and keep going sweeping the other outputs so
we sweep at least those that do not crash and don't leave
them on the table.
What changed, and why it matters
This commit is a defensive hardening fix for Electrum's Lightning channel watcher. It wraps each individual sweep operation in a try/except block so that if one output fails or crashes, the remaining outputs are still swept. Previously, an unhandled exception during one sweep could stop the entire process, potentially leaving funds unclaimed. The commit does not itself introduce a vulnerability; it reduces the risk of losing funds due to a crash or edge-case bug during channel closure recovery.
Treat as a routine hardening patch. Users running Lightning nodes should update to include this fix to reduce the chance of unrecovered funds during force closes. No immediate emergency response is warranted based on the commit alone. Monitor Electrum release notes for any later security classification.
Security signals we found
Defensive exception handling added around financial transaction creation/broadcast
Refactor isolates per-output sweep logic to prevent one failure from aborting all sweeps
Comment explicitly states goal: avoid leaving sweepable outputs 'on the table'
No new cryptographic, network, or input-validation code introduced
No CVE, advisory, or researcher attribution present in commit materials
Evidence from the diff
The patch refactors sweep_commitment_transaction() in electrum/lnwatcher.py by extracting per-output sweep logic into a new _sweep_ctx_output() helper and catching exceptions around each call. The main loop now continues sweeping other commitment-transaction outputs even if one raises. It also imports MaybeSweepInfo from lnsweep. This is a robustness improvement: a single malformed sweep_info, missing prevout, or unexpected exception no longer aborts the whole sweep batch. There is no direct evidence of an exploitable security bug being fixed; rather, it mitigates a denial-of-recovery / fund-loss scenario.
Changed components
electrum/lnwatcher.pyLightning channel force-close sweep logicInspect captured patch +62 / −46
### electrum/lnwatcher.py
@@ -14,7 +14,7 @@
from .logging import Logger
from .address_synchronizer import TX_HEIGHT_LOCAL
from .lnutil import REDEEM_AFTER_DOUBLE_SPENT_DELAY
-from .lnsweep import KeepWatchingTXO, SweepInfo
+from .lnsweep import KeepWatchingTXO, SweepInfo, MaybeSweepInfo
if TYPE_CHECKING:
from .network import Network
@@ -201,58 +201,74 @@ async def sweep_commitment_transaction(self, funding_outpoint: str, closing_tx:
chan = self.lnworker.channel_by_txo(funding_outpoint)
if not chan:
return False
- local_height = self.adb.get_local_height()
self._pending_force_closes.pop(chan, None) # recomputed below
# detect who closed and get information about how to claim outputs
is_local_ctx, sweep_info_dict = chan.get_ctx_sweep_info(closing_tx)
# note: we need to keep watching *at least* until the closing tx is deeply mined,
# possibly longer if there are TXOs to sweep
keep_watching = not self.adb.is_deeply_mined(closing_tx.txid())
# create and broadcast transactions
- for prevout, sweep_info in sweep_info_dict.items(): # FIXME isolate iterations (error-wise)
- prev_txid, prev_index = prevout.split(':')
- name = sweep_info.name + ' ' + chan.get_id_for_log()
- self.lnworker.wallet.set_default_label(prevout, name)
- if isinstance(sweep_info, KeepWatchingTXO): # haven't yet decided if we want to sweep
- keep_watching |= sweep_info.until_height > local_height
- continue
- assert isinstance(sweep_info, SweepInfo), sweep_info
- if not self.adb.get_transaction(prev_txid):
- # do not keep watching if prevout does not exist
- self.logger.info(f'prevout does not exist for {name}: {prevout}')
- continue
- watch_sweep_info = self.maybe_redeem(sweep_info)
- spender_txid = self.adb.get_spender(prevout) # note: LOCAL spenders don't count
- spender_tx = self.adb.get_transaction(spender_txid) if spender_txid else None
- if spender_tx:
- # the spender might be the remote, revoked or not
- htlc_sweepinfo = chan.maybe_sweep_htlcs(closing_tx, spender_tx)
- if htlc_sweepinfo:
- self.adb.subscribe_to_outputs(spender_txid)
- for prevout2, htlc_sweep_info in htlc_sweepinfo.items():
- self.lnworker.wallet.set_default_label(prevout2, htlc_sweep_info.name)
- if isinstance(htlc_sweep_info, KeepWatchingTXO): # haven't yet decided if we want to sweep
- keep_watching |= htlc_sweep_info.until_height > local_height
- continue
- assert isinstance(htlc_sweep_info, SweepInfo), htlc_sweep_info
- watch_htlc_sweep_info = self.maybe_redeem(htlc_sweep_info)
- htlc_tx_spender = self.adb.get_spender(prevout2)
- if htlc_tx_spender:
- keep_watching |= not self.adb.is_deeply_mined(htlc_tx_spender)
- self.maybe_add_accounting_address(htlc_tx_spender, htlc_sweep_info)
- else:
- keep_watching |= watch_htlc_sweep_info
- keep_watching |= not self.adb.is_deeply_mined(spender_txid)
- self.maybe_extract_preimage(chan, spender_tx, prevout)
- self.maybe_add_accounting_address(spender_txid, sweep_info)
- else:
- keep_watching |= watch_sweep_info
- self.maybe_add_pending_forceclose(
- chan=chan,
- spender_txid=spender_txid,
- is_local_ctx=is_local_ctx,
- sweep_info=sweep_info,
- )
+ for prevout, sweep_info in sweep_info_dict.items():
+ try:
+ keep_watching |= self._sweep_ctx_output(prevout, sweep_info, chan, closing_tx, is_local_ctx)
+ except Exception as e:
+ # in case a single sweep crashes we keep sweeping the other outputs
+ self.logger.exception(f"failed to sweep {prevout=}")
+ keep_watching = True
+ return keep_watching
+
+ def _sweep_ctx_output(
+ self,
+ prevout: str,
+ sweep_info: MaybeSweepInfo,
+ chan: 'AbstractChannel',
+ closing_tx: Transaction,
+ is_local_ctx: bool,
+ ) -> bool:
+ keep_watching = False
+ local_height = self.adb.get_local_height()
+ prev_txid, prev_index = prevout.split(':')
+ name = sweep_info.name + ' ' + chan.get_id_for_log()
+ self.lnworker.wallet.set_default_label(prevout, name)
+ if isinstance(sweep_info, KeepWatchingTXO): # haven't yet decided if we want to sweep
+ return sweep_info.until_height > local_height
+ assert isinstance(sweep_info, SweepInfo), sweep_info
+ if not self.adb.get_transaction(prev_txid):
+ # do not keep watching if prevout does not exist
+ self.logger.info(f'prevout does not exist for {name}: {prevout}')
+ return False
+ watch_sweep_info = self.maybe_redeem(sweep_info)
+ spender_txid = self.adb.get_spender(prevout) # note: LOCAL spenders don't count
+ spender_tx = self.adb.get_transaction(spender_txid) if spender_txid else None
+ if spender_tx:
+ # the spender might be the remote, revoked or not
+ htlc_sweepinfo = chan.maybe_sweep_htlcs(closing_tx, spender_tx)
+ if htlc_sweepinfo:
+ self.adb.subscribe_to_outputs(spender_txid)
+ for prevout2, htlc_sweep_info in htlc_sweepinfo.items():
+ self.lnworker.wallet.set_default_label(prevout2, htlc_sweep_info.name)
+ if isinstance(htlc_sweep_info, KeepWatchingTXO): # haven't yet decided if we want to sweep
+ keep_watching |= htlc_sweep_info.until_height > local_height
+ continue
+ assert isinstance(htlc_sweep_info, SweepInfo), htlc_sweep_info
+ watch_htlc_sweep_info = self.maybe_redeem(htlc_sweep_info)
+ htlc_tx_spender = self.adb.get_spender(prevout2)
+ if htlc_tx_spender:
+ keep_watching |= not self.adb.is_deeply_mined(htlc_tx_spender)
+ self.maybe_add_accounting_address(htlc_tx_spender, htlc_sweep_info)
+ else:
+ keep_watching |= watch_htlc_sweep_info
+ keep_watching |= not self.adb.is_deeply_mined(spender_txid)
+ self.maybe_extract_preimage(chan, spender_tx, prevout)
+ self.maybe_add_accounting_address(spender_txid, sweep_info)
+ else:
+ keep_watching |= watch_sweep_info
+ self.maybe_add_pending_forceclose(
+ chan=chan,
+ spender_txid=spender_txid,
+ is_local_ctx=is_local_ctx,
+ sweep_info=sweep_info,
+ )
return keep_watching
def get_pending_force_closes(self) -> Dict['AbstractChannel', int]:Why this scored 31/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.