xpay: actually tell askrene when a payment succeeded.
What the developer wrote
Authored by Rusty Russell
xpay: actually tell askrene when a payment succeeded.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This commit fixes a bookkeeping bug in Core Lightning's xpay plugin. Previously, when a payment succeeded, xpay did not tell the askrene routing-helper that the route worked. As a result, askrene kept stale or overly pessimistic ideas about which channels could carry payments, which could make future payments fail unnecessarily or take worse routes. The patch adds the missing 'payment succeeded' notification for every non-local hop. It is a reliability/performance fix, not a direct theft-of-funds vulnerability.
Treat as a normal bug-fix patch. Reviewers should confirm that send_payment_req cannot be called with a NULL req and that the loop correctly skips the local hop. No urgent security response is indicated, but operators relying on xpay should upgrade to benefit from improved routing accuracy.
Security signals we found
Missing success feedback to routing layer can degrade payment reliability
Stale negative impressions may bias future path selection toward worse or failing routes
No direct funds-loss primitive is introduced or fixed
Change is purely additive notification logic in success path
Evidence from the diff
The change is in plugins/xpay/xpay.c in injectpaymentonion_succeeded(). After validating the injectpaymentonion result, the code now iterates attempt->hops starting at index 1 (skipping the local channel because auto.localchans is exact) and sends an askrene-inform-channel request with inform=’succeeded’ for each hop. The layer is set to the payment’s private_layer for fake channels or ‘xpay’ otherwise, and the short_channel_id_dir and amount_out are recorded. A new test test_xpay_informs_askrene_on_success verifies that a successful xpay creates exactly one impression on the xpay layer for the l2->l3 hop. An existing test is adjusted to age the xpay layer to avoid stale impressions causing an htlc_maximum_msat failure.
Changed components
plugins/xpay/xpay.cplugins/askrene routing layertests/test_xpay.pyInspect captured patch +40 / −0
diff --git a/plugins/xpay/xpay.c b/plugins/xpay/xpay.c
index bcd30c1d..5b37463e 100644
--- a/plugins/xpay/xpay.c
+++ b/plugins/xpay/xpay.c
@@ -1323,6 +1323,25 @@ static struct command_result *injectpaymentonion_succeeded(struct command *aux_c
plugin_err(aux_cmd->plugin, "Invalid injectpaymentonion result '%.*s'",
json_tok_full_len(result), json_tok_full(buf, result));
+ /* We don't tell it about payment success for the local channel, since
+ * auto.localchans is exact: adding an offset would make it worse! */
+ for (size_t i = 1; i < tal_count(attempt->hops); i++) {
+ struct out_req *req;
+ req = payment_ignored_req(aux_cmd, attempt, "askrene-inform-channel");
+ /* Put what we learned in xpay, unless it's a fake channel */
+ json_add_string(req->js, "layer",
+ attempt->hops[i].fake_channel
+ ? attempt->payment->private_layer
+ : "xpay");
+ json_add_short_channel_id_dir(req->js,
+ "short_channel_id_dir",
+ attempt->hops[i].scidd);
+ json_add_amount_msat(req->js, "amount_msat",
+ attempt->hops[i].amount_out);
+ json_add_string(req->js, "inform", "succeeded");
+ send_payment_req(aux_cmd, attempt->payment, req);
+ }
+
outgoing_notify_success(attempt);
/* Move from current_attempts to past_attempts */
diff --git a/tests/test_xpay.py b/tests/test_xpay.py
index 90cb2a7e..eae28776 100644
--- a/tests/test_xpay.py
+++ b/tests/test_xpay.py
@@ -290,6 +290,10 @@ def test_xpay_fake_channeld(node_factory, bitcoind, chainparams, slow_mode):
f"amount={AMOUNT}msat"]).decode('utf-8').strip()
assert l1.rpc.decode(inv)['payee'] == nodeids[n]
failed_parts.append(l1.rpc.xpay(inv)['failed_parts'])
+ # FIXME: We fail on #10, due mainly to a buildup of usage on 0x2134x0/0:
+ # Failed: We could not find a usable set of paths. The shortest path is 103x1x0->0x2134x0->1725x11x1725, but 0x2134x0/0 exceeds htlc_maximum_msat ~1000448msat
+ # So we "age" the xpay layer to forget old successful payments.
+ l1.rpc.askrene_age('xpay', 1)
# Should be no reservations left (clean up happens after return though)
wait_for(lambda: l1.rpc.askrene_listreservations() == {'reservations': []})
@@ -1413,6 +1417,23 @@ def test_sendamount(node_factory):
ret = l1.rpc.sendamount(b12, "100sat")
+def test_xpay_informs_askrene_on_success(node_factory):
+ """After a successful payment, xpay should add an impression to the xpay askrene layer."""
+ l1, l2, l3 = node_factory.get_nodes(3)
+ node_factory.join_nodes([l1, l2, l3], wait_for_announce=True)
+
+ inv = l3.rpc.invoice(100000, "test-inform", "test inform")["bolt11"]
+ l1.rpc.xpay(inv)
+
+ # xpay skips the local channel (hops[0]), so only the l2->l3 hop gets an impression
+ scid23dir = first_scidd(l2, l3)
+ layers = l1.rpc.askrene_listlayers('xpay')['layers']
+ impressions = only_one(layers)['impressions']
+ assert len(impressions) == 1
+ assert impressions[0]['short_channel_id_dir'] == scid23dir
+ assert impressions[0]['amount_msat'] == 100000
+
+
def test_sendamount_bip353(node_factory):
fakebip353_plugin = Path(__file__).parent / "plugins" / "fakebip353.py"
Why this scored 24/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.