feat(spark): LNURL pay and Lightning Address support for self-custodial sends

[esaugomez31]

Spark: LNURL pay and Lightning Address support for self-custodial sends

What this PR does

Adds first-class support for paying to LNURL-pay endpoints and Lightning Addresses (user@domain.com) from a self-custodial Spark wallet. The custodial flow already handled these via the lnurl-pay library + lnInvoicePaymentSend mutation, but in self-custodial mode the destination resolver was incorrectly routing both formats to the Bolt11 path (which the SDK can't decode), so the prepare step always failed. This PR routes LNURL through the SDK's native prepareLnurlPay / lnurlPay methods, mapping the destination shape from the lnurl-pay library to the SDK contract, and propagates the SDK-returned SuccessAction back to the completed screen.

Bridge — new SDK wrappers

Three small wrappers added to app/self-custodial/bridge/send.ts next to the existing prepareSend / executeSend helpers, following the same pattern (one wrapper per SDK method, plus an extract helper for the fee):

• prepareLnurl(sdk, options) wraps sdk.prepareLnurlPay. The PrepareLnurlOptions type only exposes the fields the app actually uses today (amount, payRequest, optional comment, tokenIdentifier, conversionOptions, feePolicy). The SDK's other knobs (validateSuccessActionUrl, etc.) are left at default.
• executeLnurl(sdk, prepared, idempotencyKey?) wraps sdk.lnurlPay.
• extractLnurlFee(prepared) returns feeSats as a plain number, mirroring extractLightningFee for symmetry at call sites.

Payment-details — new self-custodial LNURL detail

New file app/self-custodial/payment-details/lnurl.ts exporting createSelfCustodialLnurlPaymentDetails. It produces a PaymentDetail<T> that satisfies the shared screen contract (with paymentType: PaymentType.Lnurl, lnurlParams, setInvoice, setSuccessAction, isMerchant) and internally talks to the SDK via the new bridge wrappers.

The piece that took the longest to get right is the prepare-options shape, because the SDK enforces a specific combination depending on the source asset:

Source wallet	amount units	tokenIdentifier	conversionOptions	feePolicy
BTC	sats	undefined	undefined	undefined
USD (USDB)	USDB base units (cents × 10⁴)	configured USDB id	ToBitcoin	FeesIncluded

For USD wallets the SDK rejects any other feePolicy with "Token conversion with token_identifier requires FeesIncluded fee policy", so this is mandatory. The behavioral consequence is documented under "Fee semantics" below.

A small helper lnurlParamsToPayRequest converts LnUrlPayServiceResponse (sats + parsed metadata array, from the lnurl-pay lib used by the existing custodial parser) into LnurlPayRequestDetails (millisats + raw metadata string, the SDK shape). The metadata string is taken verbatim from lnurlParams.rawData.metadata when present so the LUD-06 description-hash check inside the SDK matches the server's original payload byte-for-byte; if rawData.metadata isn't a string it falls back to JSON.stringify(lnurlParams.metadata) which is good enough for endpoints that don't expose the raw form.

SuccessActionProcessed returned by lnurlPay (the SDK has already deciphered AES payloads server-side) is mapped back to the LNURLPaySuccessAction shape that lnurl-pay exposes, so the existing completed-screen code paths render it without changes. AES decipher(preimage) returns the SDK-supplied plaintext directly since decryption already happened.

Routing — wrap-destination.ts

The Lnurl branch is split off from Lightning. Previously both shared buildLightningDetail, which fed destination.lnurl (a bech32 LNURL string or LN Address) into createSelfCustodialLightningPaymentDetails as the paymentRequest — the SDK can't decode that, so the very first prepareSend call failed with InvalidInput. The new buildLnurlDetail calls createSelfCustodialLnurlPaymentDetails with lnurl, lnurlParams, isMerchant, and seeds unitOfAccountAmount from lnurlParams.min (minimum sendable in sats) so the screen can render an initial value.

The LnurlDestination Extract type is narrowed by lnurlParams: LnUrlPayServiceResponse to exclude LnurlWithdrawDestination (Receive direction), keeping TypeScript happy on the runtime guard already enforced by wrapDestinationForSC.

Screen integrations

Send-bitcoin destination screen

A single-line gate: when self-custodial, pass lnurlDomains: [] to parseDestination so Blink-owned LN addresses (user@blink.sv) are not silently optimized into the Intraledger payment type. That optimization is custodial-specific (the destination becomes an internal-ledger transfer that requires backend auth, which the self-custodial wallet does not have); without this gate paying to a Blink LN Address from a self-custodial wallet would fail with Not authorized after pressing Next. With the gate, the same input flows through the regular LNURL path and resolves through the new SC LNURL detail.

Send-bitcoin details screen

The existing custodial flow runs requestInvoice(...) from the lnurl-pay library on Next to materialize a Bolt11 invoice before showing the confirmation screen. Self-custodial doesn't need that step because the SDK fetches the invoice internally as part of prepareLnurlPay. One-line gate: only run the requestInvoice block when paymentDetail.sendPaymentMutation is undefined (custodial LNURL details start with canSendPayment: false until setInvoice is called; self-custodial LNURL details have a working sendPaymentMutation from the moment the user picks an amount).

Send-bitcoin confirmation screen

Two changes:

1. PaymentSendExtraInfo now carries an optional successAction. When the SC LNURL sendPaymentMutation resolves, it includes the SDK's successAction in extraInfo (mapped to the lnurl-pay shape). The confirmation screen prefers extraInfo?.successAction ?? paymentDetail?.successAction when navigating to the completed screen, so the post-payment message/URL/AES disclosure shows up for both flows. Custodial keeps populating paymentDetail.successAction ahead of sendPayment, so the fallback covers it.

2. saveLnAddressContact is skipped when useActiveWallet().isSelfCustodial is true. That helper hits the contactCreate GraphQL mutation, which requires backend auth; without the gate a successful self-custodial LNURL payment was followed by a visible Not authorized error toast even though the payment itself had completed. Custodial users keep the existing behavior.

Fee semantics

For the BTC self-custodial wallet, feePolicy is left as the SDK default (FeesExcluded): the recipient receives the exact sats amount the user picked and the routing fee is added on top. This matches the custodial Lightning behavior.

For the USD self-custodial wallet, the SDK requires FeesIncluded whenever a tokenIdentifier + conversionOptions combination is used, so the user spends an exact USDB amount and the recipient receives the post-conversion, post-routing-fee BTC value. In practical terms the recipient gets ~1% less than the headline amount; that gap is the Spark conversion-pool spread (invisible in the UI) plus a few sats of LN routing (visible). This is a hard SDK constraint, not a design choice — a manual two-step (convert USDB → BTC first, then pay LNURL with BTC) could match custodial semantics but would require a separate transaction and rollback handling, which is out of scope for this PR.

Tests

All specs scoped to the affected files; full suite passes (3438/3438).

• __tests__/self-custodial/bridge/send.spec.ts — added 14 cases covering prepareLnurl (forwarding amount/payRequest/comment, USD-wallet shape with tokenIdentifier+conversionOptions+feePolicy, BTC-wallet shape with neither), extractLnurlFee (number coercion + zero), executeLnurl (idempotency key forwarding).
• __tests__/self-custodial/payment-details/lnurl.spec.ts — new, 22 cases covering payment type, amount handling (fixed via min===max vs range, setAmount), memo handling (description vs sender memo, setMemo), the per-currency prepareOptions matrix, comment gating by commentAllowed, getFee success/failure, sendPaymentMutation success with all three SuccessAction variants (Message, URL, AES decipher passthrough), error classification, raw metadata preservation, and millisat conversion.
• __tests__/self-custodial/payment-details/wrap-destination.spec.ts — updated the existing Lnurl case to assert routing through the new SC LNURL detail (not the lightning detail), plus a merchant-flag propagation case.
• __tests__/screens/send-confirmation.spec.tsx — added a case that mocks useActiveWallet to return { isSelfCustodial: true } and asserts saveLnAddressContact is not called after a successful LNURL payment.

Breaking notes

None for consumers. The shared PaymentDetail types gain an optional successAction field on PaymentSendExtraInfo. All in-tree senders are unaffected because the field is optional and the confirmation screen uses ?? fallback.

Manual verification done

• Lightning Address from a USD wallet: deepbassoon958@walletofsatoshi.com for $1 — pays correctly, recipient receives the converted amount, fee shown ($0.00 / 5 SAT), completed screen renders with the description.
• Lightning Address from a BTC wallet (500 sats) — pays correctly, recipient receives the exact 500 sats, fee charged on top.
• Bech32 LNURL from esaudeveloper@blink.sv — pays correctly via the new SC LNURL path (Intraledger optimization disabled in SC mode).

The original implementation iterations and the SDK's exact tokenIdentifier / conversionOptions / feePolicy requirements are documented at length in commit messages and tests.

[esaugomez31]

• feat(spark): UI/UX launch polish across settings and flows #3769
• feat(spark): production rollout and SDK hardening  #3768
• feat(spark): LNURL pay and Lightning Address support for self-custodial sends #3765 👈 (View in Graphite)
• feat(spark): stable-sats receive auto-convert and send-flow fixes #3764
• fix(spark): address settings + onboarding UI/UX issues from  #3762
• feat(spark): add Stable Balance and BTC - USD conversion flow #3761
• feat(spark): self-custodial payments and transaction history  #3758
• feat(spark): backup, recovery and safety nudges  #3755
• feat(spark): self-custodial onboarding and wallet creation #3747
• feat(spark): shared wallet abstractions and provider foundation #3746
• feat(spark): account migration to non-custodial UI screens #3742
• feat(spark): spark onboarding backup screens UI #3731
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[grimen]

Review — bugs + test coverage

Important

This PR (and the stack it sits in) needs a structural cleanup pass. Findings deferred from this round:

• Adapter-pattern leakage — isSelfCustodial is special-cased at three different screen call sites in this PR alone (send-bitcoin-destination-screen.tsx:373 for the lnurlDomains gate, send-bitcoin-details-screen.tsx:365 for the requestInvoice skip, send-bitcoin-confirmation-screen.tsx for the saveLnAddressContact skip). The shape is "if SC, opt out of the custodial behaviour" rather than going through a uniform Port. This is the same leak the parent stack's adapter pattern was meant to eliminate.
• SOLID violations — lnurl.ts and the existing lightning.ts factory both produce PaymentDetail<T> but share zero code: each has its own error classifier path, its own prepareOptions builder, its own currency-conversion logic, its own success-action mapping. Bridge wrappers (prepareLnurl / executeLnurl / extractLnurlFee) mirror the lightning ones structurally but live as duplicated functions. PaymentSendExtraInfo (shared custodial+SC) now carries an optional successAction — a cross-cutting concern bleeding into a shared type.
• High cyclomatic / closure complexity — payment-details/lnurl.ts is one ~270 LOC file holding lnurlParamsToPayRequest, extractMetadataStr, sdkSuccessActionToLib (4 variants), centsToTokenBaseUnits, the BTC-vs-USD prepareOptions branch, createGetFee (try/catch + classifier), sendPaymentMutation (try/catch + classifier + extraInfo build), plus four set* immutable rebuilders that each close over the entire factory call. Three screens gain another isSelfCustodial branch on top of the parent stack's already-noted 60+ mode branches across 11 files.
• Code smells — as unknown as WalletAmount<T> (I7); decipher: () => decrypted.plaintext is a callback designed never to be called (combined with #1, this is the bug); hard-coded centsToTokenBaseUnits(cents, 6, 2) magic numbers (USDB decimals + cents decimals) inline in the factory rather than behind named constants; executeLnurl(..., idempotencyKey?) ships a parameter the only caller never passes (Critical #2 — the function signature implies a contract that isn't enforced).
• Naming smells — inconsistent bridge verb pattern: prepareSend / executeSend (verb+direction) vs new prepareLnurl / executeLnurl (verb+method); extractLightningFee and extractLnurlFee exist as separate functions despite returning identical feeSats: number; lnurlParamsToPayRequest reflects neither source (LnUrlPayServiceResponse) nor destination (LnurlPayRequestDetails) type name. Separately on the shared PaymentDetail shape — having a uniform shape is correct (adapter mindset), but field names still carry custodial-flow vocabulary: sendPaymentMutation (Apollo's useMutation return type — for SC it's just a closure, not a mutation); setInvoice (custodial pre-confirmation step where a BOLT11 invoice gets materialised — SC implements as a no-op solely to fit the contract). Rename to mode-neutral verbs (sendPayment, setResolvedRequest?), or model setInvoice as optional rather than mandating a no-op. Keep the shape shared.

Why this is not in scope for this PR: #3765 is part of the same feat--spark-* stack as #3758 (bases on feat--spark-stable-receive-and-send-fixes, sits in the #3761–#3769 range). Doing the refactor here would force the same multi-day restack across the downstream PRs that #3758's review deferred. Refactoring against main after the stack settles is materially cheaper.

Decision: this review covers correctness + test coverage only. The structural / SOLID / complexity / naming review folds into the same cleanup ticket already opened against #3758 — adapter-pattern unification milestone, then renames, landed as a sequence of small focused PRs against main. Production rollout (#3768) should remain gated on the cleanup ticket reaching the adapter-unification milestone, otherwise the duplication ships and gets normalised.

---

Real strengths first: bridge-layer wrapper symmetry — prepareLnurl / executeLnurl / extractLnurlFee (bridge/send.ts) mirror the existing prepareSend / executeSend / extractLightningFee 1:1; lnurlParamsToPayRequest (payment-details/lnurl.ts:39-43) preserves rawData.metadata verbatim with a sound typeof === "string" guard so the SDK's LUD-06 description-hash check matches the server bytes; the BTC-vs-USD prepareOptions matrix is a single ?: on isUsdSend (no fall-through, no swap); the lnurlDomains: [] gate (send-bitcoin-destination-screen.tsx:373) correctly disables the intraledger optimisation only when isSelfCustodial; the lnurl.spec.ts 22-case spec is genuinely not padded — each test asserts a distinct branch (BTC vs USD shape, sat→millisat math, raw-vs-stringified metadata, comment matrix, fixed-vs-range amount, three SuccessAction variants, error classification). Two manual flows verified end-to-end (LN address from USD wallet + bech32 LNURL from @blink.sv) and 3438/3438 passing.

Severity:

• Critical — fund-loss, mis-attribution, or silent failure that misleads the user. Must fix before merge.
• Important — correctness/reliability that doesn't lose funds but degrades UX or hides incidents.
• Test — coverage gap or test that passes while a confirmed bug is alive.

---

Critical

1. AES success-action plaintext silently dropped on completed screen (SC only)

app/self-custodial/payment-details/lnurl.ts:91-101 + consumer app/screens/send-bitcoin-screen/send-bitcoin-completed-screen.tsx:75-92

The SC mapping stuffs the SDK-provided plaintext into the lnurl-pay decipher() callback while leaving ciphertext: null, iv: null. But useSuccessMessage does not call successAction.decipher(preimage) — it calls utils.decipherAES({ successAction, preimage }) from lnurl-pay, which short-circuits to null whenever ciphertext or iv is missing (verified at node_modules/lnurl-pay/dist/cjs/index.js:290-301). For SC AES success actions, decryptedMessage is therefore always null and textContent reduces to the public description — the actual decrypted secret / coupon / voucher payload (the whole point of the AES variant) is never shown to the user. Custodial users see it. Also: SC sendPaymentMutation doesn't propagate preimage in extraInfo, so even a future fix that called decipher() here would have nothing to pass it.

Fix: put the SDK plaintext into a real field (e.g. message) so the existing [message, description, decryptedMessage].filter(Boolean).join(" ") join captures it. Don't rely on a decipher() callback that is never invoked.

2. Idempotency key is dead at the only caller

app/self-custodial/payment-details/lnurl.ts:189 + app/self-custodial/bridge/send.ts (executeLnurl)

executeLnurl(sdk, prepared) is called with two args — the third positional idempotencyKey is always undefined. The bridge wrapper does forward an idempotency key when given one, and the bridge spec covers that forwarding correctly, so the surface looks supported and tested. But the only production caller never passes one. A network-flake retry that re-runs sendPaymentMutation against the same prepared response is therefore protected only by recipient-side replay logic. Compare the existing custodial lightning send for the expected pattern.

Fix: thread an idempotency key (paymentRequest hash, or a UUID held alongside paymentDetail) into createSelfCustodialLnurlPaymentDetails and pass it to executeLnurl. If intentionally deferred, comment the dead parameter and open a tracking issue — right now both the parameter and the test suggest it works.

---

Important

#	Item	Location
I1	sendPaymentMutation error classification — only LnurlError → InvalidInput is exercised; NetworkError and generic branches in the classifier are not asserted	app/self-custodial/payment-details/lnurl.ts:198-206
I2	AES ErrorStatus branch in sdkSuccessActionToLib exists but is never exercised; a typo like result.inner.message instead of .reason would crash the success screen on real-world AES failures	app/self-custodial/payment-details/lnurl.ts:104-117
I3	extractLnurlFee returns 0 for NaN/undefined via toNumber. Acceptable for the USD FeesIncluded path where fee genuinely is 0, but masks a SDK-side regression that would render "0 sats fee" instead of erroring	app/self-custodial/bridge/send.ts (extractLnurlFee)
I4	unitOfAccountAmount: toBtcMoneyAmount(destination.lnurlParams.min || 0) — works today, but a refactor to ?? semantics on a malformed min: undefined is silently absorbed; no test covers that input	app/self-custodial/payment-details/wrap-destination.ts:80
I5	commentAllowed > 0 && memo === "" correctly yields comment: undefined in production but is not exercised by tests; only the two extremes (commentAllowed: 0 + memo, commentAllowed: 200 + memo) are covered	app/self-custodial/payment-details/lnurl.ts:163
I6	SC sendPaymentMutation returns { status, extraInfo } only — no transaction or preimage. Completed screen lacks createdAt (no time field) and preimage (also blocks any future fix to #1 that wants to re-decrypt locally)	app/self-custodial/payment-details/lnurl.ts (return shape of sendPaymentMutation)
I7	as unknown as WalletAmount<T> in asBtcSettlementAmount. For USD wallet, fee comes back currency: Btc; downstream convertMoneyAmount dispatches on the runtime currency field so there's no miscalculation, but the type lie is a foot-gun for future readers	app/self-custodial/payment-details/lnurl.ts
I8	LnurlDestination Extract narrows on lnurlParams: LnUrlPayServiceResponse to exclude withdraw shapes — correct today, but the runtime guard at wrapDestinationForSC only checks direction. If upstream parsing ever returns a withdraw-shaped lnurlParams with paymentType: Lnurl + direction: Send, the factory reads min/max off a shape that doesn't have them	app/self-custodial/payment-details/wrap-destination.ts

---

Test coverage

Estimated behavioural coverage of the new LNURL surface: ~75%, strong on the bridge layer and lnurl.ts factory, sparse on the three screen edits (only one screen test added across them).

Tier 1 — production code with no test file (or no coverage of the new branch)

File	LOC delta	Notes
app/screens/send-bitcoin-screen/send-bitcoin-destination-screen.tsx:373	1-line gate	The lnurlDomains: isSelfCustodial ? [] : [lnAddressHostname, ...LNURL_DOMAINS] branch. No screen test asserts what parseDestination is called with. Regression silently misroutes Blink LN addresses (custodial loses intraledger fast path; SC re-introduces the original Not authorized failure)
app/screens/send-bitcoin-screen/send-bitcoin-details-screen.tsx:365	1-line gate	The paymentDetail.paymentType === "lnurl" && !paymentDetail.sendPaymentMutation skip gate. No screen test on either side. Regression either runs requestInvoice against the SC SDK or skips it for custodial — both visible to users on the happy path
app/screens/send-bitcoin-screen/send-bitcoin-confirmation-screen.tsx:212	1-line ??	The extraInfo?.successAction ?? paymentDetail?.successAction precedence. The single screen test only covers saveLnAddressContact skip; no assertion on what successAction is forwarded to navigation. Reversed ?? operands silently break the most user-visible LNURL feature in SC

Tier 2 — tests that pass while a Critical bug is alive

Test file	What it should catch but doesn't	Bug
__tests__/self-custodial/payment-details/lnurl.spec.ts (AES case)	Asserts decipher(preimage) === plaintext at the factory layer, but does not exercise the consumer (useSuccessMessage) which actually ignores decipher. The bug manifests at the consumer; the unit test is structurally blind to it	#1
__tests__/self-custodial/bridge/send.spec.ts (idempotency forwarding)	Asserts the bridge wrapper forwards a key when given one. The bug is at the caller in lnurl.ts:189 which never passes one. No integration-level test catches the dead parameter	#2
__tests__/self-custodial/payment-details/lnurl.spec.ts (error classification)	Only tests LnurlError tag. Any classifier shape regression on NetworkError / generic branches passes silently	I1
__tests__/self-custodial/payment-details/lnurl.spec.ts (AES ErrorStatus)	Only the result.tag === "Decrypted" branch is exercised; ErrorStatus is dead from the suite's perspective despite being mapped in production	I2

Tier 3 — Critical bugs with no test coverage anywhere

Critical	Recommended test
#1	End-to-end test rendering send-bitcoin-completed-screen with an SC AES success action; assert the decrypted plaintext appears in textContent, not just the description
#2	lnurl.ts factory test: spy on executeLnurl, assert it was called with a defined idempotency key. The test should fail today — write it, then thread the key in or document the deferral
Screen gates (×3)	One screen test per gate (destination lnurlDomains, details requestInvoice skip, confirmation successAction precedence) — assert both the SC-true and SC-false branches

---

Suggested order of operations

1. Fix #1 by populating message (or the equivalent rendered field) directly with the SDK plaintext. Add a Tier 3 regression test driving useSuccessMessage end-to-end on an SC AES action.
2. Decide on #2 — thread the idempotency key in (preferred; the bridge already supports it) or comment the dead parameter and open a tracking issue. Add the Tier 3 spy test in either case.
3. Add the three screen-level tests (Tier 1) — small surface, three independent regressions, highest leverage tests in this PR.
4. Round out the lnurl spec matrices — NetworkError + generic branches (I1), AES ErrorStatus (I2), commentAllowed > 0 + empty memo (I5). All small.
5. Important items I3–I8 are minor; fold into the same commits as the above or leave for the post-stack cleanup pass.

[esaugomez31]

@grimen

Status: Both Criticals + 6 Importants (I1, I2, I4, I5, I6, I7) + all 3 Tier 1 screen gates landed and green. I3 + I8 deferred to the post-stack cleanup pass — runtime guards reverted to keep extractLnurlFee and wrapDestination SRP-clean (the SDK type system already enforces both shapes), but regression tests are kept so any future drift of toNumber or parseDestination surfaces. Full suite: 3855/3855 green, tsc clean, eslint 0 errors. Restack to rollout + polish next.

Scope acknowledgement: structural items (adapter-pattern leakage across the three screen call sites, the duplicated factory + bridge wrappers between lnurl.ts / lightning.ts, centsToTokenBaseUnits(cents, 6, 2) magic numbers, naming asymmetries prepareSend/prepareLnurl + extractLightningFee/extractLnurlFee + lnurlParamsToPayRequest, sendPaymentMutation/setInvoice custodial-vocabulary on the shared shape) — all out of scope for this PR; rolling into the same cleanup ticket against main after the stack settles, and #3768 production rollout stays gated on the adapter-unification milestone. The existing strengths (bridge wrapper symmetry, raw-metadata preservation for LUD-06, BTC-vs-USD prepareOptions single-?: matrix, lnurlDomains gate correctness, the 22-case spec breadth, both manual flows verified) are all preserved in this revision — the changes here only add coverage and tighten the AES + idempotency contracts; none of those strengths regressed.

Critical

1. AES success-action plaintext silently dropped on completed screen (SC only)

sdkSuccessActionToLib now writes decrypted.plaintext into the message field for the AES Decrypted branch (instead of the decipher callback). The consumer chain — useSuccessMessage at send-bitcoin-completed-screen.tsx:75-92 — is unchanged: it still joins [message, description, decryptedMessage], so plaintext is now picked up by the same path that already renders LUD-09 messages. utils.decipherAES remains short-circuited for SC because we still send ciphertext: null, iv: null (the SDK delivered already-decrypted bytes; we don't re-encrypt them just to satisfy the lib's path), and decipher is wired to () => null since the codebase has zero callers of successAction.decipher (verified via grep). Covered by two tests: __tests__/self-custodial/payment-details/lnurl.spec.ts ("carries the decrypted plaintext on message (not via decipher) for AES Decrypted") asserts the new factory shape — message === plaintext, decipher() === null, ciphertext/iv null — and __tests__/screens/send-bitcoin-completed-screen.spec.tsx ("renders self-custodial AES plaintext from message (no ciphertext/iv)") drives the actual screen with the SC-shaped success-action and asserts ${plaintext} ${description} is rendered. The consumer test is the regression-detection that the original Tier 2 unit test was structurally blind to.

2. Idempotency key is dead at the only caller

createSelfCustodialLnurlPaymentDetails now accepts an optional idempotencyKey: string and resolves it once at factory entry — params.idempotencyKey ?? Crypto.randomUUID(). The resolved key is captured in the sendPaymentMutation closure and forwarded to executeLnurl(sdk, prepared, idempotencyKey), so any retry of the same paymentDetail instance reuses the same key and the SDK can dedup at the operator. To preserve the key across setMemo / setAmount / setSendingWalletDescriptor / setConvertMoneyAmount / setInvoice / setSuccessAction recreations (each builds a new factory by spreading params), all setters now spread a paramsWithKey object that already carries the resolved key, so the recreation chain keeps the same key for the same logical send intent. wrap-destination.ts doesn't pass a key — the optional field generates a fresh UUID per top-level intent. Covered by three tests in __tests__/self-custodial/payment-details/lnurl.spec.ts under "idempotency key forwarding": ("forwards a defined idempotency key to executeLnurl on every send") asserts the third positional arg is a non-empty string; ("reuses the same idempotency key across retries within the same paymentDetail") drives sendPaymentMutation twice on the same instance and asserts both calls received the same key (closure capture); ("preserves the idempotency key across setMemo / setAmount / setSendingWalletDescriptor recreations") locally spies on Crypto.randomUUID with a counter that yields uuid-1, uuid-2, … so a missing thread-through would surface as a fresh key — the test asserts every recreation forwards uuid-1 (regression-detection that the global mock's static value would mask).

---

Important

I1. sendPaymentMutation error classification — only LnurlError exercised

Two new tests in __tests__/self-custodial/payment-details/lnurl.spec.ts "sendPaymentMutation": ("classifies NetworkError tag with the network-error code") drives prepareLnurl to reject with { tag: "NetworkError" } and asserts errors[0].message === SelfCustodialErrorCode.NetworkError; ("classifies a generic (untagged) error with the generic code") drives a bare new Error("boom") rejection and asserts errors[0].message === SelfCustodialErrorCode.Generic. Combined with the existing LnurlError case, the classifier's three branches now have explicit coverage — any reshuffle of the classifySdkError lookup that loses a branch trips immediately.

I2. AES ErrorStatus branch never exercised

Folded into the Critical #1 commit. __tests__/self-custodial/payment-details/lnurl.spec.ts ("maps AES ErrorStatus to description with no plaintext leakage") drives the executor with result.tag === "ErrorStatus", asserts description === reason, message === null, decipher() === null. A typo regression like result.inner.message instead of .reason no longer ships silently — the test fails on the description mismatch.

I3. extractLnurlFee returns 0 for NaN/undefined silently

Acknowledged but deferred to the cleanup ticket. The cleanest fix isn't a guard at the call site — it's at app/utils/helper.ts:toNumber, which silently returns 0 for NaN inputs and is the actual source of the silent fallback. Adding a runtime guard in extractLnurlFee would (a) violate SRP — that function's job is extraction, not validation — and (b) be belt-and-suspenders against a regression that the SDK type signature (feeSats: bigint, non-optional) already excludes at compile time. The right move is to audit toNumber callers in the cleanup pass and decide whether to throw on NaN globally or split into toNumberStrict. The two existing extractLnurlFee tests still document the current contract (feeSats: BigInt(7) → 7, feeSats: BigInt(0) → 0 legitimate FeesIncluded path), so any future change to toNumber semantics that flips the 0 fallback breaks them and forces an explicit decision.

I4. lnurlParams.min || 0 would absorb malformed min: undefined if refactored to ??

New test in __tests__/self-custodial/payment-details/wrap-destination.spec.ts ("falls back to 0 sats when lnurlParams.min is undefined (still has callback + max)") drives wrapDestination with lnurlParams: { callback, max, commentAllowed } (no min field) and asserts the SC factory receives unitOfAccountAmount: { amount: 0 }. A refactor that flips || to ?? keeps the undefined → 0 mapping (since undefined ?? 0 === 0), so the test stays green; a refactor that drops the fallback entirely (destination.lnurlParams.min directly) feeds NaN into toBtcMoneyAmount and the test trips on the type/shape mismatch. Tight regression net for a single-line fallback.

I5. commentAllowed > 0 && memo === "" not exercised

New test in __tests__/self-custodial/payment-details/lnurl.spec.ts "prepareLnurl options" describe: ("omits the comment when commentAllowed > 0 but memo is empty") drives commentAllowed: 200, senderSpecifiedMemo: "" and asserts prepareLnurl was called with comment: undefined. The empty-string truthiness short-circuit (commentAllowed && memo evaluates to "" which the SDK call passes as undefined via the conditional) is now covered alongside the existing commentAllowed: 0 + memo set and commentAllowed: 200 + memo set extremes.

I6. SC sendPaymentMutation returns no transaction / preimage / createdAt

sendPaymentMutation now returns { status, transaction: { createdAt: Number(result.payment.timestamp) }, extraInfo: { preimage, successAction } }. Preimage extraction goes through a focused helper that uses the SDK's canonical type guard — PaymentDetails.Lightning.instanceOf(details) — matching the pattern already established in app/self-custodial/mappers/transaction-mapper.ts:97 (PaymentDetails.Token.instanceOf(details)). For non-Lightning payment shapes (Spark, Token), preimage is undefined; createdAt always populates from payment.timestamp (bigint seconds → number). The completed screen's transaction?.createdAt and extraInfo?.preimage reads now resolve for SC LNURL sends — closing the UX gap and unblocking any future fix to Critical #1 that wants to re-decrypt locally with the preimage. Covered by two tests in __tests__/self-custodial/payment-details/lnurl.spec.ts: ("propagates preimage from Lightning htlcDetails and createdAt from payment.timestamp on success") drives a Lightning-shaped details and asserts both fields land; ("returns undefined preimage when payment.details is non-Lightning") drives a Spark-shaped details and asserts preimage is undefined while createdAt still populates.

I7. as unknown as WalletAmount<T> type-lie in asBtcSettlementAmount

Acknowledged but deferred to the cleanup ticket alongside the broader fee-currency representation unification (the WalletAmount<T> generic carries asymmetric semantics across the adapter). Acceptable today because convertMoneyAmount dispatches on the runtime currency field (no miscalculation downstream), and the type lie is bounded to a single helper. New regression test in __tests__/self-custodial/payment-details/lnurl.spec.ts "getFee" describe: ("returns currency: Btc regardless of the sending wallet generic (USD wallet)") drives getFee from a USD-wallet factory and asserts result.amount.currency === WalletCurrency.Btc. Any future refactor that flips the runtime field to follow the generic T (or that drops the cast and lets TypeScript widen the inferred type) trips the assertion before reaching production.

I8. LnurlDestination runtime guard at wrapDestinationForSC only checks direction

Acknowledged but deferred to the cleanup ticket. The TypeScript type for LnurlDestination (Extract<ValidDestination, { paymentType: typeof PaymentType.Lnurl; lnurlParams: LnUrlPayServiceResponse }>) already excludes withdraw-shapes by structural narrowing on lnurlParams: LnUrlPayServiceResponse — adding a runtime guard at wrapDestination would violate SRP (this layer is a routing dispatcher, not a shape validator) and place the validation at the wrong level. The correct home is upstream in parseDestination, which is shared custodial+SC parsing logic — modifying it to enforce shape-narrowing before emitting LnurlDestination is the cleanup-ticket scope. The current cast (original as LnurlDestination) is the rendezvous point that documents the trust boundary between parser and dispatcher; any future parser change that returns withdraw-shaped Send + Lnurl destinations is caught at the parser-level tests, not here.

---

Test coverage (Tier 1, 2, 3)

Tier 1 — production code with no test for the new branch

• send-bitcoin-destination-screen.tsx:376 lnurlDomains gate — Two tests added in __tests__/screens/send-destination.spec.tsx under "lnurlDomains gate by active wallet type": ("passes empty lnurlDomains when active wallet is self-custodial") asserts parseDestination was called with { lnurlDomains: [] }; ("passes [lnAddressHostname, ...LNURL_DOMAINS] when active wallet is custodial") asserts the call carries ["blink.sv", "blink.sv", "pay.blink.sv", "pay.bbw.sv"]. The fix is wallet-type-driven, so the assertion is on the args of parseDestination rather than on a downstream side effect — whichever branch flips, the matching test fails.
• send-bitcoin-details-screen.tsx:368 requestInvoice skip — Two tests added in __tests__/screens/send-details.spec.tsx under "SendBitcoinDetailsScreen — LNURL requestInvoice gate": ("does NOT call lnurl-pay requestInvoice when paymentDetail.sendPaymentMutation is set (SC path)") drives a paymentDetail with the mutation defined, presses Next, asserts requestInvoice was not called and navigation.navigate("sendBitcoinConfirmation", ...) fired with the SC paymentDetail; ("calls lnurl-pay requestInvoice when paymentDetail.sendPaymentMutation is missing (custodial path)") drives the inverse, asserts requestInvoice was called once with { lnUrlOrAddress: "lnurl1abc" }. Either side flipping back to the wrong branch fails its respective test.
• send-bitcoin-confirmation-screen.tsx:215 successAction precedence — Two tests added in __tests__/screens/send-confirmation.spec.tsx under "successAction precedence on completion-screen navigation": ("forwards extraInfo.successAction to the completed screen when present") drives sendPayment to return an extraInfo.successAction distinct from the paymentDetail.successAction pre-set on the route, intercepts navigation.dispatch and asserts the dispatched sendBitcoinCompleted route's params carry the extraInfo one; ("falls back to paymentDetail.successAction when extraInfo.successAction is undefined") drives the same with extraInfo.successAction omitted and asserts the fallback to paymentDetail.successAction ships. Reversed ?? operands trip both.

Tier 2 — tests that pass while a Critical bug is alive

• AES decipher() unit test masks consumer-side bug (Critical testPR CI #1) — Replaced. The original "exposes the decrypted plaintext via decipher() for AES successAction" test was structurally blind because useSuccessMessage doesn't call decipher. The new factory test ("carries the decrypted plaintext on message (not via decipher) for AES Decrypted") asserts the consumer-readable contract — message === plaintext, decipher() === null — and the new e2e test in send-bitcoin-completed-screen.spec.tsx exercises the actual consumer end-to-end.
• Bridge spec covers idempotency forwarding while production caller never passes one (Critical show bitcoin address in sent onchain transaction #2) — Addressed. The __tests__/self-custodial/bridge/send.spec.ts executeLnurl cases stay (the bridge contract still matters), and the new caller-side suite "idempotency key forwarding" in lnurl.spec.ts closes the integration gap by spying on executeLnurl with a controllable randomUUID and asserting the key is non-empty + persistent across retries + threaded through setters.
• lnurl.spec.ts only tests LnurlError tag (I1) — Closed by the two new classifier tests (NetworkError + generic) in the "sendPaymentMutation" describe (see I1 above).
• AES ErrorStatus is dead from the suite's perspective (I2) — Closed by the new "maps AES ErrorStatus to description with no plaintext leakage" test (see I2 above).

Tier 3 — Critical bugs with no test coverage anywhere

• Critical testPR CI #1 end-to-end — Done. __tests__/screens/send-bitcoin-completed-screen.spec.tsx "renders self-custodial AES plaintext from message (no ciphertext/iv)" renders the screen with the post-fix SC shape and asserts the joined ${plaintext} ${description} string is on the rendered output. Mirrors the existing custodial AES test (which uses real ciphertext/iv/preimage and the same join) so any regression on either path is caught at the same layer.
• Critical show bitcoin address in sent onchain transaction #2 spy — Done. __tests__/self-custodial/payment-details/lnurl.spec.ts "forwards a defined idempotency key to executeLnurl on every send" — locally overrides the global react-native-quick-crypto mock with a counter so the assertion is regression-detection rather than a tautology.
• Screen gates × 3 — All three landed (destination + details + confirmation, see Tier 1 above).

[grimen]

Yet to be refactored to fully respect SOLID (i.e. ~adapter pattern), but approving since critical code/test issues were addressed so that we can get this into internal testing asap.

[grimen]

Merge activity

• May 20, 10:43 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• May 20, 10:46 AM UTC: Graphite couldn't merge this pull request because a downstack PR feat(spark): account migration to non-custodial UI screens #3742 failed to merge.
• May 20, 10:52 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• May 20, 10:56 AM UTC: Graphite couldn't merge this pull request because a downstack PR feat(spark): shared wallet abstractions and provider foundation #3746 failed to merge.
• May 20, 12:33 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• May 20, 12:34 PM UTC: @grimen merged this pull request with Graphite.

The merge-base changed after approval.