From 3e2e18c31d6553966dac1a558d9ec1546e70197c Mon Sep 17 00:00:00 2001 From: Henry Eulam Eliazar Date: Mon, 17 Aug 2026 09:12:46 +0000 Subject: [PATCH] fix(escrow): derive split-release payments from on-chain basis points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit splitRelease computed each Payment.amount by independently rounding (totalAmount * percentage / 100) in JS floats while the contract received Math.round(percentage * 100) basis points — two never-reconciled computations of the same split that could drift from escrow.amount and from what was actually instructed on-chain (#43). - Apportion percentages into basis points that sum to exactly 10,000. - Derive each Payment.amount in whole stroops from those same basis points, allocating the rounding remainder via largest-remainder so sum(payments.amount) === escrow.amount exactly. - Record the split_release result (incl. returnValue) in escrow metadata and warn when the contract's returned total diverges from the recorded total, deferring per-recipient reconciliation to the reconciliation job. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- src/common/validators/money.validator.ts | 10 +++ src/escrow/escrow.service.spec.ts | 43 +++++++++++ src/escrow/escrow.service.ts | 68 +++++++++++++++--- src/escrow/split-math.util.spec.ts | 56 +++++++++++++++ src/escrow/split-math.util.ts | 92 ++++++++++++++++++++++++ 5 files changed, 259 insertions(+), 10 deletions(-) create mode 100644 src/escrow/split-math.util.spec.ts create mode 100644 src/escrow/split-math.util.ts diff --git a/src/common/validators/money.validator.ts b/src/common/validators/money.validator.ts index b9ee98d..9ca755f 100644 --- a/src/common/validators/money.validator.ts +++ b/src/common/validators/money.validator.ts @@ -33,6 +33,16 @@ export function amountToStroops(amount: string): bigint { return BigInt(whole) * STROOP_SCALE + BigInt(fraction.padEnd(7, '0')); } +/** Inverse of {@link amountToStroops}: formats a stroop total back to a 7-decimal-place string. */ +export function stroopsToAmount(stroops: bigint): string { + if (stroops < 0n) { + throw new Error('Stroops amount must be non-negative'); + } + const whole = stroops / STROOP_SCALE; + const fraction = stroops % STROOP_SCALE; + return `${whole.toString()}.${fraction.toString().padStart(7, '0')}`; +} + export function isSupportedEscrowAsset(value: unknown): value is AssetType { return SUPPORTED_ESCROW_ASSETS.includes(value as AssetType); } diff --git a/src/escrow/escrow.service.spec.ts b/src/escrow/escrow.service.spec.ts index 9078f10..dd9dd9e 100644 --- a/src/escrow/escrow.service.spec.ts +++ b/src/escrow/escrow.service.spec.ts @@ -258,5 +258,48 @@ describe('EscrowService', () => { expect(payments[1].amount).toBe('40.0000000'); expect(payments[2].amount).toBe('20.0000000'); }); + + it('records split amounts that sum to exactly escrow.amount in stroops (#43)', async () => { + escrowRepo.findOne.mockResolvedValue({ + id: 'escrow-uneven', + status: EscrowStatus.LOCKED, + amount: '100.0000000', + asset: AssetType.USDC, + bountyId: 'bounty-uneven', + }); + + const payments = await service.splitRelease('escrow-uneven', [ + { recipientAddress: 'GA', percentage: 33.33 }, + { recipientAddress: 'GB', percentage: 33.33 }, + { recipientAddress: 'GC', percentage: 33.34 }, + ]); + + const totalStroops = payments.reduce( + (sum, p) => sum + BigInt(Math.round(Number(p.amount) * 1e7)), + 0n, + ); + expect(totalStroops).toBe(1_000_000_000n); + }); + + it('sends basis points on-chain that sum to exactly 10,000', async () => { + escrowRepo.findOne.mockResolvedValue({ + id: 'escrow-bps', + status: EscrowStatus.LOCKED, + amount: '100.0000000', + asset: AssetType.USDC, + bountyId: 'bounty-bps', + }); + + await service.splitRelease('escrow-bps', [ + { recipientAddress: 'GA', percentage: 33.333 }, + { recipientAddress: 'GB', percentage: 33.333 }, + { recipientAddress: 'GC', percentage: 33.334 }, + ]); + + const invokeCall = soroban.invoke.mock.calls[0] as unknown[]; + const splitArgs = invokeCall[1] as unknown[]; + const bps = splitArgs[2] as number[]; + expect(bps.reduce((a, b) => a + b, 0)).toBe(10_000); + }); }); }); diff --git a/src/escrow/escrow.service.ts b/src/escrow/escrow.service.ts index eb35471..9986857 100644 --- a/src/escrow/escrow.service.ts +++ b/src/escrow/escrow.service.ts @@ -12,8 +12,10 @@ import { amountToStroops, isSupportedEscrowAsset, isValidMoneyAmount, + stroopsToAmount, } from '../common/validators/money.validator'; import { SorobanClientService } from './soroban-client.service'; +import { apportionBasisPoints, splitStroops } from './split-math.util'; export interface FundEscrowInput { amount: string; @@ -129,6 +131,13 @@ export class EscrowService { /** * Splits the escrowed amount across multiple recipients by percentage * (team bounties). Percentages must sum to exactly 100. + * + * The recorded `Payment.amount` values are derived from the same + * basis-point integers sent on-chain — not recomputed independently from the + * raw percentages — so the local ledger can never drift from what was + * instructed to the contract. Shares are allocated in whole stroops via a + * largest-remainder method, guaranteeing `sum(payments.amount) === + * escrow.amount` exactly (#43). */ async splitRelease( escrowId: string, @@ -138,30 +147,36 @@ export class EscrowService { this.assertLocked(escrow); this.assertValidSplits(recipients); + const totalStroops = amountToStroops(escrow.amount); + // Single source of truth for the split: integer basis points summing to + // exactly 10,000 (100.00%), used both on-chain and to derive the ledger. + const bps = apportionBasisPoints(recipients.map((r) => r.percentage)); + const result = await this.soroban.invoke('split_release', [ escrow.bountyId ?? escrow.milestoneId ?? escrow.id, recipients.map((r) => r.recipientAddress), - recipients.map((r) => Math.round(r.percentage * 100)), // basis points-ish, 2dp -> integer + bps, ]); + const shares = splitStroops(totalStroops, bps); + this.reconcileSplitResult(escrow.id, totalStroops, result.returnValue); + escrow.status = EscrowStatus.RELEASED; escrow.releaseTxHash = result.txHash; escrow.releasedAt = new Date(); + escrow.metadata = { ...(escrow.metadata ?? {}), splitRelease: result }; await this.escrowRepo.save(escrow); - const totalAmount = Number(escrow.amount); const payments: Payment[] = []; - for (const recipient of recipients) { - const share = this.roundAmount( - (totalAmount * recipient.percentage) / 100, - ); + for (let i = 0; i < recipients.length; i++) { + const recipient = recipients[i]; const payment = this.paymentRepo.create({ escrowId: escrow.id, recipientId: recipient.recipientId ?? null, recipientAddress: recipient.recipientAddress, - amount: share.toFixed(7), + amount: stroopsToAmount(shares[i]), asset: escrow.asset, - splitPercentage: recipient.percentage.toFixed(2), + splitPercentage: (bps[i] / 100).toFixed(2), status: PaymentStatus.CONFIRMED, txHash: result.txHash, }); @@ -283,8 +298,41 @@ export class EscrowService { } } - private roundAmount(value: number): number { - return Math.round(value * 1e7) / 1e7; + /** + * The illustrative split_release contract returns a single i128 (the total + * released, in stroops) rather than a per-recipient breakdown, so the + * recorded Payment rows cannot yet be derived from `result.returnValue` + * (see the interface TODO in soroban-client.service.ts). Until the deployed + * contract returns per-recipient amounts, reconcile the scalar total against + * the locally computed total and surface any divergence as a warning for the + * reconciliation job, rather than silently discarding it (#43). + */ + private reconcileSplitResult( + escrowId: string, + totalStroops: bigint, + returnValue: unknown, + ): void { + const returned = this.toStroopsFromReturnValue(returnValue); + if (returned === null) return; + if (returned !== totalStroops) { + this.logger.warn( + `split_release returnValue (${returned} stroops) diverges from the ` + + `recorded total (${totalStroops} stroops) for escrow ${escrowId}`, + ); + } + } + + /** Best-effort conversion of a contract return value to a stroop total. */ + private toStroopsFromReturnValue(value: unknown): bigint | null { + if (value == null) return null; + if (typeof value === 'bigint') return value; + if (typeof value === 'number' && Number.isFinite(value)) { + return BigInt(Math.trunc(value)); + } + if (typeof value === 'string' && /^-?\d+$/.test(value.trim())) { + return BigInt(value.trim()); + } + return null; } private assertValidFundInput(input: FundEscrowInput): void { diff --git a/src/escrow/split-math.util.spec.ts b/src/escrow/split-math.util.spec.ts new file mode 100644 index 0000000..aa1da7f --- /dev/null +++ b/src/escrow/split-math.util.spec.ts @@ -0,0 +1,56 @@ +import { BadRequestException } from '@nestjs/common'; +import { + apportionBasisPoints, + splitStroops, + TOTAL_BASIS_POINTS, +} from './split-math.util'; + +describe('apportionBasisPoints', () => { + it('sums to exactly 10,000 basis points', () => { + const bps = apportionBasisPoints([33.33, 33.33, 33.34]); + expect(bps.reduce((a, b) => a + b, 0)).toBe(TOTAL_BASIS_POINTS); + }); + + it('normalizes naive rounding for repeating-decimal percentages', () => { + // Math.round(33.333*100) = 3333 for all three, summing to 9999 — the + // apportionment must hand the missing basis point to the largest remainder. + const bps = apportionBasisPoints([33.333, 33.333, 33.334]); + expect(bps).toEqual([3333, 3333, 3334]); + expect(bps.reduce((a, b) => a + b, 0)).toBe(TOTAL_BASIS_POINTS); + }); + + it('keeps every basis point positive for tiny shares', () => { + const bps = apportionBasisPoints([0.01, 99.99]); + expect(bps[0]).toBeGreaterThan(0); + expect(bps.reduce((a, b) => a + b, 0)).toBe(TOTAL_BASIS_POINTS); + }); + + it('rejects an empty percentage list', () => { + expect(() => apportionBasisPoints([])).toThrow(BadRequestException); + }); +}); + +describe('splitStroops', () => { + it('reproduces the issue repro exactly (100.0000000 -> 1,000,000,000 stroops)', () => { + const shares = splitStroops(1_000_000_000n, [3333, 3333, 3334]); + expect(shares).toEqual([333_300_000n, 333_300_000n, 333_400_000n]); + expect(shares.reduce((a, b) => a + b, 0n)).toBe(1_000_000_000n); + }); + + it('allocates the rounding remainder so the sum is exact', () => { + const shares = splitStroops(1_000_000_001n, [5000, 5000]); + expect(shares.reduce((a, b) => a + b, 0n)).toBe(1_000_000_001n); + }); + + it('splits a non-divisible amount across uneven thirds exactly', () => { + const total = 10_000_000_007n; // 1000.0000007 + const shares = splitStroops(total, [3333, 3333, 3334]); + expect(shares.reduce((a, b) => a + b, 0n)).toBe(total); + }); + + it('rejects basis points that do not sum to 10,000', () => { + expect(() => splitStroops(1_000_000_000n, [3333, 3333, 3333])).toThrow( + BadRequestException, + ); + }); +}); diff --git a/src/escrow/split-math.util.ts b/src/escrow/split-math.util.ts new file mode 100644 index 0000000..c107f94 --- /dev/null +++ b/src/escrow/split-math.util.ts @@ -0,0 +1,92 @@ +import { BadRequestException } from '@nestjs/common'; + +/** One hundred percent expressed in basis points (hundredths of a percent). */ +export const TOTAL_BASIS_POINTS = 10_000; + +/** + * Converts floating-point percentages into integer basis points that sum to + * exactly {@link TOTAL_BASIS_POINTS} (100.00%). + * + * Naively rounding each percentage independently (`Math.round(p * 100)`) can + * leave the total a few basis points off 10,000 (e.g. three-way splits at + * repeating-decimal percentages), which would silently under- or over-fund + * the contract. This apportions the leftover/overage to the recipients whose + * independent rounding diverged the most, so the integer vector handed to the + * contract always represents exactly 100%. + */ +export function apportionBasisPoints(percentages: number[]): number[] { + if (percentages.length === 0) { + throw new BadRequestException('At least one percentage is required'); + } + + const bps = percentages.map((p) => Math.round(p * 100)); + const delta = TOTAL_BASIS_POINTS - bps.reduce((sum, b) => sum + b, 0); + + // Distance between each rounded basis point and the exact quota. A positive + // error means the recipient was rounded down (owed the leftover); negative + // means rounded up (over-represented). + const errors = percentages.map((p, i) => ({ + index: i, + error: p * 100 - bps[i], + })); + + if (delta > 0) { + errors.sort((a, b) => b.error - a.error || a.index - b.index); + for (let i = 0; i < delta; i++) { + bps[errors[i % errors.length].index] += 1; + } + } else if (delta < 0) { + errors.sort((a, b) => a.error - b.error || a.index - b.index); + for (let i = 0; i < -delta; i++) { + bps[errors[i % errors.length].index] -= 1; + } + } + + return bps; +} + +/** + * Splits `totalStroops` into integer stroop shares proportional to `bps` + * (which must sum to exactly {@link TOTAL_BASIS_POINTS}). + * + * The returned shares always sum to `totalStroops` exactly. Each share is + * `floor(totalStroops * bps / 10000)`, with the leftover stroops handed out + * by the largest-remainder method so no remainder is ever lost or invented. + */ +export function splitStroops(totalStroops: bigint, bps: number[]): bigint[] { + if (bps.length === 0) { + throw new BadRequestException('At least one recipient is required'); + } + const bpsTotal = bps.reduce((sum, b) => sum + b, 0); + if (bpsTotal !== TOTAL_BASIS_POINTS) { + throw new BadRequestException( + `Basis points must sum to ${TOTAL_BASIS_POINTS}, got ${bpsTotal}`, + ); + } + + const scale = BigInt(TOTAL_BASIS_POINTS); + const shares = bps.map((b) => (totalStroops * BigInt(b)) / scale); + const remainder = totalStroops - shares.reduce((sum, s) => sum + s, 0n); + + const remainders = bps.map((b, i) => ({ + index: i, + remainder: (totalStroops * BigInt(b)) % scale, + })); + + // Largest-remainder allocation: give the leftover stroops to the recipients + // with the largest fractional remainder (ties broken by larger share, then + // by original order). + remainders.sort((a, b) => { + if (a.remainder > b.remainder) return -1; + if (a.remainder < b.remainder) return 1; + if (shares[a.index] > shares[b.index]) return -1; + if (shares[a.index] < shares[b.index]) return 1; + return a.index - b.index; + }); + + for (let i = 0; i < Number(remainder); i++) { + shares[remainders[i % remainders.length].index] += 1n; + } + + return shares; +}