Merge pull request #11001 from spesmilo/jit_forward_checks

### electrum/lnworker.py
@@ -78,7 +78,7 @@
 )
 from .lnonion import (
     decode_onion_error, OnionFailureCode, OnionRoutingFailure, OnionPacket,
-    ProcessedOnionPacket, calc_hops_data_for_payment, new_onion_packet,
+    ProcessedOnionPacket, calc_hops_data_for_payment, new_onion_packet, InvalidPayloadSize,
 )
 from .lnmsg import decode_msg
 from .lnrouter import (
@@ -4144,53 +4144,65 @@ def log_fail_reason(reason: str):
         else:
             next_peer = None
 
+        open_jit_channel = False
         if not next_chan and next_peer and next_peer.accepts_zeroconf():
             # check if an already existing channel can be used.
             # todo: split the payment
             for next_chan in next_peer.channels.values():
                 if next_chan.can_pay(next_amount_msat_htlc):
                     break
             else:
-                return await self.open_channel_just_in_time(
-                    next_peer=next_peer,
-                    next_amount_msat_htlc=next_amount_msat_htlc,
-                    next_cltv_abs=next_cltv_abs,
-                    payment_hash=htlc.payment_hash,
-                    next_onion=processed_onion.next_packet)
+                open_jit_channel = True
 
         local_height = chain.height()
-        if next_chan is None:
-            log_fail_reason(f"cannot find next_chan {next_chan_scid}")
-            raise OnionRoutingFailure(code=OnionFailureCode.UNKNOWN_NEXT_PEER, data=b'')
-        outgoing_chan_upd = next_chan.get_outgoing_gossip_channel_update(scid=next_chan_scid)[2:]
-        outgoing_chan_upd_len = len(outgoing_chan_upd).to_bytes(2, byteorder="big")
-        outgoing_chan_upd_message = outgoing_chan_upd_len + outgoing_chan_upd
-        if not next_chan.can_send_update_add_htlc():
-            log_fail_reason(
-                f"next_chan {next_chan.get_id_for_log()} cannot send ctx updates. "
-                f"chan state {next_chan.get_state()!r}, peer state: {next_chan.peer_state!r}")
-            raise OnionRoutingFailure(code=OnionFailureCode.TEMPORARY_CHANNEL_FAILURE, data=outgoing_chan_upd_message)
-        if not next_chan.can_pay(next_amount_msat_htlc):
-            log_fail_reason(f"transient error (likely due to insufficient funds): not next_chan.can_pay(amt)")
-            raise OnionRoutingFailure(code=OnionFailureCode.TEMPORARY_CHANNEL_FAILURE, data=outgoing_chan_upd_message)
-        if htlc.cltv_abs - next_cltv_abs < next_chan.forwarding_cltv_delta:
+        if open_jit_channel:
+            # The channel does not exist yet, so there is no channel_update to put in errors.
+            # Fee and cltv delta are those of the route hint in the invoice of the client,
+            # see calc_routing_hints_for_invoice. The opening fee is deducted from the forwarded amount.
+            outgoing_chan_upd_message = (0).to_bytes(2, byteorder="big")
+            forwarding_cltv_delta = 144
+            forwarding_fees = 0
+        else:
+            if next_chan is None:
+                log_fail_reason(f"cannot find next_chan {next_chan_scid}")
+                raise OnionRoutingFailure(code=OnionFailureCode.UNKNOWN_NEXT_PEER, data=b'')
+            outgoing_chan_upd = next_chan.get_outgoing_gossip_channel_update(scid=next_chan_scid)[2:]
+            outgoing_chan_upd_len = len(outgoing_chan_upd).to_bytes(2, byteorder="big")
+            outgoing_chan_upd_message = outgoing_chan_upd_len + outgoing_chan_upd
+            if not next_chan.can_send_update_add_htlc():
+                log_fail_reason(
+                    f"next_chan {next_chan.get_id_for_log()} cannot send ctx updates. "
+                    f"chan state {next_chan.get_state()!r}, peer state: {next_chan.peer_state!r}")
+                raise OnionRoutingFailure(code=OnionFailureCode.TEMPORARY_CHANNEL_FAILURE, data=outgoing_chan_upd_message)
+            if not next_chan.can_pay(next_amount_msat_htlc):
+                log_fail_reason(f"transient error (likely due to insufficient funds): not next_chan.can_pay(amt)")
+                raise OnionRoutingFailure(code=OnionFailureCode.TEMPORARY_CHANNEL_FAILURE, data=outgoing_chan_upd_message)
+            forwarding_cltv_delta = next_chan.forwarding_cltv_delta
+            forwarding_fees = fee_for_edge_msat(
+                forwarded_amount_msat=next_amount_msat_htlc,
+                fee_base_msat=next_chan.forwarding_fee_base_msat,
+                fee_proportional_millionths=next_chan.forwarding_fee_proportional_millionths)
+        if htlc.cltv_abs - next_cltv_abs < forwarding_cltv_delta:
             log_fail_reason(
                 f"INCORRECT_CLTV_EXPIRY. "
-                f"{htlc.cltv_abs=} - {next_cltv_abs=} < {next_chan.forwarding_cltv_delta=}")
+                f"{htlc.cltv_abs=} - {next_cltv_abs=} < {forwarding_cltv_delta=}")
             data = htlc.cltv_abs.to_bytes(4, byteorder="big") + outgoing_chan_upd_message
             raise OnionRoutingFailure(code=OnionFailureCode.INCORRECT_CLTV_EXPIRY, data=data)
         if htlc.cltv_abs - lnutil.MIN_FINAL_CLTV_DELTA_ACCEPTED <= local_height \
                 or next_cltv_abs <= local_height:
             raise OnionRoutingFailure(code=OnionFailureCode.EXPIRY_TOO_SOON, data=outgoing_chan_upd_message)
         if max(htlc.cltv_abs, next_cltv_abs) > local_height + lnutil.NBLOCK_CLTV_DELTA_TOO_FAR_INTO_FUTURE:
             raise OnionRoutingFailure(code=OnionFailureCode.EXPIRY_TOO_FAR, data=b'')
-        forwarding_fees = fee_for_edge_msat(
-            forwarded_amount_msat=next_amount_msat_htlc,
-            fee_base_msat=next_chan.forwarding_fee_base_msat,
-            fee_proportional_millionths=next_chan.forwarding_fee_proportional_millionths)
         if htlc.amount_msat - next_amount_msat_htlc < forwarding_fees:
             data = next_amount_msat_htlc.to_bytes(8, byteorder="big") + outgoing_chan_upd_message
             raise OnionRoutingFailure(code=OnionFailureCode.FEE_INSUFFICIENT, data=data)
+        if open_jit_channel:
+            return await self.open_channel_just_in_time(
+                next_peer=next_peer,
+                next_amount_msat_htlc=next_amount_msat_htlc,
+                next_cltv_abs=next_cltv_abs,
+                payment_hash=htlc.payment_hash,
+                next_onion=processed_onion.next_packet)
         self.logger.info(
             f"maybe_forward_htlc. will forward HTLC: inc_chan={incoming_chan.short_channel_id}. inc_htlc={str(htlc)}. "
             f"next_chan={next_chan.get_id_for_log()}.")
@@ -4282,25 +4294,40 @@ async def _maybe_forward_trampoline(
                     break
             # open JIT channel
             if not direct_channels and next_peer.accepts_zeroconf() and self.features.supports(LnFeatures.OPTION_ZEROCONF_OPT):
-                scid_alias = self._scid_alias_of_node(next_peer.pubkey)
-                route = [RouteEdge(
-                    start_node=next_peer.pubkey,
-                    end_node=outgoing_node_id,
-                    short_channel_id=scid_alias,
-                    fee_base_msat=0,
-                    fee_proportional_millionths=0,
-                    cltv_delta=144,
-                    node_features=0
-                )]
-                next_onion, amount_msat, cltv_abs, session_key = self.create_onion_for_route(
-                    route=route,
-                    amount_msat=amt_to_forward,
-                    total_msat=amt_to_forward,
-                    payment_hash=payment_hash,
-                    min_final_cltv_delta=cltv_budget_for_rest_of_route,
-                    payment_secret=payment_secret,
-                    trampoline_onion=next_trampoline_onion,
-                )
+                # The outgoing cltv is local_height + cltv_budget_for_rest_of_route, chosen by the sender.
+                # Check it here, as an exception other than OnionRoutingFailure would not fail the incoming htlcs.
+                if cltv_budget_for_rest_of_route < lnutil.MIN_FINAL_CLTV_DELTA_ACCEPTED:
+                    raise OnionRoutingFailure(code=OnionFailureCode.TRAMPOLINE_EXPIRY_TOO_SOON, data=b'')
+                if cltv_budget_for_rest_of_route > lnutil.NBLOCK_CLTV_DELTA_TOO_FAR_INTO_FUTURE:
+                    raise OnionRoutingFailure(code=OnionFailureCode.EXPIRY_TOO_FAR, data=b'')
+                try:
+                    scid_alias = self._scid_alias_of_node(next_peer.pubkey)
+                    route = [RouteEdge(
+                        start_node=next_peer.pubkey,
+                        end_node=outgoing_node_id,
+                        short_channel_id=scid_alias,
+                        fee_base_msat=0,
+                        fee_proportional_millionths=0,
+                        cltv_delta=144,
+                        node_features=0
+                    )]
+                    next_onion, amount_msat, cltv_abs, session_key = self.create_onion_for_route(
+                        route=route,
+                        amount_msat=amt_to_forward,
+                        total_msat=amt_to_forward,
+                        payment_hash=payment_hash,
+                        min_final_cltv_delta=cltv_budget_for_rest_of_route,
+                        payment_secret=payment_secret,
+                        trampoline_onion=next_trampoline_onion,
+                    )
+                except InvalidPayloadSize as e:
+                    raise OnionRoutingFailure(code=OnionFailureCode.INVALID_ONION_PAYLOAD, data=b'\x00\x00\x00')
+                except BaseException:
+                    self.logger.exception('failed to create onion for JIT channel')
+                    raise OnionRoutingFailure(code=OnionFailureCode.TEMPORARY_NODE_FAILURE, data=b'')
+                # same cltv delta as the JIT route hint, see calc_routing_hints_for_invoice
+                if closest_inc_cltv_abs - cltv_abs < 144:
+                    raise OnionRoutingFailure(code=OnionFailureCode.TRAMPOLINE_EXPIRY_TOO_SOON, data=b'')
                 await self.open_channel_just_in_time(
                     next_peer=next_peer,
                     next_amount_msat_htlc=amt_to_forward,
@@ -4309,6 +4336,9 @@ async def _maybe_forward_trampoline(
                     next_onion=next_onion)
                 return
 
+        # Here we only check that there is some budget, before calling pay_to_node,
+        # which is expensive (calls create_route_for_single_htlc in another thread)
+        # The real check (comparing our budget to the actual route) will be in pay_to_node
         if budget.fee_msat < (1000 if not direct_channels else 0):
             raise OnionRoutingFailure(code=OnionFailureCode.TRAMPOLINE_FEE_INSUFFICIENT, data=b'')
         if budget.cltv < (576 if not direct_channels else 0):

### tests/test_lnwallet.py
@@ -16,7 +16,8 @@
 from electrum.channel_db import UpdateStatus
 from electrum.lnutil import (
     RECEIVED, SENT, MIN_FINAL_CLTV_DELTA_ACCEPTED, serialize_htlc_key, LnFeatures, HTLCOwner, PaymentFailure,
-    LOCAL, REMOTE, ImportedChannelBackupStorage, make_commitment_output_to_anchor_address,
+    LOCAL, REMOTE, ImportedChannelBackupStorage, make_commitment_output_to_anchor_address, UpdateAddHtlc,
+    NBLOCK_CLTV_DELTA_TOO_FAR_INTO_FUTURE,
 )
 from electrum.logging import console_stderr_handler
 from electrum.lnmsg import decode_msg
@@ -26,7 +27,9 @@
 from electrum.invoices import LN_EXPIRY_NEVER, PR_UNPAID, PR_INFLIGHT, Invoice
 from electrum.lnpeer import Peer
 from electrum.lnchannel import Channel, ChannelBackup, ChannelState
-from electrum.lnonion import OnionPacket, OnionRoutingFailure, OnionFailureCode
+from electrum.lnonion import (
+    OnionPacket, OnionRoutingFailure, OnionFailureCode, ProcessedOnionPacket, OnionHopsDataSingle,
+)
 from electrum.mpp_split import SplitConfig, SplitConfigRating
 from electrum.crypto import sha256, pw_encode_with_version_and_mac
 from electrum.simple_config import SimpleConfig
@@ -379,6 +382,128 @@ async def test_open_channel_just_in_time_config_disabled(self):
                 next_onion=mock.Mock(spec=OnionPacket)
             )
 
+    async def test_maybe_forward_htlc_jit_checks_incoming_htlc(self):
+        """before opening a just-in-time channel, the incoming htlc must pass the same
+        amount and cltv checks as for a regular forward"""
+        wallet = self.lnwallet_anchors
+        wallet.config.EXPERIMENTAL_LN_FORWARD_PAYMENTS = True
+        wallet.lnpeermgr.features |= LnFeatures.OPTION_ZEROCONF_OPT
+        next_peer = mock.Mock(spec=Peer)
+        next_peer.accepts_zeroconf.return_value = True
+        next_peer.channels = {}
+        wallet.get_peer_by_static_jit_scid_alias = lambda scid: next_peer
+        wallet.open_channel_just_in_time = mock.AsyncMock(return_value='htlc_key')
+        height = wallet.network.get_local_height()
+        next_amount_msat = 1_000_000
+        next_cltv_abs = height + 1000
+
+        async def forward(*, amount_msat: int, cltv_abs: int, next_cltv_abs: int = next_cltv_abs) -> str:
+            htlc = UpdateAddHtlc(
+                amount_msat=amount_msat,
+                payment_hash=os.urandom(32),
+                cltv_abs=cltv_abs,
+                htlc_id=0,
+                timestamp=int(time.time()))
+            processed_onion = ProcessedOnionPacket(
+                are_we_final=False,
+                hop_data=OnionHopsDataSingle(payload={
+                    'short_channel_id': {'short_channel_id': os.urandom(8)},
+                    'amt_to_forward': {'amt_to_forward': next_amount_msat},
+                    'outgoing_cltv_value': {'outgoing_cltv_value': next_cltv_abs},
+                }),
+                next_packet=mock.Mock(spec=OnionPacket),
+                trampoline_onion_packet=None)
+            return await wallet._maybe_forward_htlc(
+                incoming_chan=mock.Mock(spec=Channel),
+                htlc=htlc,
+                processed_onion=processed_onion)
+
+        with self.subTest(msg="incoming amount lower than forwarded amount"):
+            with self.assertRaises(OnionRoutingFailure) as ctx:
+                await forward(amount_msat=next_amount_msat - 1, cltv_abs=next_cltv_abs + 144)
+            self.assertEqual(ctx.exception.code, OnionFailureCode.FEE_INSUFFICIENT)
+        with self.subTest(msg="incoming cltv delta too small"):
+            with self.assertRaises(OnionRoutingFailure) as ctx:
+                await forward(amount_msat=next_amount_msat, cltv_abs=next_cltv_abs + 143)
+            self.assertEqual(ctx.exception.code, OnionFailureCode.INCORRECT_CLTV_EXPIRY)
+        with self.subTest(msg="incoming htlc expires too soon"):
+            with self.assertRaises(OnionRoutingFailure) as ctx:
+                await forward(
+                    amount_msat=next_amount_msat,
+                    cltv_abs=height + MIN_FINAL_CLTV_DELTA_ACCEPTED,
+                    next_cltv_abs=height + MIN_FINAL_CLTV_DELTA_ACCEPTED - 144)
+            self.assertEqual(ctx.exception.code, OnionFailureCode.EXPIRY_TOO_SOON)
+            self.assertEqual(ctx.exception.data, b'\x00\x00')  # empty channel_update
+        wallet.open_channel_just_in_time.assert_not_called()
+
+        with self.subTest(msg="valid htlc opens the channel"):
+            htlc_key = await forward(amount_msat=next_amount_msat, cltv_abs=next_cltv_abs + 144)
+            self.assertEqual(htlc_key, 'htlc_key')
+            wallet.open_channel_just_in_time.assert_awaited_once()
+            kwargs = wallet.open_channel_just_in_time.call_args.kwargs
+            self.assertEqual(kwargs['next_amount_msat_htlc'], next_amount_msat)
+            self.assertEqual(kwargs['next_cltv_abs'], next_cltv_abs)
+
+    async def test_maybe_forward_trampoline_jit_checks_outgoing_cltv(self):
+        """the outgoing cltv of a trampoline forward to a just-in-time channel is chosen by the sender.
+        It must be checked before creating the onion, so that the incoming htlcs get failed"""
+        wallet = self.lnwallet_anchors
+        wallet.config.EXPERIMENTAL_LN_FORWARD_PAYMENTS = True
+        wallet.config.EXPERIMENTAL_LN_FORWARD_TRAMPOLINE_PAYMENTS = True
+        wallet.lnpeermgr.features |= LnFeatures.OPTION_ZEROCONF_OPT
+        next_node_id = ecc.ECPrivkey.generate_random_key().get_public_key_bytes()
+        next_peer = mock.Mock(spec=Peer)
+        next_peer.pubkey = next_node_id
+        next_peer.accepts_zeroconf.return_value = True
+        next_peer.channels = {}
+        wallet.lnpeermgr.get_peer_by_pubkey = lambda pubkey: next_peer if pubkey == next_node_id else None
+        wallet.open_channel_just_in_time = mock.AsyncMock(return_value='htlc_key')
+        height = wallet.network.get_local_height()
+        amount_msat = 1_000_000
+
+        async def forward(*, out_cltv_abs: int, cltv_delta: int = 576) -> None:
+            trampoline_onion = ProcessedOnionPacket(
+                are_we_final=False,
+                hop_data=OnionHopsDataSingle(payload={
+                    'outgoing_node_id': {'outgoing_node_id': next_node_id},
+                    'amt_to_forward': {'amt_to_forward': amount_msat},
+                    'outgoing_cltv_value': {'outgoing_cltv_value': out_cltv_abs},
+                    'invoice_features': {'invoice_features': 0},
+                    'invoice_routing_info': {'invoice_routing_info': b''},
+                }),
+                next_packet=mock.Mock(spec=OnionPacket),
+                trampoline_onion_packet=None)
+            await wallet._maybe_forward_trampoline(
+                payment_hash=os.urandom(32),
+                closest_inc_cltv_abs=out_cltv_abs + cltv_delta,
+                total_msat=amount_msat,
+                any_trampoline_onion=trampoline_onion,
+                fw_payment_key='fw_payment_key')
+
+        with self.subTest(msg="outgoing htlc already expired"):
+            with self.assertRaises(OnionRoutingFailure) as ctx:
+                await forward(out_cltv_abs=height - 1)
+            self.assertEqual(ctx.exception.code, OnionFailureCode.TRAMPOLINE_EXPIRY_TOO_SOON)
+        with self.subTest(msg="outgoing htlc expires too soon for the recipient"):
+            with self.assertRaises(OnionRoutingFailure) as ctx:
+                await forward(out_cltv_abs=height + MIN_FINAL_CLTV_DELTA_ACCEPTED - 2)
+            self.assertEqual(ctx.exception.code, OnionFailureCode.TRAMPOLINE_EXPIRY_TOO_SOON)
+        with self.subTest(msg="outgoing htlc expires too far into the future"):
+            with self.assertRaises(OnionRoutingFailure) as ctx:
+                await forward(out_cltv_abs=height + NBLOCK_CLTV_DELTA_TOO_FAR_INTO_FUTURE)
+            self.assertEqual(ctx.exception.code, OnionFailureCode.EXPIRY_TOO_FAR)
+        with self.subTest(msg="incoming cltv delta too small"):
+            with self.assertRaises(OnionRoutingFailure) as ctx:
+                await forward(out_cltv_abs=height + 1000, cltv_delta=144)
+            self.assertEqual(ctx.exception.code, OnionFailureCode.TRAMPOLINE_EXPIRY_TOO_SOON)
+        wallet.open_channel_just_in_time.assert_not_called()
+
+        with self.subTest(msg="latest accepted outgoing cltv opens the channel"):
+            await forward(out_cltv_abs=height + NBLOCK_CLTV_DELTA_TOO_FAR_INTO_FUTURE - 1)
+            wallet.open_channel_just_in_time.assert_awaited_once()
+            kwargs = wallet.open_channel_just_in_time.call_args.kwargs
+            self.assertEqual(kwargs['next_cltv_abs'], height + NBLOCK_CLTV_DELTA_TOO_FAR_INTO_FUTURE)
+
     async def test_cleanup_failed_jit_channel(self):
         wallet = self.lnwallet_anchors