Skip to content
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

Add support for AMP and zero amount invoices #116

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thomasbarrett
Copy link

Pull Request Checklist

  • PR is opened against correct version branch.
  • Version compatibility matrix in the README and minimal required version
    in lnd_services.go are updated.
  • Update macaroon_recipes.go if your PR adds a new method that is called
    differently than the RPC method it invokes.

@thomasbarrett thomasbarrett changed the base branch from lnd-16-0 to master August 3, 2022 04:38
@thomasbarrett
Copy link
Author

Hey! I am trying to use LndClient to do real-time payment streaming with a static AMP invoice. I ran into two issues using lndclient v0.16.0.

  1. Support for AMP payments does not seem to be built out yet in the AddInvoice method. Notably, the IsAmp field is not properly set in the lnrpc.Invoice object.

  2. Zero value invoices are not properly supported in the SendPayment method. Currently, a Value is only forwarded if the Invoice is not set. Ideally, we would forward the Value independently of whether the Invoice is set to enable zero-value invoices.

@guggero
Copy link
Member

guggero commented Aug 3, 2022

Hey. I agree with the IsAmp flag, that needs to be added to the AddInvoice call. Though I'm not sure about the Amt field. It might be confusing to always add the value, especially if it gets overwritten if an invoice is also passed in.
How about we add a new field to the SendPaymentRequest called AmpAmount that is only set if the invoice has a zero amount. Would that work for your use case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants