diff --git a/backend/docs/EXCHANGE_RATE_ORACLE_CACHE.md b/backend/docs/EXCHANGE_RATE_ORACLE_CACHE.md index 5b29cd33..3ad66294 100644 --- a/backend/docs/EXCHANGE_RATE_ORACLE_CACHE.md +++ b/backend/docs/EXCHANGE_RATE_ORACLE_CACHE.md @@ -254,3 +254,11 @@ npx vitest run src/lib/exchange-rate-oracle-telemetry.test.js \ tests/integration/exchange-rate-cache.test.js npm run test:load -- exchange-rate-cache-stress ``` + +## Payload sanitization and strict validation (#1442) + +`src/lib/exchange-rate-oracle-sanitizer.js` guards every trust boundary of the cache: + +- `sanitizeQuoteRequest()` validates and canonicalises request parameters before they form a cache key or reach Horizon: asset codes (1-12 alphanumeric, upper-cased, XLM is native), issuers and source accounts (full StrKey checksum), amounts (positive, at most 7 decimals, within int64 stroops) and slippage (0 to 0.5). Failures are `ExchangeRateValidationError` (400, `EXCHANGE_RATE_INVALID_PARAMS`, `retryable: false`); the quote route returns `{ error, field }`. +- `sanitizeQuote()` rebuilds every quote from an allow-list of fields. It runs on Horizon results before caching and on entries read from the shared Redis store. Unknown fields and prototype keys are dropped; a malformed entry is a cache miss, and a malformed Horizon result is a non-retryable 502. +- Invalidation uses the same canonical key and never throws on malformed input. diff --git a/backend/docs/HORIZON_CLIENT_RETRY_MANAGER.md b/backend/docs/HORIZON_CLIENT_RETRY_MANAGER.md new file mode 100644 index 00000000..c4ba0ef4 --- /dev/null +++ b/backend/docs/HORIZON_CLIENT_RETRY_MANAGER.md @@ -0,0 +1,54 @@ +# Horizon Client Retry Manager + +Covers issues **#1439** (exponential backoff), **#1440** (concurrency control +and distributed locking) and **#1441** (integration and stress tests). + +| File | Role | +|---|---| +| `src/lib/stellar/horizon-client/backoff.js` | Backoff policy, jitter, Retry-After parsing (#1439) | +| `src/lib/stellar/horizon-client/concurrency-limiter.js` | Per-process semaphore with a bounded queue (#1440) | +| `src/lib/stellar/horizon-client/retry-coordinator.js` | Cluster-wide retry slots and rate-limit cooldown over Redis (#1440) | +| `src/lib/stellar/horizon-client/retry-manager.js` | Runs operations with the above | +| `tests/integration/horizon-retry-manager.test.js` | Real SDK + HTTP Horizon stub (#1441) | +| `load-tests/horizon-retry-manager-stress.test.js` | Stress invariants (#1441) | + +## Retry policy (#1439) + +- Default: jittered exponential backoff, `delay(n) = random(0, min(maxDelayMs, baseDelayMs * multiplier^n))`. +- An explicit `retryDelays` array keeps the legacy fixed schedule. +- Retry-After is honoured; a hint above `maxRetryAfterMs` fails fast. +- `maxElapsedMs` bounds the whole operation, waits included. +- Only transient failures retry (408, 429, 5xx, network errors). Errors with `retryable: false` never retry. +- SDK transport errors without a `code` (for example `connect ECONNREFUSED`) are now recognised as retryable. + +Every failure carries `attempts` and `retryStopReason` +(`not_retryable`, `exhausted`, `budget_exceeded`, `retry_after_too_long`, `retry_slots_exhausted`). + +## Concurrency control (#1440) + +- **Limiter**: at most `maxConcurrent` Horizon calls per process, `maxQueue` waiting; overflow and queue timeouts return 503 (`HORIZON_CLIENT_QUEUE_FULL`, `HORIZON_CLIENT_QUEUE_TIMEOUT`). +- **Retry slots**: `horizon:retry:slot:::`, acquired with `SET NX PX`, released by compare-and-delete. With no free slot the retry is skipped, so N instances cannot turn an outage into a retry storm. +- **Cooldown**: a 429/503 publishes `horizon:retry:cooldown:` (capped by `maxCooldownMs`); every instance waits it out before its next attempt. +- Fails open to an in-process implementation when Redis is missing or erroring. Enabled at startup by `configureHorizonRetryCoordination()` in `app.js`. +- Keys never contain caller input: the host is hashed and operation names are restricted to `[A-Za-z0-9_]`. + +## Configuration + +| Variable | Default | +|---|---| +| `HORIZON_RETRY_STRATEGY` | `exponential` | +| `HORIZON_RETRY_MAX_RETRIES` | `2` | +| `HORIZON_RETRY_BASE_DELAY_MS` | `150` | +| `HORIZON_RETRY_MAX_DELAY_MS` | `2000` | +| `HORIZON_RETRY_MULTIPLIER` | `3` | +| `HORIZON_RETRY_JITTER` | `full` (`full`, `equal`, `none`) | +| `HORIZON_RETRY_MAX_ELAPSED_MS` | `10000` | +| `HORIZON_RETRY_MAX_RETRY_AFTER_MS` | `5000` | +| `HORIZON_CLIENT_MAX_CONCURRENCY` | `64` | +| `HORIZON_CLIENT_MAX_QUEUE` | `1000` | +| `HORIZON_CLIENT_QUEUE_TIMEOUT_MS` | `10000` | +| `HORIZON_RETRY_MAX_CONCURRENT_RETRIES` | `32` | +| `HORIZON_RETRY_SLOT_LEASE_MS` | `30000` | +| `HORIZON_RETRY_MAX_COOLDOWN_MS` | `5000` | + +All values are clamped to safe ranges; invalid values fall back to defaults. diff --git a/backend/load-tests/horizon-retry-manager-stress.test.js b/backend/load-tests/horizon-retry-manager-stress.test.js new file mode 100644 index 00000000..caac2897 --- /dev/null +++ b/backend/load-tests/horizon-retry-manager-stress.test.js @@ -0,0 +1,228 @@ +/** + * Horizon Client Retry Manager — stress suite (issue #1441). + * + * Pushes the Retry Manager well past normal load to check the invariants that + * matter during a Horizon incident: + * - per-process concurrency never exceeds the limiter cap + * - cluster-wide concurrent retries never exceed the retry-slot cap + * - a shared 429 cooldown is respected by every instance + * - overload is shed deterministically (503) instead of queueing forever + * - no slot, counter or queue entry leaks after thousands of operations + * - Redis failures during load never fail requests (fail-open) + * + * Horizon is simulated by in-process operations and Redis by the shared + * in-memory fake, so the suite is deterministic and fast. + */ +import { describe, it, expect, vi } from 'vitest'; + +vi.mock('../src/lib/logger.js', () => ({ + logger: { warn: vi.fn(), info: vi.fn(), error: vi.fn(), debug: vi.fn() }, +})); + +import { RetryManager, RETRY_STOP_REASONS } from '../src/lib/stellar/horizon-client/retry-manager.js'; +import { HorizonRetryCoordinator } from '../src/lib/stellar/horizon-client/retry-coordinator.js'; +import { RETRY_MANAGER_ERROR_CODES } from '../src/lib/stellar/horizon-client/constants.js'; +import { createFakeRedis } from '../tests/helpers/fake-redis.js'; + +const URL = 'https://horizon.stellar.org'; +const tick = () => new Promise((resolve) => setImmediate(resolve)); +/** Like Promise.allSettled for one promise, attached immediately so no rejection is ever unhandled. */ +const settle = (promise) => promise.then( + (value) => ({ status: 'fulfilled', value }), + (reason) => ({ status: 'rejected', reason }), +); +const httpError = (status) => Object.assign(new Error(`HTTP ${status}`), { response: { status } }); + +/** Seeded PRNG so failures are reproducible. */ +function mulberry32(seed) { + let a = seed; + return () => { + a |= 0; + a = (a + 0x6d2b79f5) | 0; + let t = Math.imul(a ^ (a >>> 15), 1 | a); + t = (t + Math.imul(t ^ (t >>> 7), 61 | t)) ^ t; + return ((t ^ (t >>> 14)) >>> 0) / 4294967296; + }; +} + +function makeManager({ redis = null, concurrency, coordination, backoff, sleep = tick } = {}) { + const coordinator = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: URL, ...(coordination ?? {}) }); + return new RetryManager(undefined, { + horizonUrl: URL, + backoff: { baseDelayMs: 1, maxDelayMs: 4, maxRetries: 3, maxElapsedMs: 60_000, ...(backoff ?? {}) }, + concurrency: { maxConcurrent: 32, maxQueue: 100_000, queueTimeoutMs: 60_000, ...(concurrency ?? {}) }, + coordinator, + sleep, + }); +} + +function expectNoLeaks(manager) { + const config = manager.getConfig(); + expect(config.currentConcurrentOperations).toBe(0); + expect(config.concurrency).toMatchObject({ active: 0, pending: 0 }); + expect(manager.coordinator.localSlots.size).toBe(0); +} + +describe('Horizon Client Retry Manager — stress (issue #1441)', () => { + it('2,000 concurrent operations with 30% transient failures stay within the concurrency cap', async () => { + const random = mulberry32(42); + const manager = makeManager({ concurrency: { maxConcurrent: 16 } }); + let running = 0; + let peak = 0; + + const op = async () => { + running += 1; + peak = Math.max(peak, running); + await tick(); + running -= 1; + if (random() < 0.3) throw httpError(503); + return 'ok'; + }; + + const settled = await Promise.allSettled( + Array.from({ length: 2_000 }, () => manager.executeWithRetry(op, 'loadAccount', {}, URL)), + ); + + const ok = settled.filter((s) => s.status === 'fulfilled').length; + const failed = settled.filter((s) => s.status === 'rejected'); + expect(peak).toBeLessThanOrEqual(16); + // P(4 consecutive failures) = 0.3^4 ≈ 0.8%; allow generous slack. + expect(ok).toBeGreaterThan(1_900); + for (const f of failed) { + expect(Object.values(RETRY_STOP_REASONS)).toContain(f.reason.retryStopReason); + } + expectNoLeaks(manager); + }); + + it('8 instances x 100 calls against a dead Horizon never exceed the cluster retry cap', async () => { + const redis = createFakeRedis(); + const SLOTS = 4; + const instances = Array.from({ length: 8 }, () => + makeManager({ redis, coordination: { maxConcurrentRetries: SLOTS } }), + ); + + let inFlightRetries = 0; + let peakRetries = 0; + let attempts = 0; + + const call = (manager) => { + let attempt = 0; + return manager.executeWithRetry(async () => { + attempts += 1; + const isRetry = attempt > 0; + attempt += 1; + if (isRetry) { + inFlightRetries += 1; + peakRetries = Math.max(peakRetries, inFlightRetries); + } + await tick(); + if (isRetry) inFlightRetries -= 1; + throw httpError(500); + }, 'getFeeStats', {}, URL); + }; + + const settled = await Promise.allSettled(instances.flatMap((m) => Array.from({ length: 100 }, () => call(m)))); + + expect(settled.every((s) => s.status === 'rejected')).toBe(true); + expect(peakRetries).toBeLessThanOrEqual(SLOTS); + // Without coordination this would be 800 * 4 = 3,200 attempts. + expect(attempts).toBeLessThan(800 * 2); + const shed = settled.filter((s) => s.reason.retryStopReason === RETRY_STOP_REASONS.RETRY_SLOTS_EXHAUSTED); + expect(shed.length).toBeGreaterThan(0); + expect(redis.keys('horizon:retry:slot:')).toHaveLength(0); + instances.forEach(expectNoLeaks); + }); + + it('a single 429 holds every instance until the shared cooldown expires', async () => { + const redis = createFakeRedis(); + const realSleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + const instances = Array.from({ length: 5 }, () => + makeManager({ + redis, + sleep: realSleep, + backoff: { baseDelayMs: 150, maxDelayMs: 150, jitter: 'none', maxRetries: 1 }, + }), + ); + + let throttledAt = null; + const calls = []; + const horizon = async () => { + calls.push(Date.now()); + if (throttledAt === null) { + throttledAt = Date.now(); + throw httpError(429); + } + return 'ok'; + }; + + const first = instances[0].executeWithRetry(horizon, 'loadAccount', {}, URL); + // Fire the rest as soon as the throttle has been published. + await vi.waitFor(() => expect(redis.keys('horizon:retry:cooldown:')).toHaveLength(1)); + const results = await Promise.all( + instances.slice(1).flatMap((m) => Array.from({ length: 20 }, () => m.executeWithRetry(horizon, 'loadAccount', {}, URL))), + ); + + expect(await first).toBe('ok'); + expect(results.every((r) => r === 'ok')).toBe(true); + expect(calls).toHaveLength(1 + 1 + 80); + const tooEarly = calls.slice(1).filter((at) => at - throttledAt < 140); + expect(tooEarly).toHaveLength(0); + }); + + it('sheds exactly the overflow when the queue saturates', async () => { + const manager = makeManager({ concurrency: { maxConcurrent: 4, maxQueue: 50 } }); + const gates = []; + const op = () => new Promise((resolve) => gates.push(resolve)); + + const pending = Array.from({ length: 200 }, () => settle(manager.executeWithRetry(op, 'fetchPayments', {}, URL))); + await vi.waitFor(() => expect(gates).toHaveLength(4)); + + // Drain: release whatever is running until everything settles. + const drain = setInterval(() => gates.splice(0).forEach((resolve) => resolve('ok')), 1); + const settled = await Promise.all(pending); + clearInterval(drain); + + const rejected = settled.filter((s) => s.status === 'rejected'); + expect(rejected).toHaveLength(200 - 4 - 50); + expect(rejected.every((r) => r.reason.code === RETRY_MANAGER_ERROR_CODES.QUEUE_FULL)).toBe(true); + expectNoLeaks(manager); + }); + + it('survives Redis flapping under load without failing healthy requests', async () => { + const redis = createFakeRedis(); + // Enough retry slots that shedding is not what this test measures. + const manager = makeManager({ redis, coordination: { maxConcurrentRetries: 1_024 } }); + const random = mulberry32(7); + const flap = setInterval(() => redis.setFailure(random() < 0.5 ? new Error('ECONNRESET') : null), 1); + + const op = vi.fn() + .mockImplementation(async () => { + await tick(); + if (random() < 0.2) throw httpError(502); + return 'ok'; + }); + + const settled = await Promise.allSettled( + Array.from({ length: 1_000 }, () => manager.executeWithRetry(op, 'loadAccount', {}, URL)), + ); + clearInterval(flap); + redis.setFailure(null); + + const ok = settled.filter((s) => s.status === 'fulfilled').length; + expect(ok).toBeGreaterThan(980); + expect(manager.getConfig().coordination.stats.fallbacks).toBeGreaterThan(0); + expectNoLeaks(manager); + }); + + it('sustains high throughput on the happy path', async () => { + const manager = makeManager({ concurrency: { maxConcurrent: 64 } }); + const started = Date.now(); + const results = await Promise.all( + Array.from({ length: 10_000 }, () => manager.executeWithRetry(async () => 'ok', 'getFeeStats', {}, URL)), + ); + const elapsed = Date.now() - started; + expect(results).toHaveLength(10_000); + expect(elapsed).toBeLessThan(10_000); + expectNoLeaks(manager); + }); +}); diff --git a/backend/src/app.js b/backend/src/app.js index b330d308..58fa0022 100644 --- a/backend/src/app.js +++ b/backend/src/app.js @@ -50,6 +50,7 @@ import oracleRouter from "./routes/oracle.js"; import { getPaymentSessionValidatorHealth } from "./lib/payment-session-validator.js"; import { getExchangeRateOracleHealth } from "./lib/exchange-rate-oracle-telemetry.js"; import { configureExchangeRateCoordination } from "./services/exchangeRateService.js"; +import { configureHorizonRetryCoordination } from "./lib/stellar/horizon-client/retry-coordinator.js"; export async function createApp({ redisClient }) { const app = express(); @@ -59,6 +60,10 @@ export async function createApp({ redisClient }) { // Redis the cache still coalesces concurrent misses within this process. configureExchangeRateCoordination({ redisClient: redisAvailable ? redisClient : null }); + // Cluster-wide Horizon retry slots and rate-limit cooldown (issue #1440). + // Without Redis each process still limits its own retries. + configureHorizonRetryCoordination({ redisClient: redisAvailable ? redisClient : null }); + const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); const publicDir = path.join(__dirname, "..", "public"); diff --git a/backend/src/lib/exchange-rate-coordinator.js b/backend/src/lib/exchange-rate-coordinator.js index 27e3c4ab..1abe6439 100644 --- a/backend/src/lib/exchange-rate-coordinator.js +++ b/backend/src/lib/exchange-rate-coordinator.js @@ -121,6 +121,8 @@ export class ExchangeRateCoordinator { * @param {number} [opts.pollIntervalMs] follower poll cadence * @param {string} [opts.keyPrefix] * @param {(data: unknown) => boolean} [opts.validate] shared-entry validator + * @param {(data: unknown) => unknown|null} [opts.sanitize] optional allow-list + * rebuild applied after validation; returning null rejects the entry (#1442) * @param {object|null} [opts.metrics] */ constructor({ @@ -131,6 +133,7 @@ export class ExchangeRateCoordinator { pollIntervalMs = readIntEnv('EXCHANGE_RATE_LOCK_POLL_MS', 50), keyPrefix = 'exrate:', validate = isValidQuote, + sanitize = null, metrics = DEFAULT_METRICS, } = {}) { if (!redisClient || typeof redisClient.sendCommand !== 'function') { @@ -144,6 +147,7 @@ export class ExchangeRateCoordinator { this.quotePrefix = `${keyPrefix}quote:`; this.lock = new RedisLock(redisClient, { prefix: `${keyPrefix}lock:` }); this.validate = validate; + this.sanitize = typeof sanitize === 'function' ? sanitize : null; this.metrics = metrics; } @@ -161,8 +165,11 @@ export class ExchangeRateCoordinator { try { const entry = JSON.parse(String(raw)); if (entry?.v === SHARED_ENTRY_VERSION && this.validate(entry.data)) { - this.metrics?.shared?.inc?.({ result: 'hit' }); - return entry.data; + const data = this.sanitize ? this.sanitize(entry.data) : entry.data; + if (data !== null && data !== undefined) { + this.metrics?.shared?.inc?.({ result: 'hit' }); + return data; + } } } catch { // fall through: treat unparsable content as a miss diff --git a/backend/src/lib/exchange-rate-oracle-sanitizer.js b/backend/src/lib/exchange-rate-oracle-sanitizer.js new file mode 100644 index 00000000..83a33e81 --- /dev/null +++ b/backend/src/lib/exchange-rate-oracle-sanitizer.js @@ -0,0 +1,302 @@ +/** + * Payload sanitization and strict validation for the Exchange Rate Oracle + * Cache (issue #1442). + * + * The cache sits between three trust boundaries: + * + * 1. Request parameters (route -> getExchangeRateQuote) + * asset codes and issuers come from the query string, the amount from + * the database. They form the cache key and the Horizon query. + * 2. Horizon responses (findStrictReceivePaths -> cache) + * third-party data that becomes a payer's send_max. + * 3. The shared store (Redis -> ExchangeRateCoordinator -> cache) + * anything that can write to Redis can try to plant a quote. + * + * Every boundary goes through this module: + * + * sanitizeQuoteRequest() strict input validation + canonicalisation, so + * equivalent requests share one cache key and + * malformed ones never reach Horizon or the cache. + * sanitizeQuote() allow-list rebuild of a quote. Unknown fields, + * prototype keys and wrong types are dropped or + * rejected, so a poisoned entry is a cache miss, + * never a quote. + * + * Validation failures from the request side are 400s and carry + * `retryable: false`, so the oracle retry wrapper never repeats them. + */ + +/** Stellar amounts: up to 7 decimal places, no sign, exponent or spaces. */ +const AMOUNT_PATTERN = /^\d{1,19}(\.\d{1,7})?$/; +/** Accepts extra trailing zeros (e.g. a NUMERIC column rendered as "10.50000000"). */ +const LOOSE_AMOUNT_PATTERN = /^\d{1,19}(\.\d{1,20})?$/; +/** Largest amount Stellar can represent: (2^63 - 1) stroops. */ +const MAX_AMOUNT_STROOPS = 9_223_372_036_854_775_807n; + +const ASSET_CODE_PATTERN = /^[A-Z0-9]{1,12}$/; +const ACCOUNT_ID_PATTERN = /^G[A-Z2-7]{55}$/; +/** Matches resolveAsset() in stellar/validators.js: XLM is always native. */ +const NATIVE_CODE = 'XLM'; + +/** Horizon allows at most 6 intermediate hops in a path payment. */ +export const MAX_PATH_HOPS = 6; +export const MAX_SLIPPAGE = 0.5; + +export const ORACLE_VALIDATION_ERROR_CODE = 'EXCHANGE_RATE_INVALID_PARAMS'; + +export class ExchangeRateValidationError extends Error { + /** + * @param {string} message + * @param {string} field the offending parameter + */ + constructor(message, field) { + super(message); + this.name = 'ExchangeRateValidationError'; + this.field = field; + this.code = ORACLE_VALIDATION_ERROR_CODE; + this.status = 400; + this.statusCode = 400; + this.retryable = false; + } +} + +function fail(field, message) { + throw new ExchangeRateValidationError(message, field); +} + +const BASE32_ALPHABET = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ234567'; +const ED25519_PUBLIC_KEY_VERSION_BYTE = 6 << 3; // 'G' + +function crc16XModem(bytes) { + let crc = 0; + for (const byte of bytes) { + crc ^= byte << 8; + for (let i = 0; i < 8; i += 1) { + crc = crc & 0x8000 ? ((crc << 1) ^ 0x1021) & 0xffff : (crc << 1) & 0xffff; + } + } + return crc; +} + +/** + * Full StrKey check for an ed25519 public key (version byte + CRC16 + * checksum), implemented locally so validation does not depend on how a test + * suite mocks the Stellar SDK. + */ +export function isValidAccountId(value) { + if (typeof value !== 'string' || !ACCOUNT_ID_PATTERN.test(value)) return false; + + const bytes = []; + let buffer = 0; + let bits = 0; + for (const char of value) { + buffer = (buffer << 5) | BASE32_ALPHABET.indexOf(char); + bits += 5; + if (bits >= 8) { + bits -= 8; + bytes.push((buffer >> bits) & 0xff); + } + } + // 56 chars * 5 bits = 280 bits = exactly 35 bytes, with no padding bits. + if (bytes.length !== 35 || (buffer & ((1 << bits) - 1)) !== 0) return false; + if (bytes[0] !== ED25519_PUBLIC_KEY_VERSION_BYTE) return false; + + const payload = bytes.slice(0, 33); + const checksum = bytes[33] | (bytes[34] << 8); + return crc16XModem(payload) === checksum; +} + +function toStroops(amount) { + const [whole, fraction = ''] = amount.split('.'); + return BigInt(whole) * 10_000_000n + BigInt(fraction.padEnd(7, '0')); +} + +/** + * Validate a Stellar amount string. + * + * The caller's formatting is preserved ("50.0000000" stays "50.0000000") so + * API responses do not change shape. The only rewrite is dropping zero + * padding beyond 7 decimals (a NUMERIC column rendered as "10.500000000"), + * which carries no value. + * + * @param {unknown} value + * @param {{ allowZero?: boolean }} [opts] + * @returns {string|null} the validated amount, or null when invalid + */ +export function normalizeAmount(value, { allowZero = false } = {}) { + let text; + if (typeof value === 'number') { + if (!Number.isFinite(value)) return null; + text = String(value); + } else if (typeof value === 'string') { + text = value.trim(); + } else { + return null; + } + + if (!AMOUNT_PATTERN.test(text)) { + if (!LOOSE_AMOUNT_PATTERN.test(text)) return null; + const [whole, fraction] = text.split('.'); + if (/[^0]/.test(fraction.slice(7))) return null; // more precision than a stroop + text = `${whole}.${fraction.slice(0, 7)}`; + } + + const stroops = toStroops(text); + if (stroops > MAX_AMOUNT_STROOPS) return null; + if (!allowZero && stroops === 0n) return null; + return text; +} + +/** + * Validate one asset (code + optional issuer), mirroring resolveAsset() in + * stellar/validators.js so the cache key matches what is sent to Horizon. + * + * @returns {{ code: string, issuer: string|null }} upper-cased code; XLM has no issuer + */ +export function sanitizeAsset(code, issuer, field) { + if (typeof code !== 'string') fail(field, `${field} must be a string`); + const normalized = code.trim().toUpperCase(); + if (!ASSET_CODE_PATTERN.test(normalized)) { + fail(field, `${field} must be 1-12 alphanumeric characters`); + } + if (normalized === NATIVE_CODE) { + // resolveAsset() ignores the issuer for XLM; drop it so it cannot split + // the cache key for the same native asset. + return { code: NATIVE_CODE, issuer: null }; + } + + if (issuer === undefined || issuer === null || issuer === '') { + fail(`${field}Issuer`, `${field}Issuer is required for non-native assets`); + } + if (typeof issuer !== 'string' || !isValidAccountId(issuer.trim())) { + fail(`${field}Issuer`, `${field}Issuer must be a valid Stellar public key`); + } + return { code: normalized, issuer: issuer.trim() }; +} + +/** + * Strictly validate and canonicalise getExchangeRateQuote() parameters. + * + * @param {object} params + * @returns {{ + * sourceAssetCode: string, sourceAssetIssuer: string|null, + * destAssetCode: string, destAssetIssuer: string|null, + * destAmount: string, sourceAccount: string|null, slippage: number + * }} + * @throws {ExchangeRateValidationError} + */ +export function sanitizeQuoteRequest(params) { + if (params === null || typeof params !== 'object' || Array.isArray(params)) { + fail('params', 'Quote parameters must be an object'); + } + + const source = sanitizeAsset(params.sourceAssetCode, params.sourceAssetIssuer, 'sourceAsset'); + const dest = sanitizeAsset(params.destAssetCode, params.destAssetIssuer, 'destAsset'); + + const destAmount = normalizeAmount(params.destAmount); + if (destAmount === null) { + fail('destAmount', 'destAmount must be a positive Stellar amount with at most 7 decimal places'); + } + + let sourceAccount = null; + if (params.sourceAccount !== undefined && params.sourceAccount !== null && params.sourceAccount !== '') { + if (typeof params.sourceAccount !== 'string' || !isValidAccountId(params.sourceAccount.trim())) { + fail('sourceAccount', 'sourceAccount must be a valid Stellar public key'); + } + sourceAccount = params.sourceAccount.trim(); + } + + const slippage = Number(params.slippage); + if (!Number.isFinite(slippage) || slippage < 0 || slippage > MAX_SLIPPAGE) { + fail('slippage', `slippage must be a number between 0 and ${MAX_SLIPPAGE}`); + } + + return { + sourceAssetCode: source.code, + sourceAssetIssuer: source.issuer, + destAssetCode: dest.code, + destAssetIssuer: dest.issuer, + destAmount, + sourceAccount, + slippage, + }; +} + +/** + * Validate an asset inside a quote without rewriting it: Horizon's casing is + * kept so responses do not change. + */ +function sanitizeQuoteAsset(code, issuer) { + if (typeof code !== 'string' || !ASSET_CODE_PATTERN.test(code.toUpperCase())) return null; + if (issuer === null || issuer === undefined || issuer === '') { + // Only the native asset may omit its issuer. + return code.toUpperCase() === NATIVE_CODE ? { code, issuer: null } : null; + } + return isValidAccountId(issuer) ? { code, issuer } : null; +} + +/** + * Rebuild a quote from an allow-list of fields. + * + * Accepts quotes produced by the service, read back from the in-process + * cache, or deserialised from the shared Redis store. Returns a fresh object + * containing only known fields, or null when anything is malformed. Extra + * fields (including `__proto__` / `constructor` keys from JSON.parse) never + * survive. + * + * @param {unknown} data + * @returns {object|null} + */ +export function sanitizeQuote(data) { + if (data === null || typeof data !== 'object' || Array.isArray(data)) return null; + + const source = sanitizeQuoteAsset(data.sourceAsset, data.sourceAssetIssuer); + const dest = sanitizeQuoteAsset(data.destinationAsset, data.destinationAssetIssuer); + if (!source || !dest) return null; + + // Quotes always carry amounts as strings; a number would already have lost + // precision somewhere upstream. + if (![data.sourceAmount, data.sendMax, data.destinationAmount].every((v) => typeof v === 'string')) { + return null; + } + const sourceAmount = normalizeAmount(data.sourceAmount); + const sendMax = normalizeAmount(data.sendMax); + const destinationAmount = normalizeAmount(data.destinationAmount); + if (!sourceAmount || !sendMax || !destinationAmount) return null; + // send_max is source_amount plus slippage; anything lower would make the + // payment fail, so it signals a corrupted entry. + if (toStroops(sendMax) < toStroops(sourceAmount)) return null; + + const slippage = Number(data.slippage); + if (!Number.isFinite(slippage) || slippage < 0 || slippage > MAX_SLIPPAGE) return null; + + if (!Array.isArray(data.path) || data.path.length > MAX_PATH_HOPS) return null; + const path = []; + for (const hop of data.path) { + if (hop === null || typeof hop !== 'object' || Array.isArray(hop)) return null; + const asset = sanitizeQuoteAsset(hop.asset_code, hop.asset_issuer); + if (!asset) return null; + path.push({ asset_code: asset.code, asset_issuer: asset.issuer }); + } + + return { + sourceAsset: source.code, + sourceAssetIssuer: source.issuer, + sourceAmount, + sendMax, + destinationAsset: dest.code, + destinationAssetIssuer: dest.issuer, + destinationAmount, + path, + slippage, + cached: data.cached === true, + }; +} + +/** + * Boolean form of sanitizeQuote(), used as the shared-store validator. + * @param {unknown} data + */ +export function isValidSanitizedQuote(data) { + return sanitizeQuote(data) !== null; +} diff --git a/backend/src/lib/exchange-rate-oracle-sanitizer.test.js b/backend/src/lib/exchange-rate-oracle-sanitizer.test.js new file mode 100644 index 00000000..2e979d33 --- /dev/null +++ b/backend/src/lib/exchange-rate-oracle-sanitizer.test.js @@ -0,0 +1,249 @@ +/** + * Unit tests for Exchange Rate Oracle Cache payload sanitization and strict + * validation (issue #1442). + */ + +import { describe, it, expect } from 'vitest'; +import { + ExchangeRateValidationError, + MAX_PATH_HOPS, + ORACLE_VALIDATION_ERROR_CODE, + isValidAccountId, + isValidSanitizedQuote, + normalizeAmount, + sanitizeAsset, + sanitizeQuote, + sanitizeQuoteRequest, +} from './exchange-rate-oracle-sanitizer.js'; + +const USDC_ISSUER = 'GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5'; +const OTHER_ISSUER = 'GA5HBVNVS6P3QJ2ZGLMEACPSGFG3KDCAFTVHNJXWG22E5DVZTPSSAWQW'; +const BAD_CHECKSUM = 'GA5HBVNVS6P3QJ2ZGLMEACPSGFG3KDCAFTVHNJXWG22E5DVZTPSSAWQA'; +const SECRET_SEED = 'SBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5'; + +const validRequest = (overrides = {}) => ({ + sourceAssetCode: 'XLM', + sourceAssetIssuer: null, + destAssetCode: 'USDC', + destAssetIssuer: USDC_ISSUER, + destAmount: '10.5000000', + sourceAccount: null, + slippage: 0.01, + ...overrides, +}); + +const validQuote = (overrides = {}) => ({ + sourceAsset: 'XLM', + sourceAssetIssuer: null, + sourceAmount: '20.0000000', + sendMax: '20.2000000', + destinationAsset: 'USDC', + destinationAssetIssuer: USDC_ISSUER, + destinationAmount: '10.5000000', + path: [{ asset_code: 'USDC', asset_issuer: USDC_ISSUER }], + slippage: 0.01, + cached: false, + ...overrides, +}); + +const expectValidationError = (fn, field) => { + let caught; + try { + fn(); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(ExchangeRateValidationError); + expect(caught).toMatchObject({ + status: 400, + statusCode: 400, + code: ORACLE_VALIDATION_ERROR_CODE, + retryable: false, + field, + }); +}; + +describe('isValidAccountId', () => { + it('accepts a valid ed25519 public key', () => { + expect(isValidAccountId(USDC_ISSUER)).toBe(true); + expect(isValidAccountId(OTHER_ISSUER)).toBe(true); + }); + + it.each([ + ['a bad checksum', BAD_CHECKSUM], + ['a secret seed', SECRET_SEED], + ['lower case', USDC_ISSUER.toLowerCase()], + ['the wrong length', USDC_ISSUER.slice(0, -1)], + ['a non-string', 42], + ['an empty string', ''], + ])('rejects %s', (_label, value) => { + expect(isValidAccountId(value)).toBe(false); + }); +}); + +describe('normalizeAmount', () => { + it.each([ + ['1', '1'], + ['10.5', '10.5'], + ['50.0000000', '50.0000000'], + [' 7.25 ', '7.25'], + ['10.500000000', '10.5000000'], + [25, '25'], + ['922337203685.4775807', '922337203685.4775807'], + ])('accepts %j as %j', (input, expected) => { + expect(normalizeAmount(input)).toBe(expected); + }); + + it.each([ + ['zero', '0'], + ['zero with decimals', '0.0000000'], + ['negative', '-1'], + ['exponent', '1e5'], + ['too much precision', '1.00000001'], + ['above int64 stroops', '922337203685.4775808'], + ['hex', '0x10'], + ['empty', ''], + ['whitespace inside', '1 0'], + ['NaN number', NaN], + ['Infinity', Infinity], + ['object', { toString: () => '1' }], + ['null', null], + ])('rejects %s', (_label, input) => { + expect(normalizeAmount(input)).toBeNull(); + }); + + it('allows zero when asked', () => { + expect(normalizeAmount('0', { allowZero: true })).toBe('0'); + }); +}); + +describe('sanitizeAsset', () => { + it('upper-cases codes and keeps issuers', () => { + expect(sanitizeAsset(' usdc ', ` ${USDC_ISSUER} `, 'destAsset')).toEqual({ code: 'USDC', issuer: USDC_ISSUER }); + }); + + it('treats XLM as native and drops any issuer, like resolveAsset()', () => { + expect(sanitizeAsset('xlm', USDC_ISSUER, 'sourceAsset')).toEqual({ code: 'XLM', issuer: null }); + }); + + it('requires an issuer for credit assets', () => { + expectValidationError(() => sanitizeAsset('USDC', null, 'destAsset'), 'destAssetIssuer'); + expectValidationError(() => sanitizeAsset('USDC', '', 'destAsset'), 'destAssetIssuer'); + }); + + it.each([ + ['too long', 'ABCDEFGHIJKLM'], + ['symbols', 'US$C'], + ['empty', ''], + ['a separator', 'USDC:GA'], + ])('rejects a code that is %s', (_label, code) => { + expectValidationError(() => sanitizeAsset(code, USDC_ISSUER, 'destAsset'), 'destAsset'); + }); + + it('rejects malformed issuers', () => { + expectValidationError(() => sanitizeAsset('USDC', BAD_CHECKSUM, 'destAsset'), 'destAssetIssuer'); + expectValidationError(() => sanitizeAsset('USDC', { toString: () => USDC_ISSUER }, 'destAsset'), 'destAssetIssuer'); + }); +}); + +describe('sanitizeQuoteRequest', () => { + it('returns canonical parameters for a valid request', () => { + expect(sanitizeQuoteRequest(validRequest({ sourceAssetCode: ' xlm ', destAssetCode: 'usdc' }))).toEqual({ + sourceAssetCode: 'XLM', + sourceAssetIssuer: null, + destAssetCode: 'USDC', + destAssetIssuer: USDC_ISSUER, + destAmount: '10.5000000', + sourceAccount: null, + slippage: 0.01, + }); + }); + + it('accepts and trims a valid source account', () => { + expect(sanitizeQuoteRequest(validRequest({ sourceAccount: ` ${OTHER_ISSUER}` })).sourceAccount).toBe(OTHER_ISSUER); + }); + + it('does not copy unknown fields', () => { + const result = sanitizeQuoteRequest({ ...validRequest(), extra: 'x', __proto__: { polluted: true } }); + expect(Object.keys(result).sort()).toEqual([ + 'destAmount', 'destAssetCode', 'destAssetIssuer', 'slippage', + 'sourceAccount', 'sourceAssetCode', 'sourceAssetIssuer', + ]); + expect(result.polluted).toBeUndefined(); + }); + + it.each([ + ['null params', null, 'params'], + ['array params', [], 'params'], + ['bad source code', validRequest({ sourceAssetCode: 'X L M' }), 'sourceAsset'], + ['missing dest issuer', validRequest({ destAssetIssuer: null }), 'destAssetIssuer'], + ['zero amount', validRequest({ destAmount: '0' }), 'destAmount'], + ['injected amount', validRequest({ destAmount: '1;DROP' }), 'destAmount'], + ['bad source account', validRequest({ sourceAccount: BAD_CHECKSUM }), 'sourceAccount'], + ['non-string source account', validRequest({ sourceAccount: 7 }), 'sourceAccount'], + ['negative slippage', validRequest({ slippage: -0.01 }), 'slippage'], + ['excessive slippage', validRequest({ slippage: 0.9 }), 'slippage'], + ['NaN slippage', validRequest({ slippage: 'abc' }), 'slippage'], + ])('rejects %s', (_label, params, field) => { + expectValidationError(() => sanitizeQuoteRequest(params), field); + }); +}); + +describe('sanitizeQuote', () => { + it('returns an equal, fresh object for a valid quote', () => { + const input = validQuote(); + const output = sanitizeQuote(input); + expect(output).toEqual(input); + expect(output).not.toBe(input); + expect(output.path[0]).not.toBe(input.path[0]); + }); + + it('preserves Horizon formatting and casing', () => { + const output = sanitizeQuote(validQuote({ sourceAsset: 'yXLM', sourceAssetIssuer: OTHER_ISSUER })); + expect(output.sourceAsset).toBe('yXLM'); + expect(output.sourceAmount).toBe('20.0000000'); + }); + + it('strips unknown fields and prototype keys from parsed JSON', () => { + const poisoned = JSON.parse(JSON.stringify(validQuote()).replace( + /^\{/, + '{"__proto__":{"isAdmin":true},"constructor":{"prototype":{"x":1}},"memo":"pay-me",', + )); + const output = sanitizeQuote(poisoned); + expect(output).not.toBeNull(); + expect(Object.keys(output)).not.toContain('memo'); + expect(output.isAdmin).toBeUndefined(); + expect({}.isAdmin).toBeUndefined(); + }); + + it('only keeps cached:true when it is literally true', () => { + expect(sanitizeQuote(validQuote({ cached: 'yes' })).cached).toBe(false); + expect(sanitizeQuote(validQuote({ cached: true })).cached).toBe(true); + }); + + it('allows an empty path', () => { + expect(sanitizeQuote(validQuote({ path: [] })).path).toEqual([]); + }); + + it.each([ + ['null', null], + ['an array', []], + ['a string', 'quote'], + ['a numeric sendMax', validQuote({ sendMax: 20.2 })], + ['an exponent amount', validQuote({ sourceAmount: '2e1' })], + ['sendMax below sourceAmount', validQuote({ sendMax: '19.0000000' })], + ['a zero destination amount', validQuote({ destinationAmount: '0' })], + ['a credit asset without issuer', validQuote({ destinationAssetIssuer: null })], + ['a bad issuer', validQuote({ sourceAsset: 'EUR', sourceAssetIssuer: BAD_CHECKSUM })], + ['an oversized asset code', validQuote({ sourceAsset: 'ABCDEFGHIJKLM' })], + ['slippage out of range', validQuote({ slippage: 2 })], + ['a missing slippage', validQuote({ slippage: undefined })], + ['a non-array path', validQuote({ path: 'XLM>USDC' })], + ['too many hops', validQuote({ path: Array.from({ length: MAX_PATH_HOPS + 1 }, () => ({ asset_code: 'XLM', asset_issuer: null })) })], + ['a malformed hop', validQuote({ path: [null] })], + ['a hop with a bad issuer', validQuote({ path: [{ asset_code: 'USDC', asset_issuer: 'G123' }] })], + ])('rejects %s', (_label, data) => { + expect(sanitizeQuote(data)).toBeNull(); + expect(isValidSanitizedQuote(data)).toBe(false); + }); +}); diff --git a/backend/src/lib/stellar/horizon-client.js b/backend/src/lib/stellar/horizon-client.js index 31f1f2a7..2772463d 100644 --- a/backend/src/lib/stellar/horizon-client.js +++ b/backend/src/lib/stellar/horizon-client.js @@ -1,195 +1,230 @@ -/** - * Horizon Client - Encapsulates all Horizon API interactions - * - * This module provides a clean interface for Horizon operations with built-in - * retry logic, error handling, and connection management. - * - * Refactored to use modular components for better maintainability: - * - RetryManager: Handles retry logic and metrics - * - ErrorHandler: Error classification and handling utilities - * - Constants: Centralized configuration - */ - -import * as StellarSdk from "stellar-sdk"; -import { logger } from "../logger.js"; -import { - horizonClientErrors, - horizonClientLatency, - horizonClientHealthCheckDuration, - horizonClientHealthCheckResult, - horizonClientConnections, -} from "../metrics.js"; -import { DEFAULT_RETRY_DELAYS_MS, DEFAULT_HEALTH_TIMEOUT_MS } from "./horizon-client/constants.js"; -import { createRetryManager } from "./horizon-client/retry-manager.js"; -import { classifyError, getErrorStatus, handleError } from "./horizon-client/error-handler.js"; - -export class HorizonClient { - constructor(horizonUrl, networkPassphrase, options = {}) { - this.horizonUrl = horizonUrl.replace(/\/$/, ""); - this.networkPassphrase = networkPassphrase; - this.retryDelays = options.retryDelays || DEFAULT_RETRY_DELAYS_MS; - this.healthTimeout = options.healthTimeout || DEFAULT_HEALTH_TIMEOUT_MS; - - this.server = new StellarSdk.Horizon.Server(this.horizonUrl); - this.retryManager = createRetryManager(this.retryDelays); - - // Track active connections - horizonClientConnections.inc(); - } - - /** - * Check if Horizon server is reachable - */ - async isReachable() { - const controller = new AbortController(); - const timeoutId = setTimeout( - () => controller.abort(), - this.healthTimeout, - ); - - const startTime = Date.now(); - try { - const response = await fetch(this.horizonUrl, { - method: "GET", - signal: controller.signal, - headers: { - Accept: "application/json", - }, - }); - - // Treat rate limiting as reachable so transient Horizon throttling - // doesn't fail the entire API health check. - const isReachable = response.ok || response.status === 429; - const duration = (Date.now() - startTime) / 1000; - - // Record granular health check metrics - horizonClientHealthCheckDuration.observe(duration); - horizonClientHealthCheckResult.inc({ - result: isReachable ? "success" : "failure" - }); - horizonClientLatency.observe({ - operation: "health_check", - result: isReachable ? "success" : "failure" - }, duration); - - return isReachable; - } catch (err) { - const duration = (Date.now() - startTime) / 1000; - - // Record health check failure metrics - horizonClientHealthCheckDuration.observe(duration); - horizonClientHealthCheckResult.inc({ result: "failure" }); - horizonClientErrors.inc({ - operation: "health_check", - error_type: classifyError(err) - }); - - return false; - } finally { - clearTimeout(timeoutId); - } - } - - /** - * Load account details with retry logic - */ - async loadAccount(accountId) { - return this.retryManager.executeWithRetry( - () => this.server.loadAccount(accountId), - "loadAccount", - { accountId }, - this.horizonUrl - ); - } - - /** - * Fetch payments for an account with retry logic - */ - async fetchPayments(accountId, options = {}) { - const builder = this.server - .payments() - .forAccount(accountId); - - if (options.order) builder.order(options.order); - if (options.limit) builder.limit(options.limit); - - return this.retryManager.executeWithRetry( - () => builder.call(), - "fetchPayments", - { accountId, options }, - this.horizonUrl - ); - } - - /** - * Fetch transaction details with retry logic - */ - async fetchTransaction(txHash) { - return this.retryManager.executeWithRetry( - () => this.server.transactions().transaction(txHash).call(), - "fetchTransaction", - { txHash }, - this.horizonUrl - ); - } - - /** - * Find strict-receive payment paths with retry logic - */ - async findStrictReceivePaths(sourceAssets, destAsset, destAmount) { - return this.retryManager.executeWithRetry( - () => this.server - .strictReceivePaths(sourceAssets, destAsset, destAmount) - .call(), - "findStrictReceivePaths", - { destAsset: destAsset.getCode(), destAmount }, - this.horizonUrl - ); - } - - /** - * Get fee statistics with retry logic - */ - async getFeeStats() { - return this.retryManager.executeWithRetry( - () => this.server.feeStats(), - "getFeeStats", - {}, - this.horizonUrl - ); - } - - /** - * Get client configuration - */ - getConfig() { - return { - horizonUrl: this.horizonUrl, - networkPassphrase: this.networkPassphrase, - retryDelays: this.retryDelays, - healthTimeout: this.healthTimeout, - retryManager: this.retryManager.getConfig(), - }; - } - - /** - * Get the retry manager instance (for advanced use cases) - */ - getRetryManager() { - return this.retryManager; - } - - /** - * Get the Stellar SDK server instance (for advanced use cases) - */ - getServer() { - return this.server; - } - - /** - * Cleanup resources (decrement connection counter) - */ - close() { - horizonClientConnections.dec(); - } -} \ No newline at end of file +/** + * Horizon Client - Encapsulates all Horizon API interactions + * + * This module provides a clean interface for Horizon operations with built-in + * retry logic, error handling, and connection management. + * + * Refactored to use modular components for better maintainability: + * - RetryManager: Handles retry logic and metrics + * - Backoff: Jittered exponential backoff + Retry-After handling (#1439) + * - ConcurrencyLimiter / HorizonRetryCoordinator: per-process and + * cross-instance concurrency control for retries (#1440) + * - ErrorHandler: Error classification and handling utilities + * - Constants: Centralized configuration + */ + +import * as StellarSdk from "stellar-sdk"; +import { logger } from "../logger.js"; +import { + horizonClientErrors, + horizonClientLatency, + horizonClientHealthCheckDuration, + horizonClientHealthCheckResult, + horizonClientConnections, +} from "../metrics.js"; +import { DEFAULT_RETRY_DELAYS_MS, DEFAULT_HEALTH_TIMEOUT_MS } from "./horizon-client/constants.js"; +import { createRetryManager } from "./horizon-client/retry-manager.js"; +import { HorizonRetryCoordinator } from "./horizon-client/retry-coordinator.js"; +import { classifyError, getErrorStatus, handleError } from "./horizon-client/error-handler.js"; + +export class HorizonClient { + constructor(horizonUrl, networkPassphrase, options = {}) { + this.horizonUrl = horizonUrl.replace(/\/$/, ""); + this.networkPassphrase = networkPassphrase; + this.retryDelays = options.retryDelays || DEFAULT_RETRY_DELAYS_MS; + this.healthTimeout = options.healthTimeout || DEFAULT_HEALTH_TIMEOUT_MS; + + // `serverOptions` is forwarded to the SDK (e.g. { allowHttp: true } for a + // local standalone network or an integration-test Horizon stub). + this.server = options.serverOptions + ? new StellarSdk.Horizon.Server(this.horizonUrl, options.serverOptions) + : new StellarSdk.Horizon.Server(this.horizonUrl); + // An explicit `retryDelays` keeps the legacy fixed schedule; otherwise the + // manager uses jittered exponential backoff (issue #1439). + this.retryManager = createRetryManager(options.retryDelays, { + horizonUrl: this.horizonUrl, + backoff: options.backoff, + concurrency: options.concurrency, + coordination: options.coordination, + sleep: options.sleep, + random: options.random, + }); + + // Track active connections + horizonClientConnections.inc(); + } + + /** + * Check if Horizon server is reachable + */ + async isReachable() { + const controller = new AbortController(); + const timeoutId = setTimeout( + () => controller.abort(), + this.healthTimeout, + ); + + const startTime = Date.now(); + try { + const response = await fetch(this.horizonUrl, { + method: "GET", + signal: controller.signal, + headers: { + Accept: "application/json", + }, + }); + + // Treat rate limiting as reachable so transient Horizon throttling + // doesn't fail the entire API health check. + const isReachable = response.ok || response.status === 429; + const duration = (Date.now() - startTime) / 1000; + + // Record granular health check metrics + horizonClientHealthCheckDuration.observe(duration); + horizonClientHealthCheckResult.inc({ + result: isReachable ? "success" : "failure" + }); + horizonClientLatency.observe({ + operation: "health_check", + result: isReachable ? "success" : "failure" + }, duration); + + return isReachable; + } catch (err) { + const duration = (Date.now() - startTime) / 1000; + + // Record health check failure metrics + horizonClientHealthCheckDuration.observe(duration); + horizonClientHealthCheckResult.inc({ result: "failure" }); + horizonClientErrors.inc({ + operation: "health_check", + error_type: classifyError(err) + }); + + return false; + } finally { + clearTimeout(timeoutId); + } + } + + /** + * Load account details with retry logic + */ + async loadAccount(accountId) { + return this.retryManager.executeWithRetry( + () => this.server.loadAccount(accountId), + "loadAccount", + { accountId }, + this.horizonUrl + ); + } + + /** + * Fetch payments for an account with retry logic + */ + async fetchPayments(accountId, options = {}) { + const builder = this.server + .payments() + .forAccount(accountId); + + if (options.order) builder.order(options.order); + if (options.limit) builder.limit(options.limit); + + return this.retryManager.executeWithRetry( + () => builder.call(), + "fetchPayments", + { accountId, options }, + this.horizonUrl + ); + } + + /** + * Fetch transaction details with retry logic + */ + async fetchTransaction(txHash) { + return this.retryManager.executeWithRetry( + () => this.server.transactions().transaction(txHash).call(), + "fetchTransaction", + { txHash }, + this.horizonUrl + ); + } + + /** + * Find strict-receive payment paths with retry logic + */ + async findStrictReceivePaths(sourceAssets, destAsset, destAmount) { + return this.retryManager.executeWithRetry( + () => this.server + .strictReceivePaths(sourceAssets, destAsset, destAmount) + .call(), + "findStrictReceivePaths", + { destAsset: destAsset.getCode(), destAmount }, + this.horizonUrl + ); + } + + /** + * Get fee statistics with retry logic + */ + async getFeeStats() { + return this.retryManager.executeWithRetry( + () => this.server.feeStats(), + "getFeeStats", + {}, + this.horizonUrl + ); + } + + /** + * Get client configuration + */ + getConfig() { + return { + horizonUrl: this.horizonUrl, + networkPassphrase: this.networkPassphrase, + retryDelays: this.retryDelays, + healthTimeout: this.healthTimeout, + retryManager: this.retryManager.getConfig(), + }; + } + + /** + * Enable cross-instance retry coordination (issue #1440). Pass a connected + * Redis client; a missing or closed client keeps the in-process fallback. + * + * @param {object} opts + * @param {object|null} opts.redisClient + * @returns {"redis"|"local"} the active coordination backend + */ + configureRetryCoordination({ redisClient = null, ...options } = {}) { + const coordinator = new HorizonRetryCoordinator({ + ...options, + redisClient, + horizonUrl: this.horizonUrl, + }); + this.retryManager.setCoordinator(coordinator); + return coordinator.backend; + } + + /** + * Get the retry manager instance (for advanced use cases) + */ + getRetryManager() { + return this.retryManager; + } + + /** + * Get the Stellar SDK server instance (for advanced use cases) + */ + getServer() { + return this.server; + } + + /** + * Cleanup resources (decrement connection counter) + */ + close() { + horizonClientConnections.dec(); + } +} \ No newline at end of file diff --git a/backend/src/lib/stellar/horizon-client/backoff.js b/backend/src/lib/stellar/horizon-client/backoff.js new file mode 100644 index 00000000..7d4ff515 --- /dev/null +++ b/backend/src/lib/stellar/horizon-client/backoff.js @@ -0,0 +1,231 @@ +/** + * Horizon Client Backoff Policy (issue #1439) + * + * Computes how long the Retry Manager waits between attempts. + * + * Strategies + * ---------- + * exponential (default) delay(n) = jitter(min(maxDelayMs, baseDelayMs * multiplier^n)) + * fixed delay(n) = retryDelays[n] (legacy explicit schedule) + * + * Jitter modes (exponential only) + * full random(0, ceiling) - best at spreading retry storms + * equal ceiling/2 + random(0, ceiling/2) - keeps a guaranteed minimum wait + * none ceiling - deterministic + * + * Every number is clamped to a safe range, so a bad environment variable can + * neither disable the backoff nor make a request wait for minutes. + * + * Retry-After + * ----------- + * A 429/503 from Horizon may carry a Retry-After header (delta-seconds or an + * HTTP date). parseRetryAfterMs() turns it into milliseconds; the Retry + * Manager waits at least that long and gives up instead of waiting when the + * hint exceeds maxRetryAfterMs. + */ + +import { + DEFAULT_BACKOFF_OPTIONS, + BACKOFF_LIMITS, + BACKOFF_JITTER_MODES, + BACKOFF_STRATEGIES, +} from "./constants.js"; + +function clampInt(value, fallback, min, max) { + if (value === undefined || value === null || value === "") return fallback; + const n = Number(value); + if (!Number.isFinite(n)) return fallback; + return Math.min(Math.max(Math.trunc(n), min), max); +} + +function clampFloat(value, fallback, min, max) { + if (value === undefined || value === null || value === "") return fallback; + const n = Number(value); + if (!Number.isFinite(n)) return fallback; + return Math.min(Math.max(n, min), max); +} + +function pickEnum(value, allowed, fallback) { + const normalized = String(value ?? "").trim().toLowerCase(); + return allowed.includes(normalized) ? normalized : fallback; +} + +/** + * Sanitize an explicit delay schedule. Non-finite or negative entries are + * dropped and every delay is capped, so `[NaN, -1, 1e9]` cannot stall a call. + * + * @param {unknown} delays + * @returns {number[]|null} null when no usable schedule was provided + */ +export function normalizeRetryDelays(delays) { + if (!Array.isArray(delays)) return null; + const cleaned = delays + .map((d) => Number(d)) + .filter((d) => Number.isFinite(d) && d >= 0) + .slice(0, BACKOFF_LIMITS.maxRetries) + .map((d) => Math.min(Math.trunc(d), BACKOFF_LIMITS.maxDelayMs)); + return cleaned; +} + +/** + * Resolve the effective backoff configuration. + * + * Precedence: explicit options > environment > defaults. + * When `retryDelays` is supplied the strategy defaults to "fixed" so existing + * callers keep their exact schedule. + * + * @param {object} [options] + * @param {number[]} [options.retryDelays] + * @param {"exponential"|"fixed"} [options.strategy] + * @param {number} [options.maxRetries] + * @param {number} [options.baseDelayMs] + * @param {number} [options.maxDelayMs] + * @param {number} [options.multiplier] + * @param {"full"|"equal"|"none"} [options.jitter] + * @param {number} [options.maxElapsedMs] + * @param {number} [options.maxRetryAfterMs] + * @param {NodeJS.ProcessEnv} [env] + */ +export function resolveBackoffOptions(options = {}, env = process.env) { + const retryDelays = normalizeRetryDelays(options.retryDelays); + const strategy = pickEnum( + options.strategy ?? (retryDelays ? BACKOFF_STRATEGIES.FIXED : env.HORIZON_RETRY_STRATEGY), + Object.values(BACKOFF_STRATEGIES), + DEFAULT_BACKOFF_OPTIONS.strategy, + ); + + const baseDelayMs = clampInt( + options.baseDelayMs ?? env.HORIZON_RETRY_BASE_DELAY_MS, + DEFAULT_BACKOFF_OPTIONS.baseDelayMs, + 0, + BACKOFF_LIMITS.maxDelayMs, + ); + const maxDelayMs = clampInt( + options.maxDelayMs ?? env.HORIZON_RETRY_MAX_DELAY_MS, + DEFAULT_BACKOFF_OPTIONS.maxDelayMs, + baseDelayMs, + BACKOFF_LIMITS.maxDelayMs, + ); + + const fixedSchedule = + strategy === BACKOFF_STRATEGIES.FIXED + ? retryDelays ?? [...DEFAULT_BACKOFF_OPTIONS.fixedDelays] + : null; + + const maxRetries = fixedSchedule + ? fixedSchedule.length + : clampInt( + options.maxRetries ?? env.HORIZON_RETRY_MAX_RETRIES, + DEFAULT_BACKOFF_OPTIONS.maxRetries, + 0, + BACKOFF_LIMITS.maxRetries, + ); + + return Object.freeze({ + strategy, + maxRetries, + baseDelayMs, + maxDelayMs, + multiplier: clampFloat( + options.multiplier ?? env.HORIZON_RETRY_MULTIPLIER, + DEFAULT_BACKOFF_OPTIONS.multiplier, + 1, + BACKOFF_LIMITS.maxMultiplier, + ), + jitter: pickEnum( + options.jitter ?? env.HORIZON_RETRY_JITTER, + Object.values(BACKOFF_JITTER_MODES), + DEFAULT_BACKOFF_OPTIONS.jitter, + ), + maxElapsedMs: clampInt( + options.maxElapsedMs ?? env.HORIZON_RETRY_MAX_ELAPSED_MS, + DEFAULT_BACKOFF_OPTIONS.maxElapsedMs, + 0, + BACKOFF_LIMITS.maxElapsedMs, + ), + maxRetryAfterMs: clampInt( + options.maxRetryAfterMs ?? env.HORIZON_RETRY_MAX_RETRY_AFTER_MS, + DEFAULT_BACKOFF_OPTIONS.maxRetryAfterMs, + 0, + BACKOFF_LIMITS.maxElapsedMs, + ), + fixedDelays: fixedSchedule ? Object.freeze(fixedSchedule) : null, + }); +} + +/** + * Delay before retry number `retryIndex` (0 = first retry). + * + * @param {number} retryIndex + * @param {ReturnType} policy + * @param {() => number} [random] injectable for deterministic tests + * @returns {number} milliseconds, always an integer >= 0 + */ +export function computeBackoffDelay(retryIndex, policy, random = Math.random) { + const index = Math.max(0, Math.trunc(Number(retryIndex) || 0)); + + if (policy.strategy === BACKOFF_STRATEGIES.FIXED) { + const schedule = policy.fixedDelays ?? []; + if (schedule.length === 0) return 0; + return schedule[Math.min(index, schedule.length - 1)]; + } + + // Cap the exponent before Math.pow so huge indices cannot overflow to Infinity. + const exponent = Math.min(index, 64); + const ceiling = Math.min(policy.maxDelayMs, policy.baseDelayMs * policy.multiplier ** exponent); + if (!Number.isFinite(ceiling) || ceiling <= 0) return 0; + + const r = Math.min(Math.max(Number(random()) || 0, 0), 1); + switch (policy.jitter) { + case BACKOFF_JITTER_MODES.NONE: + return Math.floor(ceiling); + case BACKOFF_JITTER_MODES.EQUAL: + return Math.floor(ceiling / 2 + r * (ceiling / 2)); + case BACKOFF_JITTER_MODES.FULL: + default: + return Math.floor(r * ceiling); + } +} + +function readHeader(headers, name) { + if (!headers) return undefined; + if (typeof headers.get === "function") { + return headers.get(name) ?? undefined; + } + if (typeof headers === "object") { + const match = Object.keys(headers).find((key) => key.toLowerCase() === name); + return match ? headers[match] : undefined; + } + return undefined; +} + +/** + * Extract a Retry-After hint from a Horizon / fetch / axios-style error. + * + * @param {unknown} err + * @param {number} [now] + * @returns {number|null} milliseconds to wait, or null when absent/invalid + */ +export function parseRetryAfterMs(err, now = Date.now()) { + const raw = + readHeader(err?.response?.headers, "retry-after") ?? + readHeader(err?.headers, "retry-after") ?? + err?.retryAfter; + + if (raw === undefined || raw === null) return null; + const value = Array.isArray(raw) ? raw[0] : raw; + const text = String(value).trim(); + if (text.length === 0 || text.length > 64) return null; + + if (/^\d+(\.\d+)?$/.test(text)) { + const ms = Math.ceil(Number(text) * 1000); + return Number.isFinite(ms) ? ms : null; + } + + // HTTP dates always contain a weekday/month name; this also stops + // Date.parse() from accepting bare values such as "-1" as a year. + if (!/[a-z]/i.test(text)) return null; + const date = Date.parse(text); + if (Number.isNaN(date)) return null; + return Math.max(0, date - now); +} diff --git a/backend/src/lib/stellar/horizon-client/backoff.test.js b/backend/src/lib/stellar/horizon-client/backoff.test.js new file mode 100644 index 00000000..31f3ef95 --- /dev/null +++ b/backend/src/lib/stellar/horizon-client/backoff.test.js @@ -0,0 +1,204 @@ +/** + * Unit tests for the Horizon Client backoff policy (issue #1439). + */ + +import { describe, it, expect } from 'vitest'; +import { + computeBackoffDelay, + normalizeRetryDelays, + parseRetryAfterMs, + resolveBackoffOptions, +} from './backoff.js'; +import { BACKOFF_LIMITS, DEFAULT_BACKOFF_OPTIONS, DEFAULT_RETRY_DELAYS_MS } from './constants.js'; + +const NO_ENV = {}; + +describe('resolveBackoffOptions', () => { + it('defaults to jittered exponential backoff with the legacy attempt count', () => { + const policy = resolveBackoffOptions({}, NO_ENV); + expect(policy.strategy).toBe('exponential'); + expect(policy.jitter).toBe('full'); + expect(policy.maxRetries).toBe(DEFAULT_RETRY_DELAYS_MS.length); + expect(policy.baseDelayMs).toBe(DEFAULT_BACKOFF_OPTIONS.baseDelayMs); + expect(policy.fixedDelays).toBeNull(); + expect(Object.isFrozen(policy)).toBe(true); + }); + + it('switches to the fixed strategy when an explicit schedule is given', () => { + const policy = resolveBackoffOptions({ retryDelays: [50, 100, 200] }, NO_ENV); + expect(policy.strategy).toBe('fixed'); + expect(policy.maxRetries).toBe(3); + expect(policy.fixedDelays).toEqual([50, 100, 200]); + }); + + it('reads environment variables when no option is given', () => { + const policy = resolveBackoffOptions({}, { + HORIZON_RETRY_MAX_RETRIES: '4', + HORIZON_RETRY_BASE_DELAY_MS: '20', + HORIZON_RETRY_MAX_DELAY_MS: '400', + HORIZON_RETRY_MULTIPLIER: '2', + HORIZON_RETRY_JITTER: 'EQUAL', + HORIZON_RETRY_MAX_ELAPSED_MS: '3000', + HORIZON_RETRY_MAX_RETRY_AFTER_MS: '1500', + }); + expect(policy).toMatchObject({ + maxRetries: 4, + baseDelayMs: 20, + maxDelayMs: 400, + multiplier: 2, + jitter: 'equal', + maxElapsedMs: 3000, + maxRetryAfterMs: 1500, + }); + }); + + it('prefers explicit options over the environment', () => { + const policy = resolveBackoffOptions({ maxRetries: 1 }, { HORIZON_RETRY_MAX_RETRIES: '5' }); + expect(policy.maxRetries).toBe(1); + }); + + it.each([ + ['garbage strings', { maxRetries: 'abc', baseDelayMs: 'x', jitter: 'chaotic', strategy: 'random' }], + ['non-finite numbers', { maxRetries: Infinity, baseDelayMs: NaN, multiplier: -Infinity }], + ])('falls back to safe defaults for %s', (_label, options) => { + const policy = resolveBackoffOptions(options, NO_ENV); + expect(policy.strategy).toBe('exponential'); + expect(policy.jitter).toBe('full'); + expect(policy.maxRetries).toBe(DEFAULT_BACKOFF_OPTIONS.maxRetries); + expect(policy.baseDelayMs).toBe(DEFAULT_BACKOFF_OPTIONS.baseDelayMs); + expect(policy.multiplier).toBe(DEFAULT_BACKOFF_OPTIONS.multiplier); + }); + + it('clamps out-of-range values to hard ceilings', () => { + const policy = resolveBackoffOptions({ + maxRetries: 1_000, + baseDelayMs: 10_000_000, + maxDelayMs: 10_000_000, + multiplier: 1_000, + maxElapsedMs: 10_000_000, + }, NO_ENV); + expect(policy.maxRetries).toBe(BACKOFF_LIMITS.maxRetries); + expect(policy.baseDelayMs).toBe(BACKOFF_LIMITS.maxDelayMs); + expect(policy.maxDelayMs).toBe(BACKOFF_LIMITS.maxDelayMs); + expect(policy.multiplier).toBe(BACKOFF_LIMITS.maxMultiplier); + expect(policy.maxElapsedMs).toBe(BACKOFF_LIMITS.maxElapsedMs); + }); + + it('never lets maxDelayMs drop below baseDelayMs', () => { + const policy = resolveBackoffOptions({ baseDelayMs: 500, maxDelayMs: 100 }, NO_ENV); + expect(policy.maxDelayMs).toBe(500); + }); +}); + +describe('normalizeRetryDelays', () => { + it('returns null for a non-array', () => { + expect(normalizeRetryDelays(undefined)).toBeNull(); + expect(normalizeRetryDelays('100,200')).toBeNull(); + }); + + it('drops invalid entries and caps large ones', () => { + expect(normalizeRetryDelays([NaN, -1, 10, '20', 1e12])).toEqual([10, 20, BACKOFF_LIMITS.maxDelayMs]); + }); + + it('limits the schedule length', () => { + expect(normalizeRetryDelays(new Array(50).fill(1))).toHaveLength(BACKOFF_LIMITS.maxRetries); + }); +}); + +describe('computeBackoffDelay', () => { + const exponential = (overrides = {}) => + resolveBackoffOptions({ baseDelayMs: 100, maxDelayMs: 1_000, multiplier: 2, ...overrides }, NO_ENV); + + it('grows exponentially without jitter', () => { + const policy = exponential({ jitter: 'none' }); + expect([0, 1, 2, 3].map((n) => computeBackoffDelay(n, policy))).toEqual([100, 200, 400, 800]); + }); + + it('is capped at maxDelayMs', () => { + const policy = exponential({ jitter: 'none' }); + expect(computeBackoffDelay(10, policy)).toBe(1_000); + expect(computeBackoffDelay(10_000, policy)).toBe(1_000); + }); + + it('full jitter stays within [0, ceiling)', () => { + const policy = exponential({ jitter: 'full' }); + expect(computeBackoffDelay(2, policy, () => 0)).toBe(0); + expect(computeBackoffDelay(2, policy, () => 0.5)).toBe(200); + expect(computeBackoffDelay(2, policy, () => 0.999)).toBeLessThan(400); + }); + + it('equal jitter keeps at least half the ceiling', () => { + const policy = exponential({ jitter: 'equal' }); + expect(computeBackoffDelay(2, policy, () => 0)).toBe(200); + expect(computeBackoffDelay(2, policy, () => 1)).toBe(400); + }); + + it('tolerates a misbehaving random source', () => { + const policy = exponential({ jitter: 'full' }); + expect(computeBackoffDelay(1, policy, () => NaN)).toBe(0); + expect(computeBackoffDelay(1, policy, () => 5)).toBe(200); + expect(computeBackoffDelay(1, policy, () => -5)).toBe(0); + }); + + it('treats negative or invalid indices as the first retry', () => { + const policy = exponential({ jitter: 'none' }); + expect(computeBackoffDelay(-3, policy)).toBe(100); + expect(computeBackoffDelay('x', policy)).toBe(100); + }); + + it('returns 0 when the base delay is 0', () => { + expect(computeBackoffDelay(3, exponential({ baseDelayMs: 0, maxDelayMs: 0 }))).toBe(0); + }); + + it('follows the fixed schedule and repeats its last entry', () => { + const policy = resolveBackoffOptions({ retryDelays: [50, 100] }, NO_ENV); + expect([0, 1, 2].map((n) => computeBackoffDelay(n, policy))).toEqual([50, 100, 100]); + }); + + it('returns 0 for an empty fixed schedule', () => { + const policy = resolveBackoffOptions({ retryDelays: [] }, NO_ENV); + expect(policy.maxRetries).toBe(0); + expect(computeBackoffDelay(0, policy)).toBe(0); + }); +}); + +describe('parseRetryAfterMs', () => { + const NOW = Date.parse('2026-10-15T12:00:00Z'); + + it('parses delta-seconds from plain object headers (any casing)', () => { + expect(parseRetryAfterMs({ response: { headers: { 'Retry-After': '2' } } }, NOW)).toBe(2_000); + expect(parseRetryAfterMs({ response: { headers: { 'retry-after': '0.5' } } }, NOW)).toBe(500); + }); + + it('parses a Headers-like object', () => { + const headers = new Headers({ 'retry-after': '3' }); + expect(parseRetryAfterMs({ response: { headers } }, NOW)).toBe(3_000); + }); + + it('parses an HTTP date relative to now', () => { + const err = { headers: { 'retry-after': 'Thu, 15 Oct 2026 12:00:04 GMT' } }; + expect(parseRetryAfterMs(err, NOW)).toBe(4_000); + }); + + it('returns 0 for a date in the past', () => { + const err = { headers: { 'retry-after': 'Thu, 15 Oct 2026 11:00:00 GMT' } }; + expect(parseRetryAfterMs(err, NOW)).toBe(0); + }); + + it('reads an explicit retryAfter property and array values', () => { + expect(parseRetryAfterMs({ retryAfter: 1 }, NOW)).toBe(1_000); + expect(parseRetryAfterMs({ headers: { 'retry-after': ['2', '9'] } }, NOW)).toBe(2_000); + }); + + it.each([ + [null], + [{}], + [{ response: { headers: {} } }], + [{ headers: { 'retry-after': '' } }], + [{ headers: { 'retry-after': '-1' } }], + [{ headers: { 'retry-after': 'soon' } }], + [{ headers: { 'retry-after': '1'.repeat(100) } }], + ])('returns null for missing or invalid hints (%#)', (err) => { + expect(parseRetryAfterMs(err, NOW)).toBeNull(); + }); +}); diff --git a/backend/src/lib/stellar/horizon-client/concurrency-limiter.js b/backend/src/lib/stellar/horizon-client/concurrency-limiter.js new file mode 100644 index 00000000..52ba890e --- /dev/null +++ b/backend/src/lib/stellar/horizon-client/concurrency-limiter.js @@ -0,0 +1,192 @@ +/** + * Horizon Client Concurrency Limiter (issue #1440) + * + * A per-process counting semaphore with a bounded FIFO wait queue. + * + * Without it every caller hits Horizon at once. During a Horizon slowdown the + * number of open sockets and pending promises grows without bound, and when + * Horizon starts answering 429 every one of those callers retries at once. + * + * Behaviour + * - Up to `maxConcurrent` operations run at the same time. + * - Up to `maxQueue` more wait in FIFO order. + * - Anything beyond that is rejected immediately with a 503 + * (HORIZON_CLIENT_QUEUE_FULL) - load shedding instead of collapse. + * - A queued operation that waits longer than `queueTimeoutMs` is rejected + * with a 503 (HORIZON_CLIENT_QUEUE_TIMEOUT) and removed from the queue. + * - release() is idempotent, so a double release can never inflate the + * number of available slots. + */ + +import { + CONCURRENCY_LIMITS, + DEFAULT_CONCURRENCY_OPTIONS, + RETRY_MANAGER_ERROR_CODES, +} from "./constants.js"; + +function clampInt(value, fallback, min, max) { + if (value === undefined || value === null || value === "") return fallback; + const n = Number(value); + if (!Number.isFinite(n)) return fallback; + return Math.min(Math.max(Math.trunc(n), min), max); +} + +/** + * Resolve limiter options. Precedence: explicit > environment > defaults. + */ +export function resolveConcurrencyOptions(options = {}, env = process.env) { + return Object.freeze({ + maxConcurrent: clampInt( + options.maxConcurrent ?? env.HORIZON_CLIENT_MAX_CONCURRENCY, + DEFAULT_CONCURRENCY_OPTIONS.maxConcurrent, + 1, + CONCURRENCY_LIMITS.maxConcurrent, + ), + maxQueue: clampInt( + options.maxQueue ?? env.HORIZON_CLIENT_MAX_QUEUE, + DEFAULT_CONCURRENCY_OPTIONS.maxQueue, + 0, + CONCURRENCY_LIMITS.maxQueue, + ), + queueTimeoutMs: clampInt( + options.queueTimeoutMs ?? env.HORIZON_CLIENT_QUEUE_TIMEOUT_MS, + DEFAULT_CONCURRENCY_OPTIONS.queueTimeoutMs, + 0, + CONCURRENCY_LIMITS.queueTimeoutMs, + ), + }); +} + +/** + * Build the error returned when the limiter sheds load. + * + * @param {string} code one of RETRY_MANAGER_ERROR_CODES + * @param {string} operation + */ +export function createBackpressureError(code, operation) { + const message = + code === RETRY_MANAGER_ERROR_CODES.QUEUE_TIMEOUT + ? "Horizon client is saturated: timed out waiting for a free request slot." + : "Horizon client is saturated: too many pending requests."; + const error = new Error(message); + error.status = 503; + error.code = code; + error.operation = operation; + // Retrying locally would only add to the queue we just rejected from. + error.retryable = false; + return error; +} + +export class ConcurrencyLimiter { + /** + * @param {object} [options] see resolveConcurrencyOptions + */ + constructor(options = {}) { + const resolved = resolveConcurrencyOptions(options); + this.maxConcurrent = resolved.maxConcurrent; + this.maxQueue = resolved.maxQueue; + this.queueTimeoutMs = resolved.queueTimeoutMs; + this.active = 0; + /** @type {Array<{resolve: Function, reject: Function, timer: any, operation: string}>} */ + this.queue = []; + this.onQueueChange = typeof options.onQueueChange === "function" ? options.onQueueChange : null; + } + + get pending() { + return this.queue.length; + } + + _notifyQueue() { + try { + this.onQueueChange?.(this.queue.length); + } catch { + // Observability must never break the request path. + } + } + + _makeRelease() { + let released = false; + return () => { + if (released) return false; + released = true; + this._releaseSlot(); + return true; + }; + } + + _releaseSlot() { + const next = this.queue.shift(); + if (next) { + // Hand the slot straight to the next waiter; `active` stays the same. + clearTimeout(next.timer); + this._notifyQueue(); + next.resolve(this._makeRelease()); + return; + } + this.active = Math.max(0, this.active - 1); + } + + /** + * Wait for a slot. + * + * @param {string} [operation] used in rejection errors + * @returns {Promise<() => boolean>} release function (idempotent) + * @throws 503 error with code HORIZON_CLIENT_QUEUE_FULL / HORIZON_CLIENT_QUEUE_TIMEOUT + */ + acquire(operation = "unknown") { + if (this.active < this.maxConcurrent) { + this.active += 1; + return Promise.resolve(this._makeRelease()); + } + + if (this.queue.length >= this.maxQueue) { + return Promise.reject( + createBackpressureError(RETRY_MANAGER_ERROR_CODES.QUEUE_FULL, operation), + ); + } + + return new Promise((resolve, reject) => { + const waiter = { resolve, reject, timer: null, operation }; + if (this.queueTimeoutMs > 0) { + waiter.timer = setTimeout(() => { + const index = this.queue.indexOf(waiter); + if (index !== -1) { + this.queue.splice(index, 1); + this._notifyQueue(); + } + reject(createBackpressureError(RETRY_MANAGER_ERROR_CODES.QUEUE_TIMEOUT, operation)); + }, this.queueTimeoutMs); + waiter.timer.unref?.(); + } + this.queue.push(waiter); + this._notifyQueue(); + }); + } + + /** + * Run `fn` while holding a slot. + * + * @template T + * @param {() => Promise} fn + * @param {string} [operation] + * @returns {Promise} + */ + async run(fn, operation) { + const release = await this.acquire(operation); + try { + return await fn(); + } finally { + release(); + } + } + + getStats() { + return { + active: this.active, + pending: this.queue.length, + maxConcurrent: this.maxConcurrent, + maxQueue: this.maxQueue, + queueTimeoutMs: this.queueTimeoutMs, + }; + } +} diff --git a/backend/src/lib/stellar/horizon-client/concurrency-limiter.test.js b/backend/src/lib/stellar/horizon-client/concurrency-limiter.test.js new file mode 100644 index 00000000..d6aa796e --- /dev/null +++ b/backend/src/lib/stellar/horizon-client/concurrency-limiter.test.js @@ -0,0 +1,149 @@ +/** + * Unit tests for the Horizon Client concurrency limiter (issue #1440). + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { + ConcurrencyLimiter, + createBackpressureError, + resolveConcurrencyOptions, +} from './concurrency-limiter.js'; +import { DEFAULT_CONCURRENCY_OPTIONS, RETRY_MANAGER_ERROR_CODES } from './constants.js'; + +const deferred = () => { + let resolve; + const promise = new Promise((r) => { resolve = r; }); + return { promise, resolve }; +}; + +afterEach(() => { + vi.useRealTimers(); +}); + +describe('resolveConcurrencyOptions', () => { + it('uses defaults, then env, then explicit options', () => { + expect(resolveConcurrencyOptions({}, {})).toEqual(DEFAULT_CONCURRENCY_OPTIONS); + expect(resolveConcurrencyOptions({}, { HORIZON_CLIENT_MAX_CONCURRENCY: '8' }).maxConcurrent).toBe(8); + expect(resolveConcurrencyOptions({ maxConcurrent: 2 }, { HORIZON_CLIENT_MAX_CONCURRENCY: '8' }).maxConcurrent).toBe(2); + }); + + it('never allows zero concurrency and rejects garbage', () => { + expect(resolveConcurrencyOptions({ maxConcurrent: 0 }, {}).maxConcurrent).toBe(1); + expect(resolveConcurrencyOptions({ maxConcurrent: 'lots' }, {}).maxConcurrent) + .toBe(DEFAULT_CONCURRENCY_OPTIONS.maxConcurrent); + expect(resolveConcurrencyOptions({ maxQueue: -5 }, {}).maxQueue).toBe(0); + }); +}); + +describe('ConcurrencyLimiter', () => { + it('runs up to maxConcurrent operations at once', async () => { + const limiter = new ConcurrencyLimiter({ maxConcurrent: 2, maxQueue: 10 }); + const gates = [deferred(), deferred(), deferred()]; + let running = 0; + let peak = 0; + + const tasks = gates.map((gate) => + limiter.run(async () => { + running += 1; + peak = Math.max(peak, running); + await gate.promise; + running -= 1; + }), + ); + + await Promise.resolve(); + expect(limiter.getStats()).toMatchObject({ active: 2, pending: 1 }); + + gates.forEach((g) => g.resolve()); + await Promise.all(tasks); + expect(peak).toBe(2); + expect(limiter.getStats()).toMatchObject({ active: 0, pending: 0 }); + }); + + it('hands slots to waiters in FIFO order', async () => { + const limiter = new ConcurrencyLimiter({ maxConcurrent: 1, maxQueue: 10 }); + const order = []; + const first = await limiter.acquire(); + const waiters = [1, 2, 3].map((n) => limiter.acquire().then((release) => { + order.push(n); + release(); + })); + first(); + await Promise.all(waiters); + expect(order).toEqual([1, 2, 3]); + }); + + it('rejects immediately with 503 when the queue is full', async () => { + const limiter = new ConcurrencyLimiter({ maxConcurrent: 1, maxQueue: 1 }); + const release = await limiter.acquire('loadAccount'); + const queued = limiter.acquire('loadAccount'); + + await expect(limiter.acquire('loadAccount')).rejects.toMatchObject({ + status: 503, + code: RETRY_MANAGER_ERROR_CODES.QUEUE_FULL, + operation: 'loadAccount', + retryable: false, + }); + + release(); + (await queued)(); + }); + + it('rejects a waiter that exceeds queueTimeoutMs and removes it from the queue', async () => { + vi.useFakeTimers(); + const onQueueChange = vi.fn(); + const limiter = new ConcurrencyLimiter({ maxConcurrent: 1, maxQueue: 5, queueTimeoutMs: 100, onQueueChange }); + const release = await limiter.acquire(); + const waiting = limiter.acquire('fetchPayments'); + const assertion = expect(waiting).rejects.toMatchObject({ + status: 503, + code: RETRY_MANAGER_ERROR_CODES.QUEUE_TIMEOUT, + }); + + await vi.advanceTimersByTimeAsync(100); + await assertion; + expect(limiter.pending).toBe(0); + expect(onQueueChange).toHaveBeenLastCalledWith(0); + + release(); + expect(limiter.getStats().active).toBe(0); + }); + + it('release is idempotent', async () => { + const limiter = new ConcurrencyLimiter({ maxConcurrent: 1, maxQueue: 0 }); + const release = await limiter.acquire(); + expect(release()).toBe(true); + expect(release()).toBe(false); + expect(limiter.getStats().active).toBe(0); + + // Still only one slot after the double release. + const again = await limiter.acquire(); + await expect(limiter.acquire()).rejects.toMatchObject({ code: RETRY_MANAGER_ERROR_CODES.QUEUE_FULL }); + again(); + }); + + it('releases the slot when the wrapped function throws', async () => { + const limiter = new ConcurrencyLimiter({ maxConcurrent: 1 }); + await expect(limiter.run(async () => { throw new Error('boom'); })).rejects.toThrow('boom'); + expect(limiter.getStats().active).toBe(0); + }); + + it('a throwing queue observer cannot break the limiter', async () => { + const limiter = new ConcurrencyLimiter({ + maxConcurrent: 1, + onQueueChange: () => { throw new Error('metrics down'); }, + }); + const release = await limiter.acquire(); + const queued = limiter.acquire(); + release(); + (await queued)(); + expect(limiter.getStats()).toMatchObject({ active: 0, pending: 0 }); + }); +}); + +describe('createBackpressureError', () => { + it('produces distinct messages per code', () => { + expect(createBackpressureError(RETRY_MANAGER_ERROR_CODES.QUEUE_FULL, 'op').message).toMatch(/too many/); + expect(createBackpressureError(RETRY_MANAGER_ERROR_CODES.QUEUE_TIMEOUT, 'op').message).toMatch(/timed out/); + }); +}); diff --git a/backend/src/lib/stellar/horizon-client/constants.js b/backend/src/lib/stellar/horizon-client/constants.js index 1c41ef73..6e4548aa 100644 --- a/backend/src/lib/stellar/horizon-client/constants.js +++ b/backend/src/lib/stellar/horizon-client/constants.js @@ -7,6 +7,82 @@ export const DEFAULT_RETRY_DELAYS_MS = Object.freeze([150, 500]); export const DEFAULT_HEALTH_TIMEOUT_MS = 2_000; +/** + * Backoff policy defaults (issue #1439). See backoff.js. + * The default strategy keeps the legacy attempt count (initial + 2 retries) + * but spreads retries with jittered exponential delays. + */ +export const BACKOFF_STRATEGIES = Object.freeze({ + EXPONENTIAL: "exponential", + FIXED: "fixed", +}); + +export const BACKOFF_JITTER_MODES = Object.freeze({ + FULL: "full", + EQUAL: "equal", + NONE: "none", +}); + +export const DEFAULT_BACKOFF_OPTIONS = Object.freeze({ + strategy: BACKOFF_STRATEGIES.EXPONENTIAL, + maxRetries: DEFAULT_RETRY_DELAYS_MS.length, + baseDelayMs: 150, + maxDelayMs: 2_000, + multiplier: 3, + jitter: BACKOFF_JITTER_MODES.FULL, + /** Total time budget for one operation including all waits. */ + maxElapsedMs: 10_000, + /** A Retry-After hint longer than this fails fast instead of waiting. */ + maxRetryAfterMs: 5_000, + fixedDelays: DEFAULT_RETRY_DELAYS_MS, +}); + +/** Hard ceilings applied to any configured value. */ +export const BACKOFF_LIMITS = Object.freeze({ + maxRetries: 10, + maxDelayMs: 30_000, + maxMultiplier: 10, + maxElapsedMs: 120_000, +}); + +/** + * Concurrency control defaults (issue #1440). See concurrency-limiter.js and + * retry-coordinator.js. + */ +export const DEFAULT_CONCURRENCY_OPTIONS = Object.freeze({ + /** Operations allowed to run against Horizon at once, per process. */ + maxConcurrent: 64, + /** Operations allowed to wait for a slot before new ones are rejected. */ + maxQueue: 1_000, + /** How long a queued operation waits before it is rejected. */ + queueTimeoutMs: 10_000, +}); + +export const DEFAULT_RETRY_COORDINATION_OPTIONS = Object.freeze({ + /** Retries allowed in flight at once per operation, across all instances. */ + maxConcurrentRetries: 32, + /** Safety lease on a retry slot; released explicitly long before this. */ + retrySlotLeaseMs: 30_000, + /** Longest shared rate-limit cooldown any instance may publish. */ + maxCooldownMs: 5_000, + keyPrefix: "horizon:retry:", +}); + +export const CONCURRENCY_LIMITS = Object.freeze({ + maxConcurrent: 1_024, + maxQueue: 100_000, + queueTimeoutMs: 120_000, + maxConcurrentRetries: 1_024, + retrySlotLeaseMs: 120_000, + maxCooldownMs: 60_000, +}); + +/** Error codes attached to errors raised by the Retry Manager itself. */ +export const RETRY_MANAGER_ERROR_CODES = Object.freeze({ + QUEUE_FULL: "HORIZON_CLIENT_QUEUE_FULL", + QUEUE_TIMEOUT: "HORIZON_CLIENT_QUEUE_TIMEOUT", +}); + export const RETRYABLE_ERROR_CODES = Object.freeze([ "ECONNABORTED", "ECONNREFUSED", diff --git a/backend/src/lib/stellar/horizon-client/error-handler.js b/backend/src/lib/stellar/horizon-client/error-handler.js index 65322d22..42e8de18 100644 --- a/backend/src/lib/stellar/horizon-client/error-handler.js +++ b/backend/src/lib/stellar/horizon-client/error-handler.js @@ -53,8 +53,8 @@ export function isRetryableError(err) { return true; } - // Retry on network errors - const code = String(err?.code || "").toUpperCase(); + // Retry on network errors. undici/fetch put the system code on `cause`. + const code = String(err?.code || err?.cause?.code || "").toUpperCase(); if (RETRYABLE_ERROR_CODES.includes(code)) { return true; } @@ -63,8 +63,14 @@ export function isRetryableError(err) { return true; } - // Retry on timeout/temporary errors in message + // Retry on timeout/temporary errors in message. The Stellar SDK rethrows + // transport failures as plain Errors ("connect ECONNREFUSED 1.2.3.4:443") + // without a `code`, so system codes are matched in the message too. const message = String(err?.message || ""); + const upperMessage = message.toUpperCase(); + if (RETRYABLE_ERROR_CODES.some((retryableCode) => upperMessage.includes(retryableCode))) { + return true; + } return RETRYABLE_MESSAGE_PATTERNS.some(pattern => new RegExp(pattern, "i").test(message) ); diff --git a/backend/src/lib/stellar/horizon-client/error-handler.test.js b/backend/src/lib/stellar/horizon-client/error-handler.test.js new file mode 100644 index 00000000..fb5a54e3 --- /dev/null +++ b/backend/src/lib/stellar/horizon-client/error-handler.test.js @@ -0,0 +1,57 @@ +/** + * Unit tests for Horizon error classification used by the Retry Manager. + */ + +import { describe, it, expect } from 'vitest'; +import { classifyError, getErrorStatus, handleError, isRetryableError } from './error-handler.js'; + +describe('isRetryableError', () => { + it.each([ + ['429', { response: { status: 429 } }], + ['408', { status: 408 }], + ['5xx', { response: { status: 503 } }], + ['a system code', { code: 'ECONNRESET' }], + ['a system code on cause (undici)', { message: 'fetch failed', cause: { code: 'UND_ERR_SOCKET' } }], + ['an SDK transport error without code', new Error('connect ECONNREFUSED 127.0.0.1:443')], + ['an abort', Object.assign(new Error('aborted'), { name: 'AbortError' })], + ['a timeout message', new Error('Request timeout')], + ])('retries %s', (_label, err) => { + expect(isRetryableError(err)).toBe(true); + }); + + it.each([ + ['404', { response: { status: 404 } }], + ['400', { response: { status: 400 } }], + ['an ordinary bug', new TypeError('x is not a function')], + ['null', null], + ])('does not retry %s', (_label, err) => { + expect(isRetryableError(err)).toBe(false); + }); +}); + +describe('classifyError / getErrorStatus', () => { + it('reads status from response or the error itself', () => { + expect(getErrorStatus({ response: { status: 418 } })).toBe(418); + expect(getErrorStatus({ status: 500 })).toBe(500); + expect(getErrorStatus({})).toBeNull(); + }); + + it('classifies common failures', () => { + expect(classifyError({ status: 429 })).toBe('rate_limit'); + expect(classifyError({ status: 404 })).toBe('not_found'); + expect(classifyError({ status: 500 })).toBe('server_error'); + expect(classifyError({ status: 400 })).toBe('client_error'); + expect(classifyError({ code: 'ECONNREFUSED' })).toBe('network_error'); + expect(classifyError({})).toBe('unknown'); + }); +}); + +describe('handleError', () => { + it('maps 5xx to 502 and keeps the operation', () => { + expect(handleError({ status: 500 }, 'loadAccount', {}, 'h')).toMatchObject({ status: 502, operation: 'loadAccount' }); + }); + + it('includes context for 404s', () => { + expect(handleError({ status: 404 }, 'loadAccount', { accountId: 'GA' }, 'h').message).toContain('accountId=GA'); + }); +}); diff --git a/backend/src/lib/stellar/horizon-client/retry-coordinator.js b/backend/src/lib/stellar/horizon-client/retry-coordinator.js new file mode 100644 index 00000000..e2bb60e4 --- /dev/null +++ b/backend/src/lib/stellar/horizon-client/retry-coordinator.js @@ -0,0 +1,323 @@ +/** + * Horizon Retry Coordinator - distributed concurrency control (issue #1440) + * + * Problem + * ------- + * Every API instance runs its own Retry Manager. When Horizon degrades (5xx, + * timeouts) or throttles (429), every pending call on every instance retries + * on its own schedule. N instances x M callers turn one Horizon hiccup into a + * retry storm that keeps Horizon throttling us. + * + * Solution + * -------- + * Two shared primitives, both keyed per Horizon host and per operation: + * + * 1. Retry slots (distributed semaphore) + * `slot:::` for i in [0, maxConcurrentRetries) + * A caller may only retry while holding a slot. Acquire is + * SET NX PX ; release is a compare-and-delete Lua + * script, so a caller can never free a slot that expired and was taken by + * someone else. When every slot is busy the retry is skipped and the + * original error is returned - load shedding instead of amplification. + * + * 2. Rate-limit cooldown + * `cooldown:` holds the epoch (ms) until which Horizon asked + * us to back off (Retry-After on a 429). Every instance waits it out + * before its next attempt instead of each discovering the 429 itself. + * + * Failure policy: FAIL OPEN to an in-process implementation of the same two + * primitives. Horizon reads are idempotent, so losing cross-instance + * coordination only costs efficiency, never correctness. A no-op/closed Redis + * client (connectRedisClient() returns one when Redis is down) is treated as + * "no Redis", because its SET would report success to every caller. + * + * Keys never contain caller input: the host is SHA-256 hashed and the + * operation name is restricted to [A-Za-z0-9_] (anything else becomes + * "unknown"), so keys cannot be injected or grow without bound. + */ + +import { createHash, randomUUID } from "node:crypto"; +import { logger } from "../../logger.js"; +import { + CONCURRENCY_LIMITS, + DEFAULT_RETRY_COORDINATION_OPTIONS, +} from "./constants.js"; + +/** Compare-and-delete: only the token holder may release the slot. */ +export const RELEASE_SLOT_SCRIPT = + 'if redis.call("get", KEYS[1]) == ARGV[1] then return redis.call("del", KEYS[1]) else return 0 end'; + +const OPERATION_NAME_PATTERN = /^[A-Za-z0-9_]{1,64}$/; + +function clampInt(value, fallback, min, max) { + if (value === undefined || value === null || value === "") return fallback; + const n = Number(value); + if (!Number.isFinite(n)) return fallback; + return Math.min(Math.max(Math.trunc(n), min), max); +} + +export function resolveCoordinationOptions(options = {}, env = process.env) { + return Object.freeze({ + maxConcurrentRetries: clampInt( + options.maxConcurrentRetries ?? env.HORIZON_RETRY_MAX_CONCURRENT_RETRIES, + DEFAULT_RETRY_COORDINATION_OPTIONS.maxConcurrentRetries, + 1, + CONCURRENCY_LIMITS.maxConcurrentRetries, + ), + retrySlotLeaseMs: clampInt( + options.retrySlotLeaseMs ?? env.HORIZON_RETRY_SLOT_LEASE_MS, + DEFAULT_RETRY_COORDINATION_OPTIONS.retrySlotLeaseMs, + 1_000, + CONCURRENCY_LIMITS.retrySlotLeaseMs, + ), + maxCooldownMs: clampInt( + options.maxCooldownMs ?? env.HORIZON_RETRY_MAX_COOLDOWN_MS, + DEFAULT_RETRY_COORDINATION_OPTIONS.maxCooldownMs, + 0, + CONCURRENCY_LIMITS.maxCooldownMs, + ), + keyPrefix: + typeof options.keyPrefix === "string" && /^[A-Za-z0-9:_-]{1,64}$/.test(options.keyPrefix) + ? options.keyPrefix + : DEFAULT_RETRY_COORDINATION_OPTIONS.keyPrefix, + }); +} + +/** Restrict operation names to a safe, bounded alphabet. */ +export function sanitizeOperationName(operation) { + return typeof operation === "string" && OPERATION_NAME_PATTERN.test(operation) + ? operation + : "unknown"; +} + +/** Stable, injection-safe identifier for a Horizon host. */ +export function hashHorizonScope(horizonUrl) { + return createHash("sha256") + .update(String(horizonUrl ?? "default")) + .digest("hex") + .slice(0, 16); +} + +function isUsableRedis(client) { + return Boolean(client) && client.isOpen !== false && typeof client.sendCommand === "function"; +} + +/** + * Process-wide Redis client used by coordinators that were not given one + * explicitly. The shared HorizonClient is created at import time, before + * Redis is connected, so it resolves the client lazily through this slot. + */ +let sharedRedisClient = null; + +/** + * Enable cross-instance Horizon retry coordination. Call once at startup with + * a connected Redis client; a missing or closed client keeps every + * coordinator on its in-process fallback. + * + * @param {{ redisClient?: object|null }} [opts] + * @returns {boolean} whether distributed coordination is active + */ +export function configureHorizonRetryCoordination({ redisClient = null } = {}) { + sharedRedisClient = isUsableRedis(redisClient) ? redisClient : null; + if (sharedRedisClient) { + logger.info("Horizon retry manager distributed coordination enabled"); + } + return Boolean(sharedRedisClient); +} + +/** Disable cross-instance coordination (test isolation / shutdown). */ +export function resetHorizonRetryCoordination() { + sharedRedisClient = null; +} + +export class HorizonRetryCoordinator { + /** + * @param {object} [options] + * @param {object|null} [options.redisClient] node-redis client exposing sendCommand + * @param {string} [options.horizonUrl] scopes keys per Horizon host + * @param {number} [options.maxConcurrentRetries] + * @param {number} [options.retrySlotLeaseMs] + * @param {number} [options.maxCooldownMs] + * @param {string} [options.keyPrefix] + * @param {() => number} [options.now] injectable clock for tests + */ + constructor({ redisClient, horizonUrl = "default", now = Date.now, ...options } = {}) { + const resolved = resolveCoordinationOptions(options); + this.maxConcurrentRetries = resolved.maxConcurrentRetries; + this.retrySlotLeaseMs = resolved.retrySlotLeaseMs; + this.maxCooldownMs = resolved.maxCooldownMs; + this.scope = hashHorizonScope(horizonUrl); + this.slotPrefix = `${resolved.keyPrefix}slot:${this.scope}:`; + this.cooldownKey = `${resolved.keyPrefix}cooldown:${this.scope}`; + // `undefined` means "use the process-wide client"; `null` forces local. + this.explicitRedis = redisClient; + this.now = now; + + /** In-process fallback state. */ + this.localSlots = new Map(); // operation -> Set + this.localCooldownUntil = 0; + this.stats = { granted: 0, denied: 0, fallbacks: 0 }; + } + + /** The Redis client to coordinate through, or null for local mode. */ + get redis() { + const client = this.explicitRedis === undefined ? sharedRedisClient : this.explicitRedis; + return isUsableRedis(client) ? client : null; + } + + /** "redis" when cross-instance coordination is active, else "local". */ + get backend() { + return this.redis ? "redis" : "local"; + } + + _fallback(err, action) { + this.stats.fallbacks += 1; + logger.warn( + { err: err?.message, action }, + "Horizon retry coordination: Redis unavailable, using in-process fallback", + ); + } + + /* ------------------------------------------------------------------ */ + /* Retry slots */ + /* ------------------------------------------------------------------ */ + + _acquireLocalSlot(operation) { + let held = this.localSlots.get(operation); + if (!held) { + held = new Set(); + this.localSlots.set(operation, held); + } + if (held.size >= this.maxConcurrentRetries) return null; + const token = randomUUID(); + held.add(token); + return { + backend: "local", + release: async () => { + const removed = held.delete(token); + if (held.size === 0) this.localSlots.delete(operation); + return removed; + }, + }; + } + + /** + * Try to reserve the right to retry `operation` once, without waiting. + * + * @param {string} operation + * @param {object} [opts] + * @param {number} [opts.leaseMs] safety TTL for the slot + * @returns {Promise Promise }>} + * null when every retry slot is taken + */ + async acquireRetrySlot(operation, { leaseMs } = {}) { + const op = sanitizeOperationName(operation); + const lease = clampInt(leaseMs, this.retrySlotLeaseMs, 1_000, CONCURRENCY_LIMITS.retrySlotLeaseMs); + + const redis = this.redis; + if (redis) { + try { + const token = randomUUID(); + // Start at a random slot so callers do not all contend on slot 0. + const offset = Math.floor(Math.random() * this.maxConcurrentRetries); + for (let i = 0; i < this.maxConcurrentRetries; i += 1) { + const key = `${this.slotPrefix}${op}:${(offset + i) % this.maxConcurrentRetries}`; + const reply = await redis.sendCommand(["SET", key, token, "NX", "PX", String(lease)]); + if (reply === "OK") { + this.stats.granted += 1; + return { + backend: "redis", + release: async () => { + try { + const released = await redis.sendCommand([ + "EVAL", RELEASE_SLOT_SCRIPT, "1", key, token, + ]); + return Number(released) === 1; + } catch (err) { + // The PX lease frees the slot on its own. + logger.warn({ err: err?.message }, "Horizon retry slot release failed"); + return false; + } + }, + }; + } + } + this.stats.denied += 1; + return null; + } catch (err) { + this._fallback(err, "acquire_retry_slot"); + } + } + + const slot = this._acquireLocalSlot(op); + this.stats[slot ? "granted" : "denied"] += 1; + return slot; + } + + /* ------------------------------------------------------------------ */ + /* Rate-limit cooldown */ + /* ------------------------------------------------------------------ */ + + /** + * Publish a cooldown so every instance waits before its next attempt. + * Only ever extends an existing cooldown, and is capped at maxCooldownMs so + * a hostile or broken Retry-After cannot stall the cluster. + * + * @param {number} durationMs + * @returns {Promise} the cooldown actually applied (ms) + */ + async recordCooldown(durationMs) { + const ms = clampInt(durationMs, 0, 0, this.maxCooldownMs); + if (ms <= 0) return 0; + const until = this.now() + ms; + this.localCooldownUntil = Math.max(this.localCooldownUntil, until); + + const redis = this.redis; + if (redis) { + try { + const current = Number(await redis.sendCommand(["GET", this.cooldownKey])); + if (!Number.isFinite(current) || current < until) { + await redis.sendCommand(["SET", this.cooldownKey, String(until), "PX", String(ms)]); + } + } catch (err) { + this._fallback(err, "record_cooldown"); + } + } + return ms; + } + + /** + * Remaining cooldown in ms (0 when none). Never larger than maxCooldownMs, + * even if Redis holds a corrupted or far-future value. + */ + async getCooldownRemainingMs() { + let until = this.localCooldownUntil; + + const redis = this.redis; + if (redis) { + try { + const raw = await redis.sendCommand(["GET", this.cooldownKey]); + const shared = Number(raw); + if (raw !== null && raw !== undefined && Number.isFinite(shared)) { + until = Math.max(until, shared); + } + } catch (err) { + this._fallback(err, "read_cooldown"); + } + } + + const remaining = until - this.now(); + if (!Number.isFinite(remaining) || remaining <= 0) return 0; + return Math.min(Math.ceil(remaining), this.maxCooldownMs); + } + + getConfig() { + return { + backend: this.backend, + maxConcurrentRetries: this.maxConcurrentRetries, + retrySlotLeaseMs: this.retrySlotLeaseMs, + maxCooldownMs: this.maxCooldownMs, + stats: { ...this.stats }, + }; + } +} diff --git a/backend/src/lib/stellar/horizon-client/retry-coordinator.test.js b/backend/src/lib/stellar/horizon-client/retry-coordinator.test.js new file mode 100644 index 00000000..6510bf25 --- /dev/null +++ b/backend/src/lib/stellar/horizon-client/retry-coordinator.test.js @@ -0,0 +1,239 @@ +/** + * Unit tests for the Horizon retry coordinator (issue #1440). + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; + +vi.mock('../../logger.js', () => ({ + logger: { warn: vi.fn(), info: vi.fn(), error: vi.fn(), debug: vi.fn() }, +})); + +import { + HorizonRetryCoordinator, + RELEASE_SLOT_SCRIPT, + configureHorizonRetryCoordination, + hashHorizonScope, + resetHorizonRetryCoordination, + resolveCoordinationOptions, + sanitizeOperationName, +} from './retry-coordinator.js'; +import { DEFAULT_RETRY_COORDINATION_OPTIONS } from './constants.js'; +import { createFakeRedis } from '../../../../tests/helpers/fake-redis.js'; +import { logger } from '../../logger.js'; + +afterEach(() => { + resetHorizonRetryCoordination(); + vi.clearAllMocks(); +}); + +describe('helpers', () => { + it('sanitizeOperationName only allows a bounded safe alphabet', () => { + expect(sanitizeOperationName('loadAccount')).toBe('loadAccount'); + expect(sanitizeOperationName('a:b')).toBe('unknown'); + expect(sanitizeOperationName('x'.repeat(65))).toBe('unknown'); + expect(sanitizeOperationName(undefined)).toBe('unknown'); + }); + + it('hashHorizonScope is stable and does not leak the URL', () => { + const a = hashHorizonScope('https://horizon.stellar.org'); + expect(a).toBe(hashHorizonScope('https://horizon.stellar.org')); + expect(a).not.toBe(hashHorizonScope('https://horizon-testnet.stellar.org')); + expect(a).toMatch(/^[0-9a-f]{16}$/); + }); + + it('resolveCoordinationOptions clamps values and rejects unsafe prefixes', () => { + expect(resolveCoordinationOptions({}, {})).toEqual(DEFAULT_RETRY_COORDINATION_OPTIONS); + const resolved = resolveCoordinationOptions({ maxConcurrentRetries: 0, keyPrefix: 'bad prefix\r\n' }, {}); + expect(resolved.maxConcurrentRetries).toBe(1); + expect(resolved.keyPrefix).toBe(DEFAULT_RETRY_COORDINATION_OPTIONS.keyPrefix); + expect(resolveCoordinationOptions({}, { HORIZON_RETRY_MAX_CONCURRENT_RETRIES: '7' }).maxConcurrentRetries).toBe(7); + }); + + it('the release script is a compare-and-delete', () => { + expect(RELEASE_SLOT_SCRIPT).toMatch(/get.*KEYS\[1\].*ARGV\[1\].*del/); + }); +}); + +describe.each([ + ['local', () => null], + ['redis', () => createFakeRedis()], +])('retry slots (%s backend)', (backend, makeRedis) => { + const make = (opts = {}) => new HorizonRetryCoordinator({ + redisClient: makeRedis(), + horizonUrl: 'https://horizon-testnet.stellar.org', + maxConcurrentRetries: 2, + ...opts, + }); + + it(`reports the ${backend} backend`, () => { + expect(make().backend).toBe(backend); + }); + + it('grants at most maxConcurrentRetries slots per operation', async () => { + const c = make(); + const a = await c.acquireRetrySlot('loadAccount'); + const b = await c.acquireRetrySlot('loadAccount'); + expect(a).not.toBeNull(); + expect(b).not.toBeNull(); + expect(await c.acquireRetrySlot('loadAccount')).toBeNull(); + // Other operations have their own budget. + expect(await c.acquireRetrySlot('getFeeStats')).not.toBeNull(); + expect(c.getConfig().stats).toMatchObject({ granted: 3, denied: 1 }); + }); + + it('frees a slot on release, and release is single-use', async () => { + const c = make({ maxConcurrentRetries: 1 }); + const slot = await c.acquireRetrySlot('loadAccount'); + expect(await slot.release()).toBe(true); + expect(await slot.release()).toBe(false); + expect(await c.acquireRetrySlot('loadAccount')).not.toBeNull(); + }); + + it('caps and only extends the cooldown', async () => { + const c = make({ maxCooldownMs: 1_000 }); + expect(await c.getCooldownRemainingMs()).toBe(0); + expect(await c.recordCooldown(60_000)).toBe(1_000); + const remaining = await c.getCooldownRemainingMs(); + expect(remaining).toBeGreaterThan(900); + expect(remaining).toBeLessThanOrEqual(1_000); + + await c.recordCooldown(10); + expect(await c.getCooldownRemainingMs()).toBeGreaterThan(900); + }); + + it('ignores zero, negative and garbage cooldowns', async () => { + const c = make(); + expect(await c.recordCooldown(0)).toBe(0); + expect(await c.recordCooldown(-50)).toBe(0); + expect(await c.recordCooldown('soon')).toBe(0); + expect(await c.getCooldownRemainingMs()).toBe(0); + }); +}); + +describe('distributed behaviour', () => { + it('shares retry slots between coordinators (instances) on the same Redis', async () => { + const redis = createFakeRedis(); + const url = 'https://horizon.stellar.org'; + const a = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: url, maxConcurrentRetries: 1 }); + const b = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: url, maxConcurrentRetries: 1 }); + + const slot = await a.acquireRetrySlot('loadAccount'); + expect(slot.backend).toBe('redis'); + expect(await b.acquireRetrySlot('loadAccount')).toBeNull(); + + await slot.release(); + expect(await b.acquireRetrySlot('loadAccount')).not.toBeNull(); + }); + + it('keeps different Horizon hosts independent', async () => { + const redis = createFakeRedis(); + const main = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: 'https://a', maxConcurrentRetries: 1 }); + const test = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: 'https://b', maxConcurrentRetries: 1 }); + expect(await main.acquireRetrySlot('op')).not.toBeNull(); + expect(await test.acquireRetrySlot('op')).not.toBeNull(); + }); + + it('shares the cooldown between instances', async () => { + const redis = createFakeRedis(); + const a = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: 'h' }); + const b = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: 'h' }); + await a.recordCooldown(2_000); + expect(await b.getCooldownRemainingMs()).toBeGreaterThan(1_500); + }); + + it('an expired lease frees the slot, and a stale holder cannot delete the new owner', async () => { + vi.useFakeTimers(); + try { + const redis = createFakeRedis(); + const a = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: 'h', maxConcurrentRetries: 1 }); + const b = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: 'h', maxConcurrentRetries: 1 }); + + const stale = await a.acquireRetrySlot('op', { leaseMs: 1_000 }); + vi.advanceTimersByTime(1_001); + const fresh = await b.acquireRetrySlot('op', { leaseMs: 5_000 }); + expect(fresh).not.toBeNull(); + + expect(await stale.release()).toBe(false); + expect(await a.acquireRetrySlot('op')).toBeNull(); + expect(await fresh.release()).toBe(true); + } finally { + vi.useRealTimers(); + } + }); + + it('never trusts a corrupted or far-future shared cooldown beyond the cap', async () => { + const redis = createFakeRedis(); + const c = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: 'h', maxCooldownMs: 500 }); + redis.store.set(c.cooldownKey, { value: String(Date.now() + 10 * 60_000), expiresAt: null }); + expect(await c.getCooldownRemainingMs()).toBe(500); + + redis.store.set(c.cooldownKey, { value: 'not-a-number', expiresAt: null }); + expect(await c.getCooldownRemainingMs()).toBe(0); + }); + + it('only writes namespaced keys built from hashed host and sanitized operation', async () => { + const redis = createFakeRedis(); + const c = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: 'https://h' }); + await c.acquireRetrySlot('evil key\r\nFLUSHALL'); + await c.recordCooldown(100); + for (const key of redis.store.keys()) { + expect(key).toMatch(/^horizon:retry:(slot|cooldown):[0-9a-f]{16}(:unknown:\d+)?$/); + } + }); +}); + +describe('fail-open behaviour', () => { + it('falls back to local slots when Redis errors', async () => { + const redis = createFakeRedis(); + redis.setFailure(new Error('ECONNRESET')); + const c = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: 'h', maxConcurrentRetries: 1 }); + + const slot = await c.acquireRetrySlot('op'); + expect(slot.backend).toBe('local'); + expect(await c.acquireRetrySlot('op')).toBeNull(); + expect(c.getConfig().stats.fallbacks).toBe(2); + expect(logger.warn).toHaveBeenCalled(); + }); + + it('keeps a local cooldown when Redis errors', async () => { + const redis = createFakeRedis(); + const c = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: 'h' }); + redis.setFailure(new Error('down')); + await c.recordCooldown(1_000); + expect(await c.getCooldownRemainingMs()).toBeGreaterThan(0); + }); + + it('treats a closed or no-op client as local', () => { + expect(new HorizonRetryCoordinator({ redisClient: { isOpen: false, sendCommand: vi.fn() } }).backend).toBe('local'); + expect(new HorizonRetryCoordinator({ redisClient: { isOpen: true } }).backend).toBe('local'); + }); + + it('a slot release error is swallowed (the lease expires on its own)', async () => { + const redis = createFakeRedis(); + const c = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: 'h' }); + const slot = await c.acquireRetrySlot('op'); + redis.setFailure(new Error('gone')); + await expect(slot.release()).resolves.toBe(false); + }); +}); + +describe('configureHorizonRetryCoordination', () => { + it('upgrades coordinators that were created without an explicit client', async () => { + const lazy = new HorizonRetryCoordinator({ horizonUrl: 'h' }); + const forcedLocal = new HorizonRetryCoordinator({ redisClient: null, horizonUrl: 'h' }); + expect(lazy.backend).toBe('local'); + + const redis = createFakeRedis(); + expect(configureHorizonRetryCoordination({ redisClient: redis })).toBe(true); + expect(lazy.backend).toBe('redis'); + expect(forcedLocal.backend).toBe('local'); + + await lazy.acquireRetrySlot('op'); + expect(redis.count('SET')).toBe(1); + }); + + it('ignores missing or closed clients', () => { + expect(configureHorizonRetryCoordination()).toBe(false); + expect(configureHorizonRetryCoordination({ redisClient: { isOpen: false, sendCommand: vi.fn() } })).toBe(false); + }); +}); diff --git a/backend/src/lib/stellar/horizon-client/retry-manager.js b/backend/src/lib/stellar/horizon-client/retry-manager.js index 68ff14da..f2570efe 100644 --- a/backend/src/lib/stellar/horizon-client/retry-manager.js +++ b/backend/src/lib/stellar/horizon-client/retry-manager.js @@ -1,7 +1,28 @@ /** * Horizon Client Retry Manager - * - * Handles retry logic with exponential backoff and configurable delays + * + * Runs Horizon reads with automated retries. + * + * Retry policy (issue #1439) + * - Jittered exponential backoff by default (see backoff.js). An explicit + * `retryDelays` array keeps the legacy fixed schedule. + * - Retry-After from Horizon is honoured: the wait is at least the hint, and + * a hint above maxRetryAfterMs fails fast instead of pinning the caller. + * - A total time budget (maxElapsedMs) bounds every operation, including + * all waits, so retries can never stretch a request indefinitely. + * - Only transient failures are retried (isRetryableError). Errors flagged + * `retryable: false` are never retried. + * + * Concurrency control (issue #1440) + * - ConcurrencyLimiter: per-process cap on in-flight Horizon operations with + * a bounded queue; overflow is rejected with 503 instead of piling up. + * - HorizonRetryCoordinator: cluster-wide retry slots (Redis SET NX PX + + * compare-and-delete release) and a shared Retry-After cooldown, so N + * instances do not multiply a Horizon outage into a retry storm. Falls + * back to an in-process implementation when Redis is unavailable. + * + * All operations wrapped here are idempotent reads, so retrying them cannot + * duplicate a payment. */ import { logger } from "../../logger.js"; @@ -14,107 +35,280 @@ import { horizonClientRetryAttemptDuration, horizonClientErrorRecovery, horizonClientConcurrentOperations, + horizonClientBackpressure, + horizonClientQueueDepth, + horizonClientRateLimitWaitTime, } from "../../metrics.js"; -import { classifyError, handleError, isRetryableError } from "./error-handler.js"; +import { classifyError, getErrorStatus, handleError, isRetryableError } from "./error-handler.js"; +import { computeBackoffDelay, parseRetryAfterMs, resolveBackoffOptions } from "./backoff.js"; +import { ConcurrencyLimiter } from "./concurrency-limiter.js"; +import { HorizonRetryCoordinator } from "./retry-coordinator.js"; + +/** Why the manager stopped retrying - attached to the thrown error. */ +export const RETRY_STOP_REASONS = Object.freeze({ + NOT_RETRYABLE: "not_retryable", + EXHAUSTED: "exhausted", + BUDGET_EXCEEDED: "budget_exceeded", + RETRY_AFTER_TOO_LONG: "retry_after_too_long", + RETRY_SLOTS_EXHAUSTED: "retry_slots_exhausted", +}); + +/** + * Metrics must never break the request path. Some test suites mock + * metrics.js with only a subset of exports, and a prom-client label mismatch + * throws synchronously; both cases are swallowed here. + */ +function recordMetric(fn) { + try { + fn(); + } catch { + // ignore + } +} + +const defaultSleep = (ms) => + new Promise((resolve) => { + if (ms <= 0) { + resolve(); + return; + } + const timer = setTimeout(resolve, ms); + timer.unref?.(); + }); /** * Retry manager class with configurable retry strategy */ export class RetryManager { - constructor(retryDelays, metricsPrefix = "") { - this.retryDelays = retryDelays; - this.metricsPrefix = metricsPrefix; + /** + * @param {number[]|undefined} retryDelays legacy fixed schedule; omit for exponential backoff + * @param {object} [options] + * @param {object} [options.backoff] see resolveBackoffOptions + * @param {object} [options.concurrency] see resolveConcurrencyOptions + * @param {object} [options.coordination] see HorizonRetryCoordinator + * @param {HorizonRetryCoordinator} [options.coordinator] + * @param {ConcurrencyLimiter} [options.limiter] + * @param {string} [options.horizonUrl] + * @param {(ms:number) => Promise} [options.sleep] injectable for tests + * @param {() => number} [options.random] injectable for tests + * @param {string} [options.metricsPrefix] kept for compatibility + */ + constructor(retryDelays, options = {}) { + // Backwards compatibility: the second positional argument used to be + // `metricsPrefix`. + const opts = typeof options === "string" ? { metricsPrefix: options } : options ?? {}; + + this.policy = resolveBackoffOptions({ ...(opts.backoff ?? {}), retryDelays }); + this.retryDelays = this.policy.fixedDelays ?? retryDelays ?? []; + this.metricsPrefix = opts.metricsPrefix ?? ""; + this.horizonUrl = opts.horizonUrl ?? "default"; + this.sleep = typeof opts.sleep === "function" ? opts.sleep : defaultSleep; + this.random = typeof opts.random === "function" ? opts.random : Math.random; + + this.limiter = + opts.limiter ?? + new ConcurrencyLimiter({ + ...(opts.concurrency ?? {}), + onQueueChange: (depth) => recordMetric(() => horizonClientQueueDepth.set(depth)), + }); + this.coordinator = + opts.coordinator ?? + new HorizonRetryCoordinator({ ...(opts.coordination ?? {}), horizonUrl: this.horizonUrl }); + this.concurrentOperations = 0; } + /** + * Swap the retry coordinator (e.g. once Redis is connected at startup). + * In-flight operations keep the coordinator they started with. + */ + setCoordinator(coordinator) { + if (!(coordinator instanceof HorizonRetryCoordinator)) { + throw new TypeError("setCoordinator expects a HorizonRetryCoordinator"); + } + this.coordinator = coordinator; + } + /** * Execute operation with automatic retry logic */ async executeWithRetry(operation, operationName, context = {}, horizonUrl) { + let releaseSlot; + try { + releaseSlot = await this.limiter.acquire(operationName); + } catch (err) { + // Load shed before reaching Horizon. + recordMetric(() => horizonClientBackpressure.inc({ operation: operationName })); + this._recordError(operationName, err); + logger.warn( + { operation: operationName, code: err?.code, ...this._safeContext(context) }, + "Horizon client: request rejected by concurrency limiter", + ); + throw err; + } + const totalStartTime = Date.now(); + const deadline = totalStartTime + this.policy.maxElapsedMs; + const coordinator = this.coordinator; this.concurrentOperations++; - horizonClientConcurrentOperations.inc(); + recordMetric(() => horizonClientConcurrentOperations.inc()); + let retryPermit = null; try { let lastError = null; - for (let attempt = 0; attempt <= this.retryDelays.length; attempt += 1) { + for (let attempt = 0; attempt <= this.policy.maxRetries; attempt += 1) { + await this._waitForCooldown(coordinator, operationName, deadline); + const attemptStartTime = Date.now(); - try { const result = await operation(); - - // Record attempt duration - const attemptDuration = (Date.now() - attemptStartTime) / 1000; - horizonClientRetryAttemptDuration.observe( - { operation: operationName, attempt: attempt + 1 }, - attemptDuration - ); - + this._observeAttempt(operationName, attempt, attemptStartTime); this._recordSuccess(operationName, totalStartTime); - - // If this was a retry (attempt > 0), record successful recovery + if (attempt > 0) { - horizonClientErrorRecovery.inc({ - operation: operationName, - error_type: classifyError(lastError), - recovery_attempt: attempt, - }); + recordMetric(() => + horizonClientErrorRecovery.inc({ + operation: operationName, + error_type: classifyError(lastError), + recovery_attempt: attempt, + }), + ); } - return result; } catch (err) { lastError = err; - - // Record attempt duration even for failed attempts - const attemptDuration = (Date.now() - attemptStartTime) / 1000; - horizonClientRetryAttemptDuration.observe( - { operation: operationName, attempt: attempt + 1 }, - attemptDuration - ); - - if (!isRetryableError(err) || attempt === this.retryDelays.length) { - this._recordError(operationName, err); - throw handleError(err, operationName, context, horizonUrl); + this._observeAttempt(operationName, attempt, attemptStartTime); + } finally { + if (retryPermit) { + await retryPermit.release(); + retryPermit = null; } + } - this._recordRetry(operationName); - - const delay = this.retryDelays[attempt]; - this._logRetry(operationName, attempt + 1, delay, err, context); - - await this._sleep(delay); + const decision = await this._planRetry(lastError, attempt, deadline, coordinator, operationName); + if (decision.stop) { + this._recordError(operationName, lastError); + this._logGiveUp(operationName, attempt + 1, decision.reason, lastError, context); + throw this._finalError(lastError, operationName, context, horizonUrl, attempt + 1, decision.reason); } + + retryPermit = decision.permit; + this._recordRetry(operationName); + this._logRetry(operationName, attempt + 1, decision.delayMs, lastError, context); + await this.sleep(decision.delayMs); } - // Should not reach here, but for completeness - throw handleError(lastError, operationName, context, horizonUrl); + // Unreachable: the loop either returns or throws. + throw this._finalError( + lastError, + operationName, + context, + horizonUrl, + this.policy.maxRetries + 1, + RETRY_STOP_REASONS.EXHAUSTED, + ); } finally { + if (retryPermit) await retryPermit.release(); + releaseSlot(); this.concurrentOperations--; - horizonClientConcurrentOperations.dec(); - - // Record total operation duration + recordMetric(() => horizonClientConcurrentOperations.dec()); + const totalDuration = (Date.now() - totalStartTime) / 1000; - horizonClientOperationDuration.observe( - { operation: operationName, phase: "total" }, - totalDuration + recordMetric(() => + horizonClientOperationDuration.observe({ operation: operationName, phase: "total" }, totalDuration), ); } } + /** + * Decide whether (and how long) to wait before the next attempt. + * @private + * @returns {Promise<{stop: true, reason: string} | {stop: false, delayMs: number, permit: object}>} + */ + async _planRetry(err, attempt, deadline, coordinator, operationName) { + if (err?.retryable === false || !isRetryableError(err)) { + return { stop: true, reason: RETRY_STOP_REASONS.NOT_RETRYABLE }; + } + if (attempt >= this.policy.maxRetries) { + return { stop: true, reason: RETRY_STOP_REASONS.EXHAUSTED }; + } + + const status = getErrorStatus(err); + const retryAfterMs = parseRetryAfterMs(err); + + if (status === 429 || status === 503) { + // Share the throttle signal with every instance, even if we stop here. + await coordinator.recordCooldown(retryAfterMs ?? computeBackoffDelay(attempt, this.policy, () => 1)); + } + + if (retryAfterMs !== null && retryAfterMs > this.policy.maxRetryAfterMs) { + return { stop: true, reason: RETRY_STOP_REASONS.RETRY_AFTER_TOO_LONG }; + } + + const delayMs = Math.max(computeBackoffDelay(attempt, this.policy, this.random), retryAfterMs ?? 0); + if (Date.now() + delayMs >= deadline) { + return { stop: true, reason: RETRY_STOP_REASONS.BUDGET_EXCEEDED }; + } + + const permit = await coordinator.acquireRetrySlot(operationName, { + leaseMs: delayMs + Math.max(deadline - Date.now(), 0), + }); + if (!permit) { + recordMetric(() => horizonClientBackpressure.inc({ operation: operationName })); + return { stop: true, reason: RETRY_STOP_REASONS.RETRY_SLOTS_EXHAUSTED }; + } + + return { stop: false, delayMs, permit }; + } + + /** + * Wait out a shared rate-limit cooldown, never past the operation deadline. + * @private + */ + async _waitForCooldown(coordinator, operationName, deadline) { + let remaining = 0; + try { + remaining = await coordinator.getCooldownRemainingMs(); + } catch { + return; + } + const waitMs = Math.min(remaining, Math.max(deadline - Date.now(), 0)); + if (waitMs <= 0) return; + recordMetric(() => horizonClientRateLimitWaitTime.observe(waitMs / 1000)); + logger.debug?.({ operation: operationName, waitMs }, "Horizon client: waiting for shared rate-limit cooldown"); + await this.sleep(waitMs); + } + + /** @private */ + _finalError(err, operationName, context, horizonUrl, attempts, reason) { + const wrapped = handleError(err ?? new Error("Unknown Horizon error"), operationName, context, horizonUrl); + wrapped.attempts = attempts; + wrapped.retryStopReason = reason; + if (err?.code && !wrapped.code) wrapped.code = err.code; + return wrapped; + } + + /** @private */ + _observeAttempt(operationName, attempt, attemptStartTime) { + const attemptDuration = (Date.now() - attemptStartTime) / 1000; + recordMetric(() => + horizonClientRetryAttemptDuration.observe({ operation: operationName, attempt: attempt + 1 }, attemptDuration), + ); + } + /** * Record successful operation metrics * @private */ _recordSuccess(operationName, startTime) { - horizonClientOperations.inc({ - operation: operationName, - result: "success" - }); - horizonClientLatency.observe({ - operation: operationName, - result: "success" - }, (Date.now() - startTime) / 1000); + recordMetric(() => horizonClientOperations.inc({ operation: operationName, result: "success" })); + recordMetric(() => + horizonClientLatency.observe( + { operation: operationName, result: "success" }, + (Date.now() - startTime) / 1000, + ), + ); } /** @@ -122,14 +316,8 @@ export class RetryManager { * @private */ _recordError(operationName, err) { - horizonClientOperations.inc({ - operation: operationName, - result: "error" - }); - horizonClientErrors.inc({ - operation: operationName, - error_type: classifyError(err) - }); + recordMetric(() => horizonClientOperations.inc({ operation: operationName, result: "error" })); + recordMetric(() => horizonClientErrors.inc({ operation: operationName, error_type: classifyError(err) })); } /** @@ -137,7 +325,22 @@ export class RetryManager { * @private */ _recordRetry(operationName) { - horizonClientRetries.inc({ operation: operationName }); + recordMetric(() => horizonClientRetries.inc({ operation: operationName })); + } + + /** + * Only primitive context values reach the logs; nested objects are dropped + * so request payloads cannot leak through retry logging. + * @private + */ + _safeContext(context) { + const safe = {}; + for (const [key, value] of Object.entries(context ?? {})) { + if (value === null || ["string", "number", "boolean"].includes(typeof value)) { + safe[key] = value; + } + } + return safe; } /** @@ -146,23 +349,40 @@ export class RetryManager { */ _logRetry(operationName, attempt, delayMs, error, context) { logger.warn( - { - operation: operationName, - attempt, + { + operation: operationName, + attempt, delayMs, - error: error.message, - ...context + status: getErrorStatus(error), + error: error?.message, + ...this._safeContext(context), + }, + "Horizon client: retrying operation after error", + ); + } + + /** @private */ + _logGiveUp(operationName, attempts, reason, error, context) { + if (reason === RETRY_STOP_REASONS.NOT_RETRYABLE) return; + logger.warn( + { + operation: operationName, + attempts, + reason, + status: getErrorStatus(error), + error: error?.message, + ...this._safeContext(context), }, - "Horizon client: retrying operation after error" + "Horizon client: giving up on operation", ); } /** - * Sleep helper + * Sleep helper (kept for compatibility with callers that stub it) * @private */ _sleep(ms) { - return new Promise((resolve) => setTimeout(resolve, ms)); + return this.sleep(ms); } /** @@ -171,8 +391,11 @@ export class RetryManager { getConfig() { return { retryDelays: this.retryDelays, - maxAttempts: this.retryDelays.length + 1, + maxAttempts: this.policy.maxRetries + 1, currentConcurrentOperations: this.concurrentOperations, + backoff: { ...this.policy }, + concurrency: this.limiter.getStats(), + coordination: this.coordinator.getConfig(), }; } } @@ -180,6 +403,6 @@ export class RetryManager { /** * Create a retry manager with default configuration */ -export function createRetryManager(retryDelays) { - return new RetryManager(retryDelays); +export function createRetryManager(retryDelays, options = {}) { + return new RetryManager(retryDelays, options); } diff --git a/backend/src/lib/stellar/horizon-client/retry-manager.test.js b/backend/src/lib/stellar/horizon-client/retry-manager.test.js new file mode 100644 index 00000000..48efbb2b --- /dev/null +++ b/backend/src/lib/stellar/horizon-client/retry-manager.test.js @@ -0,0 +1,257 @@ +/** + * Unit tests for the Horizon Client Retry Manager: + * - automated retry with exponential backoff (issue #1439) + * - concurrency control and distributed retry coordination (issue #1440) + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('../../logger.js', () => ({ + logger: { warn: vi.fn(), info: vi.fn(), error: vi.fn(), debug: vi.fn() }, +})); + +import { RetryManager, RETRY_STOP_REASONS, createRetryManager } from './retry-manager.js'; +import { HorizonRetryCoordinator } from './retry-coordinator.js'; +import { ConcurrencyLimiter } from './concurrency-limiter.js'; +import { RETRY_MANAGER_ERROR_CODES } from './constants.js'; +import { createFakeRedis } from '../../../../tests/helpers/fake-redis.js'; +import { logger } from '../../logger.js'; + +const URL = 'https://horizon-testnet.stellar.org'; + +const httpError = (status, extra = {}) => Object.assign(new Error(`HTTP ${status}`), { response: { status, ...extra } }); +const netError = (code = 'ECONNRESET') => Object.assign(new Error('socket hang up'), { code }); + +/** Manager with recorded, instant sleeps and deterministic jitter. */ +function makeManager({ backoff = {}, retryDelays, coordinator, limiter, random = () => 1 } = {}) { + const sleeps = []; + const manager = new RetryManager(retryDelays, { + horizonUrl: URL, + backoff: { baseDelayMs: 100, maxDelayMs: 1_000, multiplier: 2, jitter: 'none', ...backoff }, + coordinator: coordinator ?? new HorizonRetryCoordinator({ redisClient: null, horizonUrl: URL }), + limiter, + random, + sleep: async (ms) => { sleeps.push(ms); }, + }); + return { manager, sleeps }; +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('exponential backoff (issue #1439)', () => { + it('returns immediately on success without sleeping', async () => { + const { manager, sleeps } = makeManager(); + const op = vi.fn().mockResolvedValue('ok'); + await expect(manager.executeWithRetry(op, 'loadAccount', {}, URL)).resolves.toBe('ok'); + expect(op).toHaveBeenCalledTimes(1); + expect(sleeps).toEqual([]); + }); + + it('retries transient failures with exponentially growing delays', async () => { + const { manager, sleeps } = makeManager({ backoff: { maxRetries: 3 } }); + const op = vi.fn() + .mockRejectedValueOnce(httpError(500)) + .mockRejectedValueOnce(netError()) + .mockRejectedValueOnce(httpError(502)) + .mockResolvedValue('ok'); + + await expect(manager.executeWithRetry(op, 'loadAccount', {}, URL)).resolves.toBe('ok'); + expect(op).toHaveBeenCalledTimes(4); + expect(sleeps).toEqual([100, 200, 400]); + }); + + it('applies jitter from the injected random source', async () => { + const { manager, sleeps } = makeManager({ backoff: { jitter: 'full' }, random: () => 0.5 }); + const op = vi.fn().mockRejectedValueOnce(httpError(503)).mockResolvedValue('ok'); + await manager.executeWithRetry(op, 'getFeeStats', {}, URL); + // 503 also publishes a cooldown, which is waited out before the next attempt. + expect(sleeps).toContain(50); + }); + + it('throws a wrapped error after exhausting retries', async () => { + const { manager, sleeps } = makeManager({ backoff: { maxRetries: 2 } }); + const op = vi.fn().mockRejectedValue(httpError(500)); + + const err = await manager.executeWithRetry(op, 'loadAccount', {}, URL).catch((e) => e); + expect(op).toHaveBeenCalledTimes(3); + expect(sleeps).toEqual([100, 200]); + expect(err).toMatchObject({ + status: 502, + operation: 'loadAccount', + attempts: 3, + retryStopReason: RETRY_STOP_REASONS.EXHAUSTED, + }); + expect(logger.warn).toHaveBeenCalledWith( + expect.objectContaining({ reason: RETRY_STOP_REASONS.EXHAUSTED }), + 'Horizon client: giving up on operation', + ); + }); + + it.each([ + ['404', httpError(404), 404], + ['400', httpError(400), 400], + ['an error flagged retryable:false', Object.assign(httpError(500), { retryable: false }), 502], + ])('does not retry %s', async (_label, error, status) => { + const { manager, sleeps } = makeManager(); + const op = vi.fn().mockRejectedValue(error); + const err = await manager.executeWithRetry(op, 'loadAccount', { accountId: 'GABC' }, URL).catch((e) => e); + expect(op).toHaveBeenCalledTimes(1); + expect(sleeps).toEqual([]); + expect(err.status).toBe(status); + expect(err.retryStopReason).toBe(RETRY_STOP_REASONS.NOT_RETRYABLE); + }); + + it('keeps the legacy fixed schedule when retryDelays is given', async () => { + const { manager, sleeps } = makeManager({ retryDelays: [10, 20] }); + const op = vi.fn().mockRejectedValue(netError('ETIMEDOUT')); + await expect(manager.executeWithRetry(op, 'loadAccount', {}, URL)).rejects.toMatchObject({ status: 502 }); + expect(op).toHaveBeenCalledTimes(3); + expect(sleeps).toEqual([10, 20]); + expect(manager.getConfig()).toMatchObject({ retryDelays: [10, 20], maxAttempts: 3 }); + }); + + it('waits at least the Retry-After hint on a 429', async () => { + const { manager, sleeps } = makeManager(); + const throttled = httpError(429, { headers: { 'retry-after': '0.3' } }); + const op = vi.fn().mockRejectedValueOnce(throttled).mockResolvedValue('ok'); + + await manager.executeWithRetry(op, 'fetchPayments', {}, URL); + // backoff (100) < Retry-After (300): the hint wins. + expect(sleeps[0]).toBe(300); + }); + + it('fails fast when Retry-After exceeds maxRetryAfterMs', async () => { + const { manager, sleeps } = makeManager({ backoff: { maxRetryAfterMs: 1_000 } }); + const op = vi.fn().mockRejectedValue(httpError(429, { headers: { 'retry-after': '120' } })); + + const err = await manager.executeWithRetry(op, 'loadAccount', {}, URL).catch((e) => e); + expect(op).toHaveBeenCalledTimes(1); + expect(sleeps).toEqual([]); + expect(err).toMatchObject({ status: 429, retryStopReason: RETRY_STOP_REASONS.RETRY_AFTER_TOO_LONG }); + }); + + it('stops when the next wait would exceed the total time budget', async () => { + const { manager } = makeManager({ backoff: { maxElapsedMs: 150, maxRetries: 5 } }); + const op = vi.fn().mockRejectedValue(httpError(500)); + + const err = await manager.executeWithRetry(op, 'loadAccount', {}, URL).catch((e) => e); + // delay(0)=100 fits, delay(1)=200 does not. + expect(op).toHaveBeenCalledTimes(2); + expect(err.retryStopReason).toBe(RETRY_STOP_REASONS.BUDGET_EXCEEDED); + }); + + it('logs only primitive context values', async () => { + const { manager } = makeManager(); + const op = vi.fn().mockRejectedValueOnce(httpError(500)).mockResolvedValue('ok'); + await manager.executeWithRetry(op, 'fetchPayments', { accountId: 'GABC', options: { secret: 'x' } }, URL); + const [fields] = logger.warn.mock.calls.find(([, msg]) => msg === 'Horizon client: retrying operation after error'); + expect(fields.accountId).toBe('GABC'); + expect(fields).not.toHaveProperty('options'); + }); + + it('accepts the legacy (retryDelays, metricsPrefix) signature', () => { + const manager = new RetryManager([5], 'prefix_'); + expect(manager.metricsPrefix).toBe('prefix_'); + expect(manager.getConfig().maxAttempts).toBe(2); + expect(createRetryManager().getConfig().backoff.strategy).toBe('exponential'); + }); +}); + +describe('concurrency control (issue #1440)', () => { + it('limits concurrent operations and sheds load beyond the queue with 503', async () => { + const limiter = new ConcurrencyLimiter({ maxConcurrent: 1, maxQueue: 1 }); + const { manager } = makeManager({ limiter }); + let release; + const slow = () => new Promise((resolve) => { release = () => resolve('slow'); }); + + const first = manager.executeWithRetry(slow, 'loadAccount', {}, URL); + const second = manager.executeWithRetry(async () => 'queued', 'loadAccount', {}, URL); + await expect(manager.executeWithRetry(async () => 'never', 'loadAccount', {}, URL)) + .rejects.toMatchObject({ status: 503, code: RETRY_MANAGER_ERROR_CODES.QUEUE_FULL }); + + await vi.waitFor(() => expect(release).toBeTypeOf('function')); + release(); + await expect(first).resolves.toBe('slow'); + await expect(second).resolves.toBe('queued'); + expect(manager.getConfig().concurrency).toMatchObject({ active: 0, pending: 0 }); + }); + + it('skips a retry when every cluster retry slot is taken', async () => { + const coordinator = new HorizonRetryCoordinator({ redisClient: null, horizonUrl: URL, maxConcurrentRetries: 1 }); + const held = await coordinator.acquireRetrySlot('loadAccount'); + const { manager } = makeManager({ coordinator }); + const op = vi.fn().mockRejectedValue(httpError(500)); + + const err = await manager.executeWithRetry(op, 'loadAccount', {}, URL).catch((e) => e); + expect(op).toHaveBeenCalledTimes(1); + expect(err.retryStopReason).toBe(RETRY_STOP_REASONS.RETRY_SLOTS_EXHAUSTED); + await held.release(); + }); + + it('releases retry slots after recovering and after giving up', async () => { + const coordinator = new HorizonRetryCoordinator({ redisClient: null, horizonUrl: URL, maxConcurrentRetries: 1 }); + const { manager } = makeManager({ coordinator }); + + await manager.executeWithRetry( + vi.fn().mockRejectedValueOnce(httpError(500)).mockResolvedValue('ok'), + 'loadAccount', {}, URL, + ); + await manager.executeWithRetry(vi.fn().mockRejectedValue(httpError(500)), 'loadAccount', {}, URL).catch(() => {}); + + expect(coordinator.localSlots.size).toBe(0); + expect(await coordinator.acquireRetrySlot('loadAccount')).not.toBeNull(); + }); + + it('publishes a cooldown on 429 that a second manager waits out before calling Horizon', async () => { + const redis = createFakeRedis(); + const coordA = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: URL }); + const coordB = new HorizonRetryCoordinator({ redisClient: redis, horizonUrl: URL }); + const a = makeManager({ coordinator: coordA }); + const b = makeManager({ coordinator: coordB }); + + await a.manager.executeWithRetry( + vi.fn().mockRejectedValueOnce(httpError(429, { headers: { 'retry-after': '1' } })).mockResolvedValue('ok'), + 'loadAccount', {}, URL, + ); + + const op = vi.fn().mockResolvedValue('ok'); + await b.manager.executeWithRetry(op, 'loadAccount', {}, URL); + expect(op).toHaveBeenCalledTimes(1); + expect(b.sleeps[0]).toBeGreaterThan(500); + expect(b.sleeps[0]).toBeLessThanOrEqual(1_000); + }); + + it('never waits on a cooldown past the operation budget', async () => { + const coordinator = new HorizonRetryCoordinator({ redisClient: null, horizonUrl: URL, maxCooldownMs: 5_000 }); + await coordinator.recordCooldown(5_000); + const { manager, sleeps } = makeManager({ coordinator, backoff: { maxElapsedMs: 200 } }); + await manager.executeWithRetry(async () => 'ok', 'loadAccount', {}, URL); + expect(sleeps[0]).toBeLessThanOrEqual(200); + }); + + it('keeps working when the coordinator fails', async () => { + const coordinator = new HorizonRetryCoordinator({ redisClient: null, horizonUrl: URL }); + coordinator.getCooldownRemainingMs = vi.fn().mockRejectedValue(new Error('broken')); + const { manager } = makeManager({ coordinator }); + await expect(manager.executeWithRetry(async () => 'ok', 'loadAccount', {}, URL)).resolves.toBe('ok'); + }); + + it('setCoordinator swaps the coordinator and rejects other objects', () => { + const { manager } = makeManager(); + const next = new HorizonRetryCoordinator({ redisClient: createFakeRedis(), horizonUrl: URL }); + manager.setCoordinator(next); + expect(manager.getConfig().coordination.backend).toBe('redis'); + expect(() => manager.setCoordinator({})).toThrow(TypeError); + }); + + it('tracks concurrent operations and never leaks the counter', async () => { + const { manager } = makeManager(); + await Promise.allSettled([ + manager.executeWithRetry(async () => 'ok', 'a', {}, URL), + manager.executeWithRetry(async () => { throw httpError(404); }, 'b', {}, URL), + ]); + expect(manager.getConfig().currentConcurrentOperations).toBe(0); + }); +}); diff --git a/backend/src/routes/payments-path-quote.test.js b/backend/src/routes/payments-path-quote.test.js index dc693686..96102011 100644 --- a/backend/src/routes/payments-path-quote.test.js +++ b/backend/src/routes/payments-path-quote.test.js @@ -87,7 +87,7 @@ describe("GET /api/path-payment-quote/:id — authentication (issue #1309)", () describe("GET /api/path-payment-quote/:id", () => { const paymentId = "9f927a2c-02d4-4f76-914c-62cf44d9525e"; const sourceAccount = - "GBRPYHIL2C7Q7PGLUKSTPIY2KPJ7QMZ4ZWJHQ6GUSIW2LQAHOMK5N7BI"; + "GDESJRNS3IREXNSFMC35XM2CHEESOKAWQL2SXBK4AUIKNKCIYBMW4TDJ"; beforeEach(() => { vi.clearAllMocks(); @@ -100,7 +100,7 @@ describe("GET /api/path-payment-quote/:id", () => { id: paymentId, amount: 25, asset: "USDC", - asset_issuer: "GDQOE23W4QK6WQ4R3BVCUO3PRA4VJ7A3M7MRWWX4V67WJYQ7QXKJQ4KJ", + asset_issuer: "GA5HBVNVS6P3QJ2ZGLMEACPSGFG3KDCAFTVHNJXWG22E5DVZTPSSAWQW", recipient: "GB6REFUNDTESTRECIPIENTQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQ", status: "pending", }), @@ -136,7 +136,7 @@ describe("GET /api/path-payment-quote/:id", () => { source_amount: "60.1250000", send_max: "60.7262500", destination_asset: "USDC", - destination_asset_issuer: "GDQOE23W4QK6WQ4R3BVCUO3PRA4VJ7A3M7MRWWX4V67WJYQ7QXKJQ4KJ", + destination_asset_issuer: "GA5HBVNVS6P3QJ2ZGLMEACPSGFG3KDCAFTVHNJXWG22E5DVZTPSSAWQW", destination_amount: "25", path: [], slippage: 0.01, @@ -144,7 +144,7 @@ describe("GET /api/path-payment-quote/:id", () => { expect(findStrictReceivePaths).toHaveBeenCalledWith({ sourceAccount, destAssetCode: "USDC", - destAssetIssuer: "GDQOE23W4QK6WQ4R3BVCUO3PRA4VJ7A3M7MRWWX4V67WJYQ7QXKJQ4KJ", + destAssetIssuer: "GA5HBVNVS6P3QJ2ZGLMEACPSGFG3KDCAFTVHNJXWG22E5DVZTPSSAWQW", destAmount: "25", sourceAssetCode: "XLM", sourceAssetIssuer: null, @@ -190,7 +190,7 @@ describe("GET /api/path-payment-quote/:id", () => { id: paymentId, amount: 10, asset: "USDC", - asset_issuer: "GDQOE23W4QK6WQ4R3BVCUO3PRA4VJ7A3M7MRWWX4V67WJYQ7QXKJQ4KJ", + asset_issuer: "GA5HBVNVS6P3QJ2ZGLMEACPSGFG3KDCAFTVHNJXWG22E5DVZTPSSAWQW", recipient: "GB6REFUNDTESTRECIPIENTQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQ", status: "confirmed", }), @@ -224,7 +224,7 @@ describe("GET /api/path-payment-quote/:id", () => { describe("GET /api/path-payment-quote/:id — quote cache (issue #1045)", () => { const paymentId = "9f927a2c-02d4-4f76-914c-62cf44d9525e"; const sourceAccount = - "GBRPYHIL2C7Q7PGLUKSTPIY2KPJ7QMZ4ZWJHQ6GUSIW2LQAHOMK5N7BI"; + "GDESJRNS3IREXNSFMC35XM2CHEESOKAWQL2SXBK4AUIKNKCIYBMW4TDJ"; beforeEach(() => { vi.clearAllMocks(); @@ -234,7 +234,7 @@ describe("GET /api/path-payment-quote/:id — quote cache (issue #1045)", () => id: paymentId, amount: 25, asset: "USDC", - asset_issuer: "GDQOE23W4QK6WQ4R3BVCUO3PRA4VJ7A3M7MRWWX4V67WJYQ7QXKJQ4KJ", + asset_issuer: "GA5HBVNVS6P3QJ2ZGLMEACPSGFG3KDCAFTVHNJXWG22E5DVZTPSSAWQW", recipient: "GB6REFUNDTESTRECIPIENTQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQ", status: "pending", }), @@ -288,6 +288,7 @@ describe("GET /api/path-payment-quote/:id — quote cache (issue #1045)", () => params: { id: paymentId }, query: { source_asset: "EUR", + source_asset_issuer: "GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5", source_account: sourceAccount, }, merchant: { id: "merchant-1" }, @@ -310,7 +311,7 @@ describe("GET /api/path-payment-quote/:id — quote cache (issue #1045)", () => id: paymentId, amount: 25, asset: "USDC", - asset_issuer: "GDQOE23W4QK6WQ4R3BVCUO3PRA4VJ7A3M7MRWWX4V67WJYQ7QXKJQ4KJ", + asset_issuer: "GA5HBVNVS6P3QJ2ZGLMEACPSGFG3KDCAFTVHNJXWG22E5DVZTPSSAWQW", recipient: "GB6REFUNDTESTRECIPIENTQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQQ", status: "confirmed", }), @@ -346,3 +347,60 @@ describe("GET /api/path-payment-quote/:id — quote cache (issue #1045)", () => expect(findStrictReceivePaths).toHaveBeenCalledTimes(2); }); }); + +describe("GET /api/path-payment-quote/:id — payload validation (issue #1442)", () => { + const paymentId = "9f927a2c-02d4-4f76-914c-62cf44d9525e"; + + beforeEach(() => { + vi.clearAllMocks(); + resetExchangeRateCache(); + supabaseFrom.mockReturnValue( + createSupabaseSelectMock({ + id: paymentId, + amount: 25, + asset: "USDC", + asset_issuer: "GA5HBVNVS6P3QJ2ZGLMEACPSGFG3KDCAFTVHNJXWG22E5DVZTPSSAWQW", + recipient: "GA5HBVNVS6P3QJ2ZGLMEACPSGFG3KDCAFTVHNJXWG22E5DVZTPSSAWQW", + status: "pending", + }), + ); + }); + + it("rejects a credit source asset without an issuer with 400 before querying Horizon", async () => { + const res = createMockResponse(); + const next = vi.fn(); + await getPathPaymentQuoteHandler()( + { + params: { id: paymentId }, + query: { source_asset: "EUR" }, + merchant: { id: "merchant-1" }, + }, + res, + next, + ); + + expect(res.statusCode).toBe(400); + expect(res.body).toMatchObject({ field: "sourceAssetIssuer" }); + expect(findStrictReceivePaths).not.toHaveBeenCalled(); + expect(next).not.toHaveBeenCalled(); + }); + + it("rejects an issuer with a bad checksum", async () => { + const res = createMockResponse(); + await getPathPaymentQuoteHandler()( + { + params: { id: paymentId }, + query: { + source_asset: "EUR", + source_asset_issuer: "GA5HBVNVS6P3QJ2ZGLMEACPSGFG3KDCAFTVHNJXWG22E5DVZTPSSAWQA", + }, + merchant: { id: "merchant-1" }, + }, + res, + vi.fn(), + ); + + expect(res.statusCode).toBe(400); + expect(findStrictReceivePaths).not.toHaveBeenCalled(); + }); +}); diff --git a/backend/src/routes/payments.js b/backend/src/routes/payments.js index 847120e5..53c4101a 100644 --- a/backend/src/routes/payments.js +++ b/backend/src/routes/payments.js @@ -61,6 +61,7 @@ import { import { getExchangeRateQuote, NoPathFoundError, + ExchangeRateValidationError, } from "../services/exchangeRateService.js"; import { pathPaymentQuoteRequestsTotal, @@ -1313,6 +1314,10 @@ function createPaymentsRouter({ error: "No path found for this asset pair", }); } + if (err instanceof ExchangeRateValidationError) { + pathPaymentQuoteRequestsTotal.inc({ ...assetLabels, outcome: "invalid_params" }); + return res.status(400).json({ error: err.message, field: err.field }); + } throw err; } pathPaymentQuoteStageDuration.observe( diff --git a/backend/src/services/exchangeRateService.js b/backend/src/services/exchangeRateService.js index 548f418a..eafc4867 100644 --- a/backend/src/services/exchangeRateService.js +++ b/backend/src/services/exchangeRateService.js @@ -20,6 +20,13 @@ * shares one retry loop. Missing paths and other deterministic failures * are not retried. * + * Payload sanitization (issue #1442): + * - Request parameters are strictly validated and canonicalised before they + * form a cache key or reach Horizon (400 ExchangeRateValidationError). + * - Every quote is rebuilt from an allow-list of fields before it is cached + * or returned, and quotes read from the shared Redis store go through the + * same check, so a malformed or tampered entry is never served. + * * The route handler calls getExchangeRateQuote() and only handles HTTP concerns; * all exchange-rate logic lives here. */ @@ -32,6 +39,13 @@ import { import { ExchangeRateCoordinator } from '../lib/exchange-rate-coordinator.js'; import { withOracleRetry } from '../lib/exchange-rate-oracle-retry.js'; import { logger } from '../lib/logger.js'; +import { + sanitizeQuote, + sanitizeQuoteRequest, + ExchangeRateValidationError, +} from '../lib/exchange-rate-oracle-sanitizer.js'; + +export { ExchangeRateValidationError }; const DEFAULT_SLIPPAGE = parseFloat(process.env.PATH_PAYMENT_SLIPPAGE ?? '0.01'); @@ -57,7 +71,7 @@ export function configureExchangeRateCoordination({ redisClient, ...options } = coordinator = null; return false; } - coordinator = new ExchangeRateCoordinator({ redisClient, ...options }); + coordinator = new ExchangeRateCoordinator({ redisClient, sanitize: sanitizeQuote, ...options }); logger.info('Exchange-rate cache distributed coordination enabled'); return true; } @@ -95,15 +109,17 @@ export class NoPathFoundError extends ExchangeRateError { * @param {number} [params.slippage] * @returns {Promise} */ -export async function getExchangeRateQuote({ - sourceAssetCode, - sourceAssetIssuer = null, - destAssetCode, - destAssetIssuer = null, - destAmount, - sourceAccount = null, - slippage = DEFAULT_SLIPPAGE, -}) { +export async function getExchangeRateQuote(params) { + const { + sourceAssetCode, + sourceAssetIssuer, + destAssetCode, + destAssetIssuer, + destAmount, + sourceAccount, + slippage, + } = sanitizeQuoteRequest({ slippage: DEFAULT_SLIPPAGE, ...(params ?? {}) }); + const cache = getExchangeRateCache(); const cacheKey = generateRateCacheKey( sourceAssetCode, @@ -188,7 +204,46 @@ async function fetchQuoteFromHorizon({ cached: false, }; - return quote; + const sanitized = sanitizeQuote(quote); + if (!sanitized) { + logger.warn( + { sourceAsset: sourceAssetCode, destAsset: destAssetCode, hops: Array.isArray(path.path) ? path.path.length : null }, + 'Rejected malformed exchange rate quote from Horizon', + ); + const error = new ExchangeRateError('Horizon returned an invalid exchange rate quote', 502); + error.status = 502; + // The same Horizon answer would be rejected again; do not retry it. + error.retryable = false; + throw error; + } + return sanitized; +} + +/** + * Cache key for possibly-unsanitized parameters. Invalidation must never + * throw, so input that fails strict validation falls back to the raw key + * (it can only match an entry that was never written). + */ +function resolveCacheKey(sourceAsset, destAsset, destAmount, sourceAssetIssuer, destAssetIssuer) { + try { + const clean = sanitizeQuoteRequest({ + sourceAssetCode: sourceAsset, + sourceAssetIssuer, + destAssetCode: destAsset, + destAssetIssuer, + destAmount, + slippage: 0, + }); + return generateRateCacheKey( + clean.sourceAssetCode, + clean.destAssetCode, + clean.destAmount, + clean.sourceAssetIssuer, + clean.destAssetIssuer, + ); + } catch { + return generateRateCacheKey(sourceAsset, destAsset, destAmount, sourceAssetIssuer, destAssetIssuer); + } } /** @@ -203,7 +258,7 @@ export function invalidateExchangeRateQuote( destAssetIssuer = null, ) { const cache = getExchangeRateCache(); - const key = generateRateCacheKey(sourceAsset, destAsset, destAmount, sourceAssetIssuer, destAssetIssuer); + const key = resolveCacheKey(sourceAsset, destAsset, destAmount, sourceAssetIssuer, destAssetIssuer); const removed = cache.delete(key); // Propagate to peers via the shared store. Fire-and-forget keeps this diff --git a/backend/src/services/exchangeRateService.test.js b/backend/src/services/exchangeRateService.test.js index 9f355e27..25311aa7 100644 --- a/backend/src/services/exchangeRateService.test.js +++ b/backend/src/services/exchangeRateService.test.js @@ -6,6 +6,7 @@ import { resetExchangeRateCoordination, NoPathFoundError, ExchangeRateError, + ExchangeRateValidationError, } from './exchangeRateService.js'; import { resetExchangeRateCache, generateRateCacheKey } from '../lib/exchange-rate-cache.js'; import { createFakeRedis } from '../../tests/helpers/fake-redis.js'; @@ -20,6 +21,8 @@ vi.mock('../lib/logger.js', () => ({ import { findStrictReceivePaths } from '../lib/stellar.js'; +const USDC_ISSUER = 'GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5'; + const MOCK_PATH = { source_asset_code: 'XLM', source_asset_issuer: null, @@ -49,6 +52,7 @@ describe('getExchangeRateQuote', () => { const quote = await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', + destAssetIssuer: USDC_ISSUER, destAmount: '1.0000000', }); @@ -61,40 +65,40 @@ describe('getExchangeRateQuote', () => { }); it('calls findStrictReceivePaths exactly once on first call', async () => { - await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '1.0' }); + await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }); expect(findStrictReceivePaths).toHaveBeenCalledTimes(1); }); it('returns cached result on second identical call without hitting Horizon', async () => { - await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '1.0' }); - const second = await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '1.0' }); + await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }); + const second = await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }); expect(findStrictReceivePaths).toHaveBeenCalledTimes(1); expect(second.cached).toBe(true); }); it('does not share cache between different dest amounts', async () => { - await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '1.0' }); - await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '2.0' }); + await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }); + await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '2.0' }); expect(findStrictReceivePaths).toHaveBeenCalledTimes(2); }); it('does not share cache between different asset pairs', async () => { - await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '1.0' }); - await getExchangeRateQuote({ sourceAssetCode: 'BTC', destAssetCode: 'USDC', destAmount: '1.0' }); + await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }); + await getExchangeRateQuote({ sourceAssetCode: 'BTC', sourceAssetIssuer: USDC_ISSUER, destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }); expect(findStrictReceivePaths).toHaveBeenCalledTimes(2); }); it('throws NoPathFoundError when Horizon returns null', async () => { findStrictReceivePaths.mockResolvedValue(null); await expect( - getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '1.0' }), + getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }), ).rejects.toThrow(NoPathFoundError); }); it('NoPathFoundError has statusCode 404', async () => { findStrictReceivePaths.mockResolvedValue(null); try { - await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '1.0' }); + await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }); } catch (err) { expect(err.statusCode).toBe(404); } @@ -105,7 +109,7 @@ describe('getExchangeRateQuote', () => { horizonErr.status = 503; findStrictReceivePaths.mockRejectedValue(horizonErr); await expect( - getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '1.0' }), + getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }), ).rejects.toThrow('Horizon unavailable'); expect(findStrictReceivePaths).toHaveBeenCalledTimes(3); expect(horizonErr.retryAttempts).toBe(3); @@ -117,7 +121,7 @@ describe('getExchangeRateQuote', () => { .mockResolvedValueOnce(MOCK_PATH); const quote = await getExchangeRateQuote({ sourceAssetCode: 'XLM', - destAssetCode: 'USDC', + destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0', }); expect(quote.sourceAmount).toBe('0.5000000'); @@ -127,7 +131,7 @@ describe('getExchangeRateQuote', () => { it('does not retry a deterministic 400', async () => { findStrictReceivePaths.mockRejectedValue(Object.assign(new Error('bad request'), { status: 400 })); await expect( - getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '1.0' }), + getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }), ).rejects.toThrow('bad request'); expect(findStrictReceivePaths).toHaveBeenCalledTimes(1); }); @@ -136,6 +140,7 @@ describe('getExchangeRateQuote', () => { const quote = await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', + destAssetIssuer: USDC_ISSUER, destAmount: '1.0000000', slippage: 0.02, }); @@ -156,17 +161,17 @@ describe('invalidateExchangeRateQuote', () => { }); it('forces re-query after invalidation', async () => { - await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '1.0' }); - invalidateExchangeRateQuote('XLM', 'USDC', '1.0'); - await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '1.0' }); + await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }); + invalidateExchangeRateQuote('XLM', 'USDC', '1.0', null, USDC_ISSUER); + await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }); expect(findStrictReceivePaths).toHaveBeenCalledTimes(2); }); it('does not affect cache entries for different amounts', async () => { - await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '1.0' }); - await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '2.0' }); - invalidateExchangeRateQuote('XLM', 'USDC', '1.0'); - const quote = await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '2.0' }); + await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }); + await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '2.0' }); + invalidateExchangeRateQuote('XLM', 'USDC', '1.0', null, USDC_ISSUER); + const quote = await getExchangeRateQuote({ sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '2.0' }); // The 2.0 quote is still cached, so no third Horizon call is made. expect(findStrictReceivePaths).toHaveBeenCalledTimes(2); expect(quote.cached).toBe(true); @@ -174,7 +179,7 @@ describe('invalidateExchangeRateQuote', () => { }); describe('concurrency control (issue #1445)', () => { - const params = { sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAmount: '1.0' }; + const params = { sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }; beforeEach(() => { resetExchangeRateCache(); @@ -226,7 +231,7 @@ describe('concurrency control (issue #1445)', () => { const first = await getExchangeRateQuote(params); expect(first.cached).toBe(false); - const key = generateRateCacheKey('XLM', 'USDC', '1.0'); + const key = generateRateCacheKey('XLM', 'USDC', '1.0', null, USDC_ISSUER); expect(redis.store.has(`exrate:quote:${key}`)).toBe(true); // A second instance with an empty in-memory cache. @@ -241,7 +246,7 @@ describe('concurrency control (issue #1445)', () => { const redis = createFakeRedis(); configureExchangeRateCoordination({ redisClient: redis }); await getExchangeRateQuote(params); - invalidateExchangeRateQuote('XLM', 'USDC', '1.0'); + invalidateExchangeRateQuote('XLM', 'USDC', '1.0', null, USDC_ISSUER); await vi.waitFor(() => expect(redis.count('DEL')).toBe(1)); expect(redis.store.size).toBe(0); }); @@ -252,6 +257,127 @@ describe('concurrency control (issue #1445)', () => { redis.setFailure(new Error('ECONNREFUSED')); const quote = await getExchangeRateQuote(params); expect(quote.sourceAmount).toBe('0.5000000'); - expect(() => invalidateExchangeRateQuote('XLM', 'USDC', '1.0')).not.toThrow(); + expect(() => invalidateExchangeRateQuote('XLM', 'USDC', '1.0', null, USDC_ISSUER)).not.toThrow(); + }); +}); + +describe('payload sanitization and strict validation (issue #1442)', () => { + const params = { sourceAssetCode: 'XLM', destAssetCode: 'USDC', destAssetIssuer: USDC_ISSUER, destAmount: '1.0' }; + + beforeEach(() => { + resetExchangeRateCache(); + resetExchangeRateCoordination(); + vi.clearAllMocks(); + process.env.EXCHANGE_RATE_ORACLE_RETRY_BASE_DELAY_MS = '0'; + process.env.EXCHANGE_RATE_ORACLE_RETRY_MAX_DELAY_MS = '0'; + findStrictReceivePaths.mockResolvedValue(MOCK_PATH); + }); + + afterEach(() => { + resetExchangeRateCache(); + resetExchangeRateCoordination(); + }); + + it.each([ + ['a credit asset without issuer', { ...params, destAssetIssuer: null }, 'destAssetIssuer'], + ['a malformed amount', { ...params, destAmount: '1e9' }, 'destAmount'], + ['an invalid source account', { ...params, sourceAccount: 'GNOTANACCOUNT' }, 'sourceAccount'], + ['an out-of-range slippage', { ...params, slippage: 5 }, 'slippage'], + ])('rejects %s with a 400 before touching Horizon or the cache', async (_label, input, field) => { + await expect(getExchangeRateQuote(input)).rejects.toMatchObject({ + name: 'ExchangeRateValidationError', + status: 400, + field, + }); + expect(findStrictReceivePaths).not.toHaveBeenCalled(); + }); + + it('exposes ExchangeRateValidationError for route handlers', async () => { + await expect(getExchangeRateQuote({ ...params, destAmount: '-1' })) + .rejects.toBeInstanceOf(ExchangeRateValidationError); + }); + + it('canonicalises equivalent requests onto one cache entry', async () => { + await getExchangeRateQuote(params); + const again = await getExchangeRateQuote({ ...params, destAssetCode: ' usdc ', sourceAssetIssuer: USDC_ISSUER }); + expect(again.cached).toBe(true); + expect(findStrictReceivePaths).toHaveBeenCalledTimes(1); + }); + + it('passes sanitized parameters to Horizon', async () => { + await getExchangeRateQuote({ ...params, destAssetCode: 'usdc', sourceAssetCode: 'xlm' }); + expect(findStrictReceivePaths).toHaveBeenCalledWith(expect.objectContaining({ + sourceAssetCode: 'XLM', + sourceAssetIssuer: null, + destAssetCode: 'USDC', + destAssetIssuer: USDC_ISSUER, + })); + }); + + it('rejects a malformed Horizon quote with 502, without retrying or caching it', async () => { + findStrictReceivePaths.mockResolvedValue({ ...MOCK_PATH, source_amount: 'NaN' }); + await expect(getExchangeRateQuote(params)).rejects.toMatchObject({ statusCode: 502, retryable: false }); + expect(findStrictReceivePaths).toHaveBeenCalledTimes(1); + + findStrictReceivePaths.mockResolvedValue(MOCK_PATH); + const quote = await getExchangeRateQuote(params); + expect(quote.cached).toBe(false); + }); + + it('strips unknown fields that a Horizon response or peer tries to smuggle in', async () => { + findStrictReceivePaths.mockResolvedValue({ + ...MOCK_PATH, + path: [{ ...MOCK_PATH.path[0], memo: 'evil' }], + }); + const quote = await getExchangeRateQuote(params); + expect(quote.path).toEqual([{ asset_code: 'USDC', asset_issuer: USDC_ISSUER }]); + }); + + it('sanitizes quotes read from the shared Redis store', async () => { + const redis = createFakeRedis(); + configureExchangeRateCoordination({ redisClient: redis, pollIntervalMs: 5 }); + const key = generateRateCacheKey('XLM', 'USDC', '1.0', null, USDC_ISSUER); + const planted = { + sourceAsset: 'XLM', + sourceAssetIssuer: null, + sourceAmount: '0.5000000', + sendMax: '0.5050000', + destinationAsset: 'USDC', + destinationAssetIssuer: USDC_ISSUER, + destinationAmount: '1.0', + path: [], + slippage: 0.01, + redirectTo: 'GATTACKER', + }; + redis.store.set(`exrate:quote:${key}`, { + value: JSON.stringify({ v: 1, insertedAt: Date.now(), data: planted }), + expiresAt: null, + }); + + const quote = await getExchangeRateQuote(params); + expect(quote.cached).toBe(true); + expect(quote).not.toHaveProperty('redirectTo'); + expect(findStrictReceivePaths).not.toHaveBeenCalled(); + }); + + it('treats a shared entry that fails strict validation as a miss', async () => { + const redis = createFakeRedis(); + configureExchangeRateCoordination({ redisClient: redis, pollIntervalMs: 5 }); + const key = generateRateCacheKey('XLM', 'USDC', '1.0', null, USDC_ISSUER); + redis.store.set(`exrate:quote:${key}`, { + value: JSON.stringify({ + v: 1, + data: { sourceAsset: 'XLM', sourceAmount: '9.0000000', sendMax: '0.0000001', path: [] }, + }), + expiresAt: null, + }); + + const quote = await getExchangeRateQuote(params); + expect(quote.sendMax).toBe('0.5050000'); + expect(findStrictReceivePaths).toHaveBeenCalledTimes(1); + }); + + it('invalidation never throws on malformed input', () => { + expect(() => invalidateExchangeRateQuote(null, undefined, {})).not.toThrow(); }); }); diff --git a/backend/tests/integration/exchange-rate-cache.test.js b/backend/tests/integration/exchange-rate-cache.test.js index a81676ca..53f2df28 100644 --- a/backend/tests/integration/exchange-rate-cache.test.js +++ b/backend/tests/integration/exchange-rate-cache.test.js @@ -31,7 +31,7 @@ import { exchangeRateCacheCoalescedRequests } from '../../src/lib/path-payment-m import { createFakeRedis } from '../helpers/fake-redis.js'; const USDC_ISSUER = 'GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5'; -const SOURCE_ACCOUNT = 'GA5XIGA5C7FBPTVQ3CWHKNC7D2ZBHB24G3KUJG5WZ6S4EYWSSBFVL45T'; +const SOURCE_ACCOUNT = 'GDVWLHAZ3SC733EPYW7E27HFXIO74XSG66D7Y3J2Y5MYVNGV6Z46E6DD'; const payments = vi.hoisted(() => new Map()); diff --git a/backend/tests/integration/horizon-retry-manager.test.js b/backend/tests/integration/horizon-retry-manager.test.js new file mode 100644 index 00000000..82ec8bf6 --- /dev/null +++ b/backend/tests/integration/horizon-retry-manager.test.js @@ -0,0 +1,229 @@ +/** + * Horizon Client Retry Manager — integration suite (issue #1441). + * + * Drives the real HorizonClient -> RetryManager -> Stellar SDK -> HTTP stack + * against a local Horizon stub, so status codes, retries, backoff, the + * concurrency limiter and cross-instance coordination are exercised end to + * end. Only two edges are simulated: + * - Horizon: a node:http server scripted per test + * - Redis: the in-memory fake shared by the other coordination suites + */ +import http from 'node:http'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; + +vi.mock('../../src/lib/logger.js', () => ({ + logger: { warn: vi.fn(), info: vi.fn(), error: vi.fn(), debug: vi.fn() }, +})); + +import { HorizonClient } from '../../src/lib/stellar/horizon-client.js'; +import { RETRY_STOP_REASONS } from '../../src/lib/stellar/horizon-client/retry-manager.js'; +import { RETRY_MANAGER_ERROR_CODES } from '../../src/lib/stellar/horizon-client/constants.js'; +import { createFakeRedis } from '../helpers/fake-redis.js'; + +const PASSPHRASE = 'Test SDF Network ; September 2015'; +const TX_HASH = 'a'.repeat(64); +const FEE_STATS = { last_ledger: '1', last_ledger_base_fee: '100', fee_charged: { p50: '100' }, max_fee: { p50: '100' } }; + +/** + * Scriptable Horizon stub. `script` is consumed one entry per request; the + * last entry repeats. Each entry is { status, body?, headers?, delayMs? }. + */ +function createHorizonStub() { + let script = [{ status: 200, body: FEE_STATS }]; + let index = 0; + let inFlight = 0; + const requests = []; + const state = { peakInFlight: 0 }; + + const server = http.createServer(async (req, res) => { + inFlight += 1; + state.peakInFlight = Math.max(state.peakInFlight, inFlight); + requests.push({ url: req.url, at: Date.now() }); + const step = script[Math.min(index, script.length - 1)]; + index += 1; + if (step.delayMs) await new Promise((r) => setTimeout(r, step.delayMs)); + res.writeHead(step.status, { 'Content-Type': 'application/json', ...(step.headers ?? {}) }); + res.end(JSON.stringify(step.body ?? { status: step.status, title: `HTTP ${step.status}` })); + inFlight -= 1; + }); + + return { + server, + requests, + state, + get url() { + return `http://127.0.0.1:${server.address().port}`; + }, + script(steps) { + script = steps; + index = 0; + requests.length = 0; + state.peakInFlight = 0; + }, + listen: () => new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)), + close: () => new Promise((resolve) => server.close(resolve)), + }; +} + +function makeClient(url, options = {}) { + return new HorizonClient(url, PASSPHRASE, { + serverOptions: { allowHttp: true }, + backoff: { baseDelayMs: 10, maxDelayMs: 40, jitter: 'none', maxRetries: 2, ...(options.backoff ?? {}) }, + concurrency: options.concurrency, + coordination: options.coordination, + }); +} + +describe('Horizon Client Retry Manager — HTTP integration', () => { + const horizon = createHorizonStub(); + const clients = []; + const client = (options) => { + const c = makeClient(horizon.url, options); + clients.push(c); + return c; + }; + + beforeAll(() => horizon.listen()); + afterAll(() => horizon.close()); + beforeEach(() => { + while (clients.length) clients.pop().close(); + horizon.script([{ status: 200, body: FEE_STATS }]); + }); + + it('returns Horizon data on the first attempt', async () => { + const c = client(); + await expect(c.getFeeStats()).resolves.toMatchObject({ last_ledger_base_fee: '100' }); + expect(horizon.requests).toHaveLength(1); + expect(horizon.requests[0].url).toBe('/fee_stats'); + }); + + it('recovers from transient 503s with exponential backoff', async () => { + horizon.script([{ status: 503 }, { status: 502 }, { status: 200, body: FEE_STATS }]); + const c = client(); + await expect(c.getFeeStats()).resolves.toMatchObject({ last_ledger: '1' }); + expect(horizon.requests).toHaveLength(3); + + const [a, b, d] = horizon.requests.map((r) => r.at); + // 10ms then 30ms (base 10, multiplier 3), allowing for timer slack. + expect(b - a).toBeGreaterThanOrEqual(8); + expect(d - b).toBeGreaterThanOrEqual(25); + }); + + it('does not retry a 404 and maps it to a descriptive error', async () => { + horizon.script([{ status: 404 }]); + const c = client(); + const err = await c.fetchTransaction(TX_HASH).catch((e) => e); + expect(err).toMatchObject({ status: 404, operation: 'fetchTransaction' }); + expect(err.message).toContain(`txHash=${TX_HASH}`); + expect(horizon.requests).toHaveLength(1); + }); + + it('gives up after maxRetries on a persistent 500 and reports why', async () => { + horizon.script([{ status: 500 }]); + const c = client(); + const err = await c.getFeeStats().catch((e) => e); + expect(err).toMatchObject({ status: 502, attempts: 3, retryStopReason: RETRY_STOP_REASONS.EXHAUSTED }); + expect(horizon.requests).toHaveLength(3); + }); + + it('retries a 429 and then succeeds', async () => { + horizon.script([{ status: 429 }, { status: 200, body: FEE_STATS }]); + const c = client(); + await expect(c.getFeeStats()).resolves.toBeTruthy(); + expect(horizon.requests).toHaveLength(2); + }); + + it('honours the operation time budget', async () => { + horizon.script([{ status: 500 }]); + const c = client({ backoff: { baseDelayMs: 30, maxDelayMs: 1_000, maxRetries: 5, maxElapsedMs: 100 } }); + const started = Date.now(); + const err = await c.getFeeStats().catch((e) => e); + expect(err.retryStopReason).toBe(RETRY_STOP_REASONS.BUDGET_EXCEEDED); + expect(Date.now() - started).toBeLessThan(400); + expect(horizon.requests.length).toBeLessThan(6); + }); + + it('retries connection failures and reports the Horizon URL', async () => { + const dead = http.createServer(); + await new Promise((resolve) => dead.listen(0, '127.0.0.1', resolve)); + const deadUrl = `http://127.0.0.1:${dead.address().port}`; + await new Promise((resolve) => dead.close(resolve)); + + const c = makeClient(deadUrl); + clients.push(c); + const err = await c.getFeeStats().catch((e) => e); + expect(err.status).toBe(502); + expect(err.message).toContain(deadUrl); + expect(err.attempts).toBe(3); + }); + + it('never runs more than maxConcurrent requests against Horizon at once', async () => { + horizon.script([{ status: 200, body: FEE_STATS, delayMs: 20 }]); + const c = client({ concurrency: { maxConcurrent: 3, maxQueue: 100 } }); + const results = await Promise.all(Array.from({ length: 12 }, () => c.getFeeStats())); + expect(results).toHaveLength(12); + expect(horizon.state.peakInFlight).toBeLessThanOrEqual(3); + expect(c.getRetryManager().getConfig().concurrency).toMatchObject({ active: 0, pending: 0 }); + }); + + it('sheds load with 503 when the queue is full instead of hitting Horizon', async () => { + horizon.script([{ status: 200, body: FEE_STATS, delayMs: 50 }]); + const c = client({ concurrency: { maxConcurrent: 1, maxQueue: 1 } }); + const settled = await Promise.allSettled([c.getFeeStats(), c.getFeeStats(), c.getFeeStats()]); + const rejected = settled.filter((s) => s.status === 'rejected'); + expect(rejected).toHaveLength(1); + expect(rejected[0].reason).toMatchObject({ status: 503, code: RETRY_MANAGER_ERROR_CODES.QUEUE_FULL }); + expect(horizon.requests).toHaveLength(2); + }); + + describe('across instances sharing Redis', () => { + it('a 429 seen by one instance makes the other wait before calling Horizon', async () => { + const redis = createFakeRedis(); + const a = client({ backoff: { baseDelayMs: 200, maxDelayMs: 200 } }); + const b = client(); + expect(a.configureRetryCoordination({ redisClient: redis })).toBe('redis'); + expect(b.configureRetryCoordination({ redisClient: redis })).toBe('redis'); + + horizon.script([{ status: 429 }, { status: 200, body: FEE_STATS }]); + const first = a.getFeeStats(); + await vi.waitFor(() => expect(horizon.requests).toHaveLength(1)); + await vi.waitFor(() => expect(redis.keys('horizon:retry:cooldown:')).toHaveLength(1)); + + const startedB = Date.now(); + await b.getFeeStats(); + await first; + const bRequest = horizon.requests.find((r) => r.at >= startedB); + // B waited out (most of) the 200ms cooldown instead of hitting Horizon immediately. + expect(bRequest.at - startedB).toBeGreaterThanOrEqual(100); + }); + + it('caps concurrent retries cluster-wide and releases every slot afterwards', async () => { + const redis = createFakeRedis(); + const nodes = [client(), client(), client()]; + for (const n of nodes) { + n.configureRetryCoordination({ redisClient: redis, maxConcurrentRetries: 1 }); + } + + horizon.script([{ status: 500, delayMs: 5 }]); + const settled = await Promise.allSettled(nodes.map((n) => n.getFeeStats())); + const reasons = settled.map((s) => s.reason.retryStopReason); + + // Only one instance may hold the single retry slot at a time; the + // others shed their retry instead of piling onto a failing Horizon. + expect(reasons).toContain(RETRY_STOP_REASONS.RETRY_SLOTS_EXHAUSTED); + expect(horizon.requests.length).toBeLessThan(nodes.length * 3); + expect(redis.keys('horizon:retry:slot:')).toHaveLength(0); + }); + + it('keeps serving requests when Redis fails mid-flight', async () => { + const redis = createFakeRedis(); + const c = client(); + c.configureRetryCoordination({ redisClient: redis }); + redis.setFailure(new Error('ECONNRESET')); + + horizon.script([{ status: 503 }, { status: 200, body: FEE_STATS }]); + await expect(c.getFeeStats()).resolves.toBeTruthy(); + expect(c.getRetryManager().getConfig().coordination.stats.fallbacks).toBeGreaterThan(0); + }); + }); +});