offers: parse invoice and invoice request

[JssDWt]

Add the ability to parse and display BOLT12 invoices and invoice requests. This makes it easy for users of LDK to serialize and deserialize BOLT12 invoices and invoice requests.

[ldk-reviews-bot]

👋 Thanks for assigning @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

[tnull]

Code looks good, but I think we previously discussed this and had decided against introducing a serialization format for invoices and requests. Not sure if things changed since then though? (cc @jkczyz @TheBlueMatt)

[vincenzopalazzo]

Overall LGTM, leaving a comment

I has a similar code in a bolt12 tool that I am building for ocean that allow to make verification on the offer + invoice. So would be good to have this feature somehow to allow more usable command line tools.

However, it is also possible to leave this to the caller, exposing some minimal information like AsRef and BECH32_HRP if at the moment, rust-lightning does not want to export this kind of serializzation.

[ldk-reviews-bot]

🔔 1st Reminder

Hey @jkczyz! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[JssDWt]

Pushed a change to assert_eq rather than if ... panic in the tests

[jkczyz]

Code looks good, but I think we previously discussed this and had decided against introducing a serialization format for invoices and requests. Not sure if things changed since then though? (cc @jkczyz @TheBlueMatt)

Right, shouldn't the LengthReadable and Writeable implementations be sufficient?

[jkczyz]

Hmm... we shouldn't introduce a bech32 encoding that isn't in the spec. BOLT12 invoices are only transferred over the wire, so one isn't defined. They also only make sense in the context of a preceding offer / invoice_request (i.e., you'll never see them in a context where a user would scan it).

[tnull]

FWIW, could that be a spec bug? Seems CLN is supporting the lni format, so maybe it should be added to the spec, or was that omitted on purpose?

[jkczyz]

It was deliberately removed from the spec. IIRC, @TheBlueMatt pushed for having it removed. CLN uses it in some tests and its CLI, I believe.

[jkczyz]

Similarly here. Additionally, this clashes with Refund, which is essentially an invoice_request without a preceding offer.

[tnull]

This one is in the spec though, or at least it makes it seem lnr is used for both?:

Invoice Requests are a request for an invoice; the human-readable prefix for invoice requests is lnr.

https://github.com/lightning/bolts/blob/master/12-offer-encoding.md#invoice-requests

[jkczyz]

I raised the issue at one point: lightning/bolts#798 (comment)

Ultimately, if you overload it, then your parser can't just look at the human-readable part to determine which code to call. It needs to try parsing with one and, if it fails, fall back to the other.

[tnull]

@JssDWt Could you expand whether you really need this/for what in particular, given that parts of this seems to have been deliberately dropped from the spec? Is there a way around it?

[JssDWt]

@JssDWt Could you expand whether you really need this/for what in particular, given that parts of this seems to have been deliberately dropped from the spec? Is there a way around it?

There's a few places where this is useful:

• For swaps on mobile devices. User creates offer. Sender pays offer, destination is swap node. Swap node sends a webhook/push notification with the invoice request to the user's device. User responds to the hook with an invoice. Swap node returns invoice as onion message to sender. Although practically speaking we can define our own serialization format here, or use the wire format, it just makes sense to use a human readable serialization format that is native to bolt12.
• I'd like to support invoice requests as a receiving method. In order to do that they have to be parseable by an input parser.
• I'd like to show the user payment details for a payment after the fact. That includes the paid invoice. It can be used as a string identifier for the payment as well.

I understand the invoice serialization format is not part of the spec. But also I don't see a strong case against 'allowing' serialization of invoices.

Invoice requests I think should just be serializable as they're part of the spec.

[tnull]

it just makes sense to use a human readable serialization format that is native to bolt12.

Well, it's my understanding that this is exactly not the case, i.e. that BOLT12 deliberately doesn't provide such serialization formats (anymore). If you require this, it might make sense to raise this issue again at a spec level first.

I'd like to support invoice requests as a receiving method. In order to do that they have to be parseable by an input parser.

Wait, but this is exactly the Refund usecase, for which we already have the lnr encoding in place, no?

I'd like to show the user payment details for a payment after the fact. That includes the paid invoice. It can be used as a string identifier for the payment as well.

Hmm, the question is what that would do for the users if there is no blessed serialization format for these. What can they even do with this string?

Again, not saying we should or shouldn't do this, but if you have that requirement, it needs to be clarified on the spec level first, as adding proprietary serialization formats on the LDK side that nobody else understands will just lead to fragmentation.

[JssDWt]

Wait, but this is exactly the Refund usecase, for which we already have the lnr encoding in place, no?

I didn't realize there was a Refund for this usecase. What's the reason a Refund differs from an InvoiceRequest conceptually in the code?

Well, it's my understanding that this is exactly not the case, i.e. that BOLT12 deliberately doesn't provide such serialization formats (anymore). If you require this, it might make sense to raise this issue again at a spec level first.

I understand. I probably won't take this up on the spec level myself as I find these discussions exhausting.
The bolt11 invoice is serialized in the wild in json. For example by Boltz. Currently our SDK uses the following code to parse it:

let (hrp, data) = bech32::decode_without_checksum(invoice)?;
ensure!(hrp.as_str() == "lni", "Invalid HRP");

let data = Vec::<u8>::from_base32(&data)?;

Bolt12Invoice::try_from(data)
      .map_err(|e| anyhow!("Failed to parse BOLT12: {e:?}"))

An LDK native parsing method would be nice. If it was actively removed from the spec by @TheBlueMatt I won't go into the discussion, but I don't get the strong opposition there tbh.

[tnull]

Want to note that we are hitting the same requirement in lightningdevkit/ldk-node#542, simply because we want to provide a bindings-compatible string representation of Bolt12Invoice. Our workaround would also be to make up something on the spot, but that would be even more proprietary to LDK Node. So maybe this should indeed be revisited on the spec-level, @TheBlueMatt @jkczyz ?

[jkczyz]

I didn't realize there was a Refund for this usecase. What's the reason a Refund differs from an InvoiceRequest conceptually in the code?

They are fundamentally different types. While they share the same TLV representation, they have different semantics (i.e., which settings are considered valid) and thus cannot be used interchangeably.

An InvoiceRequest should never need to be seen by the end user -- the user's application sends it on their behalf. While a Refund is presented to them just like an Offer.

Want to note that we are hitting the same requirement in lightningdevkit/ldk-node#542, simply because we want to provide a bindings-compatible string representation of Bolt12Invoice. Our workaround would also be to make up something on the spot, but that would be even more proprietary to LDK Node. So maybe this should indeed be revisited on the spec-level, @TheBlueMatt @jkczyz ?

It's worth considering especially in conjunction with a proof-of-payment spec, which I believe Rusty is developing.

Though also note that in addition to Bolt12Invoice we will also have StaticInvoice for async payments, which again has different semantics.