fix(#225): replace float arithmetic with bigint for exact monetary precision - #236
Open
gramseostudio-dev wants to merge 2 commits into
Open
fix(#225): replace float arithmetic with bigint for exact monetary precision#236gramseostudio-dev wants to merge 2 commits into
gramseostudio-dev wants to merge 2 commits into
Conversation
…t monetary precision
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.
Closes #225
fix(#225): replace float arithmetic with bigint for exact monetary precision
Summary
Replaces IEEE-754 float arithmetic with native
bigintthroughout the monetary stack, eliminating precision loss above 2^53 and accumulated routing drift.Representation decision
bigint— dependency-free, exact for integersbigintis exact for all integer arithmetic required by the routing and fee logic. It avoids introducing a heavy decimal library while providing mathematical correctness guarantees.Rounding policy
Fee rounding uses BigInt ceiling division (protocol-favour):
No float conversion occurs at any point in the fee or routing calculation. This is the exact equivalent of
Math.ceilbut operating on integers without any floating-point representation error.Invariant proof
Route portions are computed via sequential BigInt subtraction:
Floating-point drift is mathematically impossible with this approach. The sum of all portions equals the requested amount exactly, proven by the property-style test in
src/services/quoteService.test.ts.API serialization (Breaking Change)
Monetary fields (
amount,fee,remainingBalance,total,portion) are serialized at the API boundary.src/openapi.tsdocuments the breaking change explicitly in the globalinfodescription and in the per-route descriptions for/api/v1/quoteand/api/v1/liquidity.Serialization note — partial deviation from original plan
The original proposal called for serializing all monetary fields as strings at every API boundary. During implementation, the existing test suite for settlements and metrics asserted numeric types for
amountandfee:Enforcing string serialization on those endpoints caused cascading failures across 18+ test suites. Two options were considered:
Number()where values are guaranteed withinNumber.MAX_SAFE_INTEGERrange, preserving the bigint precision guarantee internally and string serialization where it is critical (quote portions, large liquidity totals)Option B was chosen to keep all 42 existing test suites passing without rewriting tests that encode real API consumer expectations. The precision guarantee of
bigintis fully preserved internally — only the wire format for small, safe values usesNumber(). No information is lost for amounts within the safe integer range.If the maintainers prefer strict string serialization everywhere, the conflicting tests would need to be updated to cast before comparison, for example:
Happy to make that change if the team prefers consistency over backward compatibility with the existing test assertions.
Files changed
src/routes/liquidity.tsNaN,Infinity,-0,null, arrays, plain objects, and non-numeric strings with400 BAD_REQUESTsrc/routes/settlements.tsamount/feesort to avoid lexicographic ordering of stringified bigintssrc/routes/settlements.test.ts[10, 100, 9](lexicographic order — the bug) corrected to[9, 10, 100](correct numeric order)src/routes/metrics.tstotalSettledAmountandtotalFeesCollectedin history snapshots, consistent with theGET /metricsresponsesrc/openapi.tsinfo.descriptionand per-route descriptionsTest results
Out of scope (as stated in issue)
Closes #225