diff --git a/contracts/batch-processor/src/lib.rs b/contracts/batch-processor/src/lib.rs index 8aee7c27..fa2fb690 100644 --- a/contracts/batch-processor/src/lib.rs +++ b/contracts/batch-processor/src/lib.rs @@ -1,142 +1,91 @@ #![no_std] -use soroban_sdk::{contract, contracterror, contractimpl, symbol_short, Env, Symbol}; +use soroban_sdk::{contract, contracterror, contractimpl, token, Address, Env, Vec}; + +/// Maximum number of transfers permitted in a single batch. +const MAX_BATCH_SIZE: u32 = 100; #[contracterror] #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] #[repr(u32)] pub enum Error { - ProcessorLocked = 2001, - CalculationOverflow = 2002, - BatchTooLarge = 2003, - StateVersionMismatch = 2004, - StaleCallbackCleaned = 2005, + /// `recipients` and `amounts` have different lengths. + LengthMismatch = 1, + /// The batch exceeds [`MAX_BATCH_SIZE`]. + BatchTooLarge = 2, + /// An individual amount is zero or negative. + InvalidAmount = 3, + /// Integer overflow computing the total to pull from the funder. + ArithmeticOverflow = 4, } #[contract] pub struct BatchTransferProcessor; -/// Storage key for the state version counter. -const STATE_VERSION_KEY: Symbol = symbol_short!("B_Ver"); - -/// Storage key for the callback sequence counter. -const CALLBACK_SEQ_KEY: Symbol = symbol_short!("B_CbSeq"); - #[contractimpl] impl BatchTransferProcessor { - pub fn process_batch(env: Env, amounts: soroban_sdk::Vec) -> Result { - with_guard(&env, || { - // ── Defensive null / boundary checks (Issue #83) ───────────── - if amounts.is_empty() { - return Ok(0); - } - - if amounts.len() > 100 { - return Err(Error::BatchTooLarge); - } - - for amount in amounts.iter() { - if amount == 0 { - return Err(Error::CalculationOverflow); - } + /// Transfer tokens from `funder` to each address in `recipients`. + /// + /// # Auth + /// `funder.require_auth()` is called before any state mutation. The funder + /// must have pre-authorised a transfer of at least `sum(amounts)` tokens to + /// this contract, which then fans the funds out to the recipients in order. + /// + /// # Validation (all checks precede any token movement) + /// - `recipients` and `amounts` must have the same length. + /// - The batch must not exceed `MAX_BATCH_SIZE` (100 entries). + /// - Every individual `amount` must be > 0. + /// + /// # Returns + /// The total number of tokens transferred on success. + pub fn process_batch( + env: Env, + funder: Address, + token: Address, + recipients: Vec
, + amounts: Vec, + ) -> Result { + // ── Input validation (before auth and any token movement) ──────────── + + if recipients.len() != amounts.len() { + return Err(Error::LengthMismatch); + } + + if amounts.len() > MAX_BATCH_SIZE { + return Err(Error::BatchTooLarge); + } + + // Validate every amount and accumulate the total in one pass so we + // pull a single lump sum from the funder instead of N separate auths. + let mut total: i128 = 0; + for amount in amounts.iter() { + if amount <= 0 { + return Err(Error::InvalidAmount); } + total = total.checked_add(amount).ok_or(Error::ArithmeticOverflow)?; + } - // ── State-version race-condition check (Issue #84) ────────── - // Snapshot the current version under the guard so there is no - // TOCTOU window between reading the version and acquiring the - // lock. Once the snapshot is taken, any external mutation that - // bumps the version before we finish will be detected and abort - // the commit. - let snapshot = load_state_version(&env); - bump_state_version(&env); - let current = load_state_version(&env); - if current != snapshot + 1 { - cleanup_stale_callbacks(&env); - return Err(Error::StateVersionMismatch); - } + // Empty batch — nothing to do. + if total == 0 { + return Ok(0); + } - let mut total: u64 = 0; - for amount in amounts.iter() { - match total.checked_add(amount) { - Some(new_total) => total = new_total, - None => { - cleanup_stale_callbacks(&env); - return Err(Error::CalculationOverflow); - } - } - } + // ── Auth ───────────────────────────────────────────────────────────── + funder.require_auth(); - // Final check: version must still match the expected value. - let final_version = load_state_version(&env); - if final_version != snapshot + 1 { - cleanup_stale_callbacks(&env); - return Err(Error::StateVersionMismatch); - } + // ── Token transfers ─────────────────────────────────────────────────── + let tk = token::Client::new(&env, &token); + let contract_addr = env.current_contract_address(); - // Advance the callback sequence so stale callbacks from prior - // interrupted batches are invalidated. - let cb_seq = load_callback_seq(&env); - env.storage() - .instance() - .set(&CALLBACK_SEQ_KEY, &(cb_seq + 1)); + // Pull the full batch total from the funder into this contract in one + // transfer, then fan out to each recipient individually. One inbound + // transfer keeps the auth surface minimal (funder signs once). + tk.transfer(&funder, &contract_addr, &total); - Ok(total) - }) - } -} - -/// Execute `f` under the re-entrancy guard, releasing the lock afterwards. -const MAX_REENTRANCY_DEPTH: u32 = 1; - -fn with_guard(env: &Env, f: impl FnOnce() -> Result) -> Result { - let lock_key = soroban_sdk::symbol_short!("B_Lock"); - let depth: u32 = env.storage().instance().get(&lock_key).unwrap_or(0); - if depth >= MAX_REENTRANCY_DEPTH { - return Err(Error::ProcessorLocked); - } - env.storage().instance().set(&lock_key, &(depth + 1)); - let result = f(); - // Fix Issue #83: use unwrap_or(0) instead of unwrap_or(1) to avoid - // falsely incrementing the lock when the storage entry is missing. - let d: u32 = env.storage().instance().get(&lock_key).unwrap_or(0); - if d > 0 { - env.storage().instance().set(&lock_key, &(d - 1)); - } - result -} - -/// Load the current state version from storage. -fn load_state_version(env: &Env) -> u64 { - env.storage() - .instance() - .get(&STATE_VERSION_KEY) - .unwrap_or(0) -} - -/// Increment the state version. -/// -/// Uses `checked_add` for consistency with the rest of the crate's -/// checked-arithmetic convention (see `process_batch`'s amount-summing loop). -fn bump_state_version(env: &Env) { - let v = load_state_version(env) - .checked_add(1) - .expect("state version counter overflowed u64"); - env.storage().instance().set(&STATE_VERSION_KEY, &v); -} - -/// Load the callback sequence counter. -fn load_callback_seq(env: &Env) -> u64 { - env.storage().instance().get(&CALLBACK_SEQ_KEY).unwrap_or(0) -} + for (recipient, amount) in recipients.iter().zip(amounts.iter()) { + tk.transfer(&contract_addr, &recipient, &amount); + } -/// Clean up any stale pending callbacks. -/// -/// Called on every error path to prevent orphaned callback state from -/// accumulating when an operation fails mid-execution. -fn cleanup_stale_callbacks(env: &Env) { - let seq = load_callback_seq(env); - if seq > 0 { - // Invalidate all pending callbacks by advancing the sequence. - env.storage().instance().set(&CALLBACK_SEQ_KEY, &(seq + 1)); + Ok(total) } }