fix: resolve issues #138, #137, #136, #25

.gitignore

@@ -3,8 +3,23 @@ target/
 *.wasm
 .env
 .env.*
+
+# Test snapshots
 test_snapshots/
 **/test_snapshots/
+
+# Generated/tooling files
 issues.md
 generate_issues.py
 waves-issues.md
+
+# IDE and OS files
+.DS_Store
+.idea/
+.vscode/
+*.swp
+*.swo
+
+# Rust artifacts
+Cargo.lock.bak
+**/*.orig

contracts/invoice_nft/src/lib.rs

@@ -32,7 +32,6 @@ use soroban_sdk::{contract, contractimpl, contracttype, Address, Bytes, Env, Str
 // - NextId: Stores the next invoice ID to mint (instance)
 // - Admin: Stores admin address (instance)
 // - AccessControl: Stores access control contract address (instance)
-// - InvoiceCount: Stores total count of invoices minted (instance)
 // - MigrationVersion: Tracks current schema version for upgrade safety (instance)
 
 /// Storage key variants for the invoice NFT contract.
@@ -42,6 +41,7 @@ use soroban_sdk::{contract, contractimpl, contracttype, Address, Bytes, Env, Str
 /// - `Admin` — Stores the contract admin address (instance)
 /// - `AccessControl` — Stores the access control contract address (instance)
 /// - `InvoiceCount` — Stores total invoice count for metrics (instance)
+/// - `MigrationVersion` — Tracks current schema version for upgrade safety (instance)
 #[contracttype]
 pub enum DataKey {
     /// Versioned invoice storage: Invoice(id) stores Invoice struct
@@ -52,6 +52,8 @@ pub enum DataKey {
     Admin,
     /// Instance key: access control contract address for pause checks
     AccessControl,
+    /// Instance key: current schema migration version (starts at 1)
+    MigrationVersion,
 }
 
 // ── Contract ─────────────────────────────────────────────────────────────────

contracts/risk_registry/src/lib.rs

@@ -41,6 +41,7 @@ impl RiskRegistryContract {
         env.storage()
             .instance()
             .set(&DataKey::InvoiceNft, &invoice_nft);
+        events::registry_initialized(&env, &admin, &invoice_nft);
         Ok(())
     }
 

contracts/shared/src/events.rs

@@ -261,4 +261,14 @@ pub fn debtor_score_set(env: &Env, verifier: &Address, debtor_hash: &Bytes, scor
     );
 }
 
+/// Emitted when the risk registry contract is initialized.
+/// Payload: (admin, invoice_nft)
+pub fn registry_initialized(env: &Env, admin: &Address, invoice_nft: &Address) {
+    emit(
+        env,
+        symbol_short!("REG_INI"),
+        (admin.clone(), invoice_nft.clone()),
+    );
+}
+
 // AUDIT FIX: Removed duplicate sme_invoice_counted — use sme_invoice_count_incremented instead.

contracts/shared/src/reentrancy.rs

@@ -11,35 +11,6 @@ pub enum GuardKey {
     Lock,
 }
 
-// ── RAII guard ────────────────────────────────────────────────────────────────
-
-/// RAII reentrancy guard. Acquires the lock on construction and releases it
-/// when dropped, ensuring the lock is always released even on early returns.
-///
-/// # Usage
-/// ```rust,ignore
-/// let _guard = ReentrancyGuard::new(&env)?;
-/// // ... protected logic ...
-/// // lock is released automatically when _guard goes out of scope
-/// ```
-pub struct ReentrancyGuard<'a> {
-    env: &'a Env,
-}
-
-impl<'a> ReentrancyGuard<'a> {
-    /// Acquire the reentrancy lock. Returns `KoraError::Reentrancy` if already held.
-    pub fn new(env: &'a Env) -> Result<Self, KoraError> {
-        acquire_guard(env)?;
-        Ok(Self { env })
-    }
-}
-
-impl<'a> Drop for ReentrancyGuard<'a> {
-    fn drop(&mut self) {
-        release_guard(self.env);
-    }
-}
-
 // ── Low-level helpers ─────────────────────────────────────────────────────────
 
 /// Acquire the reentrancy lock.
@@ -165,12 +136,10 @@ mod tests {
 
         fn protected(env: &Env) -> Result<(), KoraError> {
             let _guard = ReentrancyGuard::new(env)?;
-            // Simulate early return via ?
             Err(KoraError::InvalidAmount)
         }
 
         let _ = protected(&env);
-        // Lock must be released even after early return
         assert!(!is_locked(&env));
     }
 
@@ -218,7 +187,6 @@ mod tests {
     fn test_raii_nested_guard_fails() {
         let env = Env::default();
         let _guard = ReentrancyGuard::new(&env).unwrap();
-        // Second guard must fail while first is held
         let result = ReentrancyGuard::new(&env);
         assert_eq!(result.unwrap_err(), KoraError::Reentrancy);
         // First guard drops here, lock released

contracts/treasury/src/lib.rs

@@ -3,6 +3,7 @@
 use kora_shared::{
     errors::KoraError,
     events,
+    reentrancy::ReentrancyGuard,
     validation::require_valid_fee_bps,
 };
 use soroban_sdk::{contract, contractimpl, contracttype, token, Address, Env};
@@ -21,8 +22,8 @@ pub enum DataKey {
     FeeBps,
     /// Accumulated fees per token (informational).
     Collected(Address),
-    /// Reentrancy guard for withdrawal functions.
-    WithdrawalLock,
+    /// Whitelisted token flag.
+    WhitelistedToken(Address),
 }
 
 // ── Contract ──────────────────────────────────────────────────────────────────
@@ -34,7 +35,6 @@ pub struct TreasuryContract;
 impl TreasuryContract {
     /// One-time initialization. Sets admin and protocol fee.
     pub fn initialize(env: Env, admin: Address, fee_bps: u32) -> Result<(), KoraError> {
-        // Use persistent storage consistently — same store read by require_admin
         if env.storage().persistent().has(&DataKey::Admin) {
             return Err(KoraError::AlreadyInitialized);
         }
@@ -96,7 +96,9 @@ impl TreasuryContract {
     /// No auth required — the token transfer itself is the proof of payment.
     /// The amount is validated to be > 0 to prevent no-op accounting entries.
     pub fn collect_fee(env: Env, token: Address, amount: i128) -> Result<(), KoraError> {
-        require_non_zero_amount(amount)?;
+        if amount <= 0 {
+            return Err(KoraError::InvalidAmount);
+        }
         Self::require_whitelisted_token(&env, &token)?;
 
         let key = DataKey::Collected(token.clone());
@@ -113,7 +115,7 @@ impl TreasuryContract {
     }
 
     /// Withdraw accumulated fees to a recipient. Admin only.
-    /// Protected against reentrancy via an instance-storage lock key.
+    /// Protected against reentrancy via RAII ReentrancyGuard.
     pub fn withdraw(
         env: Env,
         admin: Address,
@@ -124,34 +126,28 @@ impl TreasuryContract {
         // ── Checks ────────────────────────────────────────────────────────────
         admin.require_auth();
         Self::require_admin(&env, &admin)?;
-        require_non_zero_amount(amount)?;
-        Self::require_whitelisted_token(&env, &token)?;
-
-        // Validate amount before acquiring the lock to avoid unnecessary state mutation
         if amount <= 0 {
             return Err(KoraError::InvalidAmount);
         }
+        Self::require_whitelisted_token(&env, &token)?;
 
-        Self::acquire_lock(&env)?;
+        // Acquire reentrancy guard — released automatically when _guard drops
+        let _guard = ReentrancyGuard::new(&env)?;
 
         let token_client = token::Client::new(&env, &token);
         let balance = token_client.balance(&env.current_contract_address());
 
         if balance < amount {
-            // Release lock before returning error — must not leave lock stuck
-            Self::release_lock(&env);
             return Err(KoraError::InsufficientPoolBalance);
         }
 
         // ── Effects ───────────────────────────────────────────────────────────
-        // Deduct from informational accounting if tracked
         let collected_key = DataKey::Collected(token.clone());
         if let Some(collected) = env
             .storage()
             .persistent()
             .get::<_, i128>(&collected_key)
         {
-            // Saturating sub: accounting is informational, don't revert on mismatch
             let new_collected = collected.saturating_sub(amount);
             env.storage()
                 .persistent()
@@ -167,7 +163,7 @@ impl TreasuryContract {
     }
 
     /// Emergency drain — withdraw entire token balance. Admin only.
-    /// Protected against reentrancy via an instance-storage lock key.
+    /// Protected against reentrancy via RAII ReentrancyGuard.
     pub fn emergency_withdraw(
         env: Env,
         admin: Address,
@@ -185,21 +181,12 @@ impl TreasuryContract {
         let token_client = token::Client::new(&env, &token);
         let balance = token_client.balance(&env.current_contract_address());
 
+        // ── Interactions ──────────────────────────────────────────────────────
         if balance > 0 {
             token_client.transfer(&env.current_contract_address(), &recipient, &balance);
-        }
-
-        // Always release lock regardless of whether a transfer occurred
-        Self::release_lock(&env);
-
-        if balance > 0 {
             events::emergency_withdrawn(&env, &admin, &token, balance);
         }
 
-        // ── Interactions ──────────────────────────────────────────────────────
-        token_client.transfer(&env.current_contract_address(), &recipient, &balance);
-
-        events::emergency_withdrawn(&env, &admin, &token, balance);
         Ok(())
     }
 
@@ -249,8 +236,12 @@ impl TreasuryContract {
         Ok(())
     }
 
-    fn release_lock(env: &Env) {
-        env.storage().instance().set(&DataKey::WithdrawalLock, &false);
+    fn bump_persistent(env: &Env, key: &DataKey) {
+        env.storage().persistent().extend_ttl(
+            key,
+            PERSISTENT_LIFETIME_THRESHOLD,
+            PERSISTENT_BUMP_AMOUNT,
+        );
     }
 }
 
@@ -400,7 +391,6 @@ mod tests {
         env.mock_all_auths();
         let contract_id = env.register_contract(None, TreasuryContract);
         let client = TreasuryContractClient::new(&env, &contract_id);
-        // Before initialization, falls back to default 50
         assert_eq!(client.get_fee_bps(), 50);
     }
 
@@ -409,7 +399,7 @@ mod tests {
         let (env, admin, client) = setup();
         let token = Address::generate(&env);
         let recipient = Address::generate(&env);
-        // Fails due to insufficient balance — lock must be released
+        // Fails due to token not whitelisted — lock must be released
         let _ = client.try_withdraw(&admin, &token, &recipient, &1_000i128);
         // Subsequent admin operation must succeed (lock not stuck)
         assert!(client.try_set_fee_bps(&admin, &100u32).is_ok());