diff --git a/.gitignore b/.gitignore index df572d2..455342d 100644 --- a/.gitignore +++ b/.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 diff --git a/contracts/invoice_nft/src/lib.rs b/contracts/invoice_nft/src/lib.rs index 81c48a4..6054a5a 100644 --- a/contracts/invoice_nft/src/lib.rs +++ b/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 ───────────────────────────────────────────────────────────────── diff --git a/contracts/risk_registry/src/lib.rs b/contracts/risk_registry/src/lib.rs index 8938080..0adfbe5 100644 --- a/contracts/risk_registry/src/lib.rs +++ b/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(()) } diff --git a/contracts/shared/src/events.rs b/contracts/shared/src/events.rs index e99c8a1..b541dd6 100644 --- a/contracts/shared/src/events.rs +++ b/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. diff --git a/contracts/shared/src/reentrancy.rs b/contracts/shared/src/reentrancy.rs index 58d064e..be9d1db 100644 --- a/contracts/shared/src/reentrancy.rs +++ b/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 { - 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 diff --git a/contracts/treasury/src/lib.rs b/contracts/treasury/src/lib.rs index a5d3dbe..70ada75 100644 --- a/contracts/treasury/src/lib.rs +++ b/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());