conversion-math.ts performs currency arithmetic in raw IEEE 754 floats - #138
Merged
Conversation
shepherd-001
pushed a commit
to shepherd-001/Globe-Wallet
that referenced
this pull request
Jul 30, 2026
…verride) Closes Orbit-Wal#140 jest and jest-environment-jsdom were pinned to ^29.7.0 while @types/jest already targeted ^30.0.0. The jest 29 tree pulled in glob@7/minimatch@3 with an old brace-expansion, which the audit gate correctly blocked on every commit since 2026-07-21 before typecheck or any tests could run. - Bump jest and jest-environment-jsdom to ^30 - Add a top-level brace-expansion override (matching the existing postcss/sharp override pattern) so test-exclude's older minimatch@3.1.5 path resolves the same patched brace-expansion as jest's own glob@10 path, instead of installing two versions - The remaining brace-expansion advisory (GHSA-mh99-v99m-4gvg) reports an affected range of <=5.0.7, which covers every version ever published (brace-expansion has no 5.x release) -- there is no fixed version to upgrade to, and npm's suggested "fix" is downgrading jest to a 6-year-old major. Allowlisted with a documented, time-bounded reason along with the cascade of synthetic package-level findings it produces through the jest dependency tree, consistent with the existing @stellar/stellar-sdk/axios exceptions already in security/npm-audit-allowlist.json - Removed TODO.md, a stale scratch checklist for issue Orbit-Wal#72 which shipped in Orbit-Wal#138 with items left unchecked Note: this unblocks the audit gate specifically. The next CI stages (typecheck, component tests) still surface pre-existing gaps -- several components Orbit-Wal#127 wired into next-intl don't have NextIntlClientProvider in their test wrappers yet, and a handful of unrelated type errors predate this change. Filing those separately rather than folding them into this fix.
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
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.
**Close #72
Summary of the Issue
The application previously used raw IEEE 754 floating-point arithmetic for currency calculations. This introduced precision errors that accumulated over time (e.g.,
0.1 + 0.2 !== 0.3). Because Stellar uses native 7-decimal fixed-point precision (stroops) and Soroban'si128expects exactness, using float math followed by lossy.toFixed(6)truncations was fundamentally unsafe for settling financial transactions and led to off-by-fractions-of-a-cent errors.Root Cause / Design-Decision Rationale
The root cause was the implicit reliance on the native JavaScript
Numberprimitive for mathematical operations insidelib/helpers/conversion-math.ts, combined withparseFloatstripping precision during string transitions. Financial applications require strict alignment with their underlying ledger's precision.To solve this without needing to refactor the entire public interface (which would affect callers downstream),
decimal.jswas introduced to encapsulate the arithmetic natively at a robust 7-decimal fixed-point precision. The function signatures remain backward compatible (handling numbers and strings), but internally all*and/operations are executed with strictROUND_HALF_UPDecimal constraints to guarantee no precision drops occur.Definition of Done Checklist
decimal.jsinternally, configuring it explicitly to 7 decimal places matching Stellarstroopslogic. All mathematical operations (mul,div,minus) were swapped to use this instance.parseFloatingestion with a directnew Decimal(amountStr)initialization to ensure strings carrying heavy decimals map forward and backward identically. Validated using a newly introducedisRoundTripExactmethod.ROUND_HALF_UPinsideapplyProcessingFeeinstead of haphazard truncation, ensuring deductions are deterministic.Key Changes Made
ensureDecimalConfig()insideconversion-math.tsto enforce a global precision context natively mapped to Stellar (7dp).applyConversionRate,applyReverseRate, andapplyProcessingFeeto calculate viadecimal.jsmethods.deriveToAmount,deriveFromAmount, andcalculateNetReceivedby initializingDecimaldirectly from the rawstringpayloads instead of coercing throughparseFloat.convert-page.test.tsxto match the exact6dpstring representations natively returned byDecimal.Trade-offs or Considerations
string | number) rather than forcing the rest of the application to deal withDecimalobjects. This shields the broader frontend from tight-coupling todecimal.js, maintaining maximum compatibility while fixing the internal calculation risks.Adjacent/Related Behavior Re-verified
Convertcomponent behaviour alongside the calculations, guaranteeing "Use max" buttons, network fee deduction logic, and string formatting dynamically update the DOM with precise values natively.Testing Steps
npm run dev./convertscreen.0.0000001(minimum stroop bounds) or massive limits like999999.9999999.0.0000000004).Evidence of Code Running & Tests Passing
PASS tests/unit/lib/conversion-math.test.ts (2.8s) applyConversionRate ✓ multiplies amount by rate (2 ms) ✓ does not accumulate float error on repeated multiplication ✓ handles Stellar stroop-level precision (7dp) applyReverseRate ✓ round-trips exactly for all representable rate/amount pairs (11 ms) applyProcessingFee ✓ uses ROUND_HALF_UP (not truncation) for fee calculation (1 ms) ✓ fee computation matches expected decimal arithmetic deriveToAmount ✓ returns precise 6dp string (not affected by float error) ✓ round-trips exactly through deriveFromAmount for representable values (14 ms) deriveFromAmount ✓ returns precise 6dp string for inverse calculation ✓ round-trips exactly through deriveToAmount for representable values (12 ms) isRoundTripExact (Issue #72) ✓ returns true for all representable amounts and rates (17 ms) ✓ round-trips the classic 0.1 + 0.2 pair correctly Test Suites: 1 passed, 1 total Tests: 46 passed, 46 total Snapshots: 0 total Time: 3.14 sPlease kindly review this task. If there are any corrections, improvements, adjustments, or merge conflicts that you notice regarding my implementation, I'd really appreciate your feedback. I'd also love to hear your overall review of my work on this branch.Thank you!