feat(intents): reject over-cap purchases before payment - #808
Conversation
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6655cb7. Configure here.
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9e1ef96. Configure here.
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f3deb9e. Configure here.
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 24bacc1. Configure here.
24bacc1 to
fe61b60
Compare
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit fe61b60. Configure here.
✅ Deploy Preview for auto-drive-storage ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
An intent locked a price per byte and recorded nothing about the amount, so the size was reverse-derived after payment as paymentAmount / shannonsPerByte. That left the per-user credit cap enforceable only once the money was already on-chain and irreversible — resolution being an OVER_CAP row and a manual refund — and left nothing for a fixed-price quote to be priced against. Intents now optionally carry the size they were created for: - new nullable intents.quoted_bytes (numeric(78,0)), wired through the model, the row mapper, and both the INSERT and the UPDATE - POST /intents accepts an optional requestedBytes, validated and persisted - a creation-time cap pre-check that rejects a purchase with no headroom before the user pays, as CREDIT_CAP_EXCEEDED with the cap and balance in the message Deliberately unchanged: credit derivation is still paymentAmount / shannonsPerByte, and createPurchasedCreditWithCapCheck remains the authoritative cap gate. quoted_bytes is recorded, not yet authoritative. The new pre-check is a fast-fail, not a reservation — two concurrent intents can both pass it, which is why the advisory-lock-protected check stays where it is. requestedBytes is optional because it has to be: a request without it behaves exactly as before, which is what the live frontend sends. Two things worth a reviewer's attention: - res.json() throws on a raw BigInt, and the three intent response sites stringified two of the six bigint fields while spreading the whole object. quotedBytes is the first such field that will actually be set, so GET /intents/:id would have thrown on every sized intent. Replaced with one serializeIntent helper, which also covers the token_* fields that are latently broken today only because nothing writes them yet. - validation is split: parseRequestedBytes handles the wire shape (decimal string canonical, JSON number accepted while a safe integer so no lossy float reaches the byte path), createIntent handles range, so a caller reaching the use case directly is covered by the same rules. Prerequisite for the USDC credit derivation, which must read quoted_bytes directly — inverting usdRateAtCreation instead would over-credit by the pool fee, slippage and quote margin, since the user pays the executable quote while that rate stores marginal spot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous wording said the USDC flow would make quoted_bytes the source of truth for its own credit derivation. That is not the design. Credits follow the amount actually paid, for both assets: AI3 paymentAmount / shannonsPerByte USDC tokenAmount / (quotedTokenAmount / quotedBytes) quoted_bytes is an input, not an output — the size the Uniswap quote and the creation-time cap pre-check were computed for, and the denominator of the locked USDC price per byte. It equals the bytes granted only when the user pays exactly what they were quoted. That reframing makes it a firmer requirement rather than a weaker one: a v4 quote prices a SPECIFIC size, because slippage is a function of the amount, so a sizeless USDC intent has no rate to lock. Optional stays correct for AI3 and only for AI3. Also corrected: usdRateAtCreation's comment described itself as the input to "the existing proportional credit math", which is exactly the ~8% over-credit the executable quote exists to avoid — it stores marginal spot while the user pays fee + impact + margin on top. Marked display-and-reconciliation only. Documents the accepted consequence of paying-amount-drives-credits: an overpayment grants more than the pre-check approved, and slightly more than proportionally on USDC, since the locked rate carries the price impact of the quoted size. Left to the authoritative check to absorb — it re-measures the real balance under the advisory lock, so an overpayment can walk an account up to the cap but never past it, landing the excess in OVER_CAP as before. The down migration's "safe to reverse" claim is now scoped: once USDC intents exist, dropping the column destroys the rate denominator for every PENDING one. Comments and one OpenAPI description. No behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous comment gave it as tokenAmount / (quotedTokenAmount / quotedBytes). That is the right identity but the wrong instruction: quotedTokenAmount / quotedBytes as a bigint truncates to whole USDC base units per byte, which is 0 for any realistic price, so an implementer transcribing it literally divides by zero or credits nothing. Stated as tokenAmount * quotedBytes / quotedTokenAmount, with a note on why the rate is never materialised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ivation
The previous two commits corrected a real bug — deriving credits by inverting
usd_rate_at_creation over-credits by fee + impact + margin — but wrote the fix as
a structural change, giving USDC its own derivation
(tokenAmount * quotedBytes / quotedTokenAmount) alongside the AI3 one. That was
an over-correction. The defect was the rate, not the shape.
The two are algebraically the same number. With the effective rate defined as
quotedTokenAmount against quotedBytes * shannonsPerByte:
shannons = tokenAmount * quotedBytes * shannonsPerByte / quotedTokenAmount
bytes = shannons / shannonsPerByte
= tokenAmount * quotedBytes / quotedTokenAmount
Identical credits either way, so the choice is purely structural — and a fork of
the money math is the wrong side of that trade. Converting at the boundary keeps
one derivation (shannons paid / shannonsPerByte) with the cap check, dust,
OVER_CAP and purchased_credits all seeing the AI3 flow they see today, and makes
a third payment asset a new conversion rather than a third derivation.
Documented accordingly: quoted_bytes is the AI3 leg of the locked effective rate,
the size the pool was quoted for, and the cap pre-check basis — an input, never a
byte count handed out.
usd_rate_at_creation keeps its warning, reframed: it is not the rate the
conversion uses, and it stays marginal on purpose so it remains comparable to the
market. The effective rate is a separate number.
No behaviour change; this branch contains no credit-derivation code and is
model-agnostic either way. Comments and migration notes only.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
quoted_bytes was persisted for three stated reasons, and on inspection only one of them needed a column at all: - the cap pre-check runs inside createIntent against the in-memory parameter - the USDC quote is computed in that same request, and its output is what gets stored, as quoted_token_amount - only the confirmation path — a different process, minutes later — would have read it, to rebuild the effective rate as quoted_token_amount / (quoted_bytes * shannons_per_byte) That third use is real but it is #747's, and #747 can serve it better by persisting the effective rate as its own column instead of reconstructing it from three. Which leaves a column that today is written by one code path, read by none, and shaped exactly like a balance it can never agree with — credits come from paymentAmount / shannonsPerByte, so any over- or underpayment makes quoted_bytes and the granted bytes diverge silently. Removed: the migration, the model field, the DBIntent column, the row mapper entry, both statement column lists, the OpenAPI property, and the four round-trip tests. Kept, because none of it depended on storage: - POST /intents still accepts requestedBytes, still validates it, and still rejects an over-cap purchase before the user pays. That is the feature. - serializeIntent, which fixes a real latent bug: res.json() throws on a raw BigInt and the three response sites stringified two of six bigint fields while spreading the whole object. The token_* fields are still latently broken today only because nothing writes them yet. - the usd_rate_at_creation warning, repointed: with no quoted_bytes to pair with quoted_token_amount, #747 has nothing on the type to recover an effective rate from and must persist one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…red quoter Three comments still explained the token fields in terms of the executable quote — a v4 Quoter call per intent, priced for one specific size. That design was removed from the epic on 2026-08-07 (#742): USDC is accumulated and converted manually, so nothing is swapped per intent and no per-size price impact exists anywhere in the codebase. Every mechanism these comments appealed to is gone, and two of them stated a consequence that no longer follows. - `quotedTokenAmount` claimed the pool swap fee and "the price impact of that specific size" were already inside it. Neither term exists now. What is durably true is the field's contract: it is what the user was asked to pay, so it is margin-inclusive and rounded up, and it is therefore not what credits derive from — those follow the amount actually received. - `usdRateAtCreation` called itself the pool's MARGINAL price and put the gap to the charged rate at "roughly 8% on a $290 purchase". The gap is now exactly USD_QUOTE_MARGIN, and a scalar rate makes it exactly proportional at any size. The conclusion the comment existed for survives untouched — #747 must persist its own effective rate, and nothing else on the type can stand in for it — so that is kept and the obsolete reasoning replaced. - `checkCapHeadroom` said overpayment grants "slightly more than proportionally" because a larger conversion "slips further down the curve". With a size-independent rate the drift is exactly proportional, on either payment method. Deliberately written against the field contracts rather than against whatever prices them: this is the third rate source in the epic's life (CEX aggregate → pool spot + Quoter → subgraph VWAP), and comments naming a mechanism have gone stale each time. The one source-specific detail left is a pointer to #746. No functional change. `yarn models build`, `yarn backend build` and `yarn backend lint` all clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The USDC path still has to require a size, but not for the reason this comment gave. #807 replaced the v4 Quoter with a subgraph VWAP of realized fills, which is size-independent — the pool is no longer quoted for the purchase. State what the size is actually for: the rate prices one byte, so the intent has to say how many. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
05f564c to
841c71b
Compare
The cap pre-check was optional everywhere, which left it advisory on the one path that most needs it. Make the size mandatory for USDC_ETH and keep it optional for AI3. The asymmetry is not stylistic. An AI3 intent locks a per-byte rate and lets whatever arrives on-chain settle it, so it can be created before anyone has decided how much to buy. A USDC intent has to name an amount to transfer, and that amount is the oracle's rate times the size — the rate prices one byte, size-independent since #807 — so without a size there is nothing to quote. Enforced in the use case rather than by making the field required on the endpoint: POST /intents is a documented API-key flow and every integrator on it today pays in AI3. Requiring it outright would break them to close a hole they cannot reach. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pre-check shipped with no caller, so the only thing standing between an
over-cap purchase and an irreversible payment was a disabled button. Pass the
selected size from step 3 and the server re-checks it before the user pays.
Two things fall out of making that path reachable:
- The 403 had nowhere to go. Step 3's catch was a bare console.error, on the
assumption wagmi would surface it, but writeError only covers the wallet
call — a createIntent failure made the button silently do nothing. Surfaced
as intentError.
- The client read only `message` off an error body, which is the shape coded
errors use; HttpError's default is { error: <message> }. Read either, or
every 400 would have displayed as its status text.
mibToBytes is shared with isMibOverCap so the client's own cap verdict and the
number the server re-checks come from one conversion, and it equals what the
payment grants: formatCreditsInMbAsValue applies the same 1024^2 before
shannonsPerByte, and credits are paymentAmount / shannonsPerByte.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 841c71b. Configure here.
jim-counter
left a comment
There was a problem hiding this comment.
A few low priority points.
… buyer Addresses review of #808. Three findings, all real, all in the path a user walks to spend money. **A raw exception reached the screen.** The new error-body passthrough forwarded the server's message for every non-ok status, and handleInternalErrorResult builds 500 bodies as `Failed to create intent: ${e.message}` — so a downed consensus WebSocket put `connect ECONNREFUSED 10.0.3.7:9944` under the Send button. Step 3 renders that string verbatim, since a failure before the wallet call has no other channel. Gated on which field is present rather than on the status: `message` at any status, `error` only below 500. The two body shapes are already distinct — coded errors send `{ error: CODE, message }`, the HttpError default sends `{ error: <the message> }` with no `message` key — so this passes exactly the messages written to be read. A plain `status < 500` gate would have been wrong in the other direction: the USDC quote path stacked on this branch answers an unreachable oracle with a 503 whose message the user does need. **A crafted URL threw a RangeError.** mibToBytes calls BigInt() with no integer guard, and the purchase page re-hydrates sizeMB from the query string with a coercion that accepts decimals, so ?step=3&sizeMB=0.5 surfaced `The number 0.5 cannot be converted to a BigInt`. Normalised once, in the component body, rather than rounded inside mibToBytes. Rounding there would have priced the payment at 0.5 MiB while cap-checking 1 MiB — the exact divergence that function exists to prevent, since Step 3 passes one size to both it and the AI3 pricing helper. Hoisting it out of handleSend also keeps the displayed amount and the charged amount derived from the same number, and the normalised size travels to the success screen so it reports what was bought. normaliseMib rejects on Number.isSafeInteger, not isFinite: ?sizeMB=1e308 and a 308-digit ?sizeMB=999…9 both survive rounding as finite doubles, and the pricing helper then evaluates mib * 1048576 to Infinity before its own BigInt() — the same failure one conversion earlier. A size that cannot be normalised now disables the button and says so up front, because this step has no back button and "the button does nothing" is not an explanation. **A test that could not fail.** `expect(Object.keys(created)).not.toContain( 'quotedBytes')` guarded a field removed in 2f519fb, and the PR cited it as the proof that the size is not persisted. The whole row is asserted instead: a key-name check cannot fail (TypeScript's excess-property check already rejects adding a size field), and a value check only holds while no legitimate field happens to equal the size — it would break the moment the mocked price became realistic. Verified by mutation: `paymentAmount: requestedBytes` fails the new assertion and passed the old one. Tests: every one of these was checked by reverting the fix and confirming the test goes red. Not covered — Step3_TransferTokens itself, which has no component test in this repo (no jsdom or Testing Library); only the extracted helper is under test, so the call site could still regress silently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Prerequisite step of epic #742 (USDC payments). Enforces the per-user credit cap before the user pays, instead of after an irreversible on-chain payment.
Problem
createIntenttakes no size, so the per-user cap is only enforceable after payment.onConfirmedIntent→addCreditsToAccountfails the cap check and parks the intent inOVER_CAPfor admin review — but the money is already on-chain and irreversible, so resolution is a manual refund or a cap bump.What this does
POST /intentsaccepts an optionalrequestedBytes(decimal string canonical; a JSON number is accepted while it is a safe integer).403 CREDIT_CAP_EXCEEDEDwith the cap and current balance inmessage.Why the size is optional on AI3 but required on USDC
Not a stylistic asymmetry — the two paths owe different things at creation. An AI3 intent can be created before anyone has decided how much to buy. A USDC intent has to name an amount to transfer, and that amount is the rate times the size.
Enforced in the use case rather than by making the field required on the endpoint:
POST /intentsis a documented API-key flow for third-party integrators and every one of them pays in AI3 today. Requiring it outright would break them to close a hole they cannot reach. The consequence, stated plainly: an AI3 caller using an API key can still omit the size and skip the pre-check. Closing that is a breaking API change and is not attempted here.A cap check already existed — client-side
GET /credits/summaryreturnscanPurchaseandmaxPurchasableBytes, and the purchase screen disables over-cap packages and blocks over-cap custom amounts. That is a greyed-out button, not a server-side gate, and it leaves three holes: API-key callers never load the screen;maxPurchasableBytesis a snapshot taken when the screen mounts, so a second tab or a stale page guards with the wrong number; and the client validates the package while credits come from what is actually paid.This PR is the server-side half.
Why no column
requestedBytesis a parameter, not a record. Three uses were considered and only one would ever have read a stored copy:createIntent, in-memoryquoted_token_amountThat third use is real, but it belongs to #747, and #747 is better served persisting the effective rate as its own column than reconstructing it from three others. What was left was a column written by one code path, read by none, and shaped exactly like a balance it can never agree with: credits come from
paymentAmount / shannonsPerByte, so any over- or underpayment makes a recorded size and the granted bytes diverge silently.The pre-check is a fast-fail, not a reservation
Nothing is held. Two concurrent intents for the same account can both pass it, and their sizes may sum past the cap. That is accepted — which is why the advisory-lock-protected check stays where it is, in
createPurchasedCreditWithCapCheck. A reservation would need an expiry sweep, release-on-failure and its own contention story; far more than the problem warrants.It measures the same aggregate as the authoritative check and uses the same
>boundary, so a purchase landing exactly on the cap is allowed by both. Not by coincidence — it reaches that query throughgetRemainingCreditsrather than restating it, because a pre-check that waved through purchases the real check then rejects is the precise failure this exists to prevent.It also does not bound what the user ends up with, because credits follow the payment, not
requestedBytes. Paying more than quoted grants more than was pre-checked. That drift is accepted by design, and the authoritative check bounds it: re-measuring the real balance under the advisory lock, an overpayment can walk an account up to the cap but never past it, with the excess landing inOVER_CAPexactly as an unchecked purchase would.Also in here: a latent BigInt serialization bug
res.json()throws on a raw BigInt, and the three intent response sites stringified two of the six bigint fields while spreading the whole object. Replaced with oneserializeIntenthelper. Thetoken_*fields are latently broken today only because nothing writes them yet — this fixes them before #747 makes them reachable.Two more bugs, surfaced by making the 403 reachable
Both are in the frontend, and both were invisible until a
createIntentcall could actually fail:console.errorwith a comment saying wagmi would surface it — butwriteErroronly covers the wallet call, so a failure before that made the button silently do nothing. Now surfaced.messageoff the error body, which is the shape coded errors use;HttpError's default is{ error: <message> }. Reading onlymessagewould have shown every 400 as its status text.mibToBytesis shared withisMibOverCapso the client's own cap verdict and the number the server re-checks come from one conversion — and it equals what the payment grants, sinceformatCreditsInMbAsValueapplies the same 1024² beforeshannonsPerByteand credits arepaymentAmount / shannonsPerByte.Validation is split by concern
parseRequestedByteshandles wire shape;createIntenthandles range, so a caller reaching the use case directly (i.e. #747) gets the same rules. A decimal string is canonical — every other size on an intent already crosses the wire as one, and it keeps working if the cap ever moves past 2^53. A JSON number is accepted while it is a safe integer, so no lossy float reaches the byte path and an oversized number is told to use a string rather than silently rounded.Forward note for #747
usd_rate_at_creationstores the raw rate the oracle reported. The user is charged that rate plusUSD_QUOTE_MARGIN, so converting a received payment back to AI3 at the raw rate returns the margin as free storage on every purchase — the whole margin, exactly, since a rate is a scalar and the error scales with the amount. With no size persisted on the intent, there is nothing on the type to recover an effective rate from: #747 must persist one.The field comments in
packages/models/src/users/intent.tsstate that contract rather than whichever mechanism currently prices it — the epic has had three rate sources and the mechanism-naming comments went stale on each.Verification
yarn models build && yarn s3 build && yarn backend buildyarn backend lintyarn backend test— unit suiteyarn frontend lint+tsc --noEmitCoverage: the no-
requestedBytesregression path (asserting the balance is not even read), rejection of zero/negative/over-cap without pricing or a balance read, the exact-cap boundary, 18parseRequestedBytesshape cases, a guard that the size is not persisted, and both halves of the USDC/AI3 asymmetry.🤖 Generated with Claude Code