Add LNURL support for amountless Lightning receives#482
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When the user skips the amount on the receive screen and VITE_LNURL_SERVER_URL is configured, the wallet opens an SSE session with the lnurl-server and displays the resulting LNURL in the QR code. When a payer scans and chooses an amount, the wallet creates a reverse swap on-the-fly and returns the bolt11. - LNURL embedded in BIP21 URI via lightning= parameter - LNURL shown in expanded address list - Session auto-closes when navigating away Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds LNURL receive support: new streaming Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Wallet UI
participant Hook as useLnurlSession
participant LNURL as LNURL Server
participant Handler as onInvoiceRequest
participant Swap as Swap Engine
UI->>Hook: enable session (enabled = true)
Hook->>LNURL: POST /lnurl/session
LNURL-->>Hook: stream body (lines)
LNURL->>Hook: event: session_created\ndata: { sessionId, lnurl }
Hook->>UI: set lnurl, sessionId, active=true
LNURL->>Hook: event: invoice_request\ndata: { amountMsat, comment? }
Hook->>Handler: onInvoiceRequest(amountMsat)
Handler->>Swap: create reverse swap / waitAndClaim
Swap-->>Handler: pendingSwap.response.invoice (pr)
Handler-->>Hook: return pr
Hook->>LNURL: POST /lnurl/session/{sessionId}/invoice { pr }
LNURL-->>Hook: ack
UI->>Hook: disable/unmount
Hook->>LNURL: abort stream
Hook->>UI: active=false, clear lnurl/sessionId
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying wallet-bitcoin with
|
| Latest commit: |
7b38342
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1ecb99aa.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://feat-lnurl-amountless-receiv.wallet-bitcoin.pages.dev |
🔍 PR Review —
|
Deploying wallet-mutinynet with
|
| Latest commit: |
7b38342
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://911c295e.arkade-wallet.pages.dev |
| Branch Preview URL: | https://feat-lnurl-amountless-receiv.arkade-wallet.pages.dev |
- Wrap JSON.parse in try/catch to prevent SSE stream crash on malformed payloads - Validate amountMsat is a positive number before creating swap - Strip trailing slash from LNURL server URL - Add missing isAmountlessLnurl to useEffect dependency array Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The replace_all swapped the alias direction — import the correct export name from constants. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/lib/bip21.ts (1)
76-81: Add a regression test for the new QR branches.The referenced
encodeBip21test still only covers the non-zero-sats invoice case. This change adds both anlnurlfallback and theamount=omission path, so it needs an explicit fixture to keep the unified QR payload stable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/bip21.ts` around lines 76 - 81, Add a regression test for encodeBip21 to cover the new QR branches: one fixture where invoice is empty but lnurl is provided (ensuring the function returns a payload with &lightning=<lnurl>), and one fixture where sats is zero/undefined (ensuring the returned payload omits the &amount= parameter). Locate the encodeBip21 function and add tests that assert the exact returned string for both cases (lnurl fallback and amount omission) so the unified QR payload remain stable across changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useLnurlSession.ts`:
- Around line 40-49: postInvoice currently ignores HTTP error responses; modify
the function that calls
fetch(`${lnurlServerUrl}/lnurl/session/${sessionId}/invoice`) inside postInvoice
to check the Response.ok flag and treat non-2xx responses as failures: when
response.ok is false (or fetch throws), log the response/error via consoleError
and clear the lnurl session state (call setLnurl(null) or the hook's
session-clearing function) so the UI stops advertising the dead LNURL; ensure
postInvoice returns or throws after clearing state so the finally block does not
leave lnurl populated.
In `@src/screens/Wallet/Receive/QrCode.tsx`:
- Around line 57-61: handleInvoiceRequest currently floors req.amountMsat to
sats and calls createReverseSwap without validating the resulting satoshi
amount; replicate the same guard used in the fixed-amount path by converting
amountMsat to satoshis (rounding appropriately), calling validLnSwap(sats) and
throwing a user-facing error when it returns false before calling
createReverseSwap, so out-of-range or sub-sat requests are rejected early;
update the logic inside handleInvoiceRequest (and the similar block around line
75) to perform this validation and only proceed to createReverseSwap(sats) when
validLnSwap passes.
- Around line 55-77: The LNURL is currently enabled by isAmountlessLnurl
regardless of reverse-swap readiness, which can expose an unusable LNURL; change
the gating so useLnurlSession is only invoked when reverse swaps can actually be
created. Concretely, update the isAmountlessLnurl logic (or introduce a new
boolean like isLnurlReady) to require arkadeSwaps to be initialized and
createReverseSwap to be able to return a non-null pending swap (e.g., check
arkadeSwaps != null and optionally perform a lightweight readiness check or
ensure createReverseSwap won’t return null), and pass that boolean to
useLnurlSession instead of the current isAmountlessLnurl; keep
handleInvoiceRequest, createReverseSwap, arkadeSwaps and useLnurlSession names
to locate the code to change.
---
Nitpick comments:
In `@src/lib/bip21.ts`:
- Around line 76-81: Add a regression test for encodeBip21 to cover the new QR
branches: one fixture where invoice is empty but lnurl is provided (ensuring the
function returns a payload with &lightning=<lnurl>), and one fixture where sats
is zero/undefined (ensuring the returned payload omits the &amount= parameter).
Locate the encodeBip21 function and add tests that assert the exact returned
string for both cases (lnurl fallback and amount omission) so the unified QR
payload remain stable across changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bc2c7468-3e31-4fbc-ad14-6e169151b81e
📒 Files selected for processing (5)
src/components/ExpandAddresses.tsxsrc/hooks/useLnurlSession.tssrc/lib/bip21.tssrc/lib/constants.tssrc/screens/Wallet/Receive/QrCode.tsx
|
Followup review (commit 🐛 Import name mismatch — build will fail
import { lnurlServerBaseUrl as rawLnurlServerUrl } from '../lib/constants'But Fix: ✅ Good changes in this commit
|
…idation - postInvoice now checks response.ok and throws on failure - Clear LNURL state on invoice request failure and in finally block - Gate LNURL session on arkadeSwaps being initialized and connected - Add validLnSwap check before creating reverse swap Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Followup review (commit Good incremental improvements:
One minor observation: on LGTM on the incremental changes. |
The eventType variable was declared inside the while loop, causing it to reset on each chunk read. When the event: and data: lines arrive in separate chunks (common in browsers), the data line was silently ignored. Move eventType outside the loop so it persists across chunk boundaries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Followup review — Good fix. Moving One observation: the same chunk-boundary issue could theoretically affect LGTM on this commit. ✅ |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/screens/Wallet/Receive/QrCode.tsx (1)
58-61:⚠️ Potential issue | 🟠 MajorReject non-sat LNURL amounts instead of flooring them.
Line 60 silently rewrites the payer's requested amount:
1001msat becomes a1sat reverse swap. Guard non-multiples of1000and only divide once the request is sat-aligned.🧮 Proposed fix
async (req: { amountMsat: number }) => { - const sats = Math.floor(req.amountMsat / 1000) + if (req.amountMsat % 1000 !== 0) { + throw new Error('Sub-sat LNURL amounts are unsupported') + } + const sats = req.amountMsat / 1000 if (!validLnSwap(sats)) throw new Error('Amount outside Lightning receive limits')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Wallet/Receive/QrCode.tsx` around lines 58 - 61, handleInvoiceRequest currently floors req.amountMsat into sats which silently accepts non-sat-aligned amounts; instead, first verify req.amountMsat is an exact multiple of 1000 and throw an error if not (e.g., "Amount must be in whole sats"), then compute sats = req.amountMsat / 1000 (no Math.floor) and pass that to validLnSwap; update the logic in the handleInvoiceRequest callback to reject non-multiples of 1000 before any further validation or processing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useLnurlSession.ts`:
- Around line 42-49: Capture the current session id into a local const (e.g.,
const sessionId = sessionIdRef.current) before awaiting onInvoiceRequest in the
handler, update postInvoice to accept an AbortSignal and pass abort.signal into
fetch, and ensure any handler failure calls abort.abort() (the session's
AbortController) so the SSE stream and any inflight fetches are cancelled;
specifically modify useLnurlSession's onInvoiceRequest flow to use the captured
sessionId when calling postInvoice (instead of reading sessionIdRef.current
after awaits), add an abort parameter to postInvoice and pass it through to
fetch, and invoke abort.abort() on errors/cleanup to terminate the session
fetches and prevent stray invoice replies.
---
Duplicate comments:
In `@src/screens/Wallet/Receive/QrCode.tsx`:
- Around line 58-61: handleInvoiceRequest currently floors req.amountMsat into
sats which silently accepts non-sat-aligned amounts; instead, first verify
req.amountMsat is an exact multiple of 1000 and throw an error if not (e.g.,
"Amount must be in whole sats"), then compute sats = req.amountMsat / 1000 (no
Math.floor) and pass that to validLnSwap; update the logic in the
handleInvoiceRequest callback to reject non-multiples of 1000 before any further
validation or processing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bcd96db9-ad4e-4c15-851f-1504e2486549
📒 Files selected for processing (2)
src/hooks/useLnurlSession.tssrc/screens/Wallet/Receive/QrCode.tsx
| const bip21uri = isAssetReceive | ||
| ? encodeBip21Asset(arkAddress, assetId, centsToUnits(satoshis, assetMeta?.metadata?.decimals)) | ||
| : encodeBip21(btcAddress, arkAddress, invoice, satoshis) | ||
| : encodeBip21(btcAddress, arkAddress, invoice, satoshis, lnurlSession.lnurl) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "encodeBip21" | head -20Repository: arkade-os/wallet
Length of output: 147
🏁 Script executed:
cat -n src/lib/bip21.tsRepository: arkade-os/wallet
Length of output: 3399
🏁 Script executed:
cat -n src/test/lib/bip21.test.tsRepository: arkade-os/wallet
Length of output: 2137
🌐 Web query:
BIP321 lightning payment instruction LNURL specification
💡 Result:
BIP-0321 defines the bitcoin: URI scheme for Bitcoin payment instructions, replacing BIP-0021. It supports Lightning payments via the 'lightning' query parameter containing BOLT11 invoices (e.g., bitcoin:?lightning=lnbc420bogusinvoice). No direct integration or specification for LNURL (a separate Lightning protocol using lnurl: URIs and HTTP callbacks for dynamic invoices) exists in BIP-0321. LNURL is handled via its own 'lnurl:' URI scheme or Lightning Addresses, distinct from BIP-0321 bitcoin: URIs. Wallets may support both independently for unified QR codes or payment requests, but they are not formally linked in the specs.
Citations:
- 1: https://bips.dev/321/
- 2: https://mirror.b10c.me/bitcoin-bips/1555/
- 3: https://bips.dev/321
- 4: https://bitcoin.design/guide/how-it-works/payment-request-formats
- 5: https://bitcoinqr.dev/
- 6: https://github.com/lnurl/luds/blob/legacy/lnurl-pay.md
🏁 Script executed:
cat -n src/screens/Wallet/Receive/QrCode.tsx | sed -n '170,195p'Repository: arkade-os/wallet
Length of output: 1216
encodeBip21 serializes LNURL as lightning= instead of a separate key, violating BIP-0321 compliance.
Line 80 in src/lib/bip21.ts encodes LNURL as &lightning=${lnurl} when no invoice is present. BIP-0321 reserves the lightning parameter specifically for BOLT11 invoices; LNURL uses its own separate lnurl: URI scheme. This conflation works due to prefix detection in decodeBip21 (checking whether the value starts with "lnurl" or "ln"), but it creates non-standard QR codes that may not parse correctly in wallets expecting strict BIP-0321 compliance.
When the wallet can't create a swap (e.g. amount outside limits),
post { error: "reason" } back to the server so the payer gets an
immediate error instead of waiting for timeout. Remove validLnSwap
pre-check since the server enforces min/max via LNURL spec and
createReverseSwap will fail naturally with a descriptive error.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🔍 Arkana PR Review —
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/screens/Wallet/Receive/QrCode.tsx (1)
59-61:⚠️ Potential issue | 🟠 MajorReject sub-satoshi LNURL amounts instead of rounding them down.
Math.floor(req.amountMsat / 1000)can turn a payer-selected amount like1500msat into a1sat invoice. That underbills the requested amount instead of rejecting an unsupported fractional-sat request up front.🧮 Suggested fix
async (req: { amountMsat: number }) => { - const sats = Math.floor(req.amountMsat / 1000) + if (req.amountMsat % 1000 !== 0) { + throw new Error('Sub-satoshi LNURL amounts are unsupported') + } + const sats = req.amountMsat / 1000 const pendingSwap = await createReverseSwap(sats)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Wallet/Receive/QrCode.tsx` around lines 59 - 61, The handler that converts req.amountMsat to sats currently floors fractional-sat values; instead validate req.amountMsat is a whole-satoshi amount before conversion: check req.amountMsat % 1000 === 0 and if not reject/throw an error (e.g., return a 400 or throw new Error) rather than calling createReverseSwap; only compute const sats = req.amountMsat / 1000 and call createReverseSwap(sats) when the msat value is exactly divisible by 1000.src/hooks/useLnurlSession.ts (1)
42-63:⚠️ Potential issue | 🟠 MajorKeep invoice/error replies bound to the session that requested them.
After
await onInvoiceRequestRef.current(...), this code rereadssessionIdRef.current, so a request that started under session A can reply into session B if the hook is disabled/re-enabled in place. The reply POSTs also omitabort.signal, andpostErrornever inspectsresponse.ok, so stale or failed callbacks can leave a dead LNURL advertised. Capture the currentsessionIdbefore the await, passabort.signalinto both POSTs, and fail the session when the error callback cannot be delivered.Also applies to: 122-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useLnurlSession.ts` around lines 42 - 63, Capture the sessionId from sessionIdRef.current into a local const before awaiting onInvoiceRequestRef.current so the reply is tied to the session that initiated the request; then pass the current abort.signal into both fetch calls in postInvoice and postError, and in postError inspect response.ok and throw an error (or otherwise mark the session failed) when the POST fails so the session is aborted/failed if the error callback cannot be delivered; update references to postInvoice, postError, onInvoiceRequestRef.current, and sessionIdRef.current accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useLnurlSession.ts`:
- Around line 116-120: The code currently logs invalid amountMsat and drops the
invoice_request; instead, when Number(data.amountMsat) is falsy or <= 0,
construct and send an immediate error reply to the requester (using your
existing response/emit helper used elsewhere in this hook) rather than just
consoleError, then clear eventType and continue; specifically, in the branch
that checks amountMsat (referencing amountMsat, consoleError, eventType) call
the same reply routine you use for other errors (e.g.,
sendResponse/sendError/emitError in this hook) with a clear error string (e.g.,
"invalid_amount" / "amount must be > 0") so the payer receives an immediate
error before continuing.
---
Duplicate comments:
In `@src/hooks/useLnurlSession.ts`:
- Around line 42-63: Capture the sessionId from sessionIdRef.current into a
local const before awaiting onInvoiceRequestRef.current so the reply is tied to
the session that initiated the request; then pass the current abort.signal into
both fetch calls in postInvoice and postError, and in postError inspect
response.ok and throw an error (or otherwise mark the session failed) when the
POST fails so the session is aborted/failed if the error callback cannot be
delivered; update references to postInvoice, postError,
onInvoiceRequestRef.current, and sessionIdRef.current accordingly.
In `@src/screens/Wallet/Receive/QrCode.tsx`:
- Around line 59-61: The handler that converts req.amountMsat to sats currently
floors fractional-sat values; instead validate req.amountMsat is a whole-satoshi
amount before conversion: check req.amountMsat % 1000 === 0 and if not
reject/throw an error (e.g., return a 400 or throw new Error) rather than
calling createReverseSwap; only compute const sats = req.amountMsat / 1000 and
call createReverseSwap(sats) when the msat value is exactly divisible by 1000.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 050b2923-e80f-49e2-82ef-f66d00851cf6
📒 Files selected for processing (2)
src/hooks/useLnurlSession.tssrc/screens/Wallet/Receive/QrCode.tsx
Store the token from session_created SSE event and include it as Authorization: Bearer header on all invoice POST requests. Prevents unauthorized parties from submitting fake invoices even if they discover the session ID. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/hooks/useLnurlSession.ts (2)
131-151:⚠️ Potential issue | 🟠 MajorCapture session id before awaiting invoice creation to prevent cross-session replies.
At Line 139, reply generation is awaited before reading
sessionIdRef.current(Line 143/149). If a new LNURL session starts meanwhile, the old request can be posted to the new session id. CapturesessionIdonce at the start of theinvoice_requestbranch and use it for both success and error posts; also threadabort.signalinto reply POSTs so teardown cancels in-flight callbacks.🔧 Proposed hardening
- const postInvoice = useCallback( - async (sessionId: string, pr: string) => { + const postInvoice = useCallback( + async (sessionId: string, pr: string, signal: AbortSignal) => { const response = await fetch(`${lnurlServerBaseUrl}/lnurl/session/${sessionId}/invoice`, { method: 'POST', + signal, headers: authHeaders(), body: JSON.stringify({ pr }), }) @@ - const postError = useCallback( - async (sessionId: string, reason: string) => { + const postError = useCallback( + async (sessionId: string, reason: string, signal: AbortSignal) => { try { await fetch(`${lnurlServerBaseUrl}/lnurl/session/${sessionId}/invoice`, { method: 'POST', + signal, headers: authHeaders(), body: JSON.stringify({ error: reason }), }) @@ } else if (eventType === 'invoice_request') { + const sessionId = sessionIdRef.current + if (!sessionId) { + eventType = '' + continue + } const amountMsat = Number(data.amountMsat) if (!amountMsat || amountMsat <= 0) { @@ try { const pr = await onInvoiceRequestRef.current({ amountMsat, comment: data.comment as string | undefined, }) - if (sessionIdRef.current) { - await postInvoice(sessionIdRef.current, pr) - } + await postInvoice(sessionId, pr, abort.signal) } catch (err) { const reason = err instanceof Error ? err.message : 'Failed to create invoice' consoleError(err, 'Failed to handle invoice request') - if (sessionIdRef.current) { - await postError(sessionIdRef.current, reason) - } + await postError(sessionId, reason, abort.signal) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useLnurlSession.ts` around lines 131 - 151, In the invoice_request branch of useLnurlSession (around the onInvoiceRequestRef.current call), capture sessionIdRef.current into a local const (e.g. const sessionId = sessionIdRef.current) before awaiting onInvoiceRequestRef.current so subsequent awaits can't race into a new session; then use that sessionId for both postInvoice and postError calls instead of reading sessionIdRef.current again, and pass the abort.signal into postInvoice and postError so in-flight reply POSTs are cancelled when teardown occurs.
132-136:⚠️ Potential issue | 🟡 MinorReply on invalid
amountMsatinstead of silently dropping the request.At Line 133-136, malformed requests are only logged and ignored, which leaves payer-side flow waiting for timeout. Send an immediate error response for invalid amounts (using the same invoice endpoint error payload path) before continuing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useLnurlSession.ts` around lines 132 - 136, The code currently logs invalid amountMsat and skips the event, leaving the payer waiting; instead, when Number(data.amountMsat) is falsy or <= 0 you should build and send the same invoice-endpoint error payload back to the requester before continuing: detect the invalid amountMsat (the check around amountMsat in useLnurlSession.ts), construct the standard invoice error response used elsewhere in this hook (reuse the same invoice endpoint error payload structure/path used for successful errors), send it via the same send/response mechanism the hook uses for invoice responses (instead of only calling consoleError), then clear eventType and continue as before so the payer receives an immediate error. Ensure you reference amountMsat, consoleError, and the invoice response sender used in this file when implementing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/hooks/useLnurlSession.ts`:
- Around line 131-151: In the invoice_request branch of useLnurlSession (around
the onInvoiceRequestRef.current call), capture sessionIdRef.current into a local
const (e.g. const sessionId = sessionIdRef.current) before awaiting
onInvoiceRequestRef.current so subsequent awaits can't race into a new session;
then use that sessionId for both postInvoice and postError calls instead of
reading sessionIdRef.current again, and pass the abort.signal into postInvoice
and postError so in-flight reply POSTs are cancelled when teardown occurs.
- Around line 132-136: The code currently logs invalid amountMsat and skips the
event, leaving the payer waiting; instead, when Number(data.amountMsat) is falsy
or <= 0 you should build and send the same invoice-endpoint error payload back
to the requester before continuing: detect the invalid amountMsat (the check
around amountMsat in useLnurlSession.ts), construct the standard invoice error
response used elsewhere in this hook (reuse the same invoice endpoint error
payload structure/path used for successful errors), send it via the same
send/response mechanism the hook uses for invoice responses (instead of only
calling consoleError), then clear eventType and continue as before so the payer
receives an immediate error. Ensure you reference amountMsat, consoleError, and
the invoice response sender used in this file when implementing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba3f4c04-044c-4b95-9bc0-cfdda3880f05
📒 Files selected for processing (1)
src/hooks/useLnurlSession.ts
- Capture sessionId in local const before awaiting to prevent race between concurrent events - Pass abort signal to postInvoice/postError so in-flight POSTs cancel on teardown - Send error back to server on invalid amountMsat instead of silently dropping (which left payer waiting until timeout)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/hooks/useLnurlSession.ts (1)
135-138:⚠️ Potential issue | 🟡 MinorAvoid
breakhere; it can drop remaining buffered SSE lines.When
sessionIdis missing,breakexits the current line-processing loop and skips any remaining lines in the chunk. Prefer resettingeventTypeandcontinue.💡 Suggested fix
} else if (eventType === 'invoice_request') { const sessionId = sessionIdRef.current - if (!sessionId) break + if (!sessionId) { + eventType = '' + continue + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useLnurlSession.ts` around lines 135 - 138, In useLnurlSession's SSE line-processing block (the branch handling eventType === 'invoice_request'), don't use break when sessionIdRef.current is missing because that exits the per-chunk loop and drops remaining buffered lines; instead reset eventType (e.g., set eventType = null or undefined) and continue the loop so the remaining lines in the chunk are still processed; update the invoice_request branch that references sessionIdRef and eventType to perform this reset+continue flow rather than a break.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useLnurlSession.ts`:
- Around line 66-75: The postError function currently ignores non-2xx HTTP
responses; update postError (in useLnurlSession.ts) to mirror postInvoice
behavior by checking the fetch response.ok after awaiting
fetch(`${lnurlServerBaseUrl}/lnurl/session/${sessionId}/invoice`, ...) and
treating non-ok responses as failures (throw or return an error/false and
include response status/text in the log/error). Ensure you still catch network
exceptions in the existing catch block and surface server-side errors the same
way postInvoice does so callers can detect and handle rejected error
notifications.
---
Duplicate comments:
In `@src/hooks/useLnurlSession.ts`:
- Around line 135-138: In useLnurlSession's SSE line-processing block (the
branch handling eventType === 'invoice_request'), don't use break when
sessionIdRef.current is missing because that exits the per-chunk loop and drops
remaining buffered lines; instead reset eventType (e.g., set eventType = null or
undefined) and continue the loop so the remaining lines in the chunk are still
processed; update the invoice_request branch that references sessionIdRef and
eventType to perform this reset+continue flow rather than a break.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 417586e1-18a5-483b-8d74-ec0d0ec56633
📒 Files selected for processing (1)
src/hooks/useLnurlSession.ts
postError silently ignored non-2xx HTTP responses, unlike postInvoice which throws on failures. Now logs the status and response text when the server rejects the error notification.
|
Followup review (commit Good fix — No protocol or security concerns. |
🔍 Arkana PR Review —
|
1 participant
Summary
VITE_LNURL_SERVER_URLis configured, the wallet opens an SSE session with arklabshq/lnurl-server and displays the resulting LNURL in the QR codelightning=parameter and shown in the expanded address listChanges
src/lib/constants.tsVITE_LNURL_SERVER_URLenv varsrc/hooks/useLnurlSession.tssrc/screens/Wallet/Receive/QrCode.tsxsrc/lib/bip21.tslnurlparam toencodeBip21, skipamount=when 0src/components/ExpandAddresses.tsxConfiguration
Set
VITE_LNURL_SERVER_URLto the URL of a running lnurl-server instance:Test plan
VITE_LNURL_SERVER_URLand verify LNURL appears in QR when skipping amountVITE_LNURL_SERVER_URLset works as before🤖 Generated with Claude Code
Summary by CodeRabbit