InterestCalculator Code Style Findings

ICO-01C: Inefficient Assignment

Description:

The uploadRateData function accepts a dynamic array that is mandated to be of NUM_BINS length and consequently iterates over it to assign all its elements one by one to the rateData storage array.

Example:

Copy
76function uploadRateData(uint256[] calldata _rateData) external virtual {
77    requireAdmin();
78    require(rateData.length == uint256(NUM_BINS), "array of bin data must be numBins long");
79    for (uint256 i = 0; i < NUM_BINS; i++) {
80        rateData[i] = _rateData[i];
81    }
82}

Recommendation:

We advise the input argument to be set as a fixed-size one instead of a dynamic one (uint256[NUM_BINS] calldata _rateData), to omit the require check for the length and to simply conduct a direct assignment (rateData = _rateData) instead of a loop.

Alleviation:

Our recommendation was heeded and the function now properly conducts a direct assignment.

ICO-02C: Inefficient Data Retrieval

Description:

The getRateData function dictates that it returns a dynamically sized array, declares one in-memory with a NUM_BINS length and consequently assigns the elements of rateData to it one-by-one.

Example:

Copy
84function getRateData() external view returns (uint256[] memory) {
85    uint256[] memory data = new uint256[](NUM_BINS);
86    for (uint256 i = 0; i < NUM_BINS; ++i) {
87        data[i] = rateData[i];
88    }
89    return data;
90}

Recommendation:

We advise the return signature to instead become the fixed-size array (returns (uint256[NUM_BINS] memory)) and the function to directly return the rateData member of the contract.

Alleviation:

The getRateData now yields the array directly instead of iterating through it, alleviating this exhibit.

ICO-03C: Redundant require Check

Description:

The require check performed within getFracLookup is redundant as the function is declared as private and is solely used by the getBorrowerLnAPRRate function that performs the same require anyway.

Example:

Copy
112function getFracLookup(uint256 smoothIndex)
113    private
114    pure
115    returns (
116        uint256 minIndex,
117        uint256 maxIndex,
118        uint256 fracIndex
119    )
120{
121    require(smoothIndex <= (ONE_UNIT));
122    //temporary variable used twice
123    uint256 tempVar = (NUM_BINS.sub(1)).mul(smoothIndex);
124    // Floored value
125    minIndex = tempVar / ONE_UNIT;
126    // Next value up (need this min() because Ur may be exactly 1)
127    maxIndex = Math.min(minIndex.add(1), NUM_BINS.sub(1));
128    // Fraction between the two
129    fracIndex = tempVar.sub(minIndex.mul(ONE_UNIT));
130}

Recommendation:

We advise the require check to be safely omitted from the codebase.

Alleviation:

The require check has been safely omitted from the codebase.