From 9014b554b6a1c2f4d9f03150c8faa149dcc2e950 Mon Sep 17 00:00:00 2001 From: egwujiohaifesinachiperpetual-max Date: Thu, 28 May 2026 15:19:18 +0100 Subject: [PATCH] feat: validate amount/duration upper bounds in create_commitment --- contracts/README.md | 10 ++++++ contracts/escrow/src/lib.rs | 22 +++++++++--- contracts/escrow/src/test.rs | 69 +++++++++++++++++++++--------------- 3 files changed, 68 insertions(+), 33 deletions(-) diff --git a/contracts/README.md b/contracts/README.md index 356a1964..fa280314 100644 --- a/contracts/README.md +++ b/contracts/README.md @@ -53,6 +53,16 @@ create_commitment ──► fund_escrow ──► release (matured: p points (`penalty_bps`, max `10_000`) and is paid to the configured fee recipient on `refund` / adverse `resolve_dispute`. +### Commitment limits + +To prevent arithmetic overflow (e.g. during maturity timestamp calculations) and ensure input sanity, the following upper-bound limits are enforced in `create_commitment`: +- **Maximum Amount (`MAX_AMOUNT`)**: `1_000_000_000_000` (1T units) +- **Maximum Duration (`MAX_DURATION_DAYS`)**: `365` days (1 year) +- **Maximum Penalty (`MAX_PENALTY_BPS`)**: `10_000` bps (100%) + +Attempts to exceed these limits will return `InvalidAmount` or `InvalidDuration` errors, respectively. + + ### Errors Stable numeric error codes (`#[contracterror]`) are surfaced so the backend diff --git a/contracts/escrow/src/lib.rs b/contracts/escrow/src/lib.rs index 070cbf20..76bb0d01 100644 --- a/contracts/escrow/src/lib.rs +++ b/contracts/escrow/src/lib.rs @@ -19,12 +19,19 @@ use soroban_sdk::{ }; // Configuration constants for escrow contract +// Configuration constants for escrow contract +// Number of seconds in a day used for maturity calculation. const SECONDS_PER_DAY: u64 = 86_400; -// Maximum allowed commitment amount (example limit) + +/// Upper bound for commitment amount enforced by `create_commitment`. +/// Aligns with backend `CommitmentLimits.max_amount`. const MAX_AMOUNT: i128 = 1_000_000_000_000; -// Maximum allowed duration in days + +/// Upper bound for commitment duration (in days) enforced by `create_commitment`. +/// Aligns with backend `CommitmentLimits.max_duration_days`. const MAX_DURATION_DAYS: u32 = 365; -// Maximum penalty basis points (100% = 10_000 bps) + +/// Upper bound for penalty basis points (10_000 = 100%). const MAX_PENALTY_BPS: u32 = 10_000; /// Storage keys for persistent contract state. @@ -137,9 +144,14 @@ impl EscrowContract { /// Create a new (unfunded) commitment escrow. Returns the new commitment id. /// + /// Validates input against upper bounds defined by backend `CommitmentLimits`: + /// * `amount` must be > 0 and <= `MAX_AMOUNT`. + /// * `duration_days` must be > 0 and <= `MAX_DURATION_DAYS`. + /// * `penalty_bps` must be <= `MAX_PENALTY_BPS`. + /// /// `duration_days` is converted to an absolute maturity timestamp using the - /// current ledger time. `penalty_bps` is the early-exit penalty applied on - /// `refund`. + /// current ledger time with checked arithmetic to avoid overflow. `penalty_bps` + /// is the early-exit penalty applied on `refund`. pub fn create_commitment( env: Env, owner: Address, diff --git a/contracts/escrow/src/test.rs b/contracts/escrow/src/test.rs index f778aba2..fc680253 100644 --- a/contracts/escrow/src/test.rs +++ b/contracts/escrow/src/test.rs @@ -165,6 +165,18 @@ fn create_rejects_invalid_amount() { assert_eq!(res, Err(Ok(Error::InvalidAmount))); } +#[test] +fn create_rejects_overflow_duration() { + let f = setup(); + // Set timestamp close to max to cause overflow when adding duration + f.env.ledger().set_timestamp(u64::MAX - 10); + let owner = Address::generate(&f.env); + fund_owner(&f, &owner, 1_000); + // Use a duration that will overflow when added to current timestamp + let res = f.client.try_create_commitment(&owner, &f.asset, &1_000, &RiskProfile::Safe, &10u32, &2000u32); + assert_eq!(res, Err(Ok(Error::InvalidDuration))); +} + #[test] fn create_rejects_excessive_penalty() { let f = setup(); @@ -206,34 +218,35 @@ fn owner_index_tracks_commitments() { assert_eq!(ids.len(), 2); assert_eq!(ids.get(0).unwrap(), a); assert_eq!(ids.get(1).unwrap(), b); +} - #[test] - fn create_rejects_excessive_amount() { - let f = setup(); - let owner = Address::generate(&f.env); - let res = f.client.try_create_commitment( - &owner, - &f.asset, - &(MAX_AMOUNT + 1), - &RiskProfile::Safe, - &(MAX_DURATION_DAYS + 1), - &2000, - ); - assert_eq!(res, Err(Ok(Error::InvalidAmount))); - } +#[test] +fn create_rejects_excessive_amount() { + let f = setup(); + let owner = Address::generate(&f.env); + let res = f.client.try_create_commitment( + &owner, + &f.asset, + &(MAX_AMOUNT + 1), + &RiskProfile::Safe, + &30, + &2000, + ); + assert_eq!(res, Err(Ok(Error::InvalidAmount))); +} - #[test] - fn create_rejects_excessive_duration() { - let f = setup(); - let owner = Address::generate(&f.env); - let res = f.client.try_create_commitment( - &owner, - &f.asset, - &1_000, - &RiskProfile::Safe, - &(MAX_DURATION_DAYS + 1), - &2000, - ); - assert_eq!(res, Err(Ok(Error::InvalidDuration))); - } +#[test] +fn create_rejects_excessive_duration() { + let f = setup(); + let owner = Address::generate(&f.env); + let res = f.client.try_create_commitment( + &owner, + &f.asset, + &1_000, + &RiskProfile::Safe, + &(MAX_DURATION_DAYS + 1), + &2000, + ); + assert_eq!(res, Err(Ok(Error::InvalidDuration))); } +