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." 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/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( diff --git a/tests/test_pay.py b/tests/test_pay.py index a853177f5245..87aa92206b35 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, + 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'])