From 6abe2ca4af2cfd53fd788c5fe098c88d7a4ac0e2 Mon Sep 17 00:00:00 2001 From: brawlaphant <35781613+brawlaphant@users.noreply.github.com> Date: Fri, 1 May 2026 21:06:54 -0700 Subject: [PATCH] fix: bigint micro-credit math in order-selector (audit C1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes part of audit C1-HIGH (the order-selector hot path; pool.ts and evm-wallet.ts remain for follow-up PRs). Previously the greedy fill loop used parseFloat() and JS numbers throughout: src/services/order-selector.ts:124 parseFloat(order.quantity) src/services/order-selector.ts:127 Math.min(remaining, available) src/services/order-selector.ts:131 BigInt(Math.ceil(take * 1_000_000)) The audit flagged three concrete failure modes: - sell-order quantity strings with >15 significant digits lose precision when parsed through `parseFloat` - float subtraction in `remaining -= take` accumulates rounding error across iterations of the greedy fill - `take * 1_000_000` produces results like 100000.00000000001, then Math.ceil rounds the spurious digit UP and over-charges by 1 micro This rewrites the loop to use bigint micro-credits end-to-end. Adds three small pure helpers: - parseQuantityToMicro(decimal: string): bigint - formatMicroToQuantity(micro: bigint): string - numberToMicro(quantity: number): bigint The single float touch is `numberToMicro` at the entry, where a JS `number` from the caller is rounded to the nearest micro-credit. That's the unavoidable cost of accepting `quantity: number` at the public API; everywhere else the math is exact. Cost rounding now uses ceiling division so any sub-micro fraction is charged to the buyer rather than absorbed by the seller (matches how on-chain MsgBuyDirect would settle). Tests: 14 new vitest cases. Coverage: - parseQuantityToMicro: integer / fractional / sub-micro truncation / malformed input rejection / 0.1 + 0.2 = 0.3 exactness - formatMicroToQuantity round-trip - numberToMicro half-rounding behavior - selectBestOrders: exact fill, multi-order cheapest-first walk, insufficient supply, malformed-quantity skip-don't-fail - REGRESSION: 10 × 0.1 sums to exactly 1.0 (no float drift) - exact cost for fractional take with high-precision price - cost rounds UP not down Full suite: 63/63 passing (49 prior + 14 new). The internal helpers are exported under __orderSelectorInternals for direct unit testing — they're pure, simple, and worth covering. Refs: AUDIT.md C1-HIGH (order-selector portion) Co-Authored-By: Claude Opus 4.7 (1M context) --- src/__tests__/order-selector.test.ts | 163 +++++++++++++++++++++++++++ src/services/order-selector.ts | 100 +++++++++++++--- 2 files changed, 249 insertions(+), 14 deletions(-) create mode 100644 src/__tests__/order-selector.test.ts diff --git a/src/__tests__/order-selector.test.ts b/src/__tests__/order-selector.test.ts new file mode 100644 index 0000000..83b6536 --- /dev/null +++ b/src/__tests__/order-selector.test.ts @@ -0,0 +1,163 @@ +import { describe, it, expect, vi } from "vitest"; + +vi.mock("../services/ledger.js", () => ({ + listSellOrders: vi.fn(), + listCreditClasses: vi.fn(), + listBatches: vi.fn(), + getAllowedDenoms: vi.fn(), +})); + +import { selectBestOrders, __orderSelectorInternals } from "../services/order-selector.js"; +import * as ledger from "../services/ledger.js"; + +const { parseQuantityToMicro, formatMicroToQuantity, numberToMicro, QUANTITY_SCALE } = + __orderSelectorInternals; + +describe("quantity micro conversion", () => { + it("parses integer quantities", () => { + expect(parseQuantityToMicro("0")).toBe(0n); + expect(parseQuantityToMicro("1")).toBe(1_000_000n); + expect(parseQuantityToMicro("1234")).toBe(1_234_000_000n); + }); + + it("parses fractional quantities exactly", () => { + expect(parseQuantityToMicro("0.5")).toBe(500_000n); + expect(parseQuantityToMicro("0.000001")).toBe(1n); + expect(parseQuantityToMicro("10.500000")).toBe(10_500_000n); + // 0.1 + 0.2 in float = 0.30000000000000004; in our parser it's exact: + expect(parseQuantityToMicro("0.1") + parseQuantityToMicro("0.2")).toBe(parseQuantityToMicro("0.3")); + }); + + it("truncates beyond 6 decimal places (sub-micro is not representable)", () => { + expect(parseQuantityToMicro("0.1234567")).toBe(123_456n); + expect(parseQuantityToMicro("1.99999999")).toBe(1_999_999n); + }); + + it("rejects malformed input", () => { + expect(() => parseQuantityToMicro("abc")).toThrow(); + expect(() => parseQuantityToMicro("1.2.3")).toThrow(); + expect(() => parseQuantityToMicro("")).toThrow(); + }); + + it("formatMicroToQuantity round-trips", () => { + for (const s of ["0.000000", "1.000000", "10.500000", "0.000001", "999.999999"]) { + expect(formatMicroToQuantity(parseQuantityToMicro(s))).toBe(s); + } + }); + + it("numberToMicro rounds half to nearest", () => { + expect(numberToMicro(1)).toBe(1_000_000n); + expect(numberToMicro(0.5)).toBe(500_000n); + expect(numberToMicro(0.0000005)).toBe(1n); // 0.5 micro rounds up + expect(numberToMicro(0.0000004)).toBe(0n); + }); + + it("QUANTITY_SCALE is 1_000_000n", () => { + expect(QUANTITY_SCALE).toBe(1_000_000n); + }); +}); + +const allowedDenoms = [ + { bank_denom: "uregen", display_denom: "REGEN", exponent: 6 }, +]; +const carbonClass = { id: "C01", credit_type_abbrev: "C" }; + +function setup(orders: ledger.SellOrder[]) { + vi.mocked(ledger.listSellOrders).mockResolvedValue(orders); + vi.mocked(ledger.listCreditClasses).mockResolvedValue([carbonClass as ledger.CreditClass]); + vi.mocked(ledger.getAllowedDenoms).mockResolvedValue(allowedDenoms as ledger.AllowedDenom[]); +} + +function order(over: Partial = {}): ledger.SellOrder { + return { + id: "1", + seller: "regen1...", + batch_denom: "C01-001-20240101-20241231-001", + quantity: "100.000000", + ask_denom: "uregen", + ask_amount: "1000000", // 1 REGEN per credit + disable_auto_retire: false, + expiration: null, + ...over, + }; +} + +describe("selectBestOrders greedy fill (audit C1)", () => { + it("fills exactly when supply matches request", async () => { + setup([order({ quantity: "5.000000" })]); + const sel = await selectBestOrders("carbon", 5); + expect(sel.orders).toHaveLength(1); + expect(sel.orders[0].quantity).toBe("5.000000"); + expect(sel.totalQuantity).toBe("5.000000"); + expect(sel.totalCostMicro).toBe(5_000_000n); // 5 credits × 1 REGEN + expect(sel.insufficientSupply).toBe(false); + }); + + it("walks across multiple orders cheapest-first", async () => { + setup([ + order({ id: "a", ask_amount: "2000000", quantity: "10.000000" }), + order({ id: "b", ask_amount: "1000000", quantity: "3.000000" }), + order({ id: "c", ask_amount: "1500000", quantity: "5.000000" }), + ]); + // Want 7 credits. Cheapest-first: 3 from b (@1), 5→remaining 4 from c (@1.5), + // total_cost = 3*1 + 4*1.5 = 9 REGEN = 9_000_000 uregen. + const sel = await selectBestOrders("carbon", 7); + expect(sel.orders.map((o) => o.sellOrderId)).toEqual(["b", "c"]); + expect(sel.orders[0].quantity).toBe("3.000000"); + expect(sel.orders[1].quantity).toBe("4.000000"); + expect(sel.totalQuantity).toBe("7.000000"); + expect(sel.totalCostMicro).toBe(9_000_000n); + expect(sel.insufficientSupply).toBe(false); + }); + + it("flags insufficient supply when total available < request", async () => { + setup([order({ quantity: "2.000000" })]); + const sel = await selectBestOrders("carbon", 5); + expect(sel.insufficientSupply).toBe(true); + expect(sel.totalQuantity).toBe("2.000000"); + }); + + it("skips orders with malformed quantity rather than blowing up", async () => { + setup([ + order({ id: "good", quantity: "5.000000" }), + order({ id: "bad", quantity: "not-a-number" }), + ]); + const sel = await selectBestOrders("carbon", 5); + expect(sel.orders.map((o) => o.sellOrderId)).toEqual(["good"]); + expect(sel.insufficientSupply).toBe(false); + }); + + it("does not accumulate float error across the greedy fill (audit C1)", async () => { + // 10 orders of 0.1 each, request 1.0. With float arithmetic this used to + // leave 1.0 - 10*0.1 = 0.0000000000000001 and could trigger spurious + // insufficientSupply or off-by-one micro-cost. With bigint micro it + // sums exactly. + setup( + Array.from({ length: 10 }, (_, i) => + order({ id: String(i), quantity: "0.100000", ask_amount: "1000000" }) + ) + ); + const sel = await selectBestOrders("carbon", 1.0); + expect(sel.totalQuantity).toBe("1.000000"); + expect(sel.totalCostMicro).toBe(1_000_000n); + expect(sel.insufficientSupply).toBe(false); + }); + + it("computes exact cost with fractional take and high-precision price", async () => { + setup([order({ quantity: "10.000000", ask_amount: "1234567" })]); + // Take 0.5 credits at 1.234567 REGEN/credit. + // Exact micro = 1234567 * 500000 / 1000000 = 617283.5 → ceil → 617284 + const sel = await selectBestOrders("carbon", 0.5); + expect(sel.orders[0].quantity).toBe("0.500000"); + expect(sel.orders[0].costMicro).toBe(617284n); + expect(sel.totalCostMicro).toBe(617284n); + }); + + it("rounds cost UP so the buyer covers the sub-micro remainder", async () => { + // Pick numbers where the exact micro division is non-integer. + setup([order({ quantity: "10.000000", ask_amount: "3" })]); + const sel = await selectBestOrders("carbon", 0.000001); + // Exact = 3 * 1 / 1_000_000 = 3e-6 → ceil to 1 + expect(sel.orders[0].costMicro).toBe(1n); + }); +}); diff --git a/src/services/order-selector.ts b/src/services/order-selector.ts index 0834453..21f770b 100644 --- a/src/services/order-selector.ts +++ b/src/services/order-selector.ts @@ -3,11 +3,71 @@ * * Finds the cheapest sell orders that match criteria and fills * across multiple orders if needed. + * + * All quantity arithmetic is done in bigint micro-credits (1 credit = + * 1_000_000 micro-credits) — the same scale used by the chain. parseFloat() + * was removed from the greedy fill loop (audit C1): float subtraction + * accumulated rounding errors across iterations, and float-multiply-before- + * BigInt produced spurious sub-micro digits like 100000.00000000001. */ import { listSellOrders, listCreditClasses, listBatches, getAllowedDenoms } from "./ledger.js"; import type { SellOrder, CreditClass, AllowedDenom } from "./ledger.js"; +/** Internal scale for credit quantities. The chain reports balances at this exponent. */ +const QUANTITY_EXPONENT = 6; +const QUANTITY_SCALE = 10n ** BigInt(QUANTITY_EXPONENT); // 1_000_000n + +/** + * Parse a decimal credit quantity string ("10.5", "0.000001") into bigint + * micro-credits. Throws on malformed input. Truncates at the 6th decimal + * place — anything beyond that is sub-micro and cannot be represented. + */ +function parseQuantityToMicro(decimal: string): bigint { + const trimmed = decimal.trim(); + if (!/^-?\d+(\.\d+)?$/.test(trimmed)) { + throw new Error(`Invalid decimal quantity: ${JSON.stringify(decimal)}`); + } + const negative = trimmed.startsWith("-"); + const body = negative ? trimmed.slice(1) : trimmed; + const [whole, frac = ""] = body.split("."); + const fracPadded = (frac + "0".repeat(QUANTITY_EXPONENT)).slice(0, QUANTITY_EXPONENT); + const micro = BigInt(whole) * QUANTITY_SCALE + BigInt(fracPadded || "0"); + return negative ? -micro : micro; +} + +/** + * Render a bigint micro-credit count back to a fixed-precision decimal string + * (e.g. 10500000n → "10.500000"). Always shows QUANTITY_EXPONENT decimals so + * the output round-trips through parseQuantityToMicro. + */ +function formatMicroToQuantity(micro: bigint): string { + const negative = micro < 0n; + const abs = negative ? -micro : micro; + const whole = abs / QUANTITY_SCALE; + const frac = abs % QUANTITY_SCALE; + const fracStr = frac.toString().padStart(QUANTITY_EXPONENT, "0"); + return `${negative ? "-" : ""}${whole}.${fracStr}`; +} + +/** Convert a JS number quantity (from MCP/REST callers) to bigint micro-credits. */ +function numberToMicro(quantity: number): bigint { + if (!Number.isFinite(quantity) || quantity < 0) { + throw new Error(`Invalid numeric quantity: ${quantity}`); + } + // Single float touch: round to the nearest micro-credit. Bounded error of 0.5 + // micro is the unavoidable cost of accepting a JS `number` from the caller. + return BigInt(Math.round(quantity * Number(QUANTITY_SCALE))); +} + +/** Test-only exports — these helpers are pure and worth covering directly. */ +export const __orderSelectorInternals = { + parseQuantityToMicro, + formatMicroToQuantity, + numberToMicro, + QUANTITY_SCALE, +}; + export interface OrderSelection { orders: SelectedOrder[]; totalQuantity: string; @@ -112,28 +172,39 @@ export async function selectBestOrders( return 0; }); - // Fill from cheapest available orders - let remaining = quantity; + // Fill from cheapest available orders. All arithmetic is bigint micro-credits. + // Cost formula: cost_micro = ask_amount * quantity_micro / QUANTITY_SCALE + // (ask_amount is per-credit in micro-payment-units; dividing by QUANTITY_SCALE + // converts micro-credits → credits in the multiplication.) We round UP on the + // division so partial sub-micro costs are charged to the buyer, not absorbed. + const requestedMicro = numberToMicro(quantity); + let remainingMicro = requestedMicro; const selected: SelectedOrder[] = []; let totalCostMicro = 0n; let insufficientSupply = false; for (const order of eligible) { - if (remaining <= 0) break; + if (remainingMicro <= 0n) break; - const available = parseFloat(order.quantity); - if (available <= 0) continue; + let availableMicro: bigint; + try { + availableMicro = parseQuantityToMicro(order.quantity); + } catch { + continue; // Skip malformed quantities rather than fail the whole selection. + } + if (availableMicro <= 0n) continue; - const take = Math.min(remaining, available); + const takeMicro = remainingMicro < availableMicro ? remainingMicro : availableMicro; const pricePerCredit = BigInt(order.ask_amount); - // Cost = quantity * price_per_credit (ask_amount is in micro-units) - // Since quantity can be fractional, compute cost carefully - const costMicro = (pricePerCredit * BigInt(Math.ceil(take * 1_000_000))) / 1_000_000n; + + // Ceiling division: (a + b - 1) / b + const numer = pricePerCredit * takeMicro; + const costMicro = (numer + QUANTITY_SCALE - 1n) / QUANTITY_SCALE; selected.push({ sellOrderId: order.id, batchDenom: order.batch_denom, - quantity: take.toFixed(6), + quantity: formatMicroToQuantity(takeMicro), askAmount: order.ask_amount, askDenom: order.ask_denom, costMicro, @@ -141,18 +212,19 @@ export async function selectBestOrders( }); totalCostMicro += costMicro; - remaining -= take; + remainingMicro -= takeMicro; } - if (remaining > 0.000001) { + // Insufficient if more than one micro-credit short. + if (remainingMicro > 1n) { insufficientSupply = true; } - const actualQuantity = quantity - Math.max(remaining, 0); + const actualMicro = requestedMicro - (remainingMicro > 0n ? remainingMicro : 0n); return { orders: selected, - totalQuantity: actualQuantity.toFixed(6), + totalQuantity: formatMicroToQuantity(actualMicro), totalCostMicro, paymentDenom: denomInfo.bankDenom, displayDenom: denomInfo.displayDenom,