test+docs: cover cancel vault refunds

README_CANCEL_VAULT.md

@@ -1,23 +1,44 @@
-Cancel Vault: Design, Security Notes, and Tests
+# Cancel Vault: Design, Security Notes, and Tests
 
-Summary
-- `cancel_vault` requires the caller to be the vault `creator` and only allows cancellation when the vault `status` is `Active`.
-- On cancel it refunds the vault's stored token balance (simulated) back to the creator by clearing the contract-held balance and emitting a `vault_cancelled` event with the refunded amount.
+## Summary
 
-Behavior & rules
-- Cancellation allowed only when `status == Active` (i.e., before validation/completion).
-- Caller must be the `creator` (enforced via `Address::require_auth`).
-- Refund is performed against a simulated per-vault balance stored in persistent storage under key `("vault_balance", vault_id)`.
+- `cancel_vault` requires the vault `creator` to authorize the call.
+- Cancellation is only allowed while the vault status is `Active`.
+- On cancel, the contract performs a real Soroban token transfer from the contract escrow balance back to the creator, then marks the vault as `Cancelled`.
+- A successful cancellation emits `vault_cancelled`.
 
-Security notes
-- Real token transfer: the current implementation simulates token movement by updating contract-owned balance storage. For production, replace this with a real token transfer using the Soroban token client and ensure the contract has allowance/escrowed tokens before creating the vault.
-- Reentrancy: token transfers must be performed using the recommended Soroban token client patterns; consider checks-effects-interactions ordering (we clear the stored balance and set status before emitting events).
-- Authorization: `creator.require_auth()` ensures only the vault owner can cancel.
+## Behavior and Rules
 
-Tests
-- Unit tests cover: successful cancel by creator, unauthorized cancel (panics), and cancel when status is Completed (returns false).
-- A helper `get_vault_balance` exposes the simulated vault balance for assertions in tests.
+- `create_vault` transfers the escrow amount from the creator to the current contract address.
+- `cancel_vault` transfers exactly `vault.amount` from the current contract address back to `vault.creator`.
+- Funds are not sent to `success_destination` or `failure_destination` during cancellation.
+- Cancelling a `Completed`, `Failed`, or already `Cancelled` vault returns `Error::VaultNotActive`.
+- Cancelling a nonexistent vault returns `Error::VaultNotFound`.
 
-Next steps
-- Integrate with a real token contract: add a `token_contract: Address` per vault or instance-level token address and call the token `transfer` to move funds.
-- Add more tests that register and interact with a real token contract in test `Env` to assert balances on ledger entries.
+## Security Notes
+
+- Authorization: `vault.creator.require_auth()` ensures only the vault owner can cancel.
+- Refund target: the caller cannot choose a refund recipient; funds always return to the stored `vault.creator`.
+- Escrow accounting: tests assert the creator balance increases by exactly `vault.amount` and the contract token balance returns to its pre-create level after cancellation.
+- Terminal-state safety: cancellation is rejected after release, redirect, or a prior cancellation, preventing double refunds.
+
+## Tests
+
+The lifecycle integration tests register a real Soroban token contract in `Env` and assert ledger balances with `TokenClient::balance`.
+
+Run the focused coverage with:
+
+```bash
+cargo test test_cancel_vault_refunds_creator_and_empties_contract_escrow
+cargo test test_cancel_vault_rejects_completed_failed_and_cancelled_vaults
+```
+
+The cancel coverage checks:
+
+- creator balance decreases by `vault.amount` on create
+- contract escrow balance increases by `vault.amount` on create
+- creator balance increases by `vault.amount` on cancel
+- contract escrow returns to its pre-create balance on cancel
+- success and failure destinations remain untouched
+- `vault_cancelled` is emitted
+- non-active vaults reject cancellation with `Error::VaultNotActive`

TESTING_GUIDE.md

@@ -97,6 +97,20 @@ cargo test test_vault_data_integrity
 cargo test test_sequential_operations
 ```
 
+### 5. Cancel Vault Refunds
+
+`tests/lifecycle.rs` covers the real-token refund path for `cancel_vault`.
+
+```bash
+cargo test test_cancel_vault_refunds_creator_and_empties_contract_escrow
+cargo test test_cancel_vault_rejects_completed_failed_and_cancelled_vaults
+```
+
+These tests assert that cancellation transfers exactly `vault.amount` from the
+contract escrow back to the creator, leaves success and failure destinations
+untouched, emits `vault_cancelled`, and rejects cancellation once a vault is in a
+terminal state.
+
 ## Coverage Reports
 
 ### Generate HTML Report

src/lib.rs

@@ -982,6 +982,35 @@ mod tests {
         assert_eq!(vault.status, VaultStatus::Cancelled);
     }
 
+    #[test]
+    fn test_cancel_vault_emits_cancelled_event() {
+        let setup = TestSetup::new();
+        let client = setup.client();
+
+        setup.env.ledger().set_timestamp(setup.start_timestamp);
+        let vault_id = setup.create_default_vault();
+
+        client.cancel_vault(&vault_id, &setup.usdc_token);
+
+        let found_cancel_event = setup
+            .env
+            .events()
+            .all()
+            .iter()
+            .any(|(contract, topics, _)| {
+                if contract != setup.contract_id {
+                    return false;
+                }
+
+                let event_name: Symbol = topics.get(0).unwrap().try_into_val(&setup.env).unwrap();
+                let event_vault_id: u32 = topics.get(1).unwrap().try_into_val(&setup.env).unwrap();
+                event_name == Symbol::new(&setup.env, "vault_cancelled")
+                    && event_vault_id == vault_id
+            });
+
+        assert!(found_cancel_event, "vault_cancelled event must be emitted");
+    }
+
     // -----------------------------------------------------------------------
     // More upstream tests migrated
     // -----------------------------------------------------------------------

tests/lifecycle.rs

@@ -6,7 +6,7 @@ use soroban_sdk::{
     Address, BytesN, Env,
 };
 
-use disciplr_vault::{DisciplrVault, DisciplrVaultClient, VaultStatus, MIN_AMOUNT};
+use disciplr_vault::{DisciplrVault, DisciplrVaultClient, Error, VaultStatus, MIN_AMOUNT};
 
 fn setup() -> (
     Env,
@@ -30,6 +30,16 @@ fn setup() -> (
     (env, client, usdc_addr, usdc_asset, usdc_token_client)
 }
 
+fn assert_contract_error<T: core::fmt::Debug>(
+    result: Result<T, Result<Error, soroban_sdk::InvokeError>>,
+    expected: Error,
+) {
+    match result {
+        Err(Ok(actual)) => assert_eq!(actual, expected),
+        other => panic!("unexpected result: {other:?}"),
+    }
+}
+
 #[test]
 fn test_full_lifecycle_success() {
     let (env, client, usdc, usdc_asset, usdc_token) = setup();
@@ -116,3 +126,128 @@ fn test_full_lifecycle_failure_redirection() {
     assert_eq!(final_state.status, VaultStatus::Failed);
     assert_eq!(usdc_token.balance(&failure_dest), MIN_AMOUNT);
 }
+
+#[test]
+fn test_cancel_vault_refunds_creator_and_empties_contract_escrow() {
+    let (env, client, usdc, usdc_asset, usdc_token) = setup();
+
+    let creator = Address::generate(&env);
+    let success_dest = Address::generate(&env);
+    let failure_dest = Address::generate(&env);
+    let now = 1_700_000_000u64;
+    env.ledger().set_timestamp(now);
+
+    usdc_asset.mint(&creator, &MIN_AMOUNT);
+    let creator_before_create = usdc_token.balance(&creator);
+    let contract_before_create = usdc_token.balance(&client.address);
+
+    let vault_id = client.create_vault(
+        &usdc,
+        &creator,
+        &MIN_AMOUNT,
+        &now,
+        &(now + 86_400),
+        &BytesN::from_array(&env, &[2u8; 32]),
+        &None,
+        &success_dest,
+        &failure_dest,
+    );
+
+    assert_eq!(
+        usdc_token.balance(&creator),
+        creator_before_create - MIN_AMOUNT
+    );
+    assert_eq!(
+        usdc_token.balance(&client.address),
+        contract_before_create + MIN_AMOUNT
+    );
+
+    let creator_before_cancel = usdc_token.balance(&creator);
+    let contract_before_cancel = usdc_token.balance(&client.address);
+
+    let result = client.cancel_vault(&vault_id, &usdc);
+
+    assert!(result);
+    assert_eq!(
+        usdc_token.balance(&creator),
+        creator_before_cancel + MIN_AMOUNT
+    );
+    assert_eq!(
+        usdc_token.balance(&client.address),
+        contract_before_cancel - MIN_AMOUNT
+    );
+    assert_eq!(usdc_token.balance(&client.address), contract_before_create);
+    assert_eq!(usdc_token.balance(&success_dest), 0);
+    assert_eq!(usdc_token.balance(&failure_dest), 0);
+
+    let final_state = client.get_vault_state(&vault_id).unwrap();
+    assert_eq!(final_state.status, VaultStatus::Cancelled);
+}
+
+#[test]
+fn test_cancel_vault_rejects_completed_failed_and_cancelled_vaults() {
+    let (env, client, usdc, usdc_asset, _) = setup();
+    let now = 1_700_000_000u64;
+    env.ledger().set_timestamp(now);
+
+    let completed_creator = Address::generate(&env);
+    let failed_creator = Address::generate(&env);
+    let cancelled_creator = Address::generate(&env);
+    usdc_asset.mint(&completed_creator, &MIN_AMOUNT);
+    usdc_asset.mint(&failed_creator, &MIN_AMOUNT);
+    usdc_asset.mint(&cancelled_creator, &MIN_AMOUNT);
+
+    let completed_id = client.create_vault(
+        &usdc,
+        &completed_creator,
+        &MIN_AMOUNT,
+        &now,
+        &(now + 86_400),
+        &BytesN::from_array(&env, &[3u8; 32]),
+        &None,
+        &Address::generate(&env),
+        &Address::generate(&env),
+    );
+    client.validate_milestone(&completed_id);
+    client.release_funds(&completed_id, &usdc);
+    assert_contract_error(
+        client.try_cancel_vault(&completed_id, &usdc),
+        Error::VaultNotActive,
+    );
+
+    let failed_id = client.create_vault(
+        &usdc,
+        &failed_creator,
+        &MIN_AMOUNT,
+        &now,
+        &(now + 86_400),
+        &BytesN::from_array(&env, &[4u8; 32]),
+        &None,
+        &Address::generate(&env),
+        &Address::generate(&env),
+    );
+    env.ledger().set_timestamp(now + 86_401);
+    client.redirect_funds(&failed_id, &usdc);
+    assert_contract_error(
+        client.try_cancel_vault(&failed_id, &usdc),
+        Error::VaultNotActive,
+    );
+
+    env.ledger().set_timestamp(now);
+    let cancelled_id = client.create_vault(
+        &usdc,
+        &cancelled_creator,
+        &MIN_AMOUNT,
+        &now,
+        &(now + 86_400),
+        &BytesN::from_array(&env, &[5u8; 32]),
+        &None,
+        &Address::generate(&env),
+        &Address::generate(&env),
+    );
+    client.cancel_vault(&cancelled_id, &usdc);
+    assert_contract_error(
+        client.try_cancel_vault(&cancelled_id, &usdc),
+        Error::VaultNotActive,
+    );
+}

tests/proptest_timestamps.rs

@@ -279,7 +279,6 @@ fn edge_start_eq_now_succeeds() {
     assert_eq!(vault.end_timestamp, end);
 }
 
-
 #[test]
 fn edge_start_eq_end_rejected() {
     let (env, client, usdc, usdc_asset) = setup();