From 845cf4ce65d75aea0211487d82c6e8b923d26fb2 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Thu, 24 Sep 2026 18:15:10 -0400 Subject: [PATCH 01/16] feat(fx): add resilient provider caching and stale-data semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bounded TTL cache with primary→fallback providers, freshness metadata, quote versioning, stampede-safe singleflight, and reject_stale transfer pricing so outages cannot silently price on expired rates. Transfers bind quote identity (quoteId/quoteVersion) at creation. Closes #129 --- CHANGELOG.md | 8 + src/config/index.js | 10 + src/controllers/rateController.js | 8 +- src/controllers/transferController.js | 2 + src/services/fxCacheService.js | 192 ++++++++++++++ src/services/fxProviders.js | 146 +++++++++++ src/services/quoteService.js | 239 ++++++++++++++++- src/services/rateService.js | 121 +++++++-- src/services/transferService.js | 10 +- src/store/index.js | 14 + src/validators/transferValidator.js | 3 + test/config.test.js | 45 ++++ test/fxCache.test.js | 360 ++++++++++++++++++++++++++ 13 files changed, 1125 insertions(+), 33 deletions(-) create mode 100644 src/services/fxCacheService.js create mode 100644 src/services/fxProviders.js create mode 100644 test/fxCache.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index a8e4e0a..f6aa9d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,14 @@ When preparing a new release: ### Added +- Resilient FX provider caching with bounded TTL, deterministic primary→fallback + provider order, freshness metadata on rates/quotes, quote versioning + (`quoteId` / `quoteVersion`), and explicit `reject_stale` / `allow_stale` + policies. Synchronous singleflight prevents provider stampedes. Transfer + creation binds quote identity (optional client `quoteId`, otherwise a freshly + minted quote) so transfer pricing cannot silently drift. Config knobs: + `FX_CACHE_TTL_MS`, `FX_STALE_GRACE_MS`, `FX_QUOTE_TTL_MS`, + `FX_ALLOW_STALE_TRANSFERS`. - Cursor pagination for `GET /api/transfers` and `GET /api/audit`. Pass `?cursor=` (with optional `?order=asc|desc`) to page by an indexed position instead of a row offset; responses carry a `pageInfo` block with diff --git a/src/config/index.js b/src/config/index.js index 41f9a71..bb11f4c 100644 --- a/src/config/index.js +++ b/src/config/index.js @@ -59,6 +59,16 @@ const config = { ratesMaxAge: parseInt(process.env.CACHE_RATES_MAX_AGE_SECONDS, 10) || 10, }, + // FX provider cache: bounded TTL, stale grace, and transfer pricing policy. + // Transfers reject stale rates by default so an outage cannot silently price + // a remittance on an expired quote. Display paths may opt into allow_stale. + fx: { + cacheTtlMs: parseInt(process.env.FX_CACHE_TTL_MS, 10) || 30 * 1000, + staleGraceMs: parseInt(process.env.FX_STALE_GRACE_MS, 10) || 60 * 1000, + allowStaleForTransfers: process.env.FX_ALLOW_STALE_TRANSFERS === 'true', + quoteTtlMs: parseInt(process.env.FX_QUOTE_TTL_MS, 10) || 60 * 1000, + }, + pagination: { // Page size used when a request does not ask for one. defaultLimit: parseInt(process.env.PAGINATION_DEFAULT_LIMIT, 10) || 50, diff --git a/src/controllers/rateController.js b/src/controllers/rateController.js index 69afd81..beccfd1 100644 --- a/src/controllers/rateController.js +++ b/src/controllers/rateController.js @@ -10,12 +10,14 @@ const ApiError = require('../utils/ApiError'); /** * GET /api/rates - * Returns the supported currencies and their USD rate. + * Returns the supported currencies and their USD rate, plus freshness. */ function getRates(req, res) { + const listed = rateService.listRates(); res.json({ base: 'USD', - rates: rateService.listRates(), + rates: listed.rates, + freshness: listed.freshness, }); } @@ -33,7 +35,7 @@ function getRatePair(req, res) { /** * GET /api/quote?amount=&from=&to= - * Returns an FX quote including the fee breakdown. + * Returns an FX quote including the fee breakdown, quote identity, and freshness. */ function getQuote(req, res) { const { amount, from, to } = req.query; diff --git a/src/controllers/transferController.js b/src/controllers/transferController.js index 4fb2e72..0f30cce 100644 --- a/src/controllers/transferController.js +++ b/src/controllers/transferController.js @@ -58,6 +58,8 @@ function createTransfer(req, res) { amount: Number(req.body.amount), from: req.body.from, to: req.body.to, + // quoteId is part of the priced operation when the client binds one. + quoteId: req.body.quoteId || null, }); const transfer = transferService.createTransfer(req.body, req.id, { diff --git a/src/services/fxCacheService.js b/src/services/fxCacheService.js new file mode 100644 index 0000000..3fdb799 --- /dev/null +++ b/src/services/fxCacheService.js @@ -0,0 +1,192 @@ +'use strict'; + +const config = require('../config'); +const fxProviders = require('./fxProviders'); +const ApiError = require('../utils/ApiError'); + +/** + * Bounded FX rate cache with stampede protection and explicit stale policy. + * + * Cache key is a single global snapshot: RemitFlow converts via USD cross rates, + * so one provider pull feeds every pair. TTL bounds how long a snapshot is + * "fresh"; staleGraceMs bounds how long an expired snapshot may still be served + * under allow_stale / outage policy. + * + * Stampede control is synchronous singleflight: the first miss starts a + * provider fetch and marks the key in-flight. A re-entrant or concurrent caller + * that arrives while the fetch is running must NOT start another provider call. + * It either receives a still-within-grace cached snapshot marked stale, or a + * 503 with code FX_REFRESH_IN_PROGRESS when no usable cache exists. + */ + +const CACHE_KEY = 'rates:usd'; + +/** @type {Map} */ +const cache = new Map(); + +/** @type {Set} keys currently being refreshed */ +const inflight = new Set(); + +/** Counter for tests: how many times providers were actually invoked. */ +let providerFetchCount = 0; + +function ttlMs() { + return config.fx.cacheTtlMs; +} + +function staleGraceMs() { + return config.fx.staleGraceMs; +} + +/** + * Classify a cache entry relative to `now`. + * @param {{ fetchedAt: number, expiresAt: number }} entry + * @param {number} now + * @returns {'fresh'|'stale'|'expired'} + */ +function classify(entry, now) { + if (now < entry.expiresAt) return 'fresh'; + if (now < entry.expiresAt + staleGraceMs()) return 'stale'; + return 'expired'; +} + +/** + * Attach freshness metadata without mutating the stored entry. + * @param {object} entry + * @param {number} now + * @param {{ cacheHit: boolean, source: string }} extra + */ +function decorate(entry, now, extra) { + const status = classify(entry, now); + return { + ratesToUsd: { ...entry.ratesToUsd }, + providerId: entry.providerId, + fetchedAt: entry.fetchedAt, + expiresAt: entry.expiresAt, + ageMs: Math.max(0, now - entry.fetchedAt), + status, + stale: status !== 'fresh', + cacheHit: Boolean(extra.cacheHit), + source: extra.source, + }; +} + +/** + * Pull from providers (counted), store under TTL, return decorated fresh snapshot. + * @param {number} now + */ +function refresh(now) { + providerFetchCount += 1; + const snapshot = fxProviders.fetchWithFallback({ now }); + const entry = { + ratesToUsd: { ...snapshot.ratesToUsd }, + fetchedAt: snapshot.fetchedAt, + providerId: snapshot.providerId, + expiresAt: snapshot.fetchedAt + ttlMs(), + }; + cache.set(CACHE_KEY, entry); + return decorate(entry, now, { cacheHit: false, source: 'provider' }); +} + +/** + * Resolve a rate snapshot under an explicit stale policy. + * + * Policies: + * - `reject_stale` (transfer pricing): only fresh snapshots are usable. On + * provider outage we still refuse stale data rather than price a transfer + * on an expired rate. + * - `allow_stale` (display / quotes for inspection): may return a within-grace + * stale snapshot when providers are down or a refresh is in flight. The + * response always marks `stale: true` so callers cannot mistake it for current. + * + * @param {object} [opts] + * @param {number} [opts.now] + * @param {'reject_stale'|'allow_stale'} [opts.policy] + * @returns {ReturnType} + */ +function getSnapshot(opts = {}) { + const now = opts.now != null ? opts.now : Date.now(); + const policy = opts.policy || 'reject_stale'; + const entry = cache.get(CACHE_KEY); + + if (entry) { + const status = classify(entry, now); + if (status === 'fresh') { + return decorate(entry, now, { cacheHit: true, source: 'cache' }); + } + } + + // Stampede guard: someone is already talking to providers for this key. + if (inflight.has(CACHE_KEY)) { + if (entry && classify(entry, now) === 'stale' && policy === 'allow_stale') { + return decorate(entry, now, { cacheHit: true, source: 'cache-stale-inflight' }); + } + throw ApiError.serviceUnavailable('FX rate refresh in progress', { + code: 'FX_REFRESH_IN_PROGRESS', + }); + } + + inflight.add(CACHE_KEY); + try { + return refresh(now); + } catch (err) { + // Provider path failed. Under allow_stale, a within-grace entry is still + // usable for display — but it is visibly stale. Under reject_stale we + // never price with it. + if (entry && classify(entry, now) === 'stale' && policy === 'allow_stale') { + return decorate(entry, now, { cacheHit: true, source: 'cache-stale-outage' }); + } + throw err; + } finally { + inflight.delete(CACHE_KEY); + } +} + +/** + * Read the cached entry without refreshing. Used by tests and diagnostics. + * @param {number} [now] + */ +function peek(now = Date.now()) { + const entry = cache.get(CACHE_KEY); + if (!entry) return null; + return decorate(entry, now, { cacheHit: true, source: 'peek' }); +} + +/** Drop cache and inflight state. */ +function reset() { + cache.clear(); + inflight.clear(); + providerFetchCount = 0; +} + +function getProviderFetchCount() { + return providerFetchCount; +} + +/** + * Seed the cache (tests). `fetchedAt` defaults to now; TTL applied from config. + * @param {object} partial + */ +function seed(partial = {}) { + const now = partial.fetchedAt != null ? partial.fetchedAt : Date.now(); + const entry = { + ratesToUsd: { ...(partial.ratesToUsd || require('../config/rates').RATES_TO_USD) }, + fetchedAt: now, + providerId: partial.providerId || 'primary', + expiresAt: partial.expiresAt != null ? partial.expiresAt : now + ttlMs(), + }; + cache.set(CACHE_KEY, entry); + return entry; +} + +module.exports = { + CACHE_KEY, + getSnapshot, + peek, + reset, + seed, + classify, + getProviderFetchCount, + ttlMs, + staleGraceMs, +}; diff --git a/src/services/fxProviders.js b/src/services/fxProviders.js new file mode 100644 index 0000000..1f9879a --- /dev/null +++ b/src/services/fxProviders.js @@ -0,0 +1,146 @@ +'use strict'; + +const { RATES_TO_USD } = require('../config/rates'); +const ApiError = require('../utils/ApiError'); + +/** + * FX provider adapters. + * + * RemitFlow currently demos against a static rate table. These adapters wrap + * that table behind a provider interface so the cache layer can apply TTL, + * fallback, freshness, and stampede controls without rewriting callers. + * + * Providers are intentionally synchronous: the rest of the transfer path is + * sync, and stampede coverage re-enters mid-fetch the same way the + * idempotency suite does. + */ + +/** @typedef {{ providerId: string, ratesToUsd: Record, fetchedAt: number }} FxSnapshot */ + +/** + * Clone the configured rate table so callers cannot mutate the module constant. + * @returns {Record} + */ +function cloneRates() { + return { ...RATES_TO_USD }; +} + +/** + * Primary FX oracle. Throws when forced down (tests / outage simulation). + * @param {{ now?: number }} [opts] + * @returns {FxSnapshot} + */ +function primaryFetch(opts = {}) { + if (primaryFetch._down) { + throw ApiError.serviceUnavailable('Primary FX provider unavailable'); + } + const now = opts.now != null ? opts.now : Date.now(); + return { + providerId: 'primary', + ratesToUsd: cloneRates(), + fetchedAt: now, + }; +} + +/** + * Deterministic secondary oracle. Same table, distinct identity, so fallback + * is observable without inventing a second price source for the demo. + * @param {{ now?: number }} [opts] + * @returns {FxSnapshot} + */ +function fallbackFetch(opts = {}) { + if (fallbackFetch._down) { + throw ApiError.serviceUnavailable('Fallback FX provider unavailable'); + } + const now = opts.now != null ? opts.now : Date.now(); + return { + providerId: 'fallback', + ratesToUsd: cloneRates(), + fetchedAt: now, + }; +} + +primaryFetch._down = false; +fallbackFetch._down = false; + +/** Ordered registry. First success wins; order is the fallback contract. */ +const DEFAULT_PROVIDERS = [ + { id: 'primary', fetch: primaryFetch }, + { id: 'fallback', fetch: fallbackFetch }, +]; + +let providers = DEFAULT_PROVIDERS.slice(); + +/** + * Replace the provider list (tests). Pass null/undefined to restore defaults. + * @param {Array<{ id: string, fetch: Function }>|null|undefined} next + */ +function setProviders(next) { + providers = next && next.length ? next.slice() : DEFAULT_PROVIDERS.slice(); +} + +/** @returns {Array<{ id: string, fetch: Function }>} */ +function listProviders() { + return providers.slice(); +} + +/** + * Walk providers in order until one returns a snapshot. + * @param {{ now?: number }} [opts] + * @returns {FxSnapshot} + * @throws {ApiError} 503 when every provider fails. + */ +function fetchWithFallback(opts = {}) { + const errors = []; + for (const provider of providers) { + try { + const snapshot = provider.fetch(opts); + if (!snapshot || typeof snapshot.ratesToUsd !== 'object') { + throw new Error(`Provider ${provider.id} returned an invalid snapshot`); + } + return { + providerId: snapshot.providerId || provider.id, + ratesToUsd: { ...snapshot.ratesToUsd }, + fetchedAt: snapshot.fetchedAt != null ? snapshot.fetchedAt : (opts.now != null ? opts.now : Date.now()), + }; + } catch (err) { + errors.push({ + providerId: provider.id, + message: err && err.message ? err.message : String(err), + }); + } + } + + throw ApiError.serviceUnavailable('All FX providers failed', { + code: 'FX_PROVIDERS_DOWN', + attempted: errors, + }); +} + +/** + * Test helpers: force a named provider down or recover it. + * @param {'primary'|'fallback'|string} id + * @param {boolean} down + */ +function setProviderDown(id, down) { + if (id === 'primary') primaryFetch._down = Boolean(down); + if (id === 'fallback') fallbackFetch._down = Boolean(down); +} + +/** Reset provider health and registry to defaults. */ +function resetProviders() { + primaryFetch._down = false; + fallbackFetch._down = false; + providers = DEFAULT_PROVIDERS.slice(); +} + +module.exports = { + primaryFetch, + fallbackFetch, + fetchWithFallback, + setProviders, + listProviders, + setProviderDown, + resetProviders, + DEFAULT_PROVIDERS, +}; diff --git a/src/services/quoteService.js b/src/services/quoteService.js index 3cba17a..8a5a982 100644 --- a/src/services/quoteService.js +++ b/src/services/quoteService.js @@ -5,13 +5,27 @@ const rateService = require('./rateService'); const money = require('../utils/money'); const currency = require('../utils/currency'); const ApiError = require('../utils/ApiError'); +const { prefixedId } = require('../utils/ids'); +const { store } = require('../store'); /** - * Quote calculation. + * Quote calculation with versioning and explicit stale/error policy. + * * A quote tells the sender how much the recipient will receive after - * RemitFlow's fee and the FX conversion are applied. + * RemitFlow's fee and the FX conversion are applied. Each quote is given a + * stable identity (`quoteId` + `quoteVersion`) so transfer creation can bind + * to the exact quote the sender saw, rather than silently recomputing against + * a moved rate. + * + * Freshness: + * - Quotes always carry freshness metadata so stale data is visible. + * - Transfer pricing uses `reject_stale` and refuses a quote whose underlying + * FX snapshot is outside policy. */ +/** Monotonic version counter for quote identity within the process. */ +let quoteVersionSeq = 0; + /** * Compute the fee charged on a send amount. * Fee is a percentage of the amount plus a small flat component. @@ -23,14 +37,79 @@ function calculateFee(amount) { return money.round(percentFee + config.fee.flat); } +/** + * Build freshness fields from an FX snapshot. + * @param {object} snapshot + */ +function freshnessFromSnapshot(snapshot) { + return { + status: snapshot.status, + stale: snapshot.stale, + providerId: snapshot.providerId, + fetchedAt: new Date(snapshot.fetchedAt).toISOString(), + expiresAt: new Date(snapshot.expiresAt).toISOString(), + ageMs: snapshot.ageMs, + cacheHit: snapshot.cacheHit, + source: snapshot.source, + }; +} + +/** + * Persist a quote so transfer creation can bind to it later. + * @param {object} quote + */ +function remember(quote) { + store.quotes.set(quote.quoteId, quote); + return quote; +} + +/** + * Drop expired quotes opportunistically so the map cannot grow without bound. + * + * Only runs once the map crosses a size threshold so bulk transfer seeding + * (pagination cost tests) stays O(n) rather than O(n²) from a full scan on + * every mint. + * + * @param {number} now + */ +const QUOTE_GC_THRESHOLD = 256; + +function gcQuotes(now) { + if (store.quotes.size < QUOTE_GC_THRESHOLD) return; + for (const [id, quote] of store.quotes.entries()) { + const expiresAtMs = Date.parse(quote.quoteExpiresAt); + if (Number.isFinite(expiresAtMs) && expiresAtMs + config.fx.staleGraceMs < now) { + store.quotes.delete(id); + } + } + // If everything is still live (e.g. bulk seeding within quote TTL), evict the + // oldest half by insertion order so the map stays bounded. + if (store.quotes.size >= QUOTE_GC_THRESHOLD * 2) { + const excess = store.quotes.size - QUOTE_GC_THRESHOLD; + let removed = 0; + for (const id of store.quotes.keys()) { + store.quotes.delete(id); + removed += 1; + if (removed >= excess) break; + } + } +} + /** * Build a full quote for converting `amount` from `from` to `to`. + * + * Display/default policy is `allow_stale` so an outage still lets a client see + * a visibly-stale quote; transfer binding later enforces `reject_stale`. + * * @param {number} amount * @param {string} from * @param {string} to + * @param {object} [opts] + * @param {number} [opts.now] + * @param {'reject_stale'|'allow_stale'} [opts.policy] * @returns {object} quote breakdown. */ -function getQuote(amount, from, to) { +function getQuote(amount, from, to, opts = {}) { if (!money.isPositiveAmount(amount)) { throw ApiError.badRequest('amount must be a positive number'); } @@ -43,15 +122,31 @@ function getQuote(amount, from, to) { ); } + const now = opts.now != null ? opts.now : Date.now(); + const policy = opts.policy || 'allow_stale'; + gcQuotes(now); + const fromCode = currency.normalize(from); const toCode = currency.normalize(to); const numericAmount = money.round(Number(amount)); const fee = calculateFee(numericAmount); const amountAfterFee = money.round(numericAmount - fee); - const rate = rateService.getRate(fromCode, toCode); + + const snapshot = rateService.getSnapshot({ now, policy }); + if (!Object.prototype.hasOwnProperty.call(snapshot.ratesToUsd, fromCode)) { + throw ApiError.badRequest(`Unsupported source currency: ${from}`); + } + if (!Object.prototype.hasOwnProperty.call(snapshot.ratesToUsd, toCode)) { + throw ApiError.badRequest(`Unsupported target currency: ${to}`); + } + + const rate = snapshot.ratesToUsd[fromCode] / snapshot.ratesToUsd[toCode]; const receiveAmount = money.round(amountAfterFee * rate); + quoteVersionSeq += 1; - return { + const quote = { + quoteId: prefixedId('quote'), + quoteVersion: quoteVersionSeq, from: fromCode, to: toCode, sendAmount: numericAmount, @@ -59,10 +154,144 @@ function getQuote(amount, from, to) { amountAfterFee, rate: money.round(rate), receiveAmount, + stale: snapshot.stale, + freshness: freshnessFromSnapshot(snapshot), + quoteCreatedAt: new Date(now).toISOString(), + quoteExpiresAt: new Date(now + config.fx.quoteTtlMs).toISOString(), }; + + return remember(quote); +} + +/** + * Look up a previously issued quote. + * @param {string} quoteId + * @returns {object} + */ +function getQuoteById(quoteId) { + const quote = store.quotes.get(quoteId); + if (!quote) { + throw ApiError.notFound(`Quote not found: ${quoteId}`, { + code: 'QUOTE_NOT_FOUND', + }); + } + return quote; +} + +/** + * Decide whether a quote may be used under a named policy. + * + * - Quote TTL expiry always blocks transfer use (the sender must refresh). + * - Underlying FX `stale` blocks transfer use unless allowStaleForTransfers. + * - Display policy never throws for staleness; callers still see `stale: true`. + * + * @param {object} quote + * @param {object} [opts] + * @param {number} [opts.now] + * @param {'reject_stale'|'allow_stale'} [opts.policy] + * @returns {object} the same quote when usable + */ +function assertUsable(quote, opts = {}) { + const now = opts.now != null ? opts.now : Date.now(); + const policy = opts.policy || 'reject_stale'; + const expiresAtMs = Date.parse(quote.quoteExpiresAt); + + if (!Number.isFinite(expiresAtMs) || now > expiresAtMs) { + if (policy === 'allow_stale') { + return quote; + } + throw ApiError.conflict('Quote has expired; request a fresh quote', { + code: 'QUOTE_EXPIRED', + quoteId: quote.quoteId, + quoteVersion: quote.quoteVersion, + quoteExpiresAt: quote.quoteExpiresAt, + }); + } + + if (quote.stale || (quote.freshness && quote.freshness.stale)) { + const allow = + policy === 'allow_stale' || config.fx.allowStaleForTransfers === true; + if (!allow) { + throw ApiError.conflict( + 'Quote is based on a stale FX rate and cannot be used for transfer pricing', + { + code: 'QUOTE_STALE', + quoteId: quote.quoteId, + quoteVersion: quote.quoteVersion, + freshness: quote.freshness, + } + ); + } + } + + return quote; +} + +/** + * Resolve the quote a transfer will bind to. + * + * When `quoteId` is supplied the stored quote is loaded and checked under + * transfer policy, and the request amount/currencies must match so a client + * cannot bind a USD→EUR quote to a GBP→NGN transfer. + * + * When omitted, a fresh reject_stale quote is minted and bound. That keeps + * existing callers working while still attaching quote identity to every + * transfer. + * + * @param {object} data + * @param {object} [opts] + * @returns {object} + */ +function resolveForTransfer(data, opts = {}) { + const now = opts.now != null ? opts.now : Date.now(); + + if (data.quoteId) { + const quote = getQuoteById(data.quoteId); + assertUsable(quote, { now, policy: 'reject_stale' }); + + const fromCode = currency.normalize(data.from); + const toCode = currency.normalize(data.to); + const amount = money.round(Number(data.amount)); + + if ( + quote.from !== fromCode || + quote.to !== toCode || + quote.sendAmount !== amount + ) { + throw ApiError.conflict( + 'Quote identity does not match the transfer amount and currencies', + { + code: 'QUOTE_MISMATCH', + quoteId: quote.quoteId, + quoteVersion: quote.quoteVersion, + expected: { + from: quote.from, + to: quote.to, + sendAmount: quote.sendAmount, + }, + received: { from: fromCode, to: toCode, sendAmount: amount }, + } + ); + } + + return quote; + } + + return getQuote(data.amount, data.from, data.to, { + now, + policy: 'reject_stale', + }); +} + +function resetQuoteVersions() { + quoteVersionSeq = 0; } module.exports = { calculateFee, getQuote, + getQuoteById, + assertUsable, + resolveForTransfer, + resetQuoteVersions, }; diff --git a/src/services/rateService.js b/src/services/rateService.js index 95c7112..8f89680 100644 --- a/src/services/rateService.js +++ b/src/services/rateService.js @@ -1,51 +1,98 @@ 'use strict'; -const { RATES_TO_USD, SUPPORTED_CURRENCIES } = require('../config/rates'); +const { SUPPORTED_CURRENCIES } = require('../config/rates'); const money = require('../utils/money'); const currency = require('../utils/currency'); const ApiError = require('../utils/ApiError'); +const fxCacheService = require('./fxCacheService'); /** - * Foreign exchange helpers built on top of the mock rate table. + * Foreign exchange helpers built on the cached FX provider snapshot. + * + * Callers that only need a number keep using getRate/convert/getPair. Paths + * that must expose freshness (quotes, rate list responses) use the snapshot + * helpers so stale data is visible rather than silent. */ /** - * Return the full list of supported currencies and their USD rate. - * @returns {Array<{currency: string, rateToUsd: number}>} + * @param {object} [opts] + * @param {number} [opts.now] + * @param {'reject_stale'|'allow_stale'} [opts.policy] */ -function listRates() { - return SUPPORTED_CURRENCIES.map((currency) => ({ - currency, - rateToUsd: RATES_TO_USD[currency], - })); +function getSnapshot(opts = {}) { + return fxCacheService.getSnapshot({ + now: opts.now, + policy: opts.policy || 'reject_stale', + }); } /** - * Check whether a currency code is supported. - * @param {string} currency + * Return supported currencies and their USD rate, plus freshness metadata. + * Display-oriented: may surface a visibly-stale snapshot during outage. + * @param {object} [opts] + * @returns {{ rates: Array<{currency: string, rateToUsd: number}>, freshness: object }} + */ +function listRates(opts = {}) { + const snapshot = getSnapshot({ ...opts, policy: opts.policy || 'allow_stale' }); + return { + rates: SUPPORTED_CURRENCIES.map((code) => ({ + currency: code, + rateToUsd: snapshot.ratesToUsd[code], + })), + freshness: { + status: snapshot.status, + stale: snapshot.stale, + providerId: snapshot.providerId, + fetchedAt: new Date(snapshot.fetchedAt).toISOString(), + expiresAt: new Date(snapshot.expiresAt).toISOString(), + ageMs: snapshot.ageMs, + cacheHit: snapshot.cacheHit, + source: snapshot.source, + }, + }; +} + +/** + * Check whether a currency code is supported against the current snapshot. + * Falls back to the configured currency list when providers are unreachable + * so validation can still reject unknown codes without requiring a live pull. + * @param {string} code * @returns {boolean} */ -function isSupported(currency) { - return Object.prototype.hasOwnProperty.call(RATES_TO_USD, currency); +function isSupported(code) { + const normalized = currency.normalize(code); + if (!normalized) return false; + const peeked = fxCacheService.peek(); + if (peeked && Object.prototype.hasOwnProperty.call(peeked.ratesToUsd, normalized)) { + return true; + } + return SUPPORTED_CURRENCIES.includes(normalized); } /** * Compute the exchange rate to convert one unit of `from` into `to`. + * Uses reject_stale so transfer pricing never silently consumes expired data. * @param {string} from * @param {string} to + * @param {object} [opts] * @returns {number} */ -function getRate(from, to) { +function getRate(from, to, opts = {}) { const fromCode = currency.normalize(from); const toCode = currency.normalize(to); - if (!isSupported(fromCode)) { + const snapshot = getSnapshot({ + now: opts.now, + policy: opts.policy || 'reject_stale', + }); + + if (!Object.prototype.hasOwnProperty.call(snapshot.ratesToUsd, fromCode)) { throw ApiError.badRequest(`Unsupported source currency: ${from}`); } - if (!isSupported(toCode)) { + if (!Object.prototype.hasOwnProperty.call(snapshot.ratesToUsd, toCode)) { throw ApiError.badRequest(`Unsupported target currency: ${to}`); } - // Convert source -> USD -> target. - return RATES_TO_USD[fromCode] / RATES_TO_USD[toCode]; + + return snapshot.ratesToUsd[fromCode] / snapshot.ratesToUsd[toCode]; } /** @@ -53,26 +100,51 @@ function getRate(from, to) { * @param {number} amount * @param {string} from * @param {string} to + * @param {object} [opts] * @returns {number} */ -function convert(amount, from, to) { - const rate = getRate(from, to); +function convert(amount, from, to, opts = {}) { + const rate = getRate(from, to, opts); return money.round(amount * rate); } /** - * Describe a single currency pair, e.g. "USD-INR". + * Describe a single currency pair, e.g. "USD-INR", with freshness. * @param {string} from * @param {string} to - * @returns {{ from: string, to: string, rate: number }} + * @param {object} [opts] + * @returns {object} */ -function getPair(from, to) { +function getPair(from, to, opts = {}) { const fromCode = currency.normalize(from); const toCode = currency.normalize(to); + const snapshot = getSnapshot({ + now: opts.now, + policy: opts.policy || 'allow_stale', + }); + + if (!Object.prototype.hasOwnProperty.call(snapshot.ratesToUsd, fromCode)) { + throw ApiError.badRequest(`Unsupported source currency: ${from}`); + } + if (!Object.prototype.hasOwnProperty.call(snapshot.ratesToUsd, toCode)) { + throw ApiError.badRequest(`Unsupported target currency: ${to}`); + } + + const rate = snapshot.ratesToUsd[fromCode] / snapshot.ratesToUsd[toCode]; return { from: fromCode, to: toCode, - rate: money.round(getRate(fromCode, toCode)), + rate: money.round(rate), + freshness: { + status: snapshot.status, + stale: snapshot.stale, + providerId: snapshot.providerId, + fetchedAt: new Date(snapshot.fetchedAt).toISOString(), + expiresAt: new Date(snapshot.expiresAt).toISOString(), + ageMs: snapshot.ageMs, + cacheHit: snapshot.cacheHit, + source: snapshot.source, + }, }; } @@ -82,4 +154,5 @@ module.exports = { getRate, convert, getPair, + getSnapshot, }; diff --git a/src/services/transferService.js b/src/services/transferService.js index 1517f62..2f1f4f1 100644 --- a/src/services/transferService.js +++ b/src/services/transferService.js @@ -243,7 +243,10 @@ function createTransfer(data, requestId, idempotency) { * @returns {object} */ function createTransferUnchecked(data, requestId, idempotency) { - const quote = quoteService.getQuote(data.amount, data.from, data.to); + // Bind transfer pricing to a versioned quote identity. When the client + // supplies quoteId we enforce match + reject_stale policy; otherwise we mint + // a fresh quote so every transfer still carries quote provenance. + const quote = quoteService.resolveForTransfer(data); const settlement = stellarService.submitPayment({ amount: quote.sendAmount, currency: quote.from, @@ -259,6 +262,11 @@ function createTransferUnchecked(data, requestId, idempotency) { fee: quote.fee, rate: quote.rate, receiveAmount: quote.receiveAmount, + quoteId: quote.quoteId, + quoteVersion: quote.quoteVersion, + rateProvider: quote.freshness && quote.freshness.providerId, + rateFetchedAt: quote.freshness && quote.freshness.fetchedAt, + rateStale: Boolean(quote.stale), status: TRANSFER_STATUS.PENDING, stellar: settlement, createdAt: new Date().toISOString(), diff --git a/src/store/index.js b/src/store/index.js index 0024fac..f1ebdf9 100644 --- a/src/store/index.js +++ b/src/store/index.js @@ -3,6 +3,16 @@ const auditService = require('../services/auditService'); const { OrderedIndex } = require('../utils/orderedIndex'); +// Lazy requires avoid a circular init with quoteService -> store. +function resetFxState() { + const fxCacheService = require('../services/fxCacheService'); + const fxProviders = require('../services/fxProviders'); + const quoteService = require('../services/quoteService'); + fxCacheService.reset(); + fxProviders.resetProviders(); + quoteService.resetQuoteVersions(); +} + /** * Simple in-memory data store. * Data lives only for the lifetime of the process; restarting the @@ -23,6 +33,8 @@ const store = { // local so it shares the transfers' lifetime: a replay can never outlive the // transfer it would replay. idempotency: new Map(), + // Versioned FX quotes awaiting transfer binding. GC'd by quoteService. + quotes: new Map(), }; /** Remove all records from the store. Primarily used in tests/seeding. */ @@ -31,7 +43,9 @@ function reset() { store.transfers.clear(); store.transferIndex.reset(); store.idempotency.clear(); + store.quotes.clear(); auditService.reset(); + resetFxState(); } module.exports = { diff --git a/src/validators/transferValidator.js b/src/validators/transferValidator.js index 830f049..20a51b6 100644 --- a/src/validators/transferValidator.js +++ b/src/validators/transferValidator.js @@ -39,6 +39,9 @@ function validateCreateTransfer(req) { if (from && to && from === to) { errors.push('from and to currencies must differ'); } + if (body.quoteId != null && (typeof body.quoteId !== 'string' || body.quoteId.trim() === '')) { + errors.push('quoteId must be a non-empty string when provided'); + } return errors; } diff --git a/test/config.test.js b/test/config.test.js index 45c5d39..866abb4 100644 --- a/test/config.test.js +++ b/test/config.test.js @@ -86,3 +86,48 @@ test('config respects cache environment variables', () => { } }); + + +test('config loads default FX cache options', () => { + delete require.cache[require.resolve('../src/config')]; + const config = require('../src/config'); + + assert.ok(config.fx); + assert.equal(config.fx.cacheTtlMs, 30 * 1000); + assert.equal(config.fx.staleGraceMs, 60 * 1000); + assert.equal(config.fx.allowStaleForTransfers, false); + assert.equal(config.fx.quoteTtlMs, 60 * 1000); +}); + +test('config respects FX environment variables', () => { + const originalEnv = { + FX_CACHE_TTL_MS: process.env.FX_CACHE_TTL_MS, + FX_STALE_GRACE_MS: process.env.FX_STALE_GRACE_MS, + FX_ALLOW_STALE_TRANSFERS: process.env.FX_ALLOW_STALE_TRANSFERS, + FX_QUOTE_TTL_MS: process.env.FX_QUOTE_TTL_MS, + }; + + try { + process.env.FX_CACHE_TTL_MS = '5000'; + process.env.FX_STALE_GRACE_MS = '15000'; + process.env.FX_ALLOW_STALE_TRANSFERS = 'true'; + process.env.FX_QUOTE_TTL_MS = '9000'; + + delete require.cache[require.resolve('../src/config')]; + const config = require('../src/config'); + + assert.equal(config.fx.cacheTtlMs, 5000); + assert.equal(config.fx.staleGraceMs, 15000); + assert.equal(config.fx.allowStaleForTransfers, true); + assert.equal(config.fx.quoteTtlMs, 9000); + } finally { + for (const key of Object.keys(originalEnv)) { + if (originalEnv[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = originalEnv[key]; + } + } + delete require.cache[require.resolve('../src/config')]; + } +}); diff --git a/test/fxCache.test.js b/test/fxCache.test.js new file mode 100644 index 0000000..719d815 --- /dev/null +++ b/test/fxCache.test.js @@ -0,0 +1,360 @@ +'use strict'; + +const { test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); + +const { reset } = require('../src/store'); +const config = require('../src/config'); +const fxCacheService = require('../src/services/fxCacheService'); +const fxProviders = require('../src/services/fxProviders'); +const quoteService = require('../src/services/quoteService'); +const transferService = require('../src/services/transferService'); +const rateService = require('../src/services/rateService'); +const ApiError = require('../src/utils/ApiError'); + +const ORIGINAL_TTL = config.fx.cacheTtlMs; +const ORIGINAL_GRACE = config.fx.staleGraceMs; +const ORIGINAL_QUOTE_TTL = config.fx.quoteTtlMs; +const ORIGINAL_ALLOW_STALE = config.fx.allowStaleForTransfers; + +beforeEach(() => { + reset(); + config.fx.cacheTtlMs = 1_000; + config.fx.staleGraceMs = 5_000; + config.fx.quoteTtlMs = 2_000; + config.fx.allowStaleForTransfers = false; +}); + +afterEach(() => { + config.fx.cacheTtlMs = ORIGINAL_TTL; + config.fx.staleGraceMs = ORIGINAL_GRACE; + config.fx.quoteTtlMs = ORIGINAL_QUOTE_TTL; + config.fx.allowStaleForTransfers = ORIGINAL_ALLOW_STALE; + reset(); +}); + +const PAYLOAD = { + senderName: 'Ada', + recipientName: 'Bob', + amount: 100, + from: 'USD', + to: 'EUR', +}; + +// --------------------------------------------------------------------------- +// Cache TTL / freshness +// --------------------------------------------------------------------------- + +test('fresh snapshot is served from cache within TTL without re-fetching', () => { + const t0 = 1_000_000; + const first = fxCacheService.getSnapshot({ now: t0, policy: 'reject_stale' }); + assert.equal(first.status, 'fresh'); + assert.equal(first.stale, false); + assert.equal(first.cacheHit, false); + assert.equal(fxCacheService.getProviderFetchCount(), 1); + + const second = fxCacheService.getSnapshot({ now: t0 + 500, policy: 'reject_stale' }); + assert.equal(second.status, 'fresh'); + assert.equal(second.cacheHit, true); + assert.equal(second.providerId, first.providerId); + assert.equal(fxCacheService.getProviderFetchCount(), 1); +}); + +test('expiry triggers a refresh and the new snapshot is fresh', () => { + const t0 = 2_000_000; + fxCacheService.getSnapshot({ now: t0, policy: 'reject_stale' }); + assert.equal(fxCacheService.getProviderFetchCount(), 1); + + const after = fxCacheService.getSnapshot({ + now: t0 + config.fx.cacheTtlMs + 1, + policy: 'reject_stale', + }); + assert.equal(after.status, 'fresh'); + assert.equal(after.cacheHit, false); + assert.equal(fxCacheService.getProviderFetchCount(), 2); +}); + +// --------------------------------------------------------------------------- +// Provider failure + deterministic fallback +// --------------------------------------------------------------------------- + +test('primary failure falls back to the secondary provider deterministically', () => { + fxProviders.setProviderDown('primary', true); + const snapshot = fxCacheService.getSnapshot({ now: 3_000_000, policy: 'reject_stale' }); + assert.equal(snapshot.providerId, 'fallback'); + assert.equal(snapshot.status, 'fresh'); + assert.equal(fxCacheService.getProviderFetchCount(), 1); +}); + +test('all providers down with no cache rejects under reject_stale', () => { + fxProviders.setProviderDown('primary', true); + fxProviders.setProviderDown('fallback', true); + assert.throws( + () => fxCacheService.getSnapshot({ now: 4_000_000, policy: 'reject_stale' }), + (err) => + err instanceof ApiError && + err.statusCode === 503 && + err.details && + err.details.code === 'FX_PROVIDERS_DOWN' + ); +}); + +test('all providers down serves visibly-stale cache under allow_stale within grace', () => { + const t0 = 5_000_000; + fxCacheService.getSnapshot({ now: t0, policy: 'reject_stale' }); + fxProviders.setProviderDown('primary', true); + fxProviders.setProviderDown('fallback', true); + + const stale = fxCacheService.getSnapshot({ + now: t0 + config.fx.cacheTtlMs + 1, + policy: 'allow_stale', + }); + assert.equal(stale.stale, true); + assert.equal(stale.status, 'stale'); + assert.equal(stale.source, 'cache-stale-outage'); + assert.ok(stale.ageMs > config.fx.cacheTtlMs); +}); + +test('fallback order is primary-then-fallback (deterministic)', () => { + const seen = []; + fxProviders.setProviders([ + { + id: 'primary', + fetch: () => { + seen.push('primary'); + throw new Error('primary down'); + }, + }, + { + id: 'fallback', + fetch: ({ now }) => { + seen.push('fallback'); + return { + providerId: 'fallback', + ratesToUsd: { USD: 1, EUR: 1.08 }, + fetchedAt: now, + }; + }, + }, + ]); + + const snapshot = fxCacheService.getSnapshot({ now: 6_000_000 }); + assert.deepEqual(seen, ['primary', 'fallback']); + assert.equal(snapshot.providerId, 'fallback'); +}); + +// --------------------------------------------------------------------------- +// Stampede prevention +// --------------------------------------------------------------------------- + +test('stampede: re-entrant refresh does not start a second provider fetch', () => { + let reentrantError = null; + let innerCalls = 0; + + fxProviders.setProviders([ + { + id: 'primary', + fetch: ({ now }) => { + // Re-enter while this fetch is in flight — the original failure mode + // where every concurrent miss stampeded the provider. + try { + fxCacheService.getSnapshot({ now, policy: 'reject_stale' }); + } catch (err) { + reentrantError = err; + } + innerCalls += 1; + return { + providerId: 'primary', + ratesToUsd: { USD: 1, EUR: 1.08, GBP: 1.27, INR: 0.012 }, + fetchedAt: now, + }; + }, + }, + ]); + + const snapshot = fxCacheService.getSnapshot({ now: 7_000_000, policy: 'reject_stale' }); + assert.equal(snapshot.providerId, 'primary'); + // Outer refresh counted once; the re-entrant caller must not have started + // another provider invocation (innerCalls stays 1 for the outer fetch body). + assert.equal(fxCacheService.getProviderFetchCount(), 1); + assert.equal(innerCalls, 1); + assert.ok(reentrantError instanceof ApiError); + assert.equal(reentrantError.details.code, 'FX_REFRESH_IN_PROGRESS'); +}); + +test('stampede under allow_stale returns within-grace cache instead of a second fetch', () => { + const t0 = 8_000_000; + fxCacheService.seed({ + fetchedAt: t0, + expiresAt: t0 + config.fx.cacheTtlMs, + providerId: 'primary', + }); + + let providerHits = 0; + fxProviders.setProviders([ + { + id: 'primary', + fetch: ({ now }) => { + providerHits += 1; + // Re-enter as a stampede during the refresh that expiry triggered. + const nested = fxCacheService.getSnapshot({ + now: t0 + config.fx.cacheTtlMs + 1, + policy: 'allow_stale', + }); + assert.equal(nested.stale, true); + assert.equal(nested.source, 'cache-stale-inflight'); + return { + providerId: 'primary', + ratesToUsd: nested.ratesToUsd, + fetchedAt: now, + }; + }, + }, + ]); + + const refreshed = fxCacheService.getSnapshot({ + now: t0 + config.fx.cacheTtlMs + 1, + policy: 'allow_stale', + }); + assert.equal(refreshed.status, 'fresh'); + assert.equal(providerHits, 1); + assert.equal(fxCacheService.getProviderFetchCount(), 1); +}); + +// --------------------------------------------------------------------------- +// Quote versioning, visibility, and transfer binding +// --------------------------------------------------------------------------- + +test('quotes expose identity and freshness metadata', () => { + const quote = quoteService.getQuote(100, 'USD', 'EUR'); + assert.equal(typeof quote.quoteId, 'string'); + assert.ok(quote.quoteId.startsWith('quote_')); + assert.equal(typeof quote.quoteVersion, 'number'); + assert.equal(quote.stale, false); + assert.equal(quote.freshness.status, 'fresh'); + assert.equal(typeof quote.freshness.providerId, 'string'); + assert.equal(typeof quote.quoteExpiresAt, 'string'); +}); + +test('stale quote is visible but cannot be used for transfer pricing', () => { + // Use wall-clock time so transfer creation (which reads Date.now) and the + // quote TTL share the same clock. Only the FX snapshot is forced stale. + const now = Date.now(); + fxCacheService.seed({ + fetchedAt: now - config.fx.cacheTtlMs - 1, + expiresAt: now - 1, + providerId: 'primary', + }); + fxProviders.setProviderDown('primary', true); + fxProviders.setProviderDown('fallback', true); + + const quote = quoteService.getQuote(100, 'USD', 'EUR', { + now, + policy: 'allow_stale', + }); + assert.equal(quote.stale, true); + assert.equal(quote.freshness.status, 'stale'); + + assert.throws( + () => quoteService.assertUsable(quote, { now, policy: 'reject_stale' }), + (err) => + err instanceof ApiError && + err.statusCode === 409 && + err.details.code === 'QUOTE_STALE' + ); + + assert.throws( + () => + transferService.createTransfer( + { ...PAYLOAD, quoteId: quote.quoteId }, + 'req-stale' + ), + (err) => err instanceof ApiError && err.details.code === 'QUOTE_STALE' + ); +}); + +test('expired quote cannot be bound to a transfer (regression: stale mistaken for current)', () => { + const t0 = 10_000_000; + const quote = quoteService.getQuote(100, 'USD', 'EUR', { now: t0 }); + assert.equal(quote.stale, false); + + assert.throws( + () => + quoteService.assertUsable(quote, { + now: t0 + config.fx.quoteTtlMs + 1, + policy: 'reject_stale', + }), + (err) => + err instanceof ApiError && + err.details.code === 'QUOTE_EXPIRED' + ); +}); + +test('transfer creation binds quote identity (quoteId + quoteVersion)', () => { + const quote = quoteService.getQuote(100, 'USD', 'EUR'); + const transfer = transferService.createTransfer( + { ...PAYLOAD, quoteId: quote.quoteId }, + 'req-bind' + ); + + assert.equal(transfer.quoteId, quote.quoteId); + assert.equal(transfer.quoteVersion, quote.quoteVersion); + assert.equal(transfer.rate, quote.rate); + assert.equal(transfer.receiveAmount, quote.receiveAmount); + assert.equal(transfer.rateProvider, quote.freshness.providerId); + assert.equal(transfer.rateStale, false); +}); + +test('transfer rejects a quoteId that does not match amount/currencies', () => { + const quote = quoteService.getQuote(100, 'USD', 'EUR'); + assert.throws( + () => + transferService.createTransfer( + { ...PAYLOAD, amount: 200, quoteId: quote.quoteId }, + 'req-mismatch' + ), + (err) => + err instanceof ApiError && + err.details.code === 'QUOTE_MISMATCH' + ); +}); + +test('transfer without quoteId still mints and binds a fresh quote (compat)', () => { + const transfer = transferService.createTransfer(PAYLOAD, 'req-compat'); + assert.equal(typeof transfer.quoteId, 'string'); + assert.equal(typeof transfer.quoteVersion, 'number'); + assert.equal(transfer.rateStale, false); + const stored = quoteService.getQuoteById(transfer.quoteId); + assert.equal(stored.sendAmount, transfer.sendAmount); +}); + +test('provider outage blocks transfer pricing rather than using silent stale rates', () => { + // Regression for the original failure mode: outage must not price transfers + // on an expired rate that looks "current" because freshness was invisible. + const t0 = 11_000_000; + fxCacheService.seed({ + fetchedAt: t0 - config.fx.cacheTtlMs - 1, + expiresAt: t0 - 1, + providerId: 'primary', + }); + fxProviders.setProviderDown('primary', true); + fxProviders.setProviderDown('fallback', true); + + assert.throws( + () => transferService.createTransfer(PAYLOAD, 'req-outage'), + (err) => + err instanceof ApiError && + (err.details.code === 'FX_PROVIDERS_DOWN' || + err.details.code === 'QUOTE_STALE' || + err.statusCode === 503) + ); +}); + +test('rate list surfaces freshness so stale data cannot be mistaken for current', () => { + const listed = rateService.listRates({ now: 12_000_000 }); + assert.ok(Array.isArray(listed.rates)); + assert.ok(listed.rates.length > 0); + assert.equal(listed.freshness.stale, false); + assert.equal(listed.freshness.status, 'fresh'); + assert.equal(typeof listed.freshness.fetchedAt, 'string'); +}); From 321b1dac8cce694c2cf1a541b4cfdc764085fc53 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sat, 3 Oct 2026 03:25:17 -0400 Subject: [PATCH 02/16] fix(fx): revalidate quote freshness at transfer binding Reclassify the quoted FX snapshot at use time while preserving its price and identity. Enforce quote and FX grace expiry boundaries, and return current stale metadata for the explicit opt-in policy. Validation: 20 focused FX tests and 278 repository tests pass. A real local HTTP quote-to-transfer flow previously accepted an expired FX snapshot with 201 and rateStale=false; it now returns 409 QUOTE_STALE before creating a transfer. Explicit within-grace stale policy returns 201 with rateStale=true and unchanged quote terms; a fresh quote returns 201 with rateStale=false. The application uses its documented in-memory store and mock Stellar adapter. --- src/services/quoteService.js | 47 ++++++++++++++++++------ test/fxCache.test.js | 70 ++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 10 deletions(-) diff --git a/src/services/quoteService.js b/src/services/quoteService.js index 8a5a982..eb903d4 100644 --- a/src/services/quoteService.js +++ b/src/services/quoteService.js @@ -2,6 +2,7 @@ const config = require('../config'); const rateService = require('./rateService'); +const fxCacheService = require('./fxCacheService'); const money = require('../utils/money'); const currency = require('../utils/currency'); const ApiError = require('../utils/ApiError'); @@ -178,27 +179,50 @@ function getQuoteById(quoteId) { return quote; } +/** Reclassify the bound snapshot without changing the stored quote's terms. */ +function withCurrentFreshness(quote, now) { + const freshness = quote.freshness || {}; + const fetchedAt = Date.parse(freshness.fetchedAt); + const status = fxCacheService.classify({ + expiresAt: Date.parse(freshness.expiresAt), + }, now); + const stale = Boolean(quote.stale || freshness.stale || status !== 'fresh'); + + return { + ...quote, + stale, + freshness: { + ...freshness, + status: status === 'fresh' && stale ? 'stale' : status, + stale, + ageMs: Number.isFinite(fetchedAt) ? Math.max(0, now - fetchedAt) : freshness.ageMs, + }, + }; +} + /** * Decide whether a quote may be used under a named policy. * * - Quote TTL expiry always blocks transfer use (the sender must refresh). - * - Underlying FX `stale` blocks transfer use unless allowStaleForTransfers. + * - FX freshness is re-evaluated at use time, not fixed at quote issuance. + * - Stale FX blocks transfer use unless allowStaleForTransfers, within grace. * - Display policy never throws for staleness; callers still see `stale: true`. * * @param {object} quote * @param {object} [opts] * @param {number} [opts.now] * @param {'reject_stale'|'allow_stale'} [opts.policy] - * @returns {object} the same quote when usable + * @returns {object} the quote's original terms with current freshness metadata */ function assertUsable(quote, opts = {}) { const now = opts.now != null ? opts.now : Date.now(); const policy = opts.policy || 'reject_stale'; const expiresAtMs = Date.parse(quote.quoteExpiresAt); + const current = withCurrentFreshness(quote, now); - if (!Number.isFinite(expiresAtMs) || now > expiresAtMs) { + if (!Number.isFinite(expiresAtMs) || now >= expiresAtMs) { if (policy === 'allow_stale') { - return quote; + return current; } throw ApiError.conflict('Quote has expired; request a fresh quote', { code: 'QUOTE_EXPIRED', @@ -208,9 +232,10 @@ function assertUsable(quote, opts = {}) { }); } - if (quote.stale || (quote.freshness && quote.freshness.stale)) { + if (current.stale) { const allow = - policy === 'allow_stale' || config.fx.allowStaleForTransfers === true; + policy === 'allow_stale' || + (config.fx.allowStaleForTransfers === true && current.freshness.status === 'stale'); if (!allow) { throw ApiError.conflict( 'Quote is based on a stale FX rate and cannot be used for transfer pricing', @@ -218,13 +243,13 @@ function assertUsable(quote, opts = {}) { code: 'QUOTE_STALE', quoteId: quote.quoteId, quoteVersion: quote.quoteVersion, - freshness: quote.freshness, + freshness: current.freshness, } ); } } - return quote; + return current; } /** @@ -246,8 +271,10 @@ function resolveForTransfer(data, opts = {}) { const now = opts.now != null ? opts.now : Date.now(); if (data.quoteId) { - const quote = getQuoteById(data.quoteId); - assertUsable(quote, { now, policy: 'reject_stale' }); + const quote = assertUsable(getQuoteById(data.quoteId), { + now, + policy: 'reject_stale', + }); const fromCode = currency.normalize(data.from); const toCode = currency.normalize(data.to); diff --git a/test/fxCache.test.js b/test/fxCache.test.js index 719d815..790b5f4 100644 --- a/test/fxCache.test.js +++ b/test/fxCache.test.js @@ -236,6 +236,76 @@ test('quotes expose identity and freshness metadata', () => { assert.equal(typeof quote.quoteExpiresAt, 'string'); }); +test('a fresh quote becomes unusable at FX expiry before its quote TTL', () => { + const now = 9_000_000; + const quote = quoteService.getQuote(100, 'USD', 'EUR', { now }); + const data = { ...PAYLOAD, quoteId: quote.quoteId }; + const fresh = quoteService.resolveForTransfer(data, { + now: now + config.fx.cacheTtlMs - 1, + }); + assert.equal(fresh.stale, false); + assert.equal(fresh.freshness.ageMs, config.fx.cacheTtlMs - 1); + + assert.throws( + () => quoteService.resolveForTransfer(data, { now: now + config.fx.cacheTtlMs }), + (err) => err instanceof ApiError && err.statusCode === 409 && + err.details.code === 'QUOTE_STALE' && + err.details.freshness.status === 'stale' && + err.details.freshness.ageMs === config.fx.cacheTtlMs + ); + // Binding must preserve the quote the sender saw, not silently fetch/reprice. + assert.equal(fxCacheService.getProviderFetchCount(), 1); +}); + +test('explicit stale transfer policy returns current freshness with unchanged quote terms', () => { + const now = 9_100_000; + const quote = quoteService.getQuote(100, 'USD', 'EUR', { now }); + config.fx.allowStaleForTransfers = true; + const bound = quoteService.resolveForTransfer({ ...PAYLOAD, quoteId: quote.quoteId }, { + now: now + config.fx.cacheTtlMs, + }); + assert.equal(bound.stale, true); + assert.equal(bound.freshness.stale, true); + assert.equal(bound.freshness.status, 'stale'); + assert.equal(bound.freshness.ageMs, config.fx.cacheTtlMs); + for (const field of ['quoteId', 'quoteVersion', 'rate', 'receiveAmount', 'quoteExpiresAt']) { + assert.equal(bound[field], quote[field]); + } + assert.equal(quoteService.getQuoteById(quote.quoteId).stale, false); +}); + +test('stale transfer opt-in cannot extend FX grace or quote TTL boundaries', () => { + const now = 9_200_000; + config.fx.quoteTtlMs = 10_000; + config.fx.allowStaleForTransfers = true; + const quote = quoteService.getQuote(100, 'USD', 'EUR', { now }); + const data = { ...PAYLOAD, quoteId: quote.quoteId }; + assert.throws( + () => quoteService.resolveForTransfer(data, { + now: now + config.fx.cacheTtlMs + config.fx.staleGraceMs, + }), + (err) => err instanceof ApiError && err.details.code === 'QUOTE_STALE' && + err.details.freshness.status === 'expired' + ); + assert.throws( + () => quoteService.resolveForTransfer(data, { now: now + config.fx.quoteTtlMs }), + (err) => err instanceof ApiError && err.details.code === 'QUOTE_EXPIRED' + ); +}); + +test('display policy reclassifies an issued quote without changing its stored terms', () => { + const now = 9_300_000; + const quote = quoteService.getQuote(100, 'USD', 'EUR', { now }); + const displayed = quoteService.assertUsable(quote, { + now: now + config.fx.cacheTtlMs, + policy: 'allow_stale', + }); + assert.equal(displayed.stale, true); + assert.equal(displayed.freshness.status, 'stale'); + assert.equal(displayed.rate, quote.rate); + assert.equal(quote.stale, false); +}); + test('stale quote is visible but cannot be used for transfer pricing', () => { // Use wall-clock time so transfer creation (which reads Date.now) and the // quote TTL share the same clock. Only the FX snapshot is forced stale. From db41b84b5956a8ec243ebf75cb0ce188a492bf50 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sat, 3 Oct 2026 08:16:01 -0400 Subject: [PATCH 03/16] fix(fx): enforce freshness before provider fallback selection A successful fetch could return an old snapshot, bypass reject_stale during cache refresh, and mint a transfer without a supplied quoteId. Even an expired primary snapshot hid a usable fallback. Apply the caller's freshness policy before a provider wins or replaces the cache. Fresh snapshots remain eligible; allow_stale also permits visibly stale snapshots strictly within grace. Reject non-finite or non-numeric timestamps. An unusable response advances the ordered fallback registry, and a failed refresh preserves an existing within-grace display snapshot. Keep original provider timestamps and the prior bound-quote freshness repair. Validation: npm test passed 283/283, zero skipped or failed. The existing FX file now has 25 checks; all five new regressions fail on sole parent 321b1dac and its 20 previous checks pass. Node 24.19.0 reused retained packages matching all 76 lockfile package versions, without an install. Changed JavaScript syntax and git diff checks pass. Actual local Express HTTP replay with the documented in-memory store and mock Stellar adapter: snapshots at TTL and grace expiry previously produced 201/rateStale:true; both now return 503 FX_PROVIDERS_DOWN with zero transfers, quotes, or idempotency records. A fresh fallback now produces 201/rateStale:false after the stale primary, and a recovered provider can reuse the rejected request's idempotency key. No live FX, Stellar settlement, or hosted Node 22 CI result is claimed. --- CHANGELOG.md | 7 ++ src/services/fxCacheService.js | 21 ++++- src/services/fxProviders.js | 14 ++- test/fxCache.test.js | 162 ++++++++++++++++++++++++++++++++- 4 files changed, 195 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6aa9d8..8b6ff39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,13 @@ When preparing a new release: ### Fixed +- Successful provider responses now pass the requested FX freshness policy before + they can win fallback selection or replace the cache. Transfers reject a + newly fetched stale snapshot, including at the TTL boundary, while display + requests may use visibly stale rates strictly within grace. Expired or invalid + timestamps advance to the next provider; an unusable refresh preserves an + existing within-grace display snapshot. Provider timestamps are not renewed + merely because a fetch succeeded. - Offset pagination over transfer and audit history repeated or skipped rows when records were written while a client was paging, because the window was defined by a row count rather than a position. Cursor pagination anchors to diff --git a/src/services/fxCacheService.js b/src/services/fxCacheService.js index 3fdb799..df824e5 100644 --- a/src/services/fxCacheService.js +++ b/src/services/fxCacheService.js @@ -72,12 +72,25 @@ function decorate(entry, now, extra) { } /** - * Pull from providers (counted), store under TTL, return decorated fresh snapshot. + * Pull from providers (counted), accepting only snapshots within policy before + * choosing a provider or replacing the cache. A successful fetch is not proof + * that the provider's timestamp is current. * @param {number} now + * @param {'reject_stale'|'allow_stale'} policy */ -function refresh(now) { +function refresh(now, policy) { providerFetchCount += 1; - const snapshot = fxProviders.fetchWithFallback({ now }); + const snapshot = fxProviders.fetchWithFallback({ + now, + acceptSnapshot(candidate) { + const expiresAt = candidate.fetchedAt + ttlMs(); + if (!Number.isFinite(candidate.fetchedAt) || !Number.isFinite(expiresAt)) { + return false; + } + const status = classify({ expiresAt }, now); + return status === 'fresh' || (policy === 'allow_stale' && status === 'stale'); + }, + }); const entry = { ratesToUsd: { ...snapshot.ratesToUsd }, fetchedAt: snapshot.fetchedAt, @@ -128,7 +141,7 @@ function getSnapshot(opts = {}) { inflight.add(CACHE_KEY); try { - return refresh(now); + return refresh(now, policy); } catch (err) { // Provider path failed. Under allow_stale, a within-grace entry is still // usable for display — but it is visibly stale. Under reject_stale we diff --git a/src/services/fxProviders.js b/src/services/fxProviders.js index 1f9879a..68b5e2e 100644 --- a/src/services/fxProviders.js +++ b/src/services/fxProviders.js @@ -63,7 +63,7 @@ function fallbackFetch(opts = {}) { primaryFetch._down = false; fallbackFetch._down = false; -/** Ordered registry. First success wins; order is the fallback contract. */ +/** Ordered registry. First usable success wins; order is the fallback contract. */ const DEFAULT_PROVIDERS = [ { id: 'primary', fetch: primaryFetch }, { id: 'fallback', fetch: fallbackFetch }, @@ -85,8 +85,10 @@ function listProviders() { } /** - * Walk providers in order until one returns a snapshot. - * @param {{ now?: number }} [opts] + * Walk providers in order until one returns a snapshot accepted by the caller. + * A response outside the caller's freshness policy is a failed attempt, so it + * cannot hide a usable response from a later provider. + * @param {{ now?: number, acceptSnapshot?: (snapshot: FxSnapshot) => boolean }} [opts] * @returns {FxSnapshot} * @throws {ApiError} 503 when every provider fails. */ @@ -98,11 +100,15 @@ function fetchWithFallback(opts = {}) { if (!snapshot || typeof snapshot.ratesToUsd !== 'object') { throw new Error(`Provider ${provider.id} returned an invalid snapshot`); } - return { + const normalized = { providerId: snapshot.providerId || provider.id, ratesToUsd: { ...snapshot.ratesToUsd }, fetchedAt: snapshot.fetchedAt != null ? snapshot.fetchedAt : (opts.now != null ? opts.now : Date.now()), }; + if (opts.acceptSnapshot && !opts.acceptSnapshot(normalized)) { + throw new Error(`Provider ${provider.id} returned a snapshot outside the requested freshness policy`); + } + return normalized; } catch (err) { errors.push({ providerId: provider.id, diff --git a/test/fxCache.test.js b/test/fxCache.test.js index 790b5f4..b41e201 100644 --- a/test/fxCache.test.js +++ b/test/fxCache.test.js @@ -3,7 +3,7 @@ const { test, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); -const { reset } = require('../src/store'); +const { store, reset } = require('../src/store'); const config = require('../src/config'); const fxCacheService = require('../src/services/fxCacheService'); const fxProviders = require('../src/services/fxProviders'); @@ -143,6 +143,166 @@ test('fallback order is primary-then-fallback (deterministic)', () => { assert.equal(snapshot.providerId, 'fallback'); }); +test('new provider snapshots obey the requested TTL and grace boundaries before caching', () => { + const now = 6_100_000; + for (const policy of ['reject_stale', 'allow_stale']) { + for (const ageMs of [999, 1_000, 5_999, 6_000]) { + fxCacheService.reset(); + fxProviders.setProviders([{ + id: 'primary', + fetch: () => ({ ratesToUsd: { USD: 1, EUR: 1.08 }, fetchedAt: now - ageMs }), + }]); + const usable = ageMs < 1_000 || (policy === 'allow_stale' && ageMs < 6_000); + if (!usable) { + assert.throws( + () => fxCacheService.getSnapshot({ now, policy }), + (err) => err instanceof ApiError && err.statusCode === 503 && + err.details.code === 'FX_PROVIDERS_DOWN', + `${policy}, age ${ageMs}` + ); + assert.equal(fxCacheService.peek(now), null); + continue; + } + const snapshot = fxCacheService.getSnapshot({ now, policy }); + assert.equal(snapshot.status, ageMs < 1_000 ? 'fresh' : 'stale'); + assert.equal(snapshot.stale, ageMs >= 1_000); + assert.equal(snapshot.fetchedAt, now - ageMs); + assert.equal(snapshot.ageMs, ageMs); + assert.equal(snapshot.cacheHit, false); + assert.equal(fxCacheService.peek(now).fetchedAt, now - ageMs); + } + } +}); + +test('invalid provider timestamps cannot win over a usable fallback', () => { + const now = 6_200_000; + for (const fetchedAt of [NaN, Infinity, -Infinity, String(now)]) { + fxCacheService.reset(); + const attempted = []; + fxProviders.setProviders([ + { + id: 'primary', + fetch: () => { + attempted.push('primary'); + return { ratesToUsd: { USD: 1, EUR: 1.08 }, fetchedAt }; + }, + }, + { + id: 'fallback', + fetch: () => { + attempted.push('fallback'); + return { ratesToUsd: { USD: 1, EUR: 1.1 }, fetchedAt: now }; + }, + }, + ]); + const snapshot = fxCacheService.getSnapshot({ now, policy: 'allow_stale' }); + assert.deepEqual(attempted, ['primary', 'fallback']); + assert.equal(snapshot.providerId, 'fallback'); + assert.equal(snapshot.status, 'fresh'); + assert.equal(snapshot.ratesToUsd.EUR, 1.1); + } +}); + +test('fallback order skips responses outside policy and stops at the first usable snapshot', () => { + const now = 6_300_000; + for (const policy of ['reject_stale', 'allow_stale']) { + fxCacheService.reset(); + const attempted = []; + fxProviders.setProviders([ + { id: 'expired', ageMs: 6_000 }, + { id: 'stale', ageMs: 1_000 }, + { id: 'fresh', ageMs: 0 }, + { id: 'unused', ageMs: 0 }, + ].map(({ id, ageMs }) => ({ + id, + fetch: () => { + attempted.push(id); + return { ratesToUsd: { USD: 1, EUR: 1.08 }, fetchedAt: now - ageMs }; + }, + }))); + const snapshot = fxCacheService.getSnapshot({ now, policy }); + const allowsStale = policy === 'allow_stale'; + assert.deepEqual(attempted, allowsStale ? ['expired', 'stale'] : ['expired', 'stale', 'fresh']); + assert.equal(snapshot.providerId, allowsStale ? 'stale' : 'fresh'); + assert.equal(snapshot.stale, allowsStale); + assert.equal(fxCacheService.getProviderFetchCount(), 1); + } +}); + +test('unusable provider responses preserve a cached display snapshot only within its grace', () => { + const now = 6_400_000; + const fetchedAt = now - 1_500; + fxCacheService.seed({ fetchedAt, providerId: 'cached' }); + fxProviders.setProviders([{ + id: 'expired', + fetch: () => ({ ratesToUsd: { USD: 1, EUR: 1.08 }, fetchedAt: now - 6_000 }), + }]); + assert.throws( + () => fxCacheService.getSnapshot({ now, policy: 'reject_stale' }), + (err) => err instanceof ApiError && err.details.code === 'FX_PROVIDERS_DOWN' + ); + const displayed = fxCacheService.getSnapshot({ now, policy: 'allow_stale' }); + assert.equal(displayed.providerId, 'cached'); + assert.equal(displayed.fetchedAt, fetchedAt); + assert.equal(displayed.stale, true); + assert.equal(displayed.source, 'cache-stale-outage'); + assert.throws( + () => fxCacheService.getSnapshot({ now: fetchedAt + 6_000, policy: 'allow_stale' }), + (err) => err instanceof ApiError && err.details.code === 'FX_PROVIDERS_DOWN' + ); + assert.equal(fxCacheService.peek(now).fetchedAt, fetchedAt); +}); + +test('HTTP transfer rejects newly fetched stale rates without reserving a quote or retry key', async () => { + const createApp = require('../src/app'); + const originalTokens = config.apiTokens; + config.apiTokens = { 'fx-http-regression': ['transfers:write'] }; + const server = createApp().listen(0, '127.0.0.1'); + await new Promise((resolve, reject) => { + server.once('listening', resolve); + server.once('error', reject); + }); + const post = () => fetch(`http://127.0.0.1:${server.address().port}/api/transfers`, { + method: 'POST', + headers: { + Authorization: 'Bearer fx-http-regression', + 'Content-Type': 'application/json', + 'Idempotency-Key': 'fx-policy-retry', + }, + body: JSON.stringify(PAYLOAD), + }); + try { + for (const ageMs of [1_000, 6_000]) { + reset(); + fxProviders.setProviders([{ + id: 'primary', + fetch: ({ now }) => ({ ratesToUsd: { USD: 1, EUR: 1.08 }, fetchedAt: now - ageMs }), + }]); + const rejected = await post(); + const failure = await rejected.json(); + assert.equal(rejected.status, 503); + assert.equal(failure.error.details.code, 'FX_PROVIDERS_DOWN'); + assert.equal(store.transfers.size, 0); + assert.equal(store.quotes.size, 0); + assert.equal(store.idempotency.size, 0); + assert.equal(fxCacheService.peek(), null); + + fxProviders.resetProviders(); + const recovered = await post(); + const transfer = await recovered.json(); + assert.equal(recovered.status, 201); + assert.equal(transfer.rateStale, false); + assert.equal(transfer.rateProvider, 'primary'); + assert.equal(store.transfers.size, 1); + assert.equal(store.quotes.size, 1); + assert.equal(store.idempotency.size, 1); + } + } finally { + config.apiTokens = originalTokens; + await new Promise((resolve, reject) => server.close((err) => err ? reject(err) : resolve())); + } +}); + // --------------------------------------------------------------------------- // Stampede prevention // --------------------------------------------------------------------------- From cdbdb171cb4dec17e5b5352fb7ef7e4979bd5e44 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sat, 3 Oct 2026 17:05:55 -0400 Subject: [PATCH 04/16] fix(fx): reject malformed provider rates before fallback selection --- CHANGELOG.md | 6 ++ src/services/fxProviders.js | 16 +++-- test/fxCache.test.js | 133 ++++++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b6ff39..e1fc4b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,12 @@ When preparing a new release: ### Fixed +- FX providers must return a nonempty rate map containing only finite, positive + numbers before winning fallback selection or replacing the cache. Malformed + primary rates advance to the next provider. With no usable provider or cached + snapshot, quotes and transfers fail without creating records or consuming a + transfer retry key. Valid partial maps and the existing within-grace stale + display policy remain supported. - Successful provider responses now pass the requested FX freshness policy before they can win fallback selection or replace the cache. Transfers reject a newly fetched stale snapshot, including at the TTL boundary, while display diff --git a/src/services/fxProviders.js b/src/services/fxProviders.js index 68b5e2e..3c09392 100644 --- a/src/services/fxProviders.js +++ b/src/services/fxProviders.js @@ -85,9 +85,9 @@ function listProviders() { } /** - * Walk providers in order until one returns a snapshot accepted by the caller. - * A response outside the caller's freshness policy is a failed attempt, so it - * cannot hide a usable response from a later provider. + * Walk providers in order until one returns valid rates accepted by the caller. + * Malformed rates or a response outside the caller's freshness policy are + * failed attempts, so neither can hide a usable response from a later provider. * @param {{ now?: number, acceptSnapshot?: (snapshot: FxSnapshot) => boolean }} [opts] * @returns {FxSnapshot} * @throws {ApiError} 503 when every provider fails. @@ -97,12 +97,18 @@ function fetchWithFallback(opts = {}) { for (const provider of providers) { try { const snapshot = provider.fetch(opts); - if (!snapshot || typeof snapshot.ratesToUsd !== 'object') { + const rates = snapshot && snapshot.ratesToUsd; + if (!rates || typeof rates !== 'object' || Array.isArray(rates)) { throw new Error(`Provider ${provider.id} returned an invalid snapshot`); } + const ratesToUsd = { ...rates }; + const values = Object.values(ratesToUsd); + if (values.length === 0 || values.some((rate) => !Number.isFinite(rate) || rate <= 0)) { + throw new Error(`Provider ${provider.id} returned invalid rates`); + } const normalized = { providerId: snapshot.providerId || provider.id, - ratesToUsd: { ...snapshot.ratesToUsd }, + ratesToUsd, fetchedAt: snapshot.fetchedAt != null ? snapshot.fetchedAt : (opts.now != null ? opts.now : Date.now()), }; if (opts.acceptSnapshot && !opts.acceptSnapshot(normalized)) { diff --git a/test/fxCache.test.js b/test/fxCache.test.js index b41e201..59bee8d 100644 --- a/test/fxCache.test.js +++ b/test/fxCache.test.js @@ -203,6 +203,48 @@ test('invalid provider timestamps cannot win over a usable fallback', () => { } }); +test('malformed provider rates fall back before either policy can cache them', () => { + const now = 6_250_000; + const invalidMaps = [ + null, + {}, + Object.assign([], { USD: 1, EUR: 1.08 }), + ...[0, -0, -1, NaN, Infinity, -Infinity, '1.08', null, undefined, true] + .map((EUR) => ({ USD: 1, EUR })), + { USD: 1, EUR: 1.08, GBP: 0 }, + ]; + for (const policy of ['reject_stale', 'allow_stale']) { + for (const ratesToUsd of invalidMaps) { + fxCacheService.reset(); + const attempted = []; + const healthyRates = { USD: 1, EUR: 1.1 }; + fxProviders.setProviders([ + { id: 'primary', fetch: () => { + attempted.push('primary'); + return { ratesToUsd, fetchedAt: now }; + } }, + { id: 'fallback', fetch: () => { + attempted.push('fallback'); + return { ratesToUsd: healthyRates, fetchedAt: now }; + } }, + { id: 'unused', fetch: () => { + attempted.push('unused'); + throw new Error('unreachable provider'); + } }, + ]); + const snapshot = fxCacheService.getSnapshot({ now, policy }); + assert.deepEqual(attempted, ['primary', 'fallback']); + assert.equal(snapshot.providerId, 'fallback'); + assert.equal(snapshot.stale, false); + assert.deepEqual(snapshot.ratesToUsd, healthyRates); + assert.notEqual(snapshot.ratesToUsd, healthyRates); + assert.equal(fxCacheService.peek(now).providerId, 'fallback'); + assert.deepEqual(fxCacheService.peek(now).ratesToUsd, healthyRates); + assert.equal(fxCacheService.getProviderFetchCount(), 1); + } + } +}); + test('fallback order skips responses outside policy and stops at the first usable snapshot', () => { const now = 6_300_000; for (const policy of ['reject_stale', 'allow_stale']) { @@ -253,6 +295,97 @@ test('unusable provider responses preserve a cached display snapshot only within assert.equal(fxCacheService.peek(now).fetchedAt, fetchedAt); }); +test('malformed provider rates preserve a cached display snapshot only within grace', () => { + const now = 6_450_000; + const fetchedAt = now - 1_500; + const ratesToUsd = { USD: 1, EUR: 1.08 }; + fxCacheService.seed({ fetchedAt, providerId: 'cached', ratesToUsd }); + fxProviders.setProviders([ + { id: 'primary', fetch: () => ({ ratesToUsd: { USD: 1, EUR: 0 }, fetchedAt: now }) }, + { id: 'fallback', fetch: () => ({ ratesToUsd: { USD: 1, EUR: -1.1 }, fetchedAt: now }) }, + ]); + assert.throws( + () => fxCacheService.getSnapshot({ now, policy: 'reject_stale' }), + (err) => err instanceof ApiError && err.statusCode === 503 && + err.details.code === 'FX_PROVIDERS_DOWN' && + err.details.attempted.map((entry) => entry.providerId).join(',') === 'primary,fallback' + ); + const displayed = fxCacheService.getSnapshot({ now, policy: 'allow_stale' }); + assert.equal(displayed.providerId, 'cached'); + assert.equal(displayed.fetchedAt, fetchedAt); + assert.deepEqual(displayed.ratesToUsd, ratesToUsd); + assert.equal(displayed.stale, true); + assert.equal(displayed.source, 'cache-stale-outage'); + assert.throws( + () => fxCacheService.getSnapshot({ now: fetchedAt + 6_000, policy: 'allow_stale' }), + (err) => err instanceof ApiError && err.details.code === 'FX_PROVIDERS_DOWN' + ); + assert.equal(fxCacheService.peek(now).providerId, 'cached'); + assert.equal(fxCacheService.peek(now).fetchedAt, fetchedAt); + assert.deepEqual(fxCacheService.peek(now).ratesToUsd, ratesToUsd); +}); + +test('HTTP invalid FX responses create no records and allow the same transfer key after recovery', async () => { + const createApp = require('../src/app'); + const originalTokens = config.apiTokens; + config.apiTokens = { 'fx-invalid-rates-regression': ['transfers:write'] }; + const server = createApp().listen(0, '127.0.0.1'); + await new Promise((resolve, reject) => { + server.once('listening', resolve); + server.once('error', reject); + }); + const base = `http://127.0.0.1:${server.address().port}`; + const post = () => fetch(`${base}/api/transfers`, { + method: 'POST', + headers: { + Authorization: 'Bearer fx-invalid-rates-regression', + 'Content-Type': 'application/json', + 'Idempotency-Key': 'fx-invalid-rates-retry', + }, + body: JSON.stringify(PAYLOAD), + }); + try { + const attempted = []; + fxProviders.setProviders([ + { id: 'primary', fetch: ({ now }) => { + attempted.push('primary'); + return { ratesToUsd: { USD: 1, EUR: 0 }, fetchedAt: now }; + } }, + { id: 'fallback', fetch: ({ now }) => { + attempted.push('fallback'); + return { ratesToUsd: { USD: 1, EUR: -1.1 }, fetchedAt: now }; + } }, + ]); + for (const request of [() => fetch(`${base}/api/quote?amount=100&from=USD&to=EUR`), post]) { + const rejected = await request(); + const failure = await rejected.json(); + assert.equal(rejected.status, 503); + assert.equal(failure.error.details.code, 'FX_PROVIDERS_DOWN'); + assert.deepEqual(failure.error.details.attempted.map((entry) => entry.providerId), ['primary', 'fallback']); + assert.equal(store.transfers.size, 0); + assert.equal(store.quotes.size, 0); + assert.equal(store.idempotency.size, 0); + assert.equal(fxCacheService.peek(), null); + } + assert.deepEqual(attempted, ['primary', 'fallback', 'primary', 'fallback']); + + fxProviders.resetProviders(); + const recovered = await post(); + const transfer = await recovered.json(); + assert.equal(recovered.status, 201); + assert.equal(transfer.rateStale, false); + assert.equal(transfer.rateProvider, 'primary'); + assert.equal(transfer.rate, 0.93); + assert.equal(transfer.receiveAmount, 90.93); + assert.equal(store.transfers.size, 1); + assert.equal(store.quotes.size, 1); + assert.equal(store.idempotency.size, 1); + } finally { + config.apiTokens = originalTokens; + await new Promise((resolve, reject) => server.close((err) => err ? reject(err) : resolve())); + } +}); + test('HTTP transfer rejects newly fetched stale rates without reserving a quote or retry key', async () => { const createApp = require('../src/app'); const originalTokens = config.apiTokens; From 81c4e3acdf4b8e55c019c5444bc1189c9ee3f68e Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sat, 3 Oct 2026 22:59:30 -0400 Subject: [PATCH 05/16] docs(fx): explain freshness settings and optional quote binding The existing FX implementation exposes four environment settings and versioned quote binding, but the README and example environment omit that operating contract. A caller following only the documented transfer body intentionally receives a new quote rather than binding displayed terms. Document the implemented defaults, FX-versus-HTTP cache lifetimes, optional quoteId flow, current expiry/error policies, idempotent retry identity, static default providers and process-local retention. Add the already required Idempotency-Key to the existing fresh-pricing request example. README.md and .env.example only; all 174 other file blobs and modes, including the existing quote/freshness/provider repairs and tests, remain unchanged. The environment example uses the current defaults. Validation: API source/document comparison and exact prepared tree readback. Current npm test on Node 22, request examples and Markdown preview are unrun because native execution is offline. No runtime or suite replay; the prior 286 full / 28 focused test results remain historical at cdbdb171cb4dec17e5b5352fb7ef7e4979bd5e44. Refs RemitFlow/RemitFlow-Backend#129 and existing PR #141. --- .env.example | 9 +++++++ README.md | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/.env.example b/.env.example index dcba576..b3a1710 100644 --- a/.env.example +++ b/.env.example @@ -39,6 +39,15 @@ DB_POOL_CONNECTION_TIMEOUT_MS=2000 CACHE_DEFAULT_POLICY=no-store CACHE_RATES_MAX_AGE_SECONDS=10 +# FX snapshots and quote identities (milliseconds) +# Independent of HTTP response caching above; read at application startup. +FX_CACHE_TTL_MS=30000 +FX_STALE_GRACE_MS=60000 +FX_QUOTE_TTL_MS=60000 +# Opt-in applies only to an explicit quoteId within FX grace and quote lifetime. +# Omitting quoteId still requires a fresh snapshot. +FX_ALLOW_STALE_TRANSFERS=false + # History pagination (GET /api/transfers, GET /api/audit) PAGINATION_DEFAULT_LIMIT=50 # Requests above this limit are rejected with 400, not clamped. diff --git a/README.md b/README.md index 4791414..b626bff 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,10 @@ The application is configured using environment variables (typically defined in | `DB_POOL_CONNECTION_TIMEOUT_MS` | Time to wait for a connection before timing out (ms) | `2000` | | `CACHE_DEFAULT_POLICY` | Default cache policy for endpoints (`no-store`, `public`, `private`) | `no-store` | | `CACHE_RATES_MAX_AGE_SECONDS` | Cache duration for rates endpoints (seconds) | `10` | +| `FX_CACHE_TTL_MS` | FX snapshot freshness window from the provider timestamp (ms) | `30000` | +| `FX_STALE_GRACE_MS` | Additional window for visibly stale display data (ms) | `60000` | +| `FX_QUOTE_TTL_MS` | Lifetime of an issued quote for transfer binding (ms) | `60000` | +| `FX_ALLOW_STALE_TRANSFERS` | Accept an explicitly bound stale quote only within FX grace and its quote lifetime; enabled only by `true` | `false` | | `PAGINATION_DEFAULT_LIMIT` | Page size used when a request omits `limit` | `50` | | `PAGINATION_MAX_LIMIT` | Largest accepted `limit`; bigger requests are rejected | `200` | | `PAGINATION_MAX_SCAN` | Max records a single history query may examine | `10000` | @@ -244,10 +248,72 @@ at most 2 decimal places (e.g. `100.129` is rejected with a 400). This prevents floating-point/sub-cent precision loss from being silently rounded away. +#### Quote freshness and binding + +Rates and quotes expose `freshness`, including `status`, `stale`, `providerId`, +`fetchedAt`, `expiresAt`, and `ageMs`. Quotes also return `quoteId`, +`quoteVersion`, `stale`, `quoteCreatedAt`, and `quoteExpiresAt`. Show stale +data as stale; receiving a display quote does not guarantee transfer acceptance. + +`FX_CACHE_TTL_MS` measures freshness from the provider's `fetchedAt`, not from +the most recent HTTP request. A successful fetch of an old snapshot does not +renew its timestamp. Staleness begins at `freshness.expiresAt`; the grace +window ends at that timestamp plus `FX_STALE_GRACE_MS`. The separate +`quoteExpiresAt` also limits transfer use. Neither the grace window nor the +quote lifetime includes its end timestamp. + +| Request | Pricing behavior | +|---------|------------------| +| `GET /api/rates`, `GET /api/rates/:pair`, `GET /api/quote` | May return visibly stale data strictly within the FX grace window. | +| New transfer with `quoteId` | Binds the issued quote's original terms; amount and normalized currencies must match, and current freshness and quote expiry are checked again. | +| New transfer without `quoteId` | Intentionally mints and binds a fresh quote at creation. This supported compatibility path does not reuse an earlier displayed quote. | + +To bind terms the sender has reviewed: + +1. Request `GET /api/quote?amount=100&from=USD&to=INR` and retain its + `quoteId`, price/fee breakdown, and expiry metadata. +2. Include that `quoteId` in the transfer body alongside the same amount, + `from`, and `to`. Supply the normal Bearer token and `Idempotency-Key`. + The response records `quoteId`, `quoteVersion`, `rateProvider`, + `rateFetchedAt`, and `rateStale`. +3. If the request's outcome is unknown, retry the same key and body, including + `quoteId`. A completed retry replays the stored transfer without requoting. + `quoteId` is part of the idempotency fingerprint; changing it under a + completed key is a different-payload conflict. +4. After `QUOTE_EXPIRED` or `QUOTE_STALE`, fetch and review a new quote before + choosing new terms. Omitting `quoteId` deliberately selects fresh pricing + instead of preserving the previously displayed terms. + +With the default `FX_ALLOW_STALE_TRANSFERS=false`, stale FX is rejected for +transfer pricing. Setting it to exactly `true` permits an explicitly bound +stale quote only while both its FX grace window and quote lifetime remain +valid. The no-`quoteId` path still requires a fresh snapshot. + +FX/quote failures use the normal `error.details.code` field: + +| HTTP status | Code | Meaning | +|-------------|------|---------| +| `404` | `QUOTE_NOT_FOUND` | The supplied quote is not retained by this process; request a new quote. | +| `409` | `QUOTE_EXPIRED` | The quote lifetime has ended; request and review a new quote. | +| `409` | `QUOTE_STALE` | The bound snapshot is outside the configured transfer freshness policy. | +| `409` | `QUOTE_MISMATCH` | The transfer amount or currencies differ from the bound quote. | +| `503` | `FX_PROVIDERS_DOWN` | No provider/cache snapshot is usable under the requested policy. | +| `503` | `FX_REFRESH_IN_PROGRESS` | A refresh is already running and no cached snapshot is usable under the requested policy. | + +These FX settings are read at startup; restart after changing them. +`CACHE_RATES_MAX_AGE_SECONDS` controls HTTP response caching and does not +extend FX freshness or a quote's lifetime. The built-in primary and fallback +providers wrap the same static demo rate table. Cache entries and quote IDs +are process-local, disappear on restart, and are not shared across replicas. +The bounded quote map can also evict an unexpired quote, so clients must handle +`QUOTE_NOT_FOUND` rather than assuming retention until `quoteExpiresAt`. + ### Transfers - `POST /api/transfers` — create a transfer. Body: `{ senderName, recipientName, amount, from, to }` + Optional `quoteId` binds a previously issued quote; omission mints a fresh one + (see [Quote freshness and binding](#quote-freshness-and-binding)). Requires an `Idempotency-Key` header (see below). - `GET /api/transfers` — list transfers. Supports `?status=`, `?q=` (name search), `?archived=` (true/false/all), and [cursor pagination](#pagination) @@ -321,9 +387,12 @@ curl "http://localhost:3000/api/health" # Set your token once (use a demo token for local dev, or your own via API_TOKENS) TOKEN="test-token-admin" -# Create a transfer (requires transfers:write) +# Create a transfer with fresh pricing (requires transfers:write) +# Choose a new key for each logical transfer; keep it and the body on retries. +TRANSFER_KEY="demo-transfer-001" curl -X POST http://localhost:3000/api/transfers \ -H "Authorization: Bearer $TOKEN" \ + -H "Idempotency-Key: $TRANSFER_KEY" \ -H "Content-Type: application/json" \ -d '{"senderName":"Alice","recipientName":"Bob","amount":100,"from":"USD","to":"INR"}' From 823d05059cd4f0e6f190d43311ec3c8487ebdc74 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 04:46:54 -0400 Subject: [PATCH 06/16] fix(fx): reject future-dated provider snapshots before caching A finite provider fetchedAt ahead of the request time previously extended freshness beyond the configured TTL and prevented a usable fallback from being considered. Reject these candidates instead of clamping their source timestamps. Exact-now snapshots remain valid, and existing stale-cache outage/grace behavior is preserved. Add four node:test regressions covering ordered fallback under both policies (1 ms, TTL and one-day offsets), no-cache rejection and recovery, stale-cache preservation/grace expiry, and the exact-now/TTL boundary. The existing npm test glob includes the new file; no existing tests change. Validation: node --test test/fxFutureTimestamp.test.js on Node 22.16.0. Original fxCacheService blob df824e532d8ecd38e1dc250d85ec225497d12ae8: 1 pass / 3 fail (exit 1). Repaired source: 4 pass / 0 fail (exit 0). Native syntax checks pass. Execution used the exact cache/provider/error/ rate modules with a local ordinary-values config fixture because dotenv is unavailable in the offline runtime. That fixture is NOT committed. This is focused module execution, not full application, HTTP or CI proof. Refs RemitFlow/RemitFlow-Backend#129 and existing PR #141. --- src/services/fxCacheService.js | 6 +- test/fxFutureTimestamp.test.js | 123 +++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 test/fxFutureTimestamp.test.js diff --git a/src/services/fxCacheService.js b/src/services/fxCacheService.js index df824e5..44660c6 100644 --- a/src/services/fxCacheService.js +++ b/src/services/fxCacheService.js @@ -84,7 +84,11 @@ function refresh(now, policy) { now, acceptSnapshot(candidate) { const expiresAt = candidate.fetchedAt + ttlMs(); - if (!Number.isFinite(candidate.fetchedAt) || !Number.isFinite(expiresAt)) { + // A future provider timestamp would extend freshness beyond the local + // TTL. Reject it instead of clamping away its provenance so a usable + // fallback can still win. + if (!Number.isFinite(candidate.fetchedAt) || !Number.isFinite(expiresAt) || + candidate.fetchedAt > now) { return false; } const status = classify({ expiresAt }, now); diff --git a/test/fxFutureTimestamp.test.js b/test/fxFutureTimestamp.test.js new file mode 100644 index 0000000..6e8379f --- /dev/null +++ b/test/fxFutureTimestamp.test.js @@ -0,0 +1,123 @@ +'use strict'; + +const { test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const config = require('../src/config'); +const fxCacheService = require('../src/services/fxCacheService'); +const fxProviders = require('../src/services/fxProviders'); + +const ORIGINAL_TTL = config.fx.cacheTtlMs; +const ORIGINAL_GRACE = config.fx.staleGraceMs; +const NOW = 10_000_000; +const RATES = { USD: 1, EUR: 1.08 }; +const isProvidersDown = (err) => err.statusCode === 503 && + err.details && err.details.code === 'FX_PROVIDERS_DOWN'; + +beforeEach(() => { + fxCacheService.reset(); + fxProviders.resetProviders(); + config.fx.cacheTtlMs = 1_000; + config.fx.staleGraceMs = 5_000; +}); + +afterEach(() => { + fxCacheService.reset(); + fxProviders.resetProviders(); + config.fx.cacheTtlMs = ORIGINAL_TTL; + config.fx.staleGraceMs = ORIGINAL_GRACE; +}); + +test('future-dated snapshots cannot suppress a usable fallback under either policy', () => { + for (const policy of ['reject_stale', 'allow_stale']) { + for (const offset of [1, 1_000, 86_400_000]) { + fxCacheService.reset(); + const attempted = []; + fxProviders.setProviders([ + { id: 'future', fetch: () => { + attempted.push('future'); + return { ratesToUsd: RATES, fetchedAt: NOW + offset }; + } }, + { id: 'current', fetch: () => { + attempted.push('current'); + return { ratesToUsd: RATES, fetchedAt: NOW }; + } }, + { id: 'unused', fetch: () => { + attempted.push('unused'); + throw new Error('Provider after the first usable snapshot must not run'); + } }, + ]); + const snapshot = fxCacheService.getSnapshot({ now: NOW, policy }); + assert.deepEqual(attempted, ['future', 'current'], `${policy}, offset ${offset}`); + assert.equal(snapshot.providerId, 'current'); + assert.equal(snapshot.fetchedAt, NOW); + assert.equal(snapshot.expiresAt, NOW + 1_000); + assert.equal(snapshot.ageMs, 0); + assert.equal(snapshot.stale, false); + assert.equal(fxCacheService.peek(NOW).providerId, 'current'); + assert.equal(fxCacheService.getProviderFetchCount(), 1); + } + } +}); + +test('all-future providers leave no cache entry and release the refresh guard for recovery', () => { + for (const policy of ['reject_stale', 'allow_stale']) { + fxCacheService.reset(); + fxProviders.setProviders([ + { id: 'future', fetch: () => ({ ratesToUsd: RATES, fetchedAt: NOW + 86_400_000 }) }, + ]); + assert.throws(() => fxCacheService.getSnapshot({ now: NOW, policy }), isProvidersDown); + assert.equal(fxCacheService.peek(NOW), null); + fxProviders.resetProviders(); + const recovered = fxCacheService.getSnapshot({ now: NOW, policy }); + assert.equal(recovered.providerId, 'primary'); + assert.equal(recovered.fetchedAt, NOW); + assert.equal(recovered.stale, false); + assert.equal(fxCacheService.getProviderFetchCount(), 2); + } +}); + +test('future provider data cannot replace a stale display cache or extend its grace period', () => { + const fetchedAt = NOW - 1_500; + fxCacheService.seed({ fetchedAt, ratesToUsd: RATES, providerId: 'cached' }); + fxProviders.setProviders([ + { id: 'future', fetch: () => ({ ratesToUsd: { USD: 1, EUR: 2 }, fetchedAt: NOW + 86_400_000 }) }, + ]); + assert.throws( + () => fxCacheService.getSnapshot({ now: NOW, policy: 'reject_stale' }), + isProvidersDown + ); + const displayed = fxCacheService.getSnapshot({ now: NOW, policy: 'allow_stale' }); + assert.equal(displayed.providerId, 'cached'); + assert.equal(displayed.fetchedAt, fetchedAt); + assert.equal(displayed.expiresAt, fetchedAt + 1_000); + assert.equal(displayed.ageMs, 1_500); + assert.equal(displayed.stale, true); + assert.equal(displayed.source, 'cache-stale-outage'); + assert.deepEqual(displayed.ratesToUsd, RATES); + assert.throws( + () => fxCacheService.getSnapshot({ now: fetchedAt + 6_000, policy: 'allow_stale' }), + isProvidersDown + ); + assert.equal(fxCacheService.peek(NOW).fetchedAt, fetchedAt); +}); + +test('a snapshot dated exactly now is accepted but still expires at the configured TTL', () => { + let calls = 0; + fxProviders.setProviders([ + { id: 'current', fetch: ({ now }) => { + calls += 1; + return { ratesToUsd: RATES, fetchedAt: now }; + } }, + ]); + const first = fxCacheService.getSnapshot({ now: NOW }); + assert.equal(first.fetchedAt, NOW); + assert.equal(first.expiresAt, NOW + 1_000); + assert.equal(first.stale, false); + assert.equal(calls, 1); + assert.equal(fxCacheService.getSnapshot({ now: NOW + 999 }).cacheHit, true); + assert.equal(calls, 1); + const refreshed = fxCacheService.getSnapshot({ now: NOW + 1_000 }); + assert.equal(calls, 2); + assert.equal(refreshed.cacheHit, false); + assert.equal(refreshed.fetchedAt, NOW + 1_000); +}); From 6c013e9d5873d72416bbb2ff66459d97baf689fb Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 04:56:06 -0400 Subject: [PATCH 07/16] perf(fx): reuse parsed quote expiry across collection scans Memoize a quote's parsed expiry string in a WeakMap rather than reparsing it on every live-quote scan. Changed strings are reparsed, non-string values remain uncached, and grace is still applied at collection time. No quote DTO fields, expiry boundaries, capacity or eviction order change. Weak keys do not themselves retain evicted quote objects. Retain a dependency-free GC-kernel benchmark and three focused node:test regressions. The new tests report 2 pass/1 fail on baseline and 3 pass/0 fail on the patch. Existing tests and application exports remain unchanged. Measured Node 22.16.0, 20,000 live inserts, five warm timed samples: median collector time 2733.05 -> 1730.78 ms (36.67% lower, 1.58x). Separate parse count 7,587,952 -> 19,999 (99.74% lower). Both versions retain the same 288 quote IDs and matching survivor digest. These are isolated actual-GC-code results with store/config fixtures, not HTTP, application, FX-provider or fleet throughput. Heap usage and full application/CI were not measured. The report records complete source blob IDs, all timings, commands and the memory/correctness tradeoffs. Refs RemitFlow/RemitFlow-Backend#129 and existing PR #141. --- .../quote-gc-performance-20261004.md | 67 +++++++++++++++ scripts/bench-quote-gc.js | 69 +++++++++++++++ src/services/quoteService.js | 16 +++- test/quoteGc.test.js | 84 +++++++++++++++++++ 4 files changed, 235 insertions(+), 1 deletion(-) create mode 100644 docs/validation/quote-gc-performance-20261004.md create mode 100644 scripts/bench-quote-gc.js create mode 100644 test/quoteGc.test.js diff --git a/docs/validation/quote-gc-performance-20261004.md b/docs/validation/quote-gc-performance-20261004.md new file mode 100644 index 0000000..e4d7e00 --- /dev/null +++ b/docs/validation/quote-gc-performance-20261004.md @@ -0,0 +1,67 @@ +# Quote GC expiry parsing benchmark + +## Change and boundary + +The collector repeatedly scans retained quotes once the map reaches 256 +entries. Previously every scan reparsed every ISO expiry string, including +unchanged live quotes. Cache the parsed expiry per quote in a WeakMap, keyed +by both object identity and its current expiry string. Non-string values are +still parsed on each access. Grace remains read at collection time; no +retention threshold, eviction order, quote response field, or pricing term +changes. + +The tradeoff is a small cache record per scanned, reachable quote. Weak keys +do not themselves retain an evicted quote. This benchmark does not measure +heap usage or establish a memory reduction. + +## Measured result + +Node v22.16.0, Linux, 20,000 sequential still-live quote insertions with +distinct expiry strings. One warmup and five timed samples per version; +Date.parse calls are counted in a separate untimed run. + +| Measure | Baseline | Patched | +| --- | ---: | ---: | +| Median GC-kernel time | 2733.05 ms | 1730.78 ms | +| Expiry Date.parse calls | 7,587,952 | 19,999 | +| Final retained quotes | 288 | 288 | + +Measured median time falls 36.67% +(1.58x speedup); parse calls fall +99.74%. +Both versions retain exactly quote-19712 through quote-19999, with SHA-256 +`be45a565a0850672b2626ac3e772fb80c4110ea5bb93bdccf9c829a3acfdda51` +for the JSON array of retained IDs. + +Baseline complete source blob: `eb903d4b79e6aba3cfabb33ad256af9469f1b677` at commit +`823d05059cd4f0e6f190d43311ec3c8487ebdc74`. +Patched complete source blob: `0cac87ce332749f8d8ff0bea84a9149e6fbfedb5`. + +Baseline milliseconds: 2763.464, 2733.046, 2657.702, 2667.135, 2735.772. +Patched milliseconds: 1737.870, 1730.777, 1717.096, 1727.002, 1743.507. + +This is a **GC-kernel microbenchmark**, not application, HTTP, transfer, +provider, or fleet throughput. The script loads the complete service source +in an isolated Node VM and exposes its private collector for measurement; +only ordinary store/config fixtures are exercised. Unrelated imports are +not loaded or invoked. VM overhead, retained input records and machine +conditions affect timings. There are no external provider calls, dependency +installs, wall-time thresholds in tests, or production API changes. + +## Reproduce + +```sh +git show 823d05059cd4f0e6f190d43311ec3c8487ebdc74:src/services/quoteService.js > /tmp/quote-service-before.js +node scripts/bench-quote-gc.js /tmp/quote-service-before.js 20000 +node scripts/bench-quote-gc.js src/services/quoteService.js 20000 +node --test test/quoteGc.test.js +``` + +The focused regression file produces 2 passes / 1 failure against baseline +(the ten-scan case reparses 3,000 times rather than 300), and 3 passes / 0 +failures against the patched collector. It also checks unchanged payloads, +changed expiry strings, exact grace boundaries, configuration changes, +invalid/non-string expiry behavior, and insertion-order capacity eviction. +The existing npm test glob includes it. Full application tests and current +CI were not run as part of this isolated measurement; no prior full-suite +result is presented as current evidence. diff --git a/scripts/bench-quote-gc.js b/scripts/bench-quote-gc.js new file mode 100644 index 0000000..04ffae9 --- /dev/null +++ b/scripts/bench-quote-gc.js @@ -0,0 +1,69 @@ +'use strict'; + +// GC-kernel microbenchmark, not HTTP/FX throughput. Load the supplied complete +// service source unchanged, then expose its private GC function in an isolated +// VM. Only store/config are exercised; no application dependencies are loaded. +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); +const { performance } = require('node:perf_hooks'); +const { createHash } = require('node:crypto'); + +const sourcePath = process.argv[2] || path.join(__dirname, '../src/services/quoteService.js'); +const count = Number(process.argv[3] || 20_000); +if (!Number.isSafeInteger(count) || count < 1 || count > 1_000_000) { + throw new Error('Count must be an integer from 1 to 1000000'); +} +const source = fs.readFileSync(sourcePath, 'utf8'); +const now = 1_000_000; +// Distinct valid deadlines prevent identical-string memoization from biasing +// the result. This models repeated GC during a burst of still-live quotes. +const records = Array.from({ length: count }, (_, i) => ({ + quoteId: `quote-${i}`, + quoteExpiresAt: new Date(now + 60_000 + i).toISOString(), +})); + +function sample(countParses) { + const store = { quotes: new Map() }; + const config = { fx: { staleGraceMs: 60_000 } }; + let dateParses = 0; + class BenchDate extends Date {} + if (countParses) BenchDate.parse = (value) => { dateParses += 1; return Date.parse(value); }; + const context = vm.createContext({ + module: { exports: {} }, + Date: BenchDate, Map, WeakMap, Number, + require(id) { + if (id === '../config') return config; + if (id === '../store') return { store }; + return {}; // Other imports are deliberately not exercised by this kernel. + }, + }); + new vm.Script(`${source}\nmodule.exports.benchmarkGc = gcQuotes;`, { filename: sourcePath }) + .runInContext(context); + const gc = context.module.exports.benchmarkGc; + const start = performance.now(); + for (const quote of records) { + gc(now); + store.quotes.set(quote.quoteId, quote); + } + const elapsedMs = performance.now() - start; + return { + elapsedMs, dateParses, + survivors: [...store.quotes.keys()], + }; +} + +sample(false); // Warm up; excluded from reported timings. +const runs = Array.from({ length: 5 }, () => sample(false)); +const counted = sample(true); // Counts are measured separately from wall time. +const elapsed = runs.map((r) => r.elapsedMs).sort((a, b) => a - b); +const survivingIds = JSON.stringify(counted.survivors); +console.log(JSON.stringify({ + kind: 'quote-gc-kernel-not-application-throughput', + node: process.version, + sourceBlob: createHash('sha1').update(`blob ${Buffer.byteLength(source)}\0`).update(source).digest('hex'), + count, samplesMs: runs.map((r) => r.elapsedMs), medianMs: elapsed[2], + dateParses: counted.dateParses, retained: counted.survivors.length, + firstRetained: counted.survivors[0], lastRetained: counted.survivors.at(-1), + retainedIdsSha256: createHash('sha256').update(survivingIds).digest('hex'), +}, null, 2)); diff --git a/src/services/quoteService.js b/src/services/quoteService.js index eb903d4..0cac87c 100644 --- a/src/services/quoteService.js +++ b/src/services/quoteService.js @@ -75,10 +75,24 @@ function remember(quote) { */ const QUOTE_GC_THRESHOLD = 256; +// Quotes are scanned repeatedly while still live. Cache only their parsed +// string timestamp, not the grace-adjusted deadline. Weak keys do not keep +// evicted quotes alive, and a changed expiry string is parsed again. +const quoteExpiryCache = new WeakMap(); + +function quoteExpiryMs(quote) { + const value = quote.quoteExpiresAt; + const cached = quoteExpiryCache.get(quote); + if (cached && cached.value === value) return cached.ms; + const ms = Date.parse(value); + if (typeof value === 'string') quoteExpiryCache.set(quote, { value, ms }); + return ms; +} + function gcQuotes(now) { if (store.quotes.size < QUOTE_GC_THRESHOLD) return; for (const [id, quote] of store.quotes.entries()) { - const expiresAtMs = Date.parse(quote.quoteExpiresAt); + const expiresAtMs = quoteExpiryMs(quote); if (Number.isFinite(expiresAtMs) && expiresAtMs + config.fx.staleGraceMs < now) { store.quotes.delete(id); } diff --git a/test/quoteGc.test.js b/test/quoteGc.test.js new file mode 100644 index 0000000..2be0788 --- /dev/null +++ b/test/quoteGc.test.js @@ -0,0 +1,84 @@ +'use strict'; + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); + +// Exercise the actual private GC without loading unrelated FX/HTTP services. +// Exposing it here does not add an export to the production module. +function loadGc() { + const source = fs.readFileSync(path.join(__dirname, '../src/services/quoteService.js'), 'utf8'); + const store = { quotes: new Map() }; + const config = { fx: { staleGraceMs: 1_000 } }; + let parses = 0; + class CountingDate extends Date { + static parse(value) { parses += 1; return Date.parse(value); } + } + const context = vm.createContext({ + module: { exports: {} }, Date: CountingDate, Map, WeakMap, Number, + require(id) { + if (id === '../store') return { store }; + if (id === '../config') return config; + return {}; + }, + }); + new vm.Script(`${source}\nmodule.exports.testGc = gcQuotes;`).runInContext(context); + return { gc: context.module.exports.testGc, store, config, parses: () => parses }; +} + +const NOW = 1_000_000; +function seed(env, count) { + for (let i = 0; i < count; i += 1) { + env.store.quotes.set(`quote-${i}`, { + quoteId: `quote-${i}`, + quoteExpiresAt: new Date(NOW + 60_000 + i).toISOString(), + }); + } +} + +test('live quote GC parses each unchanged expiry string once without changing payloads', () => { + const env = loadGc(); + seed(env, 300); + const before = JSON.stringify([...env.store.quotes]); + for (let i = 0; i < 10; i += 1) env.gc(NOW + i); + assert.equal(env.parses(), 300); + assert.equal(JSON.stringify([...env.store.quotes]), before); +}); + +test('GC respects changed expiry strings, live grace settings and uncacheable values', () => { + const env = loadGc(); + seed(env, 256); + env.gc(NOW); + const first = env.store.quotes.get('quote-0'); + first.quoteExpiresAt = new Date(NOW - 1_000).toISOString(); + env.gc(NOW); // Exact grace boundary is retained, matching existing policy. + assert.equal(env.store.quotes.has('quote-0'), true); + env.config.fx.staleGraceMs = 999; + env.gc(NOW); + assert.equal(env.store.quotes.has('quote-0'), false); + + env.store.quotes.set('invalid', { quoteExpiresAt: 'not-a-date' }); + env.gc(NOW); + assert.equal(env.store.quotes.has('invalid'), true); + let expires = new Date(NOW + 60_000).toISOString(); + env.store.quotes.get('quote-1').quoteExpiresAt = { toString: () => expires }; + env.gc(NOW); + assert.equal(env.store.quotes.has('quote-1'), true); + expires = new Date(NOW - 1_000).toISOString(); + env.gc(NOW); + assert.equal(env.store.quotes.has('quote-1'), false); +}); + +test('capacity GC keeps the same insertion-order survivors', () => { + const env = loadGc(); + for (let i = 0; i < 530; i += 1) { + env.gc(NOW); + env.store.quotes.set(`quote-${i}`, { quoteExpiresAt: new Date(NOW + 60_000).toISOString() }); + } + assert.deepEqual( + [...env.store.quotes.keys()], + Array.from({ length: 274 }, (_, i) => `quote-${i + 256}`) + ); +}); From a4e32b33f084f8bdd548d10dcd9018b837b9698a Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 05:19:35 -0400 Subject: [PATCH 08/16] fix(fx): reject incomplete provider snapshots before fallback selection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Require every configured supported currency in the provider's copied USD snapshot before freshness admission or cache replacement. Missing or inherited-only rates now fall through to the next provider rather than hiding a healthy fallback and failing supported pairs downstream. Preserve deterministic provider order, complete default rates, freshness admission and the existing structured FX_PROVIDERS_DOWN error. Reproduced on parent 6c013e9d: a USD-only primary was accepted, EUR was missing, and the healthy fallback was never called. Focused validation: node --test test/fxProviderCompleteness.test.js — 3 passed, 0 failed; includes omission of each of the nine supported currencies, inherited-only rates, all-provider failure, complete defaults and freshness admission. Executed with Node 22.16.0 against the actual provider/config/error modules; no live oracle, HTTP deployment or full-suite result is claimed. Refs RemitFlow/RemitFlow-Backend#129; retains existing PR #141. --- src/services/fxProviders.js | 12 ++++- test/fxProviderCompleteness.test.js | 83 +++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 test/fxProviderCompleteness.test.js diff --git a/src/services/fxProviders.js b/src/services/fxProviders.js index 3c09392..e40187c 100644 --- a/src/services/fxProviders.js +++ b/src/services/fxProviders.js @@ -1,6 +1,6 @@ 'use strict'; -const { RATES_TO_USD } = require('../config/rates'); +const { RATES_TO_USD, SUPPORTED_CURRENCIES } = require('../config/rates'); const ApiError = require('../utils/ApiError'); /** @@ -86,7 +86,7 @@ function listProviders() { /** * Walk providers in order until one returns valid rates accepted by the caller. - * Malformed rates or a response outside the caller's freshness policy are + * Incomplete/malformed rates or a response outside the caller's freshness policy are * failed attempts, so neither can hide a usable response from a later provider. * @param {{ now?: number, acceptSnapshot?: (snapshot: FxSnapshot) => boolean }} [opts] * @returns {FxSnapshot} @@ -106,6 +106,14 @@ function fetchWithFallback(opts = {}) { if (values.length === 0 || values.some((rate) => !Number.isFinite(rate) || rate <= 0)) { throw new Error(`Provider ${provider.id} returned invalid rates`); } + // One global snapshot feeds every configured pair. Partial provider + // responses must not displace the complete cache or hide a usable fallback. + // Validate the own-property copy so inherited rates cannot fill gaps. + const missingCurrency = SUPPORTED_CURRENCIES.find((code) => + !Object.prototype.hasOwnProperty.call(ratesToUsd, code)); + if (missingCurrency) { + throw new Error(`Provider ${provider.id} omitted supported currency ${missingCurrency}`); + } const normalized = { providerId: snapshot.providerId || provider.id, ratesToUsd, diff --git a/test/fxProviderCompleteness.test.js b/test/fxProviderCompleteness.test.js new file mode 100644 index 0000000..aaab12a --- /dev/null +++ b/test/fxProviderCompleteness.test.js @@ -0,0 +1,83 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fxProviders = require('../src/services/fxProviders'); +const { RATES_TO_USD, SUPPORTED_CURRENCIES } = require('../src/config/rates'); + +const NOW = 1_791_100_000_000; + +test.afterEach(() => fxProviders.resetProviders()); + +test('an incomplete primary snapshot falls back for every supported currency', () => { + for (const missing of SUPPORTED_CURRENCIES) { + const partial = { ...RATES_TO_USD }; + delete partial[missing]; + const attempts = []; + const accepted = []; + fxProviders.setProviders([ + { id: 'primary', fetch: () => { + attempts.push('primary'); + return { ratesToUsd: partial, fetchedAt: NOW }; + } }, + { id: 'fallback', fetch: () => { + attempts.push('fallback'); + return { ratesToUsd: { ...RATES_TO_USD }, fetchedAt: NOW }; + } }, + ]); + + const snapshot = fxProviders.fetchWithFallback({ now: NOW, acceptSnapshot(candidate) { + accepted.push(candidate.providerId); + return true; + } }); + assert.deepEqual(attempts, ['primary', 'fallback'], missing); + assert.deepEqual(accepted, ['fallback'], 'partial rates never reach cache admission'); + assert.equal(snapshot.providerId, 'fallback'); + assert.deepEqual(snapshot.ratesToUsd, RATES_TO_USD); + } +}); + +test('inherited rates cannot make an incomplete snapshot usable', () => { + const missing = SUPPORTED_CURRENCIES.find((code) => code !== 'USD'); + const inherited = Object.assign(Object.create({ [missing]: RATES_TO_USD[missing] }), RATES_TO_USD); + delete inherited[missing]; + const attempts = []; + fxProviders.setProviders([ + { id: 'primary', fetch: () => { + attempts.push('primary'); + return { ratesToUsd: inherited, fetchedAt: NOW }; + } }, + { id: 'fallback', fetch: () => { + attempts.push('fallback'); + return { ratesToUsd: { USD: 1 }, fetchedAt: NOW }; + } }, + ]); + + assert.throws(() => fxProviders.fetchWithFallback({ now: NOW }), (error) => { + assert.equal(error.statusCode, 503); + assert.equal(error.details.code, 'FX_PROVIDERS_DOWN'); + assert.deepEqual(error.details.attempted.map((attempt) => attempt.providerId), attempts); + assert.deepEqual(attempts, ['primary', 'fallback']); + assert.ok(error.details.attempted.every((attempt) => /omitted supported currency/.test(attempt.message))); + return true; + }); +}); + +test('complete default rates retain provider ordering and freshness admission', () => { + fxProviders.resetProviders(); + const primary = fxProviders.fetchWithFallback({ now: NOW }); + assert.equal(primary.providerId, 'primary'); + assert.equal(primary.fetchedAt, NOW); + assert.deepEqual(primary.ratesToUsd, RATES_TO_USD); + primary.ratesToUsd.USD = 99; + assert.equal(RATES_TO_USD.USD, 1); + + const visited = []; + const fallback = fxProviders.fetchWithFallback({ now: NOW, acceptSnapshot(candidate) { + visited.push(candidate.providerId); + return candidate.providerId === 'fallback'; + } }); + assert.deepEqual(visited, ['primary', 'fallback']); + assert.equal(fallback.providerId, 'fallback'); + assert.equal(fallback.ratesToUsd.USD, 1); +}); From 52b48ffea2b1c228601542f4efbe201316362653 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 05:29:18 -0400 Subject: [PATCH 09/16] test(fx): retain cache regressions with complete rate fixtures Align existing provider fixtures with the all-supported-currencies snapshot contract. Preserve every assertion and the intentionally invalid EUR/GBP values, array responses, expiry boundaries and retry behavior. The new focused completeness tests retain explicit missing-currency cases. Fixture-only follow-through for a4e32b33; no additional test run claimed. --- test/fxCache.test.js | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/test/fxCache.test.js b/test/fxCache.test.js index 59bee8d..1f70d17 100644 --- a/test/fxCache.test.js +++ b/test/fxCache.test.js @@ -5,6 +5,7 @@ const assert = require('node:assert/strict'); const { store, reset } = require('../src/store'); const config = require('../src/config'); +const { RATES_TO_USD } = require('../src/config/rates'); const fxCacheService = require('../src/services/fxCacheService'); const fxProviders = require('../src/services/fxProviders'); const quoteService = require('../src/services/quoteService'); @@ -131,7 +132,7 @@ test('fallback order is primary-then-fallback (deterministic)', () => { seen.push('fallback'); return { providerId: 'fallback', - ratesToUsd: { USD: 1, EUR: 1.08 }, + ratesToUsd: { ...RATES_TO_USD, USD: 1, EUR: 1.08 }, fetchedAt: now, }; }, @@ -150,7 +151,7 @@ test('new provider snapshots obey the requested TTL and grace boundaries before fxCacheService.reset(); fxProviders.setProviders([{ id: 'primary', - fetch: () => ({ ratesToUsd: { USD: 1, EUR: 1.08 }, fetchedAt: now - ageMs }), + fetch: () => ({ ratesToUsd: { ...RATES_TO_USD, USD: 1, EUR: 1.08 }, fetchedAt: now - ageMs }), }]); const usable = ageMs < 1_000 || (policy === 'allow_stale' && ageMs < 6_000); if (!usable) { @@ -184,14 +185,14 @@ test('invalid provider timestamps cannot win over a usable fallback', () => { id: 'primary', fetch: () => { attempted.push('primary'); - return { ratesToUsd: { USD: 1, EUR: 1.08 }, fetchedAt }; + return { ratesToUsd: { ...RATES_TO_USD, USD: 1, EUR: 1.08 }, fetchedAt }; }, }, { id: 'fallback', fetch: () => { attempted.push('fallback'); - return { ratesToUsd: { USD: 1, EUR: 1.1 }, fetchedAt: now }; + return { ratesToUsd: { ...RATES_TO_USD, USD: 1, EUR: 1.1 }, fetchedAt: now }; }, }, ]); @@ -208,16 +209,16 @@ test('malformed provider rates fall back before either policy can cache them', ( const invalidMaps = [ null, {}, - Object.assign([], { USD: 1, EUR: 1.08 }), + Object.assign([], { ...RATES_TO_USD, USD: 1, EUR: 1.08 }), ...[0, -0, -1, NaN, Infinity, -Infinity, '1.08', null, undefined, true] - .map((EUR) => ({ USD: 1, EUR })), - { USD: 1, EUR: 1.08, GBP: 0 }, + .map((EUR) => ({ ...RATES_TO_USD, USD: 1, EUR })), + { ...RATES_TO_USD, USD: 1, EUR: 1.08, GBP: 0 }, ]; for (const policy of ['reject_stale', 'allow_stale']) { for (const ratesToUsd of invalidMaps) { fxCacheService.reset(); const attempted = []; - const healthyRates = { USD: 1, EUR: 1.1 }; + const healthyRates = { ...RATES_TO_USD, USD: 1, EUR: 1.1 }; fxProviders.setProviders([ { id: 'primary', fetch: () => { attempted.push('primary'); @@ -259,7 +260,7 @@ test('fallback order skips responses outside policy and stops at the first usabl id, fetch: () => { attempted.push(id); - return { ratesToUsd: { USD: 1, EUR: 1.08 }, fetchedAt: now - ageMs }; + return { ratesToUsd: { ...RATES_TO_USD, USD: 1, EUR: 1.08 }, fetchedAt: now - ageMs }; }, }))); const snapshot = fxCacheService.getSnapshot({ now, policy }); @@ -277,7 +278,7 @@ test('unusable provider responses preserve a cached display snapshot only within fxCacheService.seed({ fetchedAt, providerId: 'cached' }); fxProviders.setProviders([{ id: 'expired', - fetch: () => ({ ratesToUsd: { USD: 1, EUR: 1.08 }, fetchedAt: now - 6_000 }), + fetch: () => ({ ratesToUsd: { ...RATES_TO_USD, USD: 1, EUR: 1.08 }, fetchedAt: now - 6_000 }), }]); assert.throws( () => fxCacheService.getSnapshot({ now, policy: 'reject_stale' }), @@ -298,11 +299,11 @@ test('unusable provider responses preserve a cached display snapshot only within test('malformed provider rates preserve a cached display snapshot only within grace', () => { const now = 6_450_000; const fetchedAt = now - 1_500; - const ratesToUsd = { USD: 1, EUR: 1.08 }; + const ratesToUsd = { ...RATES_TO_USD, USD: 1, EUR: 1.08 }; fxCacheService.seed({ fetchedAt, providerId: 'cached', ratesToUsd }); fxProviders.setProviders([ - { id: 'primary', fetch: () => ({ ratesToUsd: { USD: 1, EUR: 0 }, fetchedAt: now }) }, - { id: 'fallback', fetch: () => ({ ratesToUsd: { USD: 1, EUR: -1.1 }, fetchedAt: now }) }, + { id: 'primary', fetch: () => ({ ratesToUsd: { ...RATES_TO_USD, USD: 1, EUR: 0 }, fetchedAt: now }) }, + { id: 'fallback', fetch: () => ({ ratesToUsd: { ...RATES_TO_USD, USD: 1, EUR: -1.1 }, fetchedAt: now }) }, ]); assert.throws( () => fxCacheService.getSnapshot({ now, policy: 'reject_stale' }), @@ -349,11 +350,11 @@ test('HTTP invalid FX responses create no records and allow the same transfer ke fxProviders.setProviders([ { id: 'primary', fetch: ({ now }) => { attempted.push('primary'); - return { ratesToUsd: { USD: 1, EUR: 0 }, fetchedAt: now }; + return { ratesToUsd: { ...RATES_TO_USD, USD: 1, EUR: 0 }, fetchedAt: now }; } }, { id: 'fallback', fetch: ({ now }) => { attempted.push('fallback'); - return { ratesToUsd: { USD: 1, EUR: -1.1 }, fetchedAt: now }; + return { ratesToUsd: { ...RATES_TO_USD, USD: 1, EUR: -1.1 }, fetchedAt: now }; } }, ]); for (const request of [() => fetch(`${base}/api/quote?amount=100&from=USD&to=EUR`), post]) { @@ -409,7 +410,7 @@ test('HTTP transfer rejects newly fetched stale rates without reserving a quote reset(); fxProviders.setProviders([{ id: 'primary', - fetch: ({ now }) => ({ ratesToUsd: { USD: 1, EUR: 1.08 }, fetchedAt: now - ageMs }), + fetch: ({ now }) => ({ ratesToUsd: { ...RATES_TO_USD, USD: 1, EUR: 1.08 }, fetchedAt: now - ageMs }), }]); const rejected = await post(); const failure = await rejected.json(); @@ -458,7 +459,7 @@ test('stampede: re-entrant refresh does not start a second provider fetch', () = innerCalls += 1; return { providerId: 'primary', - ratesToUsd: { USD: 1, EUR: 1.08, GBP: 1.27, INR: 0.012 }, + ratesToUsd: { ...RATES_TO_USD, USD: 1, EUR: 1.08, GBP: 1.27, INR: 0.012 }, fetchedAt: now, }; }, From 76c74a0b2a10a2724015d28e089407957dea3a37 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 07:56:20 -0400 Subject: [PATCH 10/16] fix(fx): recheck minted quotes before transfer settlement A synchronous provider can consume the remaining FX or quote TTL after resolveForTransfer captures its initial timestamp. Recheck the minted quote at completion before returning it to settlement; retain explicit caller clocks, immutable quote terms and bounded stale-transfer opt-in. Add four focused regressions to the existing FX test file. Against parent 52b48ffea2b1c228601542f4efbe201316362653 all four fail: expired FX and expired quote terms reach mock settlement, age remains zero, and opted-in stale terms are mislabeled. The repaired source passes all four plus six existing fallback, singleflight and quote-binding controls (10/10). Executed with Node 24.19.0 on the complete production modules and the repository's simulated Stellar adapter, using controlled provider/Date advancement, no sleeps. The local environment has uuid but no dotenv; only dotenv.config initialization was externally shimmed inert, leaving actual config/source unchanged. No HTTP, live-provider, full-suite or CI result is asserted. No dependency or existing assertion changed. Reproduce with installed project dependencies: node --test --test-name-pattern='^(transfer mint completion|primary failure falls back|stampede: re-entrant|transfer creation binds|transfer rejects a quoteId|transfer without quoteId|stale transfer opt-in)' test/fxCache.test.js --- src/services/quoteService.js | 8 +++- test/fxCache.test.js | 80 ++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/src/services/quoteService.js b/src/services/quoteService.js index 0cac87c..adb449d 100644 --- a/src/services/quoteService.js +++ b/src/services/quoteService.js @@ -318,10 +318,16 @@ function resolveForTransfer(data, opts = {}) { return quote; } - return getQuote(data.amount, data.from, data.to, { + const quote = getQuote(data.amount, data.from, data.to, { now, policy: 'reject_stale', }); + // A synchronous provider may consume the remaining FX or quote TTL. + // Check at completion before settlement, retaining an explicit test clock. + return assertUsable(quote, { + now: opts.now != null ? now : Date.now(), + policy: 'reject_stale', + }); } function resetQuoteVersions() { diff --git a/test/fxCache.test.js b/test/fxCache.test.js index 1f70d17..2225122 100644 --- a/test/fxCache.test.js +++ b/test/fxCache.test.js @@ -692,6 +692,86 @@ test('transfer without quoteId still mints and binds a fresh quote (compat)', () assert.equal(stored.sendAmount, transfer.sendAmount); }); +function delayedFxProvider(t, delayMs) { + const clock = { now: 40_000_000, delayMs }; + t.mock.method(Date, 'now', () => clock.now); + fxProviders.setProviders([{ + id: 'primary', + fetch: ({ now }) => { + clock.now += clock.delayMs; + return { providerId: 'primary', ratesToUsd: { ...RATES_TO_USD }, fetchedAt: now }; + }, + }]); + return clock; +} + +test('transfer mint completion rejects FX expiry before settlement', (t) => { + delayedFxProvider(t, config.fx.cacheTtlMs); + const settlement = t.mock.method(require('../src/services/stellarService'), 'submitPayment'); + assert.throws( + () => transferService.createTransfer(PAYLOAD, 'req-delayed-fx'), + (err) => err instanceof ApiError && err.details.code === 'QUOTE_STALE' && + err.details.freshness.status === 'stale' && + err.details.freshness.ageMs === config.fx.cacheTtlMs + ); + assert.equal(settlement.mock.callCount(), 0); + assert.equal(store.transfers.size, 0); +}); + +test('transfer mint completion rejects quote expiry before settlement', (t) => { + config.fx.cacheTtlMs = 3_000; + config.fx.quoteTtlMs = 1_000; + delayedFxProvider(t, config.fx.quoteTtlMs); + const settlement = t.mock.method(require('../src/services/stellarService'), 'submitPayment'); + assert.throws( + () => transferService.createTransfer(PAYLOAD, 'req-delayed-quote'), + (err) => err instanceof ApiError && err.details.code === 'QUOTE_EXPIRED' + ); + assert.equal(settlement.mock.callCount(), 0); + assert.equal(store.transfers.size, 0); +}); + +test('transfer mint completion refreshes age while preserving terms and explicit clocks', (t) => { + delayedFxProvider(t, 100); + const bound = quoteService.resolveForTransfer(PAYLOAD); + const issued = quoteService.getQuoteById(bound.quoteId); + assert.equal(bound.stale, false); + assert.equal(bound.freshness.status, 'fresh'); + assert.equal(bound.freshness.ageMs, 100); + assert.equal(issued.freshness.ageMs, 0); + for (const field of ['quoteId', 'quoteVersion', 'rate', 'receiveAmount', 'quoteExpiresAt']) { + assert.equal(bound[field], issued[field]); + } + assert.equal(fxCacheService.getProviderFetchCount(), 1); + + reset(); + // The optional clock remains deterministic even when wall time is later. + const fixed = quoteService.resolveForTransfer(PAYLOAD, { now: 12_000_000 }); + assert.equal(fixed.quoteCreatedAt, new Date(12_000_000).toISOString()); + assert.equal(fixed.freshness.ageMs, 0); + assert.equal(fixed.stale, false); +}); + +test('transfer mint completion retains only the configured stale grace opt-in', (t) => { + config.fx.allowStaleForTransfers = true; + config.fx.quoteTtlMs = 10_000; + const clock = delayedFxProvider(t, config.fx.cacheTtlMs); + const transfer = transferService.createTransfer(PAYLOAD, 'req-delayed-allowed'); + assert.equal(transfer.rateStale, true); + assert.equal(transfer.rate, 0.93); + assert.equal(transfer.receiveAmount, 90.93); + + clock.delayMs = config.fx.cacheTtlMs + config.fx.staleGraceMs; + const settlement = t.mock.method(require('../src/services/stellarService'), 'submitPayment'); + assert.throws( + () => transferService.createTransfer(PAYLOAD, 'req-delayed-expired'), + (err) => err instanceof ApiError && err.details.code === 'QUOTE_STALE' && + err.details.freshness.status === 'expired' + ); + assert.equal(settlement.mock.callCount(), 0); + assert.equal(store.transfers.size, 1); +}); + test('provider outage blocks transfer pricing rather than using silent stale rates', () => { // Regression for the original failure mode: outage must not price transfers // on an expired rate that looks "current" because freshness was invisible. From e91ea67551d9c5f58b4557b4dd93c77b319fcfe4 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 07:59:34 -0400 Subject: [PATCH 11/16] fix(fx): account for provider execution time and preserve rate precision Recheck provider admission and outage grace at completion. Each fallback sees the current clock; explicitly supplied times remain deterministic. A real 41 ms fetch under a 20 ms TTL previously returned stale data as fresh, age zero; it now selects the healthy fallback with accurate freshness. Reuse the existing rate-precision correction from b685aae: NGN/USD remains 0.00065 instead of zero in pair and quote responses. Preserve amount rounding, snapshot provenance, and the transfer-completion repair from 76c74a0. Add four timing and two precision regressions; align complete-rate and exact-ratio fixtures without dropping assertions. Focused cache checks passed 8/8, precision 2/2, and the four new transfer-completion controls pass after the exact-ratio fixture correction. Actual production modules on Node 24.19.0 with retained dotenv 17.3.1 and uuid 3.4.0; dependency versions differ from the project lock. No full-suite, HTTP, live-provider or hosted CI result asserted. --- src/services/fxCacheService.js | 28 +++++--- src/services/quoteService.js | 3 +- src/services/rateService.js | 3 +- test/exchangeRatePrecision.test.js | 49 +++++++++++++ test/fxCache.test.js | 4 +- test/fxFutureTimestamp.test.js | 4 +- test/fxProviderTiming.test.js | 107 +++++++++++++++++++++++++++++ 7 files changed, 183 insertions(+), 15 deletions(-) create mode 100644 test/exchangeRatePrecision.test.js create mode 100644 test/fxProviderTiming.test.js diff --git a/src/services/fxCacheService.js b/src/services/fxCacheService.js index 44660c6..91201bc 100644 --- a/src/services/fxCacheService.js +++ b/src/services/fxCacheService.js @@ -75,14 +75,18 @@ function decorate(entry, now, extra) { * Pull from providers (counted), accepting only snapshots within policy before * choosing a provider or replacing the cache. A successful fetch is not proof * that the provider's timestamp is current. - * @param {number} now + * @param {() => number} readNow * @param {'reject_stale'|'allow_stale'} policy */ -function refresh(now, policy) { +function refresh(readNow, policy) { providerFetchCount += 1; + let acceptedAt; const snapshot = fxProviders.fetchWithFallback({ - now, + // Each fallback starts at the current time. A slow earlier provider must + // not make the next provider inherit an already-expired request timestamp. + get now() { return readNow(); }, acceptSnapshot(candidate) { + const now = readNow(); const expiresAt = candidate.fetchedAt + ttlMs(); // A future provider timestamp would extend freshness beyond the local // TTL. Reject it instead of clamping away its provenance so a usable @@ -92,7 +96,9 @@ function refresh(now, policy) { return false; } const status = classify({ expiresAt }, now); - return status === 'fresh' || (policy === 'allow_stale' && status === 'stale'); + const accepted = status === 'fresh' || (policy === 'allow_stale' && status === 'stale'); + if (accepted) acceptedAt = now; + return accepted; }, }); const entry = { @@ -102,7 +108,7 @@ function refresh(now, policy) { expiresAt: snapshot.fetchedAt + ttlMs(), }; cache.set(CACHE_KEY, entry); - return decorate(entry, now, { cacheHit: false, source: 'provider' }); + return decorate(entry, acceptedAt, { cacheHit: false, source: 'provider' }); } /** @@ -122,7 +128,10 @@ function refresh(now, policy) { * @returns {ReturnType} */ function getSnapshot(opts = {}) { - const now = opts.now != null ? opts.now : Date.now(); + // An explicit time remains a deterministic snapshot for callers/tests. + // Normal requests must account for time spent inside synchronous providers. + const readNow = opts.now != null ? () => opts.now : () => Date.now(); + const now = readNow(); const policy = opts.policy || 'reject_stale'; const entry = cache.get(CACHE_KEY); @@ -145,13 +154,14 @@ function getSnapshot(opts = {}) { inflight.add(CACHE_KEY); try { - return refresh(now, policy); + return refresh(readNow, policy); } catch (err) { // Provider path failed. Under allow_stale, a within-grace entry is still // usable for display — but it is visibly stale. Under reject_stale we // never price with it. - if (entry && classify(entry, now) === 'stale' && policy === 'allow_stale') { - return decorate(entry, now, { cacheHit: true, source: 'cache-stale-outage' }); + const failedAt = readNow(); + if (entry && classify(entry, failedAt) === 'stale' && policy === 'allow_stale') { + return decorate(entry, failedAt, { cacheHit: true, source: 'cache-stale-outage' }); } throw err; } finally { diff --git a/src/services/quoteService.js b/src/services/quoteService.js index adb449d..37e4a22 100644 --- a/src/services/quoteService.js +++ b/src/services/quoteService.js @@ -167,7 +167,8 @@ function getQuote(amount, from, to, opts = {}) { sendAmount: numericAmount, fee, amountAfterFee, - rate: money.round(rate), + // Keep the conversion ratio; money precision applies to amounts only. + rate, receiveAmount, stale: snapshot.stale, freshness: freshnessFromSnapshot(snapshot), diff --git a/src/services/rateService.js b/src/services/rateService.js index 8f89680..fdd533c 100644 --- a/src/services/rateService.js +++ b/src/services/rateService.js @@ -134,7 +134,8 @@ function getPair(from, to, opts = {}) { return { from: fromCode, to: toCode, - rate: money.round(rate), + // Applying minor-unit rounding here can turn a valid FX rate into zero. + rate, freshness: { status: snapshot.status, stale: snapshot.stale, diff --git a/test/exchangeRatePrecision.test.js b/test/exchangeRatePrecision.test.js new file mode 100644 index 0000000..b905bba --- /dev/null +++ b/test/exchangeRatePrecision.test.js @@ -0,0 +1,49 @@ +'use strict'; + +const { test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { RATES_TO_USD } = require('../src/config/rates'); +const rateService = require('../src/services/rateService'); +const quoteService = require('../src/services/quoteService'); +const fxCacheService = require('../src/services/fxCacheService'); +const { store } = require('../src/store'); + +const NOW = 10_000_000; + +beforeEach(() => { + fxCacheService.reset(); + store.quotes.clear(); + quoteService.resetQuoteVersions(); + fxCacheService.seed({ + ratesToUsd: RATES_TO_USD, + fetchedAt: NOW, + providerId: 'precision-fixture', + }); +}); + +afterEach(() => { + fxCacheService.reset(); + store.quotes.clear(); +}); + +test('a cached pair retains a small positive FX ratio and snapshot provenance', () => { + const pair = rateService.getPair('NGN', 'USD', { now: NOW }); + assert.equal(pair.rate, 0.00065); + assert.equal(JSON.parse(JSON.stringify(pair)).rate, 0.00065); + assert.equal(pair.freshness.providerId, 'precision-fixture'); + assert.equal(pair.freshness.fetchedAt, new Date(NOW).toISOString()); + assert.equal(fxCacheService.getProviderFetchCount(), 0); +}); + +test('a quote preserves the FX ratio while fee and receive amounts stay rounded', () => { + const quote = quoteService.getQuote(10000, 'NGN', 'USD', { now: NOW }); + assert.equal(quote.rate, 0.00065); + assert.equal(JSON.parse(JSON.stringify(quote)).rate, 0.00065); + assert.deepEqual( + [quote.sendAmount, quote.fee, quote.amountAfterFee, quote.receiveAmount], + [10000, 150.3, 9849.7, 6.4] + ); + assert.equal(quote.freshness.providerId, 'precision-fixture'); + assert.equal(quote.freshness.fetchedAt, new Date(NOW).toISOString()); + assert.equal(fxCacheService.getProviderFetchCount(), 0); +}); diff --git a/test/fxCache.test.js b/test/fxCache.test.js index 2225122..8748eca 100644 --- a/test/fxCache.test.js +++ b/test/fxCache.test.js @@ -376,7 +376,7 @@ test('HTTP invalid FX responses create no records and allow the same transfer ke assert.equal(recovered.status, 201); assert.equal(transfer.rateStale, false); assert.equal(transfer.rateProvider, 'primary'); - assert.equal(transfer.rate, 0.93); + assert.equal(transfer.rate, RATES_TO_USD.USD / RATES_TO_USD.EUR); assert.equal(transfer.receiveAmount, 90.93); assert.equal(store.transfers.size, 1); assert.equal(store.quotes.size, 1); @@ -758,7 +758,7 @@ test('transfer mint completion retains only the configured stale grace opt-in', const clock = delayedFxProvider(t, config.fx.cacheTtlMs); const transfer = transferService.createTransfer(PAYLOAD, 'req-delayed-allowed'); assert.equal(transfer.rateStale, true); - assert.equal(transfer.rate, 0.93); + assert.equal(transfer.rate, RATES_TO_USD.USD / RATES_TO_USD.EUR); assert.equal(transfer.receiveAmount, 90.93); clock.delayMs = config.fx.cacheTtlMs + config.fx.staleGraceMs; diff --git a/test/fxFutureTimestamp.test.js b/test/fxFutureTimestamp.test.js index 6e8379f..1eb14aa 100644 --- a/test/fxFutureTimestamp.test.js +++ b/test/fxFutureTimestamp.test.js @@ -9,7 +9,7 @@ const fxProviders = require('../src/services/fxProviders'); const ORIGINAL_TTL = config.fx.cacheTtlMs; const ORIGINAL_GRACE = config.fx.staleGraceMs; const NOW = 10_000_000; -const RATES = { USD: 1, EUR: 1.08 }; +const { RATES_TO_USD: RATES } = require('../src/config/rates'); const isProvidersDown = (err) => err.statusCode === 503 && err.details && err.details.code === 'FX_PROVIDERS_DOWN'; @@ -80,7 +80,7 @@ test('future provider data cannot replace a stale display cache or extend its gr const fetchedAt = NOW - 1_500; fxCacheService.seed({ fetchedAt, ratesToUsd: RATES, providerId: 'cached' }); fxProviders.setProviders([ - { id: 'future', fetch: () => ({ ratesToUsd: { USD: 1, EUR: 2 }, fetchedAt: NOW + 86_400_000 }) }, + { id: 'future', fetch: () => ({ ratesToUsd: { ...RATES, EUR: 2 }, fetchedAt: NOW + 86_400_000 }) }, ]); assert.throws( () => fxCacheService.getSnapshot({ now: NOW, policy: 'reject_stale' }), diff --git a/test/fxProviderTiming.test.js b/test/fxProviderTiming.test.js new file mode 100644 index 0000000..7f3ead7 --- /dev/null +++ b/test/fxProviderTiming.test.js @@ -0,0 +1,107 @@ +'use strict'; + +const { test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const config = require('../src/config'); +const cache = require('../src/services/fxCacheService'); +const providers = require('../src/services/fxProviders'); +const { RATES_TO_USD } = require('../src/config/rates'); + +const ORIGINAL_TTL = config.fx.cacheTtlMs; +const ORIGINAL_GRACE = config.fx.staleGraceMs; +const ORIGINAL_NOW = Date.now; +const NOW = 10_000_000; +let clock; + +beforeEach(() => { + clock = NOW; + Date.now = () => clock; + config.fx.cacheTtlMs = 1_000; + config.fx.staleGraceMs = 5_000; + cache.reset(); + providers.resetProviders(); +}); + +afterEach(() => { + Date.now = ORIGINAL_NOW; + config.fx.cacheTtlMs = ORIGINAL_TTL; + config.fx.staleGraceMs = ORIGINAL_GRACE; + cache.reset(); + providers.resetProviders(); +}); + +test('a provider that expires during its fetch yields to a current fallback', () => { + const attempts = []; + providers.setProviders([ + { id: 'slow', fetch: ({ now }) => { + attempts.push('slow'); + clock += 2_000; + return { ratesToUsd: RATES_TO_USD, fetchedAt: now }; + } }, + { id: 'fallback', fetch: ({ now }) => { + attempts.push('fallback'); + return { ratesToUsd: RATES_TO_USD, fetchedAt: now }; + } }, + ]); + + const snapshot = cache.getSnapshot({ policy: 'reject_stale' }); + assert.deepEqual(attempts, ['slow', 'fallback']); + assert.equal(snapshot.providerId, 'fallback'); + assert.equal(snapshot.fetchedAt, clock); + assert.equal(snapshot.expiresAt, clock + 1_000); + assert.equal(snapshot.status, 'fresh'); + assert.equal(snapshot.ageMs, 0); + assert.equal(cache.getProviderFetchCount(), 1); +}); + +test('display data ages through provider execution and is marked stale on return', () => { + providers.setProviders([ + { id: 'slow', fetch: ({ now }) => { + clock += 1_500; + return { ratesToUsd: RATES_TO_USD, fetchedAt: now }; + } }, + ]); + + const snapshot = cache.getSnapshot({ policy: 'allow_stale' }); + assert.equal(snapshot.providerId, 'slow'); + assert.equal(snapshot.fetchedAt, NOW); + assert.equal(snapshot.expiresAt, NOW + 1_000); + assert.equal(snapshot.ageMs, 1_500); + assert.equal(snapshot.status, 'stale'); + assert.equal(snapshot.stale, true); +}); + +test('an outage cannot return cached data after its grace expires during the fetch', () => { + cache.seed({ fetchedAt: NOW - 1_500, providerId: 'cached' }); + providers.setProviders([ + { id: 'down', fetch: () => { + clock += 5_000; + throw new Error('provider unavailable'); + } }, + ]); + + assert.throws(() => cache.getSnapshot({ policy: 'allow_stale' }), (error) => { + assert.equal(error.statusCode, 503); + assert.equal(error.details.code, 'FX_PROVIDERS_DOWN'); + return true; + }); + assert.equal(cache.peek().providerId, 'cached'); + assert.equal(cache.peek().status, 'expired'); + providers.resetProviders(); + assert.equal(cache.getSnapshot().providerId, 'primary'); +}); + +test('an explicit snapshot time stays deterministic even if the wall clock advances', () => { + providers.setProviders([ + { id: 'fixed', fetch: ({ now }) => { + clock += 2_000; + return { ratesToUsd: RATES_TO_USD, fetchedAt: now }; + } }, + ]); + + const snapshot = cache.getSnapshot({ now: NOW }); + assert.equal(snapshot.providerId, 'fixed'); + assert.equal(snapshot.fetchedAt, NOW); + assert.equal(snapshot.status, 'fresh'); + assert.equal(snapshot.ageMs, 0); +}); From be2152a0deabe52b21a5f3cc89678c7d8ca2ff3b Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 08:05:46 -0400 Subject: [PATCH 12/16] fix(fx): preserve live provider clocks through quote creation Forward only explicit caller timestamps from quote and implicit-transfer paths. Normal requests now reach the provider-completion checks and healthy fallback instead of freezing admission time before provider work. Honor the existing bounded stale-transfer opt-in when minting; retain completion-time quote validation and all rate precision changes. Two added regressions fail on e91ea675: a display quote accepts an expired primary and an unquoted transfer rejects before trying the healthy fallback. Both now succeed with the current fallback. The combined six timing/boundary cases pass with actual production modules and the existing simulated settlement adapter. Expired provider responses now raise the existing FX_PROVIDERS_DOWN 503 before any quote or settlement; expired issued quote terms still raise QUOTE_EXPIRED. Within-grace opt-in, explicit clocks, zero settlement on refusal, quote identity and exact rates remain covered. Existing assertions now reflect current issuance age and the earlier provider rejection point. Node 24.19.0 with retained dotenv 17.3.1 and uuid 3.4.0; no HTTP, live-provider, full-suite or hosted CI claim. --- src/services/quoteService.js | 8 +++--- test/fxCache.test.js | 47 +++++++++++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/services/quoteService.js b/src/services/quoteService.js index 37e4a22..cae92cf 100644 --- a/src/services/quoteService.js +++ b/src/services/quoteService.js @@ -147,7 +147,9 @@ function getQuote(amount, from, to, opts = {}) { const fee = calculateFee(numericAmount); const amountAfterFee = money.round(numericAmount - fee); - const snapshot = rateService.getSnapshot({ now, policy }); + // Leave normal requests on the cache's live clock through provider work. + // Only an explicitly supplied snapshot time should freeze admission time. + const snapshot = rateService.getSnapshot({ now: opts.now, policy }); if (!Object.prototype.hasOwnProperty.call(snapshot.ratesToUsd, fromCode)) { throw ApiError.badRequest(`Unsupported source currency: ${from}`); } @@ -320,8 +322,8 @@ function resolveForTransfer(data, opts = {}) { } const quote = getQuote(data.amount, data.from, data.to, { - now, - policy: 'reject_stale', + now: opts.now, + policy: config.fx.allowStaleForTransfers ? 'allow_stale' : 'reject_stale', }); // A synchronous provider may consume the remaining FX or quote TTL. // Check at completion before settlement, retaining an explicit test clock. diff --git a/test/fxCache.test.js b/test/fxCache.test.js index 8748eca..9c5e998 100644 --- a/test/fxCache.test.js +++ b/test/fxCache.test.js @@ -705,17 +705,51 @@ function delayedFxProvider(t, delayMs) { return clock; } +test('quote refresh uses the completion clock to reach a healthy fallback', (t) => { + config.fx.quoteTtlMs = 10_000; + const clock = delayedFxProvider(t, config.fx.cacheTtlMs + config.fx.staleGraceMs); + const primary = fxProviders.listProviders()[0]; + fxProviders.setProviders([primary, { + id: 'fallback', + fetch: ({ now }) => ({ ratesToUsd: { ...RATES_TO_USD }, fetchedAt: now }), + }]); + + const quote = quoteService.getQuote(PAYLOAD.amount, PAYLOAD.from, PAYLOAD.to); + assert.equal(quote.freshness.providerId, 'fallback'); + assert.equal(quote.freshness.fetchedAt, new Date(clock.now).toISOString()); + assert.equal(quote.freshness.status, 'fresh'); + assert.equal(quote.freshness.ageMs, 0); + assert.equal(quote.stale, false); + assert.equal(fxCacheService.getProviderFetchCount(), 1); +}); + +test('unquoted transfers use the completion clock to reach a healthy fallback', (t) => { + const clock = delayedFxProvider(t, config.fx.cacheTtlMs); + const primary = fxProviders.listProviders()[0]; + fxProviders.setProviders([primary, { + id: 'fallback', + fetch: ({ now }) => ({ ratesToUsd: { ...RATES_TO_USD }, fetchedAt: now }), + }]); + + const transfer = transferService.createTransfer(PAYLOAD, 'req-current-fallback'); + assert.equal(transfer.rateProvider, 'fallback'); + assert.equal(transfer.rateFetchedAt, new Date(clock.now).toISOString()); + assert.equal(transfer.rateStale, false); + assert.equal(store.transfers.size, 1); + assert.equal(fxCacheService.getProviderFetchCount(), 1); +}); + test('transfer mint completion rejects FX expiry before settlement', (t) => { delayedFxProvider(t, config.fx.cacheTtlMs); const settlement = t.mock.method(require('../src/services/stellarService'), 'submitPayment'); assert.throws( () => transferService.createTransfer(PAYLOAD, 'req-delayed-fx'), - (err) => err instanceof ApiError && err.details.code === 'QUOTE_STALE' && - err.details.freshness.status === 'stale' && - err.details.freshness.ageMs === config.fx.cacheTtlMs + (err) => err instanceof ApiError && err.statusCode === 503 && + err.details.code === 'FX_PROVIDERS_DOWN' ); assert.equal(settlement.mock.callCount(), 0); assert.equal(store.transfers.size, 0); + assert.equal(store.quotes.size, 0); }); test('transfer mint completion rejects quote expiry before settlement', (t) => { @@ -738,7 +772,7 @@ test('transfer mint completion refreshes age while preserving terms and explicit assert.equal(bound.stale, false); assert.equal(bound.freshness.status, 'fresh'); assert.equal(bound.freshness.ageMs, 100); - assert.equal(issued.freshness.ageMs, 0); + assert.equal(issued.freshness.ageMs, 100); for (const field of ['quoteId', 'quoteVersion', 'rate', 'receiveAmount', 'quoteExpiresAt']) { assert.equal(bound[field], issued[field]); } @@ -765,11 +799,12 @@ test('transfer mint completion retains only the configured stale grace opt-in', const settlement = t.mock.method(require('../src/services/stellarService'), 'submitPayment'); assert.throws( () => transferService.createTransfer(PAYLOAD, 'req-delayed-expired'), - (err) => err instanceof ApiError && err.details.code === 'QUOTE_STALE' && - err.details.freshness.status === 'expired' + (err) => err instanceof ApiError && err.statusCode === 503 && + err.details.code === 'FX_PROVIDERS_DOWN' ); assert.equal(settlement.mock.callCount(), 0); assert.equal(store.transfers.size, 1); + assert.equal(fxCacheService.peek(clock.now).status, 'expired'); }); test('provider outage blocks transfer pricing rather than using silent stale rates', () => { From 8f3bf8b8e16ac01cc9ef9cce9d66772e03fc59ba Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 08:33:31 -0400 Subject: [PATCH 13/16] fix(fx): keep provider exception details out of public errors Provider failures were copied verbatim into ApiError.details and serialized by the HTTP error handler. Adapter exceptions may contain credentials, URLs or upstream response bodies. Keep each configured provider ID and ordered attempt, but expose only the stable message "FX provider failed". Preserve FX_PROVIDERS_DOWN, HTTP 503, snapshot validation, freshness and fallback selection. Add one maintained HTTP regression using synthetic Error and string failures. It verifies safe ordered attempts, no cached snapshot or persisted quote/transfer on failure, and successful fallback recovery while the primary still fails. Executed node --test test/fxProviderErrors.test.js with locked npm ci dependencies on Ubuntu 24.04.5 / Node 22.23.3 / npm 10.9.9. Original provider blob e40187ce1c37095d08a59baf996f1f8eef73b1ad: 0 pass, 1 expected assertion failure exposing the synthetic message. Repaired provider blob eea2bb6f4702d0a669f0de3c02b9ea04edc90f0d: 1 pass, 0 fail, 0 skipped. Test blob acc6b9959e4bb1727e6f870e6188f58f55c6f689. No full-suite or live-provider claim. Execution: https://github.com/woahwhattheheck/RemitFlow-Backend/actions/runs/37202463765 Artifact: fx-provider-error-before-after, ID 11303342530, SHA256 1744513a14263afe1bf70f717186a5ce03d2d0d7e59b76548b447b9b64a52c95. Validation workflow remains on its separate branch; original CI configuration is unchanged. --- src/services/fxProviders.js | 6 ++- test/fxProviderErrors.test.js | 71 +++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 test/fxProviderErrors.test.js diff --git a/src/services/fxProviders.js b/src/services/fxProviders.js index e40187c..eea2bb6 100644 --- a/src/services/fxProviders.js +++ b/src/services/fxProviders.js @@ -123,10 +123,12 @@ function fetchWithFallback(opts = {}) { throw new Error(`Provider ${provider.id} returned a snapshot outside the requested freshness policy`); } return normalized; - } catch (err) { + } catch { + // Attempts are serialized in public HTTP errors. Adapter exceptions can + // contain credentials or upstream response bodies, so never expose them. errors.push({ providerId: provider.id, - message: err && err.message ? err.message : String(err), + message: 'FX provider failed', }); } } diff --git a/test/fxProviderErrors.test.js b/test/fxProviderErrors.test.js new file mode 100644 index 0000000..acc6b99 --- /dev/null +++ b/test/fxProviderErrors.test.js @@ -0,0 +1,71 @@ +'use strict'; + +process.env.NODE_ENV = 'test'; + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const createApp = require('../src/app'); +const { store, reset } = require('../src/store'); +const { RATES_TO_USD } = require('../src/config/rates'); +const fxProviders = require('../src/services/fxProviders'); +const fxCacheService = require('../src/services/fxCacheService'); + +test('HTTP FX failures hide adapter messages and retain ordered fallback recovery', async () => { + reset(); + const server = createApp().listen(0, '127.0.0.1'); + await new Promise((resolve, reject) => { + server.once('listening', resolve); + server.once('error', reject); + }); + const url = `http://127.0.0.1:${server.address().port}/api/quote?amount=100&from=USD&to=EUR`; + const attempted = []; + const primary = { + id: 'primary', + fetch() { + attempted.push('primary'); + throw new Error('GET https://provider.invalid/rates?api_key=synthetic-secret-primary'); + }, + }; + try { + fxProviders.setProviders([ + primary, + { id: 'fallback', fetch() { + attempted.push('fallback'); + throw 'Bearer synthetic-secret-fallback'; + } }, + ]); + const rejected = await fetch(url); + const failure = await rejected.json(); + assert.equal(rejected.status, 503); + assert.equal(failure.error.message, 'All FX providers failed'); + assert.equal(failure.error.details.code, 'FX_PROVIDERS_DOWN'); + assert.deepEqual(attempted, ['primary', 'fallback']); + assert.deepEqual(failure.error.details.attempted, [ + { providerId: 'primary', message: 'FX provider failed' }, + { providerId: 'fallback', message: 'FX provider failed' }, + ]); + assert.equal(JSON.stringify(failure).includes('synthetic-secret'), false); + assert.equal(store.quotes.size, 0); + assert.equal(store.transfers.size, 0); + assert.equal(fxCacheService.peek(), null); + + fxProviders.setProviders([ + primary, + { id: 'fallback', fetch({ now }) { + attempted.push('fallback'); + return { ratesToUsd: { ...RATES_TO_USD }, fetchedAt: now }; + } }, + ]); + const recovered = await fetch(url); + const quote = await recovered.json(); + assert.equal(recovered.status, 200); + assert.equal(quote.freshness.providerId, 'fallback'); + assert.equal(quote.stale, false); + assert.deepEqual(attempted, ['primary', 'fallback', 'primary', 'fallback']); + assert.equal(store.quotes.size, 1); + assert.equal(store.transfers.size, 0); + } finally { + await new Promise((resolve, reject) => server.close((err) => err ? reject(err) : resolve())); + reset(); + } +}); From f2c4741ab9be4970801c9ae87089076c8cc91790 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 08:59:58 -0400 Subject: [PATCH 14/16] fix(fx): fall back when provider cross-rates are unrepresentable Positive finite table entries do not guarantee usable currency ratios. Reject a snapshot when its maximum/minimum configured rate overflows, before snapshot acceptance or cache replacement, so a usable later provider can win. This also excludes tables whose reverse ratio underflows to zero. Preserve provider order, cloned rate values, freshness admission and the existing redacted FX_PROVIDERS_DOWN error. Per-request converted-amount bounds remain separate. Add three focused Node tests for malformed-table fallback, exhaustion with ordered diagnostics, and valid large common scales. Executed actual production fxProviders, rates and ApiError modules without substitutes or dependency installation on Node v22.16.0: node --test test/fxProviderRatios.test.js Baseline provider eea2bb6f4702d0a669f0de3c02b9ea04edc90f0d: 1 pass, 2 fail. Changed provider feaff602fe217dfeeaf705dbbbd3669cbfec97dd: 3 pass, 0 fail, 0 skipped. Test blob d60d988bb97de4db1d64ac22e52c72f455f7d813; unchanged rates 52f235a875affcbf31a15ac19108d2a12c0e6bad and ApiError 6b586f60d991ab4e5174e0af4e0d94cf1e3ceb58 were byte-verified before execution. Only provider admission and its focused regression change. This is module-level local evidence, not a full-suite, HTTP, real-provider or settlement run. No CI/dependency or existing contributor-history changes. --- src/services/fxProviders.js | 12 ++++++ test/fxProviderRatios.test.js | 74 +++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 test/fxProviderRatios.test.js diff --git a/src/services/fxProviders.js b/src/services/fxProviders.js index eea2bb6..feaff60 100644 --- a/src/services/fxProviders.js +++ b/src/services/fxProviders.js @@ -114,6 +114,18 @@ function fetchWithFallback(opts = {}) { if (missingCurrency) { throw new Error(`Provider ${provider.id} omitted supported currency ${missingCurrency}`); } + // Finite positive entries can still yield Infinity (or zero in the + // reverse direction) when divided. Every configured pair uses this + // snapshot, so reject it before it can suppress a usable fallback. + let minRate = Infinity; + let maxRate = 0; + for (const code of SUPPORTED_CURRENCIES) { + minRate = Math.min(minRate, ratesToUsd[code]); + maxRate = Math.max(maxRate, ratesToUsd[code]); + } + if (!Number.isFinite(maxRate / minRate)) { + throw new Error(`Provider ${provider.id} returned unrepresentable cross-rates`); + } const normalized = { providerId: snapshot.providerId || provider.id, ratesToUsd, diff --git a/test/fxProviderRatios.test.js b/test/fxProviderRatios.test.js new file mode 100644 index 0000000..d60d988 --- /dev/null +++ b/test/fxProviderRatios.test.js @@ -0,0 +1,74 @@ +'use strict'; + +const { test, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fxProviders = require('../src/services/fxProviders'); +const { RATES_TO_USD } = require('../src/config/rates'); + +afterEach(() => fxProviders.resetProviders()); + +test('unrepresentable cross-rates try the next provider before snapshot acceptance', () => { + const malformedTables = [ + { ...RATES_TO_USD, EUR: Number.MIN_VALUE }, + { ...RATES_TO_USD, EUR: Number.MAX_VALUE }, + { ...RATES_TO_USD, EUR: Number.MIN_VALUE, GBP: Number.MAX_VALUE }, + ]; + for (const ratesToUsd of malformedTables) { + const attempts = []; + const accepted = []; + fxProviders.setProviders([ + { id: 'primary', fetch: () => { + attempts.push('primary'); + return { ratesToUsd, fetchedAt: 1000 }; + } }, + { id: 'fallback', fetch: () => { + attempts.push('fallback'); + return { ratesToUsd: RATES_TO_USD, fetchedAt: 1000 }; + } }, + ]); + const result = fxProviders.fetchWithFallback({ + now: 1000, + acceptSnapshot: (snapshot) => { + accepted.push(snapshot.providerId); + return true; + }, + }); + assert.equal(result.providerId, 'fallback'); + assert.deepEqual(attempts, ['primary', 'fallback']); + assert.deepEqual(accepted, ['fallback']); + assert.deepEqual(result.ratesToUsd, RATES_TO_USD); + assert.notStrictEqual(result.ratesToUsd, RATES_TO_USD); + } +}); + +test('only unusable cross-rate tables produce the existing ordered unavailable error', () => { + fxProviders.setProviders([ + { id: 'primary', fetch: () => ({ ratesToUsd: { ...RATES_TO_USD, EUR: Number.MIN_VALUE } }) }, + { id: 'fallback', fetch: () => ({ ratesToUsd: { ...RATES_TO_USD, GBP: Number.MAX_VALUE } }) }, + ]); + assert.throws(() => fxProviders.fetchWithFallback({ now: 1000 }), (err) => { + assert.equal(err.statusCode, 503); + assert.equal(err.details.code, 'FX_PROVIDERS_DOWN'); + assert.deepEqual(err.details.attempted, [ + { providerId: 'primary', message: 'FX provider failed' }, + { providerId: 'fallback', message: 'FX provider failed' }, + ]); + return true; + }); +}); + +test('large common rate scales retain ordinary first-success behavior', () => { + const ratesToUsd = Object.fromEntries( + Object.entries(RATES_TO_USD).map(([code, rate]) => [code, rate * 1e100]) + ); + let fallbackCalls = 0; + fxProviders.setProviders([ + { id: 'primary', fetch: () => ({ ratesToUsd, fetchedAt: 1000 }) }, + { id: 'fallback', fetch: () => { fallbackCalls += 1; throw new Error('unused'); } }, + ]); + const result = fxProviders.fetchWithFallback({ now: 1000 }); + assert.equal(result.providerId, 'primary'); + assert.deepEqual(result.ratesToUsd, ratesToUsd); + assert.notStrictEqual(result.ratesToUsd, ratesToUsd); + assert.equal(fallbackCalls, 0); +}); From 4efd7b7304484206640c04f1f0fc31d9c9269ab9 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 09:01:59 -0400 Subject: [PATCH 15/16] perf(fx): skip snapshot copies for configured currency validation Check configured currencies before materializing a cached rate snapshot. Preserve normalized input, provider-added currencies and all pricing/freshness paths. One focused regression passes with the documented offline full-source component loader. Original lookup makes 18 unnecessary cache peeks; candidate makes zero for configured and invalid inputs. Seven alternating paired microbenchmark samples measured warm configured lookup medians of 95.431 to 26.843 ms per 500,000 calls, matching outputs. These are lookup-only component results, not full-app, hosted CI or provider measurements. Preserve the concurrent provider-ratio repair f2c4741 in full. The report's parent 8f3bf8b8 identifies the benchmark baseline, not this integration parent; all four measured module preimages are unchanged by f2c4741. Exactly three leaf changes: rateService, one maintained regression and its source-pinned measurement/reproducer report. No dependency, workflow, provider or settlement changes. --- .../currency-support-lookup-20261004.md | 194 ++++++++++++++++++ src/services/rateService.js | 9 +- test/rateSupportLookup.test.js | 39 ++++ 3 files changed, 238 insertions(+), 4 deletions(-) create mode 100644 docs/validation/currency-support-lookup-20261004.md create mode 100644 test/rateSupportLookup.test.js diff --git a/docs/validation/currency-support-lookup-20261004.md b/docs/validation/currency-support-lookup-20261004.md new file mode 100644 index 0000000..9a51d2d --- /dev/null +++ b/docs/validation/currency-support-lookup-20261004.md @@ -0,0 +1,194 @@ +# Currency-support lookup — 4 October 2026 + +This continuation of RemitFlow-Backend PR 141 changes only currency-membership +lookup. Configured currencies return before `fxCacheService.peek()` materializes +an entire decorated rate snapshot. Nonconfigured codes still inspect the current +cache, preserving provider-added currencies. Currency normalization, cache +freshness, quote pricing, provider selection and HTTP contracts are unchanged. + +## Source and execution boundary + +Parent: `8f3bf8b8e16ac01cc9ef9cce9d66772e03fc59ba`. + +| Complete module | Git blob SHA | +| --- | --- | +| Original rateService.js | fdd533cc12d82ede4ace0776b83d25786214ea6c | +| Candidate rateService.js | e48be1aba814ba1443d64554b5d22ad8e1591d2d | +| Unchanged fxCacheService.js | 91201bc01251b888ad86678038c68b4f74a66013 | +| Unchanged config/rates.js | 52f235a875affcbf31a15ac19108d2a12c0e6bad | +| Unchanged utils/currency.js | a1afbf1eaceeca0ff9922ab3d39038cda981016b | +| New rateSupportLookup.test.js | 746380e00ae682f5123d440a631cc9b51b2c88df | + +The complete modules were copied through native GitHub reads and their Git blob +hashes verified. Node v22.16.0, Linux/x64, AMD EPYC 9V74. An offline CommonJS +loader executed the actual source. It supplied FX configuration defaults +(30,000 ms TTL; 60,000 ms stale grace); unused money, ApiError and provider +collaborators throw if accessed. The real cache's seed, peek, decorate, reset and +provider-count paths ran. No dependency installation, live provider, HTTP app, +full project test suite or hosted CI was run in this continuation. + +## Focused regression + +One new maintained Node test checks normalized configured currencies with cold +and expired caches, invalid input, a provider-added CAD rate, unknown/prototype +names, unchanged cache contents and zero provider fetches. The original code +fails the optimization assertion: 18 cache peeks instead of zero. The candidate +passes: 1 test, 1 pass, 0 fail, 0 skipped. The same complete test file ran through +the loader below; this is component evidence, not an installed-project run. + +Installed-checkout command (provided for reproduction, not claimed executed): +`node --test test/rateSupportLookup.test.js`. + +## Measured lookup workload + +Each sample performed 500,000 calls, including normalization. Three warmup pairs +preceded seven alternating before/after pairs. All input-level membership results +and per-sample hit counts matched. Warm fixtures used the nine configured rates +plus CAD; mixed input also included CAD, an unknown code, empty text and null. +The figures are medians, not a claim about request latency, fleet throughput, +provider quota, live settlement or peak memory. + +| Scenario | Before ms | After ms | Less time | Ratio | +| --- | ---: | ---: | ---: | ---: | +| cold-configured | 43.505870 | 25.565094 | 41.24% | 1.702x | +| warm-configured | 95.430961 | 26.843440 | 71.87% | 3.555x | +| warm-mixed | 76.241695 | 33.418068 | 56.17% | 2.281x | + +### Raw paired samples (milliseconds) + +| Scenario | Pair | Before | After | Hits (both) | +| --- | ---: | ---: | ---: | ---: | +| cold-configured | 0 | 43.305808000 | 26.070304000 | 500000 | +| cold-configured | 1 | 43.598279000 | 25.642260000 | 500000 | +| cold-configured | 2 | 43.464998000 | 25.641859000 | 500000 | +| cold-configured | 3 | 43.505870000 | 24.358448000 | 500000 | +| cold-configured | 4 | 44.560766000 | 24.981597000 | 500000 | +| cold-configured | 5 | 44.138401000 | 24.929238000 | 500000 | +| cold-configured | 6 | 42.766747000 | 25.565094000 | 500000 | +| warm-configured | 0 | 97.255455000 | 28.125888000 | 500000 | +| warm-configured | 1 | 98.593778000 | 27.937825000 | 500000 | +| warm-configured | 2 | 96.640700000 | 26.224358000 | 500000 | +| warm-configured | 3 | 94.896015000 | 26.637379000 | 500000 | +| warm-configured | 4 | 93.976282000 | 26.276546000 | 500000 | +| warm-configured | 5 | 95.430961000 | 27.398524000 | 500000 | +| warm-configured | 6 | 93.858485000 | 26.843440000 | 500000 | +| warm-mixed | 0 | 82.886692000 | 34.384162000 | 384617 | +| warm-mixed | 1 | 82.598257000 | 34.149749000 | 384617 | +| warm-mixed | 2 | 76.686774000 | 35.834523000 | 384617 | +| warm-mixed | 3 | 72.199315000 | 32.852197000 | 384617 | +| warm-mixed | 4 | 74.809962000 | 30.814562000 | 384617 | +| warm-mixed | 5 | 75.485167000 | 32.893410000 | 384617 | +| warm-mixed | 6 | 76.241695000 | 33.418068000 | 384617 | + +## Exact offline reproducer + +In a disposable checkout of this change, create `work/rateService.before.js` with +`git show 8f3bf8b8e16ac01cc9ef9cce9d66772e03fc59ba:src/services/rateService.js`. +Save the following three blocks to the named files. This reproduces the loader +boundary without installing dependencies or starting the application. + +### work/lookup-harness.cjs + +```js +'use strict'; +// Source-bound offline component loader. Does not load the HTTP application. +// All four source modules below are complete native GitHub blob copies. +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); +const root = path.resolve(__dirname, '..'); +function load(file, imports) { + const module = { exports: {} }; + const filename = path.join(root, file); + const run = vm.runInThisContext(`(function(require,module,exports){\n${fs.readFileSync(filename, 'utf8')}\n})`, { filename }); + run((name) => { + if (Object.hasOwn(imports, name)) return imports[name]; + if (name.startsWith('node:')) return require(name); + throw new Error(`Unexpected import: ${name}`); + }, module, module.exports); + return module.exports; +} +const rates = load('src/config/rates.js', {}); +const currency = load('src/utils/currency.js', {}); +const unused = new Proxy({}, { get() { throw new Error('Unused collaborator invoked'); } }); +const cache = load('src/services/fxCacheService.js', { + '../config': { fx: { cacheTtlMs: 30000, staleGraceMs: 60000 } }, + './fxProviders': unused, + '../utils/ApiError': unused, + '../config/rates': rates, +}); +const imports = { + '../config/rates': rates, '../utils/currency': currency, + '../utils/money': unused, '../utils/ApiError': unused, './fxCacheService': cache, +}; +module.exports = { root, rates, cache, load, imports }; +``` + +### work/check.cjs + +```js +'use strict'; +const h = require('./lookup-harness.cjs'); +const candidate = h.load(process.env.LOOKUP_BEFORE ? 'work/rateService.before.js' : 'src/services/rateService.js', h.imports); +h.load('test/rateSupportLookup.test.js', { + '../src/services/rateService': candidate, + '../src/services/fxCacheService': h.cache, + '../src/config/rates': h.rates, +}); +``` + +### work/bench.cjs + +```js +'use strict'; +const assert = require('node:assert/strict'); +const os = require('node:os'); +const { performance } = require('node:perf_hooks'); +const h = require('./lookup-harness.cjs'); +const before = h.load('work/rateService.before.js', h.imports).isSupported; +const after = h.load('src/services/rateService.js', h.imports).isSupported; +const codes = h.rates.SUPPORTED_CURRENCIES.map(code => ` ${code.toLowerCase()} `); +const iterations = 500000; +const raw = []; +function sample(fn, inputs) { + let hits = 0; + const start = performance.now(); + for (let i = 0; i < iterations; i++) hits += fn(inputs[i % inputs.length]) ? 1 : 0; + return { ms: performance.now() - start, hits }; +} +function median(a) { return a.slice().sort((x, y) => x - y)[Math.floor(a.length / 2)]; } +for (const scenario of ['cold-configured', 'warm-configured', 'warm-mixed']) { + h.cache.reset(); + if (scenario !== 'cold-configured') h.cache.seed({ ratesToUsd: { ...h.rates.RATES_TO_USD, CAD: 0.73 } }); + const inputs = scenario === 'warm-mixed' ? [...codes, 'cad', 'ZZZ', '', null] : codes; + assert.deepEqual(inputs.map(before), inputs.map(after)); + for (let i = 0; i < 3; i++) { sample(before, inputs); sample(after, inputs); } + const b = [], a = []; + for (let i = 0; i < 7; i++) { + const result = {}; + for (const [name, fn] of (i % 2 ? [['after', after], ['before', before]] : [['before', before], ['after', after]])) { + result[name] = sample(fn, inputs); + } + assert.equal(result.before.hits, result.after.hits); + b.push(result.before.ms); a.push(result.after.ms); + raw.push({ scenario, pair: i, ...result }); + } + const summary = { scenario, iterations, beforeMedianMs: median(b), afterMedianMs: median(a), reductionPercent: (1 - median(a)/median(b))*100, ratio: median(b)/median(a) }; + console.error(JSON.stringify(summary)); +} +console.log(JSON.stringify({ runtime: process.version, platform: `${process.platform}/${process.arch}`, cpu: os.cpus()[0].model, iterations, warmupPairs: 3, alternatingPairs: 7, providerFetches: h.cache.getProviderFetchCount(), raw }, null, 2)); +``` + +### Executed commands + +```sh +LOOKUP_BEFORE=1 node --test work/check.cjs > work/before.tap +# exit 1: expected 18-versus-0 cache-peek assertion +node --test work/check.cjs > work/after.tap +# exit 0: one test passes +node work/bench.cjs > work/raw.json 2> work/summary.jsonl +# exit 0: all paired results agree, providerFetches = 0 +``` + +The original contribution, attribution history and publisher custody remain. +No new claim, award, upstream merge or payment is established by this result. diff --git a/src/services/rateService.js b/src/services/rateService.js index fdd533c..e48be1a 100644 --- a/src/services/rateService.js +++ b/src/services/rateService.js @@ -53,20 +53,21 @@ function listRates(opts = {}) { } /** - * Check whether a currency code is supported against the current snapshot. - * Falls back to the configured currency list when providers are unreachable - * so validation can still reject unknown codes without requiring a live pull. + * Check configured currencies before inspecting any provider-added codes. + * Known currencies need no decorated snapshot or rate-table copy. Validation + * remains available during provider outages and never initiates a live pull. * @param {string} code * @returns {boolean} */ function isSupported(code) { const normalized = currency.normalize(code); if (!normalized) return false; + if (SUPPORTED_CURRENCIES.includes(normalized)) return true; const peeked = fxCacheService.peek(); if (peeked && Object.prototype.hasOwnProperty.call(peeked.ratesToUsd, normalized)) { return true; } - return SUPPORTED_CURRENCIES.includes(normalized); + return false; } /** diff --git a/test/rateSupportLookup.test.js b/test/rateSupportLookup.test.js new file mode 100644 index 0000000..746380e --- /dev/null +++ b/test/rateSupportLookup.test.js @@ -0,0 +1,39 @@ +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const rateService = require('../src/services/rateService'); +const fxCacheService = require('../src/services/fxCacheService'); +const { RATES_TO_USD, SUPPORTED_CURRENCIES } = require('../src/config/rates'); + +test('currency support avoids cache copies for configured codes and retains extensions', () => { + fxCacheService.reset(); + const originalPeek = fxCacheService.peek; + let peeks = 0; + fxCacheService.peek = (...args) => { + peeks += 1; + return originalPeek(...args); + }; + try { + // Both cold and expired caches must preserve configured currency support. + for (const seeded of [false, true]) { + if (seeded) fxCacheService.seed({ ratesToUsd: { ...RATES_TO_USD, CAD: 0.73 }, fetchedAt: 0 }); + for (const code of SUPPORTED_CURRENCIES) { + assert.equal(rateService.isSupported(` ${code.toLowerCase()} `), true); + } + for (const invalid of ['', ' ', null, undefined, 42, {}]) { + assert.equal(rateService.isSupported(invalid), false); + } + } + assert.equal(peeks, 0, 'configured and invalid codes need no cache materialization'); + assert.equal(rateService.isSupported(' cad '), true); + assert.equal(rateService.isSupported('ZZZ'), false); + assert.equal(rateService.isSupported('__proto__'), false); + assert.equal(peeks, 3, 'only nonconfigured nonempty codes inspect the cache'); + assert.deepEqual(originalPeek().ratesToUsd, { ...RATES_TO_USD, CAD: 0.73 }); + assert.equal(fxCacheService.getProviderFetchCount(), 0); + } finally { + fxCacheService.peek = originalPeek; + fxCacheService.reset(); + } +}); From 549c72c8fd5956ff4d143203fabfc229a15707ff Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 09:09:54 -0400 Subject: [PATCH 16/16] fix(fx): reject unsafe converted quote amounts before persistence Validate the unrounded FX-converted receive amount with the existing safe-money bound before rounding, issuing a quote identity or storing it. Preserve ordinary quote terms, binding, and stale/TTL policy. Retain both concurrent provider-ratio and configured-currency lookup changes from parent 4efd7b73. Exactly three leaf changes: quoteService, three focused cases in the existing precision test, and a source-pinned execution note. Actual Node 22.16 offline component run on current dependency blobs: original quote service 2 passed / 3 failed; candidate 5 passed / 0 failed / 0 skipped. Complete production quote/rate/cache/provider/money/config/store modules were loaded; only dotenv/UUID imports and unused audit/index initialization used the documented adapter. No HTTP transfer, live provider, dependency-complete suite or hosted CI claim. Source blob 4438da7b; test blob cfe95650. --- .../quote-receive-range-20261004.md | 99 +++++++++++++++++++ src/services/quoteService.js | 8 +- test/exchangeRatePrecision.test.js | 38 +++++++ 3 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 docs/validation/quote-receive-range-20261004.md diff --git a/docs/validation/quote-receive-range-20261004.md b/docs/validation/quote-receive-range-20261004.md new file mode 100644 index 0000000..4edacc8 --- /dev/null +++ b/docs/validation/quote-receive-range-20261004.md @@ -0,0 +1,99 @@ +# Quote receive-amount range + +## Repair + +`getQuote` already rejects source amounts outside the money helper's supported +cent-arithmetic range. FX multiplication can exceed that range even when the +source amount and conversion ratio are individually finite. Reject the converted +amount with the same helper before rounding, assigning a quote identity, or +inserting it into `store.quotes`. Transfer binding without a supplied quote ID +uses the same guarded path. Valid amounts, fees, rates, provenance and stale/TTL +policy remain unchanged. + +This complements the provider cross-rate check: a representable rate does not +imply that multiplying it by a valid amount produces a representable amount. + +## Focused execution + +Integration parent: `4efd7b7304484206640c04f1f0fc31d9c9269ab9`. +The concurrent provider-ratio and configured-currency lookup repairs are retained. +All 12 loaded source/test preimages were matched against their Git blob hashes; +the two changed dependency modules were refreshed to the integration parent. + +Node v22.16.0, Linux, default fee/FX configuration, one existing test file: + +| Source | Passed | Failed | Skipped | Exit | +| --- | ---: | ---: | ---: | ---: | +| Original quote service, current dependencies | 2 | 3 | 0 | 1 | +| Guarded quote service, same dependencies | 5 | 0 | 0 | 0 | + +The three added cases cover quote creation and implicit transfer binding for an +unsafe converted amount, plus multiplication overflow with a finite FX ratio. +The two existing precision/provenance cases are unchanged. The rejection cases +check that no quote is stored; ordinary subsequent quotes retain version 1, +expected receive amounts and successful binding. + +The large-input service case uses the configured USD/NGN rates. It exceeds the +application's default maximum transfer amount, so it is not evidence that the +public transfer endpoint admits that request. The separate overflow case uses +amount 100 and a controlled finite-rate snapshot (`NGN: 1e-307`). These are +service-level numeric boundary checks, not a live financial/provider scenario. + +Source blobs: + +- Original `quoteService.js`: `cae92cf78909b809777ff0e6cc9028dc003978cc`. +- Guarded `quoteService.js`: `4438da7b30a4b0486fa2afe2292b252a870d2338`. +- Executed `exchangeRatePrecision.test.js`: `cfe95650050ed04dec603c8abbbaaca42bd67ba2`. +- Current `rateService.js`: `e48be1aba814ba1443d64554b5d22ad8e1591d2d`. +- Current `fxProviders.js`: `feaff602fe217dfeeaf705dbbbd3669cbfec97dd`. +- `fxCacheService.js`: `91201bc01251b888ad86678038c68b4f74a66013`. + +## Reproduction and limits + +With the repository's normal dependencies installed, the maintained selection is: + +```sh +node --test test/exchangeRatePrecision.test.js +``` + +That dependency-complete command was not run here. The actual offline command +loaded the complete, hash-matched production quote, rate, cache, provider, money, +currency, error, ID, configuration and store modules with the following temporary +import adapter (not a product dependency or workflow): + +```js +'use strict'; +const Module = require('node:module'); +const crypto = require('node:crypto'); +const path = require('node:path'); +const original = Module._load; +// Place this temporary adapter at evidence/offline-imports.cjs. +const storePath = path.resolve(__dirname, '../src/store/index.js'); +const idsPath = path.resolve(__dirname, '../src/utils/ids.js'); +const configPath = path.resolve(__dirname, '../src/config/index.js'); +const untouched = () => { throw new Error('Unused collaborator was unexpectedly invoked'); }; +for (const key of ['TRANSFER_FEE_PERCENT', 'TRANSFER_FEE_FLAT', 'FX_CACHE_TTL_MS', + 'FX_STALE_GRACE_MS', 'FX_ALLOW_STALE_TRANSFERS', 'FX_QUOTE_TTL_MS', 'API_TOKENS']) { + delete process.env[key]; +} +Module._load = function(request, parent, isMain) { + if (request === 'dotenv' && parent?.filename === configPath) return { config: () => ({}) }; + if (request === 'uuid' && parent?.filename === idsPath) return { v4: crypto.randomUUID }; + if (parent?.filename === storePath && request === '../services/auditService') return { reset: untouched }; + if (parent?.filename === storePath && request === '../utils/orderedIndex') { + return { OrderedIndex: class { reset() { untouched(); } } }; + } + return original.apply(this, arguments); +}; +``` + +```sh +node --require ./evidence/offline-imports.cjs --test test/exchangeRatePrecision.test.js +``` + +The adapter replaces `.env` loading, the UUID package call, and unused audit/index +initialization only; it does not replace quote arithmetic, cache admission, +freshness classification or quote persistence. The same adapter and maintained +test bytes ran before and after the fix. No provider HTTP calls, transfer HTTP +handler, full application startup, dependency-complete suite or hosted CI result +is asserted. No unrelated test was removed, weakened or skipped. diff --git a/src/services/quoteService.js b/src/services/quoteService.js index cae92cf..4438da7 100644 --- a/src/services/quoteService.js +++ b/src/services/quoteService.js @@ -158,7 +158,13 @@ function getQuote(amount, from, to, opts = {}) { } const rate = snapshot.ratesToUsd[fromCode] / snapshot.ratesToUsd[toCode]; - const receiveAmount = money.round(amountAfterFee * rate); + const convertedAmount = amountAfterFee * rate; + // A safe source amount can exceed safe cent arithmetic after FX conversion. + // Reject before rounding, assigning an identity, or storing the quote. + if (!money.isSafeAmount(convertedAmount)) { + throw ApiError.badRequest('receive amount is outside the supported numeric range'); + } + const receiveAmount = money.round(convertedAmount); quoteVersionSeq += 1; const quote = { diff --git a/test/exchangeRatePrecision.test.js b/test/exchangeRatePrecision.test.js index b905bba..cfe9565 100644 --- a/test/exchangeRatePrecision.test.js +++ b/test/exchangeRatePrecision.test.js @@ -47,3 +47,41 @@ test('a quote preserves the FX ratio while fee and receive amounts stay rounded' assert.equal(quote.freshness.fetchedAt, new Date(NOW).toISOString()); assert.equal(fxCacheService.getProviderFetchCount(), 0); }); + +for (const [name, create] of [ + ['quote creation', (amount) => quoteService.getQuote(amount, 'USD', 'NGN', { now: NOW })], + ['transfer binding', (amount) => quoteService.resolveForTransfer( + { amount, from: 'USD', to: 'NGN' }, { now: NOW } + )], +]) { + test(`${name} rejects an unsafe converted amount before storing a quote`, () => { + // The source amount is safe in cents; the configured USD/NGN conversion is not. + assert.throws(() => create(100_000_000_000), { + name: 'ApiError', + statusCode: 400, + message: 'receive amount is outside the supported numeric range', + }); + assert.equal(store.quotes.size, 0); + // Rejection must not consume an identity or affect ordinary amounts. + const valid = create(100); + assert.equal(valid.quoteVersion, 1); + assert.equal(valid.receiveAmount, 151076.92); + assert.equal(store.quotes.size, 1); + assert.equal(fxCacheService.getProviderFetchCount(), 0); + }); +} + +test('a finite FX ratio cannot overflow the stored receive amount', () => { + fxCacheService.seed({ + ratesToUsd: { ...RATES_TO_USD, NGN: 1e-307 }, + fetchedAt: NOW, + providerId: 'precision-fixture', + }); + assert.throws(() => quoteService.getQuote(100, 'USD', 'NGN', { now: NOW }), { + name: 'ApiError', + statusCode: 400, + message: 'receive amount is outside the supported numeric range', + }); + assert.equal(store.quotes.size, 0); + assert.equal(fxCacheService.getProviderFetchCount(), 0); +});