bumi / lnme

Your friendly Bitcoin Lightning ⚡ payment page ⚡
MIT License
167 stars 30 forks source link

Add configuration property for allowed length of LNURL-pay comments #40

Closed yanascz closed 2 years ago

bumi commented 2 years ago

thanks for the PR! that looks good!

do you know why not more lnurlp implementations add support for comments like that?

yanascz commented 2 years ago

do you know why not more lnurlp implementations add support for comments like that?

Maybe because it’s defined under different RFC? I can only guess. I saw support for comments by ln.tips so I started digging and discovered #12.

ok300 commented 2 years ago

Looking forward to this.

ok300 commented 2 years ago

Why wait when I can run your branch directly :)

Started doing that, works great.

Just one question, why is lnurlp-comment-allowed default 0? I doubt people don't want comments by default, I'd say a reasonable number (200-500) is a good sane default. Enough for comment, but small enough to not run into any limits.

yanascz commented 2 years ago

Just one question, why is lnurlp-comment-allowed default 0? I doubt people don't want comments by default, I'd say a reasonable number (200-500) is a good sane default. Enough for comment, but small enough to not run into any limits.

I wanted the change to be backwards compatible. But I'm OK with a different default, e.g. 210 (as 21 is such a nice number 😉). Ultimately, it's up to @bumi to decide what default he wants.

bumi commented 2 years ago

ok, cool. then let's enable it by default. @yanascz can you update that, then I'll merge it and make a new release @ok300 thanks for testing and giving feedback.

yanascz commented 2 years ago

ok, cool. then let's enable it by default. @yanascz can you update that, then I'll merge it and make a new release

Done! ✅