Canto Liquidity Mining Protocol - tpiliposian's results

1. Code Optimization Suggestion

Description

In the LiquidityMiningPath.sol two functions setConcRewards and setAmbRewards share common requirements. While this doesn't represent a security issue, it's a potential area for code optimization and readability improvement.

Currently, both functions perform the same checks: they validate that weekFrom and weekTo are divisible by WEEK, and (maybe) they both have a governance check that is commented out. This repetition not only makes the code longer but also increases the chances of errors when maintaining the contract.

Code Snippet

https://github.com/code-423n4/2023-10-canto/blob/29c92a926453a49c8935025a4d3de449150fc2ff/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol#L65-L81

    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);
        }
    }

Remediation

To optimize the code and improve readability, consider creating a custom modifier, to centralize the governance and weekFrom % WEEK == 0 && weekTo % WEEK == 0 checks. Then, apply this modifier to both functions. This change will reduce code duplication and make it easier to manage access control requirements in the future.

2. Consolidation of Time-Weighted Liquidity Calculation

Description

The LiquidityMining.sol contract contains repeated code for calculating time-weighted liquidity in multiple functions. Specifically, the code responsible for tracking time-weighted liquidity over weeks is present in three separate functions with minor parameter variations. This redundancy can lead to maintenance challenges, increased risk of inconsistencies, and decreased code readability.

Specifically, the code responsible for tracking time-weighted liquidity over weeks is present in three separate functions: accrueConcentratedGlobalTimeWeightedLiquidity(), accrueConcentratedPositionTimeWeightedLiquidity(), accrueAmbientGlobalTimeWeightedLiquidity().

            uint32 time = lastAccrued;
            while (time < block.timestamp) {
                uint32 currWeek = uint32((time / WEEK) * WEEK);
                uint32 nextWeek = uint32(((time + WEEK) / WEEK) * WEEK);
                uint32 dt = uint32(
                    nextWeek < block.timestamp
                        ? nextWeek - time
                        : block.timestamp - time
                );
                timeWeightedWeeklyGlobalAmbLiquidity_[poolIdx][currWeek] += dt * liquidity;
                time += dt;
            }

Remediation

To optimize the contract, we recommend consolidating the time-weighted liquidity calculation code into a single internal function. This function can be used internally by the existing functions that require this logic, reducing code duplication and improving maintainability.