fix: enforce checks-effects-interactions ordering

USDC_INTEGRATION.md

@@ -107,13 +107,16 @@ The contract cannot guarantee:
 
 - The `usdc_token` address is supplied at call time and is not pinned inside vault state.
 - The integrated token contract is therefore part of the effective security boundary.
+- Settlement paths use checks-effects-interactions ordering so terminal status is
+  persisted before the token transfer call.
 - Production review should include both the Disciplr contract and the chosen Stellar USDC asset configuration.
 
 ## Related Documentation
 
 - [README.md](README.md)
 - [vesting.md](vesting.md)
 - [src/lib.rs](src/lib.rs)
+- [docs/SECURITY_REENTRANCY.md](docs/SECURITY_REENTRANCY.md)
 
 ## Primary References
 

docs/SECURITY_REENTRANCY.md

@@ -0,0 +1,18 @@
+# Reentrancy and Terminal State Ordering
+
+Disciplr vault settlement functions follow checks-effects-interactions ordering.
+Each fund-moving terminal path writes its final vault status before invoking the
+external token contract:
+
+- `release_funds` writes `VaultStatus::Completed` before transferring to `success_destination`.
+- `redirect_funds` writes `VaultStatus::Failed` before transferring to `failure_destination`.
+- `cancel_vault` writes `VaultStatus::Cancelled` before transferring back to `creator`.
+
+This matters because `usdc_token` is supplied as a contract address at call time.
+If a malicious or non-standard token implementation attempted to re-enter the
+vault during transfer, the nested call would observe a non-`Active` vault and fail
+with `Error::VaultNotActive`.
+
+The regression tests cover the terminal-state matrix by asserting that, after a
+successful release, redirect, or cancel call, every second terminal path is
+rejected and no second balance delta is paid.

src/lib.rs

@@ -254,6 +254,9 @@ impl DisciplrVault {
     // -----------------------------------------------------------------------
 
     /// Release vault funds to `success_destination`.
+    ///
+    /// Applies checks-effects-interactions ordering: the vault is marked
+    /// `Completed` before the external token transfer is invoked.
     pub fn release_funds(env: Env, vault_id: u32, usdc_token: Address) -> Result<bool, Error> {
         let vault_key = DataKey::Vault(vault_id);
         let mut vault: ProductivityVault = env
@@ -277,16 +280,16 @@ impl DisciplrVault {
             return Err(Error::NotAuthorized);
         }
 
+        vault.status = VaultStatus::Completed;
+        env.storage().instance().set(&vault_key, &vault);
+
         let token_client = token::Client::new(&env, &usdc_token);
         token_client.transfer(
             &env.current_contract_address(),
             &vault.success_destination,
             &vault.amount,
         );
 
-        vault.status = VaultStatus::Completed;
-        env.storage().instance().set(&vault_key, &vault);
-
         env.events().publish(
             (Symbol::new(&env, "funds_released"), vault_id),
             vault.amount,
@@ -299,6 +302,9 @@ impl DisciplrVault {
     // -----------------------------------------------------------------------
 
     /// Redirect funds to `failure_destination` (e.g. after deadline without validation).
+    ///
+    /// Applies checks-effects-interactions ordering: the vault is marked
+    /// `Failed` before the external token transfer is invoked.
     pub fn redirect_funds(env: Env, vault_id: u32, usdc_token: Address) -> Result<bool, Error> {
         let vault_key = DataKey::Vault(vault_id);
         let mut vault: ProductivityVault = env
@@ -320,16 +326,16 @@ impl DisciplrVault {
             return Err(Error::NotAuthorized);
         }
 
+        vault.status = VaultStatus::Failed;
+        env.storage().instance().set(&vault_key, &vault);
+
         let token_client = token::Client::new(&env, &usdc_token);
         token_client.transfer(
             &env.current_contract_address(),
             &vault.failure_destination,
             &vault.amount,
         );
 
-        vault.status = VaultStatus::Failed;
-        env.storage().instance().set(&vault_key, &vault);
-
         env.events().publish(
             (Symbol::new(&env, "funds_redirected"), vault_id),
             vault.amount,
@@ -342,6 +348,9 @@ impl DisciplrVault {
     // -----------------------------------------------------------------------
 
     /// Cancel vault and return funds to creator.
+    ///
+    /// Applies checks-effects-interactions ordering: the vault is marked
+    /// `Cancelled` before the external token transfer is invoked.
     pub fn cancel_vault(env: Env, vault_id: u32, usdc_token: Address) -> Result<bool, Error> {
         let vault_key = DataKey::Vault(vault_id);
         let mut vault: ProductivityVault = env
@@ -356,16 +365,16 @@ impl DisciplrVault {
             return Err(Error::VaultNotActive);
         }
 
+        vault.status = VaultStatus::Cancelled;
+        env.storage().instance().set(&vault_key, &vault);
+
         let token_client = token::Client::new(&env, &usdc_token);
         token_client.transfer(
             &env.current_contract_address(),
             &vault.creator,
             &vault.amount,
         );
 
-        vault.status = VaultStatus::Cancelled;
-        env.storage().instance().set(&vault_key, &vault);
-
         env.events()
             .publish((Symbol::new(&env, "vault_cancelled"), vault_id), ());
         Ok(true)
@@ -982,6 +991,94 @@ mod tests {
         assert_eq!(vault.status, VaultStatus::Cancelled);
     }
 
+    #[test]
+    fn release_sets_terminal_status_before_any_second_terminal_path() {
+        let setup = TestSetup::new();
+        let client = setup.client();
+
+        setup.env.ledger().set_timestamp(setup.start_timestamp);
+        let vault_id = setup.create_default_vault();
+        setup.env.ledger().set_timestamp(setup.end_timestamp + 1);
+
+        let usdc = setup.usdc_client();
+        let success_before = usdc.balance(&setup.success_dest);
+        let failure_before = usdc.balance(&setup.failure_dest);
+
+        assert!(client.release_funds(&vault_id, &setup.usdc_token));
+        assert_eq!(
+            client.get_vault_state(&vault_id).unwrap().status,
+            VaultStatus::Completed
+        );
+
+        assert!(client
+            .try_redirect_funds(&vault_id, &setup.usdc_token)
+            .is_err());
+        assert_eq!(
+            usdc.balance(&setup.success_dest) - success_before,
+            setup.amount
+        );
+        assert_eq!(usdc.balance(&setup.failure_dest), failure_before);
+    }
+
+    #[test]
+    fn redirect_sets_terminal_status_before_any_second_terminal_path() {
+        let setup = TestSetup::new();
+        let client = setup.client();
+
+        setup.env.ledger().set_timestamp(setup.start_timestamp);
+        let vault_id = setup.create_default_vault();
+        setup.env.ledger().set_timestamp(setup.end_timestamp + 1);
+
+        let usdc = setup.usdc_client();
+        let success_before = usdc.balance(&setup.success_dest);
+        let failure_before = usdc.balance(&setup.failure_dest);
+
+        assert!(client.redirect_funds(&vault_id, &setup.usdc_token));
+        assert_eq!(
+            client.get_vault_state(&vault_id).unwrap().status,
+            VaultStatus::Failed
+        );
+
+        assert!(client
+            .try_release_funds(&vault_id, &setup.usdc_token)
+            .is_err());
+        assert_eq!(usdc.balance(&setup.success_dest), success_before);
+        assert_eq!(
+            usdc.balance(&setup.failure_dest) - failure_before,
+            setup.amount
+        );
+    }
+
+    #[test]
+    fn cancel_sets_terminal_status_before_any_second_terminal_path() {
+        let setup = TestSetup::new();
+        let client = setup.client();
+
+        setup.env.ledger().set_timestamp(setup.start_timestamp);
+        let vault_id = setup.create_default_vault();
+
+        let usdc = setup.usdc_client();
+        let creator_before = usdc.balance(&setup.creator);
+        let success_before = usdc.balance(&setup.success_dest);
+        let failure_before = usdc.balance(&setup.failure_dest);
+
+        assert!(client.cancel_vault(&vault_id, &setup.usdc_token));
+        assert_eq!(
+            client.get_vault_state(&vault_id).unwrap().status,
+            VaultStatus::Cancelled
+        );
+
+        assert!(client
+            .try_release_funds(&vault_id, &setup.usdc_token)
+            .is_err());
+        assert!(client
+            .try_redirect_funds(&vault_id, &setup.usdc_token)
+            .is_err());
+        assert_eq!(usdc.balance(&setup.creator) - creator_before, setup.amount);
+        assert_eq!(usdc.balance(&setup.success_dest), success_before);
+        assert_eq!(usdc.balance(&setup.failure_dest), failure_before);
+    }
+
     // -----------------------------------------------------------------------
     // More upstream tests migrated
     // -----------------------------------------------------------------------

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();