fix: resolve CEI violations and remove state mutation from invoice read flow

contracts/invoice/src/lib.rs

@@ -1588,23 +1588,21 @@ impl InvoiceContract {
     // ── Existing view / setter methods (unchanged) ────────────────────────────
 
     pub fn get_invoice(env: Env, id: u64) -> Invoice {
-        let inv = load_invoice(&env, id);
-        maybe_expire_pending_invoice(&env, inv)
+        load_invoice(&env, id)
     }
 
     pub fn get_multiple_invoices(env: Env, ids: Vec<u64>) -> Vec<Invoice> {
         bump_instance(&env);
         let mut invoices: Vec<Invoice> = Vec::new(&env);
         for i in 0..ids.len() {
             let inv = load_invoice(&env, ids.get(i).unwrap());
-            invoices.push_back(maybe_expire_pending_invoice(&env, inv));
+            invoices.push_back(inv);
         }
         invoices
     }
 
     pub fn get_metadata(env: Env, id: u64) -> InvoiceMetadata {
         let inv = load_invoice(&env, id);
-        let inv = maybe_expire_pending_invoice(&env, inv);
         let name = concat_prefix_u64(&env, b"Astera Invoice #", inv.id);
         let symbol = concat_prefix_u64(&env, b"INV-", inv.id);
         let image = env
@@ -2256,7 +2254,8 @@ mod test {
             &String::from_str(&env, "https://example.com/meta"),
         );
         env.ledger().with_mut(|l| l.timestamp += 2);
-        let inv = client.get_invoice(&id); // trigger expiration
+        assert!(client.check_expiration(&id));
+        let inv = client.get_invoice(&id);
         assert_eq!(inv.status, InvoiceStatus::Expired);
         let removed = client.cleanup_expired_storage(&admin, &soroban_sdk::vec![&env, id]);
         assert_eq!(removed, 1);
@@ -2751,7 +2750,32 @@ mod test {
     }
 
     #[test]
-    fn test_pending_invoice_expires_after_duration_on_read() {
+    fn test_get_invoice_does_not_expire_pending_invoice() {
+        let env = Env::default();
+        env.mock_all_auths();
+        env.ledger().with_mut(|l| l.timestamp = 100_000);
+        let contract_id = env.register(InvoiceContract, ());
+        let client = InvoiceContractClient::new(&env, &contract_id);
+        let admin = Address::generate(&env);
+        let pool = Address::generate(&env);
+        let sme = Address::generate(&env);
+        client.initialize(&admin, &pool, &i128::MAX, &10u64, &90u32);
+        let id = client.create_invoice(
+            &sme,
+            &String::from_str(&env, "D"),
+            &1_000i128,
+            &(env.ledger().timestamp() + 10_000),
+            &String::from_str(&env, "x"),
+            &String::from_str(&env, "h"),
+            &String::from_str(&env, "https://example.com/meta"),
+        );
+        env.ledger().with_mut(|l| l.timestamp += 11);
+        assert_eq!(client.get_invoice(&id).status, InvoiceStatus::Pending);
+        assert_eq!(client.get_metadata(&id).status, InvoiceStatus::Pending);
+    }
+
+    #[test]
+    fn test_check_expiration_expires_pending_invoice() {
         let env = Env::default();
         env.mock_all_auths();
         env.ledger().with_mut(|l| l.timestamp = 100_000);
@@ -2771,6 +2795,7 @@ mod test {
             &String::from_str(&env, "https://example.com/meta"),
         );
         env.ledger().with_mut(|l| l.timestamp += 11);
+        assert!(client.check_expiration(&id));
         assert_eq!(client.get_invoice(&id).status, InvoiceStatus::Expired);
     }
 
@@ -3416,7 +3441,7 @@ mod test {
                 _ => {
                     // Expired
                     env.ledger().with_mut(|l| l.timestamp += 86_400 + 1);
-                    client.get_invoice(&id); // triggers expiration
+                    client.check_expiration(&id);
                 }
             }
 
@@ -3563,7 +3588,8 @@ mod test {
             env.ledger()
                 .with_mut(|l| l.timestamp += expiration_secs + 1);
 
-            // Read triggers expiration
+            // Explicit expiration check transitions the invoice.
+            client.check_expiration(&id);
             assert_eq!(client.get_invoice(&id).status, InvoiceStatus::Expired);
         }
     }

contracts/pool/src/lib.rs

@@ -810,6 +810,7 @@ fn fund_invoice_request(
         .active_funded_invoices
         .checked_add(1)
         .ok_or(PoolError::AmountOverflow)?;
+    env.storage().instance().set(&DataKey::StorageStats, stats);
 
     // Transfer principal to SME LAST - interaction
     // NAV is unchanged because the funded invoice becomes an asset.
@@ -1196,20 +1197,6 @@ impl FundingPool {
             }
         }
 
-        // Record balance before transfer (CEI: check-effects-interactions)
-        let token_client = token::Client::new(&env, &token);
-        let balance_before = token_client.balance(&env.current_contract_address());
-
-        // Transfer tokens
-        token_client.transfer(&investor, &env.current_contract_address(), &amount);
-
-        // Verify exact amount received (handles fee-on-transfer tokens)
-        let balance_after = token_client.balance(&env.current_contract_address());
-        let received = balance_after.wrapping_sub(balance_before);
-        if received != amount {
-            return Err(PoolError::TransferMismatch);
-        }
-
         // Batch read: get both token totals and share token in one go
         let token_totals_key = DataKey::TokenTotals(token.clone());
         let share_token_key = DataKey::ShareToken(token.clone());
@@ -1233,13 +1220,13 @@ impl FundingPool {
                 deposit_count: 0,
             });
 
-        // Normalise received amount to USDC equivalent using stored exchange rate
+        // Normalise deposit amount to USDC equivalent using stored exchange rate
         let rate_bps: u32 = env
             .storage()
             .instance()
             .get(&DataKey::ExchangeRate(token.clone()))
             .unwrap_or(10_000u32);
-        let usdc_received = (received * rate_bps as i128) / 10_000i128;
+        let usdc_received = (amount * rate_bps as i128) / 10_000i128;
 
         // #233: enforce maximum single-investor concentration limit
         let config = get_config_cached(&env)?;
@@ -1301,6 +1288,18 @@ impl FundingPool {
             .persistent()
             .set(&investor_pos_key, &investor_position);
 
+        // Transfer deposited stablecoin LAST - interaction.
+        let token_client = token::Client::new(&env, &token);
+        let balance_before = token_client.balance(&env.current_contract_address());
+        token_client.transfer(&investor, &env.current_contract_address(), &amount);
+
+        // Verify exact amount received (handles fee-on-transfer tokens).
+        let balance_after = token_client.balance(&env.current_contract_address());
+        let received = balance_after.wrapping_sub(balance_before);
+        if received != amount {
+            return Err(PoolError::TransferMismatch);
+        }
+
         Self::non_reentrant_end(&env);
 
         env.events().publish(
@@ -3457,6 +3456,59 @@ mod test {
         }
     }
 
+    #[contract]
+    pub struct FailingMintShare;
+    #[contractimpl]
+    impl FailingMintShare {
+        pub fn total_supply(_env: Env) -> i128 {
+            0
+        }
+        pub fn balance(_env: Env, _id: Address) -> i128 {
+            0
+        }
+        pub fn mint(_env: Env, _to: Address, _amount: i128) {
+            panic!("mint failed");
+        }
+        pub fn burn(_env: Env, _from: Address, _amount: i128) {}
+    }
+
+    #[contract]
+    pub struct PanickingOutgoingToken;
+    #[contractimpl]
+    impl PanickingOutgoingToken {
+        pub fn decimals(_env: Env) -> u32 {
+            EXPECTED_DECIMALS
+        }
+        pub fn set_pool(env: Env, pool: Address) {
+            env.storage().instance().set(&symbol_short!("pool"), &pool);
+        }
+        pub fn set_balance(env: Env, to: Address, amount: i128) {
+            env.storage().persistent().set(&to, &amount);
+        }
+        pub fn balance(env: Env, id: Address) -> i128 {
+            env.storage().persistent().get(&id).unwrap_or(0)
+        }
+        pub fn transfer(env: Env, from: Address, to: Address, amount: i128) {
+            let pool: Address = env
+                .storage()
+                .instance()
+                .get(&symbol_short!("pool"))
+                .expect("pool not set");
+            if from == pool {
+                panic!("outgoing transfer failed");
+            }
+            let from_balance = Self::balance(env.clone(), from.clone());
+            let to_balance = Self::balance(env.clone(), to.clone());
+            if from_balance < amount {
+                panic!("insufficient balance");
+            }
+            env.storage()
+                .persistent()
+                .set(&from, &(from_balance - amount));
+            env.storage().persistent().set(&to, &(to_balance + amount));
+        }
+    }
+
     #[contract]
     pub struct DummyCreditScoreContract;
     #[contractimpl]
@@ -3768,6 +3820,36 @@ mod test {
         assert_eq!(result, Err(Ok(PoolError::TokenNotAccepted)));
     }
 
+    #[test]
+    fn test_deposit_mint_failure_does_not_take_stablecoin() {
+        let env = Env::default();
+        env.mock_all_auths();
+        env.ledger().with_mut(|l| l.timestamp = 100_000);
+
+        let contract_id = env.register(FundingPool, ());
+        let client = FundingPoolClient::new(&env, &contract_id);
+        let admin = Address::generate(&env);
+        let token_admin = Address::generate(&env);
+        let usdc_id = env
+            .register_stellar_asset_contract_v2(token_admin)
+            .address();
+        let invoice_contract = env.register(DummyInvoice, ());
+        DummyInvoiceClient::new(&env, &invoice_contract).set_pool(&contract_id);
+        let failing_share = env.register(FailingMintShare, ());
+        client.initialize(&admin, &usdc_id, &failing_share, &invoice_contract);
+        client.set_max_investor_concentration(&admin, &10_000u32);
+
+        let investor = Address::generate(&env);
+        mint(&env, &usdc_id, &investor, 1_000);
+        let token_client = token::Client::new(&env, &usdc_id);
+        let balance_before = token_client.balance(&investor);
+
+        let result = client.try_deposit(&investor, &usdc_id, &1_000i128);
+        assert!(result.is_err());
+        assert_eq!(token_client.balance(&investor), balance_before);
+        assert_eq!(client.get_token_totals(&usdc_id).pool_value, 0);
+    }
+
     #[test]
     fn test_withdraw_zero_shares_panics() {
         let env = Env::default();
@@ -3793,6 +3875,50 @@ mod test {
         assert_eq!(result, Err(Ok(PoolError::InvalidAmount)));
     }
 
+    #[test]
+    fn test_withdraw_transfer_failure_rolls_back_position() {
+        let env = Env::default();
+        env.mock_all_auths();
+        env.ledger().with_mut(|l| l.timestamp = 100_000);
+
+        let contract_id = env.register(FundingPool, ());
+        let client = FundingPoolClient::new(&env, &contract_id);
+        let admin = Address::generate(&env);
+        let token = env.register(PanickingOutgoingToken, ());
+        let token_client = PanickingOutgoingTokenClient::new(&env, &token);
+        token_client.set_pool(&contract_id);
+        let invoice_contract = env.register(DummyInvoice, ());
+        DummyInvoiceClient::new(&env, &invoice_contract).set_pool(&contract_id);
+        let share_token = env.register(DummyShare, ());
+        client.initialize(&admin, &token, &share_token, &invoice_contract);
+        client.set_max_investor_concentration(&admin, &10_000u32);
+
+        let investor = Address::generate(&env);
+        token_client.set_balance(&investor, &1_000);
+        client.deposit(&investor, &token, &1_000);
+
+        let shares_before: i128 = env.invoke_contract(
+            &share_token,
+            &Symbol::new(&env, "balance"),
+            soroban_sdk::vec![&env, investor.clone().into_val(&env)],
+        );
+        let totals_before = client.get_token_totals(&token);
+        let pool_balance_before = token_client.balance(&contract_id);
+
+        let result = client.try_withdraw(&investor, &token, &500i128);
+        assert!(result.is_err());
+        let shares_after: i128 = env.invoke_contract(
+            &share_token,
+            &Symbol::new(&env, "balance"),
+            soroban_sdk::vec![&env, investor.clone().into_val(&env)],
+        );
+        let totals_after = client.get_token_totals(&token);
+        assert_eq!(shares_after, shares_before);
+        assert_eq!(totals_after.pool_value, totals_before.pool_value);
+        assert_eq!(token_client.balance(&contract_id), pool_balance_before);
+        assert_eq!(token_client.balance(&investor), 0);
+    }
+
     #[test]
     fn test_fund_invoice_zero_principal_panics() {
         let env = Env::default();
@@ -3832,6 +3958,45 @@ mod test {
         assert_eq!(result, Err(Ok(PoolError::InsufficientLiquidity)));
     }
 
+    #[test]
+    fn test_fund_invoice_transfer_failure_rolls_back_storage() {
+        let env = Env::default();
+        env.mock_all_auths();
+        env.ledger().with_mut(|l| l.timestamp = 100_000);
+
+        let contract_id = env.register(FundingPool, ());
+        let client = FundingPoolClient::new(&env, &contract_id);
+        let admin = Address::generate(&env);
+        let token = env.register(PanickingOutgoingToken, ());
+        let token_client = PanickingOutgoingTokenClient::new(&env, &token);
+        token_client.set_pool(&contract_id);
+        let invoice_contract = env.register(DummyInvoice, ());
+        DummyInvoiceClient::new(&env, &invoice_contract).set_pool(&contract_id);
+        let share_token = env.register(DummyShare, ());
+        client.initialize(&admin, &token, &share_token, &invoice_contract);
+        client.set_max_investor_concentration(&admin, &10_000u32);
+
+        let investor = Address::generate(&env);
+        let sme = Address::generate(&env);
+        token_client.set_balance(&investor, &2_000);
+        client.deposit(&investor, &token, &2_000);
+
+        let result = client.try_fund_invoice(
+            &admin,
+            &1u64,
+            &1_000i128,
+            &sme,
+            &(env.ledger().timestamp() + 86_400),
+            &token,
+        );
+        assert!(result.is_err());
+        assert!(client.get_funded_invoice(&1u64).is_none());
+        assert_eq!(client.get_token_totals(&token).total_deployed, 0);
+        assert_eq!(client.get_storage_stats().active_funded_invoices, 0);
+        assert_eq!(token_client.balance(&contract_id), 2_000);
+        assert_eq!(token_client.balance(&sme), 0);
+    }
+
     #[test]
     fn test_fund_invoice_prioritizes_liquidity_before_token_validation() {
         let env = Env::default();