chore(soroban): feature distinct-destinations

README.md

@@ -938,6 +938,7 @@ During `create_vault`, the contract enforces:
 - `start_timestamp` must not be in the past
 - `end_timestamp` must be strictly greater than `start_timestamp`
 - `end_timestamp - start_timestamp` must not exceed `MAX_VAULT_DURATION`
+- `success_destination` must differ from `failure_destination` (returns `Error::SameDestination`, code `#10`); equal destinations make the success/failure outcome financially indistinguishable, removing the accountability incentive of the vault
 
 All validations occur before event emission or state mutation, ensuring invalid vaults cannot be created.
 

src/lib.rs

@@ -8,11 +8,47 @@
 all-features = true
 workspace = true
 
+<<<<<<< feature/distinct-destinations
+// ---------------------------------------------------------------------------
+// Errors
+// ---------------------------------------------------------------------------
+//
+// Contract-specific errors used in revert paths. Follows Soroban error
+// conventions: use Result<T, Error> and return Err(Error::Variant) instead
+// of generic panics where appropriate.
+
+#[contracterror]
+#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
+#[repr(u32)]
+pub enum Error {
+    /// Vault with the given id does not exist.
+    VaultNotFound = 1,
+    /// Caller is not authorized for this operation (e.g. not verifier/creator, or release before deadline without validation).
+    NotAuthorized = 2,
+    /// Vault is not in Active status (e.g. already Completed, Failed, or Cancelled).
+    VaultNotActive = 3,
+    /// Timestamp constraint violated (e.g. redirect before end_timestamp, or invalid time window).
+    InvalidTimestamp = 4,
+    /// Validation is no longer allowed because current time is at or past end_timestamp.
+    MilestoneExpired = 5,
+    /// Vault is in an invalid status for the requested operation.
+    InvalidStatus = 6,
+    /// Amount must be positive (e.g. create_vault amount <= 0).
+    InvalidAmount = 7,
+    /// start_timestamp must be strictly less than end_timestamp.
+    InvalidTimestamps = 8,
+    /// Vault duration (end − start) exceeds MAX_VAULT_DURATION.
+    DurationTooLong = 9,
+    /// success_destination and failure_destination must be different addresses.
+    SameDestination = 10,
+}
+=======
 # Exclude test code from coverage
 exclude-files = []
 
 # Count branches for more accurate coverage
 count-branches = true
+>>>>>>> main
 
 // ---------------------------------------------------------------------------
 // Data types
@@ -84,10 +120,20 @@
 impl DisciplrVault {
     /// Create a new productivity vault.
     ///
+<<<<<<< feature/distinct-destinations
+    /// # Validation Rules
+    /// - `amount` must be within `[MIN_AMOUNT, MAX_AMOUNT]`; otherwise returns `Error::InvalidAmount`.
+    /// - `start_timestamp` must be strictly less than `end_timestamp`; otherwise returns `Error::InvalidTimestamps`.
+    /// - `end_timestamp - start_timestamp` must not exceed `MAX_VAULT_DURATION`; otherwise returns `Error::DurationTooLong`.
+    /// - `success_destination` must differ from `failure_destination`; otherwise returns `Error::SameDestination`.
+    ///   Allowing equal destinations would make the success/failure outcome indistinguishable to the
+    ///   creator, removing the accountability incentive that is the core purpose of the vault.
+=======
     /// This function follows the **Checks-Effects-Interactions** pattern:
     /// 1. **Checks**: Validates `amount`, `start_timestamp`, `end_timestamp`, and `creator` authorization.
     /// 2. **Interactions**: Transfers USDC from `creator` to the contract.
     /// 3. **Effects**: Increments `VaultCount`, creates the `ProductivityVault` record, and emits `vault_created`.
+>>>>>>> main
     ///
     /// # Errors
     /// - `Error::InvalidAmount`: if amount is not within [MIN_AMOUNT, MAX_AMOUNT].
@@ -131,6 +177,11 @@
             return Err(Error::DurationTooLong);
         }
 
+        // Validate destinations are distinct
+        if success_destination == failure_destination {
+            return Err(Error::SameDestination);
+        }
+
         // Pull USDC from creator into this contract.
         let token_client = token::Client::new(&env, &usdc_token);
         token_client.transfer(&creator, &env.current_contract_address(), &amount);
@@ -1281,7 +1332,7 @@
         let client = setup.client();
         client.cancel_vault(&999u32, &setup.usdc_token);
     }
 }
 
 #[cfg(test)]
 mod test {

tests/create_vault.rs

@@ -337,6 +337,74 @@ fn test_get_vault_state_never_created_id_returns_none() {
     assert!(client.get_vault_state(&42u32).is_none());
 }
 
+// ---------------------------------------------------------------------------
+// Issue #124: success_destination vs failure_destination equality
+// ---------------------------------------------------------------------------
+
+/// Verifica que create_vault rechaza cuando success_destination == failure_destination.
+/// Implicación UX: si ambas direcciones fueran iguales, el resultado de éxito y fracaso
+/// sería indistinguible para el creador, eliminando el incentivo de cumplir el milestone.
+#[test]
+#[should_panic(expected = "Error(Contract, #10)")]
+fn test_same_destination_rejected() {
+    let (env, client, usdc, usdc_asset) = setup();
+
+    let creator = Address::generate(&env);
+    let now = 1_725_000_000u64;
+    env.ledger().set_timestamp(now);
+
+    usdc_asset.mint(&creator, &MIN_AMOUNT);
+
+    // Misma dirección para success y failure
+    let same_dest = Address::generate(&env);
+
+    client.create_vault(
+        &usdc,
+        &creator,
+        &MIN_AMOUNT,
+        &now,
+        &(now + 86_400),
+        &BytesN::from_array(&env, &[0u8; 32]),
+        &None,
+        &same_dest,
+        &same_dest,
+    );
+}
+
+/// Verifica que create_vault acepta cuando success_destination != failure_destination.
+/// Caso explícito del constraint introducido en el issue #124.
+#[test]
+fn test_different_destinations_accepted() {
+    let (env, client, usdc, usdc_asset) = setup();
+
+    let creator = Address::generate(&env);
+    let now = 1_725_000_000u64;
+    env.ledger().set_timestamp(now);
+
+    usdc_asset.mint(&creator, &MIN_AMOUNT);
+
+    let success = Address::generate(&env);
+    let failure = Address::generate(&env);
+
+    // Las dos direcciones son distintas: debe crearse sin error
+    let vault_id = client.create_vault(
+        &usdc,
+        &creator,
+        &MIN_AMOUNT,
+        &now,
+        &(now + 86_400),
+        &BytesN::from_array(&env, &[0u8; 32]),
+        &None,
+        &success,
+        &failure,
+    );
+
+    assert_eq!(vault_id, 0u32);
+
+    let vault = client.get_vault_state(&vault_id).unwrap();
+    assert_ne!(vault.success_destination, vault.failure_destination);
+}
+
 #[test]
 fn test_get_vault_state_cancelled_vault_remains_readable() {
     let (env, client, usdc, usdc_asset) = setup();

vesting.md

@@ -155,7 +155,10 @@ pub fn create_vault(
 
 **Requirements:**
 - Caller must authorize the transaction (`creator.require_auth()`)
-- `end_timestamp` must be greater than `start_timestamp`
+- `amount` must be within `[MIN_AMOUNT, MAX_AMOUNT]`; otherwise returns `Error::InvalidAmount`
+- `start_timestamp` must be strictly less than `end_timestamp`; otherwise returns `Error::InvalidTimestamps`
+- `end_timestamp - start_timestamp` must not exceed `MAX_VAULT_DURATION`; otherwise returns `Error::DurationTooLong`
+- `success_destination` must differ from `failure_destination`; otherwise returns `Error::SameDestination` (error code `#10`)
 - USDC transfer must be approved by creator before calling
 >>>>>>> main
 
@@ -629,6 +632,13 @@ The Disciplr Vault strictly follows the **Checks-Effects-Interactions** pattern
 =======
 ## Security and Trust Model
 
+<<<<<<< feature/distinct-destinations
+1. **Per-Call Token Address**: The `usdc_token` address is passed as an argument to release/redirect functions rather than being pinned to the vault data at creation. This introduces a risk where a malicious caller could potentially pass a different token address (though they would still need the contract to hold a balance of that token).
+2. **Checks-Effects-Interactions (CEI)**: In `release_funds`, `redirect_funds`, and `cancel_vault`, the USDC transfer is initiated *before* the internal status is updated to `Completed`, `Failed`, or `Cancelled`. While Soroban's atomicity safeguards against most reentrancy/partial-success risks, this is a deviation from the strict CEI pattern.
+3. **Lack of Emergency Stops**: There is currently no circuit breaker or emergency pause mechanism.
+4. **Precision**: All amounts are handled as `i128` in stroops (7 decimals for USDC); users must ensure they provide correct decimal-adjusted amounts.
+5. **Equal Destinations Rejected (Issue #124)**: `success_destination` and `failure_destination` must be different addresses. If they were equal, the outcome of the vault (success vs. failure) would be financially indistinguishable for the creator: funds would arrive at the same address regardless of whether the milestone was completed. This eliminates the accountability incentive that is the core purpose of the vault, and could be used to disguise a vault with no real consequence for non-completion. The contract enforces this at creation time with `Error::SameDestination` (code `#10`).
+=======
 This section outlines the security properties, trust assumptions, and known limitations of the Disciplr Vault contract.
 >>>>>>> main
 
@@ -651,6 +661,7 @@ While Soroban provides inherent atomicity (reverting all changes if any part fai
 *   **Reentrancy Prevention**: Prevents a malicious recipient contract from re-entering the vault contract and attempting to double-release funds before the state is updated.
 *   **Logical Clarity**: Makes the security properties of the contract easier to audit and verify.
 *   **Resilience**: Protects against unexpected behavior in complex multi-contract interactions.
+>>>>>>> main
 
 ### Known Limitations & Security Notes