Improve JIT channel opening

f321x · f321x · commit 496f4901976d · 2025-03-07T14:13:01.000+01:00
diff --git a/electrum/gui/qml/qerequestdetails.py b/electrum/gui/qml/qerequestdetails.py
@@ -118,9 +118,12 @@ def expiration(self):
 
     @pyqtProperty(str, notify=detailsChanged)
     def bolt11(self):
-        can_receive = self._wallet.wallet.lnworker.num_sats_can_receive() if  self._wallet.wallet.lnworker else 0
-        if self._req and can_receive > 0 and (self._req.get_amount_sat() or 0) <= can_receive:
-            bolt11 = self._wallet.wallet.get_bolt11_invoice(self._req)
+        wallet = self._wallet.wallet
+        amount_sat = self._req.get_amount_sat() or 0
+        can_receive = wallet.lnworker.num_sats_can_receive() if wallet.lnworker else 0
+        will_req_zeroconf = wallet.lnworker.receive_requires_jit_channel(amount_msat=amount_sat*1000)
+        if self._req and ((can_receive > 0 and amount_sat <= can_receive) or will_req_zeroconf):
+            bolt11 = wallet.get_bolt11_invoice(self._req)
         else:
             return ''
         # encode lightning invoices as uppercase so QR encoding can use
diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py
@@ -393,7 +393,9 @@ def on_init(self, payload):
         if their_networks:
             their_chains = list(chunks(their_networks["chains"], 32))
             if constants.net.rev_genesis_bytes() not in their_chains:
-                raise GracefulDisconnect(f"no common chain found with remote. (they sent: {their_chains})")
+                raise GracefulDisconnect(f"no common chain found with remote. "
+                                         f"(they sent: {[chain.hex() for chain in their_chains]}),"
+                                         f" our chain: {constants.net.rev_genesis_bytes().hex()}")
         # all checks passed
         self.lnworker.on_peer_successfully_established(self)
         self._received_init = True
@@ -2221,7 +2223,7 @@ async def maybe_forward_trampoline(
 
         # do we have a connection to the node?
         next_peer = self.lnworker.peers.get(outgoing_node_id)
-        if next_peer and next_peer.accepts_zeroconf():
+        if next_peer and next_peer.accepts_zeroconf() and self.lnworker.features.supports(LnFeatures.OPTION_ZEROCONF_OPT):
             self.logger.info(f'JIT: found next_peer')
             for next_chan in next_peer.channels.values():
                 if next_chan.can_pay(amt_to_forward):
diff --git a/electrum/lnwatcher.py b/electrum/lnwatcher.py
@@ -65,7 +65,7 @@ def add_channel(self, outpoint: str, address: str) -> None:
         cb = lambda: self.check_onchain_situation(address, outpoint)
         self.add_callback(address, cb)
 
-    async def unwatch_channel(self, address, funding_outpoint):
+    def unwatch_channel(self, address, funding_outpoint):
         self.logger.info(f'unwatching {funding_outpoint}')
         self.remove_callback(address)
 
@@ -128,7 +128,7 @@ async def check_onchain_situation(self, address, funding_outpoint):
             closing_height=closing_height,
             keep_watching=keep_watching)
         if not keep_watching:
-            await self.unwatch_channel(address, funding_outpoint)
+            self.unwatch_channel(address, funding_outpoint)
 
     async def sweep_commitment_transaction(self, funding_outpoint: str, closing_tx: Transaction) -> bool:
         raise NotImplementedError()  # implemented by subclasses
diff --git a/electrum/lnworker.py b/electrum/lnworker.py
@@ -76,6 +76,7 @@
     create_trampoline_route_and_onion, is_legacy_relay, trampolines_by_id, hardcoded_trampoline_nodes,
     is_hardcoded_trampoline
 )
+from .network import TxBroadcastServerReturnedError
 
 if TYPE_CHECKING:
     from .network import Network
@@ -1129,7 +1130,7 @@ def get_lightning_history(self) -> Dict[str, LightningHistoryItem]:
         balance_msat = sum([x.amount_msat for x in out.values()])
         lb = sum(chan.balance(LOCAL) if not chan.is_closed_or_closing() else 0
                 for chan in self.channels.values())
-        assert balance_msat  == lb
+        assert balance_msat  == lb, f"balance_msat: {balance_msat} != lb: {lb}"
         return out
 
     def get_groups_for_onchain_history(self) -> Dict[str, str]:
@@ -1250,6 +1251,7 @@ async def open_channel_just_in_time(
 
         # prevent settling the htlc until the channel opening was successfull so we can fail it if needed
         self.dont_settle_htlcs[payment_hash.hex()] = None
+        funding_tx = None
         try:
             funding_sat = 2 * (next_amount_msat_htlc // 1000) # try to fully spend htlcs
             password = self.wallet.get_unlocked_password() if self.wallet.has_password() else None
@@ -1268,10 +1270,9 @@ async def open_channel_just_in_time(
             )
             async def wait_for_channel():
                 while not next_chan.is_open():
-                    await asyncio.sleep(1)
+                    await asyncio.sleep(0.1)
             await util.wait_for2(wait_for_channel(), LN_P2P_NETWORK_TIMEOUT)
-            next_chan.save_remote_scid_alias(self._scid_alias_of_node(next_peer.pubkey))
-            self.logger.info(f'JIT channel is open')
+            self.logger.info(f'JIT channel is open (funding not broadcasted yet)')
             next_amount_msat_htlc -= channel_opening_fee
             # fixme: some checks are missing
             htlc = next_peer.send_htlc(
@@ -1282,30 +1283,82 @@ async def wait_for_channel():
                 onion=next_onion)
             async def wait_for_preimage():
                 while self.get_preimage(payment_hash) is None:
-                    await asyncio.sleep(1)
-            await util.wait_for2(wait_for_preimage(), LN_P2P_NETWORK_TIMEOUT)
-
-            # We have been paid and can broadcast
-            # todo: if broadcasting raise an exception, we should try to rebroadcast
-            await self.network.broadcast_transaction(funding_tx)
+                    await asyncio.sleep(0.1)
+            try:
+                await util.wait_for2(wait_for_preimage(), LN_P2P_NETWORK_TIMEOUT)
+            except asyncio.TimeoutError:
+                self.logger.info(
+                    f"jit opening didn't get preimage, removing chan {next_chan.get_id_for_log()} again")
+                await self.cleanup_failed_jit_channel(next_chan)
+                raise
         except OnionRoutingFailure:
             raise
         except Exception:
             raise OnionRoutingFailure(code=OnionFailureCode.TEMPORARY_NODE_FAILURE, data=b'')
+
+        disable_zeroconf = True
+        try:
+            # after 10 mins `update_unfunded_state` will remove the channel on client side so we can fail here too
+            await util.wait_for2(self.broadcast_jit_channel_and_wait_for_mempool(next_chan, funding_tx), 60 * 10 - 30)
+            disable_zeroconf = False
+        except Exception:
+            # the risk of the funding tx getting mined later is low as we weren't able to get it into the mempool
+            self.preimages.pop(payment_hash.hex(), None)
+            await self.cleanup_failed_jit_channel(next_chan)
+            raise OnionRoutingFailure(code=OnionFailureCode.TEMPORARY_NODE_FAILURE, data=b'')
         finally:
+            if disable_zeroconf:
+                self.logger.warning(f"disabling zeroconf channels to prevent further issues. Check your wallet for consistency.")
+                self.config.ACCEPT_ZEROCONF_CHANNELS = False
+                self.features &= ~LnFeatures.OPTION_ZEROCONF_OPT
             del self.dont_settle_htlcs[payment_hash.hex()]
 
         htlc_key = serialize_htlc_key(next_chan.get_scid_or_local_alias(), htlc.htlc_id)
         return htlc_key
 
+    async def cleanup_failed_jit_channel(self, chan: Channel):
+        """Closes a channel that has no published funding tx (e.g. in case of a failed jit open)"""
+        try:
+            # try to send shutdown to signal peer that channel is dead
+            await util.wait_for2(self.close_channel(chan.channel_id), LN_P2P_NETWORK_TIMEOUT)
+        except Exception:
+            self.logger.debug(f"chan shutdown to failed zeroconf peer failed ", exc_info=True)
+        chan.set_state(ChannelState.REDEEMED, force=True)
+        self.lnwatcher.adb.remove_transaction(chan.funding_outpoint.txid)
+        self.lnwatcher.unwatch_channel(chan.get_funding_address(), chan.funding_outpoint.to_str())
+        self.remove_channel(chan.channel_id)
+
+    async def broadcast_jit_channel_and_wait_for_mempool(self, channel: Channel, funding_tx: Transaction) -> None:
+        last_broadcast_attempt = 0
+        while True:
+            if time.time() - last_broadcast_attempt > 60:
+                try:
+                    await self.network.broadcast_transaction(funding_tx)
+                    self.logger.info(f"broadcasted jit channel open txid: {funding_tx.txid()}")
+                except TxBroadcastServerReturnedError:
+                    self.logger.error(f"we constructed a weird JIT funding tx. Reverting channel again.", exc_info=True)
+                    raise
+                except Exception:
+                    self.logger.warning(f"Broadcasting jit channel open tx {funding_tx.txid()} failed.", exc_info=True)
+                last_broadcast_attempt = time.time()
+
+            # check if the funding tx is at least in the mempool by now
+            funding_info = channel.get_funding_height()
+            if funding_info is not None:
+                _, height, _ = funding_info
+                if height > TX_HEIGHT_LOCAL:
+                    return
+
+            await asyncio.sleep(1)
+
     @log_exceptions
     async def open_channel_with_peer(
             self, peer, funding_sat, *,
             push_sat: int = 0,
             public: bool = False,
             zeroconf: bool = False,
             opening_fee: int = None,
-            password=None):
+            password=None) -> Tuple[Channel, PartialTransaction]:
         if self.config.ENABLE_ANCHOR_CHANNELS:
             self.wallet.unlock(password)
         coins = self.wallet.get_spendable_coins(None)
diff --git a/electrum/plugins/watchtower/watchtower.py b/electrum/plugins/watchtower/watchtower.py
@@ -189,7 +189,7 @@ async def f():
         return self.network.run_from_another_thread(f())
 
     async def unwatch_channel(self, address, funding_outpoint):
-        await super().unwatch_channel(address, funding_outpoint)
+        super().unwatch_channel(address, funding_outpoint)
         await self.sweepstore.remove_sweep_tx(funding_outpoint)
         await self.sweepstore.remove_channel(funding_outpoint)
         if funding_outpoint in self.tx_progress:
diff --git a/electrum/simple_config.py b/electrum/simple_config.py
@@ -830,7 +830,7 @@ def __setattr__(self, name, value):
     # anchor outputs channels
     ENABLE_ANCHOR_CHANNELS = ConfigVar('enable_anchor_channels', default=False, type_=bool)
     # zeroconf channels
-    ACCEPT_ZEROCONF_CHANNELS = ConfigVar('accept_zeroconf_channels', default=False, type_=bool)
+    ACCEPT_ZEROCONF_CHANNELS = ConfigVar('accept_zeroconf_channels', default=True, type_=bool)
     ZEROCONF_TRUSTED_NODE = ConfigVar('zeroconf_trusted_node', default='', type_=str)
     ZEROCONF_MIN_OPENING_FEE = ConfigVar('zeroconf_min_opening_fee', default=5000, type_=int)
 
diff --git a/tests/regtest.py b/tests/regtest.py
@@ -135,8 +135,17 @@ class TestLightningJIT(TestLightning):
         }
     }
 
-    def test_just_in_time(self):
-        self.run_shell(['just_in_time'])
+    def test_just_in_time_no_channel(self):
+        # jit payment to node without existing channels
+        self.run_shell(['just_in_time_no_channel'])
+
+    def test_just_in_time_existing_channels(self):
+        # payment should not open additional channels as there is already a sufficient channel
+        self.run_shell(['just_in_time_existing_channels'])
+
+    def test_just_in_time_failed_htlc(self):
+        # jit client fails htlc, LSP doesn't get preimage. No channel should be opened.
+        self.run_shell(['just_in_time_failed_htlc'])
 
 
 class TestLightningJITTrampoline(TestLightningJIT):
diff --git a/tests/regtest/regtest.sh b/tests/regtest/regtest.sh
@@ -481,7 +481,7 @@ if [[ $1 == "watchtower" ]]; then
     wait_until_spent $ctx_id $output_index  # alice's to_local gets punished
 fi
 
-if [[ $1 == "just_in_time" ]]; then
+if [[ $1 == "just_in_time_no_channel" ]]; then
     bob_node=$($bob nodeid)
     $alice setconfig zeroconf_trusted_node $bob_node
     $alice setconfig use_recoverable_channels false
@@ -493,7 +493,88 @@ if [[ $1 == "just_in_time" ]]; then
     echo "carol pays alice"
     # note: set amount to 0.001 to test failure: 'payment too low'
     invoice=$($alice add_request 0.01 --lightning -m "invoice" | jq -r ".lightning_invoice")
-    $carol lnpay $invoice
+    success=$($carol lnpay $invoice | jq -r ".success")
+    if [[ "$success" != "true" ]]; then
+        echo "jit payment failed"
+        exit 1
+    fi
+    # try again, multiple jit openings should work without issues
+    new_blocks 3
+    echo "carol pays alice again"
+    invoice=$($alice add_request 0.04 --lightning -m "invoice" | jq -r ".lightning_invoice")
+    success=$($carol lnpay $invoice | jq -r ".success")
+    if [[ "$success" != "true" ]]; then
+        echo "jit payment failed"
+        exit 1
+    fi
+    alice_chan_count=$($alice list_channels | jq '. | length')
+    if [[ "$alice_chan_count" != "2" ]]; then
+        echo "alice should have two jit channels"
+        exit 1
+    fi
+fi
+
+if [[ $1 == "just_in_time_existing_channels" ]]; then
+    bob_node=$($bob nodeid)
+    alice_node=$($alice nodeid)
+    $alice setconfig zeroconf_trusted_node $bob_node
+    $alice setconfig use_recoverable_channels false
+    wait_for_balance bob 1
+    wait_for_balance carol 1
+    echo "carol opens channel with bob"
+    $carol open_channel $bob_node 0.15 --password=''
+    echo "bob opens channel with alice"
+    $bob open_channel $alice_node 0.15 --password=''
+    new_blocks 3
+    wait_until_channel_open carol
+    wait_until_channel_open bob
+    echo "carol pays alice"
+    # this should not open an additional channel
+    invoice=$($alice add_request 0.12 --lightning -m "invoice" | jq -r ".lightning_invoice")
+    success=$($carol lnpay $invoice | jq -r ".success")
+    if [[ "$success" != "true" ]]; then
+        echo "jit payment failed"
+        exit 1
+    fi
+    alice_chan_count=$($alice list_channels | jq '. | length')
+    if [[ "$alice_chan_count" != "1" ]]; then
+        echo "alice should not have channel an additional channel"
+        exit 1
+    fi
+fi
+
+
+if [[ $1 == "just_in_time_failed_htlc" ]]; then
+    bob_node=$($bob nodeid)
+    $alice setconfig zeroconf_trusted_node $bob_node
+    $alice setconfig use_recoverable_channels false
+    $alice setconfig test_fail_htlcs_with_temp_node_failure true
+    wait_for_balance carol 1
+    echo "carol opens channel with bob"
+    $carol open_channel $bob_node 0.15 --password=''
+    new_blocks 3
+    wait_until_channel_open carol
+    echo "carol tries to pay alice (but alice will fail htlc)"
+    invoice=$($alice add_request 0.02 --lightning -m "invoice" | jq -r ".lightning_invoice")
+    success=$($carol lnpay $invoice | jq -r ".success") # this will fail
+
+    bob_chan_count=$($bob list_channels | jq '. | length')
+    if [[ "$bob_chan_count" != "1" ]]; then  # failed jit channels should be removed directly
+        echo "bob should have only one channel\n$($bob list_channels)"
+        exit 1
+    fi
+    alice_chan_state=$($alice list_channels | jq -r '.[0].state')
+    if [[ "$alice_chan_state" != "CLOSING" ]]; then
+        echo "alice failed JIT channel should be in closing state\n$($alice list_channels)"
+        exit 1
+    fi
+    # after a new block triggers one iteration of update_unfunded_state the channel should be removed
+    new_blocks 1
+    alice_chan_count=$($alice list_channels | jq '. | length')
+    if [[ "$alice_chan_count" != "0" ]]; then
+        echo "alice should have no channel\n$($alice list_channels)"
+        exit 1
+    fi
 fi
 
 if [[ $1 == "unixsockets" ]]; then
