lnwallet: zeroconf/just-in-time improvements and tests

electrum/gui/qt/receive_tab.py

@@ -20,6 +20,7 @@
 
 if TYPE_CHECKING:
     from .main_window import ElectrumWindow
+    from electrum.wallet import Request
 
 
 class ReceiveTab(QWidget, MessageBoxMixin, Logger):
@@ -101,6 +102,9 @@ def __init__(self, window: 'ElectrumWindow'):
         self.receive_zeroconf_button = QPushButton(_('Accept'))
         self.receive_zeroconf_button.clicked.connect(self.on_accept_zeroconf)
 
+        self.previous_request = None  # type: Optional['Request']
+        self.confirmed_zeroconf_for_this_request = False  # type: bool
+
         def on_receive_rebalance():
             if self.receive_rebalance_button.suggestion:
                 chan1, chan2, delta = self.receive_rebalance_button.suggestion
@@ -221,14 +225,17 @@ def toggle_receive_qr(self):
 
     def update_receive_widgets(self):
         b = self.config.GUI_QT_RECEIVE_TAB_QR_VISIBLE
-        self.receive_widget.update_visibility(b)
+        self.receive_widget.update_visibility(b, bool(self.receive_help_text.text()))
 
     def update_current_request(self):
         if len(self.request_list.selectionModel().selectedRows(0)) > 1:
             key = None
         else:
             key = self.request_list.get_current_key()
         req = self.wallet.get_request(key) if key else None
+        if req != self.previous_request:
+            self.previous_request = req
+            self.confirmed_zeroconf_for_this_request = False
         if req is None:
             self.receive_e.setText('')
             self.addr = self.URI = self.lnaddr = ''
@@ -243,7 +250,7 @@ def update_current_request(self):
         self.ln_help = help_texts.ln_help
         can_rebalance = help_texts.can_rebalance()
         can_swap = help_texts.can_swap()
-        can_zeroconf = help_texts.can_zeroconf()
+        can_zeroconf = help_texts.can_zeroconf() if not self.confirmed_zeroconf_for_this_request else False
         self.receive_rebalance_button.suggestion = help_texts.ln_rebalance_suggestion
         self.receive_swap_button.suggestion = help_texts.ln_swap_suggestion
         self.receive_rebalance_button.setVisible(can_rebalance)
@@ -253,25 +260,26 @@ def update_current_request(self):
         self.receive_zeroconf_button.setVisible(can_zeroconf)
         self.receive_zeroconf_button.setEnabled(can_zeroconf)
         text, data, help_text, title = self.get_tab_data()
+        if self.confirmed_zeroconf_for_this_request and help_texts.can_zeroconf():
+            help_text = ''
+        # set help before receive_e so we don't flicker from qr to help
+        self.receive_help_text.setText(help_text)
         self.receive_e.setText(text)
         self.receive_qr.setData(data)
-        self.receive_help_text.setText(help_text)
         for w in [self.receive_e, self.receive_qr]:
             w.setEnabled(bool(text) and (not help_text or can_zeroconf))
             w.setToolTip(help_text)
         # macOS hack (similar to #4777)
         self.receive_e.repaint()
         # always show
-        if can_zeroconf:
-            # show the help message if zeroconf so user can first accept it and still sees the invoice
-            # after accepting
-            self.receive_widget.show_help()
         self.receive_widget.setVisible(True)
         self.toggle_qr_button.setEnabled(True)
         self.update_receive_qr_window()
 
     def on_accept_zeroconf(self):
         self.receive_zeroconf_button.setVisible(False)
+        self.confirmed_zeroconf_for_this_request = True
+        self.receive_help_text.setText('')
         self.update_receive_widgets()
 
     def get_tab_data(self):
@@ -386,8 +394,8 @@ def __init__(self, receive_tab: 'ReceiveTab', textedit: QWidget, qr: QWidget, he
 
         self.setLayout(vbox)
 
-    def update_visibility(self, is_qr):
-        if str(self.textedit.toPlainText()):
+    def update_visibility(self, is_qr: bool, show_help: bool):
+        if str(self.textedit.toPlainText()) and not show_help:
             self.help_widget.setVisible(False)
             self.textedit.setVisible(not is_qr)
             self.qr.setVisible(is_qr)

electrum/lnchannel.py

@@ -41,7 +41,7 @@
 from .crypto import sha256, sha256d
 from .transaction import Transaction, PartialTransaction, TxInput, Sighash
 from .logging import Logger
-from .lntransport import LNPeerAddr
+from .lntransport import LNPeerAddr, extract_nodeid, ConnStringFormatError
 from .lnonion import OnionRoutingFailure
 from . import lnutil
 from .lnutil import (Outpoint, LocalConfig, RemoteConfig, Keypair, OnlyPubkeyKeypair, ChannelConstraints,
@@ -59,7 +59,7 @@
 from .lnsweep import sweep_their_ctx_to_remote_backup
 from .lnhtlc import HTLCManager
 from .lnmsg import encode_msg, decode_msg
-from .address_synchronizer import TX_HEIGHT_LOCAL
+from .address_synchronizer import TX_HEIGHT_LOCAL, TX_HEIGHT_UNCONFIRMED
 from .lnutil import CHANNEL_OPENING_TIMEOUT_BLOCKS, CHANNEL_OPENING_TIMEOUT_SEC
 from .lnutil import ChannelBackupStorage, ImportedChannelBackupStorage, OnchainChannelBackupStorage
 from .lnutil import format_short_channel_id
@@ -224,6 +224,7 @@ def get_state(self) -> ChannelState:
         return self._state
 
     def is_funded(self) -> bool:
+        # NOTE: also true for unfunded zeroconf channels (OPEN > FUNDED)
         return self.get_state() >= ChannelState.FUNDED
 
     def is_open(self) -> bool:
@@ -375,26 +376,33 @@ def update_unfunded_state(self) -> None:
                 self.logger.warning(f"dropping incoming channel, funding tx not found in mempool")
                 self.lnworker.remove_channel(self.channel_id)
         elif self.is_zeroconf() and state in [ChannelState.OPEN, ChannelState.CLOSING, ChannelState.FORCE_CLOSING]:
-            chan_age = now() - self.storage['init_timestamp']
             # handling zeroconf channels with no funding tx, can happen if broadcasting fails on LSP side
-            # or if the LSP did double spent the funding tx/never published it intentionally
-            # only remove a timed out OPEN channel if we are connected to the network to prevent removing it if we went
-            # offline before seeing the funding tx
-            if state != ChannelState.OPEN or chan_age > ZEROCONF_TIMEOUT and self.lnworker.network.is_connected():
-                # we delete the channel if its in closing state (either initiated manually by client or by LSP on failure)
-                # or if the channel is not seeing any funding tx after 10 minutes to prevent further usage (limit damage)
-                self.set_state(ChannelState.REDEEMED, force=True)
-                local_balance_sat = int(self.balance(LOCAL) // 1000)
-                if local_balance_sat > 0:
+            # or if the LSP did double spent the funding tx/never published it intentionally.
+            if not self.lnworker.wallet.is_up_to_date() or not self.lnworker.network \
+                    or self.lnworker.network.blockchain().is_tip_stale():
+                # ensure we are up to date to prevent accidentally dropping a channel that is funded
+                return
+            chan_age = now() - self.storage['init_timestamp']
+            if chan_age > ZEROCONF_TIMEOUT:
+                # freeze the channel to avoid receiving even more into this unfunded channel.
+                # NOTE: we don't reject htlcs arriving on frozen channels, this only really
+                # stops us from including the channel in invoice routing hints.
+                if isinstance(self, Channel):
+                    self.set_frozen_for_receiving(True)
+
+                # un-trust the LSP so the user doesn't accept another channel from the same provider
+                # compare the node id's as the user might already have changed to another one
+                if self.node_id == self.lnworker.trusted_zeroconf_node_id:
+                    self.lnworker.config.ZEROCONF_TRUSTED_NODE = ''
+
+            if self.has_funding_timed_out():
+                self.lnworker.remove_channel(self.channel_id)
+                # remove remaining local transactions from the wallet, this will also remove child transactions (closing tx)
+                # self.lnworker.lnwatcher.adb.remove_transaction(self.funding_outpoint.txid)
+                if (local_balance_sat := int(self.balance(LOCAL) // 1000)) > 0:
                     self.logger.warning(
                         f"we may have been scammed out of {local_balance_sat} sat by our "
                         f"JIT provider: {self.lnworker.config.ZEROCONF_TRUSTED_NODE} or he didn't use our preimage")
-                    self.lnworker.config.ZEROCONF_TRUSTED_NODE = ''
-                # FIXME this is broken: lnwatcher.unwatch_channel does not exist
-                self.lnworker.lnwatcher.unwatch_channel(self.get_funding_address(), self.funding_outpoint.to_str())
-                # remove remaining local transactions from the wallet, this will also remove child transactions (closing tx)
-                self.lnworker.lnwatcher.adb.remove_transaction(self.funding_outpoint.txid)
-                self.lnworker.remove_channel(self.channel_id)
 
     def update_funded_state(self, *, funding_txid: str, funding_height: TxMinedInfo) -> None:
         self.save_funding_height(txid=funding_txid, height=funding_height.height(), timestamp=funding_height.timestamp)
@@ -420,6 +428,9 @@ def update_funded_state(self, *, funding_txid: str, funding_height: TxMinedInfo)
                 # remove zeroconf flag as we are now confirmed, this is to prevent an electrum server causing
                 # us to remove a channel later in update_unfunded_state by omitting its funding tx
                 self.remove_zeroconf_flag()
+                # unfreeze in case it was frozen in update_unfunded_state
+                if isinstance(self, Channel):
+                    self.set_frozen_for_receiving(False)
 
     def update_closed_state(self, *, funding_txid: str, funding_height: TxMinedInfo,
                             closing_txid: str, closing_height: TxMinedInfo, keep_watching: bool) -> None:
@@ -843,7 +854,8 @@ def can_be_deleted(self) -> bool:
         return self.is_redeemed()
 
     def has_funding_timed_out(self):
-        if self.is_initiator() or self.is_funded():
+        funding_height = self.get_funding_height()
+        if self.is_initiator() or funding_height and funding_height[1] > TX_HEIGHT_UNCONFIRMED:
             return False
         if self.lnworker.network.blockchain().is_tip_stale() or not self.lnworker.wallet.is_up_to_date():
             return False

electrum/lnpeer.py

@@ -101,6 +101,11 @@ def __init__(
         self.pubkey = pubkey  # remote pubkey
         self.privkey = self.transport.privkey  # local privkey
         self.features = self.lnworker.features  # type: LnFeatures
+        if lnworker == lnworker.network.lngossip or \
+            lnworker.config.ZEROCONF_TRUSTED_NODE and pubkey != lnworker.trusted_zeroconf_node_id:
+            # don't signal zeroconf support if we are client (a trusted node is configured),
+            # and Peer is not our trusted node
+            self.features &= ~LnFeatures.OPTION_ZEROCONF_OPT
         self.their_features = LnFeatures(0)  # type: LnFeatures
         self.node_ids = [self.pubkey, privkey_to_pubkey(self.privkey)]
         assert self.node_ids[0] != self.node_ids[1]
@@ -1260,13 +1265,16 @@ async def on_open_channel(self, payload):
         # store the temp id now, so that it is recognized for e.g. 'error' messages
         self.temp_id_to_id[temp_chan_id] = None
         self._cleanup_temp_channelids()
-        channel_opening_fee_tlv = open_channel_tlvs.get('channel_opening_fee', {})
-        channel_opening_fee = channel_opening_fee_tlv.get('channel_opening_fee')
-        if channel_opening_fee:
-            # todo check that the fee is reasonable
+        channel_opening_fee = open_channel_tlvs.get('channel_opening_fee', {}).get('channel_opening_fee')
+        if channel_opening_fee:  # just-in-time channel opening
             assert is_zeroconf
-            self.logger.info(f"just-in-time opening fee: {channel_opening_fee} msat")
-            pass
+            # the opening fee consists of the fee configured by the LSP + mining fees of the funding tx
+            channel_opening_fee_sat = channel_opening_fee // 1000
+            if channel_opening_fee_sat > funding_sat * 0.1:
+                # TODO: if there will be some discovery channel where LSPs announce their fees
+                #  we should compare against the fees they announced here.
+                raise Exception(f"{channel_opening_fee_sat=} exceeding fee limit, rejecting channel ({funding_sat=})")
+            self.logger.info(f"just-in-time channel: {channel_opening_fee_sat=}")
 
         if channel_type & ChannelType.OPTION_ANCHORS_ZERO_FEE_HTLC_TX:
             multisig_funding_keypair = lnutil.derive_multisig_funding_key_if_they_opened(