From 35b56e66d6c8376d0d18095147acaaca7d05700c Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Wed, 30 Sep 2026 15:02:21 -0700 Subject: [PATCH 1/2] pytest: show xpay retries every channel on unknown_next_peer unknown_next_peer means the reporting node cannot forward, which is what lnd returns when the next peer is offline. xpay only disables the failed channel, so getroutes hands back every other channel through the same node. This test fails a hub with four exits using that code. On current xpay it walks all four: Failed after 4 attempts. We got a weird error (unknown_next_peer) for 131x1x0/1 ... 124x1x0/1 ... 117x1x0/1 ... 110x2x0/0 assert 4 <= 2 Marked xfail until the next commit excludes the node after the repeat. Changelog-None. --- tests/plugins/fail_unknown_next_peer.py | 21 ++++++++++++ tests/test_xpay.py | 44 +++++++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100755 tests/plugins/fail_unknown_next_peer.py diff --git a/tests/plugins/fail_unknown_next_peer.py b/tests/plugins/fail_unknown_next_peer.py new file mode 100755 index 000000000000..a8587fb5a26b --- /dev/null +++ b/tests/plugins/fail_unknown_next_peer.py @@ -0,0 +1,21 @@ +#!/usr/bin/env python3 +"""Fail payment forwards with unknown_next_peer. Leave local invoices alone. + +Used by test_xpay_unknown_next_peer_excludes_node (issue 9590). +""" +from pyln.client import Plugin + +plugin = Plugin() + + +@plugin.hook("htlc_accepted") +def on_htlc_accepted(onion, plugin, **kwargs): + # Forwards carry forward_msat. Local invoices and funding HTLCs do not. + if not isinstance(onion, dict) or onion.get("forward_msat") is None: + return {"result": "continue"} + plugin.log("failing forward with unknown_next_peer") + # WIRE_UNKNOWN_NEXT_PEER = PERM | 10 = 0x400a + return {"result": "fail", "failure_message": "400a"} + + +plugin.run() diff --git a/tests/test_xpay.py b/tests/test_xpay.py index 301f89fd4708..cc21a9c44a60 100644 --- a/tests/test_xpay.py +++ b/tests/test_xpay.py @@ -1559,3 +1559,47 @@ def test_sendamount_bip353(node_factory): ret = l2.rpc.sendamount("fake@fake.com", "100sat") assert ret["successful_parts"] == 1 assert ret["amount_sent_msat"] == 100000 + + +@pytest.mark.xfail(strict=True) +def test_xpay_unknown_next_peer_excludes_node(node_factory, bitcoind): + """Issue 9590: repeated unknown_next_peer must exclude the next node. + + payer -> hub -> {a,b,c,d} -> dest + + hub fails every forward with unknown_next_peer (the code lnd returns when + the next peer is offline). A single failure may be a stale scid, so the + first channel is disabled and another path is allowed. The second failure + to the same next node must exclude that node, so xpay must not walk every + remaining channel into it. + """ + plugin = os.path.join(os.path.dirname(__file__), 'plugins/fail_unknown_next_peer.py') + # This tree has no cln-grpc plugin. The harness passes --grpc-port unless it is disabled. + payer, hub, a, b, c, d, dest = node_factory.get_nodes(7, opts={'disable-plugin': 'cln-grpc'}) + node_factory.join_nodes([payer, hub], fundamount=10**6, wait_for_announce=True) + for spoke in (a, b, c, d): + node_factory.join_nodes([hub, spoke, dest], fundamount=10**6, wait_for_announce=True) + + wait_for(lambda: len(payer.rpc.listchannels()['channels']) >= 9 * 2) + # lightningd execs the shebang. Point it at this interpreter so pyln imports. + launched = plugin + '.launch' + with open(plugin) as f: + src = f.read() + with open(launched, 'w') as f: + f.write('#!' + sys.executable + '\n' + src.split('\n', 1)[1]) + os.chmod(launched, 0o755) + hub.rpc.plugin_start(launched) + + inv = dest.rpc.invoice(10000, 'issue-9590', 'issue 9590')['bolt11'] + started = time.time() + with pytest.raises(RpcError) as err: + payer.rpc.xpay(invstring=inv, retry_for=15) + elapsed = time.time() - started + + msg = err.value.error['message'] + # One channel disable, then the node is excluded. Walking all four exits + # is the bug. The exclusion line also contains the failcode name. + unknown = msg.count('We got unknown_next_peer') + assert unknown <= 2, msg + assert elapsed < 10, 'spent the retry window walking the same node: {}'.format(msg) + assert 'disabling node' in msg or 'Repeated unknown_next_peer' in msg From a97e69741d9f41cbe4cd7e01d61a19093f5475b1 Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Wed, 30 Sep 2026 15:02:35 -0700 Subject: [PATCH 2/2] xpay: exclude a node that repeats unknown_next_peer The previous commit fails because xpay disables only the channel. A second unknown_next_peer from the same node now calls askrene-disable-node on this payment's private layer, so getroutes stops handing back the rest of its channels. The first failure still only disables that channel. One stale scid must not take a well-connected node out of every path. temporary_channel_failure and fee or CLTV errors are unchanged. Removes the xfail from test_xpay_unknown_next_peer_excludes_node. With the fix that test stops after two attempts: Repeated unknown_next_peer from : disabling node for this payment Fixes #9590. Changelog-Fixed: xpay no longer retries every channel through a node that keeps returning unknown_next_peer. --- plugins/xpay/xpay.c | 59 ++++++++++++++++++++++++++++++++++++++++++++- tests/test_xpay.py | 1 - 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/plugins/xpay/xpay.c b/plugins/xpay/xpay.c index 8e1b2f5d14f0..91491c5438c0 100644 --- a/plugins/xpay/xpay.c +++ b/plugins/xpay/xpay.c @@ -145,6 +145,11 @@ struct payment { /* Useful information from prior attempts if any. */ char *prior_results; + /* Nodes that have already returned unknown_next_peer. The first + * failure only disables that channel (it can be a stale scid). A + * second failure from the same node excludes it. */ + struct node_id *unknown_next_peers; + /* Requests currently outstanding */ struct out_req **requests; @@ -905,6 +910,41 @@ static void payment_already_paid(struct payment *payment) send_outreq(req); } +/* unknown_next_peer is returned by the node that cannot forward. One hit + * can be a stale scid, so we only disable that channel. The same node + * returning it again means every other channel through it will fail too. */ +static void maybe_exclude_unknown_next_peer(struct command *aux_cmd, + struct attempt *attempt, + size_t index) +{ + struct payment *payment = attempt->payment; + struct node_id erring; + struct out_req *req; + bool seen = false; + + if (index == 0) + return; + node_id_from_pubkey(&erring, &attempt->hops[index - 1].next_node); + for (size_t i = 0; i < tal_count(payment->unknown_next_peers); i++) { + if (node_id_eq(&payment->unknown_next_peers[i], &erring)) { + seen = true; + break; + } + } + if (!seen) { + tal_arr_expand(&payment->unknown_next_peers, erring); + return; + } + + add_result_summary(attempt, LOG_DBG, + "Repeated unknown_next_peer from %s: disabling node for this payment", + fmt_node_id(tmpctx, &erring)); + req = payment_ignored_req(aux_cmd, attempt, "askrene-disable-node"); + json_add_string(req->js, "layer", payment->private_layer); + json_add_node_id(req->js, "node", &erring); + send_payment_req(aux_cmd, attempt->payment, req); +} + static void update_knowledge_from_error(struct command *aux_cmd, const char *buf, const jsmntok_t *error, @@ -1057,6 +1097,12 @@ static void update_knowledge_from_error(struct command *aux_cmd, case WIRE_PERMANENT_CHANNEL_FAILURE: case WIRE_REQUIRED_CHANNEL_FEATURE_MISSING: case WIRE_UNKNOWN_NEXT_PEER: + /* A final node must not send this. The erring node is + * the one that cannot forward; exclude it on repeat. */ + maybe_exclude_unknown_next_peer(aux_cmd, attempt, index); + index--; + goto strange_error; + case WIRE_AMOUNT_BELOW_MINIMUM: case WIRE_FEE_INSUFFICIENT: case WIRE_INCORRECT_CLTV_EXPIRY: @@ -1130,9 +1176,19 @@ static void update_knowledge_from_error(struct command *aux_cmd, fmt_amount_msat(tmpctx, attempt->hops[index].amount_out)); goto channel_capacity; + case WIRE_UNKNOWN_NEXT_PEER: + /* One hit can be a stale scid, so disable that channel. + * The same node returning it again is excluded, or we + * walk every other channel through it. */ + add_result_summary(attempt, LOG_DBG, + "We got %s for %s: disabling it for this payment", + errmsg, + describe_scidd(attempt, index)); + maybe_exclude_unknown_next_peer(aux_cmd, attempt, index); + goto disable_channel; + case WIRE_PERMANENT_CHANNEL_FAILURE: case WIRE_REQUIRED_CHANNEL_FEATURE_MISSING: - case WIRE_UNKNOWN_NEXT_PEER: case WIRE_AMOUNT_BELOW_MINIMUM: case WIRE_FEE_INSUFFICIENT: case WIRE_INCORRECT_CLTV_EXPIRY: @@ -2648,6 +2704,7 @@ static struct payment *new_payment(const tal_t *ctx, list_head_init(&payment->past_attempts); payment->amount_being_routed = AMOUNT_MSAT(0); payment->prior_results = tal_strdup(payment, ""); + payment->unknown_next_peers = tal_arr(payment, struct node_id, 0); payment->requests = tal_arr(payment, struct out_req *, 0); payment->start_time = clock_time(); payment->pay_compat = as_pay; diff --git a/tests/test_xpay.py b/tests/test_xpay.py index cc21a9c44a60..692ef3185ac8 100644 --- a/tests/test_xpay.py +++ b/tests/test_xpay.py @@ -1561,7 +1561,6 @@ def test_sendamount_bip353(node_factory): assert ret["amount_sent_msat"] == 100000 -@pytest.mark.xfail(strict=True) def test_xpay_unknown_next_peer_excludes_node(node_factory, bitcoind): """Issue 9590: repeated unknown_next_peer must exclude the next node.