Merge pull request #9733 from f321x/fix_too_large_onion_payload

lightning: stop including all invoice r_tags in legacy trampoline onion

electrum/lnrouter.py

@@ -115,7 +115,7 @@ class RouteEdge(PathEdge):
 
 @attr.s
 class TrampolineEdge(RouteEdge):
-    invoice_routing_info = attr.ib(type=bytes, default=None)
+    invoice_routing_info = attr.ib(type=Sequence[bytes], default=None)
     invoice_features = attr.ib(type=int, default=None)
     # this is re-defined from parent just to specify a default value:
     short_channel_id = attr.ib(default=ShortChannelID(8), repr=lambda val: str(val))

electrum/trampoline.py

@@ -1,15 +1,18 @@
 import io
 import os
 import random
-from typing import Mapping, DefaultDict, Tuple, Optional, Dict, List, Iterable, Sequence, Set, Any
+from typing import Mapping, DefaultDict, Tuple, Optional, Dict, List, Iterable, Sequence, Set, Any, \
+    MutableSequence
 
 from .lnutil import LnFeatures, PaymentFeeBudget, FeeBudgetExceeded
-from .lnonion import calc_hops_data_for_payment, new_onion_packet, OnionPacket
+from .lnonion import calc_hops_data_for_payment, new_onion_packet, OnionPacket, \
+    TRAMPOLINE_HOPS_DATA_SIZE, PER_HOP_HMAC_SIZE
 from .lnrouter import RouteEdge, TrampolineEdge, LNPaymentRoute, is_route_within_budget, LNPaymentTRoute
 from .lnutil import NoPathFound
 from .lntransport import LNPeerAddr
 from . import constants
 from .logging import get_logger
+from .util import random_shuffled_copy
 
 
 _logger = get_logger(__name__)
@@ -55,10 +58,10 @@ def trampolines_by_id():
 def is_hardcoded_trampoline(node_id: bytes) -> bool:
     return node_id in trampolines_by_id()
 
-def encode_routing_info(r_tags: Sequence[Sequence[Sequence[Any]]]) -> bytes:
+def encode_routing_info(r_tags: Sequence[Sequence[Sequence[Any]]]) -> List[bytes]:
-    result = bytearray()
+    routes = []
     for route in r_tags:
-        result += bytes([len(route)])
+        result = bytes([len(route)])
         for step in route:
             pubkey, scid, feebase, feerate, cltv = step
             result += pubkey
@@ -66,7 +69,8 @@ def encode_routing_info(r_tags: Sequence[Sequence[Sequence[Any]]]) -> bytes:
             result += int.to_bytes(feebase, length=4, byteorder="big", signed=False)
             result += int.to_bytes(feerate, length=4, byteorder="big", signed=False)
             result += int.to_bytes(cltv, length=2, byteorder="big", signed=False)
-    return bytes(result)
+        routes.append(result)
+    return routes
 
 
 def decode_routing_info(rinfo: bytes) -> Sequence[Sequence[Sequence[Any]]]:
@@ -228,8 +232,9 @@ def create_trampoline_route(
             _extend_trampoline_route(route, end_node=second_trampoline)
         # the last trampoline onion must contain routing hints for the last trampoline
         # node to find the recipient
-        invoice_routing_info = encode_routing_info(r_tags)
+        # Due to space constraints it is not guaranteed for all route hints to get included in the onion
-        assert invoice_routing_info == encode_routing_info(decode_routing_info(invoice_routing_info))
+        invoice_routing_info: List[bytes] = encode_routing_info(r_tags)
+        assert invoice_routing_info == encode_routing_info(decode_routing_info(b''.join(invoice_routing_info)))
         # lnwire invoice_features for trampoline is u64
         invoice_features = invoice_features & 0xffffffffffffffff
         route[-1].invoice_routing_info = invoice_routing_info
@@ -287,6 +292,7 @@ def create_trampoline_onion(
     # detect trampoline hops.
     payment_path_pubkeys = [x.node_id for x in route]
     num_hops = len(payment_path_pubkeys)
+    routing_info_payload_index: Optional[int] = None
     for i in range(num_hops):
         route_edge = route[i]
         assert route_edge.is_trampoline()
@@ -305,11 +311,32 @@ def create_trampoline_onion(
         # legacy
         if i == num_hops - 2 and route_edge.invoice_features:
             payload["invoice_features"] = {"invoice_features":route_edge.invoice_features}
-            payload["invoice_routing_info"] = {"invoice_routing_info":route_edge.invoice_routing_info}
+            routing_info_payload_index = i
             payload["payment_data"] = {
                 "payment_secret": payment_secret,
                 "total_msat": total_msat
             }
+
+    if (index := routing_info_payload_index) is not None:
+        # fill the remaining payload space with available routing hints (r_tags)
+        payload: dict = hops_data[index].payload
+        # try different r_tag order on each attempt
+        invoice_routing_info = random_shuffled_copy(route[index].invoice_routing_info)
+        remaining_payload_space = TRAMPOLINE_HOPS_DATA_SIZE \
+                                  - sum(len(hop.to_bytes()) + PER_HOP_HMAC_SIZE for hop in hops_data)
+        routing_info_to_use = []
+        for encoded_r_tag in invoice_routing_info:
+            if remaining_payload_space < 50:
+                break  # no r_tag will fit here anymore
+            r_tag_size = len(encoded_r_tag)
+            if r_tag_size > remaining_payload_space:
+                continue
+            routing_info_to_use.append(encoded_r_tag)
+            remaining_payload_space -= r_tag_size
+        # add the chosen r_tags to the payload
+        payload["invoice_routing_info"] = {"invoice_routing_info": b''.join(routing_info_to_use)}
+        _logger.debug(f"Using {len(routing_info_to_use)} of {len(invoice_routing_info)} r_tags")
+
     trampoline_session_key = os.urandom(32)
     trampoline_onion = new_onion_packet(payment_path_pubkeys, trampoline_session_key, hops_data, associated_data=payment_hash, trampoline=True)
     trampoline_onion._debug_hops_data = hops_data

tests/test_lnrouter.py

@@ -4,19 +4,22 @@ import tempfile
 import shutil
 import asyncio
 from typing import Optional
+from os import urandom
 
 from electrum import util
 from electrum.channel_db import NodeInfo
 from electrum.onion_message import is_onion_message_node
+from electrum.trampoline import create_trampoline_onion
 from electrum.util import bfh
 from electrum.lnutil import ShortChannelID, LnFeatures
 from electrum.lnonion import (OnionHopsDataSingle, new_onion_packet,
                               process_onion_packet, _decode_onion_error, decode_onion_error,
-                              OnionFailureCode, OnionPacket)
+                              OnionFailureCode)
 from electrum import bitcoin, lnrouter
 from electrum.constants import BitcoinTestnet
 from electrum.simple_config import SimpleConfig
-from electrum.lnrouter import PathEdge, LiquidityHintMgr, DEFAULT_PENALTY_PROPORTIONAL_MILLIONTH, DEFAULT_PENALTY_BASE_MSAT, fee_for_edge_msat
+from electrum.lnrouter import (PathEdge, LiquidityHintMgr, DEFAULT_PENALTY_PROPORTIONAL_MILLIONTH,
+                               DEFAULT_PENALTY_BASE_MSAT, fee_for_edge_msat, LNPaymentTRoute, TrampolineEdge)
 
 from . import ElectrumTestCase
 from .test_bitcoin import needs_test_with_all_chacha20_implementations
@@ -450,6 +453,47 @@ class Test_LNRouter(ElectrumTestCase):
             self.assertEqual(hops_data[i].to_bytes(), processed_packet.hop_data.to_bytes())
             packet = processed_packet.next_packet
 
+    def test_create_legacy_trampoline_onion_multiple_rtags(self):
+        """Test to verify we don't overfill the trampoline onion with r_tags if there are more tags than available space"""
+        dummy_route: LNPaymentTRoute = [
+            TrampolineEdge(
+                invoice_routing_info=[
+                    bfh("010305061295fa30847df41ae6ee809b560e78d65c2a7337a41c725ea3920b65e08a03b62b00003a0002000003e8000000010050"),
+                    bfh("01037414fe3dcfedc4a0a0e153205d9a973af5096d1cd1c8c53d07ed12d7dd966f19f424000000000020000003e8000008ca0050"),
+                    bfh("01038550162fa86287884a6a052471934abb5cb261c5a2b15386df8104d3c7bcb85dddd92ee1898ee15c000003e8000000010090"),
+                    bfh("010244bb7ba2392ab2d493ad04ad4afcd482ca44a2bfe5b42bcc830bfe00e5b08082f424000000000029000003e8000008ca0050")
+                ],
+                invoice_features=LnFeatures.VAR_ONION_REQ | LnFeatures.PAYMENT_SECRET_REQ | LnFeatures.BASIC_MPP_OPT,
+                short_channel_id=ShortChannelID.from_str("0x0x0"),
+                start_node=node('a'),
+                end_node=node('b'),
+                fee_base_msat=0,
+                fee_proportional_millionths=0,
+                cltv_delta=0,
+                node_features=0
+            ),
+            TrampolineEdge(
+                invoice_routing_info=[],
+                invoice_features=None,
+                short_channel_id=ShortChannelID.from_str("0x0x0"),
+                start_node=node('b'),
+                end_node=node('c'),
+                fee_base_msat=0,
+                fee_proportional_millionths=0,
+                cltv_delta=0,
+                node_features=0
+            ),
+        ]
+        # create a trampoline onion, this shouldn't raise InvalidPayloadSize
+        create_trampoline_onion(
+            route=dummy_route,
+            amount_msat=0,
+            final_cltv_abs=0,
+            total_msat=0,
+            payment_hash=urandom(32),
+            payment_secret=urandom(32),
+        )
+
     @needs_test_with_all_chacha20_implementations
     def test_decode_onion_error(self):
         # test vector from bolt-04