trampoline: improve payment success

- better error handling: previously we stopped all attempts on any of
  TRAMPOLINE_EXPIRY_TOO_SOON, UNKNOWN_NEXT_PEER, TEMPORARY_NODE_FAILURE.
  Instead we should retry (but see code comments).
- previously payments failed if ALL of the following criteria applied:
  - sender is paying via trampoline, but not via the ACINQ node (which is
    special cased)
  - receiver only has private channels and has put r_tags into invoice, along
    with setting the trampoline feature bit in the invoice, however the receiver
    is not connected to any trampoline forwarders directly
  The sender would then assume that the private routing hints in the invoice
  correspond to trampoline forwarders.
- also, previously if both the sender and the recipient used trampoline and
  they shared a trampoline forwarder (that they were both connected to), the
  private channels the recipient had (with nodes other than the shared TF)
  would never be attempted.

electrum/lnchannel.py

@@ -1053,8 +1053,7 @@ class Channel(AbstractChannel):
                 # if we are forwarding, save error message to disk
                 if self.lnworker.get_payment_info(htlc.payment_hash) is None:
                     self.save_fail_htlc_reason(htlc.htlc_id, error_bytes, failure_message)
-                else:
-                    self.lnworker.htlc_failed(self, htlc.payment_hash, htlc.htlc_id, error_bytes, failure_message)
+                self.lnworker.htlc_failed(self, htlc.payment_hash, htlc.htlc_id, error_bytes, failure_message)
 
     def save_fail_htlc_reason(
             self,

electrum/lnpeer.py

@@ -1455,6 +1455,12 @@ class Peer(Logger):
             htlc: UpdateAddHtlc,
             trampoline_onion: ProcessedOnionPacket):
 
+        forwarding_enabled = self.network.config.get('lightning_forward_payments', False)
+        forwarding_trampoline_enabled = self.network.config.get('lightning_forward_trampoline_payments', False)
+        if not (forwarding_enabled and forwarding_trampoline_enabled):
+            self.logger.info(f"trampoline forwarding is disabled. failing htlc.")
+            raise OnionRoutingFailure(code=OnionFailureCode.PERMANENT_CHANNEL_FAILURE, data=b'')
+
         payload = trampoline_onion.hop_data.payload
         payment_hash = htlc.payment_hash
         payment_secret = os.urandom(32)
@@ -1467,6 +1473,7 @@ class Peer(Logger):
                 next_trampoline_onion = None
                 invoice_features = payload["invoice_features"]["invoice_features"]
                 invoice_routing_info = payload["invoice_routing_info"]["invoice_routing_info"]
+                # TODO use invoice_routing_info
             else:
                 self.logger.info('forward_trampoline: end-to-end')
                 invoice_features = LnFeatures.BASIC_MPP_OPT

electrum/lnworker.py

@@ -1220,12 +1220,20 @@ class LNWallet(LNWorker):
                 raise PaymentFailure(failure_msg.code_name())
             # trampoline
             if not self.channel_db:
-                if code == OnionFailureCode.TRAMPOLINE_FEE_INSUFFICIENT:
+                # FIXME The trampoline nodes in the path are chosen randomly.
+                #       Some of the errors might depend on how we have chosen them.
+                #       Having more attempts is currently useful in part because of the randomness,
+                #       instead we should give feedback to create_routes_for_payment.
+                if code in (OnionFailureCode.TRAMPOLINE_FEE_INSUFFICIENT,
+                            OnionFailureCode.TRAMPOLINE_EXPIRY_TOO_SOON):
                     # todo: parse the node parameters here (not returned by eclair yet)
                     trampoline_fee_level += 1
                     continue
                 elif use_two_trampolines:
                     use_two_trampolines = False
+                elif code in (OnionFailureCode.UNKNOWN_NEXT_PEER,
+                              OnionFailureCode.TEMPORARY_NODE_FAILURE):
+                    continue
                 else:
                     raise PaymentFailure(failure_msg.code_name())
             else:

electrum/tests/test_lnchannel.py

@@ -151,7 +151,7 @@ def create_test_channels(*, feerate=6000, local_msat=None, remote_msat=None,
                 bob_first, other_node_id=bob_pubkey, l_dust=200, r_dust=1300,
                 l_csv=5, r_csv=4
             ),
-            name=bob_name,
+            name=f"{alice_name}->{bob_name}",
             initial_feerate=feerate),
         lnchannel.Channel(
             create_channel_state(
@@ -160,7 +160,7 @@ def create_test_channels(*, feerate=6000, local_msat=None, remote_msat=None,
                 alice_first, other_node_id=alice_pubkey, l_dust=1300, r_dust=200,
                 l_csv=4, r_csv=5
             ),
-            name=alice_name,
+            name=f"{bob_name}->{alice_name}",
             initial_feerate=feerate)
     )
 

electrum/tests/test_lnpeer.py

@@ -12,6 +12,8 @@ from typing import Iterable, NamedTuple, Tuple, List, Dict
 
 from aiorpcx import TaskGroup, timeout_after, TaskTimeout
 
+import electrum
+import electrum.trampoline
 from electrum import bitcoin
 from electrum import constants
 from electrum.network import Network
@@ -152,6 +154,8 @@ class MockLNWallet(Logger, NetworkRetryManager[LNPeerAddr]):
         self.preimages = {}
         self.stopping_soon = False
 
+        self.logger.info(f"created LNWallet[{name}] with nodeID={local_keypair.pubkey.hex()}")
+
     def get_invoice_status(self, key):
         pass
 
@@ -272,8 +276,8 @@ class PutIntoOthersQueueTransport(MockTransport):
         self.other_mock_transport.queue.put_nowait(data)
 
 def transport_pair(k1, k2, name1, name2):
-    t1 = PutIntoOthersQueueTransport(k1, name2)
-    t2 = PutIntoOthersQueueTransport(k2, name1)
+    t1 = PutIntoOthersQueueTransport(k1, name1)
+    t2 = PutIntoOthersQueueTransport(k2, name2)
     t1.other_mock_transport = t2
     t2.other_mock_transport = t1
     return t1, t2
@@ -341,7 +345,7 @@ class TestPeer(TestCaseForTestnet):
         self._loop_thread.join(timeout=1)
         super().tearDown()
 
-    def prepare_peers(self, alice_channel, bob_channel):
+    def prepare_peers(self, alice_channel: Channel, bob_channel: Channel):
         k1, k2 = keypair(), keypair()
         alice_channel.node_id = k2.pubkey
         bob_channel.node_id = k1.pubkey
@@ -424,6 +428,8 @@ class TestPeer(TestCaseForTestnet):
 
         w_b.network.config.set_key('lightning_forward_payments', True)
         w_c.network.config.set_key('lightning_forward_payments', True)
+        w_b.network.config.set_key('lightning_forward_trampoline_payments', True)
+        w_c.network.config.set_key('lightning_forward_trampoline_payments', True)
 
         # forwarding fees, etc
         chan_ab.forwarding_fee_proportional_millionths *= 500
@@ -448,7 +454,7 @@ class TestPeer(TestCaseForTestnet):
         peer_cd.mark_open(chan_cd)
         peer_db.mark_open(chan_db)
         peer_dc.mark_open(chan_dc)
-        return SquareGraph(
+        graph = SquareGraph(
             w_a=w_a,
             w_b=w_b,
             w_c=w_c,
@@ -470,6 +476,7 @@ class TestPeer(TestCaseForTestnet):
             chan_db=chan_db,
             chan_dc=chan_dc,
         )
+        return graph
 
     @staticmethod
     async def prepare_invoice(
@@ -935,7 +942,14 @@ class TestPeer(TestCaseForTestnet):
     def test_multipart_payment_with_trampoline(self):
         # single attempt will fail with insufficient trampoline fee
         graph = self.prepare_chans_and_peers_in_square()
-        self._run_mpp(graph, {'alice_uses_trampoline':True, 'attempts':1}, {'alice_uses_trampoline':True, 'attempts':3})
+        electrum.trampoline._TRAMPOLINE_NODES_UNITTESTS = {
+            graph.w_b.name: LNPeerAddr(host="127.0.0.1", port=9735, pubkey=graph.w_b.node_keypair.pubkey),
+            graph.w_c.name: LNPeerAddr(host="127.0.0.1", port=9735, pubkey=graph.w_c.node_keypair.pubkey),
+        }
+        try:
+            self._run_mpp(graph, {'alice_uses_trampoline':True, 'attempts':1}, {'alice_uses_trampoline':True, 'attempts':30})
+        finally:
+            electrum.trampoline._TRAMPOLINE_NODES_UNITTESTS = {}
 
     @needs_test_with_all_chacha20_implementations
     def test_fail_pending_htlcs_on_shutdown(self):

electrum/trampoline.py

@@ -2,6 +2,8 @@ import os
 import bitstring
 import random
 
+from typing import Mapping
+
 from .logging import get_logger, Logger
 from .lnutil import LnFeatures
 from .lnonion import calc_hops_data_for_payment, new_onion_packet
@@ -67,19 +69,24 @@ TRAMPOLINE_NODES_SIGNET = {
     'wakiyamap.dev': LNPeerAddr(host='signet-electrumx.wakiyamap.dev', port=9735, pubkey=bytes.fromhex('02dadf6c28f3284d591cd2a4189d1530c1ff82c07059ebea150a33ab76e7364b4a')),
 }
 
-def hardcoded_trampoline_nodes():
+_TRAMPOLINE_NODES_UNITTESTS = {}  # used in unit tests
+
+def hardcoded_trampoline_nodes() -> Mapping[str, LNPeerAddr]:
+    nodes = {}
     if constants.net.NET_NAME == "mainnet":
-        return TRAMPOLINE_NODES_MAINNET
-    if constants.net.NET_NAME == "testnet":
-        return TRAMPOLINE_NODES_TESTNET
-    if constants.net.NET_NAME == "signet":
-        return TRAMPOLINE_NODES_SIGNET
-    return {}
+        nodes.update(TRAMPOLINE_NODES_MAINNET)
+    elif constants.net.NET_NAME == "testnet":
+        nodes.update(TRAMPOLINE_NODES_TESTNET)
+    elif constants.net.NET_NAME == "signet":
+        nodes.update(TRAMPOLINE_NODES_SIGNET)
+    nodes.update(_TRAMPOLINE_NODES_UNITTESTS)
+    return nodes
 
 def trampolines_by_id():
     return dict([(x.pubkey, x) for x in hardcoded_trampoline_nodes().values()])
 
-is_hardcoded_trampoline = lambda node_id: node_id in trampolines_by_id().keys()
+def is_hardcoded_trampoline(node_id: bytes) -> bool:
+    return node_id in trampolines_by_id()
 
 def encode_routing_info(r_tags):
     result = bitstring.BitArray()
@@ -103,13 +110,26 @@ def create_trampoline_route(
         trampoline_fee_level: int,
         use_two_trampolines: bool) -> LNPaymentRoute:
 
+    # figure out whether we can use end-to-end trampoline, or fallback to pay-to-legacy
+    is_legacy = True
+    r_tag_chosen_for_e2e_trampoline = None
     invoice_features = LnFeatures(invoice_features)
-    if invoice_features.supports(LnFeatures.OPTION_TRAMPOLINE_ROUTING_OPT)\
-        or invoice_features.supports(LnFeatures.OPTION_TRAMPOLINE_ROUTING_OPT_ECLAIR):
-        is_legacy = False
-    else:
-        is_legacy = True
-
+    if (invoice_features.supports(LnFeatures.OPTION_TRAMPOLINE_ROUTING_OPT)
+            or invoice_features.supports(LnFeatures.OPTION_TRAMPOLINE_ROUTING_OPT_ECLAIR)):
+        if not r_tags:  # presumably the recipient has public channels
+            is_legacy = False
+        else:
+            # - We choose one routing hint at random, and
+            #   use end-to-end trampoline if that node is a trampoline-forwarder (TF).
+            # - In case of e2e, the route will have either one or two TFs (one neighbour of sender,
+            #   and one neighbour of recipient; and these might coincide). Note that there are some
+            #   channel layouts where two TFs are needed for a payment to succeed, e.g. both
+            #   endpoints connected to T1 and T2, and sender only has send-capacity with T1, while
+            #   recipient only has recv-capacity with T2.
+            singlehop_r_tags = [x for x in r_tags if len(x) == 1]
+            r_tag_chosen_for_e2e_trampoline = random.choice(singlehop_r_tags)[0]
+            pubkey, scid, feebase, feerate, cltv = r_tag_chosen_for_e2e_trampoline
+            is_legacy = not is_hardcoded_trampoline(pubkey)
     # fee level. the same fee is used for all trampolines
     if trampoline_fee_level < len(TRAMPOLINE_FEES):
         params = TRAMPOLINE_FEES[trampoline_fee_level]
@@ -157,20 +177,13 @@ def create_trampoline_route(
         route[-1].invoice_routing_info = invoice_routing_info
         route[-1].invoice_features = invoice_features
         route[-1].outgoing_node_id = invoice_pubkey
-    else:
-        last_trampoline = route[-1].end_node
-        r_tags = [x for x in r_tags if len(x) == 1]
-        random.shuffle(r_tags)
-        for r_tag in r_tags:
-            pubkey, scid, feebase, feerate, cltv = r_tag[0]
-            if pubkey == trampoline_node_id:
-                break
-        else:
-            pubkey, scid, feebase, feerate, cltv = r_tag[0]
-            if route[-1].node_id != pubkey:
+    else:  # end-to-end trampoline
+        if r_tag_chosen_for_e2e_trampoline:
+            pubkey, scid, feebase, feerate, cltv = r_tag_chosen_for_e2e_trampoline
+            if route[-1].end_node != pubkey:
                 route.append(
                     TrampolineEdge(
-                        start_node=route[-1].node_id,
+                        start_node=route[-1].end_node,
                         end_node=pubkey,
                         fee_base_msat=feebase,
                         fee_proportional_millionths=feerate,