Commit message · RoxanneAdd LUD-21 (LNURL-pay Verify) support (#7250)
* Add LUD-21 (LNURL-pay Verify) support
Implements the LUD-21 verify endpoint for Lightning Address payments,
enabling external services to verify payment settlement without
authentication.
Changes:
- New GET /lnurlp/{username}/verify/{paymentHash} endpoint
- Callback response includes verify URL when LUD-21 is enabled
- Payment hashes indexed in search data for efficient lookup
- LUD21Enabled toggle in LNURL payment method config (default: true)
- Settings UI toggle matching LUD-12 pattern
- Swagger API docs updated
Closes #7248
* Use case-insensitive comparison for payment hash in LUD-21 verify
* Address Nicolas review: use AddressInvoices instead of AdditionalSearchTerms
- Replace AddSearchTerms with AddAddressInvoice for payment hash indexing
- Replace TextSearch lookup with GetInvoiceFromAddress in verify endpoint
- Add storeId validation on verify endpoint (cross-store isolation)
- Add payment hash to TrackedDestinations in LightningLikePaymentHandler
- Add rate limiting (ZoneLimits.Verify) to verify endpoint
- Add integration test for LUD-21 verify endpoint flow
* Remove rate limiting from LUD-21 verify endpoint
Rate limiting will be discussed and added in a follow-up.
* Fix verify endpoint: idempotent AddAddressInvoice, normalize payment hash
- Make AddAddressInvoice upsert to avoid duplicate key violations
on (Address, PaymentMethodId)
- Normalize payment hash to lowercase for consistent DB lookups
(store, query, and verify URL generation)
* test(LUD-21): properly exercise cross-store isolation in CanUseLUD21VerifyEndpoint
The previous storeId-isolation assertion did not actually validate the
intended behavior. store2 only had a Lightning Address configured, but
no Lightning node / LNURL payment method. The verify endpoint bails out
with 404 "Not available" at UILNURLController.cs:487-489 before
reaching the invoice-lookup / store-isolation check at L492-494, so the
old assertion would pass even if the isolation logic were broken.
Fix:
- Configure BTC-LN and BTC-LNURL (LUD21Enabled = true) on store2 via
UpdateStorePaymentMethod, mirroring what RegisterLightningNodeAsync
does for store1.
- Strengthen the assertion to also verify the response body's reason
is "Not found" rather than "Not available", proving the request
reached the actual store-isolation branch.
* feat(LUD-21): validate paymentHash is 64 hex chars before lookup
LnurlPayVerify previously accepted any non-empty paymentHash string and
only normalized casing. A Lightning payment hash is exactly 32 bytes /
64 hex characters, so reject anything else up front to avoid pointless
DB lookups on garbage input and return a consistent "Not found"
response shape.
* fix(LUD-21): standardize LnurlPayVerify not-found reason to "Not found"
LnurlPayVerify previously returned three different reason strings for
not-found cases ("Unknown username", "Not available", "Not found"),
which leaks information about which lookup branch failed and was flagged
in CodeRabbit review.
Collapse the username/store-resolution branches to a single "Not found"
reason. The LUD-21 feature-disabled branch keeps its distinct "Not
available" reason because that's a genuinely different operational
condition (feature off vs resource missing) and the new test relies on
the distinction to validate the cross-store isolation path.
* test(LUD-21): fix LN Address fetch path to use /.well-known/lnurlp/{username}
CanUseLUD21VerifyEndpoint was calling /lnurlp/{username} to fetch the
LNURL-pay request, but that path has no route. The actual LUD-16
Lightning Address resolver is published at /.well-known/lnurlp/{username}
(see ResolveLightningAddress in UILNURLController.cs and existing usage
in PlaywrightTests / TestAccount). The wrong path made the test fail at
the very first GET with 404, before the verify endpoint logic could be
exercised at all.
* fix(LUD-21): close TOCTOU race in AddAddressInvoice + harden preimage assertion
CodeRabbit followup:
1. AddAddressInvoice was check-then-insert, so two concurrent LNURL
callbacks for the same (Address, PaymentMethodId) could both observe
existing == null and both attempt to INSERT, causing the second one
to throw on the unique-key constraint. Wrap the insert path in
try/catch (DbUpdateException) and swallow the violation - both
writers are inserting identical data, so the operation is naturally
idempotent under contention. The update branch is kept on its own
SaveChangesAsync so failures there still propagate.
2. The preimage assertion in CanUseLUD21VerifyEndpoint was
Assert.Null(verifyResult["preimage"]?.ToString()), which depends on
how the global JSON serializer handles null values - if the field is
serialized as JSON null instead of being omitted, ToString() returns
the empty string and the assertion fails. Compare against the
JTokenType.Null sentinel directly so the test is robust to either
serializer mode.
* refactor(LUD-21): flush tracked payment hash via UpdatePrompt overload
Consolidates the two-call sequence in the LNURL callback (UpdatePrompt
followed by a separate AddAddressInvoice) into a single UpdatePrompt
call that accepts an optional trackedDestinations list and flushes
AddressInvoices rows inside the same DbContext transaction.
- Ensures the payment-hash index row is written atomically with the
prompt update so a crash between the two calls can no longer leave
the prompt persisted without its verify-lookup row.
- Keeps idempotency under concurrent LNURL callbacks (existing row
check + DbUpdateException swallow, matching AddAddressInvoice).
- Controller no longer reaches into the repository twice for a single
logical state transition.
No behavior change for existing UpdatePrompt callers (trackedDestinations
defaults to null).
* fix(LUD-21): split SaveChanges in UpdatePrompt, exclude concurrency from inner catch
Addresses both items from coderabbit review on dcfdf88:
1. DbUpdateConcurrencyException is a DbUpdateException subtype, so the
inner catch in the trackedDestinations flush would have silently
swallowed concurrency conflicts on the invoice row, breaking the
outer retry loop. Narrowed the inner catch with
'when (ex is not DbUpdateConcurrencyException)'.
2. Batching the prompt blob update and the AddressInvoices insert into
a single SaveChangesAsync meant a unique-key violation on the
tracked destination would roll back the entire unit of work and
silently drop the prompt blob update. Split into two
SaveChangesAsync calls: the blob update runs first and any error
propagates (concurrency still retries via the outer catch); the
tracked-destination flush runs after with its own scoped,
unique-key-only swallow.
* test(LUD-21): cover repeat-callback idempotency and hash validation
Adds three assertions inside the existing CanLNURLPayVerify integration test
to illustrate and guard the UpdatePrompt(trackedDestinations) refactor:
- Repeat LNURL callback with the SAME amount: exercises the idempotent
flush path where the AddressInvoices row for the payment hash already
exists. The second callback must return 200 and the verify endpoint
must still resolve the hash afterward (no duplicate row, no throw).
- Repeat LNURL callback with a DIFFERENT amount: a new payment hash is
minted. The new verify URL must resolve, proving the new hash is
indexed in AddressInvoices and the prompt blob transition persists.
- Verify endpoint with a malformed (non-hex / wrong-length) paymentHash
segment must return 404 with Reason 'Not found' at the 64-hex guard
before any DB lookup is performed.
* Address NicolasDorier review feedback on LUD-21
- Remove username from verify route (/lnurlp/verify/{paymentHash}),
look up store from invoice instead; enables verify for all LNURL,
not only Lightning Address
- Replace EF check-then-act in AddressInvoices with SQL INSERT ON
CONFLICT DO NOTHING (single roundtrip, no partial-dup DbContext
corruption)
- Remove IgnoreAntiforgeryToken on GET route
- Remove payment hash tracking from LightningLikePaymentHandler
(only needed for LNURL path, already handled in UILNURLController)
- Standardize all error responses to "Not found"
- Simplify tests: drop cross-store-via-username and unknown-username
checks (no longer applicable without username in route)
* Address NicolasDorier review: guard trackedDestinations on LUD21Enabled, deduplicate AddressInvoices upsert
- trackedDestinations is now null when LUD21 is not enabled, preventing
unnecessary AddressInvoices rows for stores that don't use verify.
- Extract UpsertAddressInvoice helper to eliminate the duplicated INSERT
SQL across AddAddressInvoice, UpdatePrompt, and NewPaymentPrompt.
---------
Co-authored-by: r1ckstardev <r1ckstardev@users.noreply.github.com>
Co-authored-by: r1ckstardev <me@r0ckstar.dev>
100/100 · StrongMessage clarity
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Mentions testing or verification✓ Links an issue, advisory, or supporting reference✓ Names security-relevant behavior explicitly
access controldefensive validation
AI analysis · Low 41/100This commit adds a new public feature to BTCPay Server called LUD-21, which lets anyone check whether a Lightning Network payment has settled by knowing its payment hash. The feature is enabled by default. The code went through several review rounds that fixed information-leakage issues, race conditions, and input-validation problems. The final version appears reasonably hardened, but because it intentionally exposes invoice settlement status and the payment preimage to unauthenticated callers, it carries a real privacy and operational risk if a merchant does not realize it is on by default.
