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

New plugin: lnurlp #307

Closed
wants to merge 1 commit into from
Closed

New plugin: lnurlp #307

wants to merge 1 commit into from

Conversation

Kvaciral
Copy link

A simple http server to retrieve pay requests (bolt11s at the moment) to be used in lnurls

Based on the request-invoice plugin: it made sense to create a new plugin because having only one endpoint makes things more simple and secure
This one needs description_hash, while the request-invoice endpoint doesn't which would complicate things

Co-authored-by: W. J. van der Laan

@laanwj
Copy link
Contributor

laanwj commented Oct 28, 2021

For context: we tested this code here https://twitter.com/orionwl/status/1452217218876465156
It's mostly tested with Phoenix but it seems to work from other lightning wallets with support for this protocol (at least ZEBEDEE browser plugin) as well.

This currently needs the top commit from the following branch, which adds a description_hash parameter to invoice, to work: https://github.com/laanwj/lightning/tree/2021-10-description-hash

See also discussion in ElementsProject/lightning#4705.

laanwj added a commit to laanwj/lightning that referenced this pull request Nov 1, 2021
Add an optional description_hash parameter to the invoice command. This can be specified instead of the description, to create an invoice that only commits to a 256 bit hash (generally SHA256) of the description.

This is used in the lnurlp protocol. See lightningd/plugins#307 for example usage.

Closes ElementsProject#4705.

Changelog-Added: JSON-RPC: `invoice` now accepts `description_hash` parameter.
@erikarvstedt
Copy link

Would it be possible to make rate-limiting optional?

This would allow implementing rate-limiting in the hosting stack (e.g., in the reverse proxy server).
Advantages:

  • More fine-grained rate-limiting settings
  • Improved performance (not very relevant here)
  • It would allow sharing the same rate-limiting bucket with other invoice creation services.
    We plan to use this plugin alongside btcpayserver on nixbitcoin.org/donate with a shared
    bucket.

Copy link

@erikarvstedt erikarvstedt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe merge lnurlp-ratelimit-rate and lnurlp-ratelimit-switch into option lnurlp-rate-limit which accepts extra value disable.

lnurlp/README.md Outdated

Default is "2 per minute".

To disable the use of the rate limiter:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
To disable the use of the rate limiter:
To disable rate limiting:

lnurlp/lnurlp.py Outdated
@@ -116,7 +116,7 @@ def init(options, configuration, plugin):
plugin.add_option(
"lnurlp-ratelimit",
"2 per minute",
"Change flask's ratelimiter, default is 2 per minute"
"Change flask's rate limiter, default is 2 per minute, `disable` to disable"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Change flask's rate limiter, default is 2 per minute, `disable` to disable"
"Set flask's rate limiter, default is `2 per minute`, `disable` to disable"

@erikarvstedt
Copy link

erikarvstedt commented Nov 26, 2021

Fixup.
This greatly simplifies conditional rate limiting. Untested, but should work in principle.
(Fetch with git fetch https://github.com/erikarvstedt/plugins lnurlp-ea:lnurlp-ea)

@Kvaciral Kvaciral force-pushed the lnurlp branch 3 times, most recently from 5773a7a to a5c6127 Compare November 27, 2021 00:59
@cdecker
Copy link
Contributor

cdecker commented Dec 11, 2021

@Mergifyio rebase

@mergify
Copy link

mergify bot commented Dec 11, 2021

rebase

✅ Branch has been successfully rebased

@cdecker
Copy link
Contributor

cdecker commented Dec 12, 2021

@Mergifyio rebase

A simple http server to retrieve pay requests (bolt11s at the moment) to be used in lnurls

Based on the request-invoice plugin: it made sense to create a new plugin because having only one endpoint makes things more simple and secure
This one needs description_hash, while the request-invoice endpoint doesn't which would complicate things

Co-authored-by: W. J. van der Laan <laanwj@protonmail.com>
Co-authored-by: Erik Arvstedt <erik.arvstedt@gmail.com>
@mergify
Copy link

mergify bot commented Dec 12, 2021

rebase

✅ Branch has been successfully rebased

@laanwj
Copy link
Contributor

laanwj commented Mar 31, 2022

As ElementsProject/lightning#5121 was merged instead of ElementsProject/lightning#4892, this would need to be changed to use the deschashonly parameter instead of the description_hash on invoice.

@chrisguida
Copy link
Collaborator

Needs rebase

@chrisguida
Copy link
Collaborator

I propose we add this plugin instead https://github.com/elsirion/clnurl

@Kvaciral
Copy link
Author

Kvaciral commented Jun 5, 2024

This is going nowhere :-) .. perhaps elsirion's plugin will

@Kvaciral Kvaciral closed this Jun 5, 2024
@chrisguida
Copy link
Collaborator

@Kvaciral if you want to add some tests, the CI will check everything from now on.

But I'll start moving forward on clnurl since you recommended it :)

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 this pull request may close these issues.

5 participants