Document (and test) FIX

docs/ARCHITECTURE.md

@@ -1,23 +1,50 @@
-# AnchorNet Backend Architecture & Security Guarantees
+AnchorNet Backend Architecture & Security Guarantees
+Overview
+The anchornet-backend service provides REST APIs for Stellar liquidity coordination, settlement, and routing.
 
-## Overview
+Security Architecture & Audit Log Guarantees
+Audit Endpoint (GET /api/v1/audit)
+The audit log middleware (src/middleware/auditLog.ts) captures recent mutating requests (POST, PUT, PATCH, DELETE) in an in-memory bounded ring buffer.
 
-The `anchornet-backend` service provides REST APIs for Stellar liquidity coordination, settlement, and routing.
+Captured Fields
+method: HTTP method
+path: Request path (without query parameters)
+status: Response status code
+requestId: Correlation ID from x-request-id response header
+timestamp: ISO timestamp of request completion
+Sensitive Data Redaction & Security Guarantees
+Strict Redaction via Denylist: Any header, body parameter, or metadata stored in audit log entries is processed through redactSensitiveData().
+Denylisted Fields: Secret-bearing keys such as x-api-key, authorization, cookie, set-cookie, token, access_token, refresh_token, secret, password, bearer, private_key, client_secret are matched case-insensitively and replaced with "[REDACTED]".
+Preventing Plaintext Exposure: Under no circumstances should raw credentials or API keys be captured or retained in plaintext in the in-memory audit ring buffer or exposed via GET /api/v1/audit.
+In-Memory Repositories & Future Persistence
+Settlement, anchor, and liquidity data are held in process-local in-memory
+repositories (src/repositories/*), all extending the shared
+InMemoryRepository base class.
 
-## Security Architecture & Audit Log Guarantees
+Persistence-Swap Risk (read before swapping any repository for a DB)
+Several repositories already document that they are "swappable for a
+persistent … store later" (e.g. liquidityRepository.ts). This is a forward
+design intent, but the current id-allocation contract does not survive that
+swap unchanged:
 
-### Audit Endpoint (`GET /api/v1/audit`)
+InMemoryRepository.generateId() / peekId() allocate ids under the
+assumption that they run synchronously and atomically on Node's single
+thread. peekId() exposes the id that generateId() will hand out next
+without any locking.
+SettlementRepository.peekNextId() returns that previewed id. It is safe to
+call peekNextId() and then create() only because both are synchronous
+— no other mutation can interleave between them on the event loop. The
+returned id is a hint, not a reservation.
+⚠️ If any repository is ever backed by an asynchronous store (e.g. a
+database), this guarantee breaks. Splitting allocation into a separate
+peek + create across an await boundary lets a concurrent caller consume
+the previewed id first, introducing a race that does not exist today.
 
-The audit log middleware (`src/middleware/auditLog.ts`) captures recent mutating requests (`POST`, `PUT`, `PATCH`, `DELETE`) in an in-memory bounded ring buffer.
+Required guardrails for any async-backed repository:
 
-#### Captured Fields
-- `method`: HTTP method
-- `path`: Request path (without query parameters)
-- `status`: Response status code
-- `requestId`: Correlation ID from `x-request-id` response header
-- `timestamp`: ISO timestamp of request completion
-
-#### Sensitive Data Redaction & Security Guarantees
-- **Strict Redaction via Denylist**: Any header, body parameter, or metadata stored in audit log entries is processed through `redactSensitiveData()`.
-- **Denylisted Fields**: Secret-bearing keys such as `x-api-key`, `authorization`, `cookie`, `set-cookie`, `token`, `access_token`, `refresh_token`, `secret`, `password`, `bearer`, `private_key`, `client_secret` are matched case-insensitively and replaced with `"[REDACTED]"`.
-- **Preventing Plaintext Exposure**: Under no circumstances should raw credentials or API keys be captured or retained in plaintext in the in-memory audit ring buffer or exposed via `GET /api/v1/audit`.
+Allocate ids atomically inside a single transactional/atomic operation
+rather than a separate peek + generate.
+Never use peekNextId() to reserve an id across an await.
+The synchronous-only contract is locked in by a test in
+src/repositories/settlementRepository.test.ts (preview … immediate create). That test must remain green; treat its failure as a signal that a
+non-atomic change to id allocation has been introduced.

src/repositories/inMemoryRepository.ts

@@ -42,7 +42,20 @@ export abstract class InMemoryRepository<K, T> {
     this.items.clear();
   }
 
-  /** Generates and returns the next numeric id, incrementing the counter. */
+  /**
+   * Generates and returns the next numeric id, incrementing the counter.
+   *
+   * NOTE (async-swap risk): `generateId()` and `peekId()` together form an
+   * id-allocation scheme whose atomicity depends entirely on the synchronous,
+   * single-threaded nature of this in-memory store. `peekId()` exposes the
+   * id that `generateId()` will hand out next, but performs no locking. This
+   * is safe today because Node serializes all synchronous code, so no other
+   * mutation can occur between a `peekId()` and the caller's `generateId()`.
+   * If a repository built on this base is ever swapped for an async-backed
+   * (e.g. database) store, this assumption breaks and id allocation must be
+   * made atomic (a single transactional allocation call) rather than a
+   * separate peek + generate. See `docs/ARCHITECTURE.md`.
+   */
   protected generateId(): number {
     const id = this.nextId;
     this.nextId += 1;

src/repositories/settlementRepository.test.ts

@@ -23,6 +23,47 @@ describe("SettlementRepository", () => {
     expect(repo.peekNextId()).toBe(3);
   });
 
+  describe("save anchor reindex", () => {
+    it("rebuilds the anchor index when save() changes the anchor", () => {
+      const repo = new SettlementRepository();
+      const created = repo.create(draft("anchorA", 100)); // id 1, indexed under anchorA
+
+      repo.save({ ...created, anchor: "anchorB" }); // anchor changes
+
+      expect(repo.get(created.id)?.anchor).toBe("anchorB");
+      expect(repo.byAnchor("anchorA")).toHaveLength(0);
+      expect(repo.byAnchor("anchorB")).toHaveLength(1);
+    });
+  });
+
+  describe("peekNextId", () => {
+    it("previews the id that the immediately following create() yields", () => {
+      const repo = new SettlementRepository();
+
+      const previewed = repo.peekNextId();
+      const created = repo.create(draft("anchorA", 100));
+
+      // Locks in the synchronous-only guarantee: peek -> create (no await in
+      // between) must return the exact id that was previewed. If this ever
+      // regresses (e.g. an async boundary is introduced between peek and
+      // create) the contract documented on SettlementRepository.peekNextId()
+      // would be violated.
+      expect(created.id).toBe(previewed);
+      // The counter has advanced exactly once after the create.
+      expect(repo.peekNextId()).toBe(previewed + 1);
+    });
+
+    it("yields a stable preview when no create() runs in between", () => {
+      const repo = new SettlementRepository();
+      repo.create(draft("anchorA", 100));
+
+      const first = repo.peekNextId();
+      const second = repo.peekNextId();
+      expect(first).toBe(second);
+      expect(first).toBe(2);
+    });
+  });
+
   it("saves status changes", () => {
     const repo = new SettlementRepository();
     const created = repo.create(draft("anchorA", 100));

src/repositories/settlementRepository.ts

@@ -5,10 +5,36 @@
 import { Settlement, isSettlementStatus } from "../models/settlement";
 import { InMemoryRepository } from "./inMemoryRepository";
 
-export class SettlementRepository extends InMemoryRepository<number, Settlement> {
+export class SettlementRepository extends InMemoryRepository<
+  number,
+  Settlement
+> {
   /** Secondary index: anchor -> set of settlement ids */
   private readonly anchorIndex: Map<string, Set<number>> = new Map();
-  /** Returns the id that will be assigned to the next created settlement. */
+  /**
+   * Returns the id that will be assigned to the next `create()` call.
+   *
+   * ⚠️ SYNCHRONOUS-ONLY SAFETY GUARANTEE — DO NOT RESERVE ACROSS AN AWAIT.
+   *
+   * This method MUST NOT be used to predict or reserve an id across an
+   * awaited (asynchronous) boundary. Calling `peekNextId()` and then acting on
+   * the returned value is safe *only* because `create()` is itself fully
+   * synchronous: Node's single-threaded event loop serializes all synchronous
+   * code, so no other `create()` can run "in between". No locking is performed
+   * — the previewed id is a hint, not a reservation.
+   *
+   * If this repository is ever swapped for an async-backed store (e.g. a real
+   * database), this guarantee breaks: `peekNextId()` and `create()` would no
+   * longer execute atomically, and a concurrent caller could consume the
+   * previewed id before the caller reaches `create()`. In that future design,
+   * id allocation must be performed inside a single transactional/atomic
+   * operation rather than split across a `peek` + `create`.
+   *
+   * For the current in-memory, synchronous store, `peekNextId()` immediately
+   * followed by `create()` is guaranteed to yield the previewed id. See
+   * `src/repositories/settlementRepository.test.ts` for the locked-in test and
+   * `docs/ARCHITECTURE.md` for the persistence-swap risk note.
+   */
   peekNextId(): number {
     return this.peekId();
   }
@@ -49,7 +75,6 @@ export class SettlementRepository extends InMemoryRepository<number, Settlement>
       this.anchorIndex.set(settlement.anchor, newSet);
     }
     return this.upsertByKey(settlement.id, settlement);
-
   }
 
   /** Returns the settlement with `id`, or `undefined`. */
@@ -101,4 +126,3 @@ export class SettlementRepository extends InMemoryRepository<number, Settlement>
     return result;
   }
 }
-