3.4 Improved Precision in TokenSaleDistributor::_vested()

  • ID: PVE-004

  • Severity: Low

  • Likelihood: Medium

  • Impact: Low

  • Target: TokenSaleDistributor

  • Category: Numeric Errors []

  • CWE subcategory: CWE-190 []

Description

SafeMath is a widely-used Solidity math library that is designed to support safe math operations by preventing common overflow or underflow issues when working with uint256 operands. While it indeed blocks common overflow or underflow issues, the lack of float support in Solidity may introduce another subtle, but troublesome issue: precision loss. In this section, we examine one possible precision loss source that stems from the different orders when both multiplication (mul) and division (div) are involved.

In particular, we use the TokenSaleDistributor::_vested() as an example. This routine is used to calculate the amount of vested tokens at the time of calling.

function_vested (Allocation memory allocation) internal view returns (uint)   {
if (block . timestamp < allocation . epoch + allocation . cliff) {
return 0 ;
}
uint initial Amount = allocation.amount ∗ allocation . cliff Percentage / 1e 18 ;
uint post Cliff Amount = allocation. amount − initialAmount ;
uint elapsed = block.timestamp − allocation.epoch − allocation.cliff ;
if (allocation.isLinear) {
if (elapsed >= allocation. vestingDuration) {
returnallocation . amount ;
	}
	return initialAmount+ ( postCliffAmount ∗ elapsed/ allocation .
	vestingDuration ) ;
	}
	uintelapsedPeriods= elapsed/ monthlyVestingInterval;
	if (elapsedPeriods >= allocation.vestingDuration) {
	return allocation.amount;
	}
	uint monthly Vested Amount = postCliffAmount / allocation. vestingDuration ;
	return initialAmount + (monthly Vested Amount ∗ elapsedPeriods) ;
}	
	

We notice the calculation of the resulting amount (lines 454 − 456) involves mixed multiplication and devision. For improved precision, it is better to calculate the multiplication before the division, i.e., initialAmount + (elapsedPeriods * postCliffAmount / allocation.vestingDuration). Note that the resulting precision loss may be just a small number, but it may play a critical role when certain boundary conditions are met. And it is always the preferred choice if we can avoid the precision loss as much as possible.

Recommendation

Revise the above calculations to better mitigate possible precision loss.

Status

The issue has been confirmed as part of the design.

Last updated