fix(security): address critical audit findings in presale and validator-pool pallets

presale:
- Split unbounded finalize_presale distribution into batched batch_distribute()
  extrinsic (same pattern as batch_refund_failed_presale) to prevent block weight
  exhaustion with many contributors
- Fix u128 overflow in calculate_reward_dynamic() by using
  multiply_by_rational_with_rounding() for safe intermediate multiplication
- Fix pre-existing batch_refund test assertion (platform fee deduction was not
  accounted for in expected refund amount)

validator-pool:
- Bound PoolMembers::iter() with .take(MaxPoolSize) in select_validators_for_era()
  to prevent unbounded iteration in on_initialize
- Fix on_initialize weight accounting to include all DB reads/writes from
  do_new_era() and select_validators_for_era() (was only counting 2 reads)
This commit is contained in:
2026-03-21 15:33:25 +03:00
parent 420ed35169
commit f1a7a7f872
3 changed files with 192 additions and 85 deletions
@@ -473,6 +473,12 @@ pub mod pezpallet {
additional_blocks: BlockNumberFor<T>,
new_end_block: BlockNumberFor<T>,
},
/// Batch distribution completed [presale_id, distributed_count, total_distributed]
BatchDistributionCompleted {
presale_id: PresaleId,
distributed_count: u32,
total_distributed: u128,
},
}
#[pezpallet::error]
@@ -688,7 +694,7 @@ pub mod pezpallet {
/// Finalize presale - checks soft cap and sets status to Successful or Failed
#[pezpallet::call_index(2)]
#[pezpallet::weight(T::PresaleWeightInfo::finalize_presale(Contributors::<T>::get(presale_id).len() as u32))]
#[pezpallet::weight(T::PresaleWeightInfo::finalize_presale(1))]
pub fn finalize_presale(origin: OriginFor<T>, presale_id: PresaleId) -> DispatchResult {
ensure_root(origin)?;
@@ -714,72 +720,9 @@ pub mod pezpallet {
soft_cap: presale.limits.soft_cap,
});
// Now distribute tokens to contributors
let treasury = Self::presale_account_id(presale_id);
// Distribute rewards to all contributors
for contributor in Contributors::<T>::get(presale_id).iter() {
let contribution_info = match Contributions::<T>::get(presale_id, contributor) {
Some(info) => info,
None => continue,
};
// Skip if refunded
if contribution_info.refunded || contribution_info.amount == 0 {
continue;
}
// Calculate reward tokens using dynamic rate
let reward_amount = Self::calculate_reward_dynamic(
contribution_info.amount,
total_raised,
presale.tokens_for_sale,
)?;
let bonus =
Self::calculate_bonus(&presale, contribution_info.amount, reward_amount);
let total_reward = reward_amount.saturating_add(bonus);
// Handle vesting
if let Some(ref vesting) = presale.vesting {
let immediate = total_reward
.saturating_mul(vesting.immediate_release_percent as u128)
/ 100;
if immediate > 0 {
let immediate_balance: T::Balance = immediate.into();
T::Assets::transfer(
presale.reward_asset,
&treasury,
contributor,
immediate_balance,
Preservation::Expendable,
)?;
}
// Store remaining for vesting
VestingClaimed::<T>::insert(presale_id, contributor, immediate);
} else {
// No vesting - transfer all
let total_reward_balance: T::Balance = total_reward.into();
T::Assets::transfer(
presale.reward_asset,
&treasury,
contributor,
total_reward_balance,
Preservation::Expendable,
)?;
}
Self::deposit_event(Event::Distributed {
presale_id,
who: contributor.clone(),
amount: total_reward,
});
}
presale.status = PresaleStatus::Finalized;
Presales::<T>::insert(presale_id, presale);
// Distribution is done via batch_distribute() extrinsic to avoid
// unbounded iteration. Status is now Successful — call batch_distribute
// in batches to distribute tokens to all contributors.
SuccessfulPresales::<T>::mutate(|c| *c = c.saturating_add(1));
Self::deposit_event(Event::PresaleFinalized { presale_id, total_raised });
@@ -1144,6 +1087,134 @@ pub mod pezpallet {
Ok(())
}
/// Batch distribute tokens for SUCCESSFUL presales
/// Anyone can call this to help distribute tokens to contributors
/// Processes distribution in batches to avoid block weight limits
#[pezpallet::call_index(9)]
#[pezpallet::weight(T::PresaleWeightInfo::batch_refund_failed_presale(*batch_size))]
pub fn batch_distribute(
origin: OriginFor<T>,
presale_id: PresaleId,
start_index: u32,
batch_size: u32,
) -> DispatchResult {
ensure_signed(origin)?; // Anyone can trigger
let mut presale =
Presales::<T>::get(presale_id).ok_or(Error::<T>::PresaleNotFound)?;
// Only works on SUCCESSFUL presales (soft cap reached, not yet finalized)
ensure!(
presale.status == PresaleStatus::Successful,
Error::<T>::PresaleNotSuccessful,
);
let total_raised = TotalRaised::<T>::get(presale_id);
let treasury = Self::presale_account_id(presale_id);
let contributors = Contributors::<T>::get(presale_id);
// Calculate end index (don't exceed array length)
let end_index =
start_index.saturating_add(batch_size).min(contributors.len() as u32);
let mut distributed_count = 0u32;
let mut total_distributed = 0u128;
// Process batch
for i in start_index..end_index {
let contributor = &contributors[i as usize];
let contribution_info = match Contributions::<T>::get(presale_id, contributor) {
Some(info) => info,
None => continue,
};
// Skip if refunded or zero amount
if contribution_info.refunded || contribution_info.amount == 0 {
continue;
}
// Skip if already distributed (check VestingClaimed for vesting,
// or check a distribution flag)
if VestingClaimed::<T>::contains_key(presale_id, contributor) {
continue;
}
// Calculate reward tokens using dynamic rate (overflow-safe)
let reward_amount = Self::calculate_reward_dynamic(
contribution_info.amount,
total_raised,
presale.tokens_for_sale,
)?;
let bonus =
Self::calculate_bonus(&presale, contribution_info.amount, reward_amount);
let total_reward = reward_amount.saturating_add(bonus);
// Handle vesting
if let Some(ref vesting) = presale.vesting {
let immediate = total_reward
.saturating_mul(vesting.immediate_release_percent as u128)
/ 100;
if immediate > 0 {
let immediate_balance: T::Balance = immediate.into();
T::Assets::transfer(
presale.reward_asset,
&treasury,
contributor,
immediate_balance,
Preservation::Expendable,
)?;
}
// Store remaining for vesting (also marks as distributed)
VestingClaimed::<T>::insert(presale_id, contributor, immediate);
} else {
// No vesting - transfer all
let total_reward_balance: T::Balance = total_reward.into();
T::Assets::transfer(
presale.reward_asset,
&treasury,
contributor,
total_reward_balance,
Preservation::Expendable,
)?;
// Mark as distributed (store total_reward as claimed amount)
VestingClaimed::<T>::insert(presale_id, contributor, total_reward);
}
distributed_count += 1;
total_distributed = total_distributed.saturating_add(total_reward);
Self::deposit_event(Event::Distributed {
presale_id,
who: contributor.clone(),
amount: total_reward,
});
}
// If we've processed all contributors, mark as Finalized
if end_index >= contributors.len() as u32 {
presale.status = PresaleStatus::Finalized;
Presales::<T>::insert(presale_id, &presale);
Self::deposit_event(Event::PresaleFinalized {
presale_id,
total_raised,
});
}
Self::deposit_event(Event::BatchDistributionCompleted {
presale_id,
distributed_count,
total_distributed,
});
Ok(())
}
}
impl<T: Config> Pezpallet<T> {
@@ -1262,13 +1333,17 @@ pub mod pezpallet {
) -> Result<u128, Error<T>> {
ensure!(total_raised > 0, Error::<T>::ArithmeticOverflow);
// Calculate user's share: (contribution * tokens_for_sale) / total_raised
let user_share = user_contribution
.saturating_mul(tokens_for_sale)
.checked_div(total_raised)
.ok_or(Error::<T>::ArithmeticOverflow)?;
Ok(user_share)
// Use multiply_by_rational_with_rounding to prevent u128 overflow
// in (user_contribution * tokens_for_sale) intermediate multiplication.
// This computes: user_contribution * tokens_for_sale / total_raised
// using BigUint internally when values are large.
pezsp_runtime::helpers_128bit::multiply_by_rational_with_rounding(
user_contribution,
tokens_for_sale,
total_raised,
pezsp_runtime::Rounding::Down,
)
.ok_or(Error::<T>::ArithmeticOverflow)
}
}
}
@@ -590,7 +590,14 @@ fn finalize_presale_works() {
// Finalize presale (requires root)
assert_ok!(Presale::finalize_presale(RuntimeOrigin::root(), 0));
// Check presale status changed to Finalized
// Check presale status changed to Successful (not Finalized yet - needs batch_distribute)
let presale = Presale::presales(0).unwrap();
assert!(matches!(presale.status, PresaleStatus::Successful));
// Now batch distribute to all contributors
assert_ok!(Presale::batch_distribute(RuntimeOrigin::signed(1), 0, 0, 100));
// After batch_distribute, presale should be Finalized
let presale = Presale::presales(0).unwrap();
assert!(matches!(presale.status, PresaleStatus::Finalized));
@@ -612,8 +619,9 @@ fn finalize_presale_works() {
);
}
// Check event
System::assert_last_event(
// Check that batch distribution completed event was emitted
// (PresaleFinalized is emitted before BatchDistributionCompleted)
System::assert_has_event(
Event::PresaleFinalized { presale_id: 0, total_raised: total_gross }.into(),
);
});
@@ -1102,7 +1110,14 @@ fn finalize_presale_soft_cap_reached_success() {
// Root finalizes presale
assert_ok!(Presale::finalize_presale(RuntimeOrigin::root(), 0));
// Check presale status is Finalized (went through Successful)
// Check presale status is Successful (needs batch_distribute for Finalized)
let presale = Presale::presales(0).unwrap();
assert!(matches!(presale.status, PresaleStatus::Successful));
// Batch distribute to all contributors
assert_ok!(Presale::batch_distribute(RuntimeOrigin::signed(1), 0, 0, 100));
// Now check presale is Finalized
let presale = Presale::presales(0).unwrap();
assert!(matches!(presale.status, PresaleStatus::Finalized));
@@ -1238,9 +1253,11 @@ fn batch_refund_failed_presale_works() {
10, // batch_size (refund up to 10 contributors)
));
// Check contributors got full refunds (NO FEE for failed presale)
assert_eq!(Assets::balance(2, 2), bob_initial + 500_000_000); // Full refund
assert_eq!(Assets::balance(2, 3), charlie_initial + 500_000_000); // Full refund
// Check contributors got refunds minus non-refundable platform fee portion
// Platform fee = 500M * 2% = 10M, non-refundable = 10M * 50% = 5M
// Refund = 500M - 5M = 495M
assert_eq!(Assets::balance(2, 2), bob_initial + 495_000_000);
assert_eq!(Assets::balance(2, 3), charlie_initial + 495_000_000);
// Check contributions marked as refunded
let bob_contribution = Presale::contributions(0, 2).unwrap();
+19 -4
View File
@@ -354,20 +354,33 @@ pub mod pezpallet {
#[pezpallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pezpallet<T> {
fn on_initialize(block_number: BlockNumberFor<T>) -> Weight {
let mut weight = Weight::zero();
// Always account for the era_start + era_length reads
let mut weight = T::DbWeight::get().reads(2);
// Check if we need to transition to new era
let era_start = Self::era_start();
let era_length = Self::era_length();
if block_number >= era_start + era_length && era_length > Zero::zero() {
weight = weight.saturating_add(T::DbWeight::get().reads(2));
// Account for all DB operations in do_new_era + select_validators_for_era:
// - PoolMembers::iter() reads up to MaxPoolSize entries
// - SelectionHistory::get() per member
// - PerformanceMetrics::get() per member
// - CurrentEra, EraStart, CurrentValidatorSet writes
// - SelectionHistory::mutate per selected validator
let pool_size = Self::pool_size();
weight = weight.saturating_add(
T::DbWeight::get().reads(pool_size as u64 * 2) // iter + history per member
);
weight = weight.saturating_add(
T::DbWeight::get().writes(3 + pool_size as u64) // era state + history updates
);
// Trigger new era if enough time has passed
if Self::do_new_era().is_err() {
// Log error but don't panic
}
weight = weight.saturating_add(T::WeightInfo::force_new_era(Self::pool_size()));
weight = weight.saturating_add(T::WeightInfo::force_new_era(pool_size));
}
weight
@@ -730,7 +743,9 @@ pub mod pezpallet {
let mut random_index = 0u32;
// Collect eligible validators by category
for (validator, category) in PoolMembers::<T>::iter() {
// Bounded by MaxPoolSize to prevent unbounded iteration in on_initialize
let max_pool = T::MaxPoolSize::get() as usize;
for (validator, category) in PoolMembers::<T>::iter().take(max_pool) {
// Skip if selected in last 3 eras (rotation rule)
let history = SelectionHistory::<T>::get(&validator);
let current_era = Self::current_era();