cmd: addinvoice --qr to print invoice QR#10638
cmd: addinvoice --qr to print invoice QR#10638hieblmi wants to merge 2 commits intolightningnetwork:masterfrom
addinvoice --qr to print invoice QR#10638Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a useful feature to display a QR code for an invoice via the --qr flag in the addinvoice command. The implementation is straightforward and correct. I have one suggestion to add a comment to the new code block to align with the repository's style guide regarding code documentation.
ziggie1984
left a comment
There was a problem hiding this comment.
Have you tested this with a bigger invoice including blinded paths ? I wonder what is the information limit for this QR code representation ?
go.mod
Outdated
| ) | ||
|
|
||
| require ( | ||
| github.com/mdp/qrterminal/v3 v3.2.1 // indirect |
There was a problem hiding this comment.
why are these in a separarte required block ?
There was a problem hiding this comment.
moved them, fixed.
| inputs for fee estimation. | ||
|
|
||
| * The `addinvoice` command now supports a `--qr` flag that renders the payment | ||
| request as a QR code directly in the terminal, making it easy to scan from a |
There was a problem hiding this comment.
This entry belongs under ## lncli Updates, not ## lncli Additions. Additions is for new commands; this is a new flag on an existing command. For reference, the --include_log flag added to getdebuginfo in the same file follows this convention correctly.
| inputs for fee estimation. | ||
|
|
||
| * The `addinvoice` command now supports a `--qr` flag that renders the payment | ||
| request as a QR code directly in the terminal, making it easy to scan from a |
There was a problem hiding this comment.
This entry is missing a PR hyperlink. All other entries in the surrounding sections link to their PR, e.g. [description](https://github.com/lightningnetwork/lnd/pull/XXXXX). Please update once the PR number is known.
ziggie1984
left a comment
There was a problem hiding this comment.
LGTM, expect for the below nits
regarding the new dependency I think it is ok to pull it in pretty small footprint
Dependency Evaluation: qrterminal/v3 v3.2.1 + rsc.io/qr v0.2.0
Licenses: MIT + BSD-3-Clause. Both permissive, compatible with lnd.
Footprint: Two new packages. golang.org/x/term (also pulled in) is already in lnd's tree.
What the code does: Pure data transformation — string → Unicode characters written to an io.Writer. No network I/O, no file I/O, no goroutines, no syscalls.
Known defect: qr.Encode errors are silently discarded; a nil return would panic on dereference. Not reachable in practice: lnd's MaxPaymentRequestSize (4096 bytes) fits within a Level-L
Version-40 QR code's capacity (4296 alphanumeric chars).
Provenance: qrterminal/v3 is the standard Go terminal QR library. rsc.io/qr is by Russ Cox (Go core team), stable since 2022.
Blast radius: Import exists in one file (cmd/commands/cmd_invoice.go), compiled into one binary (lncli). Unreachable from the lnd daemon — no server, RPC, channel, or payment logic is
affected. Worst-case failure is a garbled terminal.
|
@Roasbeef are you also ok with pulling this new dependency? |
| "strings" | ||
|
|
||
| "github.com/lightningnetwork/lnd/lnrpc" | ||
| qrterminal "github.com/mdp/qrterminal/v3" |
There was a problem hiding this comment.
what's our policy on adding new libraries?
Add a `--qr` flag to the `addinvoice` CLI command that renders the payment request as a QR code directly in the terminal using the qrterminal library. This makes it convenient to scan invoices from a mobile wallet without needing to copy-paste the payment request string.
|
@hieblmi, remember to re-request review from reviewers when ready |

Optionally print the QR code of an invoice, added with
lncli addinvoice --amt 1010 --qr