Canto Dex Oracle contest - lukris02's results

QA Report for Canto Dex Oracle contest

Overview

During the audit, 5 low and 7 non-critical issues were found.

Low Risk Findings (5)

L-1. Check zero denominator

Description

If the input parameter is equal to zero, this will cause the function call failure on division.

Instances

• return (reserveAverageCumulative0 / granularity, reserveAverageCumulative1 / granularity);
• return (totalSupplyCumulativeAvg / granularity);

Recommendation

Add the check to prevent function call failure.

L-2. Missing check for input variables

Description

If input variable points== 0, function will return empty array.
More critical, if input variable window== 0, function will return array with default values, which may lead to further incorrect calculations.

Instances

• function sampleReserves(uint points, uint window)
• function sampleSupply(uint points, uint window)

Recommendation

Add require statement or custom error - points!= 0 && window!= 0.

L-3. Large number of observations may cause out-of-gas error

Description

Loops that do not have a fixed number of iterations, for example, loops that depend on storage values, have to be used carefully: Due to the block gas limit, transactions can only consume a certain amount of gas. Either explicitly or just due to normal operation, the number of iterations in a loop can grow beyond the block gas limit, which can cause the complete contract to be stalled at a certain point.

Instances

• for(; i < lastIndex; i+=window) { (function sampleReserves)
• for(; i < lastIndex; i+=window) { (function sampleSupply)

Recommendation

Restrict the maximum number of sample observations (points).

L-4. Incorrect comment

Instances

// note in terms of note will always be 1

Recommendation

Probably the comment should be like this "price in terms of note will always be 1 ".

L-5. Misleading comment

Description

The comment is misleading, and there is an extra comma and an empty comment line.

Instances

Link:

        for (uint i = 0; i < _reserves0.length; ++i) {
            reserveAverageCumulative0 += _reserves0[i]; //normalize the reserves for TWAP LP Oracle pricing, 
            reserveAverageCumulative1 += _reserves1[i]; //
        }

Recommendation

Change or delete comment.

Non-Critical Risk Findings (7)

NC-1. Order of Functions

Description

Some internal functions are between public, some external functions are between public, and some public functions are between external.

Instances

• BaseV1-core.sol: 137
• BaseV1-core.sol: 224
• BaseV1-core.sol: 260
• BaseV1-core.sol: 237
• BaseV1-core.sol: 271

Recommendation

According to Style Guide, ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier.
Functions should be grouped according to their visibility and ordered:

• constructor
• receive function (if exists)
• fallback function (if exists)
• external
• public
• internal
• private

NC-2. Maximum line length exceeded

Description

Some lines of code are too long.

Instances

• observations.push(Observation(blockTimestamp, reserve0CumulativeLast, reserve1CumulativeLast, totalSupplyCumulativeLast));
• _totalSupply[index] = (observations[nextIndex].totalSupplyCumulative - observations[i].totalSupplyCumulative) / timeElapsed;
• and more, if you take into account the beginning indent and / or comments.

Recommendation

According to Style Guide, maximum suggested line length is 120 characters.
Make the lines shorter.

NC-3. Constants may be used

Description

Constants may be used instead of literal values.

Instances

• uint[] memory supply = pair.sampleSupply(8, 1);
• prices = pair.sample(token1, decimals, 8, 1);
• (unitReserves, assetReserves) = pair.sampleReserves(8, 1);
• prices = pair.sample(token0, decimals, 8, 1);
• (assetReserves, unitReserves) = pair.sampleReserves(8, 1);
• prices = pair.sample(token1, decimals, 8, 1);
• (unitReserves, assetReserves) = pair.sampleReserves(8, 1);
• prices = pair.sample(token0, decimals, 8, 1);
• (assetReserves, unitReserves) = pair.sampleReserves(8, 1);
• for(uint i; i < 8; ++i) {
• return 1e18; // Stable coins supported by the lending market are instantiated by governance and their price will always be 1 note
• return 1e18 * 1e18 / (10 ** decimals); //Scale Price as a mantissa to maintain precision in comptroller
• return 1e18 * 1e18 / (10 ** decimals); //Scale Price as a mantissa to maintain precision in comptroller
• return getPriceCanto(underlying) * getPriceNote(address(wcanto), false) / 1e18;
• LpPricesCumulative += (token0TVL + token1TVL) * 1e18 / supply[i];
• return LpPrice * getPriceNote(address(wcanto), false) / 1e18; // return the price in terms of Note
• return price * 1e18 / decimals; //return the scaled price
• return price * 1e18 / decimals; // divide by decimals now to maintain precision

Recommendation

Define constant variables for repeated values (8 and 1e18).

NC-4. Inconsistent comment spacing and location

Description

Some comments are above the line of code and some next to it. Some comments are indented between // and the comment text, some are not.

Instances

• Link:

{ //manual scope to pop symbol off of stack
 string memory symbol = ctoken.symbol();

• underlying = address(ICErc20(address(ctoken)).underlying()); // We are getting the price for a CErc20 lending market
• Link:

//set price statically to 1 when the Comptroller is retrieving Price
if (compareStrings(symbol, "cNOTE")) { // note in terms of note will always be 1 
    return 1e18; // Stable coins supported by the lending market are instantiated by governance and their price will always be 1 note

• return 1e18 * 1e18 / (10 ** decimals); //Scale Price as a mantissa to maintain precision in comptroller
• all other functions...

Recommendation

Use consistent comment spacing and location.

NC-5. Loop parameter may be changed for clarity

Description

In loop are used _reserves0.length. It is equal to input variable granularity. It can be clearer and more consistent if you use an input variable in the loop.

Instances

1. Link:

    function reserves(uint granularity) external view returns(uint, uint) {
        (uint[] memory _reserves0, uint[] memory _reserves1)= sampleReserves(granularity, 1);
        uint reserveAverageCumulative0;
        uint reserveAverageCumulative1;

        for (uint i = 0; i < _reserves0.length; ++i) {

2. Link:

    function totalSupplyAvg(uint granularity) external view returns(uint) {
        uint[] memory _totalSupplyAvg = sampleSupply(granularity, 1);
        uint totalSupplyCumulativeAvg;

        for (uint i = 0; i < _totalSupplyAvg.length; ++i) {

Recommendation

1. Change to

    function reserves(uint granularity) external view returns(uint, uint) {
        (uint[] memory _reserves0, uint[] memory _reserves1)= sampleReserves(granularity, 1);
        uint reserveAverageCumulative0;
        uint reserveAverageCumulative1;
        
        //HERE
        for (uint i = 0; i < granularity; ++i) {

2. Change to

    function totalSupplyAvg(uint granularity) external view returns(uint) {
        uint[] memory _totalSupplyAvg = sampleSupply(granularity, 1);
        uint totalSupplyCumulativeAvg;

        //HERE
        for (uint i = 0; i < granularity; ++i) {

NC-6. Functions without comments

Description

Some functions do not have comments describing them.

Instances

• function reserves(uint granularity)
• function sampleReserves(uint points, uint window)
• function totalSupplyAvg(uint granularity)
• function sampleSupply(uint points, uint window)

Recommendation

Add comments.

NC-7. Require statement may be placed before allocating memory for arrays

Instances

1. Link:

        uint[] memory _reserves0 = new uint[](points);
        uint[] memory _reserves1 = new uint[](points);
        
        uint lastIndex = observations.length-1;
        require(lastIndex >= points * window, "PAIR::NOT READY FOR PRICING");

2. Link:

        uint[] memory _totalSupply = new uint[](points);
        
        uint lastIndex = observations.length-1;
        require(lastIndex >= points * window, "PAIR::NOT READY FOR PRICING");

Recommendation

1. Change to

        uint lastIndex = observations.length-1;
        require(lastIndex >= points * window, "PAIR::NOT READY FOR PRICING");

        uint[] memory _reserves0 = new uint[](points);
        uint[] memory _reserves1 = new uint[](points);

2. Change to

        uint lastIndex = observations.length-1;
        require(lastIndex >= points * window, "PAIR::NOT READY FOR PRICING");
        
        uint[] memory _totalSupply = new uint[](points);