diff --git a/contracts/predictify-hybrid/src/errors.rs b/contracts/predictify-hybrid/src/errors.rs index 3ef3713d..9db3f9c2 100644 --- a/contracts/predictify-hybrid/src/errors.rs +++ b/contracts/predictify-hybrid/src/errors.rs @@ -186,6 +186,12 @@ pub enum Error { CircuitBreakerNotOpen = 502, /// Circuit breaker is open (operations blocked) CircuitBreakerOpen = 503, + + // ===== REENTRANCY AND EXTERNAL CALL ERRORS ===== + /// Reentrancy guard is active (operation blocked to prevent reentry) + ReentrancyGuardActive = 600, + /// External call failed (e.g., token transfer or oracle invocation) + ExternalCallFailed = 601, } // ===== ERROR CATEGORIZATION AND RECOVERY SYSTEM ===== @@ -677,6 +683,8 @@ impl Error { Error::CircuitBreakerAlreadyOpen => "Circuit breaker is already open (paused)", Error::CircuitBreakerNotOpen => "Circuit breaker is not open (cannot recover)", Error::CircuitBreakerOpen => "Circuit breaker is open (operations blocked)", + Error::ReentrancyGuardActive => "Reentrancy guard active (operation blocked)", + Error::ExternalCallFailed => "External call failed", } } @@ -792,6 +800,8 @@ impl Error { Error::CircuitBreakerAlreadyOpen => "CIRCUIT_BREAKER_ALREADY_OPEN", Error::CircuitBreakerNotOpen => "CIRCUIT_BREAKER_NOT_OPEN", Error::CircuitBreakerOpen => "CIRCUIT_BREAKER_OPEN", + Error::ReentrancyGuardActive => "REENTRANCY_GUARD_ACTIVE", + Error::ExternalCallFailed => "EXTERNAL_CALL_FAILED", } } } diff --git a/contracts/predictify-hybrid/src/fees.rs b/contracts/predictify-hybrid/src/fees.rs index f966e166..2e71873a 100644 --- a/contracts/predictify-hybrid/src/fees.rs +++ b/contracts/predictify-hybrid/src/fees.rs @@ -3,9 +3,11 @@ use soroban_sdk::{contracttype, symbol_short, vec, Address, Env, Map, String, Sy use crate::errors::Error; use crate::markets::{MarketStateManager, MarketUtils}; use crate::types::Market; +use crate::reentrancy_guard::ReentrancyGuard; use crate::config::{ConfigManager, ConfigValidator, FeeConfig}; use crate::events::EventEmitter; + /// Fee management system for Predictify Hybrid contract /// /// This module provides a comprehensive fee management system with: @@ -713,10 +715,12 @@ impl FeeManager { FeeValidator::validate_creation_fee_env(env, creation_fee)?; // Get token client + ReentrancyGuard::before_external_call(env)?; let token_client = MarketUtils::get_token_client(env)?; - // Transfer creation fee from admin to contract - token_client.transfer(admin, &env.current_contract_address(), &creation_fee); + token_client.transfer(admin, &env.current_contract_address(), &MARKET_CREATION_FEE); + ReentrancyGuard::after_external_call(env); + // Record creation fee FeeTracker::record_creation_fee(env, admin, creation_fee)?; @@ -1337,8 +1341,10 @@ pub struct FeeUtils; impl FeeUtils { /// Transfer fees to admin pub fn transfer_fees_to_admin(env: &Env, admin: &Address, amount: i128) -> Result<(), Error> { + ReentrancyGuard::before_external_call(env)?; let token_client = MarketUtils::get_token_client(env)?; token_client.transfer(&env.current_contract_address(), admin, &amount); + ReentrancyGuard::after_external_call(env); Ok(()) } diff --git a/contracts/predictify-hybrid/src/lib.rs b/contracts/predictify-hybrid/src/lib.rs index 00897492..d01f326a 100644 --- a/contracts/predictify-hybrid/src/lib.rs +++ b/contracts/predictify-hybrid/src/lib.rs @@ -19,6 +19,7 @@ mod fees; mod markets; mod oracles; mod resolution; +mod reentrancy_guard; mod storage; mod types; mod utils; @@ -44,8 +45,10 @@ use alloc::format; use soroban_sdk::{ contract, contractimpl, panic_with_error, Address, Env, Map, String, Symbol, Vec, }; +use crate::reentrancy_guard::ReentrancyGuard; use crate::config::{ConfigManager, ContractConfig, ConfigChanges, MarketLimits, ConfigUpdateRecord}; + #[contract] pub struct PredictifyHybrid; @@ -282,6 +285,9 @@ impl PredictifyHybrid { /// - Current time must be before market end time /// - Market must not be cancelled or resolved pub fn vote(env: Env, user: Address, market_id: Symbol, outcome: String, stake: i128) { + if let Err(e) = ReentrancyGuard::check_reentrancy_state(&env) { + panic_with_error!(env, e); + } user.require_auth(); let mut market: Market = env @@ -372,6 +378,9 @@ impl PredictifyHybrid { /// - User must have voted for the winning outcome /// - User must not have previously claimed winnings pub fn claim_winnings(env: Env, user: Address, market_id: Symbol) { + if let Err(e) = ReentrancyGuard::check_reentrancy_state(&env) { + panic_with_error!(env, e); + } user.require_auth(); let mut market: Market = env @@ -555,6 +564,9 @@ impl PredictifyHybrid { market_id: Symbol, winning_outcome: String, ) { + if let Err(e) = ReentrancyGuard::check_reentrancy_state(&env) { + panic_with_error!(env, e); + } admin.require_auth(); // Verify admin @@ -684,14 +696,15 @@ impl PredictifyHybrid { return Err(Error::MarketClosed); } - // Get oracle result using the resolution module - let oracle_resolution = resolution::OracleResolutionManager::fetch_oracle_result( - &env, - &market_id, - &oracle_contract, - )?; + // Guard external oracle invocation + ReentrancyGuard::check_reentrancy_state(&env)?; + ReentrancyGuard::before_external_call(&env)?; + let result = resolution::OracleResolutionManager::fetch_oracle_result( + &env, &market_id, &oracle_contract, + ); + ReentrancyGuard::after_external_call(&env); - Ok(oracle_resolution.oracle_result) + result.map(|oracle_resolution| oracle_resolution.oracle_result) } /// Resolves a market automatically using oracle data and community consensus. @@ -764,6 +777,9 @@ impl PredictifyHybrid { /// - Users can claim winnings /// - Market statistics are finalized pub fn resolve_market(env: Env, market_id: Symbol) -> Result<(), Error> { + if let Err(e) = ReentrancyGuard::check_reentrancy_state(&env) { + return Err(e); + } // Use the resolution module to resolve the market let _resolution = resolution::MarketResolutionManager::resolve_market(&env, &market_id)?; Ok(()) @@ -956,6 +972,9 @@ impl PredictifyHybrid { stake: i128, reason: Option, ) -> Result<(), Error> { + if let Err(e) = ReentrancyGuard::check_reentrancy_state(&env) { + return Err(e); + } user.require_auth(); disputes::DisputeManager::process_dispute(&env, user, market_id, stake, reason) } @@ -970,6 +989,9 @@ impl PredictifyHybrid { stake: i128, reason: Option, ) -> Result<(), Error> { + if let Err(e) = ReentrancyGuard::check_reentrancy_state(&env) { + return Err(e); + } user.require_auth(); disputes::DisputeManager::vote_on_dispute( &env, user, market_id, dispute_id, vote, stake, reason, @@ -982,6 +1004,9 @@ impl PredictifyHybrid { admin: Address, market_id: Symbol, ) -> Result { + if let Err(e) = ReentrancyGuard::check_reentrancy_state(&env) { + return Err(e); + } admin.require_auth(); // Verify admin @@ -1002,6 +1027,9 @@ impl PredictifyHybrid { /// Collect fees from a market (admin only) pub fn collect_fees(env: Env, admin: Address, market_id: Symbol) -> Result { + if let Err(e) = ReentrancyGuard::check_reentrancy_state(&env) { + return Err(e); + } admin.require_auth(); // Verify admin @@ -1029,6 +1057,9 @@ impl PredictifyHybrid { reason: String, fee_amount: i128, ) -> Result<(), Error> { + if let Err(e) = ReentrancyGuard::check_reentrancy_state(&env) { + return Err(e); + } admin.require_auth(); // Verify admin diff --git a/contracts/predictify-hybrid/src/markets.rs b/contracts/predictify-hybrid/src/markets.rs index 6190b67d..22355c40 100644 --- a/contracts/predictify-hybrid/src/markets.rs +++ b/contracts/predictify-hybrid/src/markets.rs @@ -1321,17 +1321,10 @@ impl MarketAnalytics { /// - Total amount staked across all participants /// - Total dispute stakes (if any) /// - Distribution of votes across different outcomes - /// - /// # Example - /// - /// ```rust - /// use soroban_sdk::{Env, Symbol}; - /// use crate::markets::{MarketAnalytics, MarketStateManager}; - /// - /// let env = Env::default(); /// let market_id = Symbol::new(&env, "active_market"); /// let market = MarketStateManager::get_market(&env, &market_id)?; /// + /// /// let stats = MarketAnalytics::get_market_stats(&market); /// /// println!("Total votes: {}", stats.total_votes); @@ -2358,6 +2351,8 @@ impl MarketTestHelpers { // Transfer stake let token_client = MarketUtils::get_token_client(env)?; token_client.transfer(&user, &env.current_contract_address(), &stake); + // Transfer stake via centralized, guarded utility + // VotingUtils::transfer_stake(env, &user, stake)?; // Add vote MarketStateManager::add_vote(&mut market, user, outcome, stake, None); diff --git a/contracts/predictify-hybrid/src/oracles.rs b/contracts/predictify-hybrid/src/oracles.rs index 41db27fa..dc44b213 100644 --- a/contracts/predictify-hybrid/src/oracles.rs +++ b/contracts/predictify-hybrid/src/oracles.rs @@ -4,6 +4,7 @@ use soroban_sdk::{contracttype, symbol_short, vec, Address, Env, IntoVal, String use crate::errors::Error; use crate::types::*; +use crate::reentrancy_guard::ReentrancyGuard; /// Oracle management system for Predictify Hybrid contract /// @@ -604,8 +605,14 @@ impl<'a> ReflectorOracleClient<'a> { /// Get the latest price for an asset pub fn lastprice(&self, asset: ReflectorAsset) -> Option { let args = vec![self.env, asset.into_val(self.env)]; - self.env - .invoke_contract(&self.contract_id, &symbol_short!("lastprice"), args) + if ReentrancyGuard::before_external_call(self.env).is_err() { + return None; + } + let res = self + .env + .invoke_contract(&self.contract_id, &symbol_short!("lastprice"), args); + ReentrancyGuard::after_external_call(self.env); + res } /// Get price for an asset at a specific timestamp @@ -615,8 +622,14 @@ impl<'a> ReflectorOracleClient<'a> { asset.into_val(self.env), timestamp.into_val(self.env), ]; - self.env - .invoke_contract(&self.contract_id, &symbol_short!("price"), args) + if ReentrancyGuard::before_external_call(self.env).is_err() { + return None; + } + let res = self + .env + .invoke_contract(&self.contract_id, &symbol_short!("price"), args); + ReentrancyGuard::after_external_call(self.env); + res } /// Get TWAP (Time-Weighted Average Price) for an asset @@ -626,8 +639,14 @@ impl<'a> ReflectorOracleClient<'a> { asset.into_val(self.env), records.into_val(self.env), ]; - self.env - .invoke_contract(&self.contract_id, &symbol_short!("twap"), args) + if ReentrancyGuard::before_external_call(self.env).is_err() { + return None; + } + let res = self + .env + .invoke_contract(&self.contract_id, &symbol_short!("twap"), args); + ReentrancyGuard::after_external_call(self.env); + res } /// Check if the Reflector oracle is healthy diff --git a/contracts/predictify-hybrid/src/reentrancy_guard.rs b/contracts/predictify-hybrid/src/reentrancy_guard.rs new file mode 100644 index 00000000..f83a1911 --- /dev/null +++ b/contracts/predictify-hybrid/src/reentrancy_guard.rs @@ -0,0 +1,119 @@ +use soroban_sdk::{symbol_short, Env}; + +use crate::errors::Error; + +/// Global cross-function reentrancy guard. +/// +/// This guard prevents reentry across all public entrypoints while an external +/// call (e.g., token transfer, oracle invocation) is in-flight. The lock is +/// stored in persistent storage using a single boolean flag. +pub struct ReentrancyGuard; + +impl ReentrancyGuard { + fn key() -> soroban_sdk::Symbol { + // Persistent storage key for the reentrancy lock + symbol_short!("reent_lk") + } + + /// Returns true if the reentrancy lock is currently active. + pub fn is_locked(env: &Env) -> bool { + env.storage() + .persistent() + .get::(&Self::key()) + .unwrap_or(false) + } + + /// Checks current reentrancy state. Returns an error if locked. + pub fn check_reentrancy_state(env: &Env) -> Result<(), Error> { + if Self::is_locked(env) { + return Err(Error::ReentrancyGuardActive); + } + Ok(()) + } + + /// Sets the reentrancy lock before making an external call. + /// + /// If the lock is already set, returns `Error::ReentrancyGuardActive`. + pub fn before_external_call(env: &Env) -> Result<(), Error> { + if Self::is_locked(env) { + return Err(Error::ReentrancyGuardActive); + } + env.storage().persistent().set(&Self::key(), &true); + Ok(()) + } + + /// Clears the reentrancy lock after the external call completes. + pub fn after_external_call(env: &Env) { + env.storage().persistent().set(&Self::key(), &false); + } + + /// Validates that an external call succeeded. + /// + /// This helper standardizes call-site validation and returns a specific + /// `ExternalCallFailed` error when `ok` is false. + pub fn validate_external_call_success(_env: &Env, ok: bool) -> Result<(), Error> { + if ok { + Ok(()) + } else { + Err(Error::ExternalCallFailed) + } + } + /// + /// This is a light abstraction to execute caller-provided restoration logic + /// for any provisional state touched before an external call. In most cases, + /// prefer ordering state writes after the external call succeeds, but this is + /// provided for scenarios where temporary state must be set. + pub fn restore_state_on_failure(_env: &Env, restore_fn: F) { + restore_fn(); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use soroban_sdk::{Address, Env}; + use soroban_sdk::testutils::{Address as _, }; + use crate::PredictifyHybrid; + + fn with_contract(env: &Env, f: F) { + let addr = env.register_contract(None, PredictifyHybrid); + env.as_contract(&addr, || { + f(); + }); + } + + #[test] + fn lock_cycle_sets_and_clears_flag() { + let env = Env::default(); + with_contract(&env, || { + // Initially unlocked + assert!(!ReentrancyGuard::is_locked(&env)); + + // Lock + assert!(ReentrancyGuard::before_external_call(&env).is_ok()); + assert!(ReentrancyGuard::is_locked(&env)); + + // Unlock + ReentrancyGuard::after_external_call(&env); + assert!(!ReentrancyGuard::is_locked(&env)); + }); + } + + #[test] + fn check_reentrancy_state_blocks_when_locked() { + let env = Env::default(); + with_contract(&env, || { + // Unlocked state allows operations + assert!(ReentrancyGuard::check_reentrancy_state(&env).is_ok()); + + // Lock and verify it blocks + assert!(ReentrancyGuard::before_external_call(&env).is_ok()); + let err = ReentrancyGuard::check_reentrancy_state(&env).unwrap_err(); + assert_eq!(err, Error::ReentrancyGuardActive); + + // Unlock and verify allowed again + ReentrancyGuard::after_external_call(&env); + assert!(ReentrancyGuard::check_reentrancy_state(&env).is_ok()); + }); + } +} diff --git a/contracts/predictify-hybrid/src/resolution.rs b/contracts/predictify-hybrid/src/resolution.rs index 5fccc892..34c636a0 100644 --- a/contracts/predictify-hybrid/src/resolution.rs +++ b/contracts/predictify-hybrid/src/resolution.rs @@ -6,6 +6,7 @@ use crate::markets::{CommunityConsensus, MarketAnalytics, MarketStateManager, Ma use crate::oracles::{OracleFactory, OracleUtils}; use crate::types::*; +use crate::reentrancy_guard::ReentrancyGuard; /// Resolution management system for Predictify Hybrid contract /// @@ -959,7 +960,11 @@ impl OracleResolutionManager { oracle_contract.clone(), )?; - let price = oracle.get_price(env, &market.oracle_config.feed_id)?; + // Perform external oracle call under reentrancy guard + ReentrancyGuard::before_external_call(env)?; + let price_result = oracle.get_price(env, &market.oracle_config.feed_id); + ReentrancyGuard::after_external_call(env); + let price = price_result?; // Determine the outcome based on the price and threshold using OracleUtils let outcome = OracleUtils::determine_outcome( diff --git a/contracts/predictify-hybrid/src/voting.rs b/contracts/predictify-hybrid/src/voting.rs index 7368a52a..1ce728a8 100644 --- a/contracts/predictify-hybrid/src/voting.rs +++ b/contracts/predictify-hybrid/src/voting.rs @@ -5,6 +5,7 @@ use crate::{ markets::{MarketAnalytics, MarketStateManager, MarketUtils, MarketValidator}, types::Market, }; +use crate::reentrancy_guard::ReentrancyGuard; use soroban_sdk::{contracttype, symbol_short, vec, Address, Env, Map, String, Symbol, Vec}; @@ -1100,15 +1101,20 @@ pub struct VotingUtils; impl VotingUtils { /// Transfer stake from user to contract pub fn transfer_stake(env: &Env, user: &Address, stake: i128) -> Result<(), Error> { + ReentrancyGuard::before_external_call(env)?; let token_client = MarketUtils::get_token_client(env)?; + // Soroban token transfer returns (), assume success if no panic token_client.transfer(user, &env.current_contract_address(), &stake); + ReentrancyGuard::after_external_call(env); Ok(()) } /// Transfer winnings to user pub fn transfer_winnings(env: &Env, user: &Address, amount: i128) -> Result<(), Error> { + ReentrancyGuard::before_external_call(env)?; let token_client = MarketUtils::get_token_client(env)?; token_client.transfer(&env.current_contract_address(), user, &amount); + ReentrancyGuard::after_external_call(env); Ok(()) }