Canto Liquidity Mining Protocol - lsaudit's results

[G-01] Do not calculate lowerTick + 10 and upperTick - 10 on every loop iteration

File: LiquidityMining.sol

 for (int24 i = lowerTick + 10; i <= upperTick - 10; ++i) {

File: LiquidityMining.sol

  for (int24 i = lowerTick + 10; i <= upperTick - 10; ++i) {

File: LiquidityMining.sol

 for (int24 j = lowerTick + 10; j <= upperTick - 10; ++j) {

Above loops - on every iteration perform unnecessary addition (+ 10) and substraction (- 10).

To demonstrate how gas ineffective this is, I've performed a simple test in the Remix IDE:

    function AAA(int24 lowerTick, int24 upperTick) public {
        uint x;
        uint a = gasleft();
        for (int24 i = lowerTick + 10; i <= upperTick - 10; ++i) x++;
        console.log(a - gasleft());
    }

    function BBB(int24 lowerTick, int24 upperTick) public {
        uint x;
        uint a = gasleft();
        int24 i = lowerTick + 10;
        int24 end_i = upperTick - 10;
        for (i; i <= end_i - 10; ++i) x++;
        console.log(a - gasleft());
    }

Calling AAA(1, 100) costs 44255, while calling BBB(1, 100): 39557. This proves, that much more effective solution would be to cache the result of lowerTick + 10 and upperTick - 10 outside the loop. The example fix should look as below:

int24 i = lowerTick + 10;
int24 stop_i = upperTick - 10
for (i; i <= stop_i; ++i) {

[G-02] Unnecessary declaration of numElementsExit in crossTicks in LiquidityMining.sol

Variable numElementsExit is used only once, thus it does not need to be declared:

 uint256 numElementsExit = tickTracking_[poolIdx][exitTick].length;
        tickTracking_[poolIdx][exitTick][numElementsExit - 1]
            .exitTimestamp = uint32(block.timestamp);

can be changed to:

        tickTracking_[poolIdx][exitTick][tickTracking_[poolIdx][exitTick].length - 1]
            .exitTimestamp = uint32(block.timestamp);

[G-03] Calculation of dt in LiquidityMining.sol can be unchecked

File: LiquidityMining.sol

                uint32 currWeek = uint32((time / WEEK) * WEEK);
                uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);
                uint32 dt = uint32(
                    nextWeek < block.timestamp
                        ? nextWeek - time
                        : block.timestamp - time
                );

File: LiquidityMining.sol

                    uint32 currWeek = uint32((time / WEEK) * WEEK);
                    uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);
                    uint32 dt = uint32(
                        nextWeek < block.timestamp
                            ? nextWeek - time
                            : block.timestamp - time
                    );

File: LiquidityMining.sol

                uint32 currWeek = uint32((time / WEEK) * WEEK);
                uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);
                uint32 dt = uint32(
                    nextWeek < block.timestamp
                        ? nextWeek - time
                        : block.timestamp - time
                );

File: LiquidityMining.sol

                uint32 currWeek = uint32((time / WEEK) * WEEK);
                uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);
                uint32 dt = uint32(
                    nextWeek < block.timestamp
                        ? nextWeek - time
                        : block.timestamp - time
                );

In above code blocks, subtractions: nextWeek - time and block.timestamp - time will never underflow and can be unchecked.

• Proof for block.timestamp - time Please notice, that time variable is assigned to lastAccrued: uint32 time = lastAccrued;. The value of lastAccrued is a block.timestamp from the previous function call. Since previous block.timestamp <= current block.timestamp, we can assume that block.timestamp >= time, thus this expression will never underflow and can be unchecked.

• Proof for nextWeek - time nextWeek is defined as: nextWeek = uint32(((time + WEEK) / WEEK) * WEEK); This implies that ((time + WEEK) / WEEK) * WEEK > time, thus nextWeek > time, thus this expression will never underflow and can be unchecked.

[G-04] nextWeek calculation can be simplified

File: LiquidityMining.sol

                uint32 currWeek = uint32((time / WEEK) * WEEK);
                uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);

File: LiquidityMining.sol

                    uint32 currWeek = uint32((time / WEEK) * WEEK);
                    uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);

File: LiquidityMining.sol

                uint32 currWeek = uint32((time / WEEK) * WEEK);
                uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);

File: LiquidityMining.sol

                uint32 currWeek = uint32((time / WEEK) * WEEK);
                uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);

currWeek is time rounded to full weeks. There's no need to calculate nextWeek in the same way, since nextWeek is simply currWeek + WEEK. We can save gas on unessesary division and multiplication , when we will use: uint32 nextWeek = uint32(currWeek + WEEK);

[G-5] Do-while loops are cheaper than for loops

Using do-while loops are better for saving gas than for-loops.

Multiple of for-loops can be rewritten to do-while loops.

• File: LiquidityMining.sol

88:             for (int24 i = lowerTick + 10; i <= upperTick - 10; ++i) {
139:	            for (int24 i = lowerTick + 10; i <= upperTick - 10; ++i) {
174:	        for (uint256 i; i < weeksToClaim.length; ++i) {
184:	                for (int24 j = lowerTick + 10; j <= upperTick - 10; ++j) {
266:	        for (uint256 i; i < weeksToClaim.length; ++i) {