Add support for custodial off-chain accounts
Early preview of the LiT custodial off-chain accounts feature.
~Depends on #308.~ Depends on #432.
EDIT: All TODOs have been resolved :tada:
This still has a bunch of TODOs and things that can be improved, that's why it's still in draft. But would be nice if you could take a first look at the general architecture, @ellemouton and @Roasbeef.
yep! currently doing so :)
Taking this out of WIP state since this is now a bit better tested and also received its own set of integration tests!
The following TODOs are still in place:
- add unit tests
- take in-flight payments into account when calculating remaining account balance
- add support for creating a custom "account" LNC session by adding custom caveats to the AddSession RPC.
@guggero, remember to re-request review from reviewers when ready
!lightninglabs-deploy mute
Yayy, I was finally able to resolve all TODOs :rocket: This PR is now fully ready for final reviews.
Addressed all comments, should be ready for a final look.
Thanks a lot for the review and for testing the happy path :rocket: Happy to hear you find this useful.
Does the rpc interceptor get re-registered and the TrackPayment subscriptions re-established along with it?
Yes, on every startup we re-initialize the payment tracking for each payment and invoice update stream.
So this should be pretty robust. Though in general "lnd instance that comes online & offline unpredictably" is never recommended in a production environment. Or are you thinking about a mobile setup? Curious about your use case!
Going to address your nits/comments inline.
Yes, on every startup we re-initialize the payment tracking for each payment and invoice update stream.
So this should be pretty robust. Though in general "lnd instance that comes online & offline unpredictably" is never recommended in a production environment. Or are you thinking about a mobile setup? Curious about your use case!
Gotcha, I think one thing I was overlooking was that (if I'm not mistaken) litd will stop when it sees that lnd has stopped. So this Start function will be called every time either litd or lnd restart. My concern was only that lnd goes down, all subscriptions are lost, and then when it comes back up we don't re-establish the streams.
I wasn't thinking about a mobile setup or anything like that, just about what might happen if there are unexpected outages in an otherwise stable environment.
litdwill stop when it sees thatlndhas stopped
Ah yes, that is important to know in this context. litd is indeed set up to stop (and be re-started by systemd, docker, k8s, whatever) when the connection to lnd is lost.
My last handful of thoughts about this PR & feature:
-
It might be nice to have an RPC call that increments an account's balance. That way there's an easy way to, for example, add 1000 satoshis to an account. Or possibly decrement balance (by passing a negative value), but then there'd need to be the checks for sufficient balance on the account. Right now, you can update an account's balance via RPC, but that's only good if you're trying to set the balance to a specific amount, not add or subtract from the existing balance. Trying to do that by querying the balance and then updating the account balance would be subject to race conditions (maybe the balance updates in between the two calls). I figure this can be done in a follow-up PR, though.
-
It'd be nice if self-payments from one account to another were better supported. Right now, it seems like it does work if I use the
--allow_self_paymentflag, which then sends a payment out one channel and back in on the same channel. I think it'd be cool to be able to make an account-to-account payment without actually needing to make a real lightning payment (which may incur a fee, delay, and/or chance of failure). But I can imagine that would be a complicated change since it would likely break some internal assumptions in lnd about payments, namely that they actually involve the external lightning network. Just something to think about for future work.
My last handful of thoughts about this PR & feature:
Thanks a lot for your feedback! I think both of these features make sense for follow-up PRs (given the added complexity for item 2 and the age of this PR, this has been in the works since late 2018 in some form or another). Feel free to add feature request issues to the repo so they don't drop off the radar.