Release: develop -> main

CONTRIBUTING.md

@@ -69,6 +69,7 @@ Missing any of these = changes requested.
 - **Boolean flags: positive naming**: `safetyModeActive` not `safetyModuleInactive`
 - **Short, descriptive names — no redundant prefixes**: `uid` not `transactionRequestUid`, `balances` not `assetBalances`, `txId` not `transactionId`
 - **Variable names must precisely reflect the data**: `priceChf` not `amountChf` for a price
+- **`wait` is reserved for long-polling routes**: see [Long-Polling Endpoints Must Be Named `wait`](#long-polling-endpoints-must-be-named-wait) — the segment is load-bearing for latency monitoring
 
 ### Methods
 
@@ -965,6 +966,36 @@ Keep old endpoints for backward compatibility but annotate:
 @ApiOperation({ deprecated: true })
 ```
 
+### Long-Polling Endpoints Must Be Named `wait`
+
+An endpoint that does nothing but wait for someone else to act — its response time is determined solely by when another request or process triggers the event, and it performs no work of its own meanwhile — **must** carry `wait` as its own path segment, unless it is listed as an explicit exemption below. Conversely, an endpoint expected to answer quickly **must not** use `wait` as a path segment.
+
+This does **not** cover an endpoint that starts an operation and then waits for it to finish — broadcasting a transaction and awaiting its confirmation, for example. That duration reflects work the API itself set in motion, which makes it a legitimate monitoring signal, so those endpoints stay visible and must **not** be named `wait`. Current examples: `PUT /v1/sell/paymentInfos/:id/confirm` and `PUT /v1/swap/paymentInfos/:id/confirm` (both in their `authorization` branch), `PUT /v1/realunit/sell/:id/confirm` (`eip7702` branch) and `PUT /v1/realunit/transfer/:id/confirm`.
+
+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/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    |
+| `GET /v1/node/:node/tx/:txId`        | the transaction reaches one confirmation; bounded at 600 s                                                                                                                        | no — exempt    |
+| `GET /v1/node/:node/:mode/tx/:txId`  | the same                                                                                                                                                                          | no — exempt    |
+
+**The `wait` segment is the default; exemptions must be explicit.** A *passively* waiting route without one is acceptable only if it is listed in the table above together with the reason it cannot carry the segment. A passively waiting route that is neither named `wait` nor listed here is a defect — fix it by renaming the route or by adding an entry, never by leaving it undocumented. Routes of the second kind above — those awaiting an operation they started themselves — need no entry; they are outside this rule by design.
+
+The four current exemptions keep their paths because those are fixed from outside: `/v1/lnurlp/:id` is the LNURL pay-request path encoded into LNURLs already in circulation, `/v1/lnurlp/tx/:id` is handed to the payer inside the payment request itself, and the two node routes are admin-only and `@ApiExcludeEndpoint()`. Because they carry no `wait` segment they stay visible in latency monitoring — read their duration as expected behavior, not as a regression.
+
+This is not cosmetic. Latency monitoring excludes routes matching `^.*/wait(/.*)?$` from its slowest-requests view. A long poll's duration measures how long a *customer* took to act, not how long the API computed — leaving it in that view pushes the genuine outliers out of a list with a fixed row cap.
+
+Getting the name wrong breaks monitoring in one of two directions:
+
+- **A passively waiting endpoint without a `wait` segment** appears as a permanent latency outlier and masks real regressions — that is exactly what the exemptions above cost us today, which is why the list must stay short and justified.
+- **A fast endpoint with a `wait` segment** is silently dropped from the latency view — if it ever becomes slow, nobody notices.
+
+The pattern is segment-anchored, so `/waitlist`, `/waitTime`, `/awaiting` and `/waiting/:id` are unaffected; only a complete `wait` segment matches. Matching runs on the server-side route template (`http.route`), never on the raw request path, which is caller-controlled.
+
 ### RealUnit: `/quote/*` vs `/brokerbot/*`
 
 The RealUnit purchase and sale flows historically lived under `/v1/realunit/brokerbot/*`. That naming is misleading: most of those endpoints never touch the on-chain Brokerbot smart contract. Treat them as two distinct subsystems:

migration/1785510000000-AddTradingOrderRuleIdIndex.js

@@ -0,0 +1,145 @@
+/**
+ * @typedef {import('typeorm').MigrationInterface} MigrationInterface
+ * @typedef {import('typeorm').QueryRunner} QueryRunner
+ */
+
+/**
+ * Add a composite index on `trading_order ("tradingRuleId", "id")` so the per-minute
+ * "latest trading order per rule" lookup stops doing a full sequential scan of the table.
+ * Row counts and sizes are given further down, together with where each was measured.
+ *
+ * The query this targets is:
+ * `SELECT MAX("tradingOrder"."id") AS "tradingOrderId" FROM "trading_order" "tradingOrder"
+ * INNER JOIN "trading_rule" "tradingRule" ON "tradingRule"."id" = "tradingOrder"."tradingRuleId"
+ * GROUP BY "tradingOrder"."tradingRuleId"`.
+ * Source: `TradingRuleService.getCurrentTradingOrders`, called from `LogJobService` once per
+ * minute as part of the financial-log job.
+ *
+ * Production `EXPLAIN (ANALYZE, BUFFERS)` for this exact aggregate query (measured externally
+ * against production; not reproducible from this repository) showed Execution Time 489 ms,
+ * Buffers: shared hit=3734 read=85614 — only 4.2% of blocks came from the 1 GB `shared_buffers`
+ * cache, the rest was freshly read from disk on every one-minute run. Table size at the time of
+ * measurement in production: 698 MB. Result: 17 rows (there are exactly 17 `trading_rule` rows).
+ *
+ * Column order `("tradingRuleId", "id")` is intentional: equality on `tradingRuleId` first so
+ * Postgres can jump straight to one rule's leaf range, then `id` so a backward index scan finds
+ * the maximum id for that rule without a sort or a table-wide aggregate. The same physical order
+ * is what enables the Index Only Scan plan change for the unchanged aggregate described below;
+ * it would also be what a correlated per-rule lookup needs, if this repository's test
+ * infrastructure allowed one (see the comment on the service method for that trade-off).
+ *
+ * This migration ships ONLY the index; the query above is not rewritten. Measured externally
+ * against a Postgres 17.10 rebuild with production planner settings, loaded with 5,421,152 rows
+ * (730 MB) and the 17 `trading_rule` rows in real production distribution (not reproducible
+ * from this repository):
+ * - Without this index: Parallel Seq Scan, 93,486 buffer blocks, 8,181 ms (warm).
+ * - With this index: Parallel Index Only Scan, Heap Fetches: 0, 15,020 buffer blocks, 5,676 ms
+ *   (warm).
+ * The planner picks this index for exactly the query above with no code change — this migration
+ * is therefore not a no-op. The index itself is 116 MB. That matches the fresh production
+ * measurement above (489 ms, shared hit=3734 read=85614, only 4.2% cache hit rate, 698 MB
+ * table): a 116 MB index has a realistic chance of staying resident in the 1 GB
+ * `shared_buffers`; the substantially larger table demonstrably does not. A correlated per-rule
+ * lookup (`SELECT r.id, (SELECT MAX(o.id) FROM trading_order o WHERE o."tradingRuleId" = r.id)
+ * FROM trading_rule r;`) would be faster still with this index — Index Only Scan Backward, 52
+ * buffer blocks, 1.5 ms (warm), measured on the same rebuild (same external measurement
+ * disclaimer: not reproducible from this repository) — but is NOT shipped here: it would need a
+ * correlated subquery that this repository's pg-mem-based test suite (pg-mem 3.0.14) cannot
+ * execute. See the comment on `TradingRuleService.getCurrentTradingOrders` for that trade-off.
+ *
+ * The existing single-column index `IDX_f862025cb7ca5a2d66d14fb89a` on
+ * `trading_order ("tradingRuleId")` is NOT removed by this migration. It was created by
+ * `AddForeignKeyIndexes1779802432879` (`CREATE INDEX "IDX_f862025cb7ca5a2d66d14fb89a" ON
+ * "trading_order" ("tradingRuleId")`). The new composite index makes that single-column index
+ * functionally redundant for most purposes (any query that can use the single-column index on
+ * `tradingRuleId` can equally use the new composite, since `tradingRuleId` is its leading
+ * column), but dropping the old index is out of scope for this change and is left for a
+ * separate, later migration.
+ *
+ * CREATE INDEX CONCURRENTLY is not used: migrations in this codebase run transactionally and
+ * boot-blockingly (see `src/config/config.ts`, `migrationsRun` gated by the `SQL_MIGRATE` env
+ * var). CREATE INDEX CONCURRENTLY is not allowed inside a transaction and would crash the
+ * migration.
+ *
+ * Lock behaviour, stated precisely: all pending migrations run inside a single database
+ * transaction (TypeORM default `migrationsTransactionMode: "all"`), and PostgreSQL only
+ * releases locks at COMMIT, not at the end of each statement. Evidence:
+ * `node_modules/typeorm/data-source/DataSource.js` (`migrationExecutor.transaction =
+ * options?.transaction || this.options?.migrationsTransactionMode || "all"` — default `"all"`);
+ * `src/config/config.ts` only sets `migrationsRun` and never overrides
+ * `migrationsTransactionMode` (corroborated by the comment in
+ * `src/shared/models/asset/__tests__/add-binance-custody-assets-ondo-ada.migration.spec.ts` —
+ * "no migrationsTransactionMode override → default 'all'");
+ * `node_modules/typeorm/migration/MigrationExecutor.js` starts one transaction for pending
+ * migrations and commits only at the end. A plain CREATE INDEX holds a SHARE lock for the
+ * entire build; reads continue throughout, but writes to the table are blocked while that lock
+ * is held. Because locks are held until COMMIT, if other migrations are pending in the same
+ * batch, this index's SHARE lock is held until all of them commit together, not just until this
+ * statement finishes. `SET LOCAL lock_timeout` bounds only how long we wait to ACQUIRE the lock,
+ * not how long we hold it once acquired, and it is scoped to the whole transaction — that is
+ * exactly why it is set only once at the top of `up()` and once at the top of `down()`, not per
+ * statement (there is only one `CREATE INDEX` / one `DROP INDEX` in this migration).
+ *
+ * Risk framing: this migration runs boot-blockingly at app startup (`migrationsRun`, gated by
+ * the `SQL_MIGRATE` env var), so the starting instance itself is not yet serving requests and is
+ * not itself a writer. Concurrent writers would be a still-running predecessor instance during a
+ * rolling deploy, or external consumers. If a lock conflict occurs, the migration aborts after
+ * `lock_timeout` and so does the app start — that is fail-closed and intentional, but it is a
+ * deploy abort and must be named as such.
+ *
+ * `down()` reverses this with `DROP INDEX` and is subject to a stricter lock: PostgreSQL takes
+ * an ACCESS EXCLUSIVE lock for `DROP INDEX` (vs. the SHARE lock `CREATE INDEX` takes above), and
+ * ACCESS EXCLUSIVE conflicts with every other lock mode, including the AccessShareLock a plain
+ * `SELECT` takes — so `down()` blocks reads as well as writes, not writes alone. Because
+ * `down()` also runs inside the single-batch transaction (same TypeORM default
+ * `migrationsTransactionMode: "all"`), the same hold-until-COMMIT reasoning applies.
+ *
+ * Honest disclaimer: whether the Postgres planner will actually pick this new index for the
+ * unchanged aggregate query above has NOT been verified in production itself, because the index
+ * does not exist there yet. The rebuild measurement above already shows that the plan changes
+ * from Parallel Seq Scan to Parallel Index Only Scan when this index exists — but that is still
+ * not confirmation on production itself. As a point of reference (not a guarantee), the same
+ * style of prediction was made for the `created` index in
+ * `AddTradingOrderCreatedIndex1785470000000` (selectivity + cost-model reasoning only, no prior
+ * plan-change observation) and was confirmed after that deploy (measured externally against
+ * production; not reproducible from this repository): the query's plan changed from a Seq Scan
+ * to an Index Scan, execution time dropped from 141.283 ms to 30.3 ms, and buffer reads dropped
+ * from 89,282 to 2,716.
+ *
+ * Index name: `IDX_710fd49e19d248643cb2afa70f` on `trading_order ("tradingRuleId", "id")`.
+ * This is not an arbitrary name but the deterministic name TypeORM's DefaultNamingStrategy would
+ * generate itself, since custom index naming is disallowed by CONTRIBUTING.md. The name is
+ * `IDX_` followed by the first 26 hex characters of `sha1('trading_order_id_tradingRuleId')`
+ * (table name + `_` + the two column names `id` and `tradingRuleId` sorted alphabetically and
+ * joined with `_`, per TypeORM's DefaultNamingStrategy — `id` sorts before `tradingRuleId`).
+ * The physical index column order remains `("tradingRuleId", "id")` as required for the equality
+ * + max-id access path above; only the name derivation sorts the column names.
+ *
+ * @class
+ * @implements {MigrationInterface}
+ */
+module.exports = class AddTradingOrderRuleIdIndex1785510000000 {
+  name = 'AddTradingOrderRuleIdIndex1785510000000';
+
+  /**
+   * @param {QueryRunner} queryRunner
+   */
+  async up(queryRunner) {
+    // SET LOCAL is scoped to the whole transaction. Bounds WAIT time to acquire the lock, not how
+    // long the lock is held. Set once: this migration has a single CREATE INDEX statement.
+    await queryRunner.query(`SET LOCAL lock_timeout = '5s'`);
+    await queryRunner.query(
+      `CREATE INDEX "IDX_710fd49e19d248643cb2afa70f" ON "trading_order" ("tradingRuleId", "id")`,
+    );
+  }
+
+  /**
+   * @param {QueryRunner} queryRunner
+   */
+  async down(queryRunner) {
+    // SET LOCAL is scoped to the whole transaction. Bounds WAIT time to acquire the lock, not how
+    // long the lock is held. Set once: this migration has a single DROP INDEX statement.
+    await queryRunner.query(`SET LOCAL lock_timeout = '5s'`);
+    await queryRunner.query(`DROP INDEX "public"."IDX_710fd49e19d248643cb2afa70f"`);
+  }
+};

src/shared/models/setting/__tests__/setting.repository.spec.ts

@@ -1,6 +1,35 @@
+import { Like } from 'typeorm';
 import { Setting } from '../setting.entity';
 import { SettingRepository } from '../setting.repository';
 
+describe('SettingRepository.getStatusSettings', () => {
+  function repositoryWithFind(result: Setting[]) {
+    const repository = Object.create(SettingRepository.prototype) as SettingRepository;
+    const find = jest.fn().mockResolvedValue(result);
+    Object.defineProperty(repository, 'find', { value: find });
+    return { repository, find };
+  }
+
+  it('finds status settings in ascending id order', async () => {
+    const settings = [Object.assign(new Setting(), { id: 1, key: 'paymentStatus', value: 'active' })];
+    const { repository, find } = repositoryWithFind(settings);
+
+    await expect(repository.getStatusSettings()).resolves.toBe(settings);
+
+    expect(find).toHaveBeenCalledWith({
+      where: { key: Like('%Status') },
+      order: { id: 'ASC' },
+    });
+  });
+
+  it('passes through an empty result', async () => {
+    const settings: Setting[] = [];
+    const { repository } = repositoryWithFind(settings);
+
+    await expect(repository.getStatusSettings()).resolves.toBe(settings);
+  });
+});
+
 describe('SettingRepository.setDateMax', () => {
   function repositoryWithTransaction(transactionManager: Record<string, jest.Mock>) {
     const repository = Object.create(SettingRepository.prototype) as SettingRepository;

src/shared/models/setting/setting.repository.ts

@@ -1,6 +1,6 @@
 import { Injectable } from '@nestjs/common';
 import { CachedRepository } from 'src/shared/repositories/cached.repository';
-import { EntityManager } from 'typeorm';
+import { EntityManager, Like } from 'typeorm';
 import { Setting } from './setting.entity';
 
 @Injectable()
@@ -31,4 +31,8 @@ export class SettingRepository extends CachedRepository<Setting> {
 
     this.invalidateCache();
   }
+
+  async getStatusSettings(): Promise<Setting[]> {
+    return this.find({ where: { key: Like('%Status') }, order: { id: 'ASC' } });
+  }
 }

src/shared/models/setting/setting.service.ts

@@ -16,6 +16,12 @@ export class SettingService {
     return this.settingRepo.find();
   }
 
+  // Loads only settings whose key ends with "Status" instead of transferring the entire table: in production,
+  // getAll() transfers about 1.5 MB, including one entry of about 1.5 MB, to return only a few bytes of status data.
+  async getStatusSettings(): Promise<Setting[]> {
+    return this.settingRepo.getStatusSettings();
+  }
+
   async get(key: string, defaultValue?: string): Promise<string | undefined> {
     return this.settingRepo.findOneBy({ key }).then((d) => d?.value ?? defaultValue);
   }