-
Notifications
You must be signed in to change notification settings - Fork 968
Feature/xpay payment description #8784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Feature/xpay payment description #8784
Conversation
219da3a to
c15a836
Compare
13a8ffe to
644563e
Compare
Changelog-Added: Add 'payer-note' field to the 'xpay' RPC call.
644563e to
51e5ed7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Nazarevsky Thanks for the PR! Since this is your first contribution, I’m being a bit more nitpicky about commit structure. Even if this one doesn’t end up being merged for the reason below, this review should be useful as a reference for future PRs.
That said, I wanted to better understand whether this PR serves its intended purpose. Don’t the existing invoice.description and fetchinvoice.payer_note already cover this use case? A node can already inspect these via listinvoices. How is the payer_note sent via xpay received on the other node? Adding @Lagrang3 in case I’ve missed anything.
| from pyln.grpc import node_pb2 as node__pb2 | ||
|
|
||
| GRPC_GENERATED_VERSION = '1.75.1' | ||
| GRPC_GENERATED_VERSION = '1.76.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI is currently failing due to version mismatch in your grpcio. Please downgrade your local grpcio (uv pip install grpcio==1.75.1).
| const char **layers; | ||
| u32 *maxdelay; | ||
| unsigned int *retryfor; | ||
| const char *payer_note; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: To keep the formatting consistent and avoid tabs/spaces alignment issues (especially between editors and the GitHub UI), could we format this to match the existing tab-based style?
| json_add_string(req->js, "bip353", xparams->bip353); | ||
| return send_outreq(req); | ||
| if (xparams->bip353) | ||
| json_add_string(req->js, "bip353", xparams->bip353); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mismatched alignment.
Thank you @ShahanaFarooqui for comments! I've updated my PR description a bit about my changes. Nevertheless, I'd like to answer your questions here. I think that |
Resolves #8514
Problem
xpayRPC call sends funds using Bolt11 or Bolt12 invoices provided. According to Bolt12, Bolt12 invoices support providing descriptions for invoices ('description' field) and messages to a payee ('payer_note' field).xpayhas a couple of RPC calls (likefetchinvoiceorcreate_invoice) under the hood which already have a corresponding logic for sending a custom message to a payee (usingpayer_notefield). However,xpaydoes not have a field to provide payment description.Changes
payer_notefiled toxpayRPC call;payer_notedefined.Important
26.04 FREEZE March 11th: Non-bugfix PRs not ready by this date will wait for 26.06.
RC1 is scheduled on March 23rd
The final release is scheduled for April 15th.
Checklist
Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:
tools/lightning-downgrade