Skip to content

Conversation

@f321x
Copy link
Member

@f321x f321x commented Jun 27, 2025

lnurl-withdraw is used by lightning bitcoin atms, vouchers (e.g. azteco), and web services allowing users to withdraw funds without having to copy-paste invoices (esp. between desktop <-> smartphone).
https://github.com/lnurl/luds/blob/227f850b701e9ba893c080103c683273e2feb521/03.md

Can be tested with https://demo.lnbits.com and the Withdraw Links (LNURLW) and Pay Links (LNURLP) extensions.

@f321x f321x force-pushed the lnurl_withdraw branch 2 times, most recently from 0d1e6b5 to ca7a2b9 Compare July 1, 2025 08:40
@f321x f321x marked this pull request as ready for review July 1, 2025 08:40
@f321x f321x force-pushed the lnurl_withdraw branch 4 times, most recently from 33780d9 to 07e71df Compare July 2, 2025 07:35
@accumulator
Copy link
Member

It feels somewhat out of place to integrate this in the Send flow UI for payments, and not the Receive flow UI. Also, (QE)Invoice(Parser) is used instead of (QE)Request(Details)

@f321x
Copy link
Member Author

f321x commented Jul 14, 2025

I agree seems a bit unintuitive to paste a string to receive money into the send tab, i implemented it like this because it seems to be the "standard", i don't know of any other wallet which handles it differently, e.g. Phoenix and Blink also require to paste LNURLWs into the send field.
It would also be confusing to have a separate 'paste' button in the receive tab to paste (or scan) an 'address'/voucher to receive from, or do you see more natural way how this could be integrated without adding more buttons/fields the user has to understand? I think showing the popup when pasting the lnurlw is the clearest way to separate it from "sending" ui.

Technically, besides doing opposite things, lnurlw and lnurlp have a quite similar flow which also makes a point for handling them closely together. Its not even clear if a given lnurl string is a payRequest or a withdrawRequest until we call the encoded url and got the response from the lnurl server. Otherwise logic for handling lnurls (resolving etc.) would have to be duplicated in the (QE)Request(Details) code.

@accumulator
Copy link
Member

I agree seems a bit unintuitive to paste a string to receive money into the send tab, i implemented it like this because it seems to be the "standard", i don't know of any other wallet which handles it differently, e.g. Phoenix and Blink also require to paste LNURLWs into the send field. It would also be confusing to have a separate 'paste' button in the receive tab to paste (or scan) an 'address'/voucher to receive from, or do you see more natural way how this could be integrated without adding more buttons/fields the user has to understand? I think showing the popup when pasting the lnurlw is the clearest way to separate it from "sending" ui.

Yeah, I understand this is probably the least invasive way of doing it. I do think some people will try the Receive button for this use case. The only thing I can think of UI-wise is having an small extra button 'Scan voucher' or something in the receive tab/view, which then also presents the QR scanner.

Technically, besides doing opposite things, lnurlw and lnurlp have a quite similar flow which also makes a point for handling them closely together. Its not even clear if a given lnurl string is a payRequest or a withdrawRequest until we call the encoded url and got the response from the lnurl server. Otherwise logic for handling lnurls (resolving etc.) would have to be duplicated in the (QE)Request(Details) code.

The main issue with having this code in QEInvoice(Parser) is that most of the code there assumes outgoing payments. This might trip us over in the future. I think the potential duplication of code is limited, most of the work is done in PaymentIdentifier.

@f321x f321x force-pushed the lnurl_withdraw branch 2 times, most recently from 6541a7e to abf0059 Compare July 16, 2025 11:41
@f321x
Copy link
Member Author

f321x commented Jul 16, 2025

Ok in abf0059 i removed the resolving of the 'recipient' into a separate class QEPIResolver, so it can first resolve the user input so its clear if it is a Invoice or a Request/Voucher (lnurlw) type. The lnurlw logic is now in QERequestDetails so its clearly separated from the QEInvoiceParser.
Do you think this is preferable to the implementation in ae310ad @accumulator ?

Yeah, I understand this is probably the least invasive way of doing it. I do think some people will try the Receive button for this use case. The only thing I can think of UI-wise is having an small extra button 'Scan voucher' or something in the receive tab/view, which then also presents the QR scanner.

I think its better to have a simpler UI instead of adding an extra button to the receive ui which is then used only used by a small subset of all users. I think those that want to scan a lnurlw qr code will naturally figure out to click on send as its the only view that presents a camera to scan things.

@accumulator
Copy link
Member

Do you think this is preferable to the implementation in ae310ad @accumulator ?

I will have a look after the weekend

@f321x f321x force-pushed the lnurl_withdraw branch 2 times, most recently from b71b5e6 to b0a1eaa Compare August 11, 2025 15:23
@accumulator
Copy link
Member

sorry for the long review delay.

LGTM, but added one commit

@f321x
Copy link
Member Author

f321x commented Aug 12, 2025

@accumulator thanks!

f321x added 4 commits August 27, 2025 15:31
adds handling of lnurl-withdraw payment identifiers which allow users to
withdraw bitcoin from a service by scanning a qr code or pasting the
lnurl-w code as "sending" address.
adds some more detailed tests to `test_payment_identifier.py` to test
lnurlp and lnurlw separately and mock their resolve.
separates the resolving step from the QEInvoiceParser so the 'recipient'
can be resolved first and then either an QEInvoiceParser can be used if
it is a sending request that has been resolved (invoice, address,
lnurlp, ...), or RequestDetails can be used if the resolved 'recipient'
turns out to be a voucher/LNURLW string.

# Conflicts:
#	electrum/gui/qml/qeinvoice.py
binds the walletCanReceive variable to the available inbound liquidity
so the withdraw button gets enabled when the channels reconnect if the
user opens a lnurlw request dialog before the channels have connected.
@f321x
Copy link
Member Author

f321x commented Aug 27, 2025

If #10178 gets merged needsSystemBarPadding: false needs to be added to LnurlWithdrawRequestDialog for correct padding.

it is not neccessary to sanitize all LNURLErrors as most are just source
strings.
SomberNight added a commit to SomberNight/electrum that referenced this pull request Oct 2, 2025
self.error = pr.error
self.set_state(PaymentIdentifierState.ERROR)
elif self.lnurl:
data = await request_lnurl(self.lnurl)
Copy link
Member

@SomberNight SomberNight Oct 3, 2025

Choose a reason for hiding this comment

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

When testing with the demo lnbits instance, on Android I keep getting an error:

10-03 02:04:33.443  7605  7643 I python  :  20.77 | E | payment_identifier.PaymentIdentifier | _do_resolve() got error: LNURLError('Client error: 400, message="Duplicate \'Server\' header found.", url=\'https://demo.lnbits.com/lnurlp/kYiJH3\'')

Which seems to have a corresponding ticket with aiohttp: aio-libs/aiohttp#10321 re incompatibility with the caddy webserver/reverse-proxy.

and indeed the demo lnbits instance is sending duplicate "server" headers, and looks to be using caddy:

$ curl -v "https://demo.lnbits.com/lnurlp/kYiJH3"
* Host demo.lnbits.com:443 was resolved.
* IPv6: (none)
* IPv4: 172.81.177.34
*   Trying 172.81.177.34:443...
* ALPN: curl offers h2,http/1.1
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
*  CAfile: /etc/ssl/certs/ca-certificates.crt
*  CApath: /etc/ssl/certs
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.3 (IN), TLS change cipher, Change cipher spec (1):
* TLSv1.3 (IN), TLS handshake, Encrypted Extensions (8):
* TLSv1.3 (IN), TLS handshake, Certificate (11):
* TLSv1.3 (IN), TLS handshake, CERT verify (15):
* TLSv1.3 (IN), TLS handshake, Finished (20):
* TLSv1.3 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.3 (OUT), TLS handshake, Finished (20):
* SSL connection using TLSv1.3 / TLS_AES_128_GCM_SHA256 / x25519 / id-ecPublicKey
* ALPN: server accepted h2
* Server certificate:
*  subject: CN=demo.lnbits.com
*  start date: Sep 26 13:21:21 2025 GMT
*  expire date: Dec 25 13:21:20 2025 GMT
*  subjectAltName: host "demo.lnbits.com" matched cert's "demo.lnbits.com"
*  issuer: C=US; O=Let's Encrypt; CN=E7
*  SSL certificate verify ok.
*   Certificate level 0: Public key type EC/prime256v1 (256/128 Bits/secBits), signed using ecdsa-with-SHA384
*   Certificate level 1: Public key type EC/secp384r1 (384/192 Bits/secBits), signed using sha256WithRSAEncryption
*   Certificate level 2: Public key type RSA (4096/152 Bits/secBits), signed using sha256WithRSAEncryption
* Connected to demo.lnbits.com (172.81.177.34) port 443
* using HTTP/2
* [HTTP/2] [1] OPENED stream for https://demo.lnbits.com/lnurlp/kYiJH3
* [HTTP/2] [1] [:method: GET]
* [HTTP/2] [1] [:scheme: https]
* [HTTP/2] [1] [:authority: demo.lnbits.com]
* [HTTP/2] [1] [:path: /lnurlp/kYiJH3]
* [HTTP/2] [1] [user-agent: curl/8.14.1]
* [HTTP/2] [1] [accept: */*]
> GET /lnurlp/kYiJH3 HTTP/2
> Host: demo.lnbits.com
> User-Agent: curl/8.14.1
> Accept: */*
> 
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* Request completely sent off
< HTTP/2 200 
< alt-svc: h3=":443"; ma=2592000
< content-type: application/json
< date: Fri, 03 Oct 2025 00:20:12 GMT
< server: Caddy
< server: uvicorn
< content-length: 173
< 
* Connection #0 to host demo.lnbits.com left intact
{"tag":"payRequest","callback":"https://demo.lnbits.com/lnurlp/api/v1/lnurl/cb/kYiJH3","minSendable":100000,"maxSendable":100000,"metadata":"[[\"text/plain\", \"test24\"]]"}

However, I only see this error when running on Android. On desktop, there is no error.
Many of the dependencies I am using are the same version between the two environments, e.g. aiohttp and the packages/ frozen deps are the same version. But I have different versions of e.g. cpython and openssl between Android and desktop Linux. Weird.........

EDIT: tried with python3.10 on desktop, still can't repro there.

SomberNight added a commit to SomberNight/electrum that referenced this pull request Oct 3, 2025
SomberNight added a commit to SomberNight/electrum that referenced this pull request Oct 3, 2025
@SomberNight SomberNight merged commit 7d0ac64 into spesmilo:master Oct 3, 2025
12 of 14 checks passed
@SomberNight SomberNight added this to the 4.6.3 milestone Oct 3, 2025
@SomberNight
Copy link
Member

Only did a quick review of the code. Tested it manually on both GUIs.

I wasted too much time battling the demo lnbits instance (and building apks)...
Besides #9993 (comment),
I also kept getting an error "This link requires an id_unique_hash" - which ultimately I worked around by using the "advanced withdraw links" menu in the lnbits webui to generate lnurl-withdraw qr codes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants