WATCHPUG - Expired locks should not continue to earn rewards at the original high multiplier rate

[sherlock-admin]

WATCHPUG

high

Expired locks should not continue to earn rewards at the original high multiplier rate

Summary

Expired locks should be considered as same as the deposits with no lock.

Vulnerability Detail

The current implementation allows the deposits with expired locks to continue to enjoy the original high multiplier rate, while they can withdraw anytime they want.

The multiplier of shares amount is essentially a higher reward rate (APR) for longer period of locks.

For example:

If the regular APR is 2%; Locking for 4 years will boost the APR to 10%.

• Alice deposited 1M $MC tokens and got 10% APR;
• 4 years later, Alice's deposit's lock was expired.

Expected result:

The new APR for Alice's deposit is 2%;

Actual result:

Alice can continue to enjoy a 10% APR while she can withdraw anytime.

Impact

Users with expired locks will take more rewards than expected, which means fewer rewards for other users.

Code Snippet

https://github.com/Merit-Circle/merit-liquidity-mining/blob/ce5feaae19126079d309ac8dd9a81372648437f1/contracts/TimeLockPool.sol#L116-L135

Tool used

Manual Review

Recommendation

Curve's Gauge system introduced a method called kick() which allows the expired (zeroed) veCRV users to be kicked from the rewards.

See: https://github.com/curvefi/curve-dao-contracts/blob/master/contracts/gauges/LiquidityGaugeV5.vy#L430-L446

A similar method can be added to solve this issue:

function kick(uint256 _depositId, address _user) external {
    if (_depositId >= depositsOf[_user].length) {
        revert NonExistingDepositError();
    }
    Deposit memory userDeposit = depositsOf[_user][_depositId];
    if (block.timestamp < userDeposit.end) {
        revert TooSoonError();
    }

    // burn pool shares
    _burn(_user, userDeposit.shareAmount - userDeposit.amount);
}

[federava]

Agree on the recommendation, will implement kick function. Noticing that shares go back to a 1:1 ratio and that the function can be called by anyone is a good design choice.

[federava]

PR from this issue

[jack-the-pug]

Fix confirmed