Repository navigation
feat(ngn): fake provider for end-to-end naira rail tests & rate limits on money-moving routes - #263
Conversation
Issue Miracle656#246: Fake provider for naira rails testing - Create fakeLinq.ts: single in-process fake implementing all 8 Linq B2B endpoints with documented shapes and error codes - Create E2E test: full happy path (provision, KYC, rate, order, status, webhook, settle) plus failure at each step using documented codes - Append fake-based shape tests to onramp.test.ts and bills.test.ts - Fix bills.ts call() to forward error code and Retry-After header Issue Miracle656#245: Rate limits on routes that move money - Add per-IP and per-reference rate limiters to all POST creating routes - Our refusal uses veil_rate_limited, distinguishable from provider's rate_limited - All limits configurable via env vars without a deploy - Tests prove per-IP and per-reference limits refuse past threshold
|
@teethaking Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
provisionCustomer catches the 409 and adopts the existing reference internally, returning 200 with reusedExistingRef: true. The test was wrongly expecting a 409 to reach the client.
|
Rebase needed — #261 landed, consolidating the two Linq HTTP clients into a single What changed, so the rebase is predictable:
Also worth knowing: Push and I will pick it up. |
Miracle656#261 moved every Linq request into the shared client in src/linq/http.ts, and main's bills.ts aliases it with `const call = linqCall`. The merge kept both that alias and the branch's old hand-rolled call(), which typechecked only against the client it no longer has: duplicate 'call', and CallInit, BASE_URL, TIMEOUT_MS and apiKey all gone. Same broken image build took down Integration tests and the chaos stack with it.
Every suite passes and jest prints its summary in ~30s, then hangs forever: multiNetworkIndexer.test.ts calls startAllIndexers(), whose loops are never stopped, so the process keeps open timers. CI proved it — main's own Typecheck & build ran npm test for six hours until GitHub cancelled it (4d04e0f, cancelled 10:58->16:58), and the same hang reproduces on a clean main tree locally. Results are already final when jest wants to exit, so --forceExit ends the run without hiding failures; the proper teardown fix belongs to that test's owner on main.
…live loop The multiNetworkIndexer suite started a real background loop that kept crash-restarting every 10s for the rest of the run — jest never exited on its own and the spam starved later suites into timeouts on CI. Loops now carry a stop flag, restart timers are cancellable, and the suite stops what it started in afterAll.
…es do The stale-read middleware probes getLatestLedger on every GET, so this suite paid a live network round-trip on CI and its first test blew the 5s budget. exports/transfers route tests already mock ../../rpc; accounts was the one that forgot.
Miracle656
left a comment
There was a problem hiding this comment.
Needs a rebase — src/indexer.ts conflicts with work that landed after you branched, in three places around loop lifecycle:
<<<<<<< HEAD
for (const loop of loops.values()) loop.stopped = true;
for (const timer of restartTimers.values()) clearTimeout(timer);
=======
for (const timer of restartTimers) clearTimeout(timer);
main now tracks loops in a map with a stopped flag and keyed restart timers; your branch has the older set-based timers. Take main's side for the lifecycle and reapply your changes on top — the newer version exists so a loop cannot outlive _resetIndexerLoops and keep polling between test cases.
Also note #251 landed and corrected the native XLM SAC constants in that same file. DEFAULT_XLM_SAC_MAINNET was holding testnet's SAC and DEFAULT_XLM_SAC_TESTNET a contract that is neither network's, so mainnet native XLM was never being indexed. Make sure the rebase keeps main's values and the derivation test beside them.
Both halves of this are things I want
The fake provider is the one that matters most. Linq has no sandbox — every call against the real API moves real money, which is why the whole /ngn surface is mainnet-gated at the router. That makes a canonical fake the only way to exercise the rails end to end, and making it the single source of truth for response shapes is better than the per-test fetch mocks it replaces: ad-hoc mocks drift apart, and then each test is asserting against a slightly different idea of what the provider sends.
Rate limits on the creating routes is the other half, and it is overdue. POST /ngn/onramp/orders is the one route with no local limiter — we established that when a tester hit a 429 and we could not tell Linq's rate limit from our own, because both matched the same regex. Worse, the KYC route spends a one-per-lifetime NIN attempt: an unthrottled route there does not cost quota, it costs a person their ability to verify.
So the shape is right. Rebase on current main and I will review it properly — nothing is queued ahead of it.
Integration tests is red on main itself (Postgres read path, since at least 10-16) and is not yours.
Miracle656
left a comment
There was a problem hiding this comment.
The feature is one I want badly, and I'll say why below — but the branch doesn't build, and the cause is the same mistake in two files.
Duplicate merges, in indexer.ts and accounts.test.ts
src/indexer.ts declares restartTimers twice:
154:const restartTimers = new Map<Network, ReturnType<typeof setTimeout>>(); // yours
252:const restartTimers = new Set<NodeJS.Timeout>(); // already on main
and then runs both cleanups back to back in _resetIndexerLoops:
for (const timer of restartTimers.values()) clearTimeout(timer); // Map form
restartTimers.clear();
for (const timer of restartTimers) clearTimeout(timer); // Set form
restartTimers.clear();main already had a crash-restart timer registry. Your branch adds a second one beside it instead of using it, which is a duplicate identifier and, at line 703, an unbalanced brace:
src/indexer.ts(703,1): error TS1005: '}' expected.
src/__tests__/routes/accounts.test.ts has the same shape. You add:
jest.mock("../../rpc", () => ({
getLatestLedger: jest.fn().mockResolvedValue(1_000_000),— unclosed — immediately above the jest.mock("../../rpc", ...) that is already there on main, with the same purpose and a fuller comment explaining the stale-read middleware's RPC probe. That one even uses ...jest.requireActual, which yours drops. The file stops parsing at line 22.
Both look like a merge where main's version and your version were both kept rather than reconciled. Worth re-reading the diff for your own branch against current main before pushing — git diff main...HEAD on the two files makes it obvious, and npm run typecheck finds it in one command without reading anything.
I removed the accounts.test.ts fragment locally to see what was behind it, and indexer.ts was waiting.
The feature is genuinely valuable, so please do come back to it
A fake Linq provider is the single most useful test fixture this repo could have. Linq has no sandbox — it is mainnet-gated, so every real exercise of the naira rails moves actual money against a real NIN-verified customer. That has meant the offramp, onramp and bills paths are the least testable code in the repo and simultaneously the only code that can lose someone's money. An in-process fake that speaks the provider's shapes changes what we can assert without spending naira to find out.
Rate limits on money-moving routes are the other half, and they matter more than usual here because all Veil users share one provider key. I hit 429 rate_limited from four manual probes of /ngn/onramp/rate while reviewing another PR today, so the allowance is tighter than it looks and protecting it at our own edge rather than discovering it at Linq's is right.
Two things to know when you rebase, both landed today:
POST /ngn/onramp/ordersnow refuses any coin butxlm(coin_not_available), because Linq told us they hold liquidity for XLM and not USDC.GET /ngn/onramp/ratedeliberately still quotes both — the mobile client probes it as a liveness check. If your fake provider models the coin set, model that asymmetry too.POST /offramp/ordersnow stores arequestHashand answers a concurrent duplicate as a replay rather than a 500.
Integration tests is red on main (Postgres read path, since at least 10-16) and is not yours.
Re-request review once npm run typecheck is clean — I'll prioritise it.
The three merges from main left each side stacked rather than merged: indexer.ts declared restartTimers as a Map beside main's Set and ran both cleanups back to back, and the poll loop grew a while(true) with no close (the TS1005 at the last line); accounts.test.ts gained an unclosed jest.mock above main's fuller one. Fold the branch's stopped-loop teardown into main's single timer registry — one declaration, one cleanup, one poll condition — so typecheck and the suite parse again. 🤖 Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
|
Merged. This is a lot of good work — 675 lines of fake provider and a 602-line end-to-end suite, and it takes the test count from 1,026 to 1,070. The duplicate Verified before merging: One line changed on the way in: I dropped I'd rather not keep it, and the reason is specific to this PR: One consequence worth noting: #282 asks for something that proves the fake provider still matches the real one. Until today that was hypothetical. Now that this fake exists and the e2e suite depends on it, it is the natural follow-up — a fake that drifts from the real Linq is worse than no fake, because it passes. |
…or shape, and the fake pinned tsc --noEmit clean jest 75 suites, 1154 tests passing #326 resolve NGN and offramp orders by strict network (#293) #327 one error shape on /ngn and /offramp behind a stable code (#287) #330 detect schema drift and audit migration history at boot (#277) #333 harden cache, JSON:API envelope, offramp fingerprint (#322 #321 #319) #303 pin fake provider shapes as fixtures, add drift detection (#282) #326 closes the defect where reconcileNgnOrder resolved an order with findFirst on orderId alone while uniqueness is @@unique([network, orderId]) -- the id is not unique by design, so the database was choosing the row. #303 is the right follow-up to the fake provider landing in #263: a fake that drifts from the real Linq is worse than no fake, because it passes. One fix applied on merge. #327 and #333 disagreed about the error contract: #327 adds a stable machine-readable `code` alongside the human `error` message, and three of #333's new order-access-token assertions used strict `toEqual`, so the extra field failed them: - Expected - 0 + Received + 1 Object { + "code": "unauthorized", "error": "An order access token is required", } #327 is the intended direction -- that same test file already asserts `code: "internal_error"` on line 873 -- so the three assertions became `toMatchObject`, which keeps the message exact without forbidding the code. Neither PR was wrong; they were written in parallel against different contracts. NOT merged, changes requested: #304 (data bundle catalogue) adds a route without a spec entry, so openapiCoverage fails. #328, #331 and #332 conflict and need a rebase. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB
Summary
Two improvements for the Veil backend naira rails — a canonical fake provider module that is the single source of truth for provider response shapes (replacing per-test ad-hoc fetch mocks), with an end-to-end test walking the full onramp flow, and rate limits on the three creating routes that were unthrottled despite spending provider rate budget and consuming one-per-lifetime NIN attempts.
Closes #246
Closes #245
What changed
#246 — Fake Provider and End-to-End Naira Rail Tests
src/tests/fakeNgnProvider.ts (new — the single source of truth)
Existing suites migrated
End-to-end test (src/tests/ngnRailE2E.test.ts — new)
Failure-path tests
No test reaches the real provider
#245 — Rate Limits on Creating Routes
Rate limits added to
Design
Tests
How to verify
npm testChecklist