PirexFees Manual Review Findings

PFS-01M: Improper & Inefficient Calculation of Fees

Type	Severity	Location
Mathematical Operations		PirexFees.sol:L147

Description:

The fee system of PirexFees does not take into account the innate mathematical truncation that arithmetics boast in the EVM and as such will lead to funds improperly extracted from the from account whenever distributeFees is invoked and truncation occurs, leading to uncaptured fees.

Example:

Copy
122/** 
123    @notice Distribute fees
124    @param  from    address  Fee source
125    @param  token   address  Fee token
126    @param  amount  uint256  Fee token amount
127 */
128function distributeFees(
129    address from,
130    address token,
131    uint256 amount
132) external onlyRole(FEE_DISTRIBUTOR_ROLE) {
133    emit DistributeFees(token, amount);
134
135    ERC20 t = ERC20(token);
136
137    // Favoring push over pull to reduce accounting complexity for different tokens
138    t.safeTransferFrom(
139        from,
140        treasury,
141        (amount * treasuryPercent) / PERCENT_DENOMINATOR
142    );
143
144    t.safeTransferFrom(
145        from,
146        contributors,
147        (amount * contributorsPercent) / PERCENT_DENOMINATOR
148    );
149}

Recommendation:

We advise the system to instead contain a single variable for denoting the fees of either the treasury or contributors as the remaining amount is meant to be the amount designated for the other party. Additionally, the code within distributeFees should be adjusted to transfer the remaining amount to the secondary party by simply subtracting the calculated fees of the primary party from the total amount rather than performing a new multiplication and division thus accounting for any truncation that may occur.

Alleviation:

The fee calculations as well as relevant setter functions and sanitizations have been adjusted as advised ensuring an optimal truncation-accounting fee structure and the required maintenance of a single variable over two, alleviating this exhibit in full.