Pocket pay contract 27

contracts/savings_vault/src/test/mod.rs

@@ -27,6 +27,7 @@ mod maximum_lock_duration;
 mod minimum_deposit_amount;
 mod minimum_lock_duration;
 mod multi_lock_invariants;
+mod negative_paths;
 mod pause;
 mod pause_state_read;
 mod pause_transition;
@@ -1482,6 +1483,34 @@ fn test_withdraw_emits_event() {
     );
 }
 
+#[test]
+fn test_withdraw_lock_emits_event() {
+    use soroban_sdk::{symbol_short, TryIntoVal};
+
+    let env = test_env();
+    let (contract_id, client) = init_contract(&env);
+    let (env, _admin, client, _token_client, token_admin) = test_token(env, contract_id, client);
+
+    let user = new_user(&env);
+    token_admin.mint(&user, &1000);
+    set_ledger_timestamp(&env, 1_000);
+
+    deposit_balance(&client, &user, 500);
+    let id = client.lock_funds(&user, &200, &2_000);
+
+    set_ledger_timestamp(&env, 2_000);
+    client.withdraw_lock(&user, &id);
+
+    let events = env.events().all();
+    let (_contract, topics, data) = events.get(events.len() - 1).unwrap();
+    let topic0: soroban_sdk::Symbol = topics.get(0).unwrap().try_into_val(&env).unwrap();
+    let topic1: Address = topics.get(1).unwrap().try_into_val(&env).unwrap();
+    let (amount, new_balance): (i128, i128) = data.try_into_val(&env).unwrap();
+    assert_eq!(topic0, symbol_short!("wdr_lock"));
+    assert_eq!(topic1, user);
+    assert_eq!((amount, new_balance), (200_i128, 300_i128));
+}
+
 #[test]
 fn test_lock_funds_emits_event() {
     use soroban_sdk::{symbol_short, TryIntoVal};

contracts/savings_vault/src/test/negative_paths.rs

@@ -0,0 +1,242 @@
+//! Comprehensive negative-path test suite for the Savings Vault contract.
+//!
+//! Covers unauthorized access, invalid inputs, early withdrawals, 
+//! missing initialization, and state consistency after failures.
+
+use super::*;
+use soroban_sdk::testutils::{Address as _, Events};
+use soroban_sdk::{Address, Env, IntoVal};
+use crate::ContractError;
+use test_helpers::*;
+
+// =========================================================================
+// 1. Missing Initialization
+// =========================================================================
+
+#[test]
+#[should_panic]
+fn test_uninitialized_deposit_panics() {
+    let env = test_env();
+    let contract_id = env.register(SavingsVault, ());
+    let client = SavingsVaultClient::new(&env, &contract_id);
+    let user = Address::generate(&env);
+    client.deposit(&user, &100);
+}
+
+#[test]
+#[should_panic]
+fn test_uninitialized_withdraw_panics() {
+    let env = test_env();
+    let contract_id = env.register(SavingsVault, ());
+    let client = SavingsVaultClient::new(&env, &contract_id);
+    let user = Address::generate(&env);
+    client.withdraw(&user, &100);
+}
+
+#[test]
+#[should_panic]
+fn test_uninitialized_lock_funds_panics() {
+    let env = test_env();
+    let contract_id = env.register(SavingsVault, ());
+    let client = SavingsVaultClient::new(&env, &contract_id);
+    let user = Address::generate(&env);
+    client.lock_funds(&user, &100, &2_000);
+}
+
+#[test]
+#[should_panic]
+fn test_uninitialized_pause_panics() {
+    let env = test_env();
+    let contract_id = env.register(SavingsVault, ());
+    let client = SavingsVaultClient::new(&env, &contract_id);
+    let admin = Address::generate(&env);
+    client.pause(&admin, &3600);
+}
+
+// =========================================================================
+// 2. Unauthorized Access (Admin Boundaries)
+// =========================================================================
+
+#[test]
+#[should_panic]
+fn test_unauthorized_pause_fails() {
+    let env = test_env();
+    let (_contract_id, client, _token_client, _token_admin, _vault_admin) = vault_with_sac(&env);
+    let rando = Address::generate(&env);
+
+    // Attempt to pause as a non-admin user
+    client.pause(&rando, &3600);
+}
+
+#[test]
+#[should_panic]
+fn test_unauthorized_unpause_fails() {
+    let env = test_env();
+    let (_contract_id, client, _token_client, _token_admin, vault_admin) = vault_with_sac(&env);
+    let rando = Address::generate(&env);
+
+    env.mock_all_auths();
+    client.pause(&vault_admin, &3600);
+
+    // Intentionally omit mock_all_auths() to test auth rejection
+    client.unpause(&rando);
+}
+
+#[test]
+#[should_panic]
+fn test_unauthorized_set_min_deposit_amount_fails() {
+    let env = test_env();
+    let (_contract_id, client, _token_client, _token_admin, _vault_admin) = vault_with_sac(&env);
+    let rando = Address::generate(&env);
+
+    client.set_min_deposit_amount(&rando, &1000);
+}
+
+// =========================================================================
+// 3. Invalid Inputs
+// =========================================================================
+
+#[test]
+fn test_invalid_deposit_amounts_fail() {
+    let env = test_env();
+    let (_contract_id, client, _token_client, _token_admin, _vault_admin) = vault_with_sac(&env);
+    let user = Address::generate(&env);
+
+    // Case 1: Zero deposit
+    let res = client.try_deposit(&user, &0);
+    assert!(res.is_err());
+
+    // Case 2: Negative deposit
+    let res = client.try_deposit(&user, &-1);
+    assert!(res.is_err());
+}
+
+#[test]
+fn test_deposit_below_minimum_fails() {
+    let env = test_env();
+    let (_contract_id, client, _token_client, token_admin, vault_admin) = vault_with_sac(&env);
+
+    env.mock_all_auths();
+    client.set_min_deposit_amount(&vault_admin, &1000);
+
+    let user = Address::generate(&env);
+    token_admin.mint(&user, &2000);
+
+    // Attempt deposit below minimum floor
+    let res = client.try_deposit(&user, &500);
+    assert!(res.is_err());
+}
+
+#[test]
+fn test_lock_duration_boundary_failures() {
+    let env = test_env();
+    let (_contract_id, client, _token_client, token_admin, vault_admin) = vault_with_sac(&env);
+
+    env.mock_all_auths();
+    client.set_max_lock_duration(&vault_admin, &10_000);
+    client.set_min_lock_duration(&vault_admin, &1_000);
+
+    let user = Address::generate(&env);
+    token_admin.mint(&user, &1000);
+    client.deposit(&user, &1000);
+
+    set_ledger_timestamp(&env, 1000);
+
+    // Case 1: Duration too long
+    let res = client.try_lock_funds(&user, &100, &12_000); // 11,000s duration
+    assert!(res.is_err());
+
+    // Case 2: Duration too short
+    let res = client.try_lock_funds(&user, &100, &1_500); // 500s duration
+    assert!(res.is_err());
+}
+
+// =========================================================================
+// 4. Early Withdrawals & Invalid Lock States
+// =========================================================================
+
+#[test]
+fn test_early_lock_withdrawal_fails() {
+    let env = test_env();
+    let (_contract_id, client, _token_client, token_admin, _vault_admin) = vault_with_sac(&env);
+    let user = Address::generate(&env);
+
+    env.mock_all_auths();
+    set_ledger_timestamp(&env, 1000);
+    token_admin.mint(&user, &1000);
+    client.deposit(&user, &1000);
+
+    let lock_id = client.lock_funds(&user, &500, &5000);
+
+    // Attempt withdrawal at T=4999 (1s before maturity)
+    set_ledger_timestamp(&env, 4999);
+    let res = client.try_withdraw_lock(&user, &lock_id);
+    assert!(res.is_err());
+}
+
+#[test]
+fn test_extend_withdrawn_lock_fails() {
+    let env = test_env();
+    let (_contract_id, client, _token_client, token_admin, _vault_admin) = vault_with_sac(&env);
+    let user = Address::generate(&env);
+
+    env.mock_all_auths();
+    set_ledger_timestamp(&env, 1000);
+    token_admin.mint(&user, &1000);
+    client.deposit(&user, &1000);
+
+    let lock_id = client.lock_funds(&user, &500, &5000);
+    set_ledger_timestamp(&env, 5000);
+    client.withdraw_lock(&user, &lock_id);
+
+    // Attempt to extend a lock that has already been withdrawn
+    let res = client.try_extend_lock(&user, &lock_id, &10_000);
+    assert!(res.is_err());
+}
+
+// =========================================================================
+// 5. State Consistency After Failures
+// =========================================================================
+
+#[test]
+fn test_state_remains_consistent_after_failed_lock() {
+    let env = test_env();
+    let (_contract_id, client, _token_client, token_admin, _vault_admin) = vault_with_sac(&env);
+    let user = Address::generate(&env);
+
+    env.mock_all_auths();
+    set_ledger_timestamp(&env, 1000);
+    token_admin.mint(&user, &1000);
+    client.deposit(&user, &1000);
+
+    let initial_balance = client.get_balance(&user);
+    let initial_locked = client.get_locked_balance(&user);
+
+    // Attempt to lock more than available
+    let _ = client.try_lock_funds(&user, &1001, &5000);
+
+    assert_eq!(client.get_balance(&user), initial_balance, "Balance should not change after failed lock");
+    assert_eq!(client.get_locked_balance(&user), initial_locked, "Locked balance should not change after failed lock");
+}
+
+#[test]
+fn test_state_consistency_after_failed_token_transfer() {
+    let env = test_env();
+    let (contract_id, client) = init_contract(&env);
+    let (env, _admin, client, token_client, token_admin) = test_token(env, contract_id.clone(), client);
+    let user = Address::generate(&env);
+
+    // User has 50 tokens, tries to deposit 100
+    token_admin.mint(&user, &50);
+    let initial_user_tokens = token_client.balance(&user);
+    let initial_vault_tokens = token_client.balance(&contract_id);
+
+    env.mock_all_auths();
+    let res = client.try_deposit(&user, &100);
+    assert!(res.is_err(), "Deposit should fail due to insufficient SAC balance");
+
+    // Verify internal accounting and external token balances are unchanged
+    assert_eq!(client.get_balance(&user), 0, "Internal balance should not be credited");
+    assert_eq!(token_client.balance(&user), initial_user_tokens, "User tokens should not have moved");
+    assert_eq!(token_client.balance(&contract_id), initial_vault_tokens, "Vault tokens should not have changed");
+}

docs/contribution-quality-gate.md

@@ -0,0 +1,62 @@
+# Contribution Quality Gate
+
+This document defines the objective criteria for "payment-ready" or "production-ready" work in the PocketPay Contracts repository. To maintain high security and reliability, every Pull Request (PR) must pass through this quality gate before being considered for final approval.
+
+## 1. Quality Gate Checklist
+
+Every PR must satisfy the following checklist. If any item is missing or incomplete, the PR will be flagged for further work.
+
+### Implementation
+- [ ] **Completeness**: The feature or fix is fully implemented according to the issue description.
+- [ ] **No Placeholders**: All `TODO`, `FIXME`, and `HACK` comments related to the change have been resolved.
+- [ ] **Storage Integrity**: Storage usage follows the [Storage Audit Map](storage-audit.md) and correctly manages Persistent vs. Instance storage.
+- [ ] **Authorization**: All state-changing functions correctly enforce `require_auth()` for the appropriate parties.
+- [ ] **Error Handling**: Uses structured `ContractError` codes instead of generic panics where appropriate.
+
+### Testing
+- [ ] **Unit Tests**: Every new or modified function has unit tests for both success paths and failure paths (e.g., unauthorized access, invalid inputs).
+- [ ] **Invariant Verification**: Changes to accounting logic are verified by new or existing [Property Tests](../contracts/savings_vault/src/test/property_vault_accounting.rs).
+- [ ] **Event Snapshots**: Any changes to event schemas have updated [Snapshots](../contracts/savings_vault/test_snapshots/).
+- [ ] **Local Verification**: All tests pass locally using `cargo test`.
+
+### Documentation
+- [ ] **README/Docs**: New features or architectural changes are documented in the relevant `docs/` files.
+- [ ] **Acceptance Criteria**: The PR description explicitly lists how each Acceptance Criterion from the original issue was met.
+
+### CI & Tooling
+- [ ] **Formatting**: Code is formatted via `cargo fmt`.
+- [ ] **Lints**: `cargo clippy --tests` passes with no warnings.
+- [ ] **Build**: `make build-release` succeeds and the WASM size remains within acceptable limits.
+
+---
+
+## 2. Contract-Specific Testing Expectations
+
+Soroban smart contracts require rigorous testing due to their immutable nature once deployed. We expect:
+
+1.  **Authorization Boundaries**: Tests must explicitly verify that a function fails when called by an unauthorized address.
+2.  **State Rollbacks**: Tests must verify that if a transaction fails (e.g., token transfer failure), no storage mutations occur.
+3.  **Maturity Checks**: For time-locked funds, tests must verify behavior exactly at, before, and after the maturity timestamp.
+4.  **Edge Case Amounts**: Test with `0`, `1`, `MAX`, and amounts just below/above configured limits.
+
+---
+
+## 3. Examples of Incomplete Work
+
+To avoid common pitfalls, here are examples of PRs that do **not** pass the quality gate:
+
+- **"Happy Path Only"**: A PR that adds a `deposit` feature but only tests a successful deposit, skipping unauthorized or over-limit tests.
+- **"Logic without Invariants"**: A PR that changes how balances are calculated but doesn't update the property-based tests that ensure total funds remain constant.
+- **"Stale Docs"**: A PR that changes a function signature in the contract but leaves the `api-reference.md` or `README.md` outdated.
+- **"Ignoring Clippy"**: A PR where `cargo clippy` emits warnings that the contributor describes as "unrelated" or "minor".
+- **"WIP in Main"**: PRs marked as "Work In Progress" or containing half-implemented logic should not be requested for final review.
+
+---
+
+## 4. How to Use This Gate
+
+1.  **Before Opening a PR**: Review your work against this checklist.
+2.  **In Your PR Description**: Reference this quality gate and confirm that all items are checked.
+3.  **Reviewers**: Use this checklist as the primary framework for your review. If the gate isn't met, request changes immediately.
+
+For more details on local reproduction of tests, see the [Test Reproduction Guide](reproducing-test-failures.md).

docs/test-coverage.md

@@ -7,7 +7,22 @@ new tests. It is a coverage **map**, not a line-coverage report — see
 
 Source: `contracts/savings_vault/src/test/` (`mod.rs`, `initialization.rs`,
 `balance_conservation.rs`, `withdraw_lock.rs`, `maximum_amount_boundary.rs`,
-`unauthorized_access.rs`, `lock_read_helpers.rs`, `replay_protection.rs`).
+`unauthorized_access.rs`, `lock_read_helpers.rs`, `replay_protection.rs`, `negative_paths.rs`).
+
+## Negative Path & Error States
+
+This section explicitly maps tests for invalid inputs, unauthorized access, and lifecycle violations (issue #442).
+
+| Category | Behaviour | Status | Test(s) |
+|---|---|---|---|
+| **Auth** | Unauthorized user calls state-changing functions | Covered | `test_unauthorized_deposit_fails`, `test_unauthorized_withdraw_fails`, `test_unauthorized_lock_fails`, `test_unauthorized_withdraw_lock_fails`, `test_unauthorized_extend_lock_fails` |
+| **Auth** | Non-admin user calls admin-gated functions | Covered | `test_unauthorized_pause_fails`, `test_unauthorized_unpause_fails`, `test_unauthorized_set_min_deposit_amount_fails`, `test_transfer_admin_not_authorized_panics` |
+| **Lifecycle** | Calling functions before `initialize` | Covered | `test_uninitialized_deposit_panics`, `test_uninitialized_withdraw_panics`, `test_uninitialized_lock_funds_panics`, `test_uninitialized_pause_panics`, `test_get_admin_before_initialization_panics` |
+| **Validation** | Invalid deposit amounts (zero, negative, below floor) | Covered | `test_deposit_zero_panics`, `test_deposit_negative_panics`, `test_invalid_deposit_amounts_fail`, `test_deposit_below_minimum_fails` |
+| **Validation** | Invalid lock durations (past time, min/max violation) | Covered | `test_lock_past_time_panics`, `test_lock_duration_boundary_failures` |
+| **Lock State** | Withdrawing immature locks (early withdrawal) | Covered | `test_withdraw_immature_lock_fails`, `test_early_lock_withdrawal_fails`, `test_can_withdraw_before_unlock` |
+| **Lock State** | Extending or withdrawing a spent lock | Covered | `test_withdraw_repeated_lock_fails`, `test_extend_withdrawn_lock_fails`, `error_code_5002_lock_already_withdrawn` |
+| **Rollback** | State remains unchanged after any contract error | Covered | `test_state_remains_consistent_after_failed_lock`, `test_state_consistency_after_failed_token_transfer`, `test_failed_withdraw_does_not_change_available_balance_panics` |
 
 ## Initialization
 
@@ -43,7 +58,7 @@ Source: `contracts/savings_vault/src/test/` (`mod.rs`, `initialization.rs`,
 | `withdraw` emits an event | Covered | `test_withdraw_emits_event` |
 | Withdrawals near `i128::MAX` don't overflow | Covered | `test_withdraw_i128_max_after_deposit_succeeds`, `test_withdraw_over_large_balance_does_not_mutate`, `test_withdraw_partial_from_large_balance_preserves_remainder`, `test_large_withdraw_spans_available_and_matured_locks` |
 | `withdraw_lock` (withdraw a single matured lock by ID) | Covered | `test_withdraw_matured_lock_success`, `test_withdraw_immature_lock_fails`, `test_withdraw_nonexistent_lock_fails`, `test_withdraw_repeated_lock_fails`, `test_withdraw_wrong_user_lock_fails`, `test_unauthorized_withdraw_lock_fails` |
-| `withdraw_lock` emits an event | **Gap** | No dedicated `test_withdraw_lock_emits_event`-style test, unlike `deposit`/`withdraw`/`lock_funds`. The event is emitted in `lib.rs` but not asserted. |
+| `withdraw_lock` emits an event | Covered | `test_withdraw_lock_emits_event` |
 
 ## Locking
 
@@ -80,14 +95,8 @@ Source: `contracts/savings_vault/src/test/` (`mod.rs`, `initialization.rs`,
 
 ## Known Test Gaps
 
-- **`withdraw_lock` event emission is not asserted.** `deposit`, `withdraw`,
-  `lock_funds`, and `initialize` each have a `*_emits_event` test; `withdraw_lock`
-  does not, even though `lib.rs` publishes an event for it.
 - **`list_locks` pagination edge cases.** Zero-limit and out-of-range-offset
   behaviour for `list_locks` isn't explicitly covered.
-- **No fuzz/property-based tests.** Coverage relies on hand-written cases
-  (including a table-driven test in `balance_conservation.rs`) rather than
-  randomized or property-based testing.
 
 ## Additional Notes