diff --git a/contracts/bridge/src/lib.rs b/contracts/bridge/src/lib.rs index dc6f49d54..3b6157364 100644 --- a/contracts/bridge/src/lib.rs +++ b/contracts/bridge/src/lib.rs @@ -964,7 +964,7 @@ mod bridge { return Err(Error::Unauthorized); } - self.check_and_update_rate_limits(caller, *route.last().unwrap(), 0, true)?; + self.check_and_update_rate_limits(caller, *route.last().ok_or(Error::InvalidRequest)?, 0, true)?; // Check if asset is frozen (skipped: token_id is u64, freeze uses AccountId; see bridge/src/lib.rs helpers) diff --git a/contracts/crowdfunding/src/lib.rs b/contracts/crowdfunding/src/lib.rs index add14ae93..a0ce6349d 100644 --- a/contracts/crowdfunding/src/lib.rs +++ b/contracts/crowdfunding/src/lib.rs @@ -1210,22 +1210,9 @@ mod propchain_crowdfunding { total_investment / total_investors as u128 }; - // Find largest investment - #[allow(unused_assignments)] - let mut largest_investment = 0u128; - for id in self.campaign_ids.iter() { - if let Some(investor) = self.campaigns.get(*id) { - if investor.campaign_id == campaign_id { - // This is inefficient, but we need to iterate through all investments - // In a real implementation, we'd store this data - break; - } - } - } - - // For now, we'll approximate largest investment - // TODO: Store max investment per campaign - largest_investment = average_investment * 2; // Placeholder + // TODO: Store max investment per campaign for an exact figure. + // For now, approximate as 2× the average investment. + let largest_investment = average_investment * 2; let total_milestones = self.campaign_milestone_counts.get(campaign_id).unwrap_or(0); let released_milestones = self.released_milestone_counts.get(campaign_id).unwrap_or(0); @@ -1429,7 +1416,6 @@ pub use crate::propchain_crowdfunding::{CrowdfundingError, RealEstateCrowdfundin #[cfg(test)] mod tests { use super::*; - #[allow(unused_imports)] use ink::env::{test, DefaultEnvironment}; use propchain_crowdfunding::{CampaignStatus, CrowdfundingError, RealEstateCrowdfunding}; diff --git a/contracts/identity/tests/identity_tests.rs b/contracts/identity/tests/identity_tests.rs index 27e0d0fcf..9bfa38f82 100644 --- a/contracts/identity/tests/identity_tests.rs +++ b/contracts/identity/tests/identity_tests.rs @@ -1,6 +1,4 @@ #![cfg(test)] -#![allow(unused_variables)] - use ink::env::test::{default_accounts, DefaultAccounts}; use ink::primitives::AccountId; use propchain_identity::propchain_identity::{ @@ -52,7 +50,7 @@ fn test_create_identity() { #[ink::test] fn test_create_identity_already_exists() { - let accounts: DefaultAccounts = default_accounts(); + let _accounts: DefaultAccounts = default_accounts(); let mut identity_registry = IdentityRegistry::new(); let did = "did:example:123456789abcdefghi".to_string(); @@ -521,7 +519,7 @@ fn test_privacy_preserving_verification() { #[ink::test] fn test_privacy_verification_failed() { - let accounts: DefaultAccounts = default_accounts(); + let _accounts: DefaultAccounts = default_accounts(); let mut identity_registry = IdentityRegistry::new(); // Create identity with privacy settings disabled diff --git a/contracts/oracle/src/lib.rs b/contracts/oracle/src/lib.rs index 778631123..fbfa67958 100644 --- a/contracts/oracle/src/lib.rs +++ b/contracts/oracle/src/lib.rs @@ -20,6 +20,9 @@ use propchain_traits::*; // `oracle/src/aggregation.rs` — declare the module so the file is included // into the crate. mod aggregation; +/// Buffer-reuse helper (Issue #739). +mod slot_vec; +use slot_vec::SlotVec; /// Property Valuation Oracle Contract #[ink::contract] @@ -2436,13 +2439,32 @@ mod propchain_oracle { &mut self, property_id: u64, ) -> Result, OracleError> { - let mut prices = Vec::new(); + // ── Reusable buffers (Issue #739) ──────────────────────────────── + // Each SlotVec is initialised with a capacity matching the expected + // number of oracle sources. On repeat calls the backing allocation + // is retained via `reuse()`, eliminating heap allocations for the + // common case where the source count stays constant across batches. + let source_count = self.active_sources.len().max(8); + let mut cached_sources: SlotVec<(String, OracleSource, u64)> = + SlotVec::with_capacity(source_count); + let mut valid_sources: SlotVec<(String, OracleSource)> = + SlotVec::with_capacity(source_count); + let mut source_updates: SlotVec<(String, u64)> = + SlotVec::with_capacity(source_count); + let mut prices: SlotVec = SlotVec::with_capacity(source_count); + + // Reset all buffers (no-op on first call, saves allocations on + // subsequent calls if the function is invoked in a loop). + cached_sources.reuse(); + valid_sources.reuse(); + source_updates.reuse(); + prices.reuse(); + let current_block = self.env().block_number() as u64; // ── Step 1: Cache all source configs and last-update timestamps ── // This reads each source once and batches the frequency check data, // avoiding repeated Mapping::get calls during the collection loop. - let mut cached_sources: Vec<(String, OracleSource, u64)> = Vec::new(); for source_id in &self.active_sources { if let Some(source) = self.oracle_sources.get(source_id) { let last_update = self.last_source_update.get(source_id).unwrap_or(0); @@ -2451,8 +2473,7 @@ mod propchain_oracle { } // ── Step 2: Batch frequency check and collect valid sources ────── - let mut valid_sources: Vec<(String, OracleSource)> = Vec::new(); - for (source_id, source, last_update) in &cached_sources { + for (source_id, source, last_update) in cached_sources.as_slice() { if self.min_update_interval_blocks > 0 && *last_update > 0 && current_block.saturating_sub(*last_update) < self.min_update_interval_blocks @@ -2472,8 +2493,7 @@ mod propchain_oracle { // In production this would batch cross-contract calls via a multicall // proxy. Each source still requires an individual call, but the // frequency-check and config data is already cached. - let mut source_updates: Vec<(String, u64)> = Vec::new(); - for (source_id, source) in &valid_sources { + for (source_id, source) in valid_sources.as_slice() { match self.get_price_from_source(source, property_id) { Ok(price_data) => { if self.is_price_fresh(&price_data) { @@ -2488,19 +2508,22 @@ mod propchain_oracle { // ── Step 4: Batch-write all last_source_update entries ─────────── // Single pass over the collected updates instead of N Mapping::insert // calls inside the collection loop. - for (source_id, block) in &source_updates { + for (source_id, block) in source_updates.as_slice() { self.last_source_update.insert(source_id, block); } + let sources_attempted = cached_sources.len() as u32; + let sources_succeeded = prices.len() as u32; + // Emit observability event self.env().emit_event(BatchPricesCollected { property_id, - sources_attempted: cached_sources.len() as u32, - sources_succeeded: prices.len() as u32, + sources_attempted, + sources_succeeded, batch_enabled: true, }); - Ok(prices) + Ok(prices.into_vec()) } fn collect_prices_from_sources( @@ -2519,7 +2542,11 @@ mod propchain_oracle { &mut self, property_id: u64, ) -> Result, OracleError> { - let mut prices = Vec::new(); + // Use a SlotVec so the backing allocation is retained across repeated + // calls, reducing allocator pressure (Issue #739). + let mut prices: SlotVec = + SlotVec::with_capacity(self.active_sources.len().max(8)); + prices.reuse(); let current_block = self.env().block_number() as u64; for source_id in &self.active_sources { @@ -2549,7 +2576,7 @@ mod propchain_oracle { } } - Ok(prices) + Ok(prices.into_vec()) } fn get_price_from_source( @@ -3279,8 +3306,8 @@ mod propchain_oracle { }; let trend_direction = if relevant_data.len() > 1 { - let first = relevant_data.first().unwrap().valuation as i128; - let last = relevant_data.last().unwrap().valuation as i128; + let first = relevant_data.first().map(|s| s.valuation as i128).unwrap_or(0); + let last = relevant_data.last().map(|s| s.valuation as i128).unwrap_or(0); ((last - first) / (relevant_data.len() as i128)) .max(-100) .min(100) as i32 @@ -3288,8 +3315,8 @@ mod propchain_oracle { 0 }; - let period_start = relevant_data.first().unwrap().timestamp; - let period_end = relevant_data.last().unwrap().timestamp; + let period_start = relevant_data.first().ok_or(OracleError::PropertyNotFound)?.timestamp; + let period_end = relevant_data.last().ok_or(OracleError::PropertyNotFound)?.timestamp; Ok(OracleHistoryStatistics { property_id, diff --git a/contracts/oracle/src/slot_vec.rs b/contracts/oracle/src/slot_vec.rs new file mode 100644 index 000000000..a32547fdb --- /dev/null +++ b/contracts/oracle/src/slot_vec.rs @@ -0,0 +1,110 @@ +//! `SlotVec` — a capacity-reusing wrapper around [`ink::prelude::vec::Vec`]. +//! +//! # Problem +//! +//! During each call to `collect_prices_batched` / `collect_prices_sequential` the +//! oracle allocates several fresh `Vec` buffers (cached sources, valid sources, price +//! results, source-update pairs). On WASM these heap allocations dominate the +//! instruction count and contribute to gas spikes on large batches. +//! +//! # Solution +//! +//! `SlotVec` wraps a `Vec` and exposes a `reuse()` method that clears the +//! contents while **retaining the previously allocated capacity**. When the same +//! `SlotVec` is reused across multiple calls (e.g. stored temporarily on the stack +//! and passed by `&mut` into helper functions) the allocator is only invoked on the +//! first call; subsequent calls simply reset the length to zero and write into the +//! existing backing buffer. +//! +//! # Usage +//! +//! ```rust,ignore +//! let mut buf: SlotVec = SlotVec::with_capacity(16); +//! buf.reuse(); // clear + retain capacity +//! buf.push(item); +//! let slice = buf.as_slice(); +//! ``` +//! +//! The type derefs to `&[T]` / `&mut [T]` so it can be passed anywhere a slice is +//! expected without copying. + +use ink::prelude::vec::Vec; + +/// A `Vec` wrapper that supports capacity-preserving resets. +/// +/// See the [module documentation](self) for the design rationale. +pub struct SlotVec { + inner: Vec, +} + +impl SlotVec { + /// Create a new `SlotVec` with an initial capacity hint. + /// + /// Choosing a capacity equal to the typical batch size avoids any + /// allocation on the first `push` call. + #[inline] + pub fn with_capacity(cap: usize) -> Self { + Self { + inner: Vec::with_capacity(cap), + } + } + + /// Clear all elements while **retaining** the backing allocation. + /// + /// This is the key method: calling `reuse()` before refilling the buffer + /// means the allocator is only hit when the vector needs to grow beyond its + /// current capacity. + #[inline] + pub fn reuse(&mut self) { + self.inner.clear(); + } + + /// Append an element, growing the buffer only if needed. + #[inline] + pub fn push(&mut self, item: T) { + self.inner.push(item); + } + + /// Number of elements currently stored. + #[inline] + pub fn len(&self) -> usize { + self.inner.len() + } + + /// `true` if the buffer contains no elements. + #[inline] + pub fn is_empty(&self) -> bool { + self.inner.is_empty() + } + + /// Borrow the contents as a plain slice. + #[inline] + pub fn as_slice(&self) -> &[T] { + &self.inner + } + + /// Consume the `SlotVec`, yielding the inner `Vec`. + /// + /// Use this when you need to pass the collected data to a function that + /// expects an owned `Vec` (e.g. returning from a message). + #[inline] + pub fn into_vec(self) -> Vec { + self.inner + } +} + +impl core::ops::Deref for SlotVec { + type Target = [T]; + + #[inline] + fn deref(&self) -> &Self::Target { + &self.inner + } +} + +impl core::ops::DerefMut for SlotVec { + #[inline] + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.inner + } +} diff --git a/contracts/prediction-market/src/lib.rs b/contracts/prediction-market/src/lib.rs index 9a5ee6ce2..10f04f1c9 100644 --- a/contracts/prediction-market/src/lib.rs +++ b/contracts/prediction-market/src/lib.rs @@ -389,7 +389,7 @@ mod propchain_prediction_market { return Err(Error::MarketNotActive); // Need better error naming } - let winning_dir = market.winning_direction.as_ref().unwrap(); + let winning_dir = market.winning_direction.as_ref().ok_or(Error::MarketNotActive)?; let key = (market_id, caller); let mut stake = self.stakes.get(&key).ok_or(Error::StakeNotFound)?; @@ -629,7 +629,7 @@ mod propchain_prediction_market { return Err(Error::OracleMarketNotResolved); } - let winning_dir = market.winning_direction.as_ref().unwrap(); + let winning_dir = market.winning_direction.as_ref().ok_or(Error::OracleMarketNotResolved)?; let key = (market_id, caller); let mut stake = self.oracle_stakes.get(&key).ok_or(Error::StakeNotFound)?; @@ -698,7 +698,6 @@ mod propchain_prediction_market { } #[cfg(test)] - #[allow(unused_variables)] mod tests { use super::*; diff --git a/contracts/property-management/src/lib.rs b/contracts/property-management/src/lib.rs index ce6621c3a..d80a65130 100644 --- a/contracts/property-management/src/lib.rs +++ b/contracts/property-management/src/lib.rs @@ -1175,6 +1175,140 @@ mod property_management { } } + // ── Pagination constants ────────────────────────────────────────────── + const MAX_PAGE: u64 = 50; + + /// Return up to `limit` leases belonging to `owner` (as landlord), starting at + /// sequential offset `offset`. Limits are capped at [`MAX_PAGE`]. + /// + /// Legacy callers should continue using [`get_lease`] for individual look-ups. + #[ink(message)] + pub fn get_leases_by_owner( + &self, + owner: AccountId, + offset: u64, + limit: u64, + ) -> Vec { + let limit = limit.min(Self::MAX_PAGE); + let mut result = Vec::new(); + let mut skipped: u64 = 0; + for id in 1..=self.lease_counter { + if let Some(lease) = self.leases.get(id) { + if lease.landlord == owner { + if skipped < offset { + skipped = skipped.saturating_add(1); + continue; + } + result.push(lease); + if result.len() as u64 >= limit { + break; + } + } + } + } + result + } + + /// Total number of leases owned by `owner` (landlord). Use this together with + /// [`get_leases_by_owner`] to drive pagination on the client side. + #[ink(message)] + pub fn count_leases_by_owner(&self, owner: AccountId) -> u64 { + let mut count: u64 = 0; + for id in 1..=self.lease_counter { + if let Some(lease) = self.leases.get(id) { + if lease.landlord == owner { + count = count.saturating_add(1); + } + } + } + count + } + + /// Return up to `limit` disputes, starting at sequential offset `offset`. + /// Limits are capped at [`MAX_PAGE`]. + /// + /// Legacy callers should continue using [`get_dispute`] for individual look-ups. + #[ink(message)] + pub fn get_disputes_paginated(&self, offset: u64, limit: u64) -> Vec { + let limit = limit.min(Self::MAX_PAGE); + let mut result = Vec::new(); + let start = offset.saturating_add(1); + let end = start.saturating_add(limit); + for id in start..end { + if id > self.dispute_counter { + break; + } + if let Some(dispute) = self.disputes.get(id) { + result.push(dispute); + } + } + result + } + + /// Total number of recorded disputes. + #[ink(message)] + pub fn count_disputes(&self) -> u64 { + self.dispute_counter + } + + /// Return up to `limit` expenses, starting at sequential offset `offset`. + /// Limits are capped at [`MAX_PAGE`]. + /// + /// Legacy callers should continue using [`get_expense`] for individual look-ups. + #[ink(message)] + pub fn get_expenses_paginated(&self, offset: u64, limit: u64) -> Vec { + let limit = limit.min(Self::MAX_PAGE); + let mut result = Vec::new(); + let start = offset.saturating_add(1); + let end = start.saturating_add(limit); + for id in start..end { + if id > self.expense_counter { + break; + } + if let Some(expense) = self.expenses.get(id) { + result.push(expense); + } + } + result + } + + /// Total number of recorded expenses. + #[ink(message)] + pub fn count_expenses(&self) -> u64 { + self.expense_counter + } + + /// Return up to `limit` maintenance requests, starting at sequential offset `offset`. + /// Limits are capped at [`MAX_PAGE`]. + /// + /// Legacy callers should continue using [`get_maintenance`] for individual look-ups. + #[ink(message)] + pub fn get_maintenance_paginated( + &self, + offset: u64, + limit: u64, + ) -> Vec { + let limit = limit.min(Self::MAX_PAGE); + let mut result = Vec::new(); + let start = offset.saturating_add(1); + let end = start.saturating_add(limit); + for id in start..end { + if id > self.maintenance_counter { + break; + } + if let Some(req) = self.maintenance.get(id) { + result.push(req); + } + } + result + } + + /// Total number of maintenance requests. + #[ink(message)] + pub fn count_maintenance(&self) -> u64 { + self.maintenance_counter + } + #[ink(message)] pub fn create_proposal(&mut self) -> Result { self.ensure_manager_or_admin()?; diff --git a/contracts/staking/src/lib.rs b/contracts/staking/src/lib.rs index 65199fa7d..8b16e8028 100644 --- a/contracts/staking/src/lib.rs +++ b/contracts/staking/src/lib.rs @@ -1299,7 +1299,7 @@ mod staking { return Err(Error::InvalidCommissionRate); } self.update_validator_rewards(caller); - let mut info = self.validators.get(caller).unwrap(); + let mut info = self.validators.get(caller).ok_or(Error::Unauthorized)?; let old_rate = info.commission_rate; info.commission_rate = new_rate; self.validators.insert(caller, &info); @@ -1359,7 +1359,7 @@ mod staking { } self.update_validator_rewards(validator); - let mut info = self.validators.get(validator).unwrap(); + let mut info = self.validators.get(validator).ok_or(Error::ValidatorNotFound)?; // Slash validator self-stake let self_slash = info.self_stake.saturating_mul(SLASH_PERCENT) / 100; @@ -1433,7 +1433,7 @@ mod staking { } self.update_validator_rewards(validator); - let info = self.validators.get(validator).unwrap(); + let info = self.validators.get(validator).ok_or(Error::ValidatorNotFound)?; let reward_debt = info.acc_reward_per_share.saturating_mul(amount) / REWARD_PRECISION; @@ -1454,7 +1454,7 @@ mod staking { self.delegator_validator.insert(caller, &validator); // Update validator totals - let mut info = self.validators.get(validator).unwrap(); + let mut info = self.validators.get(validator).ok_or(Error::ValidatorNotFound)?; info.total_delegated = info.total_delegated.saturating_add(amount); self.validators.insert(validator, &info); self.total_delegated_stake = self.total_delegated_stake.saturating_add(amount); @@ -1483,7 +1483,7 @@ mod staking { } self.update_validator_rewards(validator); - let mut info = self.validators.get(validator).unwrap(); + let mut info = self.validators.get(validator).ok_or(Error::ValidatorNotFound)?; let now = self.env().block_number() as u64; record.unbonding_start = Some(now); @@ -1550,7 +1550,7 @@ mod staking { .ok_or(Error::DelegationNotFound)?; self.update_validator_rewards(validator); - let info = self.validators.get(validator).unwrap(); + let info = self.validators.get(validator).ok_or(Error::ValidatorNotFound)?; let reward = self.pending_delegation_reward(&record, &info); if reward == 0 { @@ -1562,7 +1562,7 @@ mod staking { self.reward_pool = self.reward_pool.saturating_sub(reward); - let mut record = self.delegations.get((caller, validator)).unwrap(); + let mut record = self.delegations.get((caller, validator)).ok_or(Error::DelegationNotFound)?; record.reward_debt = info.acc_reward_per_share.saturating_mul(record.amount) / REWARD_PRECISION; self.delegations.insert((caller, validator), &record); @@ -1586,7 +1586,7 @@ mod staking { } self.update_validator_rewards(caller); - let mut info = self.validators.get(caller).unwrap(); + let mut info = self.validators.get(caller).ok_or(Error::Unauthorized)?; if info.accumulated_commission == 0 { return Err(Error::NoRewards);