From 17ab6ae4035f84d94634472678913f1c6cb42523 Mon Sep 17 00:00:00 2001 From: daywalker90 Date: Tue, 13 Oct 2026 12:02:44 +0200 Subject: [PATCH 1/4] tests: waitsendpay after a sendonion failure is recorded A sendonion payment whose remote failure is recorded before waitsendpay is called reports "Payment failure reason unknown" (code 209) instead of the failure. The CI ASan/UBSan shard failed test_reject_invalid_payload this way: RpcError: RPC call failed: method: waitsendpay, ... error: {'code': 209, 'message': 'Payment failure reason unknown'} test_reject_invalid_payload now takes a wait_for_failure parameter. When it is set, the test waits for the sender to log "htlc failed from 0th node" before calling waitsendpay, so the call reads the failure from the database rather than from a live waiter. The unset case is the original test and passes either way. The set case is marked xfail(strict=True) until the fix lands in the next commit. Changelog-None Signed-off-by: daywalker90 --- tests/test_pay.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/test_pay.py b/tests/test_pay.py index a853177f5245..57381a30791a 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -3467,12 +3467,23 @@ def test_sendpay_msatoshi_arg(node_factory): assert inv['amount_received_msat'] == Millisatoshi(1001) -def test_reject_invalid_payload(node_factory): +@pytest.mark.parametrize( + "wait_for_failure", + [ + False, + pytest.param(True, marks=pytest.mark.xfail(strict=True)), + ], +) +def test_reject_invalid_payload(node_factory, wait_for_failure): """Send an onion payload with an unknown even type. Recipient l2 should reject it the incoming HTLC with an invalid onion payload error. + With wait_for_failure, waitsendpay runs only after the sender has recorded + the failure, so it reads the failure from the database rather than from a + live waiter. That path used to report "Payment failure reason unknown". + """ l1, l2 = node_factory.line_graph(2) @@ -3491,6 +3502,8 @@ def test_reject_invalid_payload(node_factory): first_hop=first_hop, payment_hash=inv['payment_hash'], shared_secrets=onion['shared_secrets']) + if wait_for_failure: + l1.daemon.wait_for_log(r'htlc failed from 0th node') with pytest.raises(RpcError, match=r'WIRE_INVALID_ONION_PAYLOAD'): l1.rpc.waitsendpay(inv['payment_hash']) From a4d1f4c5910327d1b6ebd171c4ddb9dce0604ca1 Mon Sep 17 00:00:00 2001 From: daywalker90 Date: Tue, 13 Oct 2026 12:03:04 +0200 Subject: [PATCH 2/4] lightningd: report the remote failure for sendonion payments after the fact A sendonion payment has no route nodes, so remote_routing_failure() sets no erring node, and payment_failed() stores failonion as NULL. The failure is still stored with its failcode and raw failmsg. wait_payment() only looked at failonionreply and failnode, so a waitsendpay() that ran after the failure was recorded reported: RpcError: ... error: {'code': 209, 'message': 'Payment failure reason unknown'} A waitsendpay() that ran before the failure got the failure from the live waiter and was correct, so the error only showed up when the call lost the race. wait_payment() now uses the stored failmsg as the remote failure when there is no erring node, the same way the live path does. The "reason unknown" error is kept for databases with no failure information. Changelog-Fixed: waitsendpay reports the remote failure for sendonion payments that failed before the call, instead of "Payment failure reason unknown" Signed-off-by: daywalker90 --- lightningd/pay.c | 9 +++++---- tests/test_pay.py | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lightningd/pay.c b/lightningd/pay.c index 19b692894901..6bae99f7d634 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -720,7 +720,7 @@ static struct command_result *wait_payment(struct lightningd *ld, &faildirection, &failmsg); /* Old DB might not save failure information */ - if (!failonionreply && !failnode) { + if (!failonionreply && !failnode && !failmsg) { return command_fail(cmd, PAY_UNSPECIFIED_ERROR, "Payment failure reason unknown"); } else if (failonionreply) { @@ -732,13 +732,14 @@ static struct command_result *wait_payment(struct lightningd *ld, NULL, faildetail), NULL); } else { - /* Parsed onion error, get its details */ - assert(failnode); + /* Parsed onion error, get its details. A sendonion + * failure has no erring node, only the failmsg. */ + assert(failnode || failmsg); fail = tal(tmpctx, struct routing_failure); fail->erring_index = failindex; fail->failcode = failcode; fail->erring_node = - tal_dup(fail, struct node_id, failnode); + tal_dup_or_null(fail, struct node_id, failnode); if (failchannel) { fail->erring_channel = tal_dup( diff --git a/tests/test_pay.py b/tests/test_pay.py index 57381a30791a..87aa92206b35 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -3471,7 +3471,7 @@ def test_sendpay_msatoshi_arg(node_factory): "wait_for_failure", [ False, - pytest.param(True, marks=pytest.mark.xfail(strict=True)), + True, ], ) def test_reject_invalid_payload(node_factory, wait_for_failure): From 7708aa05f462077f96bd086d3f89176475e399b2 Mon Sep 17 00:00:00 2001 From: daywalker90 Date: Tue, 13 Oct 2026 14:16:54 +0200 Subject: [PATCH 3/4] docs: correct the routing failure fields of pay, keysend, sendpay and waitsendpay The routing failure objects described fields the code does not send, and called other fields always present when they are not: - erring_node: waitsendpay on a payment sent with sendonion does not record the node. pay and keysend infer it from their route, so it may be absent in rare cases. - erring_channel: the daemon omits it when the channel is not known. The "or 0:0:0" text described a value that is never sent. - erring_direction: only sent together with erring_channel. Documented for all four commands and the sendpay_failure notification. - channel_update: not sent in the routing failure object. The field that carries the remote message is raw_message, which is now documented. It is not present for failures that did not come from a node, such as a local routing failure or the pay "ran out of routes" failure. - failcodename: documented for pay, keysend and sendpay, where it is omitted for unknown failcodes. sendpay always has the route it was given, so its erring_node text is unchanged. The sendpay_failure notification already marked erring_node and erring_channel as "if known". contrib/msggen/msggen/schema.json is regenerated from the schemas by make gen. Changelog-None Signed-off-by: daywalker90 --- contrib/msggen/msggen/schema.json | 33 +++++++++++-------- doc/schemas/keysend.json | 8 +++-- doc/schemas/notification/sendpay_failure.json | 2 +- doc/schemas/pay.json | 8 +++-- doc/schemas/sendpay.json | 6 ++-- doc/schemas/waitsendpay.json | 9 ++--- 6 files changed, 40 insertions(+), 26 deletions(-) diff --git a/contrib/msggen/msggen/schema.json b/contrib/msggen/msggen/schema.json index 96fd116660ee..bd5baaf70911 100644 --- a/contrib/msggen/msggen/schema.json +++ b/contrib/msggen/msggen/schema.json @@ -18584,10 +18584,12 @@ "A routing failure object has the fields below:", "", "*erring_index*: The index of the node along the route that reported the error. 0 for the local node, 1 for the first hop, and so on.", - "*erring_node*: The hex string of the pubkey id of the node that reported the error.", - "*erring_channel*: The short channel ID of the channel that has the error, or *0:0:0* if the destination node raised the error.", + "*erring_node*: The hex string of the pubkey id of the node that reported the error. This is inferred from the route, so it may be absent in rare cases.", + "*erring_channel*: The short channel ID of the channel that has the error. Only present if the channel is known.", + "*erring_direction*: The direction of traversing the *erring_channel*. Only present with *erring_channel*.", "*failcode*: The failure code, as per BOLT #4.", - "*channel_update*: The hex string of the *channel_update* message received from the remote node. Only present if error is from the remote node and the *failcode* has the `UPDATE` bit set, as per BOLT #4." + "*failcodename*: The human-readable name corresponding to *failcode*, if known.", + "*raw_message*: The hex string of the raw failure message from the node that reported the error. Not present for failures that did not come from a node, such as a local routing failure." ], "author": [ "Christian Decker [decker@blockstream.com](mailto:decker@blockstream.com) is mainly responsible." @@ -32846,10 +32848,12 @@ "A routing failure object has the fields below:", "", "*erring_index*: The index of the node along the route that reported the error. 0 for the local node, 1 for the first hop, and so on.", - "*erring_node*: The hex string of the pubkey id of the node that reported the error.", - "*erring_channel*: The short channel ID of the channel that has the error, or *0:0:0* if the destination node raised the error.", + "*erring_node*: The hex string of the pubkey id of the node that reported the error. This is inferred from the route, so it may be absent in rare cases.", + "*erring_channel*: The short channel ID of the channel that has the error. Only present if the channel is known.", + "*erring_direction*: The direction of traversing the *erring_channel*. Only present with *erring_channel*.", "*failcode*: The failure code, as per BOLT #4.", - "*channel_update*: The hex string of the *channel_update* message received from the remote node. Only present if error is from the remote node and the *failcode* has the UPDATE bit set, as per BOLT #4.", + "*failcodename*: The human-readable name corresponding to *failcode*, if known.", + "*raw_message*: The hex string of the raw failure message from the node that reported the error. Not present for failures that did not come from a node, such as a local routing failure.", "", "The *data* field of errors will include statistics *getroute_tries* and *sendpay_tries*. It will also contain a *failures* field with detailed data about routing errors." ], @@ -35794,9 +35798,11 @@ "", "*erring_index*: The index of the node along the route that reported the error. 0 for the local node, 1 for the first hop, and so on.", "*erring_node*: The hex string of the pubkey id of the node that reported the error.", - "*erring_channel*: The short channel ID of the channel that has the error, or *0:0:0* if the destination node raised the error. In addition *erring_direction* will indicate which direction of the channel caused the failure.", + "*erring_channel*: The short channel ID of the channel that has the error. Only present if the channel is known.", + "*erring_direction*: The direction of traversing the *erring_channel*. Only present with *erring_channel*.", "*failcode*: The failure code, as per BOLT #4.", - "*channel_update*: The hex string of the *channel_update* message received from the remote node. Only present if error is from the remote node and the *failcode* has the UPDATE bit set, as per BOLT #4." + "*failcodename*: The human-readable name corresponding to *failcode*, if known.", + "*raw_message*: The hex string of the raw failure message from the node that reported the error. Not present for failures that did not come from a node, such as a local routing failure." ], "author": [ "Rusty Russell [rusty@rustcorp.com.au](mailto:rusty@rustcorp.com.au) is mainly responsible." @@ -40975,11 +40981,12 @@ "A routing failure object has the fields below:", "", "*erring_index*: The index of the node along the route that reported the error. 0 for the local node, 1 for the first hop, and so on.", - "*erring_node*: The hex string of the pubkey id of the node that reported the error.", - "*erring_channel*: The short channel ID of the channel that has the error (or the final channel if the destination raised the error).", - "*erring_direction*: The direction of traversing the *erring_channel*:", + "*erring_node*: The hex string of the pubkey id of the node that reported the error. Only present if the node is known: a payment sent with **sendonion** does not record it.", + "*erring_channel*: The short channel ID of the channel that has the error. Only present if the channel is known.", + "*erring_direction*: The direction of traversing the *erring_channel*. Only present with *erring_channel*.", "*failcode*: The failure code, as per BOLT #4.", - "*failcodename*: The human-readable name corresponding to *failcode*, if known." + "*failcodename*: The human-readable name corresponding to *failcode*, if known.", + "*raw_message*: The hex string of the raw failure message from the node that reported the error. Not present for failures that did not come from a node, such as a local routing failure." ], "author": [ "ZmnSCPxj [ZmnSCPxj@protonmail.com](mailto:ZmnSCPxj@protonmail.com) is mainly responsible." @@ -42905,7 +42912,7 @@ "erring_direction": { "type": "u32", "description": [ - "The direction within the erring channel." + "The direction within the erring channel. Only present with *erring_channel*." ] }, "raw_message": { diff --git a/doc/schemas/keysend.json b/doc/schemas/keysend.json index 06f0ed7bd222..739d7ffaed50 100644 --- a/doc/schemas/keysend.json +++ b/doc/schemas/keysend.json @@ -218,10 +218,12 @@ "A routing failure object has the fields below:", "", "*erring_index*: The index of the node along the route that reported the error. 0 for the local node, 1 for the first hop, and so on.", - "*erring_node*: The hex string of the pubkey id of the node that reported the error.", - "*erring_channel*: The short channel ID of the channel that has the error, or *0:0:0* if the destination node raised the error.", + "*erring_node*: The hex string of the pubkey id of the node that reported the error. This is inferred from the route, so it may be absent in rare cases.", + "*erring_channel*: The short channel ID of the channel that has the error. Only present if the channel is known.", + "*erring_direction*: The direction of traversing the *erring_channel*. Only present with *erring_channel*.", "*failcode*: The failure code, as per BOLT #4.", - "*channel_update*: The hex string of the *channel_update* message received from the remote node. Only present if error is from the remote node and the *failcode* has the `UPDATE` bit set, as per BOLT #4." + "*failcodename*: The human-readable name corresponding to *failcode*, if known.", + "*raw_message*: The hex string of the raw failure message from the node that reported the error. Not present for failures that did not come from a node, such as a local routing failure." ], "author": [ "Christian Decker [decker@blockstream.com](mailto:decker@blockstream.com) is mainly responsible." diff --git a/doc/schemas/notification/sendpay_failure.json b/doc/schemas/notification/sendpay_failure.json index 02e47b1c3bdd..712f6f522f8e 100644 --- a/doc/schemas/notification/sendpay_failure.json +++ b/doc/schemas/notification/sendpay_failure.json @@ -190,7 +190,7 @@ "erring_direction": { "type": "u32", "description": [ - "The direction within the erring channel." + "The direction within the erring channel. Only present with *erring_channel*." ] }, "raw_message": { diff --git a/doc/schemas/pay.json b/doc/schemas/pay.json index fb3cef85aa98..f3e6b6938e92 100644 --- a/doc/schemas/pay.json +++ b/doc/schemas/pay.json @@ -226,10 +226,12 @@ "A routing failure object has the fields below:", "", "*erring_index*: The index of the node along the route that reported the error. 0 for the local node, 1 for the first hop, and so on.", - "*erring_node*: The hex string of the pubkey id of the node that reported the error.", - "*erring_channel*: The short channel ID of the channel that has the error, or *0:0:0* if the destination node raised the error.", + "*erring_node*: The hex string of the pubkey id of the node that reported the error. This is inferred from the route, so it may be absent in rare cases.", + "*erring_channel*: The short channel ID of the channel that has the error. Only present if the channel is known.", + "*erring_direction*: The direction of traversing the *erring_channel*. Only present with *erring_channel*.", "*failcode*: The failure code, as per BOLT #4.", - "*channel_update*: The hex string of the *channel_update* message received from the remote node. Only present if error is from the remote node and the *failcode* has the UPDATE bit set, as per BOLT #4.", + "*failcodename*: The human-readable name corresponding to *failcode*, if known.", + "*raw_message*: The hex string of the raw failure message from the node that reported the error. Not present for failures that did not come from a node, such as a local routing failure.", "", "The *data* field of errors will include statistics *getroute_tries* and *sendpay_tries*. It will also contain a *failures* field with detailed data about routing errors." ], diff --git a/doc/schemas/sendpay.json b/doc/schemas/sendpay.json index 5fcba87bc6dc..f2ee53de6428 100644 --- a/doc/schemas/sendpay.json +++ b/doc/schemas/sendpay.json @@ -364,9 +364,11 @@ "", "*erring_index*: The index of the node along the route that reported the error. 0 for the local node, 1 for the first hop, and so on.", "*erring_node*: The hex string of the pubkey id of the node that reported the error.", - "*erring_channel*: The short channel ID of the channel that has the error, or *0:0:0* if the destination node raised the error. In addition *erring_direction* will indicate which direction of the channel caused the failure.", + "*erring_channel*: The short channel ID of the channel that has the error. Only present if the channel is known.", + "*erring_direction*: The direction of traversing the *erring_channel*. Only present with *erring_channel*.", "*failcode*: The failure code, as per BOLT #4.", - "*channel_update*: The hex string of the *channel_update* message received from the remote node. Only present if error is from the remote node and the *failcode* has the UPDATE bit set, as per BOLT #4." + "*failcodename*: The human-readable name corresponding to *failcode*, if known.", + "*raw_message*: The hex string of the raw failure message from the node that reported the error. Not present for failures that did not come from a node, such as a local routing failure." ], "author": [ "Rusty Russell [rusty@rustcorp.com.au](mailto:rusty@rustcorp.com.au) is mainly responsible." diff --git a/doc/schemas/waitsendpay.json b/doc/schemas/waitsendpay.json index 7fcaa096b560..125889a0c42c 100644 --- a/doc/schemas/waitsendpay.json +++ b/doc/schemas/waitsendpay.json @@ -217,11 +217,12 @@ "A routing failure object has the fields below:", "", "*erring_index*: The index of the node along the route that reported the error. 0 for the local node, 1 for the first hop, and so on.", - "*erring_node*: The hex string of the pubkey id of the node that reported the error.", - "*erring_channel*: The short channel ID of the channel that has the error (or the final channel if the destination raised the error).", - "*erring_direction*: The direction of traversing the *erring_channel*:", + "*erring_node*: The hex string of the pubkey id of the node that reported the error. Only present if the node is known: a payment sent with **sendonion** does not record it.", + "*erring_channel*: The short channel ID of the channel that has the error. Only present if the channel is known.", + "*erring_direction*: The direction of traversing the *erring_channel*. Only present with *erring_channel*.", "*failcode*: The failure code, as per BOLT #4.", - "*failcodename*: The human-readable name corresponding to *failcode*, if known." + "*failcodename*: The human-readable name corresponding to *failcode*, if known.", + "*raw_message*: The hex string of the raw failure message from the node that reported the error. Not present for failures that did not come from a node, such as a local routing failure." ], "author": [ "ZmnSCPxj [ZmnSCPxj@protonmail.com](mailto:ZmnSCPxj@protonmail.com) is mainly responsible." From b5092ed6f1c033bdfe5bd31415014148639919c5 Mon Sep 17 00:00:00 2001 From: daywalker90 Date: Tue, 13 Oct 2026 15:32:44 +0200 Subject: [PATCH 4/4] plugins: check erring_node before keysend compares it keysend compares the failing node with the destination before it reads the failcode. The plugin only fills erring_node from the route when the daemon reports an erring_index inside the route, so the field can be NULL. Check for NULL before the comparison. This is defensive: the current code paths I checked always set erring_node for keysend, so no test covers this. It prevents a NULL dereference if that invariant changes. Changelog-None Signed-off-by: daywalker90 --- plugins/keysend.c | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/keysend.c b/plugins/keysend.c index 99495373d4d5..983491d87069 100644 --- a/plugins/keysend.c +++ b/plugins/keysend.c @@ -75,6 +75,7 @@ static struct command_result *keysend_cb(struct keysend_data *d, struct payment featurebit method.*/ if (p->result != NULL && + p->result->erring_node != NULL && node_id_eq(p->route_destination, p->result->erring_node) && p->result->failcode == WIRE_INVALID_ONION_PAYLOAD) { return payment_abort(