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

[Feature] Add custodial mode #204

Open
junderw opened this issue Jun 14, 2018 · 29 comments
Open

[Feature] Add custodial mode #204

junderw opened this issue Jun 14, 2018 · 29 comments
Labels
Feature Request Any request for a new feature that does not exist (for existing features use Enhancement label)

Comments

@junderw
Copy link
Contributor

junderw commented Jun 14, 2018

Currently, the instance admin is the only one able to use Lightning, as the private keys are necessary to accept payment, and it is impossible to do the xpub style of "the server just generates addresses" for non-custodial usage.

Proposal:
Add a custodial mode where separate stores and users can be created from the instance admin's lightning and the instance admin's xpubs.

This will require a few extra changes to such accounts.

A custodial account must also store:

  1. A history of amounts paid and unpaid that the store admin can view but not change or delete.
  2. Each payment entry should have a state of "reimbursed/not reimbursed" (it will be changed to reimbursed once the custodian has paid out in fiat or crypto (whatever is agreed upon).
  3. Each store of a custodial account should have an option to select which currencies to be reimbursed in, and by what percentage. (including fiat and crypto)
  4. The admin account should have a setting for which currencies that they can reimburse with... only currencies explicitly selected in this setting can be chosen by the custodial account.

Also needs solving:

  1. What APIs should an exchange / processor provide to btcpayserver?
  2. What APIs should the btcpayserver provide to the exchange / processor?
@NicolasDorier
Copy link
Contributor

NicolasDorier commented Jun 14, 2018

instead of custodial model, what about BTCPay providing an API so that the admin can automate tasks like user and store creation? You plan to let users access btcpay?

@junderw
Copy link
Contributor Author

junderw commented Jun 14, 2018

what about BTCPay providing an API so that the admin can automate tasks like user and store creation?

That could work.

However, if the goal of this project is to replace Bitpay, it would make sense that it would have features such as the above to allow someone to actually do almost everything Bitpay does (including custodial accounts)

@pavlenex
Copy link
Contributor

pavlenex commented Dec 10, 2018

Slightly Related to #396

@pavlenex pavlenex added the Feature Request Any request for a new feature that does not exist (for existing features use Enhancement label) label Dec 10, 2018
@woutersamaey
Copy link
Contributor

woutersamaey commented Aug 7, 2019

Isn’t this the same as #677 or are there some differences?

@junderw
Copy link
Contributor Author

junderw commented Aug 7, 2019

yes, mine includes on-chain as well, not just lightning.

@pavlenex
Copy link
Contributor

pavlenex commented Aug 17, 2019

Thinking of custodial issue perhaps we can code a simple paywall on registrations so that third-party hosts can charge a monthly/annual fee to users that use their instance. It would be optional of course but might encourage more people to become a host for others and spread the adoption.

You can get X days free when registring to try it out, then after Y days you pay Z amount for J days. When J days expire, you get e-mail notification, similar to LibrePatron.

This would also encourage users to ditch third-party hosts and self-host maybe :)

Kukks added a commit to Kukks/btcpayserver that referenced this issue Aug 20, 2019
Partially related to btcpayserver#204 (but no actual management and accounting of funds in different stores occurs)
NicolasDorier pushed a commit that referenced this issue Aug 20, 2019
Partially related to #204 (but no actual management and accounting of funds in different stores occurs)
@pavlenex pavlenex added this to Backlog in BTCPay Server Project Nov 10, 2019
@Kukks
Copy link
Member

Kukks commented Apr 29, 2020

Since there are now on-chain hot wallets (generation of a seed, plus option to store seed on server OR store seed independently and provide seed to sign on demand), is this issue partially solved?

For Lightning, I am watching the LNBits project, which seems to be a more hardcore version of LNDHub, that also supports multiple LN implementations.

@junderw
Copy link
Contributor Author

junderw commented Apr 29, 2020

No. This feature is custodial. Not "I let you hold my private keys" but rather "you manage my funds on your private keys. I don't want to even know the private keys"

@losnappas
Copy link

losnappas commented Aug 7, 2020

I would like to see btcpayserver use lnbits on top of the internal lightning node, exposing only the "virtual wallets" to users. I see that this issue was closed in reference to RTL, but idk how that related to the issue itself? I like lnbits because it's easily extendable.

And that hasn't moved, either. What I'm hoping is to have a self-hosted alternative to opennode/neutronpay/whatever, that can do custodial lightning and non-custodial on-chain transactions.

@Kukks
Copy link
Member

Kukks commented Aug 7, 2020

@dennisreimann
Copy link
Member

dennisreimann commented Aug 7, 2020

Yeah, I'm working on something in that regard :)

@losnappas
Copy link

losnappas commented Aug 7, 2020

I'd like to know what those limitations were, perhaps we can work them out.

Yeah, I'm working on something in that regard :)

cool

@Kukks
Copy link
Member

Kukks commented Aug 7, 2020

@losnappas
Copy link

losnappas commented Aug 7, 2020

I noticed lack of webhooks, too, but that shouldn't be too difficult to put in.

Background processing, though, no clue about that one. My guess is that because the different backend nodes do that differently that it isn't implemented. Shouldn't be undoable...probably. Perhaps I can get some opinions from @arcbtc .

@dennisreimann
Copy link
Member

dennisreimann commented Aug 7, 2020

I'd like to know what those limitations were, perhaps we can work them out.

Mostly these two things:

  • No user accounts, so you can access wallets via the URL. At least a few weeks ago this was considered a feature, but protected accounts are going to be added to LNbits as far as I understood.
  • No background processing. I worked on this and got halfway through: I got a branch which adds websocket support for LNbits, but I didn't manage to implement a checking background process as I'm not into the Python/Flask ecosystem.

@losnappas
Copy link

losnappas commented Aug 7, 2020

I see. I am under the impression that protected accounts are being added, but how will that play with btcpayserver, would you have people create 2 different accounts, one for each? It doesn't make a whole lot of sense to me. Is there maybe a way we can make btcpayserver disable some routes, if lnbits is served from /lnbits, like how RTL is served from /rtl and so on, then disable & redirect all /usr={...} routes to the user's own address. Or is that too complicated and risky? /usr={..} is the way the accounts work in lnbits, currently.

As for the background processing, could you publish your work and post a link? I'm completely oblivious to Flask as well, but better have more heads.

@dennisreimann
Copy link
Member

dennisreimann commented Aug 7, 2020

Or is that too complicated and risky?

I'm sure we could fiddle it together somehow, but I'm already working on something that integratees more tightly with BTCPay Server and its new API, so that we could also reuse existing user accounts for example.

As for the background processing, could you publish your work and post a link?

Here you go: https://github.com/dennisreimann/lnbits/tree/websocket

@losnappas
Copy link

losnappas commented Aug 7, 2020

Alright, last question before I check out the code.

Which events does btcpayserver want to hear about? Invoice payment, sure, anything else? Invoice creation, is that a possible event? Refund?

@losnappas
Copy link

losnappas commented Aug 7, 2020

So I'm a bit confused here. If btcpayserver can listen directly to the lightning node, then why does it matter whether or not lnbits can do background?

If BTCPay gets the invoices through lnbits, instead of the node, then it can still directly listen to the node itself to see when it gets paid, no?

Users create invoices through btcpay, which creates invoices via lnbits (that works on top of btcpay), then in checkout btcpay can listen for the incoming payment to that invoice directly from the node, like it does now. No need for anything lnbits related at that point; BTCPay doesn't need to be aware of the virtual wallet balances.

@Kukks
Copy link
Member

Kukks commented Aug 7, 2020

@losnappas
Copy link

losnappas commented Aug 7, 2020

Right, in that case. But should that be the case? Isn't it better to treat lnbits as lnbits? Much easier that way, less complicated.

@losnappas
Copy link

losnappas commented Aug 7, 2020

After making a blanket statement like "much easier that way," I had to compare the two designs, in thought.

Using lnbits in a way where you'd use it for invoices, and btcpayserver to track when they get paid, you only have to touch on the "get invoice" endpoint, right? Make the request go through 2 points, essentially. I don't know if there's other endpoints that would be affected and needed change. Changing the invoice logic doesn't sound complicated.

As for treating it as if it was a node, I guess you'd have to write an adapter for lnbits, like for c-lightning and lnd, and then it would need to serve all the functions of a lightning node. Is there much to gain from this? Lnbits is quite far from having all that functionality, I think.

I can see the latter being more realistic if btcpayserver has some more advanced endpoints in use, which depend on balances. I don't think that's the case, though..? The former seems more modular to me, isn't that good?

As for the case where lnbits is using a different node... Is that a realistic thing to happen? It sounds quite bizarre and unlikely.

@Kukks
Copy link
Member

Kukks commented Aug 7, 2020

@losnappas
Copy link

losnappas commented Aug 7, 2020

You could make the redirecting-service look like a node to the outside, though. Make it forward invoice requests to lnbits and everything else to the node itself, there's no explicit coupling here. Like, let's say I changed c-lightning RPC for the redirecting rpc, shouldn't that be completely transparent? That doesn't even need code changes in btcpayserver itself, I don't think.

EDIT: maybe this approach would be more of a disservice to lnbits than a benefit to btcpay, even if it did work, so I understand the approach you've all decided to take 👍 .

@losnappas
Copy link

losnappas commented Aug 9, 2020

I'm back, asking questions.

This https://github.com/btcpayserver/BTCPayServer.Lightning/blob/master/src/BTCPayServer.Lightning.Common/ILightningClient.cs is the interface that would need to be implement, right?

I'm just wondering how lnbits, as a custodial wallet, is supposed to expose stuff like OpenChannel and ListChannels. It doesn't make a whole lot of sense from that perspective, if you ask me.

@Kukks
Copy link
Member

Kukks commented Aug 10, 2020

@losnappas
Copy link

losnappas commented Aug 15, 2020

lnbits/lnbits#64

There's some support for websockets for lnd/clightning, but not for the other ones. I'm too selfish to spend my time learning about their apis and creating accounts. At least it's a start. If someone reads this and wants to finish it, then be my most esteemed guest! You will need to add the wait_invoice method to the rest of the wallets in lnbits/wallets.

@brainharrington
Copy link

brainharrington commented Oct 15, 2020

This is a good conversation, a current pain point I have is exactly what was described up top in that I have to manually cash out merchants that I'm running btc pay for because I don't have the ability to direct lightning funds straight into their control somehow

@dennisreimann
Copy link
Member

dennisreimann commented Oct 15, 2020

@brainharrington I'm currently making good progress on that front. Hope we can release a v0.1 soonish :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Any request for a new feature that does not exist (for existing features use Enhancement label)
Projects
No open projects
Development

No branches or pull requests

10 participants