diff --git a/gas_results.json b/gas_results.json index abe3d3da..3997db87 100644 --- a/gas_results.json +++ b/gas_results.json @@ -1,15 +1,3 @@ [ - {"contract":"bill_payments","method":"get_total_unpaid","scenario":"100_bills_50_cancelled","cpu":1077221,"mem":235460}, - {"contract":"savings_goals","method":"get_all_goals","scenario":"100_goals_single_owner","cpu":2661552,"mem":480721}, - {"contract":"insurance","method":"get_total_monthly_premium","scenario":"100_active_policies","cpu":2373104,"mem":427575}, - {"contract":"family_wallet","method":"configure_multisig","scenario":"9_signers_threshold_all","cpu":342677,"mem":69106}, - {"contract":"remittance_split","method":"distribute_usdc","scenario":"4_recipients_all_nonzero","cpu":654751,"mem":86208}, - {"contract":"remittance_split","method":"create_remittance_schedule","scenario":"single_recurring_schedule","cpu":145230,"mem":28456}, - {"contract":"remittance_split","method":"create_remittance_schedule","scenario":"11th_schedule_with_existing","cpu":167890,"mem":32145}, - {"contract":"remittance_split","method":"modify_remittance_schedule","scenario":"single_schedule_modification","cpu":134567,"mem":26789}, - {"contract":"remittance_split","method":"cancel_remittance_schedule","scenario":"single_schedule_cancellation","cpu":123456,"mem":24567}, - {"contract":"remittance_split","method":"get_remittance_schedules","scenario":"empty_schedules","cpu":45678,"mem":12345}, - {"contract":"remittance_split","method":"get_remittance_schedules","scenario":"5_schedules_with_isolation","cpu":234567,"mem":45678}, - {"contract":"remittance_split","method":"get_remittance_schedule","scenario":"single_schedule_lookup","cpu":67890,"mem":15432}, - {"contract":"remittance_split","method":"get_remittance_schedules","scenario":"50_schedules_worst_case","cpu":1234567,"mem":234567} + {"contract":"family_wallet","method":"configure_multisig","scenario":"9_signers_threshold_all","cpu":343463,"mem":69170} ] diff --git a/reporting/README.md b/reporting/README.md index e2608efd..72638ffe 100644 --- a/reporting/README.md +++ b/reporting/README.md @@ -36,4 +36,3 @@ Returns an empty Vec when fewer than two points are supplied. **Determinism guarantee**: identical `history` input always produces identical output regardless of call order, ledger state, or caller identity. -## Running Tests diff --git a/reporting/src/lib.rs b/reporting/src/lib.rs index 51ff9ae2..a5305a70 100644 --- a/reporting/src/lib.rs +++ b/reporting/src/lib.rs @@ -138,6 +138,7 @@ pub enum ReportingError { NotInitialized = 2, Unauthorized = 3, AddressesNotConfigured = 4, + NotAdminProposed = 5, } impl From for soroban_sdk::Error { @@ -159,6 +160,10 @@ impl From for soroban_sdk::Error { soroban_sdk::xdr::ScErrorType::Contract, soroban_sdk::xdr::ScErrorCode::MissingValue, )), + ReportingError::NotAdminProposed => soroban_sdk::Error::from(( + soroban_sdk::xdr::ScErrorType::Contract, + soroban_sdk::xdr::ScErrorCode::InvalidAction, + )), } } } @@ -301,25 +306,25 @@ pub struct ReportingContract; impl ReportingContract { /// Initialize the reporting contract with an admin address. /// + /// This function must be called only once. The provided admin address will + /// have full control over contract configuration and maintenance. + /// /// # Arguments - /// * `admin` - Address of the contract administrator (must authorize) + /// * `admin` - Address of the initial contract administrator /// /// # Returns /// `Ok(())` on successful initialization /// /// # Errors /// * `AlreadyInitialized` - If the contract has already been initialized - /// - /// # Panics - /// * If `admin` does not authorize the transaction pub fn init(env: Env, admin: Address) -> Result<(), ReportingError> { - admin.require_auth(); - let existing: Option
= env.storage().instance().get(&symbol_short!("ADMIN")); if existing.is_some() { return Err(ReportingError::AlreadyInitialized); } + admin.require_auth(); + Self::extend_instance_ttl(&env); env.storage() .instance() @@ -328,6 +333,76 @@ impl ReportingContract { Ok(()) } + /// Propose a new administrator for the contract. + /// + /// This is the first step of a two-step admin rotation process. The proposed + /// admin must then call `accept_admin_rotation` to complete the transfer. + /// + /// # Arguments + /// * `caller` - Current administrator (must authorize) + /// * `new_admin` - Address of the proposed successor + /// + /// # Errors + /// * `NotInitialized` - If contract has not been initialized + /// * `Unauthorized` - If caller is not the current admin + pub fn propose_new_admin( + env: Env, + caller: Address, + new_admin: Address, + ) -> Result<(), ReportingError> { + caller.require_auth(); + + let admin: Address = env + .storage() + .instance() + .get(&symbol_short!("ADMIN")) + .ok_or(ReportingError::NotInitialized)?; + + if caller != admin { + return Err(ReportingError::Unauthorized); + } + + Self::extend_instance_ttl(&env); + env.storage() + .instance() + .set(&symbol_short!("PEND_ADM"), &new_admin); + + Ok(()) + } + + /// Accept the role of contract administrator. + /// + /// This is the second step of a two-step admin rotation process. Only the + /// address currently proposed via `propose_new_admin` can call this. + /// + /// # Arguments + /// * `caller` - The proposed administrator (must authorize) + /// + /// # Errors + /// * `NotAdminProposed` - If no admin rotation is currently in progress + /// * `Unauthorized` - If caller is not the proposed admin + pub fn accept_admin_rotation(env: Env, caller: Address) -> Result<(), ReportingError> { + caller.require_auth(); + + let pending_admin: Address = env + .storage() + .instance() + .get(&symbol_short!("PEND_ADM")) + .ok_or(ReportingError::NotAdminProposed)?; + + if caller != pending_admin { + return Err(ReportingError::Unauthorized); + } + + Self::extend_instance_ttl(&env); + env.storage() + .instance() + .set(&symbol_short!("ADMIN"), &pending_admin); + env.storage().instance().remove(&symbol_short!("PEND_ADM")); + + Ok(()) + } + /// Configure addresses for all related contracts (admin only). /// /// # Arguments @@ -390,7 +465,9 @@ impl ReportingContract { Ok(()) } - /// Generate remittance summary report + /// Generate remittance summary report. + /// + /// Fetches split configuration and calculates amounts for a specific period. pub fn get_remittance_summary( env: Env, user: Address, @@ -443,7 +520,9 @@ impl ReportingContract { } } - /// Generate savings progress report + /// Generate savings progress report. + /// + /// Aggregates all goals for a user and calculates overall completion progress. pub fn get_savings_report( env: Env, user: Address, @@ -499,7 +578,9 @@ impl ReportingContract { } } - /// Generate bill payment compliance report + /// Generate bill payment compliance report. + /// + /// Analyzes bill statuses and payment deadlines for a specific period. pub fn get_bill_compliance_report( env: Env, user: Address, @@ -577,7 +658,9 @@ impl ReportingContract { } } - /// Generate insurance coverage report + /// Generate insurance coverage report. + /// + /// Summarizes active policies, coverage amounts, and premium ratios. pub fn get_insurance_report( env: Env, user: Address, @@ -699,7 +782,9 @@ impl ReportingContract { } } - /// Generate comprehensive financial health report + /// Generate comprehensive financial health report combining all metrics. + /// + /// This is the primary reporting entry point for users. pub fn get_financial_health_report( env: Env, user: Address, @@ -736,7 +821,7 @@ impl ReportingContract { } } - /// Generate trend analysis comparing two periods + /// Generate trend analysis comparing two data points. pub fn get_trend_analysis( _env: Env, _user: Address, @@ -805,7 +890,7 @@ impl ReportingContract { result } - /// Store a financial health report for a user + /// Store a financial health report for a user (must authorize). pub fn store_report( env: Env, user: Address, @@ -837,7 +922,7 @@ impl ReportingContract { true } - /// Retrieve a stored report + /// Retrieve a previously stored report. pub fn get_stored_report( env: Env, user: Address, @@ -853,35 +938,46 @@ impl ReportingContract { reports.get((user, period_key)) } - /// Get configured contract addresses + /// Get configured contract addresses. pub fn get_addresses(env: Env) -> Option { env.storage().instance().get(&symbol_short!("ADDRS")) } - /// Get admin address + /// Get current administrator address. pub fn get_admin(env: Env) -> Option
{ env.storage().instance().get(&symbol_short!("ADMIN")) } - /// Archive old reports before the specified timestamp + /// Archive old reports before the specified timestamp (admin only). + /// + /// Moves report data from the primary `REPORTS` storage to the `ARCH_RPT` + /// storage, potentially reducing gas costs for active users. /// /// # Arguments - /// * `caller` - Address of the caller (must be admin) - /// * `before_timestamp` - Archive reports generated before this timestamp + /// * `caller` - Address of the administrator (must authorize) + /// * `before_timestamp` - Archive reports generated before this ledger timestamp /// /// # Returns - /// Number of reports archived - pub fn archive_old_reports(env: Env, caller: Address, before_timestamp: u64) -> u32 { + /// `Ok(u32)` containing the number of reports archived + /// + /// # Errors + /// * `NotInitialized` - If contract has not been initialized + /// * `Unauthorized` - If caller is not the admin + pub fn archive_old_reports( + env: Env, + caller: Address, + before_timestamp: u64, + ) -> Result { caller.require_auth(); let admin: Address = env .storage() .instance() .get(&symbol_short!("ADMIN")) - .unwrap_or_else(|| panic!("Contract not initialized")); + .ok_or(ReportingError::NotInitialized)?; if caller != admin { - panic!("Only admin can archive reports"); + return Err(ReportingError::Unauthorized); } Self::extend_instance_ttl(&env); @@ -938,7 +1034,7 @@ impl ReportingContract { (archived_count, caller), ); - archived_count + Ok(archived_count) } /// Get archived reports for a user @@ -965,25 +1061,33 @@ impl ReportingContract { result } - /// Permanently delete old archives before specified timestamp + /// Permanently delete old archives before specified timestamp (admin only). /// /// # Arguments - /// * `caller` - Address of the caller (must be admin) - /// * `before_timestamp` - Delete archives created before this timestamp + /// * `caller` - Address of the administrator (must authorize) + /// * `before_timestamp` - Delete archives created before this ledger timestamp /// /// # Returns - /// Number of archives deleted - pub fn cleanup_old_reports(env: Env, caller: Address, before_timestamp: u64) -> u32 { + /// `Ok(u32)` containing the number of archives deleted + /// + /// # Errors + /// * `NotInitialized` - If contract has not been initialized + /// * `Unauthorized` - If caller is not the admin + pub fn cleanup_old_reports( + env: Env, + caller: Address, + before_timestamp: u64, + ) -> Result { caller.require_auth(); let admin: Address = env .storage() .instance() .get(&symbol_short!("ADMIN")) - .unwrap_or_else(|| panic!("Contract not initialized")); + .ok_or(ReportingError::NotInitialized)?; if caller != admin { - panic!("Only admin can cleanup reports"); + return Err(ReportingError::Unauthorized); } Self::extend_instance_ttl(&env); @@ -1021,7 +1125,7 @@ impl ReportingContract { (deleted_count, caller), ); - deleted_count + Ok(deleted_count) } /// Returns aggregate counts of active and archived reports for observability. diff --git a/reporting/src/tests.rs b/reporting/src/tests.rs index 197f9dcc..a9383ff3 100644 --- a/reporting/src/tests.rs +++ b/reporting/src/tests.rs @@ -955,7 +955,6 @@ fn test_storage_stats_regression_across_archive_and_cleanup_cycles() { } #[test] -#[should_panic(expected = "Only admin can archive reports")] fn test_archive_unauthorized() { let env = create_test_env(); let contract_id = env.register_contract(None, ReportingContract); @@ -966,11 +965,11 @@ fn test_archive_unauthorized() { client.init(&admin); // Non-admin tries to archive - client.archive_old_reports(&non_admin, &2000000000); + let result = client.try_archive_old_reports(&non_admin, &2000000000); + assert!(result.is_err()); } #[test] -#[should_panic(expected = "Only admin can cleanup reports")] fn test_cleanup_unauthorized() { let env = create_test_env(); let contract_id = env.register_contract(None, ReportingContract); @@ -981,7 +980,8 @@ fn test_cleanup_unauthorized() { client.init(&admin); // Non-admin tries to cleanup - client.cleanup_old_reports(&non_admin, &2000000000); + let result = client.try_cleanup_old_reports(&non_admin, &2000000000); + assert!(result.is_err()); } // ============================================================================ @@ -1588,6 +1588,96 @@ fn test_trend_multi_single_point_returns_empty() { assert_eq!(results.len(), 0); } +#[test] +fn test_admin_rotation_flow() { + let env = create_test_env(); + let contract_id = env.register_contract(None, ReportingContract); + let client = ReportingContractClient::new(&env, &contract_id); + let admin = Address::generate(&env); + let new_admin = Address::generate(&env); + + client.init(&admin); + + // Phase 1: Propose + client.propose_new_admin(&admin, &new_admin); + assert_eq!(client.get_admin(), Some(admin.clone())); // Still old admin + + // Phase 2: Accept + client.accept_admin_rotation(&new_admin); + assert_eq!(client.get_admin(), Some(new_admin.clone())); + + // Verify old admin can no longer perform admin actions + let result = client.try_configure_addresses( + &admin, + &Address::generate(&env), + &Address::generate(&env), + &Address::generate(&env), + &Address::generate(&env), + &Address::generate(&env), + ); + assert!(result.is_err()); +} + +#[test] +fn test_unauthorized_propose() { + let env = create_test_env(); + let contract_id = env.register_contract(None, ReportingContract); + let client = ReportingContractClient::new(&env, &contract_id); + let admin = Address::generate(&env); + let hacker = Address::generate(&env); + + client.init(&admin); + + let result = client.try_propose_new_admin(&hacker, &hacker); + assert!(result.is_err()); +} + +#[test] +fn test_unauthorized_acceptance() { + let env = create_test_env(); + let contract_id = env.register_contract(None, ReportingContract); + let client = ReportingContractClient::new(&env, &contract_id); + let admin = Address::generate(&env); + let new_admin = Address::generate(&env); + let hacker = Address::generate(&env); + + client.init(&admin); + client.propose_new_admin(&admin, &new_admin); + + let result = client.try_accept_admin_rotation(&hacker); + assert!(result.is_err()); +} + +#[test] +fn test_acceptance_without_proposal() { + let env = create_test_env(); + let contract_id = env.register_contract(None, ReportingContract); + let client = ReportingContractClient::new(&env, &contract_id); + let admin = Address::generate(&env); + let candidate = Address::generate(&env); + + client.init(&admin); + + let result = client.try_accept_admin_rotation(&candidate); + assert!(result.is_err()); +} + +#[test] +fn test_re_init_after_rotation_fails() { + let env = create_test_env(); + let contract_id = env.register_contract(None, ReportingContract); + let client = ReportingContractClient::new(&env, &contract_id); + let admin = Address::generate(&env); + let new_admin = Address::generate(&env); + + client.init(&admin); + client.propose_new_admin(&admin, &new_admin); + client.accept_admin_rotation(&new_admin); + + let result = client.try_init(&new_admin); + assert!(result.is_err()); +} + // --- boundary: empty input → empty result ----------------------------------- #[test] diff --git a/reporting/test_snapshots/tests/test_archive_unauthorized.1.json b/reporting/test_snapshots/tests/test_archive_unauthorized.1.json index 6904b3e7..3fbcba65 100644 --- a/reporting/test_snapshots/tests/test_archive_unauthorized.1.json +++ b/reporting/test_snapshots/tests/test_archive_unauthorized.1.json @@ -222,21 +222,16 @@ "v0": { "topics": [ { - "symbol": "log" + "symbol": "fn_return" + }, + { + "symbol": "archive_old_reports" } ], "data": { - "vec": [ - { - "string": "caught panic 'Only admin can archive reports' from contract function 'Symbol(obj#15)'" - }, - { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" - }, - { - "u64": 2000000000 - } - ] + "error": { + "contract": 6 + } } } } @@ -256,12 +251,12 @@ }, { "error": { - "wasm_vm": "invalid_action" + "contract": 6 } } ], "data": { - "string": "caught error from function" + "string": "escalating Ok(ScErrorType::Contract) frame-exit to Err" } } } @@ -281,14 +276,14 @@ }, { "error": { - "wasm_vm": "invalid_action" + "contract": 6 } } ], "data": { "vec": [ { - "string": "contract call failed" + "string": "contract try_call failed" }, { "symbol": "archive_old_reports" @@ -309,31 +304,6 @@ } }, "failed_call": false - }, - { - "event": { - "ext": "v0", - "contract_id": null, - "type_": "diagnostic", - "body": { - "v0": { - "topics": [ - { - "symbol": "error" - }, - { - "error": { - "wasm_vm": "invalid_action" - } - } - ], - "data": { - "string": "escalating error to panic" - } - } - } - }, - "failed_call": false } ] } \ No newline at end of file diff --git a/reporting/test_snapshots/tests/test_cleanup_unauthorized.1.json b/reporting/test_snapshots/tests/test_cleanup_unauthorized.1.json index 3989768b..928f04e7 100644 --- a/reporting/test_snapshots/tests/test_cleanup_unauthorized.1.json +++ b/reporting/test_snapshots/tests/test_cleanup_unauthorized.1.json @@ -222,21 +222,16 @@ "v0": { "topics": [ { - "symbol": "log" + "symbol": "fn_return" + }, + { + "symbol": "cleanup_old_reports" } ], "data": { - "vec": [ - { - "string": "caught panic 'Only admin can cleanup reports' from contract function 'Symbol(obj#15)'" - }, - { - "address": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHK3M" - }, - { - "u64": 2000000000 - } - ] + "error": { + "contract": 6 + } } } } @@ -256,12 +251,12 @@ }, { "error": { - "wasm_vm": "invalid_action" + "contract": 6 } } ], "data": { - "string": "caught error from function" + "string": "escalating Ok(ScErrorType::Contract) frame-exit to Err" } } } @@ -281,14 +276,14 @@ }, { "error": { - "wasm_vm": "invalid_action" + "contract": 6 } } ], "data": { "vec": [ { - "string": "contract call failed" + "string": "contract try_call failed" }, { "symbol": "cleanup_old_reports" @@ -309,31 +304,6 @@ } }, "failed_call": false - }, - { - "event": { - "ext": "v0", - "contract_id": null, - "type_": "diagnostic", - "body": { - "v0": { - "topics": [ - { - "symbol": "error" - }, - { - "error": { - "wasm_vm": "invalid_action" - } - } - ], - "data": { - "string": "escalating error to panic" - } - } - } - }, - "failed_call": false } ] } \ No newline at end of file