Merge pull request #10929 from spesmilo/fix_trampoline_forwarding

### electrum/lnpeer.py
@@ -106,11 +106,13 @@ 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 \
-            self.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
+        forwarding = self.config.EXPERIMENTAL_LN_FORWARD_PAYMENTS or self.config.EXPERIMENTAL_LN_FORWARD_TRAMPOLINE_PAYMENTS
+        if lnworker == lnworker.network.lngossip \
+                or self.config.ZEROCONF_TRUSTED_NODE \
+                and pubkey != lnworker.trusted_zeroconf_node_id \
+                and not forwarding:
+            # clients signal to their trusted provider only, forwarding wallets also need to signal to peers they might fund
+            self.features &= ~(LnFeatures.OPTION_ZEROCONF_OPT | LnFeatures.OPTION_ZEROCONF_REQ)
         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]
@@ -1301,8 +1303,11 @@ async def on_open_channel(self, payload):
                 raise Exception("refusing to open new static_remotekey channel")
 
         is_zeroconf = bool(channel_type & ChannelType.OPTION_ZEROCONF)
-        if is_zeroconf and not self.config.ZEROCONF_TRUSTED_NODE.startswith(self.pubkey.hex()):
-            raise Exception(f"not accepting zeroconf from node {self.pubkey}")
+        if is_zeroconf:
+            if self.pubkey != self.lnworker.trusted_zeroconf_node_id:
+                raise Exception(f"not accepting zeroconf from node {self.pubkey}")
+            if self.config.EXPERIMENTAL_LN_FORWARD_PAYMENTS or self.config.EXPERIMENTAL_LN_FORWARD_TRAMPOLINE_PAYMENTS:
+                raise Exception(f"not accepting zeroconf as a forwarding node")
 
         if self.lnworker.has_recoverable_channels() and not is_zeroconf:
             # FIXME: we might want to keep the connection open
@@ -3060,6 +3065,13 @@ def _check_unfulfilled_htlc_set(
         Optional[Callable[[], Coroutine[Any, Any, None]]],  # callback
     ]:
         """
+        There are 5 types of htlc sets:
+            * non-trampoline, final
+            * non-trampoline, to be forwarded (cannot be MPP)
+            * trampoline, final, first stage
+            * trampoline, final, second stage
+            * trampoline, to be forwarded
+
         Returns what to do next with the given set of htlcs:
             * Fail whole set -> returns error code
             * Settle whole set -> Returns preimage
@@ -3079,37 +3091,35 @@ def _check_unfulfilled_htlc_set(
             return OnionFailureCode.TEMPORARY_NODE_FAILURE, None, None
 
         amount_msat: int = 0  # sum(amount_msat of each htlc)
-        total_msat = None  # type: Optional[int]
+        total_msat_inner_onion = None  # type: Optional[int]
+        total_msat_outer_onion = None  # type: Optional[int]
+        payment_secrets = set()
         payment_hash = mpp_set.get_payment_hash()
         closest_cltv_abs = mpp_set.get_closest_cltv_abs()
         first_htlc_timestamp = mpp_set.get_first_htlc_timestamp()
         processed_onions = {}  # type: dict[ReceivedMPPHtlc, Tuple[ProcessedOnionPacket, Optional[ProcessedOnionPacket]]]
         for mpp_htlc in mpp_set.htlcs:
-            processed_onion = self._process_incoming_onion_packet(
+            outer_onion = self._process_incoming_onion_packet(
                 onion_packet=self._parse_onion_packet(mpp_htlc.unprocessed_onion),
                 payment_hash=payment_hash,
                 is_trampoline=False,  # this is always the outer onion
             )
-            processed_onions[mpp_htlc] = (processed_onion, None)
-            inner_onion = None
-            if processed_onion.trampoline_onion_packet:
-                inner_onion = self._process_incoming_onion_packet(
-                    onion_packet=processed_onion.trampoline_onion_packet,
-                    payment_hash=payment_hash,
-                    is_trampoline=True,
-                )
-                processed_onions[mpp_htlc] = (processed_onion, inner_onion)
-
-            total_msat_outer_onion = processed_onion.total_msat
-            total_msat_inner_onion = inner_onion.total_msat if inner_onion else None
-            if total_msat is None:
-                total_msat = total_msat_inner_onion or total_msat_outer_onion
-
+            inner_onion = self._process_incoming_onion_packet(
+                onion_packet=outer_onion.trampoline_onion_packet,
+                payment_hash=payment_hash,
+                is_trampoline=True,
+            ) if outer_onion.trampoline_onion_packet else None
+            processed_onions[mpp_htlc] = (outer_onion, inner_onion)
+            payment_secrets.add(outer_onion.payment_secret)
             # check total_msat is equal for all htlcs of the set
-            if total_msat != (total_msat_inner_onion or total_msat_outer_onion):
-                _log_fail_reason(f"total_msat is not uniform: {total_msat=} != {processed_onion.total_msat=}")
-                return OnionFailureCode.INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS, None, None
-
+            if total_msat_outer_onion is None:
+                total_msat_outer_onion = outer_onion.total_msat
+            elif total_msat_outer_onion != outer_onion.total_msat:
+                if len(payment_secrets) == 1:
+                    _log_fail_reason(f"total_msat is inconsistent across outer_onions: {total_msat_outer_onion=} {outer_onion.total_msat=}")
+                    return OnionFailureCode.INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS, None, None
+            # total_msat of inner onions will be compared below (compare_trampoline_onions)
+            total_msat_inner_onion = inner_onion.total_msat if inner_onion else None
             amount_msat += mpp_htlc.htlc.amount_msat
 
         # If the set contains outer onions with different payment secrets, the set's payment_key is
@@ -3118,8 +3128,7 @@ def _check_unfulfilled_htlc_set(
         # In this case the amt_to_forward cannot be compared as it may differ between the trampoline parts.
         # However, amt_to_forward should be similar for all onions of a single trampoline part and gets
         # compared in the first stage where the htlc set represents a single trampoline part.
-        outer_onions = [onions[0] for onions in processed_onions.values()]
-        can_have_different_amt_to_fwd = not all(o.payment_secret == outer_onions[0].payment_secret for o in outer_onions)
+        can_have_different_amt_to_fwd = len(payment_secrets) > 1
         trampoline_onions = iter(onions[1] for onions in processed_onions.values())
         if not lnonion.compare_trampoline_onions(trampoline_onions, exclude_amt_to_fwd=can_have_different_amt_to_fwd):
             _log_fail_reason(f"got inconsistent {trampoline_onions=}")
@@ -3137,7 +3146,7 @@ def _check_unfulfilled_htlc_set(
                 fwd_cb = lambda: self.lnworker.maybe_forward_htlc_set(payment_key, processed_htlc_set=processed_onions)
                 return None, None, fwd_cb
 
-        assert payment_hash is not None and total_msat is not None
+        assert payment_hash is not None and total_msat_outer_onion is not None
         # check for expiry over time and potentially fail the whole set if any
         # htlc's cltv becomes too close
         blocks_to_expiry = max(0, closest_cltv_abs - local_height)
@@ -3192,12 +3201,27 @@ def _check_unfulfilled_htlc_set(
                     self.lnworker.received_mpp_htlcs[payment_key] = mpp_set._replace(
                         parent_set_key=trampoline_payment_key,
                     )
-            elif amount_msat >= (total_msat - jit_opening_fees_msat):  # regular mpp or 2nd stage trampoline
-                # set mpp_set as completed as we have received the full total_msat
-                mpp_set = self.lnworker.set_mpp_resolution(
-                    payment_key=payment_key,
-                    new_resolution=RecvMPPResolution.COMPLETE,
-                )
+            else:
+                if not any_trampoline_onion:
+                    # regular mpp
+                    total_msat = total_msat_outer_onion
+                elif not any_trampoline_onion.are_we_final:
+                    # trampoline forwarding
+                    if jit_opening_fees_msat != 0:
+                        _log_fail_reason("not accepting zeroconf channels if forwarding is enabled")
+                        return OnionFailureCode.TEMPORARY_NODE_FAILURE, None, None
+                    total_msat = total_msat_outer_onion
+                else:
+                    # 2nd stage trampoline
+                    assert trampoline_payment_key == payment_key
+                    total_msat = total_msat_inner_onion
+
+                if amount_msat >= (total_msat - jit_opening_fees_msat):
+                    # set mpp_set as completed as we have received the full total_msat
+                    mpp_set = self.lnworker.set_mpp_resolution(
+                        payment_key=payment_key,
+                        new_resolution=RecvMPPResolution.COMPLETE,
+                    )
 
         # check if this set is a trampoline forwarding and potentially return forwarding callback
         # note: all inner trampoline onions are equal (enforced above)

### electrum/lnworker.py
@@ -3028,7 +3028,7 @@ def update_or_create_mpp_with_received_htlc(
         if mpp_status.resolution > RecvMPPResolution.WAITING:
             # we are getting a htlc for a set that is not in WAITING state, it cannot be safely added
             self.logger.info(f"htlc set cannot accept htlc, failing htlc: {channel_id=} {htlc.htlc_id=}")
-            if mpp_status == RecvMPPResolution.EXPIRED:
+            if mpp_status.resolution == RecvMPPResolution.EXPIRED:
                 raise OnionRoutingFailure(code=OnionFailureCode.MPP_TIMEOUT, data=b'')
             raise OnionRoutingFailure(
                 code=OnionFailureCode.INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS,
@@ -3532,6 +3532,8 @@ def receive_requires_jit_channel(self, amount_msat: Optional[int]) -> bool:
     def can_get_zeroconf_channel(self) -> bool:
         if not self.config.OPEN_ZEROCONF_CHANNELS:
             return False
+        if self.config.EXPERIMENTAL_LN_FORWARD_PAYMENTS or self.config.EXPERIMENTAL_LN_FORWARD_TRAMPOLINE_PAYMENTS:
+            return False
         node_id = self.trusted_zeroconf_node_id
         if not node_id:
             return False
@@ -4026,10 +4028,13 @@ async def maybe_forward_htlc_set(
                 min_inc_cltv_abs = min(
                     mpp_htlc.htlc.cltv_abs
                     for mpp_htlc in processed_htlc_set.keys())  # take "min" to assume worst-case
+                total_msat = any_outer_onion.total_msat
+                sum_inc_amt_msat = sum(mpp_htlc.htlc.amount_msat for mpp_htlc in processed_htlc_set)
+                assert total_msat <= sum_inc_amt_msat, f"{total_msat=} should be <= {sum_inc_amt_msat=}"
                 await self._maybe_forward_trampoline(
                     payment_hash=any_mpp_htlc.htlc.payment_hash,
                     closest_inc_cltv_abs=min_inc_cltv_abs,
-                    total_msat=any_outer_onion.total_msat,
+                    total_msat=total_msat,
                     any_trampoline_onion=any_trampoline_onion,
                     fw_payment_key=payment_key,
                 )
@@ -4163,7 +4168,7 @@ async def _maybe_forward_trampoline(
             self, *,
             payment_hash: bytes,
             closest_inc_cltv_abs: int,
-            total_msat: int,  # total_msat of the outer onion
+            total_msat: int,  # total_msat of the outer onion. this is <= sum_inc_amt_msat
             any_trampoline_onion: ProcessedOnionPacket,  # any trampoline onion of the incoming htlc set, they should be similar
             fw_payment_key: str,
     ) -> None:
@@ -4197,6 +4202,7 @@ async def _maybe_forward_trampoline(
             self.logger.exception('')
             raise OnionRoutingFailure(code=OnionFailureCode.INVALID_ONION_PAYLOAD, data=b'\x00\x00\x00')
 
+        assert total_msat >= amt_to_forward  # sanity check: money_in >= money_out
         # these are the fee/cltv paid by the sender
         # pay_to_node will raise if they are not sufficient
         budget = PaymentFeeBudget(

### electrum/wallet.py
@@ -3607,12 +3607,7 @@ def get_help_texts_for_receive_request(self, req: Request) -> ReceiveRequestHelp
         lightning_online = self.lnworker and self.lnworker.lnpeermgr.num_peers() > 0
         num_sats_can_receive = self.lnworker.num_sats_can_receive() if self.lnworker else 0
         can_receive_lightning = self.lnworker and num_sats_can_receive > 0 and amount_sat <= num_sats_can_receive
-        try:
-            zeroconf_nodeid = extract_nodeid(self.config.ZEROCONF_TRUSTED_NODE)[0]
-        except Exception:
-            zeroconf_nodeid = None
-        can_get_zeroconf_channel = (self.lnworker and self.config.OPEN_ZEROCONF_CHANNELS
-                                    and self.lnworker.lnpeermgr.get_peer_by_pubkey(zeroconf_nodeid) is not None)
+        can_get_zeroconf_channel = self.lnworker and self.lnworker.can_get_zeroconf_channel()
         status = self.get_invoice_status(req)
 
         if status == PR_EXPIRED:

### tests/test_lnpeer.py
@@ -2728,6 +2728,33 @@ def modified_new_onion_packet_lnworker(payment_path_pubkeys, session_key, hops_d
         assert len(bob_hm.all_htlcs_ever()) == 2
         assert all(bob_hm.was_htlc_failed(htlc_id=htlc.htlc_id, htlc_proposer=HTLCOwner.REMOTE) for (_, htlc) in bob_hm.all_htlcs_ever())
 
+    async def test_trampoline_forwarder_waits_for_outer_onion_total_msat(self):
+        """
+        A trampoline forwarder must only forward once the htlcs it received sum up to the total_msat
+        of the outer onion.
+        Alice claims in the outer onion that twice the htlc amount will arrive, but only sends one htlc.
+        Bob must not forward the incomplete set and has to fail it with MPP_TIMEOUT.
+        """
+        def modified_new_onion_packet_lnworker(payment_path_pubkeys, session_key, hops_data: List[OnionHopsDataSingle], **kwargs):
+            hops_data = copy.copy(hops_data)
+            payload = dict(hops_data[-1].payload)
+            if 'trampoline_onion_packet' in payload:  # payload is alice's outer onion for bob
+                payment_data = dict(payload['payment_data'])
+                payment_data['total_msat'] *= 2  # bob should expect double the amount she actually receives
+                payload['payment_data'] = payment_data
+                hops_data[-1] = dataclasses.replace(hops_data[-1], payload=payload)
+            return electrum.lnonion.new_onion_packet(payment_path_pubkeys, session_key, hops_data, **kwargs)
+
+        graph = self.create_square_graph(direct=False, is_legacy=True)
+        alice = graph.workers['alice']
+        alice.config.INITIAL_TRAMPOLINE_FEE_LEVEL = 6  # set high so the payment would succeed if bob forwarded
+        with self.assertLogs('electrum', level='INFO') as logs, self.assertRaises(NoPathFound):
+            with mock.patch('electrum.lnworker.new_onion_packet', side_effect=modified_new_onion_packet_lnworker):
+                await self._run_trampoline_payment(graph, attempts=1)
+        self.assertTrue(any('MPP TIMEOUT' in record.getMessage() for record in logs.records))
+        bob_carol_channel = graph.channels[('bob', 'carol')][0]
+        self.assertEqual(0, len(bob_carol_channel.hm.all_htlcs_ever()))
+
     async def test_payment_with_malformed_onion(self):
         """
         Alice -> Bob -> Carol. Carol fails htlc with update_fail_malformed_htlc because she is unable