diff --git a/contracts/escrow/src/lib.rs b/contracts/escrow/src/lib.rs index f801412b..1e58c97b 100644 --- a/contracts/escrow/src/lib.rs +++ b/contracts/escrow/src/lib.rs @@ -336,7 +336,8 @@ mod contract { mergefi_common::SplitError::SelfPayout => Error::SelfPayout, mergefi_common::SplitError::Overflow => Error::ArithmeticOverflow, })?; - let treasury: Address = mergefi_common::require_treasury::(&env).unwrap(); + let treasury: Address = mergefi_common::require_treasury::(&env) + .ok_or(Error::NotInitialized)?; let token_client = token::Client::new(&env, &escrow.token); let contract_address = env.current_contract_address(); diff --git a/contracts/maintenance-pool/src/error.rs b/contracts/maintenance-pool/src/error.rs index bc81b969..c75ca9df 100644 --- a/contracts/maintenance-pool/src/error.rs +++ b/contracts/maintenance-pool/src/error.rs @@ -34,4 +34,8 @@ pub enum Error { /// `total_deposited` near `i128::MAX`). Returned instead of panicking /// under release `overflow-checks = true` + `panic = "abort"`. ArithmeticOverflow = 16, + /// Recipient basis-point weights do not sum to 10 000 (#484). + InvalidSplit = 17, + /// A recipient address equals the contract address — self-payout (#484). + SelfPayout = 18, } diff --git a/contracts/maintenance-pool/src/lib.rs b/contracts/maintenance-pool/src/lib.rs index fb2183ea..94115667 100644 --- a/contracts/maintenance-pool/src/lib.rs +++ b/contracts/maintenance-pool/src/lib.rs @@ -16,7 +16,7 @@ mod types; mod test; use error::Error; -use soroban_sdk::{contract, contractimpl, token, Address, BytesN, Env}; +use soroban_sdk::{contract, contractimpl, token, Address, BytesN, Env, Vec}; use types::{DataKey, Deposit, MaintenancePool}; use mergefi_common::BPS_DENOMINATOR; @@ -178,11 +178,17 @@ mod contract { Ok(()) } - /// Admin-only: pays `amount` (minus the protocol fee) out of the pool - /// to `recipient` (a maintainer), as authorized off-chain by the - /// backend oracle for completed maintenance work. Rejects if the pool - /// balance is insufficient. - pub fn withdraw(env: Env, pool_id: u64, recipient: Address, amount: i128) -> Result<(), Error> { + /// Oracle-authorized: pays `amount` out of the pool, split across + /// `recipients` (basis-points weights summing to 10 000) after + /// deducting the protocol fee via `mergefi_common::compute_split` + /// — the same path escrow and milestones use (#483, #484). + /// Rejects if the pool balance is insufficient. + pub fn withdraw( + env: Env, + pool_id: u64, + recipients: Vec<(Address, u32)>, + amount: i128, + ) -> Result<(), Error> { if is_paused(&env) { return Err(Error::ContractPaused); } @@ -206,13 +212,14 @@ mod contract { let fee_bps: u32 = mergefi_common::get_fee_bps::(&env) .ok_or(Error::NotInitialized)?; - let fee = amount - .checked_mul(fee_bps as i128) - .ok_or(Error::ArithmeticOverflow)? - / BPS_DENOMINATOR; - let payout = amount - .checked_sub(fee) - .ok_or(Error::ArithmeticOverflow)?; + let payouts = + mergefi_common::compute_split(&env, amount, fee_bps, &recipients).map_err( + |e| match e { + mergefi_common::SplitError::InvalidSplit => Error::InvalidSplit, + mergefi_common::SplitError::SelfPayout => Error::SelfPayout, + mergefi_common::SplitError::Overflow => Error::ArithmeticOverflow, + }, + )?; let treasury: Address = mergefi_common::require_treasury::(&env) .ok_or(Error::NotInitialized)?; @@ -231,10 +238,14 @@ mod contract { env.storage().persistent().set(&pkey, &pool); extend_ttl(&env, &pkey); - if fee > 0 { - token_client.transfer(&contract_address, &treasury, &fee); + if payouts.fee > 0 { + token_client.transfer(&contract_address, &treasury, &payouts.fee); + } + for (recipient, share) in payouts.shares.iter() { + if share > 0 { + token_client.transfer(&contract_address, &recipient, &share); + } } - token_client.transfer(&contract_address, &recipient, &payout); // Refresh all deposit sub-records on every withdrawal so historical // deposit records stay queryable across a long-running pool's lifetime. diff --git a/contracts/maintenance-pool/src/test.rs b/contracts/maintenance-pool/src/test.rs index 32b45212..1f1a6f5e 100644 --- a/contracts/maintenance-pool/src/test.rs +++ b/contracts/maintenance-pool/src/test.rs @@ -3,7 +3,7 @@ use super::*; use soroban_sdk::{ testutils::{Address as _, Ledger as _}, - token, Address, Env, + token, vec, Address, Env, }; fn create_token<'a>( @@ -108,7 +108,7 @@ fn test_withdraw_deducts_fee_and_balance() { client.deposit(&2u64, &sponsor, &token_addr, &10_000_000_000i128); let maintainer = Address::generate(&env); - client.withdraw(&2u64, &maintainer, &200_0000000i128); + client.withdraw(&2u64, &vec![&env, (maintainer.clone(), 10_000u32)], &200_0000000i128); // 10% fee -> 20_0000000 to treasury, 180_0000000 to maintainer. assert_eq!(token_client.balance(&maintainer), 180_0000000i128); @@ -137,7 +137,7 @@ fn test_withdraw_with_zero_fee_pays_full_amount() { client.deposit(&2u64, &sponsor, &token_addr, &10_000_000_000i128); let maintainer = Address::generate(&env); - client.withdraw(&2u64, &maintainer, &200_0000000i128); + client.withdraw(&2u64, &vec![&env, (maintainer.clone(), 10_000u32)], &200_0000000i128); assert_eq!(token_client.balance(&maintainer), 200_0000000i128); assert_eq!(token_client.balance(&treasury), 0i128); @@ -326,7 +326,7 @@ fn test_withdraw_exact_balance_boundary() { assert_eq!(err, Err(Ok(Error::InsufficientBalance))); // Exactly the pool's balance must succeed and drain it to zero. - client.withdraw(&7u64, &maintainer, &500_0000000i128); + client.withdraw(&7u64, &vec![&env, (maintainer.clone(), 10_000u32)], &500_0000000i128); let pool = client.get_pool(&7u64); assert_eq!(pool.balance, 0i128); assert_eq!(pool.total_withdrawn, 500_0000000i128); @@ -360,7 +360,7 @@ fn test_interleaved_deposit_withdraw_consistency() { assert_eq!(pool.balance, total_deposited - total_withdrawn); if withdraw_amt > 0 { - client.withdraw(&8u64, &maintainer, &withdraw_amt); + client.withdraw(&8u64, &vec![&env, (maintainer.clone(), 10_000u32)], &withdraw_amt); total_withdrawn += withdraw_amt; let pool = client.get_pool(&8u64); @@ -528,7 +528,7 @@ fn test_sweep_cannot_remove_owed_balances() { // Withdrawal should still work for the full pool balance let maintainer_balance_before = token_client.balance(&maintainer); - client.withdraw(&1u64, &maintainer, &100_0000000i128); // 10% fee = 10, payout = 90 + client.withdraw(&1u64, &vec![&env, (maintainer.clone(), 10_000u32)], &100_0000000i128); // 10% fee = 10, payout = 90 let maintainer_balance_after = token_client.balance(&maintainer); // Maintainer should receive (100 - 10% fee) = 90 @@ -646,7 +646,7 @@ fn test_recover_withdraw_frozen_before_recoverable_after() { client.recover_admin(&new_admin); // New admin withdraws successfully (mocked auth enables it) env.mock_all_auths(); - client.withdraw(&42u64, &maintainer, &1_000_000_000i128); + client.withdraw(&42u64, &vec![&env, (maintainer.clone(), 10_000u32)], &1_000_000_000i128); assert_eq!(token_client.balance(&maintainer), 900_000_000i128); // after 10% fee } @@ -908,7 +908,7 @@ fn test_upgrade_preserves_pool_with_multiple_deposits() { assert_eq!(client.get_deposit(&246u64, &3u32).sponsor, late_sponsor); let maintainer = Address::generate(&env); - client.withdraw(&246u64, &maintainer, &1_000i128); + client.withdraw(&246u64, &vec![&env, (maintainer.clone(), 10_000u32)], &1_000i128); let pool_after = client.get_pool(&246u64); assert_eq!(pool_after.balance, 5_500i128); assert_eq!(pool_after.total_withdrawn, 1_000i128); @@ -948,7 +948,7 @@ fn test_set_oracle_rotates_oracle_used_by_withdraw() { client.deposit(&7u64, &sponsor, &token_addr, &100_0000000i128); let maintainer = Address::generate(&env); - client.withdraw(&7u64, &maintainer, &50_0000000i128); + client.withdraw(&7u64, &vec![&env, (maintainer.clone(), 10_000u32)], &50_0000000i128); // withdraw now requires the rotated oracle's authorization, not the old one's. let auths = env.auths(); @@ -986,7 +986,7 @@ fn test_set_treasury_updates_fee_recipient() { client.deposit(&253u64, &sponsor, &token_addr, &10_000_000_000i128); let maintainer = Address::generate(&env); - client.withdraw(&253u64, &maintainer, &200_0000000i128); + client.withdraw(&253u64, &vec![&env, (maintainer.clone(), 10_000u32)], &200_0000000i128); // 10% fee lands on the new treasury, none on the old one. assert_eq!(token_client.balance(&new_treasury), 20_0000000i128); diff --git a/contracts/milestones/src/error.rs b/contracts/milestones/src/error.rs index 7472af37..4269745d 100644 --- a/contracts/milestones/src/error.rs +++ b/contracts/milestones/src/error.rs @@ -37,6 +37,10 @@ pub enum Error { /// `i128::MAX`). Returned instead of panicking under release /// `overflow-checks = true` + `panic = "abort"`. ArithmeticOverflow = 21, + /// `allocate` was called on a milestone whose deadline has already passed + /// (#489). Once past the deadline, sponsors may cancel; allowing further + /// allocations would let the admin pre-empt that cancel. + DeadlinePassed = 22, /// The deadline supplied to `create_milestone` is in the past (issue #491). DeadlineInPast = 22, } diff --git a/contracts/milestones/src/lib.rs b/contracts/milestones/src/lib.rs index 0e03528f..c3f257f2 100644 --- a/contracts/milestones/src/lib.rs +++ b/contracts/milestones/src/lib.rs @@ -338,6 +338,17 @@ mod contract { if milestone.closed { return Err(Error::MilestoneClosed); } + + // Reject allocations once the milestone's deadline has passed (#489). + // Once `now >= deadline` sponsors can trigger `cancel_milestone_after_deadline` + // after the grace period; allowing allocations beyond the deadline lets the + // admin pre-empt that cancel by moving remaining_budget into allocations that + // the permissionless cancel cannot sweep. + let now = env.ledger().timestamp(); + if now >= milestone.deadline { + return Err(Error::DeadlinePassed); + } + if milestone.allocations.contains_key(issue_id) { return Err(Error::IssueAlreadyAllocated); }