Skip to content

lightningd: fix 209 "Payment failure reason unknown" for sendonion payments - #9623

Open
daywalker90 wants to merge 4 commits into
ElementsProject:masterfrom
daywalker90:fix-sendonion-failinfo
Open

daywalker90 wants to merge 4 commits into
ElementsProject:masterfrom
daywalker90:fix-sendonion-failinfo

Conversation

@daywalker90

Copy link
Copy Markdown
Collaborator

Saw this during a CI run:

2026-10-12T17:54:04.0433344Z     def test_reject_invalid_payload(node_factory):
2026-10-12T17:54:04.0436343Z         """Send an onion payload with an unknown even type.
2026-10-12T17:54:04.0440052Z     
2026-10-12T17:54:04.0445254Z         Recipient l2 should reject it the incoming HTLC with an invalid onion
2026-10-12T17:54:04.0447886Z         payload error.
2026-10-12T17:54:04.0450163Z     
2026-10-12T17:54:04.0454843Z         """
2026-10-12T17:54:04.0458605Z     
2026-10-12T17:54:04.0460904Z         l1, l2 = node_factory.line_graph(2)
2026-10-12T17:54:04.0463275Z         amt = 10**3
2026-10-12T17:54:04.0465724Z         route = l1.single_route(l2.info['id'], amt)
2026-10-12T17:54:04.0468400Z         inv = l2.rpc.invoice(amt, "lbl", "desc")
2026-10-12T17:54:04.0471497Z     
2026-10-12T17:54:04.0495855Z         first_hop = {'id': route[0]['node_id_out'], 'amount_msat': route[0]['amount_out_msat'], 'delay': route[0]['cltv_out']}
2026-10-12T17:54:04.0499642Z     
2026-10-12T17:54:04.0501172Z         # A TLV payload with an unknown even type:
2026-10-12T17:54:04.0503980Z         payload = TlvPayload()
2026-10-12T17:54:04.0506044Z         payload.add_field(0xB000, b'Hi there')
2026-10-12T17:54:04.0508363Z         hops = [{"pubkey": l2.info['id'], "payload": payload.to_hex()}]
2026-10-12T17:54:04.0510452Z         onion = l1.rpc.createonion(hops=hops, assocdata=inv['payment_hash'])
2026-10-12T17:54:04.0513728Z         l1.rpc.sendonion(onion=onion['onion'],
2026-10-12T17:54:04.0515099Z                          first_hop=first_hop,
2026-10-12T17:54:04.0516615Z                          payment_hash=inv['payment_hash'],
2026-10-12T17:54:04.0518361Z                          shared_secrets=onion['shared_secrets'])
2026-10-12T17:54:04.0520313Z         with pytest.raises(RpcError, match=r'WIRE_INVALID_ONION_PAYLOAD'):
2026-10-12T17:54:04.0521068Z >           l1.rpc.waitsendpay(inv['payment_hash'])
2026-10-12T17:54:04.0522626Z 
2026-10-12T17:54:04.0522850Z tests/test_pay.py:3495: 
2026-10-12T17:54:04.0753397Z >       with pytest.raises(RpcError, match=r'WIRE_INVALID_ONION_PAYLOAD'):
2026-10-12T17:54:04.0754805Z E       AssertionError: Regex pattern did not match.
2026-10-12T17:54:04.0760832Z E         Expected regex: 'WIRE_INVALID_ONION_PAYLOAD'
2026-10-12T17:54:04.0763907Z E         Actual message: "RPC call failed: method: waitsendpay, payload: {'payment_hash': '43774c86cb18011de491f33473ebfe42733494def4915745b50127ca4d236039'}, error: {'code': 209, 'message': 'Payment failure reason unknown'}"

Also cleaned up related docs for routing failure object.

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 <daywalker990@gmx.de>
…e 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 <daywalker990@gmx.de>
… 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 <daywalker990@gmx.de>
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 <daywalker990@gmx.de>
@daywalker90
daywalker90 requested a review from cdecker as a code owner October 13, 2026 14:13
@cdecker
cdecker enabled auto-merge (rebase) October 14, 2026 10:09

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants