From 51b77db26ff3c513f99dea1b109fe39091970795 Mon Sep 17 00:00:00 2001 From: whitezaddy Date: Wed, 29 Apr 2026 07:12:21 +0100 Subject: [PATCH] fix: re-apply remittance split percentage validation --- reporting/src/lib.rs | 41 +++++++++++++++++++++++----------- reporting/src/tests.rs | 41 ++++++++++++++++++++++------------ reporting/src/tests_updated.rs | 18 +++++++-------- 3 files changed, 64 insertions(+), 36 deletions(-) diff --git a/reporting/src/lib.rs b/reporting/src/lib.rs index ae27f819..beae4e7c 100644 --- a/reporting/src/lib.rs +++ b/reporting/src/lib.rs @@ -203,6 +203,8 @@ pub enum ReportingError { InvalidDependencyAddressConfiguration = 6, /// Report period range is invalid (`period_start` is greater than `period_end`). InvalidPeriod = 7, + /// Invalid percentage split summing to > 10000 or != 10000 + InvalidPercentageSplit = 8, } #[contracttype] @@ -760,12 +762,12 @@ impl ReportingContract { ) -> Result { Self::validate_period(period_start, period_end)?; user.require_auth(); - Ok(Self::get_remittance_summary_internal( + Self::get_remittance_summary_internal( &env, total_amount, period_start, period_end, - )) + ) } fn get_remittance_summary_internal( @@ -773,20 +775,20 @@ impl ReportingContract { total_amount: i128, period_start: u64, period_end: u64, - ) -> RemittanceSummary { + ) -> Result { let addresses: Option = env.storage().instance().get(&symbol_short!("ADDRS")); let addresses = match addresses { Some(a) => a, - None => return RemittanceSummary { + None => return Ok(RemittanceSummary { total_received: total_amount, total_allocated: total_amount, category_breakdown: Vec::new(env), period_start, period_end, data_availability: DataAvailability::Missing, - }, + }), }; let split_client = RemittanceSplitClient::new(env, &addresses.remittance_split); let mut availability = DataAvailability::Complete; @@ -799,13 +801,26 @@ impl ReportingContract { } }; - let split_amounts = match split_client.try_calculate_split(&total_amount) { - Ok(Ok(res)) => res, - _ => { - availability = DataAvailability::Partial; - Vec::new(env) + let mut split_amounts = Vec::new(env); + if availability == DataAvailability::Complete { + let mut sum = 0u32; + // Percentages are stored as basis points (bps), where 10000 = 100.00% + for i in 0..split_percentages.len() { + let p = split_percentages.get(i).unwrap_or(0); + sum = sum.checked_add(p).ok_or(ReportingError::InvalidPercentageSplit)?; + if sum > 10000 { + return Err(ReportingError::InvalidPercentageSplit); + } + + // Formula used is (amount * percentage) / 10000 + let amount = total_amount.checked_mul(p as i128).unwrap_or(0) / 10000; + split_amounts.push_back(amount); } - }; + + if sum != 10000 { + return Err(ReportingError::InvalidPercentageSplit); + } + } let mut breakdown = Vec::new(env); let categories = [ @@ -830,7 +845,7 @@ impl ReportingContract { period_start, period_end, data_availability: availability, - } + }) } /// Generate savings progress report. @@ -1150,7 +1165,7 @@ impl ReportingContract { let health_score = Self::calculate_health_score_internal(&env, user.clone(), total_remittance); let remittance_summary = - Self::get_remittance_summary_internal(&env, total_remittance, period_start, period_end); + Self::get_remittance_summary_internal(&env, total_remittance, period_start, period_end)?; let savings_report = Self::get_savings_report_internal(&env, user.clone(), period_start, period_end); let bill_compliance = diff --git a/reporting/src/tests.rs b/reporting/src/tests.rs index 3e77d13b..22f8171a 100644 --- a/reporting/src/tests.rs +++ b/reporting/src/tests.rs @@ -28,19 +28,19 @@ mod remittance_split { impl RemittanceSplit { pub fn get_split(env: &Env) -> Vec { let mut split = Vec::new(env); - split.push_back(50); - split.push_back(30); - split.push_back(15); - split.push_back(5); + split.push_back(5000); + split.push_back(3000); + split.push_back(1500); + split.push_back(500); split } pub fn calculate_split(env: Env, total_amount: i128) -> Vec { let mut amounts = Vec::new(&env); - amounts.push_back(total_amount * 50 / 100); - amounts.push_back(total_amount * 30 / 100); - amounts.push_back(total_amount * 15 / 100); - amounts.push_back(total_amount * 5 / 100); + amounts.push_back(total_amount * 5000 / 10000); + amounts.push_back(total_amount * 3000 / 10000); + amounts.push_back(total_amount * 1500 / 10000); + amounts.push_back(total_amount * 500 / 10000); amounts } } @@ -501,7 +501,8 @@ fn test_get_remittance_summary() { let period_start = 1704067200u64; let period_end = 1706745600u64; - let result = client.try_get_remittance_summary(&user, &total_amount, &period_start, &period_end); + let result = + client.try_get_remittance_summary(&user, &total_amount, &period_start, &period_end); assert!(result.is_ok()); let summary = result.unwrap(); @@ -516,7 +517,7 @@ fn test_get_remittance_summary() { let spending = summary.category_breakdown.get(0).unwrap(); assert_eq!(spending.category, Category::Spending); assert_eq!(spending.amount, 5000); - assert_eq!(spending.percentage, 50); + assert_eq!(spending.percentage, 5000); } #[test] @@ -804,7 +805,12 @@ fn test_get_financial_health_report() { let period_start = 1704067200u64; let period_end = 1706745600u64; - let result = client.try_get_financial_health_report(&user, &total_remittance, &period_start, &period_end); + let result = client.try_get_financial_health_report( + &user, + &total_remittance, + &period_start, + &period_end, + ); assert!(result.is_ok()); let report = result.unwrap(); @@ -872,7 +878,12 @@ fn test_store_and_retrieve_report() { let period_start = 1704067200u64; let period_end = 1706745600u64; - let result = client.try_get_financial_health_report(&user, &total_remittance, &period_start, &period_end); + let result = client.try_get_financial_health_report( + &user, + &total_remittance, + &period_start, + &period_end, + ); assert!(result.is_ok()); let report = result.unwrap(); @@ -915,7 +926,8 @@ fn test_archive_old_reports() { &family_wallet, ); - let result = client.try_get_financial_health_report(&user, &10000i128, &1704067200u64, &1706745600u64); + let result = + client.try_get_financial_health_report(&user, &10000i128, &1704067200u64, &1706745600u64); assert!(result.is_ok()); let report = result.unwrap(); @@ -956,7 +968,8 @@ fn test_cleanup_old_reports() { &family_wallet, ); - let result = client.try_get_financial_health_report(&user, &10000i128, &1704067200u64, &1706745600u64); + let result = + client.try_get_financial_health_report(&user, &10000i128, &1704067200u64, &1706745600u64); assert!(result.is_ok()); let report = result.unwrap(); client.store_report(&user, &report, &202401); diff --git a/reporting/src/tests_updated.rs b/reporting/src/tests_updated.rs index 7d2f710a..438dd1ab 100644 --- a/reporting/src/tests_updated.rs +++ b/reporting/src/tests_updated.rs @@ -12,19 +12,19 @@ mod remittance_split { impl RemittanceSplit { pub fn get_split(env: &Env) -> Vec { let mut split = Vec::new(env); - split.push_back(50); - split.push_back(30); - split.push_back(15); - split.push_back(5); + split.push_back(5000); + split.push_back(3000); + split.push_back(1500); + split.push_back(500); split } pub fn calculate_split(env: Env, total_amount: i128) -> Vec { let mut amounts = Vec::new(&env); - amounts.push_back(total_amount * 50 / 100); - amounts.push_back(total_amount * 30 / 100); - amounts.push_back(total_amount * 15 / 100); - amounts.push_back(total_amount * 5 / 100); + amounts.push_back(total_amount * 5000 / 10000); + amounts.push_back(total_amount * 3000 / 10000); + amounts.push_back(total_amount * 1500 / 10000); + amounts.push_back(total_amount * 500 / 10000); amounts } } @@ -457,7 +457,7 @@ fn test_get_remittance_summary() { // Check category breakdown let spending = summary.category_breakdown.get(0).unwrap(); assert_eq!(spending.amount, 5000); - assert_eq!(spending.percentage, 50); + assert_eq!(spending.percentage, 5000); } #[test]