fix: resolve issues #120 #121 #122 #123

.gitignore

@@ -8,3 +8,8 @@ test_snapshots/
 issues.md
 generate_issues.py
 waves-issues.md
+*.log
+*.tmp
+*.bak
+.DS_Store
+Thumbs.db

contracts/risk_registry/src/lib.rs

@@ -34,21 +34,24 @@ pub struct RiskRegistryContract;
 impl RiskRegistryContract {
     /// One-time initialization. Sets admin and the authorized invoice_nft address.
     pub fn initialize(env: Env, admin: Address, invoice_nft: Address) -> Result<(), KoraError> {
-        if env.storage().instance().has(&DataKey::Admin) {
+        if env.storage().persistent().has(&DataKey::Admin) {
             return Err(KoraError::AlreadyInitialized);
         }
-        env.storage().instance().set(&DataKey::Admin, &admin);
+        env.storage().persistent().set(&DataKey::Admin, &admin);
+        Self::bump_persistent(&env, &DataKey::Admin);
         env.storage()
-            .instance()
+            .persistent()
             .set(&DataKey::InvoiceNft, &invoice_nft);
+        Self::bump_persistent(&env, &DataKey::InvoiceNft);
         Ok(())
     }
 
     /// Transfer admin role to a new address. Current admin only.
     pub fn transfer_admin(env: Env, admin: Address, new_admin: Address) -> Result<(), KoraError> {
         admin.require_auth();
         Self::require_admin(&env, &admin)?;
-        env.storage().instance().set(&DataKey::Admin, &new_admin);
+        env.storage().persistent().set(&DataKey::Admin, &new_admin);
+        Self::bump_persistent(&env, &DataKey::Admin);
         events::admin_transferred(&env, &new_admin);
         Ok(())
     }
@@ -271,7 +274,7 @@ impl RiskRegistryContract {
 
     pub fn get_admin(env: Env) -> Result<Address, KoraError> {
         env.storage()
-            .instance()
+            .persistent()
             .get(&DataKey::Admin)
             .ok_or(KoraError::NotInitialized)
     }
@@ -281,7 +284,7 @@ impl RiskRegistryContract {
     fn require_admin(env: &Env, caller: &Address) -> Result<(), KoraError> {
         let admin: Address = env
             .storage()
-            .instance()
+            .persistent()
             .get(&DataKey::Admin)
             .ok_or(KoraError::NotInitialized)?;
         if &admin != caller {
@@ -305,7 +308,7 @@ impl RiskRegistryContract {
     fn require_invoice_nft(env: &Env, caller: &Address) -> Result<(), KoraError> {
         let invoice_nft: Address = env
             .storage()
-            .instance()
+            .persistent()
             .get(&DataKey::InvoiceNft)
             .ok_or(KoraError::NotInitialized)?;
         if &invoice_nft != caller {

contracts/shared/src/lib.rs

@@ -1,21 +1,15 @@
 #![no_std]
 
-//! # Kora Shared Library — Audit Findings
+//! # Kora Shared Library
 //!
-//! ## Summary of Findings and Fixes
+//! Common types, errors, events, and validation utilities for the Kora Protocol.
 //!
-//! ### 1. Missing Doc Comments on Validation Helpers (validation.rs)
-//! - **Issue:** All public validation functions lacked doc comments
-//! - **Fix:** AUDIT FIX: Added comprehensive /// doc comments to every public function
-//! - **Severity:** Medium — Documentation completeness
-//!
-//! ### 2. Incorrect Error Type for Empty Bytes (validation.rs:39-43)
-//! - **Issue:** `require_non_empty_bytes()` returned `EmptyString` error (semantically wrong for bytes)
-//! - **Fix:** AUDIT FIX: Changed to return dedicated `EmptyBytes` error for semantic clarity
-//! - **Severity:** Low — Error categorization/semantics
-//!
-//! All arithmetic operations use checked methods. All type definitions are well-documented.
-//! Error enum is comprehensive and specific.
+//! ## Modules
+//! - `types`      — Core on-chain data structures (Invoice, Listing, Pool, etc.)
+//! - `errors`     — Protocol-wide error enum (`KoraError`)
+//! - `events`     — Standardized event emission helpers
+//! - `validation` — Input validation and safe arithmetic helpers
+//! - `reentrancy` — RAII reentrancy guard
 
 pub mod errors;
 pub mod events;

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.

contracts/treasury/src/lib.rs

@@ -3,7 +3,8 @@
 use kora_shared::{
     errors::KoraError,
     events,
-    validation::require_valid_fee_bps,
+    reentrancy::ReentrancyGuard,
+    validation::{require_non_zero_amount, 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);
         }
@@ -113,7 +113,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 guard.
     pub fn withdraw(
         env: Env,
         admin: Address,
@@ -127,31 +127,22 @@ impl TreasuryContract {
         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::acquire_lock(&env)?;
+        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 +158,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 guard.
     pub fn emergency_withdraw(
         env: Env,
         admin: Address,
@@ -179,27 +170,17 @@ impl TreasuryContract {
         Self::require_admin(&env, &admin)?;
         Self::require_whitelisted_token(&env, &token)?;
 
-        // 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());
 
+        // ── 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 +230,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,
+        );
     }
 }
 
@@ -409,7 +394,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());