diff --git a/README.md b/README.md index c7f18f3..bc04935 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,13 @@ from and to are the same anchor. GET /api/v1/liquidity – list aggregated pools { pools: [{ asset, total, anchors }] } GET /api/v1/liquidity/entries – list raw per-anchor entries GET /api/v1/liquidity/:asset – aggregated pool for one asset (404 if none) +Route ordering (liquidity): GET /:asset is a catch-all that matches any +single path segment, so every static single-segment liquidity GET +(/entries, /withdrawals) and /anchors/:anchor must stay registered before +it in src/routes/liquidity.ts. Express matches routes in registration +order; swapping them would silently make GET /api/v1/liquidity/entries +resolve as getPool("ENTRIES"). Regression tests in +src/routes/liquidity.test.ts pin this ordering. DELETE /api/v1/liquidity/:anchor/:asset – administratively remove an anchor's entire entry (404 if none). This bypasses reserved-liquidity accounting checks, so operators should first confirm that no pending diff --git a/src/openapi.ts b/src/openapi.ts index 559144f..b855602 100644 --- a/src/openapi.ts +++ b/src/openapi.ts @@ -62,7 +62,14 @@ export function buildOpenApiSpec(): Record { }, }, "/api/v1/liquidity/entries": { - get: { summary: "List raw per-anchor liquidity entries" }, + get: { + summary: "List raw per-anchor liquidity entries", + description: + "Returns { entries: [...] }. This static path is registered before the " + + "catch-all GET /api/v1/liquidity/{asset}; that ordering is load-bearing, " + + "since reversing it would make this path resolve as a pool lookup for an " + + 'asset named "ENTRIES".', + }, }, "/api/v1/liquidity/withdrawals": { diff --git a/src/routes/liquidity.test.ts b/src/routes/liquidity.test.ts index ee4f39b..33ee25d 100644 --- a/src/routes/liquidity.test.ts +++ b/src/routes/liquidity.test.ts @@ -218,6 +218,61 @@ describe("liquidity routes", () => { expect(res.body.total).toBe(500); }); + it("the /entries static route takes precedence over /:asset", async () => { + const app = createApp(); + // With no liquidity recorded, a /:asset lookup for "ENTRIES" would 404. + // Because the static /entries route is registered before the catch-all + // /:asset, GET /entries must resolve to the entries handler and return + // 200 with an entries array — not be swallowed by the asset lookup. + const res = await request(app).get("/api/v1/liquidity/entries"); + + expect(res.status).toBe(200); + expect(Array.isArray(res.body.entries)).toBe(true); + expect(res.body).toEqual({ entries: [] }); + // Guard against the pool shape returned by getPool("ENTRIES"). + expect(res.body).not.toHaveProperty("asset"); + expect(res.body).not.toHaveProperty("total"); + expect(res.body).not.toHaveProperty("error"); + }); + + it("keeps /entries resolving to the entries list even with liquidity recorded", async () => { + const app = createApp(); + await request(app) + .post("/api/v1/liquidity") + .send({ anchor: "anchorA", asset: "USDC", amount: 500 }); + + const res = await request(app).get("/api/v1/liquidity/entries"); + + expect(res.status).toBe(200); + expect(res.body.entries).toHaveLength(1); + expect(res.body.entries[0]).toMatchObject({ + anchor: "anchorA", + asset: "USDC", + amount: 500, + }); + expect(res.body).not.toHaveProperty("total"); + }); + + it("still resolves /entries to the entries list when an asset named ENTRIES exists", async () => { + const app = createApp(); + // Worst case for a swapped registration order: an asset literally named + // "ENTRIES" exists, so /:asset would return a 200 pool-shaped body and the + // shadowing bug would be invisible to a status-code-only assertion. + await request(app) + .post("/api/v1/liquidity") + .send({ anchor: "anchorA", asset: "ENTRIES", amount: 42 }); + + const res = await request(app).get("/api/v1/liquidity/entries"); + + expect(res.status).toBe(200); + expect(Array.isArray(res.body.entries)).toBe(true); + expect(res.body.entries).toHaveLength(1); + // Pool shape ({ asset, total, anchors, lastUpdated }) must NOT be returned. + expect(res.body).not.toHaveProperty("total"); + expect(res.body).not.toHaveProperty("anchors"); + expect(res.body.entries[0]).toMatchObject({ asset: "ENTRIES", amount: 42 }); + }); + it("starts with an empty withdrawal history", async () => { const res = await request(createApp()).get("/api/v1/liquidity/withdrawals"); diff --git a/src/routes/liquidity.ts b/src/routes/liquidity.ts index 1059884..e6989af 100644 --- a/src/routes/liquidity.ts +++ b/src/routes/liquidity.ts @@ -31,7 +31,22 @@ export function liquidityRouter(service: LiquidityService): Router { res.json({ pools: service.listPools() }); }); - // List raw per-anchor entries. + // --------------------------------------------------------------------- + // ROUTE ORDER IS LOAD-BEARING. + // + // Every static single-segment GET below (`/entries`, `/withdrawals`) MUST + // stay registered BEFORE the catch-all `GET /:asset`. Express matches + // routes in registration order, so if `/:asset` were moved (or a static + // route moved after it), a request to `/api/v1/liquidity/entries` would be + // matched as `getPool("ENTRIES")` and return 404 (or, worse, a pool object + // if an asset literally named "ENTRIES" existed) instead of the entries + // list. Do not reorder these `router.get(...)` calls; the regression tests + // in `liquidity.test.ts` ("the /entries static route takes precedence over + // /:asset") fail if they are swapped. + // --------------------------------------------------------------------- + + // List raw per-anchor entries. Registered before the catch-all GET /:asset + // so it is never shadowed by a single-segment asset lookup. router.get("/entries", (_req: Request, res: Response) => { res.json({ entries: service.listEntries() }); }); @@ -54,6 +69,11 @@ export function liquidityRouter(service: LiquidityService): Router { }); // Read the aggregated pool for a single asset. + // + // CATCH-ALL: this parameterized route matches ANY single path segment, so it + // must remain the LAST GET registration in this router. Registering it above + // `/entries`, `/withdrawals`, or `/anchors/:anchor` would silently shadow + // them. See the ordering note above `router.get("/entries", ...)`. router.get("/:asset", (req: Request, res: Response) => { res.json(service.getPool(req.params.asset)); });