lnworker: rework "is_dangerous"

"Should channel be closed due to expiring htlcs?"

electrum/lnchannel.py

@@ -551,10 +551,6 @@ class Channel(Logger):
         assert type(direction) is Direction
         return htlcsum(self.hm.all_settled_htlcs_ever_by_direction(LOCAL, direction))
 
-    def get_unfulfilled_htlcs(self):
-        log = self.hm.log[REMOTE]
-        return [v for x,v in log['adds'].items() if x not in log['settles']]
-
     def settle_htlc(self, preimage, htlc_id):
         """
         SettleHTLC attempts to settle an existing outstanding received HTLC.

electrum/lnhtlc.py

@@ -271,6 +271,12 @@ class HTLCManager:
         ctn = self.ctn_latest(subject) + 1
         return self.htlcs(subject, ctn)
 
+    def was_htlc_preimage_released(self, *, htlc_id: int, htlc_sender: HTLCOwner) -> bool:
+        settles = self.log[htlc_sender]['settles']
+        if htlc_id not in settles:
+            return False
+        return settles[htlc_id][htlc_sender] is not None
+
     def all_settled_htlcs_ever_by_direction(self, subject: HTLCOwner, direction: Direction,
                                             ctn: int = None) -> Sequence[UpdateAddHtlc]:
         """Return the list of all HTLCs that have been ever settled in subject's

electrum/lnutil.py

@@ -114,10 +114,31 @@ class RemoteMisbehaving(LightningError): pass
 class NotFoundChanAnnouncementForUpdate(Exception): pass
 
 
-# TODO make configurable?
+# TODO make some of these values configurable?
 DEFAULT_TO_SELF_DELAY = 144
+
+
+##### CLTV-expiry-delta-related values
+# see https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md#cltv_expiry_delta-selection
+
+# the minimum cltv_expiry accepted for terminal payments
 MIN_FINAL_CLTV_EXPIRY_ACCEPTED = 144
-MIN_FINAL_CLTV_EXPIRY_FOR_INVOICE = MIN_FINAL_CLTV_EXPIRY_ACCEPTED + 1
+# set it a tiny bit higher for invoices as blocks could get mined
+# during forward path of payment
+MIN_FINAL_CLTV_EXPIRY_FOR_INVOICE = MIN_FINAL_CLTV_EXPIRY_ACCEPTED + 3
+
+# the deadline for offered HTLCs:
+# the deadline after which the channel has to be failed and timed out on-chain
+NBLOCK_DEADLINE_AFTER_EXPIRY_FOR_OFFERED_HTLCS = 1
+
+# the deadline for received HTLCs this node has fulfilled:
+# the deadline after which the channel has to be failed and the HTLC fulfilled on-chain before its cltv_expiry
+NBLOCK_DEADLINE_BEFORE_EXPIRY_FOR_RECEIVED_HTLCS = 72
+
+# the cltv_expiry_delta for channels when we are forwarding payments
+NBLOCK_OUR_CLTV_EXPIRY_DELTA = 144
+
+NBLOCK_CLTV_EXPIRY_TOO_FAR_INTO_FUTURE = 4032
 
 
 # When we open a channel, the remote peer has to support at least this

electrum/lnworker.py

@@ -36,6 +36,7 @@ from .lnpeer import Peer
 from .lnaddr import lnencode, LnAddr, lndecode
 from .ecc import der_sig_from_sig_string
 from .lnchannel import Channel, ChannelJsonEncoder
+from . import lnutil
 from .lnutil import (Outpoint, calc_short_channel_id, LNPeerAddr,
                      get_compressed_pubkey_from_bech32, extract_nodeid,
                      PaymentFailure, split_host_port, ConnStringFormatError,
@@ -628,16 +629,38 @@ class LNWallet(LNWorker):
             except Exception as e:
                 self.logger.info(f'could not add future tx: {name}. prevout: {prevout} {str(e)}')
 
-    def is_dangerous(self, chan):
+    def should_channel_be_closed_due_to_expiring_htlcs(self, chan: Channel) -> bool:
-        for x in chan.get_unfulfilled_htlcs():
+        local_height = self.network.get_local_height()
-            dust_limit = chan.config[REMOTE].dust_limit_sat * 1000
+        htlcs_we_could_reclaim = {}  # type: Dict[Tuple[Direction, int], UpdateAddHtlc]
-            delay = x.cltv_expiry - self.network.get_local_height()
+        # If there is a received HTLC for which we already released the preimage
-            if x.amount_msat > 10 * dust_limit and delay < 3:
+        # but the remote did not revoke yet, and the CLTV of this HTLC is dangerously close
-                self.logger.info('htlc is dangerous')
+        # to the present, then unilaterally close channel
-                return True
+        recv_htlc_deadline = lnutil.NBLOCK_DEADLINE_BEFORE_EXPIRY_FOR_RECEIVED_HTLCS
-            else:
+        for sub, dir, ctn in ((LOCAL, RECEIVED, chan.get_latest_ctn(LOCAL)),
-                self.logger.info(f'htlc is not dangerous. delay {delay}')
+                              (REMOTE, SENT, chan.get_oldest_unrevoked_ctn(LOCAL)),
-        return False
+                              (REMOTE, SENT, chan.get_latest_ctn(LOCAL)),):
+            for htlc_id, htlc in chan.hm.htlcs_by_direction(subject=sub, direction=dir, ctn=ctn).items():
+                if not chan.hm.was_htlc_preimage_released(htlc_id=htlc_id, htlc_sender=REMOTE):
+                    continue
+                if htlc.cltv_expiry - recv_htlc_deadline > local_height:
+                    continue
+                htlcs_we_could_reclaim[(RECEIVED, htlc_id)] = htlc
+        # If there is an offered HTLC which has already expired (+ some grace period after), we
+        # will unilaterally close the channel and time out the HTLC
+        offered_htlc_deadline = lnutil.NBLOCK_DEADLINE_AFTER_EXPIRY_FOR_OFFERED_HTLCS
+        for sub, dir, ctn in ((LOCAL, SENT, chan.get_latest_ctn(LOCAL)),
+                              (REMOTE, RECEIVED, chan.get_oldest_unrevoked_ctn(LOCAL)),
+                              (REMOTE, RECEIVED, chan.get_latest_ctn(LOCAL)),):
+            for htlc_id, htlc in chan.hm.htlcs_by_direction(subject=sub, direction=dir, ctn=ctn).items():
+                if htlc.cltv_expiry + offered_htlc_deadline > local_height:
+                    continue
+                htlcs_we_could_reclaim[(SENT, htlc_id)] = htlc
+
+        total_value_sat = sum([htlc.amount_msat // 1000 for htlc in htlcs_we_could_reclaim.values()])
+        num_htlcs = len(htlcs_we_could_reclaim)
+        min_value_worth_closing_channel_over_sat = max(num_htlcs * 10 * chan.config[REMOTE].dust_limit_sat,
+                                                       500_000)
+        return total_value_sat > min_value_worth_closing_channel_over_sat
 
     @log_exceptions
     async def on_network_update(self, event, *args):
@@ -652,7 +675,7 @@ class LNWallet(LNWorker):
         for chan in channels:
             if chan.is_closed():
                 continue
-            if chan.get_state() in ["OPEN", "DISCONNECTED"] and self.is_dangerous(chan):
+            if chan.get_state() != 'CLOSED' and self.should_channel_be_closed_due_to_expiring_htlcs(chan):
                 await self.force_close_channel(chan.channel_id)
                 continue
             if chan.short_channel_id is None: