iamnmt - Incorrect order when setting top pool's weight will cause the LUM rewards to mint more than expected

[sherlock-admin3]

iamnmt

Medium

Incorrect order when setting top pool's weight will cause the LUM rewards to mint more than expected

Summary

Incorrect order when setting top pool's weight will cause the LUM rewards to mint more than expected.

Vulnerability Detail

From the code comments for Voter#setTopPoolIdsWithWeights

https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/Voter.sol#L250-L259

    /**
     * @dev Set farm pools with their weight;
     *
     * WARNING:
     * Caller is responsible to updateAll oldPids on masterChef before using this function
     * and also call updateAll for the new pids after.
     *
     * @param pids - list of pids
     * @param weights - list of weights
     */

the order when setting top pool's weight is

1. MasterChef#updateAll(oldPids)
2. Voter#setTopPoolIdsWithWeights(newPids)
3. MasterChef#updateAll(newPids)

This order is also used in js/sync-farm.js and confirmed by the sponsor in a private thread.

Before (3), the newPids's rewarder.lastUpdateTimestamp is less than block.timestamp. By calling (3) the LUM rewards will be minted for newPids from rewarder.lastUpdateTimestamp to current block.timestamp, which cause the LUM rewards to mint more than expected.

Impact

Setting top pool's weight will cause LUM rewards minting more than expected.

Code Snippet

https://github.com/sherlock-audit/2024-06-magicsea/blob/42e799446595c542eff9519353d3becc50cdba63/magicsea-staking/src/Voter.sol#L250-L259

Tool used

Manual Review

Recommendation

Document the correct order to set top pool's weight

1. MasterChef#updateAll(oldPids)
2. MasterChef#updateAll(newPids)
3. Voter#setTopPoolIdsWithWeights(newPids)

Duplicate of #107