Overview
EscrowService.splitRelease computes two different numbers from the same input percentages — one sent on-chain, one persisted locally as the ledger of what was actually paid — using two independent rounding schemes that are never reconciled against each other:
// src/escrow/escrow.service.ts:133-171
async splitRelease(escrowId: string, recipients: SplitRecipient[]): Promise<Payment[]> {
const escrow = await this.getOrThrow(escrowId);
this.assertLocked(escrow);
this.assertValidSplits(recipients);
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)), // <- (1) basis points sent on-chain
]);
escrow.status = EscrowStatus.RELEASED;
// ...
const totalAmount = Number(escrow.amount);
const payments: Payment[] = [];
for (const recipient of recipients) {
const share = this.roundAmount((totalAmount * recipient.percentage) / 100); // <- (2) independently rounded amount recorded locally
const payment = this.paymentRepo.create({
// ...
amount: share.toFixed(7),
splitPercentage: recipient.percentage.toFixed(2),
// ...
});
payments.push(await this.paymentRepo.save(payment));
}
return payments;
}(1) — Math.round(r.percentage * 100) — is what actually gets sent to the Soroban contract as the on-chain split instruction (basis points, per the "illustrative" contract interface documented in soroban-client.service.ts's TODO comment: split_release(env, bounty_id, recipients: Vec<Address>, bps: Vec<u32>)). (2) — roundAmount((totalAmount * recipient.percentage) / 100), itself Math.round(value * 1e7) / 1e7 — is a separate computation, in JS floating point, of what each recipient's Payment.amount row says they received. These two numbers are derived from the same recipient.percentage but through different arithmetic (integer basis-point rounding vs. 7-decimal-place float rounding) and are never checked against each other, nor against escrow.amount.
Two concrete ways this diverges:
- Sum drift. For percentages that don't divide
escrow.amount evenly (e.g. three recipients at 33.33/33.33/33.34), roundAmount on each share independently can leave sum(payments.amount) !== escrow.amount by a few stroops either way — nobody collects or distributes the rounding remainder, so the local ledger can show slightly more or less than was actually locked, in either direction, depending on how the individual roundings happen to fall.
- Ledger-vs-chain drift. The actual amount each recipient receives on-chain is whatever the deployed contract computes from the basis-point integers in
(1) (contract-side integer arithmetic on i128 stroops) — which is a different code path, in a different language, operating on a differently-rounded input (Math.round(percentage*100) truncates/rounds percentage precision to 2 decimal places at the basis-point stage, before the contract ever sees it) than the JS float computation in (2) that produces what Payment.amount claims was paid. Nothing in this function (or anywhere else) verifies sum(on-chain actual payout) against sum(payments.amount) — the local Payment rows are simply asserted to be correct, not derived from or checked against result (the actual ContractInvocationResult returned by soroban.invoke, which includes a returnValue that's currently discarded entirely for this call).
This is distinct from the repo's existing open "financial amounts computed with IEEE-754 Number" issue: that issue is about Number losing precision at extreme values. This issue exists even with perfect floating-point precision at realistic bounty sizes — it's two independently-computed, never-reconciled values, not a precision-loss bug per se.
Requirements
- Compute the recorded
Payment.amount values from the same basis-point integers actually sent on-chain (i.e. derive (2) from (1), not from recipient.percentage a second time) so the local ledger is guaranteed consistent with what was instructed on-chain, rather than merely "close."
- Allocate any leftover remainder (the difference between
escrow.amount and the naive sum of individually-rounded shares) to a single recipient (e.g. the largest share, or the first-listed) using a standard largest-remainder-style allocation, so sum(payments.amount) === escrow.amount exactly, every time — not just approximately.
- Where
soroban.invoke's result.returnValue carries any information about actual per-recipient amounts distributed (investigate what the real contract, once deployed, actually returns — the interface is currently "illustrative" per the TODO in soroban-client.service.ts), prefer recording that over any locally recomputed value, and treat a mismatch between the two as a reconciliation-worthy anomaly rather than something to silently paper over.
- Add a test asserting
sum(payments.map(p => p.amount)) === escrow.amount exactly (via BigInt/stroops comparison, not float ===) for a split that doesn't divide evenly, e.g. 3 recipients at 33.33/33.33/33.34 on a 100.0000000 USDC escrow.
Acceptance Criteria
Additional Notes
Precise references: src/escrow/escrow.service.ts:141-145 (basis-point computation sent on-chain), :152-169 (independent JS-float share computation persisted locally), :286-288 (roundAmount's own definition, Math.round(value * 1e7) / 1e7 — itself still Number-based, compounding the divergence risk from the separately-tracked IEEE-754 issue but not the same bug as this one).
Test/reproduction plan:
const recipients = [
{ recipientAddress: 'A', percentage: 33.33 },
{ recipientAddress: 'B', percentage: 33.33 },
{ recipientAddress: 'C', percentage: 33.34 },
];
// escrow.amount = '100.0000000'
const payments = await service.splitRelease(escrowId, recipients);
const total = payments.reduce((sum, p) => sum + BigInt(Math.round(Number(p.amount) * 1e7)), 0n);
expect(total).toBe(1_000_000_000n); // exactly 100.0000000 in stroops, not "close to"Also worth asserting the basis points actually sent (soroban.invoke call args) sum to exactly 10000 (100.00%) — Math.round(33.33*100) + Math.round(33.33*100) + Math.round(33.34*100) = 3333+3333+3334 = 10000, which happens to work for this example but should be asserted generally, including for splits where naive per-recipient rounding of basis points does not sum to exactly 10000 (e.g. three-way splits at repeating-decimal percentages).
Cross-references: related to but distinct from the open "financial amounts computed with IEEE-754 Number" issue (precision at scale, not reconciliation between two independently-rounded values) and the open "percentage-split validation... gamed" issue (input validation of the percentages themselves, not what happens to valid percentages once they're used in two different downstream computations).
Overview
EscrowService.splitReleasecomputes two different numbers from the same input percentages — one sent on-chain, one persisted locally as the ledger of what was actually paid — using two independent rounding schemes that are never reconciled against each other:(1)—Math.round(r.percentage * 100)— is what actually gets sent to the Soroban contract as the on-chain split instruction (basis points, per the "illustrative" contract interface documented insoroban-client.service.ts's TODO comment:split_release(env, bounty_id, recipients: Vec<Address>, bps: Vec<u32>)).(2)—roundAmount((totalAmount * recipient.percentage) / 100), itselfMath.round(value * 1e7) / 1e7— is a separate computation, in JS floating point, of what each recipient'sPayment.amountrow says they received. These two numbers are derived from the samerecipient.percentagebut through different arithmetic (integer basis-point rounding vs. 7-decimal-place float rounding) and are never checked against each other, nor againstescrow.amount.Two concrete ways this diverges:
escrow.amountevenly (e.g. three recipients at 33.33/33.33/33.34),roundAmounton each share independently can leavesum(payments.amount) !== escrow.amountby a few stroops either way — nobody collects or distributes the rounding remainder, so the local ledger can show slightly more or less than was actually locked, in either direction, depending on how the individual roundings happen to fall.(1)(contract-side integer arithmetic oni128stroops) — which is a different code path, in a different language, operating on a differently-rounded input (Math.round(percentage*100)truncates/rounds percentage precision to 2 decimal places at the basis-point stage, before the contract ever sees it) than the JS float computation in(2)that produces whatPayment.amountclaims was paid. Nothing in this function (or anywhere else) verifiessum(on-chain actual payout)againstsum(payments.amount)— the localPaymentrows are simply asserted to be correct, not derived from or checked againstresult(the actualContractInvocationResultreturned bysoroban.invoke, which includes areturnValuethat's currently discarded entirely for this call).This is distinct from the repo's existing open "financial amounts computed with IEEE-754 Number" issue: that issue is about
Numberlosing precision at extreme values. This issue exists even with perfect floating-point precision at realistic bounty sizes — it's two independently-computed, never-reconciled values, not a precision-loss bug per se.Requirements
Payment.amountvalues from the same basis-point integers actually sent on-chain (i.e. derive(2)from(1), not fromrecipient.percentagea second time) so the local ledger is guaranteed consistent with what was instructed on-chain, rather than merely "close."escrow.amountand the naive sum of individually-rounded shares) to a single recipient (e.g. the largest share, or the first-listed) using a standard largest-remainder-style allocation, sosum(payments.amount) === escrow.amountexactly, every time — not just approximately.soroban.invoke'sresult.returnValuecarries any information about actual per-recipient amounts distributed (investigate what the real contract, once deployed, actually returns — the interface is currently "illustrative" per the TODO insoroban-client.service.ts), prefer recording that over any locally recomputed value, and treat a mismatch between the two as a reconciliation-worthy anomaly rather than something to silently paper over.sum(payments.map(p => p.amount)) === escrow.amountexactly (viaBigInt/stroops comparison, not float===) for a split that doesn't divide evenly, e.g. 3 recipients at 33.33/33.33/33.34 on a 100.0000000 USDC escrow.Acceptance Criteria
Payment.amountvalues recorded bysplitReleaseare derived from the same basis-point integers sent on-chain, not independently recomputed from raw percentages.sum(payments.amount)exactly equalsescrow.amountfor every valid split, including ones that don't divide evenly, proven by a test using exact (stroops/BigInt) comparison.result.returnValue(if/when it carries real distributed amounts from a deployed contract) and the recordedPaymentrows is investigated and documented, even if full reconciliation is deferred to the reconciliation-job issue.Additional Notes
Precise references:
src/escrow/escrow.service.ts:141-145(basis-point computation sent on-chain),:152-169(independent JS-float share computation persisted locally),:286-288(roundAmount's own definition,Math.round(value * 1e7) / 1e7— itself stillNumber-based, compounding the divergence risk from the separately-tracked IEEE-754 issue but not the same bug as this one).Test/reproduction plan:
Also worth asserting the basis points actually sent (
soroban.invokecall args) sum to exactly10000(100.00%) —Math.round(33.33*100) + Math.round(33.33*100) + Math.round(33.34*100) = 3333+3333+3334 = 10000, which happens to work for this example but should be asserted generally, including for splits where naive per-recipient rounding of basis points does not sum to exactly 10000 (e.g. three-way splits at repeating-decimal percentages).Cross-references: related to but distinct from the open "financial amounts computed with IEEE-754 Number" issue (precision at scale, not reconciliation between two independently-rounded values) and the open "percentage-split validation... gamed" issue (input validation of the percentages themselves, not what happens to valid percentages once they're used in two different downstream computations).