lightningd: fix error code on waitsendpay on old errors.
What the developer wrote
Authored by Rusty Russell
lightningd: fix error code on waitsendpay on old errors.
We don't explicitly save the return code in db, so we need to reconstruct it.
We didn't cover the "peer told us our onion was bad" corner case. But it's hardly
worth a changelog message, since users will never see this.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This commit fixes a corner case where Core Lightning returned the wrong error code when a payment failed because a peer directly reported that our encrypted routing packet (the 'onion') was malformed. Previously the code only distinguished between permanent destination failures and 'try another route,' but it missed the case where the immediate peer couldn't parse the onion at all. The fix adds the correct error code for that situation and removes a test marker that expected the bug to fail.
No immediate action required beyond normal patching. Operators should update to a version containing this commit if they rely on accurate waitsendpay error codes for payment-failure diagnostics or automated retry logic.
Security signals we found
Incorrect error-code reconstruction for historical payment failures
Missing handling of BADONION failure bit in RPC error mapping
Test previously expected failure (xfail) now passing after fix
Evidence from the diff
In lightningd/pay.c, wait_payment() reconstructs RPC error codes for historical payment failures because the original return code is not stored in the database. The existing logic chose PAY_DESTINATION_PERM_FAIL or PAY_TRY_OTHER_ROUTE but did not handle failcodes with the BADONION bit, which indicates the peer rejected the onion itself. The patch adds a branch that returns PAY_UNPARSEABLE_ONION when failcode & BADONION is true. The accompanying test test_bad_onion_immediate_peer had been marked xfail(strict=True); that marker is removed because the test now passes.
Changed components
lightningd/pay.ctests/test_misc.pyInspect captured patch +7 / −3
diff --git a/lightningd/pay.c b/lightningd/pay.c
index 17a41a2e..156ecf30 100644
--- a/lightningd/pay.c
+++ b/lightningd/pay.c
@@ -745,8 +745,13 @@ static struct command_result *wait_payment(struct lightningd *ld,
/* FIXME: We don't store this! */
fail->msg = NULL;
- rpcerrorcode = faildestperm ? PAY_DESTINATION_PERM_FAIL
- : PAY_TRY_OTHER_ROUTE;
+ /* Peers which fail directly can hit this! */
+ if (failcode & BADONION)
+ rpcerrorcode = PAY_UNPARSEABLE_ONION;
+ else if (faildestperm)
+ rpcerrorcode = PAY_DESTINATION_PERM_FAIL;
+ else
+ rpcerrorcode = PAY_TRY_OTHER_ROUTE;
return sendpay_fail(
cmd, payment, rpcerrorcode, NULL, fail,
diff --git a/tests/test_misc.py b/tests/test_misc.py
index 7d85942f..0cfdbe21 100644
--- a/tests/test_misc.py
+++ b/tests/test_misc.py
@@ -2135,7 +2135,6 @@ def test_bad_onion(node_factory, bitcoind):
assert err.value.error['data']['erring_channel'] == route[1]['channel']
-@pytest.mark.xfail(strict=True)
def test_bad_onion_immediate_peer(node_factory, bitcoind):
"""Test that we handle the malformed msg when we're the origin"""
l1, l2 = node_factory.line_graph(2, opts=[{}, {'dev-fail-process-onionpacket': None}])
Why this scored 23/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.