feat: Manteca entity deposit addresses from the API (TASK-22107) - #2933
feat: Manteca entity deposit addresses from the API (TASK-22107)#2933jjramirezn wants to merge 7 commits into
Conversation
Manteca removes the shared balance pool on 2026-09-14 — each legal entity then holds its own balance, and a deposit at the wrong entity's address no longer funds the operation. The API (peanut-api-ts #1487) resolves the entity and returns depositAddress on /qr-payment/init and /withdraw/init; this makes every funding flow consume it: - qr-pay signs the spend to the lock's depositAddress, - the Manteca withdraw page signs to the price lock's depositAddress, - the claim-link offramp calls /withdraw/init BEFORE spending the one-shot link and aborts (funds unmoved, retryable) if init fails — claiming to a hardcoded address and then failing would strand the link's funds at the wrong entity. The mirrored constants remain only as a fallback for an older API that does not return the field yet. TASK-22107
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code-analysis diffPainscore total: 7063.56 → 7059.41 (-4.15) 🆕 New findings (39)
…and 19 more. ✅ Resolved (38)
…and 18 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
🖼 Visual diff — 1 screen moved1 of 60 shots changed · 59 identical · baseline
job summary · before/after/diff images — artifact Fixture screenshots, no backend. Advisory — this check never blocks a merge. Posted from the default branch by ds-shots-comment.yml; the report it renders is untrusted data. |
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chip review — changes requested
The entity-aware recipients match the paired API contract, but the critical claim-link preflight and routing invariant has no regression coverage.
Findings
-
MAJOR · src/components/Claim/Link/views/MantecaReviewStep.tsx:74 · Add coverage for the pre-claim entity lookup
This is the safety boundary that keeps a one-shot link from funding the wrong Manteca entity, but no test renders MantecaReviewStep or exercises this branch. A later refactor could ignore initData.depositAddress or let claimLinkSecure run after an init error, stranding a BRL link after the entity cutoff without any suite failure. Add component tests that assert the API-served address is passed to claimLinkSecure and that an init error calls neither claimLinkSecure nor withdraw. The QR-pay and bank-withdraw signSpend recipient selections should likewise be pinned because they move funds. -
MAJOR · src/app/(mobile-ui)/qr-pay/page.tsx:762 · [moonshotai/kimi-k3] API-served depositAddress used without runtime validation before signing fund transfers
qr-pay (and identically withdraw/manteca atrecipient: (priceLock.depositAddress as0x${string}| undefined) ?? MANTECA_DEPOSIT_ADDRESS) now takes the recipient for an irreversible spend from the/manteca/qr-payment/init(or/withdraw/init) response. The only 'check' is a TypeScriptas0x${string}`` cast, which is a compile-time assertion and validates nothing at runtime. Two concrete failure modes: (1)??only falls back on null/undefined, so an API that returns `depositAddress: ""` (or any non-null malformed value) bypasses the constant fallback and is signed to as the recipient — MantecaReviewStep correctly guards truthiness with `if (initData?.depositAddress)`, but qr-pay and withdraw do not; (2) a validly formatted but wrong address returned by an API bug, stale lock, or manipulated response is now signed to directly, whereas previously the recipient was a compile-time constant. The PR's own reasoning for the claim-link flow (wrong recipient strands funds; the QA note about a server-side 400 at complete time does not recover funds already on-chain) applies equally to signSpend here. Fix: before using the served value, run a runtime check — e.g. `viem`'s `isAddress(depositAddress)` (and non-empty) — and fall back to the constant with the `[manteca-entity-legacy-funding]` log (or abort) when it fails; ideally compare against the small static set of known legal-entity addresses rather than trusting an arbitrary address from the wire. -
MAJOR · src/app/(mobile-ui)/qr-pay/page.tsx:762 · [claude-opus] Manteca spend recipient now comes from the API with no test on either page
This diff changes WHERE user USDC is sent in two additional flows and ships no test for either. src/app/(mobile-ui)/qr-pay/page.tsx:762 now prefersfinalPaymentLock.depositAddressover the per-rail constants, and src/app/(mobile-ui)/withdraw/manteca/page.tsx:389 now preferspriceLock.depositAddressover MANTECA_DEPOSIT_ADDRESS. CONTRIBUTING.md:495 is explicit — "if code moves money or mutates shared state, it needs a test before merge" — and the spend recipient is the money-moving decision: pick the wrong address after the 2026-09-14 entity split and the USDC lands at an entity that cannot settle the operation.
Evidence that nothing covers it: grep -rn depositAddress src --include=*.test.tsx returns only rhino/sentry/add-money hits, no Manteca lock. The qr-pay suite already mocks the exact seams — mockSignSpend (src/app/(mobile-ui)/qr-pay/tests/qr-pay-states.test.tsx:119) and mockMantecaApi.initiateQrPayment, whose default lock (line ~600) has NO depositAddress — so today's green suite exercises only the fallback branch and would stay green if the new ?? were inverted or dropped. The withdraw/manteca page has no tests directory at all.
Untested cases to name: (1) qr-pay, lock returns depositAddress → signSpend called with that address, NOT the PIX/non-PIX constant (assert on mockSignSpend.mock.calls[0][0].recipient); (2) qr-pay, lock omits depositAddress → falls back to NON_AR for EQrType.PIX and AR otherwise; (3) withdraw/manteca, priceLock.depositAddress present vs absent → same two assertions. Case (1) is ~5 lines on top of the existing harness.
Checked clean
- Pinned worktree HEAD and merge base match the supplied SHAs; worktree is clean.
- Compared all four changed files with the exact base and traced each recipient into signSpend or claimLinkSecure.
- Paired API PR 1487 returns entity-aware depositAddress values from both init endpoints and validates funded addresses during completion.
- Claim-link init and completion resolve the Manteca account from the same authenticated user and currency, so discarding the price-lock code does not change entity selection.
- No existing test asserts the API-served recipients or the claim-link init-failure no-spend path.
- At review time, format, typecheck, eslint, design-system lint, provenance, and backend-baseline checks passed; unit, design-system screenshots, analysis, and preview were still running.
Second opinion by moonshotai/kimi-k3: 1 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 33cb733332bb · Context: repo, other-repo · Took 10m
| // entity. The constant remains only for an older API that | ||
| // does not return the field yet. | ||
| let depositAddress: string = MANTECA_DEPOSIT_ADDRESS | ||
| const { data: initData, error: initError } = await mantecaApi.initiateWithdraw({ amount, currency }) |
There was a problem hiding this comment.
MAJOR: Add coverage for the pre-claim entity lookup
This is the safety boundary that keeps a one-shot link from funding the wrong Manteca entity, but no test renders MantecaReviewStep or exercises this branch. A later refactor could ignore initData.depositAddress or let claimLinkSecure run after an init error, stranding a BRL link after the entity cutoff without any suite failure. Add component tests that assert the API-served address is passed to claimLinkSecure and that an init error calls neither claimLinkSecure nor withdraw. The QR-pay and bank-withdraw signSpend recipient selections should likewise be pinned because they move funds.
| // constants remain only as a fallback for an older API that | ||
| // does not return the field yet. | ||
| recipient: | ||
| (finalPaymentLock.depositAddress as `0x${string}` | undefined) ?? |
There was a problem hiding this comment.
MAJOR: [moonshotai/kimi-k3] API-served depositAddress used without runtime validation before signing fund transfers
qr-pay (and identically withdraw/manteca at recipient: (priceLock.depositAddress as 0x${string} | undefined) ?? MANTECA_DEPOSIT_ADDRESS) now takes the recipient for an irreversible spend from the /manteca/qr-payment/init (or /withdraw/init) response. The only 'check' is a TypeScript as 0x${string}`` cast, which is a compile-time assertion and validates nothing at runtime. Two concrete failure modes: (1) ?? only falls back on null/undefined, so an API that returns `depositAddress: ""` (or any non-null malformed value) bypasses the constant fallback and is signed to as the recipient — MantecaReviewStep correctly guards truthiness with `if (initData?.depositAddress)`, but qr-pay and withdraw do not; (2) a validly formatted but wrong address returned by an API bug, stale lock, or manipulated response is now signed to directly, whereas previously the recipient was a compile-time constant. The PR's own reasoning for the claim-link flow (wrong recipient strands funds; the QA note about a server-side 400 at complete time does not recover funds already on-chain) applies equally to signSpend here. Fix: before using the served value, run a runtime check — e.g. `viem`'s `isAddress(depositAddress)` (and non-empty) — and fall back to the constant with the `[manteca-entity-legacy-funding]` log (or abort) when it fails; ideally compare against the small static set of known legal-entity addresses rather than trusting an arbitrary address from the wire.
| // constants remain only as a fallback for an older API that | ||
| // does not return the field yet. | ||
| recipient: | ||
| (finalPaymentLock.depositAddress as `0x${string}` | undefined) ?? |
There was a problem hiding this comment.
MAJOR: [claude-opus] Manteca spend recipient now comes from the API with no test on either page
This diff changes WHERE user USDC is sent in two additional flows and ships no test for either. src/app/(mobile-ui)/qr-pay/page.tsx:762 now prefers finalPaymentLock.depositAddress over the per-rail constants, and src/app/(mobile-ui)/withdraw/manteca/page.tsx:389 now prefers priceLock.depositAddress over MANTECA_DEPOSIT_ADDRESS. CONTRIBUTING.md:495 is explicit — "if code moves money or mutates shared state, it needs a test before merge" — and the spend recipient is the money-moving decision: pick the wrong address after the 2026-09-14 entity split and the USDC lands at an entity that cannot settle the operation.
Evidence that nothing covers it: grep -rn depositAddress src --include=*.test.tsx returns only rhino/sentry/add-money hits, no Manteca lock. The qr-pay suite already mocks the exact seams — mockSignSpend (src/app/(mobile-ui)/qr-pay/tests/qr-pay-states.test.tsx:119) and mockMantecaApi.initiateQrPayment, whose default lock (line ~600) has NO depositAddress — so today's green suite exercises only the fallback branch and would stay green if the new ?? were inverted or dropped. The withdraw/manteca page has no tests directory at all.
Untested cases to name: (1) qr-pay, lock returns depositAddress → signSpend called with that address, NOT the PIX/non-PIX constant (assert on mockSignSpend.mock.calls[0][0].recipient); (2) qr-pay, lock omits depositAddress → falls back to NON_AR for EQrType.PIX and AR otherwise; (3) withdraw/manteca, priceLock.depositAddress present vs absent → same two assertions. Case (1) is ~5 lines on top of the existing harness.
… recipient decision Chip review fixes. pickMantecaDepositAddress runtime-validates the wire value with viem isAddress before it becomes a spend recipient — a TypeScript cast validates nothing, and ?? alone would let an empty string through — falling back to the constant and reporting to Sentry when a served value exists but fails validation. All three flows use it. Tests pin the money decision at both levels: the util's full branch matrix, and MantecaReviewStep component tests asserting the API-served address reaches claimLinkSecure, the constant fallback for an older API, and that an init error aborts BEFORE the one-shot link is spent — no claim, no associate, no withdraw.
…aim-link errors The API's legacy withdraw route now returns CLAIM_STORE_UNAVAILABLE and TX_ALREADY_USED with carefully-worded messages; rendering the raw code helps nobody whose funds already left the one-shot link. The fallback error branch prefers the message.
There was a problem hiding this comment.
Chip review — changes requested
Two major issues remain: the recipient validator accepts the zero address, and the QR-pay and bank-withdraw signing paths still lack page-level recipient wiring coverage. The claim-link preflight coverage is now present.
Findings
-
MAJOR · src/utils/manteca.utils.ts:17 · Reject the zero address before signing
The guard treats0x0000000000000000000000000000000000000000as valid because viem'sisAddresschecks address syntax and accepts zero. If/initreturns zero because of a configuration or serialization defect, all three call sites bypass the fallback and pass an unusable recipient intosignSpendorclaimLinkSecure; the transfer then fails or can become unrecoverable instead of safely using the legacy address. RejectzeroAddress(and cover it) before returningserved. -
MAJOR · src/app/(mobile-ui)/qr-pay/page.tsx:762 · Cover the page-level spend recipient wiring
The new helper tests prove only the helper in isolation, while the claim-link test covers only the third flow. The existing QR page suite invokessignSpendbut never gives/inita distinct validdepositAddressor assertsrecipient, and the bank-withdraw page has no test at all. A future swap back to a fallback constant or use of the stale lock would still leave all new tests green while sending funds to the wrong entity. Add submit-path tests on both pages that return a distinct API address and assert the exact recipient passed tosignSpend.
Checked clean
- Exact head SHA, base ref/SHA, trusted author, PR metadata, and merge base verified.
- P1 is fixed: claim-link component tests cover the served recipient, legacy fallback, and init failure before the one-shot claim.
- P2 as originally stated is fixed for malformed, empty, null, and absent wire values through the shared runtime validator; the separate zero-address case remains actionable.
- Current backend sibling routes were checked for recipient and price-lock trust boundaries; the paired open API change is not present in the policy-branch checkout.
- Exact-head CI unit, typecheck, eslint, format, analyze, and aggregate ci-success checks are green; the visual snapshot job was still in progress.
- A local focused Jest run was unavailable because the detached worktree has no installed dependencies.
Second opinion: did not run — the model did not answer in time. This review is one reviewer short.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: b45de3d0e9c7 · Context: repo, peanut-api-ts sibling · Took 18m
| * see it happening. | ||
| */ | ||
| export function pickMantecaDepositAddress(served: unknown, fallback: Address): Address { | ||
| if (typeof served === 'string' && served.length > 0 && isAddress(served, { strict: false })) { |
There was a problem hiding this comment.
MAJOR: Reject the zero address before signing
The guard treats 0x0000000000000000000000000000000000000000 as valid because viem's isAddress checks address syntax and accepts zero. If /init returns zero because of a configuration or serialization defect, all three call sites bypass the fallback and pass an unusable recipient into signSpend or claimLinkSecure; the transfer then fails or can become unrecoverable instead of safely using the legacy address. Reject zeroAddress (and cover it) before returning served.
| // from the QR and the paying Manteca account. The per-rail | ||
| // constants remain only as a fallback for an older API that | ||
| // does not return the field yet. | ||
| recipient: pickMantecaDepositAddress( |
There was a problem hiding this comment.
MAJOR: Cover the page-level spend recipient wiring
The new helper tests prove only the helper in isolation, while the claim-link test covers only the third flow. The existing QR page suite invokes signSpend but never gives /init a distinct valid depositAddress or asserts recipient, and the bank-withdraw page has no test at all. A future swap back to a fallback constant or use of the stale lock would still leave all new tests green while sending funds to the wrong entity. Add submit-path tests on both pages that return a distinct API address and assert the exact recipient passed to signSpend.
Chip round-2 fixes. The runtime validator rejects the zero address — syntactically valid to isAddress but never a recipient; a config or serialization defect returning it must fall back to the constant, not sign an unrecoverable transfer. Page-level tests on qr-pay pin the wiring the helper tests cannot see: a distinct API-served depositAddress reaches signSpend as the exact recipient, and an older API without the field falls back to the per-rail constant. The bank-withdraw page's identical one-line wiring into the same tested util is documented as such in the PR notes — it has no page harness, and a several-hundred-line mock scaffold to assert one argument would be disproportionate.
There was a problem hiding this comment.
Chip review — changes requested
Request changes: the claim-link compatibility fallback can strand a one-shot BRL claim after the entity cutoff, and the bank-withdraw signing boundary remains untested.
Findings
-
BLOCKING · src/components/Claim/Link/views/MantecaReviewStep.tsx:79 · Fail closed when a claim recipient is missing or invalid
After the 2026-09-14 cutoff, a BRL claim whose /withdraw/init response has the older 200 shape, or contains a malformed or zero depositAddress, reaches this line and selects MANTECA_DEPOSIT_ADDRESS. claimLinkSecure then irreversibly sends the one-shot link to the legacy entity, while the paired API accepts only the new BRL entity address and rejects /withdraw, stranding the funds. Require a valid API-served address in this claim-link path and abort before claimLinkSecure; retain a compatibility fallback only in paths where the backend validates the signed artifact before broadcast. -
MAJOR · src/app/(mobile-ui)/withdraw/manteca/page.tsx:390 · Cover the bank-withdraw recipient at the signing boundary
This line changes the recipient of an irreversible spend, but no withdraw-page test proves that the depositAddress returned by initiateWithdraw survives the priceLock state handoff and reaches signSpend. The utility tests cannot catch page wiring regressions, while the QR and claim-link paths now have boundary coverage. Add a focused page or extracted-flow test that returns a distinct API address and asserts the exact signSpend recipient, including the missing-field behavior if that fallback remains. -
MINOR · src/app/(mobile-ui)/withdraw/manteca/page.tsx:390 · [claude-opus] BRL offramp now funds the QR non-AR address, so peanut-api-ts classifies the Rain prepare as QR_PAY
After this change the bank-withdraw page signs to the API-served entity address (page.tsx:390). Per the paired API PR's legalEntity.ts, an off-ramp outside Argentina resolves to CRYPTO_GLOBAL, and MANTECA_ENTITY_DEPOSIT_ADDRESS[CRYPTO_GLOBAL] is the SAME value as MANTECA_QR_RECEIVE_ADDRESS_NON_AR (0x49200bF84dC26349C86ce040019063FeCE88CB1c). So a BRL bank withdrawal now sends collateral to an address that peanut-api-ts treats as a QR wallet.
On the collateral-only strategy the recipient is forwarded verbatim to POST /rain/cards/withdraw/prepare (src/hooks/wallet/useSpendBundle.ts:195, useSignSpendBundle.ts:225), and the API derives the intent kind from the destination alone: peanut-api-ts/src/rain/prepare-kind.ts:36-77 — MANTECA_QR_ADDRESSES_LOWER is checked before MANTECA_OFFRAMP_ADDRESSES_LOWER (which still holds only MANTECA_RECEIVE_ADDRESS_ARG), so classifyRainPrepare returns QR_PAY for what is a FIAT_OFFRAMP. Call site: peanut-api-ts/src/routes/rain/withdraw.ts:559, and the client-sent kind is explicitly ignored there.
Impact is bookkeeping, not funds: the row is born metadata.isDuplicate = true either way, so it stays hidden in history — until markIntentFailed clears the stamp on a real submit failure, at which point a Brazilian user sees a failed 'QR payment' entry for a bank withdrawal, and OFFRAMP/QR_PAY volume splits are skewed. Unlike the recipient validators (withdraw.ts:327, :763, :1219), an acceptedAddresses list cannot fix this one: once both flows fund CRYPTO_GLOBAL the address is genuinely ambiguous, so the classifier needs another signal (the client-declared kind cross-checked against a live offramp intent, or an entity+flow lookup) rather than a wider address set.
This is almost certainly the other half's work: peanut-api-ts#1487 is the matching PR and I cannot see all of its files (16 unshown), so it may already carry a prepare-kind.ts change — worth confirming there before acting. Flagging as major-class cross-repo risk but low real-world severity.
Checked clean
- Exact detached head, trusted author, dev base, and supplied merge base all matched.
- P1 is fixed: component coverage proves the pre-claim init call runs before claimLinkSecure and aborts on init error.
- P2 and P4 are fixed for signing: the shared selector runtime-validates EVM addresses and rejects the zero address before choosing a recipient.
- P3 and P5 are fixed for qr-pay: a distinct API-served address is asserted at the signSpend boundary; P6 remains for bank withdraw.
- Paired API PR #1487 was checked for the init response and post-cutoff recipient validators; BRL offramps resolve to the new entity and legacy acceptance expires at the cutoff.
- Exact-head unit, typecheck, eslint, format, analyze, baseline, provenance, and ownership checks succeeded; visual snapshot and preview checks were still running.
- Focused local Jest execution was unavailable because this detached worktree has no installed Jest dependency.
Second opinion by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: d71c4738fcfe · Context: repo, other-repository, ci · Took 13m
| setError(t('manteca.errors.generic')) | ||
| return | ||
| } | ||
| const depositAddress = pickMantecaDepositAddress(initData?.depositAddress, MANTECA_DEPOSIT_ADDRESS) |
There was a problem hiding this comment.
BLOCKING: Fail closed when a claim recipient is missing or invalid
After the 2026-09-14 cutoff, a BRL claim whose /withdraw/init response has the older 200 shape, or contains a malformed or zero depositAddress, reaches this line and selects MANTECA_DEPOSIT_ADDRESS. claimLinkSecure then irreversibly sends the one-shot link to the legacy entity, while the paired API accepts only the new BRL entity address and rejects /withdraw, stranding the funds. Require a valid API-served address in this claim-link path and abort before claimLinkSecure; retain a compatibility fallback only in paths where the backend validates the signed artifact before broadcast.
| // Entity-aware deposit address served by /withdraw/init | ||
| // (per-entity balances from 2026-09-14); the constant is | ||
| // only the fallback for an older API without the field. | ||
| recipient: pickMantecaDepositAddress(priceLock.depositAddress, MANTECA_DEPOSIT_ADDRESS), |
There was a problem hiding this comment.
MAJOR: Cover the bank-withdraw recipient at the signing boundary
This line changes the recipient of an irreversible spend, but no withdraw-page test proves that the depositAddress returned by initiateWithdraw survives the priceLock state handoff and reaches signSpend. The utility tests cannot catch page wiring regressions, while the QR and claim-link paths now have boundary coverage. Add a focused page or extracted-flow test that returns a distinct API address and asserts the exact signSpend recipient, including the missing-field behavior if that fallback remains.
| // Entity-aware deposit address served by /withdraw/init | ||
| // (per-entity balances from 2026-09-14); the constant is | ||
| // only the fallback for an older API without the field. | ||
| recipient: pickMantecaDepositAddress(priceLock.depositAddress, MANTECA_DEPOSIT_ADDRESS), |
There was a problem hiding this comment.
MINOR: [claude-opus] BRL offramp now funds the QR non-AR address, so peanut-api-ts classifies the Rain prepare as QR_PAY
After this change the bank-withdraw page signs to the API-served entity address (page.tsx:390). Per the paired API PR's legalEntity.ts, an off-ramp outside Argentina resolves to CRYPTO_GLOBAL, and MANTECA_ENTITY_DEPOSIT_ADDRESS[CRYPTO_GLOBAL] is the SAME value as MANTECA_QR_RECEIVE_ADDRESS_NON_AR (0x49200bF84dC26349C86ce040019063FeCE88CB1c). So a BRL bank withdrawal now sends collateral to an address that peanut-api-ts treats as a QR wallet.
On the collateral-only strategy the recipient is forwarded verbatim to POST /rain/cards/withdraw/prepare (src/hooks/wallet/useSpendBundle.ts:195, useSignSpendBundle.ts:225), and the API derives the intent kind from the destination alone: peanut-api-ts/src/rain/prepare-kind.ts:36-77 — MANTECA_QR_ADDRESSES_LOWER is checked before MANTECA_OFFRAMP_ADDRESSES_LOWER (which still holds only MANTECA_RECEIVE_ADDRESS_ARG), so classifyRainPrepare returns QR_PAY for what is a FIAT_OFFRAMP. Call site: peanut-api-ts/src/routes/rain/withdraw.ts:559, and the client-sent kind is explicitly ignored there.
Impact is bookkeeping, not funds: the row is born metadata.isDuplicate = true either way, so it stays hidden in history — until markIntentFailed clears the stamp on a real submit failure, at which point a Brazilian user sees a failed 'QR payment' entry for a bank withdrawal, and OFFRAMP/QR_PAY volume splits are skewed. Unlike the recipient validators (withdraw.ts:327, :763, :1219), an acceptedAddresses list cannot fix this one: once both flows fund CRYPTO_GLOBAL the address is genuinely ambiguous, so the classifier needs another signal (the client-declared kind cross-checked against a live offramp intent, or an entity+flow lookup) rather than a wider address set.
This is almost certainly the other half's work: peanut-api-ts#1487 is the matching PR and I cannot see all of its files (16 unshown), so it may already carry a prepare-kind.ts change — worth confirming there before acting. Flagging as major-class cross-repo risk but low real-world severity.
…handoff extracted and tested Chip round-4 fixes. The claim-link path spends a one-shot link with no server-side pre-broadcast validation, so it now FAILS CLOSED on any init problem — error, missing field, malformed or zero address — via requireMantecaDepositAddress; the constant fallback survives only in the signed flows, whose recipient the backend validates before anything broadcasts. Component tests pin the abort for every bad shape. The bank-withdraw priceLock → signSpend handoff is extracted to resolveOfframpSpendRecipient and tested (served address wins, missing field falls back, malformed never becomes the recipient), shrinking the untested page wiring to a single call.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
The runtime address guards and QR wiring are now sound, but claim-link coverage is broken in CI, bank-withdraw signing still lacks boundary coverage, and BRL collateral preparation can retain QR classification.
Findings
-
MAJOR · src/components/Claim/Link/views/tests/MantecaReviewStep.test.tsx:68 · Click the actual withdraw action in the claim-flow tests
clickConfirm()presses the first button, but the first button rendered by this component is the destination row's Copy control, not Withdraw. At this exact head the unit check therefore fails all four new cases with zero calls, and the pre-claim lookup remains unexecuted. Select the Withdraw button by accessible name and isolate the malformed-address iterations (for example withtest.each) so every case exercises the submit handler. -
MAJOR · src/app/(mobile-ui)/withdraw/manteca/page.tsx:389 · Exercise the bank-withdraw recipient at the signing boundary
The new utility test proves only thatresolveOfframpSpendRecipientreturns its input; no test renders this page and observes the argument passed tosignSpend. A later regression that restores the constant here, selects the wrong lock, or bypasses the resolver leaves that suite green while signing a money transfer to the wrong entity. Add a page-level test that seeds a price lock with a distinct served address, confirms the withdrawal, and asserts that exact address reachessignSpend. -
MINOR · src/app/(mobile-ui)/withdraw/manteca/page.tsx:389 · Keep BRL collateral offramps out of QR classification
For a collateral-only BRL bank withdrawal, this API-served recipient is also the non-AR QR funding address recognized by the current Rain prepare classifier.useSignSpendBundleforwards it to/rain/cards/withdraw/prepare, whose current server contract ignores the client-suppliedFIAT_OFFRAMPkind and classifies by recipient, so the preparation becomes a duplicateQR_PAY; a failed or cancelled funding leg is then surfaced and reconciled as a QR payment instead of an offramp. Update the paired API classification/route context for this entity address and pin the BRL direct-transfer case. -
MAJOR · src/components/Claim/Link/views/MantecaReviewStep.tsx:76 · [claude-opus] Claim-link offramp fails closed on a field the merged API does not serve — hard deploy-order dependency on the peanut-api-ts half
MantecaReviewStepnow callsmantecaApi.initiateWithdrawand aborts the whole flow unless the reply carries a validdepositAddress(src/components/Claim/Link/views/MantecaReviewStep.tsx:76-85,requireMantecaDepositAddressreturns null for a missing field). In the pinned peanut-api-ts checkout,/manteca/withdraw/initreplies with exactly{priceLockCode, price, expiresAt, usdAmount, fiatAmount, currency}— nodepositAddress(src/routes/manteca/withdraw.ts:193-200). Against that API every regional (MercadoPago/PIX) claim-link withdrawal showsmanteca.errors.genericand never claims, i.e. the feature is 100% down.
Unlike qr-pay and the bank-withdraw page — where the author deliberately kept a constant fallback, so those degrade safely — this path has no fallback by design, which is the right safety call but makes the FE unshippable ahead of the API.
Second, coupled evidence in the same direction: once the API does serve an entity address, a BRL claim will be claimed to the CRYPTO_GLOBAL address, while the merged legacy withdraw route still validates the funding transfer against MANTECA_RECEIVE_ADDRESS_ARG only (src/routes/manteca/withdraw.ts:325-327), and the Rain offramp path rejects any recipient other than that constant (src/routes/manteca/withdraw.ts:1219-1230).
This is almost certainly the paired half in the open peanut-api-ts#1487 (its legalEntity.ts documents acceptedAddresses tolerance during rollout), which I cannot read in full — so major, not blocking. What to confirm before merge: (a) that #1487 adds depositAddress to the /manteca/withdraw/init response specifically, not only to /manteca/qr-payment/init; (b) that the legacy tx-hash withdraw route accepts the entity address as well as the ARG constant. And state the deploy order in the PR: the API side must be live before this frontend.
Checked clean
- Verified the detached worktree head and merge base against the supplied head and base SHAs.
- P2 and P4 are fixed: served addresses are runtime-validated and the zero address is rejected before signing.
- P3 and P5 no longer apply to qr-pay: the page-level test passes in exact-head CI and observes the served recipient at signSpend.
- P6 is fixed in the component code: claim-link submission aborts before claimLinkSecure on missing, malformed, zero, or init-error input.
- Typecheck, eslint, format, design-system lint, analysis, provenance, and ownership checks passed at this head.
- The focused local rerun was unavailable because the detached worktree has no node_modules; exact-head CI was inspected instead.
- Reviewed the Manteca settlement model in the internal product provider reference and the current API-side Rain prepare classifier without reading any comments.
Second opinion by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 7da06adbbcdf · Context: repo, product, other_repo, ci · Took 13m
|
|
||
| function clickConfirm() { | ||
| // The single primary action button on the review card. | ||
| fireEvent.click(screen.getAllByRole('button')[0]) |
There was a problem hiding this comment.
MAJOR: Click the actual withdraw action in the claim-flow tests
clickConfirm() presses the first button, but the first button rendered by this component is the destination row's Copy control, not Withdraw. At this exact head the unit check therefore fails all four new cases with zero calls, and the pre-claim lookup remains unexecuted. Select the Withdraw button by accessible name and isolate the malformed-address iterations (for example with test.each) so every case exercises the submit handler.
| // Entity-aware deposit address served by /withdraw/init | ||
| // (per-entity balances from 2026-09-14); the constant is | ||
| // only the fallback for an older API without the field. | ||
| recipient: resolveOfframpSpendRecipient(priceLock), |
There was a problem hiding this comment.
MAJOR: Exercise the bank-withdraw recipient at the signing boundary
The new utility test proves only that resolveOfframpSpendRecipient returns its input; no test renders this page and observes the argument passed to signSpend. A later regression that restores the constant here, selects the wrong lock, or bypasses the resolver leaves that suite green while signing a money transfer to the wrong entity. Add a page-level test that seeds a price lock with a distinct served address, confirms the withdrawal, and asserts that exact address reaches signSpend.
| // Entity-aware deposit address served by /withdraw/init | ||
| // (per-entity balances from 2026-09-14); the constant is | ||
| // only the fallback for an older API without the field. | ||
| recipient: resolveOfframpSpendRecipient(priceLock), |
There was a problem hiding this comment.
MINOR: Keep BRL collateral offramps out of QR classification
For a collateral-only BRL bank withdrawal, this API-served recipient is also the non-AR QR funding address recognized by the current Rain prepare classifier. useSignSpendBundle forwards it to /rain/cards/withdraw/prepare, whose current server contract ignores the client-supplied FIAT_OFFRAMP kind and classifies by recipient, so the preparation becomes a duplicate QR_PAY; a failed or cancelled funding leg is then surfaced and reconciled as a QR payment instead of an offramp. Update the paired API classification/route context for this entity address and pin the BRL direct-transfer case.
| // a constant fallback because the backend validates their | ||
| // recipient before anything is broadcast; nothing validates a | ||
| // link claim.) | ||
| const { data: initData, error: initError } = await mantecaApi.initiateWithdraw({ amount, currency }) |
There was a problem hiding this comment.
MAJOR: [claude-opus] Claim-link offramp fails closed on a field the merged API does not serve — hard deploy-order dependency on the peanut-api-ts half
MantecaReviewStep now calls mantecaApi.initiateWithdraw and aborts the whole flow unless the reply carries a valid depositAddress (src/components/Claim/Link/views/MantecaReviewStep.tsx:76-85, requireMantecaDepositAddress returns null for a missing field). In the pinned peanut-api-ts checkout, /manteca/withdraw/init replies with exactly {priceLockCode, price, expiresAt, usdAmount, fiatAmount, currency} — no depositAddress (src/routes/manteca/withdraw.ts:193-200). Against that API every regional (MercadoPago/PIX) claim-link withdrawal shows manteca.errors.generic and never claims, i.e. the feature is 100% down.
Unlike qr-pay and the bank-withdraw page — where the author deliberately kept a constant fallback, so those degrade safely — this path has no fallback by design, which is the right safety call but makes the FE unshippable ahead of the API.
Second, coupled evidence in the same direction: once the API does serve an entity address, a BRL claim will be claimed to the CRYPTO_GLOBAL address, while the merged legacy withdraw route still validates the funding transfer against MANTECA_RECEIVE_ADDRESS_ARG only (src/routes/manteca/withdraw.ts:325-327), and the Rain offramp path rejects any recipient other than that constant (src/routes/manteca/withdraw.ts:1219-1230).
This is almost certainly the paired half in the open peanut-api-ts#1487 (its legalEntity.ts documents acceptedAddresses tolerance during rollout), which I cannot read in full — so major, not blocking. What to confirm before merge: (a) that #1487 adds depositAddress to the /manteca/withdraw/init response specifically, not only to /manteca/qr-payment/init; (b) that the legacy tx-hash withdraw route accepts the entity address as well as the ARG constant. And state the deploy order in the PR: the API side must be live before this frontend.
…'s Copy control The claim-flow tests pressed buttons[0], which is the Copy control on CI's render order, so every new case failed with the submit handler unexecuted. Click the Withdraw action by accessible name, clean up between the malformed-address iterations, and give the Sentry mock the captureMessage the strict validator calls.
…evel Drives the page amount → lock-price → review → Withdraw with the full mock harness (qr-pay-states pattern) and asserts the depositAddress served by /withdraw/init survives the priceLock state handoff as the exact signSpend recipient — plus the missing-field fallback to the legacy constant. A regression that restores the constant, selects a stale lock, or bypasses the resolver now fails this suite.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
One rollout dependency remains: the claim-link flow fails closed against the current /withdraw/init response because it does not yet include depositAddress. The earlier validation and money-boundary coverage gaps are fixed.
Findings
-
MAJOR · src/components/Claim/Link/views/MantecaReviewStep.tsx:81 · Deploy the response field before requiring it
The current API policy branch returns the price-lock fields from /manteca/withdraw/init but not depositAddress. With that response, initData?.depositAddress is undefined here, requireMantecaDepositAddress returns null, and every regional claim-link offramp aborts before spending the link. Deploy and verify the paired API response for every supported currency before releasing this UI, or gate this client path until that contract is live. -
MAJOR · src/components/Claim/Link/views/MantecaReviewStep.tsx:76 · [claude-opus] Claim-link offramp fails closed on a field the merged API does not serve
MantecaReviewStepnow callsmantecaApi.initiateWithdraw({amount,currency})and aborts the whole flow whendepositAddressis absent (requireMantecaDepositAddress→ null → generic error, no claim, no withdraw). In the pinned peanut-api-ts checkout,/manteca/withdraw/init(src/routes/manteca/withdraw.ts:84) replies with exactly{priceLockCode, price, expiresAt, usdAmount, fiatAmount, currency}— nodepositAddress. Against that API every Manteca claim-link claim (MercadoPago/PIX) is dead on arrival; unlike qr-pay and the bank-withdraw page, this path has no constant fallback by design. That checkout holds only merged code, so the serving half is almost certainly the open peanut-api-ts#1487 (Manteca legal-entity deposit routing) that I cannot read — this is a deploy-order dependency, not necessarily a design error: the API half must be deployed before this frontend, and that should be stated on the PR. Worth confirming with the API author while you're there:/manteca/withdrawstill validates the funding transfer againstMANTECA_RECEIVE_ADDRESS_ARGonly (src/routes/manteca/withdraw.ts:325-330), so oncedepositAddressis served for a non-CRYPTO_ARG entity (e.g. a BRL claim), that tx-hash validator must accept the served address too — the one-shot link is already spent by then. #1487'slegalEntity.ts acceptedAddresseslooks like it covers this, but it is not in the visible diff.
Checked clean
- Exact worktree head, supplied base SHA, merge base, trusted author, and dev base ref all matched.
- Prior P1/P6/P9: the claim step now performs the entity lookup before claiming, rejects missing, malformed, and zero addresses, and the tests click the actual Withdraw action.
- Prior P2-P5: API-served recipients are runtime-validated, the zero address is rejected, and QR page coverage asserts the exact signSpend recipient plus the legacy fallback.
- Prior P7/P10: the bank-withdraw page test drives amount through review and asserts both the API-served and fallback recipients at signSpend.
- Prior P8/P11: against the current API contract, bank withdraw still falls back to the legacy offramp address; the PR metadata identifies a paired API change for overlap-aware Rain classification.
- Security and slop passes found no additional reachable defect, unsafe trust-boundary use, secret exposure, or unnecessary abstraction in the changed code.
- All exact-head CI checks completed successfully or were intentionally skipped, including unit, typecheck, lint, format, design-system screenshots, baseline, provenance, ownership, analysis, and preview deployment.
Second opinion by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only, so treat its findings as advice.
Third opinion by claude-opus: 1 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 50e7e3d6600c · Context: repo, product · Took 11m (queued 2m)
| setError(t('manteca.errors.generic')) | ||
| return | ||
| } | ||
| const depositAddress = requireMantecaDepositAddress(initData?.depositAddress) |
There was a problem hiding this comment.
MAJOR: Deploy the response field before requiring it
The current API policy branch returns the price-lock fields from /manteca/withdraw/init but not depositAddress. With that response, initData?.depositAddress is undefined here, requireMantecaDepositAddress returns null, and every regional claim-link offramp aborts before spending the link. Deploy and verify the paired API response for every supported currency before releasing this UI, or gate this client path until that contract is live.
| // a constant fallback because the backend validates their | ||
| // recipient before anything is broadcast; nothing validates a | ||
| // link claim.) | ||
| const { data: initData, error: initError } = await mantecaApi.initiateWithdraw({ amount, currency }) |
There was a problem hiding this comment.
MAJOR: [claude-opus] Claim-link offramp fails closed on a field the merged API does not serve
MantecaReviewStep now calls mantecaApi.initiateWithdraw({amount,currency}) and aborts the whole flow when depositAddress is absent (requireMantecaDepositAddress → null → generic error, no claim, no withdraw). In the pinned peanut-api-ts checkout, /manteca/withdraw/init (src/routes/manteca/withdraw.ts:84) replies with exactly {priceLockCode, price, expiresAt, usdAmount, fiatAmount, currency} — no depositAddress. Against that API every Manteca claim-link claim (MercadoPago/PIX) is dead on arrival; unlike qr-pay and the bank-withdraw page, this path has no constant fallback by design. That checkout holds only merged code, so the serving half is almost certainly the open peanut-api-ts#1487 (Manteca legal-entity deposit routing) that I cannot read — this is a deploy-order dependency, not necessarily a design error: the API half must be deployed before this frontend, and that should be stated on the PR. Worth confirming with the API author while you're there: /manteca/withdraw still validates the funding transfer against MANTECA_RECEIVE_ADDRESS_ARG only (src/routes/manteca/withdraw.ts:325-330), so once depositAddress is served for a non-CRYPTO_ARG entity (e.g. a BRL claim), that tx-hash validator must accept the served address too — the one-shot link is already spent by then. #1487's legalEntity.ts acceptedAddresses looks like it covers this, but it is not in the visible diff.
1 participant
withdraw-manteca-recipient.test.tsx): the page is driven amount → lock-price → review → Withdraw, and the API-serveddepositAddressis asserted as the exactsignSpendrecipient, with the missing-field fallback case alongside.classifyRainPrepareoverlap-aware — on the shared CRYPTO_GLOBAL address the client-declaredFIAT_OFFRAMPkind picks between the two duplicate-at-birth Manteca labels, so a failed BRL bank withdrawal no longer renders as a failed QR payment.