c4-bot-4 commented 9 months ago

Lines of code

https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/tokenomics/contracts/Tokenomics.sol#L1010-L1029 https://github.com/code-423n4/2023-12-autonolas/blob/2a095eb1f8359be349d23af67089795fb0be4ed1/tokenomics/contracts/Tokenomics.sol#L925-L942

Vulnerability details

Impact

The checkpoint() function in the Tokenomics contract is responsible for recording global data when a new epoch starts.

This function contains a potential issue when the checkpoint() function is not called exactly at the end of an epoch that finishes very close to the end of a tokenomics year. If the new epoch rolls over a new year but the numYears variable, as calculated in the previous call to checkpoint() on line 1010, does not account for this, the maxBond and effectiveBond values will be calculated without taking into account the new year. As pointed out in the inline comments, this calculation needs to happen when the epoch that rolls over a new year is started. This could lead to inaccurate calculations and potential imbalances in the tokenomics of the system.

       // Adjust max bond value if the next epoch is going to be the year change epoch
        // Note that this computation happens before the epoch that is triggered in the next epoch (the code above) when
        // the actual year changes
        numYears = (block.timestamp + curEpochLen - timeLaunch) / ONE_YEAR;
        // Account for the year change to adjust the max bond
        if (numYears > currentYear) {
            // Calculate the inflation remainder for the passing year
            // End of the year timestamp
            uint256 yearEndTime = timeLaunch + numYears * ONE_YEAR;
            // Calculate the inflation per epoch value until the end of the year
            inflationPerEpoch = (yearEndTime - block.timestamp) * curInflationPerSecond;
            // Recalculate the inflation per second based on the new inflation for the current year
            curInflationPerSecond = getInflationForYear(numYears) / ONE_YEAR;
            // Add the remainder of the inflation for the next epoch based on a new inflation per second ratio
            inflationPerEpoch += (block.timestamp + curEpochLen - yearEndTime) * curInflationPerSecond;
            // Calculate the max bond value
            curMaxBond = (inflationPerEpoch * nextEpochPoint.epochPoint.maxBondFraction) / 100;
            // Update state maxBond value
            maxBond = uint96(curMaxBond);
            // Reset the tokenomics parameters update flag
            tokenomicsParametersUpdated = 0;
        } else if (tokenomicsParametersUpdated > 0) {

The root cause of this issue is the assumption that the checkpoint() function will be called exactly at the end of each epoch. However, in practice, there could be delays in when this function is called, leading to the potential issue described above.

Proof of Concept

Consider the following scenario:

The current epoch is about to end close to the end of a year.
The checkpoint() function is not called exactly at the end of the epoch, but some time later.
The new epoch rolls over a new year, but this wasn't considered when the epoch started.
The maxBond and effectiveBond values were calculated without taking into account the period of the current epoch year.
Users rewards and/or emissions could be affected by this inaccurate calculation.

Tools Used

Manual review

Recommended Mitigation Steps

To mitigate this issue, it is recommended to implement a mechanism within the checkpoint() function that checks if the current epoch has rolled over a new year but the calculation in the previous call to checkpoint() wouldn't have flagged it as such. Then, the discrepancies in calculation can be taken into account. This would ensure that the maxBond and effectiveBond values are accurately calculated, even if the checkpoint() function is called late.