QA Report

[code423n4]

Low Priority Findings

Logic error in update[XXXX]LimitLimits functions

All the update[XXXX]LimitLimits functions here have a logic error in the first require:

require(_min < _max || _min.mul(_max) == 0, 'UBLL1');

This require check passes if _max = 0..._min can then be bigger than max. I would suggest the following fix:

require(_min < _max || (_min == 0 && _max == 0), 'UBLL1');

This has low risk because all of the update[XXXX]LimitLimits functions are protected by onlyOwner. However if the owner did make a mistake then it would impact all limit calculations and effectively brick the contract until the owner resolved the mistake.

calculateInterestAccrued calculations differ from documentation

In the documentation it states the following:

Grace Period Interest Rate: Interest rate that will be used for calculating interest during the grace period

However in the calculateInterestAccrued function here, penalty interest is added to normal interest (where normal interest is also calculated during the grace period):

_interestAccrued = _interestAccrued.add(_penalityInterest);

Thus, the actual interest being calculated during the grace period is actually the normal interest rate plus the penalty interest rate. This is detrimental to the borrower. Either the documentation should be updated to reflect the code, or the interest calculation in the code should be modified to match the documentation.

[ritik99]

1. Max = 0 is the case where there are no restrictions on the max. value of a variable. Thus the logic works as expected - we'll update the natspec to make this explicit.
2. Looks like we weren't descriptive enough in one of the sections. In the technical spec of the document we mention that the interest rate is cumulative