lnd
lnd copied to clipboard
lightningnetwork
Add an option to subscribe to every Hold Invoice at once
Background
There is no easy way to get Hold Invoice events with one rpc subscription. You have to track them individually, and whenever you start tracking a bunch of them, it doesn't make sense keeping so many rpc subscriptions open, when you could just subscribe once and get all the events there (Just like it happens with regular invoices). This could be a problem for any service that could want to provide Hold Invoices as a solution for their users, and needed to track lots of them at once.
Your environment
- Lnd 0.6.1
- (Any environment or setup)
Steps to reproduce
-
Subscribe to
subscribeInvoices -
You will start getting standard events for invoices (even for hold invoices), such as OPEN and SETTLE, but not those advanced events used on Hold specific ones, such as ACCEPT or CANCEL.
-
You could then try subscribing to
SubscribeSingleInvoiceusing the Sub-Server Invoices, but you should do that for each of the Hold invoices you are tracking.
Problem
Whenever you are tracking a bunch of hold invoices, doesn't make sense to subscribe to each of them individually.
Possible solutions
-
One option could be including an advanced
SubscribeInvoiceson the invoicesrpc Sub-Server, that gets every STATE change for every invoice. I'm not sure if it's possible to filter it to get Hold-Invoices-only here, but if it's not, then could be used as an advanced replacement of the regularSubscribeInvoiceswe already have. -
Other option might be including Hold Invoice events on the regular
subscribeInvoicesrpc call. To avoid the hassle of receiving too many updates, it could be an optional parameter whenever you subscribe, set to false by default.
I'm not sure how this works under the hood, but I guess that maybe you would be getting all the state events for every invoice, not just for hold ones, and that's why you haven't included it as an option yet. If that's the case, and is not possible to filter out invoices by its kind before firing the events, then at least would be great to have a "get all the states for every invoice" as an advanced option (whether option 1 or option 2)
Of course, would be great to have a subscribeHoldInvoices on the sub-server invoices, that gets every STATE event, but for hold invoices only
Can't this be emulated with the existing calls by making individual subscriptions for each open invoice you know of? Depending on your language, you can likely combine all those streams into one (say using something like a python generator that takes other generators and yields them all).
I guess they could probably be combined, and for a small amount of them it would be probably fine (using Node.js). However, if you have a few hundred open hold invoices, I can imagine things would get messy; specially if you lose connection at any point and have to reconnect every stream for every (still) open invoice; I mean, it's doable, but much easier with a single subscription, just like happens with regular Invoices.
I also understand it is not a common case for regular wallets, but will probably be for any LN service using Hold invoices (That's why it could maybe be added to the Sub Server invoices as an advanced version, leaving the regular subscribeInvoices as it is)
On 27 May 2019, at 21:57, Olaoluwa Osuntokun [email protected] wrote:
Can't this be emulated with the existing calls by making individual subscriptions for each open invoice you know of? Depending on your language, you can likely combine all those streams into one (say using something like a python generator that takes other generators and yields them all).
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.
I think that a variation on Subscribe(All)Invoices that sends out updates for every event is the way to go. Not only are the Accepted and Canceled events currently not sent, but the stream also does not include updates to the invoice htlc list that don't necessarily change the invoice state. Especially for multi-part receives, it may be useful to get some insight in how the total set is coming together.
One question is how to signal to SubscribeInvoices what the starting point is from which backlogged events need to be sent. Currently there is add_index and settle_index, but with the increased state space it may be better to introduce something new. Either an always increasing global update counter (goes up for every commit to the invoice db) or alternatively a timestamp. Most data in the invoice db already includes a timestamp, so that could possibly be implemented without a db migration. A timestamp also has more meaning than just a counter value.
Feedback on these kind of design aspects and/or other ideas are welcome.
@alexbosworth @ryanthegentry this features would really be helpful for us
A workaround is to subscribe to all invoices and then individually subscribe to the invoices where you want hold state updates
A workaround is to subscribe to all invoices and then individually subscribe to the invoices where you want hold state updates
this doesn't work for our use case:
we have one pod creating the invoices, and another pod that is listening to their settlement. we really want to have the second pod listening to invoice settlement continuously without having to pass message around between pods.
There should be no need for message passing since an event is triggered for all subscriptions when invoices are created
so,
pod1 "invoice-listeners" start, run subcribeToInvoices
pod2 "invoice-creator" start, run periodically invoiceCreate
pod1 will receive event even for invoice created after subcribeToInvoices has started?
Yes exactly that should just work because subscribing to invoices means getting an event on any new invoice
ok, I thought this was only for already existing invoices. this could work then.
do you know if there could be issues listening to 10k+ invoices?
I don't think there would be any issue from the LND perspective, although you can of course cancel the single invoice subscription after it's settled or expired/canceled so I would use that to minimize overall subscriptions
@carlaKC Are you still working on this? I could write a PR if you want.
@ErikEk I'm not currently working on this, would be very happy to review a PR though!
Been playing around with different solutions here. If we would consider adding partial payment notifications, et al @joostjager, I guess we have to send the full invoice everytime, but with the latest htlc list. Seems bloated but I can't come up with a better solution. I like the idea of adding a timestamp parameter to specify which invoices to return from the subscription.
bumping this up. this is why it's a significant issue on our end:
- we are using hodl invoice because we want to some logic before settling an invoice
- it takes multiple minutes to loop through all invoices to register to back to them after a server restart (our server talking to lnd, not necessarily restarting lnd)
- we heavily rely on CD, and put to prod new code 5+ times per day
- every time our servers restarts, we have to loop through all those invoices again
- during this "restart time", invoice will not be settled as the htlc arrives, which translate to a bad user experience for our users
- in a way, the more we push code, the more negative the user experience for our users / user of the Galoy stack is
having a single subscription for all invoices would solve this issue
Hi, we're aware of the need here, thanks for that additional anecdote. We have a large project to overhaul the invoice system at the data storage level which sort of blocks this, as we'd want to ensure we can give back filled notifications as normal. Any review+testing of those PRs (#6288) is appreciated.
After fixing some bugs in this area earlier this year, I spun out this issue that outlines a number of issues with the internals of the current notification system and potential ways to address them: https://github.com/lightningnetwork/lnd/issues/8023
@AndySchroder no change yet. With the native SQL backend for invoices complete however, we're now in position to easily implement this feature, but only for the native SQL backend.
Is the SQL back end going to require more setup from the user, or does it "just work"?
Is the SQL back end going to require more setup from the user, or does it "just work"?
For sqlite, it'll "just work", as it's an embedded database. For postgres, it'll require a bit more work, as the user also needs to run a database management server. There are hosted solutions that make things easier, like Amazon RDS as an example: https://aws.amazon.com/rds/postgresql/ .
Hi folks, taking this over as part of the hodlinvoice overhaul,
I wonder if we need to keep this backwards compatibility here, otherwise there will be no way to introduce a new RPC cmd:
https://github.com/lightningnetwork/lnd/blob/8ac184a91105d3e6c877b830e37de46c211127f8/invoices/invoiceregistry.go#L318-L321
Regarding the partial payment notification, I would first introduce this only for single invoice subscriptions, otherwise the return stream can become pretty bloated as mentioned by @ErikEk
I think this is a critical feature. We really need to be encouraging the use of HOLD invoices a lot more in the industry.
Generally, I would think all merchants selling physical goods doing proper inventory management should be using HOLD invoices for every invoice. It's sloppy to accept payment for a product that you can't 100% guarantee you can deliver to a customer. At the same time, a merchant can't reserve product for a customer just because they put it in a web shopping cart and generate an invoice, as that can easily be Sybil attacked and tie up a bunch of inventory without anyone ever buying anything. Using HOLD invoices allows merchants to cancel an order and immediately and privately return payment to the customer if they no longer have enough product once the customer actually pays.
One question is how to signal to
SubscribeInvoiceswhat the starting point is from which backlogged events need to be sent. Currently there isadd_indexandsettle_index, but with the increased state space it may be better to introduce something new. Either an always increasing global update counter (goes up for every commit to the invoice db) or alternatively a timestamp. Most data in the invoice db already includes a timestamp, so that could possibly be implemented without a db migration. A timestamp also has more meaning than just a counter value.
I do find add_index and settle_index a bit confusing. Adding something like accept_index will get even more confusing.
I'm wondering if things should be separated out into separate RPCs
SubscribeAddedInvoicesadd_index
SubscribeHTLCsInvoiceshtlc_update_index
SubscribeAcceptedInvoicesaccept_index
SubscribeSettledInvoicessettle_index
SubscribeCanceledInvoicescancel_index
However, if we decide to keep these all in one RPC and instruct the RPC what we are interested in hearing back about, I think a time based or global index should be used instead.
Of course, would be great to have a
subscribeHoldInvoiceson the sub-server invoices, that gets every STATE event, but for hold invoices only
I think it would be best to just make this an option for SubscribeInvoices (or a more specific RPC for each update type like I've suggested above). However, we should have the following options:
- Subscribe to all invoices.
- Subscribe to all HOLD invoices.
- Subscribe to all non-HOLD invoices.