-
Notifications
You must be signed in to change notification settings - Fork 712
Adding support for main bolt12 commands to NIP-47 #1952
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?
Conversation
bolt11/bolt12) for the existing pay commands
TheBlueMatt
left a comment
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.
Cool! Sadly I think there's some confusion around the "API" for BOLT 12.
47.md
Outdated
| "method": "pay_invoice", | ||
| "params": { | ||
| "invoice": "lnbc50n1...", // bolt11 invoice | ||
| "invoice": "lnbc50n1...", // bolt11/bolt12 invoice |
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.
I don't see any reason why BOLT 12 invoices should be exposed pre-payment. Rather, the NWC client should submit an offer (the thing it has) for payment and let the lightning wallet deal with the Lightning stuff (fetching the invoice and paying).
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.
My base application was to extend NWC to support bolt12 so that Zeus, when used as a remote, can use NWC for bolt12 the same way it uses its other connection mechanisms for bolt12, that is by being able to scan an offer, fetch an invoice, pay it, but also it can create offers for single/multiple use and enable/disable them. @kaloudis told me NWC should be the best protocol for this application. But I agree that there could be a "pay_offer" command where the user could send an amount equal or larger than the offer amount and add a payer's note all at once, and have the fetch invoice part handled by the wallet.
47.md
Outdated
| "amount": 123, // minimum value in msats, optional | ||
| "description": "string", // offer's description, optional | ||
| "absolute_expiry": unixtimestamp, // absolute expiry time of the offer, optional | ||
| "single_use": false // only allow the offer to be used once, optional, default false |
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.
I'm not sure if any wallets actually support this as a flag? In general in lightning if someone wants to donate sats to you it's not really possible to stop them, soooo
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.
Yes CLN and Zeus certainly do support it as a flag. If single_use is set to true and the offer has been used, my understanding is that fetch_invoice will then fail for this offer.
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.
I dont think this is a universal thing, however (certainly LDK doesn't have any way to support this - our offers are completely stateless, so we'd have to add an entire DB just to store offers we want to disable), and I'm still uncertain on what the purpose is - if someone wants to pay you, let them pay you?
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.
I would think it is more for an accounting/management purpose than for a technical purpose. If you issue an offer with a description that identifies a particular invoice (with a set amount) for example, then you might want to enforce a single possible payment for it? I am not trying to request changes in the implementation of Bolt12 at the node level, but just to have it available at the NWC level so it can be used if available. Note that the response includes the flag as well, so single_use could be returned as false regardless if not supported.
47.md
Outdated
| Response: | ||
| ```jsonc | ||
| { | ||
| "result_type": "lookup_offer", |
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.
I don't see why an NWC client should concern itself with BOLT 12 invoices. They're an implementation detail of how wallets pay a BOLT 12 offer.
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.
C.f. my other comments about the use case for Zeus. Thanks!
| Errors: | ||
| - `NOT_FOUND`: The offer could not be found by the given parameters. | ||
|
|
||
| ### `disable_offer` |
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.
I'm not sure I understand the point of "disabling" an offer. I don't think many wallets support this concept.
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.
Yes CLN does allow to enable and disable offers, and Zeus supports it as well
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.
Right, as mentioned above there's no way to implement this in LDK, really. I mean in theory an LDK-based wallet can block responding to a specific offer, but honestly, why? If someone wants to pay you, let them pay you.
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.
As I mentioned I am not attempting to request changes for node implementations. My understanding is that NWC allows a subset of methods to be available, as applicable. The enable_offer/disable_offer methods do not have to be available for all node implementations, but could be available for the ones that support it. One reason I can see where disabling an offer could be useful, is if there was an issue when generating an offer (lack of blinded paths, for example), one might want to disable it so it does not get shared or used.
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 enable_offer/disable_offer methods do not have to be available for all node implementations, but could be available for the ones that support it.
In general, super flexible APIs where large portions of the API is optional are ~impossible to use. Worse if the spec doesn't indicate which parts can be relied on and which cannot. At best clients will just use what is always available (ie will never use enable/disable), at worst, things will randomly break and create a horrible UX. Unless you have a really strong reason and a really important use-case, optional features should be avoided wherever possible.
One reason I can see where disabling an offer could be useful, is if there was an issue when generating an offer (lack of blinded paths, for example), one might want to disable it so it does not get shared or used.
I don't really see why such an offer needs to be "disabled". You can just not use it if it doesn't look the way you want? Further, the NWC client shouldn't really be analyzing an offer, the wallet should be deciding how to build the offer and the NWC client just uses it.
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.
In general, super flexible APIs where large portions of the API is optional are ~impossible to use. Worse if the spec doesn't indicate which parts can be relied on and which cannot. At best clients will just use what is always available (ie will never use enable/disable), at worst, things will randomly break and create a horrible UX. Unless you have a really strong reason and a really important use-case, optional features should be avoided wherever possible.
I don't really see why such an offer needs to be "disabled". You can just not use it if it doesn't look the way you want? Further, the NWC client shouldn't really be analyzing an offer, the wallet should be deciding how to build the offer and the NWC client just uses it.
Here are some categories of use cases where I can see one might want to be able to disable offers:
-Someone provides a determined set of services/goods that are sold separately, and wants to use a separate offer for each one (e.g. a farmer with a weekly subscription for food box delivery, a shuttle service between a set of locations, a barber providing hair cuts, etc). In this scenario, the offers will have set amounts. If the person wants to update his prices, he will want to replace the existing offers by new ones. He will also want to disable the old offers so they do not get paid by frequent/existing customers by mistake, and have to deal with a refund. Also he will not want to keep seeing all the old obsolete offers in his client, and have them pilling up over time.
-For accounting purpose, a good/service provider wants to use a unique offer for each customer (i.e. the description field is unique per customer). Also this person might be mostly dealing with frequent customers / subscribers. In this case, the offers do not necessarily have set amounts, but the provider might want to easily keep track of payment history on a per customer basis. If customers end their subscription, the provider might want to disable their assigned offers so the former customers do not pay them by mistake. By disabling these offers, the provider can also completely ignore them and not even display them when scanning through the list of valid/current offers.
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.
Both of these feel a little contrived to me, because that isn't how any bitcoin PoS works today. If someone goes to check out, the PoS creates a new QR code specific to that purchase, so that it can track when that specific purchase completes. If you have a single offer for an item, and you have two people buying that item, how do you know who paid and who didn't? I admit I'd never thought of "offer per regular customer" but that too seems pretty contrived as it means the sender picks the amount and the merchant who is charging doesn't get to decide it?
I admit its possible people will want to print off an offer QR and have that be the static "pay me" QR they show people who want to pay in Bitcoin (this does happen today in some establishments where bitcoin payments are rare), but in that case you wouldn't have multiple QR codes for the different items/services you buy with fixed amounts, you just have a "pay me" QR code and check that the payment had the right amount on receipt.
Complexifying the API with optional features (implying clients wont even use them anyway) for incredibly speculative use-cases (that IMO are worse UX than the current two UXes that merchants use to receive Bitcoin described above) seems like a bad idea to me.
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.
Both of these feel a little contrived to me, because that isn't how any bitcoin PoS works today. If someone goes to check out, the PoS creates a new QR code specific to that purchase, so that it can track when that specific purchase completes.
This is the use case for a single_use offer (use case 0)
If you have a single offer for an item, and you have two people buying that item, how do you know who paid and who didn't?
This use case (case 1) is more for a synchronous payment in person. Someone gets on a shuttle to go to work, the driver shows him the offer for the ride with a specific amount, the customer pays it, and the driver sees that the sats are received. In this case the payee would not necessarily need to know who pays what, but the customer could always use a payer_note.
I admit I'd never thought of "offer per regular customer" but that too seems pretty contrived as it means the sender picks the amount and the merchant who is charging doesn't get to decide it?
In this other use case (case 2), the payee might be more interested to know who paid which offer, and the payments might not be made in person or synchronously. For example, the customers might simply send sats as credits to their account, and the payee might draw from these credits when the payment for the service is due. If the customer runs out of credits, then the service would be terminated. I use services that work this way.
I admit its possible people will want to print off an offer QR and have that be the static "pay me" QR they show people who want to pay in Bitcoin (this does happen today in some establishments where bitcoin payments are rare), but in that case you wouldn't have multiple QR codes for the different items/services you buy with fixed amounts, you just have a "pay me" QR code and check that the payment had the right amount on receipt.
Going back to case 1, as I mentioned this would not be for services where multiple items are typically purchased together, but for services where people almost exclusively buy a single item, and where there is either a single price for all offered items, or just a few different prices.
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.
I agree with BlueMatt, I don't think we should add enabling/disabling offers.
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.
I agree with BlueMatt, I don't think we should add enabling/disabling offers.
Why not? I see clear use cases for it?
47.md
Outdated
| "params": { | ||
| "invoice": "lnbc50n1...", // bolt11 invoice | ||
| "invoice": "lnbc50n1...", // bolt11/bolt12 invoice | ||
| "amount": 123, // invoice amount in msats, optional |
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.
Should this be updated to indicate that it's required if the BOLT 11 invoice or the BOLT 12 offer do not contain an explicit amount?
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.
Yes I agree
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.
This was fixed
| Response: | ||
| ```jsonc | ||
| { | ||
| "result_type": "list_offers", |
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.
Lightning wallets shouldn't be storing every offer or invoice they've ever generated, especially when they have an API to do so. That makes things vulnerable to DoS issues.
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.
Ok. I have added the make_offer and list_offers mechanism as this is how Zeus uses bolt12 to receive payments. It allows to create and list "pay codes", for single/multiple use and to enable/disable them.
|
@TheBlueMatt is correct when it comes to bolt12 invoices. They should be kept out completely and only deal with offers. That means paying an offer not bolt12 invoice and dropping the fetch_invoice method completely. I was working on a PR like this aswell: daywalker90#1 |
Thanks! As I mentioned I am also fine with dropping the fetch_invoice method. It could be replaced by a modified pay_invoice command that supports offer strings and also payer_note, and the addition of a get_offer_info command that provides the main decoded offer information. I will modify the PR to reflect this. |
-Modified the pay_invoice command to supports bolt12 offers as an input -Adding get_offer_info command so decoded info get be retrieved (as only nodes currently support decoding)
e4b5be7 to
00acfb4
Compare
overall consistency.
-Adding an optional payer_note field for a bolt12 offer
Ok so I did remove the fetch_invoice command and made the changes required to make it work with bolt12. I have also added support for the offer issuer field, as well as other currencies when making and decoding offers. The offer_issuer, offer_id and payer_note fields are now part of the results when listing transactions, when applicable. |
TheBlueMatt
left a comment
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.
I really hate that the API allows you to list invoices/offers which have been issued (as invoices/offers should be static and should never have any state on the node at all!) but there's no reason to leave the BOLT 12 API hobbled compared to the BOLT 11 API.
47.md
Outdated
| "amount": 123, // minimum value in msats, optional | ||
| "description": "string", // offer's description, optional | ||
| "absolute_expiry": unixtimestamp, // absolute expiry time of the offer, optional | ||
| "single_use": false // only allow the offer to be used once, optional, default false |
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.
I dont think this is a universal thing, however (certainly LDK doesn't have any way to support this - our offers are completely stateless, so we'd have to add an entire DB just to store offers we want to disable), and I'm still uncertain on what the purpose is - if someone wants to pay you, let them pay you?
| Errors: | ||
| - `NOT_FOUND`: The offer could not be found by the given parameters. | ||
|
|
||
| ### `disable_offer` |
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.
Right, as mentioned above there's no way to implement this in LDK, really. I mean in theory an LDK-based wallet can block responding to a specific offer, but honestly, why? If someone wants to pay you, let them pay you.
TheBlueMatt
left a comment
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.
47.md
Outdated
| { | ||
| "method": "lookup_offer", | ||
| "params": { | ||
| "offer_id": "string", // the id of the offer |
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.
I'm not sure every implementation will have an "id" to use as a lookup for their offers.
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.
What would be used by these implementations to index/filter the offers in these cases? The offer string itself?
Effectively this "offer_id" parameter is provided by the responses of the make_offer and/or the list_offers methods, so it can be anything that uniquely identifies the offer and which is used internally by the node for indexing, including the offer string itself. It can be renamed something different from offer_id if desired. Also if an offer_id hash is not used by all implementations, I can remove the field from the responses of the get_offer_info and lookup_offer methods.
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.
Honestly i find it very confusing that both lookup_offer and get_offer_info exist. I would just remove lookup_offer.
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.
Honestly i find it very confusing that both
lookup_offerandget_offer_infoexist. I would just removelookup_offer.
They are not accomplishing the same goal. get_offer_info decodes any offer, from any node, and in particular to get information about an offer before paying it.
lookup_offer provides the status of an offer generated by the node / wallet provider, in particular to know if it was used or not, if it is active or not, and the last time it was updated.
Maybe lookup_offer could be renamed get_offer_status and could only return offer_id, active, used and updated_at? This would remove the confusion?
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.
lookup_offerprovides the status of an offer generated by the node / wallet provider, in particular to know if it was used or not, if it is active or not, and the last time it was updated.
I don't know if all implementations can even provide such info? For example how do you lookup updated_at for an offer in CLN? @TheBlueMatt said there is no "disabling" in LDK so there is no active in LDK either i would assume. That means 2 out of 3 have to be optional already which makes this not very useful i think.
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.
lookup_offerprovides the status of an offer generated by the node / wallet provider, in particular to know if it was used or not, if it is active or not, and the last time it was updated.I don't know if all implementations can even provide such info? For example how do you lookup
updated_atfor an offer in CLN? @TheBlueMatt said there is no "disabling" in LDK so there is noactivein LDK either i would assume. That means 2 out of 3 have to be optional already which makes this not very useful i think.
Ok. I think the functionality of lookup_offer (getting the status for a single offer) can be integrated by list_offers. I can also remove the updated_at field.
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.
Hmm, I just noticed that CLN does not even have a way to get the creation time of an offer? This is not ideal...
47.md
Outdated
| { | ||
| "method": "get_offer_info", | ||
| "params": { | ||
| "offer_id": "string", // the id of the offer |
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.
I thought this method was intended to be used to "decode" an offer? As such i would expect it to use an offer as input.
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.
Yes thank you, this was a mistake. I fixed it.
47.md
Outdated
| "params": { | ||
| "invoice": "lnbc50n1...", // bolt11 invoice | ||
| "amount": 123, // invoice amount in msats, optional | ||
| "invoice": "lnbc50n1...", // bolt11 invoice/bolt12 offer |
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.
I think we should have a separate method pay_offer and not mix both into one method
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.
Ok. I am ambivalent myself, I can see how one could prefer having a single, but more general, method, or two, more targeted, ones. Is there a consensus on splitting the method?
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.
We currently announce what methods we support in the info event. By splitting them it's clear if there is support for paying offers or not
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.
Ok. What should we do about multi_pay_invoice in this case though? Currently it supports a blend of bolt11 invoices and bolt12 offer strings. Should I leave multi_pay_invoice as is, but split pay_invoice into pay_invoice/pay_offer?
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.
Currently it supports a blend of bolt11 invoices and bolt12 offer strings.
This feels bad to me. Offers and invoices are not the same thing and we mix the parameters and make things confusing.
I would either
- Not support multiple payments at all for offers until there is real demand
- Add
multi_pay_offermethod
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.
Currently it supports a blend of bolt11 invoices and bolt12 offer strings.
This feels bad to me. Offers and invoices are not the same thing and we mix the parameters and make things confusing.
I would either
- Not support multiple payments at all for offers until there is real demand
- Add
multi_pay_offermethod
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.
Currently it supports a blend of bolt11 invoices and bolt12 offer strings.
This feels bad to me. Offers and invoices are not the same thing and we mix the parameters and make things confusing.
I would either
* Not support multiple payments at all for offers until there is real demand * Add `multi_pay_offer` method
Ok, I will split the pay and the multi_pay methods
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.
We currently announce what methods we support in the info event. By splitting them it's clear if there is support for paying offers or not
Done
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.
Currently it supports a blend of bolt11 invoices and bolt12 offer strings.
This feels bad to me. Offers and invoices are not the same thing and we mix the parameters and make things confusing.
I would either
* Not support multiple payments at all for offers until there is real demand * Add `multi_pay_offer` method
Done
lookup_offer and get_offer_info: Adding descriptions.
list_offers -Dropping the updated_at offer field -Dropping the offer_id field from the response of the get_offer_info method.
| } | ||
| ``` | ||
|
|
||
| ### `get_offer_info` |
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.
What's the benefit of this method? shouldn't there be offer decoding libraries apps can use?
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.
There is currently none as far as I know. The decoding functionality is currently only supported by nodes
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.
This seems wrong to me. We should have simple bolt12 offer decoding libraries that apps can use, rather than add a NWC method just to decode an offer
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.
Isn't this such a library? https://github.com/Horus-Org/light-bolt12-decoder
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.
Isn't this such a library? https://github.com/Horus-Org/light-bolt12-decoder
It does not seem to support offers based on the description and the example, but I will test it. Rusty had a python decoder as well, but it does not work and they recommend using the node. Wallet apps such as Zeus use the node connection for decoding as well. I think LDK Node have some support, but it is not exactly a simple bolt12 offer decoding library...
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.
FYI I tried decoding both an offer and a bolt12 invoice using https://github.com/Horus-Org/light-bolt12-decoder and it returns "Invalid invoice format" in the console...
|
@21M4TW is there any NWC Wallet Service implementation that supports any of this? |
I've done a prototype in preparation for a spec PR with my own nip spec here: daywalker90/cln-nip47#2 PR for the library i use in cln-nip47: daywalker90#1 |
pay_invoice/pay_offer and multi_pay_invoice/multi_pay_offer methods.
6749e41 to
7b743aa
Compare
Sorry, I just noticed this question. I had this PR for nwcprovider (riccardobl/nwcprovider#12) . I also have implemented the bolt12 functionalities in lnbits (lnbits/lnbits#3092). I also wanted to help adding Bolt12 NWC support to Zeus (which uses getAlby for NWC). Zeus already supports bolt12, including enabling and disabling offers, through CLNRest. All these (except lnbits') are pretty much pending on the inclusion of Bolt12 into NIP47. |
|
Is there anything I can do to get this moving? Thanks! |
|
Looks good to me, sounds like there are 2+ implementations? If so I think we can merge. |
|
@reneaaron @rolznz any opinions on this? |
|
Imo i agree that we should remove enable/disable offer |
| "currency_minor_unit": 123, // number of decimals to apply to the amount, for currency different than msat, optional | ||
| "amount": 123, // minimum amount value, after adjustment by currency_minor_unit if applicable, optional | ||
| "offer_id": "string", // the id of the offer | ||
| "active": true, // whether the offer is enabled |
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.
This seems unneeded: I just created an offer, it better be always "active".
| "offer_id": "string", // the id of the offer | ||
| "active": true, // whether the offer is enabled | ||
| "single_use": false, // whether the offer can only be used once | ||
| "used": false, // whether the offer has been used |
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.
Same here. I just created an offer: it should always be unused.
| Errors: | ||
| - `PAYMENT_FAILED`: The payment failed. This may be due to a timeout, exhausting all routes, insufficient capacity or similar. | ||
|
|
||
| ### `multi_pay_offer` |
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.
Are the multi_* methods needed? (there has not been good adoption of the non-bolt12 ones so far).
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.
I don't need them personally, I had just put them there for consistency and completeness. If you think it would be easier to move forward with this with them removed, I don't have any objection.
|
Also I was considering extracting the decoding code from CLN and package it into a library so it can be used anywhere. It seemed to be a sticking point and I agree it would be better to do it locally instead of relying on a get_offer_info command. |
This is a request to add support for Bolt12 to NWC, such that a Lightning client can use NWC to generate and use Bolt12 offers, fetch and pay Bolt12 invoices, thus benefiting from the improvements over Bolt11.