lnchannel: add new states: `WE_ARE_TOXIC`, `REQUESTED_FCLOSE`

The `WE_ARE_TOXIC` state is added as a sanity check to ensure that if
the remote has proven that we have lost state we do not accidentally
do a local force-close. E.g. if we receive an "error" message for the
channel, we might normally do an automatic force-close. Manually
force-closing in such a state is not offered anymore by the GUI.

The `REQUESTED_FCLOSE` state is added as it is quite likely that
we receive an error message from the remote after requesting a fclose,
e.g. during a later chan-reestablish. In such a scenario, we should
not do an auto-local-fclose, however the manual option of a local-fclose
should still be offered.

electrum/gui/kivy/uix/dialogs/lightning_channels.py

@@ -8,7 +8,7 @@ from kivy.uix.popup import Popup
 from electrum.util import bh2u
 from electrum.logging import Logger
 from electrum.lnutil import LOCAL, REMOTE, format_short_channel_id
-from electrum.lnchannel import AbstractChannel, Channel, ChannelState
+from electrum.lnchannel import AbstractChannel, Channel, ChannelState, ChanCloseOption
 from electrum.gui.kivy.i18n import _
 from electrum.transaction import PartialTxOutput, Transaction
 from electrum.util import NotEnoughFunds, NoDynamicFeeEstimates, format_fee_satoshis, quantize_feerate
@@ -495,8 +495,8 @@ class ChannelDetailsPopup(Popup, Logger):
         action_dropdown = self.ids.action_dropdown  # type: ActionDropdown
         options = [
             ActionButtonOption(text=_('Backup'), func=lambda btn: self.export_backup()),
-            ActionButtonOption(text=_('Close channel'), func=lambda btn: self.close(), enabled=not self.is_closed),
-            ActionButtonOption(text=_('Force-close'), func=lambda btn: self.force_close(), enabled=not self.is_closed),
+            ActionButtonOption(text=_('Close channel'), func=lambda btn: self.close(), enabled=ChanCloseOption.COOP_CLOSE in self.chan.get_close_options()),
+            ActionButtonOption(text=_('Force-close'), func=lambda btn: self.force_close(), enabled=ChanCloseOption.LOCAL_FCLOSE in self.chan.get_close_options()),
             ActionButtonOption(text=_('Delete'), func=lambda btn: self.remove_channel(), enabled=self.can_be_deleted),
         ]
         if not self.chan.is_closed():
@@ -557,7 +557,8 @@ class ChannelDetailsPopup(Popup, Logger):
         self.app.qr_dialog(_("Channel Backup " + self.chan.short_id_for_GUI()), text, help_text=help_text)
 
     def force_close(self):
-        if self.chan.is_closed():
+        if ChanCloseOption.LOCAL_FCLOSE not in self.chan.get_close_options():
+            # note: likely channel is already closed, or could be unsafe to do local force-close (e.g. we are toxic)
             self.app.show_error(_('Channel already closed'))
             return
         to_self_delay = self.chan.config[REMOTE].to_self_delay

electrum/gui/qt/channels_list.py

@@ -13,7 +13,7 @@ from PyQt5.QtGui import QFont, QStandardItem, QBrush, QPainter, QIcon, QHelpEven
 
 from electrum.util import bh2u, NotEnoughFunds, NoDynamicFeeEstimates
 from electrum.i18n import _
-from electrum.lnchannel import AbstractChannel, PeerState, ChannelBackup, Channel, ChannelState
+from electrum.lnchannel import AbstractChannel, PeerState, ChannelBackup, Channel, ChannelState, ChanCloseOption
 from electrum.wallet import Abstract_Wallet
 from electrum.lnutil import LOCAL, REMOTE, format_short_channel_id
 from electrum.lnworker import LNWallet
@@ -243,8 +243,9 @@ class ChannelsList(MyTreeView):
         if chan:
             funding_tx = self.parent.wallet.db.get_transaction(chan.funding_outpoint.txid)
             menu.addAction(_("View funding transaction"), lambda: self.parent.show_transaction(funding_tx))
-            if chan.get_state() == ChannelState.FUNDED:
-                menu.addAction(_("Request force-close"), lambda: self.request_force_close(channel_id))
+            if close_opts := chan.get_close_options():
+                if ChanCloseOption.REQUEST_REMOTE_FCLOSE in close_opts:
+                    menu.addAction(_("Request force-close"), lambda: self.request_force_close(channel_id))
             if chan.can_be_deleted():
                 menu.addAction(_("Delete"), lambda: self.remove_channel_backup(channel_id))
             menu.exec_(self.viewport().mapToGlobal(position))
@@ -278,11 +279,12 @@ class ChannelsList(MyTreeView):
                 fm.addAction(_("Freeze for receiving"), lambda: chan.set_frozen_for_receiving(True))
             else:
                 fm.addAction(_("Unfreeze for receiving"), lambda: chan.set_frozen_for_receiving(False))
-        if not chan.is_closed():
+        if close_opts := chan.get_close_options():
             cm = menu.addMenu(_("Close"))
-            if chan.peer_state == PeerState.GOOD:
+            if ChanCloseOption.COOP_CLOSE in close_opts:
                 cm.addAction(_("Cooperative close"), lambda: self.close_channel(channel_id))
-            cm.addAction(_("Force-close"), lambda: self.force_close(channel_id))
+            if ChanCloseOption.LOCAL_FCLOSE in close_opts:
+                cm.addAction(_("Force-close"), lambda: self.force_close(channel_id))
         menu.addAction(_("Export backup"), lambda: self.export_channel_backup(channel_id))
         if chan.can_be_deleted():
             menu.addSeparator()

electrum/lnchannel.py

@@ -17,12 +17,12 @@
 # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
-
+import enum
 import os
 from collections import namedtuple, defaultdict
 import binascii
 import json
-from enum import IntEnum
+from enum import IntEnum, Enum
 from typing import (Optional, Dict, List, Tuple, NamedTuple, Set, Callable,
                     Iterable, Sequence, TYPE_CHECKING, Iterator, Union, Mapping)
 import time
@@ -82,11 +82,14 @@ class ChannelState(IntEnum):
     OPEN            = 3  # both parties have sent funding_locked
     SHUTDOWN        = 4  # shutdown has been sent.
     CLOSING         = 5  # closing negotiation done. we have a fully signed tx.
-    FORCE_CLOSING   = 6  # we force-closed, and closing tx is unconfirmed. Note that if the
+    FORCE_CLOSING   = 6  # *we* force-closed, and closing tx is unconfirmed. Note that if the
                          # remote force-closes then we remain OPEN until it gets mined -
                          # the server could be lying to us with a fake tx.
-    CLOSED          = 7  # closing tx has been mined
-    REDEEMED        = 8  # we can stop watching
+    REQUESTED_FCLOSE = 7   # Chan is open, but we have tried to request the *remote* to force-close
+    WE_ARE_TOXIC     = 8   # Chan is open, but we have lost state and the remote proved this.
+                           # The remote must force-close, it is *not* safe for us to do so.
+    CLOSED           = 9   # closing tx has been mined
+    REDEEMED         = 10  # we can stop watching
 
 
 class PeerState(IntEnum):
@@ -113,12 +116,28 @@ state_transitions = [
     (cs.OPEN,     cs.FORCE_CLOSING),
     (cs.SHUTDOWN, cs.FORCE_CLOSING),
     (cs.CLOSING,  cs.FORCE_CLOSING),
+    (cs.REQUESTED_FCLOSE, cs.FORCE_CLOSING),
+    # we can request a force-close almost any time
+    (cs.OPENING,  cs.REQUESTED_FCLOSE),
+    (cs.FUNDED,   cs.REQUESTED_FCLOSE),
+    (cs.OPEN,     cs.REQUESTED_FCLOSE),
+    (cs.SHUTDOWN, cs.REQUESTED_FCLOSE),
+    (cs.CLOSING,  cs.REQUESTED_FCLOSE),
+    (cs.REQUESTED_FCLOSE,  cs.REQUESTED_FCLOSE),
     # we can get force closed almost any time
     (cs.OPENING,  cs.CLOSED),
     (cs.FUNDED,   cs.CLOSED),
     (cs.OPEN,     cs.CLOSED),
     (cs.SHUTDOWN, cs.CLOSED),
     (cs.CLOSING,  cs.CLOSED),
+    (cs.REQUESTED_FCLOSE, cs.CLOSED),
+    (cs.WE_ARE_TOXIC,          cs.CLOSED),
+    # during channel_reestablish, we might realise we have lost state
+    (cs.OPENING,  cs.WE_ARE_TOXIC),
+    (cs.FUNDED,   cs.WE_ARE_TOXIC),
+    (cs.OPEN,     cs.WE_ARE_TOXIC),
+    (cs.SHUTDOWN, cs.WE_ARE_TOXIC),
+    (cs.REQUESTED_FCLOSE, cs.WE_ARE_TOXIC),
     #
     (cs.FORCE_CLOSING, cs.FORCE_CLOSING),  # allow multiple attempts
     (cs.FORCE_CLOSING, cs.CLOSED),
@@ -130,6 +149,12 @@ state_transitions = [
 del cs  # delete as name is ambiguous without context
 
 
+class ChanCloseOption(Enum):
+    COOP_CLOSE = enum.auto()
+    LOCAL_FCLOSE = enum.auto()
+    REQUEST_REMOTE_FCLOSE = enum.auto()
+
+
 class RevokeAndAck(NamedTuple):
     per_commitment_secret: bytes
     next_per_commitment_point: bytes
@@ -203,6 +228,10 @@ class AbstractChannel(Logger, ABC):
     def is_redeemed(self):
         return self.get_state() == ChannelState.REDEEMED
 
+    @abstractmethod
+    def get_close_options(self) -> Sequence[ChanCloseOption]:
+        pass
+
     def save_funding_height(self, *, txid: str, height: int, timestamp: Optional[int]) -> None:
         self.storage['funding_height'] = txid, height, timestamp
 
@@ -545,6 +574,12 @@ class ChannelBackup(AbstractChannel):
             return self.lnworker.node_keypair.pubkey
         raise NotImplementedError(f"unexpected cb type: {type(cb)}")
 
+    def get_close_options(self) -> Sequence[ChanCloseOption]:
+        ret = []
+        if self.get_state() == ChannelState.FUNDED:
+            ret.append(ChanCloseOption.REQUEST_REMOTE_FCLOSE)
+        return ret
+
 
 class Channel(AbstractChannel):
     # note: try to avoid naming ctns/ctxs/etc as "current" and "pending".
@@ -886,7 +921,9 @@ class Channel(AbstractChannel):
         return True
 
     def should_try_to_reestablish_peer(self) -> bool:
-        return ChannelState.PREOPENING < self._state < ChannelState.CLOSING and self.peer_state == PeerState.DISCONNECTED
+        if self.peer_state != PeerState.DISCONNECTED:
+            return False
+        return ChannelState.PREOPENING < self._state < ChannelState.CLOSING
 
     def get_funding_address(self):
         script = funding_output_script(self.config[LOCAL], self.config[REMOTE])
@@ -1497,6 +1534,16 @@ class Channel(AbstractChannel):
         assert tx.is_complete()
         return tx
 
+    def get_close_options(self) -> Sequence[ChanCloseOption]:
+        ret = []
+        if not self.is_closed() and self.peer_state == PeerState.GOOD:
+            ret.append(ChanCloseOption.COOP_CLOSE)
+            ret.append(ChanCloseOption.REQUEST_REMOTE_FCLOSE)
+        if not self.is_closed() or self.get_state() == ChannelState.REQUESTED_FCLOSE:
+            ret.append(ChanCloseOption.LOCAL_FCLOSE)
+        assert not (self.get_state() == ChannelState.WE_ARE_TOXIC and ChanCloseOption.LOCAL_FCLOSE in ret), "local force-close unsafe if we are toxic"
+        return ret
+
     def maybe_sweep_revoked_htlc(self, ctx: Transaction, htlc_tx: Transaction) -> Optional[SweepInfo]:
         # look at the output address, check if it matches
         return create_sweeptx_for_their_revoked_htlc(self, ctx, htlc_tx, self.sweep_address)

electrum/lnpeer.py

@@ -31,7 +31,7 @@ from .lnonion import (new_onion_packet, OnionFailureCode, calc_hops_data_for_pay
                       process_onion_packet, OnionPacket, construct_onion_error, OnionRoutingFailure,
                       ProcessedOnionPacket, UnsupportedOnionPacketVersion, InvalidOnionMac, InvalidOnionPubkey,
                       OnionFailureCodeMetaFlag)
-from .lnchannel import Channel, RevokeAndAck, RemoteCtnTooFarInFuture, ChannelState, PeerState
+from .lnchannel import Channel, RevokeAndAck, RemoteCtnTooFarInFuture, ChannelState, PeerState, ChanCloseOption
 from . import lnutil
 from .lnutil import (Outpoint, LocalConfig, RECEIVED, UpdateAddHtlc, ChannelConfig,
                      RemoteConfig, OnlyPubkeyKeypair, ChannelConstraints, RevocationStore,
@@ -1075,10 +1075,14 @@ class Peer(Logger):
         channels_with_peer.extend(self.temp_id_to_id.values())
         if channel_id not in channels_with_peer:
             raise ValueError(f"channel {channel_id.hex()} does not belong to this peer")
-        if channel_id in self.channels:
+        chan = self.channels.get(channel_id)
+        if not chan:
+            self.logger.warning(f"tried to force-close channel {channel_id.hex()} but it is not in self.channels yet")
+        if ChanCloseOption.LOCAL_FCLOSE in chan.get_close_options():
             self.lnworker.schedule_force_closing(channel_id)
         else:
-            self.logger.warning(f"tried to force-close channel {channel_id.hex()} but it is not in self.channels yet")
+            self.logger.info(f"tried to force-close channel {chan.get_id_for_log()} "
+                             f"but close option is not allowed. {chan.get_state()=!r}")
 
     def on_channel_reestablish(self, chan, msg):
         their_next_local_ctn = msg["next_commitment_number"]
@@ -1171,6 +1175,7 @@ class Peer(Logger):
                 f"remote is ahead of us! They should force-close. Remote PCP: {bh2u(their_local_pcp)}")
             # data_loss_protect_remote_pcp is used in lnsweep
             chan.set_data_loss_protect_remote_pcp(their_next_local_ctn - 1, their_local_pcp)
+            chan.set_state(ChannelState.WE_ARE_TOXIC)
             self.lnworker.save_channel(chan)
             chan.peer_state = PeerState.BAD
             # raise after we send channel_reestablish, so the remote can realize they are ahead
@@ -1187,6 +1192,7 @@ class Peer(Logger):
         await self.initialized
         chan_id = chan.channel_id
         if chan.should_request_force_close:
+            chan.set_state(ChannelState.REQUESTED_FCLOSE)
             await self.trigger_force_close(chan_id)
             chan.should_request_force_close = False
             return