feat(transfers): make transfer creation idempotent under retries

README.md

@@ -159,6 +159,7 @@ rounded away.
 
 - `POST /api/transfers` — create a transfer.
   Body: `{ senderName, recipientName, amount, from, to }`
+  Requires an `Idempotency-Key` header (see below).
 - `GET /api/transfers` — list transfers. Supports `?status=`, `?q=` (name
   search), `?archived=` (true/false/all), and `?limit=`/`?offset=` pagination.
   Archived transfers are excluded from results by default.
@@ -169,6 +170,34 @@ rounded away.
 - `POST /api/transfers/:id/archive` — archive a transfer, hiding it from default list results.
 - `POST /api/transfers/:id/unarchive` — unarchive a transfer, restoring it to default list results.
 
+#### Idempotency
+
+`POST /api/transfers` requires an `Idempotency-Key` header. The endpoint moves
+money, so a client that omits the header is not opting out of protection, it is
+unaware it needs it; the request is rejected with 400 rather than risking a
+duplicate.
+
+For a given (API token, key) pair the operation runs at most once:
+
+- **Retry with the same payload** replays the stored transfer, answering 201
+  with the original body. The quote is not recomputed, so a moved rate cannot
+  change the answer, and no second payment is submitted.
+- **Reuse with a different payload** answers 409. The fingerprint covers the
+  fields that determine the transfer, so property order, an amount sent as a
+  string, and unrelated extra fields all still count as the same request.
+- **A retry arriving while the first is still in flight** answers 409 rather
+  than starting a second settlement.
+- **A provider failure releases the key**, so retrying with the same key can
+  succeed once the provider recovers.
+
+Keys are scoped to the API token: two callers using the same key get two
+independent transfers, and neither can reach the other's.
+
+Records live in the same store as the transfers, so with the in-memory store
+that backs this demo a restart clears both together. That keeps them
+consistent: a surviving reservation would replay a transfer that no longer
+exists.
+
 ### Users
 
 - `GET /api/users` — list users.

src/controllers/transferController.js

@@ -1,8 +1,41 @@
 'use strict';
 
 const transferService = require('../services/transferService');
+const idempotencyService = require('../services/idempotencyService');
+const ApiError = require('../utils/ApiError');
 const { parsePagination } = require('../utils/pagination');
 
+/** Upper bound on a client-supplied key, so the map cannot be grown without limit. */
+const MAX_IDEMPOTENCY_KEY_LENGTH = 255;
+
+/**
+ * Read and validate the Idempotency-Key header.
+ *
+ * Required rather than optional: this endpoint moves money, and a client that
+ * omits the header is not opting out of protection, it is unaware it needs it.
+ * Failing the request is the only outcome that cannot silently duplicate a
+ * transfer.
+ *
+ * @param {import('express').Request} req
+ * @returns {string}
+ * @throws {ApiError} 400 when the header is missing or unusable.
+ */
+function requireIdempotencyKey(req) {
+  const raw = req.get('Idempotency-Key');
+  if (typeof raw !== 'string' || raw.trim() === '') {
+    throw ApiError.badRequest(
+      'Idempotency-Key header is required to create a transfer'
+    );
+  }
+  const key = raw.trim();
+  if (key.length > MAX_IDEMPOTENCY_KEY_LENGTH) {
+    throw ApiError.badRequest(
+      `Idempotency-Key must be at most ${MAX_IDEMPOTENCY_KEY_LENGTH} characters`
+    );
+  }
+  return key;
+}
+
 /**
  * Transfer controllers.
  */
@@ -12,7 +45,31 @@ const { parsePagination } = require('../utils/pagination');
  * Create a new transfer.
  */
 function createTransfer(req, res) {
-  const transfer = transferService.createTransfer(req.body, req.id);
+  const key = requireIdempotencyKey(req);
+
+  // The fingerprint covers the fields that determine the operation, not the raw
+  // body: an unrelated extra property must not read as a conflicting retry.
+  // `amount` is normalized because "100" and 100 both validate and produce the
+  // same transfer, so treating them as different requests would reject a
+  // legitimate retry from a client that re-serialized its payload.
+  const fingerprint = idempotencyService.fingerprint({
+    senderName: req.body.senderName,
+    recipientName: req.body.recipientName,
+    amount: Number(req.body.amount),
+    from: req.body.from,
+    to: req.body.to,
+  });
+
+  const transfer = transferService.createTransfer(req.body, req.id, {
+    actor: req.token,
+    key,
+    fingerprint,
+  });
+
+  // A replay answers 201 with the original transfer, exactly as the first call
+  // did. Replaying the stored result means replaying all of it; downgrading the
+  // status would make a successful retry look different from the response it is
+  // standing in for.
   res.status(201).json(transfer);
 }
 

src/services/transferService.js

@@ -6,6 +6,7 @@ const ApiError = require('../utils/ApiError');
 const { TRANSFER_STATUS, TRANSFER_TRANSITIONS } = require('../config/constants');
 const quoteService = require('./quoteService');
 const stellarService = require('./stellarService');
+const idempotencyService = require('./idempotencyService');
 const auditService = require('./auditService');
 
 // Keep lifecycle timestamps strictly increasing even when multiple operations
@@ -106,11 +107,58 @@ function getTransferOrThrow(id) {
 
 /**
  * Create a new transfer using a freshly computed quote.
+ *
+ * When `idempotency` is supplied the whole operation is exactly-once for that
+ * (actor, key) pair: the key is reserved before the provider is called, so a
+ * retry arriving mid-flight cannot start a second settlement, and once the
+ * transfer exists the stored result is replayed instead of re-running.
+ *
+ * The context is optional because idempotency is actor-scoped and an actor only
+ * exists at the HTTP boundary; internal callers have no token to scope to. The
+ * route requires the header, so every request-driven creation is covered.
+ *
  * @param {object} data
  * @param {string} [requestId] - optional correlation id for audit logging
+ * @param {{ actor: string, key: string, fingerprint: string }} [idempotency]
  * @returns {object}
  */
-function createTransfer(data, requestId) {
+function createTransfer(data, requestId, idempotency) {
+  if (idempotency) {
+    const outcome = idempotencyService.begin(
+      store.idempotency,
+      idempotency.actor,
+      idempotency.key,
+      idempotency.fingerprint
+    );
+    // A completed record short-circuits before the quote is recomputed. Rates
+    // move, so recomputing would hand the client a different transfer under the
+    // same key, which is the duplicate this is meant to prevent.
+    if (outcome.status === 'replay') {
+      return outcome.result;
+    }
+  }
+
+  try {
+    return createTransferUnchecked(data, requestId, idempotency);
+  } catch (err) {
+    // The operation never reached a terminal state, so the key must not stay
+    // burned: the client's correct response to a provider failure is to retry
+    // with the same key, and that has to be able to succeed.
+    if (idempotency) {
+      idempotencyService.release(store.idempotency, idempotency.actor, idempotency.key);
+    }
+    throw err;
+  }
+}
+
+/**
+ * Perform the creation itself, with the reservation already held.
+ * @param {object} data
+ * @param {string} [requestId]
+ * @param {{ actor: string, key: string }} [idempotency]
+ * @returns {object}
+ */
+function createTransferUnchecked(data, requestId, idempotency) {
   const quote = quoteService.getQuote(data.amount, data.from, data.to);
   const settlement = stellarService.submitPayment({
     amount: quote.sendAmount,
@@ -150,6 +198,15 @@ function createTransfer(data, requestId) {
     requestId,
   });
 
+  if (idempotency) {
+    idempotencyService.complete(
+      store.idempotency,
+      idempotency.actor,
+      idempotency.key,
+      transfer
+    );
+  }
+
   return transfer;
 }
 

src/store/index.js

@@ -10,12 +10,17 @@ const auditService = require('../services/auditService');
 const store = {
   users: new Map(),
   transfers: new Map(),
+  // Keyed by "<actor> <idempotency-key>". Lives here rather than in a module
+  // local so it shares the transfers' lifetime: a replay can never outlive the
+  // transfer it would replay.
+  idempotency: new Map(),
 };
 
 /** Remove all records from the store. Primarily used in tests/seeding. */
 function reset() {
   store.users.clear();
   store.transfers.clear();
+  store.idempotency.clear();
   auditService.reset();
 }
 

test/moneyPrecision.test.js

@@ -67,6 +67,7 @@ test('POST /api/transfers rejects an amount with sub-cent precision', async () =
     headers: {
       Authorization: 'Bearer test-token-admin',
       'Content-Type': 'application/json',
+      'Idempotency-Key': 'idem-precision-1',
     },
     body: JSON.stringify({
       senderName: 'Alice',
@@ -88,6 +89,7 @@ test('POST /api/transfers accepts a well-formed two-decimal amount', async () =>
     headers: {
       Authorization: 'Bearer test-token-admin',
       'Content-Type': 'application/json',
+      'Idempotency-Key': 'idem-precision-2',
     },
     body: JSON.stringify({
       senderName: 'Alice',

test/requireScope.test.js

@@ -116,7 +116,7 @@ test('GET /api/users returns 401 when token is unknown', async () => {
 test('POST /api/transfers returns 401 when token is unknown', async () => {
   const { status, body } = await fetchJson('/api/transfers', {
     method: 'POST',
-    headers: { ...authHeader('bad-token'), 'Content-Type': 'application/json' },
+    headers: { ...authHeader('bad-token'), 'Content-Type': 'application/json', 'Idempotency-Key': 'idem-requireScope-118' },
     body: JSON.stringify({ senderName: 'Alice', recipientName: 'Bob', amount: 100, from: 'USD', to: 'EUR' }),
   });
   assert.equal(status, 401);
@@ -129,7 +129,7 @@ test('POST /api/transfers returns 403 when token only has transfers:read scope',
   // test-token-readonly has: transfers:read, users:read, audit:read — no :write scopes
   const { status, body } = await fetchJson('/api/transfers', {
     method: 'POST',
-    headers: { ...authHeader('test-token-readonly'), 'Content-Type': 'application/json' },
+    headers: { ...authHeader('test-token-readonly'), 'Content-Type': 'application/json', 'Idempotency-Key': 'idem-requireScope-131' },
     body: JSON.stringify({ senderName: 'Alice', recipientName: 'Bob', amount: 100, from: 'USD', to: 'EUR' }),
   });
   assert.equal(status, 403);
@@ -153,7 +153,7 @@ test('POST /api/transfers/:id/claim returns 403 when token only has transfers:re
   // Create a transfer first using the admin token, then try to claim with read-only
   const createRes = await fetchJson('/api/transfers', {
     method: 'POST',
-    headers: { ...authHeader('test-token-admin'), 'Content-Type': 'application/json' },
+    headers: { ...authHeader('test-token-admin'), 'Content-Type': 'application/json', 'Idempotency-Key': 'idem-requireScope-155' },
     body: JSON.stringify({ senderName: 'Alice', recipientName: 'Bob', amount: 100, from: 'USD', to: 'EUR' }),
   });
   assert.equal(createRes.status, 201);
@@ -226,7 +226,7 @@ test('GET /api/transfers returns 200 with readonly token', async () => {
 test('POST /api/transfers returns 201 with admin token', async () => {
   const { status, body } = await fetchJson('/api/transfers', {
     method: 'POST',
-    headers: { ...authHeader('test-token-admin'), 'Content-Type': 'application/json' },
+    headers: { ...authHeader('test-token-admin'), 'Content-Type': 'application/json', 'Idempotency-Key': 'idem-requireScope-228' },
     body: JSON.stringify({ senderName: 'Alice', recipientName: 'Bob', amount: 100, from: 'USD', to: 'EUR' }),
   });
   assert.equal(status, 201);
@@ -236,7 +236,7 @@ test('POST /api/transfers returns 201 with admin token', async () => {
 test('POST /api/transfers returns 201 with transfers-scoped token', async () => {
   const { status, body } = await fetchJson('/api/transfers', {
     method: 'POST',
-    headers: { ...authHeader('test-token-transfers'), 'Content-Type': 'application/json' },
+    headers: { ...authHeader('test-token-transfers'), 'Content-Type': 'application/json', 'Idempotency-Key': 'idem-requireScope-238' },
     body: JSON.stringify({ senderName: 'Alice', recipientName: 'Bob', amount: 100, from: 'USD', to: 'EUR' }),
   });
   assert.equal(status, 201);
@@ -291,7 +291,7 @@ test('full transfer lifecycle: create → claim with correct scopes', async () =
   // Create
   const createRes = await fetchJson('/api/transfers', {
     method: 'POST',
-    headers: { ...authHeader('test-token-admin'), 'Content-Type': 'application/json' },
+    headers: { ...authHeader('test-token-admin'), 'Content-Type': 'application/json', 'Idempotency-Key': 'idem-requireScope-293' },
     body: JSON.stringify({ senderName: 'Alice', recipientName: 'Bob', amount: 200, from: 'USD', to: 'INR' }),
   });
   assert.equal(createRes.status, 201);
@@ -317,7 +317,7 @@ test('full transfer lifecycle: create → claim with correct scopes', async () =
 test('full transfer lifecycle: create → cancel with correct scopes', async () => {
   const createRes = await fetchJson('/api/transfers', {
     method: 'POST',
-    headers: { ...authHeader('test-token-transfers'), 'Content-Type': 'application/json' },
+    headers: { ...authHeader('test-token-transfers'), 'Content-Type': 'application/json', 'Idempotency-Key': 'idem-requireScope-319' },
     body: JSON.stringify({ senderName: 'Carlos', recipientName: 'Diaz', amount: 500, from: 'EUR', to: 'MXN' }),
   });
   assert.equal(createRes.status, 201);