diff --git a/contracts/escrow/src/lib.rs b/contracts/escrow/src/lib.rs index 7b2da8f..56af404 100644 --- a/contracts/escrow/src/lib.rs +++ b/contracts/escrow/src/lib.rs @@ -409,7 +409,7 @@ pub(crate) fn compute_split( let distributable = total - fee; let mut shares: Vec<(Address, i128)> = Vec::new(env); - let mut remainders: Vec = Vec::new(env); + let mut order: Vec<(u32, i128, Address)> = Vec::new(env); let mut allocated: i128 = 0; for (recipient, bps) in recipients.iter() { @@ -417,37 +417,99 @@ pub(crate) fn compute_split( let share = numerator / BPS_DENOMINATOR; let remainder = numerator % BPS_DENOMINATOR; allocated += share; - shares.push_back((recipient, share)); - remainders.push_back(remainder); + shares.push_back((recipient.clone(), share)); + order.push_back((order.len(), remainder, recipient)); } - let mut dust = distributable - allocated; - while dust > 0 { - let mut best_index: u32 = 0; - let mut best_remainder: i128 = -1; - for (i, remainder) in remainders.iter().enumerate() { - if remainder > best_remainder { - best_index = i as u32; - best_remainder = remainder; - } else if remainder == best_remainder && remainder != -1 { - let current_addr = shares.get(i as u32).unwrap().0; - let best_addr = shares.get(best_index).unwrap().0; - if current_addr < best_addr { - best_index = i as u32; - best_remainder = remainder; - } - } + // Distribute the rounding dust by largest remainder (with the existing + // address-based tie-break) in O(n log n): sort the (index, remainder, + // address) records once, then award one unit to each of the first `dust` + // entries. This is equivalent to the previous repeated-linear-scan loop, + // because each award only consumes the selected entry and never changes + // any other entry's remainder. `dust` is at most `recipients.len() - 1`, + // so the first `dust` sorted entries always exist. + let dust = distributable - allocated; + if dust > 0 { + sort_remainders_desc(&mut order); + for k in 0..dust as u32 { + let (index, _, _) = order.get(k).unwrap(); + let (recipient, share) = shares.get(index).unwrap(); + shares.set(index, (recipient, share + 1)); } - - let (recipient, share) = shares.get(best_index).unwrap(); - shares.set(best_index, (recipient, share + 1)); - remainders.set(best_index, -1); - dust -= 1; } Ok(Payouts { fee, shares }) } +/// True if `a` sorts before `b` in largest-remainder order: remainder +/// descending, then address ascending, then original index ascending (which +/// reproduces the address-based tie-break of the previous O(n²) loop). +fn remainder_order_less(a: &(u32, i128, Address), b: &(u32, i128, Address)) -> bool { + b.1.cmp(&a.1) + .then_with(|| a.2.cmp(&b.2)) + .then_with(|| a.0.cmp(&b.0)) + == core::cmp::Ordering::Less +} + +/// Sifts the element at `start` down a max-heap occupying `[start, end)`, +/// ordering elements by [`remainder_order_less`]. +fn sift_down_remainder_order(order: &mut Vec<(u32, i128, Address)>, start: u32, end: u32) { + let mut root = start; + loop { + let mut child = 2 * root + 1; + if child >= end { + break; + } + if child + 1 < end + && remainder_order_less(&order.get(child).unwrap(), &order.get(child + 1).unwrap()) + { + child += 1; + } + if remainder_order_less(&order.get(root).unwrap(), &order.get(child).unwrap()) { + let a = order.get(root).unwrap(); + let b = order.get(child).unwrap(); + order.set(root, b); + order.set(child, a); + root = child; + } else { + break; + } + } +} + +/// In-place heapsort of `(index, remainder, address)` records into +/// largest-remainder order. O(n log n) worst case, with no recursion and no +/// heap allocation, so it is safe under `#![no_std]` and only mutates the +/// host-backed `order` through `get`/`set`. +fn sort_remainders_desc(order: &mut Vec<(u32, i128, Address)>) { + let n = order.len(); + if n < 2 { + return; + } + + // Build a max-heap over the whole array. + let mut start = n / 2; + loop { + start -= 1; + sift_down_remainder_order(order, start, n); + if start == 0 { + break; + } + } + + // Repeatedly move the largest remaining element to the end of the array, + // shrinking the heap until the array is sorted ascending by `less`. + let mut end = n; + while end > 1 { + end -= 1; + let a = order.get(0).unwrap(); + let b = order.get(end).unwrap(); + order.set(0, b); + order.set(end, a); + sift_down_remainder_order(order, 0, end); + } +} + pub(crate) fn require_admin(env: &Env) -> Result { mergefi_common::require_admin::(env).ok_or(Error::NotInitialized) } diff --git a/contracts/escrow/src/test.rs b/contracts/escrow/src/test.rs index 982244e..ea57571 100644 --- a/contracts/escrow/src/test.rs +++ b/contracts/escrow/src/test.rs @@ -302,6 +302,91 @@ fn test_adversarial_ordering_resistance() { ); } +#[test] +fn test_large_split_distributes_dust_by_largest_remainder() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let treasury = Address::generate(&env); + let contract_id = env.register(crate::EscrowContract, ()); + let client = crate::EscrowContractClient::new(&env, &contract_id); + // 0% fee so the whole total is distributable. + client.initialize(&admin, &treasury, &0u32); + + // 60 recipients: 59 with alternating 160/170 bps, the last one receiving + // the leftover of 10000. All 170-bps recipients share an identical + // remainder, so most of the dust has to be resolved by the address-based + // tie-break, exercising both the O(n log n) sort and the tie-break at + // scale. + let mut recipients = Vec::new(&env); + let mut total_bps: u32 = 0; + for i in 0..59u32 { + let bps = if i % 5 == 0 { 160 } else { 170 }; + recipients.push_back((Address::generate(&env), bps)); + total_bps += bps; + } + let last_bps = BPS_DENOMINATOR as u32 - total_bps; + recipients.push_back((Address::generate(&env), last_bps)); + + // Chosen so that integer division leaves exactly 40 dust units to + // distribute. + let total: i128 = 123_457; + let payouts = env.as_contract(&contract_id, || { + crate::compute_split(&env, total, &recipients).unwrap() + }); + + // Reference result computed with the previous O(n²) repeated + // largest-remainder scan; the new implementation must match it exactly. + let mut expected: Vec = Vec::new(&env); + let mut remainders: Vec = Vec::new(&env); + let mut allocated: i128 = 0; + for (_, bps) in recipients.iter() { + let numerator = total * (bps as i128); + let share = numerator / BPS_DENOMINATOR; + let remainder = numerator % BPS_DENOMINATOR; + allocated += share; + expected.push_back(share); + remainders.push_back(remainder); + } + let mut dust = total - allocated; + assert!( + dust >= 2, + "test must exercise multiple dust units, got {dust}" + ); + while dust > 0 { + let mut best_index: u32 = 0; + let mut best_remainder: i128 = -1; + for (i, remainder) in remainders.iter().enumerate() { + if remainder > best_remainder { + best_index = i as u32; + best_remainder = remainder; + } else if remainder == best_remainder && remainder != -1 { + let current_addr = recipients.get(i as u32).unwrap().0; + let best_addr = recipients.get(best_index).unwrap().0; + if current_addr < best_addr { + best_index = i as u32; + best_remainder = remainder; + } + } + } + expected.set(best_index, expected.get(best_index).unwrap() + 1); + remainders.set(best_index, -1); + dust -= 1; + } + + let mut total_paid: i128 = 0; + for (i, _) in recipients.iter().enumerate() { + let (_, share) = payouts.shares.get(i as u32).unwrap(); + assert_eq!(share, expected.get(i as u32).unwrap(), "recipient {i}"); + total_paid += share; + } + assert_eq!( + total_paid, total, + "all distributable funds must be paid out" + ); +} + // --------------------------------------------------------------------------- // Access-control boundary matrix (#30) // --------------------------------------------------------------------------- diff --git a/contracts/milestones/src/lib.rs b/contracts/milestones/src/lib.rs index 8a640f8..55bb2d9 100644 --- a/contracts/milestones/src/lib.rs +++ b/contracts/milestones/src/lib.rs @@ -369,7 +369,7 @@ fn compute_split( let distributable = total - fee; let mut shares: Vec<(Address, i128)> = Vec::new(env); - let mut remainders: Vec = Vec::new(env); + let mut order: Vec<(u32, i128, Address)> = Vec::new(env); let mut allocated: i128 = 0; for (recipient, bps) in recipients.iter() { @@ -377,37 +377,97 @@ fn compute_split( let share = numerator / BPS_DENOMINATOR; let remainder = numerator % BPS_DENOMINATOR; allocated += share; - shares.push_back((recipient, share)); - remainders.push_back(remainder); + shares.push_back((recipient.clone(), share)); + order.push_back((order.len(), remainder, recipient)); } - let mut dust = distributable - allocated; - while dust > 0 { - let mut best_index: u32 = 0; - let mut best_remainder: i128 = -1; - for (i, remainder) in remainders.iter().enumerate() { - if remainder > best_remainder { - best_index = i as u32; - best_remainder = remainder; - } else if remainder == best_remainder && remainder != -1 { - let current_addr = shares.get(i as u32).unwrap().0; - let best_addr = shares.get(best_index).unwrap().0; - if current_addr < best_addr { - best_index = i as u32; - best_remainder = remainder; - } - } + // Distribute the rounding dust by largest remainder (with the existing + // address-based tie-break) in O(n log n): sort the (index, remainder, + // address) records once, then award one unit to each of the first `dust` + // entries. This is equivalent to the previous repeated-linear-scan loop, + // because each award only consumes the selected entry and never changes + // any other entry's remainder. `dust` is at most `recipients.len() - 1`, + // so the first `dust` sorted entries always exist. + let dust = distributable - allocated; + if dust > 0 { + sort_remainders_desc(&mut order); + for k in 0..dust as u32 { + let (index, _, _) = order.get(k).unwrap(); + let (recipient, share) = shares.get(index).unwrap(); + shares.set(index, (recipient, share + 1)); } - - let (recipient, share) = shares.get(best_index).unwrap(); - shares.set(best_index, (recipient, share + 1)); - remainders.set(best_index, -1); - dust -= 1; } Ok(Payouts { fee, shares }) } +/// True if `a` sorts before `b` in largest-remainder order: remainder +/// descending, then address ascending, then original index ascending (which +/// reproduces the address-based tie-break of the previous O(n²) loop). +fn remainder_order_less(a: &(u32, i128, Address), b: &(u32, i128, Address)) -> bool { + b.1.cmp(&a.1) + .then_with(|| a.2.cmp(&b.2)) + .then_with(|| a.0.cmp(&b.0)) + == core::cmp::Ordering::Less +} + +/// Sifts the element at `start` down a max-heap occupying `[start, end)`, +/// ordering elements by [`remainder_order_less`]. +fn sift_down_remainder_order(order: &mut Vec<(u32, i128, Address)>, start: u32, end: u32) { + let mut root = start; + loop { + let mut child = 2 * root + 1; + if child >= end { + break; + } + if child + 1 < end + && remainder_order_less(&order.get(child).unwrap(), &order.get(child + 1).unwrap()) + { + child += 1; + } + if remainder_order_less(&order.get(root).unwrap(), &order.get(child).unwrap()) { + let a = order.get(root).unwrap(); + let b = order.get(child).unwrap(); + order.set(root, b); + order.set(child, a); + root = child; + } else { + break; + } + } +} + +/// In-place heapsort of `(index, remainder, address)` records into +/// largest-remainder order. O(n log n) worst case, with no recursion and no +/// heap allocation, so it is safe under `#![no_std]` and only mutates the +/// host-backed `order` through `get`/`set`. +fn sort_remainders_desc(order: &mut Vec<(u32, i128, Address)>) { + let n = order.len(); + if n < 2 { + return; + } + + // Build a max-heap over the whole array. + let mut start = n / 2; + loop { + start -= 1; + sift_down_remainder_order(order, start, n); + if start == 0 { + break; + } + } + + // Repeatedly move the largest remaining element to the end of the array, + // shrinking the heap until the array is sorted ascending by `less`. + let mut end = n; + while end > 1 { + end -= 1; + let a = order.get(0).unwrap(); + let b = order.get(end).unwrap(); + order.set(0, b); + order.set(end, a); + sift_down_remainder_order(order, 0, end); + } /// Pays each contributor their share of `milestone.remaining_budget` (the /// unallocated remainder of the pool), computed as /// `remaining_budget * contribution.amount / total_budget` — i.e. in diff --git a/contracts/milestones/src/test.rs b/contracts/milestones/src/test.rs index 1a1c5fa..d7c4b7c 100644 --- a/contracts/milestones/src/test.rs +++ b/contracts/milestones/src/test.rs @@ -99,6 +99,91 @@ fn test_release_issue_distributes_rounding_dust_by_largest_remainder() { assert_eq!(token_client.balance(&carol), 32i128); } +#[test] +fn test_large_split_distributes_dust_by_largest_remainder() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let treasury = Address::generate(&env); + let contract_id = env.register(crate::MilestonesContract, ()); + let client = crate::MilestonesContractClient::new(&env, &contract_id); + // 0% fee so the whole total is distributable. + client.initialize(&admin, &treasury, &0u32); + + // 60 recipients: 59 with alternating 160/170 bps, the last one receiving + // the leftover of 10000. All 170-bps recipients share an identical + // remainder, so most of the dust has to be resolved by the address-based + // tie-break, exercising both the O(n log n) sort and the tie-break at + // scale. + let mut recipients = Vec::new(&env); + let mut total_bps: u32 = 0; + for i in 0..59u32 { + let bps = if i % 5 == 0 { 160 } else { 170 }; + recipients.push_back((Address::generate(&env), bps)); + total_bps += bps; + } + let last_bps = BPS_DENOMINATOR as u32 - total_bps; + recipients.push_back((Address::generate(&env), last_bps)); + + // Chosen so that integer division leaves exactly 40 dust units to + // distribute. + let total: i128 = 123_457; + let payouts = env.as_contract(&contract_id, || { + compute_split(&env, total, &recipients).unwrap() + }); + + // Reference result computed with the previous O(n²) repeated + // largest-remainder scan; the new implementation must match it exactly. + let mut expected: Vec = Vec::new(&env); + let mut remainders: Vec = Vec::new(&env); + let mut allocated: i128 = 0; + for (_, bps) in recipients.iter() { + let numerator = total * (bps as i128); + let share = numerator / BPS_DENOMINATOR; + let remainder = numerator % BPS_DENOMINATOR; + allocated += share; + expected.push_back(share); + remainders.push_back(remainder); + } + let mut dust = total - allocated; + assert!( + dust >= 2, + "test must exercise multiple dust units, got {dust}" + ); + while dust > 0 { + let mut best_index: u32 = 0; + let mut best_remainder: i128 = -1; + for (i, remainder) in remainders.iter().enumerate() { + if remainder > best_remainder { + best_index = i as u32; + best_remainder = remainder; + } else if remainder == best_remainder && remainder != -1 { + let current_addr = recipients.get(i as u32).unwrap().0; + let best_addr = recipients.get(best_index).unwrap().0; + if current_addr < best_addr { + best_index = i as u32; + best_remainder = remainder; + } + } + } + expected.set(best_index, expected.get(best_index).unwrap() + 1); + remainders.set(best_index, -1); + dust -= 1; + } + + let mut total_paid: i128 = 0; + for (i, _) in recipients.iter().enumerate() { + let (_, share) = payouts.shares.get(i as u32).unwrap(); + assert_eq!(share, expected.get(i as u32).unwrap(), "recipient {i}"); + total_paid += share; + } + assert_eq!( + total_paid, total, + "all distributable funds must be paid out" + ); +} + #[test] fn test_allocate_rejects_over_allocation() { let env = Env::default();