diff --git a/contracts/savings_vault/src/test/balance_conservation.rs b/contracts/savings_vault/src/test/balance_conservation.rs index ab63cf5..af5ceb0 100644 --- a/contracts/savings_vault/src/test/balance_conservation.rs +++ b/contracts/savings_vault/src/test/balance_conservation.rs @@ -147,6 +147,13 @@ fn snapshot(client: &SavingsVaultClient, user: &Address) -> (i128, i128) { (client.get_balance(user), client.get_locked_balance(user)) } +fn token_snapshot(f: &Fixture) -> (i128, i128) { + ( + f.token_client.balance(&f.contract_id), + f.token_client.balance(&f.user), + ) +} + /// Run one operation sequence, checking conservation after every step. /// /// Returns the final expected total so callers can make extra assertions if needed. @@ -160,6 +167,7 @@ fn run_sequence(ops: &[(Op, Expect)]) -> i128 { for (step, (op, expect)) in ops.iter().enumerate() { let before = snapshot(&f.client, &f.user); + let tokens_before = token_snapshot(&f); match (op, expect) { (Op::Deposit(amount), Expect::Ok) => { @@ -177,6 +185,11 @@ fn run_sequence(ops: &[(Op, Expect)]) -> i128 { before, "step {step}: failed deposit must not mutate balances" ); + assert_eq!( + token_snapshot(&f), + tokens_before, + "step {step}: failed deposit must not move tokens" + ); } (Op::Withdraw(amount), Expect::Ok) => { f.client.withdraw(&f.user, amount); @@ -193,6 +206,11 @@ fn run_sequence(ops: &[(Op, Expect)]) -> i128 { before, "step {step}: failed withdraw must not mutate balances" ); + assert_eq!( + token_snapshot(&f), + tokens_before, + "step {step}: failed withdraw must not move tokens" + ); } ( Op::Lock { @@ -223,6 +241,11 @@ fn run_sequence(ops: &[(Op, Expect)]) -> i128 { before, "step {step}: failed lock must not mutate balances" ); + assert_eq!( + token_snapshot(&f), + tokens_before, + "step {step}: failed lock must not move tokens" + ); } (Op::WithdrawLock(idx), Expect::Ok) => { let lock_id = lock_ids[*idx]; @@ -231,7 +254,26 @@ fn run_sequence(ops: &[(Op, Expect)]) -> i128 { expected_total -= lock_amt; } (Op::WithdrawLock(_), Expect::Err) => { - panic!("step {step}: WithdrawLock cannot fail for valid indices"); + let lock_id = if *idx < lock_ids.len() { + lock_ids[*idx] + } else { + u64::MAX + }; + let res = f.client.try_withdraw_lock(&f.user, &lock_id); + assert!( + res.is_err(), + "step {step}: withdraw_lock({lock_id}) was expected to fail" + ); + assert_eq!( + snapshot(&f.client, &f.user), + before, + "step {step}: failed withdraw_lock must not mutate balances" + ); + assert_eq!( + token_snapshot(&f), + tokens_before, + "step {step}: failed withdraw_lock must not move tokens" + ); } (Op::SetTime(ts), Expect::Ok) => { set_ledger_timestamp(&f.env, *ts); @@ -495,6 +537,37 @@ fn conservation_invalid_locks_do_not_mutate() { assert_eq!(total, 250); } +/// Attempting to withdraw an unmatured lock must fail and leave balances unchanged. +#[test] +fn conservation_failed_withdraw_lock_does_not_mutate() { + run_sequence(&[ + (Op::Deposit(500), Expect::Ok), + ( + Op::Lock { + amount: 400, + unlock_time: 20_000, + }, + Expect::Ok, + ), + // Lock is not yet matured; withdraw_lock must fail and roll back. + (Op::WithdrawLock(0), Expect::Err), + (Op::Withdraw(100), Expect::Ok), + // After maturity, the lock can be withdrawn. + (Op::SetTime(20_000), Expect::Ok), + (Op::WithdrawLock(0), Expect::Ok), + ]); +} + +/// Withdrawing a non-existent lock must fail and leave balances unchanged. +#[test] +fn conservation_invalid_withdraw_lock_id_does_not_mutate() { + run_sequence(&[ + (Op::Deposit(100), Expect::Ok), + (Op::WithdrawLock(999), Expect::Err), + (Op::Withdraw(100), Expect::Ok), + ]); +} + /// Withdraw that would touch only locked (unmatured) funds must fail and not mutate. #[test] fn conservation_withdraw_exceeds_available_while_locked_does_not_mutate() { diff --git a/contracts/savings_vault/src/test/lock_atomicity.rs b/contracts/savings_vault/src/test/lock_atomicity.rs index f4269b5..692fdce 100644 --- a/contracts/savings_vault/src/test/lock_atomicity.rs +++ b/contracts/savings_vault/src/test/lock_atomicity.rs @@ -165,3 +165,95 @@ fn test_failed_lock_creates_no_partial_record() { assert_eq!(client.get_balance(&user), 900); assert_eq!(client.get_locked_balance(&user), 100); } + +// --------------------------------------------------------------------------- +// Failed withdrawal leaves state unchanged +// --------------------------------------------------------------------------- + +/// A withdrawal for more than the available balance is rejected and leaves balances untouched. +#[test] +fn test_failed_withdrawal_insufficient_balance_leaves_state_intact() { + let env = test_env(); + let (_admin, client) = init_with_admin(&env); + let user = Address::generate(&env); + + env.ledger().set_timestamp(1_000); + fund(&client, &user, 1_000); + + let balance_before = client.get_balance(&user); + let locked_before = client.get_locked_balance(&user); + + let res = client.try_withdraw(&user, &(balance_before + 1)); + assert!(res.is_err()); + + assert_eq!(client.get_balance(&user), balance_before); + assert_eq!(client.get_locked_balance(&user), locked_before); +} + +/// A zero-amount withdrawal is rejected and leaves balances untouched. +#[test] +fn test_failed_withdrawal_zero_amount_leaves_state_intact() { + let env = test_env(); + let (_admin, client) = init_with_admin(&env); + let user = Address::generate(&env); + + env.ledger().set_timestamp(1_000); + fund(&client, &user, 1_000); + + let balance_before = client.get_balance(&user); + let locked_before = client.get_locked_balance(&user); + + let res = client.try_withdraw(&user, &0); + assert!(res.is_err()); + + assert_eq!(client.get_balance(&user), balance_before); + assert_eq!(client.get_locked_balance(&user), locked_before); +} + +/// A failed withdrawal does not alter existing lock records or locked balances. +#[test] +fn test_failed_withdrawal_preserves_locks() { + let env = test_env(); + let (_admin, client) = init_with_admin(&env); + let user = Address::generate(&env); + + env.ledger().set_timestamp(1_000); + fund(&client, &user, 1_000); + let id = client.lock_funds(&user, &300, &(env.ledger().timestamp() + 100)); + let lock_before = client.get_lock(&user, &id).expect("lock should exist"); + + let balance_before = client.get_balance(&user); + let res = client.try_withdraw(&user, &(balance_before + 1)); + assert!(res.is_err()); + + assert_eq!(client.get_balance(&user), balance_before); + assert_eq!(client.get_locked_balance(&user), 300); + let lock_after = client.get_lock(&user, &id).expect("lock should still exist"); + assert_eq!(lock_after.amount, lock_before.amount); + assert_eq!(lock_after.withdrawn, lock_before.withdrawn); +} + +// --------------------------------------------------------------------------- +// Failed deposit leaves state unchanged +// --------------------------------------------------------------------------- + +/// A zero-amount deposit is rejected and leaves balances untouched. +#[test] +fn test_failed_deposit_zero_amount_leaves_state_intact() { + let env = test_env(); + let (_admin, client) = init_with_admin(&env); + let user = Address::generate(&env); + + env.ledger().set_timestamp(1_000); + fund(&client, &user, 1_000); + + let balance_before = client.get_balance(&user); + let locked_before = client.get_locked_balance(&user); + + let res = client.try_deposit(&user, &0); + assert!(res.is_err()); + + assert_eq!(client.get_balance(&user), balance_before); + assert_eq!(client.get_locked_balance(&user), locked_before); +} + diff --git a/contracts/savings_vault/src/test/multi_lock_invariants.rs b/contracts/savings_vault/src/test/multi_lock_invariants.rs index 3d998bd..2c323aa 100644 --- a/contracts/savings_vault/src/test/multi_lock_invariants.rs +++ b/contracts/savings_vault/src/test/multi_lock_invariants.rs @@ -226,46 +226,74 @@ fn multi_lock_failed_operations_do_not_mutate() { assert_conserved(&client, &user, expected); // Create 3 locks: available=400, locked=600 - client.lock_funds(&user, &200, &3_000); + let id1 = client.lock_funds(&user, &200, &3_000); client.lock_funds(&user, &300, &5_000); client.lock_funds(&user, &100, &7_000); assert_conserved(&client, &user, expected); let before = snapshot(&client, &user); + let locks_before = client.list_locks(&user, &0u32, &50u32); + + macro_rules! assert_unchanged { + () => { + assert_eq!(snapshot(&client, &user), before); + assert_eq!(client.list_locks(&user, &0u32, &50u32), locks_before); + } + } // Lock more than available let res = client.try_lock_funds(&user, &401, &10_000); assert!(res.is_err()); - assert_eq!(snapshot(&client, &user), before); + assert_unchanged!(); // Lock zero let res = client.try_lock_funds(&user, &0, &10_000); assert!(res.is_err()); - assert_eq!(snapshot(&client, &user), before); + assert_unchanged!(); + + // Lock negative amount + let res = client.try_lock_funds(&user, &-50, &10_000); + assert!(res.is_err()); + assert_unchanged!(); // Lock with past unlock let res = client.try_lock_funds(&user, &50, &500); assert!(res.is_err()); - assert_eq!(snapshot(&client, &user), before); + assert_unchanged!(); // Withdraw more than available let res = client.try_withdraw(&user, &401); assert!(res.is_err()); - assert_eq!(snapshot(&client, &user), before); + assert_unchanged!(); // Withdraw zero let res = client.try_withdraw(&user, &0); assert!(res.is_err()); - assert_eq!(snapshot(&client, &user), before); + assert_unchanged!(); + + // Withdraw negative amount + let res = client.try_withdraw(&user, &-10); + assert!(res.is_err()); + assert_unchanged!(); // Deposit zero / negative let res = client.try_deposit(&user, &0); assert!(res.is_err()); - assert_eq!(snapshot(&client, &user), before); + assert_unchanged!(); let res = client.try_deposit(&user, &-10); assert!(res.is_err()); - assert_eq!(snapshot(&client, &user), before); + assert_unchanged!(); + + // Withdraw_lock on unmatured lock + let res = client.try_withdraw_lock(&user, &id1); + assert!(res.is_err()); + assert_unchanged!(); + + // Withdraw_lock on nonexistent lock + let res = client.try_withdraw_lock(&user, &999); + assert!(res.is_err()); + assert_unchanged!(); // State still intact assert_lock_sum_consistency(&env, &client, &user); @@ -626,6 +654,14 @@ fn multi_lock_deterministic_sequence_invariants() { user_idx: 2, amount: 30_000, }, + Operation::FailDeposit { + user_idx: 0, + amount: 0, + }, + Operation::FailDeposit { + user_idx: 1, + amount: -50, + }, Operation::Lock { user_idx: 0, amount: 2_000, diff --git a/contracts/savings_vault/src/test/negative_paths.rs b/contracts/savings_vault/src/test/negative_paths.rs index 0c85c17..81d6805 100644 --- a/contracts/savings_vault/src/test/negative_paths.rs +++ b/contracts/savings_vault/src/test/negative_paths.rs @@ -219,6 +219,79 @@ fn test_state_remains_consistent_after_failed_lock() { assert_eq!(client.get_locked_balance(&user), initial_locked, "Locked balance should not change after failed lock"); } +#[test] +fn test_state_remains_consistent_after_failed_withdrawal() { + 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); + let initial_balance = client.get_balance(&user); + let initial_locked = client.get_locked_balance(&user); + + // Attempt to withdraw the lock before it matures + set_ledger_timestamp(&env, 4999); + let res = client.try_withdraw_lock(&user, &lock_id); + assert!(res.is_err(), "Early withdrawal should fail"); + + assert_eq!(client.get_balance(&user), initial_balance, "Balance should not change after failed withdrawal"); + assert_eq!(client.get_locked_balance(&user), initial_locked, "Locked balance should not change after failed withdrawal"); +} + +#[test] +fn test_state_remains_consistent_after_failed_withdraw() { + 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(); + token_admin.mint(&user, &1000); + client.deposit(&user, &1000); + + let initial_balance = client.get_balance(&user); + let initial_locked = client.get_locked_balance(&user); + let initial_user_tokens = token_client.balance(&user); + let initial_vault_tokens = token_client.balance(&contract_id); + + // Attempt to withdraw more than the unlocked balance + let res = client.try_withdraw(&user, &1001); + assert!(res.is_err(), "Withdraw should fail due to insufficient unlocked balance"); + + assert_eq!(client.get_balance(&user), initial_balance, "Balance should not change after failed withdraw"); + assert_eq!(client.get_locked_balance(&user), initial_locked, "Locked balance should not change after failed withdraw"); + assert_eq!(token_client.balance(&user), initial_user_tokens, "User tokens should not change after failed withdraw"); + assert_eq!(token_client.balance(&contract_id), initial_vault_tokens, "Vault tokens should not change after failed withdraw"); +} + +#[test] +fn test_state_remains_consistent_after_invalid_amount() { + 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(); + token_admin.mint(&user, &1000); + client.deposit(&user, &1000); + + let initial_balance = client.get_balance(&user); + let initial_locked = client.get_locked_balance(&user); + + // Invalid deposit amounts must not change user state + let res = client.try_deposit(&user, &0); + assert!(res.is_err()); + assert_eq!(client.get_balance(&user), initial_balance, "Balance should not change after zero deposit"); + assert_eq!(client.get_locked_balance(&user), initial_locked, "Locked balance should not change after zero deposit"); + let res = client.try_deposit(&user, &-1); + assert!(res.is_err()); + assert_eq!(client.get_balance(&user), initial_balance, "Balance should not change after negative deposit"); + assert_eq!(client.get_locked_balance(&user), initial_locked, "Locked balance should not change after negative deposit"); +} + #[test] fn test_state_consistency_after_failed_token_transfer() { let env = test_env(); diff --git a/contracts/savings_vault/src/test/token_transfer_rollback.rs b/contracts/savings_vault/src/test/token_transfer_rollback.rs index 1d907d6..db1a998 100644 --- a/contracts/savings_vault/src/test/token_transfer_rollback.rs +++ b/contracts/savings_vault/src/test/token_transfer_rollback.rs @@ -90,7 +90,9 @@ fn test_failed_deposit_insufficient_token_balance() { ); assert_eq!( locked_after, locked_before, - "locked balance must not change on failed deposit" + "locked balance unchanged after 5 failed ops" + ); +}cked balance must not change on failed deposit" ); assert_eq!( events_after, events_before, @@ -501,3 +503,92 @@ fn test_failed_withdraw_token_transfer_failure_preserves_state() { "lock withdrawn flag unchanged after failed withdraw" ); } + +// ──────────────────────────────────────────────────────────── +// invalid amount rollback +// ──────────────────────────────────────────────────────────── + +#[test] +fn test_failed_deposit_invalid_amount_rollback() { + // Negative amounts are rejected before any state can be written. + let env = test_env(); + let (_contract_id, client, _token_client, token_admin, _) = vault_with_sac(&env); + let user = Address::generate(&env); + + token_admin.mint(&user, &1_000); + + let (bal_before, locked_before, events_before) = snapshot(&env, &client, &user); + + let result = client.try_deposit(&user, &(-1)); + assert!(result.is_err(), "deposit with negative amount must fail"); + + let (bal_after, locked_after, events_after) = snapshot(&env, &client, &user); + assert_eq!(bal_after, bal_before); + assert_eq!(locked_after, locked_before); + assert_eq!(events_after, events_before); +} + +#[test] +fn test_failed_withdraw_invalid_amount_rollback() { + // User has a real balance; a negative withdrawal must not touch it. + let env = test_env(); + let (_contract_id, client, _token_client, token_admin, _) = vault_with_sac(&env); + let user = Address::generate(&env); + + token_admin.mint(&user, &1_000); + client.deposit(&user, &500); + + let (bal_before, locked_before, events_before) = snapshot(&env, &client, &user); + + let result = client.try_withdraw(&user, &(-1)); + assert!(result.is_err(), "withdraw with negative amount must fail"); + + let (bal_after, locked_after, events_after) = snapshot(&env, &client, &user); + assert_eq!(bal_after, bal_before); + assert_eq!(locked_after, locked_before); + assert_eq!(events_after, events_before); +} + +#[test] +fn test_failed_lock_funds_invalid_amount_rollback() { + // A failed lock_funds call must leave available balance and existing lock + // state unchanged. + let env = test_env(); + let (_contract_id, client, _token_client, token_admin, _) = vault_with_sac(&env); + let user = Address::generate(&env); + + token_admin.mint(&user, &1_000); + client.deposit(&user, &500); + + let (bal_before, locked_before, events_before) = snapshot(&env, &client, &user); + + let result = client.try_lock_funds(&user, &(-1), &10_000); + assert!(result.is_err(), "lock_funds with negative amount must fail"); + + let (bal_after, locked_after, events_after) = snapshot(&env, &client, &user); + assert_eq!(bal_after, bal_before); + assert_eq!(locked_after, locked_before); + assert_eq!(events_after, events_before); +} + +#[test] +fn test_failed_lock_funds_insufficient_available_rollback() { + // Locking more than the available balance must fail without changing state. + let env = test_env(); + let (_contract_id, client, _token_client, token_admin, _) = vault_with_sac(&env); + let user = Address::generate(&env); + + token_admin.mint(&user, &1_000); + client.deposit(&user, &500); + + let (bal_before, locked_before, events_before) = snapshot(&env, &client, &user); + assert_eq!(bal_before, 500); + + let result = client.try_lock_funds(&user, &501, &10_000); + assert!(result.is_err(), "lock_funds exceeding available must fail"); + + let (bal_after, locked_after, events_after) = snapshot(&env, &client, &user); + assert_eq!(bal_after, bal_before); + assert_eq!(locked_after, locked_before); + assert_eq!(events_after, events_before); +}