From 8879e351d68296d27fb3a064626f141d84af8efc Mon Sep 17 00:00:00 2001 From: licette32 Date: Mon, 30 Mar 2026 07:47:58 -0300 Subject: [PATCH] chore(soroban): feature distinct-destinations --- README.md | 1 + src/lib.rs | 13 ++++++++- tests/create_vault.rs | 68 +++++++++++++++++++++++++++++++++++++++++++ vesting.md | 6 +++- 4 files changed, 86 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 72e375c..b5a9e21 100644 --- a/README.md +++ b/README.md @@ -761,6 +761,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. diff --git a/src/lib.rs b/src/lib.rs index 70ae578..97e0d64 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,6 +35,8 @@ pub enum Error { InvalidTimestamps = 8, /// Vault duration (end − start) exceeds MAX_VAULT_DURATION. DurationTooLong = 9, + /// success_destination and failure_destination must be different addresses. + SameDestination = 10, } // --------------------------------------------------------------------------- @@ -108,8 +110,12 @@ impl DisciplrVault { /// Create a new productivity vault. Transfers USDC from creator to contract. /// /// # Validation Rules - /// - `amount` must be positive; otherwise returns `Error::InvalidAmount`. + /// - `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. /// /// # Prerequisites /// Creator must have sufficient USDC balance and authorize the transaction. @@ -150,6 +156,11 @@ impl DisciplrVault { 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); diff --git a/tests/create_vault.rs b/tests/create_vault.rs index 545c8d2..eba9805 100644 --- a/tests/create_vault.rs +++ b/tests/create_vault.rs @@ -270,6 +270,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(); diff --git a/vesting.md b/vesting.md index d3479e5..61870d9 100644 --- a/vesting.md +++ b/vesting.md @@ -103,7 +103,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 **Emits:** [`vault_created`](#vault_created) event @@ -322,6 +325,7 @@ This section outlines the security properties, trust assumptions, and known limi 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`). ### Recommendations for Production