lnsweep: lnwatcher needs to keep_waiting for pending hold_invoice

electrum/lnchannel.py

@@ -55,7 +55,7 @@
                      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 @@ def get_closing_height(self) -> Optional[Tuple[str, int, Optional[int]]]:
     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 @@ def get_remote_scid_alias(self) -> Optional[bytes]:
     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:

electrum/lnsweep.py

@@ -53,6 +53,15 @@ def csv_delay(self):
         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 hold-invoices."""
+    name: str
+    until_height: int
+
+
+MaybeSweepInfo = SweepInfo | KeepWatchingTXO
+
+
 def sweep_their_ctx_watchtower(
         chan: 'Channel',
         ctx: Transaction,
@@ -281,7 +290,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.
 
@@ -328,7 +337,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():
@@ -417,7 +426,8 @@ def txs_htlc(
                         privkey=our_localdelayed_privkey.get_secret_bytes(),
                         is_revocation=False,
                 ):
-                    txs[actual_htlc_tx.txid() + f':{output_idx}'] = SweepInfo(
+                    prevout = actual_htlc_tx.txid() + f':{output_idx}'
+                    txs[prevout] = SweepInfo(
                         name=f'second-stage-htlc:{output_idx}',
                         cltv_abs=0,
                         txin=sweep_txin,
@@ -437,16 +447,18 @@ def txs_htlc(
         subject=LOCAL,
         ctn=ctn)
     for (direction, htlc), (ctx_output_idx, htlc_relative_idx) in htlc_to_ctx_output_idx_map.items():
+        preimage = None
         if direction == RECEIVED:
-            if not chan.lnworker.is_complete_mpp(htlc.payment_hash):
-                # do not redeem this, it might publish the preimage of an incomplete MPP
-                continue
-            preimage = chan.lnworker.get_preimage(htlc.payment_hash)
+            # 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(
+                chan=chan, htlc=htlc, txs=txs,
+                prevout=ctx.txid() + ':%d' % ctx_output_idx,
+                sweep_info_name=f"our_ctx_htlc_{ctx_output_idx}",
+            )
             if not preimage:
-                # we might not have the preimage if this is a hold invoice
                 continue
-        else:
-            preimage = None
         try:
             txs_htlc(
                 htlc=htlc,
@@ -459,6 +471,32 @@ def txs_htlc(
     return txs
 
 
+def _maybe_reveal_preimage_for_htlc(
+    *,
+    chan: 'AbstractChannel',
+    htlc: 'UpdateAddHtlc',
+    txs: Dict[str, MaybeSweepInfo],  # mutated in-place!
+    prevout: str,  # commitment txid + output_idx  (so always for first stage)
+    sweep_info_name: str,
+) -> Optional[bytes]:
+    """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
+        # - OTOH maybe this chan just got closed, and we are still receiving new htlcs
+        #   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:
+        txs[prevout] = KeepWatchingTXO(
+            name=sweep_info_name + "_for_hold_invoice",
+            until_height=htlc.cltv_abs,
+        )
+        return None
+    preimage = chan.lnworker.get_preimage(htlc.payment_hash)
+    return preimage
+
+
 def extract_ctx_secrets(chan: 'Channel', ctx: Transaction):
     # note: the remote sometimes has two valid non-revoked commitment transactions,
     # either of which could be broadcast
@@ -593,7 +631,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.
@@ -607,7 +645,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:
@@ -737,17 +775,16 @@ def tx_htlc(
         subject=REMOTE,
         ctn=ctn)
     for (direction, htlc), (ctx_output_idx, htlc_relative_idx) in htlc_to_ctx_output_idx_map.items():
+        preimage = None
         is_received_htlc = direction == RECEIVED
         if not is_received_htlc and not is_revocation:
-            if not chan.lnworker.is_complete_mpp(htlc.payment_hash):
-                # do not redeem this, it might publish the preimage of an incomplete MPP
-                continue
-            preimage = chan.lnworker.get_preimage(htlc.payment_hash)
+            preimage = _maybe_reveal_preimage_for_htlc(
+                chan=chan, htlc=htlc, txs=txs,
+                prevout=ctx.txid() + ':%d' % ctx_output_idx,
+                sweep_info_name=f"their_ctx_htlc_{ctx_output_idx}",
+            )
             if not preimage:
-                # we might not have the preimage if this is a hold invoice
                 continue
-        else:
-            preimage = None
         tx_htlc(
             htlc=htlc,
             is_received_htlc=is_received_htlc,

electrum/lnwatcher.py

@@ -11,11 +11,10 @@
 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
 
@@ -160,6 +159,7 @@ async def sweep_commitment_transaction(self, funding_outpoint: str, closing_tx:
         if not chan.need_to_subscribe():
             return False
         self.logger.info(f'sweep_commitment_transaction {funding_outpoint}')
+        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,
@@ -170,6 +170,10 @@ async def sweep_commitment_transaction(self, funding_outpoint: str, closing_tx:
             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}')