diff --git a/contracts/savings_vault/src/lib.rs b/contracts/savings_vault/src/lib.rs index 8973b54..c1e44b5 100644 --- a/contracts/savings_vault/src/lib.rs +++ b/contracts/savings_vault/src/lib.rs @@ -25,7 +25,8 @@ extern crate alloc; extern crate std; use soroban_sdk::{ - contract, contractimpl, contracttype, log, symbol_short, token, Address, Env, Symbol, Vec, + contract, contracterror, contractimpl, contracttype, log, symbol_short, token, Address, Env, + Symbol, Vec, }; const MAX_LOCK_PAGE_SIZE: u32 = 50; @@ -151,6 +152,117 @@ pub enum DataKey { pub const STORAGE_VERSION: u64 = 1; +// --------------------------------------------------------------------------- +// Error Codes +// --------------------------------------------------------------------------- +/// Structured contract errors exposed via Soroban's `#[contracterror]` +/// mechanism. Every variant maps to a stable `u32` error code that SDK and +/// mobile consumers can rely on for deterministic user-facing messaging +/// and cross-repo compatibility. +/// +/// # Category Ranges +/// +/// | Range | Category | Primary Concern | +/// |-----------|--------------|-------------------------------------------------| +/// | 1000–1099 | Validation | Input argument sanity (sign, magnitude, range) | +/// | 2000–2099 | Authorisation| Role and signature enforcement | +/// | 3000–3099 | Lifecycle | Initialize / pause / storage-version states | +/// | 4000–4099 | Accounting | Balance sufficiency / deposit minimums | +/// | 5000–5099 | Lock | Lock lookup / state / maturity / durations | +/// | 6000–6099 | Storage | Migration / storage layout / unwrap safety | +/// | 7000–7099 | Token | SAC transfer / accepted-token configuration | +/// | 8000–8099 | Admin | Admin rotation rules | +/// +/// The gaps inside each range allow new variants to be added without +/// renumbering existing codes, which would be a breaking change for +/// downstream SDKs. +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +#[repr(u32)] +pub enum ContractError { + // ---- 1000s: Validation ------------------------------------------------ + /// `amount` argument was `0` or negative. `deposit`, `withdraw`, + /// `lock_funds` all require strictly positive amounts. + AmountNotPositive = 1001, + /// Attempted to create or extend a lock with `unlock_time` in the + /// past or equal to the current ledger timestamp. + UnlockTimeNotInFuture = 1002, + /// `lock_funds` duration (`unlock_time - ledger_timestamp`) exceeds + /// the configured `MaxLockDurationSecs`. + LockDurationExceedsMaximum = 1003, + /// `lock_funds` duration (`unlock_time - ledger_timestamp`) is + /// strictly below the configured `MinLockDurationSecs`. + LockDurationBelowMinimum = 1004, + /// `deposit` amount is strictly below the configured + /// `MinDepositAmount` floor. + AmountBelowMinimumDeposit = 1005, + /// `pause(admin, duration_secs)` called with `duration_secs == 0`. + PauseDurationMustBePositive = 1006, + /// `set_min_deposit_amount` called with a negative value. + MinDepositAmountNegative = 1007, + + // ---- 2000s: Authorisation -------------------------------------------- + /// Caller is not the stored admin (failed `assert_admin` check inside + /// an admin-gated function). + NotAuthorizedAdmin = 2001, + + // ---- 3000s: Lifecycle / State ---------------------------------------- + /// `initialize` called a second time on an already-initialized + /// contract. + AlreadyInitialized = 3001, + /// Function requiring `initialize()` to have run was called before + /// initialization completed. + NotInitialized = 3002, + /// Deposit or lock attempted while the emergency pause is active and + /// not yet expired. + ContractPaused = 3003, + + // ---- 4000s: Accounting ------------------------------------------------ + /// `withdraw` or `lock_funds` requested an amount strictly greater + /// than the caller's available `Balance`. + InsufficientBalance = 4001, + /// Semantic twin of `InsufficientBalance` used specifically by + /// `lock_funds` so SDKs can map the two contexts to different copy. + InsufficientBalanceToLock = 4002, + + // ---- 5000s: Locks ---------------------------------------------------- + /// `get_lock`, `withdraw_lock` or `extend_lock` referenced a lock id + /// that does not exist for the given owner. + LockNotFound = 5001, + /// `withdraw_lock` or `extend_lock` called on a lock whose + /// `withdrawn` flag is already true. + LockAlreadyWithdrawn = 5002, + /// `withdraw_lock` attempted before `unlock_time <= ledger_timestamp`. + LockNotMatured = 5003, + /// `extend_lock` attempted with a `new_unlock_time` that does not + /// exceed the lock's current `unlock_time`. + ExtendLockTimeNotIncreased = 5004, + + // ---- 6000s: Storage / Migration -------------------------------------- + /// `try_migrate` read a `StorageVersion` greater than + /// `STORAGE_VERSION` compiled into the running WASM (would be a + /// downgrade with potential data loss – blocked). + StorageVersionUnsupported = 6001, + /// Storage read for an instance key that must always be set after + /// initialization (e.g. `Admin`, `Token`) unexpectedly returned + /// `None`. + RequiredStorageEntryMissing = 6002, + + // ---- 7000s: Token / SAC ---------------------------------------------- + /// Accepted-token configuration missing at transfer time (should + /// never happen after `initialize`; guarded by `assert_initialized` + /// at every public entrypoint that reaches a transfer). + TokenNotConfigured = 7001, + + // ---- 8000s: Admin Rotation ------------------------------------------- + /// `transfer_admin` called with the current admin as new_admin + /// (no-op self-transfer blocked to preserve audit trail). + CannotTransferAdminToSelf = 8001, + /// `transfer_admin` called with the contract's own address as + /// new_admin (tokens and admin access would become unrecoverable). + CannotTransferAdminToContractAddress = 8002, +} + // --------------------------------------------------------------------------- // Contract Definition // --------------------------------------------------------------------------- @@ -164,13 +276,15 @@ impl SavingsVault { // Helpers // ----------------------------------------------------------------------- - fn assert_initialized(env: &Env) { + fn assert_initialized(env: &Env) -> Result<(), ContractError> { if !env.storage().instance().has(&DataKey::Initialized) { - panic!("Contract is not initialized"); + Err(ContractError::NotInitialized) + } else { + Ok(()) } } - fn try_migrate(env: &Env) { + fn try_migrate(env: &Env) -> Result<(), ContractError> { let current_version: u64 = env .storage() .instance() @@ -178,7 +292,7 @@ impl SavingsVault { .unwrap_or(0); if current_version == STORAGE_VERSION { - return; + return Ok(()); } // Migrate from older versions to newer versions incrementally! @@ -196,18 +310,25 @@ impl SavingsVault { "Migrated storage from version 0 to version {}", STORAGE_VERSION ); + Ok(()) } _ => { - // If current version > STORAGE_VERSION, panic to prevent downgrades! - panic!("Unsupported storage version: {}", current_version); + // If current version > STORAGE_VERSION, error to prevent downgrades! + Err(ContractError::StorageVersionUnsupported) } } } - fn assert_admin(env: &Env, admin: &Address) { - let stored_admin: Address = env.storage().instance().get(&DataKey::Admin).unwrap(); + fn assert_admin(env: &Env, admin: &Address) -> Result<(), ContractError> { + let stored_admin: Address = env + .storage() + .instance() + .get(&DataKey::Admin) + .unwrap_or_else(|| env.error_contract(ContractError::RequiredStorageEntryMissing)); if admin != &stored_admin { - panic!("Not authorized"); + Err(ContractError::NotAuthorizedAdmin) + } else { + Ok(()) } } @@ -238,11 +359,11 @@ impl SavingsVault { /// is automatically cleared so callers do not need to invoke `unpause` /// explicitly after a time-bounded pause expires. /// - /// # Panics + /// # Errors /// - /// Panics with `"Contract is paused"` when the pause is active and has not + /// Returns [`ContractError::ContractPaused`] when the pause is active and has not /// expired. - fn require_not_paused(env: &Env) { + fn require_not_paused(env: &Env) -> Result<(), ContractError> { let paused: bool = env .storage() .instance() @@ -259,20 +380,24 @@ impl SavingsVault { if expiry != 0 && env.ledger().timestamp() >= expiry { env.storage().instance().set(&DataKey::Paused, &false); env.storage().instance().set(&DataKey::PauseExpiry, &0_u64); - return; + return Ok(()); } - panic!("Contract is paused"); + Err(ContractError::ContractPaused) + } else { + Ok(()) } } - fn assert_supported_storage_version(env: &Env) { + fn assert_supported_storage_version(env: &Env) -> Result<(), ContractError> { let stored_version: u64 = env .storage() .instance() .get(&DataKey::StorageVersion) .unwrap_or(0); if stored_version != STORAGE_VERSION { - panic!("Unsupported storage version"); + Err(ContractError::StorageVersionUnsupported) + } else { + Ok(()) } } @@ -281,14 +406,15 @@ impl SavingsVault { // Initialization // ----------------------------------------------------------------------- - /// One-time setup. Records admin and token addresses. Panics if called twice. + /// One-time setup. Records admin and token addresses. Errors with + /// [`ContractError::AlreadyInitialized`] if called twice. pub fn initialize(env: Env, admin: Address, token: Address) { if env.storage().instance().has(&DataKey::Initialized) { - panic!("Contract is already initialized"); + env.error_contract(ContractError::AlreadyInitialized) } // Try migration before initializing - Self::try_migrate(&env); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); // Require the admin to have signed this transaction admin.require_auth(); @@ -324,8 +450,9 @@ impl SavingsVault { pub fn get_version(env: Env) -> soroban_sdk::String { // No need to be initialized for version check, but check storage version if possible if env.storage().instance().has(&DataKey::Initialized) { - Self::try_migrate(&env); - Self::assert_supported_storage_version(&env); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::assert_supported_storage_version(&env) + .unwrap_or_else(|e| env.error_contract(e)); } soroban_sdk::String::from_str(&env, "0.1.0") } @@ -356,14 +483,18 @@ impl SavingsVault { /// /// No authorization required (read-only operation). /// - /// # Panics + /// # Errors /// - /// - If the contract has not been initialized. + /// - [`ContractError::NotInitialized`] – If the contract has not been initialized. pub fn get_token(env: Env) -> Address { - Self::assert_initialized(&env); - Self::try_migrate(&env); - Self::assert_supported_storage_version(&env); - env.storage().instance().get(&DataKey::Token).unwrap() + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::assert_supported_storage_version(&env) + .unwrap_or_else(|e| env.error_contract(e)); + env.storage() + .instance() + .get(&DataKey::Token) + .unwrap_or_else(|| env.error_contract(ContractError::TokenNotConfigured)) } // ----------------------------------------------------------------------- @@ -404,12 +535,12 @@ impl SavingsVault { /// - If the caller is not the admin /// - If `duration_secs` is zero pub fn pause(env: Env, admin: Address, duration_secs: u64) { - Self::assert_initialized(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); admin.require_auth(); - Self::assert_admin(&env, &admin); + Self::assert_admin(&env, &admin).unwrap_or_else(|e| env.error_contract(e)); if duration_secs == 0 { - panic!("Pause duration must be greater than zero"); + env.error_contract(ContractError::PauseDurationMustBePositive) } let expiry = env.ledger().timestamp() + duration_secs; @@ -461,9 +592,9 @@ impl SavingsVault { /// - If the contract has not been initialized /// - If the caller is not the admin pub fn unpause(env: Env, admin: Address) { - Self::assert_initialized(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); admin.require_auth(); - Self::assert_admin(&env, &admin); + Self::assert_admin(&env, &admin).unwrap_or_else(|e| env.error_contract(e)); env.storage().instance().set(&DataKey::Paused, &false); env.storage().instance().set(&DataKey::PauseExpiry, &0_u64); @@ -487,12 +618,12 @@ impl SavingsVault { /// - If the contract has not been initialized /// - If the caller is not the admin pub fn set_min_deposit_amount(env: Env, admin: Address, min_amount: i128) { - Self::assert_initialized(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); admin.require_auth(); - Self::assert_admin(&env, &admin); + Self::assert_admin(&env, &admin).unwrap_or_else(|e| env.error_contract(e)); if min_amount < 0 { - panic!("Min deposit amount cannot be negative"); + env.error_contract(ContractError::MinDepositAmountNegative) } env.storage() @@ -527,9 +658,9 @@ impl SavingsVault { /// - If the contract has not been initialized /// - If the caller is not the admin pub fn set_max_lock_duration(env: Env, admin: Address, max_duration_secs: u64) { - Self::assert_initialized(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); admin.require_auth(); - Self::assert_admin(&env, &admin); + Self::assert_admin(&env, &admin).unwrap_or_else(|e| env.error_contract(e)); env.storage() .instance() @@ -569,9 +700,9 @@ impl SavingsVault { /// - If the contract has not been initialized /// - If the caller is not the admin pub fn set_min_lock_duration(env: Env, admin: Address, min_duration_secs: u64) { - Self::assert_initialized(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); admin.require_auth(); - Self::assert_admin(&env, &admin); + Self::assert_admin(&env, &admin).unwrap_or_else(|e| env.error_contract(e)); env.storage() .instance() @@ -616,7 +747,7 @@ impl SavingsVault { /// /// No authorization required (read-only operation). pub fn is_paused(env: Env) -> bool { - Self::assert_initialized(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); let paused: bool = env .storage() @@ -646,17 +777,18 @@ impl SavingsVault { // ----------------------------------------------------------------------- /// Transfers tokens from the user into the vault and credits their balance. - /// Panics if amount <= 0. + /// Errors with [`ContractError::AmountNotPositive`] if amount <= 0. pub fn deposit(env: Env, user: Address, amount: i128) { - Self::assert_initialized(&env); - Self::try_migrate(&env); - Self::assert_supported_storage_version(&env); - Self::require_not_paused(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::assert_supported_storage_version(&env) + .unwrap_or_else(|e| env.error_contract(e)); + Self::require_not_paused(&env).unwrap_or_else(|e| env.error_contract(e)); user.require_auth(); if amount <= 0 { - panic!("Amount must be positive"); + env.error_contract(ContractError::AmountNotPositive) } let min_deposit: i128 = env @@ -665,10 +797,14 @@ impl SavingsVault { .get(&DataKey::MinDepositAmount) .unwrap_or(0); if min_deposit > 0 && amount < min_deposit { - panic!("Amount is below the minimum deposit amount"); + env.error_contract(ContractError::AmountBelowMinimumDeposit) } - let token = env.storage().instance().get(&DataKey::Token).unwrap(); + let token = env + .storage() + .instance() + .get(&DataKey::Token) + .unwrap_or_else(|| env.error_contract(ContractError::TokenNotConfigured)); let token_client = token::Client::new(&env, &token); let contract_address = env.current_contract_address(); @@ -704,16 +840,18 @@ impl SavingsVault { /// Withdraws available funds from the user's vault. /// Only touches the deposited balance (not matured locks). - /// Panics if amount <= 0 or exceeds available balance. + /// Errors with [`ContractError::AmountNotPositive`] if amount <= 0 or + /// [`ContractError::InsufficientBalance`] if it exceeds available balance. pub fn withdraw(env: Env, user: Address, amount: i128) { - Self::assert_initialized(&env); - Self::try_migrate(&env); - Self::assert_supported_storage_version(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::assert_supported_storage_version(&env) + .unwrap_or_else(|e| env.error_contract(e)); user.require_auth(); if amount <= 0 { - panic!("Amount must be positive"); + env.error_contract(ContractError::AmountNotPositive) } let mut current_balance: i128 = env @@ -723,10 +861,14 @@ impl SavingsVault { .unwrap_or(0); if amount > current_balance { - panic!("Insufficient balance"); + env.error_contract(ContractError::InsufficientBalance) } - let token = env.storage().instance().get(&DataKey::Token).unwrap(); + let token = env + .storage() + .instance() + .get(&DataKey::Token) + .unwrap_or_else(|| env.error_contract(ContractError::TokenNotConfigured)); let token_client = token::Client::new(&env, &token); let contract_address = env.current_contract_address(); @@ -752,11 +894,13 @@ impl SavingsVault { } /// Withdraws a specific matured lock entry by its ID. - /// Panics if the lock doesn't exist or hasn't matured. + /// Errors with [`ContractError::LockNotFound`] if the lock doesn't exist or + /// [`ContractError::LockNotMatured`] if it hasn't matured. pub fn withdraw_lock(env: Env, user: Address, lock_id: u64) { - Self::assert_initialized(&env); - Self::try_migrate(&env); - Self::assert_supported_storage_version(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::assert_supported_storage_version(&env) + .unwrap_or_else(|e| env.error_contract(e)); user.require_auth(); @@ -766,19 +910,23 @@ impl SavingsVault { .get::<_, LockEntry>(&DataKey::Lock(user.clone(), lock_id)) { Some(l) => l, - None => panic!("Lock not found"), + None => env.error_contract(ContractError::LockNotFound), }; if lock.withdrawn { - panic!("Lock already withdrawn"); + env.error_contract(ContractError::LockAlreadyWithdrawn) } let current_time = env.ledger().timestamp(); if current_time < lock.unlock_time { - panic!("Lock has not matured yet"); + env.error_contract(ContractError::LockNotMatured) } - let token = env.storage().instance().get(&DataKey::Token).unwrap(); + let token = env + .storage() + .instance() + .get(&DataKey::Token) + .unwrap_or_else(|| env.error_contract(ContractError::TokenNotConfigured)); let token_client = token::Client::new(&env, &token); let contract_address = env.current_contract_address(); @@ -812,9 +960,10 @@ impl SavingsVault { /// Returns the user's available balance: only the deposited (unlocked) balance. /// Matured locks must be withdrawn via `withdraw_lock`. pub fn get_balance(env: Env, user: Address) -> i128 { - Self::assert_initialized(&env); - Self::try_migrate(&env); - Self::assert_supported_storage_version(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::assert_supported_storage_version(&env) + .unwrap_or_else(|e| env.error_contract(e)); let deposited_balance: i128 = env .storage() .persistent() @@ -855,9 +1004,10 @@ impl SavingsVault { /// large number of historical locks this may become expensive; consider /// off-chain indexing in that case. pub fn get_balance_snapshot(env: Env, user: Address) -> BalanceSnapshot { - Self::assert_initialized(&env); - Self::try_migrate(&env); - Self::assert_supported_storage_version(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::assert_supported_storage_version(&env) + .unwrap_or_else(|e| env.error_contract(e)); let unlocked: i128 = env .storage() @@ -929,9 +1079,10 @@ impl SavingsVault { /// Same linear-scan caveat as [`get_balance_snapshot`]. For users with /// a very large number of historical locks, prefer off-chain indexing. pub fn get_lock_summary(env: Env, user: Address) -> LockSummary { - Self::assert_initialized(&env); - Self::try_migrate(&env); - Self::assert_supported_storage_version(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::assert_supported_storage_version(&env) + .unwrap_or_else(|e| env.error_contract(e)); let next_lock_id: u64 = env .storage() @@ -989,23 +1140,27 @@ impl SavingsVault { // ----------------------------------------------------------------------- /// Locks a portion of the user's available balance until `unlock_time`. - /// Returns the lock ID. Panics if amount <= 0, exceeds balance, or - /// unlock_time is not in the future. + /// Returns the lock ID. Errors with [`ContractError::AmountNotPositive`], + /// [`ContractError::UnlockTimeNotInFuture`], + /// [`ContractError::LockDurationExceedsMaximum`], + /// [`ContractError::LockDurationBelowMinimum`], or + /// [`ContractError::InsufficientBalanceToLock`] on invalid input. pub fn lock_funds(env: Env, user: Address, amount: i128, unlock_time: u64) -> u64 { - Self::assert_initialized(&env); - Self::try_migrate(&env); - Self::assert_supported_storage_version(&env); - Self::require_not_paused(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::assert_supported_storage_version(&env) + .unwrap_or_else(|e| env.error_contract(e)); + Self::require_not_paused(&env).unwrap_or_else(|e| env.error_contract(e)); user.require_auth(); if amount <= 0 { - panic!("Amount must be positive"); + env.error_contract(ContractError::AmountNotPositive) } let current_time = env.ledger().timestamp(); if unlock_time <= current_time { - panic!("Unlock time must be in the future"); + env.error_contract(ContractError::UnlockTimeNotInFuture) } let max_duration: u64 = env @@ -1014,7 +1169,7 @@ impl SavingsVault { .get(&DataKey::MaxLockDurationSecs) .unwrap_or(0); if max_duration > 0 && unlock_time - current_time > max_duration { - panic!("Lock duration exceeds maximum"); + env.error_contract(ContractError::LockDurationExceedsMaximum) } let min_duration: u64 = env @@ -1023,7 +1178,7 @@ impl SavingsVault { .get(&DataKey::MinLockDurationSecs) .unwrap_or(0); if min_duration > 0 && unlock_time - current_time < min_duration { - panic!("Lock duration below minimum"); + env.error_contract(ContractError::LockDurationBelowMinimum) } let mut current_balance: i128 = env @@ -1033,7 +1188,7 @@ impl SavingsVault { .unwrap_or(0); if amount > current_balance { - panic!("Insufficient balance"); + env.error_contract(ContractError::InsufficientBalanceToLock) } let next_id: u64 = env @@ -1121,10 +1276,11 @@ impl SavingsVault { /// - If `new_unlock_time` is not strictly greater than current `lock.unlock_time`. /// - If `new_unlock_time` is not in the future (`<= env.ledger().timestamp()`). pub fn extend_lock(env: Env, user: Address, lock_id: u64, new_unlock_time: u64) { - Self::assert_initialized(&env); - Self::try_migrate(&env); - Self::assert_supported_storage_version(&env); - Self::require_not_paused(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::assert_supported_storage_version(&env) + .unwrap_or_else(|e| env.error_contract(e)); + Self::require_not_paused(&env).unwrap_or_else(|e| env.error_contract(e)); user.require_auth(); @@ -1134,20 +1290,20 @@ impl SavingsVault { .get::<_, LockEntry>(&DataKey::Lock(user.clone(), lock_id)) { Some(l) => l, - None => panic!("Lock not found"), + None => env.error_contract(ContractError::LockNotFound), }; if lock.withdrawn { - panic!("Lock already withdrawn"); + env.error_contract(ContractError::LockAlreadyWithdrawn) } let current_time = env.ledger().timestamp(); if new_unlock_time <= current_time { - panic!("Unlock time must be in the future"); + env.error_contract(ContractError::UnlockTimeNotInFuture) } if new_unlock_time <= lock.unlock_time { - panic!("New unlock time must be strictly greater than current unlock time"); + env.error_contract(ContractError::ExtendLockTimeNotIncreased) } let old_unlock_time = lock.unlock_time; @@ -1175,9 +1331,10 @@ impl SavingsVault { /// (both matured and immature). Matured locks must be withdrawn via /// `withdraw_lock`. pub fn get_locked_balance(env: Env, user: Address) -> i128 { - Self::assert_initialized(&env); - Self::try_migrate(&env); - Self::assert_supported_storage_version(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::assert_supported_storage_version(&env) + .unwrap_or_else(|e| env.error_contract(e)); let next_lock_id: u64 = env .storage() .persistent() @@ -1201,9 +1358,10 @@ impl SavingsVault { /// Returns true if the user has at least one matured lock. pub fn can_withdraw(env: Env, user: Address) -> bool { - Self::assert_initialized(&env); - Self::try_migrate(&env); - Self::assert_supported_storage_version(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::assert_supported_storage_version(&env) + .unwrap_or_else(|e| env.error_contract(e)); let next_lock_id: u64 = env .storage() .persistent() @@ -1228,9 +1386,10 @@ impl SavingsVault { /// Returns a single lock entry by ID, or None if not found. pub fn get_lock(env: Env, user: Address, lock_id: u64) -> Option { - Self::assert_initialized(&env); - Self::try_migrate(&env); - Self::assert_supported_storage_version(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::assert_supported_storage_version(&env) + .unwrap_or_else(|e| env.error_contract(e)); env.storage() .persistent() .get(&DataKey::Lock(user.clone(), lock_id)) @@ -1238,9 +1397,10 @@ impl SavingsVault { /// Returns a paginated list of lock entries for a user (oldest first). pub fn list_locks(env: Env, user: Address, offset: u32, limit: u32) -> Vec { - Self::assert_initialized(&env); - Self::try_migrate(&env); - Self::assert_supported_storage_version(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::try_migrate(&env).unwrap_or_else(|e| env.error_contract(e)); + Self::assert_supported_storage_version(&env) + .unwrap_or_else(|e| env.error_contract(e)); let next_lock_id: u64 = env .storage() .persistent() @@ -1278,25 +1438,32 @@ impl SavingsVault { /// Returns the admin address set during initialization. pub fn get_admin(env: Env) -> Address { - Self::assert_initialized(&env); - env.storage().instance().get(&DataKey::Admin).unwrap() + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); + env.storage() + .instance() + .get(&DataKey::Admin) + .unwrap_or_else(|| env.error_contract(ContractError::RequiredStorageEntryMissing)) } /// Transfers admin privileges to a new address. Only the current admin /// can call this. pub fn transfer_admin(env: Env, admin: Address, new_admin: Address) { - Self::assert_initialized(&env); + Self::assert_initialized(&env).unwrap_or_else(|e| env.error_contract(e)); admin.require_auth(); - Self::assert_admin(&env, &admin); + Self::assert_admin(&env, &admin).unwrap_or_else(|e| env.error_contract(e)); if admin == new_admin { - panic!("Invalid new admin: cannot transfer to self"); + env.error_contract(ContractError::CannotTransferAdminToSelf) } if new_admin == env.current_contract_address() { - panic!("Invalid new admin: cannot set contract address as admin"); + env.error_contract(ContractError::CannotTransferAdminToContractAddress) } - let old_admin: Address = env.storage().instance().get(&DataKey::Admin).unwrap(); + let old_admin: Address = env + .storage() + .instance() + .get(&DataKey::Admin) + .unwrap_or_else(|| env.error_contract(ContractError::RequiredStorageEntryMissing)); env.storage().instance().set(&DataKey::Admin, &new_admin); let topics = (symbol_short!("xferadmin"), old_admin.clone()); diff --git a/contracts/savings_vault/src/test/contract_error_codes.rs b/contracts/savings_vault/src/test/contract_error_codes.rs new file mode 100644 index 0000000..798e64b --- /dev/null +++ b/contracts/savings_vault/src/test/contract_error_codes.rs @@ -0,0 +1,382 @@ +//! Structured contract error code tests. +//! +//! Verifies the full `ContractError` enum surface exposed by the contract via +//! `env.error_contract(variant)`. Each test exercises a single failure path +//! and asserts the panic emitted by the Soroban test harness contains the +//! exact numeric `u32` code matching the variant's `#[repr(u32)]` discriminant. +//! +//! SDK and mobile consumers map these same numeric codes to localized user +//! messages; this suite therefore doubles as a cross-repo compatibility +//! contract: any code change here is a BREAKING change for callers. + +use super::*; +use soroban_sdk::testutils::{Address as _, Events}; +use soroban_sdk::IntoVal; + +use crate::ContractError; +use test_helpers::*; + +/// Helper: wrap a fallible client call in `std::panic::catch_unwind` and +/// extract the panic payload as a `String`. All error-code tests assert on +/// substrings of this payload, which in the soroban-sdk `testutils` harness +/// contains the `Status(ContractError, CODE)` diagnostic where `CODE` is the +/// u32 from the enum's `#[repr(u32)]`. +fn catch_panic_message(f: F) -> String { + match std::panic::catch_unwind(f) { + Ok(_) => panic!("expected a panic but the call succeeded"), + Err(payload) => { + if let Some(s) = payload.downcast_ref::<&str>() { + (*s).to_string() + } else if let Some(s) = payload.downcast_ref::() { + s.clone() + } else { + format!("{:?}", payload) + } + } + } +} + +// ========================================================================= +// CATEGORY 1000: Validation +// ========================================================================= + +#[test] +fn error_code_1001_amount_not_positive_on_deposit() { + let env = test_env(); + let (_contract_id, client, _token_client, _token_admin, _) = vault_with_sac(&env); + let user = Address::generate(&env); + let msg = catch_panic_message(|| client.deposit(&user, &0)); + assert!( + msg.contains(&ContractError::AmountNotPositive as u32.to_string()) + || msg.contains("AmountNotPositive"), + "panic payload must reference error code 1001 AmountNotPositive; got: {}", + msg + ); +} + +#[test] +fn error_code_1001_amount_not_positive_on_withdraw() { + let env = test_env(); + let (_contract_id, client, _token_client, _token_admin, _) = vault_with_sac(&env); + let user = Address::generate(&env); + let msg = catch_panic_message(|| client.withdraw(&user, &-1)); + assert!( + msg.contains(&ContractError::AmountNotPositive as u32.to_string()) + || msg.contains("AmountNotPositive"), + "panic payload must reference error code 1001 AmountNotPositive; got: {}", + msg + ); +} + +#[test] +fn error_code_1001_amount_not_positive_on_lock() { + let env = test_env(); + let (_contract_id, client, _token_client, _token_admin, _) = vault_with_sac(&env); + let user = Address::generate(&env); + set_ledger_timestamp(&env, 1_000); + let msg = catch_panic_message(|| client.lock_funds(&user, &0, &5_000)); + assert!( + msg.contains(&ContractError::AmountNotPositive as u32.to_string()) + || msg.contains("AmountNotPositive"), + "panic payload must reference error code 1001; got: {}", + msg + ); +} + +#[test] +fn error_code_1002_unlock_time_not_in_future_lock_funds() { + let env = test_env(); + let (_contract_id, client, _token_client, _token_admin, _) = vault_with_sac(&env); + let user = Address::generate(&env); + set_ledger_timestamp(&env, 1_000); + let msg = catch_panic_message(|| client.lock_funds(&user, &50, &1_000)); + assert!( + msg.contains(&ContractError::UnlockTimeNotInFuture as u32.to_string()) + || msg.contains("UnlockTimeNotInFuture"), + "panic payload must reference error code 1002; got: {}", + msg + ); +} + +#[test] +fn error_code_1003_lock_duration_exceeds_maximum() { + let env = test_env(); + let (_contract_id, client, _token_client, token_admin, _) = vault_with_sac(&env); + set_ledger_timestamp(&env, 1_000); + client.set_max_lock_duration(&token_admin, &1_000); + let user = Address::generate(&env); + let msg = catch_panic_message(|| client.lock_funds(&user, &10, &10_000)); + assert!( + msg.contains(&ContractError::LockDurationExceedsMaximum as u32.to_string()) + || msg.contains("LockDurationExceedsMaximum"), + "panic payload must reference error code 1003; got: {}", + msg + ); +} + +#[test] +fn error_code_1004_lock_duration_below_minimum() { + let env = test_env(); + let (_contract_id, client, _token_client, token_admin, _) = vault_with_sac(&env); + set_ledger_timestamp(&env, 1_000); + client.set_min_lock_duration(&token_admin, &5_000); + let user = Address::generate(&env); + let msg = catch_panic_message(|| client.lock_funds(&user, &10, &2_000)); + assert!( + msg.contains(&ContractError::LockDurationBelowMinimum as u32.to_string()) + || msg.contains("LockDurationBelowMinimum"), + "panic payload must reference error code 1004; got: {}", + msg + ); +} + +#[test] +fn error_code_1005_amount_below_minimum_deposit() { + let env = test_env(); + let (_contract_id, client, _token_client, token_admin, _) = vault_with_sac(&env); + client.set_min_deposit_amount(&token_admin, &1_000); + let user = Address::generate(&env); + let msg = catch_panic_message(|| client.deposit(&user, &50)); + assert!( + msg.contains(&ContractError::AmountBelowMinimumDeposit as u32.to_string()) + || msg.contains("AmountBelowMinimumDeposit"), + "panic payload must reference error code 1005; got: {}", + msg + ); +} + +#[test] +fn error_code_1006_pause_duration_must_be_positive() { + let env = test_env(); + let (_contract_id, client, _token_client, token_admin, _) = vault_with_sac(&env); + let msg = catch_panic_message(|| client.pause(&token_admin, &0)); + assert!( + msg.contains(&ContractError::PauseDurationMustBePositive as u32.to_string()) + || msg.contains("PauseDurationMustBePositive"), + "panic payload must reference error code 1006; got: {}", + msg + ); +} + +#[test] +fn error_code_1007_min_deposit_amount_negative() { + let env = test_env(); + let (_contract_id, client, _token_client, token_admin, _) = vault_with_sac(&env); + let msg = catch_panic_message(|| client.set_min_deposit_amount(&token_admin, &-1)); + assert!( + msg.contains(&ContractError::MinDepositAmountNegative as u32.to_string()) + || msg.contains("MinDepositAmountNegative"), + "panic payload must reference error code 1007; got: {}", + msg + ); +} + +// ========================================================================= +// CATEGORY 2000: Authorisation +// ========================================================================= + +#[test] +fn error_code_2001_not_authorized_admin_wrong_caller() { + let env = test_env(); + let (_contract_id, client, _token_client, _token_admin, _) = vault_with_sac(&env); + let rando = Address::generate(&env); + let msg = catch_panic_message(|| client.unpause(&rando)); + assert!( + msg.contains(&ContractError::NotAuthorizedAdmin as u32.to_string()) + || msg.contains("NotAuthorizedAdmin"), + "panic payload must reference error code 2001; got: {}", + msg + ); +} + +// ========================================================================= +// CATEGORY 3000: Lifecycle +// ========================================================================= + +#[test] +fn error_code_3001_already_initialized() { + let env = test_env(); + let (_contract_id, client, _token_client, token_admin, _) = vault_with_sac(&env); + let other_token = Address::generate(&env); + let msg = catch_panic_message(|| client.initialize(&token_admin, &other_token)); + assert!( + msg.contains(&ContractError::AlreadyInitialized as u32.to_string()) + || msg.contains("AlreadyInitialized"), + "panic payload must reference error code 3001; got: {}", + msg + ); +} + +#[test] +fn error_code_3002_not_initialized_before_deposit() { + let env = test_env(); + let contract_id = env.register(SavingsVault, ()); + let client = SavingsVaultClient::new(&env, &contract_id); + let user = Address::generate(&env); + let msg = catch_panic_message(|| client.deposit(&user, &100)); + assert!( + msg.contains(&ContractError::NotInitialized as u32.to_string()) + || msg.contains("NotInitialized"), + "panic payload must reference error code 3002; got: {}", + msg + ); +} + +#[test] +fn error_code_3003_contract_paused_blocks_deposit() { + let env = test_env(); + let (_contract_id, client, _token_client, token_admin, _) = vault_with_sac(&env); + client.pause(&token_admin, &60); + let user = Address::generate(&env); + let msg = catch_panic_message(|| client.deposit(&user, &100)); + assert!( + msg.contains(&ContractError::ContractPaused as u32.to_string()) + || msg.contains("ContractPaused"), + "panic payload must reference error code 3003; got: {}", + msg + ); +} + +// ========================================================================= +// CATEGORY 4000: Accounting +// ========================================================================= + +#[test] +fn error_code_4001_insufficient_balance_withdraw() { + let env = test_env(); + let (_contract_id, client, _token_client, _token_admin, _) = vault_with_sac(&env); + let user = Address::generate(&env); + let msg = catch_panic_message(|| client.withdraw(&user, &999_999)); + assert!( + msg.contains(&ContractError::InsufficientBalance as u32.to_string()) + || msg.contains("InsufficientBalance"), + "panic payload must reference error code 4001; got: {}", + msg + ); +} + +#[test] +fn error_code_4002_insufficient_balance_to_lock() { + let env = test_env(); + let (_contract_id, client, _token_client, _token_admin, _) = vault_with_sac(&env); + let user = Address::generate(&env); + set_ledger_timestamp(&env, 1_000); + let msg = catch_panic_message(|| client.lock_funds(&user, &9_999, &5_000)); + assert!( + msg.contains(&ContractError::InsufficientBalanceToLock as u32.to_string()) + || msg.contains("InsufficientBalanceToLock"), + "panic payload must reference error code 4002; got: {}", + msg + ); +} + +// ========================================================================= +// CATEGORY 5000: Lock +// ========================================================================= + +#[test] +fn error_code_5001_lock_not_found_on_withdraw() { + let env = test_env(); + let (_contract_id, client, _token_client, _token_admin, _) = vault_with_sac(&env); + let user = Address::generate(&env); + let msg = catch_panic_message(|| client.withdraw_lock(&user, &1337)); + assert!( + msg.contains(&ContractError::LockNotFound as u32.to_string()) + || msg.contains("LockNotFound"), + "panic payload must reference error code 5001; got: {}", + msg + ); +} + +#[test] +fn error_code_5002_lock_already_withdrawn() { + let env = test_env(); + let (contract_id, client, _token_client, _token_admin, asset_admin) = vault_with_sac(&env); + let user = Address::generate(&env); + set_ledger_timestamp(&env, 1_000); + asset_admin.mint(&user, &10_000); + env.mock_all_auths(); + client.deposit(&user, &5_000); + let id = client.lock_funds(&user, &2_000, &2_000); + set_ledger_timestamp(&env, 10_000); + client.withdraw_lock(&user, &id); + env.set_auths(&[]); + let msg = catch_panic_message(|| client.withdraw_lock(&user, &id)); + assert!( + msg.contains(&ContractError::LockAlreadyWithdrawn as u32.to_string()) + || msg.contains("LockAlreadyWithdrawn"), + "panic payload must reference error code 5002; got: {}", + msg + ); +} + +#[test] +fn error_code_5003_lock_not_matured() { + let env = test_env(); + let (contract_id, client, _token_client, _token_admin, asset_admin) = vault_with_sac(&env); + let user = Address::generate(&env); + set_ledger_timestamp(&env, 1_000); + asset_admin.mint(&user, &10_000); + env.mock_all_auths(); + client.deposit(&user, &5_000); + let id = client.lock_funds(&user, &2_000, &9_999_999); + env.set_auths(&[]); + let msg = catch_panic_message(|| client.withdraw_lock(&user, &id)); + assert!( + msg.contains(&ContractError::LockNotMatured as u32.to_string()) + || msg.contains("LockNotMatured"), + "panic payload must reference error code 5003; got: {}", + msg + ); +} + +#[test] +fn error_code_5004_extend_lock_time_not_increased() { + let env = test_env(); + let (contract_id, client, _token_client, _token_admin, asset_admin) = vault_with_sac(&env); + let user = Address::generate(&env); + set_ledger_timestamp(&env, 1_000); + asset_admin.mint(&user, &10_000); + env.mock_all_auths(); + client.deposit(&user, &5_000); + let id = client.lock_funds(&user, &2_000, &20_000); + env.set_auths(&[]); + let msg = catch_panic_message(|| client.extend_lock(&user, &id, &15_000)); + assert!( + msg.contains(&ContractError::ExtendLockTimeNotIncreased as u32.to_string()) + || msg.contains("ExtendLockTimeNotIncreased"), + "panic payload must reference error code 5004; got: {}", + msg + ); +} + +// ========================================================================= +// CATEGORY 8000: Admin Rotation +// ========================================================================= + +#[test] +fn error_code_8001_cannot_transfer_admin_to_self() { + let env = test_env(); + let (_contract_id, client, _token_client, token_admin, _) = vault_with_sac(&env); + let same = token_admin.clone(); + let msg = catch_panic_message(|| client.transfer_admin(&token_admin, &same)); + assert!( + msg.contains(&ContractError::CannotTransferAdminToSelf as u32.to_string()) + || msg.contains("CannotTransferAdminToSelf"), + "panic payload must reference error code 8001; got: {}", + msg + ); +} + +#[test] +fn error_code_8002_cannot_transfer_admin_to_contract_address() { + let env = test_env(); + let (contract_id, client, _token_client, token_admin, _) = vault_with_sac(&env); + let msg = catch_panic_message(|| client.transfer_admin(&token_admin, &contract_id)); + assert!( + msg.contains(&ContractError::CannotTransferAdminToContractAddress as u32.to_string()) + || msg.contains("CannotTransferAdminToContractAddress"), + "panic payload must reference error code 8002; got: {}", + msg + ); +} diff --git a/contracts/savings_vault/src/test/mod.rs b/contracts/savings_vault/src/test/mod.rs index a2098d2..40ae23e 100644 --- a/contracts/savings_vault/src/test/mod.rs +++ b/contracts/savings_vault/src/test/mod.rs @@ -7,6 +7,7 @@ mod admin_rotation; mod balance_conservation; mod balance_snapshot; mod config_read_helpers; +mod contract_error_codes; mod event_compatibility; mod event_ordering; mod independent_lock_creation; diff --git a/contracts/savings_vault/src/test/token_transfer_rollback.rs b/contracts/savings_vault/src/test/token_transfer_rollback.rs index 5e8c3bd..2ac7391 100644 --- a/contracts/savings_vault/src/test/token_transfer_rollback.rs +++ b/contracts/savings_vault/src/test/token_transfer_rollback.rs @@ -446,3 +446,85 @@ fn test_failed_withdraw_lock_token_transfer_failure_preserves_state() { assert_eq!(lock.amount, 200, "lock amount should remain unchanged"); assert!(!lock.withdrawn, "lock should not be marked as withdrawn"); } + +#[test] +fn test_failed_withdraw_token_transfer_failure_preserves_state() { + // Mirror of `test_failed_withdraw_lock_token_transfer_failure_preserves_state` + // but for the plain `withdraw` entrypoint. Verifies that when the SAC + // transfer from contract -> user fails (contract custody drained below + // the user's internal available balance), every piece of vault state + // (available balance, locked balance, events, lock entries) is preserved + // exactly as it was before the call. + let env = test_env(); + let (contract_id, client, token_client, token_admin) = vault_with_sac(&env); + let user = Address::generate(&env); + + set_ledger_timestamp(&env, 1_000); + token_admin.mint(&user, &1_000); + + // Deposit + lock to build mixed state so we verify both available and + // locked sides remain untouched by a failed withdrawal. + client.deposit(&user, &800); + let _lock_id = client.lock_funds(&user, &300, &5_000); + + // Internal state before failure: 500 available, 300 locked, 800 total + let (bal_before, locked_before, events_before) = snapshot(&env, &client, &user); + assert_eq!(bal_before, 500); + assert_eq!(locked_before, 300); + + // Also snapshot the lock entry fields to rule out any mutation + let lock_snapshot_before = client.get_lock(&user, &_lock_id).unwrap(); + + // Drain the contract's SAC balance to an unrelated sink address so the + // internal `token_client.transfer(contract -> user)` inside `withdraw` + // will be rejected by the SAC even though the user's internal balance + // is sufficient. `mock_all_auths()` grants the `from.require_auth()` + // check the SAC performs on the contract address itself. + let contract_address = contract_id.clone(); + let contract_balance = token_client.balance(&contract_address); + assert_eq!(contract_balance, 800, "custody = sum of liabilities pre-drain"); + let sink = Address::generate(&env); + env.mock_all_auths(); + token_client.transfer(&contract_address, &sink, &contract_balance); + env.set_auths(&[]); // clear mocks so real user auth is required again + assert_eq!(token_client.balance(&contract_address), 0); + + // Attempt withdraw for an amount covered by the internal balance (so + // the internal check passes) but NOT covered by SAC custody (so the + // transfer must fail and roll back the host call). + let result = client.try_withdraw(&user, &200); + assert!( + result.is_err(), + "withdraw must fail when contract has insufficient SAC custody" + ); + + // Zero state drift: available balance, locked balance, event count + let (bal_after, locked_after, events_after) = snapshot(&env, &client, &user); + assert_eq!( + bal_after, bal_before, + "available balance must not change on failed withdraw SAC transfer" + ); + assert_eq!( + locked_after, locked_before, + "locked balance must not change on failed withdraw SAC transfer" + ); + assert_eq!( + events_after, events_before, + "no new events must be emitted on failed withdraw SAC transfer" + ); + + // Lock entry byte-identical to snapshot (no field drift) + let lock_snapshot_after = client.get_lock(&user, &_lock_id).unwrap(); + assert_eq!( + lock_snapshot_after.amount, lock_snapshot_before.amount, + "lock amount unchanged after failed withdraw" + ); + assert_eq!( + lock_snapshot_after.unlock_time, lock_snapshot_before.unlock_time, + "lock unlock_time unchanged after failed withdraw" + ); + assert_eq!( + lock_snapshot_after.withdrawn, lock_snapshot_before.withdrawn, + "lock withdrawn flag unchanged after failed withdraw" + ); +} diff --git a/docs/audit-evidence-index.md b/docs/audit-evidence-index.md index 7abcc43..de048d2 100644 --- a/docs/audit-evidence-index.md +++ b/docs/audit-evidence-index.md @@ -9,6 +9,7 @@ This index organizes security-relevant documentation for external audit review. | [Contract Specification](contract-specification.md) | Formal specification of the PocketPay Savings Vault contract | ✅ Complete | | [API Reference](api-reference.md) | Public API documentation with error codes and behavior | ✅ Complete | | [Storage Layout](storage-layout.md) | Storage key structure and access patterns | ✅ Complete | +| [Storage Audit Map](storage-audit.md) | Comprehensive audit of storage keys and mutation trace | ✅ Complete | ## Invariants and Safety Properties @@ -82,6 +83,6 @@ This index organizes security-relevant documentation for external audit review. --- -**Last Updated**: 2026-01-27 +**Last Updated**: 2026-07-28 **Maintained by**: Core Development Team **Review Cadence**: Updated with each release diff --git a/docs/codebase-analysis/quality-and-debt.md b/docs/codebase-analysis/quality-and-debt.md new file mode 100644 index 0000000..b2cbd98 --- /dev/null +++ b/docs/codebase-analysis/quality-and-debt.md @@ -0,0 +1,340 @@ +# Quality And Debt Assessment + +This document assesses the codebase's quality, operational readiness, and +remaining technical debt based on the current repository state. + +## 1. Overall Assessment + +The repository is in a strong **development/testnet** state, but not in a +fully production-ready state. + +### Major strengths + +- Clear business domain: token-backed savings vault with time locks +- Stable typed error model using `#[contracterror]` +- Consistent auth boundaries on mutating paths +- Good event coverage across major state transitions +- Strong local Rust test coverage, including property-based invariants +- Explicit storage-versioning support and migration hook +- Good separation between global config and per-user persistent state + +### Major weaknesses + +- Runtime implementation remains concentrated in a single large `lib.rs` +- Several docs no longer match the actual code +- Automation/CI is too thin for the size of the test and docs surface +- Some read paths scale linearly with historical lock count +- Storage TTL management depends on operations outside the contract +- Mainnet governance and upgrade posture remain unresolved + +## 2. Validation And Tooling Posture + +### Verified during this analysis + +- `cargo test` completed successfully from the workspace root +- The registered Rust suite covers initialization, auth, accounting, pause, + events, storage versioning, rollback behavior, and property-based invariants + +### Tooling/process concerns + +- The only GitHub workflow in the repo triggers an external automation dispatch. +- There is no in-repo CI workflow for: + - `cargo test` + - `cargo fmt --check` + - `cargo clippy` + - WASM build validation +- The local `Makefile` only provides: + - `build-release` + - `wasm-size` +- Root docs still mention targets such as `make test`, `make build-wasm`, and + `make clean`, but those targets do not exist. + +## 3. Code Quality Findings + +### 3.1 Monolithic implementation file + +**Observation** + +`contracts/savings_vault/src/lib.rs` contains the full runtime implementation: +state types, storage keys, errors, helpers, admin logic, user flows, and read +models. + +**Impact** + +- Harder navigation for new contributors +- Higher chance of merge conflicts +- More difficult targeted review when changing one concern + +**Risk level** + +Medium maintainability risk, low immediate correctness risk. + +### 3.2 Documentation drift + +**Observation** + +Multiple docs still describe old behavior or abandoned designs. + +### Main drift patterns observed + +- Panic-string-only errors are described even though `ContractError` now exists. +- Several docs still describe `DataKey::Locks(user)` as the main storage model. +- Some docs still mention missing pause/event support although both are present. +- Root/project structure docs still reference `test.rs` instead of `src/test/`. +- Some doc links and absolute file references point to old local machine paths. + +**Impact** + +- Misleads auditors and new maintainers +- Makes architectural review slower than necessary +- Increases chance of implementation mistakes based on outdated guidance + +**Risk level** + +High maintainability/documentation risk. + +### 3.3 Orphaned TypeScript test + +**Observation** + +`tests/atomicity/transfer-atomicity.test.ts` references services under +`src/services/...`, but the repository contains no matching Node project, +package manifest, or service implementation. + +**Impact** + +- Confusing for maintainers +- Creates a false impression of broader cross-language coverage +- Adds noise during codebase comprehension + +**Risk level** + +Low runtime risk, medium maintainability risk. + +## 4. Performance Assessment + +### 4.1 Linear scans over historical lock ids + +**Observation** + +Several read and write paths iterate `1..next_lock_id`: + +- `get_balance_snapshot` +- `get_lock_summary` +- `get_locked_balance` +- `can_withdraw` +- `load_locks` +- `lock_funds` when recomputing `new_locked` for event payload + +**Impact** + +- Cost grows with the number of locks a user has ever created +- Historical withdrawn locks still contribute to iteration cost +- Heavy users can become expensive to query on-chain + +**Risk level** + +Medium-to-high performance risk if per-user lock count grows substantially. + +### 4.2 No lock-count cap + +**Observation** + +`list_locks` caps page size at 50, but the contract does not cap the total +number of locks a user may create. + +**Impact** + +- Read helpers remain exposed to unbounded historical growth +- Gas/resource consumption can become uneven across accounts + +**Risk level** + +Medium performance and anti-DoS risk. + +### 4.3 Event payload recomputation cost + +**Observation** + +After creating a lock, `lock_funds` recomputes the user's immature locked total +by scanning every historical lock id in order to emit `new_locked`. + +**Impact** + +- Extra work on every lock creation +- Event convenience comes at the cost of growing write-time complexity + +**Risk level** + +Medium performance inefficiency, not a correctness bug. + +## 5. Security Assessment + +### 5.1 Strong areas + +- User-facing mutations require `user.require_auth()` +- Admin-facing mutations require both signature and stored-admin match +- Withdrawals remain open during emergency pause +- Deposit/withdraw/withdraw_lock rollback behavior is tested +- Stable contract error codes improve SDK-side handling and analytics +- Token custody invariants are backed by property tests + +### 5.2 Main security risks and trust assumptions + +#### Single-admin control + +- One admin address controls pause, unpause, config setters, and admin transfer +- Fine for testnet/development +- Weak for mainnet governance without multi-sig or role separation + +#### TTL expiry risk + +- Persistent entries rely on Soroban TTL mechanics +- If operators fail to extend TTL, storage can disappear while SAC-held funds + still exist at the contract address +- This is primarily an operational safety risk, not a logic bug + +#### Token-behavior assumption + +- The contract assumes the configured token behaves like a normal SAC +- Fee-on-transfer or policy-heavy token behavior could break 1:1 accounting + +#### No upgrade path + +- Logic is effectively immutable once deployed +- Storage migration exists, but it is not a logic-upgrade framework + +#### No external audit + +- The repo explicitly targets development/testnet use +- No third-party audit artifact is published in the repository + +### 5.3 Policy gap worth clarifying + +**Observation** + +Admin-configured `MinLockDurationSecs` and `MaxLockDurationSecs` are enforced in +`lock_funds`, but `extend_lock` only checks: + +- lock exists +- lock is not withdrawn +- new time is in the future +- new time is strictly greater than the current unlock time + +**Why this matters** + +If the intended policy is "all active lock durations must respect current admin +limits", then `extend_lock` currently bypasses that policy. If the intended +policy is "limits apply only at creation time", the docs should state that more +explicitly. + +**Risk level** + +Medium policy/requirements risk. This is not automatically a bug, but it is a +behavioral ambiguity that should be resolved. + +## 6. Maintainability Assessment + +### 6.1 Dead or transitional storage concepts + +`DataKey::Locks(Address)` still exists, but live lock state is keyed by +`DataKey::Lock(user, lock_id)`. + +**Consequences** + +- Readers must mentally separate historical design from current design +- Docs easily drift when they rely on the obsolete vector model + +### 6.2 Large documentation surface + +The `docs/` directory is extensive and valuable, but it has become hard to keep +fully synchronized with the code. + +**Consequences** + +- Excellent breadth of topic coverage +- Elevated long-term maintenance cost +- Higher chance of contradictory statements across documents + +### 6.3 Mixed sources of truth + +For many topics, the actual source of truth is currently split between: + +- `lib.rs` +- test modules +- newer docs such as `error-codes.md` +- older docs that still describe superseded behavior + +That makes onboarding harder than it needs to be. + +## 7. Technical Debt Inventory + +### High priority debt + +1. Sync stale docs to the current contract implementation +2. Add real CI for test/lint/build verification +3. Decide mainnet posture for admin governance and upgrades +4. Clarify TTL operational ownership and failure handling + +### Medium priority debt + +1. Break `lib.rs` into internal modules without changing the public interface +2. Remove or formally deprecate `DataKey::Locks(Address)` +3. Clarify whether lock duration limits should apply to `extend_lock` +4. Revisit lock-scan costs for high-history users +5. Remove or relocate the orphaned TypeScript test + +### Low priority debt + +1. Expand task-runner convenience targets if docs continue to reference them +2. Consolidate overlapping docs with similar subject matter + +## 8. Recommended Next Steps + +### Short term + +1. Make `docs/comprehensive-analysis.md` and this package the canonical review entry point +2. Update stale high-traffic docs: + - `README.md` + - `architecture.md` + - `storage-audit.md` + - `SECURITY_REVIEW.md` + - any docs still centered on `Locks(user)` or panic strings +3. Add a GitHub Actions workflow for fmt, clippy, test, and release build + +### Medium term + +1. Split runtime code into internal modules such as: + - `state.rs` + - `errors.rs` + - `admin.rs` + - `funds.rs` + - `locks.rs` + - `reads.rs` +2. Decide whether lock-duration config should constrain lock extension +3. Introduce either: + - a lock-count cap, or + - more scalable aggregate accounting for large lock histories + +### Long term + +1. Choose a real mainnet governance model +2. Choose an upgrade or migration strategy +3. Add a formal operational runbook for TTL maintenance and custody monitoring +4. Pursue an external security audit after the above are stabilized + +## 9. Bottom Line + +The codebase demonstrates thoughtful contract design and unusually strong local +test coverage for a project of this size. Its main weaknesses are not basic +logic hygiene, but rather: + +- scaling characteristics of lock-history reads, +- incomplete operational hardening, +- governance/upgrade gaps for serious deployment, +- and a large amount of documentation drift created by rapid iteration. + +For development, learning, and testnet usage, the repository is in good shape. +For production or auditor handoff, the highest-return work is now on process, +documentation synchronization, and operational hardening rather than on +fundamental feature completeness. diff --git a/docs/codebase-analysis/repo-map-and-workflows.md b/docs/codebase-analysis/repo-map-and-workflows.md new file mode 100644 index 0000000..00970c7 --- /dev/null +++ b/docs/codebase-analysis/repo-map-and-workflows.md @@ -0,0 +1,374 @@ +# Repo Map And Workflows + +This document maps the repository's real implementation structure, the contract +surface area, and the principal runtime workflows. + +## 1. Repository Topology + +```text +pocketpay-contracts/ +|- Cargo.toml # Workspace manifest +|- Cargo.lock +|- README.md +|- Makefile # Minimal build/size targets +|- .env.example # Deployment/runtime variable examples +|- .github/workflows/ +| \- trigger-auto-merge.yml # PR automation only, not CI validation +|- contracts/ +| \- savings_vault/ +| |- Cargo.toml # Contract crate manifest +| |- README.md +| |- src/ +| | |- lib.rs # Entire contract implementation +| | \- test/ # Registered Rust unit/property tests +| |- proptest-regressions/ # Saved failing seeds for property tests +| \- test_snapshots/ # Snapshot artifacts for event/schema tests +|- docs/ # Architecture, security, testing, ops docs +|- scripts/ +| |- deploy-testnet.sh +| \- report-wasm-size.sh +\- tests/ + \- atomicity/ + \- transfer-atomicity.test.ts +``` + +## 2. Workspace And Dependency Layout + +### Workspace + +- Root `Cargo.toml` defines a workspace over `contracts/*`. +- The workspace uses release settings optimized for small WASM output: + - `opt-level = "z"` + - `lto = true` + - `panic = "abort"` + - `codegen-units = 1` + +### Contract Crate + +- Crate name: `savings-vault` +- Edition: Rust 2021 +- Output type: `cdylib` +- Runtime dependency: `soroban-sdk` +- Dev dependencies: + - `soroban-sdk` with `testutils` + - `proptest` + +## 3. Core Modules And Responsibilities + +Although the runtime code is in a single file, it is conceptually split into +distinct subsystems. + +### `contracts/savings_vault/src/lib.rs` + +#### State types + +- `LockEntry` + - Canonical per-lock record + - Tracks owner, amount, creation time, unlock time, and withdrawn state +- `BalanceSnapshot` + - Read model for unlocked/locked/total/withdrawable balances +- `LockSummary` + - Read model for counts, totals, and unlock-time ranges + +#### Storage schema + +- `DataKey` + - Enumerates every storage key used by the contract + - Separates global instance storage from user-specific persistent storage + +#### Error model + +- `ContractError` + - Stable `u32` error taxonomy grouped by category ranges: + - 1000s validation + - 2000s authorization + - 3000s lifecycle + - 4000s accounting + - 5000s lock handling + - 6000s storage/migration + - 7000s token + - 8000s admin rotation + +#### Internal helpers + +- Initialization guard +- Storage migration and version checks +- Admin identity validation +- Pause-state enforcement with lazy auto-unpause +- Historical lock reconstruction helper + +#### Public entrypoints + +- Lifecycle and metadata +- Pause and config administration +- User deposit/withdrawal flows +- Lock creation, extension, and withdrawal +- Read-only helpers for balances and locks +- Admin rotation + +## 4. Test Suite Structure + +The registered Rust test suite lives under `contracts/savings_vault/src/test/` +and is orchestrated by `mod.rs`. + +### Main categories + +- Initialization and storage versioning +- Auth and unauthorized access +- Balance conservation and isolation +- Lock lifecycle and replay protection +- Pause behavior and pause-state reads +- Event schema and event ordering +- Token-backed custody and rollback +- Property-based invariant testing + +### Important support assets + +- `test_helpers.rs` + - Shared fixture creation + - Token setup helpers + - Ledger timestamp control + - Mocked-auth and strict-auth environments +- `test_snapshots/` + - Event and behavior snapshots used as regression fixtures +- `proptest-regressions/` + - Persisted failing seeds for reproducing property-test issues + +## 5. On-Chain Storage Schema + +This repository has no SQL or document database. The storage schema is the +Soroban `DataKey` enum. + +### Instance storage keys + +| Key | Purpose | +| --- | --- | +| `Admin` | Current admin address | +| `Initialized` | One-time initialization guard | +| `Token` | Configured SAC token address | +| `StorageVersion` | Current storage version marker | +| `Paused` | Emergency pause flag | +| `PauseExpiry` | Timestamp when pause auto-expires | +| `MinDepositAmount` | Global deposit floor | +| `MaxLockDurationSecs` | Global max lock duration | +| `MinLockDurationSecs` | Global min lock duration | + +### Persistent storage keys + +| Key | Purpose | +| --- | --- | +| `Balance(Address)` | User's available balance | +| `Lock(Address, u64)` | User lock record keyed by owner and lock id | +| `NextLockId(Address)` | Next lock id counter for a user | + +### Transitional / confusing key + +| Key | Status | +| --- | --- | +| `Locks(Address)` | Declared in `DataKey` but not used as the primary live storage model | + +That unused variant is part of the repo's technical debt because many older docs +still describe it as the active schema. + +## 6. Contract Public Interface + +This is the effective API surface of the repository. + +### Lifecycle and metadata + +- `initialize(env, admin, token)` +- `get_version(env)` +- `get_token(env)` +- `get_admin(env)` + +### Pause and configuration + +- `pause(env, admin, duration_secs)` +- `unpause(env, admin)` +- `is_paused(env)` +- `set_min_deposit_amount(env, admin, min_amount)` +- `get_min_deposit_amount(env)` +- `set_max_lock_duration(env, admin, max_duration_secs)` +- `get_max_lock_duration(env)` +- `set_min_lock_duration(env, admin, min_duration_secs)` +- `get_min_lock_duration(env)` + +### User fund flows + +- `deposit(env, user, amount)` +- `withdraw(env, user, amount)` + +### Lock lifecycle + +- `lock_funds(env, user, amount, unlock_time)` +- `extend_lock(env, user, lock_id, new_unlock_time)` +- `withdraw_lock(env, user, lock_id)` + +### Read helpers + +- `get_balance(env, user)` +- `get_balance_snapshot(env, user)` +- `get_lock_summary(env, user)` +- `get_locked_balance(env, user)` +- `can_withdraw(env, user)` +- `get_lock(env, user, lock_id)` +- `list_locks(env, user, offset, limit)` + +### Admin rotation + +- `transfer_admin(env, admin, new_admin)` + +## 7. Event Surface + +The contract publishes events for major state transitions and admin changes. + +| Event name/topic 0 | Trigger | +| --- | --- | +| `initialize` | Initial contract setup | +| `deposit` | Successful deposit | +| `withdraw` | Successful available-balance withdrawal | +| `lock` | Successful lock creation | +| `extend_lock` | Successful lock extension | +| `withdraw_lock` | Successful matured lock withdrawal | +| `pause` | Pause activated | +| `unpause` | Pause cleared | +| `xferadmin` | Admin transferred | +| `cfg_min` | Min deposit rule updated | +| `cfg_maxlk` | Max lock duration updated | +| `cfg_minlk` | Min lock duration updated | + +Events use Soroban topics for indexing and tuples/scalars as payloads. Topic 0 +is always the action name, while topic 1 is usually the user or admin address. + +## 8. Interaction Mechanisms Between Components + +At runtime, the system interaction model is: + +1. A wallet, SDK, CLI user, or test invokes a `SavingsVault` method. +2. Soroban enforces host auth when the method calls `require_auth()`. +3. The contract reads/writes Soroban instance or persistent storage. +4. Custody-sensitive flows call the configured SAC token contract. +5. The contract emits events describing the state transition. + +### Component relationship map + +- **Wallet / SDK / Soroban CLI** + - Builds the transaction + - Provides auth for the user or admin address +- **SavingsVault contract** + - Validates lifecycle, auth, and business rules + - Owns the accounting model + - Coordinates storage and event emission +- **SAC token contract** + - Performs the actual token transfer + - Represents the source of real custody +- **Indexers / mobile apps** + - Read contract state directly + - Or reconstruct state from events off-chain + +## 9. Core Workflows + +### Initialize + +1. Deployer/admin calls `initialize(admin, token)`. +2. Contract rejects repeat initialization. +3. Admin signature is required. +4. Global config is written to instance storage. +5. Storage version is set. +6. `initialize` event is emitted. + +### Deposit + +1. Contract verifies initialized state, migration state, storage version, and + pause status. +2. User signature is required. +3. Amount and configured minimum are validated. +4. SAC transfer moves tokens from user to contract. +5. Internal `Balance(user)` is incremented. +6. `deposit` event is emitted. + +### Withdraw + +1. Contract verifies initialized state and storage version. +2. User signature is required. +3. Available balance is checked against `Balance(user)`. +4. SAC transfer moves tokens from contract to user. +5. Internal `Balance(user)` is decremented. +6. `withdraw` event is emitted. + +### Lock funds + +1. Contract verifies initialized state, version, and pause status. +2. User signature is required. +3. Amount and unlock-time rules are validated. +4. Global min/max lock-duration rules are enforced on creation. +5. Available balance is checked. +6. `NextLockId(user)` is incremented. +7. `Lock(user, next_id)` is written. +8. `Balance(user)` is decremented. +9. Locked total is recomputed for the emitted payload. +10. `lock` event is emitted. + +### Extend lock + +1. Contract verifies initialized state, version, and pause status. +2. User signature is required. +3. Lock existence and non-withdrawn status are validated. +4. `new_unlock_time` must be in the future and strictly greater than the + current unlock time. +5. Lock entry is updated in place. +6. `extend_lock` event is emitted. + +### Withdraw matured lock + +1. Contract verifies initialized state and storage version. +2. User signature is required. +3. Lock is loaded by `(user, lock_id)`. +4. Contract verifies the lock exists, is not already withdrawn, and is mature. +5. SAC transfer moves the lock amount from contract to user. +6. Lock is marked withdrawn and its amount is zeroed. +7. `withdraw_lock` event is emitted. + +### Pause / unpause + +1. Admin-only methods require both `require_auth()` and an admin-role match. +2. `pause` writes `Paused = true` and sets `PauseExpiry`. +3. `unpause` clears both fields immediately. +4. Mutating non-withdrawal methods call `require_not_paused()`. +5. Expired pauses are cleared lazily on the next mutating call. + +## 10. Data Flow Summary By Responsibility + +### User funds + +- Real token movement is handled by SAC. +- Logical fund partitioning is handled by: + - `Balance(user)` for unlocked funds + - `Lock(user, id)` for time-locked funds + +### Read models + +- `get_balance` returns only available balance. +- Matured locks remain in lock storage until individually withdrawn. +- `get_balance_snapshot` and `get_lock_summary` aggregate across lock history. + +### Error propagation + +- Business-rule failures become typed `ContractError` values. +- Host-level auth failures remain Soroban auth failures rather than contract + error codes. + +## 11. What This Repo Does Not Contain + +To avoid over-generalizing this project into a traditional web system: + +- No HTTP controllers or API routes +- No database migrations +- No background workers +- No message queues +- No frontend application +- No off-chain service implementation for SDK orchestration + +The closest off-chain integration guidance lives in docs such as +`sdk-contract-sequence.md`, `invocation-examples.md`, and `read-models.md`, but +those are consumer-facing documents, not implemented services. diff --git a/docs/comprehensive-analysis.md b/docs/comprehensive-analysis.md index 19d0ce8..4560496 100644 --- a/docs/comprehensive-analysis.md +++ b/docs/comprehensive-analysis.md @@ -1,211 +1,200 @@ -# Comprehensive Codebase Analysis Report -## PocketPay Savings Vault Contract - ---- -## 1. System Architecture and Directory Structure -### Directory Structure -``` -pocketpay-contracts/ -├── .github/ -│ └── workflows/ -│ └── trigger-auto-merge.yml # GitHub Actions CI/CD -├── contracts/ -│ └── savings_vault/ # Main contract crate -│ ├── Cargo.toml # Crate config -│ ├── src/ -│ │ ├── lib.rs # Contract implementation -│ │ └── test/ # Unit & property tests -│ │ ├── mod.rs -│ │ ├── admin_invariant_guard.rs -│ │ ├── balance_conservation.rs -│ │ ├── initialization.rs -│ │ ├── lock_read_helpers.rs -│ │ ├── maximum_amount_boundary.rs -│ │ ├── property_fee_invariants.rs -│ │ ├── property_vault_accounting.rs -│ │ ├── replay_protection.rs -│ │ ├── test_helpers.rs -│ │ ├── unauthorized_access.rs -│ │ ├── withdraw_lock.rs -│ │ └── zero_duration_lock.rs -│ └── test_snapshots/ # Snapshots for tests -├── docs/ # Comprehensive documentation -│ ├── accounting-invariants.md -│ ├── admin-role.md -│ ├── architecture.md -│ ├── audit-readiness.md -│ ├── balance-reconciliation.md -│ ├── contract-id-handoff.md -│ ├── deployment-environments.md -│ ├── deployment-output-example.md -│ ├── error-codes.md -│ ├── events.md -│ ├── pause-design.md -│ ├── storage-ttl.md -│ ├── troubleshooting.md -│ ├── upgrade-strategy.md -│ └── comprehensive-analysis.md -├── .env.example # Environment variable template -├── Cargo.toml # Rust workspace root config -└── Makefile # Task runner (build, test, size) -``` - -### System Architecture Paradigm -- **Monolithic smart contract**: Single Rust crate compiled to WASM -- **Soroban (Stellar) blockchain platform**: On-chain execution -- **On-chain state only**: No off-chain databases; all state stored via Soroban persistent/instance storage -- **Core modules**: - 1. **Initialization**: `initialize` function, state checks - 2. **Token Custody**: SAC integration via `soroban_sdk::token::Client` - 3. **Accounting**: Balance/lock management, `get_balance`, `get_locked_balance` - 4. **Time-Based Logic**: Unlock time checks, `can_withdraw` - 5. **Authorization**: `require_auth()` for all state-changing operations - 6. **Events**: On-chain event emission for all state changes - ---- -## 2. Component Functionality and Tech Stack -### Tech Stack -| Category | Technology | Version/Purpose | -|----------|------------|-----------------| -| Language | Rust | 2021 Edition | -| Blockchain | Soroban (Stellar) | Smart contract platform | -| Primary Dependency | soroban-sdk | 22.0.0 (provides env, storage, auth, tokens, testutils) | -| Compilation Target | WASM | `wasm32-unknown-unknown` | -| Build Tool | Cargo | Rust package manager | -| Task Runner | Make | Build/test shortcuts | -| CI/CD | GitHub Actions | Triggered on PR merge | -| Property Testing | proptest | Randomized operation sequence testing | - -### Key Files and Their Purpose -| File | Purpose | -|------|---------| -| [lib.rs](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/lib.rs) | Main contract implementation, all public functions | -| [test/property_vault_accounting.rs](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/test/property_vault_accounting.rs) | Property-driven tests for accounting invariants and global token custody | -| [test/admin_invariant_guard.rs](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/test/admin_invariant_guard.rs) | Tests for admin role isolation | -| [test/withdraw_lock.rs](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/test/withdraw_lock.rs) | Tests for `withdraw_lock` function | -| [test/maximum_amount_boundary.rs](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/test/maximum_amount_boundary.rs) | Tests for large amount (near i128 MAX) handling | -| [Cargo.toml (workspace)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/Cargo.toml) | Workspace config, soroban-sdk dependency | -| [Cargo.toml (contract)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/Cargo.toml) | Contract-specific dependencies (proptest in dev) | - ---- -## 3. Core Business Logic and Data Flows -### Public Contract Functions -| Function | Purpose | -|----------|---------| -| [initialize(env, admin, token)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/lib.rs#L248) | Initialize contract with admin address and token SAC | -| [get_version(env)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/lib.rs#L300) | Return contract version string ("0.1.0") | -| [deposit(env, user, amount)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/lib.rs#L333) | Deposit tokens to user's vault | -| [withdraw(env, user, amount)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/lib.rs#L413) | Withdraw tokens from user's vault | -| [withdraw_lock(env, user, lock_id)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/lib.rs#L538) | Withdraw tokens from a specific matured lock | -| [get_balance(env, user)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/lib.rs#L629) | Query user's available (unlocked) balance | -| [lock_funds(env, user, amount, unlock_time)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/lib.rs#L689) | Lock user's available funds until a future time | -| [get_locked_balance(env, user)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/lib.rs#L805) | Query user's locked (unmatured) balance | -| [can_withdraw(env, user)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/lib.rs#L862) | Check if user has matured locks available to withdraw | -| [get_lock(env, user, lock_id)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/lib.rs#L895) | Get a single lock entry by lock ID | -| [list_locks(env, user, offset, limit)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/lib.rs#L921) | List user's locks with pagination (max 50 per page) | -| [get_admin(env)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/lib.rs#L961) | Get current admin address | -| [transfer_admin(env, admin, new_admin)](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/contracts/savings_vault/src/lib.rs#L985) | Transfer admin privileges to new address | - -### Critical User Journeys -#### Journey 1: Initialize Contract -1. Admin calls `initialize(env, admin, token)` -2. `admin.require_auth()` verifies admin signature -3. Check if `Initialized` is already set → panic if true -4. Store `Admin`, `Initialized`, `Token`, and `StorageVersion` in **instance storage** -5. Emit `(symbol_short!("init"), admin)` event with token as value - -#### Journey 2: Deposit Tokens -1. User calls `deposit(env, user, amount)` -2. `assert_initialized()` and `assert_supported_storage_version()` pass -3. `user.require_auth()` verifies user signature -4. Validate `amount > 0` → panic if not -5. Retrieve SAC token address and create `token::Client` -6. `token_client.transfer(user, contract_address, amount)` moves tokens to contract -7. Update user's `Balance(user)` (persistent storage) by adding `amount` -8. Emit `(symbol_short!("deposit"), user)` event with `(amount, new_balance)` as value - -#### Journey 3: Withdraw Tokens -1. User calls `withdraw(env, user, amount)` -2. `assert_initialized()`, `assert_supported_storage_version()`, and `user.require_auth()` pass -3. Validate `amount > 0` -4. Calculate available balance = deposited `Balance(user)` + sum of matured `LockEntry.amount` (where `current_time >= unlock_time`) -5. Panic if `amount > available` -6. `token_client.transfer(contract_address, user, amount)` sends tokens to user -7. Subtract amount from `Balance(user)` first, then from matured locks if needed -8. Update `Balance(user)` and `Locks(user)` in persistent storage -9. Emit `(symbol_short!("withdraw"), user)` event with `(amount, new_balance, new_locked)` as value - -#### Journey 4: Lock Funds -1. User calls `lock_funds(env, user, amount, unlock_time)` -2. `assert_initialized()`, `assert_supported_storage_version()`, and `user.require_auth()` pass -3. Validate `amount > 0`, `unlock_time > env.ledger().timestamp()`, and `amount <= available balance` -4. Retrieve `NextLockId(user)` (default to 1 if not set) -5. Create new `LockEntry { id, amount, unlock_time }` and add to `Locks(user)` -6. Subtract `amount` from `Balance(user)` -7. Update `Balance(user)`, `Locks(user)`, and `NextLockId(user)` (increment by 1) in persistent storage -8. Emit `(symbol_short!("lock"), user)` event with `(amount, unlock_time, new_balance, new_locked)` as value - -#### Journey 5: Withdraw a Specific Lock -1. User calls `withdraw_lock(env, user, lock_id)` -2. `assert_initialized()` and `user.require_auth()` pass -3. Load user's locks and find lock by ID → panic if not found -4. Verify lock is matured → panic if not -5. `token_client.transfer(contract_address, user, lock.amount)` sends tokens to user -6. Remove lock entry from `Locks(user)` -7. Update `Locks(user)` in persistent storage -8. Emit `(Symbol::new(env, "withdraw_lock"), user)` event with `(lock_id, amount)` as value - ---- -## 4. Coding Standards, Auth, and Data Validation -### Coding Standards -- Follow Rust idioms and Soroban best practices -- Comprehensive inline doc comments for all public functions -- Clear separation of concerns (initialization, deposits, withdrawals, locking, queries) -- **No custom error enum**: Uses panic strings for errors -- **Events emitted**: All state changes emit on-chain events! - -### Authorization -- **`initialize`**: Requires admin address authorization -- **`transfer_admin`**: Requires current admin address authorization -- **All state-changing user functions** (`deposit`, `withdraw`, `withdraw_lock`, `lock_funds`): Require user address authorization via `Address::require_auth()` -- **Read-only functions** (`get_balance`, `get_locked_balance`, `can_withdraw`, `get_lock`, `list_locks`, `get_version`, `get_admin`): No authorization needed (public queries) - -### Data Validation -- **Amount checks**: All functions accepting an amount panic if `amount <= 0` -- **Balance checks**: Withdraw and lock panic if amount exceeds available balance -- **Time checks**: `lock_funds` panics if `unlock_time <= current_time`; `withdraw_lock` panics if lock not matured -- **Initialization checks**: All functions except `initialize` panic if called before contract is initialized -- **Storage version check**: Most functions panic if storage version doesn't match `STORAGE_VERSION` (1) - ---- -## 5. External Integrations and Dependencies -### External Integrations -- **Stellar Asset Contract (SAC)**: Used via `soroban_sdk::token::Client` for token transfers in `deposit`, `withdraw`, and `withdraw_lock` -- **Soroban Network**: - - Testnet: `https://soroban-testnet.stellar.org:443` - - Passphrase: `Test SDF Network ; September 2015` - -### Environment-Dependent Configurations -- `.env.example`: Defines `VAULT_CONTRACT_ID`, `SOROBAN_RPC_URL`, `SOROBAN_NETWORK_PASSPHRASE` -- [deployment-environments.md](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/docs/deployment-environments.md) has full environment docs - ---- -## 6. Summary Report -### Key Architectural Decisions -1. **SAC Integration**: Real token custody implemented (internal accounting reconciled with SAC balance) -2. **Per-User Lock Entries**: Multiple locks per user, each with unique ID and independent unlock time -3. **Atomic Execution**: Soroban transactions are atomic, so failed operations leave no state changes -4. **Separate Instance/Persistent Storage**: Instance storage for admin/init/token/version; persistent storage for user data -5. **On-Chain Events**: All state changes emit events for off-chain tracking -6. **Comprehensive Property Testing**: proptest covers thousands of randomized operation sequences - -### Technical Debt -1. **No custom error enum**: Uses panic strings, which are harder for off-chain SDKs to handle consistently -2. **No pause/emergency stop mechanism**: Research exists in [pause-design.md](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/docs/pause-design.md), but not implemented -3. **No upgrade path**: Research exists in [upgrade-strategy.md](file:///c:/Users/muham/.trae/Grantfox%20Coder%20x/pocketpay-contracts/docs/upgrade-strategy.md), but not implemented - -### Addressable Edge Cases Now Covered -- ✅ **Global token custody invariant**: Tested in `property_vault_accounting::prop_global_token_custody` -- ✅ **Large amount handling**: Tested in `maximum_amount_boundary.rs` -- ✅ **User isolation**: Tested in `property_vault_accounting::prop_cross_user_isolation` -- ✅ **Admin isolation**: Tested in `admin_invariant_guard.rs` +# Comprehensive Codebase Analysis Package + +This package is the current, implementation-aligned entry point for understanding +the PocketPay Savings Vault repository. It supersedes earlier analysis notes +that were written before the current error model, pause logic, and token-backed +custody flow were fully implemented. + +## Scope + +- **Repository type:** Rust workspace for a single Soroban smart contract crate +- **Primary artifact:** `contracts/savings_vault` compiled to WASM +- **Current runtime model:** On-chain only; there is no application server, no + REST/GraphQL API surface, and no operational database schema in this repo +- **Source of truth:** `contracts/savings_vault/src/lib.rs`, crate manifests, + and the registered Rust test suite under `contracts/savings_vault/src/test/` + +## Package Contents + +- [Repo Map And Workflows](codebase-analysis/repo-map-and-workflows.md) + - Directory structure + - Core modules and responsibilities + - Storage schema and event surface + - End-to-end workflows and interaction mechanisms +- [Quality And Debt Assessment](codebase-analysis/quality-and-debt.md) + - Code quality and test posture + - Performance, security, and maintainability findings + - Technical debt inventory + - Prioritized recommendations + +## Executive Summary + +The repository is best understood as a **single-contract custody system** built +for the Stellar Soroban platform: + +- **One contract, one crate, one runtime unit** + - The `SavingsVault` contract is implemented in a single `lib.rs` file. + - Logical separation exists inside that file through sections for state, + storage keys, errors, helpers, admin/configuration, fund movement, read + models, and lock management. +- **Token-backed accounting** + - Internal balances are bookkeeping only. + - Real custody is enforced through the configured Stellar Asset Contract + (SAC), with token transfers executed in `deposit`, `withdraw`, and + `withdraw_lock`. +- **Per-user sharded storage** + - Global configuration lives in instance storage. + - User balances and lock records live in persistent storage keyed by address. + - Each lock is stored independently as `DataKey::Lock(user, lock_id)`. +- **Safety-oriented business rules** + - Mutating user operations require `require_auth()`. + - Emergency pause blocks new deposits and locks, but always leaves user exits + (`withdraw`, `withdraw_lock`) available. + - Errors are exposed through a stable `#[contracterror]` enum rather than + brittle panic strings. +- **Strong local test coverage** + - The repo contains a broad Rust test suite with focused behavior tests, + event schema regression tests, and property-based accounting checks. + +## Core Architectural Patterns + +### 1. Monolithic Contract, Segmented Internals + +The implementation is monolithic at the file/module level, but internally +organized around consistent responsibility zones: + +- Contract state types and storage keys +- Stable error taxonomy +- Initialization and migration helpers +- Admin and pause controls +- Token custody flows +- Lock lifecycle management +- Read models for SDK/mobile consumption + +### 2. Transfer-First Mutation Ordering + +The contract intentionally performs token transfers before mutating internal +storage in custody-sensitive flows: + +- `deposit`: transfer user -> contract, then credit balance +- `withdraw`: transfer contract -> user, then debit balance +- `withdraw_lock`: transfer contract -> user, then mark lock withdrawn + +On Soroban this ordering is acceptable because failed host calls roll back the +entire transaction, and the accompanying tests explicitly validate zero state +drift on failed transfers. + +### 3. Read Models Over Raw Storage + +The contract exposes raw reads (`get_balance`, `get_lock`, `list_locks`) and +higher-level aggregated reads (`get_balance_snapshot`, `get_lock_summary`). +This reduces off-chain SDK work for common screens, but creates a tradeoff: +some read helpers linearly scan a user's historical lock IDs. + +### 4. Operationally Enforced Storage Lifecycle + +The contract models storage versioning in code, but storage TTL renewal is an +operational concern rather than a self-healing in-contract mechanism. That is a +key part of the repository's technical debt and production-readiness gap. + +## Interface Inventory + +### On-chain Public Interface + +The contract exposes 23 public entry points grouped into: + +- Lifecycle and metadata +- Admin and configuration +- Pause controls +- User custody flows +- Lock lifecycle flows +- Read helpers and aggregated read models + +See the detailed inventory in +[Repo Map And Workflows](codebase-analysis/repo-map-and-workflows.md). + +### Events + +The contract emits events for all major state transitions, including: + +- `initialize` +- `deposit` +- `withdraw` +- `lock` +- `extend_lock` +- `withdraw_lock` +- `pause` +- `unpause` +- `xferadmin` +- Configuration updates: `cfg_min`, `cfg_maxlk`, `cfg_minlk` + +### API Endpoints + +There are **no HTTP API endpoints** implemented in this repository. + +- No REST handlers +- No GraphQL schema +- No web server +- No RPC service owned by this repo + +The effective API surface is the Soroban contract method set exposed by +`SavingsVault`. + +### Database Schemas + +There is **no application database schema** in this repository. + +- No SQL migrations +- No ORM models +- No Supabase/Postgres/MySQL schema +- No persistence layer outside Soroban storage + +The closest equivalent to a "schema" is the on-chain `DataKey` storage model +documented in `lib.rs` and summarized in the repo-map document. + +## Dependencies And Integrations + +### Direct Build-Time Dependencies + +- `soroban-sdk = 22.0.0` +- `proptest = 1` for dev/test only + +### Third-Party Runtime / Operational Integrations + +- **Stellar Soroban runtime** + - Contract execution environment + - Ledger timestamp source + - Authentication host for `require_auth()` +- **Stellar Asset Contract (SAC)** + - Token transfer and custody backend +- **Soroban CLI** + - Deployment and invocation tooling +- **Stellar testnet ecosystem** + - Friendbot + - Testnet RPC + - Explorer workflows +- **GitHub workflow dispatch** + - PR automation via `.github/workflows/trigger-auto-merge.yml` + +## Important Current Findings + +- The repository has evolved faster than some of its docs. +- Several existing documents still describe removed or superseded behavior: + - panic-string-only error handling + - vector-based `Locks(user)` storage as the primary model + - missing pause/events despite those features now existing + - outdated task-runner targets and old file paths +- A standalone TypeScript test exists under `tests/atomicity/`, but it does not + match the current Rust workspace structure and appears disconnected from the + actual build/test toolchain. + +## Validation Performed For This Package + +- Reviewed the full workspace layout, manifests, scripts, docs, and contract + implementation +- Mapped the full public contract interface and event surface +- Reviewed the registered Rust test suite and supporting fixtures +- Ran `cargo test` successfully from the workspace root during this analysis + +For the detailed technical debt and risk review, continue to +[Quality And Debt Assessment](codebase-analysis/quality-and-debt.md). diff --git a/docs/error-codes.md b/docs/error-codes.md index 102550a..910be91 100644 --- a/docs/error-codes.md +++ b/docs/error-codes.md @@ -1,216 +1,262 @@ -# Savings Vault Error Reference - -This reference describes error behavior in `contracts/savings_vault/src/lib.rs`. -The contract defines a custom error enum with stable numeric error codes via -the `#[contracterror]` attribute. See [error-code-standard.md](./error-code-standard.md) -for the complete error code standard and SDK mapping guidance. - -SDK and mobile callers should use the numeric error codes for reliable error -handling and display user-friendly messages based on the error category. - -## Configuration errors (1000-1999) - -### `AlreadyInitialized` (Code: 1001) - -- **Current failure:** Returns `ContractError::AlreadyInitialized` from `initialize`. -- **Meaning:** The one-time initialization flag already exists. -- **Likely cause:** A repeated initialization, a retry after success, or the - wrong contract ID. -- **Caller/developer action:** Do not retry. Confirm the contract ID and use the - existing deployment; this is not a transient network failure. - -### `NotInitialized` (Code: 1002) - -- **Current failure:** Returns `ContractError::NotInitialized` when attempting operations - that require initialization. -- **Meaning:** The contract has not been initialized. -- **Likely cause:** Operations called before initialization or instance storage unavailable. -- **Caller/developer action:** Ensure initialization succeeded before enabling - vault operations. - -## Validation errors (2000-2999) - -### `InvalidDepositAmount` (Code: 2001) - -- **Current failure:** Returns `ContractError::InvalidDepositAmount` from `deposit`. -- **Meaning:** The deposit amount is zero or negative. -- **Likely cause:** Invalid input, unit conversion, sign handling, or an empty - field converted to zero. -- **Caller/developer action:** Require a positive `i128` amount in the token's - smallest unit before invoking the contract. - -### `InvalidWithdrawAmount` (Code: 2002) - -- **Current failure:** Returns `ContractError::InvalidWithdrawAmount` from `withdraw`. -- **Meaning:** The withdrawal amount is zero or negative. -- **Likely cause:** Invalid input or an amount-conversion bug. -- **Caller/developer action:** Reject non-positive amounts before submission. - -### `InvalidLockAmount` (Code: 2003) - -- **Current failure:** Returns `ContractError::InvalidLockAmount` from `lock_funds`. -- **Meaning:** The lock amount is zero or negative. -- **Likely cause:** Invalid input or an amount-conversion bug. -- **Caller/developer action:** Require a positive amount before submission. - -### `InvalidUnlockTime` (Code: 2004) - -- **Current failure:** Returns `ContractError::InvalidUnlockTime` from `lock_funds`. -- **Meaning:** `unlock_time` is less than or equal to the current ledger - timestamp; it must be strictly later when executed. -- **Likely cause:** A past timestamp, seconds/milliseconds confusion, clock skew, - or submission too close to the selected time. -- **Caller/developer action:** Send Unix time in **seconds** and leave a safety - margin beyond the latest ledger time. - -## Balance errors (4000-4999) - -These checks use the vault's **available internal balance**, not the wallet -balance or locked balance. - -### `InsufficientBalance` (Code: 4001) - -- **Current failure:** Returns `ContractError::InsufficientBalance` from `withdraw`. -- **Meaning:** The withdrawal exceeds the available internal balance; a missing - balance is treated as zero. -- **Likely cause:** The request is too large, no deposit is recorded, or some - balance was moved to the locked bucket. -- **Caller/developer action:** Refresh `get_balance(user)`, cap the request to - that value, and explain that locked funds are unavailable. - -### `InsufficientBalanceToLock` (Code: 4002) - -- **Current failure:** Returns `ContractError::InsufficientBalanceToLock` from `lock_funds`. -- **Meaning:** The lock amount exceeds the available internal balance. -- **Likely cause:** A stale displayed balance, an excessive request, or funds - already moved to the locked bucket. -- **Caller/developer action:** Refresh `get_balance(user)` and allow no more than - the returned available amount. - -### `FundsLockedUntilMaturity` (Code: 4003) - -- **Current failure:** Returns `ContractError::FundsLockedUntilMaturity` from `withdraw`. -- **Meaning:** The withdrawal amount exceeds the available balance and would - require withdrawing from immature (unmatured) locked funds. This is a specific - error that occurs when the user has locked funds that have not yet reached - their unlock time. -- **Likely cause:** The user attempted to withdraw more than their available - (unlocked) balance, and the shortfall would need to come from locked funds - that are still immature (current_time < unlock_time). -- **Caller/developer action:** Check `get_balance(user)` to see available funds - and `get_locked_balance(user)` to see locked funds. Only withdraw up to the - available balance. Wait for locks to mature (check with `can_withdraw(user)`) - before attempting to withdraw locked funds. - -## Authorization errors (3000-3999) - -### `Unauthorized` (Code: 3001) - -- **Current failure:** Soroban host authorization failure from `require_auth()`; - the contract defines this error for documentation purposes, but the actual - failure comes from the Soroban host. -- **Meaning:** Valid authorization for the required address is absent. -- **Likely cause:** `initialize` lacks `admin` authorization, or `deposit`, - `withdraw`, or `lock_funds` lacks `user` authorization. The app may be trying - to act for another address. -- **Caller/developer action:** Build and sign with the required address. Do not - retry unchanged; request the correct wallet signature. - -Read-only calls (`get_balance`, `get_locked_balance`, and `can_withdraw`) do not -call `require_auth()`. - -## Lock and unlock time behavior - -**Zero-duration locks:** Passing `unlock_time == current ledger timestamp` -(a zero-second duration) is rejected with this same panic, because the check -is `unlock_time <= current_time`, not `<`. There is no way to create a lock -that is already matured at creation time; the smallest valid duration is one -second (`unlock_time == current_time + 1`), and funds locked that way remain -locked until the ledger timestamp advances to that value — `can_withdraw` -and `get_balance` still treat it as locked at the moment of creation. - -### `Contract is paused` - -- **Current failure:** Panic message from `deposit` and `lock_funds`. -- **Meaning:** The contract is in an emergency pause state. Deposits and lock - operations are blocked. Withdrawals (`withdraw`, `withdraw_lock`) and - read-only queries remain available. -- **Likely cause:** The admin activated a pause for an incident response. -- **Caller/developer action:** Check `is_paused()` to confirm. If the pause - has an expiry, wait for it to expire. Otherwise, the admin must call - `unpause()` to restore normal operations. Users can still withdraw funds - during a pause. - -### `Pause duration must be greater than zero` - -- **Current failure:** Panic message from `pause`. -- **Meaning:** The `duration_secs` argument to `pause()` was zero. A pause - must have a non-zero duration to ensure it auto-expires. -- **Likely cause:** Invalid input or an accidental zero value. -- **Caller/developer action:** Pass a positive duration in seconds (e.g., - 604800 for 7 days). - -### Locked funds are not yet withdrawable - -- **Current condition:** `can_withdraw(user)` returns `false`; it does not fail. -- **Meaning:** No locked funds exist, or the ledger timestamp is earlier than - the unlock time. At exactly the unlock timestamp it returns `true`. -- **Likely cause:** The lock has not matured or no lock exists. -- **Caller/developer action:** Treat `false` as normal state and disable the - action. The current contract has no operation to release or withdraw locked - funds; `can_withdraw` is only a query. - -### `Cannot withdraw: funds are locked until maturity` - -- **Current failure:** Panic message from `withdraw`. -- **Meaning:** The withdrawal amount exceeds the available balance and would - require withdrawing from immature (unmatured) locked funds. This is a specific - error that occurs when the user has locked funds that have not yet reached - their unlock time. -- **Likely cause:** The user attempted to withdraw more than their available - (unlocked) balance, and the shortfall would need to come from locked funds - that are still immature (current_time < unlock_time). -- **Caller/developer action:** Check `get_balance(user)` to see available funds - and `get_locked_balance(user)` to see locked funds. Only withdraw up to the - available balance. Wait for locks to mature (check with `can_withdraw(user)`) - before attempting to withdraw locked funds. - -## Unauthorised access errors - -### `LockNotFound` (Code: 5001) - -- **Current failure:** Reserved for future use. -- **Meaning:** No lock found for the specified lock ID. -- **Likely cause:** Invalid lock ID or lock has been consumed. -- **Caller/developer action:** Verify lock ID and check lock status. - -Read-only calls (`get_balance`, `get_locked_balance`, `get_lock`, `list_locks`, -and `can_withdraw`) do not call `require_auth()`. - -## Other existing failure conditions - -### Token transfer failure during withdrawal - -- **Current failure:** Error or trap propagated by the configured token - contract; the vault defines no wrapper error. -- **Meaning:** The token transfer from the vault contract to the user failed. -- **Likely cause:** Insufficient real token balance, an invalid or incompatible - token address, token authorization failure, or token-contract rejection. An - internal balance does not guarantee matching tokens are held. -- **Caller/developer action:** Inspect the nested token diagnostic. Verify the - configured token and vault token balance; do not label this only as an - internal-balance error. +# Savings Vault Error Reference (Canonical) + +This reference documents the **real, contract-defined `ContractError` enum** +in [`contracts/savings_vault/src/lib.rs`](../contracts/savings_vault/src/lib.rs). +The contract uses `#[contracterror]` with a `#[repr(u32)]` discriminant, so +errors are exposed to SDKs and mobile clients as **stable `u32` codes** — not +as arbitrary panic strings. + +> **Breaking-change contract.** Numeric codes in this file are part of the +> cross-repo SDK interface. Any code renumber here MUST be version-bumped and +> co-ordinated with SDK + mobile releases. See the mapping guidance in +> [`sdk-error-mapping-guide.md`](./sdk-error-mapping-guide.md). + +## Category Ranges + +| Range | Category | Examples | +| --- | --- | --- | +| **1001–1099** | **Validation** | Bad amounts, bad timestamps, bad durations | +| **2001–2099** | **Authorisation** | Wrong caller role (non-admin calling admin-only) | +| **3001–3099** | **Lifecycle** | Initialization, pause state | +| **4001–4099** | **Accounting** | Insufficient available balance | +| **5001–5099** | **Locks** | Missing lock, already withdrawn, immature, unchanged extend | +| **6001–6099** | **Storage** | Schema version, missing required entry | +| **7001–7099** | **Token** | Token configuration issues | +| **8001–8099** | **Admin rotation** | Invalid new-admin address | + +## 1000s — Validation + +### `AmountNotPositive` (1001) + +- **Raised by:** `deposit`, `withdraw`, `lock_funds` +- **Meaning:** The submitted `amount` is `0` or negative. +- **Likely cause:** Empty field coerced to `0`, a sign bug, or a unit-conversion + bug between whole-token and stroops / the asset's decimal exponent. +- **Caller action:** Block submission client-side until `amount > 0`. Format the + error to show the asset's decimal representation, not the raw `i128` stroop + value. + +### `UnlockTimeNotInFuture` (1002) + +- **Raised by:** `lock_funds`, `extend_lock` +- **Meaning:** `unlock_time <= current ledger timestamp`. +- **Likely cause:** Past time, seconds / ms confusion, clock skew, or choosing a + time too close to submission. +- **Caller action:** Send Unix time in **seconds** and leave a safety margin + (≥ 30 s) above the last-seen ledger time. + +### `LockDurationExceedsMaximum` (1003) + +- **Raised by:** `lock_funds` +- **Meaning:** `unlock_time - now > max_lock_duration`. +- **Likely cause:** Wrong asset config read, or a UI picker allowing years + beyond the configured max. +- **Caller action:** Read `MaxLockDuration` from config (or its storage-backed + setter) and clamp the picker before submission. + +### `LockDurationBelowMinimum` (1004) + +- **Raised by:** `lock_funds` +- **Meaning:** `unlock_time - now < min_lock_duration`. +- **Likely cause:** UX allowing 1-second locks when min is e.g. 1 day. +- **Caller action:** Same as above — pre-clamp. + +### `AmountBelowMinimumDeposit` (1005) + +- **Raised by:** `deposit` +- **Meaning:** `amount < min_deposit_amount`. +- **Likely cause:** Asset decimal mismatch, or a UX not honouring the configured + floor. +- **Caller action:** Read the configured min and reject below it client-side. + +### `PauseDurationMustBePositive` (1006) + +- **Raised by:** `pause` (admin) +- **Meaning:** `duration_secs == 0` on the admin pause call. +- **Likely cause:** Accidental zero input. +- **Caller action:** Admin UI only; enforce a minimum (e.g. 1 hour) when + submitting. + +### `MinDepositAmountNegative` (1007) + +- **Raised by:** `set_min_deposit_amount` (admin) +- **Meaning:** Attempt to set the global min deposit to a negative `i128`. +- **Likely cause:** Admin-console sign bug. +- **Caller action:** Admin console validation. + +## 2000s — Authorisation + +### `NotAuthorizedAdmin` (2001) + +- **Raised by:** all admin-only entrypoints (`pause`, `unpause`, + `set_min_deposit_amount`, `set_max_lock_duration`, `set_min_lock_duration`, + `transfer_admin`) +- **Meaning:** The `require_auth`-verified caller does not match the stored + `Admin`. +- **Likely cause:** Wrong wallet connected, or trying to use a governance role + that was never transferred to. +- **Caller action:** Confirm the connected address is the current admin (use + `get_admin()`), or — for app backends — route through a signer that holds + the admin role. +- **Distinction from host auth failures:** Soroban's host-level auth failures + (e.g. signature missing) raise a host `Status(Auth, …)`; this code is a + contract-level **role check** that runs AFTER the host has confirmed the + caller signed. + +## 3000s — Lifecycle + +### `AlreadyInitialized` (3001) + +- **Raised by:** `initialize` +- **Meaning:** The `StorageVersion` flag already exists. +- **Likely cause:** A double-call to `initialize` (e.g. an infra retry after a + success that the caller didn't observe) or pointing at the wrong deployed + contract. +- **Caller action:** Do not retry. Use `get_version()` / `get_token()` to + confirm the contract is already live. + +### `NotInitialized` (3002) + +- **Raised by:** every guarded public entrypoint (`get_version`, `get_token`, + `pause`, `deposit`, `withdraw`, `lock_funds`, `list_locks`, `get_admin`, …) +- **Meaning:** The contract was deployed but `initialize` hasn't run. +- **Likely cause:** A deploy script that forgot the init call, or a race where + the UI renders operations before init lands on-chain. +- **Caller action:** Gate all vault UI behind a contract-ready check + (`get_version()` succeeds). + +### `ContractPaused` (3003) + +- **Raised by:** state-mutating non-withdrawal operations (`deposit`, + `lock_funds`, `extend_lock`, plus admin-setters if the team later chooses + to tighten them). `withdraw`, `withdraw_lock`, and reads remain **allowed**. +- **Meaning:** Emergency pause active and not yet expired. +- **Likely cause:** Admin-incident response. +- **Caller action:** Show an incident banner. Allow / encourage withdrawal; + block new deposits and lock creation. Use `is_paused()` to poll expiry. + +## 4000s — Accounting + +### `InsufficientBalance` (4001) + +- **Raised by:** `withdraw` +- **Meaning:** `amount > available_balance(user)`. Locked funds are NOT in + `available_balance`. +- **Likely cause:** Stale displayed balance, or the user is trying to withdraw + more than their unlocked funds. +- **Caller action:** Call `get_balance(user)` first, cap the submit, and show + "Locked balance is unavailable. Wait for it to mature with `can_withdraw`." + +### `InsufficientBalanceToLock` (4002) + +- **Raised by:** `lock_funds` +- **Meaning:** `amount > available_balance(user)`. +- **Semantic disambiguation from 4001:** Same underlying test, different + operation. SDKs SHOULD show a distinct copy: + - 4001 → "You don't have enough available to withdraw **X** tokens." + - 4002 → "You don't have enough unlocked balance to lock **X** tokens." + +## 5000s — Locks + +### `LockNotFound` (5001) + +- **Raised by:** `withdraw_lock`, `extend_lock` +- **Meaning:** No `Lock { amount, unlock_time, withdrawn }` stored under the + `DataKey::Lock(owner, id)` key. +- **Likely cause:** Stale lock ID, lock storage TTL expired and was not + bumped, or wrong owner (the lookup is scoped to the authenticated owner). +- **Caller action:** Re-fetch via `get_lock` / `list_locks`. If TTL expiry is + plausible, check storage TTL tooling. + +### `LockAlreadyWithdrawn` (5002) + +- **Raised by:** `withdraw_lock`, `extend_lock` +- **Meaning:** The lock's `withdrawn` boolean is `true`. +- **Likely cause:** UI double-submit after a success the user didn't see, or a + retry of a completed transaction. +- **Caller action:** Idempotent on the client side: if `get_lock(owner, id)` + says `withdrawn == true`, treat as success and don't re-submit. + +### `LockNotMatured` (5003) + +- **Raised by:** `withdraw_lock` +- **Meaning:** `now < lock.unlock_time`. +- **Likely cause:** UI enabled the "withdraw lock" button too early due to + local-clock skew, or the user manually forced the call. +- **Caller action:** Gate the button behind `can_withdraw(user)` AND a + per-lock `now >= unlock_time` check using the last-seen ledger timestamp, + not local device time. + +### `ExtendLockTimeNotIncreased` (5004) + +- **Raised by:** `extend_lock` +- **Meaning:** `new_unlock_time <= lock.unlock_time`. +- **Likely cause:** UX that lets the user pick an earlier time when "extending". +- **Caller action:** Pre-clamp to `max(lock.unlock_time + 1, selection)`. + +## 6000s — Storage + +### `StorageVersionUnsupported` (6001) + +- **Raised by:** `try_migrate`, `assert_supported_storage_version` +- **Meaning:** On-chain storage has a version strictly greater than the code's + `CURRENT_STORAGE_VERSION` (downgrade / rollback attempt), OR migration code + cannot interpret the stored layout. +- **Likely cause:** Wrong WASM deployed (older version vs. newer storage), or + a failed upgrade path. +- **Caller action:** Escalate to contract deployment owners; do NOT auto-retry. + +### `RequiredStorageEntryMissing` (6002) + +- **Raised by:** paths that `.unwrap_or_else(|| RequiredStorageEntryMissing)` + a required instance-storage cell (e.g. `Admin`, `Token`). +- **Meaning:** Contract storage is internally inconsistent (a mandatory + singleton was never written or was dropped by an accidental TTL expiry). +- **Likely cause:** Deployment bug: `initialize` failed half-way, or an + admin-rotation code path omitted the write. In principle unreachable on a + correctly-initialized contract; presence of 6002 is ALERTS-level. +- **Caller action:** Pause the UI. Page on-call. + +## 7000s — Token + +### `TokenNotConfigured` (7001) + +- **Raised by:** SAC-custody paths if the `Token` storage cell is missing + (`deposit`, `withdraw`, `withdraw_lock`). +- **Meaning:** The configured asset address is not set; custody transfers can't + run. +- **Likely cause:** Corrupt initialization or migration. +- **Caller action:** Escalate. + +## 8000s — Admin Rotation + +### `CannotTransferAdminToSelf` (8001) + +- **Raised by:** `transfer_admin` +- **Meaning:** `new_admin == current_admin`. +- **Likely cause:** Form submitted with the same address. +- **Caller action:** Admin console validation; treat as no-op success if the + user intention was to "keep admin". + +### `CannotTransferAdminToContractAddress` (8002) + +- **Raised by:** `transfer_admin` +- **Meaning:** `new_admin == contract_address` (i.e. setting the vault itself + as its own admin, which would permanently orphan admin-only operations). +- **Likely cause:** Paste error selecting the contract instead of the signer. +- **Caller action:** Admin console guard that blacklists the contract's own + address. ## Error Code Stability -All error codes defined in the `ContractError` enum are stable and backward compatible: - -- Existing error codes will never change -- New error codes will be added within their category ranges -- Deprecated error codes will be marked in documentation but remain functional -- See [error-code-standard.md](./error-code-standard.md) for the complete standard +- Existing codes **will never be renumbered** within a major release line. +- New codes **will be added inside their category range** (1001–1099, …) to + keep SDK route-by-thousand-category logic correct. +- Deprecated codes **will keep their number**; deprecation is signalled via + variant docs only. +- See [`error-code-standard.md`](./error-code-standard.md) for design rationale. ## SDK Integration -For SDK mapping guidance and mobile UX recommendations, see -[error-code-standard.md](./error-code-standard.md). +For how to map these `u32` codes into TypeScript / Kotlin / Swift user-facing +messages + analytics, see [`sdk-error-mapping-guide.md`](./sdk-error-mapping-guide.md). diff --git a/docs/sdk-error-mapping-guide.md b/docs/sdk-error-mapping-guide.md index f33d8e4..4455371 100644 --- a/docs/sdk-error-mapping-guide.md +++ b/docs/sdk-error-mapping-guide.md @@ -1,114 +1,297 @@ -# SDK Error Mapping Guide +# Savings Vault — SDK / Mobile Error Mapping Guide -This guide maps savings vault contract errors to expected SDK-level handling. -It is intended for SDK and mobile app developers integrating with the vault -contract on testnet. +This guide is for SDK + mobile developers integrating against the Savings +Vault contract. The contract now exposes a **stable `u32` error code surface** +via `#[contracterror] #[repr(u32)]` defined in +[`contracts/savings_vault/src/lib.rs`](../contracts/savings_vault/src/lib.rs). -> **Not production-ready.** This contract is for educational and testnet use. -> Error behavior may change before any mainnet deployment. +> **Cross-repo compatibility contract.** These `u32` codes are the **ONLY** +> supported interface for error branching on the client side. Panic message +> text may change between soroban-sdk upgrades — never regex-match panic +> strings in production code. Match on numeric codes instead. -## How errors surface +## 1. How errors surface -The contract uses Rust `panic!()` messages for validation failures and -Soroban host-level traps for authorization and token errors. There are no -stable numeric error codes. The SDK should **not** branch on panic message -text; instead, treat any failed invocation as a general failure and surface a -user-friendly message while retaining the diagnostic for debugging. +When the contract calls `env.error_contract(ContractError::Variant)`: -A failed invocation does **not** commit that invocation's state changes. +1. The Soroban host traps with a `HostError` of kind `ContractError`. +2. The `u32` discriminant from the enum (`#[repr(u32)]`) is the **stable + identifier** of the error. +3. Off-chain SDKs receive this `u32` via the transaction result. -## Error categories +A failed invocation commits **no state changes** from that invocation (Soroban +rollback is atomic); SDKs do not need to reconcile partial writes. -| Category | Contract errors | SDK action | -|---|---|---| -| Already initialized | `"Contract is already initialized"` | Show "Vault is already set up." Do not retry. | -| Not initialized | Storage unwrap trap (no message) | Show "Vault is not initialized." Prompt admin to run `initialize`. | -| Invalid amount | `"Deposit amount must be greater than zero"`, `"Withdrawal amount must be greater than zero"`, `"Lock amount must be greater than zero"` | Validate amount > 0 before submitting. Show "Please enter a valid amount." | -| Insufficient balance | `"Insufficient balance"`, `"Insufficient balance to lock"` | Refresh `get_balance` and compare. Show "Not enough available funds." | -| Invalid unlock time | `"Unlock time must be in the future"` | Validate unlock_time > current ledger timestamp before submitting. Show "Unlock time must be in the future." | -| Unauthorized | Soroban host auth failure (no contract message) | Request the correct wallet signature. Show "Transaction not authorized." | -| Token transfer failure | Error from the configured token contract | Inspect nested diagnostic. Show "Transfer failed." Verify token address and vault token balance. | +### How to extract the code -## User-facing examples +Pseudo-code for a `try_*` contract client call: -These are the messages the mobile app should show for each failure. +```ts +// TypeScript / stellar-sdk v12+ +import { Contract } from '@stellar/stellar-sdk'; -| Trigger | User message | -|---|---| -| Re-initialization | "Vault is already set up." | -| Deposit/withdraw/lock with zero or negative amount | "Please enter a valid amount." | -| Withdraw more than available | "Not enough available funds." | -| Lock more than available | "Not enough available funds." | -| Unlock time in the past | "Unlock time must be in the future." | -| Missing wallet signature | "Transaction not authorized." | -| Token transfer error | "Transfer failed. Please try again." | -| Vault not initialized | "Vault is not initialized." | +const contract = new Contract(vaultAddress); +const sim = await txBuilder.call(contract.call('deposit', user, amount)).simulate(); +if (sim.error) { + const code = extractContractErrorCode(sim.error); // see below + await handleVaultError(code, context); +} +``` -## Developer-facing examples +Use a `code`-extract helper that walks the diagnostic until it finds the +`ContractError` status. For the `soroban-sdk`/`@stellar/stellar-sdk` variants, +the pattern is: "Status with ContractError type whose payload is the u32". +Keep this helper versioned per host SDK, but keep `handleVaultError(code)` +stable — that is the cross-repo contract. -### Pre-flight validation +## 2. Category routing (by 1000-group) -Validate inputs before submitting to avoid unnecessary round trips: +Branch SDK logic in two layers: (1) **category route** by `Math.floor(code / 1000)`, +then (2) **specific copy / analytics** by exact `code`. This way future codes +added inside a category (e.g. 1008, 1009, ...) will still route to the right +top-level UX bucket. -```rust -// Reject non-positive amounts before invocation -if amount <= 0 { - return Err(SdkError::InvalidAmount); +| `code / 1000` | Category | UX bucket | +| --- | --- | --- | +| `1` | Validation | Show inline field error next to the bad input | +| `2` | Authorisation (contract-level role check) | Show "You're not the vault admin."; route admin-only views away | +| `3` | Lifecycle | Show a banner (pause / not-initialized / already-setup) | +| `4` | Accounting | Show `get_balance(user)` and cap the submit | +| `5` | Locks | Show lock-specific copy; re-fetch lock list | +| `6` | Storage | Page on-call; show a rare "Vault configuration issue" | +| `7` | Token | Same as 6xxx — asset config issue | +| `8` | Admin rotation | Admin console form validation | + +**Host auth failures** (missing signature, wrong signer) raise a host-level +`Status(Auth, …)`, not 2001. Distinguish these: + +```ts +switch (categoryOf(err)) { + case 'HostAuth': return promptWalletSignature(); // host Auth + case 2: return showNotVaultAdmin(); // 2001 role check + … } +``` + +## 3. Per-code SDK map (canonical) + +Columns: + +- **`code`** — the stable `u32`. +- **Trigger method** — which SDK call raised it; use to scope the copy. +- **User copy** — what mobile shows the end user. Interpolate `{amount}` / + `{asset}` using the token's stored decimals (never raw `i128` stroops). +- **Analytics event** — cross-repo deterministic event name for amplitude / + mixpanel / firebase. +- **SDK retry?** — `NEVER` = do not auto-retry (user-action or logic bug), + `PAGE-ONCALL` = alert the deployment-owners, `UI-GATED` = the SDK should + have blocked it before submission. + +| code | Trigger | User copy | Analytics | Retry? | +| --- | --- | --- | --- | --- | +| 1001 | `deposit` | "Enter an amount greater than 0 {asset}." | `vault.err.1001.deposit` | UI-GATED | +| 1001 | `withdraw` | "Enter an amount greater than 0 {asset}." | `vault.err.1001.withdraw` | UI-GATED | +| 1001 | `lock_funds` | "Enter an amount greater than 0 {asset}." | `vault.err.1001.lock` | UI-GATED | +| 1002 | `lock_funds` | "Unlock time must be later than right now." | `vault.err.1002.lock` | UI-GATED | +| 1002 | `extend_lock` | "New unlock time must be later than the current one." | `vault.err.1002.extend` | UI-GATED | +| 1003 | `lock_funds` | "Lock duration exceeds the maximum allowed." | `vault.err.1003` | UI-GATED | +| 1004 | `lock_funds` | "Lock duration is below the minimum allowed." | `vault.err.1004` | UI-GATED | +| 1005 | `deposit` | "Minimum deposit is {min} {asset}." | `vault.err.1005` | UI-GATED | +| 1006 | `pause` (admin) | "Pause duration must be greater than 0 seconds." | `vault.err.1006` | UI-GATED | +| 1007 | `set_min_deposit_amount` (admin) | "Minimum deposit can't be negative." | `vault.err.1007` | UI-GATED | +| 2001 | any admin entrypoint | "This action requires the vault admin role." | `vault.err.2001.{method}` | NEVER | +| 3001 | `initialize` | "The vault is already set up." | `vault.err.3001` | NEVER | +| 3002 | any guarded method | "The vault is still being set up. Try again in a moment." | `vault.err.3002.{method}` | NEVER (poll `get_version`) | +| 3003 | `deposit`,`lock_funds`,`extend_lock` | "Vault is paused for an incident. Deposits and locks are blocked. You can still withdraw." | `vault.err.3003.{method}` | NEVER (poll `is_paused`) | +| 4001 | `withdraw` | "Insufficient available balance. You can withdraw up to {available} {asset}. Locked funds are unavailable until they mature." | `vault.err.4001` | NEVER | +| 4002 | `lock_funds` | "Insufficient unlocked balance. You can lock up to {available} {asset}." | `vault.err.4002` | NEVER | +| 5001 | `withdraw_lock` / `extend_lock` | "We couldn't find that lock. Refresh and try again." | `vault.err.5001.{method}` | NEVER (refetch list) | +| 5002 | `withdraw_lock` / `extend_lock` | "This lock has already been withdrawn." | `vault.err.5002.{method}` | NEVER (treat as success) | +| 5003 | `withdraw_lock` | "This lock hasn't matured yet. Check back at {unlockTimeISO}." | `vault.err.5003` | NEVER | +| 5004 | `extend_lock` | "New unlock time must be later than the current unlock time." | `vault.err.5004` | UI-GATED | +| 6001 | any guarded method | "A vault storage version mismatch was detected. Please contact support." | `vault.err.6001.{method}` | PAGE-ONCALL | +| 6002 | any guarded method | "A vault storage entry is missing. Please contact support." | `vault.err.6002.{method}` | PAGE-ONCALL | +| 7001 | `deposit` / `withdraw` / `withdraw_lock` | "The vault's token is misconfigured. Please contact support." | `vault.err.7001.{method}` | PAGE-ONCALL | +| 8001 | `transfer_admin` (admin) | "New admin address must be different from the current admin." | `vault.err.8001` | NEVER | +| 8002 | `transfer_admin` (admin) | "The vault cannot be its own admin." | `vault.err.8002` | NEVER | + +## 4. Reference SDK implementation (TypeScript) + +```ts +// sdk/src/vault/errors.ts +export const VaultError = { + AmountNotPositive: 1001, + UnlockTimeNotInFuture: 1002, + LockDurationExceedsMaximum: 1003, + LockDurationBelowMinimum: 1004, + AmountBelowMinimumDeposit: 1005, + PauseDurationMustBePositive: 1006, + MinDepositAmountNegative: 1007, + + NotAuthorizedAdmin: 2001, + + AlreadyInitialized: 3001, + NotInitialized: 3002, + ContractPaused: 3003, + + InsufficientBalance: 4001, + InsufficientBalanceToLock: 4002, + + LockNotFound: 5001, + LockAlreadyWithdrawn: 5002, + LockNotMatured: 5003, + ExtendLockTimeNotIncreased: 5004, + + StorageVersionUnsupported: 6001, + RequiredStorageEntryMissing: 6002, + + TokenNotConfigured: 7001, -// Reject past unlock times -if unlock_time <= current_ledger_timestamp { - return Err(SdkError::InvalidUnlockTime); + CannotTransferAdminToSelf: 8001, + CannotTransferAdminToContractAddress: 8002, +} as const; + +export type VaultErrorCode = (typeof VaultError)[keyof typeof VaultError]; + +export function isContractError(code: number): code is VaultErrorCode { + return Object.values(VaultError).includes(code as VaultErrorCode); +} + +export function categoryOf(code: number): number { + return Math.floor(code / 1000); } ``` -### Balance check before withdraw or lock +Handler skeleton: -```rust -let available = contract.get_balance(user.clone()); -if amount > available { - return Err(SdkError::InsufficientBalance); +```ts +// sdk/src/vault/onError.ts +import { VaultError, categoryOf } from './errors'; + +export async function handleVaultError( + code: number, + ctx: { method: string; asset: { decimals: number; symbol: string } }, +): Promise { + switch (categoryOf(code)) { + case 1: return renderInlineValidation(code, ctx); + case 2: return renderRoleBanner(); + case 3: return renderLifecycleBanner(code, ctx); + case 4: return renderBalanceCallout(code, ctx); + case 5: return refetchLockList(code, ctx); + case 6: + case 7: + pageOncall(`vault.err.${code}`, ctx.method); + return renderSupportBanner(); + case 8: return renderAdminFormError(code); + default: + if (isContractError(code)) { + log.warn(`unknown canonical code ${code}; forward to fallback`); + } + return renderGenericFallback(); + } } ``` -### General error handling +## 5. Reference SDK implementation (Rust client) + +```rust +// crates/vault-sdk/src/err.rs +use soroban_sdk::contracterror; -Since the contract has no stable error codes, catch failures generically: +#[contracterror] +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +#[repr(u32)] +pub enum VaultError { + AmountNotPositive = 1001, + UnlockTimeNotInFuture = 1002, + // … keep in 1:1 lockstep with the on-chain enum +} + +impl VaultError { + pub fn from_u32(code: u32) -> Option { + use VaultError::*; + Some(match code { + 1001 => AmountNotPositive, + 1002 => UnlockTimeNotInFuture, + 1003 => LockDurationExceedsMaximum, + 1004 => LockDurationBelowMinimum, + 1005 => AmountBelowMinimumDeposit, + 1006 => PauseDurationMustBePositive, + 1007 => MinDepositAmountNegative, + 2001 => NotAuthorizedAdmin, + 3001 => AlreadyInitialized, + 3002 => NotInitialized, + 3003 => ContractPaused, + 4001 => InsufficientBalance, + 4002 => InsufficientBalanceToLock, + 5001 => LockNotFound, + 5002 => LockAlreadyWithdrawn, + 5003 => LockNotMatured, + 5004 => ExtendLockTimeNotIncreased, + 6001 => StorageVersionUnsupported, + 6002 => RequiredStorageEntryMissing, + 7001 => TokenNotConfigured, + 8001 => CannotTransferAdminToSelf, + 8002 => CannotTransferAdminToContractAddress, + _ => return None, + }) + } +} +``` + +Usage against a `try_*` client: ```rust -match contract.try_deposit(user.clone(), amount) { - Ok(_) => show_success("Deposit confirmed."), - Err(_) => show_error("Something went wrong. Please try again."), +match client.try_withdraw(&user, &amount) { + Ok(res) => Ok(res?), + Err(soroban_client::Error::Contract(code)) => match VaultError::from_u32(code) { + Some(VaultError::InsufficientBalance) => { + let available = client.get_balance(&user)?; + Err(AppError::WithdrawUpTo(available)) + } + Some(other) => Err(AppError::Vault(other, method)), + None => Err(AppError::UnknownContractCode(code, method)), + }, + Err(other) => Err(AppError::Transport(other)), } ``` -For production-quality SDKs, log the full diagnostic (including any panic -message or host error) for debugging while showing the user a generic -friendly message. +## 6. Pre-flight validation (recommended) -### Authorization flow +Doing these client-side saves a round trip AND prevents 1001–1007 / 5004 / +8001–8002 from ever hitting the chain. They are still listed on-chain as a +belt-and-suspenders check, but SDKs should treat them as UI bugs when they +reach the contract. -The SDK must ensure the user signs the transaction before submission. If the -contract returns an authorization error, the SDK should prompt for a wallet -signature rather than retrying silently: +```ts +// sdk/src/vault/preflight.ts +export function validateDeposit(amountBI: bigint, cfg: { minDepositBI: bigint }) { + if (amountBI <= 0n) return VaultError.AmountNotPositive; + if (amountBI < cfg.minDepositBI) return VaultError.AmountBelowMinimumDeposit; + return null; +} -```rust -match contract.try_withdraw(user.clone(), amount) { - Ok(tx_hash) => show_success("Withdrawal confirmed."), - Err(SdkError::Unauthorized) => prompt_wallet_signature(), - Err(SdkError::InsufficientBalance) => { - let available = contract.get_balance(user.clone()); - show_error(format!("You can withdraw up to {}.", available)); - } - Err(e) => show_error("Something went wrong."), +export function validateLock( + amountBI: bigint, + unlockTimeS: number, + ledgerTimeS: number, + cfg: { minLockDurationS: number; maxLockDurationS: number; availableBI: bigint }, +) { + if (amountBI <= 0n) return VaultError.AmountNotPositive; + if (amountBI > cfg.availableBI) return VaultError.InsufficientBalanceToLock; + if (unlockTimeS <= ledgerTimeS) return VaultError.UnlockTimeNotInFuture; + const d = unlockTimeS - ledgerTimeS; + if (d < cfg.minLockDurationS) return VaultError.LockDurationBelowMinimum; + if (d > cfg.maxLockDurationS) return VaultError.LockDurationExceedsMaximum; + return null; } ``` -## Further reading +## 7. Further reading -- [Contract Error Reference](error-codes.md) — full list of current failure - conditions -- [SDK Sequence Diagrams](sdk-contract-sequence.md) — interaction flows - including error paths -- [Architecture Documentation](architecture.md) — storage model and contract - structure +- [`error-codes.md`](./error-codes.md) — per-code canonical meaning and + contract caller action. +- [`error-code-standard.md`](./error-code-standard.md) — design rationale, + stability guarantees, versioning rules for adding new codes. +- [`sdk-contract-sequence.md`](./sdk-contract-sequence.md) — flows with error + paths drawn in. diff --git a/docs/storage-audit.md b/docs/storage-audit.md index 047223d..c12decf 100644 --- a/docs/storage-audit.md +++ b/docs/storage-audit.md @@ -1,99 +1,82 @@ -# Storage Audit: Savings Vault Contract -This document provides a comprehensive audit of all storage keys used in the Savings Vault contract, including: -- Storage key definitions and types -- Mutation points (which functions modify which keys) -- Invariants that must always hold -- TTL management guidelines +# Storage Audit Map and Mutation Trace ---- - -## 1. Storage Layers -The contract uses two Soroban storage layers: +This document provides a comprehensive audit of all storage usage in the PocketPay Savings Vault contract. It maps storage keys, value types, mutation points, invariants, and test coverage to ensure a high-security posture for fund custody and accounting. -| Layer | Purpose | -|-------|---------| -| **Instance Storage** | Stores configuration and initialization state (admin, token, initialized flag, storage version) | -| **Persistent Storage** | Stores per-user state (balances, locks, lock ID counter) | +## 1. Storage Key Map ---- +The contract uses Soroban's **Instance** storage for global configuration and **Persistent** storage for user-specific data to optimize for resource usage and scalability. -## 2. Storage Key Audit +| DataKey Variant | Storage Type | Value Type | Ownership | Description | +| :--- | :--- | :--- | :--- | :--- | +| `Admin` | Instance | `Address` | Global | The current administrator address for configuration and pause controls. | +| `Initialized` | Instance | `bool` | Global | Flag indicating if the contract has been successfully initialized. | +| `Token` | Instance | `Address` | Global | The address of the SAC token managed by this vault. | +| `StorageVersion` | Instance | `u64` | Global | Current schema version for the contract storage. | +| `Paused` | Instance | `bool` | Global | Current pause status of the vault. | +| `PauseExpiry` | Instance | `u64` | Global | Timestamp (ledger seconds) when the current pause automatically expires. | +| `MinDepositAmount` | Instance | `i128` | Global | Minimum amount required for a single deposit operation. | +| `MaxLockDurationSecs` | Instance | `u64` | Global | Maximum duration a lock can be active. | +| `MinLockDurationSecs` | Instance | `u64` | Global | Minimum duration required for a new lock. | +| `Balance(Address)` | Persistent | `i128` | User | Total balance (available + locked) for a specific user. | +| `Lock(Address, u64)` | Persistent | `LockEntry` | User | Individual lock record keyed by user and a monotonic ID. | +| `NextLockId(Address)` | Persistent | `u64` | User | Monotonic counter used to generate unique IDs for a user's locks. | -### Instance Storage Keys -| Key | Type | Default | Initialization | Mutation Points | Invariants | -|-----|------|---------|----------------|-----------------|------------| -| `DataKey::Admin` | `Address` | None | Set once in `initialize` | `transfer_admin` | - Immutable after `transfer_admin`; only admin can change | -| `DataKey::Initialized` | `bool` | None | Set to `true` in `initialize` | None (never modified after initialization) | - Always `true` after initialization; never unset | -| `DataKey::Token` | `Address` | None | Set once in `initialize` | None (never modified after initialization) | - Immutable after initialization | -| `DataKey::StorageVersion` | `u64` | None | Set to `STORAGE_VERSION` (1) in `initialize` | None (never modified after initialization) | - Always equals `STORAGE_VERSION` | - -### Persistent Storage Keys -| Key | Type | Default | Initialization | Mutation Points | Invariants | -|-----|------|---------|----------------|-----------------|------------| -| `DataKey::Balance(Address)` | `i128` | `0` (implicit) | Not set at initialization | `deposit`, `withdraw`, `lock_funds` | - ≥ 0 at all times; represents available balance | -| `DataKey::Lock(Address, u64)` | `LockEntry` | None | Not set at initialization | `lock_funds`, `withdraw`, `withdraw_lock` | - `LockEntry.amount` ≥ 0; `LockEntry.id` unique for user; `LockEntry.unlock_time` ≥ 0 | -| `DataKey::NextLockId(Address)` | `u64` | `1` (implicit) | Not set at initialization | `lock_funds` | - Strictly increasing (monotonic); never decreases | +*Note: The `Locks(Address)` variant is defined in the enum but is currently unused in favor of individual `Lock(Address, u64)` entries.* --- -## 3. Mutation Point Mapping -This section maps each storage key to the functions that modify it: - -### Instance Storage -| Function | Modifies | -|----------|----------| -| `initialize` | `Admin`, `Initialized`, `Token`, `StorageVersion` | -| `transfer_admin` | `Admin` | - -### Persistent Storage -| Function | Modifies | -|----------|----------| -| `deposit` | `Balance(user)` | -| `withdraw` | `Balance(user)`, `Lock(user, id)` | -| `lock_funds` | `Balance(user)`, `Lock(user, id)`, `NextLockId(user)` | -| `withdraw_lock` | `Lock(user, id)` | +## 2. Mutation Trace + +The following table tracks which functions mutate specific storage keys. Read-only operations are omitted for brevity. + +| Function | Mutated Keys | State Change Description | +| :--- | :--- | :--- | +| `initialize` | `Admin`, `Initialized`, `Token`, `StorageVersion` | Sets global config; locks initialization. | +| `try_migrate` | `StorageVersion` | Increments schema version during upgrades. | +| `pause` | `Paused`, `PauseExpiry` | Enables emergency pause with an expiry timestamp. | +| `unpause` | `Paused`, `PauseExpiry` | Manually clears pause state. | +| `require_not_paused` | `Paused`, `PauseExpiry` | **Lazy Mutation**: Clears pause if `ledger.timestamp() >= expiry`. | +| `set_min_deposit_amount`| `MinDepositAmount` | Updates global minimum deposit threshold. | +| `set_max_lock_duration` | `MaxLockDurationSecs` | Updates global maximum lock time. | +| `set_min_lock_duration` | `MinLockDurationSecs` | Updates global minimum lock time. | +| `deposit` | `Balance(user)` | Increments user balance after successful token transfer. | +| `withdraw` | `Balance(user)` | Decrements user balance after successful token transfer. | +| `lock_funds` | `Balance(user)`, `Lock(user, id)`, `NextLockId(user)` | Debits available balance, writes lock entry, increments ID counter. | +| `extend_lock` | `Lock(user, id)` | Updates the `unlock_time` of an existing lock entry. | +| `withdraw_lock` | `Lock(user, id)` | Zeroes out the lock entry amount after maturity and token transfer. | +| `transfer_admin` | `Admin` | Rotates the administrative address. | --- -## 4. Critical Storage Invariants -These invariants must hold at all times, across all function calls: +## 3. Failure-Path State Expectations -### Invariant 1: User Available Balance ≥ 0 -- **Description**: `Balance(user)` must never be negative -- **Enforced By**: `deposit`, `withdraw`, `lock_funds` -- **Tested By**: `balance_conservation.rs` tests and `property_vault_accounting.rs` proptests +The contract employs a **"transfer-then-write"** pattern to ensure atomicity. If a transaction fails at any point, all storage changes are rolled back by the Soroban host environment. -### Invariant 2: User Lock Entry Amounts ≥ 0 -- **Description**: Every `LockEntry.amount` in `Locks(user)` must be ≥ 0 -- **Enforced By**: `lock_funds` -- **Tested By**: `balance_conservation.rs` tests +| Failure Scenario | Expected State | Implementation Detail | +| :--- | :--- | :--- | +| **Token Transfer Fails** | No mutation to `Balance` or `Lock`. | Token transfer is attempted *before* storage writes in `deposit` and `withdraw`. | +| **Auth Failure** | No mutation to any key. | `require_auth()` is called at the start of all protected methods. | +| **Invariant Violation** | Transaction panics; all state reverted. | Invariants are checked at the end of functions or via guards. | +| **Vault Paused** | `require_not_paused` panics (unless lazy clearing). | Mutations are blocked early in the call stack. | -### Invariant 3: NextLockId Is Monotonic -- **Description**: `NextLockId(user)` must never decrease; only increments by 1 per `lock_funds` call -- **Enforced By**: `lock_funds` -- **Tested By**: `lock_read_helpers.rs` tests +--- -### Invariant 4: Lock Entry IDs Are Unique Per User -- **Description**: No two `LockEntry` in `Locks(user)` share the same `id` -- **Enforced By**: `lock_funds` (uses `NextLockId` to generate unique IDs) -- **Tested By**: Implicit via monotonic `NextLockId` +## 4. Invariants and Test Coverage -### Invariant 5: Token Custody Invariant -- **Description**: The sum of all user balances + sum of all user locked balances ≤ SAC balance of the contract -- **Enforced By**: All functions that perform token transfers (`deposit`, `withdraw`, `withdraw_lock`) -- **Tested By**: `property_vault_accounting::prop_global_token_custody` (proptest) +Core accounting and security invariants are verified through a combination of unit tests and property-based tests. -### Invariant 6: Initialized Flag Remains True After Initialization -- **Description**: Once `initialize` has been called, `Initialized` remains `true` forever -- **Enforced By**: `initialize` (sets flag; no other function modifies it) -- **Tested By**: `initialization.rs` tests +| Invariant | Description | Test Reference | +| :--- | :--- | :--- | +| **Balance Conservation** | `Available + Locked == Total` for all users at all times. | [balance_conservation.rs](file:///c:/Users/abbat/.trae/GrantFox/pocketpay-contracts/contracts/savings_vault/src/test/balance_conservation.rs) | +| **Token Custody** | `Contract SAC Balance == Σ(User Balances)`. | [property_fee_invariants.rs](file:///c:/Users/abbat/.trae/GrantFox/pocketpay-contracts/contracts/savings_vault/src/test/property_fee_invariants.rs) | +| **Atomic Rollback** | Failed transfers must not credit/debit internal accounting. | [token_transfer_rollback.rs](file:///c:/Users/abbat/.trae/GrantFox/pocketpay-contracts/contracts/savings_vault/src/test/token_transfer_rollback.rs) | +| **Lock Integrity** | `Locked Balance == Σ(Active Lock Entries)`. | [multi_lock_invariants.rs](file:///c:/Users/abbat/.trae/GrantFox/pocketpay-contracts/contracts/savings_vault/src/test/multi_lock_invariants.rs) | +| **ID Uniqueness** | `NextLockId` must never produce a duplicate ID for a user. | [multi_lock_invariants.rs](file:///c:/Users/abbat/.trae/GrantFox/pocketpay-contracts/contracts/savings_vault/src/test/multi_lock_invariants.rs) | --- -## 5. TTL Management Guidelines -See [storage-ttl.md](storage-ttl.md) for full TTL management guidelines! - ---- +## 5. Technical Debt and Audit Notes -## 6. Storage Upgrade / Migration Plan -See [upgrade-strategy.md](upgrade-strategy.md) for future upgrade/migration planning! +- **Unused Storage Variant**: `DataKey::Locks(Address)` should be removed or implemented to avoid confusion during audits. +- **TTL Management**: The audit currently assumes standard Soroban TTL management for Persistent/Instance storage. A dedicated TTL extension strategy should be documented if custom intervals are required. +- **Linear Scan Risk**: Functions like `get_balance_snapshot` and `list_locks` iterate over lock IDs. While capped by `MAX_LOCK_PAGE_SIZE`, high lock counts per user could impact gas costs for complex read-aggregations. diff --git a/docs/vault_storage_audit_map.md b/docs/vault_storage_audit_map.md deleted file mode 100644 index 487e6fb..0000000 --- a/docs/vault_storage_audit_map.md +++ /dev/null @@ -1,36 +0,0 @@ -# Vault Storage Audit Map - -This document maps all storage entries for the Savings Vault contract, including keys, value types, mutating functions, and invariants. This map supports audit preparation by providing a clear overview of state management. - -## 1. Storage Keys & Value Types - -| Storage Key | Layer | Rust Type | Description | -|---|---|---|---| -| `DataKey::Admin` | Instance | `Address` | Contract administrator address. | -| `DataKey::Initialized` | Instance | `bool` | Flag indicating contract initialization. | -| `DataKey::Token` | Instance | `Address` | Token contract (SAC) address for transfers. | -| `DataKey::Balance(Address)` | Persistent | `i128` | Available (unlocked) balance of a user. | -| `DataKey::Locks(Address)` | Persistent | `Vec` | Active or matured lock records for a user. | -| `DataKey::NextLockId(Address)` | Persistent | `u64` | Counter for generating unique lock IDs per user. | - -## 2. Mutating Functions - -| Function | Storage Keys Modified | -|---|---| -| `initialize` | `Admin`, `Initialized`, `Token` | -| `deposit` | `Balance(user)` | -| `withdraw` | `Balance(user)`, `Locks(user)` | -| `withdraw_lock` | `Locks(user)` | -| `lock_funds` | `Balance(user)`, `Locks(user)`, `NextLockId(user)` | - -## 3. Storage Invariants - -- **Admin**: Set exactly once during initialization. Cannot be changed. Must be a valid `Address`. -- **Initialized**: Set to `true` during initialization. Prevents re-initialization. -- **Token**: Set exactly once during initialization. Must be a valid token contract address. -- **Balance**: Must always be $\ge 0$. Modifications must be authorized by `user.require_auth()`. -- **Locks**: - - Each `LockEntry` must have `amount > 0`. - - When a lock is created, its `unlock_time` must be strictly greater than the current ledger timestamp. -- **NextLockId**: Must monotonically increase starting from `1` for each user. -- **Global Accounting**: The total contract token balance must equal the sum of all users' `Balance` plus the sum of all users' locked amounts.