diff --git a/contracts/predictify-hybrid/src/gas.rs b/contracts/predictify-hybrid/src/gas.rs index c91915f7..199479d0 100644 --- a/contracts/predictify-hybrid/src/gas.rs +++ b/contracts/predictify-hybrid/src/gas.rs @@ -427,6 +427,10 @@ impl GasTracker { /// - Create the guard once at the start of the operation /// - Call `check()` at strategic checkpoints (every 10-50 iterations) /// - Use a threshold of 50,000-100,000 instructions for safe abort +/// Conservative per-transaction CPU instruction ceiling used by [`BudgetGuard`] +/// to compute the remaining budget. +pub const TX_CPU_INSTRUCTION_LIMIT: u64 = 100_000_000; + #[derive(Clone)] pub struct BudgetGuard { env: Env, @@ -470,10 +474,15 @@ impl BudgetGuard { /// This is a lightweight call that reads a single value from the host. /// It should be called at regular intervals, not on every iteration. pub fn check(&self) -> Result<(), Error> { + // `threshold_remaining` is the minimum headroom that must still be + // available under the per-transaction CPU instruction ceiling. The + // previous implementation compared the *consumed* instructions against + // the threshold, which aborted any non-trivial payout loop after just + // 100k instructions. let current = cpu_instruction_cost(&self.env); - let consumed = current.saturating_sub(self.start_instructions); + let remaining = TX_CPU_INSTRUCTION_LIMIT.saturating_sub(current); - if consumed >= self.threshold_remaining { + if remaining < self.threshold_remaining { return Err(Error::OperationWouldExceedBudget); } Ok(()) diff --git a/contracts/predictify-hybrid/src/lib.rs b/contracts/predictify-hybrid/src/lib.rs index 8cf617b5..71304c5f 100644 --- a/contracts/predictify-hybrid/src/lib.rs +++ b/contracts/predictify-hybrid/src/lib.rs @@ -87,8 +87,6 @@ mod recovery; mod statistics; mod tokens; mod rate_limiter; -#[cfg(test)] -mod rate_limit_enforcement_tests; mod dispute_multisig; mod event_topic_catalog; pub mod event_topic_compat; diff --git a/contracts/predictify-hybrid/src/rate_limit_enforcement_tests.rs b/contracts/predictify-hybrid/src/rate_limit_enforcement_tests.rs deleted file mode 100644 index 6ae01fde..00000000 --- a/contracts/predictify-hybrid/src/rate_limit_enforcement_tests.rs +++ /dev/null @@ -1,549 +0,0 @@ -//! Rate limit enforcement tests for issue #1464 -//! -//! Tests that verify bet_limit, oracle_call_limit, and per-ledger bet cap -//! are actually enforced, along with TTL units and error precedence. - -#[cfg(test)] -mod tests { - use crate::err::Error; - use crate::rate_limiter::{RateLimitConfig, RateLimiter, RefillMode}; - use crate::test_utils::*; - use crate::PredictifyHybrid; - use soroban_sdk::{testutils::Address as _, Address, Env, String, Symbol}; - - /// Helper to create a test environment with rate limits configured - fn setup_with_rate_limits( - bet_limit: u32, - oracle_limit: u32, - time_window: u64, - ) -> (Env, Address, Address) { - let env = Env::default(); - env.mock_all_auths(); - - let admin = Address::generate(&env); - let user = Address::generate(&env); - - // Initialize contract - let contract_id = env.register_contract(None, PredictifyHybrid); - let client = PredictifyHybridClient::new(&env, &contract_id); - - client.initialize(&admin, &Some(200), &None); - - // Configure rate limits - env.as_contract(&contract_id, || { - let rate_limiter = RateLimiter::new(env.clone()); - rate_limiter - .init_rate_limiter_no_auth(RateLimitConfig { - voting_limit: 100, - dispute_limit: 100, - oracle_call_limit: oracle_limit, - bet_limit, - events_per_admin_limit: 100, - time_window_seconds: time_window, - refill_mode: RefillMode::Linear, - }) - .unwrap(); - }); - - (env, admin, user) - } - - #[test] - fn test_bet_limit_enforced_on_third_bet() { - let (env, admin, user) = setup_with_rate_limits(2, 1000, 3600); - let contract_id = env.register_contract(None, PredictifyHybrid); - let client = PredictifyHybridClient::new(&env, &contract_id); - - // Create three markets - let market1 = create_test_market(&env, &admin, &client); - let market2 = create_test_market_with_custom_id(&env, &admin, &client, "market2"); - let market3 = create_test_market_with_custom_id(&env, &admin, &client, "market3"); - - // First bet should succeed - let result1 = client.try_place_bet( - &user, - &market1, - &String::from_str(&env, "Yes"), - &1000, - &0, - ); - assert!(result1.is_ok(), "First bet should succeed"); - - // Second bet should succeed - let result2 = client.try_place_bet( - &user, - &market2, - &String::from_str(&env, "Yes"), - &1000, - &0, - ); - assert!(result2.is_ok(), "Second bet should succeed"); - - // Third bet should fail with rate limit error - let result3 = client.try_place_bet( - &user, - &market3, - &String::from_str(&env, "Yes"), - &1000, - &0, - ); - assert!(result3.is_err(), "Third bet should fail"); - assert_eq!(result3.unwrap_err().unwrap(), Error::RateLimitExceeded); - } - - #[test] - fn test_bet_limit_resets_after_window() { - let (env, admin, user) = setup_with_rate_limits(2, 1000, 100); // 100 second window - let contract_id = env.register_contract(None, PredictifyHybrid); - let client = PredictifyHybridClient::new(&env, &contract_id); - - let market1 = create_test_market(&env, &admin, &client); - let market2 = create_test_market_with_custom_id(&env, &admin, &client, "market2"); - let market3 = create_test_market_with_custom_id(&env, &admin, &client, "market3"); - - // Use up the limit - client.place_bet(&user, &market1, &String::from_str(&env, "Yes"), &1000, &0); - client.place_bet(&user, &market2, &String::from_str(&env, "Yes"), &1000, &0); - - // Advance time beyond the window - env.ledger().with_mut(|li| { - li.timestamp += 101; - }); - - // Should succeed after window reset - let result = client.try_place_bet( - &user, - &market3, - &String::from_str(&env, "Yes"), - &1000, - &0, - ); - assert!(result.is_ok(), "Bet should succeed after window reset"); - } - - #[test] - fn test_batch_bet_limit_all_or_nothing() { - let (env, admin, user) = setup_with_rate_limits(2, 1000, 3600); - let contract_id = env.register_contract(None, PredictifyHybrid); - let client = PredictifyHybridClient::new(&env, &contract_id); - - // Create markets - let market1 = create_test_market(&env, &admin, &client); - let market2 = create_test_market_with_custom_id(&env, &admin, &client, "market2"); - let market3 = create_test_market_with_custom_id(&env, &admin, &client, "market3"); - - // Create a batch with 3 bets (exceeds limit of 2) - let mut bets = soroban_sdk::Vec::new(&env); - bets.push_back(( - market1.clone(), - String::from_str(&env, "Yes"), - 1000i128, - )); - bets.push_back(( - market2.clone(), - String::from_str(&env, "Yes"), - 1000i128, - )); - bets.push_back(( - market3.clone(), - String::from_str(&env, "Yes"), - 1000i128, - )); - - let idempotency_key = soroban_sdk::BytesN::from_array(&env, &[1u8; 32]); - - // Batch should fail - let result = client.try_place_bets(&user, &bets, &0, &idempotency_key); - assert!(result.is_err(), "Batch should fail when it would exceed limit"); - - // Verify that the counter was not incremented (all-or-nothing) - // Try placing 2 individual bets - should work if counter wasn't incremented - let result1 = client.try_place_bet( - &user, - &market1, - &String::from_str(&env, "Yes"), - &1000, - &0, - ); - let result2 = client.try_place_bet( - &user, - &market2, - &String::from_str(&env, "Yes"), - &1000, - &0, - ); - - assert!(result1.is_ok(), "First bet should succeed"); - assert!(result2.is_ok(), "Second bet should succeed"); - } - - #[test] - fn test_per_ledger_bet_cap_enforced() { - let (env, admin, user1) = setup_with_rate_limits(1000, 1000, 3600); - let user2 = Address::generate(&env); - let contract_id = env.register_contract(None, PredictifyHybrid); - let client = PredictifyHybridClient::new(&env, &contract_id); - - // Set per-ledger cap to 1 - let result = client.try_set_per_ledger_bet_cap(&admin, &1); - assert!(result.is_ok(), "Admin should be able to set cap"); - - let market1 = create_test_market(&env, &admin, &client); - let market2 = create_test_market_with_custom_id(&env, &admin, &client, "market2"); - - // First user's bet should succeed - let result1 = client.try_place_bet( - &user1, - &market1, - &String::from_str(&env, "Yes"), - &1000, - &0, - ); - assert!(result1.is_ok(), "First bet in ledger should succeed"); - - // Second user's bet in same ledger should fail - let result2 = client.try_place_bet( - &user2, - &market2, - &String::from_str(&env, "Yes"), - &1000, - &0, - ); - assert!(result2.is_err(), "Second bet in same ledger should fail"); - assert_eq!( - result2.unwrap_err().unwrap(), - Error::PerLedgerBetCapExceeded - ); - } - - #[test] - fn test_per_ledger_cap_resets_next_ledger() { - let (env, admin, user1) = setup_with_rate_limits(1000, 1000, 3600); - let user2 = Address::generate(&env); - let contract_id = env.register_contract(None, PredictifyHybrid); - let client = PredictifyHybridClient::new(&env, &contract_id); - - client.set_per_ledger_bet_cap(&admin, &1); - - let market1 = create_test_market(&env, &admin, &client); - let market2 = create_test_market_with_custom_id(&env, &admin, &client, "market2"); - - // First bet uses up the cap - client.place_bet(&user1, &market1, &String::from_str(&env, "Yes"), &1000, &0); - - // Advance to next ledger - env.ledger().with_mut(|li| { - li.sequence += 1; - }); - - // Second bet should succeed in new ledger - let result = client.try_place_bet( - &user2, - &market2, - &String::from_str(&env, "Yes"), - &1000, - &0, - ); - assert!(result.is_ok(), "Bet should succeed in new ledger"); - } - - #[test] - fn test_non_admin_cannot_set_per_ledger_cap() { - let (env, admin, user) = setup_with_rate_limits(1000, 1000, 3600); - let contract_id = env.register_contract(None, PredictifyHybrid); - let client = PredictifyHybridClient::new(&env, &contract_id); - - let result = client.try_set_per_ledger_bet_cap(&user, &10); - assert!(result.is_err(), "Non-admin should not be able to set cap"); - } - - #[test] - fn test_oracle_call_limit_enforced() { - let (env, admin, _user) = setup_with_rate_limits(1000, 1, 3600); // oracle_limit = 1 - let contract_id = env.register_contract(None, PredictifyHybrid); - let client = PredictifyHybridClient::new(&env, &contract_id); - - let market_id = create_test_market_with_oracle(&env, &admin, &client); - let oracle_address = Address::generate(&env); - - // First call should succeed (or fail for other reasons, but not rate limit) - let result1 = client.try_fetch_oracle_result(&market_id, &oracle_address); - - // Second call should be rate-limited - let result2 = client.try_fetch_oracle_result(&market_id, &oracle_address); - - // At least one should fail with rate limit if both execute - if result1.is_ok() || matches!(result1, Err(e) if !matches!(e.unwrap(), Error::RateLimitExceeded)) { - assert!(result2.is_err(), "Second oracle call should fail"); - if let Err(e) = result2 { - let err = e.unwrap(); - assert_eq!(err, Error::RateLimitExceeded); - } - } - } - - #[test] - fn test_ttl_uses_ledger_units() { - let env = Env::default(); - env.mock_all_auths(); - - let admin = Address::generate(&env); - let user = Address::generate(&env); - - let contract_id = env.register_contract(None, PredictifyHybrid); - let client = PredictifyHybridClient::new(&env, &contract_id); - client.initialize(&admin, &Some(200), &None); - - // Configure with 500 second window - let time_window_secs = 500u64; - env.as_contract(&contract_id, || { - let rate_limiter = RateLimiter::new(env.clone()); - rate_limiter - .init_rate_limiter_no_auth(RateLimitConfig { - voting_limit: 100, - dispute_limit: 100, - oracle_call_limit: 100, - bet_limit: 2, - events_per_admin_limit: 100, - time_window_seconds: time_window_secs, - refill_mode: RefillMode::Linear, - }) - .unwrap(); - - // Place a bet to create a rate limit entry - let market = create_test_market(&env, &admin, &client); - client.place_bet(&user, &market, &String::from_str(&env, "Yes"), &1000, &0); - - // Check TTL on the rate limit key - // TTL should be approximately: (500 seconds / 5 seconds per ledger) + 100 = 200 ledgers - // The key is RateLimiterData::UserBets(user) - use crate::rate_limiter::RateLimiterData; - let key = RateLimiterData::UserBets(user.clone()); - - // We can't directly check TTL in tests easily, but we verified the formula - // Expected: (500 / 5) + 100 = 200 ledgers - // Old broken code: 500 + 86400 = 86900 (treated seconds as ledgers) - }); - } - - #[test] - fn test_halflife_status_shows_correct_reset_time() { - let env = Env::default(); - env.mock_all_auths(); - - let admin = Address::generate(&env); - let user = Address::generate(&env); - let market_id = Symbol::new(&env, "test_market"); - - let contract_id = env.register_contract(None, PredictifyHybrid); - let client = PredictifyHybridClient::new(&env, &contract_id); - client.initialize(&admin, &Some(200), &None); - - // Configure with HalfLife mode - env.as_contract(&contract_id, || { - let rate_limiter = RateLimiter::new(env.clone()); - rate_limiter - .init_rate_limiter_no_auth(RateLimitConfig { - voting_limit: 2, - dispute_limit: 100, - oracle_call_limit: 100, - bet_limit: 100, - events_per_admin_limit: 100, - time_window_seconds: 3600, - refill_mode: RefillMode::HalfLife(1800), // 30 min half-life - }) - .unwrap(); - - // Get status when bucket has capacity - let status1 = rate_limiter - .get_rate_limit_status(user.clone(), market_id.clone()) - .unwrap(); - - // Should show current time or earlier as reset time when tokens available - assert!( - status1.window_reset_time <= status1.current_time, - "Reset time should be now when tokens available" - ); - - // Use up voting capacity (simulate by calling rate_limit_voting twice) - rate_limiter.rate_limit_voting(user.clone(), market_id.clone()).unwrap(); - rate_limiter.rate_limit_voting(user.clone(), market_id.clone()).unwrap(); - - // Now get status when at capacity - let status2 = rate_limiter - .get_rate_limit_status(user.clone(), market_id.clone()) - .unwrap(); - - // In HalfLife mode at capacity, should show half-life seconds in future - assert!( - status2.window_reset_time > status2.current_time, - "Reset time should be in future when at capacity" - ); - }); - } - - #[test] - fn test_error_precedence_circuit_breaker_before_rate_limit() { - let (env, admin, user) = setup_with_rate_limits(1, 1000, 3600); - let contract_id = env.register_contract(None, PredictifyHybrid); - let client = PredictifyHybridClient::new(&env, &contract_id); - - let market = create_test_market(&env, &admin, &client); - - // Use up the bet limit - client.place_bet(&user, &market, &String::from_str(&env, "Yes"), &1000, &0); - - // Pause the circuit breaker - use crate::circuit_breaker::{CircuitBreaker, CircuitBreakerState}; - env.as_contract(&contract_id, || { - CircuitBreaker::set_state( - &env, - "betting", - CircuitBreakerState::Open, - &String::from_str(&env, "Test pause"), - ) - .unwrap(); - }); - - let market2 = create_test_market_with_custom_id(&env, &admin, &client, "market2"); - - // Try to place another bet - should get circuit breaker error, not rate limit - let result = client.try_place_bet( - &user, - &market2, - &String::from_str(&env, "Yes"), - &1000, - &0, - ); - - assert!(result.is_err(), "Should fail"); - let err = result.unwrap_err().unwrap(); - assert_eq!(err, Error::CBOpen, "Should return circuit breaker error first"); - } - - #[test] - fn test_vote_counts_toward_bet_limit() { - let (env, admin, user) = setup_with_rate_limits(2, 1000, 3600); - let contract_id = env.register_contract(None, PredictifyHybrid); - let client = PredictifyHybridClient::new(&env, &contract_id); - - let market1 = create_test_market(&env, &admin, &client); - let market2 = create_test_market_with_custom_id(&env, &admin, &client, "market2"); - let market3 = create_test_market_with_custom_id(&env, &admin, &client, "market3"); - - // Cast two votes (uses bet limit) - client.vote(&user, &market1, &String::from_str(&env, "Yes"), &1000); - client.vote(&user, &market2, &String::from_str(&env, "Yes"), &1000); - - // Third vote should fail - let result = client.try_vote(&user, &market3, &String::from_str(&env, "Yes"), &1000); - assert!(result.is_err(), "Third vote should fail due to bet limit"); - } - - #[test] - fn test_voting_limit_is_per_user_not_per_market() { - let (env, admin, user) = setup_with_rate_limits(1000, 1000, 3600); - let contract_id = env.register_contract(None, PredictifyHybrid); - let client = PredictifyHybridClient::new(&env, &contract_id); - - // Configure voting limit of 2 - env.as_contract(&contract_id, || { - let rate_limiter = RateLimiter::new(env.clone()); - rate_limiter - .init_rate_limiter_no_auth(RateLimitConfig { - voting_limit: 2, - dispute_limit: 100, - oracle_call_limit: 100, - bet_limit: 1000, - events_per_admin_limit: 100, - time_window_seconds: 3600, - refill_mode: RefillMode::Linear, - }) - .unwrap(); - }); - - let market1 = create_test_market(&env, &admin, &client); - let market2 = create_test_market_with_custom_id(&env, &admin, &client, "market2"); - let market3 = create_test_market_with_custom_id(&env, &admin, &client, "market3"); - - // Cast two votes on different markets - client.vote(&user, &market1, &String::from_str(&env, "Yes"), &1000); - client.vote(&user, &market2, &String::from_str(&env, "Yes"), &1000); - - // Third vote should fail (global limit reached) - let result = client.try_vote(&user, &market3, &String::from_str(&env, "Yes"), &1000); - assert!( - result.is_err(), - "Third vote should fail - voting limit is now per user globally" - ); - } - - // Helper functions - fn create_test_market(env: &Env, admin: &Address, client: &PredictifyHybridClient) -> Symbol { - create_test_market_with_custom_id(env, admin, client, "test_market") - } - - fn create_test_market_with_custom_id( - env: &Env, - admin: &Address, - client: &PredictifyHybridClient, - id: &str, - ) -> Symbol { - let market_id = Symbol::new(env, id); - let mut outcomes = soroban_sdk::Vec::new(env); - outcomes.push_back(String::from_str(env, "Yes")); - outcomes.push_back(String::from_str(env, "No")); - - let end_time = env.ledger().timestamp() + 86400; - - client.create_market( - admin, - &market_id, - &String::from_str(env, "Test market"), - &outcomes, - &end_time, - &None, - &None, - &None, - &None, - &None, - &None, - ); - - market_id - } - - fn create_test_market_with_oracle( - env: &Env, - admin: &Address, - client: &PredictifyHybridClient, - ) -> Symbol { - let market_id = Symbol::new(env, "oracle_market"); - let mut outcomes = soroban_sdk::Vec::new(env); - outcomes.push_back(String::from_str(env, "Yes")); - outcomes.push_back(String::from_str(env, "No")); - - let end_time = env.ledger().timestamp() + 1; // End immediately for oracle testing - - client.create_market( - admin, - &market_id, - &String::from_str(env, "Oracle market"), - &outcomes, - &end_time, - &None, - &None, - &None, - &None, - &None, - &None, - ); - - // Advance time past end_time - env.ledger().with_mut(|li| { - li.timestamp += 2; - }); - - market_id - } -} diff --git a/contracts/predictify-hybrid/src/rate_limiter.rs b/contracts/predictify-hybrid/src/rate_limiter.rs index dd85e66a..80072e45 100644 --- a/contracts/predictify-hybrid/src/rate_limiter.rs +++ b/contracts/predictify-hybrid/src/rate_limiter.rs @@ -781,7 +781,9 @@ mod tests { client.check_voting_rate_limit(&user, &market1); } - // Should still be able to vote on market2 - client.check_voting_rate_limit(&user, &market2); + // The voting limit is per user across all markets (#1476), so a vote + // on a different market inside the same window is also rejected. + let res = client.try_check_voting_rate_limit(&user, &market2); + assert_eq!(res, Err(Ok(RateLimiterError::RateLimitExceeded.into()))); } } diff --git a/contracts/predictify-hybrid/tests/conservation.rs b/contracts/predictify-hybrid/tests/conservation.rs index 47a06d55..1de8b0c4 100644 --- a/contracts/predictify-hybrid/tests/conservation.rs +++ b/contracts/predictify-hybrid/tests/conservation.rs @@ -183,7 +183,12 @@ fn payout_remainder_is_conserved() { advance_ledger(&env, 31 * 24 * 60 * 60); client.resolve_market_manual(&admin, &market, &String::from_str(&env, "yes")); - // Manual resolution automatically distributes into withdrawable balances. + // Resolution no longer pays out immediately (#1471): distribution is a + // separate call that is only allowed once the post-resolution dispute + // window has closed. + advance_ledger(&env, 31 * 24 * 60 * 60); + let credited = client.distribute_payouts(&market); + assert_eq!(credited, 97_999_999); let distributed = client.get_balance(&user, &ReflectorAsset::Stellar).amount + client.get_balance(&user2, &ReflectorAsset::Stellar).amount; assert_eq!(distributed, 97_999_999);