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

Support for KeySend (a.k.a. Sphinx Send) #33

Closed
dellagustin opened this issue Dec 9, 2020 · 13 comments · Fixed by #42
Closed

Support for KeySend (a.k.a. Sphinx Send) #33

dellagustin opened this issue Dec 9, 2020 · 13 comments · Fixed by #42

Comments

@dellagustin
Copy link

Hello All, any plans to add support for key send to the spec in the near future?

I am asking this due to the work being done at podcastindex.org to monetize podcasts by streaming sats using lightning key send transfers.

References

@dellagustin dellagustin changed the title Support for Key Send (a.k.a. Sphinx Send Support for Key Send (a.k.a. Sphinx Send) Dec 9, 2020
@wbobeirne
Copy link
Member

wbobeirne commented Dec 9, 2020

Key send still feels a little "wild west" to me to fully know exactly what I'd be building, and how the UX would play out. Joule primarily interacts with web applications that hypothetically have a server involved to facilitate invoice generation. Typically the invoice generation and payment goes hand-in-hand with the authentication to enforce that the user paid for their content. How does this work with podcasts and preventing people from streaming if they haven't paid? Or is this merely for tipping?

@dellagustin
Copy link
Author

Hello @wbobeirne, thanks for your quick response. Yes, it is for tipping.
Podcasters can suggest an amount, but at the end it is up to the users and applications how much to actually send.

I don't know how the UX would be, I have not tried the existing providers yet (just found out about WebLN today).
We would certainly need some guardrails to avoid unrestricted spending from web applications.
For the streaming scenario one of the complications is that the idea is to send micropayments frequenty (i.e. once a second, 10 seconds, a minute, etc...), so prompting the user at every transaction would not be feasible.
Maybe the user could pre-approve a certain "budget" for a specific domain, once the balance is out the user would have to allocate more funds for the domain, or something like that.

@daveajones
Copy link

daveajones commented Dec 9, 2020

That's right. We're not using it as a paywall. We're using it as a way for listeners to choose to send a certain amount of sats per minute to the podcast creator as they listen. It works well, and has been fairly solid so far. I'm receiving sats on my voltage LND node each minute when someone listens to our show on the Sphinx app.

@dellagustin
Copy link
Author

@wbobeirne now that you have more information on the use case, do you see it being part of WebLN?

@dellagustin
Copy link
Author

dellagustin commented Apr 29, 2021

Hey @wbobeirne , just checking if you have any new thoughts on this topic.

@bumi
Copy link

bumi commented Apr 29, 2021

@dellagustin are you interested in working together on a proposal for this?
I think sendPayment should accept an option (just similar to makeInvoice) that would give us probably all the needed flexibility. We would just need to somehow think about backwards compatibility.

I also think it is time for an webln update and maybe a proposal for a v1. :)

@wbobeirne
Copy link
Member

Would love to see what a proposed API would look like, but given that it's a different set of arguments, not all implementations of WebLN will necessarily support it out of the box, and not all nodes will know how to send / receive them, I'm a little hesitant to try to overload sendPayment with this. Perhaps it should be a new method altogether?

@bumi
Copy link

bumi commented May 3, 2021

for me webln describes the interface how lightning apps and user's wallets communicate.
And since webln was published lightning has new features (keysend) (and evolve further) and also people started doing things like lnurl-pay/withdraw/auth, moneysocket, etc.

In my point of view WebLN specifies how the communication is done and not necessarily limits what can be communicated.
e.g. there might be users with wallets that maybe can not sign a message or create an invoice. So we have this problem that not all nodes implement all features.

@wbobeirne
Copy link
Member

Ha, so I was looking around at Sphinx's codebas and noticed they basically copy the interface for WebLN with their sphinx bridge: https://github.com/stakwork/sphinx-bridge. And they implemented a separate sphinx.keysend(pubkey, amount) method.

In my point of view WebLN specifies how the communication is done and not necessarily limits what can be communicated.
e.g. there might be users with wallets that maybe can not sign a message or create an invoice.

Totally agree! However those methods are one-off separate methods that you either do or do not support, full-stop. If keysend were just a part of sendPayment, it would be supported conditionally depending on whether or not the wallet handles the keysend signature. All old wallets would start throwing errors about invalid bolt11 strings or what have you, if they didn't update to support keysend. I assume this is the same reason Sphinx made it a separate method as well.

But I fully intended for certain wallets not to support certain features, that's why I standardized the UnsupportedMethodError: https://github.com/joule-labs/webln/blob/master/src/errors.ts#L76-L82

@bumi
Copy link

bumi commented May 6, 2021

yes, backwards compatibility must be a priority. (if sendPayment gets a String it must be treated as bolt11 invoice - kinda ugly).

I was just wondering how we can keep the spec flexible enough that we do not have to define new methods for all potential features. (or we should be quick in standardizing new features - the podcastindex example shows that the implementation there is held back by the missing support for the not-so-new-anymore keysend feature)

thinking out loud:
maybe it makes sense to define a supportedFeatures method which returns an array of methods that the wallet implements. Then a new method makes sense... (though payInvoice would be a better name for sendPayment then)

In an extension I am currently working on I add support to connect a lnbits wallet. And lnbits does not support signMessage. Thus for me a similar problem exists there.

@dellagustin dellagustin changed the title Support for Key Send (a.k.a. Sphinx Send) Support for KeySend (a.k.a. Sphinx Send) May 29, 2021
@wbobeirne
Copy link
Member

we do not have to define new methods for all potential features. (or we should be quick in standardizing new features)

This is my preference, WebLN should ideally behave more like W3 / BIPs / BOLTs in that people can feel free to draft specs and work on implementations quickly and out in the wild, with some understanding of what "stage" a feature is in.

I think defining new methods is a lot more straight-forward for developers than having uncertain behavior when calling a single method.

maybe it makes sense to define a supportedFeatures method... does not support signMessage. Thus for me a similar problem exists there.

I think we should roll with this the way browsers do; the typing assumes the feature is available in all implementations once the spec for the method is accepted, and it's up to the developer to account for missing features in various implementations. The TypeScript types can remain assuming the features exist, and developers can either throw caution to the wind and assume the methods are available, or do if ('keysend' in webln) { /* ... */ }.

though payInvoice would be a better name for sendPayment then

Haha no kidding, had I had the foresight that there'd be many alternative payment methods in Lightning, I'd have been more specific with the names!


All that said, I think given the Sphinx team has already gotten some momentum on sphinx.keysend(pubkey, amount), that would be my preferred implementation. If all agree, we can add that definition to webln, and Joule et al can implement when they're ready.

@wbobeirne
Copy link
Member

One reason I've held off on this is that I'm aware that LND is focusing on AMP instead of keysend, and plans to do a lot of changes around it, see lightningnetwork/lnd#5190 (comment) and other notes like this in their GitHub.

Perhaps we should avoid attaching to the concept of a "keysend" and instead come up with generic naming that just means payment without invoice. Spontaneous payment, push payment, etc.

@dellagustin
Copy link
Author

Hey all, I have been out for some time, finally I could read the full thread.

I think we should roll with this the way browsers do; the typing assumes the feature is available in all implementations once the spec for the method is accepted, and it's up to the developer to account for missing features in various implementations. The TypeScript types can remain assuming the features exist, and developers can either throw caution to the wind and assume the methods are available, or do if ('keysend' in webln) { /* ... */ }.

I am fine with this, I also agree that checking if a method or property exists is kind of a standard for backwards compatible web development.

All that said, I think given the Sphinx team has already gotten some momentum on sphinx.keysend(pubkey, amount), that would be my preferred implementation. If all agree, we can add that definition to webln, and Joule et al can implement when they're ready.

I see two options here,

  1. is making the sendPayment method more flexible and expose more of the common fields on the underlying API (i.e. https://api.lightning.community/#v2-router-send), the current spec only expects a payment_request, but it could instead also accept an object with other properties such as destination pub key, amount, payment hash, etc...), with a minimum set of properties, it would indirectly support keysend, but it would be more troublesome for the consumer, as he/she would have to implement the keysend protocol (currently it simply adds the pre-image as a custom TLV record)
  2. the keysend method, here we are adding to the spec something that is currently experimental (as long as it is using a custom TLV), I'm fine with that, but we should probably keep it in the experimental stage until it is made more official - one important bit here, it should support custom TLV records on the arguments, this is required for podcasts receiving value on LNPay and satoshi.streams, also, a custom record for podcast metadata is already in use, although not mandatory, exect in part for satoshis.stream

Perhaps we should avoid attaching to the concept of a "keysend" and instead come up with generic naming that just means payment without invoice. Spontaneous payment, push payment, etc.

One question that comes to mind here: how does the implementation of WebLN decide what to send, if it implements keysend now, how does it decide when to switch to spontaneous AMPs? does it try one and if it fails, tries the other as a fallback?
PS: maybe sendSpontaneousPayment would be a name to consider for the method.

References:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants