Trampoline: Remember failed routes (fixes #7967).

Upon receiving UNKNOWN_NEXT_PEER, TEMPORARY_NODE_FAILURE
or TEMPORARY_CHANNEL_FAILURE, remember the trampoline route
that was used, and try other routes before raising the
trampoline fee.

Before this commit, we used to raise the trampoline fee
upon receiving any error message, in order to ensure
termination of the payment loop.

Note that we will still retry failing routes after we have
raised the trampoline fee. This choice is questionable, it
is unclear if doing so significantly increases the probability
of success.

Tests: add a test for trampoline handling of UNKNOWN_NEXT_PEER

electrum/lnworker.py

@@ -1225,6 +1225,7 @@ class LNWallet(LNWorker):
         # of privacy
         use_two_trampolines = True
         self.trampoline_fee_level = self.INITIAL_TRAMPOLINE_FEE_LEVEL
+        self.failed_trampoline_routes = []
         start_time = time.time()
         amount_inflight = 0  # what we sent in htlcs (that receiver gets, without fees)
         while True:
@@ -1249,7 +1250,7 @@ class LNWallet(LNWorker):
                     channels=channels,
                 )
                 # 2. send htlcs
-                async for route, amount_msat, total_msat, amount_receiver_msat, cltv_delta, bucket_payment_secret, trampoline_onion in routes:
+                async for route, amount_msat, total_msat, amount_receiver_msat, cltv_delta, bucket_payment_secret, trampoline_onion, trampoline_route in routes:
                     amount_inflight += amount_receiver_msat
                     if amount_inflight > amount_to_pay:  # safety belts
                         raise Exception(f"amount_inflight={amount_inflight} > amount_to_pay={amount_to_pay}")
@@ -1262,7 +1263,8 @@ class LNWallet(LNWorker):
                         payment_secret=bucket_payment_secret,
                         min_cltv_expiry=cltv_delta,
                         trampoline_onion=trampoline_onion,
-                        trampoline_fee_level=self.trampoline_fee_level)
+                        trampoline_fee_level=self.trampoline_fee_level,
+                        trampoline_route=trampoline_route)
                 util.trigger_callback('invoice_status', self.wallet, payment_hash.hex())
             # 3. await a queue
             self.logger.info(f"amount inflight {amount_inflight}")
@@ -1301,6 +1303,7 @@ class LNWallet(LNWorker):
                 def maybe_raise_trampoline_fee(htlc_log):
                     if htlc_log.trampoline_fee_level == self.trampoline_fee_level:
                         self.trampoline_fee_level += 1
+                        self.failed_trampoline_routes = []
                         self.logger.info(f'raising trampoline fee level {self.trampoline_fee_level}')
                     else:
                         self.logger.info(f'NOT raising trampoline fee level, already at {self.trampoline_fee_level}')
@@ -1322,7 +1325,11 @@ class LNWallet(LNWorker):
                         OnionFailureCode.UNKNOWN_NEXT_PEER,
                         OnionFailureCode.TEMPORARY_NODE_FAILURE,
                         OnionFailureCode.TEMPORARY_CHANNEL_FAILURE):
-                    maybe_raise_trampoline_fee(htlc_log)
+                    trampoline_route = htlc_log.route
+                    r = [hop.end_node.hex() for hop in trampoline_route]
+                    self.logger.info(f'failed trampoline route: {r}')
+                    assert r not in self.failed_trampoline_routes
+                    self.failed_trampoline_routes.append(r)
                     continue
                 else:
                     raise PaymentFailure(failure_msg.code_name())
@@ -1340,7 +1347,8 @@ class LNWallet(LNWorker):
             payment_secret: Optional[bytes],
             min_cltv_expiry: int,
             trampoline_onion: bytes = None,
-            trampoline_fee_level: int) -> None:
+            trampoline_fee_level: int,
+            trampoline_route: Optional[List]) -> None:
 
         # send a single htlc
         short_channel_id = route[0].short_channel_id
@@ -1360,7 +1368,7 @@ class LNWallet(LNWorker):
             trampoline_onion=trampoline_onion)
 
         key = (payment_hash, short_channel_id, htlc.htlc_id)
-        self.sent_htlcs_info[key] = route, payment_secret, amount_msat, total_msat, amount_receiver_msat, trampoline_fee_level
+        self.sent_htlcs_info[key] = route, payment_secret, amount_msat, total_msat, amount_receiver_msat, trampoline_fee_level, trampoline_route
         # if we sent MPP to a trampoline, add item to sent_buckets
         if not self.channel_db and amount_msat != total_msat:
             if payment_secret not in self.sent_buckets:
@@ -1588,7 +1596,8 @@ class LNWallet(LNWorker):
                             payment_secret=payment_secret,
                             local_height=local_height,
                             trampoline_fee_level=trampoline_fee_level,
-                            use_two_trampolines=use_two_trampolines)
+                            use_two_trampolines=use_two_trampolines,
+                            failed_routes=self.failed_trampoline_routes)
                         trampoline_payment_secret = os.urandom(32)
                         trampoline_total_msat = amount_with_fees
                     if chan.available_to_spend(LOCAL, strict=True) < amount_with_fees:
@@ -1605,7 +1614,7 @@ class LNWallet(LNWorker):
                             cltv_expiry_delta=0,
                             node_features=trampoline_features)
                     ]
-                    yield route, amount_with_fees, trampoline_total_msat, amount_msat, cltv_delta, trampoline_payment_secret, trampoline_onion
+                    yield route, amount_with_fees, trampoline_total_msat, amount_msat, cltv_delta, trampoline_payment_secret, trampoline_onion, trampoline_route
                     break
                 else:
                     raise NoPathFound()
@@ -1622,7 +1631,7 @@ class LNWallet(LNWorker):
                         full_path=full_path
                     )
                 )
-                yield route, amount_msat, final_total_msat, amount_msat, min_cltv_expiry, payment_secret, fwd_trampoline_onion
+                yield route, amount_msat, final_total_msat, amount_msat, min_cltv_expiry, payment_secret, fwd_trampoline_onion, None
         except NoPathFound:  # fall back to payment splitting
             self.logger.info("no path found, trying multi-part payment")
             if not invoice_features.supports(LnFeatures.BASIC_MPP_OPT):
@@ -1672,7 +1681,8 @@ class LNWallet(LNWorker):
                                 payment_secret=payment_secret,
                                 local_height=local_height,
                                 trampoline_fee_level=trampoline_fee_level,
-                                use_two_trampolines=use_two_trampolines)
+                                use_two_trampolines=use_two_trampolines,
+                                failed_routes=self.failed_trampoline_routes)
                             # node_features is only used to determine is_tlv
                             per_trampoline_secret = os.urandom(32)
                             per_trampoline_fees = per_trampoline_amount_with_fees - per_trampoline_amount
@@ -1697,7 +1707,7 @@ class LNWallet(LNWorker):
                                         node_features=trampoline_features)
                                 ]
                                 self.logger.info(f'adding route {part_amount_msat} {delta_fee} {margin}')
-                                routes.append((route, part_amount_msat_with_fees, per_trampoline_amount_with_fees, part_amount_msat, per_trampoline_cltv_delta, per_trampoline_secret, trampoline_onion))
+                                routes.append((route, part_amount_msat_with_fees, per_trampoline_amount_with_fees, part_amount_msat, per_trampoline_cltv_delta, per_trampoline_secret, trampoline_onion, trampoline_route))
                             if per_trampoline_fees != 0:
                                 self.logger.info('not enough margin to pay trampoline fee')
                                 raise NoPathFound()
@@ -1735,7 +1745,7 @@ class LNWallet(LNWorker):
                                         full_path=None
                                     )
                                 )
-                                yield route, part_amount_msat, final_total_msat, part_amount_msat, min_cltv_expiry, payment_secret, fwd_trampoline_onion
+                                yield route, part_amount_msat, final_total_msat, part_amount_msat, min_cltv_expiry, payment_secret, fwd_trampoline_onion, None
                                 yielded_from_split_configuration = True
                             except NoPathFound:
                                 continue
@@ -1991,7 +2001,7 @@ class LNWallet(LNWorker):
         self._on_maybe_forwarded_htlc_resolved(chan=chan, htlc_id=htlc_id)
         q = self.sent_htlcs.get(payment_hash)
         if q:
-            route, payment_secret, amount_msat, bucket_msat, amount_receiver_msat, trampoline_fee_level = self.sent_htlcs_info[(payment_hash, chan.short_channel_id, htlc_id)]
+            route, payment_secret, amount_msat, bucket_msat, amount_receiver_msat, trampoline_fee_level, trampoline_route = self.sent_htlcs_info[(payment_hash, chan.short_channel_id, htlc_id)]
             htlc_log = HtlcLog(
                 success=True,
                 route=route,
@@ -2018,7 +2028,7 @@ class LNWallet(LNWorker):
             # detect if it is part of a bucket
             # if yes, wait until the bucket completely failed
             key = (payment_hash, chan.short_channel_id, htlc_id)
-            route, payment_secret, amount_msat, bucket_msat, amount_receiver_msat, trampoline_fee_level = self.sent_htlcs_info[key]
+            route, payment_secret, amount_msat, bucket_msat, amount_receiver_msat, trampoline_fee_level, trampoline_route = self.sent_htlcs_info[key]
             if error_bytes:
                 # TODO "decode_onion_error" might raise, catch and maybe blacklist/penalise someone?
                 try:
@@ -2043,6 +2053,8 @@ class LNWallet(LNWorker):
                 self.logger.info('bucket failed')
                 amount_receiver_msat = amount_sent
 
+            if trampoline_route:
+                route = trampoline_route
             htlc_log = HtlcLog(
                 success=False,
                 route=route,

electrum/tests/test_lnchannel.py

@@ -49,8 +49,8 @@ def create_channel_state(funding_txid, funding_index, funding_sat, is_initiator,
                          local_amount, remote_amount, privkeys, other_pubkeys,
                          seed, cur, nex, other_node_id, l_dust, r_dust, l_csv,
                          r_csv):
-    assert local_amount > 0
-    assert remote_amount > 0
+    #assert local_amount > 0
+    #assert remote_amount > 0
     channel_id, _ = lnpeer.channel_id_from_funding_tx(funding_txid, funding_index)
     state = {
             "channel_id":channel_id.hex(),

electrum/tests/test_lnpeer.py

@@ -331,6 +331,15 @@ low_fee_channel = {
     'remote_fee_rate_millionths': 1,
 }
 
+depleted_channel = {
+    'local_balance_msat': 0,
+    'remote_balance_msat': 10 * bitcoin.COIN * 1000,
+    'local_base_fee_msat': 1_000,
+    'local_fee_rate_millionths': 1,
+    'remote_base_fee_msat': 1_000,
+    'remote_fee_rate_millionths': 1,
+}
+
 GRAPH_DEFINITIONS = {
     'square_graph': {
         'alice': {
@@ -761,6 +770,7 @@ class TestPeer(TestCaseForTestnet):
                 min_cltv_expiry=lnaddr2.get_min_final_cltv_expiry(),
                 payment_secret=lnaddr2.payment_secret,
                 trampoline_fee_level=0,
+                trampoline_route=None,
             )
             p1.maybe_send_commitment = _maybe_send_commitment1
             # bob sends htlc BUT NOT COMMITMENT_SIGNED
@@ -776,6 +786,7 @@ class TestPeer(TestCaseForTestnet):
                 min_cltv_expiry=lnaddr1.get_min_final_cltv_expiry(),
                 payment_secret=lnaddr1.payment_secret,
                 trampoline_fee_level=0,
+                trampoline_route=None,
             )
             p2.maybe_send_commitment = _maybe_send_commitment2
             # sleep a bit so that they both receive msgs sent so far
@@ -1081,7 +1092,7 @@ class TestPeer(TestCaseForTestnet):
         graph = self.prepare_chans_and_peers_in_graph(GRAPH_DEFINITIONS['square_graph'])
         self._run_mpp(graph, {'mpp_invoice': False}, {'mpp_invoice': True})
 
-    def _run_trampoline_payment(self, is_legacy):
+    def _run_trampoline_payment(self, is_legacy, direct, drop_dave= []):
         async def turn_on_trampoline_alice():
             if graph.workers['alice'].network.channel_db:
                 graph.workers['alice'].network.channel_db.stop()
@@ -1091,9 +1102,16 @@ class TestPeer(TestCaseForTestnet):
         async def pay(lnaddr, pay_req):
             self.assertEqual(PR_UNPAID, graph.workers['dave'].get_payment_status(lnaddr.paymenthash))
             result, log = await graph.workers['alice'].pay_invoice(pay_req, attempts=10)
-            self.assertTrue(result)
-            self.assertEqual(PR_PAID, graph.workers['dave'].get_payment_status(lnaddr.paymenthash))
-            raise PaymentDone()
+            if result:
+                self.assertEqual(PR_PAID, graph.workers['dave'].get_payment_status(lnaddr.paymenthash))
+                raise PaymentDone()
+            else:
+                raise NoPathFound()
+
+        def do_drop_dave(t):
+            # this will trigger UNKNOWN_NEXT_PEER
+            dave_node_id = graph.workers['dave'].node_keypair.pubkey
+            graph.workers[t].peers.pop(dave_node_id)
 
         async def f():
             await turn_on_trampoline_alice()
@@ -1103,16 +1121,21 @@ class TestPeer(TestCaseForTestnet):
                     await group.spawn(peer.htlc_switch())
                 await asyncio.sleep(0.2)
                 lnaddr, pay_req = self.prepare_invoice(graph.workers['dave'], include_routing_hints=True)
+                for p in drop_dave:
+                    do_drop_dave(p)
                 await group.spawn(pay(lnaddr, pay_req))
 
         graph_definition = GRAPH_DEFINITIONS['square_graph'].copy()
-        # insert a channel from bob to carol for faster tests,
-        # otherwise will fail randomly
-        graph_definition['bob']['channels']['carol'] = high_fee_channel
+        if not direct:
+            # deplete channel from alice to carol
+            graph_definition['alice']['channels']['carol'] = depleted_channel
+            # insert a channel from bob to carol
+            graph_definition['bob']['channels']['carol'] = high_fee_channel
+
         graph = self.prepare_chans_and_peers_in_graph(graph_definition)
         peers = graph.peers.values()
         if is_legacy:
-            # turn off trampoline features
+            # turn off trampoline features in invoice
             graph.workers['dave'].features = graph.workers['dave'].features ^ LnFeatures.OPTION_TRAMPOLINE_ROUTING_OPT
 
         # declare routing nodes as trampoline nodes
@@ -1121,16 +1144,26 @@ class TestPeer(TestCaseForTestnet):
             graph.workers['carol'].name: LNPeerAddr(host="127.0.0.1", port=9735, pubkey=graph.workers['carol'].node_keypair.pubkey),
         }
 
+        run(f())
+
+    @needs_test_with_all_chacha20_implementations
+    def test_payment_trampoline_legacy(self):
         with self.assertRaises(PaymentDone):
-            run(f())
+            self._run_trampoline_payment(is_legacy=True, direct=False)
 
     @needs_test_with_all_chacha20_implementations
-    def test_trampoline_payment_legacy(self):
-        self._run_trampoline_payment(True)
+    def test_payment_trampoline_e2e_direct(self):
+        with self.assertRaises(PaymentDone):
+            self._run_trampoline_payment(is_legacy=False, direct=True)
 
     @needs_test_with_all_chacha20_implementations
-    def test_trampoline_payment_e2e(self):
-        self._run_trampoline_payment(False)
+    def test_payment_trampoline_e2e_indirect(self):
+        # must use two trampolines
+        with self.assertRaises(PaymentDone):
+            self._run_trampoline_payment(is_legacy=False, direct=False, drop_dave=['bob'])
+        # both trampolines drop dave
+        with self.assertRaises(NoPathFound):
+            self._run_trampoline_payment(is_legacy=False, direct=False, drop_dave=['bob', 'carol'])
 
     @needs_test_with_all_chacha20_implementations
     def test_payment_multipart_trampoline_e2e(self):
@@ -1412,6 +1445,7 @@ class TestPeer(TestCaseForTestnet):
                 payment_secret=payment_secret,
                 min_cltv_expiry=min_cltv_expiry,
                 trampoline_fee_level=0,
+                trampoline_route=None,
             )
             await asyncio.gather(pay, p1._message_loop(), p2._message_loop(), p1.htlc_switch(), p2.htlc_switch())
         with self.assertRaises(PaymentFailure):

electrum/trampoline.py

@@ -10,7 +10,6 @@ from .lnrouter import RouteEdge, TrampolineEdge, LNPaymentRoute, is_route_sane_t
 from .lnutil import NoPathFound, LNPeerAddr
 from . import constants
 
-
 # trampoline nodes are supposed to advertise their fee and cltv in node_update message
 TRAMPOLINE_FEES = [
     {
@@ -165,6 +164,18 @@ def extend_trampoline_route(
             node_features=trampoline_features))
 
 
+def choose_second_trampoline(my_trampoline, trampolines, failed_routes):
+    if my_trampoline in trampolines:
+        trampolines.remove(my_trampoline)
+    for r in failed_routes:
+        if len(r) > 2:
+            t2 = bytes.fromhex(r[1])
+            if t2 in trampolines:
+                trampolines.remove(t2)
+    if not trampolines:
+        raise NoPathFound('all routes have failed')
+    return random.choice(trampolines)
+
 def create_trampoline_route(
         *,
         amount_msat: int,
@@ -172,28 +183,28 @@ def create_trampoline_route(
         invoice_pubkey: bytes,
         invoice_features: int,
         my_pubkey: bytes,
-        trampoline_node_id: bytes,  # the first trampoline in the path; which we are directly connected to
+        my_trampoline: bytes,  # the first trampoline in the path; which we are directly connected to
         r_tags,
         trampoline_fee_level: int,
-        use_two_trampolines: bool
+        use_two_trampolines: bool,
+        failed_routes: list,
 ) -> LNPaymentRoute:
     # we decide whether to convert to a legacy payment
     is_legacy, invoice_trampolines = is_legacy_relay(invoice_features, r_tags)
 
     # we build a route of trampoline hops and extend the route list in place
     route = []
+    second_trampoline = None
 
     # our first trampoline hop is decided by the channel we use
-    extend_trampoline_route(route, my_pubkey, trampoline_node_id, trampoline_fee_level)
+    extend_trampoline_route(route, my_pubkey, my_trampoline, trampoline_fee_level)
 
     if is_legacy:
         # we add another different trampoline hop for privacy
         if use_two_trampolines:
             trampolines = trampolines_by_id()
-            del trampolines[trampoline_node_id]
-            second_trampoline_pubkey = random.choice(list(trampolines.keys()))
-            extend_trampoline_route(route, trampoline_node_id, second_trampoline_pubkey, trampoline_fee_level)
-
+            second_trampoline = choose_second_trampoline(my_trampoline, list(trampolines.keys()), failed_routes)
+            extend_trampoline_route(route, my_trampoline, second_trampoline, trampoline_fee_level)
         # 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)
@@ -201,13 +212,21 @@ def create_trampoline_route(
         route[-1].invoice_features = invoice_features
         route[-1].outgoing_node_id = invoice_pubkey
     else:
-        if invoice_trampolines and trampoline_node_id not in invoice_trampolines:
-            second_trampoline_pubkey = random.choice(invoice_trampolines)
-            extend_trampoline_route(route, trampoline_node_id, second_trampoline_pubkey, trampoline_fee_level)
+        if invoice_trampolines:
+            if my_trampoline in invoice_trampolines:
+                short_route = [my_trampoline.hex(), invoice_pubkey.hex()]
+                if short_route in failed_routes:
+                    add_trampoline = True
+                else:
+                    add_trampoline = False
+            else:
+                add_trampoline = True
+            if add_trampoline:
+                second_trampoline = choose_second_trampoline(my_trampoline, invoice_trampolines, failed_routes)
+                extend_trampoline_route(route, my_trampoline, second_trampoline, trampoline_fee_level)
 
     # final edge (not part of the route if payment is legacy, but eclair requires an encrypted blob)
     extend_trampoline_route(route, route[-1].end_node, invoice_pubkey, trampoline_fee_level, pay_fees=False)
-
     # check that we can pay amount and fees
     for edge in route[::-1]:
         amount_msat += edge.fee_for_edge(amount_msat)
@@ -269,7 +288,8 @@ def create_trampoline_route_and_onion(
         payment_secret,
         local_height: int,
         trampoline_fee_level: int,
-        use_two_trampolines: bool):
+        use_two_trampolines: bool,
+        failed_routes: list):
     # create route for the trampoline_onion
     trampoline_route = create_trampoline_route(
         amount_msat=amount_msat,
@@ -277,10 +297,11 @@ def create_trampoline_route_and_onion(
         my_pubkey=my_pubkey,
         invoice_pubkey=invoice_pubkey,
         invoice_features=invoice_features,
-        trampoline_node_id=node_id,
+        my_trampoline=node_id,
         r_tags=r_tags,
         trampoline_fee_level=trampoline_fee_level,
-        use_two_trampolines=use_two_trampolines)
+        use_two_trampolines=use_two_trampolines,
+        failed_routes=failed_routes)
     # compute onion and fees
     final_cltv = local_height + min_cltv_expiry
     trampoline_onion, amount_with_fees, bucket_cltv = create_trampoline_onion(