From b872cd5430bd37d742d217084ac56b27db536155 Mon Sep 17 00:00:00 2001 From: Anatolii Kurotych Date: Thu, 27 Mar 2025 16:48:33 +0200 Subject: [PATCH 1/2] Reproduce panic tests --- coverage_point_calculator/src/location.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/coverage_point_calculator/src/location.rs b/coverage_point_calculator/src/location.rs index 507449eb1..c7dbcd5f0 100644 --- a/coverage_point_calculator/src/location.rs +++ b/coverage_point_calculator/src/location.rs @@ -58,6 +58,16 @@ mod tests { use super::*; + #[test] + fn average_distance_should_not_panic() { + average_distance(&[]); + } + + #[test] + fn multiplier_should_not_panic() { + multiplier(&[]); + } + #[test] fn distance_does_not_effect_multiplier() { let trust_scores = vec![ From 8786b924987d7683a85191aabb7bd8cc00b689b6 Mon Sep 17 00:00:00 2001 From: Anatolii Kurotych Date: Thu, 27 Mar 2025 17:36:46 +0200 Subject: [PATCH 2/2] Don't panic, return error --- coverage_point_calculator/src/lib.rs | 25 ++++++++++++++--------- coverage_point_calculator/src/location.rs | 24 +++++++++++++--------- mobile_verifier/src/reward_shares.rs | 14 +++++++++---- 3 files changed, 39 insertions(+), 24 deletions(-) diff --git a/coverage_point_calculator/src/lib.rs b/coverage_point_calculator/src/lib.rs index ddf5d60f5..a897bcaf9 100644 --- a/coverage_point_calculator/src/lib.rs +++ b/coverage_point_calculator/src/lib.rs @@ -94,6 +94,8 @@ pub type Result = std::result::Result; pub enum Error { #[error("signal level {0:?} not allowed for {1:?}")] InvalidSignalLevel(SignalLevel, RadioType), + #[error("Input array is empty")] + ArrayIsEmpty, } /// Output of calculating coverage points for a Radio. @@ -167,13 +169,13 @@ impl CoveragePoints { ranked_coverage: Vec, oracle_boost_status: OracleBoostingStatus, ) -> Result { - let location_trust_multiplier = location::multiplier(&location_trust_scores); + let location_trust_multiplier = location::multiplier(&location_trust_scores)?; let sp_boost_eligibility = SpBoostedHexStatus::new( location_trust_multiplier, &location_trust_scores, service_provider_boosted_reward_eligibility, - ); + )?; let covered_hexes = hexes::clean_covered_hexes( radio_type, @@ -269,25 +271,27 @@ impl SpBoostedHexStatus { location_trust_multiplier: Decimal, location_trust_scores: &[LocationTrust], service_provider_boosted_reward_eligibility: SPBoostedRewardEligibility, - ) -> Self { + ) -> Result { match service_provider_boosted_reward_eligibility { // hip-84: if radio has not met minimum data and subscriber thresholds, no boosting - SPBoostedRewardEligibility::RadioThresholdNotMet => Self::RadioThresholdNotMet, + SPBoostedRewardEligibility::RadioThresholdNotMet => Ok(Self::RadioThresholdNotMet), // hip-140: radio must have enough unique connections - SPBoostedRewardEligibility::NotEnoughConnections => Self::NotEnoughConnections, + SPBoostedRewardEligibility::NotEnoughConnections => Ok(Self::NotEnoughConnections), SPBoostedRewardEligibility::Eligible => { // hip-93: if radio is wifi & location_trust score multiplier < 0.75, no boosting if location_trust_multiplier < MIN_WIFI_TRUST_MULTIPLIER { - return Self::WifiLocationScoreBelowThreshold(location_trust_multiplier); + return Ok(Self::WifiLocationScoreBelowThreshold( + location_trust_multiplier, + )); } // hip-119: if the average distance to asserted is beyond 50m, no boosting - let average_distance = location::average_distance(location_trust_scores); + let average_distance = location::average_distance(location_trust_scores)?; if average_distance > MAX_AVERAGE_DISTANCE { - return Self::AverageAssertedDistanceOverLimit(average_distance); + return Ok(Self::AverageAssertedDistanceOverLimit(average_distance)); } - Self::Eligible + Ok(Self::Eligible) } } } @@ -871,10 +875,11 @@ mod tests { let wifi_bad_trust_score = |sp_status: SPBoostedRewardEligibility| { SpBoostedHexStatus::new( - location::multiplier(&bad_location), + location::multiplier(&bad_location).unwrap(), &bad_location, sp_status, ) + .unwrap() }; assert_eq!( diff --git a/coverage_point_calculator/src/location.rs b/coverage_point_calculator/src/location.rs index c7dbcd5f0..cf8426e71 100644 --- a/coverage_point_calculator/src/location.rs +++ b/coverage_point_calculator/src/location.rs @@ -1,7 +1,7 @@ use rust_decimal::Decimal; use rust_decimal_macros::dec; -use crate::RadioType; +use crate::{Error, RadioType, Result}; type Meters = u32; @@ -34,23 +34,27 @@ pub fn asserted_distance_to_trust_multiplier( } } -pub(crate) fn average_distance(trust_scores: &[LocationTrust]) -> Decimal { - // FIXME-K: if count = 0, division by zero happens +pub(crate) fn average_distance(trust_scores: &[LocationTrust]) -> Result { + if trust_scores.is_empty() { + return Err(Error::ArrayIsEmpty); + } let count = Decimal::from(trust_scores.len()); let sum: Decimal = trust_scores .iter() .map(|l| Decimal::from(l.meters_to_asserted)) .sum(); - sum / count + Ok(sum / count) } -pub fn multiplier(trust_scores: &[LocationTrust]) -> Decimal { - // FIXME-K: if count = 0, division by zero happens +pub fn multiplier(trust_scores: &[LocationTrust]) -> Result { + if trust_scores.is_empty() { + return Err(Error::ArrayIsEmpty); + } let count = Decimal::from(trust_scores.len()); let scores: Decimal = trust_scores.iter().map(|l| l.trust_score).sum(); - scores / count + Ok(scores / count) } #[cfg(test)] @@ -60,12 +64,12 @@ mod tests { #[test] fn average_distance_should_not_panic() { - average_distance(&[]); + assert!(average_distance(&[]).is_err()); } #[test] fn multiplier_should_not_panic() { - multiplier(&[]); + assert!(multiplier(&[]).is_err()); } #[test] @@ -97,6 +101,6 @@ mod tests { }, ]; - assert_eq!(dec!(0.5), multiplier(&trust_scores)); + assert_eq!(dec!(0.5), multiplier(&trust_scores).unwrap()); } } diff --git a/mobile_verifier/src/reward_shares.rs b/mobile_verifier/src/reward_shares.rs index 755fde0b1..a256cb988 100644 --- a/mobile_verifier/src/reward_shares.rs +++ b/mobile_verifier/src/reward_shares.rs @@ -757,11 +757,17 @@ fn eligible_for_coverage_map( return false; } - let multiplier = coverage_point_calculator::location::multiplier(trust_scores); - if multiplier <= dec!(0.0) { - return false; + match coverage_point_calculator::location::multiplier(trust_scores) { + Ok(multiplier) => { + if multiplier <= dec!(0.0) { + return false; + } + } + Err(e) => { + tracing::error!(?e, "multiplier calculation failed"); + return false; + } } - true }