diff --git a/app/backend/src/onchain/utils/soroban-error.mapper.ts b/app/backend/src/onchain/utils/soroban-error.mapper.ts index 4597a172..fc24a469 100644 --- a/app/backend/src/onchain/utils/soroban-error.mapper.ts +++ b/app/backend/src/onchain/utils/soroban-error.mapper.ts @@ -125,6 +125,31 @@ export class SorobanErrorMapper { message: 'Claim cooldown is still active', errorCode: INTEGRATION_ERROR_CODES.ONCHAIN_INVALID_STATE, }, + 23: { + code: 409, + message: 'Address is already a distributor', + errorCode: INTEGRATION_ERROR_CODES.ONCHAIN_CONTRACT_ERROR, + }, + 24: { + code: 404, + message: 'Address is not a distributor', + errorCode: INTEGRATION_ERROR_CODES.ONCHAIN_CONTRACT_ERROR, + }, + 25: { + code: 400, + message: 'Maximum number of distributors reached', + errorCode: INTEGRATION_ERROR_CODES.ONCHAIN_CONTRACT_ERROR, + }, + 26: { + code: 400, + message: 'Surplus withdrawal timelock has not elapsed yet', + errorCode: INTEGRATION_ERROR_CODES.ONCHAIN_INVALID_STATE, + }, + 27: { + code: 400, + message: 'No pending surplus withdrawal in progress', + errorCode: INTEGRATION_ERROR_CODES.ONCHAIN_CONTRACT_ERROR, + }, }; /** @@ -435,6 +460,16 @@ export class SorobanErrorMapper { message: 'Claim cooldown is still active', errorCode: INTEGRATION_ERROR_CODES.ONCHAIN_INVALID_STATE, }, + TimelockNotElapsed: { + code: 400, + message: 'Surplus withdrawal timelock has not elapsed yet', + errorCode: INTEGRATION_ERROR_CODES.ONCHAIN_INVALID_STATE, + }, + NoPendingWithdrawal: { + code: 400, + message: 'No pending surplus withdrawal in progress', + errorCode: INTEGRATION_ERROR_CODES.ONCHAIN_CONTRACT_ERROR, + }, }; for (const [errorKey, errorInfo] of Object.entries(errorMap)) { diff --git a/app/onchain/contracts/aid_escrow/EVENTS.md b/app/onchain/contracts/aid_escrow/EVENTS.md index c9ebc06b..945ceef3 100644 --- a/app/onchain/contracts/aid_escrow/EVENTS.md +++ b/app/onchain/contracts/aid_escrow/EVENTS.md @@ -68,7 +68,9 @@ When making event schema changes: | `package_refunded` | `refund` | Admin refunds an expired/cancelled package. | | `package_swept` | `sweep_expired_packages` | Sweep transitions an expired `Created` package to terminal `Expired` (funds released from locked). | | `extended_event` | `extend_expiration` | Admin extends a package expiry. | -| `surplus_withdrawn_event` | `withdraw_surplus` | Admin withdraws unallocated surplus from the pool. | +| `surplus_withdrawal_proposed` | `propose_surplus_withdrawal` | Admin proposes a surplus withdrawal, starting the timelock. | +| `surplus_withdrawal_cancelled` | `cancel_surplus_withdrawal` | Admin cancels a pending surplus withdrawal. | +| `surplus_withdrawn_event` | `execute_surplus_withdrawal` | A proposed surplus withdrawal clears its timelock and funds move. | | `contract_paused_event` | `pause` | Admin pauses the whole contract. | | `contract_unpaused_event` | `unpause` | Admin unpauses the whole contract. | | `action_paused_event` | `pause_action` | Admin pauses a single action (create/claim/withdraw). | @@ -107,6 +109,8 @@ Pool / administrative events: | `EscrowFunded` | `schema_version: u32`, `from: Address`, `token: Address`, `amount: i128`, `timestamp: u64` | | `BatchCreatedEvent` | `schema_version: u32`, `ids: Vec`, `admin: Address`, `total_amount: i128` | | `ExtendedEvent` | `schema_version: u32`, `package_id: u64`, `admin: Address`, `old_expires_at: u64`, `new_expires_at: u64` | +| `SurplusWithdrawalProposed` | `schema_version: u32`, `admin: Address`, `to: Address`, `token: Address`, `amount: i128`, `unlock_time: u64`, `timestamp: u64` | +| `SurplusWithdrawalCancelled` | `schema_version: u32`, `admin: Address`, `to: Address`, `token: Address`, `amount: i128`, `timestamp: u64` | | `SurplusWithdrawnEvent` | `schema_version: u32`, `to: Address`, `token: Address`, `amount: i128` | | `ContractPausedEvent` | `schema_version: u32`, `admin: Address` | | `ContractUnpausedEvent` | `schema_version: u32`, `admin: Address` | diff --git a/app/onchain/contracts/aid_escrow/README.md b/app/onchain/contracts/aid_escrow/README.md index a0e9c029..17a26e96 100644 --- a/app/onchain/contracts/aid_escrow/README.md +++ b/app/onchain/contracts/aid_escrow/README.md @@ -46,6 +46,8 @@ expires and is refunded. | `pause_campaign(env, campaign_ref)` | Admin | Pauses `claim`/`disburse`/`refund` for packages tagged with this `campaign_ref`. | | `unpause_campaign(env, campaign_ref)` | Admin | Unpauses the campaign. | | `is_campaign_paused(env, campaign_ref)` | — | Returns true if the campaign is paused (or the contract is globally paused). | +| `get_surplus_withdrawal_delay(env)` | — | Returns the configured surplus withdrawal timelock delay, in seconds. | +| `set_surplus_withdrawal_delay(env, delay_seconds)` | Admin | Configures the surplus withdrawal timelock delay. | ### Funding @@ -75,7 +77,10 @@ expires and is refunded. | `get_package(env, id)` | — | Returns full package details. | | `view_package_status(env, id)` | — | Returns only the status (cheaper for polling). | | `get_aggregates(env, token)` | — | Returns aggregate stats: total committed, claimed, expired/cancelled for a token. | -| `withdraw_surplus(env, token, to, amount)` | Admin | Withdraws surplus (unlocked) tokens from the contract. | +| `propose_surplus_withdrawal(env, to, amount, token)` | Admin | Proposes a surplus (unlocked) withdrawal; executable only after the configured timelock delay. Overwrites any existing proposal. | +| `get_pending_surplus_withdrawal(env)` | — | Returns the pending surplus withdrawal proposal, if any. | +| `cancel_surplus_withdrawal(env)` | Admin | Cancels the pending surplus withdrawal proposal. | +| `execute_surplus_withdrawal(env)` | Admin | Executes the pending surplus withdrawal once its timelock has elapsed. | ## Package Lifecycle @@ -104,7 +109,7 @@ Cancelled --> Refunded (admin refunds) | 10 | `PackageIdExists` | Duplicate ID in `create_package`. | | 11 | `InvalidState` | Generic state violation (e.g. paused, bad config). | | 12 | `MismatchedArrays` | `recipients` and `amounts` lengths differ in batch create. | -| 13 | `InsufficientSurplus` | `withdraw_surplus` amount exceeds available surplus. | +| 13 | `InsufficientSurplus` | Requested surplus withdrawal amount exceeds available surplus. | | 14 | `ContractPaused` | Operation blocked because contract is paused. | | 15 | `ClaimTooEarly` | Claim attempted before the claim window opens. | | 16 | `InvalidProof` | Claim proof is invalid or missing. | @@ -114,6 +119,11 @@ Cancelled --> Refunded (admin refunds) | 20 | `InvalidPendingAdmin` | Pending admin address does not match the caller. | | 21 | `BatchTooLarge` | Batch operation exceeds the maximum allowed size. | | 22 | `ClaimCooldownActive` | Recipient has not yet completed the claim cooldown. | +| 23 | `DistributorAlreadyExists` | `add_distributor` called for an address that is already a distributor. | +| 24 | `DistributorNotFound` | `remove_distributor` called for an address that is not a distributor. | +| 25 | `DistributorSetFull` | `add_distributor` would exceed the configured maximum distributor set size. | +| 26 | `TimelockNotElapsed` | `execute_surplus_withdrawal` called before the proposal's timelock elapsed. | +| 27 | `NoPendingWithdrawal` | `cancel_surplus_withdrawal` / `execute_surplus_withdrawal` called with no proposal outstanding. | ### Compatibility Policy @@ -124,7 +134,7 @@ user-facing messages, so reordering or removing a variant would silently break that mapping. - **Adding a new error**: append the new variant with the **next unused code** - (currently `23`). Never reuse, renumber, or skip codes. + (currently `28`). Never reuse, renumber, or skip codes. - **Removing an error**: do **not** remove a variant. If it is no longer emitted, keep the variant and its code so existing mappings remain valid. - **Renaming**: renaming a variant is allowed only if the numeric code is diff --git a/app/onchain/contracts/aid_escrow/STORAGE_KEYS.md b/app/onchain/contracts/aid_escrow/STORAGE_KEYS.md index 7199d2b2..7d8d3d67 100644 --- a/app/onchain/contracts/aid_escrow/STORAGE_KEYS.md +++ b/app/onchain/contracts/aid_escrow/STORAGE_KEYS.md @@ -63,6 +63,8 @@ A singleton key is a bare `Symbol`; exactly one entry exists per key. | `KEY_RECIPIENT_LAST_CLAIM` | `"lastclaim"` | `Map` (recipient → successful-claim timestamp) | Successful claim paths only. Enforces the optional `Config.claim_cooldown`; absent entries have no cooldown history. | | `KEY_PKG_COUNTER` | `"pkg_cnt"` | `u64` | Package creation. Highest assigned id + 1; upper bound for id scans (`get_campaign_package_count`, etc.). | | `KEY_PKG_IDX` | `"pkg_idx"` | `u64` | Package creation. Count of aggregation-index entries; positional bound for `get_aggregates`. May exceed `KEY_PKG_COUNTER` when explicit ids are used. | +| `KEY_SURPLUS_WITHDRAWAL_DELAY` | `"wd_delay"` | `u64` | `set_surplus_withdrawal_delay`. Falls back to `DEFAULT_SURPLUS_WITHDRAWAL_DELAY` (1 day) when absent. Permanent. | +| `KEY_PENDING_SURPLUS_WITHDRAWAL` | `"pend_wd"` | `PendingSurplusWithdrawal` | `propose_surplus_withdrawal`. **Ephemeral**: removed by `execute_surplus_withdrawal` / `cancel_surplus_withdrawal`. Absent when no withdrawal is proposed. | ### Persistent storage @@ -113,12 +115,14 @@ above. Rules of thumb: `KEY_ADMIN`, `KEY_PENDING_ADMIN` *(if a transfer is mid-flight)*, all `("pkg", id)` records, `KEY_TOTAL_LOCKED`, `KEY_TOTAL_CLAIMED`, `KEY_CAMPAIGN_TOKEN_LOCKED`, `KEY_CAMPAIGN_TOKEN_CLAIMED`, - `KEY_PKG_COUNTER`, `KEY_PKG_IDX`, all `("pidx", position)` entries, and the + `KEY_PKG_COUNTER`, `KEY_PKG_IDX`, all `("pidx", position)` entries, the three delegate keys (`KEY_DELEGATES`, `KEY_DELEGATE_HISTORY`, - `KEY_DELEGATE_EXPIRY`). + `KEY_DELEGATE_EXPIRY`), and `KEY_PENDING_SURPLUS_WITHDRAWAL` *(if a + withdrawal is mid-flight)*. 3. **Safe to drop/reset without fund impact** (policy flags only): `KEY_PAUSED`, `KEY_PAUSE_*`, `KEY_CAMPAIGN_PAUSED`, `KEY_DISTRIBUTORS`, `KEY_MAX_DISTRIBUTORS` *(resets to `DEFAULT_MAX_DISTRIBUTORS`)*, + `KEY_SURPLUS_WITHDRAWAL_DELAY` *(resets to `DEFAULT_SURPLUS_WITHDRAWAL_DELAY`)*, `KEY_CONFIG` *(re-initialize before unpausing)*. Dropping them changes behaviour, not solvency. 4. **Derived/recomputable**: `KEY_TOTAL_LOCKED` can be rebuilt by scanning all diff --git a/app/onchain/contracts/aid_escrow/src/keys.rs b/app/onchain/contracts/aid_escrow/src/keys.rs index ced552fe..ecf1f6da 100644 --- a/app/onchain/contracts/aid_escrow/src/keys.rs +++ b/app/onchain/contracts/aid_escrow/src/keys.rs @@ -54,7 +54,8 @@ pub const KEY_PAUSE_CREATE: Symbol = symbol_short!("p_create"); pub const KEY_PAUSE_CLAIM: Symbol = symbol_short!("p_claim"); /// Per-action pause flag for `refund` (`bool`). pub const KEY_PAUSE_REFUND: Symbol = symbol_short!("p_refund"); -/// Per-action pause flag for `withdraw_surplus` (`bool`). +/// Per-action pause flag for `propose_surplus_withdrawal` / +/// `execute_surplus_withdrawal` (`bool`). pub const KEY_PAUSE_WITHDRAW: Symbol = symbol_short!("p_wdrw"); /// Campaign pause registry (`Map` keyed by `campaign_ref`). pub const KEY_CAMPAIGN_PAUSED: Symbol = symbol_short!("camp_pzd"); @@ -93,6 +94,16 @@ pub const KEY_PKG_COUNTER: Symbol = symbol_short!("pkg_cnt"); /// upper bound for `get_aggregates`; may exceed the counter when explicit /// ids are used. pub const KEY_PKG_IDX: Symbol = symbol_short!("pkg_idx"); +/// Configurable delay in seconds a proposed surplus withdrawal must wait +/// before it can be executed (`u64`). Falls back to +/// `DEFAULT_SURPLUS_WITHDRAWAL_DELAY` when absent. Admin-managed via +/// `set_surplus_withdrawal_delay`. +pub const KEY_SURPLUS_WITHDRAWAL_DELAY: Symbol = symbol_short!("wd_delay"); +/// Pending surplus withdrawal proposal (`Option`). +/// Written by `propose_surplus_withdrawal`; removed by +/// `execute_surplus_withdrawal` / `cancel_surplus_withdrawal`. Absent when no +/// withdrawal is proposed. +pub const KEY_PENDING_SURPLUS_WITHDRAWAL: Symbol = symbol_short!("pend_wd"); // --- Singleton keys: persistent storage --- // Persistent-storage singletons owned by the delegate module. @@ -146,7 +157,7 @@ mod tests { use super::*; /// Every singleton key, both storage families. - fn singleton_keys() -> [Symbol; 22] { + fn singleton_keys() -> [Symbol; 24] { [ KEY_ADMIN, KEY_PENDING_ADMIN, @@ -170,6 +181,8 @@ mod tests { KEY_DELEGATES, KEY_DELEGATE_HISTORY, KEY_DELEGATE_EXPIRY, + KEY_SURPLUS_WITHDRAWAL_DELAY, + KEY_PENDING_SURPLUS_WITHDRAWAL, ] } diff --git a/app/onchain/contracts/aid_escrow/src/lib.rs b/app/onchain/contracts/aid_escrow/src/lib.rs index c1c61184..750eae78 100644 --- a/app/onchain/contracts/aid_escrow/src/lib.rs +++ b/app/onchain/contracts/aid_escrow/src/lib.rs @@ -37,8 +37,9 @@ pub use crate::keys::{ package_index_entry, package_key, KEY_ADMIN, KEY_CAMPAIGN_PAUSED, KEY_CAMPAIGN_TOKEN_CLAIMED, KEY_CAMPAIGN_TOKEN_LOCKED, KEY_CONFIG, KEY_DELEGATES, KEY_DELEGATE_EXPIRY, KEY_DELEGATE_HISTORY, KEY_DISTRIBUTORS, KEY_MAX_DISTRIBUTORS, KEY_PAUSED, KEY_PAUSE_CLAIM, - KEY_PAUSE_CREATE, KEY_PAUSE_REFUND, KEY_PAUSE_WITHDRAW, KEY_PENDING_ADMIN, KEY_PKG_COUNTER, - KEY_PKG_IDX, KEY_RECIPIENT_LAST_CLAIM, KEY_TOTAL_CLAIMED, KEY_TOTAL_LOCKED, KEY_VERSION, + KEY_PAUSE_CREATE, KEY_PAUSE_REFUND, KEY_PAUSE_WITHDRAW, KEY_PENDING_ADMIN, + KEY_PENDING_SURPLUS_WITHDRAWAL, KEY_PKG_COUNTER, KEY_PKG_IDX, KEY_RECIPIENT_LAST_CLAIM, + KEY_SURPLUS_WITHDRAWAL_DELAY, KEY_TOTAL_CLAIMED, KEY_TOTAL_LOCKED, KEY_VERSION, }; /// Upper bound on the number of package ids accepted by `batch_claim` in a @@ -65,6 +66,12 @@ pub const MAX_DISTRIBUTOR_PAGE_SIZE: u32 = 50; /// in a backward-incompatible way. See EVENTS.md for compatibility policy. pub const EVENT_SCHEMA_VERSION: u32 = 1; +/// Default delay (in seconds) a proposed surplus withdrawal must wait before +/// it can be executed, used until an admin calls +/// `set_surplus_withdrawal_delay`. One day, chosen to give observers a +/// realistic window to notice and react to a suspicious proposal. +pub const DEFAULT_SURPLUS_WITHDRAWAL_DELAY: u64 = 86400; + // --- Data Types --- #[contracttype] @@ -112,6 +119,20 @@ pub struct Aggregates { pub total_expired_cancelled: i128, } +/// A surplus withdrawal proposed by the admin but not yet executed. Created +/// by `propose_surplus_withdrawal`; removed by `execute_surplus_withdrawal` +/// or `cancel_surplus_withdrawal`. At most one proposal is outstanding at a +/// time. +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct PendingSurplusWithdrawal { + pub to: Address, + pub amount: i128, + pub token: Address, + /// Ledger timestamp (Unix seconds) at or after which the proposal may be executed. + pub unlock_time: u64, +} + /// Outcome of a single package claim attempt made as part of a `batch_claim` /// call. `batch_claim` never fails a whole batch because one package could /// not be claimed; instead each id resolves to one of these statuses. @@ -187,6 +208,12 @@ pub enum Error { /// `add_distributor` would exceed the configured maximum distributor /// set size (see `get_max_distributors` / `set_max_distributors`). DistributorSetFull = 25, + /// `execute_surplus_withdrawal` was called before the pending + /// proposal's `unlock_time` was reached. + TimelockNotElapsed = 26, + /// `cancel_surplus_withdrawal` / `execute_surplus_withdrawal` was called + /// with no proposal outstanding. + NoPendingWithdrawal = 27, } // --- Contract Events (indexer-friendly; stable topics & payloads) --- @@ -308,6 +335,33 @@ pub struct ExtendedEvent { pub new_expires_at: u64, } +/// Emitted when the admin proposes a surplus withdrawal, starting the +/// timelock. Actor = admin. +#[contractevent] +pub struct SurplusWithdrawalProposed { + pub schema_version: u32, + pub admin: Address, + pub to: Address, + pub token: Address, + pub amount: i128, + pub unlock_time: u64, + pub timestamp: u64, +} + +/// Emitted when the admin cancels a pending surplus withdrawal before it is +/// executed. Actor = admin. +#[contractevent] +pub struct SurplusWithdrawalCancelled { + pub schema_version: u32, + pub admin: Address, + pub to: Address, + pub token: Address, + pub amount: i128, + pub timestamp: u64, +} + +/// Emitted once a proposed withdrawal clears its timelock and funds are +/// transferred out. Actor = admin. #[contractevent] pub struct SurplusWithdrawnEvent { pub schema_version: u32, @@ -2097,58 +2151,202 @@ impl AidEscrow { Ok(()) } - /// Admin-only function to withdraw surplus (unallocated) funds from the contract. - /// Requirements: Admin auth, valid amount, sufficient surplus available. - /// Behavior: Transfers amount of token from contract to the specified address. - pub fn withdraw_surplus( + /// Returns the configured surplus withdrawal timelock delay, in seconds. + /// Defaults to `DEFAULT_SURPLUS_WITHDRAWAL_DELAY` if never explicitly set. + pub fn get_surplus_withdrawal_delay(env: Env) -> u64 { + env.storage() + .instance() + .get(&KEY_SURPLUS_WITHDRAWAL_DELAY) + .unwrap_or(DEFAULT_SURPLUS_WITHDRAWAL_DELAY) + } + + /// Admin-only. Configures how long a proposed surplus withdrawal must + /// wait before it can be executed. Does not affect a proposal already + /// in flight (its `unlock_time` was fixed at proposal time). + /// + /// # Errors + /// Returns `Error::NotInitialized` if the contract has not been initialized. + /// Returns `Error::NotAuthorized` if the caller is not the admin. + pub fn set_surplus_withdrawal_delay(env: Env, delay_seconds: u64) -> Result<(), Error> { + let admin = Self::get_admin(env.clone())?; + admin.require_auth(); + + env.storage() + .instance() + .set(&KEY_SURPLUS_WITHDRAWAL_DELAY, &delay_seconds); + + Ok(()) + } + + /// Returns the currently pending surplus withdrawal proposal, if any. + pub fn get_pending_surplus_withdrawal(env: Env) -> Option { + env.storage().instance().get(&KEY_PENDING_SURPLUS_WITHDRAWAL) + } + + /// Admin-only. Proposes withdrawing surplus (unallocated) funds from the + /// contract. The proposal can only be executed once + /// `get_surplus_withdrawal_delay()` seconds have elapsed (see + /// `execute_surplus_withdrawal`), giving observers a window to notice and + /// react to a suspicious withdrawal before funds move. Overwrites any + /// existing pending proposal. + /// + /// # Errors + /// Returns `Error::InvalidAmount` if `amount` is not strictly positive. + /// Returns `Error::InvalidToken` if `token` does not implement the + /// expected token interface. + /// Returns `Error::InsufficientSurplus` if `amount` exceeds the token + /// balance currently unallocated to any package. + pub fn propose_surplus_withdrawal( env: Env, to: Address, amount: i128, token: Address, ) -> Result<(), Error> { Self::check_action_paused(&env, symbol_short!("withdraw"))?; - // 1. Only the admin can withdraw surplus let admin = Self::get_admin(env.clone())?; admin.require_auth(); - // 2. Validate amount if amount <= 0 { return Err(Error::InvalidAmount); } - // 3. Get contract's current balance for the token Self::validate_token(&env, &token)?; - let contract_balance = Self::token_balance(&env, &token, &env.current_contract_address())?; + let available_surplus = Self::available_surplus(&env, &token)?; + if amount > available_surplus { + return Err(Error::InsufficientSurplus); + } - // 4. Get total locked amount for the token - let locked_map: Map = env + let now = env.ledger().timestamp(); + let unlock_time = now + Self::get_surplus_withdrawal_delay(env.clone()); + + let pending = PendingSurplusWithdrawal { + to: to.clone(), + amount, + token: token.clone(), + unlock_time, + }; + env.storage() + .instance() + .set(&KEY_PENDING_SURPLUS_WITHDRAWAL, &pending); + + SurplusWithdrawalProposed { + schema_version: EVENT_SCHEMA_VERSION, + admin, + to, + token, + amount, + unlock_time, + timestamp: now, + } + .publish(&env); + + Ok(()) + } + + /// Admin-only. Cancels the pending surplus withdrawal proposal without + /// transferring any funds. + /// + /// # Errors + /// Returns `Error::NoPendingWithdrawal` if no proposal is outstanding. + pub fn cancel_surplus_withdrawal(env: Env) -> Result<(), Error> { + let admin = Self::get_admin(env.clone())?; + admin.require_auth(); + + let pending: PendingSurplusWithdrawal = env .storage() .instance() - .get(&KEY_TOTAL_LOCKED) - .unwrap_or(Map::new(&env)); - let total_locked = locked_map.get(token.clone()).unwrap_or(0); + .get(&KEY_PENDING_SURPLUS_WITHDRAWAL) + .ok_or(Error::NoPendingWithdrawal)?; - // 5. Calculate available surplus and validate - let available_surplus = contract_balance - total_locked; - if amount > available_surplus { + env.storage() + .instance() + .remove(&KEY_PENDING_SURPLUS_WITHDRAWAL); + + SurplusWithdrawalCancelled { + schema_version: EVENT_SCHEMA_VERSION, + admin, + to: pending.to, + token: pending.token, + amount: pending.amount, + timestamp: env.ledger().timestamp(), + } + .publish(&env); + + Ok(()) + } + + /// Admin-only. Executes the pending surplus withdrawal proposal once its + /// timelock has elapsed, transferring the proposed amount to the + /// proposed recipient. + /// + /// Re-checks available surplus at execution time (not just at proposal + /// time), since the contract's balance or locked total may have changed + /// while the proposal was pending. + /// + /// # Errors + /// Returns `Error::NoPendingWithdrawal` if no proposal is outstanding. + /// Returns `Error::TimelockNotElapsed` if called before `unlock_time`. + /// Returns `Error::InsufficientSurplus` if the proposed amount no longer + /// fits within the currently available surplus. + pub fn execute_surplus_withdrawal(env: Env) -> Result<(), Error> { + Self::check_action_paused(&env, symbol_short!("withdraw"))?; + let admin = Self::get_admin(env.clone())?; + admin.require_auth(); + + let pending: PendingSurplusWithdrawal = env + .storage() + .instance() + .get(&KEY_PENDING_SURPLUS_WITHDRAWAL) + .ok_or(Error::NoPendingWithdrawal)?; + + if env.ledger().timestamp() < pending.unlock_time { + return Err(Error::TimelockNotElapsed); + } + + let available_surplus = Self::available_surplus(&env, &pending.token)?; + if pending.amount > available_surplus { return Err(Error::InsufficientSurplus); } - // 6. Transfer funds from contract to recipient - Self::transfer_token(&env, &token, &env.current_contract_address(), &to, &amount)?; + env.storage() + .instance() + .remove(&KEY_PENDING_SURPLUS_WITHDRAWAL); + + Self::transfer_token( + &env, + &pending.token, + &env.current_contract_address(), + &pending.to, + &pending.amount, + )?; - // 7. Emit event SurplusWithdrawnEvent { schema_version: EVENT_SCHEMA_VERSION, - to: to.clone(), - token: token.clone(), - amount, + to: pending.to, + token: pending.token, + amount: pending.amount, } .publish(&env); Ok(()) } + /// Contract token balance for `token` minus the amount currently locked + /// in `Created` packages, i.e. the amount an admin could withdraw. + fn available_surplus(env: &Env, token: &Address) -> Result { + let contract_balance = + Self::token_balance(env, token, &env.current_contract_address())?; + + let locked_map: Map = env + .storage() + .instance() + .get(&KEY_TOTAL_LOCKED) + .unwrap_or(Map::new(env)); + let total_locked = locked_map.get(token.clone()).unwrap_or(0); + + Ok(contract_balance - total_locked) + } + // --- Helpers --- fn check_action_paused(env: &Env, action: Symbol) -> Result<(), Error> { diff --git a/app/onchain/contracts/aid_escrow/tests/error_codes.rs b/app/onchain/contracts/aid_escrow/tests/error_codes.rs index f4ee9747..11413b0e 100644 --- a/app/onchain/contracts/aid_escrow/tests/error_codes.rs +++ b/app/onchain/contracts/aid_escrow/tests/error_codes.rs @@ -35,6 +35,11 @@ fn error_discriminants_are_stable() { assert_eq!(Error::InvalidPendingAdmin as u32, 20); assert_eq!(Error::BatchTooLarge as u32, 21); assert_eq!(Error::ClaimCooldownActive as u32, 22); + assert_eq!(Error::DistributorAlreadyExists as u32, 23); + assert_eq!(Error::DistributorNotFound as u32, 24); + assert_eq!(Error::DistributorSetFull as u32, 25); + assert_eq!(Error::TimelockNotElapsed as u32, 26); + assert_eq!(Error::NoPendingWithdrawal as u32, 27); } #[test] @@ -64,10 +69,15 @@ fn error_discriminants_are_contiguous_and_unique() { Error::InvalidPendingAdmin as u32, Error::BatchTooLarge as u32, Error::ClaimCooldownActive as u32, + Error::DistributorAlreadyExists as u32, + Error::DistributorNotFound as u32, + Error::DistributorSetFull as u32, + Error::TimelockNotElapsed as u32, + Error::NoPendingWithdrawal as u32, ]; codes.sort_unstable(); codes.dedup(); - assert_eq!(codes.len(), 22, "error codes must be unique"); + assert_eq!(codes.len(), 27, "error codes must be unique"); for (i, code) in codes.iter().enumerate() { assert_eq!( *code, diff --git a/app/onchain/contracts/aid_escrow/tests/events.rs b/app/onchain/contracts/aid_escrow/tests/events.rs index f8e2a38a..44116fe5 100644 --- a/app/onchain/contracts/aid_escrow/tests/events.rs +++ b/app/onchain/contracts/aid_escrow/tests/events.rs @@ -442,6 +442,59 @@ fn test_batch_created_event() { assert_eq!(data_i128(&env, &data, "total_amount"), 3 * UNIT); } +#[test] +fn test_surplus_withdrawal_proposed_event() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let recipient = Address::generate(&env); + let (token_client, token_admin_client) = setup_token(&env, &admin); + + let contract_id = env.register(AidEscrow, ()); + let client = AidEscrowClient::new(&env, &contract_id); + client.init(&admin); + token_admin_client.mint(&admin, &(10 * UNIT)); + client.fund(&token_client.address, &admin, &(5 * UNIT)); + + let now = env.ledger().timestamp(); + client.propose_surplus_withdrawal(&recipient, &UNIT, &token_client.address); + + let data = last_event_data(&env, &contract_id, "surplus_withdrawal_proposed"); + assert_schema_version(&env, &data, 1); + assert_eq!(data_address(&env, &data, "admin"), admin); + assert_eq!(data_address(&env, &data, "to"), recipient); + assert_eq!(data_address(&env, &data, "token"), token_client.address); + assert_eq!(data_i128(&env, &data, "amount"), UNIT); + assert_eq!(data_u64(&env, &data, "unlock_time"), now + 86400); +} + +#[test] +fn test_surplus_withdrawal_cancelled_event() { + let env = Env::default(); + env.mock_all_auths(); + + let admin = Address::generate(&env); + let recipient = Address::generate(&env); + let (token_client, token_admin_client) = setup_token(&env, &admin); + + let contract_id = env.register(AidEscrow, ()); + let client = AidEscrowClient::new(&env, &contract_id); + client.init(&admin); + token_admin_client.mint(&admin, &(10 * UNIT)); + client.fund(&token_client.address, &admin, &(5 * UNIT)); + + client.propose_surplus_withdrawal(&recipient, &UNIT, &token_client.address); + client.cancel_surplus_withdrawal(); + + let data = last_event_data(&env, &contract_id, "surplus_withdrawal_cancelled"); + assert_schema_version(&env, &data, 1); + assert_eq!(data_address(&env, &data, "admin"), admin); + assert_eq!(data_address(&env, &data, "to"), recipient); + assert_eq!(data_address(&env, &data, "token"), token_client.address); + assert_eq!(data_i128(&env, &data, "amount"), UNIT); +} + #[test] fn test_surplus_withdrawn_event() { let env = Env::default(); @@ -457,7 +510,9 @@ fn test_surplus_withdrawn_event() { token_admin_client.mint(&admin, &(10 * UNIT)); client.fund(&token_client.address, &admin, &(5 * UNIT)); - client.withdraw_surplus(&recipient, &UNIT, &token_client.address); + client.set_surplus_withdrawal_delay(&0); + client.propose_surplus_withdrawal(&recipient, &UNIT, &token_client.address); + client.execute_surplus_withdrawal(); let data = last_event_data(&env, &contract_id, "surplus_withdrawn_event"); assert_schema_version(&env, &data, 1); diff --git a/app/onchain/contracts/aid_escrow/tests/pause_controls.rs b/app/onchain/contracts/aid_escrow/tests/pause_controls.rs index fd5e7eb0..ce3b58fb 100644 --- a/app/onchain/contracts/aid_escrow/tests/pause_controls.rs +++ b/app/onchain/contracts/aid_escrow/tests/pause_controls.rs @@ -197,7 +197,7 @@ fn test_pause_blocks_withdraw() { let result = f .client - .try_withdraw_surplus(&f.recipient, &UNIT, &f.token.address); + .try_propose_surplus_withdrawal(&f.recipient, &UNIT, &f.token.address); assert!(result.is_err()); assert!(f.client.is_action_paused(&sym(&f.env, "withdraw"))); @@ -211,7 +211,7 @@ fn test_unpause_resumes_withdraw() { let result = f .client - .try_withdraw_surplus(&f.recipient, &UNIT, &f.token.address); + .try_propose_surplus_withdrawal(&f.recipient, &UNIT, &f.token.address); assert!(result.is_ok()); } @@ -247,7 +247,7 @@ fn test_global_pause_blocks_actions() { assert!(f.client.try_claim(&0u64).is_err()); assert!(f .client - .try_withdraw_surplus(&f.recipient, &UNIT, &f.token.address) + .try_propose_surplus_withdrawal(&f.recipient, &UNIT, &f.token.address) .is_err()); f.client.unpause(); diff --git a/app/onchain/contracts/aid_escrow/tests/property_based_invariants.rs b/app/onchain/contracts/aid_escrow/tests/property_based_invariants.rs index 11ba7a56..d9ef369b 100644 --- a/app/onchain/contracts/aid_escrow/tests/property_based_invariants.rs +++ b/app/onchain/contracts/aid_escrow/tests/property_based_invariants.rs @@ -2,7 +2,7 @@ #![allow(clippy::all)] #![allow(dead_code)] -use aid_escrow::{AidEscrow, AidEscrowClient, Config}; +use aid_escrow::{AidEscrow, AidEscrowClient, Config, Error}; use rand::rngs::StdRng; use rand::{Rng, SeedableRng}; use soroban_sdk::{ @@ -42,6 +42,10 @@ fn setup_env() -> ( let contract_id = env.register(AidEscrow, ()); let client = AidEscrowClient::new(&env, &contract_id); client.init(&admin); + // Zero delay: fuzz iterations below collapse propose+execute into one + // step via `try_withdraw_surplus_now`, exercising the same accounting + // paths the old single-step `withdraw_surplus` did. + client.set_surplus_withdrawal_delay(&0); client.set_config(&Config { min_amount: 1, max_expires_in: 0, @@ -63,6 +67,23 @@ fn advance_time(env: &Env, seconds: u64) { env.ledger().set(info); } +/// Test-only helper collapsing propose + execute into a single call so the +/// fuzz call sites below (which pre-date the withdrawal timelock) can keep +/// asserting on one try-result, same as the old single-step +/// `withdraw_surplus`. Only behaves this way because `setup_env` configures +/// a zero-second withdrawal delay. +fn try_withdraw_surplus_now( + client: &AidEscrowClient, + to: &Address, + amount: &i128, + token: &Address, +) -> Result, Result> { + match client.try_propose_surplus_withdrawal(to, amount, token) { + Ok(Ok(())) => client.try_execute_surplus_withdrawal(), + other => other, + } +} + // --- Invariant assertion --- fn assert_invariants( @@ -297,7 +318,7 @@ fn test_fund_accounting_invariants() { // WITHDRAW_SURPLUS: try to pull surplus let amount = UNIT * iter_rng.gen_range(1..=3) as i128; let to = Address::generate(&env); - match client.try_withdraw_surplus(&to, &amount, &token) { + match try_withdraw_surplus_now(&client, &to, &amount, &token) { Ok(Ok(())) => { total_withdrawn += amount; ops_log.push(( @@ -531,7 +552,7 @@ fn test_claim_revolve_invariants() { // WITHDRAW_SURPLUS let amount = UNIT * iter_rng.gen_range(1..=5) as i128; let to = Address::generate(&env); - match client.try_withdraw_surplus(&to, &amount, &token) { + match try_withdraw_surplus_now(&client, &to, &amount, &token) { Ok(Ok(())) => { total_withdrawn += amount; ops_log.push(( @@ -800,7 +821,7 @@ fn test_full_lifecycle_invariants() { let final_step = base_step + packages.len(); let surplus_amount = UNIT * iter_rng.gen_range(1..=10) as i128; let to = Address::generate(&env); - match client.try_withdraw_surplus(&to, &surplus_amount, &token) { + match try_withdraw_surplus_now(&client, &to, &surplus_amount, &token) { Ok(Ok(())) => { total_withdrawn += surplus_amount; ops_log.push(( @@ -1025,7 +1046,7 @@ fn test_randomized_state_machine() { // WITHDRAW_SURPLUS let amount = UNIT * iter_rng.gen_range(1..=10) as i128; let to = Address::generate(&env); - match client.try_withdraw_surplus(&to, &amount, &token) { + match try_withdraw_surplus_now(&client, &to, &amount, &token) { Ok(Ok(())) => { total_withdrawn += amount; ops_log.push(( diff --git a/app/onchain/contracts/aid_escrow/tests/storage_keys.rs b/app/onchain/contracts/aid_escrow/tests/storage_keys.rs index 9a11950e..b4cef755 100644 --- a/app/onchain/contracts/aid_escrow/tests/storage_keys.rs +++ b/app/onchain/contracts/aid_escrow/tests/storage_keys.rs @@ -46,13 +46,15 @@ fn singletons() -> Vec { KEY_DELEGATES, KEY_DELEGATE_HISTORY, KEY_DELEGATE_EXPIRY, + KEY_SURPLUS_WITHDRAWAL_DELAY, + KEY_PENDING_SURPLUS_WITHDRAWAL, ] } #[test] fn singleton_keys_are_pairwise_distinct() { let all = singletons(); - assert!(all.len() >= 21); + assert!(all.len() >= 23); for i in 0..all.len() { for j in (i + 1)..all.len() { assert_ne!( @@ -191,5 +193,5 @@ fn no_two_constructors_share_a_ledger_entry_in_a_live_env() { /// STORAGE_KEYS.md. Update both together. #[test] fn singleton_catalog_matches_documented_layout() { - assert_eq!(singletons().len(), 21); + assert_eq!(singletons().len(), 23); } diff --git a/app/onchain/contracts/aid_escrow/tests/withdraw_surplus.rs b/app/onchain/contracts/aid_escrow/tests/withdraw_surplus.rs index a2392440..7d5e9a6f 100644 --- a/app/onchain/contracts/aid_escrow/tests/withdraw_surplus.rs +++ b/app/onchain/contracts/aid_escrow/tests/withdraw_surplus.rs @@ -1,14 +1,21 @@ #![cfg(test)] +//! Timelock coverage for the propose -> execute surplus withdrawal flow +//! (see #968: a single-step `withdraw_surplus` gave a compromised admin key +//! no observation window before funds moved). These tests exercise the full +//! lifecycle — propose, cancel, execute — plus the boundary timing around +//! the configurable delay. + use aid_escrow::{AidEscrow, AidEscrowClient, Error}; use soroban_sdk::{ - testutils::Address as _, + testutils::{Address as _, Ledger}, token::{StellarAssetClient, TokenClient}, Address, Env, Map, }; // We still use UNIT for funding to keep our test math clean const UNIT: i128 = 10_000_000; +const DEFAULT_DELAY: u64 = 86400; fn setup_token(env: &Env, admin: &Address) -> (TokenClient<'static>, StellarAssetClient<'static>) { let token_contract = env.register_stellar_asset_contract_v2(admin.clone()); @@ -45,27 +52,30 @@ fn setup_funded( (client, token_client, admin, token_admin) } +fn advance_time(env: &Env, seconds: u64) { + let mut info = env.ledger().get(); + info.timestamp += seconds; + env.ledger().set(info); +} + #[test] -fn test_withdraw_surplus_invalid_amount() { +fn test_propose_surplus_withdrawal_invalid_amount() { let env = Env::default(); env.mock_all_auths(); let (client, token_client, admin, _) = setup_funded(&env, 5); - // 1. Zero amount: Contract checks "amount <= 0", so this SHOULD fail. - let res_zero = client.try_withdraw_surplus(&admin, &0, &token_client.address); + let res_zero = client.try_propose_surplus_withdrawal(&admin, &0, &token_client.address); assert_eq!(res_zero, Err(Ok(Error::InvalidAmount))); - // 2. Negative amount: Contract checks "amount <= 0", so this SHOULD fail. - let res_neg = client.try_withdraw_surplus(&admin, &-UNIT, &token_client.address); + let res_neg = client.try_propose_surplus_withdrawal(&admin, &-UNIT, &token_client.address); assert_eq!(res_neg, Err(Ok(Error::InvalidAmount))); - // NOTE: We removed the check for "500" because your contract - // does not currently enforce whole-token withdrawals. + assert_eq!(client.get_pending_surplus_withdrawal(), None); } #[test] -fn test_withdraw_surplus_insufficient_surplus() { +fn test_propose_surplus_withdrawal_insufficient_surplus() { let env = Env::default(); env.mock_all_auths(); @@ -83,18 +93,197 @@ fn test_withdraw_surplus_insufficient_surplus() { ); // Balance 10, Locked 8, Surplus 2. Request 3. - let result = client.try_withdraw_surplus(&admin, &(3 * UNIT), &token_client.address); + let result = client.try_propose_surplus_withdrawal(&admin, &(3 * UNIT), &token_client.address); + assert_eq!(result, Err(Ok(Error::InsufficientSurplus))); + assert_eq!(client.get_pending_surplus_withdrawal(), None); +} + +#[test] +fn test_propose_records_pending_withdrawal_with_default_delay() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, token_client, admin, _) = setup_funded(&env, 1); + let now = env.ledger().timestamp(); + + client.propose_surplus_withdrawal(&admin, &UNIT, &token_client.address); + + let pending = client.get_pending_surplus_withdrawal().unwrap(); + assert_eq!(pending.to, admin); + assert_eq!(pending.amount, UNIT); + assert_eq!(pending.token, token_client.address); + assert_eq!(pending.unlock_time, now + DEFAULT_DELAY); + + // Funds must not move until the proposal is executed. + assert_eq!(token_client.balance(&client.address), UNIT); +} + +#[test] +fn test_execute_before_delay_elapses_fails() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, token_client, admin, _) = setup_funded(&env, 1); + + client.propose_surplus_withdrawal(&admin, &UNIT, &token_client.address); + + let result = client.try_execute_surplus_withdrawal(); + assert_eq!(result, Err(Ok(Error::TimelockNotElapsed))); + assert_eq!(token_client.balance(&client.address), UNIT); +} + +#[test] +fn test_execute_one_second_before_unlock_time_fails() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, token_client, admin, _) = setup_funded(&env, 1); + + client.propose_surplus_withdrawal(&admin, &UNIT, &token_client.address); + advance_time(&env, DEFAULT_DELAY - 1); + + let result = client.try_execute_surplus_withdrawal(); + assert_eq!(result, Err(Ok(Error::TimelockNotElapsed))); +} + +#[test] +fn test_execute_exactly_at_unlock_time_succeeds() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, token_client, admin, _) = setup_funded(&env, 1); + + client.propose_surplus_withdrawal(&admin, &UNIT, &token_client.address); + advance_time(&env, DEFAULT_DELAY); + + client.execute_surplus_withdrawal(); + + assert_eq!(token_client.balance(&client.address), 0); + assert_eq!(token_client.balance(&admin), UNIT); + assert_eq!(client.get_pending_surplus_withdrawal(), None); +} + +#[test] +fn test_execute_after_delay_succeeds() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, token_client, admin, _) = setup_funded(&env, 1); + + client.propose_surplus_withdrawal(&admin, &UNIT, &token_client.address); + advance_time(&env, DEFAULT_DELAY + 1); + + client.execute_surplus_withdrawal(); + + assert_eq!(token_client.balance(&client.address), 0); + assert_eq!(token_client.balance(&admin), UNIT); +} + +#[test] +fn test_execute_revalidates_surplus_at_execution_time() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, token_client, admin, _) = setup_funded(&env, 10); + let recipient = Address::generate(&env); + + // Surplus is 10 at proposal time, so proposing 6 succeeds. + client.propose_surplus_withdrawal(&admin, &(6 * UNIT), &token_client.address); + + // Before the timelock elapses, a new package locks up 8 of the 10, + // leaving only 2 available — no longer enough for the proposal. + client.create_package( + &admin, + &1, + &recipient, + &(8 * UNIT), + &token_client.address, + &(env.ledger().timestamp() + 1000), + &Map::new(&env), + ); + advance_time(&env, DEFAULT_DELAY); + + let result = client.try_execute_surplus_withdrawal(); assert_eq!(result, Err(Ok(Error::InsufficientSurplus))); + + // The proposal is left in place; the admin can retry once locked funds free up. + assert!(client.get_pending_surplus_withdrawal().is_some()); +} + +#[test] +fn test_execute_with_no_pending_withdrawal_fails() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, _token_client, _admin, _) = setup_funded(&env, 1); + + let result = client.try_execute_surplus_withdrawal(); + assert_eq!(result, Err(Ok(Error::NoPendingWithdrawal))); } #[test] -fn test_withdraw_surplus_no_locked_funds() { +fn test_cancel_surplus_withdrawal_removes_pending_and_blocks_execution() { let env = Env::default(); env.mock_all_auths(); let (client, token_client, admin, _) = setup_funded(&env, 1); - client.withdraw_surplus(&admin, &UNIT, &token_client.address); + client.propose_surplus_withdrawal(&admin, &UNIT, &token_client.address); + assert!(client.get_pending_surplus_withdrawal().is_some()); + + client.cancel_surplus_withdrawal(); + assert_eq!(client.get_pending_surplus_withdrawal(), None); + + advance_time(&env, DEFAULT_DELAY); + let result = client.try_execute_surplus_withdrawal(); + assert_eq!(result, Err(Ok(Error::NoPendingWithdrawal))); + assert_eq!(token_client.balance(&client.address), UNIT); +} + +#[test] +fn test_cancel_with_no_pending_withdrawal_fails() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, _token_client, _admin, _) = setup_funded(&env, 1); + + let result = client.try_cancel_surplus_withdrawal(); + assert_eq!(result, Err(Ok(Error::NoPendingWithdrawal))); +} + +#[test] +fn test_propose_overwrites_existing_pending_withdrawal() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, token_client, admin, _) = setup_funded(&env, 5); + let other_recipient = Address::generate(&env); + + client.propose_surplus_withdrawal(&admin, &UNIT, &token_client.address); + advance_time(&env, 100); + client.propose_surplus_withdrawal(&other_recipient, &(2 * UNIT), &token_client.address); + + let pending = client.get_pending_surplus_withdrawal().unwrap(); + assert_eq!(pending.to, other_recipient); + assert_eq!(pending.amount, 2 * UNIT); + assert_eq!(pending.unlock_time, env.ledger().timestamp() + DEFAULT_DELAY); +} + +#[test] +fn test_set_surplus_withdrawal_delay_applies_to_new_proposals() { + let env = Env::default(); + env.mock_all_auths(); + + let (client, token_client, admin, _) = setup_funded(&env, 1); + + assert_eq!(client.get_surplus_withdrawal_delay(), DEFAULT_DELAY); + + client.set_surplus_withdrawal_delay(&0); + assert_eq!(client.get_surplus_withdrawal_delay(), 0); + + client.propose_surplus_withdrawal(&admin, &UNIT, &token_client.address); + // Delay of zero: no need to advance the ledger before executing. + client.execute_surplus_withdrawal(); assert_eq!(token_client.balance(&client.address), 0); assert_eq!(token_client.balance(&admin), UNIT);