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..5a8abe5829 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}`. | @@ -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`). | -| `GET /paymentLink/payment/wait` | JWT or `key` | Long-poll a link's pending payment to a terminal state; returns the full link. | -| `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`. | +| 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`. | > Administrative endpoints (route-label assignment, internal link/payment edits, sticker > PDF generation, exchange-provider enrollment) exist but are internal/admin-only and are @@ -470,6 +470,12 @@ 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/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 new file mode 100644 index 0000000000..bcebdf6055 --- /dev/null +++ b/src/subdomains/core/payment-link/services/__tests__/payment-link-payment-wait.spec.ts @@ -0,0 +1,95 @@ +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 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 + // 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..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 @@ -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'; @@ -7,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'; @@ -164,7 +170,15 @@ 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((e) => { + if (!(e instanceof TimeoutError)) throw e; + + throw new RequestTimeoutException('Payment is still pending'); + }); } async handleBinanceWaiting(result: C2BWebhookResult): Promise {