diff --git a/contracts/escrow/src/lib.rs b/contracts/escrow/src/lib.rs index 82ccfaf..6231be9 100644 --- a/contracts/escrow/src/lib.rs +++ b/contracts/escrow/src/lib.rs @@ -232,12 +232,14 @@ impl Escrow { } let key = DataKey::Usage(agent.clone(), service_id.clone()); let prev: u32 = env.storage().persistent().get(&key).unwrap_or(0); + // saturate: settlement drains long before u32::MAX; never panic the hot path. let total = prev.saturating_add(requests); env.storage().persistent().set(&key, &total); // Cross-service lifetime counter for the agent. Saturates at u32::MAX. let total_key = DataKey::TotalUsageByAgent(agent.clone()); let prev_total: u32 = env.storage().persistent().get(&total_key).unwrap_or(0); + // saturate: settlement drains long before u32::MAX; never panic the hot path. env.storage() .persistent() .set(&total_key, &prev_total.saturating_add(requests)); @@ -248,6 +250,7 @@ impl Escrow { .persistent() .get(&DataKey::TotalRequestsAllTime) .unwrap_or(0); + // u64 horizon; saturate not panic. env.storage().persistent().set( &DataKey::TotalRequestsAllTime, &proto_prev.saturating_add(requests as u64), @@ -350,6 +353,8 @@ impl Escrow { .persistent() .get(&DataKey::ServicePrice(service_id)) .unwrap_or(0); + // saturate: read/settle path returns a sentinel-large value rather than + // panicking; off-chain loop treats saturation as an error signal. (requests as i128).saturating_mul(price) } @@ -377,6 +382,8 @@ impl Escrow { .persistent() .get(&DataKey::ServicePrice(service_id.clone())) .unwrap_or(0); + // saturate: read/settle path returns a sentinel-large value rather than + // panicking; off-chain loop treats saturation as an error signal. let billed = (requests as i128).saturating_mul(price); env.storage().persistent().set(&usage_key, &0u32); env.storage().persistent().set( diff --git a/contracts/escrow/src/test.rs b/contracts/escrow/src/test.rs index 2d924db..c0c30f6 100644 --- a/contracts/escrow/src/test.rs +++ b/contracts/escrow/src/test.rs @@ -697,3 +697,82 @@ fn test_pause_pause_unpause_ends_unpaused() { assert!(!client.is_paused()); } + +// --- Arithmetic overflow/saturation policy (see docs/escrow/arithmetic.md) --- + +#[test] +fn test_per_pair_usage_saturates_at_u32_max() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + + let agent = Address::generate(&env); + let service_id = Symbol::new(&env, "weather_api"); + + // record_usage takes a u32 delta, so reach the boundary across two calls: + // (u32::MAX - 1) then 5 more would overflow -> must clamp at u32::MAX. + client.record_usage(&agent, &service_id, &(u32::MAX - 1)); + let record = client.record_usage(&agent, &service_id, &5u32); + + assert_eq!(record.requests, u32::MAX); + assert_eq!(client.get_usage(&agent, &service_id), u32::MAX); +} + +#[test] +fn test_total_usage_by_agent_saturates_at_u32_max() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + + let agent = Address::generate(&env); + // Two distinct services so the per-pair counters do not themselves clamp + // before the per-agent lifetime counter does. + let svc_a = Symbol::new(&env, "svc_a"); + let svc_b = Symbol::new(&env, "svc_b"); + + client.record_usage(&agent, &svc_a, &(u32::MAX - 1)); + client.record_usage(&agent, &svc_b, &10u32); + + assert_eq!(client.get_total_usage_by_agent(&agent), u32::MAX); +} + +#[test] +fn test_compute_billing_saturates_at_i128_max() { + let env = Env::default(); + let (client, admin) = setup_initialized(&env); + let _ = &admin; + + let agent = Address::generate(&env); + let service_id = Symbol::new(&env, "expensive"); + + // Huge price; any positive usage makes requests*price overflow i128. + client.set_service_price(&service_id, &i128::MAX); + client.record_usage(&agent, &service_id, &2u32); + + assert_eq!(client.compute_billing(&agent, &service_id), i128::MAX); +} + +#[test] +fn test_compute_billing_zero_price_is_zero() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + + let agent = Address::generate(&env); + let service_id = Symbol::new(&env, "free_api"); + + // Zero price (free service): any usage bills to zero. + client.set_service_price(&service_id, &0i128); + client.record_usage(&agent, &service_id, &1000u32); + + assert_eq!(client.compute_billing(&agent, &service_id), 0); +} + +#[test] +fn test_settle_unused_pair_returns_zero() { + let env = Env::default(); + let (client, _admin) = setup_initialized(&env); + + let agent = Address::generate(&env); + let service_id = Symbol::new(&env, "never_used"); + + // No usage recorded and no price set: settle bills zero. + assert_eq!(client.settle(&agent, &service_id), 0); +} diff --git a/docs/escrow/arithmetic.md b/docs/escrow/arithmetic.md new file mode 100644 index 0000000..eee8e63 --- /dev/null +++ b/docs/escrow/arithmetic.md @@ -0,0 +1,73 @@ +# Escrow arithmetic: overflow & saturation policy + +This document records the deliberate integer-arithmetic strategy used by the +escrow contract (`contracts/escrow/src/lib.rs`). The policy is twofold: + +1. **All accumulator and billing math saturates** — it never wraps and never + panics the host on overflow. +2. **`overflow-checks = true` on the release profile** is a defense-in-depth + backstop: any *non-saturating* `+`/`*` that gets added later will panic + (trap) on overflow instead of silently wrapping. This is intentional — + wrapping is the one outcome we never want on-chain. + +## Why saturate instead of checked/panic? + +Two distinct call contexts share the same answer: + +- **Hot write path (`record_usage`).** This is the most frequently called + entrypoint. Panicking here would block an agent from recording legitimate + usage and could wedge the off-chain metering loop. The counters are designed + to be drained by `settle` long before any realistic overflow horizon, so the + safe failure mode is to clamp rather than to abort a state transition. +- **Read / settle path (`compute_billing`, `settle`).** These are read-mostly + and are consumed by an off-chain settlement loop. Returning a clamped + sentinel value lets that loop *detect* an anomaly and react, rather than + having the host trap and the loop receive no signal at all. + +## Saturating sites + +| Site | Expression | Type | Rationale | +| --- | --- | --- | --- | +| `record_usage` — per-pair counter | `prev.saturating_add(requests)` | `u32` | saturate: settlement drains long before `u32::MAX`; never panic the hot path | +| `record_usage` — `TotalUsageByAgent` | `prev_total.saturating_add(requests)` | `u32` | same: lifetime per-agent counter, clamp not panic | +| `record_usage` — `TotalRequestsAllTime` | `proto_prev.saturating_add(requests as u64)` | `u64` | `u64` horizon; saturate not panic | +| `compute_billing` | `(requests as i128).saturating_mul(price)` | `i128` | saturate: read path returns a sentinel-large value rather than panicking; off-chain loop treats saturation as an error signal | +| `settle` | `(requests as i128).saturating_mul(price)` | `i128` | same math as `compute_billing`; settle path returns sentinel rather than panicking | + +`price` is validated `>= 0` at `set_service_price`, and `requests` is a `u32`, +so the multiplicands are always non-negative and the only reachable boundary is +`i128::MAX`. + +## What a saturated value MEANS to downstream consumers + +A saturated value is **never** a silent wrap-around and **never** an ordinary +result. It is an out-of-band signal: + +- A per-pair / per-agent counter pinned at `u32::MAX` means usage has been + accumulating without ever being drained — i.e. the settlement loop is **stuck + or not running** for that pair. Settlement resets the per-pair counter to + zero, so a pinned counter is an accounting anomaly to investigate, not a bill + to charge. +- A billing value equal to `i128::MAX` means `requests * price` overflowed + `i128`. The off-chain settlement loop must treat this as an **error signal** + (stuck settlement / mis-configured price), not as a real amount to transfer. + +In all cases the contract has preserved a defined, monotonic, non-wrapping +value; correctness of the *response* to that value is the off-chain consumer's +responsibility. + +## Defense-in-depth backstop + +The release profile sets: + +```toml +[profile.release] +overflow-checks = true +``` + +This does not affect the saturating operations above (they cannot overflow by +construction). Its purpose is forward-looking: if a future change introduces a +plain `+` or `*` on a path that *can* overflow, the release build will trap +rather than wrap. Wrapping arithmetic on accounting state is the single outcome +this policy rules out; saturation is the chosen graceful behaviour and the +overflow check is the safety net for everything else.