Repository navigation
feat(fees): record every fee quote with the policy behind it (W085) - #260
Conversation
GET /fees tells a client what to charge, and the fee then rides on-chain in the user's own transaction. Nothing on this side recorded that it happened, so there was no answer to what had been charged, in which asset, on which day, or under which policy — and a misconfigured percent, floor or cap would overcharge every user until someone read a block explorer. - FeeQuote: one row per asset per quote — amount priced against, fee, the rule that decided it (percent | floor | cap | flat | no_amount), the fee as a percent of the amount, an alarm flag, and the policy id. No IP, user agent or sender address; the only address is the published destination. - FeePolicy: keyed by a hash of the policy, so a change is a new row with its own firstSeenAt and every quote names the policy that priced it. The policy is taken from the entry the client was sent, never re-read from env. - Recording runs after the response and never throws: a ledger outage is logged and counted (veil_fee_quote_record_failures_total), not a failed send. - Alarm: a fee above VEIL_FEE_ALARM_PERCENT (default 10) of its amount flags the row, increments veil_fee_quote_alarms_total and logs a line — raised before the write, so a database outage cannot silence it. - GET /fees/report?from=&to=: per UTC day and asset — quotes, fees quoted, alarms, policies in force, and what the destination actually received per the index (null, with a reason, when the asset is not indexed). Behind VEIL_FEE_REPORT_TOKEN; 503 when unset. Documented as x-internal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvL6Yz2Zfg7spM1F9DXXtz
|
@JemimahEkong 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! 🚀 |
|
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. |
…t schema Conflicts in src/openapi/build.ts and src/openapi/schemas.ts: this branch added /fees/report while main added /fees (Miracle656#215 area) and the gated /candles route at the same place. Both kept. openapi.json regenerated from the merged source in a clean environment (npm run docs:openapi): identical to main's plus /fees/report. clients/react-query/src/schema.d.ts regenerated from it. The branch had changed openapi.json without regenerating the client, which fails the "Generate, typecheck & test" job (git diff --exit-code src/schema.d.ts). Verified: typecheck, tsc --noEmit, build; jest 63 suites / 961 tests (the openapiCoverage failure noted in the PR description is gone); clients/react-query generate (no diff), typecheck, test, build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvL6Yz2Zfg7spM1F9DXXtz
Miracle656
left a comment
There was a problem hiding this comment.
Good PR, and it very nearly goes in — one missing piece, and it's the same one that bit #210 yesterday.
The migration is missing
prisma/schema.prisma gains two models, FeePolicy and FeeQuote, and nothing under prisma/migrations/ comes with them. This repo commits its migrations (there are nine now) and neither build (tsc) nor start (node dist/index.js) runs one — there's no render.yaml either. So on a deployed instance those tables simply never exist.
Please generate it with Prisma's own tooling rather than hand-writing the DDL — prisma migrate dev --name add_fee_ledger against a local database. I'd rather not write it for you here: two CREATE TABLEs plus the @db.Decimal(38,7) / @db.Decimal(20,7) columns, the @default(autoincrement()) and the two indexes have to match exactly what Prisma expects, and a hand-written migration that drifts makes every future migrate fail in a way that's much harder to diagnose than this.
Also worth knowing: a schema.prisma change in this repo means running npx prisma generate before typecheck tells you the truth. Without it I saw three phantom errors (Property 'feePolicy' does not exist on type 'PrismaClient') that vanished after regenerating — so if you saw those, they weren't real.
Severity, so you know where this sits
Lower than #210, and that's by your design. The write is fire-and-forget:
// Recorded after the answer is sent, and never awaited: the person is in
// the middle of a payment, and a ledger outage must not become a failed
// send. `recordFeeQuotes` cannot throw.
void recordFeeQuotes(...)and recordFeeQuotes catches. So a missing table degrades to "no fee is recorded" rather than "/fees breaks" — the endpoint keeps answering and nobody's payment fails. That reasoning is right and the comment earns its place. It does mean, though, that without the migration the feature would silently do nothing in production, which is the quiet failure worth avoiding.
What I liked
- Content-addressing the policy.
idas a hash of the canonical policy JSON means the same configuration is one row however many quotes it prices, and any change to percent, floor, cap, flat or destination is automatically a new row with its ownfirstSeenAt. That's what makes a historical fee explainable rather than merely logged, and it needs no versioning scheme to maintain. - The privacy note is accurate, and I checked it rather than assuming. No IP, no user agent, no sender address; the only address is the fee destination, which
/feesalready publishes to anyone who asks. On a Nigerian rail under the NDPA that distinction matters, and writing down why the table is safe is better than it merely happening to be. @@index([network, quotedAt])and@@index([network, alarm, quotedAt])— the second one means "show me the quotes that tripped the alarm" stays cheap as the table grows, which is the query someone will actually run at 2am.- Recording
basis(percent / floor / cap / flat / no_amount) rather than just the number, so a fee can be explained without re-deriving it.
Verified on a merge onto current main: clean merge, and after npx prisma generate, tsc --noEmit silent with 1,036 tests passing across 65 suites.
Add the migration and re-request review — I'll merge it straight away.
…er migration Brings the branch up to date after Miracle656#261 (one Linq HTTP client in src/linq/http.ts). This change never touched the Linq clients, so the only conflict was .env.example, where both sides appended a section: upstream's NGN rate-limit and runtime-settings blocks are kept, then the fee ledger's. Adds prisma/migrations/20261022120000_add_fee_ledger so the FeePolicy and FeeQuote tables are in the migration history alongside the schema (the schema itself is still applied by the guarded db push). openapi.json, docs/openapi.json and the react-query schema regenerate with no drift. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LvL6Yz2Zfg7spM1F9DXXtz
|
Merged to Everything I liked on the first pass still holds, and I re-checked rather than carrying it forward:
Two fixes on the way in: 1. Duplicate schema declarations. #253 landed before this and added 2. wraith: 80 suites, 1,267 tests passing, typecheck clean, and the openapi coverage guard is green (it's the thing that catches a documented-but-unregistered route, so it's worth naming). |
W085 · Record what the platform fee actually charged
Closes #242
Problem
GET /feestells a client what to charge and where to send it. The fee then rides on-chain in the user's own transaction, and nothing on this side recorded that it happened. So there was no answer to how much had been charged, in which asset, on which day, or under which policy. Because the fee is now policy (percent, floor and cap) rather than a constant, a misconfiguration would overcharge every user until someone read a block explorer.What changed
FeeQuote, one row per asset per quote. Each row holds the network and asset, and the amount priced against (null for clients that send none). It also holds the fee, the rule that decided it (percent | floor | cap | flat | no_amount), the fee as a percentage of the amount, an alarm flag and the policy id.FeePolicy, keyed by a hash of one asset's policy: mode, percent, floor, cap, flat amount, asset and destination. A policy change is a new row with its ownfirstSeenAt, and every quote names the policy that priced it. The policy comes from the exact entry the client was sent, not from re-reading environment variables, so the record cannot disagree with what the user was told.veil_fee_quote_record_failures_total; it never becomes a failed send for someone mid-payment. Each policy is written once per process, withskipDuplicatescovering races between instances.VEIL_FEE_ALARM_PERCENTof its amount (default 10, the rate the fee code's own comments call stark) does three things: it flags the row, incrementsveil_fee_quote_alarms_total{network,asset}and logs a line. The alarm is raised before the database write, so an outage cannot silence an overcharge.GET /fees/report?from=&to=(UTC dates, inclusive, at most 92 days). Per day and asset it returns the quote count, total fees quoted, alarm count and the policy ids, plus the full policies withfirstSeenAt.collectedisnullwith a note, because zero would be false.VEIL_FEE_REPORT_TOKEN, compared in constant time. It answers 503 when the token is unset, so it is never served open by accident.x-internal, andopenapi.jsonis regenerated..env.exampleanddocker-compose.ymlgain the two new variables. The existing compose guard test requires both.Nothing recorded identifies a person
No IP address, user agent or sender address is stored. The only address anywhere in the ledger is the fee destination, which
/feesalready publishes. One test pins the exact column set of a quote row. Another sends a request carrying an address, a user agent andX-Forwarded-For, and asserts none of them reaches the database.Acceptance criteria
/feesquote that returns a fee is recorded with the policy that produced it. Tested end to end through the route, field for field against what the client was sent.VEIL_FEE_PERCENTproduces new policy rows, and the quotes before and after name different policies.Validation
npm test: 763 of 764 passing. The single failure is the pre-existingopenapiCoveragetest (get /feesundocumented), which is W082's job and is unchanged by this PR. The new/fees/reportroute is documented, so it does not add to that list.src/__tests__/feeLedger.test.ts.npm run typecheck,npx tsc --noEmitandnpm run buildall pass.npx prisma validatepasses.mintevents, cut the day boundary at exactly midnight UTC, and sum a maximum-size i128 amount without overflow. Prisma's own engines couldn't be downloaded in my environment, so this exercised the SQL, not the Prisma client.