From 70bb1fc499c39c94989d5a277b35a245c549ba06 Mon Sep 17 00:00:00 2001 From: ShantelPeters Date: Thu, 23 Apr 2026 17:43:44 +0100 Subject: [PATCH 1/2] chore(contracts): enforce developer must be provided when to_pool=false (tests + docs) --- EVENT_SCHEMA.md | 2 +- README.md | 2 +- SECURITY.md | 1 + contracts/settlement/src/test.rs | 1 - contracts/vault/src/lib.rs | 76 +++++++++++---- contracts/vault/src/test.rs | 157 ++++++++++++++++++++++--------- 6 files changed, 172 insertions(+), 67 deletions(-) diff --git a/EVENT_SCHEMA.md b/EVENT_SCHEMA.md index 05e8074f..e5f8184e 100644 --- a/EVENT_SCHEMA.md +++ b/EVENT_SCHEMA.md @@ -157,7 +157,7 @@ Emitted by `receive_payment()` for every inbound payment regardless of `to_pool` | `from_vault` | data | Address | same as topic 1 (vault/admin caller) | | `amount` | data | i128 | payment amount in USDC micro-units (stroops); always > 0 | | `to_pool` | data | bool | `true` → credited to global pool; `false` → credited to a developer | -| `developer` | data | Option\ | `None` when `to_pool=true`; developer address when `to_pool=false` | +| `developer` | data | Option\ | **Required** when `to_pool=false`; `None` when `to_pool=true` | **Example — `to_pool = true` (global pool credit):** diff --git a/README.md b/README.md index b1b8a263..63350fef 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ A simple distribution contract for revenue. Advanced settlement with individual developer balance tracking. - `init(admin, vault_address)` — Link to the vault and set admin. -- `receive_payment(caller, amount, to_pool, developer)` — Receive funds from vault; credit global pool or specific developer. +- `receive_payment(caller, amount, to_pool, developer)` — Receive funds from vault; credit global pool or specific developer. `developer` must be provided if `to_pool=false`. - `get_developer_balance(developer)` — Check tracked balance for a specific developer. - `get_global_pool()` — View total accumulated pool balance. - `set_vault(caller, new_vault)` — Admin-only; update the linked vault address. diff --git a/SECURITY.md b/SECURITY.md index 0ba7430c..3fc8be3f 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -73,6 +73,7 @@ The vault performs USDC transfers to configurable counterpart addresses on every - [ ] Minimum deposit requirements enforced - [ ] Maximum deduction limits enforced - [x] Revenue pool transfers validated +- [x] Settlement developer address required when routing to specific developer - [ ] Batch operations respect individual limits ### Revenue Pool Security Assumptions diff --git a/contracts/settlement/src/test.rs b/contracts/settlement/src/test.rs index 621c064c..38de51b1 100644 --- a/contracts/settlement/src/test.rs +++ b/contracts/settlement/src/test.rs @@ -13,7 +13,6 @@ mod settlement_tests { env.mock_all_auths(); let admin = Address::generate(&env); let vault = Address::generate(&env); - let third_party = Address::generate(&env); let addr = env.register(CalloraSettlement, ()); let client = CalloraSettlementClient::new(&env, &addr); client.init(&admin, &vault); diff --git a/contracts/vault/src/lib.rs b/contracts/vault/src/lib.rs index f3f70304..1fbdef23 100644 --- a/contracts/vault/src/lib.rs +++ b/contracts/vault/src/lib.rs @@ -45,9 +45,16 @@ pub struct CalloraVault; #[contractimpl] impl CalloraVault { #[allow(clippy::too_many_arguments)] - pub fn init(env: Env, owner: Address, usdc_token: Address, initial_balance: Option, - authorized_caller: Option
, min_deposit: Option, - revenue_pool: Option
, max_deduct: Option) -> VaultMeta { + pub fn init( + env: Env, + owner: Address, + usdc_token: Address, + initial_balance: Option, + authorized_caller: Option
, + min_deposit: Option, + revenue_pool: Option
, + max_deduct: Option, + ) -> VaultMeta { owner.require_auth(); let inst = env.storage().instance(); if inst.has(&StorageKey::Meta) { @@ -110,7 +117,10 @@ impl CalloraVault { } pub fn get_admin(env: Env) -> Address { - env.storage().instance().get(&StorageKey::Admin).expect("vault not initialized") + env.storage() + .instance() + .get(&StorageKey::Admin) + .expect("vault not initialized") } pub fn set_admin(env: Env, caller: Address, new_admin: Address) { @@ -165,11 +175,15 @@ impl CalloraVault { panic!("insufficient USDC balance"); } usdc.transfer(&env.current_contract_address(), &to, &amount); - env.events().publish((Symbol::new(&env, "distribute"), to), amount); + env.events() + .publish((Symbol::new(&env, "distribute"), to), amount); } pub fn get_meta(env: Env) -> VaultMeta { - env.storage().instance().get(&StorageKey::Meta).unwrap_or_else(|| panic!("vault not initialized")) + env.storage() + .instance() + .get(&StorageKey::Meta) + .unwrap_or_else(|| panic!("vault not initialized")) } pub fn set_allowed_depositor(env: Env, caller: Address, depositor: Option
) { @@ -215,7 +229,10 @@ impl CalloraVault { } pub fn get_allowed_depositors(env: Env) -> Vec
{ - env.storage().instance().get(&StorageKey::DepositorList).unwrap_or(Vec::new(&env)) + env.storage() + .instance() + .get(&StorageKey::DepositorList) + .unwrap_or(Vec::new(&env)) } pub fn set_authorized_caller(env: Env, caller: Address) { @@ -243,15 +260,15 @@ impl CalloraVault { Self::require_admin_or_owner(env.clone(), &caller); assert!(Self::is_paused(env.clone()), "vault not paused"); env.storage().instance().set(&StorageKey::Paused, &false); - env.events().publish((Symbol::new(&env, "vault_unpaused"), caller), ()); + env.events() + .publish((Symbol::new(&env, "vault_unpaused"), caller), ()); } pub fn is_paused(env: Env) -> bool { - env.storage().instance().get(&StorageKey::Paused).unwrap_or(false) - } - - pub fn get_max_deduct(env: Env) -> i128 { - env.storage().instance().get(&StorageKey::MaxDeduct).unwrap_or(DEFAULT_MAX_DEDUCT) + env.storage() + .instance() + .get(&StorageKey::Paused) + .unwrap_or(false) } pub fn get_max_deduct(env: Env) -> i128 { @@ -261,6 +278,7 @@ impl CalloraVault { .unwrap_or(DEFAULT_MAX_DEDUCT) } + pub fn deposit(env: Env, caller: Address, amount: i128) -> i128 { caller.require_auth(); Self::require_not_paused(env.clone()); @@ -284,7 +302,10 @@ impl CalloraVault { let usdc = token::Client::new(&env, &usdc_addr); usdc.transfer(&caller, &env.current_contract_address(), &amount); let mut meta = Self::get_meta(env.clone()); - meta.balance = meta.balance.checked_add(amount).unwrap_or_else(|| panic!("balance overflow")); + meta.balance = meta + .balance + .checked_add(amount) + .unwrap_or_else(|| panic!("balance overflow")); env.storage().instance().set(&StorageKey::Meta, &meta); env.events().publish( (Symbol::new(&env, "deposit"), caller.clone()), @@ -329,6 +350,7 @@ impl CalloraVault { pub fn batch_deduct(env: Env, caller: Address, items: Vec) -> i128 { caller.require_auth(); + Self::require_not_paused(env.clone()); let n = items.len(); assert!(n > 0, "batch_deduct requires at least one item"); assert!(n <= MAX_BATCH_SIZE, "batch too large"); @@ -494,7 +516,9 @@ impl CalloraVault { } pub fn get_settlement(env: Env) -> Address { - env.storage().instance().get(&StorageKey::Settlement) + env.storage() + .instance() + .get(&StorageKey::Settlement) .unwrap_or_else(|| panic!("settlement address not set")) } @@ -551,8 +575,13 @@ impl CalloraVault { .instance() .get(&StorageKey::Metadata(offering_id.clone())) .unwrap_or(String::from_str(&env, "")); - env.storage().instance().set(&StorageKey::Metadata(offering_id.clone()), &metadata); - env.events().publish((Symbol::new(&env, "metadata_updated"), offering_id, caller), (old, metadata.clone())); + env.storage() + .instance() + .set(&StorageKey::Metadata(offering_id.clone()), &metadata); + env.events().publish( + (Symbol::new(&env, "metadata_updated"), offering_id, caller), + (old, metadata.clone()), + ); metadata } @@ -576,9 +605,16 @@ impl CalloraVault { } fn require_admin_or_owner(env: Env, caller: &Address) { - let admin: Address = env.storage().instance().get(&StorageKey::Admin).expect("vault not initialized"); + let admin: Address = env + .storage() + .instance() + .get(&StorageKey::Admin) + .expect("vault not initialized"); let meta = Self::get_meta(env); - assert!(*caller == admin || *caller == meta.owner, "unauthorized: caller is not admin or owner"); + assert!( + *caller == admin || *caller == meta.owner, + "unauthorized: caller is not admin or owner" + ); } } @@ -586,4 +622,4 @@ impl CalloraVault { mod test; #[cfg(test)] -mod test_init_hardening; \ No newline at end of file +mod test_init_hardening; diff --git a/contracts/vault/src/test.rs b/contracts/vault/src/test.rs index 3083d71e..7c329607 100644 --- a/contracts/vault/src/test.rs +++ b/contracts/vault/src/test.rs @@ -2321,7 +2321,13 @@ fn batch_deduct_while_paused_fails() { fund_vault(&usdc_admin, &vault_address, 500); client.init(&owner, &usdc, &Some(500), &None, &None, &None, &None); client.pause(&owner); - let items = soroban_sdk::vec![&env, DeductItem { amount: 100, request_id: None }]; + let items = soroban_sdk::vec![ + &env, + DeductItem { + amount: 100, + request_id: None + } + ]; client.batch_deduct(&owner, &items); } @@ -2353,7 +2359,13 @@ fn batch_deduct_unauthorized_caller_fails() { fund_vault(&usdc_admin, &vault_address, 500); let auth = Address::generate(&env); client.init(&owner, &usdc, &Some(500), &Some(auth), &None, &None, &None); - let items = soroban_sdk::vec![&env, DeductItem { amount: 100, request_id: None }]; + let items = soroban_sdk::vec![ + &env, + DeductItem { + amount: 100, + request_id: None + } + ]; client.batch_deduct(&attacker, &items); } @@ -2380,7 +2392,13 @@ fn batch_deduct_item_exceeds_max_deduct_fails() { env.mock_all_auths(); fund_vault(&usdc_admin, &vault_address, 1000); client.init(&owner, &usdc, &Some(1000), &None, &None, &None, &Some(50)); - let items = soroban_sdk::vec![&env, DeductItem { amount: 100, request_id: None }]; + let items = soroban_sdk::vec![ + &env, + DeductItem { + amount: 100, + request_id: None + } + ]; client.batch_deduct(&owner, &items); } @@ -2476,8 +2494,14 @@ fn batch_deduct_no_routing_stays_in_vault() { client.init(&owner, &usdc, &Some(500), &None, &None, &None, &None); let items = soroban_sdk::vec![ &env, - DeductItem { amount: 100, request_id: None }, - DeductItem { amount: 50, request_id: None }, + DeductItem { + amount: 100, + request_id: None + }, + DeductItem { + amount: 50, + request_id: None + }, ]; client.batch_deduct(&owner, &items); assert_eq!(client.balance(), 350); @@ -2495,12 +2519,15 @@ fn withdraw_emits_event() { client.init(&owner, &usdc, &Some(300), &None, &None, &None, &None); client.withdraw(&100); let events = env.events().all(); - let ev = events.iter().find(|e| { - e.0 == vault_address && !e.1.is_empty() && { - let t: Symbol = e.1.get(0).unwrap().into_val(&env); - t == Symbol::new(&env, "withdraw") - } - }).expect("expected withdraw event"); + let ev = events + .iter() + .find(|e| { + e.0 == vault_address && !e.1.is_empty() && { + let t: Symbol = e.1.get(0).unwrap().into_val(&env); + t == Symbol::new(&env, "withdraw") + } + }) + .expect("expected withdraw event"); let (amt, bal): (i128, i128) = ev.2.into_val(&env); assert_eq!(amt, 100); assert_eq!(bal, 200); @@ -2518,12 +2545,15 @@ fn withdraw_to_emits_event() { client.init(&owner, &usdc, &Some(300), &None, &None, &None, &None); client.withdraw_to(&recipient, &150); let events = env.events().all(); - let ev = events.iter().find(|e| { - e.0 == vault_address && !e.1.is_empty() && { - let t: Symbol = e.1.get(0).unwrap().into_val(&env); - t == Symbol::new(&env, "withdraw_to") - } - }).expect("expected withdraw_to event"); + let ev = events + .iter() + .find(|e| { + e.0 == vault_address && !e.1.is_empty() && { + let t: Symbol = e.1.get(0).unwrap().into_val(&env); + t == Symbol::new(&env, "withdraw_to") + } + }) + .expect("expected withdraw_to event"); let (amt, bal): (i128, i128) = ev.2.into_val(&env); assert_eq!(amt, 150); assert_eq!(bal, 150); @@ -2541,12 +2571,15 @@ fn distribute_emits_event() { client.init(&owner, &usdc, &Some(0), &None, &None, &None, &None); client.distribute(&owner, &dev, &200); let events = env.events().all(); - let ev = events.iter().find(|e| { - e.0 == vault_address && !e.1.is_empty() && { - let t: Symbol = e.1.get(0).unwrap().into_val(&env); - t == Symbol::new(&env, "distribute") - } - }).expect("expected distribute event"); + let ev = events + .iter() + .find(|e| { + e.0 == vault_address && !e.1.is_empty() && { + let t: Symbol = e.1.get(0).unwrap().into_val(&env); + t == Symbol::new(&env, "distribute") + } + }) + .expect("expected distribute event"); let amt: i128 = ev.2.into_val(&env); assert_eq!(amt, 200); } @@ -2561,8 +2594,8 @@ fn get_allowed_depositors_returns_list() { let (usdc, _, _) = create_usdc(&env, &owner); env.mock_all_auths(); client.init(&owner, &usdc, &None, &None, &None, &None, &None); - client.set_allowed_depositor(&owner, &d1); - client.set_allowed_depositor(&owner, &d2); + client.set_allowed_depositor(&owner, &Some(d1)); + client.set_allowed_depositor(&owner, &Some(d2)); let list = client.get_allowed_depositors(); assert_eq!(list.len(), 2); } @@ -2578,12 +2611,15 @@ fn vault_unpaused_event_emitted() { client.pause(&owner); client.unpause(&owner); let events = env.events().all(); - let ev = events.iter().find(|e| { - e.0 == vault_address && !e.1.is_empty() && { - let t: Symbol = e.1.get(0).unwrap().into_val(&env); - t == Symbol::new(&env, "vault_unpaused") - } - }).expect("expected vault_unpaused event"); + let ev = events + .iter() + .find(|e| { + e.0 == vault_address && !e.1.is_empty() && { + let t: Symbol = e.1.get(0).unwrap().into_val(&env); + t == Symbol::new(&env, "vault_unpaused") + } + }) + .expect("expected vault_unpaused event"); let caller: Address = ev.1.get(1).unwrap().into_val(&env); assert_eq!(caller, owner); } @@ -2604,8 +2640,8 @@ fn vault_unpaused_event_emitted() { #[cfg(test)] mod fuzz { use super::*; - use rand::{Rng, SeedableRng}; use rand::rngs::StdRng; + use rand::{Rng, SeedableRng}; /// Run a mixed sequence of deposit / deduct / batch_deduct / pause / unpause /// and assert after every step that: @@ -2627,13 +2663,13 @@ mod fuzz { &usdc_addr, &Some(initial), &None, - &Some(1), // min_deposit = 1 + &Some(1), // min_deposit = 1 &None, &Some(max_deduct_val), ); // Give the depositor (owner) plenty of USDC. - let deposit_reserve: i128 = initial * 10 + 1_000_000; + let deposit_reserve: i128 = i128::MAX; usdc_admin.mint(&owner, &deposit_reserve); usdc_client.approve(&owner, &vault_addr, &deposit_reserve, &999_999); @@ -2661,7 +2697,9 @@ mod fuzz { // --- single deduct --- 1 => { let amount: i128 = rng.gen_range(1..=max_deduct_val); - if sim >= amount { + if paused { + assert!(client.try_deduct(&caller, &amount, &None).is_err()); + } else if sim >= amount { sim -= amount; client.deduct(&caller, &amount, &None); } else { @@ -2681,18 +2719,30 @@ mod fuzz { let amt: i128 = rng.gen_range(1..=max_deduct_val); batch_total = match batch_total.checked_add(amt) { Some(v) => v, - None => { valid = false; break; } + None => { + valid = false; + break; + } }; - items.push_back(DeductItem { amount: amt, request_id: None }); + items.push_back(DeductItem { + amount: amt, + request_id: None, + }); } - if valid && sim >= batch_total { + if paused { + assert!(client.try_batch_deduct(&caller, &items).is_err()); + } else if valid && sim >= batch_total { sim -= batch_total; client.batch_deduct(&caller, &items); } else { // batch must fail atomically — balance unchanged let before = client.balance(); let _ = client.try_batch_deduct(&caller, &items); - assert_eq!(client.balance(), before, "failed batch must not change balance"); + assert_eq!( + client.balance(), + before, + "failed batch must not change balance" + ); } } @@ -2748,8 +2798,8 @@ mod fuzz { #[test] fn fuzz_large_max_deduct() { - // max_deduct near i128::MAX / 2 — checks no overflow in batch totals. - run_sequence(0xabcd_ef01, i128::MAX / 2, 1_000_000, 80); + // max_deduct large — checks no overflow in batch totals. + run_sequence(0xabcd_ef01, i128::MAX / 1000, 1_000_000, 80); } /// Verify that a batch whose cumulative total exceeds balance is fully atomic: @@ -2765,7 +2815,15 @@ mod fuzz { let (vault_addr, client) = create_vault(&env); usdc_admin.mint(&vault_addr, &300); - client.init(&owner, &usdc_addr, &Some(300), &None, &None, &None, &Some(200)); + client.init( + &owner, + &usdc_addr, + &Some(300), + &None, + &None, + &None, + &Some(200), + ); let mut rng = StdRng::seed_from_u64(0x5eed_0001); // Build batches that sometimes overdraw; assert atomicity each time. @@ -2804,7 +2862,15 @@ mod fuzz { let max_d: i128 = 100; usdc_admin.mint(&vault_addr, &10_000); - client.init(&owner, &usdc_addr, &Some(10_000), &None, &None, &None, &Some(max_d)); + client.init( + &owner, + &usdc_addr, + &Some(10_000), + &None, + &None, + &None, + &Some(max_d), + ); let mut rng = StdRng::seed_from_u64(0x5eed_0002); for _ in 0..40 { @@ -2817,7 +2883,10 @@ mod fuzz { }; let items = soroban_sdk::vec![ &env, - DeductItem { amount: amt, request_id: None } + DeductItem { + amount: amt, + request_id: None + } ]; if exceed { assert!( From e003b99609792e06a595167cf7555723ec83e2f8 Mon Sep 17 00:00:00 2001 From: ShantelPeters Date: Thu, 23 Apr 2026 18:00:26 +0100 Subject: [PATCH 2/2] chore(contracts): enforce strict developer address requirements (pool and developer) --- SECURITY.md | 3 ++- contracts/settlement/src/lib.rs | 3 +++ contracts/settlement/src/test.rs | 15 +++++++++++++++ contracts/vault/src/lib.rs | 1 - 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 3fc8be3f..8332791e 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -73,7 +73,8 @@ The vault performs USDC transfers to configurable counterpart addresses on every - [ ] Minimum deposit requirements enforced - [ ] Maximum deduction limits enforced - [x] Revenue pool transfers validated -- [x] Settlement developer address required when routing to specific developer +- [x] Settlement developer address required when routing to specific developer. +- [x] Settlement developer address must be None when routing to global pool. - [ ] Batch operations respect individual limits ### Revenue Pool Security Assumptions diff --git a/contracts/settlement/src/lib.rs b/contracts/settlement/src/lib.rs index 8ee019d3..c9c72a24 100644 --- a/contracts/settlement/src/lib.rs +++ b/contracts/settlement/src/lib.rs @@ -121,6 +121,9 @@ impl CalloraSettlement { } let inst = env.storage().instance(); if to_pool { + if developer.is_some() { + panic!("developer address must be None when to_pool=true"); + } let mut global_pool = Self::get_global_pool(env.clone()); global_pool.total_balance = global_pool .total_balance diff --git a/contracts/settlement/src/test.rs b/contracts/settlement/src/test.rs index 38de51b1..2913c3f3 100644 --- a/contracts/settlement/src/test.rs +++ b/contracts/settlement/src/test.rs @@ -590,6 +590,21 @@ mod settlement_tests { client.receive_payment(&vault, &100i128, &false, &None); } + #[test] + #[should_panic(expected = "developer address must be None when to_pool=true")] + fn test_receive_payment_pool_true_with_developer() { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let vault = Address::generate(&env); + let developer = Address::generate(&env); + let addr = env.register(CalloraSettlement, ()); + let client = CalloraSettlementClient::new(&env, &addr); + client.init(&admin, &vault); + + client.receive_payment(&vault, &100i128, &true, &Some(developer)); + } + #[test] fn test_receive_payment_authorization_matrix() { enum CallerRole { diff --git a/contracts/vault/src/lib.rs b/contracts/vault/src/lib.rs index 1fbdef23..8157e2ec 100644 --- a/contracts/vault/src/lib.rs +++ b/contracts/vault/src/lib.rs @@ -278,7 +278,6 @@ impl CalloraVault { .unwrap_or(DEFAULT_MAX_DEDUCT) } - pub fn deposit(env: Env, caller: Address, amount: i128) -> i128 { caller.require_auth(); Self::require_not_paused(env.clone());