add lnurl tests

[bordalix]

Preparation for #559

@Kukks please review

Summary by CodeRabbit

• Tests

  • Added comprehensive end-to-end test coverage for LNURL payment flows, including LNURL validity checks, invoice generation, and payment execution.

• Chores

  • Updated test environment configuration.

[cloudflare-workers-and-pages]

Deploying wallet-mutinynet with    Cloudflare Pages

Latest commit:	8efdc0f
Status:	✅  Deploy successful!
Preview URL:	https://d5d220c4.arkade-wallet.pages.dev
Branch Preview URL:	https://lnurl-tests.arkade-wallet.pages.dev

View logs

[coderabbitai]

Walkthrough

This change adds LNURL server support to the Playwright configuration, improves logging in the LNURL session hook, and introduces a comprehensive end-to-end test suite covering LNURL flows including condition discovery, invoice fetching, and regtest payment validation.

Changes

Cohort / File(s)	Summary
Playwright Configuration playwright.config.ts	Injects VITE_LNURL_SERVER_URL=http://localhost:9090 environment variable into the Vite development server startup command alongside existing relay and delegate configuration variables.
LNURL Session Hook src/hooks/useLnurlSession.ts	Modifies error logging for invalid amountMsat in SSE invoice_request processing to convert the value to a string format before logging.
LNURL E2E Tests src/test/e2e/lnurl.test.ts	Adds new Playwright-based end-to-end test suite validating LNURL flows: LNURL generation and validity, condition discovery API responses, invoice fetching with amount/label parameters, BOLT11 invoice decoding, and regtest payment execution with receipt verification via Docker-based LND transactions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'add lnurl tests' accurately reflects the main change in the changeset, which adds a new test suite for LNURL flows in src/test/e2e/lnurl.test.ts with 115 new lines.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch lnurl_tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cloudflare-workers-and-pages]

Deploying tmp-boltz-upstream-mainnet-arkade-wallet with    Cloudflare Pages

Latest commit:	8efdc0f
Status:	✅  Deploy successful!
Preview URL:	https://a3fab531.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev
Branch Preview URL:	https://lnurl-tests.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev

View logs

[arkanaai]

✅ Approved — LNURL e2e tests

Clean PR. Tests are well-structured and follow the existing test patterns (receive.test.ts, send.test.ts). Not protocol-critical — no changes to VTXOs, signing, forfeit, rounds, or exits.

What's here

• 4 new e2e tests covering the LNURL receive flow: LNURL creation, condition checking, invoice fetching, and full payment roundtrip
• playwright.config.ts: adds VITE_LNURL_SERVER_URL=http://localhost:9090 to the dev server env
• useLnurlSession.ts:146: minor logging fix — logs the coerced amountMsat.toString() instead of raw data.amountMsat. Correct change since the validation operates on the Number()-coerced value

Minor findings (non-blocking)

1. Magic number 1,992 — src/test/e2e/lnurl.test.ts:115: The expected received amount 1,992 (from 2,000 sent) encodes an implicit 8-sat fee assumption. If the fee structure changes, this test will break silently with a confusing error. Consider extracting a constant or adding a comment explaining the expected fee deduction.

2. Duplicate setup — Tests 1-4 repeat the same wallet-creation + LNURL-copy boilerplate (~10 lines each). Could extract a getLnurl(page) helper into utils.ts to keep tests focused on what they're actually testing. Non-blocking since the existing tests (receive.test.ts) follow a similar pattern.

3. No negative cases — No tests for edge cases like amount=0, amount exceeding maxSendable, or malformed LNURLs. Worth adding in a follow-up to exercise the error paths in useLnurlSession.ts and lnurl.ts.

4. Shell interpolation in tests — lnurl.test.ts:108: execAsync(\docker exec lnd lncli ... ${invoice} --force`)interpolates the invoice directly into a shell command. Same pattern asreceive.test.ts:26so it's consistent with the codebase, but worth noting for future hardening (e.g., passing via--or usingexecFile` to avoid accidental metacharacter issues).

LGTM. Ship it.

🤖 Reviewed by Arkana

[arkanaai]

✅ Approved — LNURL e2e tests

Clean PR. Tests are well-structured and follow the existing test patterns (receive.test.ts, send.test.ts). Not protocol-critical — no changes to VTXOs, signing, forfeit, rounds, or exits.

What's here

• 4 new e2e tests covering the LNURL receive flow: LNURL creation, condition checking, invoice fetching, and full payment roundtrip
• playwright.config.ts: adds VITE_LNURL_SERVER_URL=http://localhost:9090 to the dev server env
• useLnurlSession.ts:146: minor logging fix — logs the coerced amountMsat.toString() instead of raw data.amountMsat. Correct change since the validation operates on the Number()-coerced value

Minor findings (non-blocking)

1. Magic number 1,992 — src/test/e2e/lnurl.test.ts:115: The expected received amount 1,992 (from 2,000 sent) encodes an implicit 8-sat fee assumption. If the fee structure changes, this test will break silently with a confusing error. Consider extracting a constant or adding a comment explaining the expected fee deduction.

2. Duplicate setup — Tests 1-4 repeat the same wallet-creation + LNURL-copy boilerplate (~10 lines each). Could extract a getLnurl(page) helper into utils.ts to keep tests focused on what they're actually testing. Non-blocking since the existing tests (receive.test.ts) follow a similar pattern.

3. No negative cases — No tests for edge cases like amount=0, amount exceeding maxSendable, or malformed LNURLs. Worth adding in a follow-up to exercise the error paths in useLnurlSession.ts and lnurl.ts.

4. Shell interpolation in tests — lnurl.test.ts:108: execAsync(`docker exec lnd lncli ... ${invoice} --force`) interpolates the invoice directly into a shell command. Same pattern as receive.test.ts:26 so it's consistent with the codebase, but worth noting for future hardening (e.g., passing via -- or using execFile to avoid accidental metacharacter issues).

LGTM. Ship it.

🤖 Reviewed by Arkana

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/hooks/useLnurlSession.ts (1)

144-150: ⚠️ Potential issue | 🟡 Minor

Log the raw payload value, not the coerced number.

amountMsat here is the already-coerced Number(data.amountMsat), so when the payload is missing/invalid this logs "NaN" or "0" rather than what the server actually sent. Logging data.amountMsat (via String(...), as the summary describes) is more useful for debugging malformed payloads.

🔧 Suggested change

-                  consoleError('Invalid amountMsat in invoice request:', amountMsat.toString())
+                  consoleError('Invalid amountMsat in invoice request:', String(data.amountMsat))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useLnurlSession.ts` around lines 144 - 150, The log currently
prints the coerced Number amountMsat which becomes "NaN" or "0" for invalid
payloads; change the consoleError call in useLnurlSession's invoice handling to
log the raw payload value instead by converting data.amountMsat to a string
(e.g. String(data.amountMsat)) so you see exactly what the server sent; keep the
rest of the flow the same (postError(sessionId, 'Invalid amount', abort.signal),
clearing eventType and continue) and only adjust the consoleError argument.

🧹 Nitpick comments (3)

src/test/e2e/lnurl.test.ts (3)

106-106: Hard dependency on a running lnd docker container — document and/or guard.

docker exec lnd lncli ... will fail with a confusing error if the container isn't running or is named differently in a contributor's environment. Consider:

• Documenting the required regtest setup (container name lnd, network regtest) in the test file header or a CONTRIBUTING note.
• Optionally test.skip-ing this case when the container is unavailable (e.g. a quick docker ps pre-check) so local runs of the other three tests don't fail spuriously.

Also worth noting: the invoice string is interpolated directly into a shell command. It's bech32 so practically safe, but wrapping it in quotes ("${invoice}") is a cheap hardening step.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/e2e/lnurl.test.ts` at line 106, The e2e test in
src/test/e2e/lnurl.test.ts has a hard dependency on a running Docker container
named "lnd" and interpolates invoice directly into a shell command; update the
test to (1) document the requirement (container name "lnd", network "regtest")
in the file header or CONTRIBUTING, and (2) guard the failing call by adding a
pre-check using docker ps (or docker inspect) to verify the "lnd" container is
running and, if not, call test.skip for this specific test so other tests can
run; also harden the execInvoke by wrapping the invoice variable in quotes when
calling execAsync (the invocation using execAsync(`docker exec lnd lncli
--network=regtest payinvoice ${invoice} --force`)).

---

9-115: Extract the shared setup into a helper/beforeEach.

All four tests repeat the same ~10 lines: createWallet, navigate to Receive, click Copy + lnurl-address-copy, read clipboard, then the same three expects on the LNURL. Consider extracting a getLnurlFromReceivePage(page) helper (or a beforeEach that exposes the lnurl via a fixture) so the tests only contain the behavior unique to each case.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/e2e/lnurl.test.ts` around lines 9 - 115, The tests duplicate the
same setup steps to create a wallet and extract/validate the LNURL; factor these
into a helper (e.g., getLnurlFromReceivePage(page)) or a beforeEach that returns
the lnurl so each test only asserts unique behavior; implement
getLnurlFromReceivePage to call createWallet, click the wallet tab
(getByTestId('tab-wallet')), click 'Receive' (getByText('Receive')), click
'Copy' and the lnurl copy button (getByTestId('lnurl-address-copy')),
readClipboard(), and perform the three common expects (isValidLnUrl, contains
'LNURL', length > 100) before returning the lnurl, then replace the repeated
block in each test with a single call to that helper (or use beforeEach to set a
fixture variable).

---

18-19: Use a more specific locator than getByText('Copy').

The test uses await page.getByText('Copy').click() without exact: true, which uses substring matching and could match both the "Copy" button and the "Copy address" modal header if timing causes both to be visible. Add a data-testid attribute (e.g., lnurl-copy-trigger) to the Copy button for a stable, unambiguous selector.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/e2e/lnurl.test.ts` around lines 18 - 19, The test is using a fuzzy
text locator await page.getByText('Copy').click() which can match multiple
elements; add a stable data-testid like lnurl-copy-trigger to the actual Copy
button in the UI and change the test to use that specific locator (replace the
getByText call with getByTestId('lnurl-copy-trigger').click) so it reliably
targets the Copy button before the existing await
page.getByTestId('lnurl-address-copy').click() call.

🤖 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/test/e2e/lnurl.test.ts`:
- Line 114: The assertion uses a brittle exact string match via
expect(page.getByText('1,992', { exact: true })).toBeVisible() which will break
if the fee schedule changes; update the test to either (a) use a regex/text
match like expect(page.getByText(/1,9\d{2}/)).toBeVisible() or (b) read the
element text (via page.getByText or page.locator(...).innerText()) and compare
numerically against the known fee constant (e.g.,
parseInt(received.replace(/[,]/g, '')) <= MAX_AMOUNT && > MIN_AMOUNT or compute
expected = BASE - FEE_CONST and assert equality), referencing the page.getByText
call and the surrounding test in lnurl.test.ts to find where to replace the
exact match.

---

Outside diff comments:
In `@src/hooks/useLnurlSession.ts`:
- Around line 144-150: The log currently prints the coerced Number amountMsat
which becomes "NaN" or "0" for invalid payloads; change the consoleError call in
useLnurlSession's invoice handling to log the raw payload value instead by
converting data.amountMsat to a string (e.g. String(data.amountMsat)) so you see
exactly what the server sent; keep the rest of the flow the same
(postError(sessionId, 'Invalid amount', abort.signal), clearing eventType and
continue) and only adjust the consoleError argument.

---

Nitpick comments:
In `@src/test/e2e/lnurl.test.ts`:
- Line 106: The e2e test in src/test/e2e/lnurl.test.ts has a hard dependency on
a running Docker container named "lnd" and interpolates invoice directly into a
shell command; update the test to (1) document the requirement (container name
"lnd", network "regtest") in the file header or CONTRIBUTING, and (2) guard the
failing call by adding a pre-check using docker ps (or docker inspect) to verify
the "lnd" container is running and, if not, call test.skip for this specific
test so other tests can run; also harden the execInvoke by wrapping the invoice
variable in quotes when calling execAsync (the invocation using
execAsync(`docker exec lnd lncli --network=regtest payinvoice ${invoice}
--force`)).
- Around line 9-115: The tests duplicate the same setup steps to create a wallet
and extract/validate the LNURL; factor these into a helper (e.g.,
getLnurlFromReceivePage(page)) or a beforeEach that returns the lnurl so each
test only asserts unique behavior; implement getLnurlFromReceivePage to call
createWallet, click the wallet tab (getByTestId('tab-wallet')), click 'Receive'
(getByText('Receive')), click 'Copy' and the lnurl copy button
(getByTestId('lnurl-address-copy')), readClipboard(), and perform the three
common expects (isValidLnUrl, contains 'LNURL', length > 100) before returning
the lnurl, then replace the repeated block in each test with a single call to
that helper (or use beforeEach to set a fixture variable).
- Around line 18-19: The test is using a fuzzy text locator await
page.getByText('Copy').click() which can match multiple elements; add a stable
data-testid like lnurl-copy-trigger to the actual Copy button in the UI and
change the test to use that specific locator (replace the getByText call with
getByTestId('lnurl-copy-trigger').click) so it reliably targets the Copy button
before the existing await page.getByTestId('lnurl-address-copy').click() call.

🪄 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: 95958233-5e88-43fe-b232-c56e6d508c88

📥 Commits

Reviewing files that changed from the base of the PR and between 2cc1b3b and 8efdc0f.

📒 Files selected for processing (3)

• playwright.config.ts
• src/hooks/useLnurlSession.ts
• src/test/e2e/lnurl.test.ts

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Brittle exact-amount assertion.

Asserting '1,992' hard-codes the current swap/service fee (2000 − 8). Any fee schedule change will break this test with a confusing "text not visible" failure. Consider asserting a range (e.g. toMatch(/1,9\d{2}/) or reading the element and checking received <= 2000 && received > 1900) or deriving the expected amount from the known fee constant.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/e2e/lnurl.test.ts` at line 114, The assertion uses a brittle exact
string match via expect(page.getByText('1,992', { exact: true })).toBeVisible()
which will break if the fee schedule changes; update the test to either (a) use
a regex/text match like expect(page.getByText(/1,9\d{2}/)).toBeVisible() or (b)
read the element text (via page.getByText or page.locator(...).innerText()) and
compare numerically against the known fee constant (e.g.,
parseInt(received.replace(/[,]/g, '')) <= MAX_AMOUNT && > MIN_AMOUNT or compute
expected = BASE - FEE_CONST and assert equality), referencing the page.getByText
call and the surrounding test in lnurl.test.ts to find where to replace the
exact match.