Canto Liquidity Mining Protocol - 0xdice91's results

Low Risk & Non Critical Findings 5

QA Report

Low-Risk Findings List

Total ~ 3 Issues

Non Critical Issues List

Low Risk Findings.

L-01 Multiplication on the Result of Division

Summary

Dividing an integer by another integer will often result in a loss of precision. When the result is multiplied by another number, the loss of precision is magnified, often to material magnitudes. (X / Z) * Y should be re-written as (X * Y) / Z. Note This wasn't found in the bot report.

Vulnerability Details

you can see instances of these issues

 File: LiquidityMining.sol
51:                   uint32 currWeek = uint32((time / WEEK) * WEEK);
52:                   uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);
............... 
96:                       uint32 currWeek = uint32((time / WEEK) * WEEK);
97:                       uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);
................
208:                  uint32 currWeek = uint32((time / WEEK) * WEEK);
209:                  uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);
................
238:                  uint32 currWeek = uint32((time / WEEK) * WEEK); 
239:                  uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);

Recommended Mitigation Step.

multiply before division. An instance of such mitigation would be

 File: LiquidityMining.sol
-  51:                   uint32 currWeek = uint32((time / WEEK) * WEEK);
+  51:                   uint32 currWeek = uint32((WEEK / WEEK) * time);

L-02 Not Enough Coverage of Functions and Contracts In the Docs

Summary

There isn't enough coverage on the user flows and functions in canto liquidity mining, which can lead to confusion and incorrect assumptions made when auditing the specific codebases.

Vulnerability Details

the only place where there is anything said about the protocol is on the contest page on the code4rena site Link here and you can see that it doesn't do a good job of explaining how the contract and the functions logic are supposed to interact and be interacted with.

Recommended Mitigation Step

Create good documentation of the contracts and the functions stating and explaining how they are meant to be performed. .

Non Critical Issues List.

NC-01 dt as a Local variable name causes confusion

Summary

The local variable named dt doesn't give a complete and accurate context as to what it is meant for in LiquidityMining.sol and can cause confusion.

Vulnerability Details

Local variable dt is declared in the:

• accrueConcentratedGlobalTimeWeightedLiquidity()
• accrueConcentratedPositionTimeWeightedLiquidity()
• accrueAmbientGlobalTimeWeightedLiquidity()
• accrueAmbientPositionTimeWeightedLiquidity()

Functions respectively, however, the variable name gives little context as to what is stored and nothing is said about it in the read.md

Recommended Mitigation Step

choose a better name for the variable that entails what it is meant for.

NC-02 There should be a Min and Max weekFrom and weekTo limit to be set

Summary

There should be a minimum and maximum amount of weeks that can be set to have weekly rewards in order to help protect governance against any input mistake made.

Vulnerability Details

you can see the instance of this issue.

File: LiquidityMiningPath.sol
    function setConcRewards(bytes32 poolIdx, uint32 weekFrom, uint32 weekTo, uint64 weeklyReward) public payable {
        // require(msg.sender == governance_, "Only callable by governance");
        require(weekFrom % WEEK == 0 && weekTo % WEEK == 0, "Invalid weeks");
        while (weekFrom <= weekTo) {
            concRewardPerWeek_[poolIdx][weekFrom] = weeklyReward;
            weekFrom += uint32(WEEK);
        }
    }

    function setAmbRewards(bytes32 poolIdx, uint32 weekFrom, uint32 weekTo, uint64 weeklyReward) public payable {
        // require(msg.sender == governance_, "Only callable by governance");
        require(weekFrom % WEEK == 0 && weekTo % WEEK == 0, "Invalid weeks");
        while (weekFrom <= weekTo) {
            ambRewardPerWeek_[poolIdx][weekFrom] = weeklyReward;
            weekFrom += uint32(WEEK);
        }
    }

Recommended Mitigation Step

add a check to ensure against setting weekly rewards for too many weeks ahead.

NC-03 Missing Events for certain Critical functions.

Summary

certain critical functions lack events which are very useful for contracts deployed on the EVM Note This is not the same as the issue in the bot reports.

Vulnerability Details

you can see the Instances of critical functions that do not emit events. when claiming rewards or when accruals are being made.

• claimConcentratedRewards()
• claimAmbientRewards()
• accrueAmbientGlobalTimeWeightedLiquidity()
• accrueAmbientPositionTimeWeightedLiquidity()
• accrueConcentratedPositionTimeWeightedLiquidity()
• accrueConcentratedGlobalTimeWeightedLiquidity()

Recommended Mitigation Step

emit events for the given functions.

Analysis - Canto Liquidity Mining Protocol Contest

Description

Canto Liquidity Mining is a novel feature introduced to the canto chain, mainly for incentivizing liquidity on Ambient Finance, it makes use of an Incentive mechanism and a nice liquidity mining scheme that incentivizes LPs to provide liquidity to the aimed protocol.

Approach taken in evaluating the codebase

During This audit, I focused on getting a full understanding of the mechanisms in the protocol, the functions to be called by a user and the ones to be called by the governance, I focused on invariants that could be broken and security guidelines followed by the protocol.

Architecture recommendations

The Overall Architecture of the protocol is well designed and user flows are properly implemented as the interactions between the contracts and the mechanisms implemented all align with Canto's aimed goals.

Codebase quality analysis

The Codebase was really short and all the more easier to understand, I would mark is as Good all though there wasns't a full coverage of test on the codebase and Natspecs for certain functions were missing.

Centralization risks

Due to Canto's Governance I think the issue of centralization was well mitigated in the codebase

Mechanism review

The audit cover a total of 2 contracts.

1. LiquidityMiningPath.sol
2. LiquidityMining.sol

Mechanism where to set the rewards which is to be done by the Governance and claiming of rewards. i believe the mechanisms in the codebase were properly implemented and user flows go as planned.

Systemic risks

The Systematic risk have already been admitted by the developers in the Ambient docs.

Time spent:

12 hours