feat(topup): cap card top-up amount at the user's per-level daily limit - #693
Merged
Conversation
Reads the new l1/l2/l3DailyLimit fields from globals.fygaroTopup and blocks a card top-up larger than the user's level cap BEFORE the card is charged (over-cap payments would otherwise be captured by Fygaro and strand in manual review). Per-transaction pre-check only — the webhook enforces the rolling-24h total authoritatively. Unknown level or null fygaroTopup degrades to no client-side cap; bank transfers are uncapped.
…view Review fixes for PR #693: - AccountLevel.Zero no longer falls into the "no client-side cap" bucket: the webhook fails CLOSED for level 0 (no-daily-limit-for-level), so a level-0 card charge would be captured by Fygaro and stranded in manual review. handleContinue now refuses card top-ups for level 0 with an upgrade alert instead of relying on the home screen hiding the Transfer button (a cross-file invariant deep links could bypass). Tests pin the refusal and that bank transfers stay unaffected. - Add the missing test for the second documented degrade path: a leveled (L1) user with null fygaroTopup gets no client-side cap and still reaches CardPayment, rather than a crash or a spurious block on missing metadata. - Backfill dailyLimitAmount + upgradeRequired into all 23 translation files so check:translation-drift passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nt7JxbX5W9o7viKegnXJPH
…efuse The round-1 level-0 refusal lived only in handleContinue, so a level-0 user on the card flow (deep link / un-hidden button) who typed $50 saw "You'll receive $47.xx" and then hit "Upgrade Required" on Continue — breaking the invariant that the screen never promises a concrete receive figure for a gross Continue will refuse. Hoist a shared cardBlockedForLevel flag, gate the netAmount preview on it, reuse it in handleContinue, and add a regression test (level 0 + card + $50 renders no fee note). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nt7JxbX5W9o7viKegnXJPH
…creens - Settings -> Account -> Transaction Limits: new Card Top-Up section showing the user's per-level daily cap (via globals.fygaroTopup + level) and the card minimum; new Bank Transfer (ACH) row stating the US$20 minimum deposit; restores the Stablesat (convert) limits section — the accountLimits query always fetched convert but the screen never rendered it. - TopupDetails: info label under the amount field showing the user's daily card limit; ACH minimum notice in the bridge flow. - BankTransfer (bridge/ACH variant): ACH minimum-deposit notice. - Extracts the level->cap mapping into useCardTopupLimit, shared by TopupDetails and the limits screen.
Contributor
Author
|
Added limit visibility per follow-up request: Card Top-Up daily cap + minimum and a Bank Transfer (ACH) US$20 minimum row on Settings → Account → Transaction Limits (also restores the Stablesat/convert limits section, which was queried but never rendered); daily-limit info label on the top-up amount screen; ACH minimum notice on the Bank Transfer screen and in the bridge amount flow. New |
added 2 commits
August 14, 2026 23:57
…fix translation drift
…ends can't hide the Transfer button Requesting the new l1/l2/l3DailyLimit fields inside transferFlags made the WHOLE query fail GRAPHQL_VALIDATION_FAILED against any backend that predates flash#482 (e.g. test today, or prod after a flash rollback) — transferEnabled collapsed to false and the home screen hid the Transfer button for every leveled user. The limit fields now live in a dedicated cardTopupLimits query consumed only by useCardTopupLimit: on an old backend only that query fails and the app degrades to no client-side cap / no limit rows, with the webhook still enforcing. transferFlags is back to its long-standing field set and carries a do-not-extend warning. Regression tests pin the degrade paths.
Contributor
Author
|
Heads-up + fix: the previous commit made |
…tion on bridge Review fixes for PR #693: - useCardTopupLimit now fetches the account level directly with useLevelQuery (cache-and-network) instead of reading the cache-only useLevel() context. On a cold start / deep link the context reports an authed level-0 user as NonAuth, which put them in the "no client-side cap" degrade bucket and let a card charge through that the webhook fails closed on (captured by Fygaro, stranded in manual review). The hook exposes levelLoading, and TopupDetails holds the card flow (Continue disabled, net preview hidden) until the level resolves, degrading only when the level query settles without a level. Comments now state the actual guarantee instead of "can never". - The Transaction Limits screen's Bank Transfer (ACH) section is now gated on the same resolved bridge flag the topup entry screen uses (backend bridgeEnabled AND the bridgeTopupEnabled Firebase kill switch), so bridge-less instances no longer advertise an ACH rail the app offers nowhere. Tests cover the cold-start hold (spinner, no navigation), the card-only scope of the hold, the cache-and-network fetch itself, the signed-out skip, the hidden net preview while resolving, and the bridge-gated ACH section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nt7JxbX5W9o7viKegnXJPH
…coexist transferFlags and cardTopupLimits deliberately select disjoint subsets of globals.fygaroTopup (fees+minimum vs per-level daily limits). FygaroTopupInfo has no id, so it is embedded in the singleton Globals record; without a merge policy each response wholesale-replaced the object, clobbering the other query's cached fields. With both hooks watching cache-and-network on TopupDetails and TransactionLimitsScreen, every clobber forced a refetch whose response clobbered the other query back — a sustained ping-pong during which dailyLimit read undefined (disarming the over-cap Continue gate) and the enforced minimum fell back to the default. merge: true shallow-merges existing + incoming so both selections coexist. Adds a real-InMemoryCache regression test (no mocked hooks) covering both write orders; it fails without the merge policy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nt7JxbX5W9o7viKegnXJPH
islandbitcoin
added a commit
that referenced
this pull request
Aug 16, 2026
* fix(swap): stop reporting unsettled conversions as successful
A Cash->BTC conversion showed the success screen while no funds moved,
and the retry died on a generic error. Three defects in the swap flow,
all of which also affect production:
- swap() returned `true` for PENDING, so the confirmation screen
navigated to "Conversion successful". A payment that is merely in
flight — or one the API could not confirm at all — rendered as
settled. It now returns a discriminated {status: "success"|"pending"},
and the success screen shows an "in progress" state (payment-pending
icon, its own copy, longer dwell) for the pending case.
- swap() read `status` before `errors`. The API reports a payment it
could not confirm as PENDING *carrying* an error (see flash#483), so
errors must win; they now throw.
- prepareUsdToBtc swallowed lnUsdInvoiceFeeProbe payload errors via
`amount || 0`, sending the user to confirmation on a fee we never
obtained — the real reason ("An error occurred. Contact support…")
was discarded and resurfaced later as a vaguer send failure. Probe
errors and a missing amount are now surfaced at the details screen.
- The Breez branch of swap() fell off the end returning undefined when
the send failed, which the confirmation screen read as "nothing
happened": no success, no error, no spinner reset. It now throws on
failure and distinguishes Completed from Pending.
Also fixes the shared Breez SDK test mock, which exported PaymentStatus
as strings keyed "Complete" while the real SDK (and app code such as
app/types/transactions.ts) uses numeric `Completed` — any test touching
that comparison silently took the wrong branch.
Tests: 10 new covering each behaviour above. Full suite 67/471 green,
tsc clean, no ESLint errors on changed lines.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
* chore(i18n): carry the three new keys into every translation file
check:translation-drift requires each locale to contain every key in
en.json. Added with the English text as the placeholder, matching how
previous key-adding PRs (#689, #691, #693) seeded new keys before
translation.
* fix(swap): address review on the pending/probe-error PR
- conversion-details-screen: submit-time errors go to a separate
`submitErrorMsg` state instead of `errorMsg`. `ConversionAmountError`
only clears `errorMsg` from an effect keyed on the amount, so a probe
error parked there left Next permanently greyed out with no hint that
nudging the amount was the only escape.
- conversion-details-screen: wrap `moveToNextScreen` in try/catch/finally.
`receivePaymentBreez` has no try/catch and the Apollo fee probe rejects
on network error, so a probe timeout left the spinner up forever plus an
unhandled rejection.
- conversion-confirmation-screen: drop the `if (lnInvoice)` wrapper so the
`!lnInvoice` guard added in useSwap is reachable. `prepareBtcToUsd` can
produce `lnInvoice: ""`, and Convert was a silent dead tap for it.
- useSwap: switch on the generated `PaymentSendResult` const instead of raw
string literals, with an explicit `FAILURE` case so `default:` means only
"unrecognised". A codegen rename would otherwise break the app at runtime
with every test green.
- useSwap: correct the errors-outrank-status comment — the server does not
pair an error with a non-failed status today; the ordering is defensive.
- useSwap: `throw new Error("Invoice is already paid")` was hardcoded
English toasted straight to the user; use the translated
`ReceiveScreen.invoicePaid`.
- useSwap: delete the three remaining `console.log(">>>>>>>>")` lines; one
dumped the full Breez receivePayment response into the retained log
buffer on every USD->BTC conversion.
- i18n: drop all three new English keys. `common.somethingWentWrong` was
byte-identical to the already-translated `errors.generic`, and the
pending copy duplicates the fully-translated
`SendBitcoinScreen.notConfirmed` the send flow already renders for an
unconfirmed payment. They had shipped as English into all 23 locales;
app/i18n/ is now byte-identical to main.
- tests: new conversion-success-screen, conversion-confirmation-screen and
conversion-details-screen suites — the wiring that turns
`{status: "pending"}` into a pending screen had no coverage at all.
Verified by mutation: hardcoding `pending`, restoring the `if (lnInvoice)`
guard, moving the submit error back into `errorMsg`, or removing the
try/catch each turns them red.
- tests: useSwap's wholesale mock of @app/graphql/generated now takes
`PaymentSendResult` from `jest.requireActual`, so a codegen change fails
the test instead of passing against stale literals.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
* fix(swap): report probe failures, clear stale submit errors, fix PaymentType mock
Follow-up review fixes on the pending/probe-error PR.
- __mocks__ PaymentType was still string-valued while the real SDK enum is
numeric (Send = 0, Receive = 1) — the same trap that had already been fixed
for PaymentStatus three lines below, left armed in the same object literal.
Any test using a realistic Breez fixture (paymentType: 1) would have compared
1 === "Receive", taken the send branch, and passed green while asserting the
opposite of production behaviour. Both enums now carry the generated numbering
and one comment covering the whole block.
- conversion-details-screen: the new catch swallowed the rejection into on-screen
text and reported it nowhere, unlike its sibling in conversion-confirmation-screen.
It now records the error to Crashlytics before setting the message, so the
fee-probe failure rate is visible in the field.
- conversion-details-screen: submitErrorMsg was cleared only on submit, so a stale
"An error occurred" stayed pinned under the inputs after a direction flip or an
amount edit — describing a request that no longer matched the screen. It now
clears on the same keys ConversionAmountError re-validates on.
Tests:
- app/types/__tests__/transactions.test.ts (new): isReceiveTransaction and
getTransactionStatus against raw numeric SDK values; red with the string mock.
- conversion-details-screen tests: the amount-validation gate test now actually
produces an error (was passing purely on "no amount entered", so deleting
`|| Boolean(errorMsg)` left the suite green); a new test drives the USD→BTC
direction and asserts prepareUsdToBtc is the one called; two new tests cover
stale submit-error clearing; the reject tests now assert the Crashlytics report.
All five fixes verified by mutation: reverting each one turns its test red.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
* fix(swap): record non-Error rejections to Crashlytics too
The catch branch added for non-Error rejections showed the generic copy
and recorded nothing, so the least diagnosable failure — an Apollo link
or Breez binding throwing a string — was also the only invisible one.
Wrap instead of branching: the report now carries the raw thrown value,
while the user still sees the generic message.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
* style: prettier formatting on the crashlytics wrap
---------
Co-authored-by: Dread <dread@example.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Companion to lnflash/flash#482 — client-side handling and visibility for the per-level daily card top-up limits (L1 $125 / L2 $1,000 / L3 $2,500).
Enforcement (pre-charge)
TopupDetailsblocks a card top-up larger than the user's level cap before the card is charged (an over-cap charge would be captured by Fygaro and strand in manual review). Exactly-at-cap allowed (inclusive), matching the backend.Visibility
globals.fygaroTopup); Bank Transfer (ACH) US$20-minimum row (shown only on bridge-enabled instances); restores the Stablesat Transfers (convert) limits section — theaccountLimitsquery always fetchedconvertbut the screen never rendered it.TopupDetails: daily-limit info label under the amount field (card), ACH minimum notice (bridge flow).BankTransfer(bridge/ACH variant): ACH minimum notice.Resilience
cardTopupLimitsquery (nottransferFlags): a backend without flash#482 — or a flash rollback — fails only that query and the app degrades to no client-side cap / no limit rows. The original wiring failed the wholetransferFlagsquery on such backends and hid the home-screen Transfer button for every leveled user (observed on test).transferFlagsnow carries a do-not-extend warning.Globals.fygaroTopupresults so they can't clobber each other in the normalized cache.useLevelQuery(cache-and-network), not the cache-only level context.Known constants
Deploy order
Backend first (lnflash/frappe-flash-admin#66, then lnflash/flash#482) for the feature to be active; the app no longer hard-depends on them — against an old backend everything degrades gracefully.
Review
Two
simon-review-fixpasses (3 + 2 rounds): level-0 refusal + net-preview leak, cold-start level race, ACH gating onbridgeEnabled, cache merge policy — all applied. Final verdict clean.Tests
56 suites / 343 tests pass;
check-code(tsc, translations, codegen, graphql-check) clean.🤖 Generated with Claude Code
https://claude.ai/code/session_01Nt7JxbX5W9o7viKegnXJPH