diff --git a/contracts/predict-iq/src/lib.rs b/contracts/predict-iq/src/lib.rs index cc544358..a74a547e 100644 --- a/contracts/predict-iq/src/lib.rs +++ b/contracts/predict-iq/src/lib.rs @@ -141,9 +141,10 @@ impl PredictIQ { e: Env, bettor: Address, market_id: u64, + outcome: u32, token_address: Address, ) -> Result { - crate::modules::bets::withdraw_refund(&e, bettor, market_id, token_address) + crate::modules::bets::withdraw_refund(&e, bettor, market_id, outcome, token_address) } pub fn get_market(e: Env, id: u64) -> Option { diff --git a/contracts/predict-iq/src/modules/bets.rs b/contracts/predict-iq/src/modules/bets.rs index 34cd7528..31144c8c 100644 --- a/contracts/predict-iq/src/modules/bets.rs +++ b/contracts/predict-iq/src/modules/bets.rs @@ -348,17 +348,53 @@ pub fn withdraw_refund( .get(&bet_key) .ok_or(ErrorCode::MarketNotFound)?; - let refund_amount = bet.amount; + // Issue #51: Creator reclaims their locked creation deposit (once only). + if bettor == market.creator && market.creation_deposit > 0 { + let deposit = market.creation_deposit; + market.creation_deposit = 0; + sac::safe_transfer( + e, + &token_address, + &e.current_contract_address(), + &bettor, + &deposit, + )?; + e.events().publish( + ( + soroban_sdk::Symbol::new(e, "deposit_refunded"), + market_id, + bettor.clone(), + ), + deposit, + ); + } + + let net_amount = bet.amount; + let fee_paid = bet.fee_paid; let bet_outcome = bet.outcome; + // Gross refund = net stake + protocol fee deducted at bet time. + let refund_amount = net_amount + .checked_add(fee_paid) + .ok_or(ErrorCode::ArithmeticOverflow)?; // Update market accounting to maintain accuracy - market.total_staked = market.total_staked.saturating_sub(refund_amount); + market.total_staked = market.total_staked.saturating_sub(net_amount); let outcome_stake = market.outcome_stakes.get(bet_outcome).unwrap_or(0); market .outcome_stakes - .set(bet_outcome, outcome_stake.saturating_sub(refund_amount)); + .set(bet_outcome, outcome_stake.saturating_sub(net_amount)); markets::update_market(e, market); + // Reverse the protocol fee revenue so accounting stays consistent. + crate::modules::fees::reverse_fee(e, token_address.clone(), fee_paid); + + // Reverse any referral reward credited when this bet was placed — a + // referrer only earns rewards from markets that complete, not cancelled ones. + if let Some(referrer) = get_bet_referrer(e, market_id, bettor.clone(), bet_outcome) { + crate::modules::fees::reverse_referral_reward(e, &referrer, &token_address, fee_paid); + remove_bet_referrer(e, market_id, &bettor, bet_outcome); + } + internal_claim_amount( e, market_id, diff --git a/contracts/predict-iq/src/modules/cancellation.rs b/contracts/predict-iq/src/modules/cancellation.rs index b2ec5292..932f2f2a 100644 --- a/contracts/predict-iq/src/modules/cancellation.rs +++ b/contracts/predict-iq/src/modules/cancellation.rs @@ -1,5 +1,5 @@ use crate::errors::ErrorCode; -use crate::modules::{admin, markets, sac}; +use crate::modules::{admin, markets}; use crate::types::{MarketStatus, CANCEL_OUTCOME_INDEX}; use soroban_sdk::{Address, Env, Symbol}; @@ -62,102 +62,6 @@ pub fn cancel_market_vote(e: &Env, market_id: u64) -> Result<(), ErrorCode> { Ok(()) } -/// Withdraw refund for cancelled market (100% principal, zero fees). -/// `outcome` identifies which outcome position to refund. Bettors who placed -/// on multiple outcomes must call this once per outcome to reclaim all funds. -/// Issue #51: If the caller is the market creator, also refunds the creation deposit. -pub fn withdraw_refund( - e: &Env, - bettor: Address, - market_id: u64, - outcome: u32, -) -> Result { - bettor.require_auth(); - - // Issue #93: Refunds are outbound token movements and must respect the - // circuit breaker just like place_bet. A paused contract must not allow - // any token egress — including refunds — to prevent exploitation during - // an active incident. - crate::modules::circuit_breaker::require_not_paused_for_high_risk(e)?; - - let mut market = markets::get_market(e, market_id).ok_or(ErrorCode::MarketNotFound)?; - - if market.status != MarketStatus::Cancelled { - return Err(ErrorCode::MarketNotActive); - } - - // Issue #51: Creator reclaims their locked creation deposit (once only). - if bettor == market.creator && market.creation_deposit > 0 { - let deposit = market.creation_deposit; - market.creation_deposit = 0; - markets::update_market(e, market.clone()); - sac::safe_transfer( - e, - &market.token_address, - &e.current_contract_address(), - &bettor, - &deposit, - )?; - e.events().publish( - ( - Symbol::new(e, "deposit_refunded"), - market_id, - bettor.clone(), - ), - deposit, - ); - // If the creator also placed bets, fall through to refund those too. - } - - let bet_key = crate::modules::bets::DataKey::Bet(market_id, bettor.clone(), outcome); - let bet: crate::types::Bet = match e.storage().persistent().get(&bet_key) { - Some(b) => b, - None => return Ok(0), // creator with no bet on this outcome — deposit already refunded - }; - - // Gross refund = net amount + fee that was deducted at bet time. - // The bettor paid `amount` originally; the contract kept `fee_paid` as - // protocol revenue. On cancellation both must be returned. - let refund_amount = bet - .amount - .checked_add(bet.fee_paid) - .ok_or(crate::errors::ErrorCode::ArithmeticOverflow)?; - let fee_paid = bet.fee_paid; - e.storage().persistent().remove(&bet_key); - - // Reverse the protocol fee revenue so accounting stays consistent. - crate::modules::fees::reverse_fee(e, market.token_address.clone(), fee_paid); - - // Reverse any referral reward that was credited when this bet was placed. - // The referrer only earns rewards from markets that complete — not cancelled ones. - if let Some(referrer) = - crate::modules::bets::get_bet_referrer(e, market_id, bettor.clone(), outcome) - { - crate::modules::fees::reverse_referral_reward( - e, - &referrer, - &market.token_address, - fee_paid, - ); - crate::modules::bets::remove_bet_referrer(e, market_id, &bettor, outcome); - } - - sac::safe_transfer( - e, - &market.token_address, - &e.current_contract_address(), - &bettor, - &refund_amount, - )?; - - crate::modules::events::emit_rewards_claimed( - e, - market_id, - bettor, - refund_amount, - market.token_address, - true, - ); - - Ok(refund_amount) -} +// Issue #1189: withdraw_refund used to be duplicated here, but this module's +// copy was never reachable from any public entrypoint (see #83). The real, +// reachable implementation now lives in `bets::withdraw_refund`. diff --git a/contracts/predict-iq/src/modules/circuit_breaker.rs b/contracts/predict-iq/src/modules/circuit_breaker.rs index 27689d60..dc65cb63 100644 --- a/contracts/predict-iq/src/modules/circuit_breaker.rs +++ b/contracts/predict-iq/src/modules/circuit_breaker.rs @@ -28,6 +28,14 @@ pub fn set_state(e: &Env, state: CircuitBreakerState) -> Result<(), ErrorCode> { _set_state_internal(e, state) } +/// Issue #1190/#1191: entrypoint for governance's guardian-majority emergency +/// pause. Writes/TTL-manages CircuitBreakerState the same way every other +/// transition does, avoiding the persistent-storage TTL mismatch from directly +/// poking instance storage elsewhere. +pub(crate) fn force_pause(e: &Env) -> Result<(), ErrorCode> { + _set_state_internal(e, CircuitBreakerState::Paused) +} + fn _set_state_internal(e: &Env, state: CircuitBreakerState) -> Result<(), ErrorCode> { match state { CircuitBreakerState::Open => { diff --git a/contracts/predict-iq/src/modules/governance.rs b/contracts/predict-iq/src/modules/governance.rs index e4b4ebfe..6f427739 100644 --- a/contracts/predict-iq/src/modules/governance.rs +++ b/contracts/predict-iq/src/modules/governance.rs @@ -1,8 +1,8 @@ use crate::errors::ErrorCode; use crate::types::{ - ConfigKey, Guardian, PendingUpgrade, MAJORITY_THRESHOLD_PERCENT, TIMELOCK_DURATION, - TIMELOCK_MAX_SECONDS, TIMELOCK_MIN_SECONDS, TTL_HIGH_THRESHOLD, TTL_LOW_THRESHOLD, - UPGRADE_COOLDOWN_DURATION, + ConfigKey, Guardian, PendingUpgrade, GUARDIAN_REMOVAL_VOTE_WINDOW, MAJORITY_THRESHOLD_PERCENT, + TIMELOCK_DURATION, TIMELOCK_MAX_SECONDS, TIMELOCK_MIN_SECONDS, TTL_HIGH_THRESHOLD, + TTL_LOW_THRESHOLD, UPGRADE_COOLDOWN_DURATION, }; use soroban_sdk::{Address, BytesN, Env, Vec}; @@ -101,6 +101,7 @@ pub fn remove_guardian(e: &Env, address: Address) -> Result<(), ErrorCode> { target_guardian: address.clone(), initiated_at: e.ledger().timestamp(), votes_for: Vec::new(e), + votes_against: Vec::new(e), }; e.storage() @@ -138,15 +139,35 @@ pub fn vote_on_guardian_removal(e: &Env, voter: Address, approve: bool) -> Resul .get::<_, crate::types::PendingGuardianRemoval>(&ConfigKey::PendingGuardianRemoval) .ok_or(ErrorCode::GuardianNotSet)?; - // Check if voter already voted + // Issue #1194: a stale proposal that nobody resolved must be re-initiated + // rather than staying open forever, slowly accumulating "yes" votes. + let now = e.ledger().timestamp(); + if now.saturating_sub(pending_removal.initiated_at) > GUARDIAN_REMOVAL_VOTE_WINDOW { + e.storage() + .persistent() + .remove(&ConfigKey::PendingGuardianRemoval); + e.storage() + .persistent() + .remove(&ConfigKey::PendingGuardianRemovalPassedAt); + return Err(ErrorCode::GuardianNotSet); + } + + // Check if voter already voted, either way for v in pending_removal.votes_for.iter() { if v == voter { return Err(ErrorCode::AlreadyVotedOnUpgrade); } } + for v in pending_removal.votes_against.iter() { + if v == voter { + return Err(ErrorCode::AlreadyVotedOnUpgrade); + } + } if approve { pending_removal.votes_for.push_back(voter); + } else { + pending_removal.votes_against.push_back(voter); } // Calculate if majority reached (excluding target guardian) @@ -159,6 +180,18 @@ pub fn vote_on_guardian_removal(e: &Env, voter: Address, approve: bool) -> Resul set_guardian_removal_passed_at(e); } + // Issue #1194: a majority voting "against" formally kills the proposal + // rather than leaving it to linger until it either passes or expires. + if pending_removal.votes_against.len() as u32 >= votes_needed { + e.storage() + .persistent() + .remove(&ConfigKey::PendingGuardianRemoval); + e.storage() + .persistent() + .remove(&ConfigKey::PendingGuardianRemovalPassedAt); + return Ok(()); + } + e.storage() .persistent() .set(&ConfigKey::PendingGuardianRemoval, &pending_removal); @@ -488,7 +521,9 @@ pub fn get_upgrade_votes(e: &Env) -> Result Result<(), ErrorCode> { voter.require_auth(); @@ -497,32 +532,56 @@ pub fn emergency_pause(e: &Env, voter: Address) -> Result<(), ErrorCode> { return Err(ErrorCode::NotAuthorized); } - // Verify voter is a guardian - let mut voter_power: u32 = 0; - let mut total_power: u32 = 0; + let mut voter_is_guardian = false; for g in guardians.iter() { - total_power += g.voting_power; if g.address == voter { - voter_power = g.voting_power; + voter_is_guardian = true; + break; } } - - if voter_power == 0 { + if !voter_is_guardian { return Err(ErrorCode::NotAuthorized); } - // Check if this guardian's vote alone meets 2/3 threshold - let threshold = (total_power * 2) / 3; - if voter_power < threshold { - return Err(ErrorCode::InsufficientVotes); + let mut votes: Vec
= e + .storage() + .persistent() + .get(&ConfigKey::PendingEmergencyPause) + .unwrap_or_else(|| Vec::new(e)); + + for v in votes.iter() { + if v == voter { + return Err(ErrorCode::AlreadyVotedOnUpgrade); + } } + votes.push_back(voter); - // Trigger emergency pause - e.storage().instance().set( - &ConfigKey::CircuitBreakerState, - &crate::types::CircuitBreakerState::Paused, - ); - bump_gov_ttl(e, &ConfigKey::CircuitBreakerState); + let mut total_power: u32 = 0; + let mut power_for: u32 = 0; + for g in guardians.iter() { + total_power += g.voting_power; + for v in votes.iter() { + if v == g.address { + power_for += g.voting_power; + break; + } + } + } + + // 2/3 majority, computed as an aggregate across all votes cast so far. + let threshold_met = total_power > 0 && (power_for as u64 * 3) >= (total_power as u64 * 2); + + if threshold_met { + e.storage() + .persistent() + .remove(&ConfigKey::PendingEmergencyPause); + crate::modules::circuit_breaker::force_pause(e)?; + } else { + e.storage() + .persistent() + .set(&ConfigKey::PendingEmergencyPause, &votes); + bump_gov_ttl(e, &ConfigKey::PendingEmergencyPause); + } Ok(()) } diff --git a/contracts/predict-iq/src/types.rs b/contracts/predict-iq/src/types.rs index 21d2ded1..0440432e 100644 --- a/contracts/predict-iq/src/types.rs +++ b/contracts/predict-iq/src/types.rs @@ -132,6 +132,7 @@ pub enum ConfigKey { MaxDisputeWindow, CircuitBreakerThreshold, PendingAdmin, + PendingEmergencyPause, } #[contracttype] @@ -181,8 +182,13 @@ pub struct PendingGuardianRemoval { pub target_guardian: Address, pub initiated_at: u64, pub votes_for: Vec
, + pub votes_against: Vec
, } +// Issue #1194: a guardian-removal proposal must not be able to sit forever +// while only accumulating "yes" votes — it expires and must be re-initiated. +pub const GUARDIAN_REMOVAL_VOTE_WINDOW: u64 = 7 * 24 * 3600; // 7 days + // TTL Management Constants (in ledgers, ~5 seconds per ledger) pub const TTL_LOW_THRESHOLD: u32 = 17_280; // ~1 day (86400 seconds / 5) pub const TTL_HIGH_THRESHOLD: u32 = 518_400; // ~30 days (2592000 seconds / 5)