From b712d067e1ba7c661b0922d1f829c21c6d205861 Mon Sep 17 00:00:00 2001 From: okekefrancis112 Date: Thu, 28 May 2026 03:49:35 +0100 Subject: [PATCH] feat: add two-step vault rotation to settlement Introduce propose_vault/accept_vault with PendingVault storage and vault_proposed/vault_accepted events, preventing accidental vault misrouting. Co-authored-by: Cursor --- contracts/settlement/src/lib.rs | 155 ++++++++++++++++++++++--------- contracts/settlement/src/test.rs | 140 +++++++++++++++++++++++----- docs/ACCESS_CONTROL.md | 9 +- 3 files changed, 236 insertions(+), 68 deletions(-) diff --git a/contracts/settlement/src/lib.rs b/contracts/settlement/src/lib.rs index e18826a..7f7d248 100644 --- a/contracts/settlement/src/lib.rs +++ b/contracts/settlement/src/lib.rs @@ -2,6 +2,9 @@ use soroban_sdk::{contract, contractimpl, contracttype, Address, Env, Symbol, Vec}; +/// Maximum number of items accepted by `batch_receive_payment`. +pub const MAX_BATCH_SIZE: u32 = 50; + /// Persistent storage keys for settlement contract #[contracttype] #[derive(Clone, Debug, PartialEq)] @@ -9,6 +12,7 @@ pub enum StorageKey { Admin, Vault, PendingAdmin, + PendingVault, DeveloperIndex, DeveloperBalance(Address), GlobalPool, @@ -56,22 +60,23 @@ pub struct BalanceCreditedEvent { pub new_balance: i128, } -/// Emitted when the registered vault address is changed via `set_vault()`. +/// Emitted when a new vault address is proposed via `propose_vault()`. #[contracttype] #[derive(Clone, Debug, PartialEq)] -pub struct VaultChangedEvent { +pub struct VaultProposedEvent { + pub current_vault: Address, + pub proposed_vault: Address, +} + +/// Emitted when the proposed vault is accepted via `accept_vault()`. +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub struct VaultAcceptedEvent { pub old_vault: Address, pub new_vault: Address, + pub accepted_by: Address, } -/// Storage key for the registered vault address. -const VAULT_KEY: &str = "vault"; -/// Storage key for the admin address. -const ADMIN_KEY: &str = "admin"; -const PENDING_ADMIN_KEY: &str = "pending_admin"; -const DEVELOPER_BALANCES_KEY: &str = "developer_balances"; -/// Storage key for the global pool state. -const GLOBAL_POOL_KEY: &str = "global_pool"; #[contract] pub struct CalloraSettlement; @@ -109,10 +114,10 @@ impl CalloraSettlement { if vault_address == env.current_contract_address() { panic!("invalid config: vault_address cannot be the contract itself"); } - inst.set(&Symbol::new(&env, ADMIN_KEY), &admin); - inst.set(&Symbol::new(&env, VAULT_KEY), &vault_address); - let empty_balances: Map = Map::new(&env); - inst.set(&Symbol::new(&env, DEVELOPER_BALANCES_KEY), &empty_balances); + inst.set(&StorageKey::Admin, &admin); + inst.set(&StorageKey::Vault, &vault_address); + let empty_index: Vec
= Vec::new(&env); + inst.set(&StorageKey::DeveloperIndex, &empty_index); let global_pool = GlobalPool { total_balance: 0, last_updated: env.ledger().timestamp(), @@ -186,11 +191,11 @@ impl CalloraSettlement { // Read current balance from persistent storage - let current_balance = env + let current_balance: i128 = env .storage() .persistent() .get(&StorageKey::DeveloperBalance(dev_address.clone())) - .unwrap_or(0); + .unwrap_or(0i128); let new_balance = current_balance .checked_add(amount) .unwrap_or_else(|| panic!("developer balance overflow")); @@ -209,7 +214,7 @@ impl CalloraSettlement { let mut index: Vec
= inst .get(&StorageKey::DeveloperIndex) .unwrap_or_else(|| Vec::new(&env)); - if !index.iter().any(|addr| addr == &dev_address) { + if !index.iter().any(|addr| addr == dev_address) { index.push_back(dev_address.clone()); inst.set(&StorageKey::DeveloperIndex, &index); } @@ -274,17 +279,33 @@ impl CalloraSettlement { } let inst = env.storage().instance(); - let mut balances: Map = inst - .get(&Symbol::new(&env, DEVELOPER_BALANCES_KEY)) - .unwrap_or_else(|| Map::new(&env)); - for item in items.iter() { let (dev, amount) = item; - let current = balances.get(dev.clone()).unwrap_or(0); - let new_balance = current + let current: i128 = env + .storage() + .persistent() + .get(&StorageKey::DeveloperBalance(dev.clone())) + .unwrap_or(0i128); + let new_balance: i128 = current .checked_add(amount) .unwrap_or_else(|| panic!("developer balance overflow")); - balances.set(dev.clone(), new_balance); + + env.storage() + .persistent() + .set(&StorageKey::DeveloperBalance(dev.clone()), &new_balance); + env.storage() + .persistent() + .extend_ttl(&StorageKey::DeveloperBalance(dev.clone()), 50000, 50000); + + // Add developer to index if not already present + let mut index: Vec
= inst + .get(&StorageKey::DeveloperIndex) + .unwrap_or_else(|| Vec::new(&env)); + if !index.iter().any(|addr| addr == dev) { + index.push_back(dev.clone()); + inst.set(&StorageKey::DeveloperIndex, &index); + } + env.events().publish( (Symbol::new(&env, "balance_credited"), dev.clone()), BalanceCreditedEvent { @@ -294,8 +315,6 @@ impl CalloraSettlement { }, ); } - - inst.set(&Symbol::new(&env, DEVELOPER_BALANCES_KEY), &balances); } /// Get current admin address @@ -392,11 +411,12 @@ impl CalloraSettlement { let mut result = Vec::new(&env); for address in index.iter() { - let balance = env + let address_key = address.clone(); + let balance: i128 = env .storage() .persistent() - .get(&StorageKey::DeveloperBalance(address)) - .unwrap_or(0); + .get(&StorageKey::DeveloperBalance(address_key)) + .unwrap_or(0i128); result.push_back(DeveloperBalance { address: address.clone(), balance, @@ -478,7 +498,7 @@ impl CalloraSettlement { .publish((Symbol::new(&env, "admin_accepted"), current, pending), ()); } - /// Update vault address (admin only). + /// Propose a new vault address (admin only). /// /// # Arguments /// * `caller` - Current admin address; must match stored admin @@ -487,32 +507,83 @@ impl CalloraSettlement { /// # Access Control /// Only the current admin can call this function. /// + pub fn set_vault(env: Env, caller: Address, new_vault: Address) { + // Backwards-compatible alias: `set_vault` now behaves like `propose_vault`. + Self::propose_vault(env, caller, new_vault); + } + + /// Propose a new vault address (admin only). + /// + /// This is the first step of a two-step vault rotation: + /// 1. Admin calls `propose_vault()` to set `PendingVault` + /// 2. Proposed vault (or admin) calls `accept_vault()` to activate it + /// /// # Security - /// The vault address controls which contract can send payments to - /// the settlement contract. Only trusted addresses should be set. - /// Changing the vault address immediately revokes access from the - /// old vault, so coordinate carefully during migrations. + /// This prevents a typo from instantly routing settlement credits to the wrong contract. /// /// # Events - /// Emits `vault_changed` event with the old and new vault addresses. + /// Emits `vault_proposed` with current and proposed vault addresses. /// /// # Panics - /// Panics if caller is not the current admin. - pub fn set_vault(env: Env, caller: Address, new_vault: Address) { + /// - `"unauthorized: caller is not admin"` if caller is not admin + /// - `"invalid config: vault cannot be the contract itself"` if proposed vault is this contract + pub fn propose_vault(env: Env, caller: Address, new_vault: Address) { caller.require_auth(); let current_admin = Self::get_admin(env.clone()); if caller != current_admin { panic!("unauthorized: caller is not admin"); } + if new_vault == env.current_contract_address() { + panic!("invalid config: vault cannot be the contract itself"); + } + let inst = env.storage().instance(); + let current_vault = Self::get_vault(env.clone()); + inst.set(&StorageKey::PendingVault, &new_vault); + + env.events().publish( + (Symbol::new(&env, "vault_proposed"), caller), + VaultProposedEvent { + current_vault, + proposed_vault: new_vault, + }, + ); + } + + /// Accept the proposed vault and activate it. + /// + /// # Arguments + /// * `caller` - Must be either the proposed vault address or the admin. + /// + /// # Events + /// Emits `vault_accepted` with old vault, new vault, and acceptor. + /// + /// # Panics + /// - `"no vault rotation pending"` if no `propose_vault()` was called + /// - `"unauthorized: caller must be pending vault or admin"` if caller is neither + pub fn accept_vault(env: Env, caller: Address) { + caller.require_auth(); + + let inst = env.storage().instance(); + let pending: Address = inst + .get(&StorageKey::PendingVault) + .unwrap_or_else(|| panic!("no vault rotation pending")); + + let admin = Self::get_admin(env.clone()); + if caller != pending && caller != admin { + panic!("unauthorized: caller must be pending vault or admin"); + } + let old_vault = Self::get_vault(env.clone()); - inst.set(&Symbol::new(&env, VAULT_KEY), &new_vault); + inst.set(&StorageKey::Vault, &pending); + inst.remove(&StorageKey::PendingVault); env.events().publish( - (Symbol::new(&env, "vault_changed"), caller.clone()), - VaultChangedEvent { - old_vault: old_vault.clone(), - new_vault: new_vault.clone(), + (Symbol::new(&env, "vault_accepted"), caller.clone()), + VaultAcceptedEvent { + old_vault, + new_vault: pending, + accepted_by: caller, }, ); } diff --git a/contracts/settlement/src/test.rs b/contracts/settlement/src/test.rs index d426bab..c660227 100644 --- a/contracts/settlement/src/test.rs +++ b/contracts/settlement/src/test.rs @@ -371,7 +371,7 @@ mod settlement_tests { } #[test] - fn test_set_vault() { + fn test_propose_and_accept_vault_happy_path() { let env = Env::default(); env.mock_all_auths(); let admin = Address::generate(&env); @@ -381,12 +381,17 @@ mod settlement_tests { let client = CalloraSettlementClient::new(&env, &addr); client.init(&admin, &vault); - client.set_vault(&admin, &new_vault); + // Step 1: propose by admin + client.propose_vault(&admin, &new_vault); + assert_eq!(client.get_vault(), vault); // still old until accepted + + // Step 2: accept by pending vault + client.accept_vault(&new_vault); assert_eq!(client.get_vault(), new_vault); } #[test] - fn test_set_vault_emits_event() { + fn test_propose_vault_emits_event() { use soroban_sdk::testutils::Events as _; use soroban_sdk::{IntoVal, Symbol}; @@ -399,7 +404,7 @@ mod settlement_tests { let client = CalloraSettlementClient::new(&env, &addr); client.init(&admin, &vault); - client.set_vault(&admin, &new_vault); + client.propose_vault(&admin, &new_vault); let events = env.events().all(); let ev = events @@ -407,17 +412,54 @@ mod settlement_tests { .find(|e| { !e.1.is_empty() && { let t: Symbol = e.1.get(0).unwrap().into_val(&env); - t == Symbol::new(&env, "vault_changed") + t == Symbol::new(&env, "vault_proposed") } }) - .expect("expected vault_changed event"); + .expect("expected vault_proposed event"); let topic1: Address = ev.1.get(1).unwrap().into_val(&env); assert_eq!(topic1, admin); - let data: crate::VaultChangedEvent = ev.2.into_val(&env); + let data: crate::VaultProposedEvent = ev.2.into_val(&env); + assert_eq!(data.current_vault, vault); + assert_eq!(data.proposed_vault, new_vault); + } + + #[test] + fn test_accept_vault_emits_event() { + use soroban_sdk::testutils::Events as _; + use soroban_sdk::{IntoVal, Symbol}; + + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let vault = Address::generate(&env); + let new_vault = Address::generate(&env); + let addr = env.register(CalloraSettlement, ()); + let client = CalloraSettlementClient::new(&env, &addr); + client.init(&admin, &vault); + + client.propose_vault(&admin, &new_vault); + client.accept_vault(&new_vault); + + let events = env.events().all(); + let ev = events + .iter() + .find(|e| { + !e.1.is_empty() && { + let t: Symbol = e.1.get(0).unwrap().into_val(&env); + t == Symbol::new(&env, "vault_accepted") + } + }) + .expect("expected vault_accepted event"); + + let topic1: Address = ev.1.get(1).unwrap().into_val(&env); + assert_eq!(topic1, new_vault); + + let data: crate::VaultAcceptedEvent = ev.2.into_val(&env); assert_eq!(data.old_vault, vault); assert_eq!(data.new_vault, new_vault); + assert_eq!(data.accepted_by, new_vault); } // ── admin rotation edge cases ──────────────────────────────────────────── @@ -443,7 +485,7 @@ mod settlement_tests { #[test] fn test_set_vault_to_same_address_succeeds() { - // Admin can update vault to same address (no-op but valid) + // Admin can propose + accept the same vault (no-op but valid) let env = Env::default(); env.mock_all_auths(); let admin = Address::generate(&env); @@ -452,7 +494,8 @@ mod settlement_tests { let client = CalloraSettlementClient::new(&env, &addr); client.init(&admin, &vault); - client.set_vault(&admin, &vault); + client.propose_vault(&admin, &vault); + client.accept_vault(&vault); assert_eq!(client.get_vault(), vault); } @@ -531,7 +574,8 @@ mod settlement_tests { client.accept_admin(); // New admin updates vault - client.set_vault(&new_admin, &new_vault); + client.propose_vault(&new_admin, &new_vault); + client.accept_vault(&new_vault); assert_eq!(client.get_vault(), new_vault); assert_eq!(client.get_admin(), new_admin); } @@ -695,7 +739,7 @@ mod settlement_tests { total_balance: i128::MAX, last_updated: env.ledger().timestamp(), }; - inst.set(&Symbol::new(&env, "global_pool"), &pool); + inst.set(&StorageKey::GlobalPool, &pool); }); client.receive_payment(&vault, &1i128, &true, &None); @@ -714,11 +758,9 @@ mod settlement_tests { client.init(&admin, &vault); env.as_contract(&addr, || { - let inst = env.storage().instance(); - let mut balances: Map = - inst.get(&Symbol::new(&env, "developer_balances")).unwrap(); - balances.set(developer.clone(), i128::MAX); - inst.set(&Symbol::new(&env, "developer_balances"), &balances); + env.storage() + .persistent() + .set(&StorageKey::DeveloperBalance(developer.clone()), &i128::MAX); }); client.receive_payment(&vault, &1i128, &false, &Some(developer)); @@ -977,7 +1019,8 @@ mod settlement_tests { client.init(&admin, &vault); // Update vault - client.set_vault(&admin, &new_vault); + client.propose_vault(&admin, &new_vault); + client.accept_vault(&new_vault); // Old vault cannot send payments let result = catch_unwind(AssertUnwindSafe(|| { @@ -1037,7 +1080,8 @@ mod settlement_tests { client.receive_payment(&vault, &100i128, &false, &Some(developer.clone())); // Update vault - client.set_vault(&admin, &new_vault); + client.propose_vault(&admin, &new_vault); + client.accept_vault(&new_vault); // More payments from new vault client.receive_payment(&new_vault, &150i128, &false, &Some(developer.clone())); @@ -1164,24 +1208,72 @@ mod settlement_tests { let client = CalloraSettlementClient::new(&env, &addr); let new_vault = Address::generate(&env); - // Admin can set vault - client.set_vault(&admin, &new_vault); + // Admin can propose vault (set_vault is an alias) + client.propose_vault(&admin, &new_vault); - // Vault cannot set vault + // Vault cannot propose vault let result = catch_unwind(AssertUnwindSafe(|| { - client.set_vault(&vault, &new_vault); + client.propose_vault(&vault, &new_vault); })); assert!(result.is_err()); assert!(panic_message(result.unwrap_err()).contains("unauthorized: caller is not admin")); - // Third party cannot set vault + // Third party cannot propose vault let result = catch_unwind(AssertUnwindSafe(|| { - client.set_vault(&third_party, &new_vault); + client.propose_vault(&third_party, &new_vault); })); assert!(result.is_err()); assert!(panic_message(result.unwrap_err()).contains("unauthorized: caller is not admin")); } + #[test] + fn test_accept_vault_rejects_unauthorized_caller() { + let (env, addr, admin, vault, third_party) = setup_contract(); + let client = CalloraSettlementClient::new(&env, &addr); + let new_vault = Address::generate(&env); + + client.propose_vault(&admin, &new_vault); + assert_eq!(client.get_vault(), vault); + + let result = catch_unwind(AssertUnwindSafe(|| { + client.accept_vault(&third_party); + })); + assert!(result.is_err()); + assert!(panic_message(result.unwrap_err()) + .contains("unauthorized: caller must be pending vault or admin")); + } + + #[test] + fn test_accept_vault_allows_admin_to_finalize() { + let (env, addr, admin, vault, _third_party) = setup_contract(); + let client = CalloraSettlementClient::new(&env, &addr); + let new_vault = Address::generate(&env); + + client.propose_vault(&admin, &new_vault); + assert_eq!(client.get_vault(), vault); + + client.accept_vault(&admin); + assert_eq!(client.get_vault(), new_vault); + } + + #[test] + fn test_propose_vault_rejects_self_address() { + let env = Env::default(); + env.mock_all_auths(); + let admin = Address::generate(&env); + let vault = Address::generate(&env); + let addr = env.register(CalloraSettlement, ()); + let client = CalloraSettlementClient::new(&env, &addr); + client.init(&admin, &vault); + + let result = catch_unwind(AssertUnwindSafe(|| { + client.propose_vault(&admin, &addr); + })); + assert!(result.is_err()); + assert!(panic_message(result.unwrap_err()) + .contains("invalid config: vault cannot be the contract itself")); + } + #[test] fn test_accept_admin_authorization_matrix() { let (env, addr, admin, _vault, _third_party) = setup_contract(); diff --git a/docs/ACCESS_CONTROL.md b/docs/ACCESS_CONTROL.md index ccceed0..1ac668a 100644 --- a/docs/ACCESS_CONTROL.md +++ b/docs/ACCESS_CONTROL.md @@ -70,6 +70,7 @@ The Callora Settlement contract tracks individual developer balances and global - **Admin**: Primary authority over contract configuration and sensitive data. - **Vault**: The registered vault contract authorized to send payments. - **Pending Admin**: Nominee awaiting acceptance of the admin role. +- **Pending Vault**: Proposed vault awaiting acceptance. ### Authorization Matrix @@ -78,18 +79,22 @@ The Callora Settlement contract tracks individual developer balances and global | `receive_payment` | ✅ | ✅ | ❌ | ❌ | | `set_admin` | ✅ | ❌ | ❌ | ❌ | | `accept_admin` | ❌ | ❌ | ✅ | ❌ | -| `set_vault` | ✅ | ❌ | ❌ | ❌ | +| `propose_vault` | ✅ | ❌ | ❌ | ❌ | +| `accept_vault` | ✅ | ✅ | ❌ | ❌ | +| `set_vault` (alias of `propose_vault`) | ✅ | ❌ | ❌ | ❌ | | `get_all_developer_balances` | ✅ | ❌ | ❌ | ❌ | ### Security Model - **Two-Step Admin Rotation**: Prevents accidental loss of control by requiring the nominee to explicitly accept the role. +- **Two-Step Vault Rotation**: Prevents accidentally misrouting settlement credits by requiring the proposed vault to accept (or the admin to finalize). - **Restricted Views**: Sensitive batch queries like `get_all_developer_balances` are restricted to the admin to prevent unnecessary exposure of the full ledger via the contract interface. ## Test Coverage The implementation includes comprehensive tests covering: - ✅ Admin and Vault can call `receive_payment` - ✅ Unauthorized callers are rejected from `receive_payment` -- ✅ Only Admin can call `set_admin` and `set_vault` +- ✅ Only Admin can call `set_admin` and `propose_vault` (and the `set_vault` alias) +- ✅ Only Admin or Pending Vault can call `accept_vault` - ✅ Only Pending Admin can call `accept_admin` - ✅ Only Admin can call `get_all_developer_balances` - ✅ All rotation and update logic preserves state integrity