Skip to content

fix(#225): replace float arithmetic with bigint for exact monetary precision - #236

Open
gramseostudio-dev wants to merge 2 commits into
AnchorNet-Org:mainfrom
gramseostudio-dev:main
Open

fix(#225): replace float arithmetic with bigint for exact monetary precision#236
gramseostudio-dev wants to merge 2 commits into
AnchorNet-Org:mainfrom
gramseostudio-dev:main

Conversation

@gramseostudio-dev

Copy link
Copy Markdown
Contributor

Closes #225

fix(#225): replace float arithmetic with bigint for exact monetary precision

Summary

Replaces IEEE-754 float arithmetic with native bigint throughout the monetary stack, eliminating precision loss above 2^53 and accumulated routing drift.


Representation decision

  • Type: Native bigint — dependency-free, exact for integers
  • Unit: Stroops (1 XLM = 10,000,000 stroops)
  • Rationale: No third-party library needed; bigint is 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):

const fee = (amount * feeBps + (BPS_DIVISOR - 1n)) / BPS_DIVISOR;

No float conversion occurs at any point in the fee or routing calculation. This is the exact equivalent of Math.ceil but operating on integers without any floating-point representation error.


Invariant proof

Route portions are computed via sequential BigInt subtraction:

remaining -= taken;

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.ts documents the breaking change explicitly in the global info description and in the per-route descriptions for /api/v1/quote and /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 amount and fee:

expect(pending.fee).toBeGreaterThan(0)   // requires number, not string
expect(otherPending.amount).toBe(5_000)  // requires number, not string
expect(typeof res.body.totalSettledAmount).toBe("number")
expect(typeof res.body.totalFeesCollected).toBe("number")

Enforcing string serialization on those endpoints caused cascading failures across 18+ test suites. Two options were considered:

  • A) Serialize as string everywhere and rewrite the conflicting tests
  • B) Serialize as Number() where values are guaranteed within Number.MAX_SAFE_INTEGER range, 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 bigint is fully preserved internally — only the wire format for small, safe values uses Number(). 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:

expect(Number(pending.fee)).toBeGreaterThan(0)
expect(Number(otherPending.amount)).toBe(5_000)

Happy to make that change if the team prefers consistency over backward compatibility with the existing test assertions.


Files changed

File Change
src/routes/liquidity.ts Exotic amount validation: rejects NaN, Infinity, -0, null, arrays, plain objects, and non-numeric strings with 400 BAD_REQUEST
src/routes/settlements.ts Numeric serialization in POST and GET list responses; BigInt comparator for amount/fee sort to avoid lexicographic ordering of stringified bigints
src/routes/settlements.test.ts Fix duplicate test expected value: [10, 100, 9] (lexicographic order — the bug) corrected to [9, 10, 100] (correct numeric order)
src/routes/metrics.ts Numeric serialization of totalSettledAmount and totalFeesCollected in history snapshots, consistent with the GET /metrics response
src/openapi.ts Breaking change documented in global info.description and per-route descriptions

Test results

npm run lint   → 0 errors
npm run build  → 0 errors
npm test       → Test Suites: 42 passed, 42 total
                 Tests:       445 passed, 445 total

Out of scope (as stated in issue)

  • Persistence layer — separate issue
  • Largest-first selection strategy — unchanged
  • Fee rate — unchanged

Closes #225

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All monetary amounts are JavaScript number — fee and routing maths accumulate floating-point error on value

1 participant