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

Priority: High  ·  Area: Numeric correctness / money handling  ·  Est. effort: 10–14 h

📌 Problem

Every monetary field in src/models/liquidity.ts is a JavaScript number — an IEEE-754 double:

amount: number;             // :12, :30, :49, :63
remainingBalance: number;   // :32
total: number;              // :40
portion: number;            // :57

src/services/quoteService.ts does arithmetic on them:

const taken = Math.min(remaining, entry.amount);              // :49  (inside the routing loop)
const fee = Math.ceil((amount * this.feeBps) / BPS_DIVISOR);  // :54

Two distinct problems.

1. Precision loss above 2^53. Doubles represent integers exactly only up to 9,007,199,254,740,991. Stellar amounts are commonly held in stroops (1 XLM = 10,000,000 stroops), so 900,719,925 XLM exceeds the safe range. Past it, addition silently stops being exact.

2. Accumulated error in the routing loop. The service selects liquidity largest-first, subtracting taken from remaining each iteration. Repeated float subtraction accumulates representation error, so the sum of the route portions can fail to equal the requested amount by a small residue — and Math.ceil on the fee then rounds a value that is already slightly wrong.

The service's own doc comment calls the quote "deterministic". Float arithmetic across a variable number of iterations, in a variable order, is not reliably so.

🎯 Design decision required

Post your approach in a comment on this issue before writing code — this touches the models, the services and the API boundary. State and defend:

1. Representation. bigint for integer minor units, or a decimal library? bigint has no dependency and is exact, but does not serialise to JSON natively and cannot express fractions. Argue one, and state the unit you standardise on.
2. API compatibility. JSON has no integer type beyond double precision, so large values must serialise as strings to survive the round trip. That is a breaking API change. State it plainly and describe the migration.
3. Rounding policy. Math.ceil on the fee rounds in the protocol's favour. Whatever representation you choose, state the rounding rule explicitly, apply it consistently, and prove the route portions sum exactly to the requested amount.

🧩 Requirements and context

• Add tests demonstrating the current defect first — a value above Number.MAX_SAFE_INTEGER, and a multi-anchor route whose portions do not sum exactly. Those tests are the evidence the change is warranted.
• The invariant "sum of route portions + fee equals the requested total, exactly" must hold after the change, with a property-style test over many inputs.
• src/models/settlement.ts and the settlement path use the same amounts — check and include them, or scope explicitly and say what you left.
• All 42 test files must pass.
• Update src/openapi.ts to reflect any serialisation change.

🛠️ Suggested execution

1. Write the two failing tests (large value, non-summing route).
2. Post your representation and API-compatibility plan; wait for agreement.
3. Convert models, then services.
4. Add the exact-sum property test.
5. Update the OpenAPI spec and document the breaking change.

✅ Acceptance criteria

• Tests demonstrate precision loss and route-sum drift against the current code.
• Monetary values use an exact representation; the unit is stated.
• Route portions plus fee sum exactly to the requested amount, proven by a property-style test.
• The rounding rule is documented and applied consistently.
• Any API serialisation change is reflected in src/openapi.ts and documented as breaking.
• All existing tests pass.
• Anything deliberately left out of scope is listed.

🚫 Out of scope

• The persistence layer — separate issue.
• Changing the largest-first selection strategy.
• Changing the fee rate.

🧪 Verification

npm ci
npm test src/services
npm run lint && npm run build && npm test

📤 What your PR must include

1. The two failing tests and their output.
2. A link to the agreed representation plan.
3. Your rounding policy.
4. The breaking-change description and migration.
5. Closes #<n>.

🔒 Security notes

Float arithmetic on money is a correctness failure with financial consequences: a route whose portions do not sum to the requested amount means the system's accounting does not balance, and the discrepancy is small enough to escape notice while being systematic rather than random. Math.ceil on the fee compounds this by always rounding the same direction. Any residue that repeatedly favours one side of a settlement is a value leak, whether or not anyone is deliberately exploiting it.

📋 Guidelines

• Minimum 95% test coverage on changed lines
• Clear documentation
• Timeframe: 96 hours from assignment
• One logical change per commit; no merge commits

💬 Join our community

Working on this, or want to sanity-check your approach before you start? Come and ask — the maintainers are there and happy to help.

Telegram: https://t.me/Grainlify