Merge pull request #79 from eulami/fix/43-split-release-basis-point-reconciliation

fix(escrow): derive split-release payments from on-chain basis points

Author: chonilius

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);
 }

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);
+    });
   });
 });

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 {

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,
+    );
+  });
+});

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;
+}