lnsweep: lnwatcher needs to keep_watching if htlc in dont_settle_htlcs
What changed, and why it matters
This commit fixes a logic bug in Electrum's Lightning channel-sweeping code. Previously, if a payment hash was in a special 'do not settle yet' list used during JIT (just-in-time) channel opens, the wallet would simply ignore the related HTLC output instead of continuing to watch it. That could let an attacker or a forced channel close steal funds because the wallet might stop monitoring before it was safe to claim the money. The fix makes the watcher keep watching those outputs until either the condition clears or the HTLC's deadline expires.
Users running Lightning nodes with JIT channel support should upgrade to a version containing this commit. Reviewers should verify that KeepWatchingTXO objects are handled in all sweep-info consumers and that no other sites compare payment_hash objects to hex-string sets.
Security signals we found
Funds-at-risk in forced channel close during JIT channel open
Incorrect set membership check (payment_hash object vs hex string) bypassed intended delay
Missing keep_watching logic for delayed-settle HTLC outputs
Lightning protocol safety: preimage revelation timing
Evidence from the diff
The patch introduces a new intermediate state, KeepWatchingTXO, returned by lnsweep functions when an HTLC preimage should not yet be revealed because its payment_hash is in lnworker.dont_settle_htlcs. lnwatcher now recognizes this state and continues to keep_watching until sweep_info.until_height (the HTLC’s CLTV expiry) is reached, rather than dropping the output. It also corrects a key lookup bug: htlc.payment_hash was compared directly against dont_settle_htlcs (a set of hex strings), so the guard never actually matched; the fix uses htlc.payment_hash.hex().
Changed components
electrum/lnsweep.pyelectrum/lnwatcher.pyelectrum/lnchannel.pyInspect captured patch +44 / −17
diff --git a/electrum/lnchannel.py b/electrum/lnchannel.py
index 446ecf3..df34e8c 100644
--- a/electrum/lnchannel.py
+++ b/electrum/lnchannel.py
@@ -55,7 +55,7 @@ from .lnutil import (Outpoint, LocalConfig, RemoteConfig, Keypair, OnlyPubkeyKey
received_htlc_trim_threshold_sat, make_commitment_output_to_remote_address, FIXED_ANCHOR_SAT,
ChannelType, LNProtocolWarning, ZEROCONF_TIMEOUT)
from .lnsweep import sweep_our_ctx, sweep_their_ctx
-from .lnsweep import sweep_their_htlctx_justice, sweep_our_htlctx, SweepInfo
+from .lnsweep import sweep_their_htlctx_justice, sweep_our_htlctx, SweepInfo, MaybeSweepInfo
from .lnsweep import sweep_their_ctx_to_remote_backup
from .lnhtlc import HTLCManager
from .lnmsg import encode_msg, decode_msg
@@ -286,10 +286,10 @@ class AbstractChannel(Logger, ABC):
def delete_closing_height(self):
self.storage.pop('closing_height', None)
- def create_sweeptxs_for_our_ctx(self, ctx: Transaction) -> Dict[str, SweepInfo]:
+ def create_sweeptxs_for_our_ctx(self, ctx: Transaction) -> Dict[str, MaybeSweepInfo]:
return sweep_our_ctx(chan=self, ctx=ctx)
- def create_sweeptxs_for_their_ctx(self, ctx: Transaction) -> Dict[str, SweepInfo]:
+ def create_sweeptxs_for_their_ctx(self, ctx: Transaction) -> Dict[str, MaybeSweepInfo]:
return sweep_their_ctx(chan=self, ctx=ctx)
def is_backup(self) -> bool:
@@ -304,7 +304,7 @@ class AbstractChannel(Logger, ABC):
def get_remote_peer_sent_error(self) -> Optional[str]:
return None
- def get_ctx_sweep_info(self, ctx: Transaction) -> Tuple[bool, Dict[str, SweepInfo]]:
+ def get_ctx_sweep_info(self, ctx: Transaction) -> Tuple[bool, Dict[str, MaybeSweepInfo]]:
our_sweep_info = self.create_sweeptxs_for_our_ctx(ctx)
their_sweep_info = self.create_sweeptxs_for_their_ctx(ctx)
if our_sweep_info:
diff --git a/electrum/lnsweep.py b/electrum/lnsweep.py
index 3e1c152..761f706 100644
--- a/electrum/lnsweep.py
+++ b/electrum/lnsweep.py
@@ -54,6 +54,15 @@ class SweepInfo(NamedTuple):
return self.txin.get_block_based_relative_locktime() or 0
+class KeepWatchingTXO(NamedTuple):
+ """Used for UTXOs we don't yet know if we want to sweep, such as pending HTLCs for JIT channels."""
+ name: str
+ until_height: int
+
+
+MaybeSweepInfo = SweepInfo | KeepWatchingTXO
+
+
def sweep_their_ctx_watchtower(
chan: 'Channel',
ctx: Transaction,
@@ -282,7 +291,7 @@ def sweep_our_ctx(
*, chan: 'AbstractChannel',
ctx: Transaction,
actual_htlc_tx: Transaction=None, # if passed, return second stage htlcs
-) -> Dict[str, SweepInfo]:
+) -> Dict[str, MaybeSweepInfo]:
"""Handle the case where we force-close unilaterally with our latest ctx.
@@ -329,7 +338,7 @@ def sweep_our_ctx(
# other outputs are htlcs
# if they are spent, we need to generate the script
# so, second-stage htlc sweep should not be returned here
- txs = {} # type: Dict[str, SweepInfo]
+ txs = {} # type: Dict[str, MaybeSweepInfo]
# local anchor
if actual_htlc_tx is None and chan.has_anchors():
@@ -444,9 +453,13 @@ def sweep_our_ctx(
# note: it is the first stage (witness of htlc_tx) that reveals the preimage,
# so if we are already in second stage, it is already revealed.
# However, here, we don't make a distinction.
- preimage = _maybe_reveal_preimage_for_htlc(
+ preimage, keep_watching_txo = _maybe_reveal_preimage_for_htlc(
chan=chan, htlc=htlc,
+ sweep_info_name=f"our_ctx_htlc_{ctx_output_idx}",
)
+ if keep_watching_txo:
+ prevout = ctx.txid() + ':%d' % ctx_output_idx
+ txs[prevout] = keep_watching_txo
if not preimage:
continue
try:
@@ -465,7 +478,8 @@ def _maybe_reveal_preimage_for_htlc(
*,
chan: 'AbstractChannel',
htlc: 'UpdateAddHtlc',
-) -> Optional[bytes]:
+ sweep_info_name: str,
+) -> Tuple[Optional[bytes], Optional[KeepWatchingTXO]]:
"""Given a Remote-added-HTLC, return the preimage if it's okay to reveal it on-chain."""
if not chan.lnworker.is_complete_mpp(htlc.payment_hash):
# - do not redeem this, it might publish the preimage of an incomplete MPP
@@ -473,11 +487,16 @@ def _maybe_reveal_preimage_for_htlc(
# for this MPP set. So the MPP set might still transition to complete!
# The MPP_TIMEOUT is only around 2 minutes, so this window is short.
# The default keep_watching logic in lnwatcher is sufficient to call us again.
- return None
- if htlc.payment_hash in chan.lnworker.dont_settle_htlcs:
- return None
+ return None, None
+ if htlc.payment_hash.hex() in chan.lnworker.dont_settle_htlcs:
+ # we should not reveal the preimage *for now*, but we might still decide to reveal it later
+ keep_watching_txo = KeepWatchingTXO(
+ name=sweep_info_name + "_dont_settle_htlcs",
+ until_height=htlc.cltv_abs,
+ )
+ return None, keep_watching_txo
preimage = chan.lnworker.get_preimage(htlc.payment_hash)
- return preimage
+ return preimage, None
def extract_ctx_secrets(chan: 'Channel', ctx: Transaction):
@@ -614,7 +633,7 @@ def sweep_their_ctx_to_remote_backup(
def sweep_their_ctx(
*, chan: 'Channel',
- ctx: Transaction) -> Optional[Dict[str, SweepInfo]]:
+ ctx: Transaction) -> Optional[Dict[str, MaybeSweepInfo]]:
"""Handle the case when the remote force-closes with their ctx.
Sweep outputs that do not have a CSV delay ('to_remote' and first-stage HTLCs).
Outputs with CSV delay ('to_local' and second-stage HTLCs) are redeemed by LNWatcher.
@@ -628,7 +647,7 @@ def sweep_their_ctx(
Outputs with CSV/CLTV are redeemed by LNWatcher.
"""
- txs = {} # type: Dict[str, SweepInfo]
+ txs = {} # type: Dict[str, MaybeSweepInfo]
our_conf, their_conf = get_ordered_channel_configs(chan=chan, for_us=True)
x = extract_ctx_secrets(chan, ctx)
if not x:
@@ -761,9 +780,13 @@ def sweep_their_ctx(
preimage = None
is_received_htlc = direction == RECEIVED
if not is_received_htlc and not is_revocation:
- preimage = _maybe_reveal_preimage_for_htlc(
+ preimage, keep_watching_txo = _maybe_reveal_preimage_for_htlc(
chan=chan, htlc=htlc,
+ sweep_info_name=f"their_ctx_htlc_{ctx_output_idx}",
)
+ if keep_watching_txo:
+ prevout = ctx.txid() + ':%d' % ctx_output_idx
+ txs[prevout] = keep_watching_txo
if not preimage:
continue
tx_htlc(
diff --git a/electrum/lnwatcher.py b/electrum/lnwatcher.py
index e41497d..354b0a6 100644
--- a/electrum/lnwatcher.py
+++ b/electrum/lnwatcher.py
@@ -11,11 +11,10 @@ from .transaction import Transaction, TxOutpoint
from .logging import Logger
from .address_synchronizer import TX_HEIGHT_LOCAL
from .lnutil import REDEEM_AFTER_DOUBLE_SPENT_DELAY
-
+from .lnsweep import KeepWatchingTXO, SweepInfo
if TYPE_CHECKING:
from .network import Network
- from .lnsweep import SweepInfo
from .lnworker import LNWallet
from .lnchannel import AbstractChannel
@@ -159,6 +158,7 @@ class LNWatcher(Logger, EventListener):
chan = self.lnworker.channel_by_txo(funding_outpoint)
if not chan:
return False
+ local_height = self.adb.get_local_height()
# 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,
@@ -169,6 +169,10 @@ class LNWatcher(Logger, EventListener):
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}')
Why this scored 59/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.