From 25c15981036eea76065fd0dcef6d01985faa4fad Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Thu, 30 Jul 2026 22:23:48 +0000 Subject: [PATCH 1/2] Cap how long the payment wait endpoints block and answer 408 Both wait routes passed 0 as the timeout to AsyncMap.wait(), which arms no timer, so a request could only ever be settled by the payment itself. For an invoice with an expiry years out that means the handler stays pending indefinitely, and the CDN in front of the API ends the request after 125 s with a gateway error the caller cannot interpret. Because such a request never completes, it also never writes an access-log line, so the latency data covers only the waits that finished. Cap a single call at 110 s and answer 408, leaving the caller a defined "nothing has happened yet" to re-issue. The cap sits below the CDN limit and above the longest wait seen in production (93 s), so it does not cut short a payment that would still resolve. The timeout is raced in the service rather than passed to AsyncMap.wait(): the map hands every caller of the same payment one shared subscriber, whose timer would be armed by whoever arrived first and would reject all of them together. Note the subscriber itself still lives until the payment resolves; it is bounded by the number of pending payments and a later caller is served from it, so releasing it on client disconnect would need reference counting in AsyncMap and is left out here. --- CONTRIBUTING.md | 2 +- README.md | 2 + docs/payment-links.md | 9 +- src/config/config.ts | 7 ++ .../payment-link-payment-wait.spec.ts | 86 +++++++++++++++++++ .../services/payment-link-payment.service.ts | 16 +++- 6 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 src/subdomains/core/payment-link/services/__tests__/payment-link-payment-wait.spec.ts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1a7212d758..f5d3d336e0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -976,7 +976,7 @@ Endpoints that block by design: | Path | Blocks until | `wait` segment | | ------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------- | -| `GET /v1/lnurlp/wait/:id` | the payment resolves: completed, canceled, expired — or, for `MULTIPLE`-mode links, when a quote reaches the configured completion threshold (the payment itself may stay `Pending`) | yes | +| `GET /v1/lnurlp/wait/:id` | the payment resolves: completed, canceled, expired — or, for `MULTIPLE`-mode links, when a quote reaches the configured completion threshold (the payment itself may stay `Pending`); bounded at `Config.payment.waitTimeout` (110 s), then 408 | yes | | `GET /v1/paymentLink/payment/wait` | the same, for the authenticated payment-link flow | yes | | `GET /v1/lnurlp/:id` | a pending payment appears; bounded by `timeout` (default 10 s, caller-controllable) | no — exempt | | `GET /v1/lnurlp/tx/:id` | the payer's own broadcast reaches one confirmation (`tx` branch); 15 polls at 1 s. The `hex` branch broadcasts without awaiting confirmation, except on ICP, where it first waits for the payer's allowance (up to 3 attempts, 2 s apart) | no — exempt | diff --git a/README.md b/README.md index 6d814edf54..4f4b072b92 100644 --- a/README.md +++ b/README.md @@ -175,6 +175,8 @@ The following steps must be carried out at the POS if a customer wants to pay wi 1. [Wait for payment](https://api.dfx.swiss/swagger#/Payment%20Link/PaymentLinkController_waitForPayment): This API endpoint can be used to wait for a change on a pending payment. It blocks until the payment either is completed, cancelled or expired. The response will contain the result in the `payment.status` field. Please use the `externalPaymentId` query parameter to select the payment to be waited on. + A single call blocks for at most 110 seconds. If the payment is still pending when that window ends, the endpoint answers with HTTP 408 and no result — send the request again to keep waiting. A 408 means "nothing has happened yet", not "the customer did not pay"; only a terminal `payment.status` in a successful response is a result. + 1. [Confirm a payment](https://api.dfx.swiss/swagger#/Payment%20Link/PaymentLinkController_confirmPayment) (optional): A payment can be confirmed for documentation purposes. This can only be done after it is completed (paid by the customer). Please use the `externalPaymentId` query parameter to select the payment to be confirmed. 1. [Cancel a payment](https://api.dfx.swiss/swagger#/Payment%20Link/PaymentLinkController_cancelPayment) (optional): A payment can be cancelled as long at it is still pending. Please use the `externalPaymentId` query parameter to select the payment to be cancelled. diff --git a/docs/payment-links.md b/docs/payment-links.md index 1bf23f5f10..1a4ebdcd35 100644 --- a/docs/payment-links.md +++ b/docs/payment-links.md @@ -271,7 +271,7 @@ endpoints below are public (no authentication) unless an **Auth** value says oth | `GET /lnurlp/{id}` | LNURL-pay step 1 — returns the pay request. Optional `standard`, `timeout`. | | `GET /lnurlp/cb/{id}` | LNURL-pay step 2 (callback) — returns the actual invoice/URI. Params: `quote` (required), and per standard `amount` (msat, Bolt11) or `method`+`asset` (PayToAddress), plus optional `tx`/`hex`/`sender`. | | `GET /lnurlp/tx/{id}` | Submit a signed tx `hex` or broadcast `txId` (on-chain methods). Returns `{ txId }`. | -| `GET /lnurlp/wait/{id}` | Long-poll until the payment reaches a terminal state. Returns `{ status }`. | +| `GET /lnurlp/wait/{id}` | Long-poll until the payment reaches a terminal state. Returns `{ status }`, or 408 after 110 s (re-issue). | | `DELETE /lnurlp/cancel/{id}` | Cancel a pending payment (only if the link is `cancellable`). | | `GET /pl?lightning=LNURL1…` | Human scan target — decodes the embedded LNURL and forwards to `GET /lnurlp/{id}`. | @@ -288,7 +288,7 @@ Most of these require a merchant **JWT** (DFX login) or, where noted, a payment- | `PUT /paymentLink` | JWT | Update a link (status, label, webhook, config). | | `GET /paymentLink/history` | JWT or `key` | Payment history (`status`, `from`, `to`). | | `POST /paymentLink/payment` | optional JWT / `key` | Create a payment on a link (POS path with `key`). | -| `GET /paymentLink/payment/wait` | JWT or `key` | Long-poll a link's pending payment to a terminal state; returns the full link. | +| `GET /paymentLink/payment/wait` | JWT or `key` | Long-poll a link's pending payment to a terminal state; returns the full link. 408 after 110 s (re-issue). | | `PUT /paymentLink/payment/confirm` | JWT or `key` | Mark a completed payment confirmed. | | `DELETE /paymentLink/payment` | optional JWT / `key` | Cancel the pending payment. | | `PUT /paymentLink/pos` | JWT | Get a POS URL (`{ url }`) for a link. | @@ -470,6 +470,11 @@ Three options, from push to pull: reaches a terminal state. (Authenticated variant: `GET /v1/paymentLink/payment/wait`.) Both require the payment to be **currently pending** when you call; if it already settled, fall back to a plain read. + + A call blocks for at most **110 seconds**. If the payment is still pending by then, the + answer is **HTTP 408** — nothing has been decided, so issue the request again. Treat 408 + as "ask once more", never as "the customer did not pay": a payment that outlives one + window is normal, and only a terminal status in a 2xx response is a result. 3. **Poll** — read `GET /v1/paymentLink` / `GET /v1/paymentLink/history` (JWT or access key) and inspect `payment.status` / `payments[]`. diff --git a/src/config/config.ts b/src/config/config.ts index cf77d593ae..6c51f945ee 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -800,6 +800,13 @@ export class Configuration { defaultQuoteTimeout: 300, // sec addressQuoteTimeout: 7200, // sec + // How long the `wait` routes block before answering 408. Deliberately a constant and not + // an environment variable: the value is bounded from above by the CDN in front of the API, + // which terminates an origin request after 125 s with a 524 the client cannot interpret. + // Staying below that keeps the outcome ours to define. The longest wait observed in + // production is 93 s, so the cap does not cut short a payment that would still resolve. + waitTimeout: 110, // sec + manualMethods: ['TaprootAsset', 'Spark', 'Arkade'], webhookPublicKey: process.env.PAYMENT_WEBHOOK_PUBLIC_KEY?.split('
').join('\n'), diff --git a/src/subdomains/core/payment-link/services/__tests__/payment-link-payment-wait.spec.ts b/src/subdomains/core/payment-link/services/__tests__/payment-link-payment-wait.spec.ts new file mode 100644 index 0000000000..faa0205329 --- /dev/null +++ b/src/subdomains/core/payment-link/services/__tests__/payment-link-payment-wait.spec.ts @@ -0,0 +1,86 @@ +import { RequestTimeoutException } from '@nestjs/common'; +import { BlockchainRegistryService } from 'src/integration/blockchain/shared/services/blockchain-registry.service'; +import { FiatService } from 'src/shared/models/fiat/fiat.service'; +import * as ConfigModule from 'src/config/config'; +import { PaymentLinkPayment } from '../../entities/payment-link-payment.entity'; +import { PaymentLinkPaymentRepository } from '../../repositories/payment-link-payment.repository'; +import { PaymentActivationService } from '../payment-activation.service'; +import { PaymentLinkPaymentService } from '../payment-link-payment.service'; +import { PaymentQuoteService } from '../payment-quote.service'; +import { PaymentWebhookService } from '../payment-webhook.service'; + +const WAIT_TIMEOUT = 110; + +describe('PaymentLinkPaymentService', () => { + let service: PaymentLinkPaymentService; + + beforeEach(() => { + (ConfigModule as Record).Config = { payment: { waitTimeout: WAIT_TIMEOUT } }; + + service = new PaymentLinkPaymentService( + {} as unknown as FiatService, + {} as unknown as PaymentLinkPaymentRepository, + {} as unknown as PaymentWebhookService, + {} as unknown as PaymentQuoteService, + {} as unknown as PaymentActivationService, + {} as unknown as BlockchainRegistryService, + ); + + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + // --- waitForPayment() Tests --- // + + describe('waitForPayment()', () => { + it('should return the payment when it resolves within the wait window', async () => { + const resolved = { id: 1, status: 'Completed' } as unknown as PaymentLinkPayment; + + const wait = service.waitForPayment({ id: 1 } as PaymentLinkPayment); + service['paymentWaitMap'].resolve(1, resolved); + + await expect(wait).resolves.toBe(resolved); + }); + + it('should answer 408 instead of blocking indefinitely on a payment that stays pending', async () => { + // Without the cap the request hangs until the payment resolves — for a long-lived + // invoice that is years away — and is killed by the CDN with a 524 the caller + // cannot interpret. + const wait = service.waitForPayment({ id: 2 } as PaymentLinkPayment); + const assertion = expect(wait).rejects.toBeInstanceOf(RequestTimeoutException); + + await jest.advanceTimersByTimeAsync(WAIT_TIMEOUT * 1000); + + await assertion; + }); + + it('should give a caller joining a pending payment its own full window', async () => { + // AsyncMap hands both callers the same subscriber. Arming the timeout there would + // let the first caller's timer decide for the second one, cutting the late joiner + // off after a second instead of giving it the full window. + const first = service.waitForPayment({ id: 3 } as PaymentLinkPayment); + const firstTimedOut = expect(first).rejects.toBeInstanceOf(RequestTimeoutException); + await jest.advanceTimersByTimeAsync((WAIT_TIMEOUT - 1) * 1000); + + const second = service.waitForPayment({ id: 3 } as PaymentLinkPayment); + let secondSettled = false; + void second.then( + () => (secondSettled = true), + () => (secondSettled = true), + ); + + await jest.advanceTimersByTimeAsync(2 * 1000); + + await firstTimedOut; + expect(secondSettled).toBe(false); + + const resolved = { id: 3, status: 'Completed' } as unknown as PaymentLinkPayment; + service['paymentWaitMap'].resolve(3, resolved); + + await expect(second).resolves.toBe(resolved); + }); + }); +}); diff --git a/src/subdomains/core/payment-link/services/payment-link-payment.service.ts b/src/subdomains/core/payment-link/services/payment-link-payment.service.ts index e7f223f2d9..21865e7eb1 100644 --- a/src/subdomains/core/payment-link/services/payment-link-payment.service.ts +++ b/src/subdomains/core/payment-link/services/payment-link-payment.service.ts @@ -1,4 +1,10 @@ -import { BadRequestException, ConflictException, Injectable, NotFoundException } from '@nestjs/common'; +import { + BadRequestException, + ConflictException, + Injectable, + NotFoundException, + RequestTimeoutException, +} from '@nestjs/common'; import { Observable, Subject } from 'rxjs'; import { Config, Environment } from 'src/config/config'; import { Blockchain } from 'src/integration/blockchain/shared/enums/blockchain.enum'; @@ -164,7 +170,13 @@ export class PaymentLinkPaymentService { // --- HANDLE WAITS --- // async waitForPayment(payment: PaymentLinkPayment): Promise { - return this.paymentWaitMap.wait(payment.id, 0); + // The timeout is raced here rather than passed to AsyncMap.wait(): the map hands every + // caller of the same payment one shared subscriber, so its timer would be armed by + // whoever arrived first and would then reject all of them together — a caller joining + // late would be cut off after whatever is left of someone else's window. + return Util.timeout(this.paymentWaitMap.wait(payment.id, 0), Config.payment.waitTimeout * 1000).catch(() => { + throw new RequestTimeoutException('Payment is still pending'); + }); } async handleBinanceWaiting(result: C2BWebhookResult): Promise { From a95e58f0abef37037cfba8c4d1164491d35ae02d Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Thu, 30 Jul 2026 23:06:15 +0000 Subject: [PATCH 2/2] Convert only the timeout into a 408 and clear the losing timer Two follow-ups from review: The catch turned any rejection of the raced promise into a 408. That is harmless today because nothing ever rejects the wait, but it would silently swallow the reason if a later change starts rejecting it. Rejections now pass through and only a TimeoutError becomes a 408. Util.timeout left its timer armed for the full duration even when the wrapped promise won the race. With previous callers (10-20 s, low frequency) that went unnoticed; a 110 s timeout on a per-request path would accumulate them. The timer is now cleared once the race is decided. Also re-runs prettier on docs/payment-links.md, whose table padding the previous commit had left inconsistent. --- docs/payment-links.md | 27 ++++++++++--------- src/shared/utils/util.ts | 17 ++++++++++-- .../payment-link-payment-wait.spec.ts | 9 +++++++ .../services/payment-link-payment.service.ts | 6 +++-- 4 files changed, 42 insertions(+), 17 deletions(-) diff --git a/docs/payment-links.md b/docs/payment-links.md index 1a4ebdcd35..5a8abe5829 100644 --- a/docs/payment-links.md +++ b/docs/payment-links.md @@ -281,20 +281,20 @@ Most of these require a merchant **JWT** (DFX login) or, where noted, a payment- **access key** (`?key=…`); the **Auth** column is authoritative per row (a few, such as `assign` and `locations`, are public). -| Method & path | Auth | Purpose | -| ---------------------------------------------------- | -------------------- | ------------------------------------------------------------------------------ | -| `POST /paymentLink` | JWT or `key` | Create a persistent, managed link (`CreatePaymentLink` body). | -| `GET /paymentLink` | JWT | List links / fetch one (`linkId`/`externalLinkId`/`externalPaymentId`). | -| `PUT /paymentLink` | JWT | Update a link (status, label, webhook, config). | -| `GET /paymentLink/history` | JWT or `key` | Payment history (`status`, `from`, `to`). | -| `POST /paymentLink/payment` | optional JWT / `key` | Create a payment on a link (POS path with `key`). | +| Method & path | Auth | Purpose | +| ---------------------------------------------------- | -------------------- | ---------------------------------------------------------------------------------------------------------- | +| `POST /paymentLink` | JWT or `key` | Create a persistent, managed link (`CreatePaymentLink` body). | +| `GET /paymentLink` | JWT | List links / fetch one (`linkId`/`externalLinkId`/`externalPaymentId`). | +| `PUT /paymentLink` | JWT | Update a link (status, label, webhook, config). | +| `GET /paymentLink/history` | JWT or `key` | Payment history (`status`, `from`, `to`). | +| `POST /paymentLink/payment` | optional JWT / `key` | Create a payment on a link (POS path with `key`). | | `GET /paymentLink/payment/wait` | JWT or `key` | Long-poll a link's pending payment to a terminal state; returns the full link. 408 after 110 s (re-issue). | -| `PUT /paymentLink/payment/confirm` | JWT or `key` | Mark a completed payment confirmed. | -| `DELETE /paymentLink/payment` | optional JWT / `key` | Cancel the pending payment. | -| `PUT /paymentLink/pos` | JWT | Get a POS URL (`{ url }`) for a link. | -| `GET /paymentLink/config`, `PUT /paymentLink/config` | JWT (account) | Read/update the account-level link config (incl. POS access key). | -| `PUT /paymentLink/assign` | none | Assign an unassigned link to a route by `publicName`. | -| `GET /paymentLink/locations` | none | Distinct recipient addresses for a `publicName`. | +| `PUT /paymentLink/payment/confirm` | JWT or `key` | Mark a completed payment confirmed. | +| `DELETE /paymentLink/payment` | optional JWT / `key` | Cancel the pending payment. | +| `PUT /paymentLink/pos` | JWT | Get a POS URL (`{ url }`) for a link. | +| `GET /paymentLink/config`, `PUT /paymentLink/config` | JWT (account) | Read/update the account-level link config (incl. POS access key). | +| `PUT /paymentLink/assign` | none | Assign an unassigned link to a route by `publicName`. | +| `GET /paymentLink/locations` | none | Distinct recipient addresses for a `publicName`. | > Administrative endpoints (route-label assignment, internal link/payment edits, sticker > PDF generation, exchange-provider enrollment) exist but are internal/admin-only and are @@ -475,6 +475,7 @@ Three options, from push to pull: answer is **HTTP 408** — nothing has been decided, so issue the request again. Treat 408 as "ask once more", never as "the customer did not pay": a payment that outlives one window is normal, and only a terminal status in a 2xx response is a result. + 3. **Poll** — read `GET /v1/paymentLink` / `GET /v1/paymentLink/history` (JWT or access key) and inspect `payment.status` / `payments[]`. diff --git a/src/shared/utils/util.ts b/src/shared/utils/util.ts index 72f98c8c92..b0876655d2 100644 --- a/src/shared/utils/util.ts +++ b/src/shared/utils/util.ts @@ -22,6 +22,14 @@ export enum AmountType { FIAT_FEE = 'FiatFee', } +// thrown by Util.timeout() when the wrapped promise loses the race, so a caller can tell the +// timeout apart from a rejection of the promise it wrapped +export class TimeoutError extends Error { + constructor() { + super('Timeout'); + } +} + export class Util { // --- MATH --- // static roundReadable(amount: number, type: AmountType, assetPrecision?: number): number { @@ -640,9 +648,14 @@ export class Util { } static async timeout(promise: Promise, timeout: number): Promise { - const timeoutPromise = new Promise((_, reject) => setTimeout(() => reject(new Error('Timeout')), timeout)); + let timer: NodeJS.Timeout; + const timeoutPromise = new Promise((_, reject) => { + timer = setTimeout(() => reject(new TimeoutError()), timeout); + }); - return Promise.race([promise, timeoutPromise]); + // clear the timer once the race is decided, so a promise that wins early does not leave + // the timer pending for the rest of its duration + return Promise.race([promise, timeoutPromise]).finally(() => clearTimeout(timer)); } static async delay(ms: number): Promise { diff --git a/src/subdomains/core/payment-link/services/__tests__/payment-link-payment-wait.spec.ts b/src/subdomains/core/payment-link/services/__tests__/payment-link-payment-wait.spec.ts index faa0205329..bcebdf6055 100644 --- a/src/subdomains/core/payment-link/services/__tests__/payment-link-payment-wait.spec.ts +++ b/src/subdomains/core/payment-link/services/__tests__/payment-link-payment-wait.spec.ts @@ -57,6 +57,15 @@ describe('PaymentLinkPaymentService', () => { await assertion; }); + it('should pass a rejection of the wait itself through instead of reporting it as a timeout', async () => { + const wait = service.waitForPayment({ id: 4 } as PaymentLinkPayment); + const assertion = expect(wait).rejects.toThrow('Payment gone'); + + service['paymentWaitMap'].reject(4, 'Payment gone'); + + await assertion; + }); + it('should give a caller joining a pending payment its own full window', async () => { // AsyncMap hands both callers the same subscriber. Arming the timeout there would // let the first caller's timer decide for the second one, cutting the late joiner diff --git a/src/subdomains/core/payment-link/services/payment-link-payment.service.ts b/src/subdomains/core/payment-link/services/payment-link-payment.service.ts index 21865e7eb1..56d26fb2e7 100644 --- a/src/subdomains/core/payment-link/services/payment-link-payment.service.ts +++ b/src/subdomains/core/payment-link/services/payment-link-payment.service.ts @@ -13,7 +13,7 @@ import { LnurlpInvoiceDto } from 'src/integration/lightning/dto/lnurlp.dto'; import { LightningHelper } from 'src/integration/lightning/lightning-helper'; import { FiatService } from 'src/shared/models/fiat/fiat.service'; import { AsyncMap } from 'src/shared/utils/async-map'; -import { Util } from 'src/shared/utils/util'; +import { TimeoutError, Util } from 'src/shared/utils/util'; import { C2BWebhookResult } from 'src/subdomains/core/payment-link/share/c2b-payment-link.provider'; import { CryptoInput } from 'src/subdomains/supporting/payin/entities/crypto-input.entity'; import { IsNull, LessThan } from 'typeorm'; @@ -174,7 +174,9 @@ export class PaymentLinkPaymentService { // caller of the same payment one shared subscriber, so its timer would be armed by // whoever arrived first and would then reject all of them together — a caller joining // late would be cut off after whatever is left of someone else's window. - return Util.timeout(this.paymentWaitMap.wait(payment.id, 0), Config.payment.waitTimeout * 1000).catch(() => { + return Util.timeout(this.paymentWaitMap.wait(payment.id, 0), Config.payment.waitTimeout * 1000).catch((e) => { + if (!(e instanceof TimeoutError)) throw e; + throw new RequestTimeoutException('Payment is still pending'); }); }