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 diff --git a/contracts/predict-iq/src/modules/migration.rs b/contracts/predict-iq/src/modules/migration.rs index a8c90aef..afbd2605 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`). @@ -113,18 +123,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. @@ -191,9 +216,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(()) } @@ -252,7 +278,6 @@ mod tests { }); assert_eq!(result, Err(ErrorCode::MigrationValidationError)); - // Both keys must be genuinely restored to their pre-migration values. let restored_admin: Address = env .storage() @@ -260,13 +285,49 @@ mod tests { .get(&ConfigKey::Admin) .expect("admin should be restored"); assert_eq!(restored_admin, admin); + assert_eq!( + env.storage() + .persistent() + .get::>(&ConfigKey::GuardianSet), + Some(original_guardians) + ); + } - let restored_guardians: Vec = env - .storage() + #[test] + fn test_migration_history_preserved_across_sequential_migrations() { + use soroban_sdk::Address; + + 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) - .expect("guardian set should be restored"); - 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); // The backup snapshot must be cleaned up on the rollback path. let backup_key = format!("migration:backup:v{}", 1); 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; diff --git a/contracts/predict-iq/src/modules/resolution.rs b/contracts/predict-iq/src/modules/resolution.rs index 9c83604f..24bfdc67 100644 --- a/contracts/predict-iq/src/modules/resolution.rs +++ b/contracts/predict-iq/src/modules/resolution.rs @@ -274,6 +274,23 @@ fn calculate_voting_outcome(e: &Env, market: &crate::types::Market) -> Result Result<(u32, u64), ErrorCode> { + let market = markets::get_market(e, market_id).ok_or(ErrorCode::MarketNotFound)?; + + 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)) +} + #[cfg(test)] mod tests { use super::*; 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))); + } }