Votes are not reset after each voting epoch

Summary

Vulnerability Detail

Its expected that when a new voting epoch is started, all the previous votes be cleared from the contract storage, but this is not the case in Voter: https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L107C1-L115C6

    function startNewVotingPeriod() public onlyOwner {
        _currentVotingPeriodId++;
        VotingPeriod storage period = _startTimes[_currentVotingPeriodId];
        period.startTime = block.timestamp;
        period.endTime = block.timestamp + _periodDuration;
        emit VotingPeriodStarted();
    }

as we can see in above code, the new epoch is started, but _votes (which is an EnumerableMap.AddressToUintMap) is still the same value. _votes is only modified in one place which is in vote function: https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L204-L209

            if (_votes.contains(pool)) {
                _votes.set(pool, _votes.get(pool) + deltaAmount);
            } else {
                _votes.set(pool, deltaAmount);
            }

you can see that votes are added to old values regardless of epoch number. votes are always increasing and accumulating overtime.

Impact

Since votes are always accumulating and increasing, pools that had most accumulated rewards over all epochs will receive a higher portion of total rewards (lum tokens) from MasterChefV2, regardless of whether they have received any votes in the current epoch or not. Top pools are selected by fetching votes from _votes storage variable from index 0 to 100: https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/js/sync-farm.js#L48 https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L374-L376

    function getVotedPoolsAtIndex(
        uint256 index
    ) external view returns (address, uint256) {
        return _votes.at(index);
    }

these pools are sorted by their votes and then added to Voter: https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/Voter.sol#L260-L298 this pools will receive rewards from MasterChefV2 proportional to their weight divided by total weight of that epoch: https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MasterchefV2.sol#L525 https://github.com/sherlock-audit/2024-06-magicsea/blob/main/magicsea-staking/src/MasterchefV2.sol#L465

Code Snippet

Tool used

Manual Review

Recommendation

create a mapping from epoch number to a EnumerableMap.AddressToUintMap. mapping(uint=>EnumerableMap.AddressToUintMap) public epoch_votes; and then use this inside vote function (instead of _votes):

    function vote(
        uint256 tokenId,
        address[] calldata pools,
        uint256[] calldata deltaAmounts
    ) external {
        //...
        for (uint256 i = 0; i < pools.length; ++i) {
            address pool = pools[i];

            //pool must be a valid one
            if (address(validator) != address(0) && !validator.isValid(pool)) {
                revert Voter__PoolNotVotable();
            }

            //...
            if (epoch_votes[currentPeriodId].contains(pool)) {
                epoch_votes[currentPeriodId].set(pool, epoch_votes[currentPeriodId].get(pool) + deltaAmount);
            } else {
               epoch_votes[currentPeriodId].set(pool, deltaAmount);
            }

            //...
        }
        //...
    }

change all view functions accordingly.

sherlock-audit / 2024-06-magicsea-judging

Sticky Hickory Hare - Votes are not reset after each voting epoch #718