Skip to content

Deprecate pay, keysend, renepay and getroute#9110

Open
rustyrussell wants to merge 40 commits intoElementsProject:masterfrom
rustyrussell:guilt/deprecate-pay
Open

Deprecate pay, keysend, renepay and getroute#9110
rustyrussell wants to merge 40 commits intoElementsProject:masterfrom
rustyrussell:guilt/deprecate-pay

Conversation

@rustyrussell
Copy link
Copy Markdown
Contributor

@rustyrussell rustyrussell commented May 5, 2026

  1. Complete the xpay feature set:
    1. Add xkeysend variant for "keysend using xpay",
    2. Add label and localinvreqid parameter support,
    3. Implement CLTV shadow routing in xpay.
    4. Handle channel_updates in errors in xpay.
    5. Improve compat handling for xpay-handle-pay: "succeed" like pay did if we're already paid.
  2. Enhance getroutes and sendpay: getroutes gets clearer and more complete fields, and sendpay understands them so the two easily work together
  3. Deprecate pay, paystatus, keysend, getroute, and renepay/renepaystatus (all with removal window v26.06–v27.03), and make xpay-handle-pay the default.

@rustyrussell rustyrussell added this to the v26.06 milestone May 5, 2026
@rustyrussell rustyrussell requested a review from Lagrang3 May 5, 2026 02:24
@rustyrussell rustyrussell requested a review from cdecker as a code owner May 5, 2026 02:24
@madelinevibes madelinevibes added Status::Ready for Review The work has been completed and is now awaiting evaluation or approval. PLEASE clear CI 🫠 labels May 5, 2026
Lagrang3
Lagrang3 reviewed May 5, 2026
View reviewed changes
Comment thread plugins/xpay/xpay.c

/* hop has size prepended, so this isn't a simple "append" */
static void hop_append(u8 **hop, const u8 *appendme)
{
Copy link
Copy Markdown
Collaborator

@Lagrang3 Lagrang3 May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a check for the trivial case

if(tal_bytelen(appendme)==0) return;

Copy link
Copy Markdown
Contributor Author

@rustyrussell rustyrussell May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case I'd prefer to do that test in the caller, but it's a bit of a premature optimization. It's not like the code doesn't work in that case...

Lagrang3
Lagrang3 approved these changes May 5, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@Lagrang3 Lagrang3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been a lot of effort.
Nice work on building xkeysend and making getroutes compatible with sendpay.

@Lagrang3
Copy link
Copy Markdown
Collaborator

Lagrang3 commented May 5, 2026 •
edited
Loading

@rustyrussell, I think we could keep the old json rpc names keysend and pay to be presented as a legacy API and under the surface be calling xkeysend and xpay. We wouldn't need to deprecate them, or break existing software relying on that API and in any case the names suit for their purposes. Someone coming into core lightning one year from now will be asking "why is that command called xpay instead of simply pay".

I know there is a little bit of work to be done to digest the parameters in pay to the almost equivalent xpay call, in particular with the exclude option, but it is workable.

@rustyrussell
Copy link
Copy Markdown
Contributor Author

rustyrussell commented May 7, 2026

@rustyrussell, I think we could keep the old json rpc names keysend and pay to be presented as a legacy API and under the surface be calling xkeysend and xpay. We wouldn't need to deprecate them, or break existing software relying on that API and in any case the names suit for their purposes. Someone coming into core lightning one year from now will be asking "why is that command called xpay instead of simply pay".

I know there is a little bit of work to be done to digest the parameters in pay to the almost equivalent xpay call, in particular with the exclude option, but it is workable.

Instead of removing pay, we can just make it an alias for xpay when pay is fully deprecated? We could allow "invstring" as a parameter to pay: that covers 99% of the transition already (amount_msat and label are already the same).

Similarly, keysend just becomes an alias for xkeysend.

rustyrussell added 10 commits May 7, 2026 16:11
@rustyrussell
common: implement tal_arr_append and tal_arr_appendn, and use them.
fdd89e2 
These are useful for the common pattern of "append these bytes to this tal array".

As a bonus, we do memcheck() on all these callers, for extra checking
under valgrind.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
CCAN: update to get json_escape_dup helper.
35cd6ee 
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
askrene: all remove_small_channel_layer when maxparts=1.
4fbe683 
We were doing it for "auto.no_mpp_support", but not maxparts=1.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
askrene: improve errors we return.
5491bdc 
In particular, we can return PAY_ROUTE_TOO_EXPENSIVE.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
askrene: make failure messages clearer:
876d346 
If all channels are disabled, say so clearly.  Also reword: it's not
the source or destination which have disabled the channels.  While
we're here, remove the double space, lest people think I'm Satoshi.

Before:
	Failed after 1 attempts.
	Unexpected error (invalid_onion_payload) from final node: disabling 103x1x0/1 for this payment.
	Then routing failed: We could not find a usable set of paths.  The source has disabled 1 of 1 channels, leaving capacity only 0msat of 1000000000msat.

After:
	Failed after 1 attempts.
	Unexpected error (invalid_onion_payload) from final node: disabling 103x1x0/1 for this payment.
	Then routing failed: We could not find a usable set of paths. All 1 channels to the source are disabled.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
pytest: use maxparts=1 not the soon-to-be-deprecated "auto.no_mpp_sup…
0e9708c 
…port" layer.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
askrene: deprecate auto.no_mpp_support layer in favor of maxparts=1.
e04430c 
This layer predated the maxparts parameter.

Changelog-Deprecated: JSON-RPC: `getroutes` layer `auto.no_mpp_support`: use `maxparts=1` parameter instead (available since v25.09).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
xpay: remove internal "disable_mpp" flag.
fcff17a 
We already have `maxparts`: simply set that to 1 for the same effect.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
xpay: extract payment creation logic.
3cf10aa 
Simple refactor so we can use it for `xkeysend` too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
xpay: add extra_tlvs support (unused for now).
1b41239 
This adds them to the final onion (but it's always NULL for now).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell force-pushed the guilt/deprecate-pay branch from 5a447d1 to c2ca227 Compare May 7, 2026 06:44
@rustyrussell
Copy link
Copy Markdown
Contributor Author

rustyrussell commented May 7, 2026

@rustyrussell, I think we could keep the old json rpc names keysend and pay to be presented as a legacy API and under the surface be calling xkeysend and xpay. We wouldn't need to deprecate them, or break existing software relying on that API and in any case the names suit for their purposes. Someone coming into core lightning one year from now will be asking "why is that command called xpay instead of simply pay".
I know there is a little bit of work to be done to digest the parameters in pay to the almost equivalent xpay call, in particular with the exclude option, but it is workable.

Instead of removing pay, we can just make it an alias for xpay when pay is fully deprecated? We could allow "invstring" as a parameter to pay: that covers 99% of the transition already (amount_msat and label are already the same).

Similarly, keysend just becomes an alias for xkeysend.

I've add this change: pay now accepts invstring so people can use it today.

@rustyrussell
injectpaymentonion: add explicit destination option.
d4c73b2 
This allows us to correctly annotate the destination in listsendpays
even if invstring isn't set (xkeysend).

Changelog-Added: JSON-RPC: `injectpaymentonion` parameter `destination`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
xpay: add label and localinvreqid params
8616584 
This bring us closer to pay parity, and `localinvreq` is needed for
invoicerequest payments.

Changelog-Added: JSON-RPC: `xpay` now accepts `label` and `localinvreqid` parameters (like `pay`).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
xpay: handle localinvreqid and label parameters when we're called as …
5c014b1 
…`pay`.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
xpay: process channel_update from error for the current payment.
61667ea 
Applying it globally as `pay` did risks leaking information, but applying
it for retries doesn't really.  And doing that is fairly easy with layers.

Changelog-Added: Protocol: `xpay` will now update for the current payment if it gets a `channel_update` in an error message.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
xpay: add a CLTV shadow for single non-blinded paths.
eabdb3c 
BOLT 7 recommends this.  pay did it: in fact, pay would also add sats,
but that's much more difficult and fraught, so only do CLTV.

Changelog-Added: Protocol: `xpay` now uses shadow CLTV additions to help mask final destination as per BOLT 7.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
xpay: return the correct (documented!) error code on repeat payments.
dcd2f16 
The man page says 219, but we didn't return that.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON-RPC: `xpay` now correctly returns error code 219 on already paid invoices (not 218).
@rustyrussell
xpay: correctly identify final node when using a dummy blinded path.
2accd1e 
We would get WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS in this case if
the invoice was unknown, and think it weird because it came from an
intermediate node.  In practice the result is the same (as it would remove
the blinded path), but it was misleading.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
xpay: when acting as pay, imitate its idempotent behavior.
42d015b 
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
askrene: populate struct route with more complete information.
5ac6627 
Much of this is redundant, but now it's better labeleled and stands alone.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell added 21 commits May 7, 2026 16:33
@rustyrussell
getroutes: return more detailed fields in routes[].
79ecbc6 
This reflects a long-standing complaint from @Lagrang3 when the API
was first implemented, and I should have listened.  In particular, the
impedance mismatch with the sendpay API is annoying.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `getroutes` `route` explicit fields `node_id_in`, `node_id_out`, `amount_in_msat`, `amount_out_msat`, `cltv_in`, `cltv_out`.
@rustyrussell
getroutes: deprecate old vaguely named fields.
0a1617c 
Changelog-Deprecated: JSON-RPC: `getroutes` `route` fields `next_node_id`, `amount_msat` and `delay` (use `node_id_out`, `amount_in_msat` and `cltv_in`).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
sendpay: ignore unknown route fields, but take the getroutes-styl…
6dd25db 
…e ones.

Changelog-Added: JSON-RPC: `sendpay` now accepts one of the `paths` returned from `getroutes` as its `route` parameter.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
pyln: add getroutes wrapper to pyln-client, add single_route helper t…
afbeb1d 
…o pyln-testing.

And remove the two getroute() calls inside pyln-testing.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
tests: use getroutes, not getroute (simple cases)
b7e1803 
We add a nice getroutes wrapper and use it to replace the simple
getroute() calls which simply hand the result to sendpay().

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
pytest: convert remaining getroute cases.
824353d 
Excluding explicit getroute tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
xpay: add xkeysend support.
d1a60d4 
This uses askrene, so it benefits from previous payment experience.

Changelog-Added: JSON-RPC: `xkeysend` command for keysend with modern routing support.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
pytest: speed up test_keysend_description_size_limit
7099a2b 
Starting up two nodes for each variant is slow, so do this internally.
Also check that the descriptions are correct, not just that there is
some invoice!

Before:
================================ 7 passed in 75.63s (0:01:15) ================================

After:
===================================== 1 passed in 12.62s =====================================

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
pytest: test xkeysend as thoroughly as keysend.
989fb69 
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
offers: use xpay.
838bcd1 
Changelog-Changed: offers: we now use `xpay` not `pay` for paying invoices made with invoicerequest().
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
pytest: make pay tests more robust.
6f762a0 
1. Don't assume they send group 0 (thus waitsendpay works).  waitsendpay should be unnecessary
   after pay anyway.
2. Don't use pay for test_multichan_stress, since it relies on pay not learning that
   the channel is unreliable.
3. Use xpay for test_listhtlcs_wait, since we want no shadow.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
pyln-testing: allow bitcoind mocks to return None to mean "don't mock".
1bc0ca3 
Good for conditional mocks, like in next patch.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
pytest: use xpay, not pay in misc tests.
4f7cc71 
This usually means changing the error strings, where we test them, and
avoiding `paystatus`.

Other changes:
1. Completely get rid of `test_custom_notification_topics`: xpay tests do this
   already.
2. Add xpay wrapper to pyln-client so we can mix named and unnamed args.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
xpay: take over pay invocations by default.
6a346c8 
The main change here is the error messages got better.  Where that was
the case, and the test was not completely redundant with existing xpay
tests, I explicitly converted to calls to xpay (meaning the tests
will be kept when `pay` is finally removed).

Test changes:
1. deschash checking is old-pay only, keep that there.
2. Add debug logs for checks for using forwarding via scid in injectpaymentonion.
3. Don't assert that not all sendpays should have the invoice string: xpay does not try to be clever there.
4. We no longer check if invoices are already paid before routing, since injectpaymentonion will catch that for us.
5. Removed `test_pay_get_error_with_update` which has xpay equiv.

Changelog-Changed: JSON-RPC: `xpay` now handles `pay` command by default (use `xpay-handle-pay=false` to prevent this)
Changelog-Removed: JSON-RPC: `exclude` parameter to `pay` (when `xpay-handle-pay` is True): craft a layer with desired modifications and pass it to `xpay` `layers`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
pay: accept invstring as first parameter name.
011a147 
If you don't use complex pay options, you can use `invstring` now and when xpay
becomes pay, there's no transition.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `pay` now accepts `invstring` as a parameter name for `bolt11`, to ease transition when xpay takes over in v27.03.
@rustyrussell
pay: deprecate pay and paystatus.
b011b72 
Use xpay and listpays.

Some tests which are pay/paystatus specific simply enabled deprecations.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: JSON-RPC: `pay` and `paystatus`: use `xpay`, `listpays` (or `xpay`'s notifications for details of attempts).  `pay` will be replaced by `xpay` in v27.03.
@rustyrussell
getroute: deprecate in favor of getroutes
b117a87 
Changelog-Deprecated: JSON-RPC: `getroute` (use `getroutes` with layers `["auto.localchans","auto.sourcefree"]` and `maxparts=1`) (since v25.09).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
keysend: deprecate in favor of xkeysend.
e193ea2 
Changelog-Deprecated: JSON-RPC: `keysend` (use `xkeysend`).  `xkeysend` will take over `keysend` in v27.03.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
xpay: keep track of ongoing payments, to implement attempt_ongoing()
8dfa80d 
This will help when we implement listpays (next) to be xpay-aware.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
plugins: move listpays from pay plugin to xpay plugin.
88dca90 
This will make life easier when we remove the pay plugin, but also fixes
an issue where `listpays` can indicate ongoing payments are failed,
even though xpay is still working on it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
renepay: deprecate.
58e1b79 
I think Eduardo and I stole all the best bits to make xpay and askrene.

I simply enabled deprecations on all the renepay tests.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell force-pushed the guilt/deprecate-pay branch from c2ca227 to 58e1b79 Compare May 7, 2026 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lagrang3 Lagrang3 Lagrang3 approved these changes

@cdecker cdecker Awaiting requested review from cdecker cdecker is a code owner

Assignees

No one assigned

Labels

PLEASE clear CI 🫠 Status::Ready for Review The work has been completed and is now awaiting evaluation or approval.

Projects

None yet

Milestone

v26.06

Development

Successfully merging this pull request may close these issues.

3 participants

@rustyrussell @Lagrang3 @madelinevibes