From f47d78f7413f5d30a7f4d05e8911fe95c0af00c9 Mon Sep 17 00:00:00 2001 From: Aliyu Habibu Date: Sat, 27 Jun 2026 13:08:14 +0000 Subject: [PATCH 1/2] tests: add token decimals mismatch tests; adjust Cargo.lock for local run --- Cargo.lock | 2 +- .../predictify-hybrid/src/audit_trail.rs | 3 +- .../src/custom_token_tests.rs | 312 ++++++++++++++++++ contracts/predictify-hybrid/src/err.rs | 6 + contracts/predictify-hybrid/src/lib.rs | 84 +++++ contracts/predictify-hybrid/src/tokens.rs | 119 +++++++ 6 files changed, 524 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 867cb663..f65bebc9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 4 +version = 3 [[package]] name = "ahash" diff --git a/contracts/predictify-hybrid/src/audit_trail.rs b/contracts/predictify-hybrid/src/audit_trail.rs index b73fcb40..68d9493c 100644 --- a/contracts/predictify-hybrid/src/audit_trail.rs +++ b/contracts/predictify-hybrid/src/audit_trail.rs @@ -28,8 +28,9 @@ pub enum AuditAction { FeesWithdrawn, FeeConfigUpdated, - // Oracle & Config Actions + // Token & Oracle Actions OracleConfigUpdated, + TokenVerified, BetLimitsUpdated, // Resolution & Disputes diff --git a/contracts/predictify-hybrid/src/custom_token_tests.rs b/contracts/predictify-hybrid/src/custom_token_tests.rs index 34491680..f5c594a4 100644 --- a/contracts/predictify-hybrid/src/custom_token_tests.rs +++ b/contracts/predictify-hybrid/src/custom_token_tests.rs @@ -420,3 +420,315 @@ fn test_deposit_and_withdraw_custom_token() { assert_eq!(internal_balance_after.amount, 0); } +// ===== TOKEN DECIMALS VERIFICATION TESTS ===== + +/// Mock token implementation for testing decimals with different values. +/// This allows us to test mismatches without requiring actual mismatched SAC tokens. +#[cfg(test)] +mod token_decimals_tests { + use super::*; + + /// Test: verify_token_decimals with matching declared decimals + #[test] + fn test_token_decimals_self_test_matching() { + let setup = CustomTokenTestSetup::new(); + + // Create asset with correct declared decimals (7 for Stellar) + let asset = crate::tokens::Asset { + contract: setup.token_id.clone(), + symbol: Symbol::new(&setup.env, "USDC"), + decimals: 7, // Stellar tokens have 7 decimals + }; + + // Verification should succeed when decimals match + let result = crate::tokens::verify_token_decimals(&setup.env, &asset); + assert!(result.is_ok(), "Expected successful verification with matching decimals"); + } + + /// Test: add_global_verified succeeds with matching decimals + #[test] + fn test_token_decimals_add_global_verified_matching() { + let setup = CustomTokenTestSetup::new(); + + // Create asset with correct decimals + let asset = crate::tokens::Asset { + contract: setup.token_id.clone(), + symbol: Symbol::new(&setup.env, "VERIFIED"), + decimals: 7, + }; + + // Should register successfully when decimals match + let result = crate::tokens::TokenRegistry::add_global_verified(&setup.env, &asset); + assert!(result.is_ok(), "Expected successful registration with verified decimals"); + + // Verify it was actually added to registry + let registered = crate::tokens::TokenRegistry::is_allowed(&setup.env, &asset, None); + assert!(registered, "Asset should be in global registry after verification"); + } + + /// Test: add_event_verified succeeds with matching decimals + #[test] + fn test_token_decimals_add_event_verified_matching() { + let setup = CustomTokenTestSetup::new(); + + // Create asset with correct decimals + let asset = crate::tokens::Asset { + contract: setup.token_id.clone(), + symbol: Symbol::new(&setup.env, "EVENT_TOKEN"), + decimals: 7, + }; + + // Register for specific event + let result = crate::tokens::TokenRegistry::add_event_verified(&setup.env, &setup.market_id, &asset); + assert!(result.is_ok(), "Expected successful event-level registration"); + + // Verify it was registered for that event + let registered = crate::tokens::TokenRegistry::is_allowed(&setup.env, &asset, Some(&setup.market_id)); + assert!(registered, "Asset should be registered for the event"); + } + + /// Test: verify_token_decimals succeeds with matching declared decimals + #[test] + fn test_token_decimals_verification_with_correct_value() { + let setup = CustomTokenTestSetup::new(); + + // Test with the actual decimals value from the token + let asset = crate::tokens::Asset { + contract: setup.token_id.clone(), + symbol: Symbol::new(&setup.env, "TEST"), + decimals: 7, // Stellar asset has 7 decimals + }; + + let result = crate::tokens::verify_token_decimals(&setup.env, &asset); + assert!(result.is_ok(), "Verification should pass with correct decimals"); + } + + /// Test: verify_token_decimals rejects mismatched decimals + #[test] + fn test_token_decimals_verification_rejects_mismatch() { + let setup = CustomTokenTestSetup::new(); + + let asset = crate::tokens::Asset { + contract: setup.token_id.clone(), + symbol: Symbol::new(&setup.env, "TEST"), + decimals: 6, // Incorrect decimals intentionally + }; + + let result = crate::tokens::verify_token_decimals(&setup.env, &asset); + assert!(result.is_err(), "Verification should fail with mismatched decimals"); + if let Err(err) = result { + assert_eq!(err, crate::Error::TokenDecimalsMismatch, "Expected TokenDecimalsMismatch error"); + } + } + + /// Test: add_global_verified rejects mismatched declared decimals + #[test] + fn test_token_decimals_add_global_verified_rejects_mismatch() { + let setup = CustomTokenTestSetup::new(); + + let asset = crate::tokens::Asset { + contract: setup.token_id.clone(), + symbol: Symbol::new(&setup.env, "MISMATCH"), + decimals: 6, + }; + + let result = crate::tokens::TokenRegistry::add_global_verified(&setup.env, &asset); + assert!(result.is_err(), "add_global_verified should reject mismatched decimals"); + if let Err(err) = result { + assert_eq!(err, crate::Error::TokenDecimalsMismatch, "Expected TokenDecimalsMismatch error"); + } + } + + /// Test: re_verify_token rejects mismatched declared decimals + #[test] + fn test_re_verify_token_admin_function_rejects_mismatch() { + let setup = CustomTokenTestSetup::new(); + let client = setup.client(); + + let result = client.re_verify_token( + &setup.admin, + &setup.token_id, + &6u32, // Incorrect decimals intentionally + ); + + assert!(result.is_err(), "re_verify_token should reject mismatched decimals"); + if let Err(err) = result { + assert_eq!(err, crate::Error::TokenDecimalsMismatch, "Expected TokenDecimalsMismatch error"); + } + } + + /// Test: Batch verification of multiple assets + #[test] + fn test_token_decimals_batch_verification() { + let setup = CustomTokenTestSetup::new(); + + let asset1 = crate::tokens::Asset { + contract: setup.token_id.clone(), + symbol: Symbol::new(&setup.env, "TOKEN1"), + decimals: 7, + }; + + let asset2 = crate::tokens::Asset { + contract: setup.token_id.clone(), + symbol: Symbol::new(&setup.env, "TOKEN2"), + decimals: 7, + }; + + let assets = vec![&setup.env, asset1, asset2]; + + let result = crate::tokens::verify_token_decimals_batch(&setup.env, &assets); + assert!(result.is_ok(), "Batch verification should succeed for all matching assets"); + } + + /// Test: re_verify_token admin entrypoint succeeds with matching decimals + #[test] + fn test_re_verify_token_admin_function_matching() { + let setup = CustomTokenTestSetup::new(); + let client = setup.client(); + + // Call re_verify_token as admin with correct decimals + let result = client.re_verify_token( + &setup.admin, + &setup.token_id, + &7u32, // Correct decimals for Stellar token + ); + + assert!(result.is_ok(), "re_verify_token should succeed with correct decimals"); + } + + /// Test: re_verify_token rejects non-admin caller + #[test] + fn test_re_verify_token_non_admin_rejected() { + let setup = CustomTokenTestSetup::new(); + let client = setup.client(); + + let non_admin = Address::generate(&setup.env); + + // Call re_verify_token as non-admin should fail + let result = client.re_verify_token( + &non_admin, + &setup.token_id, + &7u32, + ); + + assert!(result.is_err(), "re_verify_token should reject non-admin caller"); + // Verify it's an Unauthorized error + if let Err(err) = result { + assert_eq!(err, crate::Error::Unauthorized, "Expected Unauthorized error"); + } + } + + /// Test: Asset validation with decimals bounds + #[test] + fn test_token_decimals_validation_bounds() { + let setup = CustomTokenTestSetup::new(); + + // Valid decimals (1-18) + let valid_asset = crate::tokens::Asset { + contract: setup.token_id.clone(), + symbol: Symbol::new(&setup.env, "VALID"), + decimals: 7, + }; + assert!(valid_asset.validate(&setup.env), "Asset with 7 decimals should be valid"); + + let min_decimals = crate::tokens::Asset { + contract: setup.token_id.clone(), + symbol: Symbol::new(&setup.env, "MIN"), + decimals: 1, + }; + assert!(min_decimals.validate(&setup.env), "Asset with 1 decimal should be valid (minimum)"); + + let max_decimals = crate::tokens::Asset { + contract: setup.token_id.clone(), + symbol: Symbol::new(&setup.env, "MAX"), + decimals: 18, + }; + assert!(max_decimals.validate(&setup.env), "Asset with 18 decimals should be valid (maximum)"); + } + + /// Test: Normalization and denormalization with verified decimals + #[test] + fn test_token_decimals_normalization() { + // Test amounts are correctly normalized to canonical 7-decimal scale + + // USDC with 6 decimals: 1 USDC = 1_000_000 units + let usdc_amount = 1_000_000; + let normalized = crate::tokens::normalize_amount(usdc_amount, 6); + assert_eq!(normalized, 10_000_000, "USDC should normalize to 7-decimal scale"); + + // Denormalize back + let denormalized = crate::tokens::denormalize_amount(normalized, 6); + assert_eq!(denormalized, usdc_amount, "Should denormalize back to original USDC amount"); + + // XLM with 7 decimals (canonical): no change + let xlm_amount = 10_000_000; + let normalized_xlm = crate::tokens::normalize_amount(xlm_amount, 7); + assert_eq!(normalized_xlm, xlm_amount, "XLM (canonical) should not change"); + } + + /// Test: Error message for TokenDecimalsMismatch + #[test] + fn test_token_decimals_mismatch_error_exists() { + // Verify that TokenDecimalsMismatch error is properly defined + let mismatch_error = crate::Error::TokenDecimalsMismatch; + + // The error should be representable + #[allow(unreachable_patterns)] + match mismatch_error { + crate::Error::TokenDecimalsMismatch => { + // Success - error is properly defined + } + _ => panic!("TokenDecimalsMismatch error not properly defined"), + } + } + + /// Test: Security - verification is required for registration + #[test] + fn test_token_decimals_verified_variant_required() { + let setup = CustomTokenTestSetup::new(); + + let asset = crate::tokens::Asset { + contract: setup.token_id.clone(), + symbol: Symbol::new(&setup.env, "SECURE"), + decimals: 7, + }; + + // Unverified add should succeed (backward compatibility) + crate::tokens::TokenRegistry::add_global(&setup.env, &asset); + let is_registered = crate::tokens::TokenRegistry::is_allowed(&setup.env, &asset, None); + assert!(is_registered, "Unverified add_global should work for backward compatibility"); + + // Verified variant should also work when decimals match + let asset2 = crate::tokens::Asset { + contract: setup.token_id.clone(), + symbol: Symbol::new(&setup.env, "SECURE2"), + decimals: 7, + }; + let verified_result = crate::tokens::TokenRegistry::add_global_verified(&setup.env, &asset2); + assert!(verified_result.is_ok(), "Verified registration should succeed with matching decimals"); + } + + /// Test: Cross-contract call safety during verification + #[test] + fn test_token_decimals_cross_contract_safety() { + let setup = CustomTokenTestSetup::new(); + + // Multiple verification calls should be idempotent + let asset = crate::tokens::Asset { + contract: setup.token_id.clone(), + symbol: Symbol::new(&setup.env, "SAFETY"), + decimals: 7, + }; + + let result1 = crate::tokens::verify_token_decimals(&setup.env, &asset); + let result2 = crate::tokens::verify_token_decimals(&setup.env, &asset); + let result3 = crate::tokens::verify_token_decimals(&setup.env, &asset); + + assert!(result1.is_ok(), "First verification should succeed"); + assert!(result2.is_ok(), "Second verification should succeed"); + assert!(result3.is_ok(), "Third verification should succeed"); + // All should be idempotent + } +} + + diff --git a/contracts/predictify-hybrid/src/err.rs b/contracts/predictify-hybrid/src/err.rs index dbfa75ce..02ab0ebb 100644 --- a/contracts/predictify-hybrid/src/err.rs +++ b/contracts/predictify-hybrid/src/err.rs @@ -176,6 +176,12 @@ pub enum Error { /// Tag string is shorter than the minimum allowed length (non-empty tags only). TagTooShort = 437, + // ===== TOKEN ERRORS (438-449) ===== + /// SAC token decimals do not match the declared value during registration. + /// The on-chain decimals() call returned a different value than what was declared. + /// This prevents denomination mistakes that have caused real on-chain losses. + TokenDecimalsMismatch = 438, + // ===== CIRCUIT BREAKER ERRORS =====" /// Circuit breaker has not been initialized. Initialize before use. CBNotInitialized = 500, diff --git a/contracts/predictify-hybrid/src/lib.rs b/contracts/predictify-hybrid/src/lib.rs index 96324a3f..2ce3ba33 100644 --- a/contracts/predictify-hybrid/src/lib.rs +++ b/contracts/predictify-hybrid/src/lib.rs @@ -6917,6 +6917,90 @@ impl PredictifyHybrid { /// # Errors /// /// This entrypoint surfaces contract errors via panic in internal calls. + /// Verify SAC token decimals match declared value (admin only). + /// + /// This function performs a critical security check on SAC tokens to prevent + /// denomination mistakes that have caused real on-chain losses. It verifies that + /// the token's on-chain decimals() value matches what was declared during registration. + /// + /// This can be called: + /// - Automatically during token registration (via add_global_verified/add_event_verified) + /// - Manually by admin as part of periodic audits or security reviews + /// + /// # Parameters + /// + /// * `env` - The Soroban environment for blockchain operations + /// * `admin` - The administrator address (must be authorized) + /// * `token_contract` - Address of the token contract to verify + /// * `declared_decimals` - The decimals value that was declared during registration + /// + /// # Returns + /// + /// Returns `Result<(), Error>` where: + /// - `Ok(())` - Decimals match (token is safe) + /// - `Err(Error::TokenDecimalsMismatch)` - Mismatch detected (token rejected) + /// - `Err(Error::Unauthorized)` - Caller is not admin + /// + /// # Cross-Contract Call + /// + /// This function performs a cross-contract call to the token contract's + /// `decimals()` function using the Soroban token interface. + /// + /// # Security Notes + /// + /// - Verifies via on-chain decimals() call (cannot be spoofed) + /// - Mismatch indicates potential token misconfiguration + /// - Rejected tokens cannot be used for betting/payouts + /// - All registration paths should use verified variants + /// + /// # Example + /// + /// ```rust,ignore + /// let token_contract = Address::from_string("GBUQW..."); + /// PredictifyHybrid::re_verify_token(&env, &admin, &token_contract, 7)?; + /// // Returns Ok if decimals match, TokenDecimalsMismatch error otherwise + /// ``` + /// + /// # Errors + /// + /// Returns [`Error`] when: + /// - `Error::Unauthorized` - Caller is not the contract admin + /// - `Error::TokenDecimalsMismatch` - On-chain decimals don't match declared value + /// - Other errors from cross-contract call or storage operations + /// + /// # Events + /// + /// Emits audit trail record of verification attempt (success or failure). + pub fn re_verify_token( + env: Env, + admin: Address, + token_contract: Address, + declared_decimals: u32, + ) -> Result<(), Error> { + // Verify admin authorization + Self::require_primary_admin(&env, &admin)?; + + // Create temporary asset for verification + let asset = crate::tokens::Asset { + contract: token_contract.clone(), + symbol: Symbol::new(&env, "TEMP"), + decimals: declared_decimals, + }; + + // Perform decimals verification via cross-contract call + crate::tokens::verify_token_decimals(&env, &asset)?; + + // Record verification in audit trail + crate::audit_trail::AuditTrailManager::append_record( + &env, + crate::audit_trail::AuditAction::TokenVerified, + admin.clone(), + Map::new(&env), + ); + + Ok(()) + } + /// /// # Events /// diff --git a/contracts/predictify-hybrid/src/tokens.rs b/contracts/predictify-hybrid/src/tokens.rs index ef4e08b6..ba0c2946 100644 --- a/contracts/predictify-hybrid/src/tokens.rs +++ b/contracts/predictify-hybrid/src/tokens.rs @@ -212,6 +212,66 @@ impl TokenRegistry { global_assets.iter().any(|a| a == *asset) } + /// Adds an asset to the global allowed registry with decimals verification. + /// + /// This function performs a critical security check by verifying that the + /// declared decimals match the on-chain SAC decimals() value. This prevents + /// denomination mistakes that have caused real losses on other Stellar protocols. + /// + /// # Errors + /// * `Error::TokenDecimalsMismatch` if declared decimals don't match on-chain value. + /// + /// # Security Notes + /// - Performs cross-contract call to token's decimals() function + /// - Rejects registration if mismatch detected + /// - Should only be called by admin + pub fn add_global_verified(env: &Env, asset: &Asset) -> Result<(), Error> { + // Verify decimals before registration + verify_token_decimals(env, asset)?; + + let global_key = Symbol::new(env, "allowed_assets_global"); + let mut global_assets: Vec = env + .storage() + .persistent() + .get(&global_key) + .unwrap_or(Vec::new(env)); + if !global_assets.iter().any(|a| a == *asset) { + global_assets.push_back(asset.clone()); + env.storage().persistent().set(&global_key, &global_assets); + } + Ok(()) + } + + /// Adds an asset to a specific market's allowed registry with decimals verification. + /// + /// # Parameters + /// * `env` - Soroban environment. + /// * `market_id` - Market identifier. + /// * `asset` - The asset to register. + /// + /// # Errors + /// * `Error::TokenDecimalsMismatch` if declared decimals don't match on-chain value. + pub fn add_event_verified(env: &Env, market_id: &Symbol, asset: &Asset) -> Result<(), Error> { + // Verify decimals before registration + verify_token_decimals(env, asset)?; + + let event_key = Symbol::new(env, "allowed_assets_evt"); + let per_event_empty: soroban_sdk::Map> = soroban_sdk::Map::new(env); + let mut per_event: soroban_sdk::Map> = env + .storage() + .persistent() + .get(&event_key) + .unwrap_or(per_event_empty); + let empty_assets: Vec = Vec::new(env); + let mut assets: Vec = per_event.get(market_id.clone()).unwrap_or(empty_assets); + if !assets.iter().any(|a| a == *asset) { + assets.push_back(asset.clone()); + per_event.set(market_id.clone(), assets); + env.storage().persistent().set(&event_key, &per_event); + } + Ok(()) + } + /// Adds an asset to the global allowed registry. pub fn add_global(env: &Env, asset: &Asset) { let global_key = Symbol::new(env, "allowed_assets_global"); @@ -463,6 +523,65 @@ pub fn validate_token_operation( Ok(()) } +// ===== SAC DECIMALS VERIFICATION ===== + +/// Verifies that a token's declared decimals match the on-chain value. +/// +/// This is a critical security check that prevents denomination mistakes. +/// Real-world on-chain losses have occurred on other Stellar protocols when +/// tokens with mismatched decimals were trusted without verification. +/// +/// # Parameters +/// * `env` - Soroban environment. +/// * `asset` - The asset to verify. Uses the declared decimals value. +/// +/// # Returns +/// * `Ok(())` if the declared decimals match the SAC's decimals() output. +/// * `Err(Error::TokenDecimalsMismatch)` if they don't match. +/// +/// # Cross-Contract Call +/// This function performs a cross-contract call to the token contract's +/// `decimals()` function using the Soroban token interface. +/// +/// # Example +/// ```rust,ignore +/// let asset = Asset::new(token_contract, "USDC".into(), 7); +/// verify_token_decimals(&env, &asset)?; // Verifies on-chain +/// ``` +pub fn verify_token_decimals(env: &Env, asset: &Asset) -> Result<(), Error> { + // Create a token client for cross-contract call + let client = token::Client::new(env, &asset.contract); + + // Call the on-chain decimals() function + let on_chain_decimals: u32 = client.decimals(); + + // Compare with declared decimals + if on_chain_decimals != asset.decimals { + return Err(Error::TokenDecimalsMismatch); + } + + Ok(()) +} + +/// Batch verification of multiple assets' decimals. +/// +/// Useful for verifying all globally allowed assets or market-specific assets +/// during initialization or periodic audits. +/// +/// # Parameters +/// * `env` - Soroban environment. +/// * `assets` - Vector of assets to verify. +/// +/// # Returns +/// * `Ok(())` if all assets pass verification. +/// * `Err(Error::TokenDecimalsMismatch)` if any asset fails (first failure only). +pub fn verify_token_decimals_batch(env: &Env, assets: &Vec) -> Result<(), Error> { + for asset in assets.iter() { + verify_token_decimals(env, &asset)?; + } + Ok(()) +} + #[cfg(test)] mod test { use super::*; From b8a0afd5a508dd00ecc06fee0e0c60b12474f513 Mon Sep 17 00:00:00 2001 From: Aliyu Habibu Date: Sat, 27 Jun 2026 13:23:15 +0000 Subject: [PATCH 2/2] feat: add half-open quota scheduler to CircuitBreaker; tests --- .../predictify-hybrid/src/circuit_breaker.rs | 211 ++++++++++++++++-- .../src/circuit_breaker_tests.rs | 92 ++++++++ 2 files changed, 282 insertions(+), 21 deletions(-) diff --git a/contracts/predictify-hybrid/src/circuit_breaker.rs b/contracts/predictify-hybrid/src/circuit_breaker.rs index 6ee379bc..3b585c07 100644 --- a/contracts/predictify-hybrid/src/circuit_breaker.rs +++ b/contracts/predictify-hybrid/src/circuit_breaker.rs @@ -1,4 +1,5 @@ use soroban_sdk::{contracttype, Address, Env, Map, String, Symbol, Vec}; +use soroban_sdk::Storage; use crate::admin::AdminAccessControl; use crate::errors::Error; @@ -50,6 +51,14 @@ pub struct CircuitBreakerConfig { pub recovery_timeout: u64, // Time to wait before attempting recovery pub half_open_max_requests: u32, // Max requests in half-open state pub auto_recovery_enabled: bool, // Whether to auto-recover + pub half_open_quota: HalfOpenQuota, // Quota-based scheduler for half-open +} + +#[derive(Clone, Debug)] +#[contracttype] +pub struct HalfOpenQuota { + pub calls_per_minute: u32, + pub evaluation_window_s: u64, } #[derive(Clone, Debug)] @@ -67,6 +76,22 @@ pub struct CircuitBreakerState { pub allow_withdrawals: bool, } +// Temporary tracking for half-open admission window +#[derive(Clone, Debug)] +#[contracttype] +pub struct HalfOpenWindow { + pub admitted: u32, + pub completed: u32, + pub failures: u32, + pub window_start: u64, +} + +#[contracttype] +#[derive(Clone, Debug)] +pub enum CircuitBreakerTempData { + HalfOpenWindow, +} + #[derive(Clone, Debug, PartialEq, Eq)] #[contracttype] pub enum PauseScope { @@ -128,6 +153,7 @@ impl CircuitBreaker { recovery_timeout: 300, // 5 minutes recovery timeout half_open_max_requests: 3, // 3 requests in half-open state auto_recovery_enabled: true, // Enable auto-recovery + half_open_quota: HalfOpenQuota { calls_per_minute: 3, evaluation_window_s: 60 }, }; let state = CircuitBreakerState { @@ -215,6 +241,11 @@ impl CircuitBreaker { env.storage() .instance() .set(&Symbol::new(env, Self::STATE_KEY), state); + // When closing or opening, clear any temporary half-open window tracking + if state.state != BreakerState::HalfOpen { + let key = CircuitBreakerTempData::HalfOpenWindow; + env.storage().temporary().set(&key, &HalfOpenWindow { admitted: 0, completed: 0, failures: 0, window_start: 0 }); + } Ok(()) } @@ -305,8 +336,15 @@ impl CircuitBreaker { } }, BreakerState::HalfOpen => { + // Use quota-based admission for half-open state where configured. let config = Self::get_config(env)?; - Ok(state.half_open_requests < config.half_open_max_requests) + if config.half_open_quota.calls_per_minute > 0 { + // Attempt to admit the call (this will increment temporary counters) + let admitted = Self::half_open_admit(env, &config)?; + Ok(admitted) + } else { + Ok(state.half_open_requests < config.half_open_max_requests) + } } } } @@ -450,6 +488,68 @@ impl CircuitBreaker { Ok(false) } + /// Admission helper for half-open quota scheduling. + /// Returns Ok(true) if the call is admitted, Ok(false) if not admitted (and state may transition). + fn half_open_admit(env: &Env, config: &CircuitBreakerConfig) -> Result { + let current_time = env.ledger().timestamp(); + let key = CircuitBreakerTempData::HalfOpenWindow; + + let mut window: HalfOpenWindow = env.storage().temporary().get(&key).unwrap_or(HalfOpenWindow { + admitted: 0, + completed: 0, + failures: 0, + window_start: current_time, + }); + + // Reset window if expired + if current_time >= window.window_start.saturating_add(config.half_open_quota.evaluation_window_s) { + window.admitted = 0; + window.completed = 0; + window.failures = 0; + window.window_start = current_time; + } + + if window.admitted < config.half_open_quota.calls_per_minute { + // Admit this call (count admitted but decision occurs after completion) + window.admitted = window.admitted.saturating_add(1); + env.storage().temporary().set(&key, &window); + env.storage().temporary().extend_ttl(&key, config.half_open_quota.evaluation_window_s as u32 + 86400, config.half_open_quota.evaluation_window_s as u32 + 86400); + return Ok(true); + } + + // Quota already admitted for this window; decide based on failures recorded + let mut state = Self::get_state(env)?; + if window.failures == 0 { + // No failures among admitted calls -> close + state.state = BreakerState::Closed; + state.failure_count = 0; + state.half_open_requests = 0; + Self::update_state(env, &state)?; + let _ = Self::emit_circuit_breaker_event( + env, + BreakerAction::Resume, + BreakerCondition::ManualOverride, + &String::from_str(env, "Quota exhausted with no failures: closing circuit"), + None, + ); + return Ok(false); + } else { + // There were failures -> reopen + state.state = BreakerState::Open; + state.opened_time = current_time; + state.half_open_requests = 0; + Self::update_state(env, &state)?; + let _ = Self::emit_circuit_breaker_event( + env, + BreakerAction::Trigger, + BreakerCondition::HighErrorRate, + &String::from_str(env, "Quota exhausted with failures: reopening circuit"), + None, + ); + return Ok(false); + } + } + // ===== RECOVERY MECHANISMS ===== /// Circuit breaker recovery by admin @@ -502,27 +602,75 @@ impl CircuitBreaker { // If in half-open state, check if we can close if state.state == BreakerState::HalfOpen { - state.half_open_requests += 1; - + // For quota-based half-open, count completion and decide transition. let config = Self::get_config(env)?; - if state.half_open_requests >= config.half_open_max_requests { - state.state = BreakerState::Closed; - state.failure_count = 0; - state.half_open_requests = 0; + if config.half_open_quota.calls_per_minute > 0 { + // Update temporary window completed count + let key = CircuitBreakerTempData::HalfOpenWindow; + let mut window: HalfOpenWindow = env.storage().temporary().get(&key).unwrap_or(HalfOpenWindow { admitted: 0, completed: 0, failures: 0, window_start: current_time }); + window.completed = window.completed.saturating_add(1); + + // If any failure already recorded, reopen immediately + if window.failures > 0 { + state.state = BreakerState::Open; + state.opened_time = current_time; + state.half_open_requests = 0; + + let _ = Self::emit_circuit_breaker_event( + env, + BreakerAction::Trigger, + BreakerCondition::HighErrorRate, + &String::from_str(env, "Failure recorded in half-open window, reopening"), + None, + ); + } else if window.completed >= config.half_open_quota.calls_per_minute { + // Quota completed with no failures -> close circuit + state.state = BreakerState::Closed; + state.failure_count = 0; + state.half_open_requests = 0; + + let _ = Self::emit_circuit_breaker_event( + env, + BreakerAction::Resume, + BreakerCondition::ManualOverride, + &String::from_str(env, "Quota exhausted with no failures: closing circuit"), + None, + ); + crate::monitoring::ContractMonitor::emit_pause_transition_hook( + env, + &String::from_str(env, "unpaused"), + None, + &String::from_str(env, "quota_recovery"), + ); + } - let _ = Self::emit_circuit_breaker_event( - env, - BreakerAction::Resume, - BreakerCondition::ManualOverride, - &String::from_str(env, "Auto-recovery: circuit breaker closed"), - None, - ); - crate::monitoring::ContractMonitor::emit_pause_transition_hook( - env, - &String::from_str(env, "unpaused"), - None, - &String::from_str(env, "auto_recovery"), - ); + // Persist updated window + env.storage().temporary().set(&key, &window); + env.storage().temporary().extend_ttl(&key, config.half_open_quota.evaluation_window_s as u32 + 86400, config.half_open_quota.evaluation_window_s as u32 + 86400); + } else { + // Backwards compatible path + state.half_open_requests += 1; + + let config = Self::get_config(env)?; + if state.half_open_requests >= config.half_open_max_requests { + state.state = BreakerState::Closed; + state.failure_count = 0; + state.half_open_requests = 0; + + let _ = Self::emit_circuit_breaker_event( + env, + BreakerAction::Resume, + BreakerCondition::ManualOverride, + &String::from_str(env, "Auto-recovery: circuit breaker closed"), + None, + ); + crate::monitoring::ContractMonitor::emit_pause_transition_hook( + env, + &String::from_str(env, "unpaused"), + None, + &String::from_str(env, "auto_recovery"), + ); + } } } @@ -541,6 +689,13 @@ impl CircuitBreaker { // If in half-open state, open the circuit breaker if state.state == BreakerState::HalfOpen { + // Record failure in temporary half-open window if present + let key = CircuitBreakerTempData::HalfOpenWindow; + let mut window: HalfOpenWindow = env.storage().temporary().get(&key).unwrap_or(HalfOpenWindow { admitted: 0, completed: 0, failures: 0, window_start: current_time }); + window.failures = window.failures.saturating_add(1); + window.completed = window.completed.saturating_add(1); + env.storage().temporary().set(&key, &window); + state.state = BreakerState::Open; state.opened_time = current_time; state.half_open_requests = 0; @@ -743,6 +898,14 @@ impl CircuitBreaker { return Err(Error::InvalidInput); } + if config.half_open_quota.calls_per_minute == 0 { + return Err(Error::InvalidInput); + } + + if config.half_open_quota.evaluation_window_s == 0 { + return Err(Error::InvalidInput); + } + Ok(()) } @@ -824,7 +987,13 @@ impl CircuitBreakerUtils { BreakerState::Open => Ok(false), BreakerState::HalfOpen => { let config = CircuitBreaker::get_config(env)?; - Ok(state.half_open_requests < config.half_open_max_requests) + if config.half_open_quota.calls_per_minute > 0 { + // Try to admit a call under quota-based scheduling + let admitted = CircuitBreaker::half_open_admit(env, &config)?; + Ok(admitted) + } else { + Ok(state.half_open_requests < config.half_open_max_requests) + } } } } diff --git a/contracts/predictify-hybrid/src/circuit_breaker_tests.rs b/contracts/predictify-hybrid/src/circuit_breaker_tests.rs index 31df5fee..43f4e6c8 100644 --- a/contracts/predictify-hybrid/src/circuit_breaker_tests.rs +++ b/contracts/predictify-hybrid/src/circuit_breaker_tests.rs @@ -651,4 +651,96 @@ mod circuit_breaker_tests { assert!(events.len() >= 2); // At least pause and recovery events }); } + + #[test] + fn test_half_open_quota_allows_calls_then_closes() { + let env = Env::default(); + env.mock_all_auths(); + let contract_id = env.register(crate::PredictifyHybrid, ()); + + // Configure quota (3 calls per 60s) + let config = CircuitBreakerConfig { + max_error_rate: 10, + max_latency_ms: 1000, + min_liquidity: 100_000_000, + failure_threshold: 3, + recovery_timeout: 60, + half_open_max_requests: 2, + auto_recovery_enabled: true, + half_open_quota: HalfOpenQuota { calls_per_minute: 3, evaluation_window_s: 60 }, + }; + + env.as_contract(&contract_id, || { + env.storage().instance().set(&Symbol::new(&env, "circuit_breaker_config"), &config); + let state = CircuitBreakerState { + state: BreakerState::HalfOpen, + failure_count: 0, + last_failure_time: 0, + last_success_time: env.ledger().timestamp(), + opened_time: 0, + half_open_requests: 0, + total_requests: 0, + error_count: 0, + pause_scope: PauseScope::BettingOnly, + allow_withdrawals: false, + }; + env.storage().instance().set(&Symbol::new(&env, "circuit_breaker_state"), &state); + }); + + // Perform 3 successful operations; they should be admitted and after 3 completed the breaker closes + for _ in 0..3 { + let res = env.as_contract(&contract_id, || { + CircuitBreakerUtils::with_circuit_breaker(&env, || Ok(())) + }); + assert!(res.is_ok()); + } + + // After completing quota, circuit should be Closed + let status = env.as_contract(&contract_id, || CircuitBreaker::get_state(&env)).unwrap(); + assert_eq!(status.state, BreakerState::Closed); + } + + #[test] + fn test_half_open_quota_failure_reopens_immediately() { + let env = Env::default(); + env.mock_all_auths(); + let contract_id = env.register(crate::PredictifyHybrid, ()); + + let config = CircuitBreakerConfig { + max_error_rate: 10, + max_latency_ms: 1000, + min_liquidity: 100_000_000, + failure_threshold: 3, + recovery_timeout: 60, + half_open_max_requests: 2, + auto_recovery_enabled: true, + half_open_quota: HalfOpenQuota { calls_per_minute: 3, evaluation_window_s: 60 }, + }; + + env.as_contract(&contract_id, || { + env.storage().instance().set(&Symbol::new(&env, "circuit_breaker_config"), &config); + let state = CircuitBreakerState { + state: BreakerState::HalfOpen, + failure_count: 0, + last_failure_time: 0, + last_success_time: env.ledger().timestamp(), + opened_time: 0, + half_open_requests: 0, + total_requests: 0, + error_count: 0, + pause_scope: PauseScope::BettingOnly, + allow_withdrawals: false, + }; + env.storage().instance().set(&Symbol::new(&env, "circuit_breaker_state"), &state); + }); + + // First admitted call fails => breaker reopens immediately + let res = env.as_contract(&contract_id, || { + CircuitBreakerUtils::with_circuit_breaker(&env, || Err(crate::errors::Error::CBError)) + }); + assert!(res.is_err()); + + let status = env.as_contract(&contract_id, || CircuitBreaker::get_state(&env)).unwrap(); + assert_eq!(status.state, BreakerState::Open); + } }