Incorrect use of 1000 for converting basis points to decimals in compoundedFactor_ calculation
Summary
The calculation of the compounded factor in the TaxCalculator::calculateCompoundedFactor() incorrectly uses 1000 as the divisor to convert basis points to decimals. Basis points represent two decimal places (e.g., 200 -> 2% -> 0.02) but should use 10000 as the divisor to convert them to decimals (10000 -> 100% -> 1). This mistake causes incorrect interest calculations.
Vulnerability Detail
In the TaxCalculator::calculateCompoundedFactor(), the interestRate is in basis points with 1e2 precision, and is correctly expanded with 1e18 to match precision for further calculations.
However, when converting the per-second rate to decimals for compounded factor calculation, the divisor used is 1000. This is incorrect, as basis points require division by 10000 to represent percentages accurately in decimal format.
The use of 1000 instead of 10000 leads to incorrect calculations for the compounded interest factor, affecting any logic dependent on the compounded interest.
Impact
Overestimation of the compounded interest rate by a factor of 10,000/1,000 = 10x. As a result, users are charged incorrect interest rates.
The impact of this error grows linearly for utilization rates below the kink (2% -> 8%) and increases rapidly after passing the kink threshold (8% -> 100%).
File: TaxCalculator.sol
46: /**
--
53: * @dev The interest rate is returned to 2 decimal places (200 = 2%)
54: *
55: * @param _utilizationRate The utilization rate for the collection
56: *
57: * @return interestRate_ The annual interest rate for the collection
58: */
59: function calculateProtectedInterest(uint _utilizationRate) public pure returns (uint interestRate_) {
60: // If we haven't reached our kink, then we can just return the base fee
61: if (_utilizationRate <= UTILIZATION_KINK) {
62: // Calculate percentage increase for input range 0 to 0.8 ether (2% to 8%)
63: interestRate_ = 200 + (_utilizationRate * 600) / UTILIZATION_KINK;
64: }
65: // If we have passed our kink value, then we need to calculate our additional fee
66: else {
67: // Convert value in the range 0.8 to 1 to the respective percentage between 8% and
68: // 100% and make it accurate to 2 decimal places.
69: interestRate_ = (((_utilizationRate - UTILIZATION_KINK) * (100 - 8)) / (1 ether - UTILIZATION_KINK) + 8) * 100;
70: }
71: }
Tool used
Manual Review
Recommendation
Replace the division by 1000 with 10000 when converting the per-second rate to decimal form in the compounded factor calculation
File: TaxCalculator.sol
80: function calculateCompoundedFactor(uint _previousCompoundedFactor, uint _utilizationRate, uint _timePeriod) public view returns (uint compoundedFactor_) {
81: // Get our interest rate from our utilization rate
82: uint interestRate = this.calculateProtectedInterest(_utilizationRate);
83:
84: // Ensure we calculate the compounded factor with correct precision. `interestRate` is
85: // in basis points per annum with 1e2 precision and we convert the annual rate to per
86: // second rate.
87: uint perSecondRate = (interestRate * 1e18) / (365 * 24 * 60 * 60);
88:
89: // Calculate new compounded factor
-90: compoundedFactor_ = _previousCompoundedFactor * (1e18 + (perSecondRate / 1000 * _timePeriod)) / 1e18;
+90: compoundedFactor_ = _previousCompoundedFactor * (1e18 + (perSecondRate / 10000 * _timePeriod)) / 1e18;
91: }
Vast Umber Walrus
Medium
Incorrect use of
1000
for converting basis points to decimals incompoundedFactor_
calculationSummary
The calculation of the compounded factor in the
TaxCalculator::calculateCompoundedFactor()
incorrectly uses1000
as the divisor to convert basis points to decimals. Basis points represent two decimal places (e.g., 200 -> 2% -> 0.02) but should use10000
as the divisor to convert them to decimals (10000 -> 100% -> 1). This mistake causes incorrect interest calculations.Vulnerability Detail
In the
TaxCalculator::calculateCompoundedFactor()
, theinterestRate
is in basis points with 1e2 precision, and is correctly expanded with 1e18 to match precision for further calculations.However, when converting the per-second rate to decimals for compounded factor calculation, the divisor used is
1000
. This is incorrect, as basis points require division by10000
to represent percentages accurately in decimal format.The use of 1000 instead of 10000 leads to incorrect calculations for the compounded interest factor, affecting any logic dependent on the compounded interest.
Impact
Overestimation of the compounded interest rate by a factor of 10,000/1,000 = 10x. As a result, users are charged incorrect interest rates.
The impact of this error grows linearly for utilization rates below the kink (2% -> 8%) and increases rapidly after passing the kink threshold (8% -> 100%).
Code Snippet
TaxCalculator::calculateCompoundedFactor()
TaxCalculator::calculateProtectedInterest()
Tool used
Manual Review
Recommendation
Replace the division by
1000
with10000
when converting the per-second rate to decimal form in the compounded factor calculation