fix(cashout): make JMD exchange-rate conversion decimal-safe [ENG-509]#449
Merged
Conversation
`JMDAmount.dollars(d)` did `new JMDAmount(BigInt(d) * 100n)`, and `BigInt(d)`
throws on any non-integer `d`. The cashout offer wraps the live NCB rate
(essentially always fractional — 152.7, 155.5, ...) via this method, so
`getCashoutExchangeRate` returned `ExchangeRateQueryError` for every real
rate and `requestCashout` failed. Rewrote it to mirror `USDAmount.dollars`
(Money-lib, HALF_TO_EVEN) so fractional rates convert correctly.
Compounding it, `error-map.ts` had no `ExchangeRateQueryError` case, so the
failure fell through to the exhaustiveness fallback ("This should never
compile ...") and returned a `data: null` top-level GraphQL error — the app
therefore showed no message, just a spinner. Added a mapped case.
Only surfaces post-#444/#445 (live NCB rate); a deploy would break JMD
cashout in every env until this lands.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bobodread876
approved these changes
Jul 15, 2026
islandbitcoin
added a commit
that referenced
this pull request
Jul 15, 2026
…ashout (#450) * fix(cashout): decimal-safe JMD rate on the ACTUAL JMDAmount used by cashout Follow-up to #449. There are two `JMDAmount` classes in the tree: `src/domain/shared/MoneyAmount.ts` (fixed in #449) and `src/domain/shared/money/JMDAmount.ts`. The barrel `domain/shared/index.ts` exports JMDAmount from `./money` (`export * from "./money"`, taking only USDTAmount from MoneyAmount.ts), so cashout — via `@domain/shared` — imports the `money/JMDAmount.ts` copy, which still did `new JMDAmount(BigInt(d) * 100n)` and threw on every fractional NCB rate. #449's conversion fix therefore had no effect on cashout (its error-map half did work). Applied the same decimal-safe rewrite (mirror `money/USDAmount.dollars`) to `money/JMDAmount.ts`. Verified: dollars(155.5)=155.50, 152.7=152.70, 0.5=0.50. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(cashout): regression test for fractional JMD rates [ENG-509] Adds a fractional-rate case to Money.spec.ts, which imports JMDAmount from @domain/shared — the exact barrel-resolved class the cashout path uses. The only pre-existing JMDAmount.dollars test used an integer (160), so the suite stayed green while every fractional NCB rate crashed cashout in prod. Verified: fails without the fix (BigIntConversionError on 155.5), passes with it; full suite 22/22, tsc --noEmit clean. --------- Co-authored-by: Dread <bobodread@bobodread.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
islandbitcoin
added a commit
that referenced
this pull request
Jul 17, 2026
…8] (#451) * refactor(shared): remove dead MoneyAmount twins (USD/JMD/BTC) [ENG-518] The top-level src/domain/shared/MoneyAmount.ts defined USDAmount, JMDAmount, and BtcAmount that were dead: the @domain/shared barrel resolves those from ./money (export * from "./money") and takes only USDTAmount from this file. The duplicates shadowed the live ./money classes, which let the cashout JMD rate fix land on the wrong copy (#449 patched here; #450 fixed the real ./money one). Delete the three dead classes; scope this file and MoneyAmount.from() to the base + USDTAmount, its only live exports. Comment added so no currency-amount class is re-added here. Verified: tsc --noEmit clean project-wide; Money.spec.ts 22/22 (it imports JMDAmount from the barrel = the live ./money class, and exercises MoneyAmount.from(Usdt)/fromJSON via the trimmed factory). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(shared): correct MoneyAmount header comment [ENG-518] The previous header claimed this file was "the canonical home of the base MoneyAmount". It is not: the @domain/shared barrel does `export * from "./money"`, so ./money/MoneyAmount.ts is the base the app resolves as `MoneyAmount`; this file only contributes `USDTAmount`. Since USDTAmount extends this file's (separate) base, `x instanceof MoneyAmount` (the barrel's) does not match it — the comment now states that and points at the OffersSerde `|| instanceof USDTAmount` guard, and flags the two-base consolidation as the real follow-up. Also notes in Money.spec.ts why it imports MoneyAmount from the file path rather than the barrel. Comment-only; no behavioral change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(shared): point MoneyAmount base-dedup follow-up at ENG-523 [ENG-518] Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Dread <bobodread@bobodread.com> Co-authored-by: Claude Opus 4.8 (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
2 participants
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.
Problem
JMD cashout (
requestCashout) silently fails on TEST — the "Next" button spins and the screen never advances, with no error shown. Two bugs stacked:JMDAmount.dollars(d)(src/domain/shared/MoneyAmount.ts) didnew JMDAmount(BigInt(d) * 100n).BigInt(155.5)throws (non-integer) →BigIntConversionError. The cashout offer wraps the live NCB rate via this method (getCashoutExchangeRate), and NCB rates are ~always fractional (152.7, 155.5…), so it returnedExchangeRateQueryErrorfor every real rate.error-map.tshad noExchangeRateQueryErrorcase → the error fell through to the exhaustiveness fallback and was returned as adata: nulltop-level GraphQL error. The mobile app readsres.data?.requestCashout.errors[0]?.message; withdatanull it gets nothing → no visible error, just a spinner.Reproduced with a real session token:
{"errors":[{"message":"This should never compile with ExchangeRateQueryError"}],"data":null}. Setting the ERPNext rate to a whole number maderequestCashoutreturn a valid offer — confirming the cause.Fix
JMDAmount.dollarsto mirror the already-correctUSDAmount.dollars(uses theMoneylib withHALF_TO_EVEN, string-based, noBigIntof a fractional). Fractional rates now convert correctly.ExchangeRateQueryErrorcase toerror-map.tsreturning a clearUnexpectedClientErrormessage, so a rate failure surfaces to the user instead of a blankdata: null.Impact / urgency
Only reachable on builds with the live-NCB-rate cashout (#444/#445). Those are not yet on prod, which is why prod JMD cashout works today — but a prod deploy of that code would break JMD cashout in every environment until this merges. Effectively a release blocker for #445.
Follow-up worth a ticket: a unit test asserting
JMDAmount.dollars(155.5)succeeds.🤖 Generated with Claude Code