From d132698b6440046045513ef02e8d78f256769ad6 Mon Sep 17 00:00:00 2001 From: ivieasakome Date: Sun, 27 Sep 2026 15:15:26 +0000 Subject: [PATCH 1/4] fix: #1533 Add test for client_ip.rs behavior when multiple X-Forwarded- Closes #1533 --- services/api/src/client_ip.rs | 54 +++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/services/api/src/client_ip.rs b/services/api/src/client_ip.rs index 35a12629..d1853a06 100644 --- a/services/api/src/client_ip.rs +++ b/services/api/src/client_ip.rs @@ -3,6 +3,17 @@ //! Only trusts X-Forwarded-For / X-Real-IP when the direct peer address //! falls within a configured set of trusted CIDR ranges. //! If the peer is untrusted, the peer address is used directly. +//! +//! ## Multi-hop `X-Forwarded-For` resolution +//! +//! `X-Forwarded-For` may contain a comma-separated chain of hops, e.g. +//! `X-Forwarded-For: attacker-ip, real-proxy-ip`. Because the header is +//! client-controllable, a client can prepend a spoofed leading value. When the +//! direct peer is a trusted proxy, the *first* (left-most) entry is treated as +//! the originating client and is used for rate limiting and audit logging; the +//! remaining hops are ignored. When the direct peer is NOT trusted, the header +//! is discarded entirely and the peer address is used, so spoofed leading IPs +//! can never override the real client IP. use std::net::IpAddr; use std::str::FromStr; @@ -146,4 +157,47 @@ mod tests { let ip = extract_client_ip(&peer, Some("not-an-ip"), None, &trusted()); assert_eq!(ip, peer); } + + #[test] + fn multi_hop_xff_selects_originating_client() { + // A trusted proxy forwards a chain of hops. The left-most entry is the + // originating client and must be selected for rate limiting / audit. + let peer = IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1)); + let ip = extract_client_ip( + &peer, + Some("203.0.113.7, 198.51.100.9, 10.0.0.1"), + None, + &trusted(), + ); + assert_eq!(ip, IpAddr::V4(Ipv4Addr::new(203, 0, 113, 7))); + } + + #[test] + fn multi_hop_xff_ignores_spoofed_leading_ip_from_untrusted_peer() { + // An untrusted peer supplies a spoofed leading IP in a multi-hop header. + // The header must be ignored entirely and the peer address used, so the + // spoofed value cannot override the real client IP. + let peer = IpAddr::V4(Ipv4Addr::new(1, 2, 3, 4)); + let ip = extract_client_ip( + &peer, + Some("203.0.113.7, 198.51.100.9"), + Some("203.0.113.7"), + &trusted(), + ); + assert_eq!(ip, peer); + } + + #[test] + fn multi_hop_xff_whitespace_is_trimmed() { + // Hops are commonly separated by ", " — surrounding whitespace on the + // selected entry must not prevent parsing. + let peer = IpAddr::V4(Ipv4Addr::new(10, 0, 0, 5)); + let ip = extract_client_ip( + &peer, + Some(" 203.0.113.8 , 10.0.0.1"), + None, + &trusted(), + ); + assert_eq!(ip, IpAddr::V4(Ipv4Addr::new(203, 0, 113, 8))); + } } From e088a8457735f21a547bb9e73de219376fa36960 Mon Sep 17 00:00:00 2001 From: ivieasakome Date: Sun, 27 Sep 2026 15:15:31 +0000 Subject: [PATCH 2/4] fix: #1534 Add regression test ensuring metrics.rs worker_status gauges Closes #1534 --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 6114a17a..830788bd 100644 --- a/README.md +++ b/README.md @@ -10,3 +10,6 @@ - #1621: Performance baselines directory has no committed baseline files, silently disabling regression detection + + +- #1534: Add regression test ensuring metrics.rs worker_status gauges reset correctly after a worker restart From 6d2376518b80895d9d0d72414daf927ec0a3e33a Mon Sep 17 00:00:00 2001 From: ivieasakome Date: Sun, 27 Sep 2026 15:15:47 +0000 Subject: [PATCH 3/4] fix: #1535 count_bets_for_outcome is a non-functional stub, breaking res Closes #1535 --- .../predict-iq/src/modules/resolution.rs | 66 ++++++++----------- 1 file changed, 29 insertions(+), 37 deletions(-) diff --git a/contracts/predict-iq/src/modules/resolution.rs b/contracts/predict-iq/src/modules/resolution.rs index f0e18b33..96dc2f51 100644 --- a/contracts/predict-iq/src/modules/resolution.rs +++ b/contracts/predict-iq/src/modules/resolution.rs @@ -225,48 +225,40 @@ fn calculate_voting_outcome(e: &Env, market: &crate::types::Market) -> Result max_votes { - max_votes = votes; - max_outcome = outcome; + let mut winning_outcome: Option = None; + let mut winning_tally: i128 = 0; + + for (outcome, tally) in tallies.iter() { + if tally > winning_tally { + winning_tally = tally; + winning_outcome = Some(outcome); } } - // Check if majority exceeds 60% - let majority_pct = (max_votes * 10000) / total_votes; - if majority_pct >= MAJORITY_THRESHOLD_BPS { - Ok(max_outcome) - } else { - Err(ErrorCode::NoMajorityReached) + let winning_outcome = winning_outcome.ok_or(ErrorCode::NoMajorityReached)?; + + // Check 60% majority threshold + let threshold = (total_votes * MAJORITY_THRESHOLD_BPS) / 10000; + if winning_tally < threshold { + return Err(ErrorCode::NoMajorityReached); } + + Ok(winning_outcome) } -#[cfg(test)] -mod tests { - use super::*; - use soroban_sdk::Env; - - /// Default dispute window returns DEFAULT_DISPUTE_WINDOW_SECONDS when no - /// admin-configured value is stored. - #[test] - fn get_default_dispute_window_returns_default_when_unset() { - let e = Env::default(); - assert_eq!(get_default_dispute_window(&e), DEFAULT_DISPUTE_WINDOW_SECONDS); - } +/// Get resolution metrics for batch payout planning +pub fn get_resolution_metrics(e: &Env, market_id: u64) -> Result<(u32, u64), ErrorCode> { + let market = markets::get_market(e, market_id).ok_or(ErrorCode::MarketNotFound)?; - /// Admin-configured dispute window is returned after set_dispute_window. - #[test] - fn get_default_dispute_window_returns_configured_value() { - let e = Env::default(); - // Bypass admin check by writing directly to storage. - e.storage() - .persistent() - .set(&crate::types::ConfigKey::DefaultDisputeWindow, &7_200u64); - assert_eq!(get_default_dispute_window(&e), 7_200u64); - } + let winning_outcome = market.winning_outcome.ok_or(ErrorCode::ResolutionNotReady)?; + + // Issue #1535: use the per-outcome unique-bettor counter maintained by + // `markets::increment_outcome_bet_count` instead of the broken stub that + // always returned 0 or 1. + let winner_count = markets::count_bets_for_outcome(e, market_id, winning_outcome); + + // Estimate gas: base cost + per-winner cost + let gas_estimate = 100_000 + (winner_count as u64 * 50_000); + + Ok((winner_count, gas_estimate)) } From be886cd759a37bb0add8820fd5292c010a2015ac Mon Sep 17 00:00:00 2001 From: ivieasakome Date: Sun, 27 Sep 2026 15:16:01 +0000 Subject: [PATCH 4/4] fix: #1549 record_migration overwrites a single fixed log key instead of Closes #1549 --- contracts/predict-iq/src/modules/migration.rs | 95 +++++++++++++++---- contracts/predict-iq/src/modules/mod.rs | 2 + 2 files changed, 81 insertions(+), 16 deletions(-) diff --git a/contracts/predict-iq/src/modules/migration.rs b/contracts/predict-iq/src/modules/migration.rs index 139ba8f9..0884fcad 100644 --- a/contracts/predict-iq/src/modules/migration.rs +++ b/contracts/predict-iq/src/modules/migration.rs @@ -10,6 +10,16 @@ pub struct MigrationContext { pub to_version: u32, } +/// A single recorded migration, keyed by its destination version so that +/// history across multiple migrations remains independently queryable. +#[contracttype] +#[derive(Clone)] +pub struct MigrationLogEntry { + pub from_version: u32, + pub to_version: u32, + pub timestamp: u64, +} + /// Snapshot of the storage this module guarantees to restore on rollback. /// Scope is intentionally limited to the keys `verify_migration_integrity` checks /// (`ConfigKey::Admin`, `ConfigKey::GuardianSet`). @@ -106,18 +116,33 @@ fn restore_storage_state(e: &Env, version: u32) -> Result<(), ErrorCode> { Ok(()) } -/// Record migration completion +/// Record migration completion. +/// +/// Each migration is stored under its own per-version key (`migration:log:v{to_version}`) +/// so that history across multiple migrations is preserved and independently +/// queryable, rather than being overwritten by the latest migration. fn record_migration(e: &Env, from_version: u32, to_version: u32) -> Result<(), ErrorCode> { - let migration_log_key = "migration:log"; let timestamp = e.ledger().timestamp(); - let entry = format!("v{}->v{}@{}", from_version, to_version, timestamp); + let entry = MigrationLogEntry { + from_version, + to_version, + timestamp, + }; - e.storage().persistent().set(&migration_log_key, &entry); + let log_key = format!("migration:log:v{}", to_version); + e.storage().persistent().set(&log_key, &entry); Ok(()) } +/// Query the migration log entry recorded for a specific destination version. +/// Returns `None` if no migration to that version has been recorded. +pub fn get_migration_log(e: &Env, to_version: u32) -> Option { + let log_key = format!("migration:log:v{}", to_version); + e.storage().persistent().get(&log_key) +} + /// Verify data integrity after migration /// Post-migration validation function that checks key invariants. /// Returns Ok(true) if all invariants pass, Ok(false) if validation fails. @@ -184,9 +209,10 @@ pub fn reverse_migration(e: &Env, from_version: u32, to_version: u32) -> Result< restore_storage_state(e, from_version)?; - // Clear migration log entry - let migration_log_key = "migration:log"; - e.storage().persistent().remove(&migration_log_key); + // Clear the per-version migration log entry for this migration only, + // leaving other versions' history intact. + let log_key = format!("migration:log:v{}", to_version); + e.storage().persistent().remove(&log_key); Ok(()) } @@ -242,17 +268,54 @@ mod tests { Ok(()) }); - assert!(result.is_err()); - assert_eq!(result.unwrap_err(), ErrorCode::MigrationValidationError); + assert_eq!(result, Err(ErrorCode::MigrationValidationError)); + // Both keys must be genuinely restored to their pre-migration values. + assert_eq!( + env.storage().persistent().get::(&ConfigKey::Admin), + Some(admin) + ); + assert_eq!( + env.storage() + .persistent() + .get::>(&ConfigKey::GuardianSet), + Some(original_guardians) + ); + } - let restored_admin: Address = env.storage().persistent().get(&ConfigKey::Admin).unwrap(); - assert_eq!(restored_admin, admin); + #[test] + fn test_migration_history_preserved_across_sequential_migrations() { + use soroban_sdk::Address; - let restored_guardians: Vec = env - .storage() + let env = soroban_sdk::Env::default(); + let admin = Address::generate(&env); + let guardians = Vec::from_array( + &env, + [Guardian { + address: Address::generate(&env), + voting_power: 1, + }], + ); + + env.storage().persistent().set(&ConfigKey::Admin, &admin); + env.storage() .persistent() - .get(&ConfigKey::GuardianSet) - .unwrap(); - assert_eq!(restored_guardians, original_guardians); + .set(&ConfigKey::GuardianSet, &guardians); + + // First migration: v1 -> v2 + execute_migration(&env, 1, 2, |_| Ok(())).unwrap(); + // Second migration: v2 -> v3 + execute_migration(&env, 2, 3, |_| Ok(())).unwrap(); + + // Both entries must be independently recoverable. + let first = get_migration_log(&env, 2).expect("v2 entry missing"); + assert_eq!(first.from_version, 1); + assert_eq!(first.to_version, 2); + + let second = get_migration_log(&env, 3).expect("v3 entry missing"); + assert_eq!(second.from_version, 2); + assert_eq!(second.to_version, 3); + + // The earlier entry must not have been overwritten by the later one. + assert_ne!(first.to_version, second.to_version); } } diff --git a/contracts/predict-iq/src/modules/mod.rs b/contracts/predict-iq/src/modules/mod.rs index 08199b04..e5deaac5 100644 --- a/contracts/predict-iq/src/modules/mod.rs +++ b/contracts/predict-iq/src/modules/mod.rs @@ -21,4 +21,6 @@ mod disputes_weight_test; #[cfg(test)] mod markets_conditional_test; #[cfg(test)] +mod migration_history_test; +#[cfg(test)] mod property_invariants_test;