Skip to content

Deprecate xpay-handle-pay - #9254

Open
Lagrang3 wants to merge 3 commits into
ElementsProject:masterfrom
Lagrang3:remove-pay-backend
Open

Deprecate xpay-handle-pay#9254
Lagrang3 wants to merge 3 commits into
ElementsProject:masterfrom
Lagrang3:remove-pay-backend

Conversation

@Lagrang3

@Lagrang3 Lagrang3 commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

@rustyrussell deprecated pay in #9110, but he forgot to deprecate xpay-handle-pay.
This PR deprecates xpay-handle-pay for v26.09.

@Lagrang3
Lagrang3 force-pushed the remove-pay-backend branch from fa2c776 to fa8038a Compare July 1, 2026 07:31
@Lagrang3 Lagrang3 changed the title Remove pay backend Deprecated xpay-handle-pay Jul 1, 2026
@Lagrang3
Lagrang3 marked this pull request as ready for review July 1, 2026 07:34
@Lagrang3
Lagrang3 requested a review from cdecker as a code owner July 1, 2026 07:34
@Lagrang3
Lagrang3 requested a review from nGoline July 1, 2026 07:35
@Lagrang3
Lagrang3 force-pushed the remove-pay-backend branch 3 times, most recently from 89f8387 to 72d3263 Compare July 1, 2026 13:00
@Lagrang3
Lagrang3 force-pushed the remove-pay-backend branch from 72d3263 to 3660f6f Compare July 9, 2026 14:14
@Lagrang3
Lagrang3 requested a review from daywalker90 as a code owner July 9, 2026 14:14
@Lagrang3

Lagrang3 commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author
  • fixed errors in tests due to the deprecation,
  • rebased

@Lagrang3 Lagrang3 added the Status::Ready for Review The work has been completed and is now awaiting evaluation or approval. label Jul 9, 2026
@Lagrang3 Lagrang3 added this to the v26.09 milestone Jul 9, 2026
@Lagrang3 Lagrang3 changed the title Deprecated xpay-handle-pay Deprecate xpay-handle-pay Jul 9, 2026
@Lagrang3
Lagrang3 force-pushed the remove-pay-backend branch from 3660f6f to cc78e39 Compare July 10, 2026 04:59
@Lagrang3

Lagrang3 commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

After this afea381 in v26.06 we were having xpay processing all payments by default. This meant that tests/test_pay.py was not covering pay's legacy backend. Here I brought back those tests by explicitly running them with xpay-handle-pay in both True and False cases. Some tests that are not aiming at pay RPC
but test something else are preferred run with xpay because the pay command used is irrelevant.

These tests could have been enabled for xpay-handle-pay=False but they fail or are flaky:

  • test_pay_avoid_low_fee_chan

@Lagrang3
Lagrang3 force-pushed the remove-pay-backend branch 5 times, most recently from c500a89 to 529fff9 Compare July 10, 2026 09:14
daywalker90
daywalker90 reviewed Jul 13, 2026
View reviewed changes

@daywalker90 daywalker90 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There is still usage of pay in test_invoices.py::test_invoice_deschash

Comment thread plugins/pay.c Resolved
@cdecker
cdecker force-pushed the remove-pay-backend branch from 529fff9 to 67837b6 Compare August 3, 2026 12:15
@cdecker

cdecker commented Aug 3, 2026

Copy link
Copy Markdown
Member

Rebased on top of master for merging.

@cdecker

cdecker commented Aug 3, 2026

Copy link
Copy Markdown
Member

@Lagrang3 the tests now fail reliably, due to the deprecation warning. Can you fix these up?

@cdecker
cdecker marked this pull request as draft August 3, 2026 13:18
cdecker
cdecker requested changes Aug 3, 2026
View reviewed changes

@cdecker cdecker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tests are failing with new deprecation warnings.

@Lagrang3
Lagrang3 force-pushed the remove-pay-backend branch 2 times, most recently from adcae60 to 5f68000 Compare August 8, 2026 11:09
@Lagrang3

Lagrang3 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author
  • replaced pay for xpay in failing tests
  • updated doc examples
  • rebased

@Lagrang3
Lagrang3 requested a review from cdecker August 8, 2026 13:58
@Lagrang3
Lagrang3 marked this pull request as ready for review August 8, 2026 13:58
@Lagrang3

Lagrang3 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

rebased

@madelinevibes madelinevibes modified the milestones: v26.09, v26.12 Oct 14, 2026
nGoline
nGoline requested changes Nov 11, 2026
View reviewed changes

@nGoline nGoline left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

CI is red for one reason only: tests/test_opening.py::test_zero_length_upfront_shutdown_script landed on master after your last round and calls l1.rpc.pay() at line 4014 to drain funds to l2. It fails with Command "pay" is deprecated in First Integration, ASan, Valgrind and the BTC v25 job. Switching it to l1.rpc.xpay() in the commit that adds the pay annotation fixes it and keeps that commit green. I scanned the rest of tests/ for pay callers without allow-deprecated-apis and found nothing else live, but Full Integration was skipped on this run, so it still needs one green pass.

With the test fix and the doc version, this is good for 26.12 from my side.

Comment thread doc/lightningd-config.5.md Resolved
Comment thread plugins/pay.c Resolved
Comment thread plugins/pay.c Resolved
Comment thread plugins/xpay/xpay.c Resolved
Comment thread plugins/libplugin.c Resolved
@Lagrang3
libplugin: add macro to deprecate dynamic options
c425451 
Changelog-None

Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
@Lagrang3
xpay: deprecate xpay-handle-pay
c7d50c7 
Changelog-Deprecated: Config: option `xpay-handle-pay`, `pay` is deprecated and will be replaced by `xpay` in v27.03, hence `xpay-handle-pay` is not needed anymore. The configuration option will no longer be shown in listconfigs.

Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
@Lagrang3
pay: add missing deprecation annotation
441fc9f 
- add missing deprecation annotation to "pay" command,
- fix tests that required allow-deprecated-apis=True in order to
continue using "pay",
- change "pay" to "xpay" in those tests that
are NOT testing "pay" directly but just want to make a payment,
- and in tests that could be relevant for both "xpay" and "pay", use
  both.

Changelog-None

Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
nGoline
nGoline approved these changes Nov 11, 2026
View reviewed changes

@nGoline nGoline left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, all addressed!

@cdecker your change request was about the deprecation test failures and those are fixed on this head, could you re-review or dismiss?

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

Reviewers

@daywalker90 daywalker90 daywalker90 left review comments
@nGoline nGoline nGoline approved these changes
@cdecker cdecker Awaiting requested review from cdecker cdecker is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

@Lagrang3 Lagrang3

Labels

needs-rebase Status::Ready for Review The work has been completed and is now awaiting evaluation or approval.

Projects

None yet

Milestone

v26.12

Development

Successfully merging this pull request may close these issues.

5 participants

@Lagrang3 @cdecker @nGoline @daywalker90 @madelinevibes