diff --git a/README_CANCEL_VAULT.md b/README_CANCEL_VAULT.md index 9bbf181..b5a0a85 100644 --- a/README_CANCEL_VAULT.md +++ b/README_CANCEL_VAULT.md @@ -2,22 +2,23 @@ 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. +- On cancel it transfers the escrowed token amount from the vault contract back to the creator and emits a `vault_cancelled` event. 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)`. +- Refund uses the Soroban token client to transfer `vault.amount` from `env.current_contract_address()` to `vault.creator`. +- The vault record remains stored and its `status` is set to `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). +- Real token transfer: `create_vault` escrows funds in the contract token balance, and `cancel_vault` sends those funds back to the creator. +- Reentrancy: token transfers should continue to use Soroban token client patterns. Keep state updates and event emission explicit when changing this flow. - Authorization: `creator.require_auth()` ensures only the vault owner can cancel. 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. +- Unit tests cover successful cancel by creator, unauthorized cancel, nonexistent vaults, double cancel, and inactive `Completed` / `Failed` / `Cancelled` vault rejection. +- The lifecycle integration test asserts the real token refund by checking that the creator receives exactly `vault.amount`, the contract escrow balance returns to its pre-create value, and the vault status becomes `Cancelled`. 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. +- Keep README, `src/doc.md`, and `contract-interface.json` aligned if the event topic, refund amount, or lifecycle status changes. +- Preserve the balance-delta assertions when adding new token escrow paths. diff --git a/tests/lifecycle.rs b/tests/lifecycle.rs index 7079583..d4c55b6 100644 --- a/tests/lifecycle.rs +++ b/tests/lifecycle.rs @@ -1,9 +1,9 @@ #![cfg(test)] use soroban_sdk::{ - testutils::{Address as _, Ledger}, + testutils::{Address as _, Events, Ledger}, token::{StellarAssetClient, TokenClient}, - Address, BytesN, Env, + Address, BytesN, Env, Symbol, TryIntoVal, }; use disciplr_vault::{DisciplrVault, DisciplrVaultClient, VaultStatus, MIN_AMOUNT}; @@ -14,6 +14,7 @@ fn setup() -> ( Address, StellarAssetClient<'static>, TokenClient<'static>, + Address, ) { let env = Env::default(); env.mock_all_auths(); @@ -27,12 +28,19 @@ fn setup() -> ( let usdc_asset = StellarAssetClient::new(&env, &usdc_addr); let usdc_token_client = TokenClient::new(&env, &usdc_addr); - (env, client, usdc_addr, usdc_asset, usdc_token_client) + ( + env, + client, + usdc_addr, + usdc_asset, + usdc_token_client, + contract_id, + ) } #[test] fn test_full_lifecycle_success() { - let (env, client, usdc, usdc_asset, usdc_token) = setup(); + let (env, client, usdc, usdc_asset, usdc_token, _contract_id) = setup(); let creator = Address::generate(&env); let verifier = Address::generate(&env); @@ -82,7 +90,7 @@ fn test_full_lifecycle_success() { #[test] fn test_full_lifecycle_failure_redirection() { - let (env, client, usdc, usdc_asset, usdc_token) = setup(); + let (env, client, usdc, usdc_asset, usdc_token, _contract_id) = setup(); let creator = Address::generate(&env); let success_dest = Address::generate(&env); @@ -116,3 +124,72 @@ 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_clears_contract_escrow() { + let (env, client, usdc, usdc_asset, usdc_token, contract_id) = setup(); + + let creator = Address::generate(&env); + let verifier = 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(&contract_id); + let milestone = BytesN::from_array(&env, &[1u8; 32]); + + let vault_id = client.create_vault( + &usdc, + &creator, + &MIN_AMOUNT, + &now, + &(now + 86_400), + &milestone, + &Some(verifier), + &success_dest, + &failure_dest, + ); + + assert_eq!( + creator_before_create - usdc_token.balance(&creator), + MIN_AMOUNT + ); + assert_eq!( + usdc_token.balance(&contract_id) - contract_before_create, + MIN_AMOUNT + ); + + let creator_before_cancel = usdc_token.balance(&creator); + 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(&contract_id), contract_before_create); + assert_eq!( + client.get_vault_state(&vault_id).unwrap().status, + VaultStatus::Cancelled + ); + + let mut found_cancel_event = false; + for (emitting_contract, topics, _) in env.events().all() { + if emitting_contract != contract_id { + continue; + } + + let event_name: Symbol = topics.get(0).unwrap().try_into_val(&env).unwrap(); + if event_name == Symbol::new(&env, "vault_cancelled") { + let event_vault_id: u32 = topics.get(1).unwrap().try_into_val(&env).unwrap(); + assert_eq!(event_vault_id, vault_id); + found_cancel_event = true; + } + } + + assert!(found_cancel_event, "vault_cancelled event must be emitted"); +}